small refactors

internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go

@@ -248,6 +248,7 @@ func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Co
 		// However, the status may be lagging behind due to the informer cache being slow to catch up
 		// after previous status updates, so always calculate the new status conditions again and check
 		// if they need to be updated.
+		logger.Info("cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations")
 		conditions = append(conditions,
 			successfulDiscoveryValidCondition(),
 			successfulJWKSURLValidCondition(),
@@ -320,7 +321,7 @@ func (c *jwtCacheFillerController) doExpensiveValidations(
 
 	newJWTAuthenticatorForCache, conditions, err := c.newCachedJWTAuthenticator(
 		client,
-		jwtAuthenticator.Spec.DeepCopy(), // deep copy to avoid caching original object
+		&jwtAuthenticator.Spec,
 		keySet,
 		caBundle.Hash(),
 		conditions,
@@ -349,7 +350,6 @@ func (c *jwtCacheFillerController) havePreviouslyValidated(
 	if authenticatorFromCache.issuer == issuer &&
 		tlsBundleOk && // if there was any error while validating the latest CA bundle, then do not consider it previously validated
 		authenticatorFromCache.caBundleHash.Equal(caBundleHash) {
-		logger.Info("cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations")
 		return true, true
 	}
 	return true, false // found the authenticator, but it had not been previously validated with these same settings

internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go

@@ -1247,7 +1247,7 @@ func TestController(t *testing.T) {
 				},
 			},
 			wantLogLines: []string{
-				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).havePreviouslyValidated","message":"cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations","jwtAuthenticator":"test-name","issuer":"%s"}`, someJWTAuthenticatorSpec.Issuer),
+				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).syncIndividualJWTAuthenticator","message":"cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations","jwtAuthenticator":"test-name","issuer":"%s"}`, someJWTAuthenticatorSpec.Issuer),
 				fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).updateStatus","message":"jwtauthenticator status successfully updated","jwtAuthenticator":"test-name","issuer":"%s","phase":"Ready"}`, someJWTAuthenticatorSpec.Issuer),
 			},
 			wantActions: func() []coretesting.Action {
@@ -1348,7 +1348,7 @@ func TestController(t *testing.T) {
 				},
 			},
 			wantLogLines: []string{
-				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).havePreviouslyValidated","message":"cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations","jwtAuthenticator":"test-name","issuer":"%s"}`, goodIssuer),
+				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).syncIndividualJWTAuthenticator","message":"cached jwt authenticator and desired jwt authenticator are the same: already cached, so skipping validations","jwtAuthenticator":"test-name","issuer":"%s"}`, goodIssuer),
 				fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"jwtcachefiller-controller","caller":"jwtcachefiller/jwtcachefiller.go:<line>$jwtcachefiller.(*jwtCacheFillerController).updateStatus","message":"choosing to not update the jwtauthenticator status since there is no update to make","jwtAuthenticator":"test-name","issuer":"%s","phase":"Ready"}`, goodIssuer),
 			},
 			wantActions: func() []coretesting.Action {

internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go

@@ -192,6 +192,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co
 		// However, the status may be lagging behind due to the informer cache being slow to catch up
 		// after previous status updates, so always calculate the new status conditions again and check
 		// if they need to be updated.
+		logger.Info("cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations")
 		conditions = append(conditions,
 			successfulWebhookConnectionValidCondition(),
 			successfulAuthenticatorValidCondition(),
@@ -290,7 +291,6 @@ func (c *webhookCacheFillerController) havePreviouslyValidated(
 	if authenticatorFromCache.endpoint == endpoint &&
 		tlsBundleOk && // if there was any error while validating the latest CA bundle, then do not consider it previously validated
 		authenticatorFromCache.caBundleHash.Equal(caBundleHash) {
-		logger.Info("cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations")
 		return true, true
 	}
 	return true, false // found the authenticator, but it had not been previously validated with these same settings

internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go

@@ -768,7 +768,7 @@ func TestController(t *testing.T) {
 				},
 			},
 			wantLogLines: []string{
-				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).havePreviouslyValidated","message":"cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations","webhookAuthenticator":"test-name","endpoint":"%s"}`, goodWebhookDefaultServingCertEndpoint),
+				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations","webhookAuthenticator":"test-name","endpoint":"%s"}`, goodWebhookDefaultServingCertEndpoint),
 				fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"choosing to not update the webhookauthenticator status since there is no update to make","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookDefaultServingCertEndpoint),
 			},
 			wantActions: func() []coretesting.Action {
@@ -987,7 +987,7 @@ func TestController(t *testing.T) {
 				},
 			},
 			wantLogLines: []string{
-				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).havePreviouslyValidated","message":"cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations","webhookAuthenticator":"test-name","endpoint":"%s"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint),
+				fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations","webhookAuthenticator":"test-name","endpoint":"%s"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint),
 				fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:<line>$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint),
 			},
 			wantActions: func() []coretesting.Action {

internal/controller/utils.go

@@ -46,18 +46,7 @@ func SimpleFilter(match func(metav1.Object) bool, parentFunc controllerlib.Paren
 }
 
 func MatchAnySecretOfTypeFilter(secretType corev1.SecretType, parentFunc controllerlib.ParentFunc, namespaces ...string) controllerlib.Filter {
-	isSecretOfType := func(obj metav1.Object) bool {
-		secret, ok := obj.(*corev1.Secret)
-		if !ok {
-			return false
-		}
-		// Only match on namespace if namespaces are provided
-		if len(namespaces) > 0 && !slices.Contains(namespaces, secret.Namespace) {
-			return false
-		}
-		return secret.Type == secretType
-	}
-	return SimpleFilter(isSecretOfType, parentFunc)
+	return MatchAnySecretOfTypesFilter([]corev1.SecretType{secretType}, parentFunc, namespaces...)
 }
 
 func MatchAnySecretOfTypesFilter(secretTypes []corev1.SecretType, parentFunc controllerlib.ParentFunc, namespaces ...string) controllerlib.Filter {

test/integration/supervisor_login_test.go

@@ -239,12 +239,13 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 
 		adIDP := testlib.CreateTestActiveDirectoryIdentityProvider(t, spec, idpv1alpha1.ActiveDirectoryPhaseReady)
 
-		expectedMsg := fmt.Sprintf(
+		expectedActiveDirectoryConnectionValidMessage := fmt.Sprintf(
 			`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 			spec.Host, env.SupervisorUpstreamActiveDirectory.BindUsername,
 			secret.Name, secret.ResourceVersion,
 		)
-		requireSuccessfulActiveDirectoryIdentityProviderConditions(t, adIDP, expectedMsg, env.SupervisorUpstreamActiveDirectory.CABundle != "")
+		requireSuccessfulActiveDirectoryIdentityProviderConditions(t,
+			adIDP, expectedActiveDirectoryConnectionValidMessage, env.SupervisorUpstreamActiveDirectory.CABundle != "")
 
 		return adIDP, secret
 	}
@@ -292,12 +293,13 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 
 		ldapIDP := testlib.CreateTestLDAPIdentityProvider(t, spec, idpv1alpha1.LDAPPhaseReady)
 
-		expectedMsg := fmt.Sprintf(
+		expectedLDAPConnectionValidMessage := fmt.Sprintf(
 			`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 			spec.Host, env.SupervisorUpstreamLDAP.BindUsername,
 			secret.Name, secret.ResourceVersion,
 		)
-		requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
+		requireSuccessfulLDAPIdentityProviderConditions(t,
+			ldapIDP, expectedLDAPConnectionValidMessage, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
 
 		return ldapIDP, secret
 	}
@@ -1124,7 +1126,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				updatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Update(ctx, secret, metav1.UpdateOptions{})
 				require.NoError(t, err)
 
-				expectedMsg := fmt.Sprintf(
+				expectedLDAPConnectionValidMessage := fmt.Sprintf(
 					`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 					env.SupervisorUpstreamLDAP.Host, env.SupervisorUpstreamLDAP.BindUsername,
 					updatedSecret.Name, updatedSecret.ResourceVersion,
@@ -1135,7 +1137,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					defer cancel()
 					idp, err = supervisorClient.IDPV1alpha1().LDAPIdentityProviders(env.SupervisorNamespace).Get(ctx, idp.Name, metav1.GetOptions{})
 					requireEventually.NoError(err)
-					requireEventuallySuccessfulLDAPIdentityProviderConditions(t, requireEventually, idp, expectedMsg, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
+					requireEventuallySuccessfulLDAPIdentityProviderConditions(t,
+						requireEventually, idp, expectedLDAPConnectionValidMessage, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
 				}, time.Minute, 500*time.Millisecond)
 				return idp.Name
 			},
@@ -1190,7 +1193,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					},
 				}, metav1.CreateOptions{})
 				require.NoError(t, err)
-				expectedMsg := fmt.Sprintf(
+				expectedLDAPConnectionValidMessage := fmt.Sprintf(
 					`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 					env.SupervisorUpstreamLDAP.Host, env.SupervisorUpstreamLDAP.BindUsername,
 					recreatedSecret.Name, recreatedSecret.ResourceVersion,
@@ -1201,7 +1204,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					defer cancel()
 					idp, err = supervisorClient.IDPV1alpha1().LDAPIdentityProviders(env.SupervisorNamespace).Get(ctx, idp.Name, metav1.GetOptions{})
 					requireEventually.NoError(err)
-					requireEventuallySuccessfulLDAPIdentityProviderConditions(t, requireEventually, idp, expectedMsg, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
+					requireEventuallySuccessfulLDAPIdentityProviderConditions(t,
+						requireEventually, idp, expectedLDAPConnectionValidMessage, len(env.SupervisorUpstreamLDAP.CABundle) != 0)
 				}, time.Minute, 500*time.Millisecond)
 				return idp.Name
 			},
@@ -1481,7 +1485,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				updatedSecret, err := client.CoreV1().Secrets(env.SupervisorNamespace).Update(ctx, secret, metav1.UpdateOptions{})
 				require.NoError(t, err)
 
-				expectedMsg := fmt.Sprintf(
+				expectedActiveDirectoryConnectionValidMessage := fmt.Sprintf(
 					`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 					env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername,
 					updatedSecret.Name, updatedSecret.ResourceVersion,
@@ -1492,7 +1496,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					defer cancel()
 					idp, err = supervisorClient.IDPV1alpha1().ActiveDirectoryIdentityProviders(env.SupervisorNamespace).Get(ctx, idp.Name, metav1.GetOptions{})
 					requireEventually.NoError(err)
-					requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t, requireEventually, idp, expectedMsg, len(env.SupervisorUpstreamActiveDirectory.CABundle) != 0)
+					requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t,
+						requireEventually, idp, expectedActiveDirectoryConnectionValidMessage, len(env.SupervisorUpstreamActiveDirectory.CABundle) != 0)
 				}, time.Minute, 500*time.Millisecond)
 				return idp.Name
 			},
@@ -1548,7 +1553,7 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 				}, metav1.CreateOptions{})
 				require.NoError(t, err)
 
-				expectedMsg := fmt.Sprintf(
+				expectedActiveDirectoryConnectionValidMessage := fmt.Sprintf(
 					`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
 					env.SupervisorUpstreamActiveDirectory.Host, env.SupervisorUpstreamActiveDirectory.BindUsername,
 					recreatedSecret.Name, recreatedSecret.ResourceVersion,
@@ -1559,7 +1564,8 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 					defer cancel()
 					idp, err = supervisorClient.IDPV1alpha1().ActiveDirectoryIdentityProviders(env.SupervisorNamespace).Get(ctx, idp.Name, metav1.GetOptions{})
 					requireEventually.NoError(err)
-					requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t, requireEventually, idp, expectedMsg, len(env.SupervisorUpstreamActiveDirectory.CABundle) != 0)
+					requireEventuallySuccessfulActiveDirectoryIdentityProviderConditions(t,
+						requireEventually, idp, expectedActiveDirectoryConnectionValidMessage, len(env.SupervisorUpstreamActiveDirectory.CABundle) != 0)
 				}, time.Minute, 500*time.Millisecond)
 				return idp.Name
 			},

test/integration/supervisor_upstream_test.go

@@ -140,19 +140,16 @@ func expectUpstreamConditions(t *testing.T, upstream *idpv1alpha1.OIDCIdentityPr
 }
 
 func expectedTLSConfigValidCondition(caBundleConfigured bool) metav1.Condition {
-	if caBundleConfigured {
-		return metav1.Condition{
-			Type:    "TLSConfigurationValid",
-			Status:  "True",
-			Reason:  "Success",
-			Message: `spec.tls is valid: using configured CA bundle`,
-		}
-	}
-
-	return metav1.Condition{
+	c := metav1.Condition{
 		Type:    "TLSConfigurationValid",
 		Status:  "True",
 		Reason:  "Success",
-		Message: `spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image`,
+		Message: "spec.tls is valid: no TLS configuration provided: using default root CA bundle from container image",
 	}
+
+	if caBundleConfigured {
+		c.Message = "spec.tls is valid: using configured CA bundle"
+	}
+
+	return c
 }