Skip to content

Commit 7cb56c0

Browse files
bvtujobvtujo
authored
fix(storage): Use template functions for LogicalID and Env Var names (#1035)
<!-- Provide summary of changes --> This PR adds template functions to the storage resource CF templates. We now only need to specify a resource name to the DynamoDB and S3 types, and template functions StorageLogicalIDSafe and EnvVarName will properly construct CF resource names and Env Vars. This PR also reorganizes and renames some common go template functions that previously lived in the "deploy/cloudformation/stack" package, moving them to the "template" package. Finally, it changes the implementation of ToSnakeCase to better match StringsWithUPPERCase (now renders as "STRINGS_WITH_UPPER_CASE" instead of "STRINGS_WITH_U_P_P_E_R_CASE") <!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" --> By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
1 parent c383f2a commit 7cb56c0

17 files changed

Lines changed: 413 additions & 215 deletions

File tree

‎internal/pkg/addon/storage.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@ const (
1313
s3AddonPath = "addons/s3/cf.yml"
1414
)
1515

16+
var storageTemplateFunctions = map[string]interface{}{
17+
"logicalIDSafe": template.StripNonAlphaNumFunc,
18+
"envVarName": template.EnvVarNameFunc,
19+
}
20+
1621
type storage struct {
17-
ResourceName *string
18-
Name *string
22+
Name *string
1923
}
2024

2125
// DynamoDB contains configuration options which fully descibe a DynamoDB table.
@@ -36,8 +40,7 @@ type S3 struct {
3640

3741
// StorageProps holds basic input properties for addon.NewDynamoDB() or addon.NewS3().
3842
type StorageProps struct {
39-
Name string
40-
ResourceName string
43+
Name string
4144
}
4245

4346
// S3Props contains S3-specific properties for addon.NewS3().
@@ -71,7 +74,7 @@ type DDBLocalSecondaryIndex struct {
7174
// MarshalBinary serializes the DynamoDB object into a binary YAML CF template.
7275
// Implements the encoding.BinaryMarshaler interface.
7376
func (d *DynamoDB) MarshalBinary() ([]byte, error) {
74-
content, err := d.parser.Parse(dynamoDbAddonPath, *d)
77+
content, err := d.parser.Parse(dynamoDbAddonPath, *d, template.WithFuncs(storageTemplateFunctions))
7578
if err != nil {
7679
return nil, err
7780
}
@@ -91,7 +94,7 @@ func NewDynamoDB(input *DynamoDBProps) *DynamoDB {
9194
// MarshalBinary serializes the S3 object into a binary YAML CF template.
9295
// Implements the encoding.BinaryMarshaler interface.
9396
func (s *S3) MarshalBinary() ([]byte, error) {
94-
content, err := s.parser.Parse(s3AddonPath, *s)
97+
content, err := s.parser.Parse(s3AddonPath, *s, template.WithFuncs(storageTemplateFunctions))
9598
if err != nil {
9699
return nil, err
97100
}

‎internal/pkg/addon/storage_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestDynamoDB_MarshalBinary(t *testing.T) {
2626
mockDependencies: func(ctrl *gomock.Controller, ddb *DynamoDB) {
2727
m := mocks.NewMockParser(ctrl)
2828
ddb.parser = m
29-
m.EXPECT().Parse(dynamoDbAddonPath, *ddb).Return(nil, errors.New("some error"))
29+
m.EXPECT().Parse(dynamoDbAddonPath, *ddb, gomock.Any()).Return(nil, errors.New("some error"))
3030
},
3131

3232
wantedError: errors.New("some error"),
@@ -35,7 +35,7 @@ func TestDynamoDB_MarshalBinary(t *testing.T) {
3535
mockDependencies: func(ctrl *gomock.Controller, ddb *DynamoDB) {
3636
m := mocks.NewMockParser(ctrl)
3737
ddb.parser = m
38-
m.EXPECT().Parse(dynamoDbAddonPath, *ddb).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
38+
m.EXPECT().Parse(dynamoDbAddonPath, *ddb, gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
3939

4040
},
4141

@@ -72,7 +72,7 @@ func TestS3_MarshalBinary(t *testing.T) {
7272
mockDependencies: func(ctrl *gomock.Controller, s3 *S3) {
7373
m := mocks.NewMockParser(ctrl)
7474
s3.parser = m
75-
m.EXPECT().Parse(s3AddonPath, *s3).Return(nil, errors.New("some error"))
75+
m.EXPECT().Parse(s3AddonPath, *s3, gomock.Any()).Return(nil, errors.New("some error"))
7676
},
7777

7878
wantedError: errors.New("some error"),
@@ -81,7 +81,7 @@ func TestS3_MarshalBinary(t *testing.T) {
8181
mockDependencies: func(ctrl *gomock.Controller, s3 *S3) {
8282
m := mocks.NewMockParser(ctrl)
8383
s3.parser = m
84-
m.EXPECT().Parse(s3AddonPath, *s3).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
84+
m.EXPECT().Parse(s3AddonPath, *s3, gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
8585

8686
},
8787

‎internal/pkg/cli/flag.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ package cli
55

66
import (
77
"fmt"
8-
"strconv"
98
"strings"
109

1110
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/manifest"
11+
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
1212
)
1313

1414
// Long flag names.
@@ -84,9 +84,9 @@ const (
8484
// Descriptions for flags.
8585
var (
8686
svcTypeFlagDescription = fmt.Sprintf(`Type of service to create. Must be one of:
87-
%s`, strings.Join(quoteAll(manifest.ServiceTypes), ", "))
87+
%s`, strings.Join(template.QuoteSliceFunc(manifest.ServiceTypes), ", "))
8888
storageTypeFlagDescription = fmt.Sprintf(`Type of storage to add. Must be one of:
89-
%s`, strings.Join(quoteAll(storageTypes), ", "))
89+
%s`, strings.Join(template.QuoteSliceFunc(storageTypes), ", "))
9090
)
9191

9292
const (
@@ -148,11 +148,3 @@ Must be of the format '<name>:<dataType>'. Can be specified multiple times.`
148148
envVarsFlagDescription = "Optional. Environment variables specified by key=value separated with commas."
149149
commandsFlagDescription = "Optional. List of commands that are passed to docker run. Can be specified multiple times."
150150
)
151-
152-
func quoteAll(elems []string) []string {
153-
quotedElems := make([]string, len(elems))
154-
for i, el := range elems {
155-
quotedElems[i] = strconv.Quote(el)
156-
}
157-
return quotedElems
158-
}

‎internal/pkg/cli/storage_init.go‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -499,13 +499,6 @@ func (o *initStorageOpts) Execute() error {
499499
return o.createAddon()
500500
}
501501

502-
var nonAlphaNum = regexp.MustCompile("[^a-zA-Z0-9]+")
503-
504-
// Strip non-alphanumeric characters from an input string
505-
func logicalIDSafe(input string) string {
506-
return nonAlphaNum.ReplaceAllString(input, "")
507-
}
508-
509502
var regexpMatchAttribute = regexp.MustCompile("^(\\S+):([sbnSBN])")
510503

511504
// getAttFromKey parses the DDB type and name out of keys specified in the form "Email:S"
@@ -637,33 +630,23 @@ func (o *initStorageOpts) newDynamoDBAddon() (*addon.DynamoDB, error) {
637630
}
638631

639632
props.StorageProps = &addon.StorageProps{
640-
Name: o.storageName,
641-
ResourceName: logicalIDSafe(o.storageName),
633+
Name: o.storageName,
642634
}
643635
return addon.NewDynamoDB(&props), nil
644636
}
645637

646638
func (o *initStorageOpts) newS3Addon() (*addon.S3, error) {
647639
props := &addon.S3Props{
648640
StorageProps: &addon.StorageProps{
649-
Name: o.storageName,
650-
ResourceName: logicalIDSafe(o.storageName),
641+
Name: o.storageName,
651642
},
652643
}
653644
return addon.NewS3(props), nil
654645
}
655646

656647
func (o *initStorageOpts) RecommendedActions() []string {
657-
var storageTypeEnvVar string
658-
switch o.storageType {
659-
case dynamoDBStorageType:
660-
storageTypeEnvVar = "TableName"
661-
case s3StorageType:
662-
storageTypeEnvVar = "BucketName"
663-
}
664648

665-
// TODO: refactor this into a template function, or standardize generating env var names in another way
666-
newVar := template.ToSnakeCase(logicalIDSafe(o.storageName) + storageTypeEnvVar)
649+
newVar := template.ToSnakeCaseFunc(template.EnvVarNameFunc(o.storageName))
667650

668651
svcDeployCmd := fmt.Sprintf("copilot svc deploy --name %s", o.storageSvc)
669652

‎internal/pkg/cli/validate.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/manifest"
14+
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
1415
)
1516

1617
var (
@@ -310,6 +311,6 @@ func validateLsi(lsis []string, attributes []string) error {
310311
}
311312

312313
func prettify(inputStrings []string) string {
313-
prettyTypes := quoteAll(inputStrings)
314+
prettyTypes := template.QuoteSliceFunc(inputStrings)
314315
return strings.Join(prettyTypes, ", ")
315316
}

‎internal/pkg/deploy/cloudformation/stack/app.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ const (
6161
appDNSDelegationRoleName = "DNSDelegationRole"
6262
)
6363

64+
var cfTemplateFunctions = map[string]interface{}{
65+
"logicalIDSafe": template.ReplaceDashesFunc,
66+
}
67+
6468
// AppConfigFrom takes a template file and extracts the metadata block,
6569
// and parses it into an AppStackConfig
6670
func AppConfigFrom(template *string) (*AppResourcesConfig, error) {
@@ -99,7 +103,7 @@ func (c *AppStackConfig) ResourceTemplate(config *AppResourcesConfig) (string, e
99103
}{
100104
config,
101105
ServiceTagKey,
102-
}, template.WithFuncs(templateFunctions))
106+
}, template.WithFuncs(cfTemplateFunctions))
103107
if err != nil {
104108
return "", err
105109
}
@@ -215,7 +219,7 @@ func ToAppRegionalResources(stack *cloudformation.Stack) (*AppRegionalResources,
215219
safeSvcName := strings.TrimPrefix(key, appOutputECRRepoPrefix)
216220
// It's possible we had to sanitize the service name (removing dashes),
217221
// so return it back to its original form.
218-
originalSvcName := safeLogicalIDToOriginal(safeSvcName)
222+
originalSvcName := template.DashReplacedLogicalIDToOriginal(safeSvcName)
219223
regionalResources.RepositoryURLs[originalSvcName] = uri
220224
}
221225
}

‎internal/pkg/deploy/cloudformation/stack/pipeline.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func (p *pipelineStackConfig) StackName() string {
2929
}
3030

3131
func (p *pipelineStackConfig) Template() (string, error) {
32-
content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(templateFunctions))
32+
content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(cfTemplateFunctions))
3333
if err != nil {
3434
return "", err
3535
}

‎internal/pkg/deploy/cloudformation/stack/svc.go‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (s *svc) templateConfiguration(tc templateConfigurer) (string, error) {
130130
Parameters: params,
131131
Tags: tc.Tags(),
132132
}, template.WithFuncs(map[string]interface{}{
133-
"inc": func(i int) int { return i + 1 },
133+
"inc": template.IncFunc,
134134
}))
135135
if err != nil {
136136
return "", err
@@ -159,3 +159,33 @@ func (s *svc) addonsOutputs() (*template.ServiceNestedStackOpts, error) {
159159
PolicyOutputs: managedPolicyOutputNames(out),
160160
}, nil
161161
}
162+
163+
func secretOutputNames(outputs []addon.Output) []string {
164+
var secrets []string
165+
for _, out := range outputs {
166+
if out.IsSecret {
167+
secrets = append(secrets, out.Name)
168+
}
169+
}
170+
return secrets
171+
}
172+
173+
func managedPolicyOutputNames(outputs []addon.Output) []string {
174+
var policies []string
175+
for _, out := range outputs {
176+
if out.IsManagedPolicy {
177+
policies = append(policies, out.Name)
178+
}
179+
}
180+
return policies
181+
}
182+
183+
func envVarOutputNames(outputs []addon.Output) []string {
184+
var envVars []string
185+
for _, out := range outputs {
186+
if !out.IsSecret && !out.IsManagedPolicy {
187+
envVars = append(envVars, out.Name)
188+
}
189+
}
190+
return envVars
191+
}

‎internal/pkg/deploy/cloudformation/stack/template_functions.go‎

Lines changed: 0 additions & 62 deletions
This file was deleted.

‎internal/pkg/manifest/backend_svc.go‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
package manifest
55

66
import (
7-
"fmt"
8-
"strconv"
9-
"strings"
107
"time"
118

129
"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
@@ -90,16 +87,8 @@ func NewBackendService(props BackendServiceProps) *BackendService {
9087
// Implements the encoding.BinaryMarshaler interface.
9188
func (s *BackendService) MarshalBinary() ([]byte, error) {
9289
content, err := s.parser.Parse(backendSvcManifestPath, *s, template.WithFuncs(map[string]interface{}{
93-
"fmtSlice": func(elems []string) string {
94-
return fmt.Sprintf("[%s]", strings.Join(elems, ", "))
95-
},
96-
"quoteSlice": func(elems []string) []string {
97-
quotedElems := make([]string, len(elems))
98-
for i, el := range elems {
99-
quotedElems[i] = strconv.Quote(el)
100-
}
101-
return quotedElems
102-
},
90+
"fmtSlice": template.FmtSliceFunc,
91+
"quoteSlice": template.QuoteSliceFunc,
10392
}))
10493
if err != nil {
10594
return nil, err

0 commit comments

Comments
 (0)