mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-01-05 04:56:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

jwtcachefiller controller loops over all jwtauthenticators

Browse Source 
Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>
This commit is contained in:
Ryan Richard
2024-07-24 16:31:01 -07:00
parent ca2dd2d476
commit 06b47a5792
4 changed files with 404 additions and 204 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go
View File

@@ -14,6 +14,7 @@ import (
"net/http"
"net/url"
"reflect"
"slices"
"strings"
"time"
@@ -21,8 +22,8 @@ import (
"github.com/go-jose/go-jose/v4"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apiserver/pkg/apis/apiserver"
"k8s.io/apiserver/pkg/authentication/authenticator"
@@ -154,7 +155,7 @@ func New(
},
withInformer(
jwtAuthenticators,
pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct
pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()),
controllerlib.InformerOption{},
),
withInformer(
@@ -165,7 +166,7 @@ func New(
corev1.SecretTypeTLS,
},
pinnipedcontroller.SingletonQueue(),
), // nil parent func is fine because each event is distinct
),
controllerlib.InformerOption{},
),
withInformer(
@@ -189,24 +190,40 @@ type jwtCacheFillerController struct {
// Sync implements controllerlib.Syncer.
func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
obj, err := c.jwtAuthenticators.Lister().Get(ctx.Key.Name)
if err != nil && apierrors.IsNotFound(err) {
c.log.Info("Sync() found that the JWTAuthenticator does not exist yet or was deleted")
return nil
}
jwtAuthenticators, err := c.jwtAuthenticators.Lister().List(labels.Everything())
if err != nil {
// no unit test for this failure
return fmt.Errorf("failed to get JWTAuthenticator %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err)
return err
}
if len(jwtAuthenticators) == 0 {
c.log.Info("No JWTAuthenticators found")
return nil
}
// Sort them by name so that order is predictable and therefore output is consistent for tests and logs.
slices.SortStableFunc(jwtAuthenticators, func(a, b *authenticationv1alpha1.JWTAuthenticator) int {
return strings.Compare(a.Name, b.Name)
})
var errs []error
for _, jwtAuthenticator := range jwtAuthenticators {
err = c.syncIndividualJWTAuthenticator(ctx.Context, jwtAuthenticator)
if err != nil {
errs = append(errs, fmt.Errorf("error for JWTAuthenticator %s: %w", jwtAuthenticator.Name, err))
}
}
return utilerrors.NewAggregate(errs)
}
func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Context, jwtAuthenticator *authenticationv1alpha1.JWTAuthenticator) error {
cacheKey := authncache.Key{
APIGroup: authenticationv1alpha1.GroupName,
Kind: "JWTAuthenticator",
Name: ctx.Key.Name,
Name: jwtAuthenticator.Name,
}
conditions := make([]*metav1.Condition, 0)
certPool, caBundlePEM, conditions, tlsBundleOk := c.validateTLSBundle(obj.Spec.TLS, conditions)
certPool, caBundlePEM, conditions, tlsBundleOk := c.validateTLSBundle(jwtAuthenticator.Spec.TLS, conditions)
caBundlePEMSHA256 := sha256.Sum256(caBundlePEM) // note that this will always return the same hash for nil input
// Only revalidate and update the cache if the cached authenticator is different from the desired authenticator.
@@ -221,10 +238,10 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
if valueFromCache := c.cache.Get(cacheKey); valueFromCache != nil {
jwtAuthenticatorFromCache = c.cacheValueAsJWTAuthenticator(valueFromCache)
if jwtAuthenticatorFromCache != nil &&
reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &obj.Spec) &&
reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &jwtAuthenticator.Spec) &&
tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status
jwtAuthenticatorFromCache.caBundlePEMSHA256 == caBundlePEMSHA256 {
c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer).
c.log.WithValues("jwtAuthenticator", klog.KObj(jwtAuthenticator), "issuer", jwtAuthenticator.Spec.Issuer).
Info("actual jwt authenticator and desired jwt authenticator are the same")
// Stop, no more work to be done. This authenticator is already validated and cached.
return nil
@@ -232,14 +249,14 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
}
var errs []error
_, conditions, issuerOk := c.validateIssuer(obj.Spec.Issuer, conditions)
_, conditions, issuerOk := c.validateIssuer(jwtAuthenticator.Spec.Issuer, conditions)
okSoFar := tlsBundleOk && issuerOk
client := phttp.Default(certPool)
client.Timeout = 30 * time.Second // copied from Kube OIDC code
coreOSCtx := coreosoidc.ClientContext(context.Background(), client)
pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, obj.Spec.Issuer, conditions, okSoFar)
pJSON, provider, conditions, providerErr := c.validateProviderDiscovery(coreOSCtx, jwtAuthenticator.Spec.Issuer, conditions, okSoFar)
errs = append(errs, providerErr)
okSoFar = okSoFar && providerErr == nil
@@ -253,7 +270,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
newJWTAuthenticatorForCache, conditions, err := c.newCachedJWTAuthenticator(
client,
obj.Spec.DeepCopy(), // deep copy to avoid caching original object
jwtAuthenticator.Spec.DeepCopy(), // deep copy to avoid caching original object
keySet,
caBundlePEMSHA256,
conditions,
@@ -267,7 +284,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
c.cache.Delete(cacheKey)
} else {
c.cache.Store(cacheKey, newJWTAuthenticatorForCache)
c.log.WithValues("jwtAuthenticator", klog.KObj(obj), "issuer", obj.Spec.Issuer).
c.log.WithValues("jwtAuthenticator", klog.KObj(jwtAuthenticator), "issuer", jwtAuthenticator.Spec.Issuer).
Info("added new jwt authenticator")
}
@@ -276,7 +293,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
// removed from the cache, because we do not want any end-user authentications to use a closed authenticator.
jwtAuthenticatorFromCache.Close()
err = c.updateStatus(ctx.Context, obj, conditions)
err = c.updateStatus(ctx, jwtAuthenticator, conditions)
errs = append(errs, err)
// Sync loop errors:

