fix(keepersecurity): make folderID optional except when creating new records via push (#6921)

* fix(keepersecurity): make folderID optional for read operations

Signed-off-by: darren <darrenmaverickz@gmail.com>

* style(keepersecurity): fix trailing whitespace from gofmt

Signed-off-by: darren <darrenmaverickz@gmail.com>

* ref(keepersecurity): use named constant for missing folderID error

Signed-off-by: darren <darrenmaverickz@gmail.com>

* test(keepersecurity): add push-create case for missing folderID

Signed-off-by: darren <darrenmaverickz@gmail.com>

* docs(keepersecurity): mark folderID optional in API reference docs

Signed-off-by: darren <darrenmaverickz@gmail.com>

* style(keepersecurity): capitalize Keeper Security in error message

Signed-off-by: darren <darrenmaverickz@gmail.com>

* test(keepersecurity): cover ValidateStore folderID relaxation

Signed-off-by: darren <darrenmaverickz@gmail.com>

---------

Signed-off-by: darren <darrenmaverickz@gmail.com>
Co-authored-by: Alexander Chernov <alexander@chernov.it>

apis/externalsecrets/v1/secretstore_keepersecurity_types.go

@@ -20,7 +20,9 @@ import smmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
 
 // KeeperSecurityProvider Configures a store to sync secrets using Keeper Security.
 type KeeperSecurityProvider struct {
-	Auth               smmeta.SecretKeySelector `json:"authRef"`
-	FolderID           string                   `json:"folderID"`
-	GetByTitleFallback bool                     `json:"getByTitleFallback,omitempty"`
+	Auth smmeta.SecretKeySelector `json:"authRef"`
+
+	// +optional
+	FolderID           string `json:"folderID,omitempty"`
+	GetByTitleFallback bool   `json:"getByTitleFallback,omitempty"`
 }

config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -4069,7 +4069,6 @@ spec:
                         type: boolean
                     required:
                     - authRef
-                    - folderID
                     type: object
                   kubernetes:
                     description: Kubernetes configures this store to sync secrets

config/crds/bases/external-secrets.io_secretstores.yaml

@@ -4069,7 +4069,6 @@ spec:
                         type: boolean
                     required:
                     - authRef
-                    - folderID
                     type: object
                   kubernetes:
                     description: Kubernetes configures this store to sync secrets

deploy/crds/bundle.yaml

@@ -6115,7 +6115,6 @@ spec:
                           type: boolean
                       required:
                         - authRef
-                        - folderID
                       type: object
                     kubernetes:
                       description: Kubernetes configures this store to sync secrets using a Kubernetes cluster provider
@@ -19573,7 +19572,6 @@ spec:
                           type: boolean
                       required:
                         - authRef
-                        - folderID
                       type: object
                     kubernetes:
                       description: Kubernetes configures this store to sync secrets using a Kubernetes cluster provider

docs/api/spec.md

@@ -7798,6 +7798,7 @@ string
 </em>
 </td>
 <td>
+<em>(Optional)</em>
 </td>
 </tr>
 <tr>

providers/v1/keepersecurity/client.go

@@ -51,6 +51,7 @@ const (
 	errInvalidRemoteRefKey                      = "match.remoteRef.remoteKey. Invalid format. Format should match secretName/key got %s"
 	errInvalidSecretType                        = "ESO can only push/delete records of type %s. Secret %s is type %s"
 	errFieldNotFound                            = "secret %s does not contain any custom field with label %s"
+	errKeeperSecurityMissingFolderIDForCreate   = "folderID must be set on the SecretStore to create a new Keeper Security record"
 
 	externalSecretType = "externalSecrets"
 	secretType         = "secret"
@@ -313,6 +314,10 @@ func (c *Client) createSecret(name, key string, value []byte) (string, error) {
 		)
 	}
 
+	if c.folderID == "" {
+		return "", errors.New(errKeeperSecurityMissingFolderIDForCreate)
+	}
+
 	uid, err := c.ksmClient.CreateSecretWithRecordData("", c.folderID, externalSecretRecord)
 	metrics.ObserveAPICall(ProviderKeeperSecurity, CallKeeperSecurityCreateSecretWithRecordData, err)
 	return uid, err

providers/v1/keepersecurity/client_test.go

@@ -919,6 +919,25 @@ func TestClientPushSecret(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Push new secret fails without folderID",
+			fields: fields{
+				ksmClient: &fake.MockKeeperClient{
+					GetSecretsByTitleFn: func(recordTitle string) (records []*ksm.Record, err error) {
+						return generateRecords()[0:0], nil
+					},
+				},
+				folderID: "",
+			},
+			args: args{
+				data: testingfake.PushSecretData{
+					SecretKey: secretKey,
+					RemoteKey: invalidRecord,
+				},
+				value: []byte("foo"),
+			},
+			wantErr: true,
+		},
 		{
 			name: "Unable to save existing valid secret",
 			fields: fields{

providers/v1/keepersecurity/provider.go

@@ -38,7 +38,6 @@ const (
 	errKeeperSecurityNilSpec                       = "nil spec"
 	errKeeperSecurityNilSpecProvider               = "nil spec.provider"
 	errKeeperSecurityNilSpecProviderKeeperSecurity = "nil spec.provider.keepersecurity"
-	errKeeperSecurityStoreMissingFolderID          = "missing: spec.provider.keepersecurity.folderID"
 )
 
 // Provider implements the necessary NewClient() and ValidateStore() funcs for Keeper Security.
@@ -102,9 +101,6 @@ func (p *Provider) ValidateStore(store esv1.GenericStore) (admission.Warnings, e
 	if err := esutils.ValidateSecretSelector(store, config.Auth); err != nil {
 		return nil, fmt.Errorf("error validating secret selector: %w", err)
 	}
-	if config.FolderID == "" {
-		return nil, errors.New(errKeeperSecurityStoreMissingFolderID)
-	}
 
 	return nil, nil
 }

providers/v1/keepersecurity/provider_test.go

@@ -0,0 +1,64 @@
+/*
+Copyright © The ESO Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package keepersecurity
+
+import (
+	"testing"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	smmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
+)
+
+func makeKeeperSecurityStore(folderID string) *esv1.SecretStore {
+	return &esv1.SecretStore{
+		Spec: esv1.SecretStoreSpec{
+			Provider: &esv1.SecretStoreProvider{
+				KeeperSecurity: &esv1.KeeperSecurityProvider{
+					Auth:     smmeta.SecretKeySelector{Name: "keeper-creds"},
+					FolderID: folderID,
+				},
+			},
+		},
+	}
+}
+
+func TestValidateStore(t *testing.T) {
+	testCases := []struct {
+		label string
+		store *esv1.SecretStore
+	}{
+		{
+			label: "valid store without folderID",
+			store: makeKeeperSecurityStore(""),
+		},
+		{
+			label: "valid store with folderID",
+			store: makeKeeperSecurityStore(folderID),
+		},
+	}
+
+	p := Provider{}
+
+	for _, tc := range testCases {
+		t.Run(tc.label, func(t *testing.T) {
+			_, err := p.ValidateStore(tc.store)
+			if err != nil {
+				t.Errorf("ValidateStore() unexpected error: %v", err)
+			}
+		})
+	}
+}