diff --git a/internal/pkg/aws/ec2/ec2.go b/internal/pkg/aws/ec2/ec2.go index 72b1b6e2d8c..091237262c0 100644 --- a/internal/pkg/aws/ec2/ec2.go +++ b/internal/pkg/aws/ec2/ec2.go @@ -472,24 +472,24 @@ func (c *EC2) managedPrefixList(prefixListName string) (*ec2.DescribeManagedPref } // CloudFrontManagedPrefixListID returns the PrefixListId of the associated cloudfront prefix list as a *string. -func (c *EC2) CloudFrontManagedPrefixListID() (*string, error) { +func (c *EC2) CloudFrontManagedPrefixListID() (string, error) { prefixListsOutput, err := c.managedPrefixList(cloudFrontPrefixListName) if err != nil { - return nil, err + return "", err } - var ids []*string + var ids []string for _, v := range prefixListsOutput.PrefixLists { - ids = append(ids, v.PrefixListId) + ids = append(ids, *v.PrefixListId) } if len(ids) == 0 { - return nil, fmt.Errorf("cannot find any prefix list with name: %s", cloudFrontPrefixListName) + return "", fmt.Errorf("cannot find any prefix list with name: %s", cloudFrontPrefixListName) } if len(ids) > 1 { - return nil, fmt.Errorf("found more than one prefix list with the name %s: %v", cloudFrontPrefixListName, ids) + return "", fmt.Errorf("found more than one prefix list with the name %s: %v", cloudFrontPrefixListName, ids) } return ids[0], nil diff --git a/internal/pkg/aws/ec2/ec2_test.go b/internal/pkg/aws/ec2/ec2_test.go index f2aeeda8f82..1142ec7ec03 100644 --- a/internal/pkg/aws/ec2/ec2_test.go +++ b/internal/pkg/aws/ec2/ec2_test.go @@ -368,7 +368,7 @@ func TestEC2_CloudFrontManagedPrefixListId(t *testing.T) { wantedError error wantedErrorMsgPrefix string - wantedId *string + wantedId string }{ "query returns error": { mockEC2Client: func(m *mocks.Mockapi) { @@ -418,7 +418,7 @@ func TestEC2_CloudFrontManagedPrefixListId(t *testing.T) { }, }, nil) }, - wantedId: aws.String(mockPrefixListId), + wantedId: mockPrefixListId, }, } diff --git a/internal/pkg/cli/deploy/env.go b/internal/pkg/cli/deploy/env.go index 9bc1e43b04d..05eae7765bb 100644 --- a/internal/pkg/cli/deploy/env.go +++ b/internal/pkg/cli/deploy/env.go @@ -10,6 +10,7 @@ import ( 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/aws/ec2" "github.com/aws/copilot-cli/internal/pkg/aws/s3" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" @@ -34,13 +35,18 @@ type environmentDeployer interface { ForceUpdateOutputID(app, env string) (string, error) } +type prefixListGetter interface { + CloudFrontManagedPrefixListID() (string, error) +} + type envDeployer struct { app *config.Application env *config.Environment // Dependencies to upload artifacts. - templateFS template.Reader - s3 uploader + templateFS template.Reader + s3 uploader + prefixListGetter prefixListGetter // Dependencies to deploy an environment. appCFN appResourcesGetter envDeployer environmentDeployer @@ -75,8 +81,9 @@ func NewEnvDeployer(in *NewEnvDeployerInput) (*envDeployer, error) { app: in.App, env: in.Env, - templateFS: template.New(), - s3: s3.New(envRegionSession), + templateFS: template.New(), + s3: s3.New(envRegionSession), + prefixListGetter: ec2.New(envRegionSession), appCFN: deploycfn.New(defaultSession), envDeployer: deploycfn.New(envManagerSession), @@ -109,6 +116,31 @@ func (d *envDeployer) uploadCustomResources(bucket string) (map[string]string, e return urls, nil } +func (d *envDeployer) cidrPrefixLists(in *DeployEnvironmentInput) ([]string, error) { + var cidrPrefixListIDs []string + + // Check if ingress is allowed from cloudfront + if in.Manifest == nil || !in.Manifest.IsIngressRestrictedToCDN() { + return nil, nil + } + cfManagedPrefixListID, err := d.cfManagedPrefixListID() + if err != nil { + return nil, err + } + cidrPrefixListIDs = append(cidrPrefixListIDs, cfManagedPrefixListID) + + return cidrPrefixListIDs, nil +} + +func (d *envDeployer) cfManagedPrefixListID() (string, error) { + id, err := d.prefixListGetter.CloudFrontManagedPrefixListID() + if err != nil { + return "", fmt.Errorf("retrieve CloudFront managed prefix list id: %w", err) + } + + return id, nil +} + // DeployEnvironmentInput contains information used to deploy the environment. type DeployEnvironmentInput struct { RootUserARN string @@ -188,6 +220,10 @@ func (d *envDeployer) buildStackInput(in *DeployEnvironmentInput) (*deploy.Creat if err != nil { return nil, err } + cidrPrefixListIDs, err := d.cidrPrefixLists(in) + if err != nil { + return nil, err + } return &deploy.CreateEnvironmentInput{ Name: d.env.Name, App: deploy.AppInformation{ @@ -199,6 +235,7 @@ func (d *envDeployer) buildStackInput(in *DeployEnvironmentInput) (*deploy.Creat CustomResourcesURLs: in.CustomResourcesURLs, ArtifactBucketARN: s3.FormatARN(partition.ID(), resources.S3Bucket), ArtifactBucketKeyARN: resources.KMSKeyARN, + CIDRPrefixListIDs: cidrPrefixListIDs, Mft: in.Manifest, ForceUpdate: in.ForceNewUpdate, RawMft: in.RawManifest, diff --git a/internal/pkg/cli/deploy/env_test.go b/internal/pkg/cli/deploy/env_test.go index de647fe9a78..d8839803a5f 100644 --- a/internal/pkg/cli/deploy/env_test.go +++ b/internal/pkg/cli/deploy/env_test.go @@ -10,12 +10,14 @@ import ( "strings" "testing" + "github.com/aws/aws-sdk-go/aws" awscfn "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/aws/copilot-cli/internal/pkg/cli/deploy/mocks" "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" "github.com/aws/copilot-cli/internal/pkg/deploy/upload/customresource" + "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" ) @@ -116,9 +118,10 @@ func TestEnvDeployer_UploadArtifacts(t *testing.T) { } type deployEnvironmentMock struct { - appCFN *mocks.MockappResourcesGetter - envDeployer *mocks.MockenvironmentDeployer - stackSerializer *mocks.MockstackSerializer + appCFN *mocks.MockappResourcesGetter + envDeployer *mocks.MockenvironmentDeployer + stackSerializer *mocks.MockstackSerializer + prefixListGetter *mocks.MockprefixListGetter } func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) { @@ -248,6 +251,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { } testCases := map[string]struct { setUpMocks func(m *deployEnvironmentMock) + inManifest *manifest.Environment wantedError error }{ "fail to get app resources by region": { @@ -257,6 +261,42 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { }, wantedError: fmt.Errorf("get app resources in region %s: some error", mockEnvRegion), }, + "fail to get prefix list id": { + setUpMocks: func(m *deployEnvironmentMock) { + m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{ + S3Bucket: "mockS3Bucket", + }, nil) + m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("", errors.New("some error")) + }, + inManifest: &manifest.Environment{ + EnvironmentConfig: manifest.EnvironmentConfig{ + HTTPConfig: manifest.EnvironmentHTTPConfig{ + Public: manifest.PublicHTTPConfig{ + SecurityGroupConfig: manifest.ALBSecurityGroupsConfig{ + Ingress: manifest.Ingress{ + RestrictiveIngress: manifest.RestrictiveIngress{ + CDNIngress: aws.Bool(true), + }, + }, + }, + }, + }, + }, + }, + wantedError: fmt.Errorf("retrieve CloudFront managed prefix list id: some error"), + }, + "prefix list not retrieved when manifest not present": { + setUpMocks: func(m *deployEnvironmentMock) { + m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{ + S3Bucket: "mockS3Bucket", + }, nil) + m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil) + m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil) + m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0) + m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + }, + inManifest: nil, + }, "fail to get existing parameters": { setUpMocks: func(m *deployEnvironmentMock) { m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{ @@ -281,6 +321,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{ S3Bucket: "mockS3Bucket", }, nil) + m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0) m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil) m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil) m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("some error")) @@ -292,6 +333,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{ S3Bucket: "mockS3Bucket", }, nil) + m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0) m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil) m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil) m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) @@ -304,8 +346,9 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { defer ctrl.Finish() m := &deployEnvironmentMock{ - appCFN: mocks.NewMockappResourcesGetter(ctrl), - envDeployer: mocks.NewMockenvironmentDeployer(ctrl), + appCFN: mocks.NewMockappResourcesGetter(ctrl), + envDeployer: mocks.NewMockenvironmentDeployer(ctrl), + prefixListGetter: mocks.NewMockprefixListGetter(ctrl), } tc.setUpMocks(m) d := envDeployer{ @@ -315,14 +358,16 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) { ManagerRoleARN: mockManagerRoleARN, Region: mockEnvRegion, }, - appCFN: m.appCFN, - envDeployer: m.envDeployer, + appCFN: m.appCFN, + envDeployer: m.envDeployer, + prefixListGetter: m.prefixListGetter, } mockIn := &DeployEnvironmentInput{ RootUserARN: "mockRootUserARN", CustomResourcesURLs: map[string]string{ "mockResource": "mockURL", }, + Manifest: tc.inManifest, } gotErr := d.DeployEnvironment(mockIn) if tc.wantedError != nil { diff --git a/internal/pkg/cli/deploy/mocks/mock_env.go b/internal/pkg/cli/deploy/mocks/mock_env.go index 74292a16ca4..59771f77b58 100644 --- a/internal/pkg/cli/deploy/mocks/mock_env.go +++ b/internal/pkg/cli/deploy/mocks/mock_env.go @@ -125,3 +125,41 @@ func (mr *MockenvironmentDeployerMockRecorder) UpdateAndRenderEnvironment(out, c varargs := append([]interface{}{out, conf, bucketARN}, opts...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateAndRenderEnvironment", reflect.TypeOf((*MockenvironmentDeployer)(nil).UpdateAndRenderEnvironment), varargs...) } + +// MockprefixListGetter is a mock of prefixListGetter interface. +type MockprefixListGetter struct { + ctrl *gomock.Controller + recorder *MockprefixListGetterMockRecorder +} + +// MockprefixListGetterMockRecorder is the mock recorder for MockprefixListGetter. +type MockprefixListGetterMockRecorder struct { + mock *MockprefixListGetter +} + +// NewMockprefixListGetter creates a new mock instance. +func NewMockprefixListGetter(ctrl *gomock.Controller) *MockprefixListGetter { + mock := &MockprefixListGetter{ctrl: ctrl} + mock.recorder = &MockprefixListGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockprefixListGetter) EXPECT() *MockprefixListGetterMockRecorder { + return m.recorder +} + +// CloudFrontManagedPrefixListID mocks base method. +func (m *MockprefixListGetter) CloudFrontManagedPrefixListID() (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CloudFrontManagedPrefixListID") + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CloudFrontManagedPrefixListID indicates an expected call of CloudFrontManagedPrefixListID. +func (mr *MockprefixListGetterMockRecorder) CloudFrontManagedPrefixListID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudFrontManagedPrefixListID", reflect.TypeOf((*MockprefixListGetter)(nil).CloudFrontManagedPrefixListID)) +} diff --git a/internal/pkg/deploy/cloudformation/stack/env.go b/internal/pkg/deploy/cloudformation/stack/env.go index 14074577316..43770cd1c8f 100644 --- a/internal/pkg/deploy/cloudformation/stack/env.go +++ b/internal/pkg/deploy/cloudformation/stack/env.go @@ -96,18 +96,19 @@ func (e *EnvStackConfig) Template() (string, error) { forceUpdateID = id.String() } content, err := e.parser.ParseEnv(&template.EnvOpts{ - AppName: e.in.App.Name, - EnvName: e.in.Name, - CustomResources: crs, - ArtifactBucketARN: e.in.ArtifactBucketARN, - ArtifactBucketKeyARN: e.in.ArtifactBucketKeyARN, - PublicImportedCertARNs: e.importPublicCertARNs(), - PrivateImportedCertARNs: e.importPrivateCertARNs(), - VPCConfig: e.vpcConfig(), - CustomInternalALBSubnets: e.internalALBSubnets(), - AllowVPCIngress: aws.BoolValue(e.in.Mft.HTTPConfig.Private.SecurityGroupsConfig.Ingress.VPCIngress), - Telemetry: e.telemetryConfig(), - CDNConfig: e.cdnConfig(), + AppName: e.in.App.Name, + EnvName: e.in.Name, + CustomResources: crs, + ArtifactBucketARN: e.in.ArtifactBucketARN, + ArtifactBucketKeyARN: e.in.ArtifactBucketKeyARN, + PublicFacingCIDRPrefixListIDs: e.in.CIDRPrefixListIDs, + PublicImportedCertARNs: e.importPublicCertARNs(), + PrivateImportedCertARNs: e.importPrivateCertARNs(), + VPCConfig: e.vpcConfig(), + CustomInternalALBSubnets: e.internalALBSubnets(), + AllowVPCIngress: aws.BoolValue(e.in.Mft.HTTPConfig.Private.SecurityGroupsConfig.Ingress.VPCIngress), + Telemetry: e.telemetryConfig(), + CDNConfig: e.cdnConfig(), Version: e.in.Version, LatestVersion: deploy.LatestEnvTemplateVersion, diff --git a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go index 3ad3ada4c85..53c239229d3 100644 --- a/internal/pkg/deploy/cloudformation/stack/env_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/env_integration_test.go @@ -30,8 +30,13 @@ func TestEnvStack_Template(t *testing.T) { rawMft := `name: test type: Environment # Create the public ALB with certificates attached. +cdn: true http: public: + security_groups: + ingress: + restrict_to: + cdn: true certificates: - cert-1 - cert-2 @@ -51,6 +56,7 @@ observability: Name: "demo", }, Name: "test", + CIDRPrefixListIDs: []string{"pl-mockid"}, ArtifactBucketARN: "arn:aws:s3:::mockbucket", ArtifactBucketKeyARN: "arn:aws:kms:us-west-2:000000000:key/1234abcd-12ab-34cd-56ef-1234567890ab", CustomResourcesURLs: map[string]string{ 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 ae3816949d4..4ced904dc8f 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 @@ -570,19 +570,31 @@ Resources: ClusterSettings: - Name: containerInsights Value: disabled - PublicLoadBalancerSecurityGroup: + PublicHTTPLoadBalancerSecurityGroup: Metadata: - 'aws:copilot:description': 'A security group for your load balancer allowing HTTP and HTTPS traffic' + 'aws:copilot:description': 'A security group for your load balancer allowing HTTP traffic' Condition: CreateALB Type: AWS::EC2::SecurityGroup Properties: - GroupDescription: Access to the public facing load balancer + GroupDescription: HTTP access to the public facing load balancer SecurityGroupIngress: - CidrIp: 0.0.0.0/0 Description: Allow from anyone on port 80 FromPort: 80 IpProtocol: tcp ToPort: 80 + VpcId: !Ref VPC + Tags: + - Key: Name + Value: !Sub 'copilot-${AppName}-${EnvironmentName}-lb' + PublicHTTPSLoadBalancerSecurityGroup: + Metadata: + 'aws:copilot:description': 'A security group for your load balancer allowing HTTPS traffic' + Condition: ExportHTTPSListener + Type: AWS::EC2::SecurityGroup + Properties: + GroupDescription: HTTPS access to the public facing load balancer + SecurityGroupIngress: - CidrIp: 0.0.0.0/0 Description: Allow from anyone on port 443 FromPort: 443 @@ -614,14 +626,22 @@ Resources: Tags: - Key: Name Value: !Sub 'copilot-${AppName}-${EnvironmentName}-env' - EnvironmentSecurityGroupIngressFromPublicALB: + EnvironmentHTTPSecurityGroupIngressFromPublicALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateALB Properties: - Description: Ingress from the public ALB + Description: HTTP ingress from the public ALB + GroupId: !Ref EnvironmentSecurityGroup + IpProtocol: -1 + SourceSecurityGroupId: !Ref PublicHTTPLoadBalancerSecurityGroup + EnvironmentHTTPSSecurityGroupIngressFromPublicALB: + Type: AWS::EC2::SecurityGroupIngress + Condition: ExportHTTPSListener + Properties: + Description: HTTPS ingress from the public ALB GroupId: !Ref EnvironmentSecurityGroup IpProtocol: -1 - SourceSecurityGroupId: !Ref PublicLoadBalancerSecurityGroup + SourceSecurityGroupId: !Ref PublicHTTPSLoadBalancerSecurityGroup EnvironmentSecurityGroupIngressFromInternalALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateInternalALB @@ -652,7 +672,9 @@ Resources: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Scheme: internet-facing - SecurityGroups: [ !GetAtt PublicLoadBalancerSecurityGroup.GroupId ] + SecurityGroups: + - !GetAtt PublicHTTPLoadBalancerSecurityGroup.GroupId + - !If [ExportHTTPSListener, !GetAtt PublicHTTPSLoadBalancerSecurityGroup.GroupId, !Ref "AWS::NoValue"] Subnets: [ !Ref PublicSubnet1, !Ref PublicSubnet2, ] Type: application # Assign a dummy target group that with no real services as targets, so that we can create 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 f1bc0d0c5b3..be8557d688b 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,8 +6,13 @@ Metadata: name: test type: Environment # Create the public ALB with certificates attached. + cdn: true http: public: + security_groups: + ingress: + restrict_to: + cdn: true certificates: - cert-1 - cert-2 @@ -263,21 +268,33 @@ Resources: ClusterSettings: - Name: containerInsights Value: enabled - PublicLoadBalancerSecurityGroup: + PublicHTTPLoadBalancerSecurityGroup: Metadata: - 'aws:copilot:description': 'A security group for your load balancer allowing HTTP and HTTPS traffic' + 'aws:copilot:description': 'A security group for your load balancer allowing HTTP traffic' Condition: CreateALB Type: AWS::EC2::SecurityGroup Properties: - GroupDescription: Access to the public facing load balancer + GroupDescription: HTTP access to the public facing load balancer SecurityGroupIngress: - - CidrIp: 0.0.0.0/0 - Description: Allow from anyone on port 80 + - SourcePrefixListId: pl-mockid + Description: Allow ingress from prefix list pl-mockid on port 80 FromPort: 80 IpProtocol: tcp ToPort: 80 - - CidrIp: 0.0.0.0/0 - Description: Allow from anyone on port 443 + VpcId: !Ref VPC + Tags: + - Key: Name + Value: !Sub 'copilot-${AppName}-${EnvironmentName}-lb' + PublicHTTPSLoadBalancerSecurityGroup: + Metadata: + 'aws:copilot:description': 'A security group for your load balancer allowing HTTPS traffic' + Condition: ExportHTTPSListener + Type: AWS::EC2::SecurityGroup + Properties: + GroupDescription: HTTPS access to the public facing load balancer + SecurityGroupIngress: + - SourcePrefixListId: pl-mockid + Description: Allow ingress from prefix list pl-mockid on port 443 FromPort: 443 IpProtocol: tcp ToPort: 443 @@ -307,14 +324,22 @@ Resources: Tags: - Key: Name Value: !Sub 'copilot-${AppName}-${EnvironmentName}-env' - EnvironmentSecurityGroupIngressFromPublicALB: + EnvironmentHTTPSecurityGroupIngressFromPublicALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateALB Properties: - Description: Ingress from the public ALB + Description: HTTP ingress from the public ALB + GroupId: !Ref EnvironmentSecurityGroup + IpProtocol: -1 + SourceSecurityGroupId: !Ref PublicHTTPLoadBalancerSecurityGroup + EnvironmentHTTPSSecurityGroupIngressFromPublicALB: + Type: AWS::EC2::SecurityGroupIngress + Condition: ExportHTTPSListener + Properties: + Description: HTTPS ingress from the public ALB GroupId: !Ref EnvironmentSecurityGroup IpProtocol: -1 - SourceSecurityGroupId: !Ref PublicLoadBalancerSecurityGroup + SourceSecurityGroupId: !Ref PublicHTTPSLoadBalancerSecurityGroup EnvironmentSecurityGroupIngressFromInternalALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateInternalALB @@ -369,7 +394,9 @@ Resources: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Scheme: internet-facing - SecurityGroups: [ !GetAtt PublicLoadBalancerSecurityGroup.GroupId ] + SecurityGroups: + - !GetAtt PublicHTTPLoadBalancerSecurityGroup.GroupId + - !If [ExportHTTPSListener, !GetAtt PublicHTTPSLoadBalancerSecurityGroup.GroupId, !Ref "AWS::NoValue"] Subnets: [ !Ref PublicSubnet1, !Ref PublicSubnet2, ] Type: application # Assign a dummy target group that with no real services as targets, so that we can create diff --git a/internal/pkg/deploy/env.go b/internal/pkg/deploy/env.go index 0576bb7603a..bdee243e7ba 100644 --- a/internal/pkg/deploy/env.go +++ b/internal/pkg/deploy/env.go @@ -38,6 +38,7 @@ type CreateEnvironmentInput struct { ImportCertARNs []string // Optional configuration if users want to import certificates. InternalALBSubnets []string // Optional configuration if users want to specify internal ALB placement. AllowVPCIngress bool // Optional configuration to allow access to internal ALB from ports 80/443. + CIDRPrefixListIDs []string // Optional configuration to specify public security group ingress based on prefix lists. Telemetry *config.Telemetry // Optional observability and monitoring configuration. Mft *manifest.Environment // Unmarshaled and interpolated manifest object. RawMft []byte // Content of the environment manifest without any modifications. diff --git a/internal/pkg/manifest/env.go b/internal/pkg/manifest/env.go index 5f117bca19f..9cbe7fe5cad 100644 --- a/internal/pkg/manifest/env.go +++ b/internal/pkg/manifest/env.go @@ -23,7 +23,7 @@ var environmentManifestPath = "environment/manifest.yml" // Environment is the manifest configuration for an environment. type Environment struct { Workload `yaml:",inline"` - environmentConfig `yaml:",inline"` + EnvironmentConfig `yaml:",inline"` parser template.Parser } @@ -49,7 +49,7 @@ func FromEnvConfig(cfg *config.Environment, parser template.Parser) *Environment var vpc environmentVPCConfig vpc.loadVPCConfig(cfg.CustomConfig) - var http environmentHTTPConfig + var http EnvironmentHTTPConfig http.loadLBConfig(cfg.CustomConfig) var obs environmentObservability @@ -60,7 +60,7 @@ func FromEnvConfig(cfg *config.Environment, parser template.Parser) *Environment Name: stringP(cfg.Name), Type: stringP(EnvironmentManifestType), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: vpc, }, @@ -83,13 +83,20 @@ func (e *Environment) MarshalBinary() ([]byte, error) { return content.Bytes(), nil } -type environmentConfig struct { - Network environmentNetworkConfig `yaml:"network,omitempty"` - Observability environmentObservability `yaml:"observability,omitempty"` - HTTPConfig environmentHTTPConfig `yaml:"http,omitempty"` +// EnvironmentConfig defines the configuration settings for an environment manifest +type EnvironmentConfig struct { + Network environmentNetworkConfig `yaml:"network,omitempty,flow"` + Observability environmentObservability `yaml:"observability,omitempty,flow"` + HTTPConfig EnvironmentHTTPConfig `yaml:"http,omitempty,flow"` CDNConfig environmentCDNConfig `yaml:"cdn,omitempty,flow"` } +// IsIngressRestrictedToCDN returns whether or not an environment has its +// Public Load Balancer ingress restricted to a Content Delivery Network. +func (mft *EnvironmentConfig) IsIngressRestrictedToCDN() bool { + return aws.BoolValue(mft.HTTPConfig.Public.SecurityGroupConfig.Ingress.RestrictiveIngress.CDNIngress) +} + type environmentNetworkConfig struct { VPC environmentVPCConfig `yaml:"vpc,omitempty"` } @@ -304,17 +311,18 @@ func (o *environmentObservability) loadObsConfig(tele *config.Telemetry) { o.ContainerInsights = &tele.EnableContainerInsights } -type environmentHTTPConfig struct { - Public publicHTTPConfig `yaml:"public,omitempty"` +// EnvironmentHTTPConfig defines the configuration settings for an environment group's HTTP connections. +type EnvironmentHTTPConfig struct { + Public PublicHTTPConfig `yaml:"public,omitempty"` Private privateHTTPConfig `yaml:"private,omitempty"` } // IsEmpty returns true if neither the public ALB nor the internal ALB is configured. -func (cfg environmentHTTPConfig) IsEmpty() bool { +func (cfg EnvironmentHTTPConfig) IsEmpty() bool { return cfg.Public.IsEmpty() && cfg.Private.IsEmpty() } -func (cfg *environmentHTTPConfig) loadLBConfig(env *config.CustomizeEnv) { +func (cfg *EnvironmentHTTPConfig) loadLBConfig(env *config.CustomizeEnv) { if env.IsEmpty() { return } @@ -328,13 +336,46 @@ func (cfg *environmentHTTPConfig) loadLBConfig(env *config.CustomizeEnv) { cfg.Public.Certificates = env.ImportCertARNs } -type publicHTTPConfig struct { - Certificates []string `yaml:"certificates,omitempty"` +// PublicHTTPConfig represents the configuration settings for an environment public ALB. +type PublicHTTPConfig struct { + SecurityGroupConfig ALBSecurityGroupsConfig `yaml:"security_groups,omitempty"` + Certificates []string `yaml:"certificates,omitempty"` +} + +// ALBSecurityGroupsConfig represents security group configuration settings for an ALB. +type ALBSecurityGroupsConfig struct { + Ingress Ingress `yaml:"ingress"` +} + +func (cfg ALBSecurityGroupsConfig) IsEmpty() bool { + return cfg.Ingress.IsEmpty() +} + +// Ingress represents allowed ingress traffic from specified fields. +type Ingress struct { + RestrictiveIngress RestrictiveIngress `yaml:"restrict_to"` + VPCIngress *bool `yaml:"from_vpc"` +} + +// RestrictiveIngress represents ingress fields which restrict +// default behavior of allowing all public ingress. +type RestrictiveIngress struct { + CDNIngress *bool `yaml:"cdn"` +} + +// IsEmpty returns true if there are no specified fields for restrictive ingress. +func (i RestrictiveIngress) IsEmpty() bool { + return i.CDNIngress == nil +} + +// IsEmpty returns true if there are no specified fields for ingress. +func (i Ingress) IsEmpty() bool { + return i.VPCIngress == nil && i.RestrictiveIngress.IsEmpty() } // IsEmpty returns true if there is no customization to the public ALB. -func (cfg publicHTTPConfig) IsEmpty() bool { - return len(cfg.Certificates) == 0 +func (cfg PublicHTTPConfig) IsEmpty() bool { + return len(cfg.Certificates) == 0 && cfg.SecurityGroupConfig.IsEmpty() } type privateHTTPConfig struct { @@ -349,17 +390,9 @@ func (cfg privateHTTPConfig) IsEmpty() bool { } type securityGroupsConfig struct { - Ingress ingress `yaml:"ingress"` + Ingress Ingress `yaml:"ingress"` } func (cfg securityGroupsConfig) isEmpty() bool { - return cfg.Ingress.isEmpty() -} - -type ingress struct { - VPCIngress *bool `yaml:"from_vpc"` -} - -func (i ingress) isEmpty() bool { - return i.VPCIngress == nil + return cfg.Ingress.IsEmpty() } diff --git a/internal/pkg/manifest/env_test.go b/internal/pkg/manifest/env_test.go index 80955fecea6..b4fb5e064fc 100644 --- a/internal/pkg/manifest/env_test.go +++ b/internal/pkg/manifest/env_test.go @@ -37,7 +37,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ CIDR: ipNetP("10.0.0.0/16"), @@ -94,7 +94,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ CIDR: ipNetP("10.0.0.0/16"), @@ -144,7 +144,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ ID: stringP("vpc-3f139646"), @@ -194,7 +194,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ Subnets: subnetsConfiguration{ @@ -209,8 +209,8 @@ func TestFromEnvConfig(t *testing.T) { }, }, }, - HTTPConfig: environmentHTTPConfig{ - Public: publicHTTPConfig{ + HTTPConfig: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"arn:aws:acm:region:account:certificate/certificate_ID_1", "arn:aws:acm:region:account:certificate/certificate_ID_2"}, }, }, @@ -231,9 +231,9 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ - HTTPConfig: environmentHTTPConfig{ - Public: publicHTTPConfig{ + EnvironmentConfig: EnvironmentConfig{ + HTTPConfig: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"arn:aws:acm:region:account:certificate/certificate_ID_1", "arn:aws:acm:region:account:certificate/certificate_ID_2"}, }, }, @@ -258,7 +258,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ Subnets: subnetsConfiguration{ @@ -274,12 +274,12 @@ func TestFromEnvConfig(t *testing.T) { }, }, }, - HTTPConfig: environmentHTTPConfig{ + HTTPConfig: EnvironmentHTTPConfig{ 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{ + Ingress: Ingress{ VPCIngress: aws.Bool(false), }, }, @@ -302,7 +302,7 @@ func TestFromEnvConfig(t *testing.T) { Name: stringP("test"), Type: stringP("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Observability: environmentObservability{ ContainerInsights: aws.Bool(false), }, @@ -355,7 +355,7 @@ network: Name: aws.String("test"), Type: aws.String("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ CIDR: &mockVPCCIDR, @@ -398,7 +398,7 @@ observability: Name: aws.String("prod"), Type: aws.String("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Observability: environmentObservability{ ContainerInsights: aws.Bool(true), }, @@ -416,7 +416,7 @@ cdn: true Name: aws.String("prod"), Type: aws.String("Environment"), }, - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ CDNConfig: environmentCDNConfig{ Enabled: aws.Bool(true), }, @@ -442,14 +442,14 @@ http: Name: aws.String("prod"), Type: aws.String("Environment"), }, - environmentConfig: environmentConfig{ - HTTPConfig: environmentHTTPConfig{ - Public: publicHTTPConfig{ + EnvironmentConfig: EnvironmentConfig{ + HTTPConfig: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"cert-1", "cert-2"}, }, Private: privateHTTPConfig{ SecurityGroupsConfig: securityGroupsConfig{ - Ingress: ingress{ + Ingress: Ingress{ VPCIngress: aws.Bool(false), }, }, @@ -711,15 +711,15 @@ func TestSubnetsConfiguration_IsEmpty(t *testing.T) { func TestEnvironmentHTTPConfig_IsEmpty(t *testing.T) { testCases := map[string]struct { - in environmentHTTPConfig + in EnvironmentHTTPConfig wanted bool }{ "empty": { wanted: true, }, "not empty": { - in: environmentHTTPConfig{ - Public: publicHTTPConfig{ + in: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"mock-cert"}, }, }, @@ -735,14 +735,14 @@ func TestEnvironmentHTTPConfig_IsEmpty(t *testing.T) { func TestPublicHTTPConfig_IsEmpty(t *testing.T) { testCases := map[string]struct { - in publicHTTPConfig + in PublicHTTPConfig wanted bool }{ "empty": { wanted: true, }, "not empty": { - in: publicHTTPConfig{ + in: PublicHTTPConfig{ Certificates: []string{"mock-cert-1"}, }, }, diff --git a/internal/pkg/manifest/validate_env.go b/internal/pkg/manifest/validate_env.go index 64c285a90cc..b5d2b64fb3b 100644 --- a/internal/pkg/manifest/validate_env.go +++ b/internal/pkg/manifest/validate_env.go @@ -19,14 +19,14 @@ var ( // Validate returns nil if Environment is configured correctly. func (e Environment) Validate() error { - if err := e.environmentConfig.validate(); err != nil { + if err := e.EnvironmentConfig.validate(); err != nil { return fmt.Errorf(`validate "network": %w`, err) } return nil } -// validate returns nil if environmentConfig is configured correctly. -func (e environmentConfig) validate() error { +// validate returns nil if EnvironmentConfig is configured correctly. +func (e EnvironmentConfig) validate() error { if err := e.Network.validate(); err != nil { return fmt.Errorf(`validate "network": %w`, err) } @@ -37,6 +37,10 @@ func (e environmentConfig) validate() error { return fmt.Errorf(`validate "http config": %w`, err) } + if e.IsIngressRestrictedToCDN() && !e.CDNConfig.CDNEnabled() { + return errors.New("CDN must be enabled to limit security group ingress to CloudFront") + } + if e.HTTPConfig.Private.InternalALBSubnets != nil { if !e.Network.VPC.imported() { return errors.New("in order to specify internal ALB subnet placement, subnets must be imported") @@ -195,8 +199,8 @@ func (o environmentObservability) validate() error { return nil } -// validate returns nil if environmentHTTPConfig is configured correctly. -func (cfg environmentHTTPConfig) validate() error { +// validate returns nil if EnvironmentHTTPConfig is configured correctly. +func (cfg EnvironmentHTTPConfig) validate() error { if err := cfg.Public.validate(); err != nil { return fmt.Errorf(`validate "public": %w`, err) } @@ -206,14 +210,22 @@ func (cfg environmentHTTPConfig) validate() error { return nil } -// validate returns nil if publicHTTPConfig is configured correctly. -func (cfg publicHTTPConfig) validate() error { +// validate returns nil if PublicHTTPConfig is configured correctly. +func (cfg PublicHTTPConfig) validate() error { for idx, certARN := range cfg.Certificates { if _, err := arn.Parse(certARN); err != nil { return fmt.Errorf(`parse "certificates[%d]": %w`, idx, err) } } - return nil + if cfg.SecurityGroupConfig.Ingress.VPCIngress != nil { + return fmt.Errorf("a public load balancer already allows vpc ingress") + } + return cfg.SecurityGroupConfig.validate() +} + +// validate returns nil if ALBSecurityGroupsConfig is configured correctly. +func (cfg ALBSecurityGroupsConfig) validate() error { + return cfg.Ingress.validate() } // validate returns nil if privateHTTPConfig is configured correctly. @@ -223,6 +235,9 @@ func (cfg privateHTTPConfig) validate() error { return fmt.Errorf(`parse "certificates[%d]": %w`, idx, err) } } + if !cfg.SecurityGroupsConfig.Ingress.RestrictiveIngress.IsEmpty() { + return fmt.Errorf("an internal load balancer cannot have restrictive ingress fields") + } if err := cfg.SecurityGroupsConfig.validate(); err != nil { return fmt.Errorf(`validate "security_groups: %w`, err) } @@ -230,19 +245,11 @@ func (cfg privateHTTPConfig) validate() error { } // validate returns nil if securityGroupsConfig is configured correctly. -func (s securityGroupsConfig) validate() error { - if s.isEmpty() { +func (cfg securityGroupsConfig) validate() error { + if cfg.isEmpty() { return nil } - return s.Ingress.validate() -} - -// validate returns nil if ingress is configured correctly. -func (i ingress) validate() error { - if i.isEmpty() { - return nil - } - return nil + return cfg.Ingress.validate() } // validate returns nil if environmentCDNConfig is configured correctly. @@ -253,12 +260,22 @@ func (cfg environmentCDNConfig) validate() error { return cfg.CDNConfig.validate() } -// validate is a no-op for advancedCDNConfig. +// validate returns nil if Ingress is configured correctly. +func (i Ingress) validate() error { + return i.RestrictiveIngress.validate() +} + +// validate is a no-op for RestrictiveIngress. +func (i RestrictiveIngress) validate() error { + return nil +} + +// validate is a no-op for AdvancedCDNConfig. func (cfg advancedCDNConfig) validate() error { return nil } -func (c environmentConfig) validateInternalALBSubnets() error { +func (c EnvironmentConfig) validateInternalALBSubnets() error { isImported := make(map[string]bool) for _, placementSubnet := range c.HTTPConfig.Private.InternalALBSubnets { for _, subnet := range append(c.Network.VPC.Subnets.Private, c.Network.VPC.Subnets.Public...) { diff --git a/internal/pkg/manifest/validate_env_test.go b/internal/pkg/manifest/validate_env_test.go index cc46ec35be5..eb700f03b17 100644 --- a/internal/pkg/manifest/validate_env_test.go +++ b/internal/pkg/manifest/validate_env_test.go @@ -5,6 +5,7 @@ package manifest import ( "errors" + "fmt" "testing" "github.com/aws/aws-sdk-go/aws" @@ -19,7 +20,7 @@ func TestEnvironment_Validate(t *testing.T) { }{ "malformed network": { in: Environment{ - environmentConfig: environmentConfig{ + EnvironmentConfig: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ ID: stringP("vpc-123"), @@ -51,11 +52,11 @@ func TestEnvironmentConfig_validate(t *testing.T) { mockPrivateSubnet1CIDR := IPNet("10.0.3.0/24") mockPrivateSubnet2CIDR := IPNet("10.0.4.0/24") testCases := map[string]struct { - in environmentConfig + in EnvironmentConfig wantedError string }{ "error if internal ALB subnet placement specified with adjusted vpc": { - in: environmentConfig{ + in: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ CIDR: ipNetP("apple cider"), @@ -84,7 +85,7 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, }, - HTTPConfig: environmentHTTPConfig{ + HTTPConfig: EnvironmentHTTPConfig{ Private: privateHTTPConfig{ InternalALBSubnets: []string{"mockSubnet"}, }, @@ -92,8 +93,27 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, wantedError: "in order to specify internal ALB subnet placement, subnets must be imported", }, + "error if security group ingress is limited to a cdn distribution not enabled": { + in: EnvironmentConfig{ + CDNConfig: environmentCDNConfig{ + Enabled: aws.Bool(false), + }, + HTTPConfig: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ + SecurityGroupConfig: ALBSecurityGroupsConfig{ + Ingress: Ingress{ + RestrictiveIngress: RestrictiveIngress{ + CDNIngress: aws.Bool(true), + }, + }, + }, + }, + }, + }, + wantedError: "CDN must be enabled to limit security group ingress to CloudFront", + }, "error if subnets specified for internal ALB placement don't exist": { - in: environmentConfig{ + in: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ ID: aws.String("mockID"), @@ -105,7 +125,7 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, }, }, - HTTPConfig: environmentHTTPConfig{ + HTTPConfig: EnvironmentHTTPConfig{ Private: privateHTTPConfig{ InternalALBSubnets: []string{"nonexistentSubnet"}, }, @@ -114,7 +134,7 @@ func TestEnvironmentConfig_validate(t *testing.T) { wantedError: "subnet(s) specified for internal ALB placement not imported", }, "valid case with internal ALB placement": { - in: environmentConfig{ + in: EnvironmentConfig{ Network: environmentNetworkConfig{ VPC: environmentVPCConfig{ ID: aws.String("mockID"), @@ -130,7 +150,7 @@ func TestEnvironmentConfig_validate(t *testing.T) { }, }, }, - HTTPConfig: environmentHTTPConfig{ + HTTPConfig: EnvironmentHTTPConfig{ Private: privateHTTPConfig{ InternalALBSubnets: []string{"existentSubnet", "anotherExistentSubnet"}, }, @@ -682,19 +702,20 @@ func TestSubnetConfiguration_validate(t *testing.T) { func TestEnvironmentHTTPConfig_validate(t *testing.T) { testCases := map[string]struct { - in environmentHTTPConfig + in EnvironmentHTTPConfig wantedErrorMsgPrefix string + wantedError error }{ "malformed public certificate": { - in: environmentHTTPConfig{ - Public: publicHTTPConfig{ + in: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"arn:aws:weird-little-arn"}, }, }, wantedErrorMsgPrefix: `parse "certificates[0]": `, }, "malformed private certificate": { - in: environmentHTTPConfig{ + in: EnvironmentHTTPConfig{ Private: privateHTTPConfig{ Certificates: []string{"arn:aws:weird-little-arn"}, }, @@ -702,19 +723,45 @@ func TestEnvironmentHTTPConfig_validate(t *testing.T) { wantedErrorMsgPrefix: `parse "certificates[0]": `, }, "success with public cert": { - in: environmentHTTPConfig{ - Public: publicHTTPConfig{ + in: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ Certificates: []string{"arn:aws:acm:us-east-1:1111111:certificate/look-like-a-good-arn"}, }, }, }, "success with private cert": { - in: environmentHTTPConfig{ + in: EnvironmentHTTPConfig{ Private: privateHTTPConfig{ Certificates: []string{"arn:aws:acm:us-east-1:1111111:certificate/look-like-a-good-arn"}, }, }, }, + "public http config with invalid security group ingress": { + in: EnvironmentHTTPConfig{ + Public: PublicHTTPConfig{ + SecurityGroupConfig: ALBSecurityGroupsConfig{ + Ingress: Ingress{ + VPCIngress: aws.Bool(true), + }, + }, + }, + }, + wantedError: fmt.Errorf(`validate "public": a public load balancer already allows vpc ingress`), + }, + "private http config with invalid security group ingress": { + in: EnvironmentHTTPConfig{ + Private: privateHTTPConfig{ + SecurityGroupsConfig: securityGroupsConfig{ + Ingress: Ingress{ + RestrictiveIngress: RestrictiveIngress{ + CDNIngress: aws.Bool(true), + }, + }, + }, + }, + }, + wantedError: fmt.Errorf(`validate "private": an internal load balancer cannot have restrictive ingress fields`), + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -722,6 +769,9 @@ func TestEnvironmentHTTPConfig_validate(t *testing.T) { if tc.wantedErrorMsgPrefix != "" { require.Error(t, gotErr) require.Contains(t, gotErr.Error(), tc.wantedErrorMsgPrefix) + } else if tc.wantedError != nil { + require.Error(t, gotErr) + require.EqualError(t, tc.wantedError, gotErr.Error()) } else { require.NoError(t, gotErr) } diff --git a/internal/pkg/manifest/validate_integration_test.go b/internal/pkg/manifest/validate_integration_test.go index fc8a66545bd..964ab1c7f34 100644 --- a/internal/pkg/manifest/validate_integration_test.go +++ b/internal/pkg/manifest/validate_integration_test.go @@ -62,7 +62,7 @@ func Test_ValidateAudit(t *testing.T) { // Audit environment manifest. t.Run("environment manifest", func(t *testing.T) { env := &Environment{} - err := isValid(reflect.ValueOf(env.environmentConfig).Type()) + err := isValid(reflect.ValueOf(env.EnvironmentConfig).Type()) require.NoError(t, err) }) } diff --git a/internal/pkg/template/env.go b/internal/pkg/template/env.go index e079a9f22d2..c7562cc7d62 100644 --- a/internal/pkg/template/env.go +++ b/internal/pkg/template/env.go @@ -105,12 +105,13 @@ type EnvOpts struct { ArtifactBucketARN string ArtifactBucketKeyARN string - VPCConfig VPCConfig - PublicImportedCertARNs []string - PrivateImportedCertARNs []string - CustomInternalALBSubnets []string - AllowVPCIngress bool - Telemetry *Telemetry + VPCConfig VPCConfig + PublicFacingCIDRPrefixListIDs []string + PublicImportedCertARNs []string + PrivateImportedCertARNs []string + CustomInternalALBSubnets []string + AllowVPCIngress bool + Telemetry *Telemetry CDNConfig *CDNConfig // If nil, no cdn is to be used diff --git a/internal/pkg/template/templates/environment/cf.yml b/internal/pkg/template/templates/environment/cf.yml index 98f8439e4ef..e49c201b8d4 100644 --- a/internal/pkg/template/templates/environment/cf.yml +++ b/internal/pkg/template/templates/environment/cf.yml @@ -97,24 +97,60 @@ Resources: Value: disabled {{- end}} {{- end}} - PublicLoadBalancerSecurityGroup: + PublicHTTPLoadBalancerSecurityGroup: Metadata: - 'aws:copilot:description': 'A security group for your load balancer allowing HTTP and HTTPS traffic' + 'aws:copilot:description': 'A security group for your load balancer allowing HTTP traffic' Condition: CreateALB Type: AWS::EC2::SecurityGroup Properties: - GroupDescription: Access to the public facing load balancer + GroupDescription: HTTP access to the public facing load balancer SecurityGroupIngress: + {{- if .PublicFacingCIDRPrefixListIDs}} + {{- range $id := .PublicFacingCIDRPrefixListIDs}} + - SourcePrefixListId: {{$id}} + Description: Allow ingress from prefix list {{$id}} on port 80 + FromPort: 80 + IpProtocol: tcp + ToPort: 80 + {{- end}} + {{- else}} - CidrIp: 0.0.0.0/0 Description: Allow from anyone on port 80 FromPort: 80 IpProtocol: tcp ToPort: 80 + {{- end}} +{{- if .VPCConfig.Imported}} + VpcId: {{.VPCConfig.Imported.ID}} +{{- else}} + VpcId: !Ref VPC +{{- end}} + Tags: + - Key: Name + Value: !Sub 'copilot-${AppName}-${EnvironmentName}-lb' + PublicHTTPSLoadBalancerSecurityGroup: + Metadata: + 'aws:copilot:description': 'A security group for your load balancer allowing HTTPS traffic' + Condition: ExportHTTPSListener + Type: AWS::EC2::SecurityGroup + Properties: + GroupDescription: HTTPS access to the public facing load balancer + SecurityGroupIngress: + {{- if .PublicFacingCIDRPrefixListIDs}} + {{- range $id := .PublicFacingCIDRPrefixListIDs}} + - SourcePrefixListId: {{$id}} + Description: Allow ingress from prefix list {{$id}} on port 443 + FromPort: 443 + IpProtocol: tcp + ToPort: 443 + {{- end}} + {{- else}} - CidrIp: 0.0.0.0/0 Description: Allow from anyone on port 443 FromPort: 443 IpProtocol: tcp ToPort: 443 + {{- end}} {{- if .VPCConfig.Imported}} VpcId: {{.VPCConfig.Imported.ID}} {{- else}} @@ -153,14 +189,22 @@ Resources: Tags: - Key: Name Value: !Sub 'copilot-${AppName}-${EnvironmentName}-env' - EnvironmentSecurityGroupIngressFromPublicALB: + EnvironmentHTTPSecurityGroupIngressFromPublicALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateALB Properties: - Description: Ingress from the public ALB + Description: HTTP ingress from the public ALB + GroupId: !Ref EnvironmentSecurityGroup + IpProtocol: -1 + SourceSecurityGroupId: !Ref PublicHTTPLoadBalancerSecurityGroup + EnvironmentHTTPSSecurityGroupIngressFromPublicALB: + Type: AWS::EC2::SecurityGroupIngress + Condition: ExportHTTPSListener + Properties: + Description: HTTPS ingress from the public ALB GroupId: !Ref EnvironmentSecurityGroup IpProtocol: -1 - SourceSecurityGroupId: !Ref PublicLoadBalancerSecurityGroup + SourceSecurityGroupId: !Ref PublicHTTPSLoadBalancerSecurityGroup EnvironmentSecurityGroupIngressFromInternalALB: Type: AWS::EC2::SecurityGroupIngress Condition: CreateInternalALB @@ -217,7 +261,9 @@ Resources: Type: AWS::ElasticLoadBalancingV2::LoadBalancer Properties: Scheme: internet-facing - SecurityGroups: [ !GetAtt PublicLoadBalancerSecurityGroup.GroupId ] + SecurityGroups: + - !GetAtt PublicHTTPLoadBalancerSecurityGroup.GroupId + - !If [ExportHTTPSListener, !GetAtt PublicHTTPSLoadBalancerSecurityGroup.GroupId, !Ref "AWS::NoValue"] {{- if .VPCConfig.Imported}} Subnets: [ {{range $id := .VPCConfig.Imported.PublicSubnetIDs}}{{$id}}, {{end}} ] {{- else}}