From 7d2b0394c79b59d40ddd68a67d5bf849b0fe9b84 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 13 Jul 2022 17:36:07 -0700 Subject: [PATCH 1/9] add sg.allow_vpc_ingress to env manifest --- internal/pkg/manifest/env.go | 15 ++++++++++++--- internal/pkg/manifest/env_test.go | 8 ++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/internal/pkg/manifest/env.go b/internal/pkg/manifest/env.go index cf8b1fc139a..2eab0d041a9 100644 --- a/internal/pkg/manifest/env.go +++ b/internal/pkg/manifest/env.go @@ -336,11 +336,20 @@ func (cfg publicHTTPConfig) IsEmpty() bool { } type privateHTTPConfig struct { - InternalALBSubnets []string `yaml:"subnets,omitempty"` - Certificates []string `yaml:"certificates,omitempty"` + InternalALBSubnets []string `yaml:"subnets,omitempty"` + Certificates []string `yaml:"certificates,omitempty"` + SecurityGroupsConfig securityGroupsConfig `yaml:"security_groups,omitempty"` } // IsEmpty returns true if there is no customization to the internal ALB. func (cfg privateHTTPConfig) IsEmpty() bool { - return len(cfg.InternalALBSubnets) == 0 && len(cfg.Certificates) == 0 + return len(cfg.InternalALBSubnets) == 0 && len(cfg.Certificates) == 0 && cfg.SecurityGroupsConfig.isEmpty() +} + +type securityGroupsConfig struct { + AllowVPCIngress *bool `yaml:"allow_vpc_ingress"` +} + +func (cfg securityGroupsConfig) isEmpty() bool { + return cfg.AllowVPCIngress == nil } diff --git a/internal/pkg/manifest/env_test.go b/internal/pkg/manifest/env_test.go index 74d39b30b35..d49a32b4d0c 100644 --- a/internal/pkg/manifest/env_test.go +++ b/internal/pkg/manifest/env_test.go @@ -427,6 +427,9 @@ http: certificates: - cert-1 - cert-2 + private: + security_groups: + allow_vpc_ingress: false `, wantedStruct: &Environment{ Workload: Workload{ @@ -438,6 +441,11 @@ http: Public: publicHTTPConfig{ Certificates: []string{"cert-1", "cert-2"}, }, + Private: privateHTTPConfig{ + SecurityGroupsConfig: securityGroupsConfig{ + AllowVPCIngress: aws.Bool(false), + }, + }, }, }, }, From 3d6ddf973802c8574875d36a8287b6426abd2d11 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 13 Jul 2022 17:51:31 -0700 Subject: [PATCH 2/9] remove env upgrade test suites --- .../pkg/deploy/cloudformation/env_test.go | 336 ------------------ 1 file changed, 336 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/env_test.go b/internal/pkg/deploy/cloudformation/env_test.go index 75e9e1239cb..dfd10577ef9 100644 --- a/internal/pkg/deploy/cloudformation/env_test.go +++ b/internal/pkg/deploy/cloudformation/env_test.go @@ -5,352 +5,16 @@ package cloudformation import ( "errors" - "fmt" - "io" "testing" "github.com/aws/aws-sdk-go/aws" awscfn "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/aws/copilot-cli/internal/pkg/aws/cloudformation" - "github.com/aws/copilot-cli/internal/pkg/deploy" "github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/mocks" - "github.com/aws/copilot-cli/internal/pkg/template" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" ) -func TestCloudFormation_UpgradeEnvironment(t *testing.T) { - mockCreateEnvInput := deploy.CreateEnvironmentInput{ - App: deploy.AppInformation{ - Name: "phonetool", - Domain: "phonetool.com", - }, - Name: "test", - Version: "v1.0.0", - ArtifactBucketARN: "arn:aws:s3:::mockbucket", - CustomResourcesURLs: map[string]string{ - template.DNSCertValidatorFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey1", - template.DNSDelegationFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey2", - template.CustomDomainFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey4", - }, - } - testCases := map[string]struct { - in *deploy.CreateEnvironmentInput - mockDeployer func(t *testing.T, ctrl *gomock.Controller) *CloudFormation - - wantedErr error - }{ - "upgrades using previous values when stack is available": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - m.EXPECT().Describe("phonetool-test").Return(&cloudformation.StackDescription{ - Parameters: []*awscfn.Parameter{ - { - ParameterKey: aws.String("ALBWorkloads"), - ParameterValue: aws.String("frontend,admin"), - }, - }, - }, nil) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(nil).Do(func(s *cloudformation.Stack) { - require.ElementsMatch(t, s.Parameters, []*awscfn.Parameter{ - { - ParameterKey: aws.String("ALBWorkloads"), - UsePreviousValue: aws.Bool(true), - }, - { - ParameterKey: aws.String("InternalALBWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("AppName"), - ParameterValue: aws.String("phonetool"), - }, - { - ParameterKey: aws.String("EnvironmentName"), - ParameterValue: aws.String("test"), - }, - { - ParameterKey: aws.String("ToolsAccountPrincipalARN"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("AppDNSName"), - ParameterValue: aws.String("phonetool.com"), - }, - { - ParameterKey: aws.String("AppDNSDelegationRole"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("NATWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("EFSWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("Aliases"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("ServiceDiscoveryEndpoint"), - ParameterValue: aws.String("test.phonetool.local"), - }, - { - ParameterKey: aws.String("CreateHTTPSListener"), - ParameterValue: aws.String("true"), - }, - { - ParameterKey: aws.String("CreateInternalHTTPSListener"), - ParameterValue: aws.String("false"), - }, - }) - }) - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload("mockbucket", gomock.Any(), gomock.Any()).DoAndReturn(func(bucket, key string, data io.Reader) (string, error) { - require.Contains(t, key, "manual/templates/phonetool-test/") - return "url", nil - }) - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - }, - "waits until stack is available for update": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - s3 := mocks.NewMocks3Client(ctrl) - - gomock.InOrder( - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", nil), - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{}, nil).AnyTimes(), - m.EXPECT().UpdateAndWait(gomock.Any()).Return(&cloudformation.ErrStackUpdateInProgress{ - Name: "phonetool-test", - }).AnyTimes(), - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{ - StackStatus: aws.String("UPDATE_IN_PROGRESS"), - }, nil).AnyTimes(), - m.EXPECT().WaitForUpdate(gomock.Any(), "phonetool-test").Return(nil).AnyTimes(), - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{}, nil).AnyTimes(), - m.EXPECT().UpdateAndWait(gomock.Any()).Return(nil), - ) - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - }, - "should exit successfully if there are no updates needed": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", nil) - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{}, nil) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(fmt.Errorf("update and wait: %w", &cloudformation.ErrChangeSetEmpty{})) - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - }, - "should retry if the changeset request becomes obsolete": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", nil) - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{}, nil).Times(2) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(fmt.Errorf("update and wait: %w", &cloudformation.ErrChangeSetNotExecutable{})) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(nil) - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - }, - "wrap error on unexpected update err": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", nil) - m.EXPECT().Describe(gomock.Any()).Return(&cloudformation.StackDescription{}, nil) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(errors.New("some error")) - - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - - wantedErr: errors.New("update and wait for stack phonetool-test: some error"), - }, - "return error when failed to upload template to s3": { - in: &mockCreateEnvInput, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", errors.New("some error")) - - return &CloudFormation{ - s3Client: s3, - } - }, - - wantedErr: errors.New("some error"), - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // GIVEN - ctrl := gomock.NewController(t) - defer ctrl.Finish() - cf := tc.mockDeployer(t, ctrl) - - // WHEN - err := cf.UpgradeEnvironment(tc.in) - - // THEN - if tc.wantedErr != nil { - require.EqualError(t, err, tc.wantedErr.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestCloudFormation_UpgradeLegacyEnvironment(t *testing.T) { - mockCreateEnvInput := deploy.CreateEnvironmentInput{ - App: deploy.AppInformation{ - Name: "phonetool", - }, - Name: "test", - Version: "v1.0.0", - ArtifactBucketARN: "arn:aws:s3:::mockbucket", - CustomResourcesURLs: map[string]string{ - template.DNSCertValidatorFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey1", - template.DNSDelegationFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey2", - template.CustomDomainFileName: "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey4", - }, - } - testCases := map[string]struct { - in *deploy.CreateEnvironmentInput - lbWebServices []string - mockDeployer func(t *testing.T, ctrl *gomock.Controller) *CloudFormation - - wantedErr error - }{ - "replaces IncludePublicLoadBalancer param with ALBWorkloads and preserves existing params": { - in: &mockCreateEnvInput, - lbWebServices: []string{"frontend", "admin"}, - mockDeployer: func(t *testing.T, ctrl *gomock.Controller) *CloudFormation { - m := mocks.NewMockcfnClient(ctrl) - m.EXPECT().Describe("phonetool-test").Return(&cloudformation.StackDescription{ - Parameters: []*awscfn.Parameter{ - { - ParameterKey: aws.String("IncludePublicLoadBalancer"), - ParameterValue: aws.String("true"), - }, - { - ParameterKey: aws.String("EnvironmentName"), - ParameterValue: aws.String("test"), - }, - }, - }, nil) - m.EXPECT().UpdateAndWait(gomock.Any()).Return(nil).Do(func(s *cloudformation.Stack) { - require.ElementsMatch(t, s.Parameters, []*awscfn.Parameter{ - { - ParameterKey: aws.String("ALBWorkloads"), - ParameterValue: aws.String("frontend,admin"), - }, - { - ParameterKey: aws.String("InternalALBWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("EnvironmentName"), - UsePreviousValue: aws.Bool(true), - }, - { - ParameterKey: aws.String("AppName"), - ParameterValue: aws.String("phonetool"), - }, - { - ParameterKey: aws.String("ToolsAccountPrincipalARN"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("AppDNSName"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("AppDNSDelegationRole"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("NATWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("EFSWorkloads"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("Aliases"), - ParameterValue: aws.String(""), - }, - { - ParameterKey: aws.String("ServiceDiscoveryEndpoint"), - ParameterValue: aws.String("test.phonetool.local"), - }, - { - ParameterKey: aws.String("CreateHTTPSListener"), - ParameterValue: aws.String("false"), - }, - { - ParameterKey: aws.String("CreateInternalHTTPSListener"), - ParameterValue: aws.String("false"), - }, - }) - }) - - s3 := mocks.NewMocks3Client(ctrl) - s3.EXPECT().Upload(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return("", nil) - - return &CloudFormation{ - cfnClient: m, - s3Client: s3, - } - }, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // GIVEN - ctrl := gomock.NewController(t) - defer ctrl.Finish() - cf := tc.mockDeployer(t, ctrl) - - // WHEN - err := cf.UpgradeLegacyEnvironment(tc.in, tc.lbWebServices...) - - // THEN - if tc.wantedErr != nil { - require.EqualError(t, err, tc.wantedErr.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - func TestCloudFormation_EnvironmentTemplate(t *testing.T) { testCases := map[string]struct { inAppName string From c1de9b7641285becf06728e0cfd4eec832ceb26a Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 13 Jul 2022 17:51:57 -0700 Subject: [PATCH 3/9] use manifest instead of SSM in env stack tests --- .../pkg/deploy/cloudformation/stack/env_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/env_test.go b/internal/pkg/deploy/cloudformation/stack/env_test.go index 1aafcd7ed51..ff8eadd5554 100644 --- a/internal/pkg/deploy/cloudformation/stack/env_test.go +++ b/internal/pkg/deploy/cloudformation/stack/env_test.go @@ -14,6 +14,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/deploy" "github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack/mocks" + "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -33,7 +34,7 @@ func TestEnv_Template(t *testing.T) { AppName: "project", EnvName: "env", VPCConfig: template.VPCConfig{ - Imported: &template.ImportVPC{}, + Imported: nil, Managed: template.ManagedVPC{ CIDR: DefaultVPCCIDR, PrivateSubnetCIDRs: DefaultPrivateSubnetCIDRs, @@ -55,6 +56,10 @@ func TestEnv_Template(t *testing.T) { Key: "mockkey4", }, }, + Telemetry: &template.Telemetry{ + EnableContainerInsights: false, + }, + SerializedManifest: "name: env\ntype: Environment\n", }, data) return &template.Content{Buffer: bytes.NewBufferString("mockTemplate")}, nil }) @@ -93,7 +98,7 @@ func TestEnv_Parameters(t *testing.T) { deploymentInputWithDNS := mockDeployEnvironmentInput() deploymentInputWithDNS.App.Domain = "ecs.aws" deploymentInputWithPrivateDNS := mockDeployEnvironmentInput() - deploymentInputWithPrivateDNS.ImportCertARNs = []string{"arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"} + deploymentInputWithPrivateDNS.Mft.HTTPConfig.Private.Certificates = []string{"arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"} testCases := map[string]struct { input *deploy.CreateEnvironmentInput oldParams []*cloudformation.Parameter @@ -669,6 +674,11 @@ func mockDeployEnvironmentInput() *deploy.CreateEnvironmentInput { "DNSDelegationFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey2", "CustomDomainFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/mockkey4", }, - ImportVPCConfig: &config.ImportVPC{}, + Mft: &manifest.Environment{ + Workload: manifest.Workload{ + Name: aws.String("env"), + Type: aws.String("Environment"), + }, + }, } } From 9a849ce65f403db5b09675c5e2aca738f1ef2ce5 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 13 Jul 2022 17:52:14 -0700 Subject: [PATCH 4/9] feed manifest to stack --- internal/pkg/deploy/cloudformation/stack/env.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/deploy/cloudformation/stack/env.go b/internal/pkg/deploy/cloudformation/stack/env.go index ea6ec27cf16..bdd6201b305 100644 --- a/internal/pkg/deploy/cloudformation/stack/env.go +++ b/internal/pkg/deploy/cloudformation/stack/env.go @@ -105,7 +105,7 @@ func (e *EnvStackConfig) Template() (string, error) { PrivateImportedCertARNs: e.importPrivateCertARNs(), VPCConfig: e.vpcConfig(), CustomInternalALBSubnets: e.internalALBSubnets(), - AllowVPCIngress: e.in.AllowVPCIngress, // TODO(jwh): fetch AllowVPCIngress from Manifest or SSM. + AllowVPCIngress: aws.BoolValue(e.in.Mft.HTTPConfig.Private.SecurityGroupsConfig.AllowVPCIngress), Telemetry: e.telemetryConfig(), CDNConfig: e.cdnConfig(), From 08f973df5b913f65bde583900b160a5eab0a96d8 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 14 Jul 2022 09:34:41 -0700 Subject: [PATCH 5/9] fix e2e; g --- .../stack/env_integration_test.go | 9 +++---- .../template-with-basic-manifest.yml | 24 ------------------- ...late-with-imported-certs-observability.yml | 2 +- 3 files changed, 6 insertions(+), 29 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go index ace0a8055b9..94dc6543411 100644 --- a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go @@ -37,6 +37,9 @@ http: certificates: - cert-1 - cert-2 + private: + security_groups: + allow_vpc_ingress: true observability: container_insights: true # Enable container insights. `), &mft) @@ -55,8 +58,7 @@ observability: "DNSDelegationFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/dns-delegation", "CustomDomainFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/custom-domain", }, - AllowVPCIngress: true, - Mft: &mft, + Mft: &mft, } }(), wantedFileName: "template-with-imported-certs-observability.yml", @@ -83,8 +85,7 @@ type: Environment "DNSDelegationFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/dns-delegation", "CustomDomainFunction": "https://mockbucket.s3-us-west-2.amazonaws.com/custom-domain", }, - AllowVPCIngress: true, - Mft: &mft, + Mft: &mft, } }(), wantedFileName: "template-with-basic-manifest.yml", diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml index d8b0d497368..85deaac73d4 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml @@ -645,30 +645,6 @@ Resources: GroupId: !Ref InternalLoadBalancerSecurityGroup IpProtocol: -1 SourceSecurityGroupId: !Ref EnvironmentSecurityGroup - InternalLoadBalancerSecurityGroupIngressFromHttp: - Metadata: - 'aws:copilot:description': 'An inbound rule to the internal load balancer security group for port 80 within the VPC' - Type: AWS::EC2::SecurityGroupIngress - Condition: CreateInternalALB - Properties: - Description: Allow from within the VPC on port 80 - CidrIp: 0.0.0.0/0 - FromPort: 80 - ToPort: 80 - IpProtocol: tcp - GroupId: !Ref InternalLoadBalancerSecurityGroup - InternalLoadBalancerSecurityGroupIngressFromHttps: - Metadata: - 'aws:copilot:description': 'An inbound rule to the internal load balancer security group for port 443 within the VPC' - Type: AWS::EC2::SecurityGroupIngress - Condition: ExportInternalHTTPSListener - Properties: - Description: Allow from within the VPC on port 443 - CidrIp: 0.0.0.0/0 - FromPort: 443 - ToPort: 443 - IpProtocol: tcp - GroupId: !Ref InternalLoadBalancerSecurityGroup PublicLoadBalancer: Metadata: 'aws:copilot:description': 'An Application Load Balancer to distribute public traffic to your services' 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 d2740dd074e..1627ff40446 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 @@ -6,7 +6,7 @@ Metadata: name: test type: Environment observability: {container_insights: true} - http: {public: {certificates: [cert-1, cert-2]}} + http: {public: {certificates: [cert-1, cert-2]}, private: {security_groups: {allow_vpc_ingress: true}}} Parameters: AppName: From b04fe340676efad45e32c604e331217556636073 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 15 Jul 2022 15:53:29 -0700 Subject: [PATCH 6/9] addr fb: change manifest design to sg.ingress.from_vpc` --- internal/pkg/deploy/cloudformation/stack/env.go | 2 +- internal/pkg/manifest/env.go | 12 ++++++++++-- internal/pkg/manifest/env_test.go | 7 +++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/env.go b/internal/pkg/deploy/cloudformation/stack/env.go index bdd6201b305..8e4fafa1b81 100644 --- a/internal/pkg/deploy/cloudformation/stack/env.go +++ b/internal/pkg/deploy/cloudformation/stack/env.go @@ -105,7 +105,7 @@ func (e *EnvStackConfig) Template() (string, error) { PrivateImportedCertARNs: e.importPrivateCertARNs(), VPCConfig: e.vpcConfig(), CustomInternalALBSubnets: e.internalALBSubnets(), - AllowVPCIngress: aws.BoolValue(e.in.Mft.HTTPConfig.Private.SecurityGroupsConfig.AllowVPCIngress), + AllowVPCIngress: aws.BoolValue(e.in.Mft.HTTPConfig.Private.SecurityGroupsConfig.Ingress.VPCIngress), Telemetry: e.telemetryConfig(), CDNConfig: e.cdnConfig(), diff --git a/internal/pkg/manifest/env.go b/internal/pkg/manifest/env.go index 2eab0d041a9..7366cf5532e 100644 --- a/internal/pkg/manifest/env.go +++ b/internal/pkg/manifest/env.go @@ -347,9 +347,17 @@ func (cfg privateHTTPConfig) IsEmpty() bool { } type securityGroupsConfig struct { - AllowVPCIngress *bool `yaml:"allow_vpc_ingress"` + Ingress ingress `yaml:"Ingress"` } func (cfg securityGroupsConfig) isEmpty() bool { - return cfg.AllowVPCIngress == nil + return cfg.Ingress.isEmpty() +} + +type ingress struct { + VPCIngress *bool `yaml:"from_vpc"` +} + +func (i ingress) isEmpty() bool { + return i.VPCIngress == nil } diff --git a/internal/pkg/manifest/env_test.go b/internal/pkg/manifest/env_test.go index d49a32b4d0c..8283f12222b 100644 --- a/internal/pkg/manifest/env_test.go +++ b/internal/pkg/manifest/env_test.go @@ -429,7 +429,8 @@ http: - cert-2 private: security_groups: - allow_vpc_ingress: false + Ingress: + from_vpc: false `, wantedStruct: &Environment{ Workload: Workload{ @@ -443,7 +444,9 @@ http: }, Private: privateHTTPConfig{ SecurityGroupsConfig: securityGroupsConfig{ - AllowVPCIngress: aws.Bool(false), + Ingress: ingress{ + VPCIngress: aws.Bool(false), + }, }, }, }, From d7b9a503ec6770d252ac1e0dda3213bc95fd0718 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 15 Jul 2022 16:06:00 -0700 Subject: [PATCH 7/9] convert from ssm to manifest --- internal/pkg/manifest/env.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/pkg/manifest/env.go b/internal/pkg/manifest/env.go index 7366cf5532e..2e9399e92a1 100644 --- a/internal/pkg/manifest/env.go +++ b/internal/pkg/manifest/env.go @@ -318,9 +318,11 @@ func (cfg *environmentHTTPConfig) loadLBConfig(env *config.CustomizeEnv) { if env.IsEmpty() { return } + if env.ImportVPC != nil && len(env.ImportVPC.PublicSubnetIDs) == 0 { cfg.Private.InternalALBSubnets = env.InternalALBSubnets cfg.Private.Certificates = env.ImportCertARNs + cfg.Private.SecurityGroupsConfig.Ingress.VPCIngress = aws.Bool(env.EnableInternalALBVPCIngress) return } cfg.Public.Certificates = env.ImportCertARNs From 86e9b66dd9d84ec3f6a79f66ba392478b272c82c Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 15 Jul 2022 17:11:40 -0700 Subject: [PATCH 8/9] reolve conflicts --- internal/pkg/deploy/cloudformation/stack/env_test.go | 6 ++++++ internal/pkg/manifest/env_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/internal/pkg/deploy/cloudformation/stack/env_test.go b/internal/pkg/deploy/cloudformation/stack/env_test.go index 3ce316e4724..195e33a5924 100644 --- a/internal/pkg/deploy/cloudformation/stack/env_test.go +++ b/internal/pkg/deploy/cloudformation/stack/env_test.go @@ -56,6 +56,9 @@ func TestEnv_Template(t *testing.T) { Key: "mockkey4", }, }, + Telemetry: &template.Telemetry{ + EnableContainerInsights: false, + }, SerializedManifest: "name: env\ntype: Environment\n", ForceUpdateID: "mockPreviousForceUpdateID", }, data) @@ -679,5 +682,8 @@ func mockDeployEnvironmentInput() *deploy.CreateEnvironmentInput { Type: aws.String("Environment"), }, }, + RawMft: []byte(`name: env +type: Environment +`), } } diff --git a/internal/pkg/manifest/env_test.go b/internal/pkg/manifest/env_test.go index 8283f12222b..9632b96939f 100644 --- a/internal/pkg/manifest/env_test.go +++ b/internal/pkg/manifest/env_test.go @@ -278,6 +278,11 @@ func TestFromEnvConfig(t *testing.T) { Private: privateHTTPConfig{ InternalALBSubnets: []string{"subnet2"}, Certificates: []string{"arn:aws:acm:region:account:certificate/certificate_ID_1", "arn:aws:acm:region:account:certificate/certificate_ID_2"}, + SecurityGroupsConfig: securityGroupsConfig{ + Ingress: ingress{ + VPCIngress: aws.Bool(false), + }, + }, }, }, }, From 079d8664ba40a72cd7bf1d0c63bca5c40899f862 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 15 Jul 2022 17:29:35 -0700 Subject: [PATCH 9/9] fix some tests` --- .../pkg/deploy/cloudformation/stack/env_integration_test.go | 3 ++- .../template-with-imported-certs-observability.yml | 2 +- internal/pkg/manifest/env.go | 2 +- internal/pkg/manifest/env_test.go | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go index cafb4b2fdaf..3ad3ada4c85 100644 --- a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go @@ -37,7 +37,8 @@ http: - cert-2 private: security_groups: - allow_vpc_ingress: true + ingress: + from_vpc: true observability: container_insights: true # Enable container insights.` var mft manifest.Environment 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 be3ffb490fa..f1bc0d0c5b3 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 @@ -4,7 +4,7 @@ Description: CloudFormation environment template for infrastructure shared among Metadata: Manifest: | name: test - type: Environmentg + type: Environment # Create the public ALB with certificates attached. http: public: diff --git a/internal/pkg/manifest/env.go b/internal/pkg/manifest/env.go index a9cfd724bc7..5f117bca19f 100644 --- a/internal/pkg/manifest/env.go +++ b/internal/pkg/manifest/env.go @@ -349,7 +349,7 @@ func (cfg privateHTTPConfig) IsEmpty() bool { } type securityGroupsConfig struct { - Ingress ingress `yaml:"Ingress"` + Ingress ingress `yaml:"ingress"` } func (cfg securityGroupsConfig) isEmpty() bool { diff --git a/internal/pkg/manifest/env_test.go b/internal/pkg/manifest/env_test.go index 9632b96939f..80955fecea6 100644 --- a/internal/pkg/manifest/env_test.go +++ b/internal/pkg/manifest/env_test.go @@ -434,7 +434,7 @@ http: - cert-2 private: security_groups: - Ingress: + ingress: from_vpc: false `, wantedStruct: &Environment{