Add some conversions to allow our REST handler to handle both old and new credential request APIs.

Eventually we could refactor to remove support for the old APIs, but they are so similar that a single implementation seems to handle both easily.

Signed-off-by: Matt Moyer <moyerm@vmware.com>

internal/apiserver/apiserver.go

@@ -18,6 +18,8 @@ import (
 	"k8s.io/client-go/pkg/version"
 	"k8s.io/klog/v2"
 
+	loginapi "github.com/suzerain-io/pinniped/generated/1.19/apis/login"
+	loginv1alpha1 "github.com/suzerain-io/pinniped/generated/1.19/apis/login/v1alpha1"
 	pinnipedapi "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped"
 	pinnipedv1alpha1 "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped/v1alpha1"
 	"github.com/suzerain-io/pinniped/internal/registry/credentialrequest"
@@ -35,6 +37,8 @@ var (
 func init() {
 	utilruntime.Must(pinnipedv1alpha1.AddToScheme(scheme))
 	utilruntime.Must(pinnipedapi.AddToScheme(scheme))
+	utilruntime.Must(loginv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(loginapi.AddToScheme(scheme))
 
 	// add the options to empty v1
 	metav1.AddToGroupVersion(scheme, schema.GroupVersion{Version: "v1"})
@@ -98,28 +102,21 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 		GenericAPIServer: genericServer,
 	}
 
-	gvr := pinnipedv1alpha1.SchemeGroupVersion.WithResource("credentialrequests")
+	restHandler := credentialrequest.NewREST(c.ExtraConfig.TokenAuthenticator, c.ExtraConfig.Issuer)
-
+	for gvr, storage := range map[schema.GroupVersionResource]rest.Storage{
-	apiGroupInfo := genericapiserver.APIGroupInfo{
+		pinnipedv1alpha1.SchemeGroupVersion.WithResource("credentialrequests"):   restHandler.PinnipedV1alpha1Storage(),
-		PrioritizedVersions:          []schema.GroupVersion{gvr.GroupVersion()},
+		loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests"): restHandler.LoginV1alpha1Storage(),
-		VersionedResourcesStorageMap: map[string]map[string]rest.Storage{},
+	} {
-		OptionsExternalVersion:       &schema.GroupVersion{Version: "v1"},
+		if err := s.GenericAPIServer.InstallAPIGroup(&genericapiserver.APIGroupInfo{
-		Scheme:                       scheme,
+			PrioritizedVersions:          []schema.GroupVersion{gvr.GroupVersion()},
-		ParameterCodec:               metav1.ParameterCodec,
+			VersionedResourcesStorageMap: map[string]map[string]rest.Storage{gvr.Version: {gvr.Resource: storage}},
-		NegotiatedSerializer:         Codecs,
+			OptionsExternalVersion:       &schema.GroupVersion{Version: "v1"},
-	}
+			Scheme:                       scheme,
-
+			ParameterCodec:               metav1.ParameterCodec,
-	credentialRequestStorage := credentialrequest.NewREST(c.ExtraConfig.TokenAuthenticator, c.ExtraConfig.Issuer)
+			NegotiatedSerializer:         Codecs,
-
+		}); err != nil {
-	v1alpha1Storage, ok := apiGroupInfo.VersionedResourcesStorageMap[gvr.Version]
+			return nil, fmt.Errorf("could not install API group %s: %w", gvr.String(), err)
-	if !ok {
+		}
-		v1alpha1Storage = map[string]rest.Storage{}
-	}
-	v1alpha1Storage[gvr.Resource] = credentialRequestStorage
-	apiGroupInfo.VersionedResourcesStorageMap[gvr.Version] = v1alpha1Storage
-
-	if err := s.GenericAPIServer.InstallAPIGroup(&apiGroupInfo); err != nil {
-		return nil, fmt.Errorf("install API group error: %w", err)
 	}
 
 	s.GenericAPIServer.AddPostStartHookOrDie("start-controllers",

internal/registry/credentialrequest/conversions.go

@@ -0,0 +1,54 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package credentialrequest
+
+import (
+	loginapi "github.com/suzerain-io/pinniped/generated/1.19/apis/login"
+	pinnipedapi "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped"
+)
+
+func convertToLoginAPI(input *pinnipedapi.CredentialRequest) *loginapi.TokenCredentialRequest {
+	if input == nil {
+		return nil
+	}
+
+	result := loginapi.TokenCredentialRequest{}
+	result.ObjectMeta = input.ObjectMeta
+	if input.Spec.Token != nil {
+		result.Spec.Token = input.Spec.Token.Value
+	}
+	result.Status.Message = input.Status.Message
+	if input.Status.Credential != nil {
+		result.Status.Credential = &loginapi.ClusterCredential{
+			ExpirationTimestamp:   input.Status.Credential.ExpirationTimestamp,
+			Token:                 input.Status.Credential.Token,
+			ClientCertificateData: input.Status.Credential.ClientCertificateData,
+			ClientKeyData:         input.Status.Credential.ClientKeyData,
+		}
+	}
+	return &result
+}
+
+func convertFromLoginAPI(input *loginapi.TokenCredentialRequest) *pinnipedapi.CredentialRequest {
+	if input == nil {
+		return nil
+	}
+
+	result := pinnipedapi.CredentialRequest{}
+	result.ObjectMeta = input.ObjectMeta
+	if input.Spec.Token != "" {
+		result.Spec.Type = pinnipedapi.TokenCredentialType
+		result.Spec.Token = &pinnipedapi.CredentialRequestTokenCredential{Value: input.Spec.Token}
+	}
+	result.Status.Message = input.Status.Message
+	if input.Status.Credential != nil {
+		result.Status.Credential = &pinnipedapi.CredentialRequestCredential{
+			ExpirationTimestamp:   input.Status.Credential.ExpirationTimestamp,
+			Token:                 input.Status.Credential.Token,
+			ClientCertificateData: input.Status.Credential.ClientCertificateData,
+			ClientKeyData:         input.Status.Credential.ClientKeyData,
+		}
+	}
+	return &result
+}

internal/registry/credentialrequest/conversions_test.go

@@ -0,0 +1,111 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package credentialrequest
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	loginapi "github.com/suzerain-io/pinniped/generated/1.19/apis/login"
+	pinnipedapi "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped"
+)
+
+func TestConversions(t *testing.T) {
+	now := time.Now()
+	errMsg := "some error message"
+
+	tests := []struct {
+		name string
+		new  *loginapi.TokenCredentialRequest
+		old  *pinnipedapi.CredentialRequest
+	}{
+		{
+			name: "nil input",
+		},
+		{
+			name: "usual request",
+			new: &loginapi.TokenCredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Spec: loginapi.TokenCredentialRequestSpec{Token: "test-token"},
+			},
+			old: &pinnipedapi.CredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Spec: pinnipedapi.CredentialRequestSpec{
+					Type:  pinnipedapi.TokenCredentialType,
+					Token: &pinnipedapi.CredentialRequestTokenCredential{Value: "test-token"},
+				},
+			},
+		},
+		{
+			name: "usual response",
+			new: &loginapi.TokenCredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Status: loginapi.TokenCredentialRequestStatus{
+					Credential: &loginapi.ClusterCredential{
+						ExpirationTimestamp:   metav1.NewTime(now),
+						Token:                 "test-cluster-token",
+						ClientCertificateData: "test-cluster-cert",
+						ClientKeyData:         "test-cluster-key",
+					},
+				},
+			},
+			old: &pinnipedapi.CredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Status: pinnipedapi.CredentialRequestStatus{
+					Credential: &pinnipedapi.CredentialRequestCredential{
+						ExpirationTimestamp:   metav1.NewTime(now),
+						Token:                 "test-cluster-token",
+						ClientCertificateData: "test-cluster-cert",
+						ClientKeyData:         "test-cluster-key",
+					},
+				},
+			},
+		},
+		{
+			name: "error response",
+			new: &loginapi.TokenCredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Status: loginapi.TokenCredentialRequestStatus{
+					Message: &errMsg,
+				},
+			},
+			old: &pinnipedapi.CredentialRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-object",
+				},
+				Status: pinnipedapi.CredentialRequestStatus{
+					Message: &errMsg,
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Run("upgrade", func(t *testing.T) {
+				require.Equal(t, tt.new, convertToLoginAPI(tt.old))
+			})
+			t.Run("downgrade", func(t *testing.T) {
+				require.Equal(t, tt.old, convertFromLoginAPI(tt.new))
+			})
+			t.Run("roundtrip", func(t *testing.T) {
+				require.Equal(t, tt.old, convertFromLoginAPI(convertToLoginAPI(tt.old)))
+				require.Equal(t, tt.new, convertToLoginAPI(convertFromLoginAPI(tt.new)))
+			})
+		})
+	}
+}

