diff --git a/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl index 1a36a441b..c84f46dbd 100644 --- a/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 21ab3e8ff..6ae17a03d 100644 --- a/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 21ab3e8ff..6ae17a03d 100644 --- a/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 40a300718..b1d762384 100644 --- a/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -114,6 +114,7 @@ spec: - organizations type: object claims: + default: {} description: Claims allows customization of the username and groups claims. properties: diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 1a36a441b..c84f46dbd 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -211,7 +211,7 @@ type GitHubIdentityProviderSpec struct { // Claims allows customization of the username and groups claims. // - // +optional + // +kubebuilder:default={} Claims GitHubClaims `json:"claims,omitempty"` // AllowAuthentication allows customization of who can authenticate using this IDP and how. diff --git a/internal/controller/conditionsutil/conditions_util_test.go b/internal/controller/conditionsutil/conditions_util_test.go index 2049e9f2b..373d6adee 100644 --- a/internal/controller/conditionsutil/conditions_util_test.go +++ b/internal/controller/conditionsutil/conditions_util_test.go @@ -30,7 +30,34 @@ func TestMergeIDPConditions(t *testing.T) { wantConditions []metav1.Condition }{ { - name: "True -> False returns true", + name: "Adding a new condition with status=True returns false", + newConditions: []*metav1.Condition{ + { + Type: "NewType", + Status: metav1.ConditionTrue, + Reason: "new reason", + Message: "new message", + }, + }, + observedGeneration: int64(999), + conditionsToUpdate: &[]metav1.Condition{}, + wantLogSnippets: []string{ + `"message":"updated condition","type":"NewType","status":"True"`, + }, + wantConditions: []metav1.Condition{ + { + Type: "NewType", + Status: metav1.ConditionTrue, + ObservedGeneration: int64(999), + LastTransitionTime: testTime, + Reason: "new reason", + Message: "new message", + }, + }, + wantResult: false, + }, + { + name: "Updating a condition status from False to True returns true", newConditions: []*metav1.Condition{ { Type: "UnchangedType", @@ -144,7 +171,13 @@ func TestMergeIDPConditions(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - result := MergeConditions(tt.newConditions, tt.observedGeneration, tt.conditionsToUpdate, logger, testTime) + result := MergeConditions( + tt.newConditions, + tt.observedGeneration, + tt.conditionsToUpdate, + logger, + testTime, + ) logString := log.String() require.Equal(t, len(tt.wantLogSnippets), strings.Count(logString, "\n")) diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index 79f380512..e6897e323 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -48,6 +48,8 @@ const ( gitHubClientSecretType corev1.SecretType = "secrets.pinniped.dev/github-client" clientIDDataKey, clientSecretDataKey string = "clientID", "clientSecret" + countExpectedConditions = 6 + HostValid string = "HostValid" TLSConfigurationValid string = "TLSConfigurationValid" OrganizationsPolicyValid string = "OrganizationsPolicyValid" @@ -55,6 +57,7 @@ const ( // 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" ) // UpstreamGitHubIdentityProviderICache is a thread safe cache that holds a list of validated upstream GitHub IDP configurations. @@ -250,10 +253,8 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro // Should there be some sort of catch-all condition to capture this? // This does not actually prevent a GitHub IDP from being added to the cache. // CRD defaulting and validation should eliminate the possibility of an error here. - groupNameAttribute, usernameAttribute, userAndGroupErr := validateUserAndGroupAttributes(upstream) - if userAndGroupErr != nil { - applicationErrors = append(applicationErrors, userAndGroupErr) - } + userAndGroupCondition, groupNameAttribute, usernameAttribute := validateUserAndGroupAttributes(upstream) + conditions = append(conditions, userAndGroupCondition) organizationPolicyCondition, policy := validateOrganizationsPolicy(&upstream.Spec.AllowAuthentication.Organizations) conditions = append(conditions, organizationPolicyCondition) @@ -277,12 +278,12 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro // The critical pattern to maintain is that every run of the sync loop will populate the exact number of the exact // same set of conditions. Conditions depending on other conditions should get Status: metav1.ConditionUnknown, or // Status: metav1.ConditionFalse, never be omitted. - if len(conditions) != 5 { // untested since all code paths result in 5 conditions - applicationErrors = append(applicationErrors, fmt.Errorf("expected %d conditions but found %d conditions", 5, len(conditions))) + if len(conditions) != countExpectedConditions { // untested since all code paths return the same number of conditions + applicationErrors = append(applicationErrors, fmt.Errorf("expected %d conditions but found %d conditions", countExpectedConditions, len(conditions))) return nil, k8sutilerrors.NewAggregate(applicationErrors) } hadErrorCondition, updateStatusErr := c.updateStatus(ctx.Context, upstream, conditions) - if updateStatusErr != nil { // untested + if updateStatusErr != nil { applicationErrors = append(applicationErrors, updateStatusErr) } // Any error condition means we will not add the IDP to the cache, so just return nil here @@ -406,15 +407,28 @@ func buildDialErrorMessage(tlsDialErr error) string { return reason } -func validateUserAndGroupAttributes(upstream *v1alpha1.GitHubIdentityProvider) (v1alpha1.GitHubGroupNameAttribute, v1alpha1.GitHubUsernameAttribute, error) { - var groupNameAttribute v1alpha1.GitHubGroupNameAttribute - var usernameAttribute v1alpha1.GitHubUsernameAttribute +func validateUserAndGroupAttributes(upstream *v1alpha1.GitHubIdentityProvider) (*metav1.Condition, v1alpha1.GitHubGroupNameAttribute, v1alpha1.GitHubUsernameAttribute) { + buildInvalidCondition := func(message string) *metav1.Condition { + return &metav1.Condition{ + Type: ClaimsValid, + Status: metav1.ConditionFalse, + Reason: "Invalid", + Message: message, + } + } - if upstream.Spec.Claims.Groups == nil || upstream.Spec.Claims.Username == nil { - return "", "", fmt.Errorf("spec.claims.groups and spec.claims.username are required") + var usernameAttribute v1alpha1.GitHubUsernameAttribute + if upstream.Spec.Claims.Username == nil { + return buildInvalidCondition("spec.claims.username is required"), "", "" + } else { + usernameAttribute = *upstream.Spec.Claims.Username + } + + var groupNameAttribute v1alpha1.GitHubGroupNameAttribute + if upstream.Spec.Claims.Groups == nil { + return buildInvalidCondition("spec.claims.groups is required"), "", "" } else { groupNameAttribute = *upstream.Spec.Claims.Groups - usernameAttribute = *upstream.Spec.Claims.Username } switch usernameAttribute { @@ -423,7 +437,7 @@ func validateUserAndGroupAttributes(upstream *v1alpha1.GitHubIdentityProvider) ( case v1alpha1.GitHubUsernameID: default: // Should not happen due to CRD enum validation - return "", "", fmt.Errorf("invalid spec.claims.username (%q)", usernameAttribute) + return buildInvalidCondition(fmt.Sprintf("spec.claims.username (%q) is not valid", usernameAttribute)), "", "" } switch groupNameAttribute { @@ -431,10 +445,15 @@ func validateUserAndGroupAttributes(upstream *v1alpha1.GitHubIdentityProvider) ( case v1alpha1.GitHubUseTeamSlugForGroupName: default: // Should not happen due to CRD enum validation - return "", "", fmt.Errorf("invalid spec.claims.groups (%q)", groupNameAttribute) + return buildInvalidCondition(fmt.Sprintf("spec.claims.groups (%q) is not valid", groupNameAttribute)), "", "" } - return groupNameAttribute, usernameAttribute, nil + return &metav1.Condition{ + Type: ClaimsValid, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.claims are valid", + }, groupNameAttribute, usernameAttribute } func (c *gitHubWatcherController) updateStatus( @@ -457,7 +476,7 @@ func (c *gitHubWatcherController) updateStatus( updated.Status.Phase = v1alpha1.GitHubPhaseError } - if equality.Semantic.DeepEqual(upstream, updated) { // untested + if equality.Semantic.DeepEqual(upstream, updated) { return hadErrorCondition, nil } diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index e1ec90e9a..d8b72110a 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -30,7 +30,7 @@ import ( "k8s.io/utils/ptr" "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" - pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" + supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/certauthority" pinnipedcontroller "go.pinniped.dev/internal/controller" @@ -45,8 +45,6 @@ import ( "go.pinniped.dev/internal/upstreamgithub" ) -const countExpectedConditions = 5 - var ( githubIDPGVR = schema.GroupVersionResource{ Group: v1alpha1.SchemeGroupVersion.Group, @@ -58,7 +56,7 @@ var ( ) func TestController(t *testing.T) { - require.Equal(t, 5, countExpectedConditions) + require.Equal(t, 6, countExpectedConditions) goodServer, goodServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { }), tlsserver.RecordTLSHello) @@ -115,6 +113,8 @@ func TestController(t *testing.T) { Client: v1alpha1.GitHubClientSpec{ SecretName: goodSecret.Name, }, + // These claims are optional when using the actual Kubernetes CRD. + // However, they are required here because CRD defaulting/validation does not occur during testing. Claims: v1alpha1.GitHubClaims{ Username: ptr.To(v1alpha1.GitHubUsernameLogin), Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), @@ -265,6 +265,30 @@ func TestController(t *testing.T) { } } + buildClaimsValidatedTrue := func(t *testing.T) metav1.Condition { + t.Helper() + return metav1.Condition{ + Type: ClaimsValid, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.claims are valid", + } + } + + buildClaimsValidatedFalse := func(t *testing.T, message string) metav1.Condition { + t.Helper() + return metav1.Condition{ + Type: ClaimsValid, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "Invalid", + Message: message, + } + } + buildGitHubConnectionValidTrue := func(t *testing.T, host string) metav1.Condition { t.Helper() @@ -304,6 +328,14 @@ func TestController(t *testing.T) { } } + buildLogForUpdatingClaimsValidTrue := func(name string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"ClaimsValid","status":"True","reason":"Success","message":"spec.claims are valid"}`, name) + } + + buildLogForUpdatingClaimsValidFalse := func(name, message string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"ClaimsValid","status":"False","reason":"Invalid","message":"%s"}`, name, message) + } + buildLogForUpdatingClientCredentialsObtained := func(name, status, reason, message string) string { return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"ClientCredentialsObtained","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, message) } @@ -374,6 +406,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseReady, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -385,6 +418,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -421,6 +455,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseReady, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), @@ -432,6 +467,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -483,6 +519,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseReady, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, goodServerIPv6Domain), buildHostValidTrue(t, goodServerIPv6Domain), @@ -494,6 +531,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, goodServerIPv6Domain), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -578,6 +616,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, `secret "no-secret-with-this-name" not found`, @@ -607,6 +646,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseReady, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, "other-secret-name"), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -621,6 +661,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseReady, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -632,6 +673,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("invalid-idp-name", "False", "SecretNotFound", `secret \"no-secret-with-this-name\" not found: secret from spec.client.SecretName (\"no-secret-with-this-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("invalid-idp-name"), buildLogForUpdatingOrganizationPolicyValid("invalid-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("invalid-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("invalid-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -639,6 +681,7 @@ func TestController(t *testing.T) { buildLogForUpdatingPhase("invalid-idp-name", "Error"), buildLogForUpdatingClientCredentialsObtained("other-idp-name", "True", "Success", `clientID and clientSecret have been read from spec.client.SecretName (\"other-secret-name\")`), + buildLogForUpdatingClaimsValidTrue("other-idp-name"), buildLogForUpdatingOrganizationPolicyValid("other-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("other-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("other-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -646,6 +689,7 @@ func TestController(t *testing.T) { buildLogForUpdatingPhase("other-idp-name", "Ready"), buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -674,6 +718,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "", "must not be empty"), @@ -685,6 +730,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: must not be empty`, ""), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -713,6 +759,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "https://example.com", `invalid port "//example.com"`), @@ -724,6 +771,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: invalid port \"//example.com\"`, "https://example.com"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -752,6 +800,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "example.com/foo", `host "example.com/foo" is not a valid hostname or IP address`), @@ -763,6 +812,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com/foo\" is not a valid hostname or IP address`, "example.com/foo"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -791,6 +841,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "u:p@example.com", `invalid port "p@example.com"`), @@ -802,6 +853,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: invalid port \"p@example.com\"`, "u:p@example.com"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -830,6 +882,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "example.com?a=b", `host "example.com?a=b" is not a valid hostname or IP address`), @@ -841,6 +894,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com?a=b\" is not a valid hostname or IP address`, "example.com?a=b"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -869,6 +923,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidFalse(t, "example.com#a", `host "example.com#a" is not a valid hostname or IP address`), @@ -880,6 +935,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com#a\" is not a valid hostname or IP address`, "example.com#a"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -912,6 +968,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidUnknown(t), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -923,6 +980,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", "spec.githubAPI.tls.certificateAuthorityData is not valid: certificateAuthorityData is not valid PEM: data does not contain any valid RSA or ECDSA certificates"), @@ -952,6 +1010,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld:443")), buildHostValidTrue(t, "nowhere.bad-tld"), @@ -963,6 +1022,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "nowhere.bad-tld"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -970,6 +1030,47 @@ func TestController(t *testing.T) { buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, + { + name: "Connection error - ipv6 without brackets", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("0:0:0:0:0:0:0:1:9876") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("0:0:0:0:0:0:0:1:9876") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "0:0:0:0:0:0:0:1:9876", `host "0:0:0:0:0:0:0:1:9876" is not a valid hostname or IP address`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"0:0:0:0:0:0:0:1:9876\") is not valid: host \"%s\" is not a valid hostname or IP address`, "0:0:0:0:0:0:0:1:9876"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, { name: "Connection error - host not trusted by system certs", secrets: []runtime.Object{goodSecret}, @@ -992,6 +1093,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host)), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1003,6 +1105,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1036,6 +1139,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host)), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1047,6 +1151,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1075,6 +1180,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1086,6 +1192,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1114,6 +1221,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1125,6 +1233,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1153,6 +1262,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1164,6 +1274,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1192,6 +1303,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1203,6 +1315,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1220,7 +1333,6 @@ func TestController(t *testing.T) { return badIDP }(), }, - wantErr: "spec.claims.groups and spec.claims.username are required", wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ { ObjectMeta: validFilledOutIDP.ObjectMeta, @@ -1230,8 +1342,9 @@ func TestController(t *testing.T) { return *badSpec }(), Status: v1alpha1.GitHubIdentityProviderStatus{ - Phase: v1alpha1.GitHubPhaseReady, + Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedFalse(t, "spec.claims.username is required"), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1243,11 +1356,12 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidFalse("some-idp-name", "spec.claims.username is required"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), - buildLogForUpdatingPhase("some-idp-name", "Ready"), + buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, { @@ -1260,7 +1374,6 @@ func TestController(t *testing.T) { return badIDP }(), }, - wantErr: "invalid spec.claims.username", wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ { ObjectMeta: validFilledOutIDP.ObjectMeta, @@ -1270,8 +1383,9 @@ func TestController(t *testing.T) { return *badSpec }(), Status: v1alpha1.GitHubIdentityProviderStatus{ - Phase: v1alpha1.GitHubPhaseReady, + Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedFalse(t, `spec.claims.username ("a") is not valid`), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1283,11 +1397,12 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidFalse("some-idp-name", `spec.claims.username (\"a\") is not valid`), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), - buildLogForUpdatingPhase("some-idp-name", "Ready"), + buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, { @@ -1300,7 +1415,6 @@ func TestController(t *testing.T) { return badIDP }(), }, - wantErr: "spec.claims.groups and spec.claims.username are required", wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ { ObjectMeta: validFilledOutIDP.ObjectMeta, @@ -1310,8 +1424,9 @@ func TestController(t *testing.T) { return *badSpec }(), Status: v1alpha1.GitHubIdentityProviderStatus{ - Phase: v1alpha1.GitHubPhaseReady, + Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedFalse(t, "spec.claims.groups is required"), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1323,11 +1438,12 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidFalse("some-idp-name", "spec.claims.groups is required"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), - buildLogForUpdatingPhase("some-idp-name", "Ready"), + buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, { @@ -1336,22 +1452,22 @@ func TestController(t *testing.T) { githubIdentityProviders: []runtime.Object{ func() runtime.Object { badIDP := validFilledOutIDP.DeepCopy() - badIDP.Spec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("a") + badIDP.Spec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("b") return badIDP }(), }, - wantErr: "invalid spec.claims.groups", wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ { ObjectMeta: validFilledOutIDP.ObjectMeta, Spec: func() v1alpha1.GitHubIdentityProviderSpec { badSpec := validFilledOutIDP.Spec.DeepCopy() - badSpec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("a") + badSpec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("b") return *badSpec }(), Status: v1alpha1.GitHubIdentityProviderStatus{ - Phase: v1alpha1.GitHubPhaseReady, + Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedFalse(t, `spec.claims.groups ("b") is not valid`), buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), @@ -1363,11 +1479,12 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingClaimsValidFalse("some-idp-name", `spec.claims.groups (\"b\") is not valid`), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), - buildLogForUpdatingPhase("some-idp-name", "Ready"), + buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, { @@ -1387,6 +1504,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, fmt.Sprintf("secret %q not found", validMinimalIDP.Spec.Client.SecretName), @@ -1404,6 +1522,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `secret \"some-secret-name\" not found: secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1428,6 +1547,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, `wrong secret type "other-type"`, @@ -1445,6 +1565,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `wrong secret type \"other-type\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1469,6 +1590,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, `missing key "clientID"`, @@ -1486,6 +1608,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `missing key \"clientID\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1510,6 +1633,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, `missing key "clientSecret"`, @@ -1527,6 +1651,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `missing key \"clientSecret\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1551,6 +1676,7 @@ func TestController(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + buildClaimsValidatedTrue(t), buildClientCredentialsObtainedFalse( t, "extra keys found", @@ -1568,6 +1694,7 @@ func TestController(t *testing.T) { }, wantLogs: []string{ buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `extra keys found: secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingClaimsValidTrue("minimal-idp-name"), buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), @@ -1582,9 +1709,8 @@ func TestController(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) - fakePinnipedClientForInformers := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) - pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClientForInformers, 0) + fakeSupervisorClient := supervisorfake.NewSimpleClientset(tt.githubIdentityProviders...) + supervisorInformers := pinnipedinformers.NewSharedInformerFactory(fakeSupervisorClient, 0) fakeKubeClient := kubernetesfake.NewSimpleClientset(tt.secrets...) kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(fakeKubeClient, 0) @@ -1597,12 +1723,12 @@ func TestController(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - gitHubIdentityProviderInformer := pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders() + gitHubIdentityProviderInformer := supervisorInformers.IDP().V1alpha1().GitHubIdentityProviders() controller := New( namespace, cache, - fakePinnipedClient, + fakeSupervisorClient, gitHubIdentityProviderInformer, kubeInformers.Core().V1().Secrets(), logger, @@ -1613,7 +1739,7 @@ func TestController(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - pinnipedInformers.Start(ctx.Done()) + supervisorInformers.Start(ctx.Done()) kubeInformers.Start(ctx.Done()) controllerlib.TestRunSynchronously(t, controller) @@ -1663,11 +1789,13 @@ func TestController(t *testing.T) { } // Verify the status conditions as reported in Kubernetes - allGitHubIDPs, err := fakePinnipedClient.IDPV1alpha1().GitHubIdentityProviders(namespace).List(ctx, metav1.ListOptions{}) + allGitHubIDPs, err := fakeSupervisorClient.IDPV1alpha1().GitHubIdentityProviders(namespace).List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, len(tt.wantResultingUpstreams), len(allGitHubIDPs.Items)) for i := 0; i < len(tt.wantResultingUpstreams); i++ { + require.Len(t, tt.wantResultingUpstreams[i].Status.Conditions, countExpectedConditions) + // Do not expect any particular order in the K8s objects var actualIDP *v1alpha1.GitHubIdentityProvider for _, possibleMatch := range allGitHubIDPs.Items { @@ -1678,7 +1806,7 @@ func TestController(t *testing.T) { } require.NotNil(t, actualIDP, "must find IDP with name %s", tt.wantResultingUpstreams[i].Name) - require.Equal(t, countExpectedConditions, len(actualIDP.Status.Conditions)) + require.Len(t, actualIDP.Status.Conditions, countExpectedConditions) // Update all expected conditions to the frozenTime. // TODO: Push this out to the test table @@ -1698,18 +1826,21 @@ func TestController(t *testing.T) { require.Equal(t, expectedLogs, log.String()) // This needs to happen after the expected condition LastTransitionTime has been updated. - wantActions := make([]coretesting.Action, 1+len(tt.wantResultingUpstreams)) + wantActions := make([]coretesting.Action, 3+len(tt.wantResultingUpstreams)) + wantActions[0] = coretesting.NewListAction(githubIDPGVR, githubIDPKind, "", metav1.ListOptions{}) + wantActions[1] = coretesting.NewWatchAction(githubIDPGVR, "", metav1.ListOptions{}) for i, want := range tt.wantResultingUpstreams { - wantActions[i] = coretesting.NewUpdateSubresourceAction(githubIDPGVR, "status", want.Namespace, ptr.To(want)) + wantActions[2+i] = coretesting.NewUpdateSubresourceAction(githubIDPGVR, "status", want.Namespace, ptr.To(want)) } - wantActions[len(tt.wantResultingUpstreams)] = coretesting.NewListAction(githubIDPGVR, githubIDPKind, namespace, metav1.ListOptions{}) - require.Equal(t, wantActions, fakePinnipedClient.Actions()) + // This List action is coming from the test code when it pulls the GitHubIdentityProviders from K8s + wantActions[len(wantActions)-1] = coretesting.NewListAction(githubIDPGVR, githubIDPKind, namespace, metav1.ListOptions{}) + require.Equal(t, wantActions, fakeSupervisorClient.Actions()) }) } } -func TestController_WithExistingConditions(t *testing.T) { - require.Equal(t, 5, countExpectedConditions) +func TestController_OnlyWantActions(t *testing.T) { + require.Equal(t, 6, countExpectedConditions) goodServer, goodServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { }), tlsserver.RecordTLSHello) @@ -1722,6 +1853,49 @@ func TestController_WithExistingConditions(t *testing.T) { nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + goodSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-secret-name", + Namespace: namespace, + }, + Type: "secrets.pinniped.dev/github-client", + Data: map[string][]byte{ + "clientID": []byte("some-client-id"), + "clientSecret": []byte("some-client-secret"), + }, + } + + validMinimalIDP := &v1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "minimal-idp-name", + Namespace: namespace, + UID: types.UID("minimal-uid"), + Generation: 1234, + }, + Spec: v1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: v1alpha1.GitHubAPIConfig{ + Host: ptr.To(goodServerDomain), + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerCAB64, + }, + }, + // These claims are optional when using the actual Kubernetes CRD. + // However, they are required here because CRD defaulting/validation does not occur during testing. + Claims: v1alpha1.GitHubClaims{ + Username: ptr.To(v1alpha1.GitHubUsernameLogin), + Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), + }, + Client: v1alpha1.GitHubClientSpec{ + SecretName: goodSecret.Name, + }, + AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: v1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + }, + } + alreadyInvalidExistingIDP := &v1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ Name: "already-existing-invalid-idp-name", @@ -1742,8 +1916,7 @@ func TestController_WithExistingConditions(t *testing.T) { }, }, Claims: v1alpha1.GitHubClaims{ - Username: ptr.To(v1alpha1.GitHubUsernameLogin), - Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), + Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), }, Client: v1alpha1.GitHubClientSpec{ SecretName: "unknown-secret", @@ -1752,6 +1925,14 @@ func TestController_WithExistingConditions(t *testing.T) { Status: v1alpha1.GitHubIdentityProviderStatus{ Phase: v1alpha1.GitHubPhaseError, Conditions: []metav1.Condition{ + { + Type: ClaimsValid, + Status: metav1.ConditionFalse, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: "Invalid", + Message: "spec.claims.username is required", + }, { Type: ClientCredentialsObtained, Status: metav1.ConditionFalse, @@ -1800,6 +1981,8 @@ func TestController_WithExistingConditions(t *testing.T) { name string secrets []runtime.Object githubIdentityProviders []runtime.Object + addSupervisorReactors func(*supervisorfake.Clientset) + wantErr string wantActions []coretesting.Action }{ { @@ -1838,6 +2021,76 @@ func TestController_WithExistingConditions(t *testing.T) { }(), }, }, + { + name: "K8s client error - cannot update githubidentityproviders", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantErr: "error from reactor - unable to update", + addSupervisorReactors: func(fake *supervisorfake.Clientset) { + fake.PrependReactor("update", "githubidentityproviders", func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("error from reactor - unable to update") + }) + }, + wantActions: []coretesting.Action{ + coretesting.NewUpdateSubresourceAction(githubIDPGVR, "status", namespace, func() runtime.Object { + idpWithConditions := validMinimalIDP.DeepCopy() + idpWithConditions.Status = v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + { + Type: ClaimsValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.claims are valid", + }, + { + Type: ClientCredentialsObtained, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: `clientID and clientSecret have been read from spec.client.SecretName ("some-secret-name")`, + }, + { + Type: GitHubConnectionValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", goodServerDomain), + }, + { + Type: HostValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", goodServerDomain), + }, + { + Type: OrganizationsPolicyValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, + }, + { + Type: TLSConfigurationValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 1234, + LastTransitionTime: metav1.NewTime(frozenClock.Now()), + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + }, + }, + } + return idpWithConditions + }()), + }, + }, } for _, tt := range tests { @@ -1845,8 +2098,12 @@ func TestController_WithExistingConditions(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) - pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...), 0) + fakeSupervisorClient := supervisorfake.NewSimpleClientset(tt.githubIdentityProviders...) + supervisorInformers := pinnipedinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(tt.githubIdentityProviders...), 0) + + if tt.addSupervisorReactors != nil { + tt.addSupervisorReactors(fakeSupervisorClient) + } kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(tt.secrets...), 0) @@ -1856,8 +2113,8 @@ func TestController_WithExistingConditions(t *testing.T) { controller := New( namespace, dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), - fakePinnipedClient, - pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), + fakeSupervisorClient, + supervisorInformers.IDP().V1alpha1().GitHubIdentityProviders(), kubeInformers.Core().V1().Secrets(), logger, controllerlib.WithInformer, @@ -1867,16 +2124,19 @@ func TestController_WithExistingConditions(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - pinnipedInformers.Start(ctx.Done()) + supervisorInformers.Start(ctx.Done()) kubeInformers.Start(ctx.Done()) controllerlib.TestRunSynchronously(t, controller) syncCtx := controllerlib.Context{Context: ctx, Key: controllerlib.Key{}} - err := controllerlib.TestSync(t, controller, syncCtx) - require.NoError(t, err) - - require.Equal(t, tt.wantActions, fakePinnipedClient.Actions()) + if err := controllerlib.TestSync(t, controller, syncCtx); len(tt.wantErr) > 0 { + require.ErrorContains(t, err, controllerlib.ErrSyntheticRequeue.Error()) + require.ErrorContains(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantActions, fakeSupervisorClient.Actions()) }) } } @@ -1963,8 +2223,8 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { _ = New( namespace, dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), - pinnipedfake.NewSimpleClientset(), - pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), + supervisorfake.NewSimpleClientset(), + pinnipedinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), secretInformer, logger, observableInformers.WithInformer, @@ -2026,13 +2286,13 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - gitHubIdentityProviderInformer := pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() + gitHubIdentityProviderInformer := pinnipedinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() observableInformers := testutil.NewObservableWithInformerOption() _ = New( namespace, dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), - pinnipedfake.NewSimpleClientset(), + supervisorfake.NewSimpleClientset(), gitHubIdentityProviderInformer, k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().Secrets(), logger, diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 2a7d3d739..1afe994cf 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -68,7 +68,6 @@ import ( "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/leaderelection" - "go.pinniped.dev/internal/net/phttp" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/pversion" "go.pinniped.dev/internal/secret" @@ -334,7 +333,6 @@ func prepareControllers( plog.New(), controllerlib.WithInformer, clock.RealClock{}, - phttp.Default, ), singletonWorker). WithController( diff --git a/test/integration/supervisor_github_idp_test.go b/test/integration/supervisor_github_idp_test.go index 608ae66c2..5192f5442 100644 --- a/test/integration/supervisor_github_idp_test.go +++ b/test/integration/supervisor_github_idp_test.go @@ -7,6 +7,8 @@ import ( "context" "encoding/base64" "fmt" + "os" + "path/filepath" "testing" "time" @@ -16,6 +18,7 @@ import ( "k8s.io/utils/ptr" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/test/testlib" @@ -254,6 +257,76 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { } } +func TestGitHubIDPSetsDefaultsWithKubectl_Parallel(t *testing.T) { + env := testlib.IntegrationEnv(t) + + adminClient := testlib.NewKubernetesClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + namespaceClient := adminClient.CoreV1().Namespaces() + + ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, namespaceClient.Delete(ctx, ns.Name, metav1.DeleteOptions{})) + }) + t.Logf("Created namespace %s", ns.Name) + + idpName := generateNamePrefix + testlib.RandHex(t, 16) + + githubIDPYaml := []byte(here.Doc(fmt.Sprintf(` + --- + apiVersion: idp.supervisor.%s/v1alpha1 + kind: GitHubIdentityProvider + metadata: + name: %s + namespace: %s + spec: + allowAuthentication: + organizations: + policy: AllGitHubUsers + client: + secretName: any-secret-name`, env.APIGroupSuffix, idpName, ns.Name))) + + githubIDPYamlFilepath := filepath.Join(t.TempDir(), "github-idp.yaml") + + require.NoError(t, os.WriteFile(githubIDPYamlFilepath, githubIDPYaml, 0600)) + + stdOut, stdErr := runTestKubectlCommand(t, "create", "-f", githubIDPYamlFilepath) + + require.Equal(t, fmt.Sprintf("githubidentityprovider.idp.supervisor.%s/%s created\n", env.APIGroupSuffix, idpName), stdOut) + require.Empty(t, stdErr) + + gitHubIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().GitHubIdentityProviders(ns.Name) + + idp, err := gitHubIDPClient.Get(ctx, idpName, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, idpv1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: idpv1alpha1.GitHubAPIConfig{ + Host: ptr.To("github.com"), + }, + Claims: idpv1alpha1.GitHubClaims{ + Username: ptr.To(idpv1alpha1.GitHubUsernameLoginAndID), + Groups: ptr.To(idpv1alpha1.GitHubUseTeamSlugForGroupName), + }, + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Client: idpv1alpha1.GitHubClientSpec{ + SecretName: "any-secret-name", + }, + }, idp.Spec) +} + func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { // These operations must be performed in the Supervisor's namespace so that the controller can find GitHubIdentityProvider supervisorNamespace := testlib.IntegrationEnv(t).SupervisorNamespace @@ -304,6 +377,12 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { }, wantPhase: idpv1alpha1.GitHubPhaseReady, wantConditions: []*metav1.Condition{ + { + Type: "ClaimsValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: "spec.claims are valid", + }, { Type: "ClientCredentialsObtained", Status: metav1.ConditionTrue, @@ -365,6 +444,12 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { }, wantPhase: idpv1alpha1.GitHubPhaseError, wantConditions: []*metav1.Condition{ + { + Type: "ClaimsValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: "spec.claims are valid", + }, { Type: "ClientCredentialsObtained", Status: metav1.ConditionFalse, @@ -566,6 +651,12 @@ func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { testlib.WaitForGitHubIDPPhase(ctx, t, gitHubIDPClient, created.Name, idpv1alpha1.GitHubPhaseError) testlib.WaitForGitHubIdentityProviderStatusConditions(ctx, t, gitHubIDPClient, created.Name, []*metav1.Condition{ + { + Type: "ClaimsValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: "spec.claims are valid", + }, { Type: "ClientCredentialsObtained", Status: metav1.ConditionFalse,