adding tests

Co-authored-by: Ryan Richard <richardry@vmware.com>
Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>

internal/crud/crud.go

@@ -14,10 +14,8 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	"go.pinniped.dev/internal/constable"
@@ -95,6 +93,8 @@ func (s *secretsStorage) Get(ctx context.Context, signature string, data JSON) (
 	return secret.ResourceVersion, nil
 }
 
+// Update takes a resourceVersion because it assumes Get has been recently called to obtain the latest resource version.
+// This is to ensure that concurrent edits are treated as conflict errors (only one will win).
 func (s *secretsStorage) Update(ctx context.Context, signature, resourceVersion string, data JSON) (string, error) {
 	secret, err := s.toSecret(signature, resourceVersion, data, nil, nil)
 	if err != nil {
@@ -105,11 +105,6 @@ func (s *secretsStorage) Update(ctx context.Context, signature, resourceVersion
 	if err != nil {
 		return "", fmt.Errorf("failed to get %s for signature %s: %w", s.resource, signature, err)
 	}
-	// do not assume that our secret client does live reads
-	if oldSecret.ResourceVersion != resourceVersion {
-		return "", errors.NewConflict(schema.GroupResource{Resource: "Secret"}, secret.Name,
-			fmt.Errorf("resource version %s does not match expected value: %s", oldSecret.ResourceVersion, resourceVersion))
-	}
 
 	// preserve these fields - they are effectively immutable on update
 	secret.Labels = oldSecret.Labels

internal/crud/crud_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/ory/fosite/compose"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -64,6 +65,7 @@ func TestStorage(t *testing.T) {
 		mocks       func(*testing.T, mocker)
 		lifetime    func() time.Duration
 		run         func(*testing.T, Storage, *clocktesting.FakeClock) error
+		useNilClock bool
 		wantActions []coretesting.Action
 		wantSecrets []corev1.Secret
 		wantErr     string
@@ -270,69 +272,129 @@ func TestStorage(t *testing.T) {
 			wantErr: "",
 		},
 		{
-			name:     "create and get with additional labels",
+			name:     "create and get and update with additional labels, annotations, and ownerRefs",
-			resource: "access-tokens",
+			resource: "kittens",
-			mocks:    nil,
+			mocks: func(t *testing.T, mock mocker) {
+				mock.PrependReactor("create", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+					secret := action.(coretesting.UpdateAction).GetObject().(*corev1.Secret)
+					secret.ResourceVersion = "1"
+					return false, nil, nil
+				})
+
+				mock.PrependReactor("update", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+					secret := action.(coretesting.UpdateAction).GetObject().(*corev1.Secret)
+					secret.ResourceVersion = "45"
+					return false, nil, nil
+				})
+			},
 			run: func(t *testing.T, storage Storage, fakeClock *clocktesting.FakeClock) error {
 				signature := hmac.AuthorizeCodeSignature(authorizationCode1)
 				require.NotEmpty(t, signature)
 				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
 
 				data := &testJSON{Data: "create-and-get"}
-				rv1, err := storage.Create(ctx, signature, data, map[string]string{"label1": "value1", "label2": "value2"}, nil)
+				rv1, err := storage.Create(ctx, signature, data, map[string]string{"label1": "value1", "label2": "value2"}, []metav1.OwnerReference{{
-				require.Empty(t, rv1) // fake client does not set this
+					APIVersion: "some-api-version",
+					Kind:       "some-kind",
+					Name:       "some-owner",
+					UID:        "123",
+				}})
+				require.Equal(t, "1", rv1)
 				require.NoError(t, err)
 
 				out := &testJSON{}
 				rv2, err := storage.Get(ctx, signature, out)
-				require.Empty(t, rv2) // fake client does not set this
+				require.Equal(t, "1", rv2)
 				require.NoError(t, err)
 				require.Equal(t, data, out)
 
+				newData := &testJSON{Data: "performed-an-update"}
+				rv3, err := storage.Update(ctx, signature, rv2, newData)
+				require.Equal(t, "45", rv3)
+				require.NoError(t, err)
+
 				return nil
 			},
 			wantActions: []coretesting.Action{
 				coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						Name:            "pinniped-storage-kittens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
 						ResourceVersion: "",
 						Labels: map[string]string{
-							"storage.pinniped.dev/type": "access-tokens",
+							"storage.pinniped.dev/type": "kittens",
 							"label1":                    "value1",
 							"label2":                    "value2",
 						},
 						Annotations: map[string]string{
 							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
 						},
+						OwnerReferences: []metav1.OwnerReference{{
+							APIVersion: "some-api-version",
+							Kind:       "some-kind",
+							Name:       "some-owner",
+							UID:        "123",
+						}},
 					},
 					Data: map[string][]byte{
 						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
 						"pinniped-storage-version": []byte("1"),
 					},
-					Type: "storage.pinniped.dev/access-tokens",
+					Type: "storage.pinniped.dev/kittens",
+				}),
+				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-kittens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"),
+				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-kittens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"),
+				coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-kittens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						ResourceVersion: "1",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "kittens",
+							"label1":                    "value1",
+							"label2":                    "value2",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+						OwnerReferences: []metav1.OwnerReference{{
+							APIVersion: "some-api-version",
+							Kind:       "some-kind",
+							Name:       "some-owner",
+							UID:        "123",
+						}},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"performed-an-update"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/kittens",
 				}),