internal/registry/credentialrequest/rest.go

@@ -18,18 +18,19 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/utils/trace"
 
+	loginapi "github.com/suzerain-io/pinniped/generated/1.19/apis/login"
 	pinnipedapi "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped"
 )
 
 // clientCertificateTTL is the TTL for short-lived client certificates returned by this API.
 const clientCertificateTTL = 1 * time.Hour
 
-var (
+type Storage interface {
-	_ rest.Creater                 = &REST{}
+	rest.Creater
-	_ rest.NamespaceScopedStrategy = &REST{}
+	rest.NamespaceScopedStrategy
-	_ rest.Scoper                  = &REST{}
+	rest.Scoper
-	_ rest.Storage                 = &REST{}
+	rest.Storage
-)
+}
 
 type CertIssuer interface {
 	IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error)
@@ -47,18 +48,38 @@ type REST struct {
 	issuer             CertIssuer
 }
 
-func (r *REST) New() runtime.Object {
+// PinnipedV1alpha1Storage returns a wrapper of the REST which serves the pinniped.dev/v1alpha1 API.
-	return &pinnipedapi.CredentialRequest{}
+func (r *REST) PinnipedV1alpha1Storage() Storage { return &oldAPIREST{r} }
-}
 
-func (r *REST) NamespaceScoped() bool {
+type oldAPIREST struct{ *REST }
-	return false
+
-}
+func (*oldAPIREST) New() runtime.Object { return &pinnipedapi.CredentialRequest{} }
+
+func (*oldAPIREST) NamespaceScoped() bool { return false }
+
+// LoginV1alpha1Storage returns a wrapper of the REST which serves the login.pinniped.dev/v1alpha1 API.
+func (r *REST) LoginV1alpha1Storage() Storage { return &newAPIREST{r} }
+
+type newAPIREST struct{ *REST }
+
+func (*newAPIREST) New() runtime.Object { return &loginapi.TokenCredentialRequest{} }
+
+func (*newAPIREST) NamespaceScoped() bool { return true }
 
 func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	t := trace.FromContext(ctx).Nest("create CredentialRequest")
