From 13360ebac2da436bae4525a110e0ba5fe31f1546 Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Thu, 22 Feb 2024 23:50:09 -0500 Subject: [PATCH] Update jwtauthenticator unit tests to check actions - Add test to verify timestamps are particularly updated - Improve diff output in tests for actions - Make jwtauthenticator status tests parallel - Update copyright headers in multiple files --- .../v1alpha1/types_jwtauthenticator.go.tmpl | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../v1alpha1/types_jwtauthenticator.go | 2 +- .../jwtcachefiller/jwtcachefiller.go | 398 +++++------ .../jwtcachefiller/jwtcachefiller_test.go | 641 +++++++++++++----- .../ldap_upstream_watcher.go | 2 +- .../concierge_credentialrequest_test.go | 2 +- .../concierge_jwtauthenticator_status_test.go | 32 +- test/integration/kube_api_discovery_test.go | 2 +- 16 files changed, 729 insertions(+), 368 deletions(-) diff --git a/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go.tmpl b/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go.tmpl index da1cf354e2..f75d507769 100644 --- a/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go.tmpl +++ b/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go.tmpl @@ -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 v1alpha1 diff --git a/generated/1.21/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.21/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.21/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.21/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.22/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.22/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.22/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.22/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.23/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.23/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.23/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.23/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.24/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.24/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.24/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.24/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.25/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.25/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.25/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.25/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.26/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.26/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.26/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.26/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.27/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.27/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.27/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.27/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/1.28/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/1.28/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/1.28/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/1.28/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/generated/latest/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go b/generated/latest/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go index da1cf354e2..f75d507769 100644 --- a/generated/latest/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.go +++ b/generated/latest/apis/concierge/authentication/v1alpha1/types_jwtauthenticator.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 v1alpha1 diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index b632413496..9a6710df13 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -8,6 +8,7 @@ package jwtcachefiller import ( "context" "crypto/x509" + "errors" "fmt" "net/http" "net/url" @@ -17,7 +18,6 @@ import ( 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" @@ -42,8 +42,6 @@ import ( "go.pinniped.dev/internal/plog" ) -// These default values come from the way that the Supervisor issues and signs tokens. We make these -// the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor. const ( controllerName = "jwtcachefiller-controller" @@ -65,13 +63,17 @@ const ( reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidDiscoveryProbe = "InvalidDiscoveryProbe" reasonInvalidAuthenticator = "InvalidAuthenticator" - reasonInvalidTokenSigning = "InvalidTokenSigning" + reasonInvalidTokenSigningFailure = "InvalidTokenSigningFailure" reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS" msgUnableToValidate = "unable to validate; see other conditions for details" + // These default values come from the way that the Supervisor issues and signs tokens. We make these + // the defaults for a JWTAuthenticator so that they can easily integrate with the Supervisor. defaultUsernameClaim = oidcapi.IDTokenClaimUsername defaultGroupsClaim = oidcapi.IDTokenClaimGroups + + minimalJWTToTriggerJWKSFetch = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.e30." ) type providerJSON struct { @@ -177,19 +179,23 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { rootCAs, conditions, tlsOk := c.validateTLS(specCopy.TLS, conditions) _, conditions, issuerOk := c.validateIssuer(specCopy.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, tlsOk && issuerOk) + pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, specCopy.Issuer, conditions, okSoFar) errs = append(errs, providerErr) + okSoFar = okSoFar && providerErr == nil - jwksURL, conditions, jwksErr := c.validateProviderJWKSURL(provider, pJSON, conditions, tlsOk && issuerOk && providerErr == nil) + jwksURL, conditions, jwksErr := c.validateProviderJWKSURL(provider, pJSON, conditions, okSoFar) errs = append(errs, jwksErr) + okSoFar = okSoFar && jwksErr == nil - keySet, conditions, jwksFetchErr := c.validateJWKSFetch(coreOSCtx, jwksURL, conditions, tlsOk && issuerOk && providerErr == nil && jwksErr == nil) + keySet, conditions, jwksFetchErr := c.validateJWKSFetch(coreOSCtx, jwksURL, conditions, okSoFar) 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. @@ -198,7 +204,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error { obj.Spec.DeepCopy(), keySet, conditions, - tlsOk && issuerOk && providerErr == nil && jwksErr == nil && jwksFetchErr == nil) + okSoFar) errs = append(errs, err) if !conditionsutil.HadErrorCondition(conditions) { @@ -209,10 +215,10 @@ func (c *jwtCacheFillerController) 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 errorsutil.NewAggregate(errs) } @@ -230,174 +236,63 @@ func (c *jwtCacheFillerController) extractValueAsJWTAuthenticator(value authncac return jwtAuthenticator } -func (c *jwtCacheFillerController) updateStatus( - ctx context.Context, - original *auth1alpha1.JWTAuthenticator, - conditions []*metav1.Condition, -) error { - updated := original.DeepCopy() - - if conditionsutil.HadErrorCondition(conditions) { - updated.Status.Phase = auth1alpha1.JWTAuthenticatorPhaseError +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{ - Type: typeReady, + Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, - Reason: reasonNotReady, - Message: "the JWTAuthenticator is not ready: see other conditions for details", - }) - } else { - updated.Status.Phase = auth1alpha1.JWTAuthenticatorPhaseReady - conditions = append(conditions, &metav1.Condition{ - Type: typeReady, - Status: metav1.ConditionTrue, - Reason: reasonSuccess, - Message: "the JWTAuthenticator is ready", + Reason: reasonInvalidTLSConfiguration, + Message: msg, }) + return rootCAs, conditions, false } - _ = conditionsutil.MergeConfigConditions( - conditions, - original.Generation, - &updated.Status.Conditions, - plog.New().WithName(controllerName), - metav1.NewTime(c.clock.Now()), - ) - - if equality.Semantic.DeepEqual(original, updated) { - return nil + msg := "successfully parsed specified CA bundle" + if rootCAs == nil { + msg = "no CA bundle specified" } - _, err := c.client.AuthenticationV1alpha1().JWTAuthenticators().UpdateStatus(ctx, updated, metav1.UpdateOptions{}) - return err + conditions = append(conditions, &metav1.Condition{ + Type: typeTLSConfigurationValid, + Status: metav1.ConditionTrue, + Reason: reasonSuccess, + Message: msg, + }) + return rootCAs, conditions, true } -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) +func (c *jwtCacheFillerController) validateIssuer(issuer string, conditions []*metav1.Condition) (*url.URL, []*metav1.Condition, bool) { + issuerURL, err := url.Parse(issuer) + if err != nil { + msg := fmt.Sprintf("%s: %s", "spec.issuer URL is invalid", err.Error()) conditions = append(conditions, &metav1.Condition{ - Type: typeJWKSFetchValid, + Type: typeIssuerURLValid, Status: metav1.ConditionFalse, - Reason: reasonInvalidCouldNotFetchJWKS, + Reason: reasonInvalidIssuerURL, 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(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, - Status: metav1.ConditionUnknown, - Reason: reasonUnableToValidate, - Message: msgUnableToValidate, - }) - return nil, conditions, nil - } - - usernameClaim := spec.Claims.Username - if usernameClaim == "" { - usernameClaim = defaultUsernameClaim - } - groupsClaim := spec.Claims.Groups - if groupsClaim == "" { - groupsClaim = defaultGroupsClaim + return nil, conditions, false } - oidcAuthenticator, err := oidc.New(oidc.Options{ - JWTAuthenticator: apiserver.JWTAuthenticator{ - Issuer: apiserver.Issuer{ - URL: spec.Issuer, - Audiences: []string{spec.Audience}, - }, - ClaimMappings: apiserver.ClaimMappings{ - Username: apiserver.PrefixedClaimOrExpression{ - Claim: usernameClaim, - Prefix: ptr.To(""), - }, - Groups: apiserver.PrefixedClaimOrExpression{ - Claim: groupsClaim, - Prefix: ptr.To(""), - }, - }, - }, - KeySet: keySet, - SupportedSigningAlgs: defaultSupportedSigningAlgos(), - Client: client, - }) - if err != nil { - // no unit test for this failure. - // it seems that our production code doesn't provide config knobs that would allow - // incorrect configuration of oidc.New(). We validate inputs before we get to this point - // and exit early if there are problems. In the future, if we allow more configuration, - // such as supported signing algorithm config, we may be able to test this. - errText := "could not initialize oidc authenticator" - msg := fmt.Sprintf("%s: %s", errText, err.Error()) + if issuerURL.Scheme != "https" { + msg := fmt.Sprintf("spec.issuer %s has invalid scheme, require 'https'", issuer) conditions = append(conditions, &metav1.Condition{ - Type: typeAuthenticatorValid, + Type: typeIssuerURLValid, Status: metav1.ConditionFalse, - Reason: reasonInvalidAuthenticator, + Reason: reasonInvalidIssuerURLScheme, Message: msg, }) - // resync err, lots of possible issues that may or may not be machine related - return nil, conditions, fmt.Errorf("%s: %w", errText, err) + return nil, conditions, false } - msg := "authenticator initialized" + conditions = append(conditions, &metav1.Condition{ - Type: typeAuthenticatorValid, + Type: typeIssuerURLValid, Status: metav1.ConditionTrue, Reason: reasonSuccess, - Message: msg, + Message: "issuer is a valid URL", }) - return &cachedJWTAuthenticator{ - tokenAuthenticatorCloser: oidcAuthenticator, - spec: spec, - }, conditions, nil + return issuerURL, conditions, true } func (c *jwtCacheFillerController) validateProviderDiscovery(ctx context.Context, issuer string, conditions []*metav1.Condition, prereqOk bool) (*providerJSON, *coreosoidc.Provider, []*metav1.Condition, error) { @@ -493,61 +388,186 @@ func (c *jwtCacheFillerController) validateProviderJWKSURL(provider *coreosoidc. return pJSON.JWKSURL, conditions, nil } -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()) +// validateJWKSFetch deliberately takes an unsigned JWT to trigger coreosoidc.NewRemoteKeySet to +// indirectly fetch the JWKS. This lets us report a status about the endpoint, even though +// we expect the verfication checks to actually fail. This also pre-warms the cache of keys +// in the remote keyset object. +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: typeTLSConfigurationValid, + Type: typeJWKSFetchValid, + Status: metav1.ConditionUnknown, + Reason: reasonUnableToValidate, + Message: msgUnableToValidate, + }) + return nil, conditions, nil + } + keySet := coreosoidc.NewRemoteKeySet(ctx, jwksURL) + + // keySet.verifySignature calls functions which may error in a couple of ways that + // we will treat as success because we are really only concerned here that we could + // fetch the keys at all. + _, verifyWithKeySetErr := keySet.VerifySignature(ctx, minimalJWTToTriggerJWKSFetch) + if verifyWithKeySetErr == nil { + // No unit test. + // Since we hard-coded this token we expect there to always be a verification error. + // The purpose of this function is really to test if we can get the JWKS, not to actually validate a token. + // Therefore, we should never hit this path, nevertheless, lets handle just in case something unexpected happens. + errText := "jwks should not have verified unsigned jwt token" + conditions = append(conditions, &metav1.Condition{ + Type: typeJWKSFetchValid, + Status: metav1.ConditionUnknown, + Reason: reasonUnableToValidate, + Message: errText, + }) + return nil, conditions, errors.New(errText) + } + + verifyErrString := verifyWithKeySetErr.Error() + // We need to fetch the keys. This is the main concern of this function. + if strings.HasPrefix(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: reasonInvalidTLSConfiguration, + Reason: reasonInvalidCouldNotFetchJWKS, Message: msg, }) - return rootCAs, conditions, false + return nil, conditions, fmt.Errorf("%s: %w", errText, verifyWithKeySetErr) } - - msg := "successfully parsed specified CA bundle" - if rootCAs == nil { - msg = "no CA bundle specified" + // 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 } + + // No unit tests, currently no way to reach this code path. + errText := "unexpected verification error while fetching jwks" + msg := fmt.Sprintf("%s: %s", errText, verifyErrString) conditions = append(conditions, &metav1.Condition{ - Type: typeTLSConfigurationValid, - Status: metav1.ConditionTrue, - Reason: reasonSuccess, + Type: typeJWKSFetchValid, + Status: metav1.ConditionUnknown, + Reason: reasonUnableToValidate, Message: msg, }) - return rootCAs, conditions, true + return nil, conditions, fmt.Errorf("%s: %w", errText, verifyWithKeySetErr) } -func (c *jwtCacheFillerController) validateIssuer(issuer string, conditions []*metav1.Condition) (*url.URL, []*metav1.Condition, bool) { - issuerURL, err := url.Parse(issuer) - if err != nil { - msg := fmt.Sprintf("%s: %s", "spec.issuer URL is invalid", err.Error()) +// newCachedJWTAuthenticator creates a jwt authenticator from the provided spec. +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: typeIssuerURLValid, - Status: metav1.ConditionFalse, - Reason: reasonInvalidIssuerURL, - Message: msg, + Type: typeAuthenticatorValid, + Status: metav1.ConditionUnknown, + Reason: reasonUnableToValidate, + Message: msgUnableToValidate, }) - return nil, conditions, false + return nil, conditions, nil } - if issuerURL.Scheme != "https" { - msg := fmt.Sprintf("spec.issuer %s has invalid scheme, require 'https'", issuer) + usernameClaim := spec.Claims.Username + if usernameClaim == "" { + usernameClaim = defaultUsernameClaim + } + groupsClaim := spec.Claims.Groups + if groupsClaim == "" { + groupsClaim = defaultGroupsClaim + } + + oidcAuthenticator, err := oidc.New(oidc.Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: spec.Issuer, + Audiences: []string{spec.Audience}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Claim: usernameClaim, + Prefix: ptr.To(""), + }, + Groups: apiserver.PrefixedClaimOrExpression{ + Claim: groupsClaim, + Prefix: ptr.To(""), + }, + }, + }, + KeySet: keySet, + SupportedSigningAlgs: defaultSupportedSigningAlgos(), + Client: client, + }) + if err != nil { + // no unit test for this failure. + // it seems that our production code doesn't provide config knobs that would allow + // incorrect configuration of oidc.New(). We validate inputs before we get to this point + // and exit early if there are problems. In the future, if we allow more configuration, + // such as supported signing algorithm config, we may be able to test this. + errText := "could not initialize oidc authenticator" + msg := fmt.Sprintf("%s: %s", errText, err.Error()) conditions = append(conditions, &metav1.Condition{ - Type: typeIssuerURLValid, + Type: typeAuthenticatorValid, Status: metav1.ConditionFalse, - Reason: reasonInvalidIssuerURLScheme, + Reason: reasonInvalidAuthenticator, Message: msg, }) - return nil, conditions, false + // resync err, lots of possible issues that may or may not be machine related + return nil, conditions, fmt.Errorf("%s: %w", errText, err) } - + msg := "authenticator initialized" conditions = append(conditions, &metav1.Condition{ - Type: typeIssuerURLValid, + Type: typeAuthenticatorValid, Status: metav1.ConditionTrue, Reason: reasonSuccess, - Message: "issuer is a valid URL", + Message: msg, }) - return issuerURL, conditions, true + return &cachedJWTAuthenticator{ + tokenAuthenticatorCloser: oidcAuthenticator, + spec: spec, + }, conditions, nil +} + +func (c *jwtCacheFillerController) updateStatus( + ctx context.Context, + original *auth1alpha1.JWTAuthenticator, + conditions []*metav1.Condition, +) error { + updated := original.DeepCopy() + + if conditionsutil.HadErrorCondition(conditions) { + updated.Status.Phase = auth1alpha1.JWTAuthenticatorPhaseError + conditions = append(conditions, &metav1.Condition{ + Type: typeReady, + Status: metav1.ConditionFalse, + Reason: reasonNotReady, + Message: "the JWTAuthenticator is not ready: see other conditions for details", + }) + } else { + updated.Status.Phase = auth1alpha1.JWTAuthenticatorPhaseReady + conditions = append(conditions, &metav1.Condition{ + Type: typeReady, + Status: metav1.ConditionTrue, + Reason: reasonSuccess, + Message: "the JWTAuthenticator is ready", + }) + } + + _ = conditionsutil.MergeConfigConditions( + conditions, + original.Generation, + &updated.Status.Conditions, + plog.New().WithName(controllerName), + metav1.NewTime(c.clock.Now()), + ) + + if equality.Semantic.DeepEqual(original, updated) { + return nil + } + _, err := c.client.AuthenticationV1alpha1().JWTAuthenticators().UpdateStatus(ctx, updated, metav1.UpdateOptions{}) + return err } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index c5da9ffd5f..1663282537 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -24,9 +24,13 @@ import ( "github.com/go-jose/go-jose/v3" "github.com/go-jose/go-jose/v3/jwt" "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + fositejwt "github.com/ory/fosite/token/jwt" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" @@ -46,6 +50,13 @@ import ( "go.pinniped.dev/internal/testutil/tlsserver" ) +func TestMinimalJWTToTriggerJWKSFetch(t *testing.T) { + tinyJWT := fositejwt.NewWithClaims(fositejwt.SigningMethodNone, fositejwt.MapClaims{}) + tinyJWTStr, err := tinyJWT.SignedString(fositejwt.UnsafeAllowNoneSignatureType) + require.NoError(t, err) + require.Equal(t, tinyJWTStr, minimalJWTToTriggerJWKSFetch) +} + func TestController(t *testing.T) { t.Parallel() @@ -192,6 +203,9 @@ func TestController(t *testing.T) { frozenMetav1Now := metav1.NewTime(nowDoesntMatter) frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + timeInThePast := time.Date(1111, time.January, 1, 1, 1, 1, 111111, time.Local) + frozenTimeInThePast := metav1.NewTime(timeInThePast) + someJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: goodIssuer, Audience: goodAudience, @@ -497,7 +511,16 @@ func TestController(t *testing.T) { happyTLSConfigurationValidCAParsed(someTime, observedGeneration), }) } - + jwtAuthenticatorsGVR := schema.GroupVersionResource{ + Group: "authentication.concierge.pinniped.dev", + Version: "v1alpha1", + Resource: "jwtauthenticators", + } + jwtAUthenticatorGVK := schema.GroupVersionKind{ + Group: "authentication.concierge.pinniped.dev", + Version: "v1alpha1", + Kind: "JWTAuthenticator", + } tests := []struct { name string cache func(*testing.T, *authncache.Cache, bool) @@ -512,15 +535,14 @@ func TestController(t *testing.T) { // something can be automatically corrected on a retry (ie an error that might be networking). wantSyncLoopErr testutil.RequireErrorStringFunc wantLogs []map[string]any - wantStatusConditions []metav1.Condition - wantStatusPhase auth1alpha1.JWTAuthenticatorPhase + wantActions func() []coretesting.Action wantCacheEntries int wantUsernameClaim string wantGroupsClaim string runTestsOnResultingAuthenticator bool }{ { - name: "404: jwt authenticator not found will abort sync loop and not attempt to write status", + name: "404: JWTAuthenticator not found will abort sync loop, no status conditions.", syncKey: controllerlib.Key{Name: "test-name"}, wantLogs: []map[string]any{ { @@ -530,12 +552,15 @@ func TestController(t *testing.T) { "message": "Sync() found that the JWTAuthenticator does not exist yet or was deleted", }, }, + wantActions: func() []coretesting.Action { + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + } + }, }, - // 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", + name: "Sync: valid and unchanged JWTAuthenticator: loop will preserve existing status conditions", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -559,12 +584,78 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", - wantCacheEntries: 1, + wantActions: func() []coretesting.Action { + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + } + }, + wantCacheEntries: 1, + }, { + name: "Sync: changed JWTAuthenticator: loop will update timestamps only on relevant statuses", + syncKey: controllerlib.Key{Name: "test-name"}, + jwtAuthenticators: []runtime.Object{ + &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + // sad and unknwn will update with new statuses and timestamps + sadReadyCondition(frozenTimeInThePast, 0), + sadDiscoveryURLValidx509(goodIssuer, frozenTimeInThePast, 0), + unknownAuthenticatorValid(frozenTimeInThePast, 0), + unknownJWKSURLValid(frozenTimeInThePast, 0), + unknownJWKSFetch(frozenTimeInThePast, 0), + // this one will remain unchanged as it was good to begin with + happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 0), + }, + ), + Phase: "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", + }, + }}, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + // this timestamp should not have updated, it didn't change. + happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 0), + }, + ), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, }, { - name: "valid jwt authenticator with CA will complete sync loop successfully with success conditions and ready phase", + name: "Sync: valid JWTAuthenticator with CA: loop will complete successfully and update status conditions.", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -584,13 +675,29 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantCacheEntries: 1, runTestsOnResultingAuthenticator: true, }, { - name: "valid jwt authenticator with custom username claim will complete sync loop successfully with success conditions and ready phase", + name: "Sync: JWTAuthenticator with custom username claim: loop will complete successfully and update status conditions.", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -610,14 +717,30 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpecWithUsernameClaim, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantCacheEntries: 1, wantUsernameClaim: someJWTAuthenticatorSpecWithUsernameClaim.Claims.Username, runTestsOnResultingAuthenticator: true, }, { - name: "valid jwt authenticator with custom groups claim will complete sync loop successfully with success conditions and ready phase", + name: "Sync: JWTAuthenticator with custom groups claim: loop will complete successfully and update status conditions.", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -637,14 +760,30 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpecWithGroupsClaim, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantCacheEntries: 1, wantGroupsClaim: someJWTAuthenticatorSpecWithGroupsClaim.Claims.Groups, runTestsOnResultingAuthenticator: true, }, { - name: "updating jwt authenticator with new fields closes previous instance and will complete sync loop successfully with success conditions and ready phase", + name: "Sync: JWTAuthenticator with new fields: loop will close previous instance of JWTAuthenticator and complete successfully and update status conditions.", cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { cache.Store( authncache.Key{ @@ -675,13 +814,29 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantCacheEntries: 1, runTestsOnResultingAuthenticator: true, }, { - name: "updating jwt authenticator with the same value does not trigger sync loop and will not update conditions or phase", + name: "Sync: JWTAuthenticator with no change: loop will abort early and not update status conditions.", cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { cache.Store( authncache.Key{ @@ -712,11 +867,17 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, + wantActions: func() []coretesting.Action { + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + } + }, wantCacheEntries: 1, runTestsOnResultingAuthenticator: false, // skip the tests because the authenticator left in the cache is the mock version that was added above }, { - name: "updating jwt authenticator when cache value is wrong type will complete sync loop successfully with success conditions and ready phase", + name: "Sync: JWTAuthenticator update when cached authenticator is different type: loop will complete successfully and update status conditions.", cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) { cache.Store( authncache.Key{ @@ -752,13 +913,29 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantCacheEntries: 1, runTestsOnResultingAuthenticator: true, }, { - name: "TLS: valid jwt authenticator without CA will fail to cache the authenticator and will fail with failed and unknown conditions and Error phase and will enqueue a resync in case of machine error", + name: "Sync: valid JWTAuthenticator without CA: loop will fail to cache the authenticator, will write failed and unknown status conditions, and will enqueue resync", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -768,25 +945,41 @@ func TestController(t *testing.T) { Spec: *missingTLSJWTAuthenticatorSpec, }, }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *missingTLSJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: 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), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, // no explicit logs, this is an issue of config, the user must provide TLS config for the // custom cert provided for this server. - 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), - }, - ), - wantStatusPhase: "Error", wantSyncLoopErr: testutil.WantX509UntrustedCertErrorString(`could not perform oidc discovery on provider issuer: Get "`+goodIssuer+`/.well-known/openid-configuration": %s`, "Acme Co"), wantCacheEntries: 0, }, { - name: "validateTLS: invalid jwt authenticator CA 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", + name: "validateTLS: JWTAuthenticator with invalid CA: loop will fail, will write failed and unknown status conditions, but will not enqueue a resync due to user config error", syncKey: controllerlib.Key{Name: "test-name"}, jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ @@ -796,21 +989,37 @@ func TestController(t *testing.T) { Spec: *invalidTLSJWTAuthenticatorSpec, }, }, - wantStatusConditions: conditionstestutil.Replace( - allHappyConditionsSuccess(someOtherIssuer, frozenMetav1Now, 0), - []metav1.Condition{ - sadReadyCondition(frozenMetav1Now, 0), - sadTLSConfigurationValid(frozenMetav1Now, 0), - unknownDiscoveryURLValid(frozenMetav1Now, 0), - unknownAuthenticatorValid(frozenMetav1Now, 0), - unknownJWKSURLValid(frozenMetav1Now, 0), - unknownJWKSFetch(frozenMetav1Now, 0), - }, - ), - wantStatusPhase: "Error", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *invalidTLSJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(someOtherIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + sadReadyCondition(frozenMetav1Now, 0), + sadTLSConfigurationValid(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, }, { - name: "validateIssuer: parsing error (spec.issuer 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", + 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{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -820,20 +1029,36 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: 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), - }, - ), - wantStatusPhase: "Error", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *invalidIssuerJWTAuthenticatorSpec, + Status: auth1alpha1.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, + } + }, }, { - name: "validateIssuer: parsing error (spec.issuer URL has invalid scheme, requires https) 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", + name: "validateIssuer: parsing error (spec.issuer URL has invalid scheme, requires https): loop will fail sync, will write failed and unknown conditions, but will not enqueue a resync due to user config error", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -843,20 +1068,36 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: conditionstestutil.Replace( - allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - []metav1.Condition{ - sadReadyCondition(frozenMetav1Now, 0), - sadIssuerURLValidInvalidScheme("http://.café.com/café/café/café/coffee", frozenMetav1Now, 0), - unknownDiscoveryURLValid(frozenMetav1Now, 0), - unknownAuthenticatorValid(frozenMetav1Now, 0), - unknownJWKSURLValid(frozenMetav1Now, 0), - unknownJWKSFetch(frozenMetav1Now, 0), - }, - ), - wantStatusPhase: "Error", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *invalidIssuerSchemeJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + sadReadyCondition(frozenMetav1Now, 0), + sadIssuerURLValidInvalidScheme("http://.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, + } + }, }, { - name: "validateProviderDiscovery: could not perform oidc discovery on provider issuer will fail sync loop and will report failed and unknown conditions and Error phase and will enqueue new sync", + name: "validateProviderDiscovery: could not perform oidc discovery on provider issuer: loop will fail sync, will write failed and unknown conditions, and will enqueue new sync", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -866,19 +1107,35 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - wantStatusConditions: conditionstestutil.Replace( - allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - []metav1.Condition{ - happyIssuerURLValid(frozenMetav1Now, 0), - sadReadyCondition(frozenMetav1Now, 0), - sadDiscoveryURLValidConnectionRefused(goodIssuer+"/foo/bar/baz/shizzle", frozenMetav1Now, 0), - unknownAuthenticatorValid(frozenMetav1Now, 0), - unknownJWKSURLValid(frozenMetav1Now, 0), - unknownJWKSFetch(frozenMetav1Now, 0), - happyTLSConfigurationValidNoCA(frozenMetav1Now, 0), - }, - ), - wantStatusPhase: "Error", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *validIssuerURLButDoesNotExistJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + happyIssuerURLValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + sadDiscoveryURLValidConnectionRefused(goodIssuer+"/foo/bar/baz/shizzle", frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + unknownJWKSURLValid(frozenMetav1Now, 0), + unknownJWKSFetch(frozenMetav1Now, 0), + happyTLSConfigurationValidNoCA(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, 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. @@ -886,7 +1143,7 @@ func TestController(t *testing.T) { // which ensures the .Claims() parsing cannot fail (in the current impl) // { name: "validateProviderJWKSURL: could not get provider jwks_uri... ",}, { - name: "validateProviderJWKSURL: could not parse provider jwks_uri will fail sync loop and will report failed and unknown conditions and Error phase and will enqueue new sync", + name: "validateProviderJWKSURL: could not parse provider jwks_uri: loop will fail sync, will write failed and unknown conditions, and will enqueue new sync", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -896,20 +1153,36 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - 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", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *badIssuerJWKSURIJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: 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), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, 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': loop will fail sync, will write failed and unknown conditions, and will enqueue new sync", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -919,23 +1192,39 @@ func TestController(t *testing.T) { }, }, syncKey: controllerlib.Key{Name: "test-name"}, - 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", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *badIssuerJWKSURISchemeJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: 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), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, 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", + name: "validateJWKSFetch: could not fetch keys: loop will fail sync, will write failed and unknown status conditions, and will enqueue a resync", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -945,20 +1234,36 @@ func TestController(t *testing.T) { }, }, 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", + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *jwksFetchShouldFailJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + []metav1.Condition{ + happyIssuerURLValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + sadJWKSFetch(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, 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", + name: "updateStatus: called with matching original and updated conditions: will not make request to update conditions", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -982,12 +1287,16 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", - wantCacheEntries: 1, + wantActions: func() []coretesting.Action { + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + } + }, + wantCacheEntries: 1, }, { - name: "updateStatus: when updateStatus() is called with different original and updated conditions, it will update the conditions", + name: "updateStatus: called with different original and updated conditions: will make request to update conditions", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -1016,12 +1325,28 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - wantStatusConditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - wantStatusPhase: "Ready", - wantCacheEntries: 1, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, }, { - name: "updateStatus: when updateStatus() fails an error will enqueue a new sync", + name: "updateStatus: when update request fails: error will enqueue a resync", jwtAuthenticators: []runtime.Object{ &auth1alpha1.JWTAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -1049,6 +1374,27 @@ func TestController(t *testing.T) { }, ) }, + wantActions: func() []coretesting.Action { + // This captures that there was an attempt to update to Ready, allHappyConditions, + // but the wantSyncLoopErr indicates that there is a failure, so the JWTAuthenticator + // remains with a bad phase and at least 1 sad condition + updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: *someJWTAuthenticatorSpec, + Status: auth1alpha1.JWTAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, wantLogs: []map[string]any{{ "level": "info", "timestamp": "2099-08-08T13:57:36.123456Z", @@ -1059,14 +1405,6 @@ func TestController(t *testing.T) { "name": "test-name", }, }}, - // conditions and phase match previous state since update failed - wantStatusConditions: conditionstestutil.Replace( - allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0), - []metav1.Condition{ - sadReadyCondition(frozenMetav1Now, 0), - }, - ), - wantStatusPhase: "SomethingThatWontUpdate", wantSyncLoopErr: testutil.WantExactErrorString("some update error"), wantCacheEntries: 1, }, @@ -1143,14 +1481,11 @@ func TestController(t *testing.T) { } } - if tt.jwtAuthenticators != nil { - var jwtAuthSubject *auth1alpha1.JWTAuthenticator - getCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - 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, "status.phase should be correct") + if !assert.ElementsMatch(t, tt.wantActions(), pinnipedAPIClient.Actions()) { + // cmp.Diff is superior to require.ElementsMatch in terms of readability here. + // require.ElementsMatch will handle pointers better than require.Equal, but + // the timestamps are still incredibly verbose. + require.Fail(t, cmp.Diff(tt.wantActions(), pinnipedAPIClient.Actions()), "actions should be exactly the expected number of actions and also contain the correct resources") } 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())) diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index 90a910d995..f8a0271449 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.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 ldapupstreamwatcher implements a controller which watches LDAPIdentityProviders. diff --git a/test/integration/concierge_credentialrequest_test.go b/test/integration/concierge_credentialrequest_test.go index 544e2f0756..f05c93134c 100644 --- a/test/integration/concierge_credentialrequest_test.go +++ b/test/integration/concierge_credentialrequest_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 integration diff --git a/test/integration/concierge_jwtauthenticator_status_test.go b/test/integration/concierge_jwtauthenticator_status_test.go index 4776eaff1b..24927f67f3 100644 --- a/test/integration/concierge_jwtauthenticator_status_test.go +++ b/test/integration/concierge_jwtauthenticator_status_test.go @@ -18,8 +18,7 @@ import ( "go.pinniped.dev/test/testlib" ) -// Never run this test in parallel since deleting all federation domains is disruptive, see main_test.go. -func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { +func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { env := testlib.IntegrationEnv(t) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) t.Cleanup(cancel) @@ -31,28 +30,30 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { { name: "valid spec with no errors and all good status conditions and phase will result in a jwt authenticator that is ready", run: func(t *testing.T) { + caBundleString := base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)) jwtAuthenticator := testlib.CreateTestJWTAuthenticator(ctx, t, v1alpha1.JWTAuthenticatorSpec{ Issuer: env.SupervisorUpstreamOIDC.Issuer, Audience: "some-fake-audience", TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), + CertificateAuthorityData: caBundleString, }, }, v1alpha1.JWTAuthenticatorPhaseReady) testlib.WaitForJWTAuthenticatorStatusConditions( ctx, t, jwtAuthenticator.Name, - allSuccessfulJWTAuthenticatorConditions()) + allSuccessfulJWTAuthenticatorConditions(len(caBundleString) != 0)) }, }, { name: "valid spec with invalid CA in TLS config will result in a jwt authenticator that is not ready", run: func(t *testing.T) { + caBundleString := "invalid base64-encoded data" jwtAuthenticator := testlib.CreateTestJWTAuthenticator(ctx, t, v1alpha1.JWTAuthenticatorSpec{ Issuer: env.SupervisorUpstreamOIDC.Issuer, Audience: "some-fake-audience", TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: "invalid base64-encoded data", + CertificateAuthorityData: caBundleString, }, }, v1alpha1.JWTAuthenticatorPhaseError) @@ -60,7 +61,7 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { ctx, t, jwtAuthenticator.Name, replaceSomeConditions( - allSuccessfulJWTAuthenticatorConditions(), + allSuccessfulJWTAuthenticatorConditions(len(caBundleString) != 0), []metav1.Condition{ { Type: "Ready", @@ -100,6 +101,7 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { { name: "valid spec with valid CA in TLS config but does not match issuer server will result in a jwt authenticator that is not ready", run: func(t *testing.T) { + caBundleString := "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K" jwtAuthenticator := testlib.CreateTestJWTAuthenticator(ctx, t, v1alpha1.JWTAuthenticatorSpec{ Issuer: env.SupervisorUpstreamOIDC.Issuer, Audience: "some-fake-audience", @@ -107,7 +109,7 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { // Issuer: C=US, O=Pivotal // No SAN provided TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", + CertificateAuthorityData: caBundleString, }, }, v1alpha1.JWTAuthenticatorPhaseError) @@ -115,7 +117,7 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { ctx, t, jwtAuthenticator.Name, replaceSomeConditions( - allSuccessfulJWTAuthenticatorConditions(), + allSuccessfulJWTAuthenticatorConditions(len(caBundleString) != 0), []metav1.Condition{ { Type: "Ready", @@ -155,12 +157,13 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { { name: "invalid with bad issuer will result in a jwt authenticator that is not ready", run: func(t *testing.T) { + caBundleString := base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)) fakeIssuerURL := "https://127.0.0.1:443/some-fake-issuer" jwtAuthenticator := testlib.CreateTestJWTAuthenticator(ctx, t, v1alpha1.JWTAuthenticatorSpec{ Issuer: fakeIssuerURL, Audience: "some-fake-audience", TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), + CertificateAuthorityData: caBundleString, }, }, v1alpha1.JWTAuthenticatorPhaseError) @@ -168,7 +171,7 @@ func TestConciergeJWTAuthenticatorStatus_Disruptive(t *testing.T) { ctx, t, jwtAuthenticator.Name, replaceSomeConditions( - allSuccessfulJWTAuthenticatorConditions(), + allSuccessfulJWTAuthenticatorConditions(len(caBundleString) != 0), []metav1.Condition{ { Type: "Ready", @@ -347,12 +350,15 @@ func TestConciergeJWTAuthenticatorCRDValidations_Parallel(t *testing.T) { } else { require.NoError(t, createErr) } - }) } } -func allSuccessfulJWTAuthenticatorConditions() []metav1.Condition { +func allSuccessfulJWTAuthenticatorConditions(caBundleExists bool) []metav1.Condition { + tlsConfigValidMsg := "no CA bundle specified" + if caBundleExists { + tlsConfigValidMsg = "successfully parsed specified CA bundle" + } return []metav1.Condition{{ Type: "AuthenticatorValid", Status: "True", @@ -387,6 +393,6 @@ func allSuccessfulJWTAuthenticatorConditions() []metav1.Condition { Type: "TLSConfigurationValid", Status: "True", Reason: "Success", - Message: "successfully parsed specified CA bundle", + Message: tlsConfigValidMsg, }} } diff --git a/test/integration/kube_api_discovery_test.go b/test/integration/kube_api_discovery_test.go index fd21288232..f7756f3936 100644 --- a/test/integration/kube_api_discovery_test.go +++ b/test/integration/kube_api_discovery_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 integration