-				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"),
 			},
 			wantSecrets: []corev1.Secret{
 				{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						Name:            "pinniped-storage-kittens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
 						Namespace:       namespace,
-						ResourceVersion: "",
+						ResourceVersion: "45",
 						Labels: map[string]string{
-							"storage.pinniped.dev/type": "access-tokens",
+							"storage.pinniped.dev/type": "kittens",
 							"label1":                    "value1",
 							"label2":                    "value2",
 						},
 						Annotations: map[string]string{
 							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
 						},
+						OwnerReferences: []metav1.OwnerReference{{
+							APIVersion: "some-api-version",
+							Kind:       "some-kind",
+							Name:       "some-owner",
+							UID:        "123",
+						}},
 					},
 					Data: map[string][]byte{
-						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
+						"pinniped-storage-data":    []byte(`{"Data":"performed-an-update"}`),
 						"pinniped-storage-version": []byte("1"),
 					},
-					Type: "storage.pinniped.dev/access-tokens",
+					Type: "storage.pinniped.dev/kittens",
 				},
 			},
 			wantErr: "",
@@ -499,6 +561,92 @@ func TestStorage(t *testing.T) {
 			},
 			wantErr: "",
 		},
+		{
+			name:     "update failed, correctly wrap kubernetes conflict error",
+			resource: "stores",
+			mocks: func(t *testing.T, mock mocker) {
+				err := mock.Tracker().Add(&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba",
+						Namespace:       namespace,
+						ResourceVersion: "35",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "stores",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"pants"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/stores",
+				})
+				require.NoError(t, err)
+
+				mock.PrependReactor("update", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, apierrors.NewConflict(schema.GroupResource{
+						Group:    corev1.GroupName,
+						Resource: "secrets",
+					}, "v1.", fmt.Errorf("there was a conflict"))
+				})
+			},
+			run: func(t *testing.T, storage Storage, fakeClock *clocktesting.FakeClock) error {
+				signature := hmac.AuthorizeCodeSignature(authorizationCode3)
+				require.NotEmpty(t, signature)
+				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
+
+				newData := &testJSON{Data: "shirts"}
+				rv2, err := storage.Update(ctx, signature, "35", newData)
+				require.Empty(t, rv2)
+				require.EqualError(t, err, "failed to update stores for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I at resource version 35: Operation cannot be fulfilled on secrets \"v1.\": there was a conflict")
+				require.True(t, apierrors.IsConflict(err))
+
+				return nil
+			},
+			wantActions: []coretesting.Action{
+				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"),
+				coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba",
+						ResourceVersion: "35", // update at initial RV
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "stores",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"shirts"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/stores",
+				}),
+			},
+			wantSecrets: []corev1.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba",
+						Namespace:       namespace,
+						ResourceVersion: "35",
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "stores",
+						},
+						Annotations: map[string]string{
+							"storage.pinniped.dev/garbage-collect-after": fakeNowPlusLifetimeAsString,
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"pants"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/stores",
+				},
+			},
+			wantErr: "",
+		},
 		{
 			name:     "delete existing",
 			resource: "seals",
@@ -1077,6 +1225,68 @@ func TestStorage(t *testing.T) {
 			},
 			wantErr: "",
 		},
