internal/groupsuffix: mutate TokenCredentialRequest's Authenticator

This is a partial revert of 288d9c999e. For some reason it didn't occur to me
that we could do it this way earlier. Whoops.

This also contains a middleware update: mutation funcs can return an error now
and short-circuit the rest of the request/response flow. The idea here is that
if someone is configuring their kubeclient to use middleware, they are agreeing
to a narrow-er client contract by doing so (e.g., their TokenCredentialRequest's
must have an Spec.Authenticator.APIGroup set).

I also updated some internal/groupsuffix tests to be more realistic.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

internal/groupsuffix/groupsuffix.go

@@ -5,6 +5,7 @@ package groupsuffix
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -12,8 +13,10 @@ import (
 	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation"
 
+	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/constable"
 	"go.pinniped.dev/internal/kubeclient"
+	"go.pinniped.dev/internal/plog"
 )
 
 const (
@@ -35,7 +38,7 @@ func New(apiGroupSuffix string) kubeclient.Middleware {
 				return // ignore APIs that do not have our group
 			}
 
-			rt.MutateRequest(func(obj kubeclient.Object) {
+			rt.MutateRequest(func(obj kubeclient.Object) error {
 				typeMeta := obj.GetObjectKind()
 				origGVK := typeMeta.GroupVersionKind()
 				newGVK := schema.GroupVersionKind{
@@ -44,6 +47,7 @@ func New(apiGroupSuffix string) kubeclient.Middleware {
 					Kind:    origGVK.Kind,
 				}
 				typeMeta.SetGroupVersionKind(newGVK)
+				return nil
 			})
 		}),
 
@@ -61,6 +65,49 @@ func New(apiGroupSuffix string) kubeclient.Middleware {
 			rt.MutateRequest(mutateOwnerRefs(Replace, apiGroupSuffix))
 		}),
 
+		kubeclient.MiddlewareFunc(func(_ context.Context, rt kubeclient.RoundTrip) {
+			// we only care if this is a create on a TokenCredentialRequest without a subresource
+			if rt.Resource() != loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests") ||
+				rt.Verb() != kubeclient.VerbCreate ||
+				rt.Subresource() != "" {
+				return
+			}
+
+			// we only do this on the way out, since on the way back in we don't set a spec in our
+			// TokenCredentialRequest
+			rt.MutateRequest(func(obj kubeclient.Object) error {
+				tokenCredentialRequest, ok := obj.(*loginv1alpha1.TokenCredentialRequest)
+				if !ok {
+					return fmt.Errorf("cannot cast obj of type %T to *loginv1alpha1.TokenCredentialRequest", obj)
+				}
+
+				if tokenCredentialRequest.Spec.Authenticator.APIGroup == nil {
+					// technically, the APIGroup field is optional, so clients are free to do this, but we
+					// want our middleware to be opinionated so that it can be really good at a specific task
+					// and give us specific feedback when it can't do that specific task
+					return fmt.Errorf(
+						"cannot replace token credential request %q without authenticator API group",
+						plog.KObj(obj),
+					)
+				}
+
+				mutatedAuthenticatorAPIGroup, ok := Replace(*tokenCredentialRequest.Spec.Authenticator.APIGroup, apiGroupSuffix)
+				if !ok {
+					// see comment above about specificity of middleware
+					return fmt.Errorf(
+						"cannot replace token credential request %q authenticator API group %q with group suffix %q",
+						plog.KObj(obj),
+						*tokenCredentialRequest.Spec.Authenticator.APIGroup,
+						apiGroupSuffix,
+					)
+				}
+
+				tokenCredentialRequest.Spec.Authenticator.APIGroup = &mutatedAuthenticatorAPIGroup
+
+				return nil
+			})
+		}),
+
 		kubeclient.MiddlewareFunc(func(_ context.Context, rt kubeclient.RoundTrip) {
 			// always unreplace owner refs with apiGroupSuffix because we can consume those objects across all verbs
 			rt.MutateResponse(mutateOwnerRefs(Unreplace, apiGroupSuffix))
@@ -68,12 +115,12 @@ func New(apiGroupSuffix string) kubeclient.Middleware {
 	}
 }
 
-func mutateOwnerRefs(replaceFunc func(baseAPIGroup, apiGroupSuffix string) (string, bool), apiGroupSuffix string) func(kubeclient.Object) {
+func mutateOwnerRefs(replaceFunc func(baseAPIGroup, apiGroupSuffix string) (string, bool), apiGroupSuffix string) func(kubeclient.Object) error {
-	return func(obj kubeclient.Object) {
+	return func(obj kubeclient.Object) error {
 		// fix up owner refs because they are consumed by external and internal actors
 		oldRefs := obj.GetOwnerReferences()
 		if len(oldRefs) == 0 {
-			return
+			return nil
 		}
 
 		var changedGroup bool
@@ -94,10 +141,12 @@ func mutateOwnerRefs(replaceFunc func(baseAPIGroup, apiGroupSuffix string) (stri
 		}
 
 		if !changedGroup {
-			return
+			return nil
 		}
 
 		obj.SetOwnerReferences(newRefs)
+
+		return nil
 	}
 }
 

internal/groupsuffix/groupsuffix_test.go

@@ -14,6 +14,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	authv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/authentication/v1alpha1"
 	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	configv1alpha1 "go.pinniped.dev/generated/1.20/apis/supervisor/config/v1alpha1"
 	"go.pinniped.dev/internal/kubeclient"
@@ -31,6 +32,7 @@ func ExampleReplace_string() {
 	fmt.Println(s)
 	// Output: idp.supervisor.marlin.io
 }
+
 func TestMiddlware(t *testing.T) {
 	const newSuffix = "some.suffix.com"
 
@@ -117,14 +119,14 @@ func TestMiddlware(t *testing.T) {
 			},
 		},
 	}
-	federationDomainWithPinnipedOwnerWithNewGroup := &configv1alpha1.FederationDomain{
+	federationDomainWithNewGroupAndPinnipedOwner := &configv1alpha1.FederationDomain{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: configv1alpha1.SchemeGroupVersion.String(),
+			APIVersion: replaceGV(t, configv1alpha1.SchemeGroupVersion, newSuffix).String(),
 			Kind:       "FederationDomain",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			OwnerReferences: []metav1.OwnerReference{
-				*metav1.NewControllerRef(pinnipedOwner, pinnipedOwnerWithNewGroupGVK),
+				*metav1.NewControllerRef(pinnipedOwner, pinnipedOwnerGVK),
 
 				// make sure we don't update the non-pinniped owner
 				*metav1.NewControllerRef(nonPinnipedOwner, nonPinnipedOwnerGVK),
@@ -133,7 +135,7 @@ func TestMiddlware(t *testing.T) {
 	}
 	federationDomainWithNewGroupAndPinnipedOwnerWithNewGroup := &configv1alpha1.FederationDomain{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: replaceGV(t, configv1alpha1.SchemeGroupVersion, newSuffix),
+			APIVersion: replaceGV(t, configv1alpha1.SchemeGroupVersion, newSuffix).String(),
 			Kind:       "FederationDomain",
 		},
 		ObjectMeta: metav1.ObjectMeta{
@@ -146,29 +148,40 @@ func TestMiddlware(t *testing.T) {
 		},
 	}
 
-	tokenCredentialRequest := &loginv1alpha1.TokenCredentialRequest{
+	tokenCredentialRequest := with(
-		TypeMeta: metav1.TypeMeta{
+		&loginv1alpha1.TokenCredentialRequest{},
-			APIVersion: loginv1alpha1.SchemeGroupVersion.String(),
+		gvk(replaceGV(t, loginv1alpha1.SchemeGroupVersion, newSuffix).WithKind("TokenCredentialRequest")),
-			Kind:       "TokenCredentialRequest",
+	)
-		},
+	tokenCredentialRequestWithPinnipedAuthenticator := with(
-	}
+		tokenCredentialRequest,
-	tokenCredentialRequestWithNewGroup := &loginv1alpha1.TokenCredentialRequest{
+		authenticatorAPIGroup(authv1alpha1.SchemeGroupVersion.Group),
-		TypeMeta: metav1.TypeMeta{
+	)
-			APIVersion: replaceGV(t, loginv1alpha1.SchemeGroupVersion, newSuffix),
+	tokenCredentialRequestWithCustomAPIGroupAuthenticator := with(
-			Kind:       "TokenCredentialRequest",
+		tokenCredentialRequest,
-		},
+		authenticatorAPIGroup(replaceGV(t, authv1alpha1.SchemeGroupVersion, newSuffix).Group),
-	}
+	)
+	tokenCredentialRequestWithNewGroup := with(
+		tokenCredentialRequest,
+		gvk(replaceGV(t, loginv1alpha1.SchemeGroupVersion, newSuffix).WithKind("TokenCredentialRequest")),
+	)
+	tokenCredentialRequestWithNewGroupAndPinnipedAuthenticator := with(
+		tokenCredentialRequestWithNewGroup,
+		authenticatorAPIGroup(authv1alpha1.SchemeGroupVersion.Group),
+	)
+	tokenCredentialRequestWithNewGroupAndCustomAPIGroupAuthenticator := with(
+		tokenCredentialRequestWithNewGroup,
+		authenticatorAPIGroup(replaceGV(t, authv1alpha1.SchemeGroupVersion, newSuffix).Group),
+	)
 
 	tests := []struct {
-		name                                    string
+		name                                              string
-		apiGroupSuffix                          string
+		apiGroupSuffix                                    string
-		rt                                      *testutil.RoundTrip
+		rt                                                *testutil.RoundTrip
-		requestObj, responseObj                 kubeclient.Object
+		requestObj, responseObj                           kubeclient.Object
-		wantNilMiddleware                       bool
+		wantNilMiddleware                                 bool
-		wantMutateRequests, wantMutateResponses int
+		wantMutateRequests, wantMutateResponses           int
-		wantRequestObj, wantResponseObj         kubeclient.Object
+		wantMutateRequestErrors, wantMutateResponseErrors []string
-
+		wantRequestObj, wantResponseObj                   kubeclient.Object
-		skip bool
 	}{
 		{
 			name:           "api group suffix is empty",
@@ -227,12 +240,12 @@ func TestMiddlware(t *testing.T) {
 			rt: (&testutil.RoundTrip{}).
 				WithVerb(kubeclient.VerbGet).
 				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
-			requestObj:          tokenCredentialRequest,
+			requestObj:          with(&metav1.PartialObjectMetadata{}, gvk(loginv1alpha1.SchemeGroupVersion.WithKind("TokenCredentialRequest"))),
-			responseObj:         tokenCredentialRequest,
+			responseObj:         tokenCredentialRequestWithNewGroup,
 			wantMutateRequests:  1,
 			wantMutateResponses: 1,
-			wantRequestObj:      tokenCredentialRequestWithNewGroup,
+			wantRequestObj:      with(&metav1.PartialObjectMetadata{}, gvk(replaceGV(t, loginv1alpha1.SchemeGroupVersion, newSuffix).WithKind("TokenCredentialRequest"))),
-			wantResponseObj:     tokenCredentialRequest,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
 		},
 		{
 			name:           "create resource without pinniped.dev and without owner ref",
@@ -289,7 +302,7 @@ func TestMiddlware(t *testing.T) {
 			wantResponseObj:     podWithPinnipedOwner,
 		},
 		{
-			// test that both of our middleware request mutations play nicely with each other
+			// test that multiple of our middleware request mutations play nicely with each other
 			name:           "create resource with pinniped.dev and with owner ref that has pinniped.dev owner",
 			apiGroupSuffix: newSuffix,
 			rt: (&testutil.RoundTrip{}).
@@ -297,11 +310,11 @@ func TestMiddlware(t *testing.T) {
 				WithNamespace("some-namespace").
 				WithResource(configv1alpha1.SchemeGroupVersion.WithResource("federationdomains")),
 			requestObj:          federationDomainWithPinnipedOwner,
-			responseObj:         federationDomainWithPinnipedOwnerWithNewGroup,
+			responseObj:         federationDomainWithNewGroupAndPinnipedOwnerWithNewGroup,
 			wantMutateRequests:  2,
 			wantMutateResponses: 1,
 			wantRequestObj:      federationDomainWithNewGroupAndPinnipedOwnerWithNewGroup,
-			wantResponseObj:     federationDomainWithPinnipedOwner,
+			wantResponseObj:     federationDomainWithNewGroupAndPinnipedOwner, // the middleware will reset object GVK for us
 		},
 		{
 			name:           "update resource without pinniped.dev",
@@ -321,11 +334,11 @@ func TestMiddlware(t *testing.T) {
 				WithVerb(kubeclient.VerbUpdate).
 				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
 			requestObj:          tokenCredentialRequest,
-			responseObj:         tokenCredentialRequest,
+			responseObj:         tokenCredentialRequestWithNewGroup,
 			wantMutateRequests:  1,
 			wantMutateResponses: 1,
 			wantRequestObj:      tokenCredentialRequestWithNewGroup,
-			wantResponseObj:     tokenCredentialRequest,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
 		},
 		{
 			name:           "list resource without pinniped.dev",
@@ -345,11 +358,11 @@ func TestMiddlware(t *testing.T) {
 				WithVerb(kubeclient.VerbList).
 				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
 			requestObj:          tokenCredentialRequest,
-			responseObj:         tokenCredentialRequest,
+			responseObj:         tokenCredentialRequestWithNewGroup,
 			wantMutateRequests:  1,
 			wantMutateResponses: 1,
 			wantRequestObj:      tokenCredentialRequestWithNewGroup,
-			wantResponseObj:     tokenCredentialRequest,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
 		},
 		{
 			name:           "watch resource without pinniped.dev",
@@ -369,11 +382,11 @@ func TestMiddlware(t *testing.T) {
 				WithVerb(kubeclient.VerbList).
 				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
 			requestObj:          tokenCredentialRequest,
-			responseObj:         tokenCredentialRequest,
+			responseObj:         tokenCredentialRequestWithNewGroup,
 			wantMutateRequests:  1,
 			wantMutateResponses: 1,
 			wantRequestObj:      tokenCredentialRequestWithNewGroup,
-			wantResponseObj:     tokenCredentialRequest,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
 		},
 		{
 			name:           "patch resource without pinniped.dev",
@@ -390,23 +403,98 @@ func TestMiddlware(t *testing.T) {
 			name:           "patch resource with pinniped.dev",
 			apiGroupSuffix: newSuffix,
 			rt: (&testutil.RoundTrip{}).
-				WithVerb(kubeclient.VerbList).
+				WithVerb(kubeclient.VerbPatch).
 				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
 			requestObj:          tokenCredentialRequest,
-			responseObj:         tokenCredentialRequest,
+			responseObj:         tokenCredentialRequestWithNewGroup,
 			wantMutateRequests:  1,
 			wantMutateResponses: 1,
 			wantRequestObj:      tokenCredentialRequestWithNewGroup,
-			wantResponseObj:     tokenCredentialRequest,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "create tokencredentialrequest with pinniped.dev authenticator api group",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbCreate).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
+			requestObj:          tokenCredentialRequestWithPinnipedAuthenticator,
+			responseObj:         tokenCredentialRequestWithNewGroup, // a token credential response does not contain a spec
+			wantMutateRequests:  3,
+			wantMutateResponses: 1,
+			wantRequestObj:      tokenCredentialRequestWithNewGroupAndCustomAPIGroupAuthenticator,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "create tokencredentialrequest with custom authenticator api group fails because api group is expected to be pinniped.dev",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbCreate).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
+			requestObj:              tokenCredentialRequestWithCustomAPIGroupAuthenticator,
+			responseObj:             tokenCredentialRequestWithNewGroup, // a token credential response does not contain a spec
+			wantMutateRequests:      3,
+			wantMutateResponses:     1,
+			wantMutateRequestErrors: []string{`cannot replace token credential request "/" authenticator API group "authentication.concierge.some.suffix.com" with group suffix "some.suffix.com"`},
+			wantResponseObj:         tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "create tokencredentialrequest with pinniped.dev authenticator api group and subresource",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbCreate).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")).
+				WithSubresource("some-subresource"),
+			requestObj:          tokenCredentialRequestWithPinnipedAuthenticator,
+			responseObj:         tokenCredentialRequestWithNewGroup, // a token credential response does not contain a spec
+			wantMutateRequests:  1,
+			wantMutateResponses: 1,
+			wantRequestObj:      tokenCredentialRequestWithNewGroupAndPinnipedAuthenticator,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "non-create tokencredentialrequest with pinniped.dev authenticator api group",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbList).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
+			requestObj:          tokenCredentialRequestWithPinnipedAuthenticator,
+			responseObj:         tokenCredentialRequestWithNewGroup, // a token credential response does not contain a spec
+			wantMutateRequests:  1,
+			wantMutateResponses: 1,
+			wantRequestObj:      tokenCredentialRequestWithNewGroupAndPinnipedAuthenticator,
+			wantResponseObj:     tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "create tokencredentialrequest with nil authenticator api group",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbCreate).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
+			requestObj:              tokenCredentialRequest,
+			responseObj:             tokenCredentialRequestWithNewGroup, // a token credential response does not contain a spec
+			wantMutateRequests:      3,
+			wantMutateResponses:     1,
+			wantMutateRequestErrors: []string{`cannot replace token credential request "/" without authenticator API group`},
+			wantResponseObj:         tokenCredentialRequestWithNewGroup, // the middleware will reset object GVK for us
+		},
+		{
+			name:           "create tokencredentialrequest with non-*loginv1alpha1.TokenCredentialRequest",
+			apiGroupSuffix: newSuffix,
+			rt: (&testutil.RoundTrip{}).
+				WithVerb(kubeclient.VerbCreate).
+				WithResource(loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests")),
+			requestObj:              podWithoutOwner,
+			responseObj:             podWithoutOwner,
+			wantMutateRequests:      3,
+			wantMutateResponses:     1,
+			wantMutateRequestErrors: []string{`cannot cast obj of type *v1.Pod to *loginv1alpha1.TokenCredentialRequest`},
+			wantResponseObj:         podWithoutOwner,
 		},
 	}
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			if test.skip {
-				t.Skip()
-			}
-
 			m := New(test.apiGroupSuffix)
 			if test.wantNilMiddleware {
 				require.Nil(t, m, "wanted nil middleware")
@@ -414,27 +502,41 @@ func TestMiddlware(t *testing.T) {
 			}
 
 			m.Handle(context.Background(), test.rt)
-			require.Len(t, test.rt.MutateRequests, test.wantMutateRequests)
+			require.Len(t, test.rt.MutateRequests, test.wantMutateRequests, "undesired request mutation count")
-			require.Len(t, test.rt.MutateResponses, test.wantMutateResponses)
+			require.Len(t, test.rt.MutateResponses, test.wantMutateResponses, "undesired response mutation count")
 
 			if test.wantMutateRequests != 0 {
 				require.NotNil(t, test.requestObj, "expected test.requestObj to be set")
 				objMutated := test.requestObj.DeepCopyObject().(kubeclient.Object)
+				var mutateRequestErrors []string
 				for _, mutateRequest := range test.rt.MutateRequests {
 					mutateRequest := mutateRequest
-					mutateRequest(objMutated)
+					if err := mutateRequest(objMutated); err != nil {
+						mutateRequestErrors = append(mutateRequestErrors, err.Error())
+					}
+				}
+				if len(test.wantMutateRequestErrors) > 0 {
+					require.Equal(t, test.wantMutateRequestErrors, mutateRequestErrors, "mutate request errors did not match")
+				} else {
+					require.Equal(t, test.wantRequestObj, objMutated, "request obj did not match")
 				}
-				require.Equal(t, test.wantRequestObj, objMutated, "request obj did not match")
 			}
 
 			if test.wantMutateResponses != 0 {
 				require.NotNil(t, test.responseObj, "expected test.responseObj to be set")
 				objMutated := test.responseObj.DeepCopyObject().(kubeclient.Object)
+				var mutateResponseErrors []string
 				for _, mutateResponse := range test.rt.MutateResponses {
 					mutateResponse := mutateResponse
-					mutateResponse(objMutated)
+					if err := mutateResponse(objMutated); err != nil {
+						mutateResponseErrors = append(mutateResponseErrors, err.Error())
+					}
+				}
+				if len(test.wantMutateRequestErrors) > 0 {
+					require.Equal(t, test.wantMutateResponseErrors, mutateResponseErrors, "mutate response errors did not match")
+				} else {
+					require.Equal(t, test.wantResponseObj, objMutated, "response obj did not match")
 				}
-				require.Equal(t, test.wantResponseObj, objMutated, "response obj did not match")
 			}
 		})
 	}
@@ -519,11 +621,35 @@ func TestValidate(t *testing.T) {
 	}
 }
 
-func replaceGV(t *testing.T, baseGV schema.GroupVersion, apiGroupSuffix string) string {
+type withFunc func(obj kubeclient.Object)
+
+func with(obj kubeclient.Object, withFuncs ...withFunc) kubeclient.Object {
+	obj = obj.DeepCopyObject().(kubeclient.Object)
+	for _, withFunc := range withFuncs {
+		withFunc(obj)
+	}
+	return obj
+}
+
+func gvk(gvk schema.GroupVersionKind) withFunc {
+	return func(obj kubeclient.Object) {
+		obj.GetObjectKind().SetGroupVersionKind(gvk)
+	}
+}
+
+func authenticatorAPIGroup(apiGroup string) withFunc {
+	return func(obj kubeclient.Object) {
+		tokenCredentialRequest := obj.(*loginv1alpha1.TokenCredentialRequest)
+		tokenCredentialRequest.Spec.Authenticator.APIGroup = &apiGroup
+	}
+}
+
+//nolint:unparam // the apiGroupSuffix parameter might always be the same, but this is nice for test readability
+func replaceGV(t *testing.T, baseGV schema.GroupVersion, apiGroupSuffix string) schema.GroupVersion {
 	t.Helper()
 	groupName, ok := Replace(baseGV.Group, apiGroupSuffix)
 	require.True(t, ok, "expected to be able to replace %q's suffix with %q", baseGV.Group, apiGroupSuffix)
-	return schema.GroupVersion{Group: groupName, Version: baseGV.Version}.String()
+	return schema.GroupVersion{Group: groupName, Version: baseGV.Version}
 }
 
 func chars(count int) string {

internal/kubeclient/kubeclient_test.go

@@ -492,8 +492,9 @@ func TestKubeclient(t *testing.T) {
 				return []*spyMiddleware{{
 					name: "non-pertinent mutater",
 					t:    t,
-					mutateReq: func(rt RoundTrip, obj Object) {
+					mutateReq: func(rt RoundTrip, obj Object) error {
 						clusterName()(obj)
+						return nil
 					},
 				}}
 			},
@@ -529,6 +530,64 @@ func TestKubeclient(t *testing.T) {
 			wantMiddlewareReqs:  [][]Object{{with(&metav1.PartialObjectMetadata{}, gvk(federationDomainGVK))}},
 			wantMiddlewareResps: [][]Object{nil},
 		},
+		{
+			name: "when there are request middleware failures, we return an error and don't send the request",
+			middlewares: func(t *testing.T) []*spyMiddleware {
+				return []*spyMiddleware{
+					// use 3 middleware to ensure that we collect all errors from all middlewares
+					newFailingMiddleware(t, "aaa", true, false),
+					newFailingMiddleware(t, "bbb", false, false),
+					newFailingMiddleware(t, "ccc", true, false),
+				}
+			},
+			reallyRunTest: func(t *testing.T, c *Client) {
+				_, err := c.PinnipedSupervisor.
+					ConfigV1alpha1().
+					FederationDomains(goodFederationDomain.Namespace).
+					Get(context.Background(), goodFederationDomain.Name, metav1.GetOptions{})
+				require.Error(t, err)
+				require.Contains(t, err.Error(), ": request mutation failed: [aaa: request error, ccc: request error]")
+			},
+			wantMiddlewareReqs: [][]Object{
+				{with(&metav1.PartialObjectMetadata{}, gvk(federationDomainGVK))},
+				{with(&metav1.PartialObjectMetadata{}, gvk(federationDomainGVK))},
+				{with(&metav1.PartialObjectMetadata{}, gvk(federationDomainGVK))},
+			},
+			wantMiddlewareResps: [][]Object{
+				nil,
+				nil,
+				nil,
+			},
+		},
+		{
+			name: "when there are response middleware failures, we return an error",
+			middlewares: func(t *testing.T) []*spyMiddleware {
+				return []*spyMiddleware{
+					// use 3 middleware to ensure that we collect all errors from all middlewares
+					newFailingMiddleware(t, "aaa", false, true),
+					newFailingMiddleware(t, "bbb", false, false),
+					newFailingMiddleware(t, "ccc", false, true),
+				}
+			},
+			reallyRunTest: func(t *testing.T, c *Client) {
+				_, err := c.PinnipedSupervisor.
+					ConfigV1alpha1().
+					FederationDomains(goodFederationDomain.Namespace).
+					Create(context.Background(), goodFederationDomain, metav1.CreateOptions{})
+				require.Error(t, err)
+				require.Contains(t, err.Error(), ": response mutation failed: [aaa: response error, ccc: response error]")
+			},
+			wantMiddlewareReqs: [][]Object{
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+			},
+			wantMiddlewareResps: [][]Object{
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+				{with(goodFederationDomain, gvk(federationDomainGVK))},
+			},
+		},
 	}
 	for _, test := range tests {
 		test := test
@@ -568,8 +627,8 @@ func TestKubeclient(t *testing.T) {
 type spyMiddleware struct {
 	name       string
 	t          *testing.T
-	mutateReq  func(RoundTrip, Object)
+	mutateReq  func(RoundTrip, Object) error
-	mutateResp func(RoundTrip, Object)
+	mutateResp func(RoundTrip, Object) error
 	reqObjs    []Object
 	respObjs   []Object
 }
@@ -578,18 +637,18 @@ func (s *spyMiddleware) Handle(_ context.Context, rt RoundTrip) {
 	s.t.Log(s.name, "handling", reqStr(rt, nil))
 
 	if s.mutateReq != nil {
-		rt.MutateRequest(func(obj Object) {
+		rt.MutateRequest(func(obj Object) error {
 			s.t.Log(s.name, "mutating request", reqStr(rt, obj))
 			s.reqObjs = append(s.reqObjs, obj.DeepCopyObject().(Object))
-			s.mutateReq(rt, obj)
+			return s.mutateReq(rt, obj)
 		})
 	}
 
 	if s.mutateResp != nil {
-		rt.MutateResponse(func(obj Object) {
+		rt.MutateResponse(func(obj Object) error {
 			s.t.Log(s.name, "mutating response", reqStr(rt, obj))
 			s.respObjs = append(s.respObjs, obj.DeepCopyObject().(Object))
-			s.mutateResp(rt, obj)
+			return s.mutateResp(rt, obj)
 		})
 	}
 }
@@ -611,17 +670,19 @@ func newAnnotationMiddleware(t *testing.T) *spyMiddleware {
 	return &spyMiddleware{
 		name: "annotater",
 		t:    t,
-		mutateReq: func(rt RoundTrip, obj Object) {
+		mutateReq: func(rt RoundTrip, obj Object) error {
 			if rt.Verb() == VerbCreate {
 				annotations()(obj)
 			}
+			return nil
 		},
-		mutateResp: func(rt RoundTrip, obj Object) {
+		mutateResp: func(rt RoundTrip, obj Object) error {
 			if rt.Verb() == VerbCreate {
 				for key := range middlewareAnnotations {
 					delete(obj.GetAnnotations(), key)
 				}
 			}
+			return nil
 		},
 	}
 }
@@ -630,41 +691,69 @@ func newLabelMiddleware(t *testing.T) *spyMiddleware {
 	return &spyMiddleware{
 		name: "labeler",
 		t:    t,
-		mutateReq: func(rt RoundTrip, obj Object) {
+		mutateReq: func(rt RoundTrip, obj Object) error {
 			if rt.Verb() == VerbCreate {
 				labels()(obj)
 			}
+			return nil
 		},
-		mutateResp: func(rt RoundTrip, obj Object) {
+		mutateResp: func(rt RoundTrip, obj Object) error {
 			if rt.Verb() == VerbCreate {
 				for key := range middlewareLabels {
 					delete(obj.GetLabels(), key)
 				}
 			}
+			return nil
 		},
 	}
 }
 
 func newSimpleMiddleware(t *testing.T, hasMutateReqFunc, mutatedReq, hasMutateRespFunc bool) *spyMiddleware {
 	m := &spyMiddleware{
-		name: "nop",
+		name: "simple",
 		t:    t,
 	}
 	if hasMutateReqFunc {
-		m.mutateReq = func(rt RoundTrip, obj Object) {
+		m.mutateReq = func(rt RoundTrip, obj Object) error {
 			if mutatedReq {
 				if rt.Verb() == VerbCreate {
 					obj.SetClusterName(someClusterName)
 				}
 			}
+			return nil
 		}
 	}
 	if hasMutateRespFunc {
-		m.mutateResp = func(rt RoundTrip, obj Object) {}
+		m.mutateResp = func(rt RoundTrip, obj Object) error {
+			return nil
+		}
 	}
 	return m
 }
 
+func newFailingMiddleware(t *testing.T, name string, mutateReqFails, mutateRespFails bool) *spyMiddleware {
+	m := &spyMiddleware{
+		name: "failing-middleware-" + name,
+		t:    t,
+	}
+
+	m.mutateReq = func(rt RoundTrip, obj Object) error {
+		if mutateReqFails {
+			return fmt.Errorf("%s: request error", name)
+		}
+		return nil
+	}
+
+	m.mutateResp = func(rt RoundTrip, obj Object) error {
+		if mutateRespFails {
+			return fmt.Errorf("%s: response error", name)
+		}
+		return nil
+	}
+
+	return m
+}
+
 type wantCloser struct {
 	io.ReadCloser
 	closeCount                      int

internal/kubeclient/middleware.go

@@ -12,6 +12,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/errors"
 )
 
 type Middleware interface {
@@ -43,8 +44,8 @@ type RoundTrip interface {
 	NamespaceScoped() bool
 	Resource() schema.GroupVersionResource
 	Subresource() string
-	MutateRequest(f func(obj Object))
+	MutateRequest(f func(obj Object) error)
-	MutateResponse(f func(obj Object))
+	MutateResponse(f func(obj Object) error)
 }
 
 type Object interface {
@@ -58,7 +59,7 @@ type request struct {
 	verb                Verb
 	namespace           string
 	resource            schema.GroupVersionResource
-	reqFuncs, respFuncs []func(obj Object)
+	reqFuncs, respFuncs []func(obj Object) error
 	subresource         string
 }
 
@@ -89,11 +90,11 @@ func (r *request) Subresource() string {
 	return r.subresource
 }
 
-func (r *request) MutateRequest(f func(obj Object)) {
+func (r *request) MutateRequest(f func(obj Object) error) {
 	r.reqFuncs = append(r.reqFuncs, f)
 }
 
-func (r *request) MutateResponse(f func(obj Object)) {
+func (r *request) MutateResponse(f func(obj Object) error) {
 	r.respFuncs = append(r.respFuncs, f)
 }
 
@@ -113,9 +114,15 @@ func (r *request) mutateRequest(obj Object) (*mutationResult, error) {
 		return nil, fmt.Errorf("invalid deep copy semantics for %T: %#v", obj, r)
 	}
 
+	var errs []error
 	for _, reqFunc := range r.reqFuncs {
 		reqFunc := reqFunc
-		reqFunc(obj)
+		if err := reqFunc(obj); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	if err := errors.NewAggregate(errs); err != nil {
+		return nil, fmt.Errorf("request mutation failed: %w", err)
 	}
 
 	newGVK := obj.GetObjectKind().GroupVersionKind()
@@ -137,9 +144,15 @@ func (r *request) mutateResponse(obj Object) (bool, error) {
 		return false, fmt.Errorf("invalid deep copy semantics for %T: %#v", obj, r)
 	}
 
+	var errs []error
 	for _, respFunc := range r.respFuncs {
 		respFunc := respFunc
-		respFunc(obj)
+		if err := respFunc(obj); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	if err := errors.NewAggregate(errs); err != nil {
+		return false, fmt.Errorf("response mutation failed: %w", err)
 	}
 
 	mutated := !apiequality.Semantic.DeepEqual(origObj, obj)

internal/kubeclient/middleware_test.go

@@ -15,7 +15,7 @@ import (
 func Test_request_mutate(t *testing.T) {
 	tests := []struct {
 		name     string
-		reqFuncs []func(Object)
+		reqFuncs []func(Object) error
 		obj      Object
 		want     *mutationResult
 		wantObj  Object
@@ -23,10 +23,11 @@ func Test_request_mutate(t *testing.T) {
 	}{
 		{
 			name: "mutate config map data",
-			reqFuncs: []func(Object){
+			reqFuncs: []func(Object) error{
-				func(obj Object) {
+				func(obj Object) error {
 					cm := obj.(*corev1.ConfigMap)
 					cm.Data = map[string]string{"new": "stuff"}
+					return nil
 				},
 			},
 			obj: &corev1.ConfigMap{

internal/kubeclient/verb_test.go

@@ -42,7 +42,7 @@ func Test_verb(t *testing.T) {
 			f: func() string {
 				return fmt.Errorf("%#v", request{verb: VerbPatch}).Error()
 			},
-			want: `kubeclient.request{verb:"patch", namespace:"", resource:schema.GroupVersionResource{Group:"", Version:"", Resource:""}, reqFuncs:[]func(kubeclient.Object)(nil), respFuncs:[]func(kubeclient.Object)(nil), subresource:""}`,
+			want: `kubeclient.request{verb:"patch", namespace:"", resource:schema.GroupVersionResource{Group:"", Version:"", Resource:""}, reqFuncs:[]func(kubeclient.Object) error(nil), respFuncs:[]func(kubeclient.Object) error(nil), subresource:""}`,
 		},
 	}
 	for _, tt := range tests {

internal/ownerref/ownerref.go

@@ -51,13 +51,15 @@ func New(refObj kubeclient.Object) kubeclient.Middleware {
 			return
 		}
 
-		rt.MutateRequest(func(obj kubeclient.Object) {
+		rt.MutateRequest(func(obj kubeclient.Object) error {
 			// we only want to set the owner ref on create and when one is not already present
 			if len(obj.GetOwnerReferences()) != 0 {
-				return
+				return nil
 			}
 
 			obj.SetOwnerReferences([]metav1.OwnerReference{ref})
+
+			return nil
 		})
 	})
 }

internal/ownerref/ownerref_test.go

@@ -213,7 +213,7 @@ func TestOwnerReferenceMiddleware(t *testing.T) {
 			orig := tt.args.obj.DeepCopyObject().(kubeclient.Object)
 			for _, mutateRequest := range rt.MutateRequests {
 				mutateRequest := mutateRequest
-				mutateRequest(tt.args.obj)
+				require.NoError(t, mutateRequest(tt.args.obj))
 			}
 			if !tt.wantMutates {
 				require.Equal(t, orig, tt.args.obj)

internal/testutil/roundtrip.go

@@ -17,7 +17,7 @@ type RoundTrip struct {
 	resource        schema.GroupVersionResource
 	subresource     string
 
-	MutateRequests, MutateResponses []func(kubeclient.Object)
+	MutateRequests, MutateResponses []func(kubeclient.Object) error
 }
 
 func (rt *RoundTrip) WithVerb(verb kubeclient.Verb) *RoundTrip {
@@ -61,10 +61,10 @@ func (rt *RoundTrip) Subresource() string {
 	return rt.subresource
 }
 
-func (rt *RoundTrip) MutateRequest(fn func(kubeclient.Object)) {
+func (rt *RoundTrip) MutateRequest(fn func(kubeclient.Object) error) {
 	rt.MutateRequests = append(rt.MutateRequests, fn)
 }
 
-func (rt *RoundTrip) MutateResponse(fn func(kubeclient.Object)) {
+func (rt *RoundTrip) MutateResponse(fn func(kubeclient.Object) error) {
 	rt.MutateResponses = append(rt.MutateResponses, fn)
 }

pkg/conciergeclient/conciergeclient.go

@@ -12,7 +12,7 @@ import (
 	"net/url"
 	"strings"
 
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1"
 	"k8s.io/client-go/tools/clientcmd"
@@ -34,12 +34,11 @@ type Option func(*Client) error
 
 // Client is a configuration for talking to the Pinniped concierge.
 type Client struct {
-	namespace         string
+	namespace      string
-	authenticatorName string
+	authenticator  *corev1.TypedLocalObjectReference
-	authenticatorKind string
+	caBundle       string
-	caBundle          string
+	endpoint       *url.URL
-	endpoint          *url.URL
+	apiGroupSuffix string
-	apiGroupSuffix    string
 }
 
 // WithNamespace configures the namespace where the TokenCredentialRequest is to be sent.
@@ -56,15 +55,18 @@ func WithAuthenticator(authType, authName string) Option {
 		if authName == "" {
 			return fmt.Errorf("authenticator name must not be empty")
 		}
-		c.authenticatorName = authName
+		authenticator := corev1.TypedLocalObjectReference{Name: authName}
 		switch strings.ToLower(authType) {
 		case "webhook":
-			c.authenticatorKind = "WebhookAuthenticator"
+			authenticator.APIGroup = &auth1alpha1.SchemeGroupVersion.Group
+			authenticator.Kind = "WebhookAuthenticator"
 		case "jwt":
-			c.authenticatorKind = "JWTAuthenticator"
+			authenticator.APIGroup = &auth1alpha1.SchemeGroupVersion.Group
+			authenticator.Kind = "JWTAuthenticator"
 		default:
 			return fmt.Errorf(`invalid authenticator type: %q, supported values are "webhook" and "jwt"`, authType)
 		}
+		c.authenticator = &authenticator
 		return nil
 	}
 }
@@ -131,7 +133,7 @@ func New(opts ...Option) (*Client, error) {
 			return nil, err
 		}
 	}
-	if c.authenticatorName == "" {
+	if c.authenticator == nil {
 		return nil, fmt.Errorf("WithAuthenticator must be specified")
 	}
 	if c.endpoint == nil {
@@ -178,18 +180,13 @@ func (c *Client) ExchangeToken(ctx context.Context, token string) (*clientauthen
 	if err != nil {
 		return nil, err
 	}
-	replacedAPIGroupName, _ := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, c.apiGroupSuffix)
 	resp, err := clientset.LoginV1alpha1().TokenCredentialRequests(c.namespace).Create(ctx, &loginv1alpha1.TokenCredentialRequest{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: c.namespace,
 		},
 		Spec: loginv1alpha1.TokenCredentialRequestSpec{
-			Token: token,
+			Token:         token,
-			Authenticator: v1.TypedLocalObjectReference{
+			Authenticator: *c.authenticator,
-				APIGroup: &replacedAPIGroupName,
-				Kind:     c.authenticatorKind,
-				Name:     c.authenticatorName,
-			},
 		},
 	}, metav1.CreateOptions{})
 	if err != nil {

pkg/conciergeclient/conciergeclient_test.go

@@ -21,7 +21,6 @@ import (
 
 	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/certauthority"
-	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/testutil"
 )
 
@@ -221,7 +220,47 @@ func TestExchangeToken(t *testing.T) {
 		t.Parallel()
 		expires := metav1.NewTime(time.Now().Truncate(time.Second))
 
-		caBundle, endpoint := runFakeServer(t, expires, "pinniped.dev")
+		// Start a test server that returns successfully and asserts various properties of the request.
+		caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) {
+			require.Equal(t, http.MethodPost, r.Method)
+			require.Equal(t, "/apis/login.concierge.pinniped.dev/v1alpha1/namespaces/test-namespace/tokencredentialrequests", r.URL.Path)
+			require.Equal(t, "application/json", r.Header.Get("content-type"))
+
+			body, err := ioutil.ReadAll(r.Body)
+			require.NoError(t, err)
+			require.JSONEq(t,
+				`{
+				  "kind": "TokenCredentialRequest",
+				  "apiVersion": "login.concierge.pinniped.dev/v1alpha1",
+				  "metadata": {
+					"creationTimestamp": null,
+					"namespace": "test-namespace"
+				  },
+				  "spec": {
+					"token": "test-token",
+					"authenticator": {
+						"apiGroup": "authentication.concierge.pinniped.dev",
+						"kind": "WebhookAuthenticator",
+						"name": "test-webhook"
+					}
+				  },
+				  "status": {}
+				}`,
+				string(body),
+			)
+
+			w.Header().Set("content-type", "application/json")
+			_ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{
+				TypeMeta: metav1.TypeMeta{APIVersion: "login.concierge.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"},
+				Status: loginv1alpha1.TokenCredentialRequestStatus{
+					Credential: &loginv1alpha1.ClusterCredential{
+						ExpirationTimestamp:   expires,
+						ClientCertificateData: "test-certificate",
+						ClientKeyData:         "test-key",
+					},
+				},
+			})
+		})
 
 		client, err := New(WithNamespace("test-namespace"), WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("webhook", "test-webhook"))
 		require.NoError(t, err)
@@ -240,78 +279,4 @@ func TestExchangeToken(t *testing.T) {
 			},
 		}, got)
 	})
-
-	t.Run("changing the API group suffix for the client sends the custom suffix on the CredentialRequest's APIGroup and on its spec.Authenticator.APIGroup", func(t *testing.T) {
-		t.Parallel()
-		expires := metav1.NewTime(time.Now().Truncate(time.Second))
-
-		caBundle, endpoint := runFakeServer(t, expires, "suffix.com")
-
-		client, err := New(WithAPIGroupSuffix("suffix.com"), WithNamespace("test-namespace"), WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("webhook", "test-webhook"))
-		require.NoError(t, err)
-
-		got, err := client.ExchangeToken(ctx, "test-token")
-		require.NoError(t, err)
-		require.Equal(t, &clientauthenticationv1beta1.ExecCredential{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "ExecCredential",
-				APIVersion: "client.authentication.k8s.io/v1beta1",
-			},
-			Status: &clientauthenticationv1beta1.ExecCredentialStatus{
-				ClientCertificateData: "test-certificate",
-				ClientKeyData:         "test-key",
-				ExpirationTimestamp:   &expires,
-			},
-		}, got)
-	})
-}
-
-// Start a test server that returns successfully and asserts various properties of the request.
-func runFakeServer(t *testing.T, expires metav1.Time, pinnipedAPIGroupSuffix string) (string, string) {
-	caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) {
-		require.Equal(t, http.MethodPost, r.Method)
-		require.Equal(t,
-			fmt.Sprintf("/apis/login.concierge.%s/v1alpha1/namespaces/test-namespace/tokencredentialrequests", pinnipedAPIGroupSuffix),
-			r.URL.Path)
-		require.Equal(t, "application/json", r.Header.Get("content-type"))
-
-		body, err := ioutil.ReadAll(r.Body)
-		require.NoError(t, err)
-		require.JSONEq(t, here.Docf(
-			`{
-				  "kind": "TokenCredentialRequest",
-				  "apiVersion": "login.concierge.%s/v1alpha1",
-				  "metadata": {
-					"creationTimestamp": null,
-					"namespace": "test-namespace"
-				  },
-				  "spec": {
-					"token": "test-token",
-					"authenticator": {
-						"apiGroup": "authentication.concierge.%s",
-						"kind": "WebhookAuthenticator",
-						"name": "test-webhook"
-					}
-				  },
-				  "status": {}
-				}`, pinnipedAPIGroupSuffix, pinnipedAPIGroupSuffix),
-			string(body),
-		)
-
-		w.Header().Set("content-type", "application/json")
-		_ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: fmt.Sprintf("login.concierge.%s/v1alpha1", pinnipedAPIGroupSuffix),
-				Kind:       "TokenCredentialRequest",
-			},
-			Status: loginv1alpha1.TokenCredentialRequestStatus{
-				Credential: &loginv1alpha1.ClusterCredential{
-					ExpirationTimestamp:   expires,
-					ClientCertificateData: "test-certificate",
-					ClientKeyData:         "test-key",
-				},
-			},
-		})
-	})
-	return caBundle, endpoint
 }

test/integration/concierge_api_serving_certs_test.go

@@ -78,11 +78,15 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
 			kubeClient := library.NewKubernetesClientset(t)
 			aggregatedClient := library.NewAggregatedClientset(t)
 			conciergeClient := library.NewConciergeClientset(t)
-			ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+			ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
 			defer cancel()
 
 			apiServiceName := "v1alpha1.login.concierge." + env.APIGroupSuffix
 
+			// Create a testWebhook so we have a legitimate authenticator to pass to the
+			// TokenCredentialRequest API.
+			testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
+
 			// Get the initial auto-generated version of the Secret.
 			secret, err := kubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, defaultServingCertResourceName, metav1.GetOptions{})
 			require.NoError(t, err)
@@ -145,7 +149,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
 					_, err = conciergeClient.LoginV1alpha1().TokenCredentialRequests(env.ConciergeNamespace).Create(ctx, &loginv1alpha1.TokenCredentialRequest{
 						TypeMeta:   metav1.TypeMeta{},
 						ObjectMeta: metav1.ObjectMeta{},
-						Spec:       loginv1alpha1.TokenCredentialRequestSpec{Token: "not a good token"},
+						Spec:       loginv1alpha1.TokenCredentialRequestSpec{Token: "not a good token", Authenticator: testWebhook},
 					}, metav1.CreateOptions{})
 					if err != nil {
 						break

test/integration/concierge_credentialrequest_test.go

@@ -135,7 +135,16 @@ func TestFailedCredentialRequestWhenTheRequestIsValidButTheTokenDoesNotAuthentic
 
 	library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
 
-	response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{Token: "not a good token"})
+	// Create a testWebhook so we have a legitimate authenticator to pass to the
+	// TokenCredentialRequest API.
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
+	defer cancel()
+	testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
+
+	response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{
+		Token:         "not a good token",
+		Authenticator: testWebhook,
+	})
 
 	require.NoError(t, err)
 
@@ -149,7 +158,16 @@ func TestCredentialRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T
 
 	library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
 
-	response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{Token: ""})
+	// Create a testWebhook so we have a legitimate authenticator to pass to the
+	// TokenCredentialRequest API.
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
+	defer cancel()
+	testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
+
+	response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{
+		Token:         "",
+		Authenticator: testWebhook,
+	})
 
 	require.Error(t, err)
 	statusError, isStatus := err.(*errors.StatusError)

test/library/client.go

@@ -180,11 +180,8 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
 		require.NoErrorf(t, err, "could not cleanup test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
 	})
 
-	apiGroup, replacedSuffix := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, testEnv.APIGroupSuffix)
-	require.True(t, replacedSuffix)
-
 	return corev1.TypedLocalObjectReference{
-		APIGroup: &apiGroup,
+		APIGroup: &auth1alpha1.SchemeGroupVersion.Group,
 		Kind:     "WebhookAuthenticator",
 		Name:     webhook.Name,
 	}
@@ -253,11 +250,8 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp
 		require.NoErrorf(t, err, "could not cleanup test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
 	})
 
-	apiGroup, replacedSuffix := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, testEnv.APIGroupSuffix)
-	require.True(t, replacedSuffix)
-
 	return corev1.TypedLocalObjectReference{
-		APIGroup: &apiGroup,
+		APIGroup: &auth1alpha1.SchemeGroupVersion.Group,
 		Kind:     "JWTAuthenticator",
 		Name:     jwtAuthenticator.Name,
 	}