From 29b76b54caf5132a4058b75879631f4a9a2aee7a Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Wed, 7 Jun 2023 16:41:11 -0700 Subject: [PATCH 01/17] readme updated --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index bda328997a2..f4657d2c723 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ The AWS Copilot CLI is a tool for developers to build, release and operate produ on AWS App Runner or Amazon ECS on AWS Fargate. Use Copilot to: + * Deploy production-ready, scalable services on AWS from a Dockerfile in one command. * Add databases or inject secrets to your services. * Grow from one microservice to a collection of related microservices in an application. From 042c746b949eb9adb199e5fa6ef30e80c7754b37 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Wed, 7 Jun 2023 16:57:01 -0700 Subject: [PATCH 02/17] readme updated --- README.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/README.md b/README.md index f4657d2c723..931dbbcf9b3 100644 --- a/README.md +++ b/README.md @@ -11,12 +11,7 @@ on AWS App Runner or Amazon ECS on AWS Fargate. Use Copilot to: -* Deploy production-ready, scalable services on AWS from a Dockerfile in one command. -* Add databases or inject secrets to your services. -* Grow from one microservice to a collection of related microservices in an application. -* Set up test and production environments, across regions and accounts. -* Set up CI/CD pipelines to release your services to your environments. -* Monitor and debug your services from your terminal. +

init From ed95e3fa0fa4467b0cf4433fc80e1743a93d7385 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Wed, 7 Jun 2023 17:00:09 -0700 Subject: [PATCH 03/17] original readme --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 931dbbcf9b3..bda328997a2 100644 --- a/README.md +++ b/README.md @@ -10,8 +10,12 @@ The AWS Copilot CLI is a tool for developers to build, release and operate produ on AWS App Runner or Amazon ECS on AWS Fargate. Use Copilot to: - - +* Deploy production-ready, scalable services on AWS from a Dockerfile in one command. +* Add databases or inject secrets to your services. +* Grow from one microservice to a collection of related microservices in an application. +* Set up test and production environments, across regions and accounts. +* Set up CI/CD pipelines to release your services to your environments. +* Monitor and debug your services from your terminal.

