From 8c709cfa43d3ab7a6a027a03302e30db83d6ad43 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:29:07 +0200 Subject: [PATCH] feat: add prefix definition to all secret keys for aws parameter store (#3718) * feat: add prefix definition to all secret keys for aws parameter store Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> * added a push secret test to verify called parameter has a prefix Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> --------- Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> --- .../v1beta1/secretstore_aws_types.go | 4 ++ ...ternal-secrets.io_clustersecretstores.yaml | 3 + .../external-secrets.io_secretstores.yaml | 3 + deploy/crds/bundle.yaml | 6 ++ docs/api/spec.md | 12 ++++ pkg/provider/aws/parameterstore/fake/fake.go | 16 ++--- .../aws/parameterstore/parameterstore.go | 16 ++--- .../aws/parameterstore/parameterstore_test.go | 60 +++++++++++++++++-- pkg/provider/aws/provider.go | 4 +- 9 files changed, 104 insertions(+), 20 deletions(-) diff --git a/apis/externalsecrets/v1beta1/secretstore_aws_types.go b/apis/externalsecrets/v1beta1/secretstore_aws_types.go index ebc4c7976..0455c39cc 100644 --- a/apis/externalsecrets/v1beta1/secretstore_aws_types.go +++ b/apis/externalsecrets/v1beta1/secretstore_aws_types.go @@ -124,4 +124,8 @@ type AWSProvider struct { // AWS STS assume role transitive session tags. Required when multiple rules are used with the provider // +optional TransitiveTagKeys []*string `json:"transitiveTagKeys,omitempty"` + + // Prefix adds a prefix to all retrieved values. + // +optional + Prefix string `json:"prefix,omitempty"` } diff --git a/config/crds/bases/external-secrets.io_clustersecretstores.yaml b/config/crds/bases/external-secrets.io_clustersecretstores.yaml index cc6f887b7..9b7ff0ca2 100644 --- a/config/crds/bases/external-secrets.io_clustersecretstores.yaml +++ b/config/crds/bases/external-secrets.io_clustersecretstores.yaml @@ -2084,6 +2084,9 @@ spec: externalID: description: AWS External ID set on assumed IAM roles type: string + prefix: + description: Prefix adds a prefix to all retrieved values. + type: string region: description: AWS Region to be used for the provider type: string diff --git a/config/crds/bases/external-secrets.io_secretstores.yaml b/config/crds/bases/external-secrets.io_secretstores.yaml index e03e4f279..ddde61765 100644 --- a/config/crds/bases/external-secrets.io_secretstores.yaml +++ b/config/crds/bases/external-secrets.io_secretstores.yaml @@ -2084,6 +2084,9 @@ spec: externalID: description: AWS External ID set on assumed IAM roles type: string + prefix: + description: Prefix adds a prefix to all retrieved values. + type: string region: description: AWS Region to be used for the provider type: string diff --git a/deploy/crds/bundle.yaml b/deploy/crds/bundle.yaml index 7014d317f..bb14b0da1 100644 --- a/deploy/crds/bundle.yaml +++ b/deploy/crds/bundle.yaml @@ -2613,6 +2613,9 @@ spec: externalID: description: AWS External ID set on assumed IAM roles type: string + prefix: + description: Prefix adds a prefix to all retrieved values. + type: string region: description: AWS Region to be used for the provider type: string @@ -8244,6 +8247,9 @@ spec: externalID: description: AWS External ID set on assumed IAM roles type: string + prefix: + description: Prefix adds a prefix to all retrieved values. + type: string region: description: AWS Region to be used for the provider type: string diff --git a/docs/api/spec.md b/docs/api/spec.md index 7ff62480a..bf5dcbb96 100644 --- a/docs/api/spec.md +++ b/docs/api/spec.md @@ -281,6 +281,18 @@ SecretsManager

AWS STS assume role transitive session tags. Required when multiple rules are used with the provider

+ + +prefix
+ +string + + + +(Optional) +

Prefix adds a prefix to all retrieved values.

+ +

AWSServiceType diff --git a/pkg/provider/aws/parameterstore/fake/fake.go b/pkg/provider/aws/parameterstore/fake/fake.go index 591e37167..fd196bd98 100644 --- a/pkg/provider/aws/parameterstore/fake/fake.go +++ b/pkg/provider/aws/parameterstore/fake/fake.go @@ -26,13 +26,14 @@ import ( // Client implements the aws parameterstore interface. type Client struct { - GetParameterWithContextFn GetParameterWithContextFn - GetParametersByPathWithContextFn GetParametersByPathWithContextFn - PutParameterWithContextFn PutParameterWithContextFn - PutParameterWithContextCalledN int - DeleteParameterWithContextFn DeleteParameterWithContextFn - DescribeParametersWithContextFn DescribeParametersWithContextFn - ListTagsForResourceWithContextFn ListTagsForResourceWithContextFn + GetParameterWithContextFn GetParameterWithContextFn + GetParametersByPathWithContextFn GetParametersByPathWithContextFn + PutParameterWithContextFn PutParameterWithContextFn + PutParameterWithContextCalledN int + PutParameterWithContextFnCalledWith [][]*ssm.PutParameterInput + DeleteParameterWithContextFn DeleteParameterWithContextFn + DescribeParametersWithContextFn DescribeParametersWithContextFn + ListTagsForResourceWithContextFn ListTagsForResourceWithContextFn } type GetParameterWithContextFn func(aws.Context, *ssm.GetParameterInput, ...request.Option) (*ssm.GetParameterOutput, error) @@ -88,6 +89,7 @@ func NewDescribeParametersWithContextFn(output *ssm.DescribeParametersOutput, er func (sm *Client) PutParameterWithContext(ctx aws.Context, input *ssm.PutParameterInput, options ...request.Option) (*ssm.PutParameterOutput, error) { sm.PutParameterWithContextCalledN++ + sm.PutParameterWithContextFnCalledWith = append(sm.PutParameterWithContextFnCalledWith, []*ssm.PutParameterInput{input}) return sm.PutParameterWithContextFn(ctx, input, options...) } diff --git a/pkg/provider/aws/parameterstore/parameterstore.go b/pkg/provider/aws/parameterstore/parameterstore.go index 84e67181a..c9307a02c 100644 --- a/pkg/provider/aws/parameterstore/parameterstore.go +++ b/pkg/provider/aws/parameterstore/parameterstore.go @@ -60,6 +60,7 @@ type ParameterStore struct { sess *session.Session client PMInterface referentAuth bool + prefix string } // PMInterface is a subset of the parameterstore api. @@ -79,11 +80,12 @@ const ( ) // New constructs a ParameterStore Provider that is specific to a store. -func New(sess *session.Session, cfg *aws.Config, referentAuth bool) (*ParameterStore, error) { +func New(sess *session.Session, cfg *aws.Config, prefix string, referentAuth bool) (*ParameterStore, error) { return &ParameterStore{ sess: sess, referentAuth: referentAuth, client: ssm.New(sess, cfg), + prefix: prefix, }, nil } @@ -105,7 +107,7 @@ func (pm *ParameterStore) getTagsByName(ctx aws.Context, ref *ssm.GetParameterOu } func (pm *ParameterStore) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushSecretRemoteRef) error { - secretName := remoteRef.GetRemoteKey() + secretName := pm.prefix + remoteRef.GetRemoteKey() secretValue := ssm.GetParameterInput{ Name: &secretName, } @@ -179,7 +181,7 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret, } stringValue := string(value) - secretName := data.GetRemoteKey() + secretName := pm.prefix + data.GetRemoteKey() secretRequest := ssm.PutParameterInput{ Name: &secretName, @@ -466,7 +468,7 @@ func (pm *ParameterStore) GetSecret(ctx context.Context, ref esv1beta1.ExternalS func (pm *ParameterStore) getParameterTags(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (*ssm.GetParameterOutput, error) { param := ssm.GetParameterOutput{ Parameter: &ssm.Parameter{ - Name: parameterNameWithVersion(ref), + Name: pm.parameterNameWithVersion(ref), }, } tags, err := pm.getTagsByName(ctx, ¶m) @@ -487,7 +489,7 @@ func (pm *ParameterStore) getParameterTags(ctx context.Context, ref esv1beta1.Ex func (pm *ParameterStore) getParameterValue(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (*ssm.GetParameterOutput, error) { out, err := pm.client.GetParameterWithContext(ctx, &ssm.GetParameterInput{ - Name: parameterNameWithVersion(ref), + Name: pm.parameterNameWithVersion(ref), WithDecryption: aws.Bool(true), }) @@ -518,8 +520,8 @@ func (pm *ParameterStore) GetSecretMap(ctx context.Context, ref esv1beta1.Extern return secretData, nil } -func parameterNameWithVersion(ref esv1beta1.ExternalSecretDataRemoteRef) *string { - name := ref.Key +func (pm *ParameterStore) parameterNameWithVersion(ref esv1beta1.ExternalSecretDataRemoteRef) *string { + name := pm.prefix + ref.Key if ref.Version != "" { // see docs: https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-paramstore-versions.html#reference-parameter-version name += ":" + ref.Version diff --git a/pkg/provider/aws/parameterstore/parameterstore_test.go b/pkg/provider/aws/parameterstore/parameterstore_test.go index 4aeb307eb..2e1ccfdf0 100644 --- a/pkg/provider/aws/parameterstore/parameterstore_test.go +++ b/pkg/provider/aws/parameterstore/parameterstore_test.go @@ -42,6 +42,11 @@ const ( invalidProp = "INVALPROP" ) +var ( + fakeSecretKey = "fakeSecretKey" + fakeValue = "fakeValue" +) + type parameterstoreTestCase struct { fakeClient *fakeps.Client apiInput *ssm.GetParameterInput @@ -51,6 +56,7 @@ type parameterstoreTestCase struct { expectError string expectedSecret string expectedData map[string][]byte + prefix string } func makeValidParameterStoreTestCase() *parameterstoreTestCase { @@ -60,6 +66,7 @@ func makeValidParameterStoreTestCase() *parameterstoreTestCase { apiOutput: makeValidAPIOutput(), remoteRef: makeValidRemoteRef(), apiErr: nil, + prefix: "", expectError: "", expectedSecret: "", expectedData: make(map[string][]byte), @@ -270,8 +277,6 @@ const remoteKey = "fake-key" func TestPushSecret(t *testing.T) { invalidParameters := errors.New(ssm.ErrCodeInvalidParameters) alreadyExistsError := errors.New(ssm.ErrCodeAlreadyExistsException) - fakeSecretKey := "fakeSecretKey" - fakeValue := "fakeValue" fakeSecret := &corev1.Secret{ Data: map[string][]byte{ fakeSecretKey: []byte(fakeValue), @@ -518,9 +523,43 @@ func TestPushSecret(t *testing.T) { } } +func TestPushSecretWithPrefix(t *testing.T) { + fakeSecret := &corev1.Secret{ + Data: map[string][]byte{ + fakeSecretKey: []byte(fakeValue), + }, + } + managedByESO := ssm.Tag{ + Key: &managedBy, + Value: &externalSecrets, + } + putParameterOutput := &ssm.PutParameterOutput{} + getParameterOutput := &ssm.GetParameterOutput{} + describeParameterOutput := &ssm.DescribeParametersOutput{} + validListTagsForResourceOutput := &ssm.ListTagsForResourceOutput{ + TagList: []*ssm.Tag{&managedByESO}, + } + + client := fakeps.Client{ + PutParameterWithContextFn: fakeps.NewPutParameterWithContextFn(putParameterOutput, nil), + GetParameterWithContextFn: fakeps.NewGetParameterWithContextFn(getParameterOutput, nil), + DescribeParametersWithContextFn: fakeps.NewDescribeParametersWithContextFn(describeParameterOutput, nil), + ListTagsForResourceWithContextFn: fakeps.NewListTagsForResourceWithContextFn(validListTagsForResourceOutput, nil), + } + + psd := fake.PushSecretData{SecretKey: fakeSecretKey, RemoteKey: remoteKey} + ps := ParameterStore{ + client: &client, + prefix: "/test/this/thing/", + } + err := ps.PushSecret(context.TODO(), fakeSecret, psd) + require.NoError(t, err) + + input := client.PutParameterWithContextFnCalledWith[0][0] + assert.Equal(t, "/test/this/thing/fake-key", *input.Name) +} + func TestPushSecretCalledOnlyOnce(t *testing.T) { - fakeSecretKey := "fakeSecretKey" - fakeValue := "fakeValue" fakeSecret := &corev1.Secret{ Data: map[string][]byte{ fakeSecretKey: []byte(fakeValue), @@ -569,6 +608,17 @@ func TestGetSecret(t *testing.T) { pstc.expectedSecret = "RRRRR" } + // good case: key is passed in and prefix is set, output is sent back + setSecretStringWithPrefix := func(pstc *parameterstoreTestCase) { + pstc.apiInput = &ssm.GetParameterInput{ + Name: aws.String("/test/this/baz"), + WithDecryption: aws.Bool(true), + } + pstc.prefix = "/test/this" + pstc.apiOutput.Parameter.Value = aws.String("RRRRR") + pstc.expectedSecret = "RRRRR" + } + // good case: extract property setExtractProperty := func(pstc *parameterstoreTestCase) { pstc.apiOutput.Parameter.Value = aws.String(`{"/shmoo": "bang"}`) @@ -649,6 +699,7 @@ func TestGetSecret(t *testing.T) { } successCases := []*parameterstoreTestCase{ + makeValidParameterStoreTestCaseCustom(setSecretStringWithPrefix), makeValidParameterStoreTestCaseCustom(setSecretString), makeValidParameterStoreTestCaseCustom(setExtractProperty), makeValidParameterStoreTestCaseCustom(setMissingProperty), @@ -665,6 +716,7 @@ func TestGetSecret(t *testing.T) { ps := ParameterStore{} for k, v := range successCases { ps.client = v.fakeClient + ps.prefix = v.prefix out, err := ps.GetSecret(context.Background(), *v.remoteRef) if !ErrorContains(err, v.expectError) { t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError) diff --git a/pkg/provider/aws/provider.go b/pkg/provider/aws/provider.go index 77e1cc136..0089f5fb7 100644 --- a/pkg/provider/aws/provider.go +++ b/pkg/provider/aws/provider.go @@ -156,7 +156,7 @@ func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Cl case esv1beta1.AWSServiceSecretsManager: return secretsmanager.New(sess, cfg, prov.SecretsManager, true) case esv1beta1.AWSServiceParameterStore: - return parameterstore.New(sess, cfg, true) + return parameterstore.New(sess, cfg, storeSpec.Provider.AWS.Prefix, true) } return nil, fmt.Errorf(errUnknownProviderService, prov.Service) } @@ -195,7 +195,7 @@ func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Cl case esv1beta1.AWSServiceSecretsManager: return secretsmanager.New(sess, cfg, prov.SecretsManager, false) case esv1beta1.AWSServiceParameterStore: - return parameterstore.New(sess, cfg, false) + return parameterstore.New(sess, cfg, storeSpec.Provider.AWS.Prefix, false) } return nil, fmt.Errorf(errUnknownProviderService, prov.Service) }