From 20391a323f4d83f6490d01a66c376ad08d9f46b1 Mon Sep 17 00:00:00 2001
From: Monis Khan <i@monis.app>
Date: Wed, 20 Jul 2022 16:44:41 -0400
Subject: [PATCH] wip006

Signed-off-by: Monis Khan <mok@vmware.com>
---
 deploy/supervisor/deployment.yaml             |  1 +
 .../oidcclientsecretstorage.go                | 20 ++++++++-----------
 internal/registry/clientsecretrequest/rest.go |  7 ++++---
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml
index a116d08e..e1db7428 100644
--- a/deploy/supervisor/deployment.yaml
+++ b/deploy/supervisor/deployment.yaml
@@ -98,6 +98,7 @@ spec:
             readOnlyRootFilesystem: true
           resources:
             requests:
+              #! NOTE for BEN before we commit: make sure that IAM pick this up :)
               cpu: "2048m"
               memory: "128Mi"
             limits:
diff --git a/internal/oidcclientsecretstorage/oidcclientsecretstorage.go b/internal/oidcclientsecretstorage/oidcclientsecretstorage.go
index 4bd98ad4..1c452356 100644
--- a/internal/oidcclientsecretstorage/oidcclientsecretstorage.go
+++ b/internal/oidcclientsecretstorage/oidcclientsecretstorage.go
@@ -45,28 +45,27 @@ func New(secrets corev1client.SecretInterface) *OIDCClientSecretStorage {
 	return &OIDCClientSecretStorage{storage: crud.New(TypeLabelValue, secrets, nil, 0)}
 }
 
-func (s *OIDCClientSecretStorage) Get(ctx context.Context, oidcClientUID types.UID) ([]string, error) {
+func (s *OIDCClientSecretStorage) Get(ctx context.Context, oidcClientUID types.UID) (string, []string, error) {
 	secret := &storedClientSecret{}
-	_, err := s.storage.Get(ctx, uidToName(oidcClientUID), secret)
+	rv, err := s.storage.Get(ctx, uidToName(oidcClientUID), secret)
 	if errors.IsNotFound(err) {
-		return nil, nil
+		return "", nil, nil
 	}
 	if err != nil {
-		return nil, fmt.Errorf("failed to get client secret for uid %s: %w", oidcClientUID, err)
+		return "", nil, fmt.Errorf("failed to get client secret for uid %s: %w", oidcClientUID, err)
 	}
 
-	return secret.SecretHashes, nil
+	return rv, secret.SecretHashes, nil
 }
 
-func (s *OIDCClientSecretStorage) Set(ctx context.Context, oidcClientName string, oidcClientUID types.UID, secretHashes []string) error {
+func (s *OIDCClientSecretStorage) Set(ctx context.Context, resourceVersion, oidcClientName string, oidcClientUID types.UID, secretHashes []string) error {
 	secret := &storedClientSecret{
 		SecretHashes: secretHashes,
 		Version:      oidcClientSecretStorageVersion,
 	}
 	name := uidToName(oidcClientUID)
 
-	rv, err := s.storage.Get(ctx, name, &storedClientSecret{})
-	if errors.IsNotFound(err) {
+	if mustBeCreate := len(resourceVersion) == 0; mustBeCreate {
 		ownerReferences := []metav1.OwnerReference{
 			{
 				APIVersion:         configv1alpha1.SchemeGroupVersion.String(),
@@ -83,11 +82,8 @@ func (s *OIDCClientSecretStorage) Set(ctx context.Context, oidcClientName string
 		}
 		return nil
 	}
-	if err != nil {
-		return fmt.Errorf("failed to get client secret for uid %s: %w", oidcClientUID, err)
-	}
 
-	_, err = s.storage.Update(ctx, name, rv, secret)
+	_, err := s.storage.Update(ctx, name, resourceVersion, secret)
 	if err != nil {
 		return fmt.Errorf("failed to update client secret for uid %s: %w", oidcClientUID, err)
 	}
diff --git a/internal/registry/clientsecretrequest/rest.go b/internal/registry/clientsecretrequest/rest.go
index f1997606..16aec9c7 100644
--- a/internal/registry/clientsecretrequest/rest.go
+++ b/internal/registry/clientsecretrequest/rest.go
@@ -102,6 +102,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	})
 	defer t.Log()
 
+	// TODO actually validate the request like checking that the namespace is the supervisor's namespace
 	req, err := validateRequest(obj, t)
 	if err != nil {
 		return nil, err
@@ -114,7 +115,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	}
 	t.Step("clients.Get")
 
-	hashes, err := r.secretStorage.Get(ctx, oidcClient.UID)
+	rv, hashes, err := r.secretStorage.Get(ctx, oidcClient.UID)
 	if err != nil {
 		return nil, err // TODO obfuscate
 	}
@@ -145,8 +146,8 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	// TODO do not let them have more than 100? secrets
 
 	if req.Spec.GenerateNewSecret || needsRevoke {
-		if err := r.secretStorage.Set(ctx, oidcClient.Name, oidcClient.UID, hashes); err != nil {
-			return nil, err // TODO obfuscate
+		if err := r.secretStorage.Set(ctx, rv, oidcClient.Name, oidcClient.UID, hashes); err != nil {
+			return nil, err // TODO obfuscate, also return good errors for cases like when the secret now exists but previously did not
 		}
 		t.Step("secretStorage.Set")
 	}