fix integration test for WebhookAuthenticator status conditions

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{
 					{

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,

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(
+				testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t,
-					allSuccessfulLegacyFederationDomainConditions("", fd.Spec),
+					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(
+				testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t,
-					allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider2.Name, fd.Spec),
+					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 {
+func allSuccessfulLegacyFederationDomainConditions(t *testing.T, idpName string, federationDomainSpec supervisorconfigv1alpha1.FederationDomainSpec) []metav1.Condition {
-	return replaceSomeConditions(
+	return replaceSomeConditions(t,
 		allSuccessfulFederationDomainConditions(federationDomainSpec),
 		[]metav1.Condition{
 			{