diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 56c84a7e4..ed1acf2bb 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -119,6 +119,9 @@ type cachedJWTAuthenticator struct { } func (c *cachedJWTAuthenticator) Close() { + if c == nil { + return + } c.cancel() } @@ -162,12 +165,12 @@ type jwtCacheFillerController struct { // Sync implements controllerlib.Syncer. func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { obj, err := c.jwtAuthenticators.Lister().Get(ctx.Key.Name) - if err != nil && apierrors.IsNotFound(err) { c.log.Info("Sync() found that the JWTAuthenticator does not exist yet or was deleted") return nil } if err != nil { + // no unit test for this failure return fmt.Errorf("failed to get JWTAuthenticator %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err) } @@ -177,35 +180,37 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { Name: ctx.Key.Name, } - // If this authenticator already exists, then only recreate it if is different from the desired - // authenticator. We don't want to be creating a new authenticator for every resync period. - // - // If we do need to recreate the authenticator, then make sure cancel the old one to avoid - // goroutine leaks. - if value := c.cache.Get(cacheKey); value != nil { - jwtAuthenticator := c.extractValueAsJWTAuthenticator(value) - if jwtAuthenticator != nil { - if reflect.DeepEqual(jwtAuthenticator.spec, &obj.Spec) { - c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer).Info("actual jwt authenticator and desired jwt authenticator are the same") - return nil - } - jwtAuthenticator.Close() + // 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 + // configuration could have changed, because it is also possible that the network could be flaky. We are choosing + // to prefer to keep the authenticator cached (available for end-user auth attempts) during times of network flakes + // rather than trying to show the most up-to-date status possible. These validations are for administrator + // convenience at the time of a configuration change, to catch typos and blatant misconfigurations, rather + // than to constantly monitor for external issues. + var jwtAuthenticatorFromCache *cachedJWTAuthenticator + if valueFromCache := c.cache.Get(cacheKey); valueFromCache != nil { + jwtAuthenticatorFromCache = c.cacheValueAsJWTAuthenticator(valueFromCache) + if jwtAuthenticatorFromCache != nil && reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &obj.Spec) { + c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer). + Info("actual jwt authenticator and desired jwt authenticator are the same") + // Stop, no more work to be done. This authenticator is already validated and cached. + return nil } } conditions := make([]*metav1.Condition, 0) - specCopy := obj.Spec.DeepCopy() var errs []error - rootCAs, conditions, tlsOk := c.validateTLS(specCopy.TLS, conditions) - _, conditions, issuerOk := c.validateIssuer(specCopy.Issuer, conditions) + rootCAs, conditions, tlsOk := c.validateTLSBundle(obj.Spec.TLS, conditions) + _, conditions, issuerOk := c.validateIssuer(obj.Spec.Issuer, conditions) okSoFar := tlsOk && issuerOk client := phttp.Default(rootCAs) client.Timeout = 30 * time.Second // copied from Kube OIDC code coreOSCtx := coreosoidc.ClientContext(context.Background(), client) - pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, specCopy.Issuer, conditions, okSoFar) + pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, obj.Spec.Issuer, conditions, okSoFar) errs = append(errs, providerErr) okSoFar = okSoFar && providerErr == nil @@ -217,21 +222,30 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { errs = append(errs, jwksFetchErr) okSoFar = okSoFar && jwksFetchErr == nil - // Make a deep copy of the spec so we aren't storing pointers to something that the informer cache - // may mutate! We don't store status as status is derived from spec. - cachedAuthenticator, conditions, err := c.newCachedJWTAuthenticator( + newJWTAuthenticatorForCache, conditions, err := c.newCachedJWTAuthenticator( client, - obj.Spec.DeepCopy(), + obj.Spec.DeepCopy(), // deep copy to avoid caching original object keySet, conditions, okSoFar) errs = append(errs, err) - if !conditionsutil.HadErrorCondition(conditions) { - c.cache.Store(cacheKey, cachedAuthenticator) - c.log.Info("added new jwt authenticator", "jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer) + if conditionsutil.HadErrorCondition(conditions) { + // The authenticator was determined to be invalid. Remove it from the cache, in case it was previously + // validated and cached. Do not allow an old, previously validated spec of the authenticator to continue + // being used for authentication. + c.cache.Delete(cacheKey) + } else { + c.cache.Store(cacheKey, newJWTAuthenticatorForCache) + c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer). + Info("added new jwt authenticator") } + // In case we just overwrote or deleted the authenticator from the cache, clean up the old instance + // to avoid leaking goroutines. It's safe to call Close() on nil. We avoid calling Close() until it is + // removed from the cache, because we do not want any end-user authentications to use a closed authenticator. + jwtAuthenticatorFromCache.Close() + err = c.updateStatus(ctx.Context, obj, conditions) errs = append(errs, err) @@ -243,7 +257,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { return utilerrors.NewAggregate(errs) } -func (c *jwtCacheFillerController) extractValueAsJWTAuthenticator(value authncache.Value) *cachedJWTAuthenticator { +func (c *jwtCacheFillerController) cacheValueAsJWTAuthenticator(value authncache.Value) *cachedJWTAuthenticator { jwtAuthenticator, ok := value.(*cachedJWTAuthenticator) if !ok { actualType := "" @@ -256,7 +270,7 @@ func (c *jwtCacheFillerController) extractValueAsJWTAuthenticator(value authncac return jwtAuthenticator } -func (c *jwtCacheFillerController) validateTLS(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) { +func (c *jwtCacheFillerController) validateTLSBundle(tlsSpec *authenticationv1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) { rootCAs, _, err := pinnipedcontroller.BuildCertPoolAuth(tlsSpec) if err != nil { msg := fmt.Sprintf("%s: %s", "invalid TLS configuration", err.Error()) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index eb982844d..cdc207b1b 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -998,7 +998,7 @@ func TestController(t *testing.T) { runTestsOnResultingAuthenticator: false, // skip the tests because the authenticator left in the cache is the mock version that was added above }, { - name: "Sync: JWTAuthenticator update when cached authenticator is different type: loop will complete successfully and update status conditions.", + name: "Sync: authenticator update when cached authenticator is the wrong data type, which should never really happen: loop will complete successfully and update status conditions.", cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { cache.Store( authncache.Key{ @@ -1006,6 +1006,9 @@ func TestController(t *testing.T) { Kind: "JWTAuthenticator", APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, }, + // Only entries of type cachedJWTAuthenticator are ever put into the cache, so this should never really happen. + // This test is to provide coverage on the production code which reads from the cache and casts those entries to + // the appropriate data type. struct{ authenticator.Token }{}, ) }, @@ -1143,6 +1146,58 @@ func TestController(t *testing.T) { }, wantCacheEntries: 0, }, + { + 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, wantClose bool) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "JWTAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + newCacheValue(t, *someJWTAuthenticatorSpec, wantClose), + ) + }, + jwtAuthenticators: []runtime.Object{ + &authenticationv1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *invalidIssuerJWTAuthenticatorSpec, + }, + }, + syncKey: controllerlib.Key{Name: "test-name"}, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *invalidIssuerJWTAuthenticatorSpec, + Status: authenticationv1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + sadReadyCondition(frozenMetav1Now, 0), + sadIssuerURLValidInvalid("https://.café .com/café/café/café/coffee", frozenMetav1Now, 0), + unknownDiscoveryURLValid(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 0, // removed from cache + wantClose: true, // the removed cache entry was also closed + }, { name: "validateIssuer: parsing error (spec.issuer URL is invalid): loop will fail sync, will write failed and unknown status conditions, but will not enqueue a resync due to user config error", jwtAuthenticators: []runtime.Object{ diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index e0fd73af5..f9400db4b 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "fmt" "net/url" + "reflect" "time" k8sauthv1beta1 "k8s.io/api/authentication/v1beta1" @@ -56,6 +57,11 @@ const ( msgUnableToValidate = "unable to validate; see other conditions for details" ) +type cachedWebhookAuthenticator struct { + authenticator.Token + spec *authenticationv1alpha1.WebhookAuthenticatorSpec +} + // New instantiates a new controllerlib.Controller which will populate the provided authncache.Cache. func New( cache *authncache.Cache, @@ -103,19 +109,44 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { return fmt.Errorf("failed to get WebhookAuthenticator %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err) } + cacheKey := authncache.Key{ + APIGroup: authenticationv1alpha1.GroupName, + Kind: "WebhookAuthenticator", + Name: ctx.Key.Name, + } + + // 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 + // configuration could have changed, because it is also possible that the network could be flaky. We are choosing + // to prefer to keep the authenticator cached (available for end-user auth attempts) during times of network flakes + // rather than trying to show the most up-to-date status possible. These validations are for administrator + // convenience at the time of a configuration change, to catch typos and blatant misconfigurations, rather + // 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) { + 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. + return nil + } + } + 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 + conditions, tlsNegotiateErr := c.validateConnection(certPool, endpointHostPort, conditions, okSoFar) errs = append(errs, tlsNegotiateErr) okSoFar = okSoFar && tlsNegotiateErr == nil - webhookAuthenticator, conditions, err := newWebhookAuthenticator( + newWebhookAuthenticatorForCache, conditions, err := newWebhookAuthenticator( // Note that we use the whole URL when constructing the webhook client, - // not just the host and port that ew validated above. We need the path, etc. + // not just the host and port that we validated above. We need the path, etc. obj.Spec.Endpoint, pemBytes, conditions, @@ -123,13 +154,18 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { ) errs = append(errs, err) - if !conditionsutil.HadErrorCondition(conditions) { - c.cache.Store(authncache.Key{ - APIGroup: authenticationv1alpha1.GroupName, - Kind: "WebhookAuthenticator", - Name: ctx.Key.Name, - }, webhookAuthenticator) - c.log.WithValues("webhook", klog.KObj(obj), "endpoint", obj.Spec.Endpoint).Info("added new webhook authenticator") + if conditionsutil.HadErrorCondition(conditions) { + // The authenticator was determined to be invalid. Remove it from the cache, in case it was previously + // validated and cached. Do not allow an old, previously validated spec of the authenticator to continue + // being used for authentication. + c.cache.Delete(cacheKey) + } else { + c.cache.Store(cacheKey, &cachedWebhookAuthenticator{ + Token: newWebhookAuthenticatorForCache, + spec: obj.Spec.DeepCopy(), // deep copy to avoid caching original object + }) + c.log.WithValues("webhook", klog.KObj(obj), "endpoint", obj.Spec.Endpoint). + Info("added new webhook authenticator") } err = c.updateStatus(ctx.Context, obj, conditions) @@ -143,6 +179,19 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { return utilerrors.NewAggregate(errs) } +func (c *webhookCacheFillerController) cacheValueAsWebhookAuthenticator(value authncache.Value) *cachedWebhookAuthenticator { + webhookAuthenticator, ok := value.(*cachedWebhookAuthenticator) + if !ok { + actualType := "" + if t := reflect.TypeOf(value); t != nil { + actualType = t.String() + } + c.log.WithValues("actualType", actualType).Info("wrong webhook authenticator type in cache") + return nil + } + return webhookAuthenticator +} + // newWebhookAuthenticator creates a webhook from the provided API server url and caBundle // used to validate TLS connections. func newWebhookAuthenticator( diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index e923f0ebb..b2c60418c 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authentication/authenticator" coretesting "k8s.io/client-go/testing" clocktesting "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" @@ -360,6 +361,7 @@ func TestController(t *testing.T) { tests := []struct { name string + cache func(*testing.T, *authncache.Cache) syncKey controllerlib.Key webhooks []runtime.Object // for modifying the clients to hack in arbitrary api responses @@ -424,7 +426,125 @@ func TestController(t *testing.T) { wantCacheEntries: 1, }, { - name: "Sync: changed WebhookAuthenticator: loop will update timestamps only on relevant statuses", + 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) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWithCA}, + ) + }, + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + Phase: "Ready", + }, + }, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "actual webhook authenticator and desired webhook authenticator are the same", + "endpoint": goodWebhookDefaultServingCertEndpoint, + "webhook": map[string]any{ + "name": "test-name", + }, + }, + }, + wantActions: func() []coretesting.Action { + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + } + }, + wantCacheEntries: 1, + }, + { + name: "Sync: authenticator update when cached authenticator is the wrong data type, which should never really happen: loop will complete successfully and update status conditions.", + cache: func(t *testing.T, cache *authncache.Cache) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + // Only entries of type cachedWebhookAuthenticator are ever put into the cache, so this should never really happen. + // This test is to provide coverage on the production code which reads from the cache and casts those entries to + // the appropriate data type. + struct{ authenticator.Token }{}, + ) + }, + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + }, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "wrong webhook authenticator type in cache", + "actualType": "struct { authenticator.Token }", + }, + { + "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: goodWebhookAuthenticatorSpecWithCA, + 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: changed WebhookAuthenticator: loop will update timestamps only on relevant statuses", + cache: func(t *testing.T, cache *authncache.Cache) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWith404Endpoint}, + ) + }, syncKey: controllerlib.Key{Name: "test-name"}, webhooks: []runtime.Object{ &authenticationv1alpha1.WebhookAuthenticator{ @@ -662,6 +782,60 @@ func TestController(t *testing.T) { }, wantCacheEntries: 0, }, + { + 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) { + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + &cachedWebhookAuthenticator{spec: &goodWebhookAuthenticatorSpecWithCA}, + ) + }, + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: authenticationv1alpha1.WebhookAuthenticatorSpec{ + Endpoint: badEndpointInvalidURL, + }, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: authenticationv1alpha1.WebhookAuthenticatorSpec{ + Endpoint: badEndpointInvalidURL, + }, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + []metav1.Condition{ + happyTLSConfigurationValidNoCA(frozenMetav1Now, 0), + sadEndpointURLValid("https://.café .com/café/café/café/coffee", 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, // removed from cache + }, { name: "validateEndpoint: parsing error (spec.endpoint URL is invalid) will fail sync loop and will report failed and unknown conditions and Error phase, but will not enqueue a resync due to user config error", syncKey: controllerlib.Key{Name: "test-name"}, @@ -1319,6 +1493,10 @@ func TestController(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) + if tt.cache != nil { + tt.cache(t, cache) + } + controller := New( cache, pinnipedAPIClient, diff --git a/test/integration/concierge_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index 2a68ec5fa..99d06034a 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/test/integration/concierge_webhookauthenticator_status_test.go @@ -266,12 +266,13 @@ func TestConciergeWebhookAuthenticatorCRDValidations_Parallel(t *testing.T) { } func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { - return []metav1.Condition{{ - Type: "AuthenticatorValid", - Status: "True", - Reason: "Success", - Message: "authenticator initialized", - }, + return []metav1.Condition{ + { + Type: "AuthenticatorValid", + Status: "True", + Reason: "Success", + Message: "authenticator initialized", + }, { Type: "EndpointURLValid", Status: "True",