+	t := trace.FromContext(ctx).Nest("create", trace.Field{
+		Key:   "kind",
+		Value: obj.GetObjectKind().GroupVersionKind().Kind,
+	})
 	defer t.Log()
 
+	// If the incoming request is from the newer version of the API, convert it into the older API and map the result back later.
+	convertResponse := func(in *pinnipedapi.CredentialRequest) runtime.Object { return in }
+	if req, ok := obj.(*loginapi.TokenCredentialRequest); ok {
+		obj = convertFromLoginAPI(req)
+		convertResponse = func(in *pinnipedapi.CredentialRequest) runtime.Object { return convertToLoginAPI(in) }
+	}
+
 	credentialRequest, err := validateRequest(ctx, obj, createValidation, options, t)
 	if err != nil {
 		return nil, err
@@ -79,11 +100,11 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	authResponse, authenticated, err := r.tokenAuthenticator.AuthenticateToken(cancelCtx, credentialRequest.Spec.Token.Value)
 	if err != nil {
 		traceFailureWithError(t, "webhook authentication", err)
-		return failureResponse(), nil
+		return convertResponse(failureResponse()), nil
 	}
 	if !authenticated || authResponse == nil || authResponse.User == nil || authResponse.User.GetName() == "" {
 		traceSuccess(t, authResponse, authenticated, false)
-		return failureResponse(), nil
+		return convertResponse(failureResponse()), nil
 	}
 
 	username := authResponse.User.GetName()
@@ -104,7 +125,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 
 	traceSuccess(t, authResponse, authenticated, true)
 
-	return &pinnipedapi.CredentialRequest{
+	return convertResponse(&pinnipedapi.CredentialRequest{
 		Status: pinnipedapi.CredentialRequestStatus{
 			Credential: &pinnipedapi.CredentialRequestCredential{
 				ExpirationTimestamp:   metav1.NewTime(time.Now().UTC().Add(clientCertificateTTL)),
@@ -112,7 +133,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 				ClientKeyData:         string(keyPEM),
 			},
 		},
-	}, nil
+	}), nil
 }
 
 func validateRequest(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions, t *trace.Trace) (*pinnipedapi.CredentialRequest, error) {

internal/registry/credentialrequest/rest_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/klog/v2"
 
+	loginapi "github.com/suzerain-io/pinniped/generated/1.19/apis/login"
 	pinnipedapi "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped"
 	"github.com/suzerain-io/pinniped/internal/mocks/mockcertissuer"
 	"github.com/suzerain-io/pinniped/internal/testutil"
@@ -123,6 +124,61 @@ func TestCreate(t *testing.T) {
 			requireOneLogStatement(r, logger, `"success" userID:test-user-uid,idpAuthenticated:true`)
 		})
 
