From ca3ed84e73602ea1256767d6e045a5a3a03c6831 Mon Sep 17 00:00:00 2001 From: danny randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 7 Oct 2022 13:46:12 -0700 Subject: [PATCH 01/10] fix: allow cdn certificate import without http cert import (#4061) Also fixes a bug where CDN fails to create if there are any internal ALB services deployed. Also fixes a bug where on `svc/env package` we don't escape param values By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- cf-custom-resources/lib/unique-json-values.js | 25 ++++- .../test/unique-json-values-test.js | 92 +++++++++++++--- internal/pkg/cli/deploy/env.go | 9 +- internal/pkg/cli/deploy/env_test.go | 47 ++++++-- internal/pkg/cli/deploy/errors.go | 7 +- internal/pkg/cli/deploy/svc.go | 19 ++-- internal/pkg/cli/deploy/svc_test.go | 25 ++++- internal/pkg/cli/env_package.go | 1 + .../cloudformation/stack/lb_web_svc_test.go | 103 ++++++++---------- .../cloudformation/stack/rd_web_svc_test.go | 89 ++++++--------- .../stack/scheduled_job_test.go | 96 +++++++--------- ...late-with-imported-certs-observability.yml | 1 + .../stack/testdata/stacklocal/cf.params.json | 28 ++--- .../testdata/workloads/job-test.params.json | 20 ++-- .../workloads/svc-grpc-test.params.json | 28 ++--- .../workloads/svc-nlb-dev.params.json | 30 ++--- .../workloads/svc-nlb-prod.params.json | 26 ++--- .../workloads/svc-nlb-test.params.json | 26 ++--- .../testdata/workloads/svc-prod.params.json | 28 ++--- .../workloads/svc-staging.params.json | 28 ++--- .../testdata/workloads/svc-test.params.json | 28 ++--- .../workloads/windows-svc-test.params.json | 28 ++--- .../workloads/worker-test.params.json | 18 +-- .../deploy/cloudformation/stack/workload.go | 37 ++++--- internal/pkg/manifest/validate_env.go | 7 +- internal/pkg/manifest/validate_env_test.go | 14 ++- internal/pkg/template/env.go | 8 ++ .../environment/partials/cdn-resources.yml | 1 + .../partials/environment-manager-role.yml | 2 +- .../templates/workloads/params.json.tmpl | 8 -- 30 files changed, 493 insertions(+), 386 deletions(-) delete mode 100644 internal/pkg/template/templates/workloads/params.json.tmpl diff --git a/cf-custom-resources/lib/unique-json-values.js b/cf-custom-resources/lib/unique-json-values.js index fd9d2d19d50..75fc17f7fbe 100644 --- a/cf-custom-resources/lib/unique-json-values.js +++ b/cf-custom-resources/lib/unique-json-values.js @@ -81,10 +81,20 @@ const deadlineExpired = function () { * { * "svc1": ["svc1.com", "example.com"], * "svc2": ["example.com"] + * "svc3": ["svc3.com"] * } * - * This function returns a list of unique values found in that list. - * From the previous example, UniqueValues would be: + * The input event.ResourceProperties.FilterFor is a comma delimated list + * of keys in event.ResourceProperties.Aliases to filter for. + * Taking the above map, and event.ResourceProperties.FilterFor = "svc1,svc2" + * The services considered for uniqueness is: + * { + * "svc1": ["svc1.com", "example.com"], + * "svc2": ["example.com"] + * } + * + * This function returns a list of unique values found in this map. + * For this example, UniqueValues would be: * ["svc1.com", "example.com"] */ exports.handler = async function (event, context) { @@ -96,7 +106,13 @@ exports.handler = async function (event, context) { case "Create": case "Update": const aliasesForService = JSON.parse(event.ResourceProperties.Aliases || "{}"); - const unique = new Set(Object.values(aliasesForService).flat()); + const filterFor = new Set(event.ResourceProperties.FilterFor.split(",")); + const filteredAliasesForService = Object.fromEntries( + Object.entries(aliasesForService).filter( + ([key]) => filterFor.has(key) + ) + ); + const unique = new Set(Object.values(filteredAliasesForService).flat()); responseData.UniqueValues = Array.from(unique).sort(); break; case "Delete": @@ -104,9 +120,10 @@ exports.handler = async function (event, context) { break; default: throw new Error(`Unsupported request type ${event.RequestType}`); - } + }; }; + try { await Promise.race([deadlineExpired(), handler()]); await report(event, context, "SUCCESS", physicalResourceId, responseData); diff --git a/cf-custom-resources/test/unique-json-values-test.js b/cf-custom-resources/test/unique-json-values-test.js index 59df8cbe4b3..b3a53e31b9d 100644 --- a/cf-custom-resources/test/unique-json-values-test.js +++ b/cf-custom-resources/test/unique-json-values-test.js @@ -75,8 +75,8 @@ describe("Unique Aliases", () => { }); }); - const aliasTest = (name, input, expectedOutput) => { - const tt = (name, reqType, input, expectedOutput) => { + const aliasTest = (name, props, expectedOutput) => { + const tt = (name, reqType, props, expectedOutput) => { test(name, () => { const request = nock(responseURL) .put("/", (body) => { @@ -95,9 +95,7 @@ describe("Unique Aliases", () => { ResponseURL: responseURL, RequestType: reqType, RequestId: testRequestId, - ResourceProperties: { - Aliases: JSON.stringify(input), // aliases get passed as a string - }, + ResourceProperties: props, LogicalResourceId: "mockID", }) .expectResolve(() => { @@ -106,28 +104,90 @@ describe("Unique Aliases", () => { }); }; - tt(`Create/${name}`, "Create", input, expectedOutput); - tt(`Update/${name}`, "Update", input, expectedOutput); + tt(`Create/${name}`, "Create", props, expectedOutput); + tt(`Update/${name}`, "Update", props, expectedOutput); }; - aliasTest("no aliases", {}, []); + aliasTest("no aliases", { + Aliases: "", + FilterFor: "" + }, []); aliasTest("one service", { - "svc1": ["svc1.com", "example.com"], + Aliases: JSON.stringify({ + "svc1": ["svc1.com", "example.com"] + }), + FilterFor: "svc1", }, ["example.com", "svc1.com"]); + aliasTest("one service excluded", { + Aliases: JSON.stringify({ + "svc1": ["svc1.com", "example.com"] + }), + FilterFor: "svc2", + }, []); + + aliasTest("one service empty filter for", { + Aliases: JSON.stringify({ + "svc1": ["svc1.com", "example.com"] + }), + FilterFor: "", + }, []); + aliasTest("two services no common aliases", { - "svc1": ["svc1.com"], - "svc2": ["svc2.com"] + Aliases: JSON.stringify({ + "svc1": ["svc1.com"], + "svc2": ["svc2.com"] + }), + FilterFor: "svc1,svc2", }, ["svc1.com", "svc2.com"]); aliasTest("two services, one with multiple common aliases", { - "svc1": ["svc1.com"], - "svc2": ["svc2.com", "example.com"] + Aliases: JSON.stringify({ + "svc1": ["svc1.com"], + "svc2": ["svc2.com", "example.com"] + }), + FilterFor: "svc1,svc2", }, ["example.com", "svc1.com", "svc2.com"]); aliasTest("two services with a common alias", { - "svc1": ["svc1.com", "example.com"], - "svc2": ["svc2.com", "example.com"] + Aliases: JSON.stringify({ + "svc1": ["svc1.com", "example.com"], + "svc2": ["svc2.com", "example.com"] + }), + FilterFor: "svc1,svc2", }, ["example.com", "svc1.com", "svc2.com"]); -}); \ No newline at end of file + + aliasTest("three services with a common alias one service filtered out", { + Aliases: JSON.stringify({ + "svc1": ["svc1.com", "example.com"], + "svc2": ["svc2.com", "example.com", "example2.com"], + "svc3": ["svc3.com", "example.com", "example2.com"] + }), + FilterFor: "svc2,svc1", + }, ["example.com", "example2.com", "svc1.com", "svc2.com"]); + + aliasTest("bunch of services with single alias, some filtered out, out of order", { + Aliases: JSON.stringify({ + "lbws3": ["three.lbws.com"], + "backend1": ["one.backend.internal"], + "lbws1": ["one.lbws.com"], + "lbws4": ["four.lbws.com"], + "backend2": ["two.backend.internal"], + "lbws2": ["two.lbws.com"], + "lbws6": ["lbws.com"], + "lbws5": ["lbws.com"], + }), + FilterFor: "lbws2,lbws3,lbws4,lbws1,lbws5,lbws6", + }, ["four.lbws.com", "lbws.com", "one.lbws.com", "three.lbws.com", "two.lbws.com"]); + + aliasTest("bunch of services with single alias, some FilterFor services don't exist", { + Aliases: JSON.stringify({ + "lbws1": ["lbws1.com"], + "lbws2": ["lbws2.com"], + "lbws3": ["lbws3.com"], + "lbws4": ["lbws4.com"], + }), + FilterFor: "lbws1,lbws5,lbws6,lbws3", + }, ["lbws1.com", "lbws3.com"]); +}); diff --git a/internal/pkg/cli/deploy/env.go b/internal/pkg/cli/deploy/env.go index d250d1aacbb..9c34706be7d 100644 --- a/internal/pkg/cli/deploy/env.go +++ b/internal/pkg/cli/deploy/env.go @@ -160,7 +160,7 @@ func (d *envDeployer) validateCDN(mft *manifest.Environment) error { } } - if mft.CDNEnabled() && mft.CDNDoesTLSTermination() && (mft.HasImportedPublicALBCerts() || d.app.Domain != "") { + if mft.CDNEnabled() && mft.CDNDoesTLSTermination() { err := d.validateALBWorkloadsDontRedirect() var redirErr *errEnvHasPublicServicesWithRedirect switch { @@ -222,9 +222,7 @@ func (d *envDeployer) validateALBWorkloadsDontRedirect() error { } // lbServiceRedirects returns true if svc's HTTP listener rule redirects. We only check -// HTTPListenerRuleWithDomain because HTTPListenerRule: -// a) doesn't ever redirect -// b) won't work with cloudfront anyways (can't point ALB default DNS to CF) +// HTTPListenerRuleWithDomain because HTTPListenerRule doesn't ever redirect. func (d *envDeployer) lbServiceRedirects(ctx context.Context, svc string) (bool, error) { stackDescriber := d.newServiceStackDescriber(svc) resources, err := stackDescriber.Resources() @@ -240,7 +238,8 @@ func (d *envDeployer) lbServiceRedirects(ctx context.Context, svc string) (bool, } } if ruleARN == "" { - return false, fmt.Errorf("http listener not found on service %q", svc) + // this will happen if the service doesn't support https. + return false, nil } rule, err := d.lbDescriber.DescribeRule(ctx, ruleARN) diff --git a/internal/pkg/cli/deploy/env_test.go b/internal/pkg/cli/deploy/env_test.go index b60fc81dd10..599e2d0df9e 100644 --- a/internal/pkg/cli/deploy/env_test.go +++ b/internal/pkg/cli/deploy/env_test.go @@ -415,7 +415,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, }, } - mftCDNTerminateTLS := &manifest.Environment{ + mftCDNTerminateTLSAndHTTPCert := &manifest.Environment{ EnvironmentConfig: manifest.EnvironmentConfig{ HTTPConfig: manifest.EnvironmentHTTPConfig{ Public: manifest.PublicHTTPConfig{ @@ -429,6 +429,16 @@ func TestEnvDeployer_Validate(t *testing.T) { }, }, } + mftCDNTerminateTLS := &manifest.Environment{ + EnvironmentConfig: manifest.EnvironmentConfig{ + CDNConfig: manifest.EnvironmentCDNConfig{ + Config: manifest.AdvancedCDNConfig{ + Certificate: aws.String("mockCDNCertARN"), + TerminateTLS: aws.Bool(true), + }, + }, + }, + } tests := map[string]struct { app *config.Application mft *manifest.Environment @@ -436,7 +446,7 @@ func TestEnvDeployer_Validate(t *testing.T) { expected string expectedStdErr string }{ - "cdn enabled, domain imported, no public http certs and validate aliases fails": { + "cdn enabled, domain imported, no public http certs, and validate aliases fails": { app: &config.Application{ Domain: "example.com", }, @@ -452,7 +462,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, expected: "some error", }, - "cdn enabled, domain imported, no public http certs and validate aliases succeeds": { + "cdn enabled, domain imported, no public http certs, and validate aliases succeeds": { app: &config.Application{ Domain: "example.com", }, @@ -469,7 +479,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, "cdn tls termination enabled, fail to get env stack params": { app: &config.Application{}, - mft: mftCDNTerminateTLS, + mft: mftCDNTerminateTLSAndHTTPCert, setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { m.envDescriber.EXPECT().Params().Return(nil, errors.New("some error")) }, @@ -477,7 +487,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, "cdn tls termination enabled, fail to get service resources": { app: &config.Application{}, - mft: mftCDNTerminateTLS, + mft: mftCDNTerminateTLSAndHTTPCert, setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { m.stackDescribers = map[string]*mocks.MockstackDescriber{ "svc1": mocks.NewMockstackDescriber(ctrl), @@ -492,7 +502,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, "cdn tls termination enabled, fail to check listener rule": { app: &config.Application{}, - mft: mftCDNTerminateTLS, + mft: mftCDNTerminateTLSAndHTTPCert, setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { m.stackDescribers = map[string]*mocks.MockstackDescriber{ "svc1": mocks.NewMockstackDescriber(ctrl), @@ -513,7 +523,7 @@ func TestEnvDeployer_Validate(t *testing.T) { }, "cdn tls termination enabled, warn with one service that doesn't redirect, two that do redirect": { app: &config.Application{}, - mft: mftCDNTerminateTLS, + mft: mftCDNTerminateTLSAndHTTPCert, setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { m.stackDescribers = map[string]*mocks.MockstackDescriber{ "svc1": mocks.NewMockstackDescriber(ctrl), @@ -551,7 +561,7 @@ These services will not be reachable through the CDN. To fix this, set the following field in each manifest: %s http: - redirect_to_https: true + redirect_to_https: false %s and run %scopilot svc deploy%s. If you'd like to use these services without a CDN, ensure each service's A record is pointed to the ALB. @@ -617,7 +627,7 @@ If you'd like to use these services without a CDN, ensure each service's A recor }, "cdn tls termination enabled, success with three services that don't redirect": { app: &config.Application{}, - mft: mftCDNTerminateTLS, + mft: mftCDNTerminateTLSAndHTTPCert, setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { m.stackDescribers = map[string]*mocks.MockstackDescriber{ "svc1": mocks.NewMockstackDescriber(ctrl), @@ -651,6 +661,25 @@ If you'd like to use these services without a CDN, ensure each service's A recor m.lbDescriber.EXPECT().DescribeRule(gomock.Any(), "svc3RuleARN").Return(listenerRuleNoRedirect, nil) }, }, + "cdn tls termination enabled, one http only service deployed": { + app: &config.Application{}, + mft: mftCDNTerminateTLS, + setUpMocks: func(m *envDeployerMocks, ctrl *gomock.Controller) { + m.stackDescribers = map[string]*mocks.MockstackDescriber{ + "svc1": mocks.NewMockstackDescriber(ctrl), + } + m.envDescriber.EXPECT().Params().Return(map[string]string{ + "ALBWorkloads": "svc1", + }, nil) + m.stackDescribers["svc1"].EXPECT().Resources().Return([]*stack.Resource{ + { + LogicalID: "HTTPListenerRuleWithDomain", + PhysicalID: "svc1RuleARN", + }, + }, nil) + m.lbDescriber.EXPECT().DescribeRule(gomock.Any(), "svc1RuleARN").Return(listenerRuleNoRedirect, nil) + }, + }, } for name, tc := range tests { diff --git a/internal/pkg/cli/deploy/errors.go b/internal/pkg/cli/deploy/errors.go index b2ed47b31a0..32d33f2bca1 100644 --- a/internal/pkg/cli/deploy/errors.go +++ b/internal/pkg/cli/deploy/errors.go @@ -46,7 +46,10 @@ type errEnvHasPublicServicesWithRedirect struct { } func (e *errEnvHasPublicServicesWithRedirect) Error() string { - return fmt.Sprintf("%v services redirect HTTP to HTTPS", len(e.services)) + return fmt.Sprintf("%v %s HTTP to HTTPS", + len(e.services), + english.PluralWord(len(e.services), "service redirects", "services redirect"), + ) } // RecommendActions returns recommended actions to be taken after the error. @@ -68,7 +71,7 @@ and run %s.`, english.PluralWord(n, "redirects", "redirect"), color.Emphasize(english.PluralWord(n, "This service", "These services")+" will not be reachable through the CDN."), english.PluralWord(n, "its", "each"), - color.HighlightCodeBlock("http:\n redirect_to_https: true"), + color.HighlightCodeBlock("http:\n redirect_to_https: false"), color.HighlightCode("copilot svc deploy"), ) } diff --git a/internal/pkg/cli/deploy/svc.go b/internal/pkg/cli/deploy/svc.go index 5cd89259a54..74815000114 100644 --- a/internal/pkg/cli/deploy/svc.go +++ b/internal/pkg/cli/deploy/svc.go @@ -1360,7 +1360,9 @@ func (d *backendSvcDeployer) validateALBRuntime() error { } func (d *lbWebSvcDeployer) validateALBRuntime() error { - hasImportedCerts := len(d.envConfig.HTTPConfig.Public.Certificates) != 0 + hasALBCerts := len(d.envConfig.HTTPConfig.Public.Certificates) != 0 + hasCDNCerts := d.envConfig.CDNConfig.Config.Certificate != nil + hasImportedCerts := hasALBCerts || hasCDNCerts if d.lbMft.RoutingRule.RedirectToHTTPS != nil && d.app.Domain == "" && !hasImportedCerts { return fmt.Errorf("cannot configure http to https redirect without having a domain associated with the app %q or importing any certificates in env %q", d.app.Name, d.env.Name) } @@ -1390,14 +1392,15 @@ func (d *lbWebSvcDeployer) validateALBRuntime() error { return fmt.Errorf("convert aliases to string slice: %w", err) } - cdnCert := d.envConfig.CDNConfig.Config.Certificate - albCertValidator := d.newAliasCertValidator(nil) - cfCertValidator := d.newAliasCertValidator(aws.String(cloudfront.CertRegion)) - if err := albCertValidator.ValidateCertAliases(aliases, d.envConfig.HTTPConfig.Public.Certificates); err != nil { - return fmt.Errorf("validate aliases against the imported public ALB certificate for env %s: %w", d.env.Name, err) + if hasALBCerts { + albCertValidator := d.newAliasCertValidator(nil) + if err := albCertValidator.ValidateCertAliases(aliases, d.envConfig.HTTPConfig.Public.Certificates); err != nil { + return fmt.Errorf("validate aliases against the imported public ALB certificate for env %s: %w", d.env.Name, err) + } } - if cdnCert != nil { - if err := cfCertValidator.ValidateCertAliases(aliases, []string{*cdnCert}); err != nil { + if hasCDNCerts { + cfCertValidator := d.newAliasCertValidator(aws.String(cloudfront.CertRegion)) + if err := cfCertValidator.ValidateCertAliases(aliases, []string{*d.envConfig.CDNConfig.Config.Certificate}); err != nil { return fmt.Errorf("validate aliases against the imported CDN certificate for env %s: %w", d.env.Name, err) } } diff --git a/internal/pkg/cli/deploy/svc_test.go b/internal/pkg/cli/deploy/svc_test.go index 074a8da2804..ff1528dda25 100644 --- a/internal/pkg/cli/deploy/svc_test.go +++ b/internal/pkg/cli/deploy/svc_test.go @@ -993,7 +993,7 @@ func TestWorkloadDeployer_DeployWorkload(t *testing.T) { m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), "mockBucket", gomock.Any()).Return(nil) }, }, - "success with http redirect disabled and certs imported": { + "success with http redirect disabled and alb certs imported": { inRedirectToHTTPS: aws.Bool(false), inAliases: manifest.Alias{ AdvancedAliases: mockMultiAliases, @@ -1017,6 +1017,29 @@ func TestWorkloadDeployer_DeployWorkload(t *testing.T) { m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), "mockBucket", gomock.Any()).Return(nil) }, }, + "success with only cdn certs imported": { + inAliases: manifest.Alias{ + AdvancedAliases: mockMultiAliases, + }, + inEnvironment: &config.Environment{ + Name: mockEnvName, + Region: "us-west-2", + }, + inEnvironmentConfig: func() *manifest.Environment { + envConfig := &manifest.Environment{} + envConfig.CDNConfig.Config.Certificate = aws.String(mockCDNCertARN) + return envConfig + }, + inApp: &config.Application{ + Name: mockAppName, + }, + mock: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockValidator.EXPECT().ValidateCertAliases([]string{"example.com", "foobar.com"}, []string{mockCDNCertARN}).Return(nil) + m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), "mockBucket", gomock.Any()).Return(nil) + }, + }, "success with http redirect disabled and domain imported": { inRedirectToHTTPS: aws.Bool(false), inAliases: manifest.Alias{ diff --git a/internal/pkg/cli/env_package.go b/internal/pkg/cli/env_package.go index 9c1ea684c41..108160a8ebd 100644 --- a/internal/pkg/cli/env_package.go +++ b/internal/pkg/cli/env_package.go @@ -113,6 +113,7 @@ func newPackageEnvOpts(vars packageEnvVars) (*packageEnvOpts, error) { App: appCfg, Env: envCfg, SessionProvider: sessProvider, + ConfigStore: opts.cfgStore, }) } return opts, nil 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 cabce3e5f45..066207281dc 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -905,7 +905,7 @@ func TestLoadBalancedWebService_Parameters(t *testing.T) { } func TestLoadBalancedWebService_SerializedParameters(t *testing.T) { - var testLBWebServiceManifest = manifest.NewLoadBalancedWebService(&manifest.LoadBalancedWebServiceProps{ + testLBWebServiceManifest := manifest.NewLoadBalancedWebService(&manifest.LoadBalancedWebServiceProps{ WorkloadProps: &manifest.WorkloadProps{ Name: "frontend", Dockerfile: "frontend/Dockerfile", @@ -913,66 +913,57 @@ func TestLoadBalancedWebService_SerializedParameters(t *testing.T) { Path: "frontend", Port: 80, }) - testCases := map[string]struct { - mockDependencies func(ctrl *gomock.Controller, c *LoadBalancedWebService) - wantedParams string - wantedError error - }{ - "unavailable template": { - mockDependencies: func(ctrl *gomock.Controller, c *LoadBalancedWebService) { - m := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")) - c.wkld.parser = m - }, - wantedParams: "", - wantedError: errors.New("some error"), - }, - "render params template": { - mockDependencies: func(ctrl *gomock.Controller, c *LoadBalancedWebService) { - m := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("params")}, nil) - c.wkld.parser = m + c := &LoadBalancedWebService{ + ecsWkld: &ecsWkld{ + wkld: &wkld{ + name: "frontend", + env: testEnvName, + app: testAppName, + rc: RuntimeConfig{ + Image: &ECRImage{ + RepoURL: testImageRepoURL, + ImageTag: testImageTag, + }, + AdditionalTags: map[string]string{ + "owner": "boss", + }, + }, }, - wantedParams: "params", + tc: testLBWebServiceManifest.TaskConfig, }, + manifest: testLBWebServiceManifest, } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // GIVEN - ctrl := gomock.NewController(t) - defer ctrl.Finish() - c := &LoadBalancedWebService{ - ecsWkld: &ecsWkld{ - wkld: &wkld{ - name: aws.StringValue(testLBWebServiceManifest.Name), - env: testEnvName, - app: testAppName, - rc: RuntimeConfig{ - Image: &ECRImage{ - RepoURL: testImageRepoURL, - ImageTag: testImageTag, - }, - AdditionalTags: map[string]string{ - "owner": "boss", - }, - }, - }, - tc: testLBWebServiceManifest.TaskConfig, - }, - manifest: testLBWebServiceManifest, - } - tc.mockDependencies(ctrl, c) - - // WHEN - params, err := c.SerializedParameters() - - // THEN - require.Equal(t, tc.wantedError, err) - require.Equal(t, tc.wantedParams, params) - }) - } + params, err := c.SerializedParameters() + require.NoError(t, err) + require.Equal(t, params, `{ + "Parameters": { + "AddonsTemplateURL": "", + "AppName": "phonetool", + "ContainerImage": "111111111111.dkr.ecr.us-west-2.amazonaws.com/phonetool/frontend:manual-bf3678c", + "ContainerPort": "80", + "DNSDelegated": "false", + "EnvFileARN": "", + "EnvName": "test", + "HTTPSEnabled": "false", + "LogRetention": "30", + "RulePath": "frontend", + "Stickiness": "false", + "TargetContainer": "frontend", + "TargetPort": "80", + "TaskCPU": "256", + "TaskCount": "1", + "TaskMemory": "512", + "WorkloadName": "frontend" + }, + "Tags": { + "copilot-application": "phonetool", + "copilot-environment": "test", + "copilot-service": "frontend", + "owner": "boss" + } +}`) } func TestLoadBalancedWebService_Tags(t *testing.T) { diff --git a/internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go index edd7b45b65d..ae77d1f53ce 100644 --- a/internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go @@ -444,62 +444,43 @@ func TestRequestDrivenWebService_Parameters(t *testing.T) { } func TestRequestDrivenWebService_SerializedParameters(t *testing.T) { - testCases := map[string]struct { - mockDependencies func(ctrl *gomock.Controller, c *RequestDrivenWebService) - - wantedParams string - wantedError error - }{ - "unavailable template": { - mockDependencies: func(ctrl *gomock.Controller, c *RequestDrivenWebService) { - m := mocks.NewMockrequestDrivenWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(nil, errors.New("serialization error")) - c.wkld.parser = m - }, - wantedParams: "", - wantedError: errors.New("serialization error"), - }, - "render params template": { - mockDependencies: func(ctrl *gomock.Controller, c *RequestDrivenWebService) { - m := mocks.NewMockrequestDrivenWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("params")}, nil) - c.wkld.parser = m + c := &RequestDrivenWebService{ + appRunnerWkld: &appRunnerWkld{ + wkld: &wkld{ + name: aws.StringValue(testRDWebServiceManifest.Name), + env: testEnvName, + app: testAppName, + rc: RuntimeConfig{ + Image: &ECRImage{ + RepoURL: testImageRepoURL, + ImageTag: testImageTag, + }, + }, }, - wantedParams: "params", + instanceConfig: testRDWebServiceManifest.InstanceConfig, + imageConfig: testRDWebServiceManifest.ImageConfig, }, + manifest: testRDWebServiceManifest, } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // GIVEN - ctrl := gomock.NewController(t) - defer ctrl.Finish() - c := &RequestDrivenWebService{ - appRunnerWkld: &appRunnerWkld{ - wkld: &wkld{ - name: aws.StringValue(testRDWebServiceManifest.Name), - env: testEnvName, - app: testAppName, - rc: RuntimeConfig{ - Image: &ECRImage{ - RepoURL: testImageRepoURL, - ImageTag: testImageTag, - }, - }, - }, - instanceConfig: testRDWebServiceManifest.InstanceConfig, - imageConfig: testRDWebServiceManifest.ImageConfig, - }, - manifest: testRDWebServiceManifest, - } - tc.mockDependencies(ctrl, c) - - // WHEN - params, err := c.SerializedParameters() - - // THEN - require.Equal(t, tc.wantedError, err) - require.Equal(t, tc.wantedParams, params) - }) - } + params, err := c.SerializedParameters() + require.NoError(t, err) + require.Equal(t, params, `{ + "Parameters": { + "AddonsTemplateURL": "", + "AppName": "phonetool", + "ContainerImage": "111111111111.dkr.ecr.us-west-2.amazonaws.com/phonetool/frontend:manual-bf3678c", + "ContainerPort": "80", + "EnvName": "test", + "ImageRepositoryType": "ECR", + "InstanceCPU": "256", + "InstanceMemory": "512", + "WorkloadName": "frontend" + }, + "Tags": { + "copilot-application": "phonetool", + "copilot-environment": "test", + "copilot-service": "frontend" + } +}`) } diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go index b917c0fc465..a4295bc4c5d 100644 --- a/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go +++ b/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go @@ -554,64 +554,48 @@ func TestScheduledJob_SerializedParameters(t *testing.T) { Timeout: "1h30m", Retries: 3, }) - testCases := map[string]struct { - mockDependencies func(ctrl *gomock.Controller, c *ScheduledJob) - wantedParams string - wantedError error - }{ - "unavailable template": { - mockDependencies: func(ctrl *gomock.Controller, c *ScheduledJob) { - m := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")) - c.wkld.parser = m - }, - wantedParams: "", - wantedError: errors.New("some error"), - }, - "render params template": { - mockDependencies: func(ctrl *gomock.Controller, c *ScheduledJob) { - m := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) - m.EXPECT().Parse(wkldParamsTemplatePath, gomock.Any(), gomock.Any()).Return(&template.Content{Buffer: bytes.NewBufferString("params")}, nil) - c.wkld.parser = m - }, - wantedParams: "params", - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // GIVEN - ctrl := gomock.NewController(t) - defer ctrl.Finish() - c := &ScheduledJob{ - ecsWkld: &ecsWkld{ - wkld: &wkld{ - name: aws.StringValue(testScheduledJobManifest.Name), - env: testEnvName, - app: testAppName, - rc: RuntimeConfig{ - Image: &ECRImage{ - RepoURL: testImageRepoURL, - ImageTag: testImageTag, - }, - AdditionalTags: map[string]string{ - "owner": "boss", - }, - }, + c := &ScheduledJob{ + ecsWkld: &ecsWkld{ + wkld: &wkld{ + name: aws.StringValue(testScheduledJobManifest.Name), + env: testEnvName, + app: testAppName, + rc: RuntimeConfig{ + Image: &ECRImage{ + RepoURL: testImageRepoURL, + ImageTag: testImageTag, + }, + AdditionalTags: map[string]string{ + "owner": "boss", }, - tc: testScheduledJobManifest.TaskConfig, }, - manifest: testScheduledJobManifest, - } - tc.mockDependencies(ctrl, c) - - // WHEN - params, err := c.SerializedParameters() - - // THEN - require.Equal(t, tc.wantedError, err) - require.Equal(t, tc.wantedParams, params) - }) + }, + tc: testScheduledJobManifest.TaskConfig, + }, + manifest: testScheduledJobManifest, } + params, err := c.SerializedParameters() + require.NoError(t, err) + require.Equal(t, params, `{ + "Parameters": { + "AddonsTemplateURL": "", + "AppName": "phonetool", + "ContainerImage": "111111111111.dkr.ecr.us-west-2.amazonaws.com/phonetool/frontend:manual-bf3678c", + "EnvFileARN": "", + "EnvName": "test", + "LogRetention": "30", + "Schedule": "cron(0 0 * * ? *)", + "TaskCPU": "256", + "TaskCount": "1", + "TaskMemory": "512", + "WorkloadName": "mailer" + }, + "Tags": { + "copilot-application": "phonetool", + "copilot-environment": "test", + "copilot-service": "mailer", + "owner": "boss" + } +}`) } diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-observability.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-observability.yml index 665ea595512..3541cd4ed09 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-observability.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-observability.yml @@ -310,6 +310,7 @@ Resources: Properties: ServiceToken: !GetAtt UniqueJSONValuesFunction.Arn Aliases: !Ref Aliases + FilterFor: !Ref ALBWorkloads CloudFrontDistribution: Metadata: 'aws:copilot:description': 'A CloudFront distribution for global content delivery' diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json index 7e78d1d04b6..975bb99caee 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/cf.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "my-svc", "ContainerImage": "mockImageURL:latest", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "1", - "LogRetention": "30", "ContainerPort": "5000", "DNSDelegated": "false", - "TargetContainer": "my-svc", - "TargetPort": "5000", "EnvFileARN": "", - "RulePath": "my-svc", + "EnvName": "test", "HTTPSEnabled": "true", - "Stickiness": "false" + "LogRetention": "30", + "RulePath": "my-svc", + "Stickiness": "false", + "TargetContainer": "my-svc", + "TargetPort": "5000", + "TaskCPU": "256", + "TaskCount": "1", + "TaskMemory": "512", + "WorkloadName": "my-svc" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "my-svc" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/job-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/job-test.params.json index b75d1e4c58f..7fe737d404c 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/job-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/job-test.params.json @@ -1,20 +1,20 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "job", "ContainerImage": "alpine", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "1", + "EnvFileARN": "", + "EnvName": "test", "LogRetention": "30", "Schedule": "cron(0 12 ? * MON *)", - "EnvFileARN": "" + "TaskCPU": "256", + "TaskCount": "1", + "TaskMemory": "512", + "WorkloadName": "job" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "job" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-test.params.json index d129f428da4..a3361798d96 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-test.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "gRPC", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "2", - "LogRetention": "30", "ContainerPort": "50051", "DNSDelegated": "false", - "TargetContainer": "gRPC", - "TargetPort": "50051", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "test", "HTTPSEnabled": "true", - "Stickiness": "false" + "LogRetention": "30", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "gRPC", + "TargetPort": "50051", + "TaskCPU": "256", + "TaskCount": "2", + "TaskMemory": "512", + "WorkloadName": "gRPC" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "gRPC" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-dev.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-dev.params.json index 73b29504270..20e585c6ed3 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-dev.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-dev.params.json @@ -1,28 +1,28 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "dev", - "WorkloadName": "fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "5", - "LogRetention": "30", "ContainerPort": "80", "DNSDelegated": "true", - "TargetContainer": "fe", - "TargetPort": "80", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "dev", "HTTPSEnabled": "true", - "Stickiness": "false", + "LogRetention": "30", "NLBAliases": "", - "NLBPort": "81" + "NLBPort": "81", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "fe", + "TargetPort": "80", + "TaskCPU": "256", + "TaskCount": "5", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "dev", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-prod.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-prod.params.json index 395d21503fd..85fead9b5e6 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-prod.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-prod.params.json @@ -1,25 +1,25 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "prod", - "WorkloadName": "fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "5", - "LogRetention": "30", "ContainerPort": "80", "DNSDelegated": "true", - "TargetContainer": "fe", - "TargetPort": "80", "EnvFileARN": "", + "EnvName": "prod", + "LogRetention": "30", "NLBAliases": "nlb.example.com", - "NLBPort": "443" + "NLBPort": "443", + "TargetContainer": "fe", + "TargetPort": "80", + "TaskCPU": "256", + "TaskCount": "5", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "prod", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.params.json index 4ae9e0702ae..59c43adb725 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.params.json @@ -1,25 +1,25 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "5", - "LogRetention": "30", "ContainerPort": "80", "DNSDelegated": "true", - "TargetContainer": "fe", - "TargetPort": "80", "EnvFileARN": "", + "EnvName": "test", + "LogRetention": "30", "NLBAliases": "", - "NLBPort": "443" + "NLBPort": "443", + "TargetContainer": "fe", + "TargetPort": "80", + "TaskCPU": "256", + "TaskCount": "5", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json index 024d53b78ff..48bae789f8e 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "prod", - "WorkloadName": "fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "3", - "LogRetention": "1", "ContainerPort": "4000", "DNSDelegated": "false", - "TargetContainer": "fe", - "TargetPort": "4000", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "prod", "HTTPSEnabled": "true", - "Stickiness": "false" + "LogRetention": "1", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "fe", + "TargetPort": "4000", + "TaskCPU": "256", + "TaskCount": "3", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "prod", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-staging.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-staging.params.json index 3d70556b075..d702419ca17 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-staging.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-staging.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "staging", - "WorkloadName": "fe", "ContainerImage": "123456789000.dkr.ecr.us-east-1.amazonaws.com/vault/e2e:cicdtest", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "5", - "LogRetention": "30", "ContainerPort": "4000", "DNSDelegated": "false", - "TargetContainer": "fe", - "TargetPort": "4000", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "staging", "HTTPSEnabled": "true", - "Stickiness": "false" + "LogRetention": "30", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "fe", + "TargetPort": "4000", + "TaskCPU": "256", + "TaskCount": "5", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "staging", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.params.json index 84798649319..4d6e8ec6530 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "256", - "TaskMemory": "512", - "TaskCount": "2", - "LogRetention": "30", "ContainerPort": "4000", "DNSDelegated": "false", - "TargetContainer": "fe", - "TargetPort": "4000", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "test", "HTTPSEnabled": "true", - "Stickiness": "false" + "LogRetention": "30", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "fe", + "TargetPort": "4000", + "TaskCPU": "256", + "TaskCount": "2", + "TaskMemory": "512", + "WorkloadName": "fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-test.params.json index d5deb4a71f9..99edaf0ed1a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-test.params.json @@ -1,26 +1,26 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "windows-fe", "ContainerImage": "", - "AddonsTemplateURL": "", - "TaskCPU": "1024", - "TaskMemory": "2048", - "TaskCount": "1", - "LogRetention": "30", "ContainerPort": "80", "DNSDelegated": "false", - "TargetContainer": "windows-fe", - "TargetPort": "80", "EnvFileARN": "", - "RulePath": "/", + "EnvName": "test", "HTTPSEnabled": "false", - "Stickiness": "false" + "LogRetention": "30", + "RulePath": "/", + "Stickiness": "false", + "TargetContainer": "windows-fe", + "TargetPort": "80", + "TaskCPU": "1024", + "TaskCount": "1", + "TaskMemory": "2048", + "WorkloadName": "windows-fe" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "windows-fe" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/worker-test.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/worker-test.params.json index 0ed82e13df7..752c4dd48eb 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/worker-test.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/worker-test.params.json @@ -1,19 +1,19 @@ { - "Parameters" : { + "Parameters": { + "AddonsTemplateURL": "", "AppName": "my-app", - "EnvName": "test", - "WorkloadName": "dogworker", "ContainerImage": "amazon/ecs-example", - "AddonsTemplateURL": "", + "EnvFileARN": "", + "EnvName": "test", + "LogRetention": "30", "TaskCPU": "256", - "TaskMemory": "512", "TaskCount": "1", - "LogRetention": "30", - "EnvFileARN": "" + "TaskMemory": "512", + "WorkloadName": "dogworker" }, - "Tags": { + "Tags": { "copilot-application": "my-app", "copilot-environment": "test", "copilot-service": "dogworker" } -} +} \ No newline at end of file diff --git a/internal/pkg/deploy/cloudformation/stack/workload.go b/internal/pkg/deploy/cloudformation/stack/workload.go index b8932e46de6..03d3ed79220 100644 --- a/internal/pkg/deploy/cloudformation/stack/workload.go +++ b/internal/pkg/deploy/cloudformation/stack/workload.go @@ -4,6 +4,7 @@ package stack import ( + "encoding/json" "fmt" "strconv" @@ -17,11 +18,6 @@ import ( "github.com/aws/copilot-cli/internal/pkg/template/override" ) -// Template rendering configuration common across workloads. -const ( - wkldParamsTemplatePath = "workloads/params.json.tmpl" -) - // Parameter logical IDs common across workloads. const ( WorkloadAppNameParamKey = "AppName" @@ -186,19 +182,30 @@ func serializeTemplateConfig(parser template.Parser, stack templateConfigurer) ( if err != nil { return "", err } - doc, err := parser.Parse(wkldParamsTemplatePath, struct { - Parameters []*cloudformation.Parameter - Tags []*cloudformation.Tag + + tags := stack.Tags() + + config := struct { + Parameters map[string]*string `json:"Parameters"` + Tags map[string]*string `json:"Tags,omitempty"` }{ - Parameters: params, - Tags: stack.Tags(), - }, template.WithFuncs(map[string]interface{}{ - "inc": template.IncFunc, - })) + Parameters: make(map[string]*string, len(params)), + Tags: make(map[string]*string, len(tags)), + } + + for _, param := range params { + config.Parameters[aws.StringValue(param.ParameterKey)] = param.ParameterValue + } + for _, tag := range tags { + config.Tags[aws.StringValue(tag.Key)] = tag.Value + } + + str, err := json.MarshalIndent(config, "", " ") if err != nil { - return "", err + return "", fmt.Errorf("marshal stack parameters to JSON: %v", err) } - return doc.String(), nil + + return string(str), nil } func (w *wkld) addonsOutputs() (*template.WorkloadNestedStackOpts, error) { diff --git a/internal/pkg/manifest/validate_env.go b/internal/pkg/manifest/validate_env.go index 3081fa9c22d..465d258dbfd 100644 --- a/internal/pkg/manifest/validate_env.go +++ b/internal/pkg/manifest/validate_env.go @@ -49,11 +49,8 @@ func (e EnvironmentConfig) validate() error { if e.CDNEnabled() { cdnCert := e.CDNConfig.Config.Certificate if e.HTTPConfig.Public.Certificates == nil { - if cdnCert != nil { - return &errFieldMustBeSpecified{ - missingField: "http.public.certificates", - conditionalFields: []string{"cdn.certificate"}, - } + if cdnCert != nil && !aws.BoolValue(e.CDNConfig.Config.TerminateTLS) { + return errors.New(`"cdn.terminate_tls" must be true if "cdn.certificate" is set without "http.public.certificates"`) } } else { if cdnCert == nil { diff --git a/internal/pkg/manifest/validate_env_test.go b/internal/pkg/manifest/validate_env_test.go index e89bc0efcc8..8f24b8e67a7 100644 --- a/internal/pkg/manifest/validate_env_test.go +++ b/internal/pkg/manifest/validate_env_test.go @@ -214,7 +214,7 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, }, }, - "error if cdn cert specified but public certs not specified": { + "error if cdn cert specified, cdn not terminating tls, and public certs not specified": { in: EnvironmentConfig{ CDNConfig: EnvironmentCDNConfig{ Config: AdvancedCDNConfig{ @@ -222,7 +222,17 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, }, }, - wantedError: "\"http.public.certificates\" must be specified if \"cdn.certificate\" is specified", + wantedError: `"cdn.terminate_tls" must be true if "cdn.certificate" is set without "http.public.certificates"`, + }, + "success if cdn cert specified, cdn terminating tls, and no public certs": { + in: EnvironmentConfig{ + CDNConfig: EnvironmentCDNConfig{ + Config: AdvancedCDNConfig{ + Certificate: aws.String("arn:aws:acm:us-east-1:1111111:certificate/look-like-a-good-arn"), + TerminateTLS: aws.Bool(true), + }, + }, + }, }, "error if cdn cert not specified but public certs imported": { in: EnvironmentConfig{ diff --git a/internal/pkg/template/env.go b/internal/pkg/template/env.go index 6da87be2fed..8d8f9d516ea 100644 --- a/internal/pkg/template/env.go +++ b/internal/pkg/template/env.go @@ -121,6 +121,14 @@ type EnvOpts struct { DelegateDNS bool } +// HasImportedCerts returns true if any https certificates have been +// imported to the environment. +func (e *EnvOpts) HasImportedCerts() bool { + return len(e.PublicHTTPConfig.ImportedCertARNs) > 0 || + len(e.PrivateHTTPConfig.ImportedCertARNs) > 0 || + (e.CDNConfig != nil && e.CDNConfig.ImportedCertificate != nil) +} + // HTTPConfig represents configuration for a Load Balancer. type HTTPConfig struct { CIDRPrefixListIDs []string diff --git a/internal/pkg/template/templates/environment/partials/cdn-resources.yml b/internal/pkg/template/templates/environment/partials/cdn-resources.yml index 543e7b7f28d..64e16c98ef6 100644 --- a/internal/pkg/template/templates/environment/partials/cdn-resources.yml +++ b/internal/pkg/template/templates/environment/partials/cdn-resources.yml @@ -44,6 +44,7 @@ UniqueAliasesAction: Properties: ServiceToken: !GetAtt UniqueJSONValuesFunction.Arn Aliases: !Ref Aliases + FilterFor: !Ref ALBWorkloads {{- end}}{{/* endif or .CDNConfig.ImportedCertificate .DelegateDNS */}} CloudFrontDistribution: diff --git a/internal/pkg/template/templates/environment/partials/environment-manager-role.yml b/internal/pkg/template/templates/environment/partials/environment-manager-role.yml index 797ffebdeca..39f0c89d4d2 100644 --- a/internal/pkg/template/templates/environment/partials/environment-manager-role.yml +++ b/internal/pkg/template/templates/environment/partials/environment-manager-role.yml @@ -22,7 +22,7 @@ EnvironmentManagerRole: PolicyDocument: Version: '2012-10-17' Statement: -{{- if or .PublicHTTPConfig.ImportedCertARNs .PrivateHTTPConfig.ImportedCertARNs}} +{{- if .HasImportedCerts}} - Sid: ImportedCertificates Effect: Allow Action: [ diff --git a/internal/pkg/template/templates/workloads/params.json.tmpl b/internal/pkg/template/templates/workloads/params.json.tmpl deleted file mode 100644 index 6125a5285ee..00000000000 --- a/internal/pkg/template/templates/workloads/params.json.tmpl +++ /dev/null @@ -1,8 +0,0 @@ -{ - "Parameters" : { {{$paramsSize := len .Parameters}}{{range $i, $p := .Parameters}} - {{if eq (inc $i) $paramsSize}}"{{$p.ParameterKey}}": "{{$p.ParameterValue}}"{{else}}"{{$p.ParameterKey}}": "{{$p.ParameterValue}}",{{end}}{{end}} - },{{$n := len .Tags}}{{if gt $n 0}} - "Tags": { {{range $i, $t := .Tags}} - {{if eq (inc $i) $n}}"{{$t.Key}}": "{{$t.Value}}"{{else}}"{{$t.Key}}": "{{$t.Value}}",{{end}}{{end}} - }{{end}} -} From bfedebceacf578a375476e6fee8fbd78939ab172 Mon Sep 17 00:00:00 2001 From: danny randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 7 Oct 2022 14:13:50 -0700 Subject: [PATCH 02/10] e2e(apprunner): reduce size of dependencies (#4066) - upgrade to go 1.19 - use stdlib http router - migrate to aws sdk v2 to reduce dependency size - use distroless container By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- e2e/apprunner/back-end/Dockerfile | 40 +++++------------- e2e/apprunner/back-end/go.mod | 4 +- e2e/apprunner/back-end/go.sum | 2 - e2e/apprunner/back-end/main.go | 19 +++++---- e2e/apprunner/front-end/Dockerfile | 40 +++++------------- e2e/apprunner/front-end/go.mod | 20 +++++++-- e2e/apprunner/front-end/go.sum | 47 ++++++++++++--------- e2e/apprunner/front-end/main.go | 65 ++++++++++++++++-------------- 8 files changed, 112 insertions(+), 125 deletions(-) diff --git a/e2e/apprunner/back-end/Dockerfile b/e2e/apprunner/back-end/Dockerfile index 02429bdb926..6efeda25764 100644 --- a/e2e/apprunner/back-end/Dockerfile +++ b/e2e/apprunner/back-end/Dockerfile @@ -1,33 +1,15 @@ -# We specify the base image we need for our -# go application -FROM golang:1.14 AS builder -# We create an /app directory within our -# image that will hold our application source -# files -RUN mkdir /app -# We copy everything in the root directory -# into our /app directory -ADD . /app -# We specify that we now wish to execute -# any further commands inside our /app -# directory -WORKDIR /app -# Avoid the GoProxy +FROM public.ecr.aws/docker/library/golang:1.19 as builder + ENV GOPROXY=direct -# we run go build to compile the binary -# executable of our Go program -RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o e2e-service ./ +WORKDIR /go/src/app + +COPY . . + +RUN go mod download +RUN CGO_ENABLED=0 go build -o /go/bin/app ./ -# To make our images smaller, use alpine and copy in the service binary. -FROM alpine:latest -# Install certs -RUN apk --no-cache add ca-certificates -# Copy the binary from the builder image -COPY --from=builder /app ./ -# Make the binary executable -RUN chmod +x ./e2e-service +FROM gcr.io/distroless/static -# Start the service -ENTRYPOINT ["./e2e-service"] -# The service runs on port 80 +COPY --from=builder /go/bin/app / EXPOSE 80 +ENTRYPOINT ["/app"] \ No newline at end of file diff --git a/e2e/apprunner/back-end/go.mod b/e2e/apprunner/back-end/go.mod index f477f35065f..fceae6aab61 100644 --- a/e2e/apprunner/back-end/go.mod +++ b/e2e/apprunner/back-end/go.mod @@ -1,5 +1,3 @@ module github.com/aws/copilot-cli/e2e/appprunner/back-end -go 1.14 - -require github.com/julienschmidt/httprouter v1.3.0 // indirect +go 1.19 diff --git a/e2e/apprunner/back-end/go.sum b/e2e/apprunner/back-end/go.sum index 096c54e6309..e69de29bb2d 100644 --- a/e2e/apprunner/back-end/go.sum +++ b/e2e/apprunner/back-end/go.sum @@ -1,2 +0,0 @@ -github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U= -github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= diff --git a/e2e/apprunner/back-end/main.go b/e2e/apprunner/back-end/main.go index 279a6f6e0e3..65571639e4e 100644 --- a/e2e/apprunner/back-end/main.go +++ b/e2e/apprunner/back-end/main.go @@ -4,33 +4,32 @@ package main import ( + "errors" "log" "net/http" - - "github.com/julienschmidt/httprouter" ) var message = "hello world" // HealthCheck just returns true if the service is up. -func HealthCheck(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { +func HealthCheck(w http.ResponseWriter, req *http.Request) { log.Println("🚑 healthcheck ok!") w.WriteHeader(http.StatusOK) } // ServiceDiscoveryGet just returns true no matter what -func ServiceDiscoveryGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { +func ServiceDiscoveryGet(w http.ResponseWriter, req *http.Request) { log.Printf("Get on ServiceDiscovery endpoint Succeeded with message %s\n", message) w.WriteHeader(http.StatusOK) w.Write([]byte(message)) } func main() { - router := httprouter.New() - router.GET("/service-discovery/", ServiceDiscoveryGet) - - // Health Check - router.GET("/", HealthCheck) + http.Handle("/", http.HandlerFunc(HealthCheck)) + http.Handle("/service-discovery", http.HandlerFunc(ServiceDiscoveryGet)) - log.Fatal(http.ListenAndServe(":80", router)) + err := http.ListenAndServe(":80", nil) + if !errors.Is(err, http.ErrServerClosed) { + log.Fatalf("listen and serve: %s", err) + } } diff --git a/e2e/apprunner/front-end/Dockerfile b/e2e/apprunner/front-end/Dockerfile index 02429bdb926..6efeda25764 100644 --- a/e2e/apprunner/front-end/Dockerfile +++ b/e2e/apprunner/front-end/Dockerfile @@ -1,33 +1,15 @@ -# We specify the base image we need for our -# go application -FROM golang:1.14 AS builder -# We create an /app directory within our -# image that will hold our application source -# files -RUN mkdir /app -# We copy everything in the root directory -# into our /app directory -ADD . /app -# We specify that we now wish to execute -# any further commands inside our /app -# directory -WORKDIR /app -# Avoid the GoProxy +FROM public.ecr.aws/docker/library/golang:1.19 as builder + ENV GOPROXY=direct -# we run go build to compile the binary -# executable of our Go program -RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o e2e-service ./ +WORKDIR /go/src/app + +COPY . . + +RUN go mod download +RUN CGO_ENABLED=0 go build -o /go/bin/app ./ -# To make our images smaller, use alpine and copy in the service binary. -FROM alpine:latest -# Install certs -RUN apk --no-cache add ca-certificates -# Copy the binary from the builder image -COPY --from=builder /app ./ -# Make the binary executable -RUN chmod +x ./e2e-service +FROM gcr.io/distroless/static -# Start the service -ENTRYPOINT ["./e2e-service"] -# The service runs on port 80 +COPY --from=builder /go/bin/app / EXPOSE 80 +ENTRYPOINT ["/app"] \ No newline at end of file diff --git a/e2e/apprunner/front-end/go.mod b/e2e/apprunner/front-end/go.mod index 6e8c75a4840..fb69e0b1cec 100644 --- a/e2e/apprunner/front-end/go.mod +++ b/e2e/apprunner/front-end/go.mod @@ -1,9 +1,23 @@ module github.com/aws/copilot-cli/e2e/apprunner/front-end -go 1.14 +go 1.19 require ( - github.com/aws/aws-sdk-go v1.43.6 - github.com/julienschmidt/httprouter v1.3.0 + github.com/aws/aws-sdk-go-v2 v1.16.16 + github.com/aws/aws-sdk-go-v2/config v1.17.8 + github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.16.2 github.com/lib/pq v1.10.4 ) + +require ( + github.com/aws/aws-sdk-go-v2/credentials v1.12.21 // indirect + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.17 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.23 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.17 // indirect + github.com/aws/aws-sdk-go-v2/internal/ini v1.3.24 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.17 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.11.23 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.6 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.16.19 // indirect + github.com/aws/smithy-go v1.13.3 // indirect +) diff --git a/e2e/apprunner/front-end/go.sum b/e2e/apprunner/front-end/go.sum index c1f8c44d873..2ace2e1ac00 100644 --- a/e2e/apprunner/front-end/go.sum +++ b/e2e/apprunner/front-end/go.sum @@ -1,28 +1,37 @@ -github.com/aws/aws-sdk-go v1.43.6 h1:FkwmndZR4LjnT2fiKaD18bnqfQ188E8A1IMNI5rcv00= -github.com/aws/aws-sdk-go v1.43.6/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/aws/aws-sdk-go-v2 v1.16.16 h1:M1fj4FE2lB4NzRb9Y0xdWsn2P0+2UHVxwKyOa4YJNjk= +github.com/aws/aws-sdk-go-v2 v1.16.16/go.mod h1:SwiyXi/1zTUZ6KIAmLK5V5ll8SiURNUYOqTerZPaF9k= +github.com/aws/aws-sdk-go-v2/config v1.17.8 h1:b9LGqNnOdg9vR4Q43tBTVWk4J6F+W774MSchvKJsqnE= +github.com/aws/aws-sdk-go-v2/config v1.17.8/go.mod h1:UkCI3kb0sCdvtjiXYiU4Zx5h07BOpgBTtkPu/49r+kA= +github.com/aws/aws-sdk-go-v2/credentials v1.12.21 h1:4tjlyCD0hRGNQivh5dN8hbP30qQhMLBE/FgQR1vHHWM= +github.com/aws/aws-sdk-go-v2/credentials v1.12.21/go.mod h1:O+4XyAt4e+oBAoIwNUYkRg3CVMscaIJdmZBOcPgJ8D8= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.17 h1:r08j4sbZu/RVi+BNxkBJwPMUYY3P8mgSDuKkZ/ZN1lE= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.12.17/go.mod h1:yIkQcCDYNsZfXpd5UX2Cy+sWA1jPgIhGTw9cOBzfVnQ= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.23 h1:s4g/wnzMf+qepSNgTvaQQHNxyMLKSawNhKCPNy++2xY= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.23/go.mod h1:2DFxAQ9pfIRy0imBCJv+vZ2X6RKxves6fbnEuSry6b4= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.17 h1:/K482T5A3623WJgWT8w1yRAFK4RzGzEl7y39yhtn9eA= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.17/go.mod h1:pRwaTYCJemADaqCbUAxltMoHKata7hmB5PjEXeu0kfg= +github.com/aws/aws-sdk-go-v2/internal/ini v1.3.24 h1:wj5Rwc05hvUSvKuOF29IYb9QrCLjU+rHAy/x/o0DK2c= +github.com/aws/aws-sdk-go-v2/internal/ini v1.3.24/go.mod h1:jULHjqqjDlbyTa7pfM7WICATnOv+iOhjletM3N0Xbu8= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.17 h1:Jrd/oMh0PKQc6+BowB+pLEwLIgaQF29eYbe7E1Av9Ug= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.17/go.mod h1:4nYOrY41Lrbk2170/BGkcJKBhws9Pfn8MG3aGqjjeFI= +github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.16.2 h1:3x1Qilin49XQ1rK6pDNAfG+DmCFPfB7Rrpl+FUDAR/0= +github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.16.2/go.mod h1:HEBBc70BYi5eUvxBqC3xXjU/04NO96X/XNUe5qhC7Bc= +github.com/aws/aws-sdk-go-v2/service/sso v1.11.23 h1:pwvCchFUEnlceKIgPUouBJwK81aCkQ8UDMORfeFtW10= +github.com/aws/aws-sdk-go-v2/service/sso v1.11.23/go.mod h1:/w0eg9IhFGjGyyncHIQrXtU8wvNsTJOP0R6PPj0wf80= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.6 h1:OwhhKc1P9ElfWbMKPIbMMZBV6hzJlL2JKD76wNNVzgQ= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.13.6/go.mod h1:csZuQY65DAdFBt1oIjO5hhBR49kQqop4+lcuCjf2arA= +github.com/aws/aws-sdk-go-v2/service/sts v1.16.19 h1:9pPi0PsFNAGILFfPCk8Y0iyEBGc6lu6OQ97U7hmdesg= +github.com/aws/aws-sdk-go-v2/service/sts v1.16.19/go.mod h1:h4J3oPZQbxLhzGnk+j9dfYHi5qIOVJ5kczZd658/ydM= +github.com/aws/smithy-go v1.13.3 h1:l7LYxGuzK6/K+NzJ2mC+VvLUbae0sL3bXU//04MkmnA= +github.com/aws/smithy-go v1.13.3/go.mod h1:Tg+OJXh4MB2R/uN61Ko2f6hTZwB/ZYGOtib8J3gBHzA= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= -github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= -github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U= -github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/lib/pq v1.10.4 h1:SO9z7FRPzA03QhHKJrH5BXA6HU1rS4V2nIVrrNC1iYk= github.com/lib/pq v1.10.4/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd h1:O7DYs+zxREGLKzKoMQrtrEacpb0ZVXA5rIwylE2Xchk= -golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= -golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/e2e/apprunner/front-end/main.go b/e2e/apprunner/front-end/main.go index c2d75123ccb..5d86bb48858 100644 --- a/e2e/apprunner/front-end/main.go +++ b/e2e/apprunner/front-end/main.go @@ -4,8 +4,10 @@ package main import ( + "context" "database/sql" "encoding/json" + "errors" "fmt" "io/ioutil" "log" @@ -13,11 +15,10 @@ import ( "os" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/secretsmanager" - "github.com/julienschmidt/httprouter" _ "github.com/lib/pq" // https://www.calhoun.io/why-we-import-sql-drivers-with-the-blank-identifier/ ) @@ -29,7 +30,7 @@ const ( ) // SimpleGet just returns true no matter what -func SimpleGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { +func SimpleGet(w http.ResponseWriter, req *http.Request) { log.Println("Get Succeeded") w.WriteHeader(http.StatusOK) w.Write([]byte(os.Getenv("COPILOT_APPLICATION_NAME") + "-" + os.Getenv("COPILOT_ENVIRONMENT_NAME") + "-" + os.Getenv("COPILOT_SERVICE_NAME"))) @@ -40,11 +41,11 @@ func SimpleGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { // This test assumes the backend app is called "back-end". The 'service-discovery' endpoint // of the back-end service is unreachable from the LB, so the only way to get it is // through service discovery. The response should be `back-end-service-discovery` -func ServiceDiscoveryGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { +func ServiceDiscoveryGet(w http.ResponseWriter, req *http.Request) { endpoint := fmt.Sprintf("http://back-end.%s/service-discovery/", os.Getenv("COPILOT_SERVICE_DISCOVERY_ENDPOINT")) resp, err := http.Get(endpoint) if err != nil { - log.Printf("🚨 could call service discovery endpoint: err=%s\n", err) + log.Printf("🚨 could call service discovery endpoint: err=%s", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -64,17 +65,21 @@ type Secret struct { } // DBGet calls an aurora DB and returns a timestamp from the database. -func DBGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { - sess, err := session.NewSession(&aws.Config{ - Region: aws.String(os.Getenv("AWS_DEFAULT_REGION")), - }) +func DBGet(w http.ResponseWriter, req *http.Request) { + ctx, cancel := context.WithTimeout(req.Context(), 20*time.Second) + defer cancel() + + cfg, err := config.LoadDefaultConfig(ctx, + config.WithRegion(os.Getenv("AWS_DEFAULT_REGION")), + ) if err != nil { - log.Printf("🚨 initial new aws session: err=%s\n", err) + log.Printf("🚨 load aws config: %s", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - ssm := secretsmanager.New(sess) - out, err := ssm.GetSecretValue(&secretsmanager.GetSecretValueInput{ + + mgr := secretsmanager.NewFromConfig(cfg) + out, err := mgr.GetSecretValue(ctx, &secretsmanager.GetSecretValueInput{ SecretId: aws.String(os.Getenv("FRONTENDCLUSTER_SECRET_ARN")), }) if err != nil { @@ -82,18 +87,21 @@ func DBGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - secretValue := aws.StringValue(out.SecretString) + + secretValue := aws.ToString(out.SecretString) if secretValue == "" { log.Print("🚨 empty aurora secret value\n") http.Error(w, err.Error(), http.StatusInternalServerError) return } + secret := Secret{} if err := json.Unmarshal([]byte(secretValue), &secret); err != nil { log.Printf("🚨 unmarshal rds secret: err=%s\n", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } + source := fmt.Sprintf("host=%s port=%d user=%s "+ "password=%s dbname=%s", secret.Host, secret.Port, secret.Username, secret.Password, secret.DBName) @@ -103,29 +111,26 @@ func DBGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - err = db.Ping() // Force open a connection to the database. - if err != nil { + defer db.Close() + + // Force open a connection to the database. + if err := db.Ping(); err != nil { log.Printf("🚨 ping: err=%s\n", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - close := func() error { - err := db.Close() - if err != nil { - return fmt.Errorf("close db: %w", err) - } - return nil - } - defer close() + w.WriteHeader(http.StatusOK) w.Write([]byte(fmt.Sprint(time.Now()))) } func main() { - router := httprouter.New() - router.GET("/", SimpleGet) - router.GET("/service-discovery-test", ServiceDiscoveryGet) - router.GET("/db", DBGet) + http.Handle("/", http.HandlerFunc(SimpleGet)) + http.Handle("/service-discovery-test", http.HandlerFunc(ServiceDiscoveryGet)) + http.Handle("/db", http.HandlerFunc(DBGet)) - log.Fatal(http.ListenAndServe(":80", router)) + err := http.ListenAndServe(":80", nil) + if !errors.Is(err, http.ErrServerClosed) { + log.Fatalf("listen and serve: %s", err) + } } From 440ab22f8b5c967512dda03c8b9ce40a3b568a0d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Oct 2022 15:23:16 +0000 Subject: [PATCH 03/10] chore: Bump github.com/aws/aws-sdk-go from 1.44.109 to 1.44.114 (#4074) Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.44.109 to 1.44.114.
Release notes

Sourced from github.com/aws/aws-sdk-go's releases.

Release v1.44.114 (2022-10-07)

Service Client Updates

Release v1.44.113 (2022-10-06)

Service Client Updates

Release v1.44.112 (2022-10-05)

Service Client Updates

Release v1.44.111 (2022-10-04)

Service Client Updates

Release v1.44.110 (2022-10-03)

Service Client Updates

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/aws/aws-sdk-go&package-manager=go_modules&previous-version=1.44.109&new-version=1.44.114)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
--- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 837a0c7a872..1d29538e224 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/AlecAivazis/survey/v2 v2.3.2 - github.com/aws/aws-sdk-go v1.44.109 + github.com/aws/aws-sdk-go v1.44.114 github.com/briandowns/spinner v1.19.0 github.com/dustin/go-humanize v1.0.0 github.com/fatih/color v1.13.0 diff --git a/go.sum b/go.sum index cfff907e388..1c596af2f45 100644 --- a/go.sum +++ b/go.sum @@ -180,8 +180,8 @@ github.com/aws/aws-sdk-go v1.20.6/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN github.com/aws/aws-sdk-go v1.25.11/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.27.1/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.31.6/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.44.109 h1:+Na5JPeS0kiEHoBp5Umcuuf+IDqXqD0lXnM920E31YI= -github.com/aws/aws-sdk-go v1.44.109/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= +github.com/aws/aws-sdk-go v1.44.114 h1:plIkWc/RsHr3DXBj4MEw9sEW4CcL/e2ryokc+CKyq1I= +github.com/aws/aws-sdk-go v1.44.114/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod h1:q/89r3U2H7sSsE2t6Kca0lfwTK8JdoNGS/yzM/4iH5I= github.com/beorn7/perks v0.0.0-20160804104726-4c0e84591b9a/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= From 27d6fb62e33469e8a019b29b6a8fb873ea7d2876 Mon Sep 17 00:00:00 2001 From: danny randall <10566468+dannyrandall@users.noreply.github.com> Date: Mon, 10 Oct 2022 12:08:05 -0700 Subject: [PATCH 04/10] chore: bump env template patch version (#4076) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/deploy/env.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/deploy/env.go b/internal/pkg/deploy/env.go index 6930769f2e6..fc65ac59ce7 100644 --- a/internal/pkg/deploy/env.go +++ b/internal/pkg/deploy/env.go @@ -14,7 +14,7 @@ const ( // LegacyEnvTemplateVersion is the version associated with the environment template before we started versioning. LegacyEnvTemplateVersion = "v0.0.0" // LatestEnvTemplateVersion is the latest version number available for environment templates. - LatestEnvTemplateVersion = "v1.12.2" + LatestEnvTemplateVersion = "v1.12.3" EnvTemplateVersionBootstrap = "bootstrap" ) From d3ad7787ab108a7e37382de5d6bd409bd5ce96a0 Mon Sep 17 00:00:00 2001 From: Janice Huang <60631893+huanjani@users.noreply.github.com> Date: Mon, 10 Oct 2022 14:32:27 -0700 Subject: [PATCH 05/10] chore: validate permissions boundary policy name so as to fail faster if invalid (#4035) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/aws/iam/iam.go | 27 ++++++++++ internal/pkg/aws/iam/iam_test.go | 61 +++++++++++++++++++++++ internal/pkg/aws/iam/mocks/mock_iam.go | 15 ++++++ internal/pkg/cli/app_init.go | 21 ++++++++ internal/pkg/cli/app_init_test.go | 28 +++++++++-- internal/pkg/cli/interfaces.go | 4 ++ internal/pkg/cli/mocks/mock_interfaces.go | 38 ++++++++++++++ 7 files changed, 190 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/iam/iam.go b/internal/pkg/aws/iam/iam.go index 92e674449d2..44232140bbe 100644 --- a/internal/pkg/aws/iam/iam.go +++ b/internal/pkg/aws/iam/iam.go @@ -25,6 +25,7 @@ type api interface { ListRolePolicies(input *iam.ListRolePoliciesInput) (*iam.ListRolePoliciesOutput, error) DeleteRole(input *iam.DeleteRoleInput) (*iam.DeleteRoleOutput, error) CreateServiceLinkedRole(input *iam.CreateServiceLinkedRoleInput) (*iam.CreateServiceLinkedRoleOutput, error) + ListPolicies(input *iam.ListPoliciesInput) (*iam.ListPoliciesOutput, error) } // IAM wraps the AWS SDK's IAM client. @@ -98,6 +99,32 @@ func (c *IAM) CreateECSServiceLinkedRole() error { return nil } +// ListPolicyNames returns a list of local policy names. +func (c *IAM) ListPolicyNames() ([]string, error) { + var policies []*iam.Policy + var marker *string + for { + output, err := c.client.ListPolicies(&iam.ListPoliciesInput{ + Marker: marker, + Scope: aws.String("Local"), + PolicyUsageFilter: aws.String("PermissionsBoundary"), + }) + if err != nil { + return nil, fmt.Errorf("list IAM policies: %w", err) + } + policies = append(policies, output.Policies...) + if !aws.BoolValue(output.IsTruncated) { + break + } + marker = output.Marker + } + var policyNames = make([]string, len(policies)) + for i, policy := range policies { + policyNames[i] = aws.StringValue(policy.PolicyName) + } + return policyNames, nil +} + func (c *IAM) deleteRolePolicies(roleName string) error { policyNames, err := c.listRolePolicyNames(roleName) if err != nil { diff --git a/internal/pkg/aws/iam/iam_test.go b/internal/pkg/aws/iam/iam_test.go index 7223baf0d2f..705f8da0bb6 100644 --- a/internal/pkg/aws/iam/iam_test.go +++ b/internal/pkg/aws/iam/iam_test.go @@ -242,3 +242,64 @@ func TestIAM_CreateECSServiceLinkedRole(t *testing.T) { }) } } + +func TestIAM_ListPolicies(t *testing.T) { + testCases := map[string]struct { + inClient func(ctrl *gomock.Controller) *mocks.Mockapi + + wantedPolicies []string + wantedErr error + }{ + "wraps error on failure": { + inClient: func(ctrl *gomock.Controller) *mocks.Mockapi { + m := mocks.NewMockapi(ctrl) + m.EXPECT(). + ListPolicies(gomock.Any()). + Return(nil, errors.New("some error")) + return m + }, + + wantedErr: errors.New("list IAM policies: some error"), + }, + "returns list of policy names": { + inClient: func(ctrl *gomock.Controller) *mocks.Mockapi { + m := mocks.NewMockapi(ctrl) + m.EXPECT(). + ListPolicies(gomock.Any()). + Return(&iam.ListPoliciesOutput{ + Policies: []*iam.Policy{ + { + PolicyName: aws.String("myFirstPolicyName"), + }, + { + PolicyName: aws.String("mySecondPolicyName"), + }, + }, + }, nil) + return m + }, + wantedPolicies: []string{"myFirstPolicyName", "mySecondPolicyName"}, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + iam := &IAM{ + client: tc.inClient(ctrl), + } + + // WHEN + output, err := iam.ListPolicyNames() + + // THEN + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedPolicies, output) + } + }) + } +} diff --git a/internal/pkg/aws/iam/mocks/mock_iam.go b/internal/pkg/aws/iam/mocks/mock_iam.go index 0858de5b034..973b3781dd3 100644 --- a/internal/pkg/aws/iam/mocks/mock_iam.go +++ b/internal/pkg/aws/iam/mocks/mock_iam.go @@ -79,6 +79,21 @@ func (mr *MockapiMockRecorder) DeleteRolePolicy(input interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRolePolicy", reflect.TypeOf((*Mockapi)(nil).DeleteRolePolicy), input) } +// ListPolicies mocks base method. +func (m *Mockapi) ListPolicies(input *iam.ListPoliciesInput) (*iam.ListPoliciesOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListPolicies", input) + ret0, _ := ret[0].(*iam.ListPoliciesOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListPolicies indicates an expected call of ListPolicies. +func (mr *MockapiMockRecorder) ListPolicies(input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicies", reflect.TypeOf((*Mockapi)(nil).ListPolicies), input) +} + // ListRolePolicies mocks base method. func (m *Mockapi) ListRolePolicies(input *iam.ListRolePoliciesInput) (*iam.ListRolePoliciesOutput, error) { m.ctrl.T.Helper() diff --git a/internal/pkg/cli/app_init.go b/internal/pkg/cli/app_init.go index c97a45514a3..3126348d175 100644 --- a/internal/pkg/cli/app_init.go +++ b/internal/pkg/cli/app_init.go @@ -6,6 +6,7 @@ package cli import ( "errors" "fmt" + "github.com/aws/copilot-cli/internal/pkg/aws/iam" "os" "github.com/aws/aws-sdk-go/aws" @@ -51,6 +52,7 @@ type initAppOpts struct { cfn appDeployer prompt prompter prog progress + iam policyLister isSessionFromEnvVars func() (bool, error) cachedHostedZoneID string @@ -77,6 +79,7 @@ func newInitAppOpts(vars initAppVars) (*initAppOpts, error) { cfn: cloudformation.New(sess, cloudformation.WithProgressTracker(os.Stderr)), prompt: prompt.New(), prog: termprogress.NewSpinner(log.DiagnosticWriter), + iam: iam.New(sess), isSessionFromEnvVars: func() (bool, error) { return sessions.AreCredsFromEnvVars(sess) }, @@ -90,6 +93,11 @@ func (o *initAppOpts) Validate() error { return err } } + if o.permissionsBoundary != "" { + if err := o.validatePermBound(o.permissionsBoundary); err != nil { + return err + } + } if o.domainName != "" { if err := validateDomainName(o.domainName); err != nil { return fmt.Errorf("domain name %s is invalid: %w", o.domainName, err) @@ -234,6 +242,19 @@ func (o *initAppOpts) validateAppName(name string) error { return nil } +func (o *initAppOpts) validatePermBound(policyName string) error { + IAMPolicies, err := o.iam.ListPolicyNames() + if err != nil { + return fmt.Errorf("list permissions boundary policies: %w", err) + } + for _, policy := range IAMPolicies { + if policy == policyName { + return nil + } + } + return fmt.Errorf("IAM policy %q not found in this account", policyName) +} + func (o *initAppOpts) isDomainOwned() error { err := o.domainInfoGetter.IsRegisteredDomain(o.domainName) if err == nil { diff --git a/internal/pkg/cli/app_init_test.go b/internal/pkg/cli/app_init_test.go index 5ac5ea9c64d..b0b9461bcfb 100644 --- a/internal/pkg/cli/app_init_test.go +++ b/internal/pkg/cli/app_init_test.go @@ -22,12 +22,14 @@ type initAppMocks struct { mockRoute53Svc *mocks.MockdomainHostedZoneGetter mockStore *mocks.Mockstore mockDomainInfoGetter *mocks.MockdomainInfoGetter + mockPolicyLister *mocks.MockpolicyLister } func TestInitAppOpts_Validate(t *testing.T) { testCases := map[string]struct { - inAppName string - inDomainName string + inAppName string + inDomainName string + inPBPolicyName string mock func(m *initAppMocks) @@ -90,6 +92,21 @@ func TestInitAppOpts_Validate(t *testing.T) { }, wantedError: errors.New("check if domain is owned by the account: some error"), }, + "wrap error from ListPolicies": { + inPBPolicyName: "nonexistentPolicyName", + mock: func(m *initAppMocks) { + m.mockPolicyLister.EXPECT().ListPolicyNames().Return(nil, errors.New("some error")) + }, + wantedError: errors.New("list permissions boundary policies: some error"), + }, + "invalid permissions boundary policy name": { + inPBPolicyName: "nonexistentPolicyName", + mock: func(m *initAppMocks) { + m.mockPolicyLister.EXPECT().ListPolicyNames().Return( + []string{"existentPolicyName"}, nil) + }, + wantedError: errors.New("IAM policy \"nonexistentPolicyName\" not found in this account"), + }, "invalid domain name that doesn't have a hosted zone": { inDomainName: "badMockDomain.com", mock: func(m *initAppMocks) { @@ -132,6 +149,7 @@ func TestInitAppOpts_Validate(t *testing.T) { mockStore: mocks.NewMockstore(ctrl), mockRoute53Svc: mocks.NewMockdomainHostedZoneGetter(ctrl), mockDomainInfoGetter: mocks.NewMockdomainInfoGetter(ctrl), + mockPolicyLister: mocks.NewMockpolicyLister(ctrl), } tc.mock(m) @@ -139,9 +157,11 @@ func TestInitAppOpts_Validate(t *testing.T) { route53: m.mockRoute53Svc, domainInfoGetter: m.mockDomainInfoGetter, store: m.mockStore, + iam: m.mockPolicyLister, initAppVars: initAppVars{ - name: tc.inAppName, - domainName: tc.inDomainName, + name: tc.inAppName, + domainName: tc.inDomainName, + permissionsBoundary: tc.inPBPolicyName, }, } diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 85734437452..783f3554d30 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -555,6 +555,10 @@ type roleDeleter interface { DeleteRole(string) error } +type policyLister interface { + ListPolicyNames() ([]string, error) +} + type serviceDescriber interface { DescribeService(app, env, svc string) (*ecs.ServiceDesc, error) } diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index ed151f1aa8f..f3df9b139cd 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -5891,6 +5891,44 @@ func (mr *MockroleDeleterMockRecorder) DeleteRole(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRole", reflect.TypeOf((*MockroleDeleter)(nil).DeleteRole), arg0) } +// MockpolicyLister is a mock of policyLister interface. +type MockpolicyLister struct { + ctrl *gomock.Controller + recorder *MockpolicyListerMockRecorder +} + +// MockpolicyListerMockRecorder is the mock recorder for MockpolicyLister. +type MockpolicyListerMockRecorder struct { + mock *MockpolicyLister +} + +// NewMockpolicyLister creates a new mock instance. +func NewMockpolicyLister(ctrl *gomock.Controller) *MockpolicyLister { + mock := &MockpolicyLister{ctrl: ctrl} + mock.recorder = &MockpolicyListerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockpolicyLister) EXPECT() *MockpolicyListerMockRecorder { + return m.recorder +} + +// ListPolicyNames mocks base method. +func (m *MockpolicyLister) ListPolicyNames() ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListPolicyNames") + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListPolicyNames indicates an expected call of ListPolicyNames. +func (mr *MockpolicyListerMockRecorder) ListPolicyNames() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListPolicyNames", reflect.TypeOf((*MockpolicyLister)(nil).ListPolicyNames)) +} + // MockserviceDescriber is a mock of serviceDescriber interface. type MockserviceDescriber struct { ctrl *gomock.Controller From b3dba84f82c27a8518fa393fdfbbe558439ff050 Mon Sep 17 00:00:00 2001 From: Parag Sanjay Bhingre Date: Mon, 10 Oct 2022 15:00:58 -0700 Subject: [PATCH 06/10] fix(FIFO): worker service manifest with fifo subscriptions (#4056) This PR fix the following scenario for FIFO SNS and SQS. Initiate a new worker service and select FIFO topic for the subscriptions from the listed items and manifest gets populated with for example - ``` subscribe: topics: - name: orders.fifo service: api ``` Whereas in this scenario manifest should be - ``` subscribe: topics: - name: orders service: api queue: fifo: true ``` By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/initialize/workload.go | 2 + internal/pkg/initialize/workload_test.go | 48 ++++ .../initial_manifest_integration_test.go | 22 ++ .../worker-svc-with-default-fifo-queue.yml | 40 ++++ internal/pkg/manifest/worker_svc.go | 35 +++ internal/pkg/manifest/worker_svc_test.go | 217 ++++++++++++++++++ .../workloads/services/worker/manifest.yml | 7 + 7 files changed, 371 insertions(+) create mode 100644 internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml diff --git a/internal/pkg/initialize/workload.go b/internal/pkg/initialize/workload.go index 84bc50442c0..2ef4c6d822c 100644 --- a/internal/pkg/initialize/workload.go +++ b/internal/pkg/initialize/workload.go @@ -67,6 +67,7 @@ type WorkloadProps struct { Image string Platform manifest.PlatformArgsOrString Topics []manifest.TopicSubscription + Queue manifest.SQSQueue } // JobProps contains the information needed to represent a Job. @@ -370,6 +371,7 @@ func newWorkerServiceManifest(i *ServiceProps) (*manifest.WorkerService, error) HealthCheck: i.HealthCheck, Platform: i.Platform, Topics: i.Topics, + Queue: i.Queue, }), nil } diff --git a/internal/pkg/initialize/workload_test.go b/internal/pkg/initialize/workload_test.go index 307eff4c6cf..1b4ffb00ed1 100644 --- a/internal/pkg/initialize/workload_test.go +++ b/internal/pkg/initialize/workload_test.go @@ -735,6 +735,54 @@ func TestWorkloadInitializer_Service(t *testing.T) { }, "worker") }, }, + "topic subscriptions enabled with default fifo queue": { + inSvcType: manifest.WorkerServiceType, + inAppName: "app", + inSvcName: "worker", + inDockerfilePath: "worker/Dockerfile", + inSvcPort: 80, + inTopics: []manifest.TopicSubscription{ + { + Name: aws.String("theTopic.fifo"), + Service: aws.String("publisher"), + }, + }, + + mockWriter: func(m *mocks.MockWorkspace) { + // workspace root: "/worker" + gomock.InOrder( + m.EXPECT().Rel("worker/Dockerfile").Return("Dockerfile", nil), + m.EXPECT().Rel("/worker/manifest.yml").Return("manifest.yml", nil)) + m.EXPECT().WriteServiceManifest(gomock.Any(), "worker"). + Do(func(m *manifest.WorkerService, _ string) { + require.Equal(t, *m.Workload.Type, manifest.WorkerServiceType) + require.Equal(t, *m.Subscribe.Queue.FIFO.Enable, true) + require.Empty(t, m.ImageConfig.HealthCheck) + }).Return("/worker/manifest.yml", nil) + }, + mockstore: func(m *mocks.MockStore) { + m.EXPECT().CreateService(gomock.Any()). + Do(func(app *config.Workload) { + require.Equal(t, &config.Workload{ + Name: "worker", + App: "app", + Type: manifest.WorkerServiceType, + }, app) + }). + Return(nil) + + m.EXPECT().GetApplication("app").Return(&config.Application{ + Name: "app", + AccountID: "1234", + }, nil) + }, + mockappDeployer: func(m *mocks.MockWorkloadAdder) { + m.EXPECT().AddServiceToApp(&config.Application{ + Name: "app", + AccountID: "1234", + }, "worker") + }, + }, } for name, tc := range testCases { diff --git a/internal/pkg/manifest/initial_manifest_integration_test.go b/internal/pkg/manifest/initial_manifest_integration_test.go index 8a0e299fd2a..62163c058cc 100644 --- a/internal/pkg/manifest/initial_manifest_integration_test.go +++ b/internal/pkg/manifest/initial_manifest_integration_test.go @@ -161,6 +161,28 @@ func TestWorkerSvc_InitialManifestIntegration(t *testing.T) { }, wantedTestdata: "worker-svc-subscribe.yml", }, + "with fifo topic subscription with default fifo queue": { + inProps: WorkerServiceProps{ + WorkloadProps: WorkloadProps{ + Name: "testers", + Dockerfile: "./testers/Dockerfile", + }, + Platform: PlatformArgsOrString{ + PlatformString: nil, + PlatformArgs: PlatformArgs{ + OSFamily: nil, + Arch: nil, + }, + }, + Topics: []TopicSubscription{ + { + Name: aws.String("testTopic.fifo"), + Service: aws.String("service4TestTopic"), + }, + }, + }, + wantedTestdata: "worker-svc-with-default-fifo-queue.yml", + }, } for name, tc := range testCases { diff --git a/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml b/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml new file mode 100644 index 00000000000..00e1da5e041 --- /dev/null +++ b/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml @@ -0,0 +1,40 @@ +# The manifest for the "testers" service. +# Read the full specification for the "Worker Service" type at: +# https://aws.github.io/copilot-cli/docs/manifest/worker-service/ + +# Your service name will be used in naming your resources like log groups, ECS services, etc. +name: testers +type: Worker Service + +# Configuration for your containers and service. +image: + # Docker build arguments. + build: ./testers/Dockerfile + +cpu: 256 # Number of CPU units for the task. +memory: 512 # Amount of memory in MiB used by the task. +count: 1 # Number of tasks that should be running in your service. +exec: true # Enable running commands in your container. + +# The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. +subscribe: + topics: + - name: testTopic + service: service4TestTopic + queue: + fifo: true + +# Optional fields for more advanced use-cases. +# +#variables: # Pass environment variables as key value pairs. +# LOG_LEVEL: info + +#secrets: # Pass secrets from AWS Systems Manager (SSM) Parameter Store. +# GITHUB_TOKEN: GITHUB_TOKEN # The key is the name of the environment variable, the value is the name of the SSM parameter. + +# You can override any of the values defined above by environment. +#environments: +# test: +# count: 2 # Number of tasks to run for the "test" environment. +# deployment: # The deployment strategy for the "test" environment. +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. diff --git a/internal/pkg/manifest/worker_svc.go b/internal/pkg/manifest/worker_svc.go index 1d4f172ec37..0dc91613495 100644 --- a/internal/pkg/manifest/worker_svc.go +++ b/internal/pkg/manifest/worker_svc.go @@ -5,6 +5,7 @@ package manifest import ( "errors" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -190,6 +191,7 @@ type WorkerServiceProps struct { HealthCheck ContainerHealthCheck // Optional healthcheck configuration. Platform PlatformArgsOrString // Optional platform configuration. Topics []TopicSubscription // Optional topics for subscriptions + Queue SQSQueue // Optional queue configuration. } // NewWorkerService applies the props to a default Worker service configuration with @@ -206,12 +208,45 @@ func NewWorkerService(props WorkerServiceProps) *WorkerService { svc.WorkerServiceConfig.TaskConfig.CPU = aws.Int(MinWindowsTaskCPU) svc.WorkerServiceConfig.TaskConfig.Memory = aws.Int(MinWindowsTaskMemory) } + if len(props.Topics) > 0 { + setSubscriptionQueueDefaults(props.Topics, &svc.WorkerServiceConfig.Subscribe.Queue) + } svc.WorkerServiceConfig.Subscribe.Topics = props.Topics svc.WorkerServiceConfig.Platform = props.Platform svc.parser = template.New() return svc } +// setSubscriptionQueueDefaults function modifies the manifest to have +// 1. FIFO Topic names without ".fifo" suffix. +// 2. If there are both FIFO and Standard topic subscriptions are specified then set +// default events queue to FIFO and add standard topic-specific queue for all the standard topic subscriptions. +// 3. If there are only Standard topic subscriptions are specified then do nothing and return. +func setSubscriptionQueueDefaults(topics []TopicSubscription, eventsQueue *SQSQueue) { + var isFIFOEnabled bool + for _, topic := range topics { + if isFIFO(aws.StringValue(topic.Name)) { + isFIFOEnabled = true + break + } + } + if !isFIFOEnabled { + return + } + eventsQueue.FIFO.Enable = aws.Bool(true) + for idx, topic := range topics { + if isFIFO(aws.StringValue(topic.Name)) { + topics[idx].Name = aws.String(strings.TrimSuffix(aws.StringValue(topic.Name), ".fifo")) + } else { + topics[idx].Queue.Enabled = aws.Bool(true) + } + } +} + +func isFIFO(topic string) bool { + return strings.HasSuffix(topic, ".fifo") +} + // MarshalBinary serializes the manifest object into a binary YAML document. // Implements the encoding.BinaryMarshaler interface. func (s *WorkerService) MarshalBinary() ([]byte, error) { diff --git a/internal/pkg/manifest/worker_svc_test.go b/internal/pkg/manifest/worker_svc_test.go index 9edbc38779c..110dd4cf198 100644 --- a/internal/pkg/manifest/worker_svc_test.go +++ b/internal/pkg/manifest/worker_svc_test.go @@ -142,6 +142,223 @@ func TestNewWorkerSvc(t *testing.T) { }, }, }, + "should return a worker service instance with 2 subscriptions to the default fifo queue and 2 standard topic specific queues": { + inProps: WorkerServiceProps{ + WorkloadProps: WorkloadProps{ + Name: "testers", + Dockerfile: "./testers/Dockerfile", + }, + Topics: []TopicSubscription{ + { + Name: aws.String("fifoTopic1.fifo"), + Service: aws.String("fifoService1"), + }, + { + Name: aws.String("fifoTopic2.fifo"), + Service: aws.String("fifoService2"), + }, + { + Name: aws.String("standardTopic1"), + Service: aws.String("standardService1"), + }, + { + Name: aws.String("standardTopic2"), + Service: aws.String("standardService2"), + }, + }, + }, + wantedManifest: &WorkerService{ + Workload: Workload{ + Name: aws.String("testers"), + Type: aws.String(WorkerServiceType), + }, + WorkerServiceConfig: WorkerServiceConfig{ + ImageConfig: ImageWithHealthcheck{ + Image: Image{ + Build: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Dockerfile: aws.String("./testers/Dockerfile"), + }, + }, + }, + }, + Subscribe: SubscribeConfig{ + Topics: []TopicSubscription{ + { + Name: aws.String("fifoTopic1"), + Service: aws.String("fifoService1"), + }, + { + Name: aws.String("fifoTopic2"), + Service: aws.String("fifoService2"), + }, + { + Name: aws.String("standardTopic1"), + Service: aws.String("standardService1"), + Queue: SQSQueueOrBool{Enabled: aws.Bool(true)}, + }, + { + Name: aws.String("standardTopic2"), + Service: aws.String("standardService2"), + Queue: SQSQueueOrBool{Enabled: aws.Bool(true)}, + }, + }, + Queue: SQSQueue{ + FIFO: FIFOAdvanceConfigOrBool{Enable: aws.Bool(true)}, + }, + }, + TaskConfig: TaskConfig{ + CPU: aws.Int(256), + Memory: aws.Int(512), + Count: Count{ + Value: aws.Int(1), + }, + ExecuteCommand: ExecuteCommand{ + Enable: aws.Bool(false), + }, + }, + Network: NetworkConfig{ + VPC: vpcConfig{ + Placement: PlacementArgOrString{ + PlacementString: placementStringP(PublicSubnetPlacement), + }, + }, + }, + }, + }, + }, + "should return a worker service instance with 2 subscriptions to the default fifo queue": { + inProps: WorkerServiceProps{ + WorkloadProps: WorkloadProps{ + Name: "testers", + Dockerfile: "./testers/Dockerfile", + }, + Topics: []TopicSubscription{ + { + Name: aws.String("fifoTopic1.fifo"), + Service: aws.String("fifoService1"), + }, + { + Name: aws.String("fifoTopic2.fifo"), + Service: aws.String("fifoService2"), + }, + }, + }, + wantedManifest: &WorkerService{ + Workload: Workload{ + Name: aws.String("testers"), + Type: aws.String(WorkerServiceType), + }, + WorkerServiceConfig: WorkerServiceConfig{ + ImageConfig: ImageWithHealthcheck{ + Image: Image{ + Build: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Dockerfile: aws.String("./testers/Dockerfile"), + }, + }, + }, + }, + Subscribe: SubscribeConfig{ + Topics: []TopicSubscription{ + { + Name: aws.String("fifoTopic1"), + Service: aws.String("fifoService1"), + }, + { + Name: aws.String("fifoTopic2"), + Service: aws.String("fifoService2"), + }, + }, + Queue: SQSQueue{ + FIFO: FIFOAdvanceConfigOrBool{Enable: aws.Bool(true)}, + }, + }, + TaskConfig: TaskConfig{ + CPU: aws.Int(256), + Memory: aws.Int(512), + Count: Count{ + Value: aws.Int(1), + }, + ExecuteCommand: ExecuteCommand{ + Enable: aws.Bool(false), + }, + }, + Network: NetworkConfig{ + VPC: vpcConfig{ + Placement: PlacementArgOrString{ + PlacementString: placementStringP(PublicSubnetPlacement), + }, + }, + }, + }, + }, + }, + "should return a worker service instance with 2 subscriptions to the default standard queue": { + inProps: WorkerServiceProps{ + WorkloadProps: WorkloadProps{ + Name: "testers", + Dockerfile: "./testers/Dockerfile", + }, + Topics: []TopicSubscription{ + { + Name: aws.String("standardTopic1"), + Service: aws.String("standardService1"), + }, + { + Name: aws.String("standardTopic2"), + Service: aws.String("standardService2"), + }, + }, + }, + wantedManifest: &WorkerService{ + Workload: Workload{ + Name: aws.String("testers"), + Type: aws.String(WorkerServiceType), + }, + WorkerServiceConfig: WorkerServiceConfig{ + ImageConfig: ImageWithHealthcheck{ + Image: Image{ + Build: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Dockerfile: aws.String("./testers/Dockerfile"), + }, + }, + }, + }, + Subscribe: SubscribeConfig{ + Topics: []TopicSubscription{ + { + Name: aws.String("standardTopic1"), + Service: aws.String("standardService1"), + }, + { + Name: aws.String("standardTopic2"), + Service: aws.String("standardService2"), + }, + }, + Queue: SQSQueue{}, + }, + TaskConfig: TaskConfig{ + CPU: aws.Int(256), + Memory: aws.Int(512), + Count: Count{ + Value: aws.Int(1), + }, + ExecuteCommand: ExecuteCommand{ + Enable: aws.Bool(false), + }, + }, + Network: NetworkConfig{ + VPC: vpcConfig{ + Placement: PlacementArgOrString{ + PlacementString: placementStringP(PublicSubnetPlacement), + }, + }, + }, + }, + }, + }, } for name, tc := range testCases { diff --git a/internal/pkg/template/templates/workloads/services/worker/manifest.yml b/internal/pkg/template/templates/workloads/services/worker/manifest.yml index 7663c1d16aa..a2f36c7a064 100644 --- a/internal/pkg/template/templates/workloads/services/worker/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/worker/manifest.yml @@ -41,7 +41,14 @@ subscribe: {{- range $topic := .Subscribe.Topics}} - name: {{$topic.Name}} service: {{$topic.Service}} + {{- if $topic.Queue.Enabled }} + queue: {{ $topic.Queue.Enabled }} + {{- end }} {{- end}} + {{- if .Subscribe.Queue.FIFO.Enable }} + queue: + fifo: {{ .Subscribe.Queue.FIFO.Enable }} + {{- end }} {{- else}} # You can register to topics from other services. # The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. From ba55334906364ecdef5b4044f00f5b642d24389e Mon Sep 17 00:00:00 2001 From: danny randall <10566468+dannyrandall@users.noreply.github.com> Date: Mon, 10 Oct 2022 16:33:31 -0700 Subject: [PATCH 07/10] chore: always expect target container/port params in backend svc (#4078) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- .../stack/testdata/workloads/backend/simple-template.yml | 4 ++++ .../pkg/template/templates/workloads/services/backend/cf.yml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml index 2421825e317..d15c1f13db2 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml @@ -30,6 +30,10 @@ Parameters: LogRetention: Type: Number Default: 30 + TargetContainer: + Type: String + TargetPort: + Type: Number Conditions: HasAddons: !Not [!Equals [!Ref AddonsTemplateURL, ""]] HasEnvFile: !Not [!Equals [!Ref EnvFileARN, ""]] diff --git a/internal/pkg/template/templates/workloads/services/backend/cf.yml b/internal/pkg/template/templates/workloads/services/backend/cf.yml index 10bcc418524..38d4d788f20 100644 --- a/internal/pkg/template/templates/workloads/services/backend/cf.yml +++ b/internal/pkg/template/templates/workloads/services/backend/cf.yml @@ -35,11 +35,11 @@ Parameters: LogRetention: Type: Number Default: 30 - {{- if .ALBEnabled}} TargetContainer: Type: String TargetPort: Type: Number + {{- if .ALBEnabled}} HTTPSEnabled: Type: String AllowedValues: [true, false] From 101b95df4ebba7be7bd3713b5250f882cfeb8de3 Mon Sep 17 00:00:00 2001 From: danny randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 12 Oct 2022 09:28:42 -0700 Subject: [PATCH 08/10] docs: correct cdn.terminate_tls field (#4081) Resolves part of #4080 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- site/content/blogs/release-v122.en.md | 2 +- site/content/docs/developing/content-delivery.en.md | 2 +- site/content/docs/manifest/environment.en.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/site/content/blogs/release-v122.en.md b/site/content/blogs/release-v122.en.md index 5faf972ee93..49ee5864443 100644 --- a/site/content/blogs/release-v122.en.md +++ b/site/content/blogs/release-v122.en.md @@ -110,7 +110,7 @@ You can now configure your env manifest to have CloudFront terminate TLS for you ```yaml cdn: - tls_termination: true + terminate_tls: true ``` The configuration above uses CloudFront for TLS termination, which means the traffic from `CF → ALB → ECS` will be HTTP only. This brings faster TLS termination and shorter page loading for viewers, since the CloudFront edges are usually geographically closer to them. diff --git a/site/content/docs/developing/content-delivery.en.md b/site/content/docs/developing/content-delivery.en.md index 0d526c14f67..a85e68b5234 100644 --- a/site/content/docs/developing/content-delivery.en.md +++ b/site/content/docs/developing/content-delivery.en.md @@ -67,7 +67,7 @@ You can optionally use CloudFront for TLS termination by configuring the env man ```yaml cdn: - tls_termination: true + terminate_tls: true ``` And traffic from `CloudFront → Application Load Balancer (ALB) → ECS` will be HTTP only. This brings the benefit of terminating TLS at a geographically closer endpoint to the end user for faster TLS handshakes. diff --git a/site/content/docs/manifest/environment.en.md b/site/content/docs/manifest/environment.en.md index af772fcd20d..e16cd3b3e7e 100644 --- a/site/content/docs/manifest/environment.en.md +++ b/site/content/docs/manifest/environment.en.md @@ -211,7 +211,7 @@ cdn: certificate: "arn:aws:acm:us-east-1:1234567890:certificate/e5a6e114-b022-45b1-9339-38fbfd6db3e2" ``` -cdn.`tls_termination` Boolean +cdn.`terminate_tls` Boolean Enable TLS termination for CloudFront. From 0d0436a7f074d9c3715bad37e667a71c722a1c64 Mon Sep 17 00:00:00 2001 From: Penghao He Date: Wed, 12 Oct 2022 15:40:28 -0500 Subject: [PATCH 09/10] feat: add service connect mft (#4046) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- .../cloudformation/stack/backend_svc.go | 13 +++- .../cloudformation/stack/backend_svc_test.go | 23 +++--- ...lb_network_web_service_integration_test.go | 1 + .../stack/lb_web_service_integration_test.go | 1 + .../deploy/cloudformation/stack/lb_web_svc.go | 13 +++- .../cloudformation/stack/lb_web_svc_test.go | 16 +++- .../stack/testdata/stacklocal/manifest.yml | 2 + .../backend/http-autoscaling-manifest.yml | 3 + .../backend/http-full-config-manifest.yml | 3 + .../backend/http-only-path-manifest.yml | 3 + .../backend/https-path-alias-manifest.yml | 3 + .../workloads/backend/simple-template.yml | 23 ++++-- .../testdata/workloads/svc-grpc-manifest.yml | 3 + .../stack/testdata/workloads/svc-manifest.yml | 11 ++- .../testdata/workloads/svc-nlb-manifest.yml | 4 + .../testdata/workloads/svc-nlb-test.stack.yml | 17 ++++ .../testdata/workloads/svc-prod.params.json | 4 +- .../testdata/workloads/svc-prod.stack.yml | 29 +++++-- .../testdata/workloads/svc-test.stack.yml | 17 ++++ .../workloads/windows-svc-manifest.yml | 3 + .../cloudformation/stack/transformers.go | 8 +- .../cloudformation/stack/transformers_test.go | 20 ++--- .../deploy/cloudformation/stack/worker_svc.go | 2 +- internal/pkg/manifest/backend_svc.go | 17 ++++ internal/pkg/manifest/backend_svc_test.go | 77 +++++++++++++++++++ internal/pkg/manifest/lb_web_svc.go | 5 ++ internal/pkg/manifest/lb_web_svc_test.go | 35 +++++++++ internal/pkg/manifest/svc_test.go | 5 ++ internal/pkg/manifest/transform.go | 27 +++++++ internal/pkg/manifest/transform_test.go | 58 ++++++++++++++ internal/pkg/manifest/validate.go | 36 ++++++++- internal/pkg/manifest/validate_test.go | 45 +++++++++-- internal/pkg/manifest/workload.go | 53 +++++++++++-- internal/pkg/manifest/workload_test.go | 43 +++++++++++ .../partials/cf/service-base-properties.yml | 6 +- .../workloads/partials/cf/sidecars.yml | 9 +-- .../partials/cf/workload-container.yml | 13 ++-- internal/pkg/template/workload.go | 11 +-- 38 files changed, 581 insertions(+), 81 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 622824ec047..a2da5bc51d4 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -33,7 +33,8 @@ type BackendService struct { httpsEnabled bool albEnabled bool - parser backendSvcReadParser + parser backendSvcReadParser + SCFeatureFlag bool } // BackendServiceConfig contains data required to initialize a backend service stack. @@ -141,8 +142,11 @@ func (s *BackendService) Template() (string, error) { for _, ipNet := range s.manifest.RoutingRule.AllowedSourceIps { allowedSourceIPs = append(allowedSourceIPs, string(ipNet)) } - - _, targetContainerPort := s.httpLoadBalancerTarget() + var scConfig *template.ServiceConnect + if s.manifest.ServiceConnectEnabled() { + scConfig = convertServiceConnect(s.manifest.Network.Connect) + } + targetContainer, targetContainerPort := s.httpLoadBalancerTarget() content, err := s.parser.ParseBackendService(template.WorkloadOpts{ AppName: s.app, EnvName: s.env, @@ -166,7 +170,9 @@ func (s *BackendService) Template() (string, error) { HealthCheck: convertContainerHealthCheck(s.manifest.BackendServiceConfig.ImageConfig.HealthCheck), HTTPTargetContainer: template.HTTPTargetContainer{ Port: aws.StringValue(targetContainerPort), + Name: aws.StringValue(targetContainer), }, + ServiceConnect: scConfig, HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, AllowedSourceIps: allowedSourceIPs, @@ -190,6 +196,7 @@ func (s *BackendService) Template() (string, error) { }, HostedZoneAliases: hostedZoneAliases, PermissionsBoundary: s.permBound, + SCFeatureFlag: s.SCFeatureFlag, }) if err != nil { return "", fmt.Errorf("parse backend service template: %w", err) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go index e0ad2d2f0d7..0bb472bf5c6 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -271,6 +271,7 @@ Outputs: HostedZoneAliases: make(template.AliasesForHostedZone), HTTPTargetContainer: template.HTTPTargetContainer{ Port: "8080", + Name: "api", }, HTTPHealthCheck: template.HTTPHealthCheckOpts{ HealthCheckPath: manifest.DefaultHealthCheckPath, @@ -287,6 +288,7 @@ Outputs: Key: "sha2/count.zip", }, }, + ServiceConnect: &template.ServiceConnect{}, ExecuteCommand: &template.ExecuteCommandOpts{}, NestedStack: &template.WorkloadNestedStackOpts{ StackName: addon.StackName, @@ -430,13 +432,15 @@ Outputs: }, Sidecars: []*template.SidecarOpts{ { - Name: aws.String("envoy"), + Name: "envoy", Port: aws.String("443"), }, }, HTTPTargetContainer: template.HTTPTargetContainer{ + Name: "envoy", Port: "443", }, + ServiceConnect: &template.ServiceConnect{}, HTTPHealthCheck: template.HTTPHealthCheckOpts{ HealthCheckPath: "/healthz", Port: "4200", @@ -537,14 +541,6 @@ func TestBackendService_Parameters(t *testing.T) { ParameterKey: aws.String(WorkloadContainerPortParamKey), ParameterValue: aws.String("8080"), }, - { - ParameterKey: aws.String(WorkloadTargetContainerParamKey), - ParameterValue: aws.String("frontend"), - }, - { - ParameterKey: aws.String(WorkloadTargetPortParamKey), - ParameterValue: aws.String("8080"), - }, { ParameterKey: aws.String(WorkloadTaskCPUParamKey), ParameterValue: aws.String("256"), @@ -569,6 +565,14 @@ func TestBackendService_Parameters(t *testing.T) { ParameterKey: aws.String(WorkloadEnvFileARNParamKey), ParameterValue: aws.String(""), }, + { + ParameterKey: aws.String(WorkloadTargetContainerParamKey), + ParameterValue: aws.String("frontend"), + }, + { + ParameterKey: aws.String(WorkloadTargetPortParamKey), + ParameterValue: aws.String("8080"), + }, }, params) } @@ -650,6 +654,7 @@ func TestBackendService_TemplateAndParamsGeneration(t *testing.T) { EnvVersion: "v1.42.0", }, }) + serializer.SCFeatureFlag = true require.NoError(t, err) // mock parser for lambda functions diff --git a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go index 9f997e833ec..503a3263e3a 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go @@ -105,6 +105,7 @@ func TestNetworkLoadBalancedWebService_Template(t *testing.T) { }, RootUserARN: "arn:aws:iam::123456789123:root", }, stack.WithNLB([]string{"10.0.0.0/24", "10.1.0.0/24"})) + serializer.SCFeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go index 34042cac16c..59fb5dd04ac 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go @@ -105,6 +105,7 @@ func TestLoadBalancedWebService_Template(t *testing.T) { EnvVersion: "v1.42.0", }, }) + serializer.SCFeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index 25916287594..73bf9ed985a 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -38,7 +38,8 @@ type LoadBalancedWebService struct { publicSubnetCIDRBlocks []string appInfo deploy.AppInformation - parser loadBalancedWebSvcReadParser + parser loadBalancedWebSvcReadParser + SCFeatureFlag bool } // LoadBalancedWebServiceOption is used to configuring an optional field for LoadBalancedWebService. @@ -190,6 +191,10 @@ func (s *LoadBalancedWebService) Template() (string, error) { if s.manifest.RoutingRule.RedirectToHTTPS != nil { httpRedirect = aws.BoolValue(s.manifest.RoutingRule.RedirectToHTTPS) } + var scConfig *template.ServiceConnect + if s.manifest.ServiceConnectEnabled() { + scConfig = convertServiceConnect(s.manifest.Network.Connect) + } targetContainer, targetContainerPort := s.httpLoadBalancerTarget() content, err := s.parser.ParseLoadBalancedWebService(template.WorkloadOpts{ AppName: s.app, @@ -214,9 +219,10 @@ func (s *LoadBalancedWebService) Template() (string, error) { ExecuteCommand: convertExecuteCommand(&s.manifest.ExecuteCommand), WorkloadType: manifest.LoadBalancedWebServiceType, HTTPTargetContainer: template.HTTPTargetContainer{ - Port: aws.StringValue(targetContainerPort), - Container: aws.StringValue(targetContainer), + Port: aws.StringValue(targetContainerPort), + Name: aws.StringValue(targetContainer), }, + ServiceConnect: scConfig, HealthCheck: convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck), HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, @@ -242,6 +248,7 @@ func (s *LoadBalancedWebService) Template() (string, error) { }, HostedZoneAliases: aliasesFor, PermissionsBoundary: s.permBound, + SCFeatureFlag: s.SCFeatureFlag, }) if err != nil { return "", err 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 066207281dc..7d9d57e977f 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -207,6 +207,11 @@ Outputs: String: nil, StringSlice: []string{"world"}, } + mft.Network.Connect = manifest.ServiceConnectBoolOrArgs{ + ServiceConnectArgs: manifest.ServiceConnectArgs{ + Alias: aws.String("frontend"), + }, + } var actual template.WorkloadOpts parser := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) @@ -262,8 +267,11 @@ Outputs: HTTPRedirect: true, DeregistrationDelay: aws.Int64(60), HTTPTargetContainer: template.HTTPTargetContainer{ - Container: "frontend", - Port: "80", + Name: "frontend", + Port: "80", + }, + ServiceConnect: &template.ServiceConnect{ + Alias: aws.String("frontend"), }, HealthCheck: &template.ContainerHealthCheck{ Command: []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}, @@ -415,8 +423,8 @@ Outputs: // THEN require.NoError(t, err) require.Equal(t, template.HTTPTargetContainer{ - Port: "443", - Container: "envoy", + Port: "443", + Name: "envoy", }, actual.HTTPTargetContainer) }) } diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml index bb95b6f1bb2..be2565b4a81 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml @@ -31,6 +31,8 @@ count: memory_percentage: 80 # Enable running commands in your container. exec: true +network: + connect: false taskdef_overrides: - path: "ContainerDefinitions[0].Ulimits[-].Name" diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml index 0547c19f8f7..f77c95217ce 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml @@ -10,6 +10,9 @@ image: # Port exposed through your container to route traffic to it. port: 8080 +network: + connect: false + cpu: 512 # Number of CPU units for the task. memory: 1024 # Amount of memory in MiB used by the task. exec: true # Enable running commands in your container. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml index e856a9f9207..409382394aa 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml @@ -16,6 +16,9 @@ http: timeout: 10s grace_period: 45s +network: + connect: false + image: # Docker build arguments. For additional overrides: https://aws.github.io/copilot-cli/docs/manifest/backend-service/#image-build build: Dockerfile diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml index 677fb876bd7..6a0fd564f78 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml @@ -10,6 +10,9 @@ image: # Port exposed through your container to route traffic to it. port: 8080 +network: + connect: false + cpu: 512 # Number of CPU units for the task. memory: 1024 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml index 91e06831d51..cb80c24ceef 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml @@ -10,6 +10,9 @@ http: - name: "*.foobar.com" hosted_zone: mockHostedZone1 +network: + connect: false + image: # Docker build arguments. For additional overrides: https://aws.github.io/copilot-cli/docs/manifest/backend-service/#image-build build: Dockerfile diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml index d15c1f13db2..7757d5601af 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml @@ -89,12 +89,7 @@ Resources: awslogs-region: !Ref AWS::Region awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot - PortMappings: - !If [ - ExposePort, - [{ ContainerPort: !Ref ContainerPort }], - !Ref "AWS::NoValue", - ] + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] ExecutionRole: Metadata: "aws:copilot:description": "An IAM Role for the Fargate agent to make AWS API calls on your behalf" @@ -279,6 +274,22 @@ Resources: PropagateTags: SERVICE EnableExecuteCommand: true LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: my-env.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml index 32ecf794a21..5478c9d5504 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml @@ -37,6 +37,9 @@ publish: topics: - name: givesdogs +network: + connect: false + # Optional fields for more advanced use-cases. # variables: # Pass environment variables as key value pairs. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml index 87c8d6dc759..07ddad01086 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml @@ -60,6 +60,8 @@ environments: path: / grace_period: 30s deregistration_delay: 30s + network: + connect: false prod: count: range: @@ -80,9 +82,16 @@ environments: TEST: TEST secrets: GITHUB_TOKEN: GITHUB_TOKEN + http: + path: '/' + alias: example.com + target_container: nginx + network: + connect: + alias: api sidecars: nginx: - port: 80 + port: 8080 image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1 variables: NGINX_PORT: 80 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml index 2c2e8e5a404..f6a11dd4f3a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml @@ -10,6 +10,8 @@ image: build: ./Dockerfile port: 80 http: false +network: + connect: false nlb: port: 443/tls count: 5 @@ -26,6 +28,8 @@ environments: nlb: healthcheck: port: 80 + network: + connect: true dev: http: path: '/' diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml index 317e9e88e8d..16616e3d6e0 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml @@ -95,6 +95,7 @@ Resources: # If a bucket URL is specified, that means the template exists. awslogs-stream-prefix: copilot PortMappings: - ContainerPort: !Ref ContainerPort + Name: target - ContainerPort: 443 Protocol: tcp ExecutionRole: @@ -294,6 +295,22 @@ Resources: # If a bucket URL is specified, that means the template exists. MaximumPercent: 200 PropagateTags: SERVICE LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: test.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json index 48bae789f8e..25d6ed6f63e 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json @@ -11,8 +11,8 @@ "LogRetention": "1", "RulePath": "/", "Stickiness": "false", - "TargetContainer": "fe", - "TargetPort": "4000", + "TargetContainer": "nginx", + "TargetPort": "8080", "TaskCPU": "256", "TaskCount": "3", "TaskMemory": "512", diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml index 7c7f08f223d..8003a5f4009 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml @@ -143,7 +143,8 @@ Resources: # If a bucket URL is specified, that means the template exists. - Name: nginx Image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1 PortMappings: - - ContainerPort: 80 + - ContainerPort: 8080 + Name: target HealthCheck: Command: ["CMD-SHELL", "curl -f http://localhost:8080 || exit 1"] Interval: 10 @@ -190,11 +191,11 @@ Resources: # If a bucket URL is specified, that means the template exists. - 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 + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot Volumes: - Name: persistence ExecutionRole: @@ -518,6 +519,22 @@ Resources: # If a bucket URL is specified, that means the template exists. - CapacityProvider: FARGATE Weight: 0 Base: 5 + ServiceConnectConfiguration: + Enabled: True + Namespace: prod.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: api NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml index a71f21d5ea2..fd0fc9a4f43 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml @@ -109,6 +109,7 @@ Resources: # If a bucket URL is specified, that means the template exists. SourceVolume: persistence PortMappings: - ContainerPort: !Ref ContainerPort + Name: target Volumes: - Name: persistence ExecutionRole: @@ -430,6 +431,22 @@ Resources: # If a bucket URL is specified, that means the template exists. MaximumPercent: 200 PropagateTags: SERVICE LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: test.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml index 8b8638ab2e9..2138052f58a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml @@ -21,6 +21,9 @@ image: # Port exposed through your container to route traffic to it. port: 80 +network: + connect: false + cpu: 1024 # Number of CPU units for the task. memory: 2048 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index b69966f058c..88fb9267082 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -96,7 +96,7 @@ func convertSidecar(s map[string]*manifest.SidecarConfig) ([]*template.SidecarOp } mp := convertSidecarMountPoints(config.MountPoints) sidecars = append(sidecars, &template.SidecarOpts{ - Name: aws.String(name), + Name: name, Image: config.Image, Essential: config.Essential, Port: port, @@ -496,6 +496,12 @@ func convertExecuteCommand(e *manifest.ExecuteCommand) *template.ExecuteCommandO return &template.ExecuteCommandOpts{} } +func convertServiceConnect(s manifest.ServiceConnectBoolOrArgs) *template.ServiceConnect { + return &template.ServiceConnect{ + Alias: s.ServiceConnectArgs.Alias, + } +} + func convertLogging(lc manifest.Logging) *template.LogConfigOpts { if lc.IsEmpty() { return nil diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index 37ac078239d..9baaca2bce0 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -44,7 +44,7 @@ func Test_convertSidecar(t *testing.T) { inEssential: true, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -58,7 +58,7 @@ func Test_convertSidecar(t *testing.T) { inEssential: true, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), Protocol: aws.String("udp"), CredsParam: mockCredsParam, @@ -76,7 +76,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -96,7 +96,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -110,7 +110,7 @@ func Test_convertSidecar(t *testing.T) { }, "do not specify image override": { wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -126,7 +126,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -142,7 +142,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -158,7 +158,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -174,7 +174,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -190,7 +190,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, diff --git a/internal/pkg/deploy/cloudformation/stack/worker_svc.go b/internal/pkg/deploy/cloudformation/stack/worker_svc.go index 73abd19eef4..f40a5f97311 100644 --- a/internal/pkg/deploy/cloudformation/stack/worker_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/worker_svc.go @@ -5,11 +5,11 @@ package stack import ( "fmt" - "github.com/aws/copilot-cli/internal/pkg/config" "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudformation" + "github.com/aws/copilot-cli/internal/pkg/config" "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" diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index f79f9243db3..e064eabcfc8 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -98,6 +98,23 @@ func (s *BackendService) Port() (port uint16, ok bool) { return aws.Uint16Value(value), true } +// ServiceConnectEnabled returns if ServiceConnect is enabled or not. +// Unless explicitly disabled in the manifest or if no server is configured we default to true. +func (s *BackendService) ServiceConnectEnabled() bool { + if s.Network.Connect.EnableServiceConnect != nil { + return *s.Network.Connect.EnableServiceConnect + } + if !s.Network.Connect.ServiceConnectArgs.isEmpty() { + return true + } + // Try our best to enable Service Connect by default. + if s.BackendServiceConfig.ImageConfig.Port != nil || + s.BackendServiceConfig.RoutingRule.TargetContainer != nil { + return true + } + return false +} + // Publish returns the list of topics where notifications can be published. func (s *BackendService) Publish() []Topic { return s.BackendServiceConfig.PublishConfig.publishedTopics() diff --git a/internal/pkg/manifest/backend_svc_test.go b/internal/pkg/manifest/backend_svc_test.go index aaa883bd831..84ac90473cd 100644 --- a/internal/pkg/manifest/backend_svc_test.go +++ b/internal/pkg/manifest/backend_svc_test.go @@ -297,6 +297,83 @@ func TestBackendService_Port(t *testing.T) { } } +func TestBackendService_ServiceConnectEnabled(t *testing.T) { + testCases := map[string]struct { + mft *BackendService + + wanted bool + }{ + "enabled by default if main container exposes port": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: ImageWithHealthcheckAndOptionalPort{ + ImageWithOptionalPort: ImageWithOptionalPort{ + Port: uint16P(80), + }, + }, + }, + }, + wanted: true, + }, + "enabled by default if target container is set": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + RoutingRule: RoutingRuleConfiguration{ + TargetContainer: aws.String("nginx"), + }, + }, + }, + wanted: true, + }, + "disabled by default if no exposed port or no target container": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnectBoolOrArgs{}, + }, + }, + }, + wanted: false, + }, + "set by bool": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnectBoolOrArgs{ + EnableServiceConnect: aws.Bool(true), + }, + }, + }, + }, + wanted: true, + }, + "set by args": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnectBoolOrArgs{ + ServiceConnectArgs: ServiceConnectArgs{ + Alias: aws.String("api"), + }, + }, + }, + }, + }, + wanted: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // WHEN + enabled := tc.mft.ServiceConnectEnabled() + + // THEN + require.Equal(t, tc.wanted, enabled) + }) + } +} + func TestBackendService_Publish(t *testing.T) { testCases := map[string]struct { mft *BackendService diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index 7d9b23ecf26..0f6cd323dd0 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -169,6 +169,11 @@ func (s *LoadBalancedWebService) Port() (port uint16, ok bool) { return aws.Uint16Value(s.ImageConfig.Port), true } +// ServiceConnectEnabled returns if ServiceConnect is enabled or not. +func (s *LoadBalancedWebService) ServiceConnectEnabled() bool { + return s.Network.Connect.EnableServiceConnect == nil || *s.Network.Connect.EnableServiceConnect +} + // Publish returns the list of topics where notifications can be published. func (s *LoadBalancedWebService) Publish() []Topic { return s.LoadBalancedWebServiceConfig.PublishConfig.publishedTopics() diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index 27ad87879bf..2cf7fcbce31 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -1338,6 +1338,41 @@ func TestLoadBalancedWebService_Port(t *testing.T) { require.Equal(t, uint16(80), actual) } +func TestLoadBalancedWebService_ServiceConnectEnabled(t *testing.T) { + testCases := map[string]struct { + mft *LoadBalancedWebService + + wanted bool + }{ + "enabled by default": { + mft: &LoadBalancedWebService{}, + wanted: true, + }, + "disable if explicitly set": { + mft: &LoadBalancedWebService{ + LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnectBoolOrArgs{ + EnableServiceConnect: aws.Bool(false), + }, + }, + }, + }, + wanted: false, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // WHEN + enabled := tc.mft.ServiceConnectEnabled() + + // THEN + require.Equal(t, tc.wanted, enabled) + }) + } +} + func TestLoadBalancedWebService_Publish(t *testing.T) { testCases := map[string]struct { mft *LoadBalancedWebService diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index b0ddf5bc17a..192e6b4bad0 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -41,6 +41,8 @@ cpu: 512 memory: 1024 count: 1 exec: true +network: + connect: true http: path: "svc" target_container: "frontend" @@ -161,6 +163,9 @@ environments: PlacementString: placementStringP(PublicSubnetPlacement), }, }, + Connect: ServiceConnectBoolOrArgs{ + EnableServiceConnect: aws.Bool(true), + }, }, TaskDefOverrides: []OverrideRule{ { diff --git a/internal/pkg/manifest/transform.go b/internal/pkg/manifest/transform.go index b28b294109b..79cc426968c 100644 --- a/internal/pkg/manifest/transform.go +++ b/internal/pkg/manifest/transform.go @@ -23,6 +23,7 @@ var defaultTransformers = []mergo.Transformers{ stringSliceOrStringTransformer{}, platformArgsOrStringTransformer{}, securityGroupsIDsOrConfigTransformer{}, + serviceConnectTransformer{}, placementArgOrStringTransformer{}, subnetListOrArgsTransformer{}, healthCheckArgsOrStringTransformer{}, @@ -238,6 +239,32 @@ func (t placementArgOrStringTransformer) Transformer(typ reflect.Type) func(dst, } } +type serviceConnectTransformer struct{} + +// Transformer returns custom merge logic for serviceConnectTransformer's fields. +func (t serviceConnectTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { + if typ != reflect.TypeOf(ServiceConnectBoolOrArgs{}) { + return nil + } + + return func(dst, src reflect.Value) error { + dstStruct, srcStruct := dst.Interface().(ServiceConnectBoolOrArgs), src.Interface().(ServiceConnectBoolOrArgs) + + if srcStruct.EnableServiceConnect != nil { + dstStruct.ServiceConnectArgs = ServiceConnectArgs{} + } + + if !srcStruct.ServiceConnectArgs.isEmpty() { + dstStruct.EnableServiceConnect = nil + } + + if dst.CanSet() { // For extra safety to prevent panicking. + dst.Set(reflect.ValueOf(dstStruct)) + } + return nil + } +} + type subnetListOrArgsTransformer struct{} // Transformer returns custom merge logic for subnetListOrArgsTransformer's fields. diff --git a/internal/pkg/manifest/transform_test.go b/internal/pkg/manifest/transform_test.go index 0286bfbeaa4..9f7ed4cdf7c 100644 --- a/internal/pkg/manifest/transform_test.go +++ b/internal/pkg/manifest/transform_test.go @@ -592,6 +592,64 @@ func TestSubnetListOrArgsTransformer_Transformer(t *testing.T) { } } +func TestServiceConnectTransformer_Transformer(t *testing.T) { + testCases := map[string]struct { + original func(p *ServiceConnectBoolOrArgs) + override func(p *ServiceConnectBoolOrArgs) + wanted func(p *ServiceConnectBoolOrArgs) + }{ + "bool set to empty if args is not nil": { + original: func(s *ServiceConnectBoolOrArgs) { + s.EnableServiceConnect = aws.Bool(false) + }, + override: func(s *ServiceConnectBoolOrArgs) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + wanted: func(s *ServiceConnectBoolOrArgs) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + }, + "args set to empty if bool is not nil": { + original: func(s *ServiceConnectBoolOrArgs) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + override: func(s *ServiceConnectBoolOrArgs) { + s.EnableServiceConnect = aws.Bool(true) + }, + wanted: func(s *ServiceConnectBoolOrArgs) { + s.EnableServiceConnect = aws.Bool(true) + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + var dst, override, wanted ServiceConnectBoolOrArgs + + tc.original(&dst) + tc.override(&override) + tc.wanted(&wanted) + + // Perform default merge. + err := mergo.Merge(&dst, override, mergo.WithOverride) + require.NoError(t, err) + + // Use custom transformer. + err = mergo.Merge(&dst, override, mergo.WithOverride, mergo.WithTransformers(serviceConnectTransformer{})) + require.NoError(t, err) + + require.NoError(t, err) + require.Equal(t, wanted, dst) + }) + } +} + func TestHealthCheckArgsOrStringTransformer_Transformer(t *testing.T) { testCases := map[string]struct { original func(h *HealthCheckArgsOrString) diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 0c8d511b7f9..bd4a442ee8e 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -78,6 +78,7 @@ func (l LoadBalancedWebService) validate() error { } if err = validateTargetContainer(validateTargetContainerOpts{ mainContainerName: aws.StringValue(l.Name), + mainContainerPort: l.ImageConfig.Port, targetContainer: l.RoutingRule.GetTargetContainer(), sidecarConfig: l.Sidecars, }); err != nil { @@ -85,6 +86,7 @@ func (l LoadBalancedWebService) validate() error { } if err = validateTargetContainer(validateTargetContainerOpts{ mainContainerName: aws.StringValue(l.Name), + mainContainerPort: l.ImageConfig.Port, targetContainer: l.NLBConfig.TargetContainer, sidecarConfig: l.Sidecars, }); err != nil { @@ -193,6 +195,19 @@ func (b BackendService) validate() error { if err = b.Workload.validate(); err != nil { return err } + if err = validateTargetContainer(validateTargetContainerOpts{ + mainContainerName: aws.StringValue(b.Name), + mainContainerPort: b.ImageConfig.Port, + targetContainer: b.RoutingRule.GetTargetContainer(), + sidecarConfig: b.Sidecars, + }); err != nil { + return fmt.Errorf("validate HTTP load balancer target: %w", err) + } + if b.ServiceConnectEnabled() { + if b.RoutingRule.GetTargetContainer() == nil && b.ImageConfig.Port == nil { + return fmt.Errorf(`cannot enable "network.connect" when no port exposed`) + } + } if err = validateContainerDeps(validateDependenciesOpts{ sidecarConfig: b.Sidecars, imageConfig: b.ImageConfig.Image, @@ -1245,6 +1260,19 @@ func (n NetworkConfig) validate() error { if err := n.VPC.validate(); err != nil { return fmt.Errorf(`validate "vpc": %w`, err) } + if err := n.Connect.validate(); err != nil { + return fmt.Errorf(`validate "connect": %w`, err) + } + return nil +} + +// validate returns nil if ServiceConnectBoolOrArgs is configured correctly. +func (s ServiceConnectBoolOrArgs) validate() error { + return s.ServiceConnectArgs.validate() +} + +// validate is a no-op for ServiceConnectArgs. +func (ServiceConnectArgs) validate() error { return nil } @@ -1590,6 +1618,7 @@ type containerDependency struct { type validateTargetContainerOpts struct { mainContainerName string + mainContainerPort *uint16 targetContainer *string sidecarConfig map[string]*SidecarConfig } @@ -1609,14 +1638,17 @@ func validateTargetContainer(opts validateTargetContainerOpts) error { } targetContainer := aws.StringValue(opts.targetContainer) if targetContainer == opts.mainContainerName { + if opts.mainContainerPort == nil { + return fmt.Errorf("target container %q doesn't expose a port", targetContainer) + } return nil } sidecar, ok := opts.sidecarConfig[targetContainer] if !ok { - return fmt.Errorf("target container %s doesn't exist", targetContainer) + return fmt.Errorf("target container %q doesn't exist", targetContainer) } if sidecar.Port == nil { - return fmt.Errorf("target container %s doesn't expose any port", targetContainer) + return fmt.Errorf("target container %q doesn't expose a port", targetContainer) } return nil } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 7a123d4f2b1..d1d0515f7d3 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -82,7 +82,7 @@ func TestLoadBalancedWebService_validate(t *testing.T) { LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -409,7 +409,7 @@ func TestBackendService_validate(t *testing.T) { BackendServiceConfig: BackendServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -580,6 +580,37 @@ func TestBackendService_validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate "publish": `, }, + "error if target container not found": { + config: BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: testImageConfig, + RoutingRule: RoutingRuleConfiguration{ + TargetContainer: aws.String("api"), + Path: aws.String("/"), + }, + }, + Workload: Workload{ + Name: aws.String("api"), + }, + }, + wantedError: fmt.Errorf(`validate HTTP load balancer target: target container "api" doesn't expose a port`), + }, + "error if service connect is enabled without any port exposed": { + config: BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: testImageConfig, + Network: NetworkConfig{ + Connect: ServiceConnectBoolOrArgs{ + EnableServiceConnect: aws.Bool(true), + }, + }, + }, + Workload: Workload{ + Name: aws.String("api"), + }, + }, + wantedError: fmt.Errorf(`cannot enable "network.connect" when no port exposed`), + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -786,7 +817,7 @@ func TestWorkerService_validate(t *testing.T) { WorkerServiceConfig: WorkerServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -992,7 +1023,7 @@ func TestScheduledJob_validate(t *testing.T) { ScheduledJobConfig: ScheduledJobConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -3033,9 +3064,9 @@ func TestValidateLoadBalancerTarget(t *testing.T) { mainContainerName: "mockMainContainer", targetContainer: aws.String("foo"), }, - wanted: fmt.Errorf("target container foo doesn't exist"), + wanted: fmt.Errorf(`target container "foo" doesn't exist`), }, - "should return an error if target container doesn't expose any port": { + "should return an error if target container doesn't expose a port": { in: validateTargetContainerOpts{ mainContainerName: "mockMainContainer", targetContainer: aws.String("foo"), @@ -3043,7 +3074,7 @@ func TestValidateLoadBalancerTarget(t *testing.T) { "foo": {}, }, }, - wanted: fmt.Errorf("target container foo doesn't expose any port"), + wanted: fmt.Errorf(`target container "foo" doesn't expose a port`), }, "success with no target container set": { in: validateTargetContainerOpts{ diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 2767a3a9066..8a02e0184b8 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -47,13 +47,14 @@ var ( var ( ErrAppRunnerInvalidPlatformWindows = errors.New("Windows is not supported for App Runner services") - errUnmarshalBuildOpts = errors.New("unable to unmarshal build field into string or compose-style map") - errUnmarshalPlatformOpts = errors.New("unable to unmarshal platform field into string or compose-style map") - errUnmarshalSecurityGroupOpts = errors.New(`unable to unmarshal "security_groups" field into slice of strings or compose-style map`) - errUnmarshalPlacementOpts = errors.New("unable to unmarshal placement field into string or compose-style map") - errUnmarshalSubnetsOpts = errors.New("unable to unmarshal subnets field into string slice or compose-style map") - errUnmarshalCountOpts = errors.New(`unable to unmarshal "count" field to an integer or autoscaling configuration`) - errUnmarshalRangeOpts = errors.New(`unable to unmarshal "range" field`) + errUnmarshalBuildOpts = errors.New("unable to unmarshal build field into string or compose-style map") + errUnmarshalPlatformOpts = errors.New("unable to unmarshal platform field into string or compose-style map") + errUnmarshalSecurityGroupOpts = errors.New(`unable to unmarshal "security_groups" field into slice of strings or compose-style map`) + errUnmarshalPlacementOpts = errors.New("unable to unmarshal placement field into string or compose-style map") + errUnmarshalServiceConnectOpts = errors.New(`unable to unmarshal "connect" field into boolean or compose-style map`) + errUnmarshalSubnetsOpts = errors.New("unable to unmarshal subnets field into string slice or compose-style map") + errUnmarshalCountOpts = errors.New(`unable to unmarshal "count" field to an integer or autoscaling configuration`) + errUnmarshalRangeOpts = errors.New(`unable to unmarshal "range" field`) errUnmarshalExec = errors.New(`unable to unmarshal "exec" field into boolean or exec configuration`) errUnmarshalEntryPoint = errors.New(`unable to unmarshal "entrypoint" into string or slice of strings`) @@ -458,7 +459,8 @@ func (t *FIFOTopicAdvanceConfigOrBool) UnmarshalYAML(value *yaml.Node) error { // NetworkConfig represents options for network connection to AWS resources within a VPC. type NetworkConfig struct { - VPC vpcConfig `yaml:"vpc"` + VPC vpcConfig `yaml:"vpc"` + Connect ServiceConnectBoolOrArgs `yaml:"connect"` } // IsEmpty returns empty if the struct has all zero members. @@ -473,6 +475,41 @@ func (c *NetworkConfig) requiredEnvFeatures() []string { return nil } +// ServiceConnectBoolOrArgs represents ECS Service Connect configuration. +type ServiceConnectBoolOrArgs struct { + EnableServiceConnect *bool + ServiceConnectArgs +} + +// UnmarshalYAML overrides the default YAML unmarshaling logic for the ServiceConnect +// struct, allowing it to perform more complex unmarshaling behavior. +// This method implements the yaml.Unmarshaler (v3) interface. +func (s *ServiceConnectBoolOrArgs) UnmarshalYAML(value *yaml.Node) error { + if err := value.Decode(&s.ServiceConnectArgs); err != nil { + var yamlTypeErr *yaml.TypeError + if !errors.As(err, &yamlTypeErr) { + return err + } + } + if !s.ServiceConnectArgs.isEmpty() { + s.EnableServiceConnect = nil + return nil + } + if err := value.Decode(&s.EnableServiceConnect); err != nil { + return errUnmarshalServiceConnectOpts + } + return nil +} + +// ServiceConnectArgs includes the advanced configuration for ECS Service Connect. +type ServiceConnectArgs struct { + Alias *string +} + +func (s *ServiceConnectArgs) isEmpty() bool { + return s.Alias == nil +} + // PlacementArgOrString represents where to place tasks. type PlacementArgOrString struct { *PlacementString diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index bf18c7eb5cd..9fa794ee71b 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -354,6 +354,49 @@ func TestPlatformArgsOrString_UnmarshalYAML(t *testing.T) { } } +func TestServiceConnect_UnmarshalYAML(t *testing.T) { + testCases := map[string]struct { + inContent []byte + + wantedStruct ServiceConnectBoolOrArgs + wantedError error + }{ + "returns error if both bool and args specified": { + inContent: []byte(`connect: true + alias: api`), + + wantedError: errors.New("yaml: line 2: mapping values are not allowed in this context"), + }, + "error if unmarshalable": { + inContent: []byte(`connect: + ohess: linus + archie: leg64`), + wantedError: errUnmarshalServiceConnectOpts, + }, + "success": { + inContent: []byte(`connect: + alias: api`), + wantedStruct: ServiceConnectBoolOrArgs{ + ServiceConnectArgs: ServiceConnectArgs{ + Alias: aws.String("api"), + }, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + v := NetworkConfig{} + err := yaml.Unmarshal(tc.inContent, &v) + if tc.wantedError != nil { + require.EqualError(t, err, tc.wantedError.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedStruct, v.Connect) + } + }) + } +} + func TestPlacementArgOrString_UnmarshalYAML(t *testing.T) { testCases := map[string]struct { inContent []byte diff --git a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml index 03e9a27ec1d..e066eaf68f6 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml @@ -33,10 +33,10 @@ CapacityProviderStrategy: {{- end}} {{- end}} {{- end }} -{{- if .ServiceConnect }} +{{- if and .ServiceConnect .SCFeatureFlag }} ServiceConnectConfiguration: Enabled: True - Namespace: {{.ServiceConnect.Namespace}} + Namespace: {{.ServiceDiscoveryEndpoint}} LogConfiguration: LogDriver: awslogs Options: @@ -44,7 +44,7 @@ ServiceConnectConfiguration: awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot Services: - - PortName: TargetPort + - PortName: target # Avoid using the same service with Service Discovery in a namespace. DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] ClientAliases: diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index 770df616c85..64691acb590 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -48,11 +48,10 @@ {{- if $sidecar.Port}} PortMappings: - ContainerPort: {{$sidecar.Port}} - {{- if $.ServiceConnect }} # remove when release - {{- if eq $.HTTPTargetContainer.Container $sidecar.Name}} - {{- if eq $.HTTPTargetContainer.Port $sidecar.Port}} - Name: TargetPort - {{- end}} + {{- if and $.ServiceConnect $.SCFeatureFlag}} # remove when release + {{/* Multiple exposed port is not supported yet. Therefore, if the target container is the sidecar, the target port must be the one and only one port that the container exposes. */}} + {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} + Name: target {{- end}} {{- end}} {{- if $sidecar.Protocol}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 9845a28f0cb..e451cb152ad 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -29,9 +29,10 @@ {{- if eq .WorkloadType "Load Balanced Web Service"}} PortMappings: - ContainerPort: !Ref ContainerPort - {{- if .ServiceConnect }} # remove when release - {{- if eq .HTTPTargetContainer.Container .WorkloadName}} - Name: TargetPort + {{- if and .ServiceConnect .SCFeatureFlag}} # remove when release + {{/* Multiple exposed port is not supported yet. Therefore, if the target container is the sidecar, the target port must be the one and only one port that the container exposes. */}} + {{- if eq .HTTPTargetContainer.Name .WorkloadName}} + Name: target {{- end}} {{- end}} {{- if .NLB}} {{ $nlbListener := .NLB.Listener }} @@ -43,12 +44,14 @@ {{- end}} {{- end}} {{/* end if eq .WorkloadType "Load Balanced Web Service"*/}} {{- if eq .WorkloadType "Backend Service"}} -{{- if .ServiceConnect }} # remove when release - PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: !Ref WorkloadName}], !Ref "AWS::NoValue"] +{{- if eq .HTTPTargetContainer.Name .WorkloadName}} +{{- if and .ServiceConnect .SCFeatureFlag}} # remove when release + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] {{- else}} PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort}], !Ref "AWS::NoValue"] {{- end}} {{- end}} +{{- end}} {{- if .HealthCheck}} HealthCheck: Command: {{quoteSlice .HealthCheck.Command | fmtSlice}} diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index a5bd3f555cb..b10baf5933a 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -121,7 +121,7 @@ type WorkloadNestedStackOpts struct { // SidecarOpts holds configuration that's needed if the service has sidecar containers. type SidecarOpts struct { - Name *string + Name string Image *string Essential *bool Port *string @@ -210,8 +210,8 @@ type LogConfigOpts struct { // HTTPTargetContainer represents the target group of a load balancer that points to a container. type HTTPTargetContainer struct { - Container string - Port string // Port of the container. + Name string + Port string } // IsHTTPS returns true if the target container's port is 443. @@ -331,8 +331,7 @@ type NetworkLoadBalancer struct { // ServiceConnect holds configuration for ECS Service Connect. type ServiceConnect struct { - Namespace string - Alias *string + Alias *string } // AdvancedCount holds configuration for autoscaling and capacity provider @@ -597,6 +596,8 @@ type WorkloadOpts struct { // Additional options for worker service templates. Subscribe *SubscribeOpts + + SCFeatureFlag bool } // HealthCheckProtocol returns the protocol for the Load Balancer health check, From 2a387f56d293aa5c8bad5157e5068fe2544ff92c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 13 Oct 2022 17:00:41 +0000 Subject: [PATCH 10/10] chore: Bump github.com/onsi/gomega from 1.20.2 to 1.21.1 (#4073) Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.20.2 to 1.21.1.
Release notes

Sourced from github.com/onsi/gomega's releases.

v1.21.1

Features

  • Eventually and Consistently that are passed a SpecContext can provide reports when an interrupt occurs [0d063c9]

v1.21.0

1.21.0

Features

  • Eventually and Consistently can take a context.Context [65c01bc] This enables integration with Ginkgo 2.3.0's interruptible nodes and node timeouts.
  • Introduces Eventually.Within.ProbeEvery with tests and documentation (#591) [f633800]
  • New BeKeyOf matcher with documentation and unit tests (#590) [fb586b3]

Fixes

  • Cover the entire gmeasure suite with leak detection [8c54344]
  • Fix gmeasure leak [119d4ce]
  • Ignore new Ginkgo ProgressSignal goroutine in gleak [ba548e2]

Maintenance

  • Fixes crashes on newer Ruby 3 installations by upgrading github-pages gem dependency (#596) [12469a0]
Changelog

Sourced from github.com/onsi/gomega's changelog.

v1.21.1

Features

  • Eventually and Consistently that are passed a SpecContext can provide reports when an interrupt occurs [0d063c9]

1.21.0

Features

  • Eventually and Consistently can take a context.Context [65c01bc] This enables integration with Ginkgo 2.3.0's interruptible nodes and node timeouts.
  • Introduces Eventually.Within.ProbeEvery with tests and documentation (#591) [f633800]
  • New BeKeyOf matcher with documentation and unit tests (#590) [fb586b3]

Fixes

  • Cover the entire gmeasure suite with leak detection [8c54344]
  • Fix gmeasure leak [119d4ce]
  • Ignore new Ginkgo ProgressSignal goroutine in gleak [ba548e2]

Maintenance

  • Fixes crashes on newer Ruby 3 installations by upgrading github-pages gem dependency (#596) [12469a0]
Commits
  • 2e34979 v1.21.1
  • 0d063c9 Eventually and Consistently that are passed a SpecContext can provide reports...
  • 2ba5763 v1.21.0
  • 65c01bc Eventually and Consistently can take a context.Context
  • 12469a0 fixes crashes on newer Ruby 3 installations by upgrading github-pages gem dep...
  • b8636ad documentation updates for BeKeyOf and well-known non-leaky goroutines (#592)
  • f633800 introduces Eventually.Within.ProbeEvery with tests and documentation (#591)
  • fb586b3 new BeKeyOf matcher with documentation and unit tests (#590)
  • 647a36b welp; remove local Ginkgo replace directive in go.mod
  • 8c54344 cover the entire gmeasure suite with leak detection
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/onsi/gomega&package-manager=go_modules&previous-version=1.20.2&new-version=1.21.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
--- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1d29538e224..bcfc20296e0 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/lnquy/cron v1.1.1 github.com/moby/buildkit v0.9.3 github.com/onsi/ginkgo v1.16.5 - github.com/onsi/gomega v1.20.2 + github.com/onsi/gomega v1.21.1 github.com/robfig/cron/v3 v3.0.1 github.com/spf13/afero v1.9.2 github.com/spf13/cobra v1.5.0 diff --git a/go.sum b/go.sum index 1c596af2f45..33975da8e8c 100644 --- a/go.sum +++ b/go.sum @@ -889,8 +889,8 @@ github.com/onsi/gomega v1.8.1/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoT github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDsH8xc= -github.com/onsi/gomega v1.20.2 h1:8uQq0zMgLEfa0vRrrBgaJF2gyW9Da9BmfGV+OyUzfkY= -github.com/onsi/gomega v1.20.2/go.mod h1:iYAIXgPSaDHak0LCMA+AWBpIKBr8WZicMxnE8luStNc= +github.com/onsi/gomega v1.21.1 h1:OB/euWYIExnPBohllTicTHmGTrMaqJ67nIu80j0/uEM= +github.com/onsi/gomega v1.21.1/go.mod h1:iYAIXgPSaDHak0LCMA+AWBpIKBr8WZicMxnE8luStNc= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=