Update copyright year in modified files

internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go

@@ -63,7 +63,7 @@ const (
 	reasonInvalidDiscoveryProbe        = "InvalidDiscoveryProbe"
 	reasonInvalidAuthenticator         = "InvalidAuthenticator"
 
-	msgUnableToValidate = "unable to validate; other issues present"
+	msgUnableToValidate = "unable to validate; see other conditions for details"
 
 	defaultUsernameClaim = oidcapi.IDTokenClaimUsername
 	defaultGroupsClaim   = oidcapi.IDTokenClaimGroups
@@ -170,14 +170,14 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
 	specCopy := obj.Spec.DeepCopy()
 	var errs []error
 
-	rootCAs, conditions, tlsOk := c.validateTLS(specCopy, conditions)
+	rootCAs, conditions, tlsOk := c.validateTLS(specCopy.TLS, conditions)
 	_, conditions, issuerOk := c.validateIssuer(specCopy.Issuer, conditions)
 
 	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, conditions, tlsOk && issuerOk)
+	pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, specCopy.Issuer, conditions, tlsOk && issuerOk)
 	errs = append(errs, providerErr)
 
 	jwksURL, conditions, jwksErr := c.validateProviderJWKSURL(provider, pJSON, conditions, tlsOk && issuerOk && providerErr == nil)
@@ -188,9 +188,9 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
 	cachedAuthenticator, conditions, err := c.newCachedJWTAuthenticator(coreOSCtx, client, obj.Spec.DeepCopy(), jwksURL, conditions, tlsOk && issuerOk && providerErr == nil && jwksErr == nil)
 	errs = append(errs, err)
 
