:sparkles: Adds DecodingStrategy to ExternalSecrets (#1294)

Fixes #920

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

apis/externalsecrets/v1beta1/externalsecret_types.go

@@ -182,6 +182,11 @@ type ExternalSecretDataRemoteRef struct {
 	// Used to define a conversion Strategy
 	// +kubebuilder:default="Default"
 	ConversionStrategy ExternalSecretConversionStrategy `json:"conversionStrategy,omitempty"`
+
+	// +optional
+	// Used to define a conversion Strategy
+	// +kubebuilder:default="None"
+	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
 }
 
 type ExternalSecretMetadataPolicy string
@@ -198,6 +203,15 @@ const (
 	ExternalSecretConversionUnicode ExternalSecretConversionStrategy = "Unicode"
 )
 
+type ExternalSecretDecodingStrategy string
+
+const (
+	ExternalSecretDecodeAuto      ExternalSecretDecodingStrategy = "Auto"
+	ExternalSecretDecodeBase64    ExternalSecretDecodingStrategy = "Base64"
+	ExternalSecretDecodeBase64URL ExternalSecretDecodingStrategy = "Base64URL"
+	ExternalSecretDecodeNone      ExternalSecretDecodingStrategy = "None"
+)
+
 // +kubebuilder:validation:MinProperties=1
 // +kubebuilder:validation:MaxProperties=1
 type ExternalSecretDataFromRemoteRef struct {
@@ -225,6 +239,11 @@ type ExternalSecretFind struct {
 	// Used to define a conversion Strategy
 	// +kubebuilder:default="Default"
 	ConversionStrategy ExternalSecretConversionStrategy `json:"conversionStrategy,omitempty"`
+
+	// +optional
+	// Used to define a conversion Strategy
+	// +kubebuilder:default="None"
+	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
 }
 
 type FindName struct {

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

@@ -62,6 +62,10 @@ spec:
                               default: Default
                               description: Used to define a conversion Strategy
                               type: string
+                            decodingStrategy:
+                              default: None
+                              description: Used to define a conversion Strategy
+                              type: string
                             key:
                               description: Key is the key used in the Provider, mandatory
                               type: string
@@ -104,6 +108,10 @@ spec:
                               default: Default
                               description: Used to define a conversion Strategy
                               type: string
+                            decodingStrategy:
+                              default: None
+                              description: Used to define a conversion Strategy
+                              type: string
                             key:
                               description: Key is the key used in the Provider, mandatory
                               type: string
@@ -131,6 +139,10 @@ spec:
                               default: Default
                               description: Used to define a conversion Strategy
                               type: string
+                            decodingStrategy:
+                              default: None
+                              description: Used to define a conversion Strategy
+                              type: string
                             name:
                               description: Finds secrets based on the name.
                               properties:

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

@@ -307,6 +307,10 @@ spec:
                           default: Default
                           description: Used to define a conversion Strategy
                           type: string
+                        decodingStrategy:
+                          default: None
+                          description: Used to define a conversion Strategy
+                          type: string
                         key:
                           description: Key is the key used in the Provider, mandatory
                           type: string
@@ -349,6 +353,10 @@ spec:
                           default: Default
                           description: Used to define a conversion Strategy
                           type: string
+                        decodingStrategy:
+                          default: None
+                          description: Used to define a conversion Strategy
+                          type: string
                         key:
                           description: Key is the key used in the Provider, mandatory
                           type: string
@@ -375,6 +383,10 @@ spec:
                           default: Default
                           description: Used to define a conversion Strategy
                           type: string
+                        decodingStrategy:
+                          default: None
+                          description: Used to define a conversion Strategy
+                          type: string
                         name:
                           description: Finds secrets based on the name.
                           properties:

deploy/crds/bundle.yaml

@@ -52,6 +52,10 @@ spec:
                                 default: Default
                                 description: Used to define a conversion Strategy
                                 type: string
+                              decodingStrategy:
+                                default: None
+                                description: Used to define a conversion Strategy
+                                type: string
                               key:
                                 description: Key is the key used in the Provider, mandatory
                                 type: string
@@ -87,6 +91,10 @@ spec:
                                 default: Default
                                 description: Used to define a conversion Strategy
                                 type: string
+                              decodingStrategy:
+                                default: None
+                                description: Used to define a conversion Strategy
+                                type: string
                               key:
                                 description: Key is the key used in the Provider, mandatory
                                 type: string
@@ -109,6 +117,10 @@ spec:
                                 default: Default
                                 description: Used to define a conversion Strategy
                                 type: string
+                              decodingStrategy:
+                                default: None
+                                description: Used to define a conversion Strategy
+                                type: string
                               name:
                                 description: Finds secrets based on the name.
                                 properties:
@@ -2762,6 +2774,10 @@ spec:
                             default: Default
                             description: Used to define a conversion Strategy
                             type: string
+                          decodingStrategy:
+                            default: None
+                            description: Used to define a conversion Strategy
+                            type: string
                           key:
                             description: Key is the key used in the Provider, mandatory
                             type: string
@@ -2797,6 +2813,10 @@ spec:
                             default: Default
                             description: Used to define a conversion Strategy
                             type: string
+                          decodingStrategy:
+                            default: None
+                            description: Used to define a conversion Strategy
+                            type: string
                           key:
                             description: Key is the key used in the Provider, mandatory
                             type: string
@@ -2819,6 +2839,10 @@ spec:
                             default: Default
                             description: Used to define a conversion Strategy
                             type: string
+                          decodingStrategy:
+                            default: None
+                            description: Used to define a conversion Strategy
+                            type: string
                           name:
                             description: Finds secrets based on the name.
                             properties:

docs/guides-decoding-strategy.md

@@ -0,0 +1,50 @@
+# Decoding Strategies
+The External Secrets Operator has the feature to allow multiple decoding strategies during an object generation.
+
+The `decodingStrategy` field allows the user to set the following Decoding Strategies based on their needs. `decodingStrategy` can be placed under `spec.data.remoteRef`, `spec.dataFrom.extract` or `spec.dataFrom.find`. It will configure the decoding strategy for that specific operation, leaving others with the default behavior if not set.
+
+### None (default)
+ESO will not try to decode the secret value.
+
+### Base64
+ESO will try to decode the secret value using [base64](https://datatracker.ietf.org/doc/html/rfc4648#section-4) method. If the decoding fails, an error is produced.
+
+### Base64URL
+ESO will try to decode the secret value using [base64url](https://datatracker.ietf.org/doc/html/rfc4648#section-5) method. If the decoding fails, an error is produced.
+
+### Auto
+ESO will try to decode using Base64/Base64URL strategies. If the decoding fails, ESO will apply decoding strategy None. No error is produced to the user.
+
+## Examples
+
+### Setting Decoding strategy Auto in a DataFrom.Extract
+Given that we have the given secret information:
+```
+{
+    "name": "Gustavo",
+    "surname": "Fring",
+    "address":"aGFwcHkgc3RyZWV0",
+}
+```
+if we apply the following dataFrom:
+```
+spec:
+  dataFrom:
+  - extract:
+      key: my-secret
+      decodingStrategy: Auto
+```
+It will render the following Kubernetes Secret:
+```
+data:
+  name: R3VzdGF2bw==        #Gustavo
+  surname: RnJpbmc=         #Fring
+  address: aGFwcHkgc3RyZWV0 #happy street
+```
+
+## Limitations
+
+At this time, decoding Strategy Auto is only trying to check if the original input is valid to perform Base64 operations. This means that some non-encoded secret values might end up being decoded, producing gibberish. This is the case for numbered values like `123456` or some specially crafted string values such as `happy/street`. 
+
+!!! note 
+    If you are using `decodeStrategy: Auto` and start to see ESO pulling completely wrong secret values into your kubernetes secret, consider changing it to `None` to investigate it.

docs/snippets/full-external-secret.yaml

@@ -74,6 +74,7 @@ spec:
         key: provider-key
         version: provider-key-version
         property: provider-key-property
+        decodingStrategy: None # can be None, Base64, Base64URL or Auto
 
   # Used to fetch all properties from the Provider key
   # If multiple dataFrom are specified, secrets are merged in the specified order
@@ -83,6 +84,7 @@ spec:
       version: provider-key-version
       property: provider-key-property
       conversionStrategy: Default
+      decodingStrategy: Auto
   - find:
       path: path-to-filter
       name:
@@ -90,6 +92,7 @@ spec:
       tags:
         foo: bar
       conversionStrategy: Unicode
+      decodingStrategy: Base64
 
 status:
   # refreshTime is the time and date the external secret was fetched and

e2e/suites/provider/cases/common/common.go

@@ -605,3 +605,55 @@ func DeletionPolicyDelete(f *framework.Framework) (string, func(*framework.TestC
 		}
 	}
 }
+
+func DecodingPolicySync(f *framework.Framework) (string, func(*framework.TestCase)) {
+	return "[common] should decode secrets with data and dataFrom", func(tc *framework.TestCase) {
+		secretKey1 := fmt.Sprintf("%s-%s", f.Namespace.Name, "one")
+		targetSecretKey1 := "name"
+		targetSecretValue1 := "Z3JlYXQtbmFtZQ=="
+		targetSecretKey2 := "surname"
+		targetSecretValue2 := "Z3JlYXQtc3VybmFtZQ=="
+		targetSecretKey3 := "address"
+		targetSecretValue3 := "happy calm street No. 1"
+		secretValue := fmt.Sprintf("{ %q: %q, %q: %q, %q: %q }", targetSecretKey1, targetSecretValue1, targetSecretKey2, targetSecretValue2, targetSecretKey3, targetSecretValue3)
+		tc.Secrets = map[string]framework.SecretEntry{
+			secretKey1: {Value: secretValue},
+		}
+		tc.ExpectedSecret = &v1.Secret{
+			Type: v1.SecretTypeOpaque,
+			Data: map[string][]byte{
+				targetSecretKey1: []byte("great-name"),
+				targetSecretKey2: []byte("great-surname"),
+				targetSecretKey3: []byte(targetSecretValue3),
+				"base64":         []byte("great-name"),
+				"none":           []byte(targetSecretValue1),
+			},
+		}
+		tc.ExternalSecret.Spec.DataFrom = []esv1beta1.ExternalSecretDataFromRemoteRef{
+			{
+				Extract: &esv1beta1.ExternalSecretDataRemoteRef{
+					Key:              secretKey1,
+					DecodingStrategy: esv1beta1.ExternalSecretDecodeAuto,
+				},
+			},
+		}
+		tc.ExternalSecret.Spec.Data = []esv1beta1.ExternalSecretData{
+			{
+				SecretKey: "base64",
+				RemoteRef: esv1beta1.ExternalSecretDataRemoteRef{
+					Key:              secretKey1,
+					Property:         targetSecretKey1,
+					DecodingStrategy: esv1beta1.ExternalSecretDecodeBase64,
+				},
+			},
+			{
+				SecretKey: "none",
+				RemoteRef: esv1beta1.ExternalSecretDataRemoteRef{
+					Key:              secretKey1,
+					Property:         targetSecretKey1,
+					DecodingStrategy: esv1beta1.ExternalSecretDecodeNone,
+				},
+			},
+		}
+	}
+}

e2e/suites/provider/cases/vault/vault.go

@@ -49,6 +49,7 @@ var _ = Describe("[vault]", Label("vault"), func() {
 		framework.Compose(withTokenAuth, f, common.DataPropertyDockerconfigJSON, useTokenAuth),
 		framework.Compose(withTokenAuth, f, common.JSONDataWithoutTargetName, useTokenAuth),
 		framework.Compose(withTokenAuth, f, common.SyncV1Alpha1, useTokenAuth),
+		framework.Compose(withTokenAuth, f, common.DecodingPolicySync, useTokenAuth),
 		// use cert auth
 		framework.Compose(withCertAuth, f, common.FindByName, useCertAuth),
 		framework.Compose(withCertAuth, f, common.JSONDataFromSync, useCertAuth),

hack/api-docs/mkdocs.yml

@@ -42,6 +42,7 @@ nav:
     - Common K8S Secret Types: guides-common-k8s-secret-types.md
     - Controller Classes: guides-controller-class.md
     - "Lifecycle: ownership & deletion": guides-ownership-deletion-policy.md
+    - Decoding Strategies: guides-decoding-strategy.md
     - Getting Multiple Secrets: guides-getallsecrets.md
     - Multi Tenancy: guides-multi-tenancy.md
     - Metrics: guides-metrics.md

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -50,6 +50,7 @@ const (
 	fieldOwnerTemplate       = "externalsecrets.external-secrets.io/%v"
 	errGetES                 = "could not get ExternalSecret"
 	errConvert               = "could not apply conversion strategy to keys: %v"
+	errDecode                = "could not apply decoding strategy to %v[%d]: %v"
 	errUpdateSecret          = "could not update Secret"
 	errPatchStatus           = "unable to patch status"
 	errGetSecretStore        = "could not get SecretStore %q, %w"
@@ -536,6 +537,10 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient e
 			if err != nil {
 				return nil, fmt.Errorf(errConvert, err)
 			}
+			secretMap, err = utils.DecodeMap(remoteRef.Find.DecodingStrategy, secretMap)
+			if err != nil {
+				return nil, fmt.Errorf(errDecode, "spec.dataFrom", i, err)
+			}
 		} else if remoteRef.Extract != nil {
 			secretMap, err = providerClient.GetSecretMap(ctx, *remoteRef.Extract)
 			if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
@@ -549,6 +554,10 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient e
 			if err != nil {
 				return nil, fmt.Errorf(errConvert, err)
 			}
+			secretMap, err = utils.DecodeMap(remoteRef.Extract.DecodingStrategy, secretMap)
+			if err != nil {
+				return nil, fmt.Errorf(errDecode, "spec.dataFrom", i, err)
+			}
 		}
 
 		providerData = utils.MergeByteMap(providerData, secretMap)
@@ -563,7 +572,10 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient e
 		if err != nil {
 			return nil, err
 		}
-
+		secretData, err = utils.Decode(secretRef.RemoteRef.DecodingStrategy, secretData)
+		if err != nil {
+			return nil, fmt.Errorf(errDecode, "spec.data", i, err)
+		}
 		providerData[secretRef.SecretKey] = secretData
 	}
 

pkg/utils/utils.go

@@ -18,6 +18,7 @@ import (
 
 	// nolint:gosec
 	"crypto/md5"
+	"encoding/base64"
 	"errors"
 	"fmt"
 	"net"
@@ -39,6 +40,50 @@ func MergeByteMap(dst, src map[string][]byte) map[string][]byte {
 	return dst
 }
 
+// DecodeValues decodes values from a secretMap.
+func DecodeMap(strategy esv1beta1.ExternalSecretDecodingStrategy, in map[string][]byte) (map[string][]byte, error) {
+	out := make(map[string][]byte, len(in))
+	for k, v := range in {
+		val, err := Decode(strategy, v)
+		if err != nil {
+			return nil, fmt.Errorf("failure decoding key %v: %w", k, err)
+		}
+		out[k] = val
+	}
+	return out, nil
+}
+
+func Decode(strategy esv1beta1.ExternalSecretDecodingStrategy, in []byte) ([]byte, error) {
+	switch strategy {
+	case esv1beta1.ExternalSecretDecodeBase64:
+		out, err := base64.StdEncoding.DecodeString(string(in))
+		if err != nil {
+			return nil, err
+		}
+		return out, nil
+	case esv1beta1.ExternalSecretDecodeBase64URL:
+		out, err := base64.URLEncoding.DecodeString(string(in))
+		if err != nil {
+			return nil, err
+		}
+		return out, nil
+	case esv1beta1.ExternalSecretDecodeNone:
+		return in, nil
+	case esv1beta1.ExternalSecretDecodeAuto:
+		out, err := Decode(esv1beta1.ExternalSecretDecodeBase64, in)
+		if err != nil {
+			out, err := Decode(esv1beta1.ExternalSecretDecodeBase64URL, in)
+			if err != nil {
+				return Decode(esv1beta1.ExternalSecretDecodeNone, in)
+			}
+			return out, nil
+		}
+		return out, nil
+	default:
+		return nil, fmt.Errorf("decoding strategy %v is not supported", strategy)
+	}
+}
+
 // ConvertKeys converts a secret map into a valid key.
 // Replaces any non-alphanumeric characters depending on convert strategy.
 func ConvertKeys(strategy esv1beta1.ExternalSecretConversionStrategy, in map[string][]byte) (map[string][]byte, error) {

pkg/utils/utils_test.go

@@ -25,6 +25,12 @@ import (
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 )
 
+const (
+	base64DecodedValue    string = "foo%_?bar"
+	base64EncodedValue    string = "Zm9vJV8/YmFy"
+	base64URLEncodedValue string = "Zm9vJV8_YmFy"
+)
+
 func TestObjectHash(t *testing.T) {
 	tests := []struct {
 		name  string
@@ -226,6 +232,103 @@ func TestConvertKeys(t *testing.T) {
 	}
 }
 
+func TestDecode(t *testing.T) {
+	type args struct {
+		strategy esv1beta1.ExternalSecretDecodingStrategy
+		in       map[string][]byte
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    map[string][]byte
+		wantErr bool
+	}{
+		{
+			name: "base64 decoded",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeBase64,
+				in: map[string][]byte{
+					"foo": []byte("YmFy"),
+				},
+			},
+			want: map[string][]byte{
+				"foo": []byte("bar"),
+			},
+		},
+		{
+			name: "invalid base64",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeBase64,
+				in: map[string][]byte{
+					"foo": []byte("foo"),
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "base64url decoded",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeBase64URL,
+				in: map[string][]byte{
+					"foo": []byte(base64URLEncodedValue),
+				},
+			},
+			want: map[string][]byte{
+				"foo": []byte(base64DecodedValue),
+			},
+		},
+		{
+			name: "invalid base64url",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeBase64URL,
+				in: map[string][]byte{
+					"foo": []byte("foo"),
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "none",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeNone,
+				in: map[string][]byte{
+					"foo": []byte(base64URLEncodedValue),
+				},
+			},
+			want: map[string][]byte{
+				"foo": []byte(base64URLEncodedValue),
+			},
+		},
+		{
+			name: "auto",
+			args: args{
+				strategy: esv1beta1.ExternalSecretDecodeAuto,
+				in: map[string][]byte{
+					"b64":        []byte(base64EncodedValue),
+					"invalidb64": []byte("foo"),
+					"b64url":     []byte(base64URLEncodedValue),
+				},
+			},
+			want: map[string][]byte{
+				"b64":        []byte(base64DecodedValue),
+				"invalidb64": []byte("foo"),
+				"b64url":     []byte(base64DecodedValue),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := DecodeMap(tt.args.strategy, tt.args.in)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("DecodeMap() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("DecodeMap() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
 func TestValidate(t *testing.T) {
 	err := NetworkValidate("http://google.com", 10*time.Second)
 	if err != nil {