fix: clarify namespace requirements for ClusterSecretStore + improve test

Signed-off-by: Jordan Sauvain <jordan.sauvain@ovhcloud.com>

apis/externalsecrets/v1/secretstore_ovh_types.go

@@ -21,20 +21,20 @@ import esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
 // OvhProvider holds the configuration to synchronize secrets with OVHcloud's Secret Manager.
 // +optional
 type OvhProvider struct {
-	// specifies the OKMS server endpoint
+	// specifies the OKMS server endpoint.
 	// +required
 	Server string `json:"server"`
-	// specifies the OKMS ID
+	// specifies the OKMS ID.
 	// +required
 	OkmsID string `json:"okmsid"`
-	// Enables or disables check-and-set (CAS) (default: false)
+	// Enables or disables check-and-set (CAS) (default: false).
 	// +optional
 	CasRequired *bool `json:"casRequired,omitempty"`
-	// Setup a timeout in seconds when requests to the KMS are made (default: 30)
+	// Setup a timeout in seconds when requests to the KMS are made (default: 30).
 	// +optional
 	// +kubebuilder:default=30
 	OkmsTimeout *uint32 `json:"okmsTimeout,omitempty"`
-	// Authentication method (mtls or token)
+	// Authentication method (mtls or token).
 	// +required
 	Auth OvhAuth `json:"auth"`
 }

docs/provider/ovhcloud.md

@@ -1,6 +1,6 @@
 ## Secrets Manager
 
