From 741057b0476c0129308f889e1138c266bf59fc28 Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Thu, 25 Feb 2021 11:17:35 -0800 Subject: [PATCH 1/4] refactor(manifest): group consts and variables in workload.go --- internal/pkg/manifest/workload.go | 32 ++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 8fc9719ae8d..f6a510f7346 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -14,17 +14,26 @@ import ( "gopkg.in/yaml.v3" ) +const ( + defaultFluentbitImage = "amazon/aws-for-fluent-bit:latest" + defaultDockerfileName = "Dockerfile" +) + var ( + // WorkloadTypes holds all workload manifest types. + WorkloadTypes = append(ServiceTypes, JobTypes...) + + // Error definitions. errUnmarshalBuildOpts = errors.New("can't unmarshal build field into string or compose-style map") errUnmarshalCountOpts = errors.New(`can't unmarshal "count" field to an integer or autoscaling configuration`) ) -const defaultFluentbitImage = "amazon/aws-for-fluent-bit:latest" - -var dockerfileDefaultName = "Dockerfile" - -// WorkloadTypes holds all workload manifest types. -var WorkloadTypes = append(ServiceTypes, JobTypes...) +// WorkloadProps contains properties for creating a new workload manifest. +type WorkloadProps struct { + Name string + Dockerfile string + Image string +} // Workload holds the basic data that every workload manifest file needs to have. type Workload struct { @@ -52,7 +61,7 @@ func (i Image) GetLocation() string { func (i *Image) BuildConfig(rootDirectory string) *DockerBuildArgs { df := i.dockerfile() ctx := i.context() - dockerfile := aws.String(filepath.Join(rootDirectory, dockerfileDefaultName)) + dockerfile := aws.String(filepath.Join(rootDirectory, defaultDockerfileName)) context := aws.String(rootDirectory) if df != "" && ctx != "" { @@ -64,7 +73,7 @@ func (i *Image) BuildConfig(rootDirectory string) *DockerBuildArgs { context = aws.String(filepath.Join(rootDirectory, filepath.Dir(df))) } if df == "" && ctx != "" { - dockerfile = aws.String(filepath.Join(rootDirectory, ctx, dockerfileDefaultName)) + dockerfile = aws.String(filepath.Join(rootDirectory, ctx, defaultDockerfileName)) context = aws.String(filepath.Join(rootDirectory, ctx)) } return &DockerBuildArgs{ @@ -218,13 +227,6 @@ type TaskConfig struct { Storage *Storage `yaml:"storage"` } -// WorkloadProps contains properties for creating a new workload manifest. -type WorkloadProps struct { - Name string - Dockerfile string - Image string -} - // UnmarshalWorkload deserializes the YAML input stream into a workload manifest object. // If an error occurs during deserialization, then returns the error. // If the workload type in the manifest is invalid, then returns an ErrInvalidManifestType. From 4d6b7b5f4ca340394a03fe90611c2dce70516861 Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Thu, 25 Feb 2021 16:30:41 -0800 Subject: [PATCH 2/4] feat(manifest): enable "network" field to specify subnets and sgs --- internal/pkg/manifest/backend_svc.go | 6 ++ internal/pkg/manifest/backend_svc_test.go | 10 +++ internal/pkg/manifest/job.go | 6 ++ internal/pkg/manifest/lb_web_svc.go | 8 ++- internal/pkg/manifest/lb_web_svc_test.go | 85 +++++++++++++++++++++++ internal/pkg/manifest/svc_test.go | 10 +++ internal/pkg/manifest/workload.go | 47 +++++++++++++ internal/pkg/manifest/workload_test.go | 67 ++++++++++++++++++ 8 files changed, 237 insertions(+), 2 deletions(-) diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index c1fe53df186..ce5b4749843 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -39,6 +39,7 @@ type BackendServiceConfig struct { TaskConfig `yaml:",inline"` *Logging `yaml:"logging,flow"` Sidecars map[string]*SidecarConfig `yaml:"sidecars"` + Network networkConfig `yaml:"network"` } type imageWithPortAndHealthcheck struct { @@ -133,6 +134,11 @@ func newDefaultBackendService() *BackendService { Value: aws.Int(1), }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + }, + }, }, } } diff --git a/internal/pkg/manifest/backend_svc_test.go b/internal/pkg/manifest/backend_svc_test.go index 24356b7339f..dfc776e82b1 100644 --- a/internal/pkg/manifest/backend_svc_test.go +++ b/internal/pkg/manifest/backend_svc_test.go @@ -51,6 +51,11 @@ func TestNewBackendSvc(t *testing.T) { Value: aws.Int(1), }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + }, + }, }, }, }, @@ -93,6 +98,11 @@ func TestNewBackendSvc(t *testing.T) { Value: aws.Int(1), }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + }, + }, }, }, }, diff --git a/internal/pkg/manifest/job.go b/internal/pkg/manifest/job.go index b256273cc8c..4b1bc190027 100644 --- a/internal/pkg/manifest/job.go +++ b/internal/pkg/manifest/job.go @@ -42,6 +42,7 @@ type ScheduledJobConfig struct { Sidecars map[string]*SidecarConfig `yaml:"sidecars"` On JobTriggerConfig `yaml:"on,flow"` JobFailureHandlerConfig `yaml:",inline"` + Network networkConfig `yaml:"network"` } // JobTriggerConfig represents the configuration for the event that triggers the job. @@ -78,6 +79,11 @@ func newDefaultScheduledJob() *ScheduledJob { Value: aws.Int(1), }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + }, + }, }, } } diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index bdb5435417e..b00bcc6d436 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -20,8 +20,6 @@ const ( // Default values for HTTPHealthCheck for a load balanced web service. const ( - // LogRetentionInDays is the default log retention time in days. - LogRetentionInDays = 30 DefaultHealthCheckPath = "/" ) @@ -53,6 +51,7 @@ type LoadBalancedWebServiceConfig struct { TaskConfig `yaml:",inline"` *Logging `yaml:"logging,flow"` Sidecars map[string]*SidecarConfig `yaml:"sidecars"` + Network networkConfig `yaml:"network"` } // HTTPHealthCheckArgs holds the configuration to determine if the load balanced web service is healthy. @@ -150,6 +149,11 @@ func newDefaultLoadBalancedWebService() *LoadBalancedWebService { Value: aws.Int(1), }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + }, + }, }, } } diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index 74de8d3a3e1..e7f90096b08 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -18,6 +18,74 @@ import ( "gopkg.in/yaml.v3" ) +func TestNewLoadBalancedWebService(t *testing.T) { + testCases := map[string]struct { + props LoadBalancedWebServiceProps + + wanted *LoadBalancedWebService + }{ + "translates to default load balanced web service": { + props: LoadBalancedWebServiceProps{ + WorkloadProps: &WorkloadProps{ + Name: "frontend", + Dockerfile: "./Dockerfile", + }, + Path: "/", + Port: 80, + }, + + wanted: &LoadBalancedWebService{ + Workload: Workload{ + Name: stringP("frontend"), + Type: stringP("Load Balanced Web Service"), + }, + LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ + ImageConfig: ServiceImageWithPort{ + Image: Image{ + Build: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Dockerfile: stringP("./Dockerfile"), + }, + }, + }, + Port: aws.Uint16(80), + }, + RoutingRule: RoutingRule{ + Path: stringP("/"), + HealthCheck: HealthCheckArgsOrString{ + HealthCheckPath: stringP("/"), + }, + }, + TaskConfig: TaskConfig{ + CPU: aws.Int(256), + Memory: aws.Int(512), + Count: Count{ + Value: aws.Int(1), + }, + }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + }, + }, + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // WHEN + manifest := NewLoadBalancedWebService(&tc.props) + + // THEN + require.Equal(t, tc.wanted.Workload, manifest.Workload) + require.Equal(t, tc.wanted.LoadBalancedWebServiceConfig, manifest.LoadBalancedWebServiceConfig) + require.Equal(t, tc.wanted.Environments, manifest.Environments) + }) + } +} + func TestNewLoadBalancedWebService_UnmarshalYaml(t *testing.T) { testCases := map[string]struct { inContent []byte @@ -287,6 +355,12 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { Logging: &Logging{ ConfigFile: aws.String("mockConfigFile"), }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + SecurityGroups: []string{"sg-123"}, + }, + }, }, Environments: map[string]*LoadBalancedWebServiceConfig{ "prod-iad": { @@ -343,6 +417,11 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { "FOO": "BAR", }, }, + Network: networkConfig{ + VPC: vpcConfig{ + SecurityGroups: []string{"sg-456", "sg-789"}, + }, + }, }, }, }, @@ -425,6 +504,12 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { "FOO": "BAR", }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + SecurityGroups: []string{"sg-456", "sg-789"}, + }, + }, }, }, }, diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index d2f76f52180..0591b985f0e 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -109,6 +109,11 @@ environments: "LOG_TOKEN": "LOG_TOKEN", }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + }, + }, }, Environments: map[string]*LoadBalancedWebServiceConfig{ "test": { @@ -182,6 +187,11 @@ secrets: "API_TOKEN": "SUBS_API_TOKEN", }, }, + Network: networkConfig{ + VPC: vpcConfig{ + Placement: stringP("public"), + }, + }, }, } require.Equal(t, wantedManifest, actualManifest) diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index f6a510f7346..80cdda4e47f 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -17,12 +17,19 @@ import ( const ( defaultFluentbitImage = "amazon/aws-for-fluent-bit:latest" defaultDockerfileName = "Dockerfile" + + // AWS VPC subnet placement options. + publicPlacement = "public" + privatePlacement = "private" ) var ( // WorkloadTypes holds all workload manifest types. WorkloadTypes = append(ServiceTypes, JobTypes...) + // All placement options. + subnetPlacements = []string{publicPlacement, privatePlacement} + // Error definitions. errUnmarshalBuildOpts = errors.New("can't unmarshal build field into string or compose-style map") errUnmarshalCountOpts = errors.New(`can't unmarshal "count" field to an integer or autoscaling configuration`) @@ -227,6 +234,46 @@ type TaskConfig struct { Storage *Storage `yaml:"storage"` } +// networkConfig represents options for network connection to AWS resources within a VPC. +type networkConfig struct { + VPC vpcConfig `yaml:"vpc"` +} + +func (c *networkConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { + type networkWithDefaults networkConfig + conf := networkWithDefaults{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + }, + } + if err := unmarshal(&conf); err != nil { + return err + } + if !conf.VPC.isValidPlacement() { + return fmt.Errorf("field '%s' is '%v' must be one of %#v", "network.vpc.placement", aws.StringValue(conf.VPC.Placement), subnetPlacements) + } + *c = networkConfig(conf) + return nil +} + +// vpcConfig represents the security groups and subnets attached to a task. +type vpcConfig struct { + Placement *string `yaml:"placement"` + SecurityGroups []string `yaml:"security_groups"` +} + +func (c vpcConfig) isValidPlacement() bool { + if c.Placement == nil { + return false + } + for _, allowed := range subnetPlacements { + if *c.Placement == allowed { + return true + } + } + return false +} + // UnmarshalWorkload deserializes the YAML input stream into a workload manifest object. // If an error occurs during deserialization, then returns the error. // If the workload type in the manifest is invalid, then returns an ErrInvalidManifestType. diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index 68212e54982..5bf51859aa6 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -4,6 +4,7 @@ package manifest import ( + "errors" "path/filepath" "testing" @@ -265,3 +266,69 @@ func TestLogging_GetEnableMetadata(t *testing.T) { }) } } + +func TestNetworkConfig_UnmarshalYAML(t *testing.T) { + testCases := map[string]struct { + data string + + wantedConfig networkConfig + wantedErr error + }{ + "defaults to public placement if vpc is empty": { + data: ` +network: + vpc: +`, + wantedConfig: networkConfig{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + }, + }, + }, + "returns error if placement option is invalid": { + data: ` +network: + vpc: + placement: 'tartarus' +`, + wantedErr: errors.New(`field 'network.vpc.placement' is 'tartarus' must be one of []string{"public", "private"}`), + }, + "unmarshals successfully for public placement with security groups": { + data: ` +network: + vpc: + placement: 'public' + security_groups: + - 'sg-1234' + - 'sg-4567' +`, + wantedConfig: networkConfig{ + VPC: vpcConfig{ + Placement: stringP(publicPlacement), + SecurityGroups: []string{"sg-1234", "sg-4567"}, + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + type manifest struct { + Network networkConfig `yaml:"network"` + } + var m manifest + + // WHEN + err := yaml.Unmarshal([]byte(tc.data), &m) + + // THEN + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedConfig, m.Network) + } + }) + } +} From bd251568086c4d6e2704405d70487f56dac82ebc Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Thu, 25 Feb 2021 16:43:45 -0800 Subject: [PATCH 3/4] refactor(stack): rename transformers to convert* for consistency --- .../cloudformation/stack/transformers.go | 24 +++++++++---------- .../cloudformation/stack/transformers_test.go | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index df406e4d7b1..4c37c4299a0 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -53,7 +53,7 @@ func convertSidecar(s map[string]*manifest.SidecarConfig) ([]*template.SidecarOp if err != nil { return nil, err } - mp, err := renderSidecarMountPoints(config.MountPoints) + mp, err := convertSidecarMountPoints(config.MountPoints) if err != nil { return nil, err } @@ -161,15 +161,15 @@ func convertStorageOpts(in *manifest.Storage) (*template.StorageOpts, error) { if in == nil { return nil, nil } - v, err := renderVolumes(in.Volumes) + v, err := convertVolumes(in.Volumes) if err != nil { return nil, err } - mp, err := renderMountPoints(in.Volumes) + mp, err := convertMountPoints(in.Volumes) if err != nil { return nil, err } - perms, err := renderStoragePermissions(in.Volumes) + perms, err := convertEFSPermissions(in.Volumes) if err != nil { return nil, err } @@ -180,14 +180,14 @@ func convertStorageOpts(in *manifest.Storage) (*template.StorageOpts, error) { }, nil } -// renderSidecarMountPoints is used to convert from manifest to template objects. -func renderSidecarMountPoints(in []manifest.SidecarMountPoint) ([]*template.MountPoint, error) { +// convertSidecarMountPoints is used to convert from manifest to template objects. +func convertSidecarMountPoints(in []manifest.SidecarMountPoint) ([]*template.MountPoint, error) { if len(in) == 0 { return nil, nil } var output []*template.MountPoint for _, smp := range in { - mp, err := renderMountPoint(smp.SourceVolume, smp.ContainerPath, smp.ReadOnly) + mp, err := convertMountPoint(smp.SourceVolume, smp.ContainerPath, smp.ReadOnly) if err != nil { return nil, err } @@ -196,7 +196,7 @@ func renderSidecarMountPoints(in []manifest.SidecarMountPoint) ([]*template.Moun return output, nil } -func renderMountPoint(sourceVolume, containerPath *string, readOnly *bool) (*template.MountPoint, error) { +func convertMountPoint(sourceVolume, containerPath *string, readOnly *bool) (*template.MountPoint, error) { // containerPath must be specified. if aws.StringValue(containerPath) == "" { return nil, errNoContainerPath @@ -221,13 +221,13 @@ func renderMountPoint(sourceVolume, containerPath *string, readOnly *bool) (*tem }, nil } -func renderMountPoints(input map[string]manifest.Volume) ([]*template.MountPoint, error) { +func convertMountPoints(input map[string]manifest.Volume) ([]*template.MountPoint, error) { if len(input) == 0 { return nil, nil } var output []*template.MountPoint for name, volume := range input { - mp, err := renderMountPoint(aws.String(name), volume.ContainerPath, volume.ReadOnly) + mp, err := convertMountPoint(aws.String(name), volume.ContainerPath, volume.ReadOnly) if err != nil { return nil, err } @@ -236,7 +236,7 @@ func renderMountPoints(input map[string]manifest.Volume) ([]*template.MountPoint return output, nil } -func renderStoragePermissions(input map[string]manifest.Volume) ([]*template.EFSPermission, error) { +func convertEFSPermissions(input map[string]manifest.Volume) ([]*template.EFSPermission, error) { if len(input) == 0 { return nil, nil } @@ -260,7 +260,7 @@ func renderStoragePermissions(input map[string]manifest.Volume) ([]*template.EFS return output, nil } -func renderVolumes(input map[string]manifest.Volume) ([]*template.Volume, error) { +func convertVolumes(input map[string]manifest.Volume) ([]*template.Volume, error) { if len(input) == 0 { return nil, nil } diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index afa8a864c3d..93d34932878 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -512,7 +512,7 @@ func Test_convertSidecarMountPoints(t *testing.T) { } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - got, err := renderSidecarMountPoints(tc.inMountPoints) + got, err := convertSidecarMountPoints(tc.inMountPoints) if tc.wantErr != "" { require.EqualError(t, err, tc.wantErr) } else { From 839c9ed2c2cd5f92d0b2bff6d6775278c76fa064 Mon Sep 17 00:00:00 2001 From: Efe Karakus Date: Thu, 25 Feb 2021 17:27:06 -0800 Subject: [PATCH 4/4] chore(stack): covert manifest NetworkConfig to template NetworkOpts --- .../cloudformation/stack/backend_svc.go | 1 + .../cloudformation/stack/backend_svc_test.go | 9 +++++++++ .../deploy/cloudformation/stack/lb_web_svc.go | 1 + .../cloudformation/stack/lb_web_svc_test.go | 8 ++++++++ .../cloudformation/stack/scheduled_job.go | 1 + .../stack/scheduled_job_test.go | 8 ++++++++ .../cloudformation/stack/transformers.go | 13 ++++++++++++ internal/pkg/manifest/backend_svc.go | 6 +++--- internal/pkg/manifest/backend_svc_test.go | 4 ++-- internal/pkg/manifest/job.go | 6 +++--- internal/pkg/manifest/lb_web_svc.go | 6 +++--- internal/pkg/manifest/lb_web_svc_test.go | 8 ++++---- internal/pkg/manifest/svc_test.go | 4 ++-- internal/pkg/manifest/workload.go | 20 ++++++++++--------- internal/pkg/manifest/workload_test.go | 12 +++++------ internal/pkg/template/workload.go | 13 ++++++++++-- 16 files changed, 86 insertions(+), 34 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index fd67ef40405..2ff34f6c896 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -97,6 +97,7 @@ func (s *BackendService) Template() (string, error) { LogConfig: convertLogging(s.manifest.Logging), DesiredCountLambda: desiredCountLambda.String(), Storage: storage, + Network: convertNetworkConfig(s.manifest.Network), }) 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 c4def720bf7..13f607aaf90 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -161,6 +161,11 @@ Outputs: StackName: addon.StackName, VariableOutputs: []string{"MyTable"}, }, + Network: &template.NetworkOpts{ + AssignPublicIP: template.DisablePublicIP, + SubnetsType: template.PrivateSubnetsPlacement, + SecurityGroups: []string{"sg-1234"}, + }, }).Return(&template.Content{Buffer: bytes.NewBufferString("template")}, nil) svc.parser = m svc.addons = mockTemplater{ @@ -196,6 +201,10 @@ Outputs: }, manifest: tc.manifest, } + if tc.manifest != nil { + conf.manifest.Network.VPC.Placement = aws.String(manifest.PrivateSubnetPlacement) + conf.manifest.Network.VPC.SecurityGroups = []string{"sg-1234"} + } tc.mockDependencies(t, ctrl, conf) // WHEN diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index bd149066d02..45d19f7e7d3 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -131,6 +131,7 @@ func (s *LoadBalancedWebService) Template() (string, error) { DesiredCountLambda: desiredCountLambda.String(), EnvControllerLambda: envControllerLambda.String(), Storage: storage, + Network: convertNetworkConfig(s.manifest.Network), }) 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 54a26ee5974..d0fba95b8df 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -177,6 +177,10 @@ Outputs: RulePriorityLambda: "lambda", DesiredCountLambda: "something", EnvControllerLambda: "something", + Network: &template.NetworkOpts{ + AssignPublicIP: template.EnablePublicIP, + SubnetsType: template.PublicSubnetsPlacement, + }, }).Return(&template.Content{Buffer: bytes.NewBufferString("template")}, nil) addons := mockTemplater{err: &addon.ErrAddonsDirNotExist{}} @@ -205,6 +209,10 @@ Outputs: RulePriorityLambda: "lambda", DesiredCountLambda: "something", EnvControllerLambda: "something", + Network: &template.NetworkOpts{ + AssignPublicIP: template.EnablePublicIP, + SubnetsType: template.PublicSubnetsPlacement, + }, }).Return(&template.Content{Buffer: bytes.NewBufferString("template")}, nil) addons := mockTemplater{ tpl: `Resources: diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job.go index 21c995b2457..7ed57d32263 100644 --- a/internal/pkg/deploy/cloudformation/stack/scheduled_job.go +++ b/internal/pkg/deploy/cloudformation/stack/scheduled_job.go @@ -151,6 +151,7 @@ func (j *ScheduledJob) Template() (string, error) { StateMachine: stateMachine, LogConfig: convertLogging(j.manifest.Logging), Storage: storage, + Network: convertNetworkConfig(j.manifest.Network), }) if err != nil { return "", fmt.Errorf("parse scheduled job template: %w", err) diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go index 0179de59f34..029a86a212b 100644 --- a/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go +++ b/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go @@ -53,6 +53,10 @@ func TestScheduledJob_Template(t *testing.T) { Timeout: aws.Int(5400), Retries: aws.Int(3), }, + Network: &template.NetworkOpts{ + AssignPublicIP: template.EnablePublicIP, + SubnetsType: template.PublicSubnetsPlacement, + }, })).Return(&template.Content{Buffer: bytes.NewBufferString("template")}, nil) addons := mockTemplater{err: &addon.ErrAddonsDirNotExist{}} j.parser = m @@ -75,6 +79,10 @@ func TestScheduledJob_Template(t *testing.T) { Timeout: aws.Int(5400), Retries: aws.Int(3), }, + Network: &template.NetworkOpts{ + AssignPublicIP: template.EnablePublicIP, + SubnetsType: template.PublicSubnetsPlacement, + }, })).Return(&template.Content{Buffer: bytes.NewBufferString("template")}, nil) addons := mockTemplater{ tpl: `Resources: diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 4c37c4299a0..0983db92db4 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -317,3 +317,16 @@ func convertVolumes(input map[string]manifest.Volume) ([]*template.Volume, error } return output, nil } + +func convertNetworkConfig(network manifest.NetworkConfig) *template.NetworkOpts { + opts := &template.NetworkOpts{ + AssignPublicIP: template.EnablePublicIP, + SubnetsType: template.PublicSubnetsPlacement, + SecurityGroups: network.VPC.SecurityGroups, + } + if aws.StringValue(network.VPC.Placement) != manifest.PublicSubnetPlacement { + opts.AssignPublicIP = template.DisablePublicIP + opts.SubnetsType = template.PrivateSubnetsPlacement + } + return opts +} diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index ce5b4749843..2f04481f197 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -39,7 +39,7 @@ type BackendServiceConfig struct { TaskConfig `yaml:",inline"` *Logging `yaml:"logging,flow"` Sidecars map[string]*SidecarConfig `yaml:"sidecars"` - Network networkConfig `yaml:"network"` + Network NetworkConfig `yaml:"network"` } type imageWithPortAndHealthcheck struct { @@ -134,9 +134,9 @@ func newDefaultBackendService() *BackendService { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), }, }, }, diff --git a/internal/pkg/manifest/backend_svc_test.go b/internal/pkg/manifest/backend_svc_test.go index dfc776e82b1..43f22f5df32 100644 --- a/internal/pkg/manifest/backend_svc_test.go +++ b/internal/pkg/manifest/backend_svc_test.go @@ -51,7 +51,7 @@ func TestNewBackendSvc(t *testing.T) { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), }, @@ -98,7 +98,7 @@ func TestNewBackendSvc(t *testing.T) { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), }, diff --git a/internal/pkg/manifest/job.go b/internal/pkg/manifest/job.go index 4b1bc190027..fc9c30bd3d9 100644 --- a/internal/pkg/manifest/job.go +++ b/internal/pkg/manifest/job.go @@ -42,7 +42,7 @@ type ScheduledJobConfig struct { Sidecars map[string]*SidecarConfig `yaml:"sidecars"` On JobTriggerConfig `yaml:"on,flow"` JobFailureHandlerConfig `yaml:",inline"` - Network networkConfig `yaml:"network"` + Network NetworkConfig `yaml:"network"` } // JobTriggerConfig represents the configuration for the event that triggers the job. @@ -79,9 +79,9 @@ func newDefaultScheduledJob() *ScheduledJob { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), }, }, }, diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index b00bcc6d436..b1f5507cf6f 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -51,7 +51,7 @@ type LoadBalancedWebServiceConfig struct { TaskConfig `yaml:",inline"` *Logging `yaml:"logging,flow"` Sidecars map[string]*SidecarConfig `yaml:"sidecars"` - Network networkConfig `yaml:"network"` + Network NetworkConfig `yaml:"network"` } // HTTPHealthCheckArgs holds the configuration to determine if the load balanced web service is healthy. @@ -149,9 +149,9 @@ func newDefaultLoadBalancedWebService() *LoadBalancedWebService { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), }, }, }, diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index e7f90096b08..2226b23553b 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -63,7 +63,7 @@ func TestNewLoadBalancedWebService(t *testing.T) { Value: aws.Int(1), }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), }, @@ -355,7 +355,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { Logging: &Logging{ ConfigFile: aws.String("mockConfigFile"), }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), SecurityGroups: []string{"sg-123"}, @@ -417,7 +417,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { "FOO": "BAR", }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ SecurityGroups: []string{"sg-456", "sg-789"}, }, @@ -504,7 +504,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { "FOO": "BAR", }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), SecurityGroups: []string{"sg-456", "sg-789"}, diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index 0591b985f0e..6a507f08a35 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -109,7 +109,7 @@ environments: "LOG_TOKEN": "LOG_TOKEN", }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), }, @@ -187,7 +187,7 @@ secrets: "API_TOKEN": "SUBS_API_TOKEN", }, }, - Network: networkConfig{ + Network: NetworkConfig{ VPC: vpcConfig{ Placement: stringP("public"), }, diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 80cdda4e47f..1a9d7b153e7 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -19,8 +19,8 @@ const ( defaultDockerfileName = "Dockerfile" // AWS VPC subnet placement options. - publicPlacement = "public" - privatePlacement = "private" + PublicSubnetPlacement = "public" + PrivateSubnetPlacement = "private" ) var ( @@ -28,7 +28,7 @@ var ( WorkloadTypes = append(ServiceTypes, JobTypes...) // All placement options. - subnetPlacements = []string{publicPlacement, privatePlacement} + subnetPlacements = []string{PublicSubnetPlacement, PrivateSubnetPlacement} // Error definitions. errUnmarshalBuildOpts = errors.New("can't unmarshal build field into string or compose-style map") @@ -234,16 +234,18 @@ type TaskConfig struct { Storage *Storage `yaml:"storage"` } -// networkConfig represents options for network connection to AWS resources within a VPC. -type networkConfig struct { +// NetworkConfig represents options for network connection to AWS resources within a VPC. +type NetworkConfig struct { VPC vpcConfig `yaml:"vpc"` } -func (c *networkConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { - type networkWithDefaults networkConfig +// UnmarshalYAML ensures that a NetworkConfig always defaults to public subnets. +// If the user specified a placement that's not valid then throw an error. +func (c *NetworkConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { + type networkWithDefaults NetworkConfig conf := networkWithDefaults{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), }, } if err := unmarshal(&conf); err != nil { @@ -252,7 +254,7 @@ func (c *networkConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if !conf.VPC.isValidPlacement() { return fmt.Errorf("field '%s' is '%v' must be one of %#v", "network.vpc.placement", aws.StringValue(conf.VPC.Placement), subnetPlacements) } - *c = networkConfig(conf) + *c = NetworkConfig(conf) return nil } diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index 5bf51859aa6..f0174c94d39 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -271,7 +271,7 @@ func TestNetworkConfig_UnmarshalYAML(t *testing.T) { testCases := map[string]struct { data string - wantedConfig networkConfig + wantedConfig NetworkConfig wantedErr error }{ "defaults to public placement if vpc is empty": { @@ -279,9 +279,9 @@ func TestNetworkConfig_UnmarshalYAML(t *testing.T) { network: vpc: `, - wantedConfig: networkConfig{ + wantedConfig: NetworkConfig{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), }, }, }, @@ -302,9 +302,9 @@ network: - 'sg-1234' - 'sg-4567' `, - wantedConfig: networkConfig{ + wantedConfig: NetworkConfig{ VPC: vpcConfig{ - Placement: stringP(publicPlacement), + Placement: stringP(PublicSubnetPlacement), SecurityGroups: []string{"sg-1234", "sg-4567"}, }, }, @@ -315,7 +315,7 @@ network: t.Run(name, func(t *testing.T) { // GIVEN type manifest struct { - Network networkConfig `yaml:"network"` + Network NetworkConfig `yaml:"network"` } var m manifest diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index 055264d4023..ea60a315eb6 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -28,6 +28,15 @@ const ( scheduledJobTplName = "scheduled-job" ) +// Constants for workload options. +const ( + // AWS VPC networking configuration. + EnablePublicIP = "ENABLED" + DisablePublicIP = "DISABLED" + PublicSubnetsPlacement = "PublicSubnets" + PrivateSubnetsPlacement = "PrivateSubnets" +) + var ( // Template names under "workloads/partials/cf/". partialsWorkloadCFTemplateNames = []string{ @@ -151,8 +160,8 @@ type NetworkOpts struct { func defaultNetworkOpts() *NetworkOpts { return &NetworkOpts{ - AssignPublicIP: "ENABLED", - SubnetsType: "PublicSubnets", + AssignPublicIP: EnablePublicIP, + SubnetsType: PublicSubnetsPlacement, } }