diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 2808c9913..ed61a436c 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -209,7 +209,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { } conditions := make([]*metav1.Condition, 0) - rootCAs, conditions, caBundlePEM, tlsOk := c.validateTLSBundle(obj.Spec.TLS, conditions) + certPool, caBundlePEM, conditions, tlsBundleOk := c.validateTLSBundle(obj.Spec.TLS, conditions) caBundlePEMSHA256 := sha256.Sum256(caBundlePEM) // note that this will always return the same hash for nil input // Only revalidate and update the cache if the cached authenticator is different from the desired authenticator. @@ -225,7 +225,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { jwtAuthenticatorFromCache = c.cacheValueAsJWTAuthenticator(valueFromCache) if jwtAuthenticatorFromCache != nil && reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &obj.Spec) && - tlsOk && // if there was any error while validating the CA bundle, then run remaining validations and update status + tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status jwtAuthenticatorFromCache.caBundlePEMSHA256 == caBundlePEMSHA256 { c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer). Info("actual jwt authenticator and desired jwt authenticator are the same") @@ -236,9 +236,9 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { var errs []error _, conditions, issuerOk := c.validateIssuer(obj.Spec.Issuer, conditions) - okSoFar := tlsOk && issuerOk + okSoFar := tlsBundleOk && issuerOk - client := phttp.Default(rootCAs) + client := phttp.Default(certPool) client.Timeout = 30 * time.Second // copied from Kube OIDC code coreOSCtx := coreosoidc.ClientContext(context.Background(), client) @@ -303,8 +303,8 @@ func (c *jwtCacheFillerController) cacheValueAsJWTAuthenticator(value authncache return jwtAuthenticator } -func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, []byte, bool) { - condition, pemBundle, rootCAs := tlsconfigutil.ValidateTLSConfig( +func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []byte, []*metav1.Condition, bool) { + condition, pemBundle, certPool := tlsconfigutil.ValidateTLSConfig( tlsconfigutil.TLSSpecForConcierge(tlsSpec), "spec.tls", c.namespace, @@ -312,7 +312,7 @@ func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1al c.configMapInformer) conditions = append(conditions, condition) - return rootCAs, conditions, pemBundle, condition.Status == metav1.ConditionTrue + return certPool, pemBundle, conditions, condition.Status == metav1.ConditionTrue } func (c *jwtCacheFillerController) validateIssuer(issuer string, conditions []*metav1.Condition) (*url.URL, []*metav1.Condition, bool) { diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index c4ca47609..ac6c615cc 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -6,6 +6,7 @@ package webhookcachefiller import ( "context" + "crypto/sha256" "crypto/tls" "crypto/x509" "fmt" @@ -42,11 +43,13 @@ import ( ) const ( - controllerName = "webhookcachefiller-controller" - typeReady = "Ready" - typeWebhookConnectionValid = "WebhookConnectionValid" - typeEndpointURLValid = "EndpointURLValid" - typeAuthenticatorValid = "AuthenticatorValid" + controllerName = "webhookcachefiller-controller" + + typeReady = "Ready" + typeWebhookConnectionValid = "WebhookConnectionValid" + typeEndpointURLValid = "EndpointURLValid" + typeAuthenticatorValid = "AuthenticatorValid" + reasonNotReady = "NotReady" reasonUnableToValidate = "UnableToValidate" reasonUnableToCreateClient = "UnableToCreateClient" @@ -54,12 +57,14 @@ const ( reasonInvalidEndpointURL = "InvalidEndpointURL" reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme" reasonUnableToDialServer = "UnableToDialServer" - msgUnableToValidate = "unable to validate; see other conditions for details" + + msgUnableToValidate = "unable to validate; see other conditions for details" ) type cachedWebhookAuthenticator struct { authenticator.Token - spec *authenticationv1alpha1.WebhookAuthenticatorSpec + spec *authenticationv1alpha1.WebhookAuthenticatorSpec + caBundlePEMSHA256 [32]byte } // New instantiates a new controllerlib.Controller which will populate the provided authncache.Cache. @@ -113,8 +118,8 @@ func New( } type webhookCacheFillerController struct { - cache *authncache.Cache namespace string + cache *authncache.Cache webhooks authinformers.WebhookAuthenticatorInformer secretInformer corev1informers.SecretInformer configMapInformer corev1informers.ConfigMapInformer @@ -141,6 +146,10 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { Name: ctx.Key.Name, } + conditions := make([]*metav1.Condition, 0) + certPool, caBundlePEM, conditions, tlsBundleOk := c.validateTLSBundle(obj.Spec.TLS, conditions) + caBundlePEMSHA256 := sha256.Sum256(caBundlePEM) // note that this will always return the same hash for nil input + // Only revalidate and update the cache if the cached authenticator is different from the desired authenticator. // There is no need to repeat validations for a spec that was already successfully validated. We are making a // design decision to avoid repeating the validation which dials the server, even though the server's TLS @@ -151,7 +160,10 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { // than to constantly monitor for external issues. if valueFromCache := c.cache.Get(cacheKey); valueFromCache != nil { webhookAuthenticatorFromCache := c.cacheValueAsWebhookAuthenticator(valueFromCache) - if webhookAuthenticatorFromCache != nil && reflect.DeepEqual(webhookAuthenticatorFromCache.spec, &obj.Spec) { + if webhookAuthenticatorFromCache != nil && + reflect.DeepEqual(webhookAuthenticatorFromCache.spec, &obj.Spec) && + tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status + webhookAuthenticatorFromCache.caBundlePEMSHA256 == caBundlePEMSHA256 { c.log.WithValues("webhookAuthenticator", klog.KObj(obj), "endpoint", obj.Spec.Endpoint). Info("actual webhook authenticator and desired webhook authenticator are the same") // Stop, no more work to be done. This authenticator is already validated and cached. @@ -159,10 +171,7 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { } } - conditions := make([]*metav1.Condition, 0) var errs []error - - certPool, pemBytes, conditions, tlsBundleOk := c.validateTLSBundle(obj.Spec.TLS, conditions) endpointHostPort, conditions, endpointOk := c.validateEndpoint(obj.Spec.Endpoint, conditions) okSoFar := tlsBundleOk && endpointOk @@ -174,7 +183,7 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { // Note that we use the whole URL when constructing the webhook client, // not just the host and port that we validated above. We need the path, etc. obj.Spec.Endpoint, - pemBytes, + caBundlePEM, conditions, okSoFar, ) @@ -187,8 +196,9 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { c.cache.Delete(cacheKey) } else { c.cache.Store(cacheKey, &cachedWebhookAuthenticator{ - Token: newWebhookAuthenticatorForCache, - spec: obj.Spec.DeepCopy(), // deep copy to avoid caching original object + Token: newWebhookAuthenticatorForCache, + spec: obj.Spec.DeepCopy(), // deep copy to avoid caching original object + caBundlePEMSHA256: caBundlePEMSHA256, }) c.log.WithValues("webhook", klog.KObj(obj), "endpoint", obj.Spec.Endpoint). Info("added new webhook authenticator") @@ -197,10 +207,10 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { err = c.updateStatus(ctx.Context, obj, conditions) errs = append(errs, err) - // sync loop errors: - // - should not be configuration errors. config errors a user must correct belong on the .Status + // Sync loop errors: + // - Should not be configuration errors. Config errors a user must correct belong on the .Status // object. The controller simply must wait for a user to correct before running again. - // - other errors, such as networking errors, etc. are the types of errors that should return here + // - Other errors, such as networking errors, etc. are the types of errors that should return here // and signal the controller to retry the sync loop. These may be corrected by machines. return utilerrors.NewAggregate(errs) } @@ -218,6 +228,18 @@ func (c *webhookCacheFillerController) cacheValueAsWebhookAuthenticator(value au return webhookAuthenticator } +func (c *webhookCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []byte, []*metav1.Condition, bool) { + condition, pemBundle, certPool := tlsconfigutil.ValidateTLSConfig( + tlsconfigutil.TLSSpecForConcierge(tlsSpec), + "spec.tls", + c.namespace, + c.secretInformer, + c.configMapInformer) + + conditions = append(conditions, condition) + return certPool, pemBundle, conditions, condition.Status == metav1.ConditionTrue +} + // newWebhookAuthenticator creates a webhook from the provided API server url and caBundle // used to validate TLS connections. func newWebhookAuthenticator( @@ -338,18 +360,6 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo return conditions, nil } -func (c *webhookCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []byte, []*metav1.Condition, bool) { - condition, pemBytes, rootCAs := tlsconfigutil.ValidateTLSConfig( - tlsconfigutil.TLSSpecForConcierge(tlsSpec), - "spec.tls", - c.namespace, - c.secretInformer, - c.configMapInformer) - - conditions = append(conditions, condition) - return rootCAs, pemBytes, conditions, condition.Status == metav1.ConditionTrue -} - func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditions []*metav1.Condition) (*endpointaddr.HostPort, []*metav1.Condition, bool) { endpointURL, err := url.Parse(endpoint) if err != nil { @@ -432,7 +442,6 @@ func (c *webhookCacheFillerController) updateStatus( if equality.Semantic.DeepEqual(original, updated) { return nil } - _, err := c.client.AuthenticationV1alpha1().WebhookAuthenticators().UpdateStatus(ctx, updated, metav1.UpdateOptions{}) return err } diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index b2aa7f482..f1dd871d4 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -6,6 +6,7 @@ package webhookcachefiller import ( "bytes" "context" + "crypto/sha256" "crypto/tls" "encoding/base64" "encoding/json" @@ -149,6 +150,45 @@ func TestController(t *testing.T) { Endpoint: goodWebhookDefaultServingCertEndpoint, TLS: hostGoodDefaultServingCertServerTLSSpec, } + goodWebhookAuthenticatorSpecWithCAFromSecret := authenticationv1alpha1.WebhookAuthenticatorSpec{ + Endpoint: goodWebhookDefaultServingCertEndpoint, + TLS: &authenticationv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &authenticationv1alpha1.CABundleSource{ + Kind: "Secret", + Name: "secret-with-ca", + Key: "ca.crt", + }, + }, + } + someSecretWithCA := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-with-ca", + Namespace: "concierge", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "ca.crt": hostGoodDefaultServingCertServerCAPEM, + }, + } + goodWebhookAuthenticatorSpecWithCAFromConfigMap := authenticationv1alpha1.WebhookAuthenticatorSpec{ + Endpoint: goodWebhookDefaultServingCertEndpoint, + TLS: &authenticationv1alpha1.TLSSpec{ + CertificateAuthorityDataSource: &authenticationv1alpha1.CABundleSource{ + Kind: "ConfigMap", + Name: "configmap-with-ca", + Key: "ca.crt", + }, + }, + } + someConfigMapWithCA := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "configmap-with-ca", + Namespace: "concierge", + }, + Data: map[string]string{ + "ca.crt": string(hostGoodDefaultServingCertServerCAPEM), + }, + } localWithExampleDotComWeebhookAuthenticatorSpec := authenticationv1alpha1.WebhookAuthenticatorSpec{ // CA for example.com, TLS serving cert for example.com, but endpoint is still localhost Endpoint: hostLocalWithExampleDotComCertServer.URL, @@ -365,10 +405,11 @@ func TestController(t *testing.T) { } tests := []struct { - name string - cache func(*testing.T, *authncache.Cache) - syncKey controllerlib.Key - webhooks []runtime.Object + name string + cache func(*testing.T, *authncache.Cache) + syncKey controllerlib.Key + webhooks []runtime.Object + secretsAndConfigMaps []runtime.Object // for modifying the clients to hack in arbitrary api responses configClient func(*conciergefake.Clientset) wantSyncLoopErr testutil.RequireErrorStringFunc @@ -431,7 +472,100 @@ func TestController(t *testing.T) { wantCacheEntries: 1, }, { - name: "Sync: valid and unchanged WebhookAuthenticator which was already cached: skips any updates to status or cache", + name: "Sync: valid WebhookAuthenticator with CA from Secret: loop will complete successfully and update status conditions", + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromSecret, + }, + }, + secretsAndConfigMaps: []runtime.Object{ + someSecretWithCA, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "added new webhook authenticator", + "endpoint": goodWebhookDefaultServingCertEndpoint, + "webhook": map[string]any{ + "name": "test-name", + }, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromSecret, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, + }, + { + name: "Sync: valid WebhookAuthenticator with CA from ConfigMap: loop will complete successfully and update status conditions", + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromConfigMap, + }, + }, + secretsAndConfigMaps: []runtime.Object{ + someConfigMapWithCA, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "added new webhook authenticator", + "endpoint": goodWebhookDefaultServingCertEndpoint, + "webhook": map[string]any{ + "name": "test-name", + }, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromConfigMap, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, + }, + { + name: "Sync: valid WebhookAuthenticator with external and changed CA bundle: loop will complete successfully and update status conditions", + syncKey: controllerlib.Key{Name: "test-name"}, cache: func(t *testing.T, cache *authncache.Cache) { cache.Store( authncache.Key{ @@ -439,7 +573,117 @@ func TestController(t *testing.T) { Kind: "WebhookAuthenticator", APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, }, - &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWithCA}, + newCacheValue(t, goodWebhookAuthenticatorSpecWithCAFromConfigMap, "some-stale-ca-bundle-pem-content-from-secret"), + ) + }, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromConfigMap, + }, + }, + secretsAndConfigMaps: []runtime.Object{ + someConfigMapWithCA, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "added new webhook authenticator", + "endpoint": goodWebhookDefaultServingCertEndpoint, + "webhook": map[string]any{ + "name": "test-name", + }, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCAFromConfigMap, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, + }, + { + name: "previously valid cached authenticator (which did not specify a CA bundle) changes and becomes invalid due to any problem with the CA bundle: loop will fail sync, will write failed and unknown status conditions, and will remove authenticator from cache", + syncKey: controllerlib.Key{Name: "test-name"}, + cache: func(t *testing.T, cache *authncache.Cache) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + // Force an invalid spec into the cache, which is not very realistic, but it simulates a case + // where the CA bundle goes from being cached as empty to being an error during validation, + // without causing any changes in the spec. This test wants to prove that the rest of the + // validations get run and the resource is update, just in case that can happen somehow. + newCacheValue(t, badWebhookAuthenticatorSpecInvalidTLS, ""), + ) + }, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: badWebhookAuthenticatorSpecInvalidTLS, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: badWebhookAuthenticatorSpecInvalidTLS, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + []metav1.Condition{ + sadTLSConfigurationValid(frozenMetav1Now, 0), + unknownWebhookConnectionValid(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 0, + }, + { + name: "Sync: valid and unchanged WebhookAuthenticator which was already cached: skips any updates to status or cache", + cache: func(t *testing.T, cache *authncache.Cache) { + oldCA, err := base64.StdEncoding.DecodeString(goodWebhookAuthenticatorSpecWithCA.TLS.CertificateAuthorityData) + require.NoError(t, err) + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + newCacheValue(t, goodWebhookAuthenticatorSpecWithCA, string(oldCA)), ) }, syncKey: controllerlib.Key{Name: "test-name"}, @@ -541,13 +785,15 @@ func TestController(t *testing.T) { { name: "Sync: changed WebhookAuthenticator: loop will update timestamps only on relevant statuses", cache: func(t *testing.T, cache *authncache.Cache) { + oldCA, err := base64.StdEncoding.DecodeString(goodWebhookAuthenticatorSpecWith404Endpoint.TLS.CertificateAuthorityData) + require.NoError(t, err) cache.Store( authncache.Key{ Name: "test-name", Kind: "WebhookAuthenticator", APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, }, - &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWith404Endpoint}, + newCacheValue(t, goodWebhookAuthenticatorSpecWith404Endpoint, string(oldCA)), ) }, syncKey: controllerlib.Key{Name: "test-name"}, @@ -790,13 +1036,15 @@ func TestController(t *testing.T) { { name: "previously valid cached authenticator's spec changes and becomes invalid (e.g. spec.issuer URL is invalid): loop will fail sync, will write failed and unknown status conditions, and will remove authenticator from cache", cache: func(t *testing.T, cache *authncache.Cache) { + oldCA, err := base64.StdEncoding.DecodeString(goodWebhookAuthenticatorSpecWithCA.TLS.CertificateAuthorityData) + require.NoError(t, err) cache.Store( authncache.Key{ Name: "test-name", Kind: "WebhookAuthenticator", APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, }, - &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWithCA}, + newCacheValue(t, goodWebhookAuthenticatorSpecWithCA, string(oldCA)), ) }, syncKey: controllerlib.Key{Name: "test-name"}, @@ -1492,8 +1740,8 @@ func TestController(t *testing.T) { if tt.configClient != nil { tt.configClient(pinnipedAPIClient) } - informers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) - kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(), 0) + pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) + kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0) cache := authncache.New() var log bytes.Buffer @@ -1507,7 +1755,7 @@ func TestController(t *testing.T) { "concierge", // namespace for controller cache, pinnipedAPIClient, - informers.Authentication().V1alpha1().WebhookAuthenticators(), + pinnipedInformers.Authentication().V1alpha1().WebhookAuthenticators(), kubeInformers.Core().V1().Secrets(), kubeInformers.Core().V1().ConfigMaps(), controllerlib.WithInformer, @@ -1517,7 +1765,7 @@ func TestController(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - informers.Start(ctx.Done()) + pinnipedInformers.Start(ctx.Done()) kubeInformers.Start(ctx.Done()) controllerlib.TestRunSynchronously(t, controller) @@ -1528,6 +1776,11 @@ func TestController(t *testing.T) { } else { require.NoError(t, err) } + + require.NotEmpty(t, tt.wantActions, "wantActions is required for test %s", tt.name) + require.Equal(t, tt.wantActions(), pinnipedAPIClient.Actions()) + require.Equal(t, tt.wantCacheEntries, len(cache.Keys()), fmt.Sprintf("expected cache entries is incorrect. wanted:%d, got: %d, keys: %v", tt.wantCacheEntries, len(cache.Keys()), cache.Keys())) + actualLogLines := testutil.SplitByNewline(log.String()) require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct") @@ -1536,6 +1789,7 @@ func TestController(t *testing.T) { var lineStruct map[string]any err := json.Unmarshal([]byte(logLine), &lineStruct) require.NoError(t, err) + require.Equal(t, tt.wantLogs[logLineNum]["level"], lineStruct["level"], fmt.Sprintf("log line (%d) log level should be correct (in: %s)", logLineNum, lineStruct)) require.Equal(t, tt.wantLogs[logLineNum]["timestamp"], lineStruct["timestamp"], fmt.Sprintf("log line (%d) timestamp should be correct (in: %s)", logLineNum, lineStruct)) @@ -1548,11 +1802,10 @@ func TestController(t *testing.T) { if lineStruct["endpoint"] != nil { require.Equal(t, tt.wantLogs[logLineNum]["endpoint"], lineStruct["endpoint"], fmt.Sprintf("log line (%d) endpoint should be correct", logLineNum)) } + if lineStruct["actualType"] != nil { + require.Equal(t, tt.wantLogs[logLineNum]["actualType"], lineStruct["actualType"], fmt.Sprintf("log line (%d) actualType should be correct", logLineNum)) + } } - - require.NotEmpty(t, tt.wantActions, "wantActions is required for test %s", tt.name) - require.Equal(t, tt.wantActions(), pinnipedAPIClient.Actions()) - require.Equal(t, tt.wantCacheEntries, len(cache.Keys()), fmt.Sprintf("expected cache entries is incorrect. wanted:%d, got: %d, keys: %v", tt.wantCacheEntries, len(cache.Keys()), cache.Keys())) }) } } @@ -1683,6 +1936,15 @@ func TestNewWebhookAuthenticator(t *testing.T) { } } +func newCacheValue(t *testing.T, spec authenticationv1alpha1.WebhookAuthenticatorSpec, caBundle string) authncache.Value { + t.Helper() + + return &cachedWebhookAuthenticator{ + spec: &spec, + caBundlePEMSHA256: sha256.Sum256([]byte(caBundle)), + } +} + func TestControllerFilterSecret(t *testing.T) { tests := []struct { name string