wip015

Signed-off-by: Monis Khan <mok@vmware.com>

internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go

@@ -251,7 +251,7 @@ func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition) ([]*v1a
 		return conditions, 0
 	}
 
-	storedClientSecret, err := oidcclientsecretstorage.ReadFromSecret(secret)
+	hashes, err := oidcclientsecretstorage.ReadFromSecret(secret)
 	if err != nil {
 		// Invalid: storage Secret exists but its data could not be parsed.
 		conditions = append(conditions, &v1alpha1.Condition{
@@ -264,7 +264,7 @@ func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition) ([]*v1a
 	}
 
 	// Successfully read the stored client secrets, so check if there are any stored in the list.
-	storedClientSecretsCount := len(storedClientSecret.SecretHashes)
+	storedClientSecretsCount := len(hashes)
 	if storedClientSecretsCount == 0 {
 		// Invalid: no client secrets stored.
 		conditions = append(conditions, &v1alpha1.Condition{
@@ -278,7 +278,7 @@ func validateSecret(secret *v1.Secret, conditions []*v1alpha1.Condition) ([]*v1a
 
 	// Check each hashed password's format and bcrypt cost.
 	bcryptErrs := make([]string, 0, storedClientSecretsCount)
-	for i, p := range storedClientSecret.SecretHashes {
+	for i, p := range hashes {
 		cost, err := bcrypt.Cost([]byte(p))
 		if err != nil {
 			bcryptErrs = append(bcryptErrs, fmt.Sprintf(

internal/crud/crud_test.go

@@ -119,7 +119,7 @@ func TestStorage(t *testing.T) {
 				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)
+				rv1, err := storage.Create(ctx, signature, data, nil, nil)
 				require.Empty(t, rv1) // fake client does not set this
 				require.NoError(t, err)
 
@@ -179,14 +179,14 @@ func TestStorage(t *testing.T) {
 			mocks:    nil,
 			run: func(t *testing.T, storage Storage, fakeClock *clocktesting.FakeClock) error {
 				data := &testJSON{Data: "create1"}
-				rv1, err := storage.Create(ctx, "sig1", data, nil)
+				rv1, err := storage.Create(ctx, "sig1", data, nil, nil)
 				require.Empty(t, rv1) // fake client does not set this
 				require.NoError(t, err)
 
 				fakeClock.Step(42 * time.Minute) // simulate that a known amount of time has passed
 
 				data = &testJSON{Data: "create2"}
-				rv1, err = storage.Create(ctx, "sig2", data, nil)
+				rv1, err = storage.Create(ctx, "sig2", data, nil, nil)
 				require.Empty(t, rv1) // fake client does not set this
 				require.NoError(t, err)
 
@@ -278,7 +278,7 @@ func TestStorage(t *testing.T) {
 				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"})
+				rv1, err := storage.Create(ctx, signature, data, map[string]string{"label1": "value1", "label2": "value2"}, nil)
 				require.Empty(t, rv1) // fake client does not set this
 				require.NoError(t, err)
 

internal/oidcclientsecretstorage/oidcclientsecretstorage.go

@@ -98,8 +98,8 @@ func uidToName(oidcClientUID types.UID) string {
 	return base64.RawURLEncoding.EncodeToString([]byte(oidcClientUID))
 }
 
-// ReadFromSecret reads the contents of a Secret as a storedClientSecret.
-func ReadFromSecret(s *corev1.Secret) (*storedClientSecret, error) {
+// ReadFromSecret reads the contents of a Secret as a storedClientSecret and returns the associated hashes.
+func ReadFromSecret(s *corev1.Secret) ([]string, error) {
 	secret := &storedClientSecret{}
 	err := crud.FromSecret(TypeLabelValue, s, secret)
 	if err != nil {
@@ -109,5 +109,5 @@ func ReadFromSecret(s *corev1.Secret) (*storedClientSecret, error) {
 		return nil, fmt.Errorf("%w: OIDC client secret storage has version %s instead of %s",
 			ErrOIDCClientSecretStorageVersion, secret.Version, oidcClientSecretStorageVersion)
 	}
-	return secret, nil
+	return secret.SecretHashes, nil
 }

internal/oidcclientsecretstorage/oidcclientsecretstorage_test.go

@@ -13,7 +13,7 @@ import (
 
 func TestGetName(t *testing.T) {
 	// Note that GetName() should not depend on the constructor params, to make it easier to use in various contexts.
-	subject := New(nil, nil)
+	subject := New(nil)
 
 	require.Equal(t,
 		"pinniped-storage-oidc-client-secret-onxw2zjnmv4gc3lqnrss25ljmqyq",
@@ -28,7 +28,7 @@ func TestReadFromSecret(t *testing.T) {
 	tests := []struct {
 		name       string
 		secret     *corev1.Secret
-		wantStored *storedClientSecret
+		wantHashes []string
 		wantErr    string
 	}{
 		{
@@ -47,10 +47,7 @@ func TestReadFromSecret(t *testing.T) {
 				},
 				Type: "storage.pinniped.dev/oidc-client-secret",
 			},
-			wantStored: &storedClientSecret{
-				Version:      "1",
-				SecretHashes: []string{"first-hash", "second-hash"},
-			},
+			wantHashes: []string{"first-hash", "second-hash"},
 		},
 		{
 			name: "wrong secret type",
@@ -112,13 +109,13 @@ func TestReadFromSecret(t *testing.T) {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			session, err := ReadFromSecret(tt.secret)
+			hashes, err := ReadFromSecret(tt.secret)
 			if tt.wantErr == "" {
 				require.NoError(t, err)
-				require.Equal(t, tt.wantStored, session)
+				require.Equal(t, tt.wantHashes, hashes)
 			} else {
 				require.EqualError(t, err, tt.wantErr)
-				require.Nil(t, session)
+				require.Nil(t, hashes)
 			}
 		})
 	}

internal/registry/clientsecretrequest/rest.go

@@ -38,9 +38,10 @@ import (
 // this value is expected to be increased over time to match CPU improvements
 // thus it must be kept private because validation of client secrets cannot rely
 // on a cost that changes without some form client secret storage migration
-// TODO write a unit test that fails in 2023 to ask this to be updated to latest recommendation
+// TODO write a unit test that fails in 2023 to ask this to be updated to latest recommendation.
 const cost = 12
 
+// nolint: gochecknoglobals
 var tableConvertor = func() rest.TableConvertor {
 	// sadly this is not useful at the moment because `kubectl create` does not support table output
 	columns := []apiextensionsv1.CustomResourceColumnDefinition{
@@ -101,7 +102,7 @@ func (*REST) NewList() runtime.Object {
 
 // support `kubectl get pinniped`
 // to make sure all resources are in the pinniped category and
-// avoid kubectl errors when kubectl lists you must support the list verb
+// avoid kubectl errors when kubectl lists you must support the list verb.
 func (*REST) List(_ context.Context, _ *metainternalversion.ListOptions) (runtime.Object, error) {
 	return &clientsecretapi.OIDCClientSecretRequestList{
 		ListMeta: metav1.ListMeta{

test/integration/supervisor_oidc_client_test.go

@@ -624,7 +624,7 @@ func TestOIDCClientControllerValidations_Parallel(t *testing.T) {
 
 			if tt.secret != nil {
 				// Force the Secret's name to match the client created above.
-				tt.secret.Name = oidcclientsecretstorage.New(nil, nil).GetName(client.UID)
+				tt.secret.Name = oidcclientsecretstorage.New(nil).GetName(client.UID)
 				secret, err := secrets.Create(ctx, tt.secret, metav1.CreateOptions{})
 				require.NoError(t, err)
 				t.Cleanup(func() {