450
internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go
View File

@@ -703,7 +703,6 @@ func TestController(t *testing.T) {
tests := []struct {
name string
cache func(*testing.T, *authncache.Cache, bool)
syncKey controllerlib.Key
jwtAuthenticators []runtime.Object
secretsAndConfigMaps []runtime.Object
// for modifying the clients to hack in arbitrary api responses
@@ -713,23 +712,22 @@ func TestController(t *testing.T) {
// Errors such as url.Parse of the issuer are not returned as they imply a user error.
// Since these errors trigger a resync, we are careful only to return an error when
// something can be automatically corrected on a retry (ie an error that might be networking).
wantSyncLoopErr testutil.RequireErrorStringFunc
wantLogs []map[string]any
wantActions func() []coretesting.Action
wantCacheEntries int
wantUsernameClaim string
wantGroupsClaim string
runTestsOnResultingAuthenticator bool
wantSyncLoopErr testutil.RequireErrorStringFunc
wantLogs []map[string]any
wantActions func() []coretesting.Action
wantUsernameClaim string
wantGroupsClaim string
wantNamesOfJWTAuthenticatorsInCache []string
skipTestingCachedAuthenticator bool
}{
{
name: "Sync: JWTAuthenticator not found will abort sync loop, no status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: no JWTAuthenticators found results in no errors and no status conditions",
wantLogs: []map[string]any{
{
"level": "info",
"timestamp": "2099-08-08T13:57:36.123456Z",
"logger": "jwtcachefiller-controller",
"message": "Sync() found that the JWTAuthenticator does not exist yet or was deleted",
"message": "No JWTAuthenticators found",
},
},
wantActions: func() []coretesting.Action {
@@ -740,8 +738,7 @@ func TestController(t *testing.T) {
},
},
{
name: "Sync: valid and unchanged JWTAuthenticator: loop will preserve existing status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: valid and unchanged JWTAuthenticator: loop will preserve existing status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -770,11 +767,136 @@ func TestController(t *testing.T) {
coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}),
}
},
wantCacheEntries: 1,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: changed JWTAuthenticator: loop will update timestamps only on relevant statuses",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: multiple valid and multiple invalid JWTAuthenticators",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "existing-jwt-authenticator",
},
Spec: *someJWTAuthenticatorSpec,
Status: authenticationv1alpha1.JWTAuthenticatorStatus{
Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
Phase: "Ready",
},
},
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "new-jwt-authenticator",
},
Spec: *someJWTAuthenticatorSpec,
},
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "invalid-jwt-authenticator",
},
Spec: *badIssuerJWKSURIJWTAuthenticatorSpec,
},
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "another-invalid-jwt-authenticator",
},
Spec: *badIssuerJWKSURIJWTAuthenticatorSpec,
},
},
wantSyncLoopErr: testutil.WantExactErrorString("[" +
`error for JWTAuthenticator another-invalid-jwt-authenticator: could not parse provider jwks_uri: parse "https://.café .com/café/café/café/coffee/jwks.json": invalid character " " in host name` +
", " +
`error for JWTAuthenticator invalid-jwt-authenticator: could not parse provider jwks_uri: parse "https://.café .com/café/café/café/coffee/jwks.json": invalid character " " in host name` +
"]",
),
wantLogs: []map[string]any{
{
"level": "info",
"timestamp": "2099-08-08T13:57:36.123456Z",
"logger": "jwtcachefiller-controller",
"message": "added new jwt authenticator",
"issuer": goodIssuer,
"jwtAuthenticator": map[string]any{
"name": "existing-jwt-authenticator",
},
},
{
"level": "info",
"timestamp": "2099-08-08T13:57:36.123456Z",
"logger": "jwtcachefiller-controller",
"message": "added new jwt authenticator",
"issuer": goodIssuer,
"jwtAuthenticator": map[string]any{
"name": "new-jwt-authenticator",
},
},
},
wantActions: func() []coretesting.Action {
updateValidStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "new-jwt-authenticator",
},
Spec: *someJWTAuthenticatorSpec,
Status: authenticationv1alpha1.JWTAuthenticatorStatus{
Conditions: allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
Phase: "Ready",
},
})
updateValidStatusAction.Subresource = "status"
updateInvalidStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "invalid-jwt-authenticator",
},
Spec: *badIssuerJWKSURIJWTAuthenticatorSpec,
Status: authenticationv1alpha1.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",
},
})
updateInvalidStatusAction.Subresource = "status"
updateValidStatusAction.Subresource = "status"
updateAnotherInvalidStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
Name: "another-invalid-jwt-authenticator",
},
Spec: *badIssuerJWKSURIJWTAuthenticatorSpec,
Status: authenticationv1alpha1.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",
},
})
updateAnotherInvalidStatusAction.Subresource = "status"
return []coretesting.Action{
coretesting.NewListAction(jwtAuthenticatorsGVR, jwtAUthenticatorGVK, "", metav1.ListOptions{}),
coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}),
updateAnotherInvalidStatusAction,
updateInvalidStatusAction,
updateValidStatusAction,
}
},
wantNamesOfJWTAuthenticatorsInCache: []string{
"existing-jwt-authenticator",
"new-jwt-authenticator",
},
},
{
name: "Sync: changed JWTAuthenticator: loop will update timestamps only on relevant statuses",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -835,11 +957,10 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: valid JWTAuthenticator with CA: loop will complete successfully and update status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: valid JWTAuthenticator with CA: loop will complete successfully and update status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -876,12 +997,10 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: valid JWTAuthenticator with CA from Secret: loop will complete successfully and update status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: valid JWTAuthenticator with CA from Secret: loop will complete successfully and update status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -921,12 +1040,10 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: valid JWTAuthenticator with CA from ConfigMap: loop will complete successfully and update status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: valid JWTAuthenticator with CA from ConfigMap: loop will complete successfully and update status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -966,12 +1083,10 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: JWTAuthenticator with custom username claim: loop will complete successfully and update status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: JWTAuthenticator with custom username claim: loop will complete successfully and update status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1008,13 +1123,11 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
wantUsernameClaim: someJWTAuthenticatorSpecWithUsernameClaim.Claims.Username,
runTestsOnResultingAuthenticator: true,
wantUsernameClaim: someJWTAuthenticatorSpecWithUsernameClaim.Claims.Username,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: JWTAuthenticator with custom groups claim: loop will complete successfully and update status conditions",
syncKey: controllerlib.Key{Name: "test-name"},
name: "Sync: JWTAuthenticator with custom groups claim: loop will complete successfully and update status conditions",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1051,9 +1164,8 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
wantGroupsClaim: someJWTAuthenticatorSpecWithGroupsClaim.Claims.Groups,
runTestsOnResultingAuthenticator: true,
wantGroupsClaim: someJWTAuthenticatorSpecWithGroupsClaim.Claims.Groups,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: JWTAuthenticator with new spec fields: loop will close previous instance of JWTAuthenticator and complete successfully and update status conditions",
@@ -1070,7 +1182,6 @@ func TestController(t *testing.T) {
)
},
wantClose: true,
syncKey: controllerlib.Key{Name: "test-name"},
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1107,8 +1218,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: JWTAuthenticator with external and changed CA bundle: loop will close previous instance of JWTAuthenticator and complete successfully and update status conditions",
@@ -1123,7 +1233,6 @@ func TestController(t *testing.T) {
)
},
wantClose: true,
syncKey: controllerlib.Key{Name: "test-name"},
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1163,8 +1272,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "Sync: JWTAuthenticator with no change: loop will abort early and not update status conditions",
@@ -1181,7 +1289,6 @@ func TestController(t *testing.T) {
)
},
wantClose: false,
syncKey: controllerlib.Key{Name: "test-name"},
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1206,8 +1313,9 @@ func TestController(t *testing.T) {
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
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
// skip the tests because the authenticator pre-loaded into the cache is the mock version that was added above
skipTestingCachedAuthenticator: true,
},
{
name: "Sync: authenticator update when cached authenticator is the wrong data type, which should never really happen: loop will complete successfully and update status conditions",
@@ -1224,7 +1332,6 @@ func TestController(t *testing.T) {
struct{ authenticator.Token }{},
)
},
syncKey: controllerlib.Key{Name: "test-name"},
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1270,12 +1377,10 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
runTestsOnResultingAuthenticator: true,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
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"},
name: "Sync: valid JWTAuthenticator without CA: loop will fail to cache the authenticator, will write failed and unknown status conditions, and will enqueue resync",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1314,12 +1419,10 @@ 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.
wantSyncLoopErr: testutil.WantSprintfErrorString(`could not perform oidc discovery on provider issuer: Get "%s/.well-known/openid-configuration": tls: failed to verify certificate: x509: certificate signed by unknown authority`, goodIssuer),
wantCacheEntries: 0,
wantSyncLoopErr: testutil.WantSprintfErrorString(`error for JWTAuthenticator test-name: could not perform oidc discovery on provider issuer: Get "%s/.well-known/openid-configuration": tls: failed to verify certificate: x509: certificate signed by unknown authority`, goodIssuer),
},
{
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"},
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",
jwtAuthenticators: []runtime.Object{
&authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1356,11 +1459,9 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 0,
},
{
name: "previously valid cached authenticator (which did not specify a CA bundle) changes and becomes invalid due to any problem with the CA bundle: loop will fail sync, will write failed and unknown status conditions, and will remove authenticator from cache",
syncKey: controllerlib.Key{Name: "test-name"},
name: "previously valid cached authenticator (which did not specify a CA bundle) changes and becomes invalid due to any problem with the CA bundle: loop will fail sync, will write failed and unknown status conditions, and will remove authenticator from cache",
cache: func(t *testing.T, cache *authncache.Cache, wantClose bool) {
cache.Store(
authncache.Key{
@@ -1411,8 +1512,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantClose: true,
wantCacheEntries: 0,
wantClose: true,
},
{
name: "previously valid cached authenticator's spec changes and becomes invalid for any other reason (this test uses an invalid spec.issuer URL): loop will fail sync, will write failed and unknown status conditions, and will remove authenticator from cache",
@@ -1436,7 +1536,6 @@ func TestController(t *testing.T) {
Spec: *invalidIssuerJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1465,8 +1564,8 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 0, // removed from cache
wantClose: true, // the removed cache entry was also closed
wantNamesOfJWTAuthenticatorsInCache: []string{}, // it was removed from the cache
wantClose: true, // the removed cache entry was also closed
},
{
name: "validateIssuer: parsing error (spec.issuer URL is invalid): loop will fail sync, will write failed and unknown status conditions, but will not enqueue a resync due to user config error",
@@ -1478,7 +1577,6 @@ func TestController(t *testing.T) {
Spec: *invalidIssuerJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1518,7 +1616,6 @@ func TestController(t *testing.T) {
Spec: *invalidIssuerSchemeJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1562,7 +1659,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1610,7 +1706,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1658,7 +1753,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1702,7 +1796,6 @@ func TestController(t *testing.T) {
Spec: *validIssuerURLButDoesNotExistJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1732,7 +1825,7 @@ func TestController(t *testing.T) {
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`),
wantSyncLoopErr: testutil.WantExactErrorString(`error for JWTAuthenticator test-name: 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`),
},
{
name: "validateProviderDiscovery: excessively long errors truncated: loop will fail sync, will write failed and unknown conditions, and will enqueue new sync",
@@ -1748,7 +1841,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1783,7 +1875,7 @@ func TestController(t *testing.T) {
}
},
// not currently truncating the logged err
wantSyncLoopErr: testutil.WantExactErrorString("could not perform oidc discovery on provider issuer: 404 Not Found: <html>\n\t\t \t<head><title>404 not found page</title></head>\n\t\t\t<body>lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz should not reach end of string</body>\n\t\t</html>"),
wantSyncLoopErr: testutil.WantExactErrorString("error for JWTAuthenticator test-name: could not perform oidc discovery on provider issuer: 404 Not Found: <html>\n\t\t \t<head><title>404 not found page</title></head>\n\t\t\t<body>lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz lots of text that is at least 300 characters long 0123456789 abcdefghijklmnopqrstuvwxyz should not reach end of string</body>\n\t\t</html>"),
},
// cannot be tested currently the way the coreos lib works.
// the constructor requires an issuer in the payload and validates the issuer matches the actual issuer,
@@ -1799,7 +1891,6 @@ func TestController(t *testing.T) {
Spec: *badIssuerJWKSURIJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1827,7 +1918,7 @@ func TestController(t *testing.T) {
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`),
wantSyncLoopErr: testutil.WantExactErrorString(`error for JWTAuthenticator test-name: 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': loop will fail sync, will write failed and unknown conditions, and will enqueue new sync",
@@ -1839,7 +1930,6 @@ func TestController(t *testing.T) {
Spec: *badIssuerJWKSURISchemeJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1867,7 +1957,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantSyncLoopErr: testutil.WantExactErrorString("jwks_uri http://.café.com/café/café/café/coffee/jwks.json has invalid scheme, require 'https'"),
wantSyncLoopErr: testutil.WantExactErrorString("error for JWTAuthenticator test-name: jwks_uri http://.café.com/café/café/café/coffee/jwks.json has invalid scheme, require 'https'"),
},
{
name: "validateProviderJWKSURL: remote jwks should not have been able to verify hardcoded test jwt token: loop will fail sync, will write failed and unknown conditions, and will enqueue new sync",
@@ -1879,7 +1969,6 @@ func TestController(t *testing.T) {
Spec: *badIssuerUsesOurHardcodedPrivateKeyJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1908,7 +1997,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantSyncLoopErr: testutil.WantExactErrorString("remote jwks should not have been able to verify hardcoded test jwt token"),
wantSyncLoopErr: testutil.WantExactErrorString("error for JWTAuthenticator test-name: remote jwks should not have been able to verify hardcoded test jwt token"),
},
{
name: "validateJWKSFetch: could not fetch keys: loop will fail sync, will write failed and unknown status conditions, and will enqueue a resync",
@@ -1920,7 +2009,6 @@ func TestController(t *testing.T) {
Spec: *jwksFetchShouldFailJWTAuthenticatorSpec,
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantActions: func() []coretesting.Action {
updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &authenticationv1alpha1.JWTAuthenticator{
ObjectMeta: metav1.ObjectMeta{
@@ -1948,7 +2036,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantSyncLoopErr: testutil.WantExactErrorString("could not fetch keys: fetching keys oidc: get keys failed: 404 Not Found 404 page not found\n"),
wantSyncLoopErr: testutil.WantExactErrorString("error for JWTAuthenticator test-name: could not fetch keys: fetching keys oidc: get keys failed: 404 Not Found 404 page not found\n"),
},
{
name: "updateStatus: called with matching original and updated conditions: will not make request to update conditions",
@@ -1964,7 +2052,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantLogs: []map[string]any{{
"level": "info",
"timestamp": "2099-08-08T13:57:36.123456Z",
@@ -1981,7 +2068,7 @@ func TestController(t *testing.T) {
coretesting.NewWatchAction(jwtAuthenticatorsGVR, "", metav1.ListOptions{}),
}
},
wantCacheEntries: 1,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "updateStatus: called with different original and updated conditions: will make request to update conditions",
@@ -2002,7 +2089,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
wantLogs: []map[string]any{{
"level": "info",
"timestamp": "2099-08-08T13:57:36.123456Z",
@@ -2031,7 +2117,7 @@ func TestController(t *testing.T) {
updateStatusAction,
}
},
wantCacheEntries: 1,
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
{
name: "updateStatus: when update request fails: error will enqueue a resync",
@@ -2052,7 +2138,6 @@ func TestController(t *testing.T) {
},
},
},
syncKey: controllerlib.Key{Name: "test-name"},
configClient: func(client *conciergefake.Clientset) {
client.PrependReactor(
"update",
@@ -2093,8 +2178,8 @@ func TestController(t *testing.T) {
"name": "test-name",
},
}},
wantSyncLoopErr: testutil.WantExactErrorString("some update error"),
wantCacheEntries: 1,
wantSyncLoopErr: testutil.WantExactErrorString("error for JWTAuthenticator test-name: some update error"),
wantNamesOfJWTAuthenticatorsInCache: []string{"test-name"},
},
// cannot be tested the way we are invoking oidc.New as we don't provide enough configuration
// knobs to actually invoke the code in a broken way. We always give a good client, good keys, and
@@ -2140,7 +2225,7 @@ func TestController(t *testing.T) {
kubeInformers.Start(ctx.Done())
controllerlib.TestRunSynchronously(t, controller)
syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey}
syncCtx := controllerlib.Context{Context: ctx}
if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantSyncLoopErr != nil {
testutil.RequireErrorStringFromErr(t, err, tt.wantSyncLoopErr)
@@ -2155,7 +2240,7 @@ func TestController(t *testing.T) {
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()))
require.Equal(t, len(tt.wantNamesOfJWTAuthenticatorsInCache), len(cache.Keys()), fmt.Sprintf("expected cache entries is incorrect. wanted:%d, got: %d, keys: %v", len(tt.wantNamesOfJWTAuthenticatorsInCache), len(cache.Keys()), cache.Keys()))
actualLogLines := testutil.SplitByNewline(log.String())
require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct")
@@ -2183,105 +2268,112 @@ func TestController(t *testing.T) {
}
}
if !tt.runTestsOnResultingAuthenticator {
return // end of test unless we wanted to run tests on the resulting authenticator from the cache
}
for _, name := range tt.wantNamesOfJWTAuthenticatorsInCache {
// We expected the cache to have an entry, so pull that entry from the cache and test it.
expectedCacheKey := authncache.Key{
APIGroup: authenticationv1alpha1.GroupName,
Kind: "JWTAuthenticator",
Name: name,
}
temp := cache.Get(expectedCacheKey)
require.NotNil(t, temp)
require.IsType(t, &cachedJWTAuthenticator{}, temp)
// We expected the cache to have an entry, so pull that entry from the cache and test it.
expectedCacheKey := authncache.Key{
APIGroup: authenticationv1alpha1.GroupName,
Kind: "JWTAuthenticator",
Name: syncCtx.Key.Name,
}
cachedAuthenticator, ok := cache.Get(expectedCacheKey).(tokenAuthenticatorCloser)
require.True(t, ok)
require.NotNil(t, cachedAuthenticator)
if tt.skipTestingCachedAuthenticator {
continue // skip the rest of this test for this authenticator
}
// Schedule it to be closed at the end of the test.
t.Cleanup(cachedAuthenticator.Close)
require.NotNil(t, temp.(*cachedJWTAuthenticator).Token)
cachedAuthenticator, ok := temp.(tokenAuthenticatorCloser)
require.True(t, ok)
const (
goodSubject = "some-subject"
group0 = "some-group-0"
group1 = "some-group-1"
goodUsername = "pinny123"
)
// Schedule it to be closed at the end of the test.
t.Cleanup(cachedAuthenticator.Close)
if tt.wantUsernameClaim == "" {
tt.wantUsernameClaim = "username"
}
const (
goodSubject = "some-subject"
group0 = "some-group-0"
group1 = "some-group-1"
goodUsername = "pinny123"
)
if tt.wantGroupsClaim == "" {
tt.wantGroupsClaim = "groups"
}
if tt.wantUsernameClaim == "" {
tt.wantUsernameClaim = "username"
}
for _, test := range testTableForAuthenticateTokenTests(
t,
goodRSASigningKey,
goodRSASigningAlgo,
goodRSASigningKeyID,
group0,
group1,
goodUsername,
tt.wantUsernameClaim,
tt.wantGroupsClaim,
goodIssuer,
) {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
if tt.wantGroupsClaim == "" {
tt.wantGroupsClaim = "groups"
}
wellKnownClaims := josejwt.Claims{
Issuer: goodIssuer,
Subject: goodSubject,
Audience: []string{goodAudience},
Expiry: josejwt.NewNumericDate(time.Now().Add(time.Hour)),
NotBefore: josejwt.NewNumericDate(time.Now().Add(-time.Hour)),
IssuedAt: josejwt.NewNumericDate(time.Now().Add(-time.Hour)),
}
var groups any
username := goodUsername
if test.jwtClaims != nil {
test.jwtClaims(&wellKnownClaims, &groups, &username)
}
for _, test := range testTableForAuthenticateTokenTests(
t,
goodRSASigningKey,
goodRSASigningAlgo,
goodRSASigningKeyID,
group0,
group1,
goodUsername,
tt.wantUsernameClaim,
tt.wantGroupsClaim,
goodIssuer,
) {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
var signingKey any = goodECSigningKey
signingAlgo := goodECSigningAlgo
signingKID := goodECSigningKeyID
if test.jwtSignature != nil {
test.jwtSignature(&signingKey, &signingAlgo, &signingKID)
}
wellKnownClaims := josejwt.Claims{
Issuer: goodIssuer,
Subject: goodSubject,
Audience: []string{goodAudience},
Expiry: josejwt.NewNumericDate(time.Now().Add(time.Hour)),
NotBefore: josejwt.NewNumericDate(time.Now().Add(-time.Hour)),
IssuedAt: josejwt.NewNumericDate(time.Now().Add(-time.Hour)),
}
var groups any
username := goodUsername
if test.jwtClaims != nil {
test.jwtClaims(&wellKnownClaims, &groups, &username)
}
jwt := createJWT(
t,
signingKey,
signingAlgo,
signingKID,
&wellKnownClaims,
tt.wantGroupsClaim,
groups,
test.distributedGroupsClaimURL,
tt.wantUsernameClaim,
username,
)
var signingKey any = goodECSigningKey
signingAlgo := goodECSigningAlgo
signingKID := goodECSigningKeyID
if test.jwtSignature != nil {
test.jwtSignature(&signingKey, &signingAlgo, &signingKID)
}
// Loop for a while here to allow the underlying OIDC authenticator to initialize itself asynchronously.
var (
rsp *authenticator.Response
authenticated bool
err error
)
_ = wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) {
rsp, authenticated, err = cachedAuthenticator.AuthenticateToken(context.Background(), jwt)
return !isNotInitialized(err), nil
jwt := createJWT(
t,
signingKey,
signingAlgo,
signingKID,
&wellKnownClaims,
tt.wantGroupsClaim,
groups,
test.distributedGroupsClaimURL,
tt.wantUsernameClaim,
username,
)
// Loop for a while here to allow the underlying OIDC authenticator to initialize itself asynchronously.
var (
rsp *authenticator.Response
authenticated bool
err error
)
_ = wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (bool, error) {
require.NotEmpty(t, cachedAuthenticator)
rsp, authenticated, err = cachedAuthenticator.AuthenticateToken(context.Background(), jwt)
return !isNotInitialized(err), nil
})
if test.wantErr != nil {
testutil.RequireErrorStringFromErr(t, err, test.wantErr)
} else {
require.NoError(t, err)
require.Equal(t, test.wantResponse, rsp)
require.Equal(t, test.wantAuthenticated, authenticated)
}
})
if test.wantErr != nil {
testutil.RequireErrorStringFromErr(t, err, test.wantErr)
} else {
require.NoError(t, err)
require.Equal(t, test.wantResponse, rsp)
require.Equal(t, test.wantAuthenticated, authenticated)
}
})
}
}
})
}

7
internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go
View File

@@ -92,7 +92,7 @@ func New(
},
withInformer(
webhooks,
pinnipedcontroller.MatchAnythingFilter(nil), // nil parent func is fine because each event is distinct
pinnipedcontroller.MatchAnythingFilter(nil), // TODO: use pinnipedcontroller.SingletonQueue()
controllerlib.InformerOption{},
),
withInformer(
@@ -103,7 +103,7 @@ func New(
corev1.SecretTypeTLS,
},
pinnipedcontroller.SingletonQueue(),
), // nil parent func is fine because each event is distinct
),
controllerlib.InformerOption{},
),
withInformer(
@@ -127,6 +127,9 @@ type webhookCacheFillerController struct {
// Sync implements controllerlib.Syncer.
func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error {
// TODO: can ctx.Key.Name be the name of a Secret or ConfigMap??????
// Because the withInformer function calls above for secrets and configmaps use SingletonQueue(), the key will be empty for secrets and configmaps
// Every Sync should loop over all webhookAuthenticators because any could have a CA bundle that was indirectly changed.
obj, err := c.webhooks.Lister().Get(ctx.Key.Name)
if err != nil && apierrors.IsNotFound(err) {
c.log.Info("Sync() found that the WebhookAuthenticator does not exist yet or was deleted")