From 40dcc8a7f1d1a95d0ee16d977efc4761379200d2 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 10 Jul 2023 17:23:27 -0700 Subject: [PATCH] Update integration tests for new FederationDomain phase behavior - Refactor testlib.CreateTestFederationDomain helper - Call testlib.WaitForTestFederationDomainStatus after each integration test creates an IDP and expects the FederationDomain to become ready - Create an IDP for some tests which want the FederationDomain to be ready but were previously not creating any IDP - Expect the new FederationDomain condition type "IdentityProvidersFound" in those tests where it is needed Co-authored-by: Joshua Casey --- test/integration/e2e_test.go | 22 ++++++- test/integration/supervisor_discovery_test.go | 61 +++++++++++++++---- test/integration/supervisor_login_test.go | 22 ++++--- test/integration/supervisor_secrets_test.go | 8 ++- test/integration/supervisor_warnings_test.go | 11 +++- test/testlib/client.go | 21 +------ 6 files changed, 99 insertions(+), 46 deletions(-) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 5e8c800b..09bf7e31 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -101,9 +101,11 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // Create the downstream FederationDomain and expect it to go into the success status condition. downstream := testlib.CreateTestFederationDomain(topSetupCtx, t, - issuerURL.String(), - certSecret.Name, - configv1alpha1.FederationDomainPhaseReady, + configv1alpha1.FederationDomainSpec{ + Issuer: issuerURL.String(), + TLS: &configv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, + }, + configv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created ) // Create a JWTAuthenticator that will validate the tokens from the downstream issuer. @@ -156,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -238,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -322,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -442,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -569,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -638,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -710,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -765,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -824,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -891,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -946,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1015,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1066,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" @@ -1117,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/test-sessions.yaml" diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index d591c139..c971584a 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/util/retry" "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/crypto/ptls" @@ -82,6 +83,12 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { t.Skip("no address defined") } + // Create any IDP so that any FederationDomain created later by this test will see that exactly one IDP exists. + 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) + // Test that there is no default discovery endpoint available when there are no FederationDomains. requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, fmt.Sprintf("%s://%s", scheme, addr)) @@ -124,16 +131,18 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { // When the same issuer is added twice, both issuers are marked as duplicates, and neither provider is serving. config6Duplicate1, _ := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer6, client) - config6Duplicate2 := testlib.CreateTestFederationDomain(ctx, t, issuer6, "", "") + config6Duplicate2 := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: issuer6}, v1alpha1.FederationDomainPhaseError) requireStatus(t, client, ns, config6Duplicate1.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{ "Ready": v1alpha1.ConditionFalse, "IssuerIsUnique": v1alpha1.ConditionFalse, + "IdentityProvidersFound": v1alpha1.ConditionTrue, "OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue, "IssuerURLValid": v1alpha1.ConditionTrue, }) requireStatus(t, client, ns, config6Duplicate2.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{ "Ready": v1alpha1.ConditionFalse, "IssuerIsUnique": v1alpha1.ConditionFalse, + "IdentityProvidersFound": v1alpha1.ConditionTrue, "OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue, "IssuerURLValid": v1alpha1.ConditionTrue, }) @@ -153,10 +162,11 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config7, client, ns, scheme, addr, caBundle, issuer7) // When we create a provider with an invalid issuer, the status is set to invalid. - badConfig := testlib.CreateTestFederationDomain(ctx, t, badIssuer, "", "") + badConfig := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: badIssuer}, v1alpha1.FederationDomainPhaseError) requireStatus(t, client, ns, badConfig.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{ "Ready": v1alpha1.ConditionFalse, "IssuerIsUnique": v1alpha1.ConditionTrue, + "IdentityProvidersFound": v1alpha1.ConditionTrue, "OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue, "IssuerURLValid": v1alpha1.ConditionFalse, }) @@ -176,6 +186,12 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() + // Create any IDP so that any FederationDomain created later by this test will see that exactly one IDP exists. + 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) + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, defaultTLSCertSecretName(env), pinnipedClient, kubeClient) scheme := "https" @@ -186,7 +202,11 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { certSecretName1 := "integration-test-cert-1" // Create an FederationDomain with a spec.tls.secretName. - federationDomain1 := testlib.CreateTestFederationDomain(ctx, t, issuer1, certSecretName1, "") + federationDomain1 := testlib.CreateTestFederationDomain(ctx, t, + v1alpha1.FederationDomainSpec{ + Issuer: issuer1, + TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: certSecretName1}, + }, v1alpha1.FederationDomainPhaseReady) requireFullySuccessfulStatus(t, pinnipedClient, federationDomain1.Namespace, federationDomain1.Name) // The spec.tls.secretName Secret does not exist, so the endpoints should fail with TLS errors. @@ -210,7 +230,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { return err })) - // The the endpoints should fail with TLS errors again. + // The endpoints should fail with TLS errors again. requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create a Secret at the updated name. @@ -226,7 +246,11 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { certSecretName2 := "integration-test-cert-2" // Create an FederationDomain with a spec.tls.secretName. - federationDomain2 := testlib.CreateTestFederationDomain(ctx, t, issuer2, certSecretName2, "") + federationDomain2 := testlib.CreateTestFederationDomain(ctx, t, + v1alpha1.FederationDomainSpec{ + Issuer: issuer2, + TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: certSecretName2}, + }, v1alpha1.FederationDomainPhaseReady) requireFullySuccessfulStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name) // Create the Secret. @@ -248,6 +272,12 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() + // Create any IDP so that any FederationDomain created later by this test will see that exactly one IDP exists. + 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) + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, defaultTLSCertSecretName(env), pinnipedClient, kubeClient) scheme := "https" @@ -270,7 +300,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { issuerUsingHostname := fmt.Sprintf("%s://%s/issuer1", scheme, address) // Create an FederationDomain without a spec.tls.secretName. - federationDomain1 := testlib.CreateTestFederationDomain(ctx, t, issuerUsingIPAddress, "", "") + federationDomain1 := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: issuerUsingIPAddress}, v1alpha1.FederationDomainPhaseReady) requireFullySuccessfulStatus(t, pinnipedClient, federationDomain1.Namespace, federationDomain1.Name) // There is no default TLS cert and the spec.tls.secretName was not set, so the endpoints should fail with TLS errors. @@ -284,7 +314,11 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { // Create an FederationDomain with a spec.tls.secretName. certSecretName := "integration-test-cert-1" - federationDomain2 := testlib.CreateTestFederationDomain(ctx, t, issuerUsingHostname, certSecretName, "") + federationDomain2 := testlib.CreateTestFederationDomain(ctx, t, + v1alpha1.FederationDomainSpec{ + Issuer: issuerUsingHostname, + TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: certSecretName}, + }, v1alpha1.FederationDomainPhaseReady) requireFullySuccessfulStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name) // Create the Secret. @@ -471,7 +505,7 @@ func requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear( client pinnipedclientset.Interface, ) (*v1alpha1.FederationDomain, *ExpectedJWKSResponseFormat) { t.Helper() - newFederationDomain := testlib.CreateTestFederationDomain(ctx, t, issuerName, "", "") + newFederationDomain := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: issuerName}, v1alpha1.FederationDomainPhaseReady) jwksResult := requireDiscoveryEndpointsAreWorking(t, supervisorScheme, supervisorAddress, supervisorCABundle, issuerName, nil) requireFullySuccessfulStatus(t, client, newFederationDomain.Namespace, newFederationDomain.Name) return newFederationDomain, jwksResult @@ -645,12 +679,13 @@ func requireFullySuccessfulStatus(t *testing.T, client pinnipedclientset.Interfa requireStatus(t, client, ns, name, v1alpha1.FederationDomainPhaseReady, map[string]v1alpha1.ConditionStatus{ "Ready": v1alpha1.ConditionTrue, "IssuerIsUnique": v1alpha1.ConditionTrue, + "IdentityProvidersFound": v1alpha1.ConditionTrue, "OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue, "IssuerURLValid": v1alpha1.ConditionTrue, }) } -func requireStatus(t *testing.T, client pinnipedclientset.Interface, ns, name string, phase v1alpha1.FederationDomainPhase, conditionTypeToStatus map[string]v1alpha1.ConditionStatus) { +func requireStatus(t *testing.T, client pinnipedclientset.Interface, ns, name string, wantPhase v1alpha1.FederationDomainPhase, wantConditionTypeToStatus map[string]v1alpha1.ConditionStatus) { t.Helper() testlib.RequireEventually(t, func(requireEventually *require.Assertions) { @@ -660,14 +695,16 @@ func requireStatus(t *testing.T, client pinnipedclientset.Interface, ns, name st federationDomain, err := client.ConfigV1alpha1().FederationDomains(ns).Get(ctx, name, metav1.GetOptions{}) requireEventually.NoError(err) - t.Logf("found FederationDomain %s/%s with phase %s", ns, name, federationDomain.Status.Phase) - requireEventually.Equalf(phase, federationDomain.Status.Phase, "unexpected phase (conditions = '%#v')", federationDomain.Status.Conditions) + actualPhase := federationDomain.Status.Phase + t.Logf("found FederationDomain %s/%s with phase %s, wanted phase %s", ns, name, actualPhase, wantPhase) + requireEventually.Equalf(wantPhase, actualPhase, "unexpected phase (conditions = '%#v')", federationDomain.Status.Conditions) actualConditionTypeToStatus := map[string]v1alpha1.ConditionStatus{} for _, c := range federationDomain.Status.Conditions { actualConditionTypeToStatus[c.Type] = c.Status } - requireEventually.Equal(conditionTypeToStatus, actualConditionTypeToStatus, "unexpected statuses for conditions by type") + t.Logf("found FederationDomain %s/%s with conditions %#v, wanted condtions %#v", ns, name, actualConditionTypeToStatus, wantConditionTypeToStatus) + requireEventually.Equal(wantConditionTypeToStatus, actualConditionTypeToStatus, "unexpected statuses for conditions by type") }, 5*time.Minute, 200*time.Millisecond) } diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 8b472810..3fe6c70d 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -2078,11 +2078,21 @@ func testSupervisorLogin( // Create the downstream FederationDomain and expect it to go into the success status condition. downstream := testlib.CreateTestFederationDomain(ctx, t, - issuerURL.String(), - certSecret.Name, - configv1alpha1.FederationDomainPhaseReady, // TODO: expect another phase because this is a legacy FederationDomain and there is no IDP yet, so it is not safe to try to do logins until the IDP exists and the controller has a chance to run again to set the default IDP + configv1alpha1.FederationDomainSpec{ + Issuer: issuerURL.String(), + TLS: &configv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, + }, + // This is a legacy FederationDomain (does not explicitly list any IDPs) and there is no IDP yet, + // so it should not be ready yet. + configv1alpha1.FederationDomainPhaseError, ) + // Create upstream IDP and wait for it to become ready. + 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) + // 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 // calling the token endpoint from this test until the JWKS data has been loaded into @@ -2101,12 +2111,6 @@ func testSupervisorLogin( requireEventually.Equal(http.StatusOK, rsp.StatusCode) }, 30*time.Second, 200*time.Millisecond) - // Create upstream IDP and wait for it to become ready. - 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) - // Start a callback server on localhost. localCallbackServer := startLocalCallbackServer(t) diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 51b1977c..66321f59 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -6,6 +6,7 @@ package integration import ( "context" "encoding/json" + "fmt" "testing" "time" @@ -28,7 +29,12 @@ func TestSupervisorSecrets_Parallel(t *testing.T) { defer cancel() // Create our FederationDomain under test. - federationDomain := testlib.CreateTestFederationDomain(ctx, t, "", "", "") + federationDomain := testlib.CreateTestFederationDomain(ctx, t, + configv1alpha1.FederationDomainSpec{ + Issuer: fmt.Sprintf("http://test-issuer-%s.pinniped.dev", testlib.RandHex(t, 8)), + }, + configv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created, but this test does not care + ) tests := []struct { name string diff --git a/test/integration/supervisor_warnings_test.go b/test/integration/supervisor_warnings_test.go index 6068ee8c..9f6df59f 100644 --- a/test/integration/supervisor_warnings_test.go +++ b/test/integration/supervisor_warnings_test.go @@ -83,9 +83,11 @@ func TestSupervisorWarnings_Browser(t *testing.T) { // Create the downstream FederationDomain and expect it to go into the success status condition. downstream := testlib.CreateTestFederationDomain(ctx, t, - issuerURL.String(), - certSecret.Name, - configv1alpha1.FederationDomainPhaseReady, + configv1alpha1.FederationDomainSpec{ + Issuer: issuerURL.String(), + TLS: &configv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, + }, + configv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created ) // Create a JWTAuthenticator that will validate the tokens from the downstream issuer. @@ -107,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/ldap-test-refresh-sessions.yaml" @@ -251,6 +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) // Use a specific session cache for this test. sessionCachePath := tempDir + "/ldap-test-refresh-sessions.yaml" @@ -390,6 +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) // 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 4ab526a5..9fba764b 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -267,16 +267,13 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp } } -// CreateTestFederationDomain creates and returns a test FederationDomain in +// CreateTestFederationDomain creates and returns a test FederationDomain in the // $PINNIPED_TEST_SUPERVISOR_NAMESPACE, which will be automatically deleted at the end of the // current test's lifetime. -// If the provided issuer is not the empty string, then it will be used for the -// FederationDomain.Spec.Issuer field. Else, a random issuer will be generated. func CreateTestFederationDomain( ctx context.Context, t *testing.T, - issuer string, - certSecretName string, + spec configv1alpha1.FederationDomainSpec, expectStatus configv1alpha1.FederationDomainPhase, ) *configv1alpha1.FederationDomain { t.Helper() @@ -285,17 +282,10 @@ func CreateTestFederationDomain( createContext, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() - if issuer == "" { - issuer = fmt.Sprintf("http://test-issuer-%s.pinniped.dev", RandHex(t, 8)) - } - federationDomainsClient := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) federationDomain, err := federationDomainsClient.Create(createContext, &configv1alpha1.FederationDomain{ ObjectMeta: testObjectMeta(t, "oidc-provider"), - Spec: configv1alpha1.FederationDomainSpec{ - Issuer: issuer, - TLS: &configv1alpha1.FederationDomainTLSSpec{SecretName: certSecretName}, - }, + Spec: spec, }, metav1.CreateOptions{}) require.NoError(t, err, "could not create test FederationDomain") t.Logf("created test FederationDomain %s/%s", federationDomain.Namespace, federationDomain.Name) @@ -313,11 +303,6 @@ func CreateTestFederationDomain( } }) - // If we're not expecting any particular status, just return the new FederationDomain immediately. - if expectStatus == "" { - return federationDomain - } - // Wait for the FederationDomain to enter the expected phase (or time out). WaitForTestFederationDomainStatus(ctx, t, federationDomain.Name, expectStatus)