clarify some comments based on PR feedback

This commit is contained in:
Ryan Richard
2024-07-17 09:58:26 -07:00
parent b5a509f27f
commit a2be4b7b5e
5 changed files with 33 additions and 13 deletions
@@ -115,8 +115,14 @@ func (c *webhookCacheFillerController) 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.
// 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) {
@@ -471,7 +471,7 @@ func TestController(t *testing.T) {
wantCacheEntries: 1,
},
{
name: "Sync: authenticator 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) {
cache.Store(
authncache.Key{
@@ -479,6 +479,9 @@ func TestController(t *testing.T) {
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 }{},
)
},