diff --git a/test/integration/concierge_jwtauthenticator_status_test.go b/test/integration/concierge_jwtauthenticator_status_test.go index bc2d502b9..2a34d5816 100644 --- a/test/integration/concierge_jwtauthenticator_status_test.go +++ b/test/integration/concierge_jwtauthenticator_status_test.go @@ -192,7 +192,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(true), []metav1.Condition{ { @@ -242,7 +242,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(true), []metav1.Condition{ { @@ -286,7 +286,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(len(env.SupervisorUpstreamOIDC.CABundle) != 0), []metav1.Condition{ { diff --git a/test/integration/concierge_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index 985c3e19e..1444ac204 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/test/integration/concierge_webhookauthenticator_status_test.go @@ -165,7 +165,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { finalConditions []metav1.Condition }{ { - name: "basic test to see if the WebhookAuthenticator wakes up or not", + name: "happy path", spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { return &env.TestWebhook }, @@ -174,11 +174,6 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, { name: "valid spec with invalid CA in TLS config will result in a WebhookAuthenticator that is not ready", - maybeSkip: func(t *testing.T) { - if conciergeUsingHTTPSProxy { - t.Skip("Skipping test that requires HTTPS_PROXY to be unset") - } - }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { caBundleString := "invalid base64-encoded data" webhookSpec := env.TestWebhook.DeepCopy() @@ -188,7 +183,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( + finalConditions: replaceSomeConditions(t, allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { @@ -219,6 +214,9 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { name: "valid spec with valid CA in TLS config but does not match issuer server will result in a WebhookAuthenticator that is not ready", maybeSkip: func(t *testing.T) { if conciergeUsingHTTPSProxy { + // Skip this test when HTTPS_PROXY is in use, because WebhookConnectionValid will have status Success + // with a message saying that the dialing was skipped due to the proxy setting, so the expectations + // below are wrong for that case. t.Skip("Skipping test that requires HTTPS_PROXY to be unset") } }, @@ -230,7 +228,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( + finalConditions: replaceSomeConditions(t, allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { @@ -254,21 +252,20 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, { name: "invalid with unresponsive endpoint will result in a WebhookAuthenticator that is not ready", - maybeSkip: func(t *testing.T) { - if conciergeUsingHTTPSProxy { - t.Skip("Skipping test that requires HTTPS_PROXY to be unset") - } - }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { webhookSpec := env.TestWebhook.DeepCopy() webhookSpec.TLS = &authenticationv1alpha1.TLSSpec{ CertificateAuthorityData: caBundleSomePivotalCA, } - webhookSpec.Endpoint = "https://127.0.0.1:443/some-fake-endpoint" + // Note that requests to 127.0.0.1 will not be proxied even when HTTPS_PROXY is set on the Concierge, + // so it's okay to run this test in that case too. The Concierge will attempt the dial because it sees + // that the request would not be proxied anyway. 127.0.0.1:8781 will be seen by the Concierge as a port + // local to its own pod, which does not exist so the connection will fail. + webhookSpec.Endpoint = "https://127.0.0.1:8781/some-fake-endpoint" return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( + finalConditions: replaceSomeConditions(t, allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { @@ -285,7 +282,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { Type: "WebhookConnectionValid", Status: "False", Reason: "UnableToDialServer", - Message: "cannot dial server: dial tcp 127.0.0.1:443: connect: connection refused", + Message: "cannot dial server: dial tcp 127.0.0.1:8781: connect: connection refused", }, }, ), @@ -296,6 +293,10 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() + if tt.maybeSkip != nil { + tt.maybeSkip(t) + } + webhookAuthenticator := testlib.CreateTestWebhookAuthenticator( ctx, t, diff --git a/test/integration/supervisor_federationdomain_status_test.go b/test/integration/supervisor_federationdomain_status_test.go index 27877b616..712bb6577 100644 --- a/test/integration/supervisor_federationdomain_status_test.go +++ b/test/integration/supervisor_federationdomain_status_test.go @@ -46,8 +46,8 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { fd := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{ Issuer: "https://example.com/fake", }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( - allSuccessfulLegacyFederationDomainConditions("", fd.Spec), + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, + allSuccessfulLegacyFederationDomainConditions(t, "", fd.Spec), []metav1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "LegacyConfigurationIdentityProviderNotFound", @@ -67,7 +67,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady) testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, - allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider1.Name, fd.Spec)) + allSuccessfulLegacyFederationDomainConditions(t, oidcIdentityProvider1.Name, fd.Spec)) // Creating a second IDP should put the FederationDomain back into an error status again. oidcIdentityProvider2 := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ @@ -75,8 +75,8 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( - allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider2.Name, fd.Spec), + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, + allSuccessfulLegacyFederationDomainConditions(t, oidcIdentityProvider2.Name, fd.Spec), []metav1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProviderNotSpecified", @@ -121,7 +121,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, }, }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -145,7 +145,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, oidcIDP1Meta, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -173,7 +173,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { err := oidcIDPClient.Delete(ctx, oidcIdentityProvider1.Name, metav1.DeleteOptions{}) require.NoError(t, err) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -340,7 +340,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -483,7 +483,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }) require.NoError(t, err) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -950,22 +950,30 @@ func TestSupervisorFederationDomainCRDValidations_Parallel(t *testing.T) { } } -func replaceSomeConditions(conditions []metav1.Condition, replaceWithTheseConditions []metav1.Condition) []metav1.Condition { +func replaceSomeConditions(t *testing.T, conditions []metav1.Condition, replaceWithTheseConditions []metav1.Condition) []metav1.Condition { cp := make([]metav1.Condition, len(conditions)) copy(cp, conditions) for _, replacementCond := range replaceWithTheseConditions { + found := false for i, cond := range cp { if replacementCond.Type == cond.Type { cp[i] = replacementCond + found = true break } } + if !found { + require.Failf(t, + "test setup problem", + "replaceSomeConditions() helper was called to replace condition of type %q but no such condition was found in conditions slice", + replacementCond.Type) + } } return cp } -func allSuccessfulLegacyFederationDomainConditions(idpName string, federationDomainSpec supervisorconfigv1alpha1.FederationDomainSpec) []metav1.Condition { - return replaceSomeConditions( +func allSuccessfulLegacyFederationDomainConditions(t *testing.T, idpName string, federationDomainSpec supervisorconfigv1alpha1.FederationDomainSpec) []metav1.Condition { + return replaceSomeConditions(t, allSuccessfulFederationDomainConditions(federationDomainSpec), []metav1.Condition{ {