init From f455731fa616f8e679ed174e82067c7b067bb27e Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Thu, 20 Jul 2023 23:54:38 -0700 Subject: [PATCH 04/17] decryting secrets from ssm and secrets manager --- .../mocks/mock_secretsmanager.go | 15 +++ .../pkg/aws/secretsmanager/secretsmanager.go | 13 +++ internal/pkg/aws/ssm/mocks/mock_ssm.go | 15 +++ internal/pkg/aws/ssm/ssm.go | 14 +++ internal/pkg/cli/interfaces.go | 5 + internal/pkg/cli/local_run.go | 63 +++++++++++-- internal/pkg/cli/mocks/mock_interfaces.go | 91 +++++++++++++++++++ internal/pkg/ecs/ecs.go | 36 +++++++- 8 files changed, 241 insertions(+), 11 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/mocks/mock_secretsmanager.go b/internal/pkg/aws/secretsmanager/mocks/mock_secretsmanager.go index 86a731505eb..c1e09ba1cb3 100644 --- a/internal/pkg/aws/secretsmanager/mocks/mock_secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/mocks/mock_secretsmanager.go @@ -78,3 +78,18 @@ func (mr *MockapiMockRecorder) DescribeSecret(input interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSecret", reflect.TypeOf((*Mockapi)(nil).DescribeSecret), input) } + +// GetSecretValue mocks base method. +func (m *Mockapi) GetSecretValue(input *secretsmanager.GetSecretValueInput) (*secretsmanager.GetSecretValueOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecretValue", input) + ret0, _ := ret[0].(*secretsmanager.GetSecretValueOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretValue indicates an expected call of GetSecretValue. +func (mr *MockapiMockRecorder) GetSecretValue(input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*Mockapi)(nil).GetSecretValue), input) +} diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index 7cb95ba59fd..5b470140055 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -19,6 +19,7 @@ type api interface { CreateSecret(*secretsmanager.CreateSecretInput) (*secretsmanager.CreateSecretOutput, error) DeleteSecret(*secretsmanager.DeleteSecretInput) (*secretsmanager.DeleteSecretOutput, error) DescribeSecret(input *secretsmanager.DescribeSecretInput) (*secretsmanager.DescribeSecretOutput, error) + GetSecretValue(input *secretsmanager.GetSecretValueInput) (*secretsmanager.GetSecretValueOutput, error) } // SecretsManager wraps the AWS SecretManager client. @@ -114,6 +115,18 @@ func (s *SecretsManager) DescribeSecret(secretName string) (*DescribeSecretOutpu }, nil } +// GetSecretValue retrieves the value of a secret from AWS Secrets Manager. +// It takes the name of the secret as input and returns the corresponding value as a string. +func (s *SecretsManager) GetSecretValue(name string) (string, error) { + resp, err := s.secretsManager.GetSecretValue(&secretsmanager.GetSecretValueInput{ + SecretId: aws.String(name), + }) + if err != nil { + return "", fmt.Errorf("get secret %s from secrets manager: %w", name, err) + } + return aws.StringValue(resp.SecretString), nil +} + // ErrSecretAlreadyExists occurs if a secret with the same name already exists. type ErrSecretAlreadyExists struct { secretName string diff --git a/internal/pkg/aws/ssm/mocks/mock_ssm.go b/internal/pkg/aws/ssm/mocks/mock_ssm.go index e470c30a1a2..09dda821f97 100644 --- a/internal/pkg/aws/ssm/mocks/mock_ssm.go +++ b/internal/pkg/aws/ssm/mocks/mock_ssm.go @@ -49,6 +49,21 @@ func (mr *MockapiMockRecorder) AddTagsToResource(input interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddTagsToResource", reflect.TypeOf((*Mockapi)(nil).AddTagsToResource), input) } +// GetParameter mocks base method. +func (m *Mockapi) GetParameter(input *ssm.GetParameterInput) (*ssm.GetParameterOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetParameter", input) + ret0, _ := ret[0].(*ssm.GetParameterOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetParameter indicates an expected call of GetParameter. +func (mr *MockapiMockRecorder) GetParameter(input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParameter", reflect.TypeOf((*Mockapi)(nil).GetParameter), input) +} + // PutParameter mocks base method. func (m *Mockapi) PutParameter(input *ssm.PutParameterInput) (*ssm.PutParameterOutput, error) { m.ctrl.T.Helper() diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index d8096e2d7c2..37e499d7a05 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -19,6 +19,7 @@ import ( type api interface { PutParameter(input *ssm.PutParameterInput) (*ssm.PutParameterOutput, error) AddTagsToResource(input *ssm.AddTagsToResourceInput) (*ssm.AddTagsToResourceOutput, error) + GetParameter(input *ssm.GetParameterInput) (*ssm.GetParameterOutput, error) } // SSM wraps an AWS SSM client. @@ -61,6 +62,19 @@ func (s *SSM) PutSecret(in PutSecretInput) (*PutSecretOutput, error) { return nil, err } +// GetSecretValue retrieves the value of a parameter from AWS Systems Manager Parameter Store. +// It takes the name of the parameter as input and returns the corresponding value as a string. +func (s *SSM) GetSecretValue(name string) (string, error) { + resp, err := s.client.GetParameter(&ssm.GetParameterInput{ + Name: aws.String(name), + WithDecryption: aws.Bool(true), + }) + if err != nil { + return "", fmt.Errorf("get parameter %s from SSM with decryption: %w", name, err) + } + return aws.StringValue(resp.Parameter.Value), nil +} + func (s *SSM) createSecret(in PutSecretInput) (*PutSecretOutput, error) { // Create a secret while adding the tags in a single call instead of separate calls to `PutParameter` and // `AddTagsToResource` so that there won't be a case where the parameter is created while the tags are not added. diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index c1c33fbc4f1..023e9496a25 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -157,6 +157,11 @@ type deployedEnvironmentLister interface { type secretsManager interface { secretCreator secretDeleter + secretGetter +} + +type secretGetter interface { + GetSecretValue(secretName string) (string, error) } type secretCreator interface { diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 9fc9129bf00..0c18695ab04 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -5,13 +5,18 @@ package cli import ( "fmt" + "strings" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ssm" + "github.com/aws/aws-sdk-go/aws/session" + awsssm "github.com/aws/aws-sdk-go/service/ssm" + awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/identity" + "github.com/aws/copilot-cli/internal/pkg/aws/secretsmanager" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/deploy" + "github.com/aws/copilot-cli/internal/pkg/ecs" "github.com/aws/copilot-cli/internal/pkg/term/prompt" "github.com/aws/copilot-cli/internal/pkg/term/selector" "github.com/spf13/cobra" @@ -19,6 +24,11 @@ import ( const workloadAskPrompt = "Which workload would you like to run locally?" +type ecsLocalClient interface { + TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) + DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret, secretGetter *session.Session) ([]ecs.EnvVar, error) +} + type localRunVars struct { wkldName string wkldType string @@ -29,8 +39,11 @@ type localRunVars struct { type localRunOpts struct { localRunVars - sel deploySelector - store store + sel deploySelector + ecsLocalClient ecsLocalClient + secretsManager secretsManager + sess *session.Session + store store } func newLocalRunOpts(vars localRunVars) (*localRunOpts, error) { @@ -40,15 +53,18 @@ func newLocalRunOpts(vars localRunVars) (*localRunOpts, error) { return nil, err } - store := config.NewSSMStore(identity.New(defaultSess), ssm.New(defaultSess), aws.StringValue(defaultSess.Config.Region)) + store := config.NewSSMStore(identity.New(defaultSess), awsssm.New(defaultSess), aws.StringValue(defaultSess.Config.Region)) deployStore, err := deploy.NewStore(sessProvider, store) if err != nil { return nil, err } opts := &localRunOpts{ - localRunVars: vars, - sel: selector.NewDeploySelect(prompt.New(), store, deployStore), - store: store, + localRunVars: vars, + sel: selector.NewDeploySelect(prompt.New(), store, deployStore), + store: store, + secretsManager: secretsmanager.New(defaultSess), + ecsLocalClient: ecs.New(defaultSess), + sess: defaultSess, } return opts, nil } @@ -94,11 +110,42 @@ func (o *localRunOpts) validateAndAskWkldEnvName() error { // Execute builds and runs the workload images locally. func (o *localRunOpts) Execute() error { - //TODO(varun359): Get build information from the manifest and task definition for workloads + taskDef, err := o.ecsLocalClient.TaskDefinition(o.appName, o.envName, o.wkldName) + if err != nil { + return fmt.Errorf("get task definition: %w", err) + } + + secrets := taskDef.Secrets() + _, err = o.ecsLocalClient.DecryptedSSMSecrets(secrets, o.sess) + if err != nil { + return err + } + + _, err = o.decryptedSecretManagerSecrets(secrets, o.sess) + if err != nil { + return err + } return nil } +func (o *localRunOpts) decryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret, awsSession *session.Session) ([]ecs.EnvVar, error) { + var secretManagerSecrets []ecs.EnvVar + for _, secret := range secrets { + if strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { + secretValue, err := o.secretsManager.GetSecretValue(secret.ValueFrom) + if err != nil { + return nil, err + } + secretManagerSecrets = append(secretManagerSecrets, ecs.EnvVar{ + Name: secret.Name, + Value: secretValue, + }) + } + } + return secretManagerSecrets, nil +} + // BuildLocalRunCmd builds the command for running a workload locally func BuildLocalRunCmd() *cobra.Command { vars := localRunVars{} diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 9f4fcc40d1b..3b35c0b8044 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1465,6 +1465,59 @@ func (mr *MocksecretsManagerMockRecorder) DescribeSecret(secretName interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSecret", reflect.TypeOf((*MocksecretsManager)(nil).DescribeSecret), secretName) } +// GetSecretValue mocks base method. +func (m *MocksecretsManager) GetSecretValue(secretName string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecretValue", secretName) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretValue indicates an expected call of GetSecretValue. +func (mr *MocksecretsManagerMockRecorder) GetSecretValue(secretName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretsManager)(nil).GetSecretValue), secretName) +} + +// MocksecretGetter is a mock of secretGetter interface. +type MocksecretGetter struct { + ctrl *gomock.Controller + recorder *MocksecretGetterMockRecorder +} + +// MocksecretGetterMockRecorder is the mock recorder for MocksecretGetter. +type MocksecretGetterMockRecorder struct { + mock *MocksecretGetter +} + +// NewMocksecretGetter creates a new mock instance. +func NewMocksecretGetter(ctrl *gomock.Controller) *MocksecretGetter { + mock := &MocksecretGetter{ctrl: ctrl} + mock.recorder = &MocksecretGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocksecretGetter) EXPECT() *MocksecretGetterMockRecorder { + return m.recorder +} + +// GetSecretValue mocks base method. +func (m *MocksecretGetter) GetSecretValue(secretName string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecretValue", secretName) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretValue indicates an expected call of GetSecretValue. +func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) +} + // MocksecretCreator is a mock of secretCreator interface. type MocksecretCreator struct { ctrl *gomock.Controller @@ -7257,6 +7310,44 @@ func (mr *MocksecretPutterMockRecorder) PutSecret(in interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PutSecret", reflect.TypeOf((*MocksecretPutter)(nil).PutSecret), in) } +// MocksecretDecrypter is a mock of secretDecrypter interface. +type MocksecretDecrypter struct { + ctrl *gomock.Controller + recorder *MocksecretDecrypterMockRecorder +} + +// MocksecretDecrypterMockRecorder is the mock recorder for MocksecretDecrypter. +type MocksecretDecrypterMockRecorder struct { + mock *MocksecretDecrypter +} + +// NewMocksecretDecrypter creates a new mock instance. +func NewMocksecretDecrypter(ctrl *gomock.Controller) *MocksecretDecrypter { + mock := &MocksecretDecrypter{ctrl: ctrl} + mock.recorder = &MocksecretDecrypterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocksecretDecrypter) EXPECT() *MocksecretDecrypterMockRecorder { + return m.recorder +} + +// GetSecretValue mocks base method. +func (m *MocksecretDecrypter) GetSecretValue(s string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecretValue", s) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretValue indicates an expected call of GetSecretValue. +func (mr *MocksecretDecrypterMockRecorder) GetSecretValue(s interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretDecrypter)(nil).GetSecretValue), s) +} + // MockservicePauser is a mock of servicePauser interface. type MockservicePauser struct { ctrl *gomock.Controller diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 598a5be5a1d..ecf2e46151e 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -11,13 +11,14 @@ import ( "strings" "time" - "github.com/aws/aws-sdk-go/aws/arn" - "github.com/aws/copilot-cli/internal/pkg/aws/stepfunctions" - "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/resourcegroups" + "github.com/aws/copilot-cli/internal/pkg/aws/ssm" + "github.com/aws/copilot-cli/internal/pkg/aws/stepfunctions" "github.com/aws/copilot-cli/internal/pkg/deploy" ) @@ -52,6 +53,15 @@ type ecsClient interface { type stepFunctionsClient interface { StateMachineDefinition(stateMachineARN string) (string, error) } +type secretGetter interface { + GetSecretValue(secretName string) (string, error) +} + +// EnvVar contains values of the environmental variables +type EnvVar struct { + Name string + Value string +} // ServiceDesc contains the description of an ECS service. type ServiceDesc struct { @@ -64,6 +74,7 @@ type ServiceDesc struct { // Client retrieves Copilot information from ECS endpoint. type Client struct { rgGetter resourceGetter + secretGetter secretGetter ecsClient ecsClient StepFuncClient stepFunctionsClient } @@ -73,6 +84,7 @@ func New(sess *session.Session) *Client { return &Client{ rgGetter: resourcegroups.New(sess), ecsClient: ecs.New(sess), + secretGetter: ssm.New(sess), StepFuncClient: stepfunctions.New(sess), } } @@ -232,6 +244,24 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { return c.ecsClient.StopTasks(taskIDs, ecs.WithStopTaskReason(taskStopReason)) } +// DecryptedSSMSecrets returns the decrypted parameters from the SSM store. +func (c Client) DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret, secretGetter *session.Session) ([]EnvVar, error) { + var ssmSecrets []EnvVar + for _, secret := range secrets { + if !strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { + secretValue, err := c.secretGetter.GetSecretValue(secret.ValueFrom) + if err != nil { + return nil, err + } + ssmSecrets = append(ssmSecrets, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) + } + } + return ssmSecrets, nil +} + // TaskDefinition returns the task definition of the service. func (c Client) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { taskDefName := fmt.Sprintf("%s-%s-%s", app, env, svc) From 449617ed44c2ccb87db0b4fa131b96c5a5b0008a Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Thu, 20 Jul 2023 23:59:52 -0700 Subject: [PATCH 05/17] removed extra dependency --- internal/pkg/cli/local_run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 0c18695ab04..7be19af6ffd 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -9,7 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" - awsssm "github.com/aws/aws-sdk-go/service/ssm" + "github.com/aws/aws-sdk-go/service/ssm" awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/identity" "github.com/aws/copilot-cli/internal/pkg/aws/secretsmanager" @@ -53,7 +53,7 @@ func newLocalRunOpts(vars localRunVars) (*localRunOpts, error) { return nil, err } - store := config.NewSSMStore(identity.New(defaultSess), awsssm.New(defaultSess), aws.StringValue(defaultSess.Config.Region)) + store := config.NewSSMStore(identity.New(defaultSess), ssm.New(defaultSess), aws.StringValue(defaultSess.Config.Region)) deployStore, err := deploy.NewStore(sessProvider, store) if err != nil { return nil, err From 8b7927f85ffa5829fdd41ced48b8ba003fc61819 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 10:14:56 -0700 Subject: [PATCH 06/17] mocks generated --- internal/pkg/cli/mocks/mock_interfaces.go | 38 ----------------------- internal/pkg/ecs/mocks/mock_ecs.go | 38 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 3b35c0b8044..4b986489761 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -7310,44 +7310,6 @@ func (mr *MocksecretPutterMockRecorder) PutSecret(in interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PutSecret", reflect.TypeOf((*MocksecretPutter)(nil).PutSecret), in) } -// MocksecretDecrypter is a mock of secretDecrypter interface. -type MocksecretDecrypter struct { - ctrl *gomock.Controller - recorder *MocksecretDecrypterMockRecorder -} - -// MocksecretDecrypterMockRecorder is the mock recorder for MocksecretDecrypter. -type MocksecretDecrypterMockRecorder struct { - mock *MocksecretDecrypter -} - -// NewMocksecretDecrypter creates a new mock instance. -func NewMocksecretDecrypter(ctrl *gomock.Controller) *MocksecretDecrypter { - mock := &MocksecretDecrypter{ctrl: ctrl} - mock.recorder = &MocksecretDecrypterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MocksecretDecrypter) EXPECT() *MocksecretDecrypterMockRecorder { - return m.recorder -} - -// GetSecretValue mocks base method. -func (m *MocksecretDecrypter) GetSecretValue(s string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSecretValue", s) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSecretValue indicates an expected call of GetSecretValue. -func (mr *MocksecretDecrypterMockRecorder) GetSecretValue(s interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretDecrypter)(nil).GetSecretValue), s) -} - // MockservicePauser is a mock of servicePauser interface. type MockservicePauser struct { ctrl *gomock.Controller diff --git a/internal/pkg/ecs/mocks/mock_ecs.go b/internal/pkg/ecs/mocks/mock_ecs.go index 848f1989aca..a5b8f0df55f 100644 --- a/internal/pkg/ecs/mocks/mock_ecs.go +++ b/internal/pkg/ecs/mocks/mock_ecs.go @@ -302,3 +302,41 @@ func (mr *MockstepFunctionsClientMockRecorder) StateMachineDefinition(stateMachi mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StateMachineDefinition", reflect.TypeOf((*MockstepFunctionsClient)(nil).StateMachineDefinition), stateMachineARN) } + +// MocksecretGetter is a mock of secretGetter interface. +type MocksecretGetter struct { + ctrl *gomock.Controller + recorder *MocksecretGetterMockRecorder +} + +// MocksecretGetterMockRecorder is the mock recorder for MocksecretGetter. +type MocksecretGetterMockRecorder struct { + mock *MocksecretGetter +} + +// NewMocksecretGetter creates a new mock instance. +func NewMocksecretGetter(ctrl *gomock.Controller) *MocksecretGetter { + mock := &MocksecretGetter{ctrl: ctrl} + mock.recorder = &MocksecretGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocksecretGetter) EXPECT() *MocksecretGetterMockRecorder { + return m.recorder +} + +// GetSecretValue mocks base method. +func (m *MocksecretGetter) GetSecretValue(secretName string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecretValue", secretName) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretValue indicates an expected call of GetSecretValue. +func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) +} From 10b7cd3c06f8813e2ff77c1d01b241d0056afbda Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 14:38:37 -0700 Subject: [PATCH 07/17] refactored and updated decrypting secrets --- .../pkg/aws/secretsmanager/secretsmanager.go | 15 ++++++ internal/pkg/aws/ssm/ssm.go | 15 ++++++ internal/pkg/cli/interfaces.go | 5 -- internal/pkg/cli/local_run.go | 44 ++++++++------- internal/pkg/cli/mocks/mock_interfaces.go | 53 ------------------- internal/pkg/ecs/ecs.go | 41 +++++++++++--- internal/pkg/ecs/mocks/mock_ecs.go | 15 ++++++ 7 files changed, 101 insertions(+), 87 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index 5b470140055..57b2dd8ef5d 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -8,6 +8,7 @@ import ( "fmt" "time" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/aws" @@ -15,6 +16,8 @@ import ( "github.com/aws/aws-sdk-go/service/secretsmanager" ) +const namespace = "secretsmanager" + type api interface { CreateSecret(*secretsmanager.CreateSecretInput) (*secretsmanager.CreateSecretOutput, error) DeleteSecret(*secretsmanager.DeleteSecretInput) (*secretsmanager.DeleteSecretOutput, error) @@ -127,6 +130,18 @@ func (s *SecretsManager) GetSecretValue(name string) (string, error) { return aws.StringValue(resp.SecretString), nil } +// IsService returns true if the given ARN is secrets manager. +func (s *SecretsManager) IsService(name string) (bool, error) { + parseArn, err := arn.Parse(name) + if err != nil { + return false, fmt.Errorf("parse secrets manager arn: %w", err) + } + if parseArn.Service == namespace { + return true, nil + } + return false, nil +} + // ErrSecretAlreadyExists occurs if a secret with the same name already exists. type ErrSecretAlreadyExists struct { secretName string diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index 37e499d7a05..28c27b49bc5 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -9,6 +9,7 @@ import ( "fmt" "sort" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws" @@ -16,6 +17,8 @@ import ( "github.com/aws/aws-sdk-go/service/ssm" ) +const namespace = "ssm" + type api interface { PutParameter(input *ssm.PutParameterInput) (*ssm.PutParameterOutput, error) AddTagsToResource(input *ssm.AddTagsToResourceInput) (*ssm.AddTagsToResourceOutput, error) @@ -62,6 +65,18 @@ func (s *SSM) PutSecret(in PutSecretInput) (*PutSecretOutput, error) { return nil, err } +// IsService returns true if the given ARN is ssm. +func (s *SSM) IsService(name string) (bool, error) { + parseArn, err := arn.Parse(name) + if err != nil { + return false, fmt.Errorf("parse ssm arn: %w", err) + } + if parseArn.Service == namespace { + return true, nil + } + return false, nil +} + // GetSecretValue retrieves the value of a parameter from AWS Systems Manager Parameter Store. // It takes the name of the parameter as input and returns the corresponding value as a string. func (s *SSM) GetSecretValue(name string) (string, error) { diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 023e9496a25..c1c33fbc4f1 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -157,11 +157,6 @@ type deployedEnvironmentLister interface { type secretsManager interface { secretCreator secretDeleter - secretGetter -} - -type secretGetter interface { - GetSecretValue(secretName string) (string, error) } type secretCreator interface { diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 7be19af6ffd..4dcacc6bb86 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -5,14 +5,12 @@ package cli import ( "fmt" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ssm" awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/identity" - "github.com/aws/copilot-cli/internal/pkg/aws/secretsmanager" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/deploy" @@ -26,7 +24,8 @@ const workloadAskPrompt = "Which workload would you like to run locally?" type ecsLocalClient interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) - DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret, secretGetter *session.Session) ([]ecs.EnvVar, error) + DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) + DecryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) } type localRunVars struct { @@ -41,7 +40,6 @@ type localRunOpts struct { sel deploySelector ecsLocalClient ecsLocalClient - secretsManager secretsManager sess *session.Session store store } @@ -62,7 +60,6 @@ func newLocalRunOpts(vars localRunVars) (*localRunOpts, error) { localRunVars: vars, sel: selector.NewDeploySelect(prompt.New(), store, deployStore), store: store, - secretsManager: secretsmanager.New(defaultSess), ecsLocalClient: ecs.New(defaultSess), sess: defaultSess, } @@ -116,12 +113,12 @@ func (o *localRunOpts) Execute() error { } secrets := taskDef.Secrets() - _, err = o.ecsLocalClient.DecryptedSSMSecrets(secrets, o.sess) + _, err = o.ecsLocalClient.DecryptedSSMSecrets(secrets) if err != nil { return err } - _, err = o.decryptedSecretManagerSecrets(secrets, o.sess) + _, err = o.ecsLocalClient.DecryptedSecretManagerSecrets(secrets) if err != nil { return err } @@ -129,22 +126,23 @@ func (o *localRunOpts) Execute() error { return nil } -func (o *localRunOpts) decryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret, awsSession *session.Session) ([]ecs.EnvVar, error) { - var secretManagerSecrets []ecs.EnvVar - for _, secret := range secrets { - if strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { - secretValue, err := o.secretsManager.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err - } - secretManagerSecrets = append(secretManagerSecrets, ecs.EnvVar{ - Name: secret.Name, - Value: secretValue, - }) - } - } - return secretManagerSecrets, nil -} +// func (o *localRunOpts) decryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret, awsSession *session.Session) ([]ecs.EnvVar, error) { +// var secretManagerSecrets []ecs.EnvVar +// for _, secret := range secrets { + +// if strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { +// secretValue, err := o.secretsManager.GetSecretValue(secret.ValueFrom) +// if err != nil { +// return nil, err +// } +// secretManagerSecrets = append(secretManagerSecrets, ecs.EnvVar{ +// Name: secret.Name, +// Value: secretValue, +// }) +// } +// } +// return secretManagerSecrets, nil +// } // BuildLocalRunCmd builds the command for running a workload locally func BuildLocalRunCmd() *cobra.Command { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 4b986489761..9f4fcc40d1b 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1465,59 +1465,6 @@ func (mr *MocksecretsManagerMockRecorder) DescribeSecret(secretName interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSecret", reflect.TypeOf((*MocksecretsManager)(nil).DescribeSecret), secretName) } -// GetSecretValue mocks base method. -func (m *MocksecretsManager) GetSecretValue(secretName string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSecretValue", secretName) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSecretValue indicates an expected call of GetSecretValue. -func (mr *MocksecretsManagerMockRecorder) GetSecretValue(secretName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretsManager)(nil).GetSecretValue), secretName) -} - -// MocksecretGetter is a mock of secretGetter interface. -type MocksecretGetter struct { - ctrl *gomock.Controller - recorder *MocksecretGetterMockRecorder -} - -// MocksecretGetterMockRecorder is the mock recorder for MocksecretGetter. -type MocksecretGetterMockRecorder struct { - mock *MocksecretGetter -} - -// NewMocksecretGetter creates a new mock instance. -func NewMocksecretGetter(ctrl *gomock.Controller) *MocksecretGetter { - mock := &MocksecretGetter{ctrl: ctrl} - mock.recorder = &MocksecretGetterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MocksecretGetter) EXPECT() *MocksecretGetterMockRecorder { - return m.recorder -} - -// GetSecretValue mocks base method. -func (m *MocksecretGetter) GetSecretValue(secretName string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSecretValue", secretName) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSecretValue indicates an expected call of GetSecretValue. -func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) -} - // MocksecretCreator is a mock of secretCreator interface. type MocksecretCreator struct { ctrl *gomock.Controller diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index ecf2e46151e..06fc5daa5ba 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -15,8 +15,8 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/resourcegroups" + "github.com/aws/copilot-cli/internal/pkg/aws/secretsmanager" "github.com/aws/copilot-cli/internal/pkg/aws/ssm" "github.com/aws/copilot-cli/internal/pkg/aws/stepfunctions" "github.com/aws/copilot-cli/internal/pkg/deploy" @@ -55,6 +55,7 @@ type stepFunctionsClient interface { } type secretGetter interface { GetSecretValue(secretName string) (string, error) + IsService(secretName string) (bool, error) } // EnvVar contains values of the environmental variables @@ -75,6 +76,7 @@ type ServiceDesc struct { type Client struct { rgGetter resourceGetter secretGetter secretGetter + secretManger secretGetter ecsClient ecsClient StepFuncClient stepFunctionsClient } @@ -85,6 +87,7 @@ func New(sess *session.Session) *Client { rgGetter: resourcegroups.New(sess), ecsClient: ecs.New(sess), secretGetter: ssm.New(sess), + secretManger: secretsmanager.New(sess), StepFuncClient: stepfunctions.New(sess), } } @@ -245,21 +248,47 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { } // DecryptedSSMSecrets returns the decrypted parameters from the SSM store. -func (c Client) DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret, secretGetter *session.Session) ([]EnvVar, error) { +func (c Client) DecryptedSSMSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { var ssmSecrets []EnvVar for _, secret := range secrets { - if !strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { - secretValue, err := c.secretGetter.GetSecretValue(secret.ValueFrom) + if arn.IsARN(secret.ValueFrom) { + isSSM, err := c.secretGetter.IsService(secret.ValueFrom) + if err != nil || !isSSM { + continue + } + } + secretValue, err := c.secretGetter.GetSecretValue(secret.ValueFrom) + if err != nil { + return nil, err + } + ssmSecrets = append(ssmSecrets, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) + } + return ssmSecrets, nil +} + +// DecryptedSecretManagerSecrets returns the decrypted parameters from the secretsmanager. +func (c Client) DecryptedSecretManagerSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { + var secretManagerSecrets []EnvVar + for _, secret := range secrets { + isSecretsManager, err := c.secretManger.IsService(secret.ValueFrom) + if err != nil { + continue + } + if isSecretsManager { + secretValue, err := c.secretManger.GetSecretValue(secret.ValueFrom) if err != nil { return nil, err } - ssmSecrets = append(ssmSecrets, EnvVar{ + secretManagerSecrets = append(secretManagerSecrets, EnvVar{ Name: secret.Name, Value: secretValue, }) } } - return ssmSecrets, nil + return secretManagerSecrets, nil } // TaskDefinition returns the task definition of the service. diff --git a/internal/pkg/ecs/mocks/mock_ecs.go b/internal/pkg/ecs/mocks/mock_ecs.go index a5b8f0df55f..217c490faba 100644 --- a/internal/pkg/ecs/mocks/mock_ecs.go +++ b/internal/pkg/ecs/mocks/mock_ecs.go @@ -340,3 +340,18 @@ func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) * mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) } + +// IsService mocks base method. +func (m *MocksecretGetter) IsService(secretName string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsService", secretName) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsService indicates an expected call of IsService. +func (mr *MocksecretGetterMockRecorder) IsService(secretName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsService", reflect.TypeOf((*MocksecretGetter)(nil).IsService), secretName) +} From aa15cd92128f6f7ed1c8256508a3b5f14923bc75 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 14:42:32 -0700 Subject: [PATCH 08/17] removed unnecessary comment --- internal/pkg/cli/local_run.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 4dcacc6bb86..6b4cd320ea2 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -126,24 +126,6 @@ func (o *localRunOpts) Execute() error { return nil } -// func (o *localRunOpts) decryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret, awsSession *session.Session) ([]ecs.EnvVar, error) { -// var secretManagerSecrets []ecs.EnvVar -// for _, secret := range secrets { - -// if strings.HasPrefix(secret.ValueFrom, "arn:aws:secretsmanager:") { -// secretValue, err := o.secretsManager.GetSecretValue(secret.ValueFrom) -// if err != nil { -// return nil, err -// } -// secretManagerSecrets = append(secretManagerSecrets, ecs.EnvVar{ -// Name: secret.Name, -// Value: secretValue, -// }) -// } -// } -// return secretManagerSecrets, nil -// } - // BuildLocalRunCmd builds the command for running a workload locally func BuildLocalRunCmd() *cobra.Command { vars := localRunVars{} From 9b709cb03490658d5fb83e1e6593fddf1fa4c9fd Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 16:26:23 -0700 Subject: [PATCH 09/17] combined both secret methods --- .../pkg/aws/secretsmanager/secretsmanager.go | 7 +-- internal/pkg/aws/ssm/ssm.go | 7 +-- internal/pkg/cli/local_run.go | 13 ++-- internal/pkg/ecs/ecs.go | 60 +++++++++---------- 4 files changed, 37 insertions(+), 50 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index 57b2dd8ef5d..1b1d49c8b54 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -131,15 +131,12 @@ func (s *SecretsManager) GetSecretValue(name string) (string, error) { } // IsService returns true if the given ARN is secrets manager. -func (s *SecretsManager) IsService(name string) (bool, error) { +func (s *SecretsManager) IsServiceARN(name string) (bool, error) { parseArn, err := arn.Parse(name) if err != nil { return false, fmt.Errorf("parse secrets manager arn: %w", err) } - if parseArn.Service == namespace { - return true, nil - } - return false, nil + return parseArn.Service == namespace, nil } // ErrSecretAlreadyExists occurs if a secret with the same name already exists. diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index 28c27b49bc5..915561841a4 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -66,15 +66,12 @@ func (s *SSM) PutSecret(in PutSecretInput) (*PutSecretOutput, error) { } // IsService returns true if the given ARN is ssm. -func (s *SSM) IsService(name string) (bool, error) { +func (s *SSM) IsServiceARN(name string) (bool, error) { parseArn, err := arn.Parse(name) if err != nil { return false, fmt.Errorf("parse ssm arn: %w", err) } - if parseArn.Service == namespace { - return true, nil - } - return false, nil + return parseArn.Service == namespace, nil } // GetSecretValue retrieves the value of a parameter from AWS Systems Manager Parameter Store. diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 6b4cd320ea2..1143aae8605 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -24,8 +24,8 @@ const workloadAskPrompt = "Which workload would you like to run locally?" type ecsLocalClient interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) - DecryptedSSMSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) - DecryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) + DecryptedSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) + // DecryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) } type localRunVars struct { @@ -113,14 +113,9 @@ func (o *localRunOpts) Execute() error { } secrets := taskDef.Secrets() - _, err = o.ecsLocalClient.DecryptedSSMSecrets(secrets) + _, err = o.ecsLocalClient.DecryptedSecrets(secrets) if err != nil { - return err - } - - _, err = o.ecsLocalClient.DecryptedSecretManagerSecrets(secrets) - if err != nil { - return err + return fmt.Errorf("get secret values: %w:", err) } return nil diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 06fc5daa5ba..3b7c84d9223 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -55,7 +55,7 @@ type stepFunctionsClient interface { } type secretGetter interface { GetSecretValue(secretName string) (string, error) - IsService(secretName string) (bool, error) + IsServiceARN(secretName string) (bool, error) } // EnvVar contains values of the environmental variables @@ -75,8 +75,8 @@ type ServiceDesc struct { // Client retrieves Copilot information from ECS endpoint. type Client struct { rgGetter resourceGetter - secretGetter secretGetter - secretManger secretGetter + ssm secretGetter + secretManager secretGetter ecsClient ecsClient StepFuncClient stepFunctionsClient } @@ -86,8 +86,8 @@ func New(sess *session.Session) *Client { return &Client{ rgGetter: resourcegroups.New(sess), ecsClient: ecs.New(sess), - secretGetter: ssm.New(sess), - secretManger: secretsmanager.New(sess), + ssm: ssm.New(sess), + secretManager: secretsmanager.New(sess), StepFuncClient: stepfunctions.New(sess), } } @@ -247,17 +247,35 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { return c.ecsClient.StopTasks(taskIDs, ecs.WithStopTaskReason(taskStopReason)) } -// DecryptedSSMSecrets returns the decrypted parameters from the SSM store. -func (c Client) DecryptedSSMSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { +// DecryptedSecrets returns the decrypted parameters from either the SSM store or Secrets Manager. +func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { var ssmSecrets []EnvVar + var secretManagerSecrets []EnvVar + for _, secret := range secrets { if arn.IsARN(secret.ValueFrom) { - isSSM, err := c.secretGetter.IsService(secret.ValueFrom) + isSecretsManager, err := c.secretManager.IsServiceARN(secret.ValueFrom) + if err != nil { + return nil, err + } + if isSecretsManager { + secretValue, err := c.secretManager.GetSecretValue(secret.ValueFrom) + if err != nil { + return nil, err + } + secretManagerSecrets = append(secretManagerSecrets, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) + continue + } + } else { + isSSM, err := c.ssm.IsServiceARN(secret.ValueFrom) if err != nil || !isSSM { continue } } - secretValue, err := c.secretGetter.GetSecretValue(secret.ValueFrom) + secretValue, err := c.ssm.GetSecretValue(secret.ValueFrom) if err != nil { return nil, err } @@ -266,29 +284,9 @@ func (c Client) DecryptedSSMSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, e Value: secretValue, }) } - return ssmSecrets, nil -} -// DecryptedSecretManagerSecrets returns the decrypted parameters from the secretsmanager. -func (c Client) DecryptedSecretManagerSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { - var secretManagerSecrets []EnvVar - for _, secret := range secrets { - isSecretsManager, err := c.secretManger.IsService(secret.ValueFrom) - if err != nil { - continue - } - if isSecretsManager { - secretValue, err := c.secretManger.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err - } - secretManagerSecrets = append(secretManagerSecrets, EnvVar{ - Name: secret.Name, - Value: secretValue, - }) - } - } - return secretManagerSecrets, nil + allSecrets := append(ssmSecrets, secretManagerSecrets...) + return allSecrets, nil } // TaskDefinition returns the task definition of the service. From c6b3905287634195a93a71928d52412a2fdf622c Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 16:29:24 -0700 Subject: [PATCH 10/17] comment edited --- internal/pkg/aws/secretsmanager/secretsmanager.go | 2 +- internal/pkg/aws/ssm/ssm.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index 1b1d49c8b54..e2b15afeece 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -130,7 +130,7 @@ func (s *SecretsManager) GetSecretValue(name string) (string, error) { return aws.StringValue(resp.SecretString), nil } -// IsService returns true if the given ARN is secrets manager. +// IsServiceARN returns true if the given ARN is secrets manager. func (s *SecretsManager) IsServiceARN(name string) (bool, error) { parseArn, err := arn.Parse(name) if err != nil { diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index 915561841a4..6dfac946f97 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -65,7 +65,7 @@ func (s *SSM) PutSecret(in PutSecretInput) (*PutSecretOutput, error) { return nil, err } -// IsService returns true if the given ARN is ssm. +// IsServiceARN returns true if the given ARN is ssm. func (s *SSM) IsServiceARN(name string) (bool, error) { parseArn, err := arn.Parse(name) if err != nil { From 56f4832692154bbcd14e1475a947c08698ece335 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 16:40:47 -0700 Subject: [PATCH 11/17] mocks updated --- internal/pkg/ecs/mocks/mock_ecs.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/ecs/mocks/mock_ecs.go b/internal/pkg/ecs/mocks/mock_ecs.go index 217c490faba..aac59e1c3e0 100644 --- a/internal/pkg/ecs/mocks/mock_ecs.go +++ b/internal/pkg/ecs/mocks/mock_ecs.go @@ -341,17 +341,17 @@ func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) } -// IsService mocks base method. -func (m *MocksecretGetter) IsService(secretName string) (bool, error) { +// IsServiceARN mocks base method. +func (m *MocksecretGetter) IsServiceARN(secretName string) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsService", secretName) + ret := m.ctrl.Call(m, "IsServiceARN", secretName) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } -// IsService indicates an expected call of IsService. -func (mr *MocksecretGetterMockRecorder) IsService(secretName interface{}) *gomock.Call { +// IsServiceARN indicates an expected call of IsServiceARN. +func (mr *MocksecretGetterMockRecorder) IsServiceARN(secretName interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsService", reflect.TypeOf((*MocksecretGetter)(nil).IsService), secretName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsServiceARN", reflect.TypeOf((*MocksecretGetter)(nil).IsServiceARN), secretName) } From 38dbdaed5cb102d65dd64e0cdd75960520d9f527 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 17:12:53 -0700 Subject: [PATCH 12/17] refactored code removing a method --- .../pkg/aws/secretsmanager/secretsmanager.go | 12 +---- internal/pkg/aws/ssm/ssm.go | 12 +---- internal/pkg/ecs/ecs.go | 44 +++++++------------ internal/pkg/ecs/mocks/mock_ecs.go | 15 ------- 4 files changed, 18 insertions(+), 65 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index e2b15afeece..4ee12254527 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -8,7 +8,6 @@ import ( "fmt" "time" - "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/aws" @@ -16,7 +15,7 @@ import ( "github.com/aws/aws-sdk-go/service/secretsmanager" ) -const namespace = "secretsmanager" +const Namespace = "secretsmanager" type api interface { CreateSecret(*secretsmanager.CreateSecretInput) (*secretsmanager.CreateSecretOutput, error) @@ -130,15 +129,6 @@ func (s *SecretsManager) GetSecretValue(name string) (string, error) { return aws.StringValue(resp.SecretString), nil } -// IsServiceARN returns true if the given ARN is secrets manager. -func (s *SecretsManager) IsServiceARN(name string) (bool, error) { - parseArn, err := arn.Parse(name) - if err != nil { - return false, fmt.Errorf("parse secrets manager arn: %w", err) - } - return parseArn.Service == namespace, nil -} - // ErrSecretAlreadyExists occurs if a secret with the same name already exists. type ErrSecretAlreadyExists struct { secretName string diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index 6dfac946f97..ccdaaffac87 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -9,7 +9,6 @@ import ( "fmt" "sort" - "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws" @@ -17,7 +16,7 @@ import ( "github.com/aws/aws-sdk-go/service/ssm" ) -const namespace = "ssm" +const Namespace = "ssm" type api interface { PutParameter(input *ssm.PutParameterInput) (*ssm.PutParameterOutput, error) @@ -65,15 +64,6 @@ func (s *SSM) PutSecret(in PutSecretInput) (*PutSecretOutput, error) { return nil, err } -// IsServiceARN returns true if the given ARN is ssm. -func (s *SSM) IsServiceARN(name string) (bool, error) { - parseArn, err := arn.Parse(name) - if err != nil { - return false, fmt.Errorf("parse ssm arn: %w", err) - } - return parseArn.Service == namespace, nil -} - // GetSecretValue retrieves the value of a parameter from AWS Systems Manager Parameter Store. // It takes the name of the parameter as input and returns the corresponding value as a string. func (s *SSM) GetSecretValue(name string) (string, error) { diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 3b7c84d9223..f6317c3b063 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -55,7 +55,6 @@ type stepFunctionsClient interface { } type secretGetter interface { GetSecretValue(secretName string) (string, error) - IsServiceARN(secretName string) (bool, error) } // EnvVar contains values of the environmental variables @@ -251,40 +250,29 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { var ssmSecrets []EnvVar var secretManagerSecrets []EnvVar - for _, secret := range secrets { - if arn.IsARN(secret.ValueFrom) { - isSecretsManager, err := c.secretManager.IsServiceARN(secret.ValueFrom) + parsed, err := arn.Parse(secret.ValueFrom) + if err != nil || parsed.Service == ssm.Namespace { + secretValue, err := c.ssm.GetSecretValue(secret.ValueFrom) if err != nil { return nil, err } - if isSecretsManager { - secretValue, err := c.secretManager.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err - } - secretManagerSecrets = append(secretManagerSecrets, EnvVar{ - Name: secret.Name, - Value: secretValue, - }) - continue - } - } else { - isSSM, err := c.ssm.IsServiceARN(secret.ValueFrom) - if err != nil || !isSSM { - continue - } + ssmSecrets = append(ssmSecrets, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) } - secretValue, err := c.ssm.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err + if parsed.Service == secretsmanager.Namespace { + secretValue, err := c.secretManager.GetSecretValue(secret.ValueFrom) + if err != nil { + return nil, err + } + secretManagerSecrets = append(secretManagerSecrets, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) } - ssmSecrets = append(ssmSecrets, EnvVar{ - Name: secret.Name, - Value: secretValue, - }) } - allSecrets := append(ssmSecrets, secretManagerSecrets...) return allSecrets, nil } diff --git a/internal/pkg/ecs/mocks/mock_ecs.go b/internal/pkg/ecs/mocks/mock_ecs.go index aac59e1c3e0..a5b8f0df55f 100644 --- a/internal/pkg/ecs/mocks/mock_ecs.go +++ b/internal/pkg/ecs/mocks/mock_ecs.go @@ -340,18 +340,3 @@ func (mr *MocksecretGetterMockRecorder) GetSecretValue(secretName interface{}) * mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretValue", reflect.TypeOf((*MocksecretGetter)(nil).GetSecretValue), secretName) } - -// IsServiceARN mocks base method. -func (m *MocksecretGetter) IsServiceARN(secretName string) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsServiceARN", secretName) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IsServiceARN indicates an expected call of IsServiceARN. -func (mr *MocksecretGetterMockRecorder) IsServiceARN(secretName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsServiceARN", reflect.TypeOf((*MocksecretGetter)(nil).IsServiceARN), secretName) -} From 1f5e77bfd2a408dc21c611ad4a459263aba38d50 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Fri, 21 Jul 2023 17:21:56 -0700 Subject: [PATCH 13/17] Namespace commits added --- internal/pkg/aws/secretsmanager/secretsmanager.go | 1 + internal/pkg/aws/ssm/ssm.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index 4ee12254527..a4c7c6f00ce 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/service/secretsmanager" ) +// Namespace represents the AWS Secrets Manager service namespace. const Namespace = "secretsmanager" type api interface { diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index ccdaaffac87..8ba026aae86 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -16,6 +16,7 @@ import ( "github.com/aws/aws-sdk-go/service/ssm" ) +// Namespace represents the AWS Systems Manager(SSM) service namespace. const Namespace = "ssm" type api interface { From 78cf75260363ae6475d9274e9536ffcf4775243c Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Mon, 24 Jul 2023 11:41:11 -0700 Subject: [PATCH 14/17] test cases added' --- internal/pkg/cli/interfaces.go | 5 + internal/pkg/cli/local_run.go | 9 +- internal/pkg/cli/local_run_test.go | 110 ++++++++++++++++++++++ internal/pkg/cli/mocks/mock_interfaces.go | 53 +++++++++++ 4 files changed, 169 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index c1c33fbc4f1..cd497fbee01 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -181,6 +181,11 @@ type repositoryService interface { imageBuilderPusher } +type ecsLocalClient interface { + TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) + DecryptedSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) +} + type logEventsWriter interface { WriteLogEvents(opts logging.WriteLogEventsOpts) error } diff --git a/internal/pkg/cli/local_run.go b/internal/pkg/cli/local_run.go index 1143aae8605..410b82ebfac 100644 --- a/internal/pkg/cli/local_run.go +++ b/internal/pkg/cli/local_run.go @@ -9,7 +9,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ssm" - awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/identity" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" @@ -22,12 +21,6 @@ import ( const workloadAskPrompt = "Which workload would you like to run locally?" -type ecsLocalClient interface { - TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) - DecryptedSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) - // DecryptedSecretManagerSecrets(secrets []*awsecs.ContainerSecret) ([]ecs.EnvVar, error) -} - type localRunVars struct { wkldName string wkldType string @@ -115,7 +108,7 @@ func (o *localRunOpts) Execute() error { secrets := taskDef.Secrets() _, err = o.ecsLocalClient.DecryptedSecrets(secrets) if err != nil { - return fmt.Errorf("get secret values: %w:", err) + return fmt.Errorf("get secret values: %w", err) } return nil diff --git a/internal/pkg/cli/local_run_test.go b/internal/pkg/cli/local_run_test.go index c8d1f4e0444..425272b6ace 100644 --- a/internal/pkg/cli/local_run_test.go +++ b/internal/pkg/cli/local_run_test.go @@ -8,8 +8,12 @@ import ( "fmt" "testing" + "github.com/aws/aws-sdk-go/aws" + ecsapi "github.com/aws/aws-sdk-go/service/ecs" + awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/cli/mocks" "github.com/aws/copilot-cli/internal/pkg/config" + "github.com/aws/copilot-cli/internal/pkg/ecs" "github.com/aws/copilot-cli/internal/pkg/term/selector" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -187,3 +191,109 @@ func TestLocalRunOpts_Ask(t *testing.T) { }) } } + +type localRunExecuteMocks struct { + ecsLocalClient *mocks.MockecsLocalClient +} + +func TestLocalRunOpts_Execute(t *testing.T) { + const ( + testAppName = "testApp" + testEnvName = "testEnv" + testWkldName = "testWkld" + testWkldType = "testWkldType" + ) + var taskDefinition = &awsecs.TaskDefinition{ + ContainerDefinitions: []*ecsapi.ContainerDefinition{ + { + Name: aws.String("container"), + Environment: []*ecsapi.KeyValuePair{ + { + Name: aws.String("COPILOT_SERVICE_NAME"), + Value: aws.String("testWkld"), + }, + { + Name: aws.String("COPILOT_ENVIRONMENT_NAME"), + Value: aws.String("testEnv"), + }, + }, + }, + }, + } + testCases := map[string]struct { + inputAppName string + inputEnvName string + inputWkldName string + + setupMocks func(m *localRunExecuteMocks) + wantedWkldName string + wantedEnvName string + wantedWkldType string + wantedError error + }{ + "error getting the task Definition": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + setupMocks: func(m *localRunExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(nil, testError) + }, + wantedError: fmt.Errorf("get task definition: %w", testError), + }, + "error decryting secrets from task definition": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + setupMocks: func(m *localRunExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDefinition, nil) + m.ecsLocalClient.EXPECT().DecryptedSecrets(gomock.Any()).Return(nil, testError) + }, + wantedError: fmt.Errorf("get secret values: %w", testError), + }, + "success decrypting secrets from task definition": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + setupMocks: func(m *localRunExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDefinition, nil) + m.ecsLocalClient.EXPECT().DecryptedSecrets(gomock.Any()).Return([]ecs.EnvVar{{ + Name: "my-secret", + Value: "Password123", + }, { + Name: "secret2", + Value: "admin123", + }, + }, nil) + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + m := &localRunExecuteMocks{ + ecsLocalClient: mocks.NewMockecsLocalClient(ctrl), + } + tc.setupMocks(m) + opts := localRunOpts{ + localRunVars: localRunVars{ + appName: tc.inputAppName, + wkldName: tc.inputWkldName, + envName: tc.inputEnvName, + }, + ecsLocalClient: m.ecsLocalClient, + } + + // WHEN + err := opts.Execute() + + // THEN + if tc.wantedError == nil { + require.NoError(t, err) + } else { + require.EqualError(t, err, tc.wantedError.Error()) + } + }) + } +} diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 9f4fcc40d1b..d5e73a4035c 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1684,6 +1684,59 @@ func (mr *MockrepositoryServiceMockRecorder) Login() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Login", reflect.TypeOf((*MockrepositoryService)(nil).Login)) } +// MockecsLocalClient is a mock of ecsLocalClient interface. +type MockecsLocalClient struct { + ctrl *gomock.Controller + recorder *MockecsLocalClientMockRecorder +} + +// MockecsLocalClientMockRecorder is the mock recorder for MockecsLocalClient. +type MockecsLocalClientMockRecorder struct { + mock *MockecsLocalClient +} + +// NewMockecsLocalClient creates a new mock instance. +func NewMockecsLocalClient(ctrl *gomock.Controller) *MockecsLocalClient { + mock := &MockecsLocalClient{ctrl: ctrl} + mock.recorder = &MockecsLocalClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockecsLocalClient) EXPECT() *MockecsLocalClientMockRecorder { + return m.recorder +} + +// DecryptedSecrets mocks base method. +func (m *MockecsLocalClient) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]ecs0.EnvVar, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DecryptedSecrets", secrets) + ret0, _ := ret[0].([]ecs0.EnvVar) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DecryptedSecrets indicates an expected call of DecryptedSecrets. +func (mr *MockecsLocalClientMockRecorder) DecryptedSecrets(secrets interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DecryptedSecrets", reflect.TypeOf((*MockecsLocalClient)(nil).DecryptedSecrets), secrets) +} + +// TaskDefinition mocks base method. +func (m *MockecsLocalClient) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TaskDefinition", app, env, svc) + ret0, _ := ret[0].(*ecs.TaskDefinition) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TaskDefinition indicates an expected call of TaskDefinition. +func (mr *MockecsLocalClientMockRecorder) TaskDefinition(app, env, svc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsLocalClient)(nil).TaskDefinition), app, env, svc) +} + // MocklogEventsWriter is a mock of logEventsWriter interface. type MocklogEventsWriter struct { ctrl *gomock.Controller From 0bb7c6007f9c23b82b49a69e46aae62ee81a7056 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Mon, 24 Jul 2023 17:04:03 -0700 Subject: [PATCH 15/17] nits updated --- internal/pkg/aws/secretsmanager/secretsmanager.go | 2 +- internal/pkg/aws/ssm/ssm.go | 2 +- internal/pkg/ecs/ecs.go | 15 +++++++-------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/internal/pkg/aws/secretsmanager/secretsmanager.go b/internal/pkg/aws/secretsmanager/secretsmanager.go index a4c7c6f00ce..a735345a24d 100644 --- a/internal/pkg/aws/secretsmanager/secretsmanager.go +++ b/internal/pkg/aws/secretsmanager/secretsmanager.go @@ -125,7 +125,7 @@ func (s *SecretsManager) GetSecretValue(name string) (string, error) { SecretId: aws.String(name), }) if err != nil { - return "", fmt.Errorf("get secret %s from secrets manager: %w", name, err) + return "", fmt.Errorf("get secret %q from secrets manager: %w", name, err) } return aws.StringValue(resp.SecretString), nil } diff --git a/internal/pkg/aws/ssm/ssm.go b/internal/pkg/aws/ssm/ssm.go index 8ba026aae86..d5ac6c66395 100644 --- a/internal/pkg/aws/ssm/ssm.go +++ b/internal/pkg/aws/ssm/ssm.go @@ -73,7 +73,7 @@ func (s *SSM) GetSecretValue(name string) (string, error) { WithDecryption: aws.Bool(true), }) if err != nil { - return "", fmt.Errorf("get parameter %s from SSM with decryption: %w", name, err) + return "", fmt.Errorf("get parameter %q from SSM: %w", name, err) } return aws.StringValue(resp.Parameter.Value), nil } diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index f6317c3b063..17798849a80 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -53,11 +53,12 @@ type ecsClient interface { type stepFunctionsClient interface { StateMachineDefinition(stateMachineARN string) (string, error) } + type secretGetter interface { GetSecretValue(secretName string) (string, error) } -// EnvVar contains values of the environmental variables +// EnvVar contains the value of an environment variable type EnvVar struct { Name string Value string @@ -246,10 +247,9 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { return c.ecsClient.StopTasks(taskIDs, ecs.WithStopTaskReason(taskStopReason)) } -// DecryptedSecrets returns the decrypted parameters from either the SSM store or Secrets Manager. +// DecryptedSecrets returns the decrypted parameters from either SSM parameter store or Secrets Manager. func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { - var ssmSecrets []EnvVar - var secretManagerSecrets []EnvVar + var vars []EnvVar for _, secret := range secrets { parsed, err := arn.Parse(secret.ValueFrom) if err != nil || parsed.Service == ssm.Namespace { @@ -257,7 +257,7 @@ func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, erro if err != nil { return nil, err } - ssmSecrets = append(ssmSecrets, EnvVar{ + vars = append(vars, EnvVar{ Name: secret.Name, Value: secretValue, }) @@ -267,14 +267,13 @@ func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, erro if err != nil { return nil, err } - secretManagerSecrets = append(secretManagerSecrets, EnvVar{ + vars = append(vars, EnvVar{ Name: secret.Name, Value: secretValue, }) } } - allSecrets := append(ssmSecrets, secretManagerSecrets...) - return allSecrets, nil + return vars, nil } // TaskDefinition returns the task definition of the service. From 0346b192ee085525c4119fca02e9eab6037bfc5b Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Tue, 25 Jul 2023 15:27:18 -0700 Subject: [PATCH 16/17] nit updated --- internal/pkg/ecs/ecs.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 17798849a80..3b14618489b 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -261,8 +261,7 @@ func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, erro Name: secret.Name, Value: secretValue, }) - } - if parsed.Service == secretsmanager.Namespace { + } else if parsed.Service == secretsmanager.Namespace { secretValue, err := c.secretManager.GetSecretValue(secret.ValueFrom) if err != nil { return nil, err From bc7b3fbed3e244c800abef1be8939cb3a0a93625 Mon Sep 17 00:00:00 2001 From: VarunReddy Date: Wed, 26 Jul 2023 16:56:13 -0700 Subject: [PATCH 17/17] proper comments added --- internal/pkg/ecs/ecs.go | 52 ++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 3b14618489b..e3c4f71075a 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -247,30 +247,44 @@ func (c Client) StopDefaultClusterTasks(familyName string) error { return c.ecsClient.StopTasks(taskIDs, ecs.WithStopTaskReason(taskStopReason)) } +func secretNameSpace(secret string) (string, error) { + // A secret value can be a SSM Parameter Name/ SSM Parameter ARN/ Secrets Manager ARN + // Note: If there is an error while parsing the secret value, this functions assumes it to be SSM Parameter. + // Refer to https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_Secret.html + parsed, err := arn.Parse(secret) + if err != nil { + return ssm.Namespace, nil + } + switch parsed.Service { + case ssm.Namespace, secretsmanager.Namespace: + return parsed.Service, nil + default: + return "", fmt.Errorf("invalid ARN: not an SSM or Secrets Manager ARN") + } +} + // DecryptedSecrets returns the decrypted parameters from either SSM parameter store or Secrets Manager. func (c Client) DecryptedSecrets(secrets []*ecs.ContainerSecret) ([]EnvVar, error) { var vars []EnvVar for _, secret := range secrets { - parsed, err := arn.Parse(secret.ValueFrom) - if err != nil || parsed.Service == ssm.Namespace { - secretValue, err := c.ssm.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err - } - vars = append(vars, EnvVar{ - Name: secret.Name, - Value: secretValue, - }) - } else if parsed.Service == secretsmanager.Namespace { - secretValue, err := c.secretManager.GetSecretValue(secret.ValueFrom) - if err != nil { - return nil, err - } - vars = append(vars, EnvVar{ - Name: secret.Name, - Value: secretValue, - }) + namespace, err := secretNameSpace(secret.ValueFrom) + if err != nil { + return nil, err + } + var secretValue string + switch namespace { + case ssm.Namespace: + secretValue, err = c.ssm.GetSecretValue(secret.ValueFrom) + case secretsmanager.Namespace: + secretValue, err = c.secretManager.GetSecretValue(secret.ValueFrom) + } + if err != nil { + return nil, err } + vars = append(vars, EnvVar{ + Name: secret.Name, + Value: secretValue, + }) } return vars, nil }