From 4820cc9165f06a81566b86967d3726a7c1651668 Mon Sep 17 00:00:00 2001 From: Merlin Date: Wed, 6 Apr 2022 06:02:19 +0200 Subject: [PATCH] Ignore ExternalSecret processing if the store is not usuable (e.g. NotReady). --- .../v1beta1/externalsecret_types.go | 1 + apis/externalsecrets/v1beta1/provider.go | 17 +++++- .../v1beta1/provider_schema_test.go | 4 +- .../v1beta1/secretstore_types.go | 4 ++ cmd/root.go | 5 +- ...ternal-secrets.io_clustersecretstores.yaml | 4 ++ .../external-secrets.io_secretstores.yaml | 4 ++ deploy/crds/bundle.yaml | 6 +++ e2e/suite/akeyless/akeyless.go | 2 - e2e/suite/alibaba/alibaba.go | 2 - e2e/suite/gitlab/gitlab.go | 2 - e2e/suite/oracle/oracle.go | 2 - .../externalsecret_controller.go | 29 ++++++++-- pkg/controllers/secretstore/common.go | 8 ++- pkg/provider/akeyless/akeyless.go | 8 ++- pkg/provider/alibaba/kms.go | 7 ++- .../aws/parameterstore/parameterstore.go | 7 ++- .../aws/secretsmanager/secretsmanager.go | 7 ++- pkg/provider/azure/keyvault/keyvault.go | 4 +- pkg/provider/fake/fake.go | 4 +- .../gcp/secretmanager/secretsmanager.go | 4 +- pkg/provider/gitlab/gitlab.go | 8 +-- pkg/provider/gitlab/gitlab_test.go | 53 +++++++++++-------- pkg/provider/ibm/provider.go | 4 +- pkg/provider/kubernetes/kubernetes.go | 8 +-- pkg/provider/kubernetes/kubernetes_test.go | 16 ++++-- pkg/provider/oracle/oracle.go | 4 +- pkg/provider/testing/fake/fake.go | 11 ++-- pkg/provider/vault/vault.go | 6 +-- pkg/provider/webhook/webhook.go | 7 ++- pkg/provider/yandex/lockbox/lockbox.go | 4 +- 31 files changed, 169 insertions(+), 83 deletions(-) diff --git a/apis/externalsecrets/v1beta1/externalsecret_types.go b/apis/externalsecrets/v1beta1/externalsecret_types.go index ab8ca907d..c305abda8 100644 --- a/apis/externalsecrets/v1beta1/externalsecret_types.go +++ b/apis/externalsecrets/v1beta1/externalsecret_types.go @@ -273,6 +273,7 @@ const ( ConditionReasonSecretDeleted = "SecretDeleted" ReasonInvalidStoreRef = "InvalidStoreRef" + ReasonUnavailableStore = "UnavailableStore" ReasonProviderClientConfig = "InvalidProviderClientConfig" ReasonUpdateFailed = "UpdateFailed" ReasonUpdated = "Updated" diff --git a/apis/externalsecrets/v1beta1/provider.go b/apis/externalsecrets/v1beta1/provider.go index 567712d5e..b1797caf8 100644 --- a/apis/externalsecrets/v1beta1/provider.go +++ b/apis/externalsecrets/v1beta1/provider.go @@ -20,6 +20,18 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + ValidationResultReady ValidationResult = iota + ValidationResultUnknown + ValidationResultError +) + +type ValidationResult uint8 + +func (v ValidationResult) String() string { + return [...]string{"Ready", "Unknown", "Error"}[v] +} + // +kubebuilder:object:root=false // +kubebuilder:object:generate:false // +k8s:deepcopy-gen:interfaces=nil @@ -47,8 +59,9 @@ type SecretsClient interface { GetSecret(ctx context.Context, ref ExternalSecretDataRemoteRef) ([]byte, error) // Validate checks if the client is configured correctly - // and is able to retrieve secrets from the provider - Validate() error + // and is able to retrieve secrets from the provider. + // If the validation result is unknown it will be ignored. + Validate() (ValidationResult, error) // GetSecretMap returns multiple k/v pairs from the provider GetSecretMap(ctx context.Context, ref ExternalSecretDataRemoteRef) (map[string][]byte, error) diff --git a/apis/externalsecrets/v1beta1/provider_schema_test.go b/apis/externalsecrets/v1beta1/provider_schema_test.go index 4a90412cf..3c1f1c77a 100644 --- a/apis/externalsecrets/v1beta1/provider_schema_test.go +++ b/apis/externalsecrets/v1beta1/provider_schema_test.go @@ -50,8 +50,8 @@ func (p *PP) Close(ctx context.Context) error { return nil } -func (p *PP) Validate() error { - return nil +func (p *PP) Validate() (ValidationResult, error) { + return ValidationResultReady, nil } func (p *PP) ValidateStore(store GenericStore) error { diff --git a/apis/externalsecrets/v1beta1/secretstore_types.go b/apis/externalsecrets/v1beta1/secretstore_types.go index 6108f8b88..77c0b62aa 100644 --- a/apis/externalsecrets/v1beta1/secretstore_types.go +++ b/apis/externalsecrets/v1beta1/secretstore_types.go @@ -32,6 +32,10 @@ type SecretStoreSpec struct { // Used to configure http retries if failed // +optional RetrySettings *SecretStoreRetrySettings `json:"retrySettings,omitempty"` + + // Used to configure store refresh interval in seconds. Empty or 0 will default to the controller config. + // +optional + RefreshInterval int `json:"refreshInterval"` } // SecretStoreProvider contains the provider-specific configration. diff --git a/cmd/root.go b/cmd/root.go index ece73f416..fbbaede7b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -55,6 +55,7 @@ var ( namespace string enableClusterStoreReconciler bool enableClusterExternalSecretReconciler bool + enableFloodGate bool storeRequeueInterval time.Duration serviceName, serviceNamespace string secretName, secretNamespace string @@ -138,6 +139,7 @@ var rootCmd = &cobra.Command{ ControllerClass: controllerClass, RequeueInterval: time.Hour, ClusterSecretStoreEnabled: enableClusterStoreReconciler, + EnableFloodGate: enableFloodGate, }).SetupWithManager(mgr, controller.Options{ MaxConcurrentReconciles: concurrent, }); err != nil { @@ -181,5 +183,6 @@ func init() { rootCmd.Flags().StringVar(&namespace, "namespace", "", "watch external secrets scoped in the provided namespace only. ClusterSecretStore can be used but only work if it doesn't reference resources from other namespaces") rootCmd.Flags().BoolVar(&enableClusterStoreReconciler, "enable-cluster-store-reconciler", true, "Enable cluster store reconciler.") rootCmd.Flags().BoolVar(&enableClusterExternalSecretReconciler, "enable-cluster-external-secret-reconciler", true, "Enable cluster external secret reconciler.") - rootCmd.Flags().DurationVar(&storeRequeueInterval, "store-requeue-interval", time.Minute*5, "Time duration between reconciling (Cluster)SecretStores") + rootCmd.Flags().DurationVar(&storeRequeueInterval, "store-requeue-interval", time.Minute*5, "Default Time duration between reconciling (Cluster)SecretStores") + rootCmd.Flags().BoolVar(&enableFloodGate, "enable-flood-gate", true, "Enable flood gate. External secret will be reconciled only if the ClusterStore or Store have an healthy or unknown state.") } diff --git a/config/crds/bases/external-secrets.io_clustersecretstores.yaml b/config/crds/bases/external-secrets.io_clustersecretstores.yaml index ad45abb64..16b297888 100644 --- a/config/crds/bases/external-secrets.io_clustersecretstores.yaml +++ b/config/crds/bases/external-secrets.io_clustersecretstores.yaml @@ -2704,6 +2704,10 @@ spec: - auth type: object type: object + refreshInterval: + description: Used to configure store refresh interval in seconds. + Empty or 0 will default to the controller config. + type: integer retrySettings: description: Used to configure http retries if failed properties: diff --git a/config/crds/bases/external-secrets.io_secretstores.yaml b/config/crds/bases/external-secrets.io_secretstores.yaml index f9ef87fbf..7cb8139ed 100644 --- a/config/crds/bases/external-secrets.io_secretstores.yaml +++ b/config/crds/bases/external-secrets.io_secretstores.yaml @@ -2707,6 +2707,10 @@ spec: - auth type: object type: object + refreshInterval: + description: Used to configure store refresh interval in seconds. + Empty or 0 will default to the controller config. + type: integer retrySettings: description: Used to configure http retries if failed properties: diff --git a/deploy/crds/bundle.yaml b/deploy/crds/bundle.yaml index 719c5a93e..617c9e65d 100644 --- a/deploy/crds/bundle.yaml +++ b/deploy/crds/bundle.yaml @@ -2316,6 +2316,9 @@ spec: - auth type: object type: object + refreshInterval: + description: Used to configure store refresh interval in seconds. Empty or 0 will default to the controller config. + type: integer retrySettings: description: Used to configure http retries if failed properties: @@ -4870,6 +4873,9 @@ spec: - auth type: object type: object + refreshInterval: + description: Used to configure store refresh interval in seconds. Empty or 0 will default to the controller config. + type: integer retrySettings: description: Used to configure http retries if failed properties: diff --git a/e2e/suite/akeyless/akeyless.go b/e2e/suite/akeyless/akeyless.go index d10292e0e..232a8a0c2 100644 --- a/e2e/suite/akeyless/akeyless.go +++ b/e2e/suite/akeyless/akeyless.go @@ -18,8 +18,6 @@ import ( // nolint . "github.com/onsi/ginkgo/v2" - // nolint - . "github.com/onsi/ginkgo/v2/extensions/table" "github.com/external-secrets/external-secrets/e2e/framework" "github.com/external-secrets/external-secrets/e2e/suite/common" diff --git a/e2e/suite/alibaba/alibaba.go b/e2e/suite/alibaba/alibaba.go index 7d1cba300..f56b5c496 100644 --- a/e2e/suite/alibaba/alibaba.go +++ b/e2e/suite/alibaba/alibaba.go @@ -18,8 +18,6 @@ import ( // nolint . "github.com/onsi/ginkgo/v2" - // nolint - . "github.com/onsi/ginkgo/v2/extensions/table" "github.com/external-secrets/external-secrets/e2e/framework" "github.com/external-secrets/external-secrets/e2e/suite/common" diff --git a/e2e/suite/gitlab/gitlab.go b/e2e/suite/gitlab/gitlab.go index 712a5442f..dc65440ba 100644 --- a/e2e/suite/gitlab/gitlab.go +++ b/e2e/suite/gitlab/gitlab.go @@ -21,8 +21,6 @@ import ( // nolint . "github.com/onsi/ginkgo/v2" - // nolint - . "github.com/onsi/ginkgo/v2/extensions/table" "github.com/external-secrets/external-secrets/e2e/framework" "github.com/external-secrets/external-secrets/e2e/suite/common" diff --git a/e2e/suite/oracle/oracle.go b/e2e/suite/oracle/oracle.go index 4fd54a22d..0e20e5f32 100644 --- a/e2e/suite/oracle/oracle.go +++ b/e2e/suite/oracle/oracle.go @@ -16,8 +16,6 @@ import ( // nolint . "github.com/onsi/ginkgo/v2" - // nolint - . "github.com/onsi/ginkgo/v2/extensions/table" "github.com/external-secrets/external-secrets/e2e/framework" "github.com/external-secrets/external-secrets/e2e/suite/common" diff --git a/pkg/controllers/externalsecret/externalsecret_controller.go b/pkg/controllers/externalsecret/externalsecret_controller.go index 1f93e9b1c..ea07e4342 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller.go +++ b/pkg/controllers/externalsecret/externalsecret_controller.go @@ -53,8 +53,10 @@ const ( errUpdateSecret = "could not update Secret" errPatchStatus = "unable to patch status" errGetSecretStore = "could not get SecretStore %q, %w" + errSecretStoreNotReady = "the desired SecretStore %s is not ready" errGetClusterSecretStore = "could not get ClusterSecretStore %q, %w" errStoreRef = "could not get store reference" + errStoreUsability = "could not use store reference" errStoreProvider = "could not get store provider" errStoreClient = "could not get provider client" errGetExistingSecret = "could not get existing secret: %w" @@ -82,6 +84,7 @@ type Reconciler struct { ControllerClass string RequeueInterval time.Duration ClusterSecretStoreEnabled bool + EnableFloodGate bool recorder record.EventRecorder } @@ -133,7 +136,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionFalse, esv1beta1.ConditionReasonSecretSyncedError, errStoreRef) SetExternalSecretCondition(&externalSecret, *conditionSynced) syncCallsError.With(syncCallsMetricLabels).Inc() - return ctrl.Result{RequeueAfter: requeueAfter}, nil + return ctrl.Result{}, err } log = log.WithValues("SecretStore", store.GetNamespacedName()) @@ -144,6 +147,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, nil } + if r.EnableFloodGate { + if err = assertStoreIsUsable(store); err != nil { + log.Error(err, errStoreUsability) + r.recorder.Event(&externalSecret, v1.EventTypeWarning, esv1beta1.ReasonUnavailableStore, err.Error()) + conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionFalse, esv1beta1.ConditionReasonSecretSyncedError, errStoreUsability) + SetExternalSecretCondition(&externalSecret, *conditionSynced) + syncCallsError.With(syncCallsMetricLabels).Inc() + return ctrl.Result{}, err + } + } + storeProvider, err := esv1beta1.GetProvider(store) if err != nil { log.Error(err, errStoreProvider) @@ -158,7 +172,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu SetExternalSecretCondition(&externalSecret, *conditionSynced) r.recorder.Event(&externalSecret, v1.EventTypeWarning, esv1beta1.ReasonProviderClientConfig, err.Error()) syncCallsError.With(syncCallsMetricLabels).Inc() - return ctrl.Result{RequeueAfter: requeueAfter}, nil + return ctrl.Result{}, err } defer func() { @@ -467,7 +481,14 @@ func isSecretValid(existingSecret v1.Secret) bool { return true } -// getStore returns the store with the provided ExternalSecret. +// assertStoreIsUsable assert that the store is ready to use. +func assertStoreIsUsable(store esv1beta1.GenericStore) error { + if secretstore.GetSecretStoreCondition(store.GetStatus(), esv1beta1.SecretStoreReady).Status != v1.ConditionTrue { + return fmt.Errorf(errSecretStoreNotReady, store.GetName()) + } + return nil +} + func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1beta1.ExternalSecret) (esv1beta1.GenericStore, error) { ref := types.NamespacedName{ Name: externalSecret.Spec.SecretStoreRef.Name, @@ -479,7 +500,6 @@ func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1beta1.Ext if err != nil { return nil, fmt.Errorf(errGetClusterSecretStore, ref.Name, err) } - return &store, nil } @@ -490,6 +510,7 @@ func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1beta1.Ext if err != nil { return nil, fmt.Errorf(errGetSecretStore, ref.Name, err) } + return &store, nil } diff --git a/pkg/controllers/secretstore/common.go b/pkg/controllers/secretstore/common.go index cfb5d86f7..c6714beea 100644 --- a/pkg/controllers/secretstore/common.go +++ b/pkg/controllers/secretstore/common.go @@ -46,6 +46,10 @@ func reconcile(ctx context.Context, req ctrl.Request, ss esapi.GenericStore, cl return ctrl.Result{}, nil } + if ss.GetSpec().RefreshInterval != 0 { + requeueInterval = time.Second * time.Duration(ss.GetSpec().RefreshInterval) + } + // patch status when done processing p := client.MergeFrom(ss.Copy()) defer func() { @@ -93,8 +97,8 @@ func validateStore(ctx context.Context, namespace string, store esapi.GenericSto } defer cl.Close(ctx) - err = cl.Validate() - if err != nil { + validationResult, err := cl.Validate() + if err != nil && validationResult != esapi.ValidationResultUnknown { cond := NewSecretStoreCondition(esapi.SecretStoreReady, v1.ConditionFalse, esapi.ReasonValidationFailed, errUnableValidateStore) SetExternalSecretCondition(store, *cond) recorder.Event(store, v1.EventTypeWarning, esapi.ReasonValidationFailed, err.Error()) diff --git a/pkg/provider/akeyless/akeyless.go b/pkg/provider/akeyless/akeyless.go index abd385785..883bedfa6 100644 --- a/pkg/provider/akeyless/akeyless.go +++ b/pkg/provider/akeyless/akeyless.go @@ -111,11 +111,15 @@ func (a *Akeyless) Close(ctx context.Context) error { return nil } -func (a *Akeyless) Validate() error { +func (a *Akeyless) Validate() (esv1beta1.ValidationResult, error) { timeout := 15 * time.Second url := a.url - return utils.NetworkValidate(url, timeout) + if err := utils.NetworkValidate(url, timeout); err != nil { + return esv1beta1.ValidationResultError, err + } + + return esv1beta1.ValidationResultReady, nil } // Implements store.Client.GetSecret Interface. diff --git a/pkg/provider/alibaba/kms.go b/pkg/provider/alibaba/kms.go index 0333588b0..7358f493d 100644 --- a/pkg/provider/alibaba/kms.go +++ b/pkg/provider/alibaba/kms.go @@ -197,11 +197,14 @@ func (kms *KeyManagementService) Close(ctx context.Context) error { return nil } -func (kms *KeyManagementService) Validate() error { +func (kms *KeyManagementService) Validate() (esv1beta1.ValidationResult, error) { timeout := 15 * time.Second url := kms.url - return utils.NetworkValidate(url, timeout) + if err := utils.NetworkValidate(url, timeout); err != nil { + return esv1beta1.ValidationResultError, err + } + return esv1beta1.ValidationResultReady, nil } func (kms *KeyManagementService) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/aws/parameterstore/parameterstore.go b/pkg/provider/aws/parameterstore/parameterstore.go index 208e8e4e6..102c83384 100644 --- a/pkg/provider/aws/parameterstore/parameterstore.go +++ b/pkg/provider/aws/parameterstore/parameterstore.go @@ -225,7 +225,10 @@ func (pm *ParameterStore) Close(ctx context.Context) error { return nil } -func (pm *ParameterStore) Validate() error { +func (pm *ParameterStore) Validate() (esv1beta1.ValidationResult, error) { _, err := pm.sess.Config.Credentials.Get() - return err + if err != nil { + return esv1beta1.ValidationResultError, err + } + return esv1beta1.ValidationResultReady, nil } diff --git a/pkg/provider/aws/secretsmanager/secretsmanager.go b/pkg/provider/aws/secretsmanager/secretsmanager.go index c3f0b9ba3..cbb37c44c 100644 --- a/pkg/provider/aws/secretsmanager/secretsmanager.go +++ b/pkg/provider/aws/secretsmanager/secretsmanager.go @@ -287,7 +287,10 @@ func (sm *SecretsManager) Close(ctx context.Context) error { return nil } -func (sm *SecretsManager) Validate() error { +func (sm *SecretsManager) Validate() (esv1beta1.ValidationResult, error) { _, err := sm.sess.Config.Credentials.Get() - return err + if err != nil { + return esv1beta1.ValidationResultError, err + } + return esv1beta1.ValidationResultReady, nil } diff --git a/pkg/provider/azure/keyvault/keyvault.go b/pkg/provider/azure/keyvault/keyvault.go index 5b39bdb6d..3a1a64687 100644 --- a/pkg/provider/azure/keyvault/keyvault.go +++ b/pkg/provider/azure/keyvault/keyvault.go @@ -480,8 +480,8 @@ func (a *Azure) Close(ctx context.Context) error { return nil } -func (a *Azure) Validate() error { - return nil +func (a *Azure) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func getObjType(ref esv1beta1.ExternalSecretDataRemoteRef) (string, string) { diff --git a/pkg/provider/fake/fake.go b/pkg/provider/fake/fake.go index a290329aa..8d68c9aed 100644 --- a/pkg/provider/fake/fake.go +++ b/pkg/provider/fake/fake.go @@ -94,8 +94,8 @@ func (p *Provider) Close(ctx context.Context) error { return nil } -func (p *Provider) Validate() error { - return nil +func (p *Provider) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func (p *Provider) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/gcp/secretmanager/secretsmanager.go b/pkg/provider/gcp/secretmanager/secretsmanager.go index 62908a2f3..697bf35ff 100644 --- a/pkg/provider/gcp/secretmanager/secretsmanager.go +++ b/pkg/provider/gcp/secretmanager/secretsmanager.go @@ -423,8 +423,8 @@ func (sm *ProviderGCP) Close(ctx context.Context) error { return nil } -func (sm *ProviderGCP) Validate() error { - return nil +func (sm *ProviderGCP) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func (sm *ProviderGCP) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index bd86ca0a7..ca91529da 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -226,14 +226,14 @@ func (g *Gitlab) Close(ctx context.Context) error { } // Validate will use the gitlab client to validate the gitlab provider using the ListVariable call to ensure get permissions without needing a specific key. -func (g *Gitlab) Validate() error { +func (g *Gitlab) Validate() (esv1beta1.ValidationResult, error) { _, resp, err := g.client.ListVariables(g.projectID, nil) if err != nil { - return fmt.Errorf(errList, err) + return esv1beta1.ValidationResultError, fmt.Errorf(errList, err) } else if resp == nil || resp.StatusCode != http.StatusOK { - return fmt.Errorf(errAuth) + return esv1beta1.ValidationResultError, fmt.Errorf(errAuth) } - return nil + return esv1beta1.ValidationResultReady, nil } func (g *Gitlab) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index cdb3bf5f1..aa2c27c4c 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -28,33 +28,35 @@ import ( ) type secretManagerTestCase struct { - mockClient *fakegitlab.GitlabMockClient - apiInputProjectID string - apiInputKey string - apiOutput *gitlab.ProjectVariable - apiResponse *gitlab.Response - ref *esv1beta1.ExternalSecretDataRemoteRef - projectID *string - apiErr error - expectError string - expectedSecret string + mockClient *fakegitlab.GitlabMockClient + apiInputProjectID string + apiInputKey string + apiOutput *gitlab.ProjectVariable + apiResponse *gitlab.Response + ref *esv1beta1.ExternalSecretDataRemoteRef + projectID *string + apiErr error + expectError string + expectedSecret string + expectedValidationResult esv1beta1.ValidationResult // for testing secretmap expectedData map[string][]byte } func makeValidSecretManagerTestCase() *secretManagerTestCase { smtc := secretManagerTestCase{ - mockClient: &fakegitlab.GitlabMockClient{}, - apiInputProjectID: makeValidAPIInputProjectID(), - apiInputKey: makeValidAPIInputKey(), - ref: makeValidRef(), - projectID: nil, - apiOutput: makeValidAPIOutput(), - apiResponse: makeValidAPIResponse(), - apiErr: nil, - expectError: "", - expectedSecret: "", - expectedData: map[string][]byte{}, + mockClient: &fakegitlab.GitlabMockClient{}, + apiInputProjectID: makeValidAPIInputProjectID(), + apiInputKey: makeValidAPIInputKey(), + ref: makeValidRef(), + projectID: nil, + apiOutput: makeValidAPIOutput(), + apiResponse: makeValidAPIResponse(), + apiErr: nil, + expectError: "", + expectedSecret: "", + expectedValidationResult: esv1beta1.ValidationResultReady, + expectedData: map[string][]byte{}, } smtc.mockClient.WithValue(smtc.apiInputProjectID, smtc.apiInputKey, smtc.apiOutput, smtc.apiResponse, smtc.apiErr) return &smtc @@ -104,22 +106,26 @@ func makeValidSecretManagerTestCaseCustom(tweaks ...func(smtc *secretManagerTest var setAPIErr = func(smtc *secretManagerTestCase) { smtc.apiErr = fmt.Errorf("oh no") smtc.expectError = "oh no" + smtc.expectedValidationResult = esv1beta1.ValidationResultError } var setListAPIErr = func(smtc *secretManagerTestCase) { err := fmt.Errorf("oh no") smtc.apiErr = err smtc.expectError = fmt.Errorf(errList, err).Error() + smtc.expectedValidationResult = esv1beta1.ValidationResultError } var setListAPIRespNil = func(smtc *secretManagerTestCase) { smtc.apiResponse = nil smtc.expectError = errAuth + smtc.expectedValidationResult = esv1beta1.ValidationResultError } var setListAPIRespBadCode = func(smtc *secretManagerTestCase) { smtc.apiResponse.StatusCode = http.StatusUnauthorized smtc.expectError = errAuth + smtc.expectedValidationResult = esv1beta1.ValidationResultError } var setNilMockClient = func(smtc *secretManagerTestCase) { @@ -172,10 +178,13 @@ func TestValidate(t *testing.T) { for k, v := range successCases { sm.client = v.mockClient t.Logf("%+v", v) - err := sm.Validate() + validationResult, err := sm.Validate() if !ErrorContains(err, v.expectError) { t.Errorf("[%d], unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError) } + if validationResult != v.expectedValidationResult { + t.Errorf("[%d], unexpected validationResult: %s, expected: '%s'", k, validationResult, v.expectedValidationResult) + } } } diff --git a/pkg/provider/ibm/provider.go b/pkg/provider/ibm/provider.go index 28ea3ee5b..e24dfdba3 100644 --- a/pkg/provider/ibm/provider.go +++ b/pkg/provider/ibm/provider.go @@ -496,8 +496,8 @@ func (ibm *providerIBM) Close(ctx context.Context) error { return nil } -func (ibm *providerIBM) Validate() error { - return nil +func (ibm *providerIBM) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func (ibm *providerIBM) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/kubernetes/kubernetes.go b/pkg/provider/kubernetes/kubernetes.go index c9cdaa68c..6a520f608 100644 --- a/pkg/provider/kubernetes/kubernetes.go +++ b/pkg/provider/kubernetes/kubernetes.go @@ -241,7 +241,7 @@ func (k *BaseClient) fetchSecretKey(ctx context.Context, key esmeta.SecretKeySel return val, nil } -func (k *ProviderKubernetes) Validate() error { +func (k *ProviderKubernetes) Validate() (esv1beta1.ValidationResult, error) { ctx := context.Background() authReview, err := k.ReviewClient.Create(ctx, &authv1.SelfSubjectAccessReview{ @@ -255,14 +255,14 @@ func (k *ProviderKubernetes) Validate() error { }, metav1.CreateOptions{}) if err != nil { - return fmt.Errorf("could not verify if client is valid: %w", err) + return esv1beta1.ValidationResultUnknown, fmt.Errorf("could not verify if client is valid: %w", err) } if !authReview.Status.Allowed { - return fmt.Errorf("client is not allowed to get secrets") + return esv1beta1.ValidationResultError, fmt.Errorf("client is not allowed to get secrets") } - return nil + return esv1beta1.ValidationResultReady, nil } func (k *ProviderKubernetes) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/kubernetes/kubernetes_test.go b/pkg/provider/kubernetes/kubernetes_test.go index 89534d75b..57cb44e17 100644 --- a/pkg/provider/kubernetes/kubernetes_test.go +++ b/pkg/provider/kubernetes/kubernetes_test.go @@ -373,10 +373,14 @@ func TestValidate(t *testing.T) { } fakeClient := fakeReviewClient{authReview: &authReview} k := ProviderKubernetes{ReviewClient: fakeClient} - err := k.Validate() + validationResult, err := k.Validate() if err != nil { t.Errorf("Test Failed! %v", err) } + if validationResult != esv1beta1.ValidationResultReady { + t.Errorf("Test Failed! Wanted could not indicate validationResult is %s, got: %s", esv1beta1.ValidationResultReady, validationResult) + } + authReview = authv1.SelfSubjectAccessReview{ Status: authv1.SubjectAccessReviewStatus{ Allowed: false, @@ -384,15 +388,21 @@ func TestValidate(t *testing.T) { } fakeClient = fakeReviewClient{authReview: &authReview} k = ProviderKubernetes{ReviewClient: fakeClient} - err = k.Validate() + validationResult, err = k.Validate() if err.Error() != "client is not allowed to get secrets" { t.Errorf("Test Failed! Wanted client is not allowed to get secrets got: %v", err) } + if validationResult != esv1beta1.ValidationResultError { + t.Errorf("Test Failed! Wanted could not indicate validationResult is %s, got: %s", esv1beta1.ValidationResultError, validationResult) + } fakeClient = fakeReviewClient{} k = ProviderKubernetes{ReviewClient: fakeClient} - err = k.Validate() + validationResult, err = k.Validate() if err.Error() != "could not verify if client is valid: Something went wrong" { t.Errorf("Test Failed! Wanted could not verify if client is valid: Something went wrong got: %v", err) } + if validationResult != esv1beta1.ValidationResultUnknown { + t.Errorf("Test Failed! Wanted could not indicate validationResult is %s, got: %s", esv1beta1.ValidationResultUnknown, validationResult) + } } diff --git a/pkg/provider/oracle/oracle.go b/pkg/provider/oracle/oracle.go index 122cea5ab..a0fd4a757 100644 --- a/pkg/provider/oracle/oracle.go +++ b/pkg/provider/oracle/oracle.go @@ -223,8 +223,8 @@ func (vms *VaultManagementService) Close(ctx context.Context) error { return nil } -func (vms *VaultManagementService) Validate() error { - return nil +func (vms *VaultManagementService) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func (vms *VaultManagementService) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/testing/fake/fake.go b/pkg/provider/testing/fake/fake.go index 7c6f9f634..df11397b4 100644 --- a/pkg/provider/testing/fake/fake.go +++ b/pkg/provider/testing/fake/fake.go @@ -26,8 +26,7 @@ var _ esv1beta1.Provider = &Client{} // Client is a fake client for testing. type Client struct { - NewFn func(context.Context, esv1beta1.GenericStore, client.Client, - string) (esv1beta1.SecretsClient, error) + NewFn func(context.Context, esv1beta1.GenericStore, client.Client, string) (esv1beta1.SecretsClient, error) GetSecretFn func(context.Context, esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) GetSecretMapFn func(context.Context, esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) GetAllSecretsFn func(context.Context, esv1beta1.ExternalSecretFind) (map[string][]byte, error) @@ -59,7 +58,7 @@ func (v *Client) RegisterAs(provider *esv1beta1.SecretStoreProvider) { esv1beta1.ForceRegister(v, provider) } -// GetSecret implements the provider.Provider interface. +// GetAllSecrets implements the provider.Provider interface. func (v *Client) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) { return v.GetAllSecretsFn(ctx, ref) } @@ -77,7 +76,7 @@ func (v *Client) WithGetSecret(secData []byte, err error) *Client { return v } -// GetSecretMap imeplements the provider.Provider interface. +// GetSecretMap implements the provider.Provider interface. func (v *Client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) { return v.GetSecretMapFn(ctx, ref) } @@ -86,8 +85,8 @@ func (v *Client) Close(ctx context.Context) error { return nil } -func (v *Client) Validate() error { - return nil +func (v *Client) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func (v *Client) ValidateStore(store esv1beta1.GenericStore) error { diff --git a/pkg/provider/vault/vault.go b/pkg/provider/vault/vault.go index 57312f2d5..eb1d6684a 100644 --- a/pkg/provider/vault/vault.go +++ b/pkg/provider/vault/vault.go @@ -576,12 +576,12 @@ func (v *client) Close(ctx context.Context) error { return nil } -func (v *client) Validate() error { +func (v *client) Validate() (esv1beta1.ValidationResult, error) { _, err := checkToken(context.Background(), v) if err != nil { - return fmt.Errorf(errInvalidCredentials, err) + return esv1beta1.ValidationResultError, fmt.Errorf(errInvalidCredentials, err) } - return nil + return esv1beta1.ValidationResultReady, nil } func (v *client) buildMetadataPath(path string) (string, error) { diff --git a/pkg/provider/webhook/webhook.go b/pkg/provider/webhook/webhook.go index 2fa3a9b56..dc19f704c 100644 --- a/pkg/provider/webhook/webhook.go +++ b/pkg/provider/webhook/webhook.go @@ -394,11 +394,14 @@ func (w *WebHook) Close(ctx context.Context) error { return nil } -func (w *WebHook) Validate() error { +func (w *WebHook) Validate() (esv1beta1.ValidationResult, error) { timeout := 15 * time.Second url := w.url - return utils.NetworkValidate(url, timeout) + if err := utils.NetworkValidate(url, timeout); err != nil { + return esv1beta1.ValidationResultError, err + } + return esv1beta1.ValidationResultReady, nil } func executeTemplateString(tmpl string, data map[string]map[string]string) (string, error) { diff --git a/pkg/provider/yandex/lockbox/lockbox.go b/pkg/provider/yandex/lockbox/lockbox.go index fdfc3a5e0..3c32332f6 100644 --- a/pkg/provider/yandex/lockbox/lockbox.go +++ b/pkg/provider/yandex/lockbox/lockbox.go @@ -288,8 +288,8 @@ func (c *lockboxSecretsClient) Close(ctx context.Context) error { return nil } -func (c *lockboxSecretsClient) Validate() error { - return nil +func (c *lockboxSecretsClient) Validate() (esv1beta1.ValidationResult, error) { + return esv1beta1.ValidationResultReady, nil } func getValueAsIs(entry *lockbox.Payload_Entry) (interface{}, error) {