From 42acf8dcce1d30a482e8951875347b71a619e950 Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Thu, 22 Feb 2024 14:52:40 -0500 Subject: [PATCH] Add Status & tests for jwks key fetching --- .../jwtcachefiller/jwtcachefiller.go | 77 ++++++++++++- .../jwtcachefiller/jwtcachefiller_test.go | 108 +++++++++++++++--- .../federation_domain_watcher_test.go | 46 ++++---- .../conditionstestutil.go} | 14 +-- 4 files changed, 198 insertions(+), 47 deletions(-) rename internal/testutil/{status/condition.go => conditionstestutil/conditionstestutil.go} (52%) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 7aa7715db..b63241349 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -12,10 +12,12 @@ import ( "net/http" "net/url" "reflect" + "strings" "time" coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/go-jose/go-jose/v3" + "github.com/ory/fosite/token/jwt" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -50,6 +52,7 @@ const ( typeIssuerURLValid = "IssuerURLValid" typeDiscoveryValid = "DiscoveryURLValid" typeJWKSURLValid = "JWKSURLValid" + typeJWKSFetchValid = "JWKSFetchValid" typeAuthenticatorValid = "AuthenticatorValid" reasonSuccess = "Success" @@ -62,6 +65,8 @@ const ( reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidDiscoveryProbe = "InvalidDiscoveryProbe" reasonInvalidAuthenticator = "InvalidAuthenticator" + reasonInvalidTokenSigning = "InvalidTokenSigning" + reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS" msgUnableToValidate = "unable to validate; see other conditions for details" @@ -183,9 +188,17 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { jwksURL, conditions, jwksErr := c.validateProviderJWKSURL(provider, pJSON, conditions, tlsOk && issuerOk && providerErr == nil) errs = append(errs, jwksErr) + keySet, conditions, jwksFetchErr := c.validateJWKSFetch(coreOSCtx, jwksURL, conditions, tlsOk && issuerOk && providerErr == nil && jwksErr == nil) + errs = append(errs, jwksFetchErr) + // 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(coreOSCtx, client, obj.Spec.DeepCopy(), jwksURL, conditions, tlsOk && issuerOk && providerErr == nil && jwksErr == nil) + cachedAuthenticator, conditions, err := c.newCachedJWTAuthenticator( + client, + obj.Spec.DeepCopy(), + keySet, + conditions, + tlsOk && issuerOk && providerErr == nil && jwksErr == nil && jwksFetchErr == nil) errs = append(errs, err) if !conditionsutil.HadErrorCondition(conditions) { @@ -257,8 +270,66 @@ func (c *jwtCacheFillerController) updateStatus( return err } +func (c *jwtCacheFillerController) validateJWKSFetch(ctx context.Context, jwksURL string, conditions []*metav1.Condition, prereqOk bool) (*coreosoidc.RemoteKeySet, []*metav1.Condition, error) { + if !prereqOk { + conditions = append(conditions, &metav1.Condition{ + Type: typeJWKSFetchValid, + Status: metav1.ConditionUnknown, + Reason: reasonUnableToValidate, + Message: msgUnableToValidate, + }) + return nil, conditions, nil + } + keySet := coreosoidc.NewRemoteKeySet(ctx, jwksURL) + testJWTToken := jwt.NewWithClaims(jwt.SigningMethodNone, jwt.MapClaims{ + "aud": "fake-audience-for-verification-probe", + }) + rawToken, signErr := testJWTToken.SignedString(jwt.UnsafeAllowNoneSignatureType) + // no unit tests for this block. + // this is not configurable, there is no way to change the token we are using + // for testing, so we simply shouldn't hit this block. + if signErr != nil { + errText := "could not sign tokens" + msg := fmt.Sprintf("%s: %s", errText, signErr.Error()) + conditions = append(conditions, &metav1.Condition{ + Type: typeJWKSFetchValid, + Status: metav1.ConditionFalse, + Reason: reasonInvalidTokenSigning, + Message: msg, + }) + return keySet, conditions, fmt.Errorf("%s: %w", errText, signErr) + } + _, verifyWithKeySetErr := keySet.VerifySignature(ctx, rawToken) + verifyErrString := verifyWithKeySetErr.Error() + // we need to fetch the keys. this is the main concern of this function + if strings.Contains(verifyErrString, "fetching keys") { + errText := "could not fetch keys" + msg := fmt.Sprintf("%s: %s", errText, verifyErrString) + conditions = append(conditions, &metav1.Condition{ + Type: typeJWKSFetchValid, + Status: metav1.ConditionFalse, + Reason: reasonInvalidCouldNotFetchJWKS, + Message: msg, + }) + return keySet, conditions, fmt.Errorf("%s: %w", errText, verifyWithKeySetErr) + } + // this error indicates success of this check. we only wanted to test if we could fetch, we aren't actually + // testing for valid signature verification. + if strings.Contains(verifyErrString, "failed to verify id token signature") { + conditions = append(conditions, &metav1.Condition{ + Type: typeJWKSFetchValid, + Status: metav1.ConditionTrue, + Reason: reasonSuccess, + Message: "successfully fetched jwks", + }) + return keySet, conditions, nil + } + // any other errors we will ignore and treat this as a success. + return keySet, conditions, nil +} + // newCachedJWTAuthenticator creates a jwt authenticator from the provided spec. -func (c *jwtCacheFillerController) newCachedJWTAuthenticator(ctx context.Context, client *http.Client, spec *auth1alpha1.JWTAuthenticatorSpec, jwksURL string, conditions []*metav1.Condition, prereqOk bool) (*cachedJWTAuthenticator, []*metav1.Condition, error) { +func (c *jwtCacheFillerController) newCachedJWTAuthenticator(client *http.Client, spec *auth1alpha1.JWTAuthenticatorSpec, keySet *coreosoidc.RemoteKeySet, conditions []*metav1.Condition, prereqOk bool) (*cachedJWTAuthenticator, []*metav1.Condition, error) { if !prereqOk { conditions = append(conditions, &metav1.Condition{ Type: typeAuthenticatorValid, @@ -295,7 +366,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(ctx context.Context }, }, }, - KeySet: coreosoidc.NewRemoteKeySet(ctx, jwksURL), + KeySet: keySet, SupportedSigningAlgs: defaultSupportedSigningAlgos(), Client: client, }) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 90fdd6d7b..c5da9ffd5 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -42,7 +42,7 @@ import ( "go.pinniped.dev/internal/mocks/mocktokenauthenticatorcloser" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" - "go.pinniped.dev/internal/testutil/status" + "go.pinniped.dev/internal/testutil/conditionstestutil" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -172,6 +172,17 @@ func TestController(t *testing.T) { require.NoError(t, err) })) + jwksFetchShouldFailMux := http.NewServeMux() + jwksFetchShouldFailServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + tlsserver.AssertTLS(t, r, ptls.Default) + jwksFetchShouldFailMux.ServeHTTP(w, r) + }), tlsserver.RecordTLSHello) + jwksFetchShouldFailMux.Handle("/.well-known/openid-configuration", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, err := fmt.Fprintf(w, `{"issuer": "%s", "jwks_uri": "%s"}`, jwksFetchShouldFailServer.URL, jwksFetchShouldFailServer.URL+"/fetch/will/fail/jwks.json") + require.NoError(t, err) + })) + goodIssuer := goodOIDCIssuerServer.URL badIssuerInvalidJWKSURI := badOIDCIssuerServerInvalidJWKSURI.URL badIssuerInvalidJWKSURIScheme := badOIDCIssuerServerInvalidJWKSURIScheme.URL @@ -246,6 +257,12 @@ func TestController(t *testing.T) { TLS: tlsSpecFromTLSConfig(badOIDCIssuerServerInvalidJWKSURIScheme.TLS), } + jwksFetchShouldFailJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ + Issuer: jwksFetchShouldFailServer.URL, + Audience: goodAudience, + TLS: tlsSpecFromTLSConfig(jwksFetchShouldFailServer.TLS), + } + happyReadyCondition := func(time metav1.Time, observedGeneration int64) metav1.Condition { return metav1.Condition{ Type: "Ready", @@ -438,13 +455,44 @@ func TestController(t *testing.T) { Message: `jwks_uri ` + issuer + ` has invalid scheme, require 'https'`, } } + happyJWKSFetch := func(time metav1.Time, observedGeneration int64) metav1.Condition { + return metav1.Condition{ + Type: "JWKSFetchValid", + Status: "True", + ObservedGeneration: observedGeneration, + LastTransitionTime: time, + Reason: "Success", + Message: "successfully fetched jwks", + } + } + unknownJWKSFetch := func(time metav1.Time, observedGeneration int64) metav1.Condition { + return metav1.Condition{ + Type: "JWKSFetchValid", + Status: "Unknown", + ObservedGeneration: observedGeneration, + LastTransitionTime: time, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + } + } + sadJWKSFetch := func(time metav1.Time, observedGeneration int64) metav1.Condition { + return metav1.Condition{ + Type: "JWKSFetchValid", + Status: "False", + ObservedGeneration: observedGeneration, + LastTransitionTime: time, + Reason: "InvalidCouldNotFetchJWKS", + Message: "could not fetch keys: fetching keys oidc: get keys failed: 404 Not Found 404 page not found\n", + } + } allHappyConditionsSuccess := func(issuer string, someTime metav1.Time, observedGeneration int64) []metav1.Condition { - return status.SortConditionsByType([]metav1.Condition{ + return conditionstestutil.SortByType([]metav1.Condition{ happyAuthenticatorValid(someTime, observedGeneration), happyDiscoveryURLValid(someTime, observedGeneration), happyIssuerURLValid(someTime, observedGeneration), happyJWKSURLValid(someTime, observedGeneration), + happyJWKSFetch(someTime, observedGeneration), happyReadyCondition(someTime, observedGeneration), happyTLSConfigurationValidCAParsed(someTime, observedGeneration), }) @@ -722,13 +770,14 @@ func TestController(t *testing.T) { }, // no explicit logs, this is an issue of config, the user must provide TLS config for the // custom cert provided for this server. - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), sadDiscoveryURLValidx509(goodIssuer, frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), happyTLSConfigurationValidNoCA(frozenMetav1Now, 0), }, ), @@ -747,7 +796,7 @@ func TestController(t *testing.T) { Spec: *invalidTLSJWTAuthenticatorSpec, }, }, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(someOtherIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -755,6 +804,7 @@ func TestController(t *testing.T) { unknownDiscoveryURLValid(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), }, ), wantStatusPhase: "Error", @@ -770,7 +820,7 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -778,6 +828,7 @@ func TestController(t *testing.T) { unknownDiscoveryURLValid(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), }, ), wantStatusPhase: "Error", @@ -792,7 +843,7 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -800,6 +851,7 @@ func TestController(t *testing.T) { unknownDiscoveryURLValid(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), }, ), wantStatusPhase: "Error", @@ -814,7 +866,7 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ happyIssuerURLValid(frozenMetav1Now, 0), @@ -822,6 +874,7 @@ func TestController(t *testing.T) { sadDiscoveryURLValidConnectionRefused(goodIssuer+"/foo/bar/baz/shizzle", frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), happyTLSConfigurationValidNoCA(frozenMetav1Now, 0), }, ), @@ -843,13 +896,14 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ happyIssuerURLValid(frozenMetav1Now, 0), sadReadyCondition(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), sadJWKSURLValidParseURI("https://.café .com/café/café/café/coffee/jwks.json", frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), }, ), wantStatusPhase: "Error", @@ -865,18 +919,44 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ happyIssuerURLValid(frozenMetav1Now, 0), sadReadyCondition(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), sadJWKSURLValidScheme("http://.café.com/café/café/café/coffee/jwks.json", frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), }, ), wantStatusPhase: "Error", wantSyncLoopErr: testutil.WantExactErrorString("jwks_uri http://.café.com/café/café/café/coffee/jwks.json has invalid scheme, require 'https'"), }, + // since this is a hard-coded token we can't do any meaningful testing for this case (and should also never have an error) + // {name: "validateJWKSFetch: could not sign tokens"}, + { + name: "validateJWKSFetch: could not fetch keys.... this should be a resync err", + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *jwksFetchShouldFailJWTAuthenticatorSpec, + }, + }, + syncKey: controllerlib.Key{Name: "test-name"}, + wantStatusConditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + happyIssuerURLValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + sadJWKSFetch(frozenMetav1Now, 0), + }, + ), + wantStatusPhase: "Error", + wantSyncLoopErr: testutil.WantExactErrorString("could not fetch keys: fetching keys oidc: get keys failed: 404 Not Found 404 page not found\n"), + }, { name: "updateStatus: when updateStatus() is called with matching original and updated conditions, it will not update", jwtAuthenticators: []runtime.Object{ @@ -915,7 +995,7 @@ func TestController(t *testing.T) { }, Spec: *someJWTAuthenticatorSpec, Status: auth1alpha1.JWTAuthenticatorStatus{ - Conditions: status.ReplaceConditions( + Conditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -949,7 +1029,7 @@ func TestController(t *testing.T) { }, Spec: *someJWTAuthenticatorSpec, Status: auth1alpha1.JWTAuthenticatorStatus{ - Conditions: status.ReplaceConditions( + Conditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -980,7 +1060,7 @@ func TestController(t *testing.T) { }, }}, // conditions and phase match previous state since update failed - wantStatusConditions: status.ReplaceConditions( + wantStatusConditions: conditionstestutil.Replace( allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), []metav1.Condition{ sadReadyCondition(frozenMetav1Now, 0), @@ -1038,7 +1118,7 @@ func TestController(t *testing.T) { } actualLogLines := logLines(log.String()) - require.Equal(t, len(actualLogLines), len(tt.wantLogs), "log line count should be correct") + require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct") for logLineNum, logLine := range actualLogLines { require.NotNil(t, tt.wantLogs[logLineNum], "expected log line should never be empty") @@ -1049,7 +1129,7 @@ func TestController(t *testing.T) { 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)) - require.Equal(t, lineStruct["logger"], tt.wantLogs[logLineNum]["logger"], fmt.Sprintf("log line (%d) logger should be correct", logLineNum)) + require.Equal(t, tt.wantLogs[logLineNum]["logger"], lineStruct["logger"], fmt.Sprintf("log line (%d) logger should be correct", logLineNum)) require.NotEmpty(t, lineStruct["caller"], fmt.Sprintf("log line (%d) caller should not be empty", logLineNum)) require.Equal(t, tt.wantLogs[logLineNum]["message"], lineStruct["message"], fmt.Sprintf("log line (%d) message should be correct", logLineNum)) if lineStruct["issuer"] != nil { diff --git a/internal/controller/supervisorconfig/federation_domain_watcher_test.go b/internal/controller/supervisorconfig/federation_domain_watcher_test.go index 57fe396e3..1f8876a6f 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher_test.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher_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 supervisorconfig @@ -32,7 +32,7 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/testutil" - "go.pinniped.dev/internal/testutil/status" + "go.pinniped.dev/internal/testutil/conditionstestutil" ) func TestFederationDomainWatcherControllerInformerFilters(t *testing.T) { @@ -492,7 +492,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { } allHappyConditionsSuccess := func(issuer string, time metav1.Time, observedGeneration int64) []metav1.Condition { - return status.SortConditionsByType([]metav1.Condition{ + return conditionstestutil.SortByType([]metav1.Condition{ happyTransformationExamplesCondition(frozenMetav1Now, 123), happyTransformationExpressionsCondition(frozenMetav1Now, 123), happyKindCondition(frozenMetav1Now, 123), @@ -507,7 +507,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { } allHappyConditionsLegacyConfigurationSuccess := func(issuer string, idpName string, time metav1.Time, observedGeneration int64) []metav1.Condition { - return status.ReplaceConditions( + return conditionstestutil.Replace( allHappyConditionsSuccess(issuer, time, observedGeneration), []metav1.Condition{ happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(idpName, time, observedGeneration), @@ -716,7 +716,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { wantStatusUpdates: []*configv1alpha1.FederationDomain{ expectedFederationDomainStatusUpdate(invalidIssuerURLFederationDomain, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123), @@ -758,7 +758,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { wantStatusUpdates: []*configv1alpha1.FederationDomain{ expectedFederationDomainStatusUpdate(invalidIssuerURLFederationDomain, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123), @@ -798,7 +798,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "duplicate1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess("https://iSSueR-duPlicAte.cOm/a", oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadIssuerIsUniqueCondition(frozenMetav1Now, 123), @@ -810,7 +810,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess("https://issuer-duplicate.com/a", oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadIssuerIsUniqueCondition(frozenMetav1Now, 123), @@ -871,7 +871,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "fd1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess("https://iSSueR-duPlicAte-adDress.cOm/path1", oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123), @@ -883,7 +883,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "fd2", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess("https://issuer-duplicate-address.com:1234/path2", oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123), @@ -895,7 +895,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLFederationDomain", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(invalidIssuerURL, oidcIdentityProvider.Name, frozenMetav1Now, 123), []metav1.Condition{ unknownIssuerIsUniqueCondition(frozenMetav1Now, 123), @@ -923,7 +923,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { wantStatusUpdates: []*configv1alpha1.FederationDomain{ expectedFederationDomainStatusUpdate(federationDomain1, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, "", frozenMetav1Now, 123), []metav1.Condition{ sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123), @@ -932,7 +932,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ), expectedFederationDomainStatusUpdate(federationDomain2, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, "", frozenMetav1Now, 123), []metav1.Condition{ sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123), @@ -953,7 +953,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { wantStatusUpdates: []*configv1alpha1.FederationDomain{ expectedFederationDomainStatusUpdate(federationDomain1, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, "", frozenMetav1Now, 123), []metav1.Condition{ sadIdentityProvidersFoundConditionIdentityProviderNotSpecified(3, frozenMetav1Now, 123), @@ -1005,7 +1005,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(here.Doc( @@ -1159,7 +1159,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadDisplayNamesUniqueCondition(`"duplicate1", "duplicate2"`, frozenMetav1Now, 123), @@ -1222,7 +1222,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadAPIGroupSuffixCondition(`"", "", "wrong.example.com"`, frozenMetav1Now, 123), @@ -1284,7 +1284,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadKindCondition(`"", "wrong"`, frozenMetav1Now, 123), @@ -1334,7 +1334,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadTransformationExpressionsCondition(here.Doc( @@ -1480,7 +1480,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadTransformationExamplesCondition(here.Doc( @@ -1579,7 +1579,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []metav1.Condition{ sadTransformationExamplesCondition(here.Doc( @@ -1726,7 +1726,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://not-unique.com", frozenMetav1Now, 123), []metav1.Condition{ sadAPIGroupSuffixCondition(`"this is wrong"`, frozenMetav1Now, 123), @@ -1786,7 +1786,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace, Generation: 123}, }, configv1alpha1.FederationDomainPhaseError, - status.ReplaceConditions( + conditionstestutil.Replace( allHappyConditionsSuccess("https://not-unique.com", frozenMetav1Now, 123), []metav1.Condition{ sadIssuerIsUniqueCondition(frozenMetav1Now, 123), diff --git a/internal/testutil/status/condition.go b/internal/testutil/conditionstestutil/conditionstestutil.go similarity index 52% rename from internal/testutil/status/condition.go rename to internal/testutil/conditionstestutil/conditionstestutil.go index 4f7e1bb13..4ab78e765 100644 --- a/internal/testutil/status/condition.go +++ b/internal/testutil/conditionstestutil/conditionstestutil.go @@ -1,7 +1,7 @@ // Copyright 2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -package status +package conditionstestutil import ( "sort" @@ -9,7 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func SortConditionsByType(c []metav1.Condition) []metav1.Condition { +func SortByType(c []metav1.Condition) []metav1.Condition { cp := make([]metav1.Condition, len(c)) copy(cp, c) sort.SliceStable(cp, func(i, j int) bool { @@ -18,14 +18,14 @@ func SortConditionsByType(c []metav1.Condition) []metav1.Condition { return cp } -func ReplaceConditions(conditions []metav1.Condition, sadConditions []metav1.Condition) []metav1.Condition { - for _, sadReplaceCondition := range sadConditions { - for origIndex, origCondition := range conditions { +func Replace(originals []metav1.Condition, replacements []metav1.Condition) []metav1.Condition { + for _, sadReplaceCondition := range replacements { + for origIndex, origCondition := range originals { if origCondition.Type == sadReplaceCondition.Type { - conditions[origIndex] = sadReplaceCondition + originals[origIndex] = sadReplaceCondition break } } } - return conditions + return originals }