From 53413220716ef72842bd9f52eac2f78cce44e03d Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 20 Jul 2023 15:49:01 -0700 Subject: [PATCH] add integration test for FederationDomain status updates - Also fix small bug in controller where it used Sprintf wrong - Rename WaitForTestFederationDomainStatus test helper to WaitForFederationDomainStatusPhase --- .../federation_domain_watcher.go | 8 +- .../federation_domain_watcher_test.go | 2 +- test/integration/e2e_test.go | 28 +- ...supervisor_federationdomain_status_test.go | 268 ++++++++++++++++++ test/integration/supervisor_login_test.go | 2 +- test/integration/supervisor_warnings_test.go | 6 +- test/testlib/client.go | 71 ++++- 7 files changed, 349 insertions(+), 36 deletions(-) create mode 100644 test/integration/supervisor_federationdomain_status_test.go diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 6de4b784b..98f91fb72 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -35,6 +35,8 @@ import ( ) const ( + controllerName = "FederationDomainWatcherController" + typeReady = "Ready" typeIssuerURLValid = "IssuerURLValid" typeOneTLSSecretPerIssuerHostname = "OneTLSSecretPerIssuerHostname" @@ -109,7 +111,7 @@ func NewFederationDomainWatcherController( allowedKinds := sets.New(kindActiveDirectoryIdentityProvider, kindLDAPIdentityProvider, kindOIDCIdentityProvider) return controllerlib.New( controllerlib.Config{ - Name: "FederationDomainWatcherController", + Name: controllerName, Syncer: &federationDomainWatcherController{ federationDomainsSetter: federationDomainsSetter, apiGroup: fmt.Sprintf("idp.supervisor.%s", apiGroupSuffix), @@ -305,7 +307,7 @@ func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer( Status: configv1alpha1.ConditionFalse, Reason: reasonIdentityProviderNotSpecified, // vs LegacyConfigurationIdentityProviderNotFound as this is more specific Message: fmt.Sprintf("no resources were specified by .spec.identityProviders[].objectRef "+ - "and %q identity provider resources have been found: "+ + "and %d identity provider resources have been found: "+ "please update .spec.identityProviders to specify which identity providers "+ "this federation domain should use", idpCRsCount), }) @@ -850,7 +852,7 @@ func (c *federationDomainWatcherController) updateStatus( } _ = conditionsutil.MergeConfigConditions(conditions, - federationDomain.Generation, &updated.Status.Conditions, plog.New(), metav1.NewTime(c.clock.Now())) + federationDomain.Generation, &updated.Status.Conditions, plog.New().WithName(controllerName), metav1.NewTime(c.clock.Now())) if equality.Semantic.DeepEqual(federationDomain, updated) { return nil diff --git a/internal/controller/supervisorconfig/federation_domain_watcher_test.go b/internal/controller/supervisorconfig/federation_domain_watcher_test.go index 41287aeab..504e3596e 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher_test.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher_test.go @@ -361,7 +361,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { LastTransitionTime: time, Reason: "IdentityProviderNotSpecified", Message: fmt.Sprintf("no resources were specified by .spec.identityProviders[].objectRef "+ - "and %q identity provider resources have been found: "+ + "and %d identity provider resources have been found: "+ "please update .spec.identityProviders to specify which identity providers "+ "this federation domain should use", idpCRsCount), } diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 09bf7e315..dd3741784 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -158,7 +158,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -241,7 +241,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -326,7 +326,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -447,7 +447,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -575,7 +575,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -645,7 +645,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -718,7 +718,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -774,7 +774,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -834,7 +834,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -902,7 +902,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames createdProvider := setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -958,7 +958,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames createdProvider := setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1028,7 +1028,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1080,7 +1080,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames createdProvider := setupClusterForEndToEndActiveDirectoryTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1132,7 +1132,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(testCtx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" diff --git a/test/integration/supervisor_federationdomain_status_test.go b/test/integration/supervisor_federationdomain_status_test.go new file mode 100644 index 000000000..762b95048 --- /dev/null +++ b/test/integration/supervisor_federationdomain_status_test.go @@ -0,0 +1,268 @@ +// Copyright 2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" + + "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/test/testlib" +) + +// Never run this test in parallel since deleting all federation domains is disruptive, see main_test.go. +func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { + env := testlib.IntegrationEnv(t) + client := testlib.NewSupervisorClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, env.SupervisorNamespace, defaultTLSCertSecretName(env), client, testlib.NewKubernetesClientset(t)) + + tests := []struct { + name string + run func(t *testing.T) + }{ + { + name: "valid spec in without explicit identity providers makes status error unless there is exactly one identity provider", + run: func(t *testing.T) { + // Creating FederationDomain without any explicit IDPs should put the FederationDomain into an error status. + fd := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{ + Issuer: "https://example.com/fake", + }, v1alpha1.FederationDomainPhaseError) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + allSuccessfulLegacyFederationDomainConditions("", fd.Spec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "False", Reason: "LegacyConfigurationIdentityProviderNotFound", + Message: "no resources were specified by .spec.identityProviders[].objectRef and no identity provider resources have been found: please create an identity provider resource", + }, + { + Type: "Ready", Status: "False", Reason: "NotReady", + Message: "the FederationDomain is not ready: see other conditions for details", + }, + }, + )) + + // Creating an IDP should put the FederationDomain into a successful status. + oidcIdentityProvider1 := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: "https://example.cluster.local/fake-issuer-url-does-not-matter", + Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, + }, idpv1alpha1.PhaseError) + testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, v1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, + allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider1.Name, fd.Spec)) + + // Creating a second IDP should put the FederationDomain back into an error status again. + oidcIdentityProvider2 := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: "https://example.cluster.local/fake-issuer-url-does-not-matter", + Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, + }, idpv1alpha1.PhaseError) + testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, v1alpha1.FederationDomainPhaseError) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider2.Name, fd.Spec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProviderNotSpecified", + Message: "no resources were specified by .spec.identityProviders[].objectRef and 2 identity provider " + + "resources have been found: please update .spec.identityProviders to specify which identity providers " + + "this federation domain should use", + }, + { + Type: "Ready", Status: "False", Reason: "NotReady", + Message: "the FederationDomain is not ready: see other conditions for details", + }, + }, + )) + }, + }, + { + name: "valid spec with explicit identity providers makes status error until those identity providers all exist", + run: func(t *testing.T) { + oidcIDP1Meta := testlib.ObjectMetaWithRandomName(t, "upstream-oidc-idp") + oidcIDP2Meta := testlib.ObjectMetaWithRandomName(t, "upstream-oidc-idp") + // Creating FederationDomain with explicit IDPs that don't exist should put the FederationDomain into an error status. + fd := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{ + Issuer: "https://example.com/fake", + IdentityProviders: []v1alpha1.FederationDomainIdentityProvider{ + { + DisplayName: "idp1", + ObjectRef: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String("idp.supervisor." + env.APIGroupSuffix), + Kind: "OIDCIdentityProvider", + Name: oidcIDP1Meta.Name, + }, + Transforms: v1alpha1.FederationDomainTransforms{}, + }, + { + DisplayName: "idp2", + ObjectRef: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String("idp.supervisor." + env.APIGroupSuffix), + Kind: "OIDCIdentityProvider", + Name: oidcIDP2Meta.Name, + }, + Transforms: v1alpha1.FederationDomainTransforms{}, + }, + }, + }, v1alpha1.FederationDomainPhaseError) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + allSuccessfulFederationDomainConditions(fd.Spec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", + Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[0] with displayName "idp1", .spec.identityProviders[1] with displayName "idp2"`, + }, + { + Type: "Ready", Status: "False", Reason: "NotReady", + Message: "the FederationDomain is not ready: see other conditions for details", + }, + }, + )) + + // Creating the first IDP should not be enough to put the FederationDomain into a successful status. + oidcIdentityProvider1 := testlib.CreateTestOIDCIdentityProviderWithObjectMeta(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: "https://example.cluster.local/fake-issuer-url-does-not-matter", + Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, + }, oidcIDP1Meta, idpv1alpha1.PhaseError) + testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, v1alpha1.FederationDomainPhaseError) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + allSuccessfulFederationDomainConditions(fd.Spec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", + Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[1] with displayName "idp2"`, + }, + { + Type: "Ready", Status: "False", Reason: "NotReady", + Message: "the FederationDomain is not ready: see other conditions for details", + }, + }, + )) + + // Creating the second IDP should put the FederationDomain into a successful status. + testlib.CreateTestOIDCIdentityProviderWithObjectMeta(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: "https://example.cluster.local/fake-issuer-url-does-not-matter", + Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, + }, oidcIDP2Meta, idpv1alpha1.PhaseError) + testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, v1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, + allSuccessfulFederationDomainConditions(fd.Spec)) + + // Removing one IDP should put the FederationDomain back into an error status again. + oidcIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().OIDCIdentityProviders(env.SupervisorNamespace) + err := oidcIDPClient.Delete(ctx, oidcIdentityProvider1.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, v1alpha1.FederationDomainPhaseError) + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + allSuccessfulFederationDomainConditions(fd.Spec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", + Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[0] with displayName "idp1"`, + }, + { + Type: "Ready", Status: "False", Reason: "NotReady", + Message: "the FederationDomain is not ready: see other conditions for details", + }, + }, + )) + }, + }, + } + + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + tt.run(t) + }) + } +} + +func replaceSomeConditions(conditions []v1alpha1.Condition, replaceWithTheseConditions []v1alpha1.Condition) []v1alpha1.Condition { + cp := make([]v1alpha1.Condition, len(conditions)) + copy(cp, conditions) + for _, replacementCond := range replaceWithTheseConditions { + for i, cond := range cp { + if replacementCond.Type == cond.Type { + cp[i] = replacementCond + break + } + } + } + return cp +} + +func allSuccessfulLegacyFederationDomainConditions(idpName string, federationDomainSpec v1alpha1.FederationDomainSpec) []v1alpha1.Condition { + return replaceSomeConditions( + allSuccessfulFederationDomainConditions(federationDomainSpec), + []v1alpha1.Condition{ + { + Type: "IdentityProvidersFound", Status: "True", Reason: "LegacyConfigurationSuccess", + Message: fmt.Sprintf(`no resources were specified by .spec.identityProviders[].objectRef but exactly one `+ + `identity provider resource has been found: using "%s" as identity provider: `+ + `please explicitly list identity providers in .spec.identityProviders `+ + `(this legacy configuration mode may be removed in a future version of Pinniped)`, idpName), + }, + }, + ) +} + +func allSuccessfulFederationDomainConditions(federationDomainSpec v1alpha1.FederationDomainSpec) []v1alpha1.Condition { + return []v1alpha1.Condition{ + { + Type: "IdentityProvidersDisplayNamesUnique", Status: "True", Reason: "Success", + Message: "the names specified by .spec.identityProviders[].displayName are unique", + }, + { + Type: "IdentityProvidersFound", Status: "True", Reason: "Success", + Message: "the resources specified by .spec.identityProviders[].objectRef were found", + }, + { + Type: "IdentityProvidersObjectRefAPIGroupSuffixValid", Status: "True", Reason: "Success", + Message: "the API groups specified by .spec.identityProviders[].objectRef.apiGroup are recognized", + }, + { + Type: "IdentityProvidersObjectRefKindValid", Status: "True", Reason: "Success", + Message: "the kinds specified by .spec.identityProviders[].objectRef.kind are recognized", + }, + { + Type: "IssuerIsUnique", Status: "True", Reason: "Success", + Message: "spec.issuer is unique among all FederationDomains", + }, + { + Type: "IssuerURLValid", Status: "True", Reason: "Success", + Message: "spec.issuer is a valid URL", + }, + { + Type: "OneTLSSecretPerIssuerHostname", Status: "True", Reason: "Success", + Message: "all FederationDomains are using the same TLS secret when using the same hostname in the spec.issuer URL", + }, + { + Type: "Ready", Status: "True", Reason: "Success", + Message: fmt.Sprintf("the FederationDomain is ready and its endpoints are available: "+ + "the discovery endpoint is %s/.well-known/openid-configuration", federationDomainSpec.Issuer), + }, + { + Type: "TransformsConstantsNamesUnique", Status: "True", Reason: "Success", + Message: "the names specified by .spec.identityProviders[].transforms.constants[].name are unique", + }, + { + Type: "TransformsExamplesPassed", Status: "True", Reason: "Success", + Message: "the examples specified by .spec.identityProviders[].transforms.examples[] had no errors", + }, + { + Type: "TransformsExpressionsValid", Status: "True", Reason: "Success", + Message: "the expressions specified by .spec.identityProviders[].transforms.expressions[] are valid", + }, + } +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 3fe6c70db..dcaf45d9f 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -2091,7 +2091,7 @@ func testSupervisorLogin( idpName := createIDP(t) // Now that both the FederationDomain and the IDP are created, the FederationDomain should be ready. - testlib.WaitForTestFederationDomainStatus(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Ensure the the JWKS data is created and ready for the new FederationDomain by waiting for // the `/jwks.json` endpoint to succeed, because there is no point in proceeding and eventually diff --git a/test/integration/supervisor_warnings_test.go b/test/integration/supervisor_warnings_test.go index 9f6df59f2..3327bd4ad 100644 --- a/test/integration/supervisor_warnings_test.go +++ b/test/integration/supervisor_warnings_test.go @@ -109,7 +109,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue createdProvider := setupClusterForEndToEndLDAPTest(t, expectedUsername, env) - testlib.WaitForTestFederationDomainStatus(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/ldap-test-refresh-sessions.yaml" @@ -254,7 +254,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { sAMAccountName := expectedUsername + "@" + env.SupervisorUpstreamActiveDirectory.Domain setupClusterForEndToEndActiveDirectoryTest(t, sAMAccountName, env) - testlib.WaitForTestFederationDomainStatus(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/ldap-test-refresh-sessions.yaml" @@ -394,7 +394,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) { SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, }, }, idpv1alpha1.PhaseReady) - testlib.WaitForTestFederationDomainStatus(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) + testlib.WaitForFederationDomainStatusPhase(ctx, t, downstream.Name, configv1alpha1.FederationDomainPhaseReady) // Use a specific session cache for this test. sessionCachePath := tempDir + "/ldap-test-refresh-sessions.yaml" diff --git a/test/testlib/client.go b/test/testlib/client.go index 9fba764b2..c80c2e20d 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -304,31 +304,57 @@ func CreateTestFederationDomain( }) // Wait for the FederationDomain to enter the expected phase (or time out). - WaitForTestFederationDomainStatus(ctx, t, federationDomain.Name, expectStatus) + WaitForFederationDomainStatusPhase(ctx, t, federationDomain.Name, expectStatus) return federationDomain } -func WaitForTestFederationDomainStatus(ctx context.Context, t *testing.T, federationDomainName string, expectStatus configv1alpha1.FederationDomainPhase) { +func WaitForFederationDomainStatusPhase(ctx context.Context, t *testing.T, federationDomainName string, expectPhase configv1alpha1.FederationDomainPhase) { t.Helper() testEnv := IntegrationEnv(t) federationDomainsClient := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) - var result *configv1alpha1.FederationDomain RequireEventuallyf(t, func(requireEventually *require.Assertions) { - var err error - result, err = federationDomainsClient.Get(ctx, federationDomainName, metav1.GetOptions{}) + fd, err := federationDomainsClient.Get(ctx, federationDomainName, metav1.GetOptions{}) requireEventually.NoError(err) - requireEventually.Equal(expectStatus, result.Status.Phase) + requireEventually.Equalf(expectPhase, fd.Status.Phase, "actual status conditions were: %#v", fd.Status.Conditions) // If the FederationDomain was successfully created, ensure all secrets are present before continuing - if expectStatus == configv1alpha1.FederationDomainPhaseReady { - requireEventually.NotEmpty(result.Status.Secrets.JWKS.Name, "expected status.secrets.jwks.name not to be empty") - requireEventually.NotEmpty(result.Status.Secrets.TokenSigningKey.Name, "expected status.secrets.tokenSigningKey.name not to be empty") - requireEventually.NotEmpty(result.Status.Secrets.StateSigningKey.Name, "expected status.secrets.stateSigningKey.name not to be empty") - requireEventually.NotEmpty(result.Status.Secrets.StateEncryptionKey.Name, "expected status.secrets.stateEncryptionKey.name not to be empty") + if expectPhase == configv1alpha1.FederationDomainPhaseReady { + requireEventually.NotEmpty(fd.Status.Secrets.JWKS.Name, "expected status.secrets.jwks.name not to be empty") + requireEventually.NotEmpty(fd.Status.Secrets.TokenSigningKey.Name, "expected status.secrets.tokenSigningKey.name not to be empty") + requireEventually.NotEmpty(fd.Status.Secrets.StateSigningKey.Name, "expected status.secrets.stateSigningKey.name not to be empty") + requireEventually.NotEmpty(fd.Status.Secrets.StateEncryptionKey.Name, "expected status.secrets.stateEncryptionKey.name not to be empty") } - }, 60*time.Second, 1*time.Second, "expected the FederationDomain to have status %q", expectStatus) + }, 60*time.Second, 1*time.Second, "expected the FederationDomain to have status %q", expectPhase) +} + +func WaitForFederationDomainStatusConditions(ctx context.Context, t *testing.T, federationDomainName string, expectConditions []configv1alpha1.Condition) { + t.Helper() + testEnv := IntegrationEnv(t) + federationDomainsClient := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) + + RequireEventuallyf(t, func(requireEventually *require.Assertions) { + fd, err := federationDomainsClient.Get(ctx, federationDomainName, metav1.GetOptions{}) + requireEventually.NoError(err) + + requireEventually.Lenf(fd.Status.Conditions, len(expectConditions), + "wanted status conditions: %#v", expectConditions) + + for i, wantCond := range expectConditions { + actualCond := fd.Status.Conditions[i] + + // This is a cheat to avoid needing to make equality assertions on these fields. + requireEventually.NotZero(actualCond.LastTransitionTime) + wantCond.LastTransitionTime = actualCond.LastTransitionTime + requireEventually.NotZero(actualCond.ObservedGeneration) + wantCond.ObservedGeneration = actualCond.ObservedGeneration + + requireEventually.Equalf(wantCond, actualCond, + "wanted status conditions: %#v\nactual status conditions were: %#v\nnot equal at index %d", + expectConditions, fd.Status.Conditions, i) + } + }, 60*time.Second, 1*time.Second, "wanted FederationDomain conditions") } func RandBytes(t *testing.T, numBytes int) []byte { @@ -475,6 +501,11 @@ func createOIDCClientSecret(t *testing.T, forOIDCClient *configv1alpha1.OIDCClie } func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityProviderSpec, expectedPhase idpv1alpha1.OIDCIdentityProviderPhase) *idpv1alpha1.OIDCIdentityProvider { + t.Helper() + return CreateTestOIDCIdentityProviderWithObjectMeta(t, spec, testObjectMeta(t, "upstream-oidc-idp"), expectedPhase) +} + +func CreateTestOIDCIdentityProviderWithObjectMeta(t *testing.T, spec idpv1alpha1.OIDCIdentityProviderSpec, objectMeta metav1.ObjectMeta, expectedPhase idpv1alpha1.OIDCIdentityProviderPhase) *idpv1alpha1.OIDCIdentityProvider { t.Helper() env := IntegrationEnv(t) client := NewSupervisorClientset(t) @@ -485,7 +516,7 @@ func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityP // Create the OIDCIdentityProvider using GenerateName to get a random name. created, err := upstreams.Create(ctx, &idpv1alpha1.OIDCIdentityProvider{ - ObjectMeta: testObjectMeta(t, "upstream-oidc-idp"), + ObjectMeta: objectMeta, Spec: spec, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -494,7 +525,11 @@ func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityP t.Cleanup(func() { t.Logf("cleaning up test OIDCIdentityProvider %s/%s", created.Namespace, created.Name) err := upstreams.Delete(context.Background(), created.Name, metav1.DeleteOptions{}) - require.NoError(t, err) + notFound := k8serrors.IsNotFound(err) + // It's okay if it is not found, because it might have been deleted by another part of this test. + if !notFound { + require.NoErrorf(t, err, "could not cleanup test OIDCIdentityProvider %s/%s", created.Namespace, created.Name) + } }) t.Logf("created test OIDCIdentityProvider %s", created.Name) @@ -724,3 +759,11 @@ func testObjectMeta(t *testing.T, baseName string) metav1.ObjectMeta { Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, } } + +func ObjectMetaWithRandomName(t *testing.T, baseName string) metav1.ObjectMeta { + return metav1.ObjectMeta{ + Name: fmt.Sprintf("test-%s-%s", baseName, RandHex(t, 8)), + Labels: map[string]string{"pinniped.dev/test": ""}, + Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, + } +}