فهرست منبع

fix: fix sonarQube issues (cognitive complexity + duplication)

Signed-off-by: Jordan Sauvain <jordan.sauvain@ovhcloud.com>
Jordan Sauvain 4 ماه پیش
والد
کامیت
441c06f28e

+ 53 - 26
providers/v1/ovh/client_get_all_secrets.go

@@ -28,6 +28,8 @@ import (
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
 
+const retrieveMultipleSecretsError = "failed to retrieve multiple secrets"
+
 // 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.
@@ -35,7 +37,7 @@ func (cl *ovhClient) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretF
 	// List Secret Manager secrets.
 	secrets, err := getSecretsList(ctx, cl.okmsClient, cl.okmsID, ref.Path)
 	if err != nil {
-		return map[string][]byte{}, fmt.Errorf("failed to retrieve multiple secrets: %w", err)
+		return map[string][]byte{}, fmt.Errorf("%s: %w", retrieveMultipleSecretsError, err)
 	}
 	if len(secrets) == 0 {
 		path := ""
@@ -43,7 +45,8 @@ func (cl *ovhClient) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretF
 			path = *ref.Path
 		}
 		return map[string][]byte{}, fmt.Errorf(
-			"failed to retrieve multiple secrets: no secrets under path %q were found in the secret manager",
+			"%s: no secrets under path %q were found in the secret manager",
+			retrieveMultipleSecretsError,
 			path,
 		)
 	}
@@ -55,12 +58,15 @@ func (cl *ovhClient) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretF
 		regex, err = regexp.Compile(ref.Name.RegExp)
 		if err != nil {
 			return map[string][]byte{}, fmt.Errorf(
-				"failed to retrieve multiple secrets: could not parse regex: %w", err,
+				"%s: could not parse regex: %w",
+				retrieveMultipleSecretsError,
+				err,
 			)
 		}
 		if regex == nil {
 			return map[string][]byte{}, fmt.Errorf(
-				"failed to retrieve multiple secrets: compiled regex is nil for expression %q",
+				"%s: compiled regex is nil for expression %q",
+				retrieveMultipleSecretsError,
 				ref.Name.RegExp,
 			)
 		}
@@ -68,7 +74,7 @@ func (cl *ovhClient) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretF
 
 	secretsMap, err := filterSecretsListWithRegexp(ctx, cl, secrets, regex, ref)
 	if err != nil {
-		return map[string][]byte{}, fmt.Errorf("failed to retrieve multiple secrets: %w", err)
+		return map[string][]byte{}, fmt.Errorf("%s: %w", retrieveMultipleSecretsError, err)
 	}
 
 	return secretsMap, nil
