Merge branch 'main' into feature/conversion-webhook

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

docs/provider-hashicorp-vault.md

@@ -71,9 +71,92 @@ data:
   foobar: czNjcjN0
 ```
 
-#### Limitations
+#### Fetching Raw Values
 
-Vault supports only simple key/value pairs - nested objects are not supported. Hence specifying `gjson` properties like other providers support it is not supported.
+You can fetch all key/value pairs for a given path If you leave the `remoteRef.property` empty. This returns the json-encoded secret value for that path.
+
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: ExternalSecret
+metadata:
+  name: vault-example
+spec:
+  # ...
+  data:
+  - secretKey: foobar
+    remoteRef:
+      key: /dev/package.json
+```
+
+#### Nested Values
+
+Vault supports nested key/value pairs. You can specify a [gjson](https://github.com/tidwall/gjson) expression at `remoteRef.property` to get a nested value.
+
+Given the following secret - assume its path is `/dev/config`:
+```json
+{
+  "foo": {
+    "nested": {
+      "bar": "mysecret"
+    }
+  }
+}
+```
+
+You can set the `remoteRef.property` to point to the nested key using a [gjson](https://github.com/tidwall/gjson) expression.
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: ExternalSecret
+metadata:
+  name: vault-example
+spec:
+  # ...
+  data:
+  - secretKey: foobar
+    remoteRef:
+      key: /dev/config
+      property: foo.nested.bar
+---
+# creates a secret with:
+# foobar=mysecret
+```
+
+If you would set the `remoteRef.property` to just `foo` then you would get the json-encoded value of that property: `{"nested":{"bar":"mysecret"}}`.
+
+#### Multiple nested Values
+
+You can extract multiple keys from a nested secret using `dataFrom`.
+
+Given the following secret - assume its path is `/dev/config`:
+```json
+{
+  "foo": {
+    "nested": {
+      "bar": "mysecret",
+      "baz": "bang"
+    }
+  }
+}
+```
+
+You can set the `remoteRef.property` to point to the nested key using a [gjson](https://github.com/tidwall/gjson) expression.
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: ExternalSecret
+metadata:
+  name: vault-example
+spec:
+  # ...
+  dataFrom:
+  - key: /dev/config
+    property: foo.nested
+```
+
+That results in a secret with these values:
+```
+bar=mysecret
+baz=bang
+```
 
 ### Authentication
 

e2e/suite/vault/vault.go

@@ -13,10 +13,13 @@ limitations under the License.
 package vault
 
 import (
+	"fmt"
 
 	// nolint
 	. "github.com/onsi/ginkgo/v2"
+	v1 "k8s.io/api/core/v1"
 
+	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	"github.com/external-secrets/external-secrets/e2e/framework"
 	"github.com/external-secrets/external-secrets/e2e/suite/common"
 )
@@ -72,6 +75,11 @@ var _ = Describe("[vault]", Label("vault"), func() {
 		framework.Compose(withK8s, f, common.JSONDataWithTemplate, useKubernetesProvider),
 		framework.Compose(withK8s, f, common.DataPropertyDockerconfigJSON, useKubernetesProvider),
 		framework.Compose(withK8s, f, common.JSONDataWithoutTargetName, useKubernetesProvider),
+		// vault-specific test cases
+		Entry("secret value via data without property should return json-encoded string", Label("json"), testJSONWithoutProperty),
+		Entry("secret value via data with property should return json-encoded string", Label("json"), testJSONWithProperty),
+		Entry("dataFrom without property should extract key/value pairs", Label("json"), testDataFromJSONWithoutProperty),
+		Entry("dataFrom with property should extract key/value pairs", Label("json"), testDataFromJSONWithProperty),
 	)
 })
 
@@ -98,3 +106,92 @@ func useJWTProvider(tc *framework.TestCase) {
 func useKubernetesProvider(tc *framework.TestCase) {
 	tc.ExternalSecret.Spec.SecretStoreRef.Name = kubernetesProviderName
 }
+
+const jsonVal = `{"foo":{"nested":{"bar":"mysecret","baz":"bang"}}}`
+
+// when no property is set it should return the json-encoded at path.
+func testJSONWithoutProperty(tc *framework.TestCase) {
+	secretKey := fmt.Sprintf("%s-%s", tc.Framework.Namespace.Name, "json")
+	tc.Secrets = map[string]string{
+		secretKey: jsonVal,
+	}
+	tc.ExpectedSecret = &v1.Secret{
+		Type: v1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			secretKey: []byte(jsonVal),
+		},
+	}
+	tc.ExternalSecret.Spec.Data = []esapi.ExternalSecretData{
+		{
+			SecretKey: secretKey,
+			RemoteRef: esapi.ExternalSecretDataRemoteRef{
+				Key: secretKey,
+			},
+		},
+	}
+}
+
+// when property is set it should return the json-encoded at path.
+func testJSONWithProperty(tc *framework.TestCase) {
+	secretKey := fmt.Sprintf("%s-%s", tc.Framework.Namespace.Name, "json")
+	expectedVal := `{"bar":"mysecret","baz":"bang"}`
+	tc.Secrets = map[string]string{
+		secretKey: jsonVal,
+	}
+	tc.ExpectedSecret = &v1.Secret{
+		Type: v1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			secretKey: []byte(expectedVal),
+		},
+	}
+	tc.ExternalSecret.Spec.Data = []esapi.ExternalSecretData{
+		{
+			SecretKey: secretKey,
+			RemoteRef: esapi.ExternalSecretDataRemoteRef{
+				Key:      secretKey,
+				Property: "foo.nested",
+			},
+		},
+	}
+}
+
+// when no property is set it should extract the key/value pairs at the given path
+// note: it should json-encode if a value contains nested data
+func testDataFromJSONWithoutProperty(tc *framework.TestCase) {
+	secretKey := fmt.Sprintf("%s-%s", tc.Framework.Namespace.Name, "json")
+	tc.Secrets = map[string]string{
+		secretKey: jsonVal,
+	}
+	tc.ExpectedSecret = &v1.Secret{
+		Type: v1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			"foo": []byte(`{"nested":{"bar":"mysecret","baz":"bang"}}`),
+		},
+	}
+	tc.ExternalSecret.Spec.DataFrom = []esapi.ExternalSecretDataRemoteRef{
+		{
+			Key: secretKey,
+		},
+	}
+}
+
+// when property is set it should extract values with dataFrom at the given path.
+func testDataFromJSONWithProperty(tc *framework.TestCase) {
+	secretKey := fmt.Sprintf("%s-%s", tc.Framework.Namespace.Name, "json")
+	tc.Secrets = map[string]string{
+		secretKey: jsonVal,
+	}
+	tc.ExpectedSecret = &v1.Secret{
+		Type: v1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			"bar": []byte(`mysecret`),
+			"baz": []byte(`bang`),
+		},
+	}
+	tc.ExternalSecret.Spec.DataFrom = []esapi.ExternalSecretDataRemoteRef{
+		{
+			Key:      secretKey,
+			Property: "foo.nested",
+		},
+	}
+}

