Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resolve remaining GitHub todos #1976

Merged
merged 7 commits into from
Jun 3, 2024
Merged

Resolve remaining GitHub todos #1976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5b54ec
resolve a todo in supervisor_discovery_test.go
cfryanr May 30, 2024
eee7371
Clean up how lastTransitionTime and observedGeneration are checked in…
joshuatcasey May 31, 2024
e89daad
dump debugging info when a browser-based test fails
cfryanr May 31, 2024
6364ac9
change status condition type name to be "ClientCredentialsSecretValid"
cfryanr May 31, 2024
0ace5cf
update github proposal doc to reflect current status
cfryanr May 31, 2024
e3d8c71
sleep longer before generating any GitHib OTP codes
cfryanr May 31, 2024
58b4ecc
user sees error msg when GitHub login is denied due to allowed orgs
joshuatcasey May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,12 @@ const (

countExpectedConditions = 6

HostValid string = "HostValid"
TLSConfigurationValid string = "TLSConfigurationValid"
OrganizationsPolicyValid string = "OrganizationsPolicyValid"
// ClientCredentialsObtained is different from other status conditions because it only checks that the client credentials
// have been obtained. The controller has no way to verify whether they are valid.
ClientCredentialsObtained string = "ClientCredentialsObtained" //nolint:gosec // this is not a credential
GitHubConnectionValid string = "GitHubConnectionValid"
ClaimsValid string = "ClaimsValid"
HostValid string = "HostValid"
TLSConfigurationValid string = "TLSConfigurationValid"
OrganizationsPolicyValid string = "OrganizationsPolicyValid"
ClientCredentialsSecretValid string = "ClientCredentialsSecretValid" //nolint:gosec // this is not a credential
GitHubConnectionValid string = "GitHubConnectionValid"
ClaimsValid string = "ClaimsValid"

defaultHost = "github.com"
defaultApiBaseURL = "https://api.github.com"
Expand Down Expand Up @@ -166,7 +164,7 @@ func (c *gitHubWatcherController) validateClientSecret(secretName string) (*meta

buildFalseCondition := func(prefix string) (*metav1.Condition, string, string, error) {
return &metav1.Condition{
Type: ClientCredentialsObtained,
Type: ClientCredentialsSecretValid,
Status: metav1.ConditionFalse,
Reason: upstreamwatchers.ReasonNotFound,
Message: fmt.Sprintf("%s: secret from spec.client.SecretName (%q) must be found in namespace %q with type %q and keys %q and %q",
Expand Down Expand Up @@ -202,7 +200,7 @@ func (c *gitHubWatcherController) validateClientSecret(secretName string) (*meta
}

return &metav1.Condition{
Type: ClientCredentialsObtained,
Type: ClientCredentialsSecretValid,
Status: metav1.ConditionTrue,
Reason: upstreamwatchers.ReasonSuccess,
Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", secretName),
Expand Down
243 changes: 119 additions & 124 deletions internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const (
oidcValidatorCacheTTL = 15 * time.Minute

// Constants related to conditions.
typeClientCredentialsValid = "ClientCredentialsValid" //nolint:gosec // this is not a credential
typeClientCredentialsSecretValid = "ClientCredentialsSecretValid" //nolint:gosec // this is not a credential
typeAdditionalAuthorizeParametersValid = "AdditionalAuthorizeParametersValid"
typeOIDCDiscoverySucceeded = "OIDCDiscoverySucceeded"

Expand Down Expand Up @@ -260,15 +260,15 @@ func (c *oidcWatcherController) validateUpstream(ctx controllerlib.Context, upst
return nil
}

// validateSecret validates the .spec.client.secretName field and returns the appropriate ClientCredentialsValid condition.
// validateSecret validates the .spec.client.secretName field and returns the appropriate ClientCredentialsSecretValid condition.
func (c *oidcWatcherController) validateSecret(upstream *v1alpha1.OIDCIdentityProvider, result *upstreamoidc.ProviderConfig) *metav1.Condition {
secretName := upstream.Spec.Client.SecretName

// Fetch the Secret from informer cache.
secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName)
if err != nil {
return &metav1.Condition{
Type: typeClientCredentialsValid,
Type: typeClientCredentialsSecretValid,
Status: metav1.ConditionFalse,
Reason: upstreamwatchers.ReasonNotFound,
Message: err.Error(),
Expand All @@ -278,7 +278,7 @@ func (c *oidcWatcherController) validateSecret(upstream *v1alpha1.OIDCIdentityPr
// Validate the secret .type field.
if secret.Type != oidcClientSecretType {
return &metav1.Condition{
Type: typeClientCredentialsValid,
Type: typeClientCredentialsSecretValid,
Status: metav1.ConditionFalse,
Reason: upstreamwatchers.ReasonWrongType,
Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)", secretName, secret.Type, oidcClientSecretType),
Expand All @@ -290,7 +290,7 @@ func (c *oidcWatcherController) validateSecret(upstream *v1alpha1.OIDCIdentityPr
clientSecret := secret.Data[clientSecretDataKey]
if len(clientID) == 0 || len(clientSecret) == 0 {
return &metav1.Condition{
Type: typeClientCredentialsValid,
Type: typeClientCredentialsSecretValid,
Status: metav1.ConditionFalse,
Reason: upstreamwatchers.ReasonMissingKeys,
Message: fmt.Sprintf("referenced Secret %q is missing required keys %q", secretName, []string{clientIDDataKey, clientSecretDataKey}),
Expand All @@ -301,7 +301,7 @@ func (c *oidcWatcherController) validateSecret(upstream *v1alpha1.OIDCIdentityPr
result.Config.ClientID = string(clientID)
result.Config.ClientSecret = string(clientSecret)
return &metav1.Condition{
Type: typeClientCredentialsValid,
Type: typeClientCredentialsSecretValid,
Status: metav1.ConditionTrue,
Reason: upstreamwatchers.ReasonSuccess,
Message: "loaded client credentials",
Expand Down
Loading