diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 15aaf476a01..7ec456ba0c9 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -12,6 +12,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/addon" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" ) // Parameter logical IDs for a backend service. @@ -56,7 +57,7 @@ func NewBackendService(mft *manifest.BackendService, env, app string, rc Runtime addons: addons, }, tc: mft.TaskConfig, - taskDefOverrideFunc: mockCloudFormationOverrideFunc, + taskDefOverrideFunc: override.CloudFormationTemplate, }, manifest: mft, diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index ef2cdccd2c9..a746a5935f8 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -12,6 +12,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/addon" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" ) // Template rendering configuration. @@ -64,7 +65,7 @@ func NewLoadBalancedWebService(mft *manifest.LoadBalancedWebService, env, app st addons: addons, }, tc: mft.TaskConfig, - taskDefOverrideFunc: mockCloudFormationOverrideFunc, + taskDefOverrideFunc: override.CloudFormationTemplate, }, manifest: mft, httpsEnabled: false, diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go index b748e6c5f2b..36bc0a93bf7 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -17,6 +17,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack/mocks" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" ) @@ -40,6 +41,10 @@ func (m mockTemplater) Template() (string, error) { return m.tpl, nil } +var mockCloudFormationOverrideFunc = func(overrideRules []override.Rule, origTemp []byte) ([]byte, error) { + return origTemp, nil +} + func TestLoadBalancedWebService_StackName(t *testing.T) { testCases := map[string]struct { inSvcName string diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job.go index 1aa5ec2bc97..96481110c18 100644 --- a/internal/pkg/deploy/cloudformation/stack/scheduled_job.go +++ b/internal/pkg/deploy/cloudformation/stack/scheduled_job.go @@ -16,6 +16,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/addon" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" "github.com/robfig/cron/v3" ) @@ -107,7 +108,7 @@ func NewScheduledJob(mft *manifest.ScheduledJob, env, app string, rc RuntimeConf addons: addons, }, tc: mft.TaskConfig, - taskDefOverrideFunc: mockCloudFormationOverrideFunc, + taskDefOverrideFunc: override.CloudFormationTemplate, }, manifest: mft, diff --git a/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go b/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go index 322735f297a..8c7f93c1224 100644 --- a/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go @@ -15,9 +15,7 @@ import ( ) const ( - manifestPath = "autoscaling-svc-manifest.yml" - wantedCFNTemplatePath = "autoscaling-svc-cf.yml" - wantedCFNParameterPath = "autoscaling-svc-cf.params.json" + autoScalingManifestPath = "manifest.yml" appName = "my-app" envName = "test" @@ -25,8 +23,13 @@ const ( imageTag = "latest" ) -func Test_Autoscaling_Integration(t *testing.T) { - path := filepath.Join("testdata", "autoscaling", manifestPath) +func Test_Stack_Local_Integration(t *testing.T) { + const ( + wantedAutoScalingCFNTemplatePath = "autoscaling-svc-cf.yml" + wantedAutoScalingCFNParameterPath = "cf.params.json" + wantedOverrideCFNTemplatePath = "override-cf.yml" + ) + path := filepath.Join("testdata", "stacklocal", autoScalingManifestPath) wantedManifestBytes, err := ioutil.ReadFile(path) require.NoError(t, err) mft, err := manifest.UnmarshalWorkload(wantedManifestBytes) @@ -44,7 +47,15 @@ func Test_Autoscaling_Integration(t *testing.T) { require.NoError(t, err) t.Run("CloudFormation template must contain autoscaling resources", func(t *testing.T) { - path := filepath.Join("testdata", "autoscaling", wantedCFNTemplatePath) + path := filepath.Join("testdata", "stacklocal", wantedAutoScalingCFNTemplatePath) + wantedCFNBytes, err := ioutil.ReadFile(path) + require.NoError(t, err) + + require.Contains(t, tpl, string(wantedCFNBytes)) + }) + + t.Run("CloudFormation template must be overridden correctly", func(t *testing.T) { + path := filepath.Join("testdata", "stacklocal", wantedOverrideCFNTemplatePath) wantedCFNBytes, err := ioutil.ReadFile(path) require.NoError(t, err) @@ -55,7 +66,7 @@ func Test_Autoscaling_Integration(t *testing.T) { params, err := serializer.SerializedParameters() require.NoError(t, err) - path := filepath.Join("testdata", "autoscaling", wantedCFNParameterPath) + path := filepath.Join("testdata", "stacklocal", wantedAutoScalingCFNParameterPath) wantedCFNParamsBytes, err := ioutil.ReadFile(path) require.NoError(t, err) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-cf.yml b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/autoscaling-svc-cf.yml similarity index 95% rename from internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-cf.yml rename to internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/autoscaling-svc-cf.yml index 510740114ba..fd2f136cb66 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-cf.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/autoscaling-svc-cf.yml @@ -9,7 +9,6 @@ Action: 'sts:AssumeRole' ManagedPolicyArns: - !Sub 'arn:${AWS::Partition}:iam::aws:policy/service-role/AmazonEC2ContainerServiceAutoscaleRole' - AutoScalingTarget: Metadata: 'aws:copilot:description': "An AutoScaling target to scale your service's desired count" @@ -21,13 +20,11 @@ Fn::Join: - '/' - - 'service' - - Fn::ImportValue: - !Sub '${AppName}-${EnvName}-ClusterId' + - Fn::ImportValue: !Sub '${AppName}-${EnvName}-ClusterId' - !GetAtt Service.Name ScalableDimension: ecs:service:DesiredCount ServiceNamespace: ecs RoleARN: !GetAtt AutoScalingRole.Arn - AutoScalingPolicyECSServiceAverageCPUUtilization: Type: AWS::ApplicationAutoScaling::ScalingPolicy Properties: @@ -40,7 +37,6 @@ ScaleInCooldown: 120 ScaleOutCooldown: 60 TargetValue: 70 - AutoScalingPolicyECSServiceAverageMemoryUtilization: Type: AWS::ApplicationAutoScaling::ScalingPolicy Properties: diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-cf.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json similarity index 100% rename from internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-cf.params.json rename to internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml similarity index 79% rename from internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-manifest.yml rename to internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml index 900858325f8..bb95b6f1bb2 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/autoscaling/autoscaling-svc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml @@ -32,6 +32,18 @@ count: # Enable running commands in your container. exec: true +taskdef_overrides: + - path: "ContainerDefinitions[0].Ulimits[-].Name" + value: "cpu" + - path: "ContainerDefinitions[0].Ulimits[0].SoftLimit" + value: 1024 + - path: "ContainerDefinitions[0].Ulimits[0].HardLimit" + value: 2048 + - path: "ContainerDefinitions[0].PortMappings[-].ContainerPort" + value: 2056 + - path: "ContainerDefinitions[0].PortMappings[1].Protocol" + value: "udp" + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/override-cf.yml b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/override-cf.yml new file mode 100644 index 00000000000..f1ecc15dd94 --- /dev/null +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/override-cf.yml @@ -0,0 +1,45 @@ + TaskDefinition: + Metadata: + 'aws:copilot:description': 'An ECS task definition to group your containers and run them on ECS' + Type: AWS::ECS::TaskDefinition + DependsOn: LogGroup + Properties: + Family: !Join ['', [!Ref AppName, '-', !Ref EnvName, '-', !Ref WorkloadName]] + NetworkMode: awsvpc + RequiresCompatibilities: + - FARGATE + Cpu: !Ref TaskCPU + Memory: !Ref TaskMemory + ExecutionRoleArn: !Ref ExecutionRole + TaskRoleArn: !Ref TaskRole + ContainerDefinitions: + - Name: !Ref WorkloadName + Image: !Ref ContainerImage + # We pipe certain environment variables directly into the task definition. + # This lets customers have access to, for example, their LB endpoint - which they'd + # have no way of otherwise determining. + Environment: + - Name: COPILOT_APPLICATION_NAME + Value: !Sub '${AppName}' + - Name: COPILOT_SERVICE_DISCOVERY_ENDPOINT + Value: + - Name: COPILOT_ENVIRONMENT_NAME + Value: !Sub '${EnvName}' + - Name: COPILOT_SERVICE_NAME + Value: !Sub '${WorkloadName}' + - Name: COPILOT_LB_DNS + Value: !GetAtt EnvControllerAction.PublicLoadBalancerDNSName + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + PortMappings: + - ContainerPort: !Ref ContainerPort + - ContainerPort: 2056 + Protocol: "udp" + Ulimits: + - Name: "cpu" + SoftLimit: 1024 + HardLimit: 2048 \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 88befdcf7e6..523e44b54da 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "hash/crc32" + "regexp" "strings" "time" @@ -59,6 +60,9 @@ const ( var ( errEphemeralBadSize = errors.New("ephemeral storage must be between 20 GiB and 200 GiB") errInvalidSpotConfig = errors.New(`"count.spot" and "count.range" cannot be specified together`) + + taskDefOverrideRulePrefixes = []string{"Resources", "TaskDefinition", "Properties"} + invalidTaskDefOverridePathRegexp = []string{`Family`, `ContainerDefinitions\[\d+\].Name`} ) type convertSidecarOpts struct { @@ -333,15 +337,29 @@ func logConfigOpts(lc *manifest.Logging) *template.LogConfigOpts { func convertTaskDefOverrideRules(inRules []manifest.OverrideRule) []override.Rule { var res []override.Rule + suffixStr := strings.Join(taskDefOverrideRulePrefixes, override.PathSegmentSeparator) for _, r := range inRules { + if !isValidTaskDefOverridePath(r.Path) { + continue + } res = append(res, override.Rule{ - Path: r.Path, + Path: strings.Join([]string{suffixStr, r.Path}, override.PathSegmentSeparator), Value: r.Value, }) } return res } +func isValidTaskDefOverridePath(path string) bool { + for _, s := range invalidTaskDefOverridePathRegexp { + re := regexp.MustCompile(fmt.Sprintf(`^%s$`, s)) + if re.MatchString(path) { + return false + } + } + return true +} + // convertStorageOpts converts a manifest Storage field into template data structures which can be used // to execute CFN templates func convertStorageOpts(wlName *string, in *manifest.Storage) (*template.StorageOpts, error) { diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index d54da871ca4..fcef36ef001 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -11,7 +11,9 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func Test_convertSidecar(t *testing.T) { @@ -545,6 +547,44 @@ func Test_convertAutoscaling(t *testing.T) { } } +func Test_convertTaskDefOverrideRules(t *testing.T) { + testCases := map[string]struct { + inRule []manifest.OverrideRule + + wanted []override.Rule + }{ + "should have proper prefix": { + inRule: []manifest.OverrideRule{ + { + Path: "ContainerDefinitions[0].Ulimits[-].HardLimit", + Value: yaml.Node{}, + }, + { + Path: "ContainerDefinitions[0].Name", + Value: yaml.Node{}, + }, + { + Path: "Family", + Value: yaml.Node{}, + }, + }, + wanted: []override.Rule{ + { + Path: "Resources.TaskDefinition.Properties.ContainerDefinitions[0].Ulimits[-].HardLimit", + Value: yaml.Node{}, + }, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got := convertTaskDefOverrideRules(tc.inRule) + + require.Equal(t, tc.wanted, got) + }) + } +} + func Test_convertHTTPHealthCheck(t *testing.T) { // These are used by reference to represent the output of the manifest.durationp function. duration15Seconds := 15 * time.Second diff --git a/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go b/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go index 5d99dfd2f92..b28ae726794 100644 --- a/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go @@ -18,7 +18,7 @@ import ( ) func TestAutoscalingIntegration_Validate(t *testing.T) { - path := filepath.Join("testdata", "autoscaling", manifestPath) + path := filepath.Join("testdata", "stacklocal", autoScalingManifestPath) wantedManifestBytes, err := ioutil.ReadFile(path) require.NoError(t, err) mft, err := manifest.UnmarshalWorkload(wantedManifestBytes) diff --git a/internal/pkg/deploy/cloudformation/stack/worker_svc.go b/internal/pkg/deploy/cloudformation/stack/worker_svc.go index d3566e8d28e..0940e3a8474 100644 --- a/internal/pkg/deploy/cloudformation/stack/worker_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/worker_svc.go @@ -11,6 +11,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/addon" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" + "github.com/aws/copilot-cli/internal/pkg/template/override" ) type workerSvcReadParser interface { @@ -46,7 +47,7 @@ func NewWorkerService(mft *manifest.WorkerService, env, app string, rc RuntimeCo addons: addons, }, tc: mft.TaskConfig, - taskDefOverrideFunc: mockCloudFormationOverrideFunc, + taskDefOverrideFunc: override.CloudFormationTemplate, }, manifest: mft, allowedTopics: allowedTopics, diff --git a/internal/pkg/deploy/cloudformation/stack/workload.go b/internal/pkg/deploy/cloudformation/stack/workload.go index 8b05d39d7b4..0a53cd4ec78 100644 --- a/internal/pkg/deploy/cloudformation/stack/workload.go +++ b/internal/pkg/deploy/cloudformation/stack/workload.go @@ -255,11 +255,6 @@ func envVarOutputNames(outputs []addon.Output) []string { return envVars } -// TODO: Remove this mock function and use override.CloudFormationTemplate when it is finished. -var mockCloudFormationOverrideFunc = func(overrideRules []override.Rule, origTemp []byte) ([]byte, error) { - return origTemp, nil -} - type ecsWkld struct { *wkld tc manifest.TaskConfig diff --git a/internal/pkg/template/override/rule.go b/internal/pkg/template/override/rule.go index 07ed3397880..e24111f32ef 100644 --- a/internal/pkg/template/override/rule.go +++ b/internal/pkg/template/override/rule.go @@ -13,9 +13,10 @@ import ( ) const ( + // PathSegmentSeparator is the separator for path segments. + PathSegmentSeparator = "." // seqAppendToLastSymbol is the symbol used to add a node to the tail of a list. seqAppendToLastSymbol = "-" - pathSegmentSeparator = "." ) // Subset of YAML tag values: http://yaml.org/type/ @@ -51,7 +52,7 @@ func (r Rule) validate() error { if r.Path == "" { return fmt.Errorf("rule path is empty") } - pathSegments := strings.Split(r.Path, pathSegmentSeparator) + pathSegments := strings.Split(r.Path, PathSegmentSeparator) for _, pathSegment := range pathSegments { if !pathSegmentRegexp.MatchString(pathSegment) { return fmt.Errorf(`invalid override path segment "%s": segments must be of the form "array[0]", "array[%s]" or "key"`, @@ -62,7 +63,7 @@ func (r Rule) validate() error { } func (r Rule) parse() (nodeUpserter, error) { - pathSegments := strings.SplitN(r.Path, pathSegmentSeparator, 2) + pathSegments := strings.SplitN(r.Path, PathSegmentSeparator, 2) segment, err := parsePathSegment(pathSegments[0]) if err != nil { return nil, err