From c7f6258d37fff95a2ec5ae1bd285ac83f7b35368 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 18 Mar 2025 18:41:03 -0700 Subject: [PATCH 01/12] Add support for multiple keys per secret Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/metadata.yaml | 8 +- .../aws/secretmanager/secretmanager.go | 45 +++++-- .../aws/secretmanager/secretmanager_test.go | 114 ++++++++++++++++++ 3 files changed, 153 insertions(+), 14 deletions(-) diff --git a/secretstores/aws/secretmanager/metadata.yaml b/secretstores/aws/secretmanager/metadata.yaml index 21bfbd5b2c..ce66e2ef77 100644 --- a/secretstores/aws/secretmanager/metadata.yaml +++ b/secretstores/aws/secretmanager/metadata.yaml @@ -16,4 +16,10 @@ metadata: description: | The Secrets manager endpoint. The AWS SDK will generate a default endpoint if not specified. Useful for local testing with AWS LocalStack example: '"http://localhost:4566"' - type: string \ No newline at end of file + type: string + - name: multipleKeysPerSecret + required: false + description: | + A boolean value to indicate if the secrets with multiple keys should break keys out. + example: "true" + type: bool \ No newline at end of file diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 979739be5b..21bd2ac157 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -40,16 +40,18 @@ func NewSecretManager(logger logger.Logger) secretstores.SecretStore { } type SecretManagerMetaData struct { - Region string `json:"region" mapstructure:"region" mdignore:"true"` - AccessKey string `json:"accessKey" mapstructure:"accessKey" mdignore:"true"` - SecretKey string `json:"secretKey" mapstructure:"secretKey" mdignore:"true"` - SessionToken string `json:"sessionToken" mapstructure:"sessionToken" mdignore:"true"` - Endpoint string `json:"endpoint" mapstructure:"endpoint"` + Region string `json:"region" mapstructure:"region" mdignore:"true"` + AccessKey string `json:"accessKey" mapstructure:"accessKey" mdignore:"true"` + SecretKey string `json:"secretKey" mapstructure:"secretKey" mdignore:"true"` + SessionToken string `json:"sessionToken" mapstructure:"sessionToken" mdignore:"true"` + Endpoint string `json:"endpoint" mapstructure:"endpoint"` + MultipleKeysPerSecret bool `json:"multipleKeysPerSecret" mapstructure:"multipleKeysPerSecret"` } type smSecretStore struct { - authProvider awsAuth.Provider - logger logger.Logger + authProvider awsAuth.Provider + logger logger.Logger + multipleKeysPerSecret bool } // Init creates an AWS secret manager client. @@ -67,6 +69,7 @@ func (s *smSecretStore) Init(ctx context.Context, metadata secretstores.Metadata SessionToken: meta.SessionToken, Endpoint: meta.Endpoint, } + s.multipleKeysPerSecret = meta.MultipleKeysPerSecret provider, err := awsAuth.NewProvider(ctx, opts, awsAuth.GetConfig(opts)) if err != nil { @@ -76,6 +79,26 @@ func (s *smSecretStore) Init(ctx context.Context, metadata secretstores.Metadata return nil } +func (s *smSecretStore) formatSecret(output *secretsmanager.GetSecretValueOutput) map[string]string { + result := map[string]string{} + + if output.Name != nil && output.SecretString != nil { + if s.multipleKeysPerSecret { + data := map[string]string{} + err := json.Unmarshal([]byte(*output.SecretString), &data) + if err == nil { + result = data + } else { + result[*output.Name] = *output.SecretString + } + } else { + result[*output.Name] = *output.SecretString + } + } + + return result +} + // GetSecret retrieves a secret using a key and returns a map of decrypted string/string values. func (s *smSecretStore) GetSecret(ctx context.Context, req secretstores.GetSecretRequest) (secretstores.GetSecretResponse, error) { var versionID *string @@ -98,9 +121,7 @@ func (s *smSecretStore) GetSecret(ctx context.Context, req secretstores.GetSecre resp := secretstores.GetSecretResponse{ Data: map[string]string{}, } - if output.Name != nil && output.SecretString != nil { - resp.Data[*output.Name] = *output.SecretString - } + resp.Data = s.formatSecret(output) return resp, nil } @@ -131,9 +152,7 @@ func (s *smSecretStore) BulkGetSecret(ctx context.Context, req secretstores.Bulk return secretstores.BulkGetSecretResponse{Data: nil}, fmt.Errorf("couldn't get secret: %s", *entry.Name) } - if entry.Name != nil && secrets.SecretString != nil { - resp.Data[*entry.Name] = map[string]string{*entry.Name: *secrets.SecretString} - } + resp.Data[*entry.Name] = s.formatSecret(secrets) } nextToken = output.NextToken diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index fce8169f62..1d23fc358b 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -162,6 +162,120 @@ func TestGetSecret(t *testing.T) { require.NoError(t, e) assert.Equal(t, secretValue, output.Data[req.Name]) }) + + t.Run("with multiple keys per secret", func(t *testing.T) { + mockSSM := &awsAuth.MockSecretManager{ + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + secret := `{"key1":"value1","key2":"value2"}` + + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secret, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + multipleKeysPerSecret: true, + } + + req := secretstores.GetSecretRequest{ + Name: "/aws/secret/testing", + Metadata: map[string]string{}, + } + output, e := s.GetSecret(t.Context(), req) + require.NoError(t, e) + assert.Len(t, output.Data, 2) + assert.Equal(t, "value1", output.Data["key1"]) + assert.Equal(t, "value2", output.Data["key2"]) + }) + + t.Run("with multiple keys per secret and option disabled", func(t *testing.T) { + mockSSM := &awsAuth.MockSecretManager{ + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + secret := `{"key1":"value1","key2":"value2"}` + + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secret, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + } + + req := secretstores.GetSecretRequest{ + Name: "/aws/secret/testing", + Metadata: map[string]string{}, + } + output, e := s.GetSecret(t.Context(), req) + require.NoError(t, e) + assert.Len(t, output.Data, 1) + assert.Equal(t, `{"key1":"value1","key2":"value2"}`, output.Data["/aws/secret/testing"]) + }) + + t.Run("with multiple keys per secret and secret is NOT json", func(t *testing.T) { + mockSSM := &awsAuth.MockSecretManager{ + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + secret := "not json" + + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secret, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + multipleKeysPerSecret: true, + } + + req := secretstores.GetSecretRequest{ + Name: "/aws/secret/testing", + Metadata: map[string]string{}, + } + output, e := s.GetSecret(t.Context(), req) + require.NoError(t, e) + assert.Len(t, output.Data, 1) + assert.Equal(t, "not json", output.Data["/aws/secret/testing"]) + }) }) t.Run("unsuccessfully retrieve secret", func(t *testing.T) { From b5e8867f1ccc8b7b9342851ea5a5a8c3accbfd36 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 18 Mar 2025 18:55:22 -0700 Subject: [PATCH 02/12] Advertise feature if metadata property is set to tru Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager.go | 6 +++++- .../aws/secretmanager/secretmanager_test.go | 14 +++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 21bd2ac157..e387871782 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -179,7 +179,11 @@ func (s *smSecretStore) getSecretManagerMetadata(spec secretstores.Metadata) (*S // Features returns the features available in this secret store. func (s *smSecretStore) Features() []secretstores.Feature { - return []secretstores.Feature{} // No Feature supported. + if s.multipleKeysPerSecret { + return []secretstores.Feature{secretstores.FeatureMultipleKeyValuesPerSecret} + } + + return []secretstores.Feature{} } func (s *smSecretStore) GetComponentMetadata() (metadataInfo metadata.MetadataMap) { diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 1d23fc358b..a5d3381066 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -310,7 +310,19 @@ func TestGetSecret(t *testing.T) { func TestGetFeatures(t *testing.T) { s := smSecretStore{} - t.Run("no features are advertised", func(t *testing.T) { + t.Run("when multipleKeysPerSecret = true, return feature", func(t *testing.T) { + s.multipleKeysPerSecret = true + f := s.Features() + assert.True(t, secretstores.FeatureMultipleKeyValuesPerSecret.IsPresent(f)) + }) + + t.Run("when multipleKeysPerSecret = false, no feature advertised", func(t *testing.T) { + s.multipleKeysPerSecret = false + f := s.Features() + assert.Empty(t, f) + }) + + t.Run("by default, no feature advertised", func(t *testing.T) { f := s.Features() assert.Empty(t, f) }) From aa86c4303479e261bc41dbcaccc7235f8d4a784b Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 18 Mar 2025 19:22:44 -0700 Subject: [PATCH 03/12] Add BulkGetSecret tests Signed-off-by: Patrick Assuied --- common/authentication/aws/client_fake.go | 6 + .../aws/secretmanager/secretmanager_test.go | 133 ++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/common/authentication/aws/client_fake.go b/common/authentication/aws/client_fake.go index c9e23641ba..35f752a96a 100644 --- a/common/authentication/aws/client_fake.go +++ b/common/authentication/aws/client_fake.go @@ -43,12 +43,18 @@ func (m *MockParameterStore) DescribeParametersWithContext(ctx context.Context, type MockSecretManager struct { GetSecretValueFn func(context.Context, *secretsmanager.GetSecretValueInput, ...request.Option) (*secretsmanager.GetSecretValueOutput, error) secretsmanageriface.SecretsManagerAPI + + ListSecretsFn func(context.Context, *secretsmanager.ListSecretsInput, ...request.Option) (*secretsmanager.ListSecretsOutput, error) } func (m *MockSecretManager) GetSecretValueWithContext(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { return m.GetSecretValueFn(ctx, input, option...) } +func (m *MockSecretManager) ListSecretsWithContext(ctx context.Context, input *secretsmanager.ListSecretsInput, option ...request.Option) (*secretsmanager.ListSecretsOutput, error) { + return m.ListSecretsFn(ctx, input, option...) +} + type MockDynamoDB struct { GetItemWithContextFn func(ctx context.Context, input *dynamodb.GetItemInput, op ...request.Option) (*dynamodb.GetItemOutput, error) PutItemWithContextFn func(ctx context.Context, input *dynamodb.PutItemInput, op ...request.Option) (*dynamodb.PutItemOutput, error) diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index a5d3381066..45455d2ff6 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -308,6 +308,139 @@ func TestGetSecret(t *testing.T) { }) } +func TestBulkGetSecret(t *testing.T) { + t.Run("returns all secrets in store", func(t *testing.T) { + secret1 := "/aws/secret/testing1" + secretValue1 := "secret1" + secret2 := "/aws/secret/testing2" + secretValue2 := "secret2" + + mockSSM := &awsAuth.MockSecretManager{ + + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + + if input.SecretId == &secret1 { + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secretValue1, + }, nil + } else { + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secretValue2, + }, nil + } + + }, + + ListSecretsFn: func(ctx context.Context, input *secretsmanager.ListSecretsInput, option ...request.Option) (*secretsmanager.ListSecretsOutput, error) { + return &secretsmanager.ListSecretsOutput{ + SecretList: []*secretsmanager.SecretListEntry{ + {Name: &secret1}, + {Name: &secret2}, + }, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + } + + req := secretstores.BulkGetSecretRequest{ + Metadata: map[string]string{}, + } + output, e := s.BulkGetSecret(t.Context(), req) + require.NoError(t, e) + assert.Equal(t, map[string]map[string]string{ + secret1: { + secret1: secretValue1, + }, + secret2: { + secret2: secretValue2, + }, + }, output.Data) + }) + + t.Run("when multipleKeysPerSecret = true, returns all secrets in store broken out by key", func(t *testing.T) { + secret1 := "/aws/secret/testing1" + secretValue1 := `{"key1":"value1","key2":"value2"}` + secret2 := "/aws/secret/testing2" + secretValue2 := `{"key3":"value3","key4":"value4"}` + + mockSSM := &awsAuth.MockSecretManager{ + + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + + if input.SecretId == &secret1 { + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secretValue1, + }, nil + } else { + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secretValue2, + }, nil + } + + }, + + ListSecretsFn: func(ctx context.Context, input *secretsmanager.ListSecretsInput, option ...request.Option) (*secretsmanager.ListSecretsOutput, error) { + return &secretsmanager.ListSecretsOutput{ + SecretList: []*secretsmanager.SecretListEntry{ + {Name: &secret1}, + {Name: &secret2}, + }, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + multipleKeysPerSecret: true, + } + + req := secretstores.BulkGetSecretRequest{ + Metadata: map[string]string{}, + } + output, e := s.BulkGetSecret(t.Context(), req) + require.NoError(t, e) + assert.Equal(t, map[string]map[string]string{ + secret1: { + "key1": "value1", + "key2": "value2", + }, + secret2: { + "key3": "value3", + "key4": "value4", + }, + }, output.Data) + }) +} + func TestGetFeatures(t *testing.T) { s := smSecretStore{} t.Run("when multipleKeysPerSecret = true, return feature", func(t *testing.T) { From fac17725f0b3c55be8378e902e23f5f63493a21e Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Mon, 24 Mar 2025 12:52:59 -0700 Subject: [PATCH 04/12] format Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 45455d2ff6..564b933436 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -168,6 +168,7 @@ func TestGetSecret(t *testing.T) { GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) + // #nosec G101: This is a mock secret used for testing purposes. secret := `{"key1":"value1","key2":"value2"}` return &secretsmanager.GetSecretValueOutput{ @@ -207,6 +208,7 @@ func TestGetSecret(t *testing.T) { GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) + // #nosec G101: This is a mock secret used for testing purposes. secret := `{"key1":"value1","key2":"value2"}` return &secretsmanager.GetSecretValueOutput{ @@ -236,7 +238,7 @@ func TestGetSecret(t *testing.T) { output, e := s.GetSecret(t.Context(), req) require.NoError(t, e) assert.Len(t, output.Data, 1) - assert.Equal(t, `{"key1":"value1","key2":"value2"}`, output.Data["/aws/secret/testing"]) + assert.JSONEq(t, `{"key1":"value1","key2":"value2"}`, output.Data["/aws/secret/testing"]) }) t.Run("with multiple keys per secret and secret is NOT json", func(t *testing.T) { @@ -316,7 +318,6 @@ func TestBulkGetSecret(t *testing.T) { secretValue2 := "secret2" mockSSM := &awsAuth.MockSecretManager{ - GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) @@ -332,7 +333,6 @@ func TestBulkGetSecret(t *testing.T) { SecretString: &secretValue2, }, nil } - }, ListSecretsFn: func(ctx context.Context, input *secretsmanager.ListSecretsInput, option ...request.Option) (*secretsmanager.ListSecretsOutput, error) { @@ -375,12 +375,13 @@ func TestBulkGetSecret(t *testing.T) { t.Run("when multipleKeysPerSecret = true, returns all secrets in store broken out by key", func(t *testing.T) { secret1 := "/aws/secret/testing1" + // #nosec G101: This is a mock secret used for testing purposes. secretValue1 := `{"key1":"value1","key2":"value2"}` secret2 := "/aws/secret/testing2" + // #nosec G101: This is a mock secret used for testing purposes. secretValue2 := `{"key3":"value3","key4":"value4"}` mockSSM := &awsAuth.MockSecretManager{ - GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) @@ -396,7 +397,6 @@ func TestBulkGetSecret(t *testing.T) { SecretString: &secretValue2, }, nil } - }, ListSecretsFn: func(ctx context.Context, input *secretsmanager.ListSecretsInput, option ...request.Option) (*secretsmanager.ListSecretsOutput, error) { From 04d68d8c40adfcbb23ed5ba8afbbc8660b0f78d7 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 1 Apr 2025 08:54:27 -0700 Subject: [PATCH 05/12] Rename metadata to match feature name Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/metadata.yaml | 4 ++-- .../aws/secretmanager/secretmanager.go | 24 +++++++++---------- .../aws/secretmanager/secretmanager_test.go | 22 ++++++++--------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/secretstores/aws/secretmanager/metadata.yaml b/secretstores/aws/secretmanager/metadata.yaml index ce66e2ef77..5f7769cb76 100644 --- a/secretstores/aws/secretmanager/metadata.yaml +++ b/secretstores/aws/secretmanager/metadata.yaml @@ -17,9 +17,9 @@ metadata: The Secrets manager endpoint. The AWS SDK will generate a default endpoint if not specified. Useful for local testing with AWS LocalStack example: '"http://localhost:4566"' type: string - - name: multipleKeysPerSecret + - name: multipleKeyValuesPerSecret required: false description: | - A boolean value to indicate if the secrets with multiple keys should break keys out. + A boolean value to indicate if the secrets with multiple key/values should break keys out. example: "true" type: bool \ No newline at end of file diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index e387871782..4dced45683 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -40,18 +40,18 @@ func NewSecretManager(logger logger.Logger) secretstores.SecretStore { } type SecretManagerMetaData struct { - Region string `json:"region" mapstructure:"region" mdignore:"true"` - AccessKey string `json:"accessKey" mapstructure:"accessKey" mdignore:"true"` - SecretKey string `json:"secretKey" mapstructure:"secretKey" mdignore:"true"` - SessionToken string `json:"sessionToken" mapstructure:"sessionToken" mdignore:"true"` - Endpoint string `json:"endpoint" mapstructure:"endpoint"` - MultipleKeysPerSecret bool `json:"multipleKeysPerSecret" mapstructure:"multipleKeysPerSecret"` + Region string `json:"region" mapstructure:"region" mdignore:"true"` + AccessKey string `json:"accessKey" mapstructure:"accessKey" mdignore:"true"` + SecretKey string `json:"secretKey" mapstructure:"secretKey" mdignore:"true"` + SessionToken string `json:"sessionToken" mapstructure:"sessionToken" mdignore:"true"` + Endpoint string `json:"endpoint" mapstructure:"endpoint"` + MultipleKeyValuesPerSecret bool `json:"multipleKeyValuesPerSecret" mapstructure:"multipleKeysPerSecret"` } type smSecretStore struct { - authProvider awsAuth.Provider - logger logger.Logger - multipleKeysPerSecret bool + authProvider awsAuth.Provider + logger logger.Logger + multipleKeyValuesPerSecret bool } // Init creates an AWS secret manager client. @@ -69,7 +69,7 @@ func (s *smSecretStore) Init(ctx context.Context, metadata secretstores.Metadata SessionToken: meta.SessionToken, Endpoint: meta.Endpoint, } - s.multipleKeysPerSecret = meta.MultipleKeysPerSecret + s.multipleKeyValuesPerSecret = meta.MultipleKeyValuesPerSecret provider, err := awsAuth.NewProvider(ctx, opts, awsAuth.GetConfig(opts)) if err != nil { @@ -83,7 +83,7 @@ func (s *smSecretStore) formatSecret(output *secretsmanager.GetSecretValueOutput result := map[string]string{} if output.Name != nil && output.SecretString != nil { - if s.multipleKeysPerSecret { + if s.multipleKeyValuesPerSecret { data := map[string]string{} err := json.Unmarshal([]byte(*output.SecretString), &data) if err == nil { @@ -179,7 +179,7 @@ func (s *smSecretStore) getSecretManagerMetadata(spec secretstores.Metadata) (*S // Features returns the features available in this secret store. func (s *smSecretStore) Features() []secretstores.Feature { - if s.multipleKeysPerSecret { + if s.multipleKeyValuesPerSecret { return []secretstores.Feature{secretstores.FeatureMultipleKeyValuesPerSecret} } diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 564b933436..f955f8e735 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -188,8 +188,8 @@ func TestGetSecret(t *testing.T) { mockAuthProvider := &awsAuth.StaticAuth{} mockAuthProvider.WithMockClients(&mockedClients) s := smSecretStore{ - authProvider: mockAuthProvider, - multipleKeysPerSecret: true, + authProvider: mockAuthProvider, + multipleKeyValuesPerSecret: true, } req := secretstores.GetSecretRequest{ @@ -265,8 +265,8 @@ func TestGetSecret(t *testing.T) { mockAuthProvider := &awsAuth.StaticAuth{} mockAuthProvider.WithMockClients(&mockedClients) s := smSecretStore{ - authProvider: mockAuthProvider, - multipleKeysPerSecret: true, + authProvider: mockAuthProvider, + multipleKeyValuesPerSecret: true, } req := secretstores.GetSecretRequest{ @@ -373,7 +373,7 @@ func TestBulkGetSecret(t *testing.T) { }, output.Data) }) - t.Run("when multipleKeysPerSecret = true, returns all secrets in store broken out by key", func(t *testing.T) { + t.Run("when multipleKeyValuesPerSecret = true, returns all secrets in store broken out by key", func(t *testing.T) { secret1 := "/aws/secret/testing1" // #nosec G101: This is a mock secret used for testing purposes. secretValue1 := `{"key1":"value1","key2":"value2"}` @@ -419,8 +419,8 @@ func TestBulkGetSecret(t *testing.T) { mockAuthProvider := &awsAuth.StaticAuth{} mockAuthProvider.WithMockClients(&mockedClients) s := smSecretStore{ - authProvider: mockAuthProvider, - multipleKeysPerSecret: true, + authProvider: mockAuthProvider, + multipleKeyValuesPerSecret: true, } req := secretstores.BulkGetSecretRequest{ @@ -443,14 +443,14 @@ func TestBulkGetSecret(t *testing.T) { func TestGetFeatures(t *testing.T) { s := smSecretStore{} - t.Run("when multipleKeysPerSecret = true, return feature", func(t *testing.T) { - s.multipleKeysPerSecret = true + t.Run("when multipleKeyValuesPerSecret = true, return feature", func(t *testing.T) { + s.multipleKeyValuesPerSecret = true f := s.Features() assert.True(t, secretstores.FeatureMultipleKeyValuesPerSecret.IsPresent(f)) }) - t.Run("when multipleKeysPerSecret = false, no feature advertised", func(t *testing.T) { - s.multipleKeysPerSecret = false + t.Run("when multipleKeyValuesPerSecret = false, no feature advertised", func(t *testing.T) { + s.multipleKeyValuesPerSecret = false f := s.Features() assert.Empty(t, f) }) From adcb27f6e2f4913d0f389e7ff9b181b505edd3c2 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Wed, 2 Apr 2025 11:16:31 -0700 Subject: [PATCH 06/12] fixed missed ref in rename Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 4dced45683..308b96e02b 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -45,7 +45,7 @@ type SecretManagerMetaData struct { SecretKey string `json:"secretKey" mapstructure:"secretKey" mdignore:"true"` SessionToken string `json:"sessionToken" mapstructure:"sessionToken" mdignore:"true"` Endpoint string `json:"endpoint" mapstructure:"endpoint"` - MultipleKeyValuesPerSecret bool `json:"multipleKeyValuesPerSecret" mapstructure:"multipleKeysPerSecret"` + MultipleKeyValuesPerSecret bool `json:"multipleKeyValuesPerSecret" mapstructure:"multipleKeyValuesPerSecret"` } type smSecretStore struct { From a0d9c2feca47919da1c6d88b7bb673b0036b4986 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 22 Apr 2025 15:00:50 -0700 Subject: [PATCH 07/12] fixed cases where the stored values are nested json. Make sure to properly deserialize an stringify in those use cases. Signed-off-by: Patrick Assuied --- .../aws/secretmanager/secretmanager.go | 19 +++++++++++++++++-- .../aws/secretmanager/secretmanager_test.go | 9 +++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 308b96e02b..4660c5d2dc 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -79,15 +79,30 @@ func (s *smSecretStore) Init(ctx context.Context, metadata secretstores.Metadata return nil } +func convertMapAnyToString(m map[string]any) map[string]string { + result := make(map[string]string, len(m)) + for k, v := range m { + switch v := v.(type) { + case string: + result[k] = v + default: + jVal, _ := json.Marshal(v) + result[k] = string(jVal) + } + } + return result +} + func (s *smSecretStore) formatSecret(output *secretsmanager.GetSecretValueOutput) map[string]string { result := map[string]string{} if output.Name != nil && output.SecretString != nil { if s.multipleKeyValuesPerSecret { - data := map[string]string{} + data := map[string]any{} err := json.Unmarshal([]byte(*output.SecretString), &data) if err == nil { - result = data + // In case of a nested JSON value, we need to stringify it + result = convertMapAnyToString(data) } else { result[*output.Name] = *output.SecretString } diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index f955f8e735..9c0f003233 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -169,7 +169,7 @@ func TestGetSecret(t *testing.T) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) // #nosec G101: This is a mock secret used for testing purposes. - secret := `{"key1":"value1","key2":"value2"}` + secret := `{"key1":"value1","key2":"value2","key3":{"nested":"value3"}}` return &secretsmanager.GetSecretValueOutput{ Name: input.SecretId, @@ -198,9 +198,10 @@ func TestGetSecret(t *testing.T) { } output, e := s.GetSecret(t.Context(), req) require.NoError(t, e) - assert.Len(t, output.Data, 2) + assert.Len(t, output.Data, 3) assert.Equal(t, "value1", output.Data["key1"]) assert.Equal(t, "value2", output.Data["key2"]) + assert.Equal(t, `{"nested":"value3"}`, output.Data["key3"]) }) t.Run("with multiple keys per secret and option disabled", func(t *testing.T) { @@ -379,7 +380,7 @@ func TestBulkGetSecret(t *testing.T) { secretValue1 := `{"key1":"value1","key2":"value2"}` secret2 := "/aws/secret/testing2" // #nosec G101: This is a mock secret used for testing purposes. - secretValue2 := `{"key3":"value3","key4":"value4"}` + secretValue2 := `{"key3":"value3","key4":{"nested":"value4"}}` mockSSM := &awsAuth.MockSecretManager{ GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { @@ -435,7 +436,7 @@ func TestBulkGetSecret(t *testing.T) { }, secret2: { "key3": "value3", - "key4": "value4", + "key4": `{"nested":"value4"}`, }, }, output.Data) }) From 07a0095e223359ad020757e31bc9b01641895630 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 22 Apr 2025 15:20:29 -0700 Subject: [PATCH 08/12] linting error Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 9c0f003233..08de1be0c1 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -201,7 +201,7 @@ func TestGetSecret(t *testing.T) { assert.Len(t, output.Data, 3) assert.Equal(t, "value1", output.Data["key1"]) assert.Equal(t, "value2", output.Data["key2"]) - assert.Equal(t, `{"nested":"value3"}`, output.Data["key3"]) + assert.JSONEq(t, `{"nested":"value3"}`, output.Data["key3"]) }) t.Run("with multiple keys per secret and option disabled", func(t *testing.T) { From c694d58394e986feca0ab7859744255df52bb084 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 22 Apr 2025 18:46:15 -0700 Subject: [PATCH 09/12] Add'l test case per PR feedback Signed-off-by: Patrick Assuied --- .../aws/secretmanager/secretmanager_test.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 08de1be0c1..9e050d2168 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -279,6 +279,44 @@ func TestGetSecret(t *testing.T) { assert.Len(t, output.Data, 1) assert.Equal(t, "not json", output.Data["/aws/secret/testing"]) }) + + t.Run("with multiple keys per secret and secret is json collection", func(t *testing.T) { + mockSSM := &awsAuth.MockSecretManager{ + GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { + assert.Nil(t, input.VersionId) + assert.Nil(t, input.VersionStage) + secret := `[{"key1":"value1"},{"key2":"value2"}]` + + return &secretsmanager.GetSecretValueOutput{ + Name: input.SecretId, + SecretString: &secret, + }, nil + }, + } + + secret := awsAuth.SecretManagerClients{ + Manager: mockSSM, + } + + mockedClients := awsAuth.Clients{ + Secret: &secret, + } + mockAuthProvider := &awsAuth.StaticAuth{} + mockAuthProvider.WithMockClients(&mockedClients) + s := smSecretStore{ + authProvider: mockAuthProvider, + multipleKeyValuesPerSecret: true, + } + + req := secretstores.GetSecretRequest{ + Name: "/aws/secret/testing", + Metadata: map[string]string{}, + } + output, e := s.GetSecret(t.Context(), req) + require.NoError(t, e) + assert.Len(t, output.Data, 1) + assert.JSONEq(t, `[{"key1":"value1"},{"key2":"value2"}]`, output.Data["/aws/secret/testing"]) + }) }) t.Run("unsuccessfully retrieve secret", func(t *testing.T) { From 3eb02fbd15e049039f3eca5dd113456d7a11dd1e Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 24 Jun 2025 09:05:43 -0700 Subject: [PATCH 10/12] addressing linting errors Signed-off-by: Patrick Assuied --- common/authentication/aws/x509.go | 2 +- common/authentication/aws/x509_test.go | 2 +- secretstores/aws/secretmanager/secretmanager_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/authentication/aws/x509.go b/common/authentication/aws/x509.go index 6556ece74c..566e64c389 100644 --- a/common/authentication/aws/x509.go +++ b/common/authentication/aws/x509.go @@ -159,7 +159,7 @@ func (a *x509) Close() error { func (a *x509) getCertPEM(ctx context.Context) error { // retrieve svid from spiffe context - svid, ok := spiffecontext.From(ctx) + svid, ok := spiffecontext.X509From(ctx) if !ok { return errors.New("no SVID found in context") } diff --git a/common/authentication/aws/x509_test.go b/common/authentication/aws/x509_test.go index 1c1985f364..5b6b06a196 100644 --- a/common/authentication/aws/x509_test.go +++ b/common/authentication/aws/x509_test.go @@ -117,7 +117,7 @@ func TestGetX509Client(t *testing.T) { require.NoError(t, err) // inject the SVID source into the context - ctx = spiffecontext.With(ctx, s) + ctx = spiffecontext.WithX509(ctx, s.X509SVIDSource()) session, err := mockAWS.createOrRefreshSession(ctx) require.NoError(t, err) diff --git a/secretstores/aws/secretmanager/secretmanager_test.go b/secretstores/aws/secretmanager/secretmanager_test.go index 9e050d2168..01fc3dbac1 100644 --- a/secretstores/aws/secretmanager/secretmanager_test.go +++ b/secretstores/aws/secretmanager/secretmanager_test.go @@ -285,7 +285,7 @@ func TestGetSecret(t *testing.T) { GetSecretValueFn: func(ctx context.Context, input *secretsmanager.GetSecretValueInput, option ...request.Option) (*secretsmanager.GetSecretValueOutput, error) { assert.Nil(t, input.VersionId) assert.Nil(t, input.VersionStage) - secret := `[{"key1":"value1"},{"key2":"value2"}]` + secret := `[{"key1":"value1"},{"key2":"value2"}]` // #nosec G101: This is a mock secret used for testing purposes. return &secretsmanager.GetSecretValueOutput{ Name: input.SecretId, From 40e099f022968bfbd9be9ec9b3245e1c01866e64 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 24 Jun 2025 15:13:08 -0700 Subject: [PATCH 11/12] PR feedback Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 4660c5d2dc..187116eaef 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -99,8 +99,7 @@ func (s *smSecretStore) formatSecret(output *secretsmanager.GetSecretValueOutput if output.Name != nil && output.SecretString != nil { if s.multipleKeyValuesPerSecret { data := map[string]any{} - err := json.Unmarshal([]byte(*output.SecretString), &data) - if err == nil { + if err := json.Unmarshal([]byte(*output.SecretString), &data); err == nil { // In case of a nested JSON value, we need to stringify it result = convertMapAnyToString(data) } else { From b5f26b18f8e1888949f8b9591a471e1211a68fd2 Mon Sep 17 00:00:00 2001 From: Patrick Assuied Date: Tue, 24 Jun 2025 15:20:18 -0700 Subject: [PATCH 12/12] PR feedback #2 Signed-off-by: Patrick Assuied --- secretstores/aws/secretmanager/secretmanager.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/secretstores/aws/secretmanager/secretmanager.go b/secretstores/aws/secretmanager/secretmanager.go index 187116eaef..2f2cd63cd3 100644 --- a/secretstores/aws/secretmanager/secretmanager.go +++ b/secretstores/aws/secretmanager/secretmanager.go @@ -99,11 +99,11 @@ func (s *smSecretStore) formatSecret(output *secretsmanager.GetSecretValueOutput if output.Name != nil && output.SecretString != nil { if s.multipleKeyValuesPerSecret { data := map[string]any{} - if err := json.Unmarshal([]byte(*output.SecretString), &data); err == nil { + if err := json.Unmarshal([]byte(*output.SecretString), &data); err != nil { + result[*output.Name] = *output.SecretString + } else { // In case of a nested JSON value, we need to stringify it result = convertMapAnyToString(data) - } else { - result[*output.Name] = *output.SecretString } } else { result[*output.Name] = *output.SecretString