From 238fd708679d4534b2f4c58cc3b7a85e6e1a768d Mon Sep 17 00:00:00 2001 From: Austin Ely Date: Fri, 20 Nov 2020 15:51:52 -0800 Subject: [PATCH] fix(storage)!: allow attaching storage resources to jobs via `storage init` (#1701) This PR enables customers to select from the list of services AND jobs when creating a new addons template through Storage Init. It also replaces the `--svc/-s` flag with a generic `--for` flag, which may be a breaking change for some customers. Addresses #1595 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --- internal/pkg/cli/flag.go | 34 +++++------ internal/pkg/cli/interfaces.go | 2 +- internal/pkg/cli/mocks/mock_interfaces.go | 27 ++------- internal/pkg/cli/storage_init.go | 72 +++++++++++------------ internal/pkg/cli/storage_init_test.go | 40 ++++++------- 5 files changed, 77 insertions(+), 98 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index 3c1df4a96b6..a9af68a6885 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -14,17 +14,17 @@ import ( // Long flag names. const ( // Common flags. - nameFlag = "name" - appFlag = "app" - envFlag = "env" - svcFlag = "svc" - svcTypeFlag = "svc-type" - jobTypeFlag = "job-type" - typeFlag = "type" - profileFlag = "profile" - yesFlag = "yes" - jsonFlag = "json" - allFlag = "all" + nameFlag = "name" + appFlag = "app" + envFlag = "env" + workloadFlag = "workload" + svcTypeFlag = "svc-type" + jobTypeFlag = "job-type" + typeFlag = "type" + profileFlag = "profile" + yesFlag = "yes" + jsonFlag = "json" + allFlag = "all" // Command specific flags. dockerFileFlag = "dockerfile" @@ -92,11 +92,11 @@ const ( // Short flag names. // A short flag only exists if the flag or flag set is mandatory by the command. const ( - nameFlagShort = "n" - appFlagShort = "a" - envFlagShort = "e" - svcFlagShort = "s" - typeFlagShort = "t" + nameFlagShort = "n" + appFlagShort = "a" + envFlagShort = "e" + typeFlagShort = "t" + workloadFlagShort = "w" dockerFileFlagShort = "d" imageFlagShort = "i" @@ -178,7 +178,7 @@ Defaults to all logs. Only one of end-time / follow may be used.` svcPortFlagDescription = "Optional. The port on which your service listens." storageFlagDescription = "Name of the storage resource to create." - storageServiceFlagDescription = "Name of the service to associate with storage." + storageWorkloadFlagDescription = "Name of the service or job to associate with storage." storagePartitionKeyFlagDescription = `Partition key for the DDB table. Must be of the format ':'.` storageSortKeyFlagDescription = `Optional. Sort key for the DDB table. diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 2ff1b642ff5..d12bb7750a1 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -286,7 +286,7 @@ type wsAppManager interface { type wsAddonManager interface { WriteAddon(f encoding.BinaryMarshaler, svc, name string) (string, error) - wsSvcReader + wsWlReader } type artifactUploader interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index f617f8dd866..346ef5fe508 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -2844,34 +2844,19 @@ func (mr *MockwsAddonManagerMockRecorder) WriteAddon(f, svc, name interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteAddon", reflect.TypeOf((*MockwsAddonManager)(nil).WriteAddon), f, svc, name) } -// ServiceNames mocks base method -func (m *MockwsAddonManager) ServiceNames() ([]string, error) { +// WorkloadNames mocks base method +func (m *MockwsAddonManager) WorkloadNames() ([]string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ServiceNames") + ret := m.ctrl.Call(m, "WorkloadNames") ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } -// ServiceNames indicates an expected call of ServiceNames -func (mr *MockwsAddonManagerMockRecorder) ServiceNames() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServiceNames", reflect.TypeOf((*MockwsAddonManager)(nil).ServiceNames)) -} - -// ReadServiceManifest mocks base method -func (m *MockwsAddonManager) ReadServiceManifest(svcName string) ([]byte, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReadServiceManifest", svcName) - ret0, _ := ret[0].([]byte) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ReadServiceManifest indicates an expected call of ReadServiceManifest -func (mr *MockwsAddonManagerMockRecorder) ReadServiceManifest(svcName interface{}) *gomock.Call { +// WorkloadNames indicates an expected call of WorkloadNames +func (mr *MockwsAddonManagerMockRecorder) WorkloadNames() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadServiceManifest", reflect.TypeOf((*MockwsAddonManager)(nil).ReadServiceManifest), svcName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WorkloadNames", reflect.TypeOf((*MockwsAddonManager)(nil).WorkloadNames)) } // MockartifactUploader is a mock of artifactUploader interface diff --git a/internal/pkg/cli/storage_init.go b/internal/pkg/cli/storage_init.go index 11781782e61..4e70cddf006 100644 --- a/internal/pkg/cli/storage_init.go +++ b/internal/pkg/cli/storage_init.go @@ -42,16 +42,14 @@ var storageTypes = []string{ // General-purpose prompts, collected for all storage resources. var ( fmtStorageInitTypePrompt = "What " + color.Emphasize("type") + " of storage would you like to associate with %s?" - storageInitTypeHelp = `The type of storage you'd like to add to your service. + storageInitTypeHelp = `The type of storage you'd like to add to your workload. DynamoDB is a key-value and document database that delivers single-digit millisecond performance at any scale. S3 is a web object store built to store and retrieve any amount of data from anywhere on the Internet.` fmtStorageInitNamePrompt = "What would you like to " + color.Emphasize("name") + " this %s?" storageInitNameHelp = "The name of this storage resource. You can use the following characters: a-zA-Z0-9-_" - storageInitSvcPrompt = "Which " + color.Emphasize("service") + " would you like to associate with this storage resource?" - storageInitSvcHelp = `The service you'd like to have access to this storage resource. -We'll deploy the resources for the storage when you run 'svc deploy'.` + storageInitSvcPrompt = "Which " + color.Emphasize("workload") + " would you like to associate with this storage resource?" ) // DDB-specific questions and help prompts. @@ -89,9 +87,9 @@ var attributeTypes = []string{ } type initStorageVars struct { - storageType string - storageName string - storageSvc string + storageType string + storageName string + workloadName string // Dynamo DB specific values collected via flags or prompts partitionKey string @@ -141,8 +139,8 @@ func (o *initStorageOpts) Validate() error { if o.appName == "" { return errNoAppInWorkspace } - if o.storageSvc != "" { - if err := o.validateServiceName(); err != nil { + if o.workloadName != "" { + if err := o.validateWorkloadName(); err != nil { return err } } @@ -201,7 +199,7 @@ func (o *initStorageOpts) validateDDB() error { } func (o *initStorageOpts) Ask() error { - if err := o.askStorageSvc(); err != nil { + if err := o.askStorageWl(); err != nil { return err } if err := o.askStorageType(); err != nil { @@ -231,7 +229,7 @@ func (o *initStorageOpts) askStorageType() error { } storageType, err := o.prompt.SelectOne(fmt.Sprintf( - fmtStorageInitTypePrompt, color.HighlightUserInput(o.storageSvc)), + fmtStorageInitTypePrompt, color.HighlightUserInput(o.workloadName)), storageInitTypeHelp, storageTypes, prompt.WithFinalMessage("Storage type:")) @@ -271,15 +269,15 @@ func (o *initStorageOpts) askStorageName() error { return nil } -func (o *initStorageOpts) askStorageSvc() error { - if o.storageSvc != "" { +func (o *initStorageOpts) askStorageWl() error { + if o.workloadName != "" { return nil } - svc, err := o.sel.Service(storageInitSvcPrompt, storageInitSvcHelp) + workload, err := o.sel.Workload(storageInitSvcPrompt, "") if err != nil { - return fmt.Errorf("retrieve local service names: %w", err) + return fmt.Errorf("retrieve local workload names: %w", err) } - o.storageSvc = svc + o.workloadName = workload return nil } @@ -422,17 +420,17 @@ func (o *initStorageOpts) askDynamoLSIConfig() error { } } -func (o *initStorageOpts) validateServiceName() error { - names, err := o.ws.ServiceNames() +func (o *initStorageOpts) validateWorkloadName() error { + names, err := o.ws.WorkloadNames() if err != nil { - return fmt.Errorf("retrieve local service names: %w", err) + return fmt.Errorf("retrieve local workload names: %w", err) } for _, name := range names { - if o.storageSvc == name { + if o.workloadName == name { return nil } } - return fmt.Errorf("service %s not found in the workspace", o.storageSvc) + return fmt.Errorf("workload %s not found in the workspace", o.workloadName) } func (o *initStorageOpts) Execute() error { @@ -446,7 +444,7 @@ func (o *initStorageOpts) createAddon() error { return err } - addonPath, err := o.ws.WriteAddon(addonCf, o.storageSvc, o.storageName) + addonPath, err := o.ws.WriteAddon(addonCf, o.workloadName, o.storageName) if err != nil { e, ok := err.(*workspace.ErrFileExists) if !ok { @@ -474,10 +472,6 @@ func (o *initStorageOpts) createAddon() error { color.HighlightUserInput(o.storageName), color.HighlightResource(addonPath), ) - log.Infoln(color.Help(`The Cloudformation template is a nested stack which fully describes your resource, -the IAM policy necessary for an ECS task to access that resource, and outputs -which are injected as environment variables into the Copilot service this addon -is associated with.`)) log.Infoln() return nil @@ -532,11 +526,11 @@ func (o *initStorageOpts) RecommendedActions() []string { newVar := template.ToSnakeCaseFunc(template.EnvVarNameFunc(o.storageName)) - svcDeployCmd := fmt.Sprintf("copilot svc deploy --name %s", o.storageSvc) + deployCmd := fmt.Sprintf("copilot deploy --name %s", o.workloadName) return []string{ - fmt.Sprintf("Update your service code to leverage the injected environment variable %s", color.HighlightCode(newVar)), - fmt.Sprintf("Run %s to deploy your storage resources to your environments.", color.HighlightCode(svcDeployCmd)), + fmt.Sprintf("Update %s's code to leverage the injected environment variable %s", color.HighlightUserInput(o.workloadName), color.HighlightCode(newVar)), + fmt.Sprintf("Run %s to deploy your storage resources.", color.HighlightCode(deployCmd)), } } @@ -545,18 +539,18 @@ func buildStorageInitCmd() *cobra.Command { vars := initStorageVars{} cmd := &cobra.Command{ Use: "init", - Short: "Creates a new storage config file in a service's addons directory.", - Long: `Creates a new storage config file in a service's addons directory. -Storage resources are deployed to your service's environments when you run -'copilot svc deploy'. Resource names are injected into your service containers as -environment variables for easy access.`, + Short: "Creates a new AWS CloudFormation template for a storage resource.", + Long: `Creates a new AWS CloudFormation template for a storage resource. +Storage resources are stored in the Copilot addons directory (e.g. ./copilot/frontend/addons) for a given +workload and deployed to your environments when you run ` + color.HighlightCode("copilot deploy") + `. Resource names +are injected into your containers as environment variables for easy access.`, Example: ` Create an S3 bucket named "my-bucket" attached to the "frontend" service. - /code $ copilot storage init -n my-bucket -t S3 -s frontend + /code $ copilot storage init -n my-bucket -t S3 -w frontend Create a basic DynamoDB table named "my-table" attached to the "frontend" service with a sort key specified. - /code $ copilot storage init -n my-table -t DynamoDB -s frontend --partition-key Email:S --sort-key UserId:N --no-lsi + /code $ copilot storage init -n my-table -t DynamoDB -w frontend --partition-key Email:S --sort-key UserId:N --no-lsi Create a DynamoDB table with multiple alternate sort keys. - /code $ copilot storage init -n my-table -t DynamoDB -s frontend --partition-key Email:S --sort-key UserId:N --lsi Points:N --lsi Goodness:N`, + /code $ copilot storage init -n my-table -t DynamoDB -w frontend --partition-key Email:S --sort-key UserId:N --lsi Points:N --lsi Goodness:N`, RunE: runCmdE(func(cmd *cobra.Command, args []string) error { opts, err := newStorageInitOpts(vars) if err != nil { @@ -580,7 +574,7 @@ environment variables for easy access.`, } cmd.Flags().StringVarP(&vars.storageName, nameFlag, nameFlagShort, "", storageFlagDescription) cmd.Flags().StringVarP(&vars.storageType, storageTypeFlag, typeFlagShort, "", storageTypeFlagDescription) - cmd.Flags().StringVarP(&vars.storageSvc, svcFlag, svcFlagShort, "", storageServiceFlagDescription) + cmd.Flags().StringVarP(&vars.workloadName, workloadFlag, workloadFlagShort, "", storageWorkloadFlagDescription) cmd.Flags().StringVar(&vars.partitionKey, storagePartitionKeyFlag, "", storagePartitionKeyFlagDescription) cmd.Flags().StringVar(&vars.sortKey, storageSortKeyFlag, "", storageSortKeyFlagDescription) @@ -591,7 +585,7 @@ environment variables for easy access.`, requiredFlags := pflag.NewFlagSet("Required", pflag.ContinueOnError) requiredFlags.AddFlag(cmd.Flags().Lookup(nameFlag)) requiredFlags.AddFlag(cmd.Flags().Lookup(storageTypeFlag)) - requiredFlags.AddFlag(cmd.Flags().Lookup(svcFlag)) + requiredFlags.AddFlag(cmd.Flags().Lookup(workloadFlag)) ddbFlags := pflag.NewFlagSet("DynamoDB", pflag.ContinueOnError) ddbFlags.AddFlag(cmd.Flags().Lookup(storagePartitionKeyFlag)) diff --git a/internal/pkg/cli/storage_init_test.go b/internal/pkg/cli/storage_init_test.go index a000a02206e..c00b245e197 100644 --- a/internal/pkg/cli/storage_init_test.go +++ b/internal/pkg/cli/storage_init_test.go @@ -41,7 +41,7 @@ func TestStorageInitOpts_Validate(t *testing.T) { }, "svc not in workspace": { mockWs: func(m *mocks.MockwsAddonManager) { - m.EXPECT().ServiceNames().Return([]string{"bad", "workspace"}, nil) + m.EXPECT().WorkloadNames().Return([]string{"bad", "workspace"}, nil) }, mockStore: func(m *mocks.Mockstore) {}, @@ -49,11 +49,11 @@ func TestStorageInitOpts_Validate(t *testing.T) { inStorageType: s3StorageType, inSvcName: "frontend", inStorageName: "my-bucket", - wantedErr: errors.New("service frontend not found in the workspace"), + wantedErr: errors.New("workload frontend not found in the workspace"), }, "workspace error": { mockWs: func(m *mocks.MockwsAddonManager) { - m.EXPECT().ServiceNames().Return(nil, errors.New("wanted err")) + m.EXPECT().WorkloadNames().Return(nil, errors.New("wanted err")) }, mockStore: func(m *mocks.Mockstore) {}, @@ -61,7 +61,7 @@ func TestStorageInitOpts_Validate(t *testing.T) { inStorageType: s3StorageType, inSvcName: "frontend", inStorageName: "my-bucket", - wantedErr: errors.New("retrieve local service names: wanted err"), + wantedErr: errors.New("retrieve local workload names: wanted err"), }, "successfully validates valid s3 bucket name": { mockWs: func(m *mocks.MockwsAddonManager) {}, @@ -175,7 +175,7 @@ func TestStorageInitOpts_Validate(t *testing.T) { initStorageVars: initStorageVars{ storageType: tc.inStorageType, storageName: tc.inStorageName, - storageSvc: tc.inSvcName, + workloadName: tc.inSvcName, partitionKey: tc.inPartition, sortKey: tc.inSort, lsiSorts: tc.inLSISorts, @@ -251,14 +251,14 @@ func TestStorageInitOpts_Ask(t *testing.T) { wantedErr: fmt.Errorf("select storage type: some error"), }, - "asks for storage svc": { + "asks for storage workload": { inAppName: wantedAppName, inStorageName: wantedBucketName, inStorageType: s3StorageType, mockPrompt: func(m *mocks.Mockprompter) {}, mockCfg: func(m *mocks.MockwsSelector) { - m.EXPECT().Service(gomock.Eq(storageInitSvcPrompt), gomock.Any()).Return(wantedSvcName, nil) + m.EXPECT().Workload(gomock.Eq(storageInitSvcPrompt), gomock.Any()).Return(wantedSvcName, nil) }, wantedErr: nil, @@ -270,10 +270,10 @@ func TestStorageInitOpts_Ask(t *testing.T) { mockPrompt: func(m *mocks.Mockprompter) {}, mockCfg: func(m *mocks.MockwsSelector) { - m.EXPECT().Service(gomock.Any(), gomock.Any()).Return("", errors.New("some error")) + m.EXPECT().Workload(gomock.Any(), gomock.Any()).Return("", errors.New("some error")) }, - wantedErr: fmt.Errorf("retrieve local service names: some error"), + wantedErr: fmt.Errorf("retrieve local workload names: some error"), }, "asks for storage name": { inAppName: wantedAppName, @@ -557,9 +557,9 @@ func TestStorageInitOpts_Ask(t *testing.T) { mockCfg: func(m *mocks.MockwsSelector) {}, wantedVars: &initStorageVars{ - storageName: wantedTableName, - storageSvc: wantedSvcName, - storageType: dynamoDBStorageType, + storageName: wantedTableName, + workloadName: wantedSvcName, + storageType: dynamoDBStorageType, partitionKey: wantedPartitionKey, sortKey: wantedSortKey, @@ -585,9 +585,9 @@ func TestStorageInitOpts_Ask(t *testing.T) { mockCfg: func(m *mocks.MockwsSelector) {}, wantedVars: &initStorageVars{ - storageName: wantedTableName, - storageSvc: wantedSvcName, - storageType: dynamoDBStorageType, + storageName: wantedTableName, + workloadName: wantedSvcName, + storageType: dynamoDBStorageType, partitionKey: wantedPartitionKey, sortKey: wantedSortKey, @@ -611,9 +611,9 @@ func TestStorageInitOpts_Ask(t *testing.T) { mockCfg: func(m *mocks.MockwsSelector) {}, wantedVars: &initStorageVars{ - storageName: wantedTableName, - storageSvc: wantedSvcName, - storageType: dynamoDBStorageType, + storageName: wantedTableName, + workloadName: wantedSvcName, + storageType: dynamoDBStorageType, partitionKey: wantedPartitionKey, noLSI: true, @@ -720,7 +720,7 @@ func TestStorageInitOpts_Ask(t *testing.T) { initStorageVars: initStorageVars{ storageType: tc.inStorageType, storageName: tc.inStorageName, - storageSvc: tc.inSvcName, + workloadName: tc.inSvcName, partitionKey: tc.inPartition, sortKey: tc.inSort, lsiSorts: tc.inLSISorts, @@ -852,7 +852,7 @@ func TestStorageInitOpts_Execute(t *testing.T) { initStorageVars: initStorageVars{ storageType: tc.inStorageType, storageName: tc.inStorageName, - storageSvc: tc.inSvcName, + workloadName: tc.inSvcName, partitionKey: tc.inPartition, sortKey: tc.inSort, lsiSorts: tc.inLSISorts,