diff --git a/e2e/go.mod b/e2e/go.mod index 99f6bdef1..1f8f20993 100644 --- a/e2e/go.mod +++ b/e2e/go.mod @@ -141,6 +141,7 @@ require ( github.com/tidwall/gjson v1.14.4 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect + github.com/tidwall/sjson v1.2.5 // indirect go.opencensus.io v0.24.0 // indirect golang.org/x/crypto v0.10.0 // indirect golang.org/x/net v0.11.0 // indirect diff --git a/e2e/go.sum b/e2e/go.sum index 2cda3bc37..868935d19 100644 --- a/e2e/go.sum +++ b/e2e/go.sum @@ -396,6 +396,7 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM= github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= @@ -403,6 +404,8 @@ github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JT github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= +github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/uber/jaeger-client-go v2.30.0+incompatible h1:D6wyKGCecFaSRUpo8lCVbaOOb6ThwMmTEbhRwtKR97o= github.com/uber/jaeger-client-go v2.30.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMWRy2pZqQJSXxYSwNYOkTr/Z6d3Kk= github.com/uber/jaeger-lib v2.4.1+incompatible h1:td4jdvLcExb4cBISKIpHuGoVXh+dVKhn2Um6rjCsSsg= diff --git a/go.mod b/go.mod index d15094dbf..03e62ca2a 100644 --- a/go.mod +++ b/go.mod @@ -44,8 +44,8 @@ require ( go.uber.org/zap v1.24.0 golang.org/x/crypto v0.10.0 golang.org/x/oauth2 v0.9.0 - google.golang.org/api v0.129.0 - google.golang.org/genproto v0.0.0-20230629202037-9506855d4529 // indirect + google.golang.org/api v0.128.0 + google.golang.org/genproto v0.0.0-20230530153820-e85fd2cbaebc google.golang.org/grpc v1.56.1 gopkg.in/yaml.v3 v3.0.1 grpc.go4.org v0.0.0-20170609214715-11d0a25b4919 @@ -83,6 +83,7 @@ require ( github.com/scaleway/scaleway-sdk-go v1.0.0-beta.17 github.com/sethvargo/go-password v0.2.0 github.com/spf13/pflag v1.0.5 + github.com/tidwall/sjson v1.2.5 sigs.k8s.io/yaml v1.3.0 ) diff --git a/go.sum b/go.sum index ceea199c9..25f2e9085 100644 --- a/go.sum +++ b/go.sum @@ -617,6 +617,7 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM= github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= @@ -624,6 +625,8 @@ github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JT github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= +github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/tjfoc/gmsm v1.3.2/go.mod h1:HaUcFuY0auTiaHB9MHFGCPx5IaLhTUd2atbCFBQXn9w= github.com/tjfoc/gmsm v1.4.1 h1:aMe1GlZb+0bLjn+cKTPEvvn9oUEBlJitaZiiBwsbgho= github.com/tjfoc/gmsm v1.4.1/go.mod h1:j4INPkHWMrhJb38G+J6W4Tw0AbuN8Thu3PbdVYhVcTE= @@ -987,8 +990,8 @@ google.golang.org/api v0.40.0/go.mod h1:fYKFpnQN0DsDSKRVRcQSDQNtqWPfM9i+zNPxepjR google.golang.org/api v0.41.0/go.mod h1:RkxM5lITDfTzmyKFPt+wGrCJbVfniCr2ool8kTBzRTU= google.golang.org/api v0.43.0/go.mod h1:nQsDGjRXMo4lvh5hP0TKqF244gqhGcr/YSIykhUk/94= google.golang.org/api v0.45.0/go.mod h1:ISLIJCedJolbZvDfAk+Ctuq5hf+aJ33WgtUsfyFoLXA= -google.golang.org/api v0.129.0 h1:2XbdjjNfFPXQyufzQVwPf1RRnHH8Den2pfNE2jw7L8w= -google.golang.org/api v0.129.0/go.mod h1:dFjiXlanKwWE3612X97llhsoI36FAoIiRj3aTl5b/zE= +google.golang.org/api v0.128.0 h1:RjPESny5CnQRn9V6siglged+DZCgfu9l6mO9dkX9VOg= +google.golang.org/api v0.128.0/go.mod h1:Y611qgqaE92On/7g65MQgxYul3c0rEB894kniWLY750= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= @@ -1040,8 +1043,8 @@ google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1/go.mod h1:9lPAdzaE google.golang.org/genproto v0.0.0-20210413151531-c14fb6ef47c3/go.mod h1:P3QM42oQyzQSnHPnZ/vqoCdDmzH28fzWByN9asMeM8A= google.golang.org/genproto v0.0.0-20211021150943-2b146023228c/go.mod h1:5CzLGKJ67TSI2B9POpiiyGha0AjJvZIUgRMt1dSmuhc= google.golang.org/genproto v0.0.0-20220107163113-42d7afdf6368/go.mod h1:5CzLGKJ67TSI2B9POpiiyGha0AjJvZIUgRMt1dSmuhc= -google.golang.org/genproto v0.0.0-20230629202037-9506855d4529 h1:9JucMWR7sPvCxUFd6UsOUNmA5kCcWOfORaT3tpAsKQs= -google.golang.org/genproto v0.0.0-20230629202037-9506855d4529/go.mod h1:xZnkP7mREFX5MORlOPEzLMr+90PPZQ2QWzrVTWfAq64= +google.golang.org/genproto v0.0.0-20230530153820-e85fd2cbaebc h1:8DyZCyvI8mE1IdLy/60bS+52xfymkE72wv1asokgtao= +google.golang.org/genproto v0.0.0-20230530153820-e85fd2cbaebc/go.mod h1:xZnkP7mREFX5MORlOPEzLMr+90PPZQ2QWzrVTWfAq64= google.golang.org/genproto/googleapis/api v0.0.0-20230629202037-9506855d4529 h1:s5YSX+ZH5b5vS9rnpGymvIyMpLRJizowqDlOuyjXnTk= google.golang.org/genproto/googleapis/api v0.0.0-20230629202037-9506855d4529/go.mod h1:vHYtlOoi6TsQ3Uk2yxR7NI5z8uoV+3pZtR4jmHIkRig= google.golang.org/genproto/googleapis/rpc v0.0.0-20230629202037-9506855d4529 h1:DEH99RbiLZhMxrpEJCZ0A+wdTe0EOgou/poSLx9vWf4= diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 04225764f..3c0394875 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -44,6 +44,7 @@ const ( CallGCPSMGetSecret = "GetSecret" CallGCPSMDeleteSecret = "DeleteSecret" CallGCPSMCreateSecret = "CreateSecret" + CallGCPSMUpdateSecret = "UpdateSecret" CallGCPSMAccessSecretVersion = "AccessSecretVersion" CallGCPSMAddSecretVersion = "AddSecretVersion" CallGCPSMListSecrets = "ListSecrets" diff --git a/pkg/provider/gcp/secretmanager/client.go b/pkg/provider/gcp/secretmanager/client.go index 867878621..713d8b000 100644 --- a/pkg/provider/gcp/secretmanager/client.go +++ b/pkg/provider/gcp/secretmanager/client.go @@ -27,8 +27,11 @@ import ( "github.com/googleapis/gax-go/v2" "github.com/googleapis/gax-go/v2/apierror" "github.com/tidwall/gjson" + "github.com/tidwall/sjson" "google.golang.org/api/iterator" + "google.golang.org/genproto/protobuf/field_mask" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ctrl "sigs.k8s.io/controller-runtime" kclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -61,6 +64,9 @@ const ( errInvalidAuthSecretRef = "invalid auth secret ref: %w" errInvalidWISARef = "invalid workload identity service account reference: %w" errUnexpectedFindOperator = "unexpected find operator" + + managedByKey = "managed-by" + managedByValue = "external-secrets" ) type Client struct { @@ -82,32 +88,25 @@ type GoogleSecretManagerClient interface { CreateSecret(ctx context.Context, req *secretmanagerpb.CreateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error) Close() error GetSecret(ctx context.Context, req *secretmanagerpb.GetSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error) + UpdateSecret(context.Context, *secretmanagerpb.UpdateSecretRequest, ...gax.CallOption) (*secretmanagerpb.Secret, error) } var log = ctrl.Log.WithName("provider").WithName("gcp").WithName("secretsmanager") func (c *Client) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemoteRef) error { - var gcpSecret *secretmanagerpb.Secret - var err error - - gcpSecret, err = c.smClient.GetSecret(ctx, &secretmanagerpb.GetSecretRequest{ + gcpSecret, err := c.smClient.GetSecret(ctx, &secretmanagerpb.GetSecretRequest{ Name: fmt.Sprintf("projects/%s/secrets/%s", c.store.ProjectID, remoteRef.GetRemoteKey()), }) metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMGetSecret, err) - var gErr *apierror.APIError - - if errors.As(err, &gErr) { - if gErr.GRPCStatus().Code() == codes.NotFound { + if err != nil { + if status.Code(err) == codes.NotFound { return nil } - return err - } - if err != nil { - return err - } - manager, ok := gcpSecret.Labels["managed-by"] - if !ok || manager != "external-secrets" { + return err + } + + if manager, ok := gcpSecret.Labels[managedByKey]; !ok || manager != managedByValue { return nil } @@ -129,47 +128,60 @@ func parseError(err error) error { // PushSecret pushes a kubernetes secret key into gcp provider Secret. func (c *Client) PushSecret(ctx context.Context, payload []byte, remoteRef esv1beta1.PushRemoteRef) error { - createSecretReq := &secretmanagerpb.CreateSecretRequest{ - Parent: fmt.Sprintf("projects/%s", c.store.ProjectID), - SecretId: remoteRef.GetRemoteKey(), - Secret: &secretmanagerpb.Secret{ - Labels: map[string]string{ - "managed-by": "external-secrets", - }, - Replication: &secretmanagerpb.Replication{ - Replication: &secretmanagerpb.Replication_Automatic_{ - Automatic: &secretmanagerpb.Replication_Automatic{}, - }, - }, - }, - } - - var gcpSecret *secretmanagerpb.Secret - var err error - - gcpSecret, err = c.smClient.GetSecret(ctx, &secretmanagerpb.GetSecretRequest{ + gcpSecret, err := c.smClient.GetSecret(ctx, &secretmanagerpb.GetSecretRequest{ Name: fmt.Sprintf("projects/%s/secrets/%s", c.store.ProjectID, remoteRef.GetRemoteKey()), }) metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMGetSecret, err) - var gErr *apierror.APIError + if err != nil { + if status.Code(err) != codes.NotFound { + return err + } - if err != nil && errors.As(err, &gErr) { - if gErr.GRPCStatus().Code() == codes.NotFound { - gcpSecret, err = c.smClient.CreateSecret(ctx, createSecretReq) - metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMCreateSecret, err) - if err != nil { - return err - } - } else { + gcpSecret, err = c.smClient.CreateSecret(ctx, &secretmanagerpb.CreateSecretRequest{ + Parent: fmt.Sprintf("projects/%s", c.store.ProjectID), + SecretId: remoteRef.GetRemoteKey(), + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{ + managedByKey: managedByValue, + }, + Replication: &secretmanagerpb.Replication{ + Replication: &secretmanagerpb.Replication_Automatic_{ + Automatic: &secretmanagerpb.Replication_Automatic{}, + }, + }, + }, + }) + metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMCreateSecret, err) + if err != nil { return err } } - manager, ok := gcpSecret.Labels["managed-by"] + manager, ok := gcpSecret.Labels[managedByKey] + if !ok || manager != managedByValue { + if remoteRef.GetProperty() == "" { + return fmt.Errorf("secret %v is not managed by external secrets", remoteRef.GetRemoteKey()) + } - if !ok || manager != "external-secrets" { - return fmt.Errorf("secret %v is not managed by external secrets", remoteRef.GetRemoteKey()) + labels := map[string]string{} + for k, v := range gcpSecret.Labels { + labels[k] = v + } + labels[managedByKey] = managedByValue + _, err = c.smClient.UpdateSecret(ctx, &secretmanagerpb.UpdateSecretRequest{ + Secret: &secretmanagerpb.Secret{ + Name: gcpSecret.Name, + Labels: labels, + }, + UpdateMask: &field_mask.FieldMask{ + Paths: []string{"labels"}, + }, + }) + metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMUpdateSecret, err) + if err != nil { + return err + } } gcpVersion, err := c.smClient.AccessSecretVersion(ctx, &secretmanagerpb.AccessSecretVersionRequest{ @@ -177,32 +189,46 @@ func (c *Client) PushSecret(ctx context.Context, payload []byte, remoteRef esv1b }) metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMAccessSecretVersion, err) - if errors.As(err, &gErr) { - if err != nil && gErr.GRPCStatus().Code() != codes.NotFound { - return err - } - } else if err != nil { + if err != nil && status.Code(err) != codes.NotFound { return err } - if gcpVersion != nil && gcpVersion.Payload != nil && bytes.Equal(payload, gcpVersion.Payload.Data) { - return nil + if gcpVersion != nil && gcpVersion.Payload != nil { + if remoteRef.GetProperty() == "" && bytes.Equal(payload, gcpVersion.Payload.Data) { + return nil + } + + if remoteRef.GetProperty() != "" { + val := getDataByProperty(gcpVersion, remoteRef.GetProperty()) + if val.Exists() && val.String() == string(payload) { + return nil + } + } + } + + data := payload + if remoteRef.GetProperty() != "" { + var base []byte + if gcpVersion != nil && gcpVersion.Payload != nil { + base = gcpVersion.Payload.Data + } + + data, err = sjson.SetBytes(base, remoteRef.GetProperty(), payload) + if err != nil { + return err + } } addSecretVersionReq := &secretmanagerpb.AddSecretVersionRequest{ Parent: fmt.Sprintf("projects/%s/secrets/%s", c.store.ProjectID, remoteRef.GetRemoteKey()), Payload: &secretmanagerpb.SecretPayload{ - Data: payload, + Data: data, }, } _, err = c.smClient.AddSecretVersion(ctx, addSecretVersionReq) metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMAddSecretVersion, err) - if err != nil { - return err - } - - return nil + return err } // GetAllSecrets syncs multiple secrets from gcp provider into a single Kubernetes Secret. @@ -363,20 +389,7 @@ func (c *Client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretData return nil, fmt.Errorf("invalid secret received. no secret string for key: %s", ref.Key) } - var payload string - if result.Payload.Data != nil { - payload = string(result.Payload.Data) - } - idx := strings.Index(ref.Property, ".") - refProperty := ref.Property - if idx > 0 { - refProperty = strings.ReplaceAll(refProperty, ".", "\\.") - val := gjson.Get(payload, refProperty) - if val.Exists() { - return []byte(val.String()), nil - } - } - val := gjson.Get(payload, ref.Property) + val := getDataByProperty(result, ref.Property) if !val.Exists() { return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key) } @@ -509,3 +522,20 @@ func (c *Client) Validate() (esv1beta1.ValidationResult, error) { } return esv1beta1.ValidationResultReady, nil } + +func getDataByProperty(resp *secretmanagerpb.AccessSecretVersionResponse, property string) gjson.Result { + var payload string + if resp.Payload.Data != nil { + payload = string(resp.Payload.Data) + } + idx := strings.Index(property, ".") + refProperty := property + if idx > 0 { + refProperty = strings.ReplaceAll(refProperty, ".", "\\.") + val := gjson.Get(payload, refProperty) + if val.Exists() { + return val + } + } + return gjson.Get(payload, property) +} diff --git a/pkg/provider/gcp/secretmanager/client_test.go b/pkg/provider/gcp/secretmanager/client_test.go index 106614ae9..49c6bba56 100644 --- a/pkg/provider/gcp/secretmanager/client_test.go +++ b/pkg/provider/gcp/secretmanager/client_test.go @@ -22,11 +22,13 @@ import ( "testing" "cloud.google.com/go/secretmanager/apiv1/secretmanagerpb" + "github.com/googleapis/gax-go/v2" "github.com/googleapis/gax-go/v2/apierror" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/utils/pointer" + esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1" esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1" v1 "github.com/external-secrets/external-secrets/apis/meta/v1" fakesm "github.com/external-secrets/external-secrets/pkg/provider/gcp/secretmanager/fake" @@ -197,7 +199,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { tests := []struct { name string ref esv1beta1.ExternalSecretDataRemoteRef - getSecretMockReturn fakesm.GetSecretMockReturn + getSecretMockReturn fakesm.SecretMockReturn expectedSecret string expectedErr string }{ @@ -208,7 +210,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "annotations.managed-by", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Annotations: map[string]string{ @@ -226,7 +228,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "labels.managed-by", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Labels: map[string]string{ @@ -244,7 +246,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "annotations", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Annotations: map[string]string{ @@ -267,7 +269,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "labels", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Annotations: map[string]string{ @@ -289,7 +291,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { Key: "bar", MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Labels: map[string]string{ @@ -310,7 +312,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "annotations.unknown", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Annotations: map[string]string{ @@ -328,7 +330,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "labels.unknown", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Labels: map[string]string{ @@ -346,7 +348,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) { MetadataPolicy: esv1beta1.ExternalSecretMetadataPolicyFetch, Property: "invalid.managed-by", }, - getSecretMockReturn: fakesm.GetSecretMockReturn{ + getSecretMockReturn: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", Labels: map[string]string{ @@ -414,7 +416,7 @@ func TestDeleteSecret(t *testing.T) { fakeClient := fakesm.MockSMClient{} type args struct { client fakesm.MockSMClient - getSecretOutput fakesm.GetSecretMockReturn + getSecretOutput fakesm.SecretMockReturn deleteSecretErr error } type want struct { @@ -429,7 +431,7 @@ func TestDeleteSecret(t *testing.T) { "Deletes Successfully": { args: args{ client: fakeClient, - getSecretOutput: fakesm.GetSecretMockReturn{ + getSecretOutput: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", @@ -444,7 +446,7 @@ func TestDeleteSecret(t *testing.T) { "Not Managed by ESO": { args: args{ client: fakeClient, - getSecretOutput: fakesm.GetSecretMockReturn{ + getSecretOutput: fakesm.SecretMockReturn{ Secret: &secretmanagerpb.Secret{ Name: "projects/foo/secret/bar", @@ -457,7 +459,7 @@ func TestDeleteSecret(t *testing.T) { "Secret Not Found": { args: args{ client: fakeClient, - getSecretOutput: fakesm.GetSecretMockReturn{ + getSecretOutput: fakesm.SecretMockReturn{ Secret: nil, Err: notFoundError, }, @@ -466,7 +468,7 @@ func TestDeleteSecret(t *testing.T) { "Random Error": { args: args{ client: fakeClient, - getSecretOutput: fakesm.GetSecretMockReturn{ + getSecretOutput: fakesm.SecretMockReturn{ Secret: nil, Err: errors.New("This errored out"), }, @@ -478,7 +480,7 @@ func TestDeleteSecret(t *testing.T) { "Random GError": { args: args{ client: fakeClient, - getSecretOutput: fakesm.GetSecretMockReturn{ + getSecretOutput: fakesm.SecretMockReturn{ Secret: nil, Err: permissionDeniedError, }, @@ -515,7 +517,7 @@ func TestDeleteSecret(t *testing.T) { } } -func TestSetSecret(t *testing.T) { +func TestPushSecret(t *testing.T) { ref := fakeRef{key: "/baz"} notFoundError := status.Error(codes.NotFound, "failed") @@ -584,10 +586,10 @@ func TestSetSecret(t *testing.T) { type args struct { mock *fakesm.MockSMClient - GetSecretMockReturn fakesm.GetSecretMockReturn + GetSecretMockReturn fakesm.SecretMockReturn AccessSecretVersionMockReturn fakesm.AccessSecretVersionMockReturn AddSecretVersionMockReturn fakesm.AddSecretVersionMockReturn - CreateSecretMockReturn fakesm.CreateSecretMockReturn + CreateSecretMockReturn fakesm.SecretMockReturn } type want struct { @@ -602,7 +604,7 @@ func TestSetSecret(t *testing.T) { reason: "SetSecret successfully pushes a secret", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &secret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil}, AddSecretVersionMockReturn: fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}}, want: want{ @@ -613,7 +615,7 @@ func TestSetSecret(t *testing.T) { reason: "secret not pushed if AddSecretVersion errors", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &secret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil}, AddSecretVersionMockReturn: fakesm.AddSecretVersionMockReturn{SecretVersion: nil, Err: APIerror}, }, @@ -625,7 +627,7 @@ func TestSetSecret(t *testing.T) { reason: "secret not pushed if AccessSecretVersion errors", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &secret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: APIerror}, }, want: want{ @@ -636,7 +638,7 @@ func TestSetSecret(t *testing.T) { reason: "secret not pushed if not managed-by external-secrets", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &wrongLabelSecret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &wrongLabelSecret, Err: nil}, }, want: want{ err: labelError, @@ -647,7 +649,7 @@ func TestSetSecret(t *testing.T) { args: args{ mock: smtc.mockClient, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res2, Err: nil}, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &secret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, }, want: want{ err: nil, @@ -657,10 +659,10 @@ func TestSetSecret(t *testing.T) { reason: "secret is created if one doesn't already exist", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: nil, Err: notFoundError}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: notFoundError}, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: notFoundError}, AddSecretVersionMockReturn: fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}, - CreateSecretMockReturn: fakesm.CreateSecretMockReturn{Secret: &secret, Err: nil}, + CreateSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, }, want: want{ err: nil, @@ -670,8 +672,8 @@ func TestSetSecret(t *testing.T) { reason: "secret not created if CreateSecret returns not found error", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: nil, Err: notFoundError}, - CreateSecretMockReturn: fakesm.CreateSecretMockReturn{Secret: &secret, Err: notFoundError}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: notFoundError}, + CreateSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: notFoundError}, }, want: want{ err: notFoundError, @@ -681,7 +683,7 @@ func TestSetSecret(t *testing.T) { reason: "secret not created if CreateSecret returns error", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: nil, Err: canceledError}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: canceledError}, }, want: want{ err: canceledError, @@ -691,7 +693,7 @@ func TestSetSecret(t *testing.T) { reason: "access secret version for an existing secret returns error", args: args{ mock: smtc.mockClient, - GetSecretMockReturn: fakesm.GetSecretMockReturn{Secret: &secret, Err: nil}, + GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}, AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: canceledError}, }, want: want{ @@ -728,6 +730,202 @@ func TestSetSecret(t *testing.T) { } } +func TestPushSecret_Property(t *testing.T) { + defaultAddSecretVersionMockReturn := func(gotPayload, expectedPayload string) (*secretmanagerpb.SecretVersion, error) { + if gotPayload != expectedPayload { + t.Fatalf("payload does not match: got %s, expected: %s", gotPayload, expectedPayload) + } + + return nil, nil + } + + tests := []struct { + desc string + payload string + ref esv1beta1.PushRemoteRef + getSecretMockReturn fakesm.SecretMockReturn + createSecretMockReturn fakesm.SecretMockReturn + updateSecretMockReturn fakesm.SecretMockReturn + accessSecretVersionMockReturn fakesm.AccessSecretVersionMockReturn + addSecretVersionMockReturn func(gotPayload, expectedPayload string) (*secretmanagerpb.SecretVersion, error) + expectedPayload string + expectedErr string + }{ + { + desc: "Add new key value paris", + payload: "testValue2", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey2", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{ + managedByKey: managedByValue, + }, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Res: &secretmanagerpb.AccessSecretVersionResponse{ + Payload: &secretmanagerpb.SecretPayload{ + Data: []byte(`{"testKey1":"testValue1"}`), + }, + }, + }, + addSecretVersionMockReturn: defaultAddSecretVersionMockReturn, + expectedPayload: `{"testKey1":"testValue1","testKey2":"testValue2"}`, + }, + { + desc: "Update existing value", + payload: "testValue2", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey1.testKey2", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{ + managedByKey: managedByValue, + }, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Res: &secretmanagerpb.AccessSecretVersionResponse{ + Payload: &secretmanagerpb.SecretPayload{ + Data: []byte(`{"testKey1":{"testKey2":"testValue1"}}`), + }, + }, + }, + addSecretVersionMockReturn: defaultAddSecretVersionMockReturn, + expectedPayload: `{"testKey1":{"testKey2":"testValue2"}}`, + }, + { + desc: "Secret not found", + payload: "testValue2", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey1.testKey3", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{}, + Err: status.Error(codes.NotFound, "failed to find a Secret"), + }, + createSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{managedByKey: managedByValue}, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Res: &secretmanagerpb.AccessSecretVersionResponse{ + Payload: &secretmanagerpb.SecretPayload{ + Data: []byte(`{"testKey1":{"testKey2":"testValue1"}}`), + }, + }, + }, + addSecretVersionMockReturn: defaultAddSecretVersionMockReturn, + expectedPayload: `{"testKey1":{"testKey2":"testValue1","testKey3":"testValue2"}}`, + }, + { + desc: "Secret version is not found", + payload: "testValue1", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey1", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{managedByKey: managedByValue}, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Err: status.Error(codes.NotFound, "failed to find a Secret Version"), + }, + addSecretVersionMockReturn: defaultAddSecretVersionMockReturn, + expectedPayload: `{"testKey1":"testValue1"}`, + }, + { + desc: "Secret is not managed by the controller", + payload: "testValue1", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey1.testKey2", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{}, + }, + updateSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{managedByKey: managedByValue}, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Res: &secretmanagerpb.AccessSecretVersionResponse{ + Payload: &secretmanagerpb.SecretPayload{ + Data: []byte(""), + }, + }, + }, + addSecretVersionMockReturn: defaultAddSecretVersionMockReturn, + expectedPayload: `{"testKey1":{"testKey2":"testValue1"}}`, + }, + { + desc: "Payload is the same with the existing one", + payload: "testValue1", + ref: esv1alpha1.PushSecretRemoteRef{ + Property: "testKey1.testKey2", + }, + getSecretMockReturn: fakesm.SecretMockReturn{ + Secret: &secretmanagerpb.Secret{ + Labels: map[string]string{ + managedByKey: managedByValue, + }, + }, + }, + accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{ + Res: &secretmanagerpb.AccessSecretVersionResponse{ + Payload: &secretmanagerpb.SecretPayload{ + Data: []byte(`{"testKey1":{"testKey2":"testValue1"}}`), + }, + }, + }, + addSecretVersionMockReturn: func(gotPayload, expectedPayload string) (*secretmanagerpb.SecretVersion, error) { + return nil, errors.New("should not be called") + }, + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + smClient := &fakesm.MockSMClient{ + AddSecretFn: func(_ context.Context, req *secretmanagerpb.AddSecretVersionRequest, _ ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { + return tc.addSecretVersionMockReturn(string(req.Payload.Data), tc.expectedPayload) + }, + } + smClient.NewGetSecretFn(tc.getSecretMockReturn) + smClient.NewCreateSecretFn(tc.createSecretMockReturn) + smClient.NewUpdateSecretFn(tc.updateSecretMockReturn) + smClient.NewAccessSecretVersionFn(tc.accessSecretVersionMockReturn) + + client := Client{ + smClient: smClient, + store: &esv1beta1.GCPSMProvider{}, + } + + err := client.PushSecret(context.Background(), []byte(tc.payload), tc.ref) + if err != nil { + if tc.expectedErr == "" { + t.Fatalf("PushSecret returns unexpected error: %v", err) + } + + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("PushSecret returns unexpected error: %q is supposed to contain %q", err, tc.expectedErr) + } + + return + } + + if tc.expectedErr != "" { + t.Fatal("PushSecret is expected to return error but got nil") + } + }) + } +} + func TestGetSecretMap(t *testing.T) { // good case: default version & deserialization setDeserialization := func(smtc *secretManagerTestCase) { diff --git a/pkg/provider/gcp/secretmanager/fake/fake.go b/pkg/provider/gcp/secretmanager/fake/fake.go index 107c224a3..da55b10aa 100644 --- a/pkg/provider/gcp/secretmanager/fake/fake.go +++ b/pkg/provider/gcp/secretmanager/fake/fake.go @@ -28,8 +28,9 @@ import ( type MockSMClient struct { accessSecretFn func(ctx context.Context, req *secretmanagerpb.AccessSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error) ListSecretsFn func(ctx context.Context, req *secretmanagerpb.ListSecretsRequest, opts ...gax.CallOption) *secretmanager.SecretIterator - addSecretFn func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) + AddSecretFn func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) createSecretFn func(ctx context.Context, req *secretmanagerpb.CreateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error) + updateSecretFn func(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error) closeFn func() error GetSecretFn func(ctx context.Context, req *secretmanagerpb.GetSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error) DeleteSecretFn func(ctx context.Context, req *secretmanagerpb.DeleteSecretRequest, opts ...gax.CallOption) error @@ -45,12 +46,7 @@ type AddSecretVersionMockReturn struct { Err error } -type GetSecretMockReturn struct { - Secret *secretmanagerpb.Secret - Err error -} - -type CreateSecretMockReturn struct { +type SecretMockReturn struct { Secret *secretmanagerpb.Secret Err error } @@ -67,7 +63,7 @@ func (mc *MockSMClient) GetSecret(ctx context.Context, req *secretmanagerpb.GetS return mc.GetSecretFn(ctx, req) } -func (mc *MockSMClient) NewGetSecretFn(mock GetSecretMockReturn) { +func (mc *MockSMClient) NewGetSecretFn(mock SecretMockReturn) { mc.GetSecretFn = func(_ context.Context, _ *secretmanagerpb.GetSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) { return mock.Secret, mock.Err } @@ -91,11 +87,11 @@ func (mc *MockSMClient) Close() error { } func (mc *MockSMClient) AddSecretVersion(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, _ ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { - return mc.addSecretFn(ctx, req) + return mc.AddSecretFn(ctx, req) } func (mc *MockSMClient) NewAddSecretVersionFn(mock AddSecretVersionMockReturn) { - mc.addSecretFn = func(_ context.Context, _ *secretmanagerpb.AddSecretVersionRequest, _ ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { + mc.AddSecretFn = func(_ context.Context, _ *secretmanagerpb.AddSecretVersionRequest, _ ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { return mock.SecretVersion, mock.Err } } @@ -104,7 +100,7 @@ func (mc *MockSMClient) CreateSecret(ctx context.Context, req *secretmanagerpb.C return mc.createSecretFn(ctx, req) } -func (mc *MockSMClient) NewCreateSecretFn(mock CreateSecretMockReturn) { +func (mc *MockSMClient) NewCreateSecretFn(mock SecretMockReturn) { mc.createSecretFn = func(_ context.Context, _ *secretmanagerpb.CreateSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) { return mock.Secret, mock.Err } @@ -146,7 +142,7 @@ func (mc *MockSMClient) DefaultCreateSecret(wantedSecretID, wantedParent string) } func (mc *MockSMClient) DefaultAddSecretVersion(wantedData, wantedParent, versionName string) { - mc.addSecretFn = func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { + mc.AddSecretFn = func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error) { if string(req.Payload.Data) != wantedData { return nil, fmt.Errorf("add version req wrong data got: %v want %v ", req.Payload.Data, wantedData) } @@ -177,6 +173,16 @@ func (mc *MockSMClient) AccessSecretVersionWithError(err error) { } } +func (mc *MockSMClient) UpdateSecret(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) { + return mc.updateSecretFn(ctx, req) +} + +func (mc *MockSMClient) NewUpdateSecretFn(mock SecretMockReturn) { + mc.updateSecretFn = func(_ context.Context, _ *secretmanagerpb.UpdateSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) { + return mock.Secret, mock.Err + } +} + func (mc *MockSMClient) WithValue(_ context.Context, req *secretmanagerpb.AccessSecretVersionRequest, val *secretmanagerpb.AccessSecretVersionResponse, err error) { if mc != nil { mc.accessSecretFn = func(paramCtx context.Context, paramReq *secretmanagerpb.AccessSecretVersionRequest, paramOpts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error) {