-External Secrets Operator integrates with [OVHcloud KMS](https://www.ovhcloud.com/fr/identity-security-operations/key-management-service/).  
+External Secrets Operator integrates with [OVHcloud KMS](https://www.ovhcloud.com/en/identity-security-operations/key-management-service/).  
 
 This guide demonstrates:
 - how to set up a `ClusterSecretStore`/`SecretStore` with the OVH provider.
@@ -27,7 +27,6 @@ mTLS authentication:
 
 !!! note
      A `ClusterSecretStore` configuration is the same except you have to provide the `tokenSecretRef` `namespace`.  
-     `ExternalSecret` objects must be in the same `namespace` as the `tokenSecretRef` provided to your `ClusterSecretStore`.
 
 ### <u>ExternalSecret</u>
  
@@ -195,7 +194,7 @@ Resulting Kubernetes Secret data:
 ### <u>PushSecret</u>
 
 #### Check-And-Set
-Check-And-Set can be enabled/disabled (default: enabled), in the Secret Store configuration:
+Check-And-Set can be enabled/disabled (default: disabled), in the Secret Store configuration:
 ```yaml
 {% include 'ovh-secret-store-cas.yaml' %}
 ```

docs/snippets/ovh-push-secret-rotation.yaml

@@ -28,4 +28,4 @@ spec:
     - match:
         secretKey: password # property in the generator output
         remoteRef:
-          remoteKey: prod/myql/password
+          remoteKey: prod/mysql/password

docs/snippets/ovh-token-secret-store.yaml

@@ -19,4 +19,4 @@ kind: Secret
 metadata:
   name: ovh-token
 data:
-  token: dG9rZW4gdmFsdWU= # "token value"
+  token: BASE64-TOKEN-VALUE-PLACEHOLDER

providers/v1/ovh/client_get_all_secrets.go

@@ -27,7 +27,7 @@ import (
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
 
-// GetAllSecret retrieves multiple secrets from the Secret Manager.
+// GetAllSecrets retrieves multiple secrets from the Secret Manager.
 // You can optionally filter secrets by name using a regular expression.
 // When path is set to "/" or left empty, the search starts from the Secret Manager root.
 func (cl *ovhClient) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretFind) (map[string][]byte, error) {
@@ -190,7 +190,7 @@ func filterSecretsListWithRegexp(ctx context.Context, cl *ovhClient, secrets []s
 		}
 	}
 	if len(secretsDataMap) == 0 {
-		return map[string][]byte{}, errors.New("no secrets matched the regexp")
+		return map[string][]byte{}, errors.New("no secrets could be retrieved")
 	}
 	return secretsDataMap, nil
 }

providers/v1/ovh/client_get_secret_map_test.go

@@ -49,7 +49,7 @@ func TestGetSecretMap(t *testing.T) {
 			},
 		},
 		"Secret without data": {
-			shouldmap: map[string][]byte{},
+			errshould: "secret version data is missing",
 			ref: esv1.ExternalSecretDataRemoteRef{
 				Key: "key",
 			},

providers/v1/ovh/client_get_secret_test.go

@@ -46,7 +46,7 @@ func TestGetSecret(t *testing.T) {
 			},
 		},
 		"Secret without data": {
-			errshould: "empty secret",
+			errshould: "secret version data is missing",
 			ref: esv1.ExternalSecretDataRemoteRef{
 				Key: "key",
 			},

providers/v1/ovh/client_secret_exists.go

@@ -26,9 +26,11 @@ import (
 func (cl *ovhClient) SecretExists(ctx context.Context, remoteRef esv1.PushSecretRemoteRef) (bool, error) {
 	// Check if the secret exists using the OVH SDK.
 	_, err := cl.okmsClient.GetSecretV2(ctx, cl.okmsID, remoteRef.GetRemoteKey(), nil, nil)
-	if err != nil && errors.Is(handleOkmsError(err), esv1.NoSecretErr) {
-		return false, nil
-	} else if err != nil {
+	if err != nil {
+		err = handleOkmsError(err)
+		if errors.Is(err, esv1.NoSecretErr) {
+			return false, nil
+		}
 		return false, err
 	}
 

providers/v1/ovh/client_secret_exists_test.go

@@ -42,7 +42,7 @@ func TestSecretExists(t *testing.T) {
 			remoteRef: testingfake.PushSecretData{},
 		},
 		"Error case": {
-			errshould: "SecretExists error",
+			errshould: "failed to parse okms error: SecretExists error",
 			remoteRef: testingfake.PushSecretData{},
 		},
 	}
@@ -56,9 +56,19 @@ func TestSecretExists(t *testing.T) {
 			}
 			ctx := context.Background()
 			exists, err := cl.SecretExists(ctx, testCase.remoteRef)
-			if testCase.errshould != "" && err != nil && err.Error() != testCase.errshould {
-				t.Error()
-			} else if (testCase.should && !exists) || (!testCase.should && exists) {
+			if testCase.errshould != "" {
+				if err == nil {
+					t.Error()
+				}
+				if err.Error() != testCase.errshould {
+					t.Error()
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if exists != testCase.should {
 				t.Error()
 			}
 		})

providers/v1/ovh/client_utils.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"strconv"
 
 	"github.com/google/uuid"
@@ -80,20 +81,22 @@ func getSecretWithOvhSDK(ctx context.Context, kmsClient OkmsClient, okmsID uuid.
 //
 // Returns nil if no version is provided; in that case, the OVH SDK uses the latest version.
 func decodeSecretVersion(strVersion string) (*uint32, error) {
-	var version *uint32
+	var version uint32
 
 	if strVersion != "" {
 		v, err := strconv.Atoi(strVersion)
 		if err != nil {
-			return version, err
+			return nil, err
 		}
-		tmpVersion := uint32(v)
-		if int(tmpVersion) != v {
+		if v < 0 || v > math.MaxUint32 {
 			return nil, errors.New("overflow occurred while decoding secret version")
 		}
-		version = &tmpVersion
+		version = uint32(v)
+	} else {
+		return nil, nil
 	}
-	return version, nil
+
+	return &version, nil
 }
 
 // Retrieve the value of the secret property.

providers/v1/ovh/fake/fake_okms_client.go

@@ -28,6 +28,8 @@ import (
 	"github.com/ovh/okms-sdk-go/types"
 )
 
+const str = "string"
+
 type FakeOkmsClient struct {
 	TestCase string
 }
@@ -37,10 +39,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 
 	// Metadata
 	CasRequired := true
-	CreatedAt := "string"
-	DeactivateVersionAfter := "string"
+	CreatedAt := str
+	DeactivateVersionAfter := str
 	MaxVersions := uint32(10)
-	UpdatedAt := "string"
+	UpdatedAt := str
+	var State types.SecretV2State = str
 
 	// Version
 	Data := map[string]any{
@@ -65,11 +68,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 		},
 		Path: &kmsClient.TestCase,
 		Version: &types.SecretV2Version{
-			CreatedAt:     "string",
+			CreatedAt:     CreatedAt,
 			Data:          &Data,
 			DeactivatedAt: &DeactivatedAt,
 			Id:            1,
-			State:         "string",
+			State:         State,
 			Warnings:      &Warnings,
 		},
 	}
@@ -83,11 +86,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 		},
 		Path: &kmsClient.TestCase,
 		Version: &types.SecretV2Version{
-			CreatedAt:     "string",
+			CreatedAt:     CreatedAt,
 			Data:          &NestedData,
 			DeactivatedAt: &DeactivatedAt,
 			Id:            1,
-			State:         "string",
+			State:         State,
 			Warnings:      &Warnings,
 		},
 	}
@@ -112,11 +115,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
-				Data:          &map[string]interface{}{},
+				CreatedAt:     CreatedAt,
+				Data:          nil,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -188,11 +191,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data1,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -207,11 +210,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data2,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -226,11 +229,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data3,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -245,11 +248,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data4,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -264,11 +267,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data5,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -283,11 +286,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data6,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -302,11 +305,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data7,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil
@@ -321,11 +324,11 @@ func (kmsClient FakeOkmsClient) GetSecretV2(_ context.Context, _ uuid.UUID, path
 			},
 			Path: &kmsClient.TestCase,
 			Version: &types.SecretV2Version{
-				CreatedAt:     "string",
+				CreatedAt:     CreatedAt,
 				Data:          &data8,
 				DeactivatedAt: &DeactivatedAt,
 				Id:            1,
-				State:         "string",
+				State:         State,
 				Warnings:      &Warnings,
 			},
 		}, nil