diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 5d11b83bb..297c0fbd1 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/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 diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 28cf8a590..bd5ea7c2b 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/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:$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:$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:$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:$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:$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:$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 { diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index 94e1c21dc..2361381cc 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/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 diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index a40287eb4..35c530240 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/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:$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:$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:$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:$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:$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:$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint), }, wantActions: func() []coretesting.Action { diff --git a/internal/controller/utils.go b/internal/controller/utils.go index a1600e705..e41b34dab 100644 --- a/internal/controller/utils.go +++ b/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 { diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 4c6c662e2..8ac9086e8 100644 --- a/test/integration/supervisor_login_test.go +++ b/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 }, diff --git a/test/integration/supervisor_upstream_test.go b/test/integration/supervisor_upstream_test.go index 876839f07..c20da7301 100644 --- a/test/integration/supervisor_upstream_test.go +++ b/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 }