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.

internal/pkg/addon/storage.go

@@ -13,9 +13,13 @@ const (
 	s3AddonPath       = "addons/s3/cf.yml"
 )
 
+var storageTemplateFunctions = map[string]interface{}{
+	"logicalIDSafe": template.StripNonAlphaNumFunc,
+	"envVarName":    template.EnvVarNameFunc,
+}
+
 type storage struct {
-	ResourceName *string
-	Name         *string
+	Name *string
 }
 
 // DynamoDB contains configuration options which fully descibe a DynamoDB table.
@@ -36,8 +40,7 @@ type S3 struct {
 
 // StorageProps holds basic input properties for addon.NewDynamoDB() or addon.NewS3().
 type StorageProps struct {
-	Name         string
-	ResourceName string
+	Name string
 }
 
 // S3Props contains S3-specific properties for addon.NewS3().
@@ -71,7 +74,7 @@ type DDBLocalSecondaryIndex struct {
 // MarshalBinary serializes the DynamoDB object into a binary YAML CF template.
 // Implements the encoding.BinaryMarshaler interface.
 func (d *DynamoDB) MarshalBinary() ([]byte, error) {
-	content, err := d.parser.Parse(dynamoDbAddonPath, *d)
+	content, err := d.parser.Parse(dynamoDbAddonPath, *d, template.WithFuncs(storageTemplateFunctions))
 	if err != nil {
 		return nil, err
 	}
@@ -91,7 +94,7 @@ func NewDynamoDB(input *DynamoDBProps) *DynamoDB {
 // MarshalBinary serializes the S3 object into a binary YAML CF template.
 // Implements the encoding.BinaryMarshaler interface.
 func (s *S3) MarshalBinary() ([]byte, error) {
-	content, err := s.parser.Parse(s3AddonPath, *s)
+	content, err := s.parser.Parse(s3AddonPath, *s, template.WithFuncs(storageTemplateFunctions))
 	if err != nil {
 		return nil, err
 	}

internal/pkg/addon/storage_test.go

@@ -26,7 +26,7 @@ func TestDynamoDB_MarshalBinary(t *testing.T) {
 			mockDependencies: func(ctrl *gomock.Controller, ddb *DynamoDB) {
 				m := mocks.NewMockParser(ctrl)
 				ddb.parser = m
-				m.EXPECT().Parse(dynamoDbAddonPath, *ddb).Return(nil, errors.New("some error"))
+				m.EXPECT().Parse(dynamoDbAddonPath, *ddb, gomock.Any()).Return(nil, errors.New("some error"))
 			},
 
 			wantedError: errors.New("some error"),
@@ -35,7 +35,7 @@ func TestDynamoDB_MarshalBinary(t *testing.T) {
 			mockDependencies: func(ctrl *gomock.Controller, ddb *DynamoDB) {
 				m := mocks.NewMockParser(ctrl)
 				ddb.parser = m
-				m.EXPECT().Parse(dynamoDbAddonPath, *ddb).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
+				m.EXPECT().Parse(dynamoDbAddonPath, *ddb, gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
 
 			},
 
@@ -72,7 +72,7 @@ func TestS3_MarshalBinary(t *testing.T) {
 			mockDependencies: func(ctrl *gomock.Controller, s3 *S3) {
 				m := mocks.NewMockParser(ctrl)
 				s3.parser = m
-				m.EXPECT().Parse(s3AddonPath, *s3).Return(nil, errors.New("some error"))
+				m.EXPECT().Parse(s3AddonPath, *s3, gomock.Any()).Return(nil, errors.New("some error"))
 			},
 
 			wantedError: errors.New("some error"),
@@ -81,7 +81,7 @@ func TestS3_MarshalBinary(t *testing.T) {
 			mockDependencies: func(ctrl *gomock.Controller, s3 *S3) {
 				m := mocks.NewMockParser(ctrl)
 				s3.parser = m
-				m.EXPECT().Parse(s3AddonPath, *s3).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
+				m.EXPECT().Parse(s3AddonPath, *s3, gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("hello")}, nil)
 
 			},
 

internal/pkg/cli/flag.go

@@ -5,10 +5,10 @@ package cli
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
 
 	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/manifest"
+	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
 )
 
 // Long flag names.
@@ -84,9 +84,9 @@ const (
 // Descriptions for flags.
 var (
 	svcTypeFlagDescription = fmt.Sprintf(`Type of service to create. Must be one of:
-%s`, strings.Join(quoteAll(manifest.ServiceTypes), ", "))
+%s`, strings.Join(template.QuoteSliceFunc(manifest.ServiceTypes), ", "))
 	storageTypeFlagDescription = fmt.Sprintf(`Type of storage to add. Must be one of:
-%s`, strings.Join(quoteAll(storageTypes), ", "))
+%s`, strings.Join(template.QuoteSliceFunc(storageTypes), ", "))
 )
 
 const (
@@ -148,11 +148,3 @@ Must be of the format '<name>:<dataType>'. Can be specified multiple times.`
 	envVarsFlagDescription        = "Optional. Environment variables specified by key=value separated with commas."
 	commandsFlagDescription       = "Optional. List of commands that are passed to docker run. Can be specified multiple times."
 )
-
-func quoteAll(elems []string) []string {
-	quotedElems := make([]string, len(elems))
-	for i, el := range elems {
-		quotedElems[i] = strconv.Quote(el)
-	}
-	return quotedElems
-}

internal/pkg/cli/storage_init.go

@@ -499,13 +499,6 @@ func (o *initStorageOpts) Execute() error {
 	return o.createAddon()
 }
 
-var nonAlphaNum = regexp.MustCompile("[^a-zA-Z0-9]+")
-
-// Strip non-alphanumeric characters from an input string
-func logicalIDSafe(input string) string {
-	return nonAlphaNum.ReplaceAllString(input, "")
-}
-
 var regexpMatchAttribute = regexp.MustCompile("^(\\S+):([sbnSBN])")
 
 // 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) {
 	}
 
 	props.StorageProps = &addon.StorageProps{
-		Name:         o.storageName,
-		ResourceName: logicalIDSafe(o.storageName),
+		Name: o.storageName,
 	}
 	return addon.NewDynamoDB(&props), nil
 }
 
 func (o *initStorageOpts) newS3Addon() (*addon.S3, error) {
 	props := &addon.S3Props{
 		StorageProps: &addon.StorageProps{
-			Name:         o.storageName,
-			ResourceName: logicalIDSafe(o.storageName),
+			Name: o.storageName,
 		},
 	}
 	return addon.NewS3(props), nil
 }
 
 func (o *initStorageOpts) RecommendedActions() []string {
-	var storageTypeEnvVar string
-	switch o.storageType {
-	case dynamoDBStorageType:
-		storageTypeEnvVar = "TableName"
-	case s3StorageType:
-		storageTypeEnvVar = "BucketName"
-	}
 
-	// TODO: refactor this into a template function, or standardize generating env var names in another way
-	newVar := template.ToSnakeCase(logicalIDSafe(o.storageName) + storageTypeEnvVar)
+	newVar := template.ToSnakeCaseFunc(template.EnvVarNameFunc(o.storageName))
 
 	svcDeployCmd := fmt.Sprintf("copilot svc deploy --name %s", o.storageSvc)
 

internal/pkg/cli/validate.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/manifest"
+	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
 )
 
 var (
@@ -310,6 +311,6 @@ func validateLsi(lsis []string, attributes []string) error {
 }
 
 func prettify(inputStrings []string) string {
-	prettyTypes := quoteAll(inputStrings)
+	prettyTypes := template.QuoteSliceFunc(inputStrings)
 	return strings.Join(prettyTypes, ", ")
 }

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

@@ -61,6 +61,10 @@ const (
 	appDNSDelegationRoleName      = "DNSDelegationRole"
 )
 
+var cfTemplateFunctions = map[string]interface{}{
+	"logicalIDSafe": template.ReplaceDashesFunc,
+}
+
 // AppConfigFrom takes a template file and extracts the metadata block,
 // and parses it into an AppStackConfig
 func AppConfigFrom(template *string) (*AppResourcesConfig, error) {
@@ -99,7 +103,7 @@ func (c *AppStackConfig) ResourceTemplate(config *AppResourcesConfig) (string, e
 	}{
 		config,
 		ServiceTagKey,
-	}, template.WithFuncs(templateFunctions))
+	}, template.WithFuncs(cfTemplateFunctions))
 	if err != nil {
 		return "", err
 	}
@@ -215,7 +219,7 @@ func ToAppRegionalResources(stack *cloudformation.Stack) (*AppRegionalResources,
 			safeSvcName := strings.TrimPrefix(key, appOutputECRRepoPrefix)
 			// It's possible we had to sanitize the service name (removing dashes),
 			// so return it back to its original form.
-			originalSvcName := safeLogicalIDToOriginal(safeSvcName)
+			originalSvcName := template.DashReplacedLogicalIDToOriginal(safeSvcName)
 			regionalResources.RepositoryURLs[originalSvcName] = uri
 		}
 	}

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

@@ -29,7 +29,7 @@ func (p *pipelineStackConfig) StackName() string {
 }
 
 func (p *pipelineStackConfig) Template() (string, error) {
-	content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(templateFunctions))
+	content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(cfTemplateFunctions))
 	if err != nil {
 		return "", err
 	}

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

@@ -130,7 +130,7 @@ func (s *svc) templateConfiguration(tc templateConfigurer) (string, error) {
 		Parameters: params,
 		Tags:       tc.Tags(),
 	}, template.WithFuncs(map[string]interface{}{
-		"inc": func(i int) int { return i + 1 },
+		"inc": template.IncFunc,
 	}))
 	if err != nil {
 		return "", err
@@ -159,3 +159,33 @@ func (s *svc) addonsOutputs() (*template.ServiceNestedStackOpts, error) {
 		PolicyOutputs:   managedPolicyOutputNames(out),
 	}, nil
 }
+
+func secretOutputNames(outputs []addon.Output) []string {
+	var secrets []string
+	for _, out := range outputs {
+		if out.IsSecret {
+			secrets = append(secrets, out.Name)
+		}
+	}
+	return secrets
+}
+
+func managedPolicyOutputNames(outputs []addon.Output) []string {
+	var policies []string
+	for _, out := range outputs {
+		if out.IsManagedPolicy {
+			policies = append(policies, out.Name)
+		}
+	}
+	return policies
+}
+
+func envVarOutputNames(outputs []addon.Output) []string {
+	var envVars []string
+	for _, out := range outputs {
+		if !out.IsSecret && !out.IsManagedPolicy {
+			envVars = append(envVars, out.Name)
+		}
+	}
+	return envVars
+}

internal/pkg/manifest/backend_svc.go

@@ -4,9 +4,6 @@
 package manifest
 
 import (
-	"fmt"
-	"strconv"
-	"strings"
 	"time"
 
 	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/template"
@@ -90,16 +87,8 @@ func NewBackendService(props BackendServiceProps) *BackendService {
 // Implements the encoding.BinaryMarshaler interface.
 func (s *BackendService) MarshalBinary() ([]byte, error) {
 	content, err := s.parser.Parse(backendSvcManifestPath, *s, template.WithFuncs(map[string]interface{}{
-		"fmtSlice": func(elems []string) string {
-			return fmt.Sprintf("[%s]", strings.Join(elems, ", "))
-		},
-		"quoteSlice": func(elems []string) []string {
-			quotedElems := make([]string, len(elems))
-			for i, el := range elems {
-				quotedElems[i] = strconv.Quote(el)
-			}
-			return quotedElems
-		},
+		"fmtSlice":   template.FmtSliceFunc,
+		"quoteSlice": template.QuoteSliceFunc,
 	}))
 	if err != nil {
 		return nil, err