@@ -196,30 +202,51 @@ func filterSecretsListWithRegexp(ctx context.Context, cl *ovhClient, secrets []s
 	for _, secret := range secrets {
 		// Insert the secret if no regex is provided;
 		// otherwise, insert only matching secrets.
-		if ref.Name == nil || (regex != nil && regex.MatchString(secret)) {
-			secretToInsert, err := cl.GetSecret(ctx, esv1.ExternalSecretDataRemoteRef{
-				Key:                secret,
-				ConversionStrategy: ref.ConversionStrategy,
-				DecodingStrategy:   ref.DecodingStrategy,
-			})
-			if err != nil && !errors.Is(err, esv1.NoSecretErr) {
-				return map[string][]byte{}, err
-			}
-			if !errors.Is(err, esv1.NoSecretErr) {
-				secretsDataMap[secret] = secretToInsert
-			}
+		secretData, ok, err := fetchSecretData(ctx, cl, secret, regex, ref)
+		if ok {
+			secretsDataMap[secret] = secretData
+		}
+		if err != nil {
+			return map[string][]byte{}, err
 		}
 	}
 	if len(secretsDataMap) == 0 {
-		expr := ""
-		if ref.Name != nil {
-			expr = ref.Name.RegExp
-		}
-		path := ""
-		if ref.Path != nil {
-			path = *ref.Path
-		}
-		return map[string][]byte{}, fmt.Errorf("regex expression %q did not match any secret at path %q", expr, path)
+		return map[string][]byte{}, buildNoMatchError(ref.Name, ref.Path)
 	}
 	return secretsDataMap, nil
 }
+
+// fetchSecretData retrieves a secret data if it passes the name/regex filter.
+func fetchSecretData(ctx context.Context, cl *ovhClient, secret string, regex *regexp.Regexp, ref esv1.ExternalSecretFind) ([]byte, bool, error) {
+	// Skip the secret if a name filter is defined but the regex is nil or does not match.
+	if ref.Name != nil && (regex == nil || !regex.MatchString(secret)) {
+		return nil, false, nil
+	}
+
+	// fetch secret data
+	secretData, err := cl.GetSecret(ctx, esv1.ExternalSecretDataRemoteRef{
+		Key:                secret,
+		ConversionStrategy: ref.ConversionStrategy,
+		DecodingStrategy:   ref.DecodingStrategy,
+	})
+	if err != nil {
+		if errors.Is(err, esv1.NoSecretErr) {
+			return nil, false, nil
+		}
+		return nil, false, err
+	}
+
+	return secretData, true, nil
+}
+
+func buildNoMatchError(findName *esv1.FindName, findPath *string) error {
+	expr := ""
+	if findName != nil {
+		expr = findName.RegExp
+	}
+	path := ""
+	if findPath != nil {
+		path = *findPath
+	}
+	return fmt.Errorf("regex expression %q did not match any secret at path %q", expr, path)
+}

+ 5 - 3
providers/v1/ovh/client_get_secret_map.go

@@ -26,6 +26,8 @@ import (
 	"github.com/external-secrets/external-secrets/runtime/esutils"
 )
 
+const retrieveSecretError = "failed to retrieve secret at path"
+
 // GetSecretMap retrieves a single secret from the provider.
 // The created secret will have the same keys as the Secret Manager secret.
 // You can specify a key, a property, and a version.
@@ -34,7 +36,7 @@ func (cl *ovhClient) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDa
 	// Retrieve secret from KMS.
 	secretDataBytes, _, err := getSecretWithOvhSDK(ctx, cl.okmsClient, cl.okmsID, ref)
 	if err != nil && !errors.Is(err, esv1.NoSecretErr) {
-		return map[string][]byte{}, fmt.Errorf("failed to retrieve secret at path %q: %w", ref.Key, err)
+		return map[string][]byte{}, fmt.Errorf("%s %q: %w", retrieveSecretError, ref.Key, err)
 	} else if err != nil {
 		return map[string][]byte{}, err
 	}
@@ -47,7 +49,7 @@ func (cl *ovhClient) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDa
 	var rawSecretDataMap map[string]any
 	err = json.Unmarshal(secretDataBytes, &rawSecretDataMap)
 	if err != nil {
-		return map[string][]byte{}, fmt.Errorf("failed to retrieve secret at path %q: %w", ref.Key, err)
+		return map[string][]byte{}, fmt.Errorf("%s %q: %w", retrieveSecretError, ref.Key, err)
 	}
 
 	// Convert the map[string]any into map[string][]byte.
@@ -55,7 +57,7 @@ func (cl *ovhClient) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDa
 	for key := range rawSecretDataMap {
 		secretDataMap[key], err = esutils.GetByteValueFromMap(rawSecretDataMap, key)
 		if err != nil {
-			return map[string][]byte{}, fmt.Errorf("failed to retrieve secret at path %q: %w", ref.Key, err)
+			return map[string][]byte{}, fmt.Errorf("%s %q: %w", retrieveSecretError, ref.Key, err)
 		}
 	}
 

+ 8 - 6
providers/v1/ovh/client_push_secret.go

@@ -30,16 +30,18 @@ import (
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
 
+const pushSecretError = "failed to push secret at path"
+
 // PushSecret pushes a secret to the Secret Manager according to the updatePolicy
 // defined in the PushSecret (create or update).
 func (cl *ovhClient) PushSecret(ctx context.Context, secret *corev1.Secret, data esv1.PushSecretData) error {
 	remoteKey := data.GetRemoteKey()
 
 	if secret == nil {
-		return fmt.Errorf("failed to push secret at path %q: provided secret is nil", remoteKey)
+		return fmt.Errorf("%s %q: provided secret is nil", pushSecretError, remoteKey)
 	}
 	if len(secret.Data) == 0 {
-		return fmt.Errorf("failed to push secret at path %q: provided secret is empty", remoteKey)
+		return fmt.Errorf("%s %q: provided secret is empty", pushSecretError, remoteKey)
 	}
 
 	// Check if the secret already exists.
@@ -49,20 +51,20 @@ func (cl *ovhClient) PushSecret(ctx context.Context, secret *corev1.Secret, data
 	})
 	noSecretErr := errors.Is(err, esv1.NoSecretErr)
 	if err != nil && !noSecretErr {
-		return fmt.Errorf("failed to push secret at path %q: %w", remoteKey, err)
+		return fmt.Errorf("%s %q: %w", pushSecretError, remoteKey, err)
 	}
 	secretExists := !noSecretErr
 
 	// Build the secret to be pushed.
 	secretToPush, err := buildSecretToPush(secret, data)
 	if err != nil {
-		return fmt.Errorf("failed to push secret at path %q: %w", remoteKey, err)
+		return fmt.Errorf("%s %q: %w", pushSecretError, remoteKey, err)
 	}
 
 	// Compare the data of secretToPush with that of remoteSecret.
 	equal, err := compareSecretsData(secretToPush, remoteSecret)
 	if err != nil {
-		return fmt.Errorf("failed to push secret at path %q: %w", remoteKey, err)
+		return fmt.Errorf("%s %q: %w", pushSecretError, remoteKey, err)
 	}
 	if equal {
 		return nil
@@ -76,7 +78,7 @@ func (cl *ovhClient) PushSecret(ctx context.Context, secret *corev1.Secret, data
 	// Push the secret.
 	err = pushNewSecret(ctx, cl.okmsClient, cl.okmsID, secretToPush, remoteKey, currentVersion, secretExists)
 	if err != nil {
-		return fmt.Errorf("failed to push secret at path %q: %w", remoteKey, err)
+		return fmt.Errorf("%s %q: %w", pushSecretError, remoteKey, err)
 	}
 	return nil
 }

+ 34 - 17
providers/v1/ovh/fake/fake_okms_client.go

@@ -201,29 +201,46 @@ func NewGetSecretsMetadataFn(path string, err error) GetSecretsMetadataFn {
 		}
 
 		for key := range fakeSecretStorage {
-			if path == "" && key != "nil-secret" && key != "empty-secret" {
-				*resp.Data.Keys = append(*resp.Data.Keys, key)
-				continue
-			}
-			posStart := strings.Index(key, path)
-			if posStart == 0 {
-				if len(path) == len(key) {
-					*resp.Data.Keys = append(*resp.Data.Keys, key)
-				} else if len(path) < len(key) && key[len(path)] == '/' {
-					key = key[len(path)+1:]
-					before, _, ok := strings.Cut(key, "/")
-					if ok {
-						*resp.Data.Keys = append(*resp.Data.Keys, before+"/")
-					} else {
-						*resp.Data.Keys = append(*resp.Data.Keys, key)
-					}
-				}
+			toAppend, ok := retrieveKeyToAppend(path, key)
+			if ok {
+				*resp.Data.Keys = append(*resp.Data.Keys, toAppend)
 			}
 		}
 
 		return resp, nil
 	}
 }
+func retrieveKeyToAppend(path, key string) (string, bool) {
+	// If no path is specified, append all non-empty secrets.
+	if path == "" && len(fakeSecretStorage[key]) != 0 {
+		return key, true
+	}
+
+	// Append the secret if key exactly matches path.
+	if path == key {
+		return key, true
+	}
+	// Skip the secret if path is not a prefix of key.
+	if !strings.HasPrefix(key, path+"/") {
+		return "", false
+	}
+
+	// The key starts with path.
+	// Return the first segment after path, adding a trailing slash if there are more segments.
+	// Examples:
+	//   path = "foo/bar", key = "foo/bar/baz/qux"
+	//   returns "baz/", because "baz" is the first segment after the path and there are more segments.
+	//
+	//   path = "foo/bar", key = "foo/bar/baz"
+	//   returns "baz", because it's the last segment.
+	key = key[len(path)+1:]
+	before, _, ok := strings.Cut(key, "/")
+	if ok {
+		return before + "/", true
+	} else {
+		return key, true
+	}
+}
 
 func (f FakeOkmsClient) WithCustomHeader(key, value string) *okms.Client {
 	return nil