fix: webhook support more types when parsing response (external-secrets#2899)

* fix: support more types in webhook response

Signed-off-by: Moritz Johner <[email protected]>

* fix: properly decode json

Signed-off-by: Moritz Johner <[email protected]>

* Update pkg/provider/webhook/webhook.go

Co-authored-by: Gergely Brautigam <[email protected]>
Signed-off-by: Moritz Johner <[email protected]>

* Update pkg/provider/webhook/webhook.go

Co-authored-by: Gergely Brautigam <[email protected]>
Signed-off-by: Moritz Johner <[email protected]>

* fix: expose errors

Signed-off-by: Moritz Johner <[email protected]>

---------

Signed-off-by: Moritz Johner <[email protected]>
Signed-off-by: Moritz Johner <[email protected]>
Co-authored-by: Gergely Brautigam <[email protected]>

Author: moolen

pkg/generator/vault/vault.go

@@ -29,6 +29,7 @@ import (
 
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 	provider "github.com/external-secrets/external-secrets/pkg/provider/vault"
+	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 
 type Generator struct{}
@@ -114,7 +115,7 @@ func (g *Generator) generate(ctx context.Context, c *provider.Connector, jsonSpe
 	}
 
 	for k := range data {
-		response[k], err = provider.GetTypedKey(data, k)
+		response[k], err = utils.GetByteValueFromMap(data, k)
 		if err != nil {
 			return nil, err
 		}

pkg/provider/delinea/client.go

@@ -17,22 +17,13 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
-	"reflect"
-	"strconv"
-	"strings"
 
 	"github.com/DelineaXPM/dsv-sdk-go/v2/vault"
 	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
-)
-
-const (
-	errSecretKeyFmt  = "cannot find secret data for key: %q"
-	errUnexpectedKey = "unexpected key in data: %s"
-	errSecretFormat  = "secret data for property %s not in expected format: %s"
+	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 
 type client struct {
@@ -91,7 +82,7 @@ func (c *client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretD
 	}
 	byteMap := make(map[string][]byte, len(secret.Data))
 	for k := range secret.Data {
-		byteMap[k], err = getTypedKey(secret.Data, k)
+		byteMap[k], err = utils.GetByteValueFromMap(secret.Data, k)
 		if err != nil {
 			return nil, err
 		}
@@ -116,34 +107,3 @@ func (c *client) getSecret(_ context.Context, ref esv1beta1.ExternalSecretDataRe
 	}
 	return c.api.Secret(ref.Key)
 }
-
-// getTypedKey is copied from pkg/provider/vault/vault.go.
-func getTypedKey(data map[string]interface{}, key string) ([]byte, error) {
-	v, ok := data[key]
-	if !ok {
-		return nil, fmt.Errorf(errUnexpectedKey, key)
-	}
-	switch t := v.(type) {
-	case string:
-		return []byte(t), nil
-	case map[string]interface{}:
-		return json.Marshal(t)
-	case []string:
-		return []byte(strings.Join(t, "\n")), nil
-	case []byte:
-		return t, nil
-	// also covers int and float32 due to json.Marshal
-	case float64:
-		return []byte(strconv.FormatFloat(t, 'f', -1, 64)), nil
-	case json.Number:
-		return []byte(t.String()), nil
-	case []interface{}:
-		return json.Marshal(t)
-	case bool:
-		return []byte(strconv.FormatBool(t)), nil
-	case nil:
-		return []byte(nil), nil
-	default:
-		return nil, fmt.Errorf(errSecretFormat, key, reflect.TypeOf(t))
-	}
-}

pkg/provider/ibm/provider.go

@@ -18,7 +18,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"strconv"
 	"strings"
 	"time"
 
@@ -565,37 +564,14 @@ func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1beta1.ExternalSe
 func byteArrayMap(secretData map[string]interface{}, secretMap map[string][]byte) map[string][]byte {
 	var err error
 	for k, v := range secretData {
-		secretMap[k], err = getTypedKey(v)
+		secretMap[k], err = utils.GetByteValue(v)
 		if err != nil {
 			return nil
 		}
 	}
 	return secretMap
 }
 
-// kudos Vault Provider - convert from various types.
-func getTypedKey(v interface{}) ([]byte, error) {
-	switch t := v.(type) {
-	case string:
-		return []byte(t), nil
-	case map[string]interface{}:
-		return json.Marshal(t)
-	case map[string]string:
-		return json.Marshal(t)
-	case []byte:
-		return t, nil
-		// also covers int and float32 due to json.Marshal
-	case float64:
-		return []byte(strconv.FormatFloat(t, 'f', -1, 64)), nil
-	case bool:
-		return []byte(strconv.FormatBool(t)), nil
-	case nil:
-		return []byte(nil), nil
-	default:
-		return nil, fmt.Errorf("secret not in expected format")
-	}
-}
-
 func (ibm *providerIBM) Close(_ context.Context) error {
 	return nil
 }

pkg/provider/vault/vault.go

@@ -24,8 +24,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"reflect"
-	"strconv"
 	"strings"
 	"time"
 
@@ -89,7 +87,6 @@ const (
 	errDataField                    = "failed to find data field"
 	errJSONUnmarshall               = "failed to unmarshall JSON"
 	errPathInvalid                  = "provided Path isn't a valid kv v2 path"
-	errSecretFormat                 = "secret data for property %s not in expected format: %s"
 	errUnexpectedKey                = "unexpected key in data: %s"
 	errVaultToken                   = "cannot parse Vault authentication token: %w"
 	errVaultRequest                 = "error from Vault request: %w"
@@ -758,7 +755,7 @@ func (v *client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretData
 	// actual keys to take precedence over gjson syntax
 	// (2): extract key from secret with property
 	if _, ok := data[ref.Property]; ok {
-		return GetTypedKey(data, ref.Property)
+		return utils.GetByteValueFromMap(data, ref.Property)
 	}
 
 	// (3): extract key from secret using gjson
@@ -785,7 +782,7 @@ func (v *client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretD
 	}
 	byteMap := make(map[string][]byte, len(secretData))
 	for k := range secretData {
-		byteMap[k], err = GetTypedKey(secretData, k)
+		byteMap[k], err = utils.GetByteValueFromMap(secretData, k)
 		if err != nil {
 			return nil, err
 		}
@@ -794,36 +791,6 @@ func (v *client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretD
 	return byteMap, nil
 }
 
-func GetTypedKey(data map[string]interface{}, key string) ([]byte, error) {
-	v, ok := data[key]
-	if !ok {
-		return nil, fmt.Errorf(errUnexpectedKey, key)
-	}
-	switch t := v.(type) {
-	case string:
-		return []byte(t), nil
-	case map[string]interface{}:
-		return json.Marshal(t)
-	case []string:
-		return []byte(strings.Join(t, "\n")), nil
-	case []byte:
-		return t, nil
-	// also covers int and float32 due to json.Marshal
-	case float64:
-		return []byte(strconv.FormatFloat(t, 'f', -1, 64)), nil
-	case json.Number:
-		return []byte(t.String()), nil
-	case []interface{}:
-		return json.Marshal(t)
-	case bool:
-		return []byte(strconv.FormatBool(t)), nil
-	case nil:
-		return []byte(nil), nil
-	default:
-		return nil, fmt.Errorf(errSecretFormat, key, reflect.TypeOf(t))
-	}
-}
-
 func (v *client) Close(ctx context.Context) error {
 	// Revoke the token if we have one set, it wasn't sourced from a TokenSecretRef,
 	// and token caching isn't enabled

pkg/provider/webhook/webhook.go

@@ -19,16 +19,17 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 	tpl "text/template"
 	"time"
 
 	"github.com/PaesslerAG/jsonpath"
-	"gopkg.in/yaml.v3"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -152,30 +153,54 @@ func (w *WebHook) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDat
 	}
 	if resultJSONPath != "" {
 		jsondata := interface{}(nil)
-		if err := yaml.Unmarshal(result, &jsondata); err != nil {
+		if err := json.Unmarshal(result, &jsondata); err != nil {
 			return nil, fmt.Errorf("failed to parse response json: %w", err)
 		}
 		jsondata, err = jsonpath.Get(resultJSONPath, jsondata)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get response path %s: %w", resultJSONPath, err)
 		}
-		jsonvalue, ok := jsondata.(string)
-		if !ok {
-			jsonvalues, ok := jsondata.([]interface{})
-			if !ok {
-				return nil, fmt.Errorf("failed to get response (wrong type: %T)", jsondata)
-			}
-			if len(jsonvalues) == 0 {
-				return nil, fmt.Errorf("filter worked but didn't get any result")
-			}
-			jsonvalue = jsonvalues[0].(string)
-		}
-		return []byte(jsonvalue), nil
+		return extractSecretData(jsondata)
 	}
 
 	return result, nil
 }
 
+// tries to extract data from an interface{}
+// it is supposed to return a single value.
+func extractSecretData(jsondata any) ([]byte, error) {
+	switch val := jsondata.(type) {
+	case bool:
+		return []byte(strconv.FormatBool(val)), nil
+	case nil:
+		return []byte{}, nil
+	case int:
+		return []byte(strconv.Itoa(val)), nil
+	case float64:
+		return []byte(strconv.FormatFloat(val, 'f', 0, 64)), nil
+	case []byte:
+		return val, nil
+	case string:
+		return []byte(val), nil
+
+	// due to backwards compatibility we must keep this!
+	// in case we see a []something we pick the first element and return it
+	case []any:
+		if len(val) == 0 {
+			return nil, fmt.Errorf("filter worked but didn't get any result")
+		}
+		return extractSecretData(val[0])
+
+	// in case we encounter a map we serialize it instead of erroring out
+	// The user should use that data from within a template and figure
+	// out how to deal with it.
+	case map[string]any:
+		return json.Marshal(val)
+	default:
+		return nil, fmt.Errorf("failed to get response (wrong type: %T)", jsondata)
+	}
+}
+
 func (w *WebHook) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	provider, err := getProvider(w.store)
 	if err != nil {
@@ -188,7 +213,7 @@ func (w *WebHook) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecret
 
 	// We always want json here, so just parse it out
 	jsondata := interface{}(nil)
-	if err := yaml.Unmarshal(result, &jsondata); err != nil {
+	if err := json.Unmarshal(result, &jsondata); err != nil {
 		return nil, fmt.Errorf("failed to parse response json: %w", err)
 	}
 	// Get subdata via jsonpath, if given
@@ -203,7 +228,7 @@ func (w *WebHook) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecret
 	if ok {
 		// This could also happen if the response was a single json-encoded string
 		// but that is an extremely unlikely scenario
-		if err := yaml.Unmarshal([]byte(jsonstring), &jsondata); err != nil {
+		if err := json.Unmarshal([]byte(jsonstring), &jsondata); err != nil {
 			return nil, fmt.Errorf("failed to parse response json from jsonpath: %w", err)
 		}
 	}

pkg/provider/webhook/webhook_test.go

@@ -119,7 +119,7 @@ want:
   path: /api/getsecret?id=testkey&version=1
   err: failed to get response path
 ---
-case: error bad json data
+case: pull data out of map
 args:
   url: /api/getsecret?id={{ .remoteRef.key }}&version={{ .remoteRef.version }}
   key: testkey
@@ -128,7 +128,8 @@ args:
   response: '{"result":{"thesecret":{"one":"secret-value"}}}'
 want:
   path: /api/getsecret?id=testkey&version=1
-  err: failed to get response (wrong type
+  err: ''
+  result: '{"one":"secret-value"}'
 ---
 case: error timeout
 args:
@@ -199,10 +200,8 @@ args:
   response: 'some simple string'
 want:
   path: /api/getsecret?id=testkey&version=1
-  err: failed to get response (wrong type
-  resultmap:
-    thesecret: secret-value
-    alsosecret: another-value
+  err: "failed to parse response json: invalid character"
+  resultmap: {}
 ---
 case: error json map
 args:
@@ -213,10 +212,8 @@ args:
   response: '{"result":{"thesecret":"secret-value","alsosecret":"another-value"}}'
 want:
   path: /api/getsecret?id=testkey&version=1
-  err: failed to get response (wrong type
-  resultmap:
-    thesecret: secret-value
-    alsosecret: another-value
+  err: "failed to parse response json from jsonpath"
+  resultmap: {}
 ---
 case: good json with good templated jsonpath
 args:
@@ -265,6 +262,42 @@ args:
 want:
   path: /api/getsecret?id=testkey&version=1
   err: "filter worked but didn't get any result"
+---
+case: success with jsonpath filter and result array
+args:
+  url: /api/getsecret?id={{ .remoteRef.key }}&version={{ .remoteRef.version }}
+  key: testkey
+  version: 1
+  jsonpath: $..name
+  response: '{"secrets": [{"name": "thesecret", "value": "secret-value"}, {"name": "alsosecret", "value": "another-value"}]}'
+want:
+  path: /api/getsecret?id=testkey&version=1
+  err: ''
+  result: 'thesecret'
+---
+case: success with jsonpath filter and result array of ints
+args:
+  url: /api/getsecret?id={{ .remoteRef.key }}&version={{ .remoteRef.version }}
+  key: testkey
+  version: 1
+  jsonpath: $..name
+  response: '{"secrets": [{"name": 123, "value": "secret-value"}, {"name": 456, "value": "another-value"}]}'
+want:
+  path: /api/getsecret?id=testkey&version=1
+  err: ''
+  result: 123
+---
+case: support backslash
+args:
+  url: /api/getsecret?id={{ .remoteRef.key }}&version={{ .remoteRef.version }}
+  key: testkey
+  version: 1
+  jsonpath: $.refresh_token
+  response: '{"access_token":"REDACTED","refresh_token":"RE\/DACTED=="}'
+want:
+  path: /api/getsecret?id=testkey&version=1
+  err: ''
+  result: "RE/DACTED=="
 `
 
 func TestWebhookGetSecret(t *testing.T) {