diff --git a/pkg/provider/kubernetes/client.go b/pkg/provider/kubernetes/client.go index 748b0a0de..0a819469d 100644 --- a/pkg/provider/kubernetes/client.go +++ b/pkg/provider/kubernetes/client.go @@ -20,14 +20,13 @@ import ( "encoding/json" "fmt" "strings" - "unicode/utf8" "github.com/tidwall/gjson" v1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - labels "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/labels" esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1" "github.com/external-secrets/external-secrets/pkg/constants" @@ -46,64 +45,33 @@ func (c *Client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretData if err != nil { return nil, err } - serializedSecret, err := serializeSecret(secret, ref) - if err != nil { - return nil, err - } // if property is not defined, we will return the json-serialized secret if ref.Property == "" { - return serializedSecret, nil - } + if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch { + m := map[string]map[string]string{} + m[metaLabels] = secret.Labels + m[metaAnnotations] = secret.Annotations - jsonStr := string(serializedSecret) - // We need to search if a given key with a . exists before using gjson operations. - idx := strings.Index(ref.Property, ".") - if idx > -1 { - refProperty := strings.ReplaceAll(ref.Property, ".", "\\.") - val := gjson.Get(jsonStr, refProperty) - if val.Exists() { - return []byte(val.Str), nil + j, err := jsonMarshal(m) + if err != nil { + return nil, err + } + return j, nil } - } - val := gjson.Get(jsonStr, ref.Property) - if !val.Exists() { - return nil, fmt.Errorf("property %s does not exist in key %s", ref.Property, ref.Key) - } - return []byte(val.String()), nil -} -// serializeSecret serializes a secret map[string][]byte into a flat []byte. -func serializeSecret(secret *v1.Secret, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) { - // metadata is treated differently, because it - // contains nested maps which can be queried with `ref.Property` - if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch { - values, err := getSecretMetadata(secret) + m := map[string]string{} + for key, val := range secret.Data { + m[key] = string(val) + } + j, err := jsonMarshal(m) if err != nil { return nil, err } - data := make(map[string]json.RawMessage, len(values)) - for k, v := range values { - data[k] = encodeBinaryData(v) - } - return jsonMarshal(data) + return j, nil } - strMap := make(map[string]string) - for k, v := range secret.Data { - strMap[k] = string(encodeBinaryData(v)) - } - return jsonMarshal(strMap) -} - -// encode binary data encodes non UTF-8 data -// as base64. This is needed to support proper json serialization. -// if binary data would not be encoded, it would be utf-8 escaped: `\uffed`. -func encodeBinaryData(input []byte) []byte { - if utf8.Valid(input) { - return input - } - return []byte(base64.StdEncoding.EncodeToString(input)) + return getSecret(secret, ref) } func jsonMarshal(t interface{}) ([]byte, error) { @@ -372,3 +340,87 @@ func (c *Client) updateProperty(ctx context.Context, extSecret *v1.Secret, remot metrics.ObserveAPICall(constants.ProviderKubernetes, constants.CallKubernetesUpdateSecret, uErr) return uErr } + +func getSecret(secret *v1.Secret, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) { + if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch { + s, found, err := getFromSecretMetadata(secret, ref) + if err != nil { + return nil, err + } + + if !found { + return nil, fmt.Errorf("property %s does not exist in metadata of secret %q", ref.Property, ref.Key) + } + + return s, nil + } + + s, found := getFromSecretData(secret, ref) + if !found { + return nil, fmt.Errorf("property %s does not exist in data of secret %q", ref.Property, ref.Key) + } + + return s, nil +} + +func getFromSecretData(secret *v1.Secret, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, bool) { + // Check if a property with "." exists first such as file.png + v, ok := secret.Data[ref.Property] + if ok { + return v, true + } + + idx := strings.Index(ref.Property, ".") + if idx == -1 || idx == 0 || idx == len(ref.Property)-1 { + return nil, false + } + + v, ok = secret.Data[ref.Property[:idx]] + if !ok { + return nil, false + } + + val := gjson.Get(string(v), ref.Property[idx+1:]) + if !val.Exists() { + return nil, false + } + + return []byte(val.String()), true +} + +func getFromSecretMetadata(secret *v1.Secret, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, bool, error) { + path := strings.Split(ref.Property, ".") + + var metadata map[string]string + switch path[0] { + case metaLabels: + metadata = secret.Labels + case metaAnnotations: + metadata = secret.Annotations + default: + return nil, false, nil + } + + if len(path) == 1 { + j, err := jsonMarshal(metadata) + if err != nil { + return nil, false, err + } + return j, true, nil + } + + v, ok := metadata[path[1]] + if !ok { + return nil, false, nil + } + if len(path) == 2 { + return []byte(v), true, nil + } + + val := gjson.Get(v, strings.Join(path[2:], "")) + if !val.Exists() { + return nil, false, nil + } + + return []byte(val.String()), true, nil +} diff --git a/pkg/provider/kubernetes/client_test.go b/pkg/provider/kubernetes/client_test.go index 1ebb089f7..a081e6a6b 100644 --- a/pkg/provider/kubernetes/client_test.go +++ b/pkg/provider/kubernetes/client_test.go @@ -15,9 +15,9 @@ package kubernetes import ( "context" - "encoding/base64" "errors" "reflect" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -102,91 +102,22 @@ func (fk *fakeClient) Update(_ context.Context, secret *v1.Secret, _ metav1.Upda var binaryTestData = []byte{0x00, 0xff, 0x00, 0xff, 0xac, 0xab, 0x28, 0x21} func TestGetSecret(t *testing.T) { - type fields struct { - Client KClient - ReviewClient RClient - Namespace string - } tests := []struct { - name string - fields fields - ref esv1beta1.ExternalSecretDataRemoteRef - - want []byte - wantErr bool + desc string + secrets map[string]*v1.Secret + clientErr error + ref esv1beta1.ExternalSecretDataRemoteRef + want []byte + wantErr string }{ { - name: "secretNotFound", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "token": []byte(`foobar`), - }, - }, - }, - err: apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "Secret"}, "secret"), - }, - Namespace: "default", - }, - ref: esv1beta1.ExternalSecretDataRemoteRef{ - Key: "mysec", - Property: "token", - }, - wantErr: true, - }, - { - name: "err GetSecretMap", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{}, - }, - Namespace: "default", - }, - ref: esv1beta1.ExternalSecretDataRemoteRef{ - Key: "mysec", - Property: "token", - }, - wantErr: true, - }, - { - name: "wrong property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "token": []byte(`foobar`), - }, - }, + desc: "secret data with correct property", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "token": []byte(`foobar`), }, }, - Namespace: "default", - }, - ref: esv1beta1.ExternalSecretDataRemoteRef{ - Key: "mysec", - Property: "not-the-token", - }, - wantErr: true, - }, - { - name: "successful case", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "token": []byte(`foobar`), - }, - }, - }, - }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ Key: "mysec", @@ -195,19 +126,44 @@ func TestGetSecret(t *testing.T) { want: []byte(`foobar`), }, { - name: "successful case with html chars", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "html": []byte(``), - }, - }, + desc: "secret data with multi level property", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "foo": []byte(`{"huga":{"bar":"val"}}`), + }, + }, + }, + ref: esv1beta1.ExternalSecretDataRemoteRef{ + Key: "mysec", + Property: "foo.huga.bar", + }, + want: []byte(`val`), + }, + { + desc: "secret data with property containing .", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "foo.png": []byte(`correct`), + "foo": []byte(`{"png":"wrong"}`), + }, + }, + }, + ref: esv1beta1.ExternalSecretDataRemoteRef{ + Key: "mysec", + Property: "foo.png", + }, + want: []byte(`correct`), + }, + { + desc: "secret data contains html characters", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "html": []byte(``), }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ Key: "mysec", @@ -215,20 +171,14 @@ func TestGetSecret(t *testing.T) { want: []byte(`{"html":""}`), }, { - name: "successful case metadata with html special chars and without property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"date": "today"}, - Labels: map[string]string{"dev": ""}, - }, - }, + desc: "secret metadata contains html characters", + secrets: map[string]*v1.Secret{ + "mysec": { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"date": "today"}, + Labels: map[string]string{"dev": ""}, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, @@ -237,40 +187,28 @@ func TestGetSecret(t *testing.T) { want: []byte(`{"annotations":{"date":"today"},"labels":{"dev":""}}`), }, { - name: "successful case with binary data", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "bindata": binaryTestData, - }, - }, + desc: "secret data contains binary", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "bindata": binaryTestData, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ Key: "mysec", Property: "bindata", }, - want: []byte(base64.StdEncoding.EncodeToString(binaryTestData)), + want: binaryTestData, }, { - name: "successful case without property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - Data: map[string][]byte{ - "token": []byte(`foobar`), - }, - }, + desc: "secret data without property", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "token": []byte(`foobar`), }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ Key: "mysec", @@ -278,20 +216,14 @@ func TestGetSecret(t *testing.T) { want: []byte(`{"token":"foobar"}`), }, { - name: "successful case metadata without property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"date": "today"}, - Labels: map[string]string{"dev": "seb"}, - }, - }, + desc: "secret metadata without property", + secrets: map[string]*v1.Secret{ + "mysec": { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"date": "today"}, + Labels: map[string]string{"dev": "seb"}, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, @@ -300,20 +232,14 @@ func TestGetSecret(t *testing.T) { want: []byte(`{"annotations":{"date":"today"},"labels":{"dev":"seb"}}`), }, { - name: "successful case metadata with single property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"date": "today"}, - Labels: map[string]string{"dev": "seb"}, - }, - }, + desc: "secret metadata with single level property", + secrets: map[string]*v1.Secret{ + "mysec": { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"date": "today"}, + Labels: map[string]string{"dev": "seb"}, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, @@ -323,20 +249,14 @@ func TestGetSecret(t *testing.T) { want: []byte(`{"dev":"seb"}`), }, { - name: "successful case metadata with multiple properties", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"date": "today"}, - Labels: map[string]string{"dev": "seb"}, - }, - }, + desc: "secret metadata with multiple level property", + secrets: map[string]*v1.Secret{ + "mysec": { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"date": "today"}, + Labels: map[string]string{"dev": "seb"}, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, @@ -346,43 +266,72 @@ func TestGetSecret(t *testing.T) { want: []byte(`seb`), }, { - name: "error case metadata with wrong property", - fields: fields{ - Client: &fakeClient{ - t: t, - secretMap: map[string]*v1.Secret{ - "mysec": { - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"date": "today"}, - Labels: map[string]string{"dev": "seb"}, - }, - }, + desc: "secret is not found", + clientErr: apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "Secret"}, "secret"), + ref: esv1beta1.ExternalSecretDataRemoteRef{ + Key: "mysec", + Property: "token", + }, + wantErr: `Secret "secret" not found`, + }, + { + desc: "secret data with wrong property", + secrets: map[string]*v1.Secret{ + "mysec": { + Data: map[string][]byte{ + "token": []byte(`foobar`), + }, + }, + }, + ref: esv1beta1.ExternalSecretDataRemoteRef{ + Key: "mysec", + Property: "not-the-token", + }, + wantErr: "property not-the-token does not exist in data of secret", + }, + { + desc: "secret metadata with wrong property", + secrets: map[string]*v1.Secret{ + "mysec": { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"date": "today"}, + Labels: map[string]string{"dev": "seb"}, }, }, - Namespace: "default", }, ref: esv1beta1.ExternalSecretDataRemoteRef{ MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Key: "mysec", Property: "foo", }, - wantErr: true, + wantErr: "property foo does not exist in metadata of secret", }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.desc, func(t *testing.T) { p := &Client{ - userSecretClient: tt.fields.Client, - userReviewClient: tt.fields.ReviewClient, - namespace: tt.fields.Namespace, + userSecretClient: &fakeClient{t: t, secretMap: tt.secrets, err: tt.clientErr}, + namespace: "default", } got, err := p.GetSecret(context.Background(), tt.ref) - if (err != nil) != tt.wantErr { - t.Errorf("ProviderKubernetes.GetSecret() error = %v, wantErr %v", err, tt.wantErr) + if err != nil { + if tt.wantErr == "" { + t.Fatalf("failed to call GetSecret: %v", err) + } + + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("received an unexpected error: %q should have contained %q", err.Error(), tt.wantErr) + } + return } + + if tt.wantErr != "" { + t.Fatalf("expected to receive an error but got nil") + } + if !reflect.DeepEqual(got, tt.want) { - t.Errorf("ProviderKubernetes.GetSecret() = %s, want %s", got, tt.want) + t.Fatalf("received an unexpected secret: got: %s, want %s", got, tt.want) } }) }