-	if !hadErrorCondition(conditions) {
-		c.cache.Store(cacheKey, cachedAuthenticator) // the in-memory cache must need the extra functionality
-		c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer).Info("added new jwt authenticator")
+	if !conditionsutil.HadErrorCondition(conditions) {
+		c.cache.Store(cacheKey, cachedAuthenticator)
+		c.log.Info("added new jwt authenticator", "jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer)
 	}
 
 	err = c.updateStatus(ctx.Context, obj, conditions)
@@ -224,7 +224,7 @@ func (c *jwtCacheFillerController) updateStatus(
 ) error {
 	updated := original.DeepCopy()
 
-	if hadErrorCondition(conditions) {
+	if conditionsutil.HadErrorCondition(conditions) {
 		updated.Status.Phase = auth1alpha1.JWTAuthenticatorPhaseError
 		conditions = append(conditions, &metav1.Condition{
 			Type:    typeReady,
@@ -253,11 +253,7 @@ func (c *jwtCacheFillerController) updateStatus(
 	if equality.Semantic.DeepEqual(original, updated) {
 		return nil
 	}
-
 	_, err := c.client.AuthenticationV1alpha1().JWTAuthenticators().UpdateStatus(ctx, updated, metav1.UpdateOptions{})
-	if err != nil {
-		c.log.Info(fmt.Sprintf("ERROR: %v", err))
-	}
 	return err
 }
 
@@ -333,7 +329,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(ctx context.Context
 	}, conditions, nil
 }
 
-func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context, spec *auth1alpha1.JWTAuthenticatorSpec, conditions []*metav1.Condition, prereqOk bool) (*providerJSON, *coreosoidc.Provider, []*metav1.Condition, error) {
+func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context, issuer string, conditions []*metav1.Condition, prereqOk bool) (*providerJSON, *coreosoidc.Provider, []*metav1.Condition, error) {
 	if !prereqOk {
 		conditions = append(conditions, &metav1.Condition{
 			Type:    typeDiscoveryValid,
@@ -343,7 +339,7 @@ func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context
 		})
 		return nil, nil, conditions, nil
 	}
-	provider, err := coreosoidc.NewProvider(ctx, spec.Issuer)
+	provider, err := coreosoidc.NewProvider(ctx, issuer)
 	pJSON := &providerJSON{}
 	if err != nil {
 		errText := "could not perform oidc discovery on provider issuer"
@@ -426,8 +422,8 @@ func (c *jwtCacheFillerController) validateProviderJWKSURL(provider *coreosoidc.
 	return pJSON.JWKSURL, conditions, nil
 }
 
-func (c *jwtCacheFillerController) validateTLS(spec *auth1alpha1.JWTAuthenticatorSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) {
-	rootCAs, _, err := pinnipedauthenticator.CABundle(spec.TLS)
+func (c *jwtCacheFillerController) validateTLS(tlsSpec *auth1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) {
+	rootCAs, _, err := pinnipedauthenticator.CABundle(tlsSpec)
 	if err != nil {
 		msg := fmt.Sprintf("%s: %s", "invalid TLS configuration", err.Error())
 		conditions = append(conditions, &metav1.Condition{
@@ -439,7 +435,10 @@ func (c *jwtCacheFillerController) validateTLS(spec *auth1alpha1.JWTAuthenticato
 		return rootCAs, conditions, false
 	}
 
-	msg := "valid TLS configuration"
+	msg := "successfully parsed specified CA bundle"
+	if rootCAs == nil {
+		msg = "no CA bundle specified"
+	}
 	conditions = append(conditions, &metav1.Condition{
 		Type:    typeTLSConfigurationValid,
 		Status:  metav1.ConditionTrue,
@@ -481,12 +480,3 @@ func (c *jwtCacheFillerController) validateIssuer(issuer string, conditions []*m
 	})
 	return issuerURL, conditions, true
 }
-
-func hadErrorCondition(conditions []*metav1.Condition) bool {
-	for _, c := range conditions {
-		if c.Status != metav1.ConditionTrue {
-			return true
-		}
-	}
-	return false
-}

internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package jwtcachefiller
@@ -14,9 +14,9 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
+	coretesting "k8s.io/client-go/testing"
 	clocktesting "k8s.io/utils/clock/testing"
 
 	auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
@@ -175,12 +176,8 @@ func TestController(t *testing.T) {
 	badIssuerInvalidJWKSURI := badOIDCIssuerServerInvalidJWKSURI.URL
 	badIssuerInvalidJWKSURIScheme := badOIDCIssuerServerInvalidJWKSURIScheme.URL
 	someOtherIssuer := "https://some-other-issuer.com" // placeholder only for tests that don't get far enough to make requests
-	someOtherLocalhostIssuer := "https://127.0.0.1:443/some-other-issuer"
-	someOtherLocalhostIssuerParsed, err := url.Parse(someOtherLocalhostIssuer)
-	require.NoError(t, err)
 
 	nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)
-	require.NoError(t, err)
 	frozenMetav1Now := metav1.NewTime(nowDoesntMatter)
 	frozenClock := clocktesting.NewFakeClock(nowDoesntMatter)
 
@@ -235,7 +232,7 @@ func TestController(t *testing.T) {
 	}
 
 	validIssuerURLButDoesNotExistJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{
-		Issuer:   someOtherLocalhostIssuer,
+		Issuer:   goodIssuer + "/foo/bar/baz/shizzle",
 		Audience: goodAudience,
 	}
 	badIssuerJWKSURIJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{
@@ -270,14 +267,24 @@ func TestController(t *testing.T) {
 		}
 	}
 
-	happyTLSConfigurationValid := func(time metav1.Time, observedGeneration int64) metav1.Condition {
+	happyTLSConfigurationValidCAParsed := func(time metav1.Time, observedGeneration int64) metav1.Condition {
 		return metav1.Condition{
 			Type:               "TLSConfigurationValid",
 			Status:             "True",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "Success",
-			Message:            "valid TLS configuration",
+			Message:            "successfully parsed specified CA bundle",
+		}
+	}
+	happyTLSConfigurationValidNoCA := func(time metav1.Time, observedGeneration int64) metav1.Condition {
+		return metav1.Condition{
+			Type:               "TLSConfigurationValid",
+			Status:             "True",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "Success",
+			Message:            "no CA bundle specified",
 		}
 	}
 	sadTLSConfigurationValid := func(time metav1.Time, observedGeneration int64) metav1.Condition {
@@ -340,7 +347,7 @@ func TestController(t *testing.T) {
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "UnableToValidate",
-			Message:            "unable to validate; other issues present",
+			Message:            "unable to validate; see other conditions for details",
 		}
 	}
 	// NOTE: we can't reach this error the way our code is written.
@@ -367,7 +374,7 @@ func TestController(t *testing.T) {
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "UnableToValidate",
-			Message:            "unable to validate; other issues present",
+			Message:            "unable to validate; see other conditions for details",
 		}
 	}
 	sadDiscoveryURLValidx509 := func(issuer string, time metav1.Time, observedGeneration int64) metav1.Condition {
@@ -381,15 +388,13 @@ func TestController(t *testing.T) {
 		}
 	}
 	sadDiscoveryURLValidConnectionRefused := func(issuer string, time metav1.Time, observedGeneration int64) metav1.Condition {
-		parsed, err := url.Parse(issuer)
-		require.NoError(t, err)
 		return metav1.Condition{
 			Type:               "DiscoveryURLValid",
 			Status:             "False",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "InvalidDiscoveryProbe",
-			Message:            fmt.Sprintf(`could not perform oidc discovery on provider issuer: Get "%s/.well-known/openid-configuration": dial tcp %s: connect: connection refused`, issuer, parsed.Host),
+			Message:            fmt.Sprintf(`could not perform oidc discovery on provider issuer: Get "%s/.well-known/openid-configuration": tls: failed to verify certificate: x509: certificate signed by unknown authority`, issuer),
 		}
 	}
 
@@ -410,7 +415,7 @@ func TestController(t *testing.T) {
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "UnableToValidate",
-			Message:            "unable to validate; other issues present",
+			Message:            "unable to validate; see other conditions for details",
 		}
 	}
 	sadJWKSURLValidParseURI := func(issuer string, time metav1.Time, observedGeneration int64) metav1.Condition {
@@ -441,18 +446,18 @@ func TestController(t *testing.T) {
 			happyIssuerURLValid(someTime, observedGeneration),
 			happyJWKSURLValid(someTime, observedGeneration),
 			happyReadyCondition(someTime, observedGeneration),
-			happyTLSConfigurationValid(someTime, observedGeneration),
+			happyTLSConfigurationValidCAParsed(someTime, observedGeneration),
 		})
 	}
 
 	tests := []struct {
-		name  string
-		cache func(*testing.T, *authncache.Cache, bool)
-		// syncKey is used behind the scenes to simulate the pre-existence of jwtAuthenticator(s).
-		// if not provided, then the test assumes the jwtAuthenticator(s) are new Authenticator(s).
+		name              string
+		cache             func(*testing.T, *authncache.Cache, bool)
 		syncKey           controllerlib.Key
 		jwtAuthenticators []runtime.Object
-		wantClose         bool
+		// for modifying the clients to hack in arbitrary api responses
+		configClient func(*pinnipedfake.Clientset)
+		wantClose    bool
 		// Only errors that are non-config related errors are returned from the sync loop.
 		// Errors such as url.Parse of the issuer are not returned as they imply a user error.
 		// Since these errors trigger a resync, we are careful only to return an error when
@@ -481,6 +486,35 @@ func TestController(t *testing.T) {
 		// Existing code that was never tested. We would likely have to create a server with bad clients to
 		// simulate this.
 		// { name: "non-404 `failed to get JWTAuthenticator` for other API server reasons" }
+		{
+			name:    "valid jwt authenticator sync loop with no change will preserve existing status",
+			syncKey: controllerlib.Key{Name: "test-name"},
+			jwtAuthenticators: []runtime.Object{
+				&auth1alpha1.JWTAuthenticator{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-name",
+					},
+					Spec: *someJWTAuthenticatorSpec,
+					Status: auth1alpha1.JWTAuthenticatorStatus{
+						Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+						Phase:      "Ready",
+					},
+				},
+			},
+			wantLogs: []map[string]any{{
+				"level":     "info",
+				"timestamp": "2099-08-08T13:57:36.123456Z",
+				"logger":    "jwtcachefiller-controller",
+				"message":   "added new jwt authenticator",
+				"issuer":    goodIssuer,
+				"jwtAuthenticator": map[string]interface{}{
+					"name": "test-name",
+				},
+			}},
+			wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+			wantStatusPhase:      "Ready",
+			wantCacheEntries:     1,
+		},
 		{
 			name:    "valid jwt authenticator with CA will complete sync loop successfully with success conditions and ready phase",
 			syncKey: controllerlib.Key{Name: "test-name"},
@@ -695,6 +729,7 @@ func TestController(t *testing.T) {
 					sadDiscoveryURLValidx509(goodIssuer, frozenMetav1Now, 0),
 					unknownAuthenticatorValid(frozenMetav1Now, 0),
 					unknownJWKSURLValid(frozenMetav1Now, 0),
+					happyTLSConfigurationValidNoCA(frozenMetav1Now, 0),
 				},
 			),
 			wantStatusPhase:  "Error",
@@ -784,13 +819,14 @@ func TestController(t *testing.T) {
 				[]metav1.Condition{
 					happyIssuerURLValid(frozenMetav1Now, 0),
 					sadReadyCondition(frozenMetav1Now, 0),
-					sadDiscoveryURLValidConnectionRefused(someOtherLocalhostIssuer, frozenMetav1Now, 0),
+					sadDiscoveryURLValidConnectionRefused(goodIssuer+"/foo/bar/baz/shizzle", frozenMetav1Now, 0),
 					unknownAuthenticatorValid(frozenMetav1Now, 0),
 					unknownJWKSURLValid(frozenMetav1Now, 0),
+					happyTLSConfigurationValidNoCA(frozenMetav1Now, 0),
 				},
 			),
 			wantStatusPhase: "Error",
-			wantSyncLoopErr: testutil.WantExactErrorString(`could not perform oidc discovery on provider issuer: Get "` + someOtherLocalhostIssuer + `/.well-known/openid-configuration": dial tcp ` + someOtherLocalhostIssuerParsed.Host + `: connect: connection refused`),
+			wantSyncLoopErr: testutil.WantExactErrorString(`could not perform oidc discovery on provider issuer: Get "` + goodIssuer + `/foo/bar/baz/shizzle/.well-known/openid-configuration": tls: failed to verify certificate: x509: certificate signed by unknown authority`),
 		},
 		// cannot be tested currently the way the coreos lib works.
 		// the constructor requires an issuer in the payload and validates the issuer matches the actual issuer,
@@ -819,7 +855,7 @@ func TestController(t *testing.T) {
 			wantStatusPhase: "Error",
 			wantSyncLoopErr: testutil.WantExactErrorString(`could not parse provider jwks_uri: parse "https://.café   .com/café/café/café/coffee/jwks.json": invalid character " " in host name`),
 		}, {
-			name: "validateProviderJWKSURL: invalid scheme, requires 'https'  will fail sync loop and will report failed and unknown conditions and Error phase and will enqueue new sync",
+			name: "validateProviderJWKSURL: invalid scheme, requires 'https' will fail sync loop and will report failed and unknown conditions and Error phase and will enqueue new sync",
 			jwtAuthenticators: []runtime.Object{
 				&auth1alpha1.JWTAuthenticator{
 					ObjectMeta: metav1.ObjectMeta{
@@ -841,6 +877,119 @@ func TestController(t *testing.T) {
 			wantStatusPhase: "Error",
 			wantSyncLoopErr: testutil.WantExactErrorString("jwks_uri http://.café.com/café/café/café/coffee/jwks.json has invalid scheme, require 'https'"),
 		},
+		{
+			name: "updateStatus: when updateStatus() is called with matching original and updated conditions, it will not update",
+			jwtAuthenticators: []runtime.Object{
+				&auth1alpha1.JWTAuthenticator{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-name",
+					},
+					Spec: *someJWTAuthenticatorSpec,
+					Status: auth1alpha1.JWTAuthenticatorStatus{
+						Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+						Phase:      "Ready",
+					},
+				},
+			},
+			syncKey: controllerlib.Key{Name: "test-name"},
+			wantLogs: []map[string]any{{
+				"level":     "info",
+				"timestamp": "2099-08-08T13:57:36.123456Z",
+				"logger":    "jwtcachefiller-controller",
+				"message":   "added new jwt authenticator",
+				"issuer":    goodIssuer,
+				"jwtAuthenticator": map[string]interface{}{
+					"name": "test-name",
+				},
+			}},
+			wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+			wantStatusPhase:      "Ready",
+			wantCacheEntries:     1,
+		},
+		{
+			name: "updateStatus: when updateStatus() is called with different original and updated conditions, it will update the conditions",
+			jwtAuthenticators: []runtime.Object{
+				&auth1alpha1.JWTAuthenticator{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-name",
+					},
+					Spec: *someJWTAuthenticatorSpec,
+					Status: auth1alpha1.JWTAuthenticatorStatus{
+						Conditions: status.ReplaceConditions(
+							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+							[]metav1.Condition{
+								sadReadyCondition(frozenMetav1Now, 0),
+							},
+						),
+						Phase: "SomethingBeforeUpdating",
+					},
+				},
+			},
+			syncKey: controllerlib.Key{Name: "test-name"},
+			wantLogs: []map[string]any{{
+				"level":     "info",
+				"timestamp": "2099-08-08T13:57:36.123456Z",
+				"logger":    "jwtcachefiller-controller",
+				"message":   "added new jwt authenticator",
+				"issuer":    goodIssuer,
+				"jwtAuthenticator": map[string]interface{}{
+					"name": "test-name",
+				},
+			}},
+			wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+			wantStatusPhase:      "Ready",
+			wantCacheEntries:     1,
+		},
+		{
+			name: "updateStatus: when updateStatus() fails an error will enqueue a new sync",
+			jwtAuthenticators: []runtime.Object{
+				&auth1alpha1.JWTAuthenticator{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-name",
+					},
+					Spec: *someJWTAuthenticatorSpec,
+					Status: auth1alpha1.JWTAuthenticatorStatus{
+						Conditions: status.ReplaceConditions(
+							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+							[]metav1.Condition{
+								sadReadyCondition(frozenMetav1Now, 0),
+							},
+						),
+						Phase: "SomethingThatWontUpdate",
+					},
+				},
+			},
+			syncKey: controllerlib.Key{Name: "test-name"},
+			configClient: func(client *pinnipedfake.Clientset) {
+				client.PrependReactor(
+					"update",
+					"jwtauthenticators",
+					func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, errors.New("some update error")
+					},
+				)
+			},
+			wantLogs: []map[string]any{{
+				"level":     "info",
+				"timestamp": "2099-08-08T13:57:36.123456Z",
+				"logger":    "jwtcachefiller-controller",
+				"message":   "added new jwt authenticator",
+				"issuer":    goodIssuer,
+				"jwtAuthenticator": map[string]interface{}{
+					"name": "test-name",
+				},
+			}},
+			// conditions and phase match previous state since update failed
+			wantStatusConditions: status.ReplaceConditions(
+				allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+				[]metav1.Condition{
+					sadReadyCondition(frozenMetav1Now, 0),
+				},
+			),
+			wantStatusPhase:  "SomethingThatWontUpdate",
+			wantSyncLoopErr:  testutil.WantExactErrorString("some update error"),
+			wantCacheEntries: 1,
+		},
 		// cannot be tested the way we are invoking oidc.New as we don't provide enough configuration
 		// knobs to actually invoke the code in a broken way.  We always give a good client, good keys, and
 		// good signing algos.  In the future if we allow any of these to be configured we may have opportunity
@@ -854,6 +1003,9 @@ func TestController(t *testing.T) {
 			t.Parallel()
 
 			pinnipedAPIClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...)
+			if tt.configClient != nil {
+				tt.configClient(pinnipedAPIClient)
+			}
 			pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(pinnipedAPIClient, 0)
 			cache := authncache.New()
 
@@ -892,9 +1044,8 @@ func TestController(t *testing.T) {
 				require.NotNil(t, tt.wantLogs[logLineNum], "expected log line should never be empty")
 				var lineStruct map[string]any
 				err := json.Unmarshal([]byte(logLine), &lineStruct)
-				if err != nil {
-					t.Error(err)
-				}
+				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))
@@ -919,7 +1070,7 @@ func TestController(t *testing.T) {
 				jwtAuthSubject, getErr := pinnipedAPIClient.AuthenticationV1alpha1().JWTAuthenticators().Get(getCtx, "test-name", metav1.GetOptions{})
 				require.NoError(t, getErr)
 				require.Equal(t, tt.wantStatusConditions, jwtAuthSubject.Status.Conditions, "status.conditions must be correct")
-				require.Equal(t, tt.wantStatusPhase, jwtAuthSubject.Status.Phase, "jwt authenticator status.phase should be correct")
+				require.Equal(t, tt.wantStatusPhase, jwtAuthSubject.Status.Phase, "status.phase should be correct")
 			}
 
 			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()))

internal/controller/conditionsutil/conditions_util.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package conditionsutil
@@ -119,3 +119,12 @@ func mergeConfigCondition(existing *[]metav1.Condition, new *metav1.Condition) b
 	// Otherwise the entry is already up to date.
 	return false
 }
+
+func HadErrorCondition(conditions []*metav1.Condition) bool {
+	for _, c := range conditions {
+		if c.Status != metav1.ConditionTrue {
+			return true
+		}
+	}
+	return false
+}

internal/controller/supervisorconfig/federation_domain_watcher.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package supervisorconfig
@@ -211,7 +211,7 @@ func (c *federationDomainWatcherController) processAllFederationDomains(
 		// made the FederationDomain's endpoints available.
 		fdToConditionsMap[federationDomain] = conditions
 
-		if !hadErrorCondition(conditions) {
+		if !conditionsutil.HadErrorCondition(conditions) {
 			// Successfully validated the FederationDomain, so allow it to be loaded.
 			federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer)
 		}
@@ -794,7 +794,7 @@ func (c *federationDomainWatcherController) updateStatus(
 ) error {
 	updated := federationDomain.DeepCopy()
 
-	if hadErrorCondition(conditions) {
+	if conditionsutil.HadErrorCondition(conditions) {
 		updated.Status.Phase = configv1alpha1.FederationDomainPhaseError
 		conditions = append(conditions, &metav1.Condition{
 			Type:    typeReady,
@@ -950,15 +950,6 @@ func newCrossFederationDomainConfigValidator(federationDomains []*configv1alpha1
 	}
 }
 
-func hadErrorCondition(conditions []*metav1.Condition) bool {
-	for _, c := range conditions {
-		if c.Status != metav1.ConditionTrue {
-			return true
-		}
-	}
-	return false
-}
-
 func stringSetsEqual(a []string, b []string) bool {
 	aSet := sets.New(a...)
 	bSet := sets.New(b...)

internal/controllermanager/prepare_controllers.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package controllermanager provides an entrypoint into running all of the controllers that run as
@@ -236,8 +236,6 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol
 		WithController(
 			webhookcachefiller.New(
 				c.AuthenticatorCache,
-				// TODO (BEN): add the client here for next story
-				// client.PinnipedConcierge.AuthenticationV1alpha1().WebhookAuthenticators(),
 				informers.pinniped.Authentication().V1alpha1().WebhookAuthenticators(),
 				plog.Logr(), //nolint:staticcheck // old controller with lots of log statements
 			),