pkg/provider/vault/vault.go

@@ -18,15 +18,18 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"os"
+	"strconv"
 	"strings"
 
 	"github.com/go-logr/logr"
 	vault "github.com/hashicorp/vault/api"
+	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -165,15 +168,56 @@ func (v *client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretData
 	if err != nil {
 		return nil, err
 	}
-	value, exists := data[ref.Property]
-	if !exists {
+
+	// return raw json if no property is defined
+	if ref.Property == "" {
+		return data, nil
+	}
+
+	val := gjson.Get(string(data), ref.Property)
+	if !val.Exists() {
 		return nil, fmt.Errorf(errSecretKeyFmt, ref.Property)
 	}
-	return value, nil
+	return []byte(val.String()), nil
 }
 
 func (v *client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
-	return v.readSecret(ctx, ref.Key, ref.Version)
+	data, err := v.GetSecret(ctx, ref)
+	if err != nil {
+		return nil, err
+	}
+
+	var secretData map[string]interface{}
+	err = json.Unmarshal(data, &secretData)
+	if err != nil {
+		return nil, err
+	}
+	byteMap := make(map[string][]byte, len(secretData))
+	for k, v := range secretData {
+		switch t := v.(type) {
+		case string:
+			byteMap[k] = []byte(t)
+		case map[string]interface{}:
+			jsonData, err := json.Marshal(t)
+			if err != nil {
+				return nil, err
+			}
+			byteMap[k] = jsonData
+		case []byte:
+			byteMap[k] = t
+		// also covers int and float32 due to json.Marshal
+		case float64:
+			byteMap[k] = []byte(strconv.FormatFloat(t, 'f', -1, 64))
+		case bool:
+			byteMap[k] = []byte(strconv.FormatBool(t))
+		case nil:
+			byteMap[k] = []byte(nil)
+		default:
+			return nil, errors.New(errSecretFormat)
+		}
+	}
+
+	return byteMap, nil
 }
 
 func (v *client) Close(ctx context.Context) error {
@@ -224,7 +268,7 @@ func (v *client) buildPath(path string) string {
 	return returnPath
 }
 
-func (v *client) readSecret(ctx context.Context, path, version string) (map[string][]byte, error) {
+func (v *client) readSecret(ctx context.Context, path, version string) ([]byte, error) {
 	dataPath := v.buildPath(path)
 
 	// path formated according to vault docs for v1 and v2 API
@@ -260,21 +304,8 @@ func (v *client) readSecret(ctx context.Context, path, version string) (map[stri
 		}
 	}
 
-	byteMap := make(map[string][]byte, len(secretData))
-	for k, v := range secretData {
-		switch t := v.(type) {
-		case string:
-			byteMap[k] = []byte(t)
-		case []byte:
-			byteMap[k] = t
-		case nil:
-			byteMap[k] = []byte(nil)
-		default:
-			return nil, errors.New(errSecretFormat)
-		}
-	}
-
-	return byteMap, nil
+	// return json string
+	return json.Marshal(secretData)
 }
 
 func (v *client) newConfig() (*vault.Config, error) {

pkg/provider/vault/vault_test.go

@@ -551,6 +551,169 @@ func vaultTest(t *testing.T, name string, tc testCase) {
 	}
 }
 
+func TestGetSecret(t *testing.T) {
+	errBoom := errors.New("boom")
+	secret := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+	}
+	secretWithNilVal := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+		"token":         nil,
+	}
+	secretWithNestedVal := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+		"nested": map[string]string{
+			"foo": "oke",
+		},
+	}
+
+	type args struct {
+		store   *esv1beta1.VaultProvider
+		kube    kclient.Client
+		vClient Client
+		ns      string
+		data    esv1beta1.ExternalSecretDataRemoteRef
+	}
+
+	type want struct {
+		err error
+		val []byte
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"ReadSecret": {
+			reason: "Should return the secret with property",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV1).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "access_key",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secret), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: []byte("access_key"),
+			},
+		},
+		"ReadSecretWithNil": {
+			reason: "Should return the secret with property if it has a nil val",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV1).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "access_key",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secretWithNilVal), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: []byte("access_key"),
+			},
+		},
+		"ReadSecretWithoutProperty": {
+			reason: "Should return the json encoded secret without property",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV1).Spec.Provider.Vault,
+				data:  esv1beta1.ExternalSecretDataRemoteRef{},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secret), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: []byte(`{"access_key":"access_key","access_secret":"access_secret"}`),
+			},
+		},
+		"ReadSecretWithNestedValue": {
+			reason: "Should return a nested property",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV1).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "nested.foo",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secretWithNestedVal), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: []byte("oke"),
+			},
+		},
+		"NonexistentProperty": {
+			reason: "Should return error property does not exist.",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV1).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "nop.doesnt.exist",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secretWithNestedVal), nil,
+					),
+				},
+			},
+			want: want{
+				err: fmt.Errorf(errSecretKeyFmt, "nop.doesnt.exist"),
+			},
+		},
+		"ReadSecretError": {
+			reason: "Should return error if vault client fails to read secret.",
+			args: args{
+				store: makeSecretStore().Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest:            fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(nil, errBoom),
+				},
+			},
+			want: want{
+				err: fmt.Errorf(errReadSecret, errBoom),
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			vStore := &client{
+				kube:      tc.args.kube,
+				client:    tc.args.vClient,
+				store:     tc.args.store,
+				namespace: tc.args.ns,
+			}
+			val, err := vStore.GetSecret(context.Background(), tc.args.data)
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nvault.GetSecret(...): -want error, +got error:\n%s", tc.reason, diff)
+			}
+			if diff := cmp.Diff(string(tc.want.val), string(val)); diff != "" {
+				t.Errorf("\n%s\nvault.GetSecret(...): -want val, +got val:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}
+
 func TestGetSecretMap(t *testing.T) {
 	errBoom := errors.New("boom")
 	secret := map[string]interface{}{
@@ -562,6 +725,24 @@ func TestGetSecretMap(t *testing.T) {
 		"access_secret": "access_secret",
 		"token":         nil,
 	}
+	secretWithNestedVal := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+		"nested": map[string]interface{}{
+			"foo": map[string]string{
+				"oke":    "yup",
+				"mhkeih": "yada yada",
+			},
+		},
+	}
+	secretWithTypes := map[string]interface{}{
+		"access_secret": "access_secret",
+		"f32":           float32(2.12),
+		"f64":           float64(2.1234534153423423),
+		"int":           42,
+		"bool":          true,
+		"bt":            []byte("foobar"),
+	}
 
 	type args struct {
 		store   *esv1beta1.VaultProvider
@@ -573,6 +754,7 @@ func TestGetSecretMap(t *testing.T) {
 
 	type want struct {
 		err error
+		val map[string][]byte
 	}
 
 	cases := map[string]struct {
@@ -593,6 +775,10 @@ func TestGetSecretMap(t *testing.T) {
 			},
 			want: want{
 				err: nil,
+				val: map[string][]byte{
+					"access_key":    []byte("access_key"),
+					"access_secret": []byte("access_secret"),
+				},
 			},
 		},
 		"ReadSecretKV2": {
@@ -612,6 +798,10 @@ func TestGetSecretMap(t *testing.T) {
 			},
 			want: want{
 				err: nil,
+				val: map[string][]byte{
+					"access_key":    []byte("access_key"),
+					"access_secret": []byte("access_secret"),
+				},
 			},
 		},
 		"ReadSecretWithNilValueKV1": {
@@ -627,6 +817,11 @@ func TestGetSecretMap(t *testing.T) {
 			},
 			want: want{
 				err: nil,
+				val: map[string][]byte{
+					"access_key":    []byte("access_key"),
+					"access_secret": []byte("access_secret"),
+					"token":         []byte(nil),
+				},
 			},
 		},
 		"ReadSecretWithNilValueKV2": {
@@ -646,6 +841,89 @@ func TestGetSecretMap(t *testing.T) {
 			},
 			want: want{
 				err: nil,
+				val: map[string][]byte{
+					"access_key":    []byte("access_key"),
+					"access_secret": []byte("access_secret"),
+					"token":         []byte(nil),
+				},
+			},
+		},
+		"ReadSecretWithTypesKV2": {
+			reason: "Should map the secret even if it has other types",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV2).Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(
+							map[string]interface{}{
+								"data": secretWithTypes,
+							},
+						), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: map[string][]byte{
+					"access_secret": []byte("access_secret"),
+					"f32":           []byte("2.12"),
+					"f64":           []byte("2.1234534153423423"),
+					"int":           []byte("42"),
+					"bool":          []byte("true"),
+					"bt":            []byte("Zm9vYmFy"), // base64
+				},
+			},
+		},
+		"ReadNestedSecret": {
+			reason: "Should map the secret for deeply nested property",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV2).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "nested",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(
+							map[string]interface{}{
+								"data": secretWithNestedVal,
+							},
+						), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: map[string][]byte{
+					"foo": []byte(`{"mhkeih":"yada yada","oke":"yup"}`),
+				},
+			},
+		},
+		"ReadDeeplyNestedSecret": {
+			reason: "Should map the secret for deeply nested property",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1beta1.VaultKVStoreV2).Spec.Provider.Vault,
+				data: esv1beta1.ExternalSecretDataRemoteRef{
+					Property: "nested.foo",
+				},
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(
+							map[string]interface{}{
+								"data": secretWithNestedVal,
+							},
+						), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+				val: map[string][]byte{
+					"oke":    []byte("yup"),
+					"mhkeih": []byte("yada yada"),
+				},
 			},
 		},
 		"ReadSecretError": {
@@ -671,10 +949,13 @@ func TestGetSecretMap(t *testing.T) {
 				store:     tc.args.store,
 				namespace: tc.args.ns,
 			}
-			_, err := vStore.GetSecretMap(context.Background(), tc.args.data)
+			val, err := vStore.GetSecretMap(context.Background(), tc.args.data)
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
 				t.Errorf("\n%s\nvault.GetSecretMap(...): -want error, +got error:\n%s", tc.reason, diff)
 			}
+			if diff := cmp.Diff(tc.want.val, val); diff != "" {
+				t.Errorf("\n%s\nvault.GetSecretMap(...): -want val, +got val:\n%s", tc.reason, diff)
+			}
 		})
 	}
 }