internal/concierge/impersonator: handle custom login API group

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

cmd/pinniped/cmd/login_oidc.go

@@ -267,6 +267,7 @@ func execCredentialForImpersonationProxy(
 	tokenExpiry *metav1.Time,
 ) (*clientauthv1beta1.ExecCredential, error) {
 	// TODO maybe de-dup this with conciergeclient.go
+	// TODO reuse code from internal/testutil/impersonationtoken here to create token
 	var kind string
 	switch strings.ToLower(conciergeAuthenticatorType) {
 	case "webhook":

internal/concierge/impersonator/impersonator.go

@@ -5,7 +5,6 @@ package impersonator
 
 import (
 	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httputil"
@@ -13,12 +12,12 @@ import (
 	"strings"
 
 	"github.com/go-logr/logr"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/transport"
 
 	"go.pinniped.dev/generated/1.20/apis/concierge/login"
-	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/controller/authenticator/authncache"
 	"go.pinniped.dev/internal/kubeclient"
 )
@@ -34,13 +33,14 @@ var allowedHeaders = []string{
 }
 
 type proxy struct {
-	cache *authncache.Cache
+	cache       *authncache.Cache
-	proxy *httputil.ReverseProxy
+	jsonDecoder runtime.Decoder
-	log   logr.Logger
+	proxy       *httputil.ReverseProxy
+	log         logr.Logger
 }
 
-func New(cache *authncache.Cache, log logr.Logger) (http.Handler, error) {
+func New(cache *authncache.Cache, jsonDecoder runtime.Decoder, log logr.Logger) (http.Handler, error) {
-	return newInternal(cache, log, func() (*rest.Config, error) {
+	return newInternal(cache, jsonDecoder, log, func() (*rest.Config, error) {
 		client, err := kubeclient.New()
 		if err != nil {
 			return nil, err
@@ -49,7 +49,7 @@ func New(cache *authncache.Cache, log logr.Logger) (http.Handler, error) {
 	})
 }
 
-func newInternal(cache *authncache.Cache, log logr.Logger, getConfig func() (*rest.Config, error)) (*proxy, error) {
+func newInternal(cache *authncache.Cache, jsonDecoder runtime.Decoder, log logr.Logger, getConfig func() (*rest.Config, error)) (*proxy, error) {
 	kubeconfig, err := getConfig()
 	if err != nil {
 		return nil, fmt.Errorf("could not get in-cluster config: %w", err)
@@ -75,9 +75,10 @@ func newInternal(cache *authncache.Cache, log logr.Logger, getConfig func() (*re
 	reverseProxy.Transport = kubeRoundTripper
 
 	return &proxy{
-		cache: cache,
+		cache:       cache,
-		proxy: reverseProxy,
+		jsonDecoder: jsonDecoder,
-		log:   log,
+		proxy:       reverseProxy,
+		log:         log,
 	}, nil
 }
 
@@ -87,7 +88,7 @@ func (p *proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		"method", r.Method,
 	)
 
-	tokenCredentialReq, err := extractToken(r)
+	tokenCredentialReq, err := extractToken(r, p.jsonDecoder)
 	if err != nil {
 		log.Error(err, "invalid token encoding")
 		http.Error(w, "invalid token encoding", http.StatusBadRequest)
@@ -134,7 +135,7 @@ func getProxyHeaders(userInfo user.Info, requestHeaders http.Header) http.Header
 	return newHeaders
 }
 
-func extractToken(req *http.Request) (*login.TokenCredentialRequest, error) {
+func extractToken(req *http.Request, jsonDecoder runtime.Decoder) (*login.TokenCredentialRequest, error) {
 	authHeader := req.Header.Get("Authorization")
 	if authHeader == "" {
 		return nil, fmt.Errorf("missing authorization header")
@@ -148,13 +149,14 @@ func extractToken(req *http.Request) (*login.TokenCredentialRequest, error) {
 		return nil, fmt.Errorf("invalid base64 in encoded bearer token: %w", err)
 	}
 
-	var v1alpha1Req loginv1alpha1.TokenCredentialRequest
+	obj, err := runtime.Decode(jsonDecoder, tokenCredentialRequestJSON)
-	if err := json.Unmarshal(tokenCredentialRequestJSON, &v1alpha1Req); err != nil {
+	if err != nil {
-		return nil, fmt.Errorf("invalid TokenCredentialRequest encoded in bearer token: %w", err)
+		return nil, fmt.Errorf("invalid object encoded in bearer token: %w", err)
 	}
-	var internalReq login.TokenCredentialRequest
+	tokenCredentialRequest, ok := obj.(*login.TokenCredentialRequest)
-	if err := loginv1alpha1.Convert_v1alpha1_TokenCredentialRequest_To_login_TokenCredentialRequest(&v1alpha1Req, &internalReq, nil); err != nil {
+	if !ok {
-		return nil, fmt.Errorf("failed to convert v1alpha1 TokenCredentialRequest to internal version: %w", err)
+		return nil, fmt.Errorf("invalid TokenCredentialRequest encoded in bearer token: got %T", obj)
 	}
-	return &internalReq, nil
+
+	return tokenCredentialRequest, nil
 }

internal/concierge/impersonator/impersonator_test.go

@@ -5,8 +5,6 @@ package impersonator
 
 import (
 	"context"
-	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -16,20 +14,31 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd/api"
 
-	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
+	authenticationv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/authentication/v1alpha1"
+	"go.pinniped.dev/generated/1.20/apis/concierge/login"
+	conciergescheme "go.pinniped.dev/internal/concierge/scheme"
 	"go.pinniped.dev/internal/controller/authenticator/authncache"
+	"go.pinniped.dev/internal/groupsuffix"
 	"go.pinniped.dev/internal/mocks/mocktokenauthenticator"
 	"go.pinniped.dev/internal/testutil"
+	"go.pinniped.dev/internal/testutil/impersonationtoken"
 	"go.pinniped.dev/internal/testutil/testlogger"
 )
 
 func TestImpersonator(t *testing.T) {
+	const (
+		defaultAPIGroup = "pinniped.dev"
+		customAPIGroup  = "walrus.tld"
+	)
+
 	validURL, _ := url.Parse("http://pinniped.dev/blah")
 	testServerCA, testServerURL := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		// Expect that the request is authenticated based on the kubeconfig credential.
@@ -61,15 +70,25 @@ func TestImpersonator(t *testing.T) {
 		return r
 	}
 
+	goodAuthenticator := corev1.TypedLocalObjectReference{
+		Name:     "authenticator-one",
+		APIGroup: stringPtr(authenticationv1alpha1.GroupName),
+	}
+	badAuthenticator := corev1.TypedLocalObjectReference{
+		Name:     "",
+		APIGroup: stringPtr(authenticationv1alpha1.GroupName),
+	}
+
 	tests := []struct {
-		name            string
+		name             string
-		getKubeconfig   func() (*rest.Config, error)
+		apiGroupOverride string
-		wantCreationErr string
+		getKubeconfig    func() (*rest.Config, error)
-		request         *http.Request
+		wantCreationErr  string
-		wantHTTPBody    string
+		request          *http.Request
-		wantHTTPStatus  int
+		wantHTTPBody     string
-		wantLogs        []string
+		wantHTTPStatus   int
-		expectMockToken func(*testing.T, *mocktokenauthenticator.MockTokenMockRecorder)
+		wantLogs         []string
+		expectMockToken  func(*testing.T, *mocktokenauthenticator.MockTokenMockRecorder)
 	}{
 		{
 			name: "fail to get in-cluster config",
@@ -119,7 +138,7 @@ func TestImpersonator(t *testing.T) {
 		{
 			name:           "authorization header missing bearer prefix",
 			getKubeconfig:  func() (*rest.Config, error) { return &testServerKubeconfig, nil },
-			request:        newRequest(map[string][]string{"Authorization": {makeTestTokenRequest("foo", "authenticator-one", "test-token")}}),
+			request:        newRequest(map[string][]string{"Authorization": {impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)}}),
 			wantHTTPBody:   "invalid token encoding\n",
 			wantHTTPStatus: http.StatusBadRequest,
 			wantLogs:       []string{"\"error\"=\"authorization header must be of type Bearer\" \"msg\"=\"invalid token encoding\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
@@ -138,33 +157,50 @@ func TestImpersonator(t *testing.T) {
 			request:        newRequest(map[string][]string{"Authorization": {"Bearer abc"}}),
 			wantHTTPBody:   "invalid token encoding\n",
 			wantHTTPStatus: http.StatusBadRequest,
-			wantLogs:       []string{"\"error\"=\"invalid TokenCredentialRequest encoded in bearer token: invalid character 'i' looking for beginning of value\" \"msg\"=\"invalid token encoding\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
+			wantLogs:       []string{"\"error\"=\"invalid object encoded in bearer token: couldn't get version/kind; json parse error: invalid character 'i' looking for beginning of value\" \"msg\"=\"invalid token encoding\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
+		},
+		{
+			name:             "base64 encoded token is encoded with default api group but we are expecting custom api group",
+			apiGroupOverride: customAPIGroup,
+			getKubeconfig:    func() (*rest.Config, error) { return &testServerKubeconfig, nil },
+			request:          newRequest(map[string][]string{"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)}}),
+			wantHTTPBody:     "invalid token encoding\n",
+			wantHTTPStatus:   http.StatusBadRequest,
+			wantLogs:         []string{"\"error\"=\"invalid object encoded in bearer token: no kind \\\"TokenCredentialRequest\\\" is registered for version \\\"login.concierge.pinniped.dev/v1alpha1\\\" in scheme \\\"pkg/runtime/scheme.go:100\\\"\" \"msg\"=\"invalid token encoding\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
+		},
+		{
+			name:           "base64 encoded token is encoded with custom api group but we are expecting default api group",
+			getKubeconfig:  func() (*rest.Config, error) { return &testServerKubeconfig, nil },
+			request:        newRequest(map[string][]string{"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, customAPIGroup)}}),
+			wantHTTPBody:   "invalid token encoding\n",
+			wantHTTPStatus: http.StatusBadRequest,
+			wantLogs:       []string{"\"error\"=\"invalid object encoded in bearer token: no kind \\\"TokenCredentialRequest\\\" is registered for version \\\"login.concierge.walrus.tld/v1alpha1\\\" in scheme \\\"pkg/runtime/scheme.go:100\\\"\" \"msg\"=\"invalid token encoding\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
 		},
 		{
 			name:           "token could not be authenticated",
 			getKubeconfig:  func() (*rest.Config, error) { return &testServerKubeconfig, nil },
-			request:        newRequest(map[string][]string{"Authorization": {"Bearer " + makeTestTokenRequest("", "", "")}}),
+			request:        newRequest(map[string][]string{"Authorization": {"Bearer " + impersonationtoken.Make(t, "", &badAuthenticator, defaultAPIGroup)}}),
 			wantHTTPBody:   "invalid token\n",
 			wantHTTPStatus: http.StatusUnauthorized,
-			wantLogs:       []string{"\"error\"=\"no such authenticator\" \"msg\"=\"received invalid token\" \"authenticator\"={\"apiGroup\":null,\"kind\":\"\",\"name\":\"\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
+			wantLogs:       []string{"\"error\"=\"no such authenticator\" \"msg\"=\"received invalid token\" \"authenticator\"={\"apiGroup\":\"authentication.concierge.pinniped.dev\",\"kind\":\"\",\"name\":\"\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
 		},
 		{
 			name:          "token authenticates as nil",
 			getKubeconfig: func() (*rest.Config, error) { return &testServerKubeconfig, nil },
-			request:       newRequest(map[string][]string{"Authorization": {"Bearer " + makeTestTokenRequest("foo", "authenticator-one", "test-token")}}),
+			request:       newRequest(map[string][]string{"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)}}),
 			expectMockToken: func(t *testing.T, recorder *mocktokenauthenticator.MockTokenMockRecorder) {
 				recorder.AuthenticateToken(gomock.Any(), "test-token").Return(nil, false, nil)
 			},
 			wantHTTPBody:   "not authenticated\n",
 			wantHTTPStatus: http.StatusUnauthorized,
-			wantLogs:       []string{"\"level\"=0 \"msg\"=\"received token that did not authenticate\" \"authenticator\"={\"apiGroup\":null,\"kind\":\"\",\"name\":\"authenticator-one\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
+			wantLogs:       []string{"\"level\"=0 \"msg\"=\"received token that did not authenticate\" \"authenticator\"={\"apiGroup\":\"authentication.concierge.pinniped.dev\",\"kind\":\"\",\"name\":\"authenticator-one\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
 		},
 		// happy path
 		{
 			name:          "token validates",
 			getKubeconfig: func() (*rest.Config, error) { return &testServerKubeconfig, nil },
 			request: newRequest(map[string][]string{
-				"Authorization":    {"Bearer " + makeTestTokenRequest("foo", "authenticator-one", "test-token")},
+				"Authorization":    {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)},
 				"Malicious-Header": {"test-header-value-1"},
 				"User-Agent":       {"test-user-agent"},
 			}),
@@ -179,7 +215,29 @@ func TestImpersonator(t *testing.T) {
 			},
 			wantHTTPBody:   "successful proxied response",
 			wantHTTPStatus: http.StatusOK,
-			wantLogs:       []string{"\"level\"=0 \"msg\"=\"proxying authenticated request\" \"authenticator\"={\"apiGroup\":null,\"kind\":\"\",\"name\":\"authenticator-one\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\" \"userID\"=\"test-uid\""},
+			wantLogs:       []string{"\"level\"=0 \"msg\"=\"proxying authenticated request\" \"authenticator\"={\"apiGroup\":\"authentication.concierge.pinniped.dev\",\"kind\":\"\",\"name\":\"authenticator-one\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\" \"userID\"=\"test-uid\""},
+		},
+		{
+			name:             "token validates with custom api group",
+			apiGroupOverride: customAPIGroup,
+			getKubeconfig:    func() (*rest.Config, error) { return &testServerKubeconfig, nil },
+			request: newRequest(map[string][]string{
+				"Authorization":    {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, customAPIGroup)},
+				"Malicious-Header": {"test-header-value-1"},
+				"User-Agent":       {"test-user-agent"},
+			}),
+			expectMockToken: func(t *testing.T, recorder *mocktokenauthenticator.MockTokenMockRecorder) {
+				userInfo := user.DefaultInfo{
+					Name:   "test-user",
+					Groups: []string{"test-group-1", "test-group-2"},
+					UID:    "test-uid",
+				}
+				response := &authenticator.Response{User: &userInfo}
+				recorder.AuthenticateToken(gomock.Any(), "test-token").Return(response, true, nil)
+			},
+			wantHTTPBody:   "successful proxied response",
+			wantHTTPStatus: http.StatusOK,
+			wantLogs:       []string{"\"level\"=0 \"msg\"=\"proxying authenticated request\" \"authenticator\"={\"apiGroup\":\"authentication.concierge.pinniped.dev\",\"kind\":\"\",\"name\":\"authenticator-one\"} \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\" \"userID\"=\"test-uid\""},
 		},
 	}
 
@@ -187,11 +245,19 @@ func TestImpersonator(t *testing.T) {
 		tt := tt
 		testLog := testlogger.New(t)
 		t.Run(tt.name, func(t *testing.T) {
+			defer func() {
+				if t.Failed() {
+					for i, line := range testLog.Lines() {
+						t.Logf("testLog line %d: %q", i+1, line)
+					}
+				}
+			}()
+
 			// stole this from cache_test, hopefully it is sufficient
 			cacheWithMockAuthenticator := authncache.New()
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
-			key := authncache.Key{Name: "authenticator-one"}
+			key := authncache.Key{Name: "authenticator-one", APIGroup: *goodAuthenticator.APIGroup}
 			mockToken := mocktokenauthenticator.NewMockToken(ctrl)
 			cacheWithMockAuthenticator.Store(key, mockToken)
 
@@ -199,7 +265,12 @@ func TestImpersonator(t *testing.T) {
 				tt.expectMockToken(t, mockToken.EXPECT())
 			}
 
-			proxy, err := newInternal(cacheWithMockAuthenticator, testLog, tt.getKubeconfig)
+			apiGroup := defaultAPIGroup
+			if tt.apiGroupOverride != "" {
+				apiGroup = tt.apiGroupOverride
+			}
+
+			proxy, err := newInternal(cacheWithMockAuthenticator, makeDecoder(t, apiGroup), testLog, tt.getKubeconfig)
 			if tt.wantCreationErr != "" {
 				require.EqualError(t, err, tt.wantCreationErr)
 				return
@@ -223,22 +294,21 @@ func TestImpersonator(t *testing.T) {
 	}
 }
 
-func makeTestTokenRequest(namespace string, name string, token string) string {
+func stringPtr(s string) *string { return &s }
-	reqJSON, err := json.Marshal(&loginv1alpha1.TokenCredentialRequest{
+
-		ObjectMeta: metav1.ObjectMeta{
+func makeDecoder(t *testing.T, apiGroupSuffix string) runtime.Decoder {
-			Namespace: namespace,
+	t.Helper()
-		},
+
-		TypeMeta: metav1.TypeMeta{
+	loginConciergeGroupName, ok := groupsuffix.Replace(login.GroupName, apiGroupSuffix)
-			Kind:       "TokenCredentialRequest",
+	require.True(t, ok, "couldn't replace suffix of %q with %q", login.GroupName, apiGroupSuffix)
-			APIVersion: loginv1alpha1.GroupName + "/v1alpha1",
+
-		},
+	scheme := conciergescheme.New(loginConciergeGroupName, apiGroupSuffix)
-		Spec: loginv1alpha1.TokenCredentialRequestSpec{
+	codecs := serializer.NewCodecFactory(scheme)
-			Token:         token,
+	respInfo, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), runtime.ContentTypeJSON)
-			Authenticator: corev1.TypedLocalObjectReference{Name: name},
+	require.True(t, ok, "couldn't find serializer info for media type")
-		},
+
+	return codecs.DecoderToVersion(respInfo.Serializer, schema.GroupVersion{
+		Group:   loginConciergeGroupName,
+		Version: login.SchemeGroupVersion.Version,
 	})
-	if err != nil {
-		panic(err)
-	}
-	return base64.RawURLEncoding.EncodeToString(reqJSON)
 }

internal/concierge/scheme/scheme.go

@@ -0,0 +1,113 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package scheme contains code to construct a proper runtime.Scheme for the Concierge aggregated
+// API.
+package scheme
+
+import (
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+
+	loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login"
+	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
+	"go.pinniped.dev/internal/groupsuffix"
+	"go.pinniped.dev/internal/plog"
+)
+
+// New returns a runtime.Scheme for use by the Concierge aggregated API. The provided
+// loginConciergeAPIGroup should be the API group that the Concierge is serving (e.g.,
+// login.concierge.pinniped.dev, login.concierge.walrus.tld, etc.). The provided apiGroupSuffix is
+// the API group suffix of the provided loginConciergeAPIGroup (e.g., pinniped.dev, walrus.tld,
+// etc.).
+func New(loginConciergeAPIGroup, apiGroupSuffix string) *runtime.Scheme {
+	// standard set up of the server side scheme
+	scheme := runtime.NewScheme()
+
+	// add the options to empty v1
+	metav1.AddToGroupVersion(scheme, metav1.Unversioned)
+
+	// nothing fancy is required if using the standard group
+	if loginConciergeAPIGroup == loginv1alpha1.GroupName {
+		utilruntime.Must(loginv1alpha1.AddToScheme(scheme))
+		utilruntime.Must(loginapi.AddToScheme(scheme))
+		return scheme
+	}
+
+	// we need a temporary place to register our types to avoid double registering them
+	tmpScheme := runtime.NewScheme()
+	utilruntime.Must(loginv1alpha1.AddToScheme(tmpScheme))
+	utilruntime.Must(loginapi.AddToScheme(tmpScheme))
+
+	for gvk := range tmpScheme.AllKnownTypes() {
+		if gvk.GroupVersion() == metav1.Unversioned {
+			continue // metav1.AddToGroupVersion registers types outside of our aggregated API group that we need to ignore
+		}
+
+		if gvk.Group != loginv1alpha1.GroupName {
+			panic("tmp scheme has types not in the aggregated API group: " + gvk.Group) // programmer error
+		}
+
+		obj, err := tmpScheme.New(gvk)
+		if err != nil {
+			panic(err) // programmer error, scheme internal code is broken
+		}
+		newGVK := schema.GroupVersionKind{
+			Group:   loginConciergeAPIGroup,
+			Version: gvk.Version,
+			Kind:    gvk.Kind,
+		}
+
+		// register the existing type but with the new group in the correct scheme
+		scheme.AddKnownTypeWithName(newGVK, obj)
+	}
+
+	// manually register conversions and defaulting into the correct scheme since we cannot directly call loginv1alpha1.AddToScheme
+	utilruntime.Must(loginv1alpha1.RegisterConversions(scheme))
+	utilruntime.Must(loginv1alpha1.RegisterDefaults(scheme))
+
+	// we do not want to return errors from the scheme and instead would prefer to defer
+	// to the REST storage layer for consistency.  The simplest way to do this is to force
+	// a cache miss from the authenticator cache.  Kube API groups are validated via the
+	// IsDNS1123Subdomain func thus we can easily create a group that is guaranteed never
+	// to be in the authenticator cache.  Add a timestamp just to be extra sure.
+	const authenticatorCacheMissPrefix = "_INVALID_API_GROUP_"
+	authenticatorCacheMiss := authenticatorCacheMissPrefix + time.Now().UTC().String()
+
+	// we do not have any defaulting functions for *loginv1alpha1.TokenCredentialRequest
+	// today, but we may have some in the future.  Calling AddTypeDefaultingFunc overwrites
+	// any previously registered defaulting function.  Thus to make sure that we catch
+	// a situation where we add a defaulting func, we attempt to call it here with a nil
+	// *loginv1alpha1.TokenCredentialRequest.  This will do nothing when there is no
+	// defaulting func registered, but it will almost certainly panic if one is added.
+	scheme.Default((*loginv1alpha1.TokenCredentialRequest)(nil))
+
+	// on incoming requests, restore the authenticator API group to the standard group
+	// note that we are responsible for duplicating this logic for every external API version
+	scheme.AddTypeDefaultingFunc(&loginv1alpha1.TokenCredentialRequest{}, func(obj interface{}) {
+		credentialRequest := obj.(*loginv1alpha1.TokenCredentialRequest)
+
+		if credentialRequest.Spec.Authenticator.APIGroup == nil {
+			// force a cache miss because this is an invalid request
+			plog.Debug("invalid token credential request, nil group", "authenticator", credentialRequest.Spec.Authenticator)
+			credentialRequest.Spec.Authenticator.APIGroup = &authenticatorCacheMiss
+			return
+		}
+
+		restoredGroup, ok := groupsuffix.Unreplace(*credentialRequest.Spec.Authenticator.APIGroup, apiGroupSuffix)
+		if !ok {
+			// force a cache miss because this is an invalid request
+			plog.Debug("invalid token credential request, wrong group", "authenticator", credentialRequest.Spec.Authenticator)
+			credentialRequest.Spec.Authenticator.APIGroup = &authenticatorCacheMiss
+			return
+		}
+
+		credentialRequest.Spec.Authenticator.APIGroup = &restoredGroup
+	})
+
+	return scheme
+}

internal/concierge/scheme/scheme_test.go

@@ -0,0 +1,184 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package scheme
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
+	loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login"
+	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
+	"go.pinniped.dev/internal/groupsuffix"
+)
+
+func TestNew(t *testing.T) {
+	// the standard group
+	regularGV := schema.GroupVersion{
+		Group:   "login.concierge.pinniped.dev",
+		Version: "v1alpha1",
+	}
+	regularGVInternal := schema.GroupVersion{
+		Group:   "login.concierge.pinniped.dev",
+		Version: runtime.APIVersionInternal,
+	}
+
+	// the canonical other group
+	otherGV := schema.GroupVersion{
+		Group:   "login.concierge.walrus.tld",
+		Version: "v1alpha1",
+	}
+	otherGVInternal := schema.GroupVersion{
+		Group:   "login.concierge.walrus.tld",
+		Version: runtime.APIVersionInternal,
+	}
+
+	// kube's core internal
+	internalGV := schema.GroupVersion{
+		Group:   "",
+		Version: runtime.APIVersionInternal,
+	}
+
+	tests := []struct {
+		name           string
+		apiGroupSuffix string
+		want           map[schema.GroupVersionKind]reflect.Type
+	}{
+		{
+			name:           "regular api group",
+			apiGroupSuffix: "pinniped.dev",
+			want: map[schema.GroupVersionKind]reflect.Type{
+				// all the types that are in the aggregated API group
+
+				regularGV.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginv1alpha1.TokenCredentialRequest{}).Elem(),
+				regularGV.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginv1alpha1.TokenCredentialRequestList{}).Elem(),
+
+				regularGVInternal.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginapi.TokenCredentialRequest{}).Elem(),
+				regularGVInternal.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginapi.TokenCredentialRequestList{}).Elem(),
+
+				regularGV.WithKind("CreateOptions"): reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
+				regularGV.WithKind("DeleteOptions"): reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
+				regularGV.WithKind("ExportOptions"): reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
+				regularGV.WithKind("GetOptions"):    reflect.TypeOf(&metav1.GetOptions{}).Elem(),
+				regularGV.WithKind("ListOptions"):   reflect.TypeOf(&metav1.ListOptions{}).Elem(),
+				regularGV.WithKind("PatchOptions"):  reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
+				regularGV.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
+				regularGV.WithKind("WatchEvent"):    reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
+
+				regularGVInternal.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
+
+				// the types below this line do not really matter to us because they are in the core group
+
+				internalGV.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
+
+				metav1.Unversioned.WithKind("APIGroup"):        reflect.TypeOf(&metav1.APIGroup{}).Elem(),
+				metav1.Unversioned.WithKind("APIGroupList"):    reflect.TypeOf(&metav1.APIGroupList{}).Elem(),
+				metav1.Unversioned.WithKind("APIResourceList"): reflect.TypeOf(&metav1.APIResourceList{}).Elem(),
+				metav1.Unversioned.WithKind("APIVersions"):     reflect.TypeOf(&metav1.APIVersions{}).Elem(),
+				metav1.Unversioned.WithKind("CreateOptions"):   reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
+				metav1.Unversioned.WithKind("DeleteOptions"):   reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
+				metav1.Unversioned.WithKind("ExportOptions"):   reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
+				metav1.Unversioned.WithKind("GetOptions"):      reflect.TypeOf(&metav1.GetOptions{}).Elem(),
+				metav1.Unversioned.WithKind("ListOptions"):     reflect.TypeOf(&metav1.ListOptions{}).Elem(),
+				metav1.Unversioned.WithKind("PatchOptions"):    reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
+				metav1.Unversioned.WithKind("Status"):          reflect.TypeOf(&metav1.Status{}).Elem(),
+				metav1.Unversioned.WithKind("UpdateOptions"):   reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
+				metav1.Unversioned.WithKind("WatchEvent"):      reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
+			},
+		},
+		{
+			name:           "other api group",
+			apiGroupSuffix: "walrus.tld",
+			want: map[schema.GroupVersionKind]reflect.Type{
+				// all the types that are in the aggregated API group
+
+				otherGV.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginv1alpha1.TokenCredentialRequest{}).Elem(),
+				otherGV.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginv1alpha1.TokenCredentialRequestList{}).Elem(),
+
+				otherGVInternal.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginapi.TokenCredentialRequest{}).Elem(),
+				otherGVInternal.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginapi.TokenCredentialRequestList{}).Elem(),
+
+				otherGV.WithKind("CreateOptions"): reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
+				otherGV.WithKind("DeleteOptions"): reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
+				otherGV.WithKind("ExportOptions"): reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
+				otherGV.WithKind("GetOptions"):    reflect.TypeOf(&metav1.GetOptions{}).Elem(),
+				otherGV.WithKind("ListOptions"):   reflect.TypeOf(&metav1.ListOptions{}).Elem(),
+				otherGV.WithKind("PatchOptions"):  reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
+				otherGV.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
+				otherGV.WithKind("WatchEvent"):    reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
+
+				otherGVInternal.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
+
+				// the types below this line do not really matter to us because they are in the core group
+
+				internalGV.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
+
+				metav1.Unversioned.WithKind("APIGroup"):        reflect.TypeOf(&metav1.APIGroup{}).Elem(),
+				metav1.Unversioned.WithKind("APIGroupList"):    reflect.TypeOf(&metav1.APIGroupList{}).Elem(),
+				metav1.Unversioned.WithKind("APIResourceList"): reflect.TypeOf(&metav1.APIResourceList{}).Elem(),
+				metav1.Unversioned.WithKind("APIVersions"):     reflect.TypeOf(&metav1.APIVersions{}).Elem(),
+				metav1.Unversioned.WithKind("CreateOptions"):   reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
+				metav1.Unversioned.WithKind("DeleteOptions"):   reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
+				metav1.Unversioned.WithKind("ExportOptions"):   reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
+				metav1.Unversioned.WithKind("GetOptions"):      reflect.TypeOf(&metav1.GetOptions{}).Elem(),
+				metav1.Unversioned.WithKind("ListOptions"):     reflect.TypeOf(&metav1.ListOptions{}).Elem(),
+				metav1.Unversioned.WithKind("PatchOptions"):    reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
+				metav1.Unversioned.WithKind("Status"):          reflect.TypeOf(&metav1.Status{}).Elem(),
+				metav1.Unversioned.WithKind("UpdateOptions"):   reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
+				metav1.Unversioned.WithKind("WatchEvent"):      reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			loginConciergeAPIGroup, ok := groupsuffix.Replace("login.concierge.pinniped.dev", tt.apiGroupSuffix)
+			require.True(t, ok)
+
+			scheme := New(loginConciergeAPIGroup, tt.apiGroupSuffix)
+			require.Equal(t, tt.want, scheme.AllKnownTypes())
+
+			// make a credential request like a client would send
+			authenticationConciergeAPIGroup := "authentication.concierge." + tt.apiGroupSuffix
+			credentialRequest := &loginv1alpha1.TokenCredentialRequest{
+				Spec: loginv1alpha1.TokenCredentialRequestSpec{
+					Authenticator: corev1.TypedLocalObjectReference{
+						APIGroup: &authenticationConciergeAPIGroup,
+					},
+				},
+			}
+
+			// run defaulting on it
+			scheme.Default(credentialRequest)
+
+			// make sure the group is restored if needed
+			require.Equal(t, "authentication.concierge.pinniped.dev", *credentialRequest.Spec.Authenticator.APIGroup)
+
+			// make a credential request in the standard group
+			defaultAuthenticationConciergeAPIGroup := "authentication.concierge.pinniped.dev"
+			defaultCredentialRequest := &loginv1alpha1.TokenCredentialRequest{
+				Spec: loginv1alpha1.TokenCredentialRequestSpec{
+					Authenticator: corev1.TypedLocalObjectReference{
+						APIGroup: &defaultAuthenticationConciergeAPIGroup,
+					},
+				},
+			}
+
+			// run defaulting on it
+			scheme.Default(defaultCredentialRequest)
+
+			if tt.apiGroupSuffix == "pinniped.dev" { // when using the standard group, this should just work
+				require.Equal(t, "authentication.concierge.pinniped.dev", *defaultCredentialRequest.Spec.Authenticator.APIGroup)
+			} else { // when using any other group, this should always be a cache miss
+				require.True(t, strings.HasPrefix(*defaultCredentialRequest.Spec.Authenticator.APIGroup, "_INVALID_API_GROUP_2"))
+			}
+		})
+	}
+}

internal/concierge/server/server.go

@@ -11,18 +11,17 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 
-	loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login"
+	"go.pinniped.dev/generated/1.20/apis/concierge/login"
 	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/certauthority/dynamiccertauthority"
 	"go.pinniped.dev/internal/concierge/apiserver"
+	conciergescheme "go.pinniped.dev/internal/concierge/scheme"
 	"go.pinniped.dev/internal/config/concierge"
 	"go.pinniped.dev/internal/controller/authenticator/authncache"
 	"go.pinniped.dev/internal/controllermanager"
@@ -123,6 +122,14 @@ func (a *App) runServer(ctx context.Context) error {
 	// cert issuer used to issue certs to Pinniped clients wishing to login.
 	dynamicSigningCertProvider := dynamiccert.New()
 
+	// Get the "real" name of the login concierge API group (i.e., the API group name with the
+	// injected suffix).
+	loginConciergeAPIGroup, ok := groupsuffix.Replace(loginv1alpha1.GroupName, *cfg.APIGroupSuffix)
+	if !ok {
+		return fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, *cfg.APIGroupSuffix)
+	}
+	loginConciergeScheme := conciergescheme.New(loginConciergeAPIGroup, *cfg.APIGroupSuffix)
+
 	// Prepare to start the controllers, but defer actually starting them until the
 	// post start hook of the aggregated API server.
 	startControllersFunc, err := controllermanager.PrepareControllers(
@@ -138,6 +145,7 @@ func (a *App) runServer(ctx context.Context) error {
 			ServingCertDuration:        time.Duration(*cfg.APIConfig.ServingCertificateConfig.DurationSeconds) * time.Second,
 			ServingCertRenewBefore:     time.Duration(*cfg.APIConfig.ServingCertificateConfig.RenewBeforeSeconds) * time.Second,
 			AuthenticatorCache:         authenticators,
+			LoginJSONDecoder:           getLoginJSONDecoder(loginConciergeAPIGroup, loginConciergeScheme),
 		},
 	)
 	if err != nil {
@@ -150,7 +158,8 @@ func (a *App) runServer(ctx context.Context) error {
 		authenticators,
 		dynamiccertauthority.New(dynamicSigningCertProvider),
 		startControllersFunc,
-		*cfg.APIGroupSuffix,
+		loginConciergeAPIGroup,
+		loginConciergeScheme,
 	)
 	if err != nil {
 		return fmt.Errorf("could not configure aggregated API server: %w", err)
@@ -172,14 +181,10 @@ func getAggregatedAPIServerConfig(
 	authenticator credentialrequest.TokenCredentialRequestAuthenticator,
 	issuer credentialrequest.CertIssuer,
 	startControllersPostStartHook func(context.Context),
-	apiGroupSuffix string,
+	loginConciergeAPIGroup string,
+	loginConciergeScheme *runtime.Scheme,
 ) (*apiserver.Config, error) {
-	loginConciergeAPIGroup, ok := groupsuffix.Replace(loginv1alpha1.GroupName, apiGroupSuffix)
+	scheme := loginConciergeScheme
-	if !ok {
-		return nil, fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, apiGroupSuffix)
-	}
-
-	scheme := getAggregatedAPIServerScheme(loginConciergeAPIGroup, apiGroupSuffix)
 	codecs := serializer.NewCodecFactory(scheme)
 
 	defaultEtcdPathPrefix := fmt.Sprintf("/registry/%s", loginConciergeAPIGroup)
@@ -224,90 +229,15 @@ func getAggregatedAPIServerConfig(
 	return apiServerConfig, nil
 }
 
-func getAggregatedAPIServerScheme(loginConciergeAPIGroup, apiGroupSuffix string) *runtime.Scheme {
+func getLoginJSONDecoder(loginConciergeAPIGroup string, loginConciergeScheme *runtime.Scheme) runtime.Decoder {
-	// standard set up of the server side scheme
+	scheme := loginConciergeScheme
-	scheme := runtime.NewScheme()
+	codecs := serializer.NewCodecFactory(scheme)
-
+	respInfo, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), runtime.ContentTypeJSON)
-	// add the options to empty v1
+	if !ok {
-	metav1.AddToGroupVersion(scheme, metav1.Unversioned)
+		panic(fmt.Errorf("unknown content type: %s ", runtime.ContentTypeJSON)) // static input, programmer error
-
-	// nothing fancy is required if using the standard group
-	if loginConciergeAPIGroup == loginv1alpha1.GroupName {
-		utilruntime.Must(loginv1alpha1.AddToScheme(scheme))
-		utilruntime.Must(loginapi.AddToScheme(scheme))
-		return scheme
 	}
-
+	return codecs.DecoderToVersion(respInfo.Serializer, schema.GroupVersion{
-	// we need a temporary place to register our types to avoid double registering them
+		Group:   loginConciergeAPIGroup,
-	tmpScheme := runtime.NewScheme()
+		Version: login.SchemeGroupVersion.Version,
-	utilruntime.Must(loginv1alpha1.AddToScheme(tmpScheme))
-	utilruntime.Must(loginapi.AddToScheme(tmpScheme))
-
-	for gvk := range tmpScheme.AllKnownTypes() {
-		if gvk.GroupVersion() == metav1.Unversioned {
-			continue // metav1.AddToGroupVersion registers types outside of our aggregated API group that we need to ignore
-		}
-
-		if gvk.Group != loginv1alpha1.GroupName {
-			panic("tmp scheme has types not in the aggregated API group: " + gvk.Group) // programmer error
-		}
-
-		obj, err := tmpScheme.New(gvk)
-		if err != nil {
-			panic(err) // programmer error, scheme internal code is broken
-		}
-		newGVK := schema.GroupVersionKind{
-			Group:   loginConciergeAPIGroup,
-			Version: gvk.Version,
-			Kind:    gvk.Kind,
-		}
-
-		// register the existing type but with the new group in the correct scheme
-		scheme.AddKnownTypeWithName(newGVK, obj)
-	}
-
-	// manually register conversions and defaulting into the correct scheme since we cannot directly call loginv1alpha1.AddToScheme
-	utilruntime.Must(loginv1alpha1.RegisterConversions(scheme))
-	utilruntime.Must(loginv1alpha1.RegisterDefaults(scheme))
-
-	// we do not want to return errors from the scheme and instead would prefer to defer
-	// to the REST storage layer for consistency.  The simplest way to do this is to force
-	// a cache miss from the authenticator cache.  Kube API groups are validated via the
-	// IsDNS1123Subdomain func thus we can easily create a group that is guaranteed never
-	// to be in the authenticator cache.  Add a timestamp just to be extra sure.
-	const authenticatorCacheMissPrefix = "_INVALID_API_GROUP_"
-	authenticatorCacheMiss := authenticatorCacheMissPrefix + time.Now().UTC().String()
-
-	// we do not have any defaulting functions for *loginv1alpha1.TokenCredentialRequest
-	// today, but we may have some in the future.  Calling AddTypeDefaultingFunc overwrites
-	// any previously registered defaulting function.  Thus to make sure that we catch
-	// a situation where we add a defaulting func, we attempt to call it here with a nil
-	// *loginv1alpha1.TokenCredentialRequest.  This will do nothing when there is no
-	// defaulting func registered, but it will almost certainly panic if one is added.
-	scheme.Default((*loginv1alpha1.TokenCredentialRequest)(nil))
-
-	// on incoming requests, restore the authenticator API group to the standard group
-	// note that we are responsible for duplicating this logic for every external API version
-	scheme.AddTypeDefaultingFunc(&loginv1alpha1.TokenCredentialRequest{}, func(obj interface{}) {
-		credentialRequest := obj.(*loginv1alpha1.TokenCredentialRequest)
-
-		if credentialRequest.Spec.Authenticator.APIGroup == nil {
-			// force a cache miss because this is an invalid request
-			plog.Debug("invalid token credential request, nil group", "authenticator", credentialRequest.Spec.Authenticator)
-			credentialRequest.Spec.Authenticator.APIGroup = &authenticatorCacheMiss
-			return
-		}
-
-		restoredGroup, ok := groupsuffix.Unreplace(*credentialRequest.Spec.Authenticator.APIGroup, apiGroupSuffix)
-		if !ok {
-			// force a cache miss because this is an invalid request
-			plog.Debug("invalid token credential request, wrong group", "authenticator", credentialRequest.Spec.Authenticator)
-			credentialRequest.Spec.Authenticator.APIGroup = &authenticatorCacheMiss
-			return
-		}
-
-		credentialRequest.Spec.Authenticator.APIGroup = &restoredGroup
 	})
-
-	return scheme
 }

internal/concierge/server/server_test.go

@@ -6,21 +6,12 @@ package server
 import (
 	"bytes"
 	"context"
-	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-
-	loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login"
-	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
-	"go.pinniped.dev/internal/groupsuffix"
 )
 
 const knownGoodUsage = `
@@ -97,167 +88,3 @@ func TestCommand(t *testing.T) {
 		})
 	}
 }
-
-func Test_getAggregatedAPIServerScheme(t *testing.T) {
-	// the standard group
-	regularGV := schema.GroupVersion{
-		Group:   "login.concierge.pinniped.dev",
-		Version: "v1alpha1",
-	}
-	regularGVInternal := schema.GroupVersion{
-		Group:   "login.concierge.pinniped.dev",
-		Version: runtime.APIVersionInternal,
-	}
-
-	// the canonical other group
-	otherGV := schema.GroupVersion{
-		Group:   "login.concierge.walrus.tld",
-		Version: "v1alpha1",
-	}
-	otherGVInternal := schema.GroupVersion{
-		Group:   "login.concierge.walrus.tld",
-		Version: runtime.APIVersionInternal,
-	}
-
-	// kube's core internal
-	internalGV := schema.GroupVersion{
-		Group:   "",
-		Version: runtime.APIVersionInternal,
-	}
-
-	tests := []struct {
-		name           string
-		apiGroupSuffix string
-		want           map[schema.GroupVersionKind]reflect.Type
-	}{
-		{
-			name:           "regular api group",
-			apiGroupSuffix: "pinniped.dev",
-			want: map[schema.GroupVersionKind]reflect.Type{
-				// all the types that are in the aggregated API group
-
-				regularGV.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginv1alpha1.TokenCredentialRequest{}).Elem(),
-				regularGV.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginv1alpha1.TokenCredentialRequestList{}).Elem(),
-
-				regularGVInternal.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginapi.TokenCredentialRequest{}).Elem(),
-				regularGVInternal.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginapi.TokenCredentialRequestList{}).Elem(),
-
-				regularGV.WithKind("CreateOptions"): reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
-				regularGV.WithKind("DeleteOptions"): reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
-				regularGV.WithKind("ExportOptions"): reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
-				regularGV.WithKind("GetOptions"):    reflect.TypeOf(&metav1.GetOptions{}).Elem(),
-				regularGV.WithKind("ListOptions"):   reflect.TypeOf(&metav1.ListOptions{}).Elem(),
-				regularGV.WithKind("PatchOptions"):  reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
-				regularGV.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
-				regularGV.WithKind("WatchEvent"):    reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
-
-				regularGVInternal.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
-
-				// the types below this line do not really matter to us because they are in the core group
-
-				internalGV.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
-
-				metav1.Unversioned.WithKind("APIGroup"):        reflect.TypeOf(&metav1.APIGroup{}).Elem(),
-				metav1.Unversioned.WithKind("APIGroupList"):    reflect.TypeOf(&metav1.APIGroupList{}).Elem(),
-				metav1.Unversioned.WithKind("APIResourceList"): reflect.TypeOf(&metav1.APIResourceList{}).Elem(),
-				metav1.Unversioned.WithKind("APIVersions"):     reflect.TypeOf(&metav1.APIVersions{}).Elem(),
-				metav1.Unversioned.WithKind("CreateOptions"):   reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
-				metav1.Unversioned.WithKind("DeleteOptions"):   reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
-				metav1.Unversioned.WithKind("ExportOptions"):   reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
-				metav1.Unversioned.WithKind("GetOptions"):      reflect.TypeOf(&metav1.GetOptions{}).Elem(),
-				metav1.Unversioned.WithKind("ListOptions"):     reflect.TypeOf(&metav1.ListOptions{}).Elem(),
-				metav1.Unversioned.WithKind("PatchOptions"):    reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
-				metav1.Unversioned.WithKind("Status"):          reflect.TypeOf(&metav1.Status{}).Elem(),
-				metav1.Unversioned.WithKind("UpdateOptions"):   reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
-				metav1.Unversioned.WithKind("WatchEvent"):      reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
-			},
-		},
-		{
-			name:           "other api group",
-			apiGroupSuffix: "walrus.tld",
-			want: map[schema.GroupVersionKind]reflect.Type{
-				// all the types that are in the aggregated API group
-
-				otherGV.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginv1alpha1.TokenCredentialRequest{}).Elem(),
-				otherGV.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginv1alpha1.TokenCredentialRequestList{}).Elem(),
-
-				otherGVInternal.WithKind("TokenCredentialRequest"):     reflect.TypeOf(&loginapi.TokenCredentialRequest{}).Elem(),
-				otherGVInternal.WithKind("TokenCredentialRequestList"): reflect.TypeOf(&loginapi.TokenCredentialRequestList{}).Elem(),
-
-				otherGV.WithKind("CreateOptions"): reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
-				otherGV.WithKind("DeleteOptions"): reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
-				otherGV.WithKind("ExportOptions"): reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
-				otherGV.WithKind("GetOptions"):    reflect.TypeOf(&metav1.GetOptions{}).Elem(),
-				otherGV.WithKind("ListOptions"):   reflect.TypeOf(&metav1.ListOptions{}).Elem(),
-				otherGV.WithKind("PatchOptions"):  reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
-				otherGV.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
-				otherGV.WithKind("WatchEvent"):    reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
-
-				otherGVInternal.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
-
-				// the types below this line do not really matter to us because they are in the core group
-
-				internalGV.WithKind("WatchEvent"): reflect.TypeOf(&metav1.InternalEvent{}).Elem(),
-
-				metav1.Unversioned.WithKind("APIGroup"):        reflect.TypeOf(&metav1.APIGroup{}).Elem(),
-				metav1.Unversioned.WithKind("APIGroupList"):    reflect.TypeOf(&metav1.APIGroupList{}).Elem(),
-				metav1.Unversioned.WithKind("APIResourceList"): reflect.TypeOf(&metav1.APIResourceList{}).Elem(),
-				metav1.Unversioned.WithKind("APIVersions"):     reflect.TypeOf(&metav1.APIVersions{}).Elem(),
-				metav1.Unversioned.WithKind("CreateOptions"):   reflect.TypeOf(&metav1.CreateOptions{}).Elem(),
-				metav1.Unversioned.WithKind("DeleteOptions"):   reflect.TypeOf(&metav1.DeleteOptions{}).Elem(),
-				metav1.Unversioned.WithKind("ExportOptions"):   reflect.TypeOf(&metav1.ExportOptions{}).Elem(),
-				metav1.Unversioned.WithKind("GetOptions"):      reflect.TypeOf(&metav1.GetOptions{}).Elem(),
-				metav1.Unversioned.WithKind("ListOptions"):     reflect.TypeOf(&metav1.ListOptions{}).Elem(),
-				metav1.Unversioned.WithKind("PatchOptions"):    reflect.TypeOf(&metav1.PatchOptions{}).Elem(),
-				metav1.Unversioned.WithKind("Status"):          reflect.TypeOf(&metav1.Status{}).Elem(),
-				metav1.Unversioned.WithKind("UpdateOptions"):   reflect.TypeOf(&metav1.UpdateOptions{}).Elem(),
-				metav1.Unversioned.WithKind("WatchEvent"):      reflect.TypeOf(&metav1.WatchEvent{}).Elem(),
-			},
-		},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			loginConciergeAPIGroup, ok := groupsuffix.Replace("login.concierge.pinniped.dev", tt.apiGroupSuffix)
-			require.True(t, ok)
-
-			scheme := getAggregatedAPIServerScheme(loginConciergeAPIGroup, tt.apiGroupSuffix)
-			require.Equal(t, tt.want, scheme.AllKnownTypes())
-
-			// make a credential request like a client would send
-			authenticationConciergeAPIGroup := "authentication.concierge." + tt.apiGroupSuffix
-			credentialRequest := &loginv1alpha1.TokenCredentialRequest{
-				Spec: loginv1alpha1.TokenCredentialRequestSpec{
-					Authenticator: corev1.TypedLocalObjectReference{
-						APIGroup: &authenticationConciergeAPIGroup,
-					},
-				},
-			}
-
-			// run defaulting on it
-			scheme.Default(credentialRequest)
-
-			// make sure the group is restored if needed
-			require.Equal(t, "authentication.concierge.pinniped.dev", *credentialRequest.Spec.Authenticator.APIGroup)
-
-			// make a credential request in the standard group
-			defaultAuthenticationConciergeAPIGroup := "authentication.concierge.pinniped.dev"
-			defaultCredentialRequest := &loginv1alpha1.TokenCredentialRequest{
-				Spec: loginv1alpha1.TokenCredentialRequestSpec{
-					Authenticator: corev1.TypedLocalObjectReference{
-						APIGroup: &defaultAuthenticationConciergeAPIGroup,
-					},
-				},
-			}
-
-			// run defaulting on it
-			scheme.Default(defaultCredentialRequest)
-
-			if tt.apiGroupSuffix == "pinniped.dev" { // when using the standard group, this should just work
-				require.Equal(t, "authentication.concierge.pinniped.dev", *defaultCredentialRequest.Spec.Authenticator.APIGroup)
-			} else { // when using any other group, this should always be a cache miss
-				require.True(t, strings.HasPrefix(*defaultCredentialRequest.Spec.Authenticator.APIGroup, "_INVALID_API_GROUP_2"))
-			}
-		})
-	}
-}

internal/controllermanager/prepare_controllers.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"time"
 
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/clock"
 	k8sinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
@@ -82,6 +83,10 @@ type Config struct {
 	// AuthenticatorCache is a cache of authenticators shared amongst various authenticated-related controllers.
 	AuthenticatorCache *authncache.Cache
 
+	// LoginJSONDecoder can decode login.concierge.pinniped.dev types (e.g., TokenCredentialRequest)
+	// into their internal representation.
+	LoginJSONDecoder runtime.Decoder
+
 	// Labels are labels that should be added to any resources created by the controllers.
 	Labels map[string]string
 }
@@ -289,7 +294,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) {
 				"pinniped-concierge-impersonation-proxy-load-balancer", // TODO this string should come from `c.NamesConfig`
 				tls.Listen,
 				func() (http.Handler, error) {
-					impersonationProxyHandler, err := impersonator.New(c.AuthenticatorCache, klogr.New().WithName("impersonation-proxy"))
+					impersonationProxyHandler, err := impersonator.New(c.AuthenticatorCache, c.LoginJSONDecoder, klogr.New().WithName("impersonation-proxy"))
 					if err != nil {
 						return nil, fmt.Errorf("could not create impersonation proxy: %w", err)
 					}

internal/testutil/impersonationtoken/impersonationtoken.go

@@ -0,0 +1,67 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package impersonationtoken contains a test utility to generate a token to be used against our
+// impersonation proxy.
+//
+// It is its own package to fix import cycles involving concierge/scheme, testutil, and groupsuffix.
+package impersonationtoken
+
+import (
+	"encoding/base64"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+
+	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
+	conciergescheme "go.pinniped.dev/internal/concierge/scheme"
+	"go.pinniped.dev/internal/groupsuffix"
+)
+
+func Make(
+	t *testing.T,
+	token string,
+	authenticator *corev1.TypedLocalObjectReference,
+	apiGroupSuffix string,
+) string {
+	t.Helper()
+
+	// The impersonation test token should be a base64-encoded TokenCredentialRequest object.  The API
+	// group of the TokenCredentialRequest object, and its Spec.Authenticator, should match whatever
+	// is installed on the cluster. This API group is usually replaced by the kubeclient middleware,
+	// but this object is not touched by the middleware since it is in a HTTP header. Therefore, we
+	// need to make a manual edit here.
+	loginConciergeGroupName, ok := groupsuffix.Replace(loginv1alpha1.GroupName, apiGroupSuffix)
+	require.True(t, ok, "couldn't replace suffix of %q with %q", loginv1alpha1.GroupName, apiGroupSuffix)
+	tokenCredentialRequest := loginv1alpha1.TokenCredentialRequest{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "TokenCredentialRequest",
+			APIVersion: loginConciergeGroupName + "/v1alpha1",
+		},
+		Spec: loginv1alpha1.TokenCredentialRequestSpec{
+			Token:         token,
+			Authenticator: *authenticator.DeepCopy(),
+		},
+	}
+
+	// It is assumed that the provided authenticator uses the default pinniped.dev API group, since
+	// this is usually replaced by the kubeclient middleware. Since we are not going through the
+	// kubeclient middleware, we need to make this replacement ourselves.
+	require.NotNil(t, tokenCredentialRequest.Spec.Authenticator.APIGroup, "expected authenticator to have non-nil API group")
+	authenticatorAPIGroup, ok := groupsuffix.Replace(*tokenCredentialRequest.Spec.Authenticator.APIGroup, apiGroupSuffix)
+	require.True(t, ok, "couldn't replace suffix of %q with %q", *tokenCredentialRequest.Spec.Authenticator.APIGroup, apiGroupSuffix)
+	tokenCredentialRequest.Spec.Authenticator.APIGroup = &authenticatorAPIGroup
+
+	scheme := conciergescheme.New(loginConciergeGroupName, apiGroupSuffix)
+	codecs := serializer.NewCodecFactory(scheme)
+	respInfo, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), runtime.ContentTypeJSON)
+	require.True(t, ok, "couldn't find serializer info for media type")
+
+	reqJSON, err := runtime.Encode(respInfo.PrettySerializer, &tokenCredentialRequest)
+	require.NoError(t, err)
+	return base64.RawURLEncoding.EncodeToString(reqJSON)
+}

test/integration/concierge_impersonation_proxy_test.go

@@ -5,8 +5,6 @@ package integration
 
 import (
 	"context"
-	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -20,8 +18,8 @@ import (
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/yaml"
 
-	loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/concierge/impersonator"
+	"go.pinniped.dev/internal/testutil/impersonationtoken"
 	"go.pinniped.dev/test/library"
 )
 
@@ -49,7 +47,7 @@ func TestImpersonationProxy(t *testing.T) {
 	kubeconfig := &rest.Config{
 		Host:            proxyServiceURL,
 		TLSClientConfig: rest.TLSClientConfig{Insecure: true},
-		BearerToken:     makeImpersonationTestToken(t, authenticator),
+		BearerToken:     impersonationtoken.Make(t, env.TestUser.Token, &authenticator, env.APIGroupSuffix),
 		Proxy: func(req *http.Request) (*url.URL, error) {
 			proxyURL, err := url.Parse(env.Proxy)
 			require.NoError(t, err)
@@ -143,24 +141,3 @@ func hasLoadBalancerService(ctx context.Context, t *testing.T, client kubernetes
 	}
 	return false
 }
-
-func makeImpersonationTestToken(t *testing.T, authenticator corev1.TypedLocalObjectReference) string {
-	t.Helper()
-
-	env := library.IntegrationEnv(t)
-	reqJSON, err := json.Marshal(&loginv1alpha1.TokenCredentialRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: env.ConciergeNamespace,
-		},
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "TokenCredentialRequest",
-			APIVersion: loginv1alpha1.GroupName + "/v1alpha1",
-		},
-		Spec: loginv1alpha1.TokenCredentialRequestSpec{
-			Token:         env.TestUser.Token,
-			Authenticator: authenticator,
-		},
-	})
-	require.NoError(t, err)
-	return base64.RawURLEncoding.EncodeToString(reqJSON)
-}