+		it("CreateSucceedsWhenGivenANewLoginAPITokenAndTheWebhookAuthenticatesTheToken", func() {
+			webhook := FakeToken{
+				returnResponse: &authenticator.Response{
+					User: &user.DefaultInfo{
+						Name:   "test-user",
+						UID:    "test-user-uid",
+						Groups: []string{"test-group-1", "test-group-2"},
+					},
+				},
+				returnUnauthenticated: false,
+			}
+
+			issuer := mockcertissuer.NewMockCertIssuer(ctrl)
+			issuer.EXPECT().IssuePEM(
+				pkix.Name{
+					CommonName:   "test-user",
+					Organization: []string{"test-group-1", "test-group-2"}},
+				[]string{},
+				1*time.Hour,
+			).Return([]byte("test-cert"), []byte("test-key"), nil)
+
+			storage := NewREST(&webhook, issuer)
+			requestToken := "a token"
+
+			response, err := callCreate(context.Background(), storage, &loginapi.TokenCredentialRequest{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "request name",
+				},
+				Spec: loginapi.TokenCredentialRequestSpec{
+					Token: requestToken,
+				},
+			})
+
+			r.NoError(err)
+			r.IsType(&loginapi.TokenCredentialRequest{}, response)
+
+			expires := response.(*loginapi.TokenCredentialRequest).Status.Credential.ExpirationTimestamp
+			r.NotNil(expires)
+			r.InDelta(time.Now().Add(1*time.Hour).Unix(), expires.Unix(), 5)
+			response.(*loginapi.TokenCredentialRequest).Status.Credential.ExpirationTimestamp = metav1.Time{}
+
+			r.Equal(response, &loginapi.TokenCredentialRequest{
+				Status: loginapi.TokenCredentialRequestStatus{
+					Credential: &loginapi.ClusterCredential{
+						ExpirationTimestamp:   metav1.Time{},
+						ClientCertificateData: "test-cert",
+						ClientKeyData:         "test-key",
+					},
+				},
+			})
+			r.Equal(requestToken, webhook.calledWithToken)
+			requireOneLogStatement(r, logger, `"success" userID:test-user-uid,idpAuthenticated:true`)
+		})
+
 		it("CreateFailsWithValidTokenWhenCertIssuerFails", func() {
 			webhook := FakeToken{
 				returnResponse: &authenticator.Response{
@@ -442,10 +498,10 @@ func requireOneLogStatement(r *require.Assertions, logger *testutil.TranscriptLo
 	r.Contains(transcript[0].Message, messageContains)
 }
 
-func callCreate(ctx context.Context, storage *REST, credentialRequest *pinnipedapi.CredentialRequest) (runtime.Object, error) {
+func callCreate(ctx context.Context, storage *REST, obj runtime.Object) (runtime.Object, error) {
 	return storage.Create(
 		ctx,
-		credentialRequest,
+		obj,
 		rest.ValidateAllObjectFunc,
 		&metav1.CreateOptions{
 			DryRun: []string{},