+		{
+			name:        "create and get with infinite lifetime when lifetime is specified as zero and clock is specified as nil",
+			resource:    "access-tokens",
+			useNilClock: true,
+			mocks:       nil,
+			lifetime:    func() time.Duration { return 0 }, // 0 == infinity
+			run: func(t *testing.T, storage Storage, fakeClock *clocktesting.FakeClock) error {
+				signature := hmac.AuthorizeCodeSignature(authorizationCode1)
+				require.NotEmpty(t, signature)
+				require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is
+
+				data := &testJSON{Data: "create-and-get"}
+				rv1, err := storage.Create(ctx, signature, data, nil, nil)
+				require.Empty(t, rv1) // fake client does not set this
+				require.NoError(t, err)
+
+				out := &testJSON{}
+				rv2, err := storage.Get(ctx, signature, out)
+				require.Empty(t, rv2) // fake client does not set this
+				require.NoError(t, err)
+				require.Equal(t, data, out)
+
+				return nil
+			},
+			wantActions: []coretesting.Action{
+				coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						ResourceVersion: "",
+						// No garbage collection annotation was added.
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				}),
+				coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"),
+			},
+			wantSecrets: []corev1.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq",
+						Namespace:       namespace,
+						ResourceVersion: "",
+						// No garbage collection annotation was added.
+						Labels: map[string]string{
+							"storage.pinniped.dev/type": "access-tokens",
+						},
+					},
+					Data: map[string][]byte{
+						"pinniped-storage-data":    []byte(`{"Data":"create-and-get"}`),
+						"pinniped-storage-version": []byte("1"),
+					},
+					Type: "storage.pinniped.dev/access-tokens",
+				},
+			},
+			wantErr: "",
+		},
 	}
 
 	for _, tt := range tests {
@@ -1093,8 +1303,15 @@ func TestStorage(t *testing.T) {
 				useLifetime = tt.lifetime()
 			}
 			secrets := client.CoreV1().Secrets(namespace)
+
 			fakeClock := clocktesting.NewFakeClock(fakeNow)
-			storage := New(tt.resource, secrets, fakeClock.Now, useLifetime)
+			clock := fakeClock.Now
+			if tt.useNilClock {
+				fakeClock = nil
+				clock = nil
+			}
+
+			storage := New(tt.resource, secrets, clock, useLifetime)
 
 			err := tt.run(t, storage, fakeClock)
 

internal/registry/clientsecretrequest/rest.go

@@ -6,15 +6,12 @@ package clientsecretrequest
 
 import (
 	"context"
-	"crypto/rand"
 	"encoding/hex"
 	"fmt"
 	"io"
 	"strings"
 
 	"golang.org/x/crypto/bcrypt"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	genericvalidation "k8s.io/apimachinery/pkg/api/validation"
@@ -34,48 +31,27 @@ import (
 	"go.pinniped.dev/internal/oidcclientsecretstorage"
 )
 
-// cost is a good bcrypt cost for 2022, should take about 250 ms to validate.
+// Cost is a good bcrypt cost for 2022, should take about 250 ms to validate.
 // This value is expected to be increased over time to match CPU improvements.
-const cost = 12
+const Cost = 12
 
-//nolint:gochecknoglobals
+func NewREST(resource schema.GroupResource, secrets corev1client.SecretInterface, clients configv1alpha1clientset.OIDCClientInterface, namespace string, cost int) *REST {
-var tableConvertor = func() rest.TableConvertor {
-	// sadly this is not useful at the moment because `kubectl create` does not support table output
-	columns := []apiextensionsv1.CustomResourceColumnDefinition{
-		{
-			Name:        "Secret",
-			Type:        "string",
-			Description: "", // TODO generate SwaggerDoc() method to fill this field
-			JSONPath:    ".status.generatedSecret",
-		},
-		{
-			Name:        "Total",
-			Type:        "integer",
-			Description: "", // TODO generate SwaggerDoc() method to fill this field
-			JSONPath:    ".status.totalClientSecrets",
-		},
-	}
-	tc, err := tableconvertor.New(columns) // just re-use the CRD table code so we do not have to implement the interface ourselves
-	if err != nil {
-		panic(err) // inputs are static so this should never happen
-	}
-	return tc
-}()
-
-func NewREST(secrets corev1client.SecretInterface, clients configv1alpha1clientset.OIDCClientInterface, namespace string) *REST {
 	return &REST{
-		secretStorage: oidcclientsecretstorage.New(secrets),
+		secretStorage:  oidcclientsecretstorage.New(secrets),
-		clients:       clients,
+		clients:        clients,
-		namespace:     namespace,
+		namespace:      namespace,
-		rand:          rand.Reader,
+		cost:           cost,
+		tableConvertor: rest.NewDefaultTableConvertor(resource),
 	}
 }
 
 type REST struct {
-	secretStorage *oidcclientsecretstorage.OIDCClientSecretStorage
+	secretStorage  *oidcclientsecretstorage.OIDCClientSecretStorage
-	clients       configv1alpha1clientset.OIDCClientInterface
+	clients        configv1alpha1clientset.OIDCClientInterface
-	namespace     string
+	namespace      string
-	rand          io.Reader
+	rand           io.Reader
+	cost           int
+	tableConvertor rest.TableConvertor
 }
 
 // Assert that our *REST implements all the optional interfaces that we expect it to implement.
@@ -111,7 +87,7 @@ func (*REST) List(_ context.Context, _ *metainternalversion.ListOptions) (runtim
 }
 
 func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
-	return tableConvertor.ConvertToTable(ctx, obj, tableOptions)
+	return r.tableConvertor.ConvertToTable(ctx, obj, tableOptions)
 }
 
 func (*REST) NamespaceScoped() bool {
@@ -130,6 +106,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	defer t.Log()
 
 	// Validate the create request before honoring it.
+	// This function is provided from kube kube-api server calling validating admission webhooks if there are any registered.
 	req, err := r.validateRequest(ctx, obj, createValidation, options, t)
 	if err != nil {
 		return nil, err
@@ -168,7 +145,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 		}
 		t.Step("generateSecret")
 
-		hash, err := bcrypt.GenerateFromPassword([]byte(secret), cost)
+		hash, err := bcrypt.GenerateFromPassword([]byte(secret), r.cost)
 		if err != nil {
 			traceFailureWithError(t, "bcrypt.GenerateFromPassword", err)
 			return nil, apierrors.NewInternalError(fmt.Errorf("hash generation failed"))

internal/registry/clientsecretrequest/rest_test.go

@@ -0,0 +1,404 @@
+// NOTES:
+// Take a look at the unit tests from the sibling files as we already have some there.
+// whoamirequest is super simple, zero side effects...
+// - add bits similar to what we did for crud.go to seed kube with secrets & prove your updates
+// - perhaps look at oidcclientsecretstorage.go also for the seeding code we wrote
+//   - the things we are seeding here IS the storage of client secrets + hashes
+//     - this is the specific secret formats that we need
+//     - but instead of hello world hashes we will want to put real BCrypt hashes in the seeds
+//     - there is a test helper with some real BCrypt hashes we can use in oidcclient.go -- HashedPassword1AtGoMinCost - 04 for unit tests for speed
+//       - look for a test using HashedPassword1AtGoMinCost to see how we step around the min value of 11 tho production code must do that
+// 			- NewKubeStorage, bcrypt.MinCost will show this
+// 			// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
+// 			oauthStore := oidc.NewKubeStorage(secrets, oidcClientsClient, timeoutsConfiguration, bcrypt.MinCost)
+// credentialrequest also has no side effects...
+//
+//
+// Copyright 2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+package clientsecretrequest
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/registry/rest"
+
+	clientsecretapi "go.pinniped.dev/generated/latest/apis/supervisor/clientsecret"
+)
+
+func TestNew(t *testing.T) {
+
+	r := NewREST(schema.GroupResource{Group: "bears", Resource: "panda"}, nil, nil, "foobar", 4)
+
+	require.NotNil(t, r)
+	require.True(t, r.NamespaceScoped())
+	require.Equal(t, []string{"pinniped"}, r.Categories())
+
+	require.IsType(t, &clientsecretapi.OIDCClientSecretRequest{}, r.New())
+	require.IsType(t, &clientsecretapi.OIDCClientSecretRequestList{}, r.NewList())
+
+	ctx := context.Background()
+
+	// check the simple invariants of our no-op list
+	list, err := r.List(ctx, nil)
+	require.NoError(t, err)
+	require.NotNil(t, list)
+	require.IsType(t, &clientsecretapi.OIDCClientSecretRequestList{}, list)
+	require.Equal(t, "0", list.(*clientsecretapi.OIDCClientSecretRequestList).ResourceVersion)
+	require.NotNil(t, list.(*clientsecretapi.OIDCClientSecretRequestList).Items)
+	require.Len(t, list.(*clientsecretapi.OIDCClientSecretRequestList).Items, 0)
+
+	// make sure we can turn lists into tables if needed
+	table, err := r.ConvertToTable(ctx, list, nil)
+	require.NoError(t, err)
+	require.NotNil(t, table)
+	require.Equal(t, "0", table.ResourceVersion)
+	require.Nil(t, table.Rows)
+
+	// exercise group resource - force error by passing a runtime.Object that does not have an embedded object meta
+	_, err = r.ConvertToTable(ctx, &metav1.APIGroup{}, nil)
+	require.Error(t, err, "the resource panda.bears does not support being converted to a Table")
+}
+
+func TestCreate(t *testing.T) {
+	type args struct {
+		ctx              context.Context
+		obj              runtime.Object
+		createValidation rest.ValidateObjectFunc
+		options          *metav1.CreateOptions
+	}
+	namespace := "some-namespace"
+
+	tests := []struct {
+		name          string
+		args          args
+		want          runtime.Object
+		wantErrStatus metav1.Status
+	}{
+		{
+			name: "wrong type",
+			args: args{
+				ctx:              genericapirequest.NewContext(),
+				obj:              &metav1.Status{},
+				createValidation: nil,
+				options:          nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: `not an OIDCClientSecretRequest: &v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:"", Continue:"", RemainingItemCount:(*int64)(nil)}, Status:"", Message:"", Reason:"", Details:(*v1.StatusDetails)(nil), Code:0}`,
+				Reason:  metav1.StatusReasonBadRequest,
+				Code:    http.StatusBadRequest,
+			},
+		},
+		{
+			name: "bad options for dry run",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-some-client-name",
+					},
+				},
+				createValidation: nil,
+				options:          &metav1.CreateOptions{DryRun: []string{"stuff"}},
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: `OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev "client.oauth.pinniped.dev-some-client-name" is invalid: dryRun: Unsupported value: []string{"stuff"}`,
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "client.oauth.pinniped.dev-some-client-name",
+					Causes: []metav1.StatusCause{{
+						Type:    "FieldValueNotSupported",
+						Message: "Unsupported value: []string{\"stuff\"}",
+						Field:   "dryRun",
+					}},
+				},
+			},
+		},
+		{
+			name: "incorrect namespace",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), "wrong-namespace"),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-some-client-name",
+					},
+				},
+				createValidation: nil,
+				options:          nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: `namespace must be some-namespace on OIDCClientSecretRequest, was wrong-namespace`,
+				Reason:  metav1.StatusReasonBadRequest,
+				Code:    http.StatusBadRequest,
+			},
+		},
+		{
+			name: "create validation: failure from kube api-server rest.ValidateObjectFunc",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-some-client-name",
+					},
+				},
+				createValidation: func(ctx context.Context, obj runtime.Object) error {
+					return apierrors.NewInternalError(errors.New("some-error-here"))
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "Internal error occurred: some-error-here",
+				Reason:  metav1.StatusReasonInternalError,
+				Code:    http.StatusInternalServerError,
+				Details: &metav1.StatusDetails{
+					Causes: []metav1.StatusCause{{
+						Message: "some-error-here",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: no namespace on the request context",
+			args: args{
+				ctx: genericapirequest.NewContext(),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-some-client-name",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "Internal error occurred: no namespace information found in request context",
+				Reason:  metav1.StatusReasonInternalError,
+				Code:    http.StatusInternalServerError,
+				Details: &metav1.StatusDetails{
+					Causes: []metav1.StatusCause{{
+						Message: "no namespace information found in request context",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: namespace on object does not match namespace on request",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "client.oauth.pinniped.dev-some-client-name",
+						Namespace: "not-a-matching-namespace",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "the namespace of the provided object does not match the namespace sent on the request",
+				Reason:  metav1.StatusReasonBadRequest,
+				Code:    http.StatusBadRequest,
+			},
+		},
+		{
+			name: "create validation: generateName is unsupported",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "foo",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev \"\" is invalid: [metadata.generateName: Invalid value: \"foo\": generateName is not supported, metadata.name: Required value: name or generateName is required]",
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "",
+					Causes: []metav1.StatusCause{{
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: "Invalid value: \"foo\": generateName is not supported",
+						Field:   "metadata.generateName",
+					}, {
+						Type:    metav1.CauseTypeFieldValueRequired,
+						Message: "Required value: name or generateName is required",
+						Field:   "metadata.name",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: name cannot exactly match client.oauth.pinniped.dev-",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev \"client.oauth.pinniped.dev-\" is invalid: metadata.name: Invalid value: \"client.oauth.pinniped.dev-\": must not equal 'client.oauth.pinniped.dev-'",
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "client.oauth.pinniped.dev-",
+					Causes: []metav1.StatusCause{{
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: "Invalid value: \"client.oauth.pinniped.dev-\": must not equal 'client.oauth.pinniped.dev-'",
+						Field:   "metadata.name",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: name must contain prefix client.oauth.pinniped.dev-",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "does-not-contain-the-prefix",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: "OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev \"does-not-contain-the-prefix\" is invalid: metadata.name: Invalid value: \"does-not-contain-the-prefix\": must start with 'client.oauth.pinniped.dev-'",
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "does-not-contain-the-prefix",
+					Causes: []metav1.StatusCause{{
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: "Invalid value: \"does-not-contain-the-prefix\": must start with 'client.oauth.pinniped.dev-'",
+						Field:   "metadata.name",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: name with invalid characters should error",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "client.oauth.pinniped.dev-contains/invalid/characters",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: `OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev "client.oauth.pinniped.dev-contains/invalid/characters" is invalid: metadata.name: Invalid value: "client.oauth.pinniped.dev-contains/invalid/characters": may not contain '/'`,
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "client.oauth.pinniped.dev-contains/invalid/characters",
+					Causes: []metav1.StatusCause{{
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: `Invalid value: "client.oauth.pinniped.dev-contains/invalid/characters": may not contain '/'`,
+						Field:   "metadata.name",
+					}},
+				},
+			},
+		},
+		{
+			name: "create validation: name validation may return multiple errors",
+			args: args{
+				ctx: genericapirequest.WithNamespace(genericapirequest.NewContext(), namespace),
+				obj: &clientsecretapi.OIDCClientSecretRequest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:         "multiple/errors/aggregated",
+						GenerateName: "no-generate-allowed",
+					},
+				},
+				options: nil,
+			},
+			want: nil,
+			wantErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Message: `OIDCClientSecretRequest.clientsecret.supervisor.pinniped.dev "multiple/errors/aggregated" is invalid: [metadata.generateName: Invalid value: "no-generate-allowed": generateName is not supported, metadata.name: Invalid value: "multiple/errors/aggregated": must start with 'client.oauth.pinniped.dev-', metadata.name: Invalid value: "multiple/errors/aggregated": may not contain '/']`,
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+				Details: &metav1.StatusDetails{
+					Group: "clientsecret.supervisor.pinniped.dev",
+					Kind:  "OIDCClientSecretRequest",
+					Name:  "multiple/errors/aggregated",
+					Causes: []metav1.StatusCause{{
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: `Invalid value: "no-generate-allowed": generateName is not supported`,
+						Field:   "metadata.generateName",
+					}, {
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: `Invalid value: "multiple/errors/aggregated": must start with 'client.oauth.pinniped.dev-'`,
+						Field:   "metadata.name",
+					}, {
+						Type:    metav1.CauseTypeFieldValueInvalid,
+						Message: `Invalid value: "multiple/errors/aggregated": may not contain '/'`,
+						Field:   "metadata.name",
+					}},
+				},
+			},
+		},
+		// {name: ""},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			// TODO: update and fill these in with actual values, not nil!
+			r := NewREST(schema.GroupResource{Group: "bears", Resource: "panda"}, nil, nil, namespace, 4)
+			got, err := r.Create(tt.args.ctx, tt.args.obj, tt.args.createValidation, tt.args.options)
+			require.Equal(t, &apierrors.StatusError{
+				ErrStatus: tt.wantErrStatus,
+			}, err)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func errString(err error) string {
+	if err == nil {
+		return ""
+	}
+
+	return err.Error()
+}

internal/supervisor/apiserver/apiserver.go

@@ -80,7 +80,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	for _, f := range []func() (schema.GroupVersionResource, rest.Storage){
 		func() (schema.GroupVersionResource, rest.Storage) {
 			clientSecretReqGVR := c.ExtraConfig.ClientSecretSupervisorGroupVersion.WithResource("oidcclientsecretrequests")
-			clientSecretReqStorage := clientsecretrequest.NewREST(c.ExtraConfig.Secrets, c.ExtraConfig.OIDCClients, c.ExtraConfig.Namespace)
+			clientSecretReqStorage := clientsecretrequest.NewREST(clientSecretReqGVR.GroupResource(), c.ExtraConfig.Secrets, c.ExtraConfig.OIDCClients, c.ExtraConfig.Namespace, clientsecretrequest.Cost)
 			return clientSecretReqGVR, clientSecretReqStorage
 		},
 	} {