From a8487b78c9838825e068b03cd881e1a360afd45e Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 16 Sep 2020 14:57:18 -0500 Subject: [PATCH] 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 --- internal/apiserver/apiserver.go | 41 +++---- .../registry/credentialrequest/conversions.go | 54 +++++++++ .../credentialrequest/conversions_test.go | 111 ++++++++++++++++++ internal/registry/credentialrequest/rest.go | 55 ++++++--- .../registry/credentialrequest/rest_test.go | 60 +++++++++- 5 files changed, 280 insertions(+), 41 deletions(-) create mode 100644 internal/registry/credentialrequest/conversions.go create mode 100644 internal/registry/credentialrequest/conversions_test.go diff --git a/internal/apiserver/apiserver.go b/internal/apiserver/apiserver.go index 822dbdb6..146bbb0d 100644 --- a/internal/apiserver/apiserver.go +++ b/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") - - apiGroupInfo := genericapiserver.APIGroupInfo{ - PrioritizedVersions: []schema.GroupVersion{gvr.GroupVersion()}, - VersionedResourcesStorageMap: map[string]map[string]rest.Storage{}, - OptionsExternalVersion: &schema.GroupVersion{Version: "v1"}, - Scheme: scheme, - ParameterCodec: metav1.ParameterCodec, - NegotiatedSerializer: Codecs, - } - - credentialRequestStorage := credentialrequest.NewREST(c.ExtraConfig.TokenAuthenticator, c.ExtraConfig.Issuer) - - v1alpha1Storage, ok := apiGroupInfo.VersionedResourcesStorageMap[gvr.Version] - 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) + restHandler := credentialrequest.NewREST(c.ExtraConfig.TokenAuthenticator, c.ExtraConfig.Issuer) + for gvr, storage := range map[schema.GroupVersionResource]rest.Storage{ + pinnipedv1alpha1.SchemeGroupVersion.WithResource("credentialrequests"): restHandler.PinnipedV1alpha1Storage(), + loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests"): restHandler.LoginV1alpha1Storage(), + } { + if err := s.GenericAPIServer.InstallAPIGroup(&genericapiserver.APIGroupInfo{ + PrioritizedVersions: []schema.GroupVersion{gvr.GroupVersion()}, + VersionedResourcesStorageMap: map[string]map[string]rest.Storage{gvr.Version: {gvr.Resource: storage}}, + OptionsExternalVersion: &schema.GroupVersion{Version: "v1"}, + Scheme: scheme, + ParameterCodec: metav1.ParameterCodec, + NegotiatedSerializer: Codecs, + }); err != nil { + return nil, fmt.Errorf("could not install API group %s: %w", gvr.String(), err) + } } s.GenericAPIServer.AddPostStartHookOrDie("start-controllers", diff --git a/internal/registry/credentialrequest/conversions.go b/internal/registry/credentialrequest/conversions.go new file mode 100644 index 00000000..923a57c5 --- /dev/null +++ b/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 +} diff --git a/internal/registry/credentialrequest/conversions_test.go b/internal/registry/credentialrequest/conversions_test.go new file mode 100644 index 00000000..b909e3ba --- /dev/null +++ b/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))) + }) + }) + } +} diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index 0a37c699..45a49f89 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/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 ( - _ rest.Creater = &REST{} - _ rest.NamespaceScopedStrategy = &REST{} - _ rest.Scoper = &REST{} - _ rest.Storage = &REST{} -) +type Storage interface { + rest.Creater + rest.NamespaceScopedStrategy + rest.Scoper + 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 { - return &pinnipedapi.CredentialRequest{} -} +// PinnipedV1alpha1Storage returns a wrapper of the REST which serves the pinniped.dev/v1alpha1 API. +func (r *REST) PinnipedV1alpha1Storage() Storage { return &oldAPIREST{r} } -func (r *REST) NamespaceScoped() bool { - return false -} +type oldAPIREST struct{ *REST } + +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) { diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index 8199d72e..1238cbc1 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/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{},