From f2ad79627856127815450fc385ae81529d6be148 Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Mon, 3 Oct 2022 09:43:50 -0700 Subject: [PATCH 1/7] adding ReadonlyFS field --- .../cloudformation/stack/transformers.go | 8 ++++++ .../cloudformation/stack/transformers_test.go | 27 +++++++++++++++++++ internal/pkg/manifest/storage.go | 7 ++--- internal/pkg/manifest/storage_test.go | 5 ++++ internal/pkg/manifest/validate.go | 6 +++++ internal/pkg/manifest/validate_test.go | 6 +++++ .../partials/cf/workload-container.yml | 3 +++ internal/pkg/template/workload.go | 1 + 8 files changed, 60 insertions(+), 3 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index b69966f058c..37d73c34fbc 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -531,6 +531,7 @@ func convertStorageOpts(wlName *string, in manifest.Storage) *template.StorageOp } return &template.StorageOpts{ Ephemeral: convertEphemeral(in.Ephemeral), + ReadonlyRootFS: convertReadOnlyFS(in.ReadonlyRootFS), Volumes: convertVolumes(in.Volumes), MountPoints: convertMountPoints(in.Volumes), EFSPerms: convertEFSPermissions(in.Volumes), @@ -547,6 +548,13 @@ func convertEphemeral(in *int) *int { return in } +func convertReadOnlyFS(in *bool) *bool { + if in != nil { + in = aws.Bool(defaultReadOnly) + } + return in +} + // convertSidecarMountPoints is used to convert from manifest to template objects. func convertSidecarMountPoints(in []manifest.SidecarMountPoint) []*template.MountPoint { if len(in) == 0 { diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index 37ac078239d..3f691bdad16 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -1349,6 +1349,33 @@ func Test_convertEphemeral(t *testing.T) { } } +func Test_convertReadOnlyFS(t *testing.T) { + testCases := map[string]struct { + inReadonly_rootfs *bool + wanted *bool + wantedError error + }{ + "without readonlyfs enabled": { + inReadonly_rootfs: nil, + wanted: nil, + }, + "readonlyfs specified true": { + inReadonly_rootfs: aws.Bool(true), + wanted: aws.Bool(true), + }, + "readonlyrootfs specified false": { + inReadonly_rootfs: aws.Bool(false), + wanted: aws.Bool(true), + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got := convertReadOnlyFS(tc.inReadonly_rootfs) + require.Equal(t, got, tc.wanted) + }) + } +} + func Test_convertPublish(t *testing.T) { accountId := "123456789123" partition := "aws" diff --git a/internal/pkg/manifest/storage.go b/internal/pkg/manifest/storage.go index a83c79f415a..f0db4f4baf0 100644 --- a/internal/pkg/manifest/storage.go +++ b/internal/pkg/manifest/storage.go @@ -17,13 +17,14 @@ var ( // Storage represents the options for external and native storage. type Storage struct { - Ephemeral *int `yaml:"ephemeral"` - Volumes map[string]*Volume `yaml:"volumes"` // NOTE: keep the pointers because `mergo` doesn't automatically deep merge map's value unless it's a pointer type. + Ephemeral *int `yaml:"ephemeral"` + ReadonlyRootFS *bool `yaml:"readonly_fs"` + Volumes map[string]*Volume `yaml:"volumes"` // NOTE: keep the pointers because `mergo` doesn't automatically deep merge map's value unless it's a pointer type. } // IsEmpty returns empty if the struct has all zero members. func (s *Storage) IsEmpty() bool { - return s.Ephemeral == nil && s.Volumes == nil + return s.Ephemeral == nil && s.Volumes == nil && s.ReadonlyRootFS == nil } func (s *Storage) requiredEnvFeatures() []string { diff --git a/internal/pkg/manifest/storage_test.go b/internal/pkg/manifest/storage_test.go index 848a7183609..4d49d698345 100644 --- a/internal/pkg/manifest/storage_test.go +++ b/internal/pkg/manifest/storage_test.go @@ -257,6 +257,11 @@ func TestStorage_IsEmpty(t *testing.T) { in: Storage{}, wanted: true, }, + "non empty storage with ReadOnlyFS": { + in: Storage{ + ReadonlyRootFS: aws.Bool(true), + }, + }, "non empty storage": { in: Storage{ Volumes: map[string]*Volume{ diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 0c8d511b7f9..fe7538d90ab 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -1106,6 +1106,12 @@ func (s Storage) validate() error { return fmt.Errorf(`validate "ephemeral": ephemeral storage must be between 20 GiB and 200 GiB`) } } + if s.ReadonlyRootFS != nil { + readOnlyRootFs := aws.BoolValue(s.ReadonlyRootFS) + if !readOnlyRootFs { + return fmt.Errorf(`validate "readOnlyRootFs": readonlyRootFS must be true`) + } + } var hasManagedVolume bool for k, v := range s.Volumes { if err := v.validate(); err != nil { diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 7a123d4f2b1..682f43df6bd 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -2101,6 +2101,12 @@ func TestStorage_validate(t *testing.T) { }, wantedError: fmt.Errorf(`validate "ephemeral": ephemeral storage must be between 20 GiB and 200 GiB`), }, + "error if readonlyfs is invalid": { + Storage: Storage{ + ReadonlyRootFS: aws.Bool(false), + }, + wantedError: fmt.Errorf(`validate "readOnlyRootFs": readonlyRootFS must be true`), + }, "error if fail to validate volumes": { Storage: Storage{ Volumes: map[string]*Volume{ diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 0baab5c7fb6..122885d4918 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -48,6 +48,9 @@ StartPeriod: {{.HealthCheck.StartPeriod}} Timeout: {{.HealthCheck.Timeout}} {{- end}} +{{- if .Storage}} + ReadonlyRootFilesystem: {{.Storage.ReadonlyRootFS}} +{{- end}} {{- if .CredentialsParameter}} RepositoryCredentials: CredentialsParameter: {{.CredentialsParameter}} diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index 4fb8c2f0c92..88f197aa8d3 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -140,6 +140,7 @@ type SidecarStorageOpts struct { // StorageOpts holds data structures for rendering Volumes and Mount Points type StorageOpts struct { Ephemeral *int + ReadonlyRootFS *bool Volumes []*Volume MountPoints []*MountPoint EFSPerms []*EFSPermission From a507cba88e58209604a35a31c95bfa1b66f97e1f Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Wed, 5 Oct 2022 12:13:13 -0700 Subject: [PATCH 2/7] Validation of Readonlyfs for a Windows Container --- .../cloudformation/stack/transformers.go | 9 +- .../cloudformation/stack/transformers_test.go | 27 ------ internal/pkg/manifest/validate.go | 14 +-- internal/pkg/manifest/validate_test.go | 86 +++++++++++-------- .../partials/cf/workload-container.yml | 2 + 5 files changed, 59 insertions(+), 79 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 37d73c34fbc..7d11c64d9ee 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -531,7 +531,7 @@ func convertStorageOpts(wlName *string, in manifest.Storage) *template.StorageOp } return &template.StorageOpts{ Ephemeral: convertEphemeral(in.Ephemeral), - ReadonlyRootFS: convertReadOnlyFS(in.ReadonlyRootFS), + ReadonlyRootFS: in.ReadonlyRootFS, Volumes: convertVolumes(in.Volumes), MountPoints: convertMountPoints(in.Volumes), EFSPerms: convertEFSPermissions(in.Volumes), @@ -548,13 +548,6 @@ func convertEphemeral(in *int) *int { return in } -func convertReadOnlyFS(in *bool) *bool { - if in != nil { - in = aws.Bool(defaultReadOnly) - } - return in -} - // convertSidecarMountPoints is used to convert from manifest to template objects. func convertSidecarMountPoints(in []manifest.SidecarMountPoint) []*template.MountPoint { if len(in) == 0 { diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index 3f691bdad16..37ac078239d 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -1349,33 +1349,6 @@ func Test_convertEphemeral(t *testing.T) { } } -func Test_convertReadOnlyFS(t *testing.T) { - testCases := map[string]struct { - inReadonly_rootfs *bool - wanted *bool - wantedError error - }{ - "without readonlyfs enabled": { - inReadonly_rootfs: nil, - wanted: nil, - }, - "readonlyfs specified true": { - inReadonly_rootfs: aws.Bool(true), - wanted: aws.Bool(true), - }, - "readonlyrootfs specified false": { - inReadonly_rootfs: aws.Bool(false), - wanted: aws.Bool(true), - }, - } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - got := convertReadOnlyFS(tc.inReadonly_rootfs) - require.Equal(t, got, tc.wanted) - }) - } -} - func Test_convertPublish(t *testing.T) { accountId := "123456789123" partition := "aws" diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index fe7538d90ab..543810fd1c4 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -160,6 +160,7 @@ func (l LoadBalancedWebServiceConfig) validate() error { if l.TaskConfig.IsWindows() { if err = validateWindows(validateWindowsOpts{ efsVolumes: l.Storage.Volumes, + readOnlyFS: l.Storage.ReadonlyRootFS, }); err != nil { return fmt.Errorf("validate Windows: %w", err) } @@ -247,6 +248,7 @@ func (b BackendServiceConfig) validate() error { if b.TaskConfig.IsWindows() { if err = validateWindows(validateWindowsOpts{ efsVolumes: b.Storage.Volumes, + readOnlyFS: b.Storage.ReadonlyRootFS, }); err != nil { return fmt.Errorf("validate Windows: %w", err) } @@ -359,6 +361,7 @@ func (w WorkerServiceConfig) validate() error { if w.TaskConfig.IsWindows() { if err = validateWindows(validateWindowsOpts{ efsVolumes: w.Storage.Volumes, + readOnlyFS: w.Storage.ReadonlyRootFS, }); err != nil { return fmt.Errorf(`validate Windows: %w`, err) } @@ -434,6 +437,7 @@ func (s ScheduledJobConfig) validate() error { if s.TaskConfig.IsWindows() { if err = validateWindows(validateWindowsOpts{ efsVolumes: s.Storage.Volumes, + readOnlyFS: s.Storage.ReadonlyRootFS, }); err != nil { return fmt.Errorf(`validate Windows: %w`, err) } @@ -1106,12 +1110,6 @@ func (s Storage) validate() error { return fmt.Errorf(`validate "ephemeral": ephemeral storage must be between 20 GiB and 200 GiB`) } } - if s.ReadonlyRootFS != nil { - readOnlyRootFs := aws.BoolValue(s.ReadonlyRootFS) - if !readOnlyRootFs { - return fmt.Errorf(`validate "readOnlyRootFs": readonlyRootFS must be true`) - } - } var hasManagedVolume bool for k, v := range s.Volumes { if err := v.validate(); err != nil { @@ -1601,6 +1599,7 @@ type validateTargetContainerOpts struct { } type validateWindowsOpts struct { + readOnlyFS *bool efsVolumes map[string]*Volume } @@ -1746,6 +1745,9 @@ func isValidSubSvcName(name string) bool { } func validateWindows(opts validateWindowsOpts) error { + if aws.BoolValue(opts.readOnlyFS) { + return errors.New(`'ReadOnlyFS' can not be set to true when deploying a Windows container`) + } for _, volume := range opts.efsVolumes { if !volume.EmptyVolume() { return errors.New(`'EFS' is not supported when deploying a Windows container`) diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 682f43df6bd..daeee691a2c 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -211,17 +211,18 @@ func TestLoadBalancedWebService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), + Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), + }, }, }, }, - }, }, RoutingRule: RoutingRuleConfigOrBool{ RoutingRuleConfiguration: RoutingRuleConfiguration{ @@ -477,17 +478,18 @@ func TestBackendService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), + Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), + }, }, }, }, - }, }, }, }, @@ -869,17 +871,18 @@ func TestWorkerService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), + Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), + }, }, }, }, - }, }, }, }, @@ -1084,17 +1087,18 @@ func TestScheduledJob_validate(t *testing.T) { }, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), + Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), + }, }, }, }, - }, }, }, }, @@ -2101,12 +2105,6 @@ func TestStorage_validate(t *testing.T) { }, wantedError: fmt.Errorf(`validate "ephemeral": ephemeral storage must be between 20 GiB and 200 GiB`), }, - "error if readonlyfs is invalid": { - Storage: Storage{ - ReadonlyRootFS: aws.Bool(false), - }, - wantedError: fmt.Errorf(`validate "readOnlyRootFs": readonlyRootFS must be true`), - }, "error if fail to validate volumes": { Storage: Storage{ Volumes: map[string]*Volume{ @@ -3248,6 +3246,18 @@ func TestValidateWindows(t *testing.T) { in: validateWindowsOpts{}, wantedError: nil, }, + "error if readonlyfs is true": { + in: validateWindowsOpts{ + readOnlyFS: aws.Bool(true), + }, + wantedError: errors.New(`'ReadOnlyFS' can not be set to true when deploying a Windows container`), + }, + "should return nil readonlyfs is false or nil": { + in: validateWindowsOpts{ + readOnlyFS: aws.Bool(false), + }, + wantedError: nil, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 122885d4918..976096d5516 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -49,8 +49,10 @@ Timeout: {{.HealthCheck.Timeout}} {{- end}} {{- if .Storage}} +{{- if .Storage.ReadonlyRootFS}} ReadonlyRootFilesystem: {{.Storage.ReadonlyRootFS}} {{- end}} +{{- end}} {{- if .CredentialsParameter}} RepositoryCredentials: CredentialsParameter: {{.CredentialsParameter}} From 07520d18d97ea05e59b3e63ac81a47d3c31cf314 Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Wed, 5 Oct 2022 21:10:07 -0700 Subject: [PATCH 3/7] adding ReadOnlyFS to manifest file --- .../testdata/backend-svc-customhealthcheck.yml | 4 +++- .../testdata/backend-svc-nohealthcheck.yml | 4 +++- internal/pkg/manifest/testdata/lb-svc.yml | 2 ++ .../manifest/testdata/worker-svc-nosubscribe.yml | 2 ++ .../pkg/manifest/testdata/worker-svc-subscribe.yml | 2 ++ internal/pkg/manifest/validate.go | 2 +- internal/pkg/manifest/validate_test.go | 14 +++++++------- .../workloads/services/backend/manifest.yml | 4 +++- .../workloads/services/lb-web/manifest.yml | 2 ++ .../workloads/services/worker/manifest.yml | 2 ++ 10 files changed, 27 insertions(+), 11 deletions(-) diff --git a/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml b/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml index 82f3dc93561..3b25812169f 100644 --- a/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml +++ b/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml @@ -39,4 +39,6 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml b/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml index d8854755d4c..f76cfb7c823 100644 --- a/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml +++ b/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml @@ -31,4 +31,6 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/lb-svc.yml b/internal/pkg/manifest/testdata/lb-svc.yml index acf26414154..478fb30d41a 100644 --- a/internal/pkg/manifest/testdata/lb-svc.yml +++ b/internal/pkg/manifest/testdata/lb-svc.yml @@ -40,3 +40,5 @@ exec: true # Enable running commands in your container. # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. # rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml b/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml index 2bf3924104b..1ca90494481 100644 --- a/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml +++ b/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml @@ -37,3 +37,5 @@ exec: true # Enable running commands in your container. # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. # rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/worker-svc-subscribe.yml b/internal/pkg/manifest/testdata/worker-svc-subscribe.yml index 457dbabf944..b541dad3f11 100644 --- a/internal/pkg/manifest/testdata/worker-svc-subscribe.yml +++ b/internal/pkg/manifest/testdata/worker-svc-subscribe.yml @@ -38,3 +38,5 @@ subscribe: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. # rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 543810fd1c4..a0e7a2cb89d 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -1746,7 +1746,7 @@ func isValidSubSvcName(name string) bool { func validateWindows(opts validateWindowsOpts) error { if aws.BoolValue(opts.readOnlyFS) { - return errors.New(`'ReadOnlyFS' can not be set to true when deploying a Windows container`) + return fmt.Errorf(`%q can not be set to 'true' when deploying a Windows container`, "readonly_fs") } for _, volume := range opts.efsVolumes { if !volume.EmptyVolume() { diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index daeee691a2c..6267d6d7540 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -211,7 +211,7 @@ func TestLoadBalancedWebService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Storage: Storage{ Volumes: map[string]*Volume{ "foo": { EFS: EFSConfigOrBool{ @@ -478,7 +478,7 @@ func TestBackendService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Storage: Storage{ Volumes: map[string]*Volume{ "foo": { EFS: EFSConfigOrBool{ @@ -871,7 +871,7 @@ func TestWorkerService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Storage: Storage{ Volumes: map[string]*Volume{ "foo": { EFS: EFSConfigOrBool{ @@ -1087,7 +1087,7 @@ func TestScheduledJob_validate(t *testing.T) { }, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ReadonlyRootFS: aws.Bool(false), + Storage: Storage{ Volumes: map[string]*Volume{ "foo": { EFS: EFSConfigOrBool{ @@ -3242,7 +3242,7 @@ func TestValidateWindows(t *testing.T) { }, wantedError: errors.New(`'EFS' is not supported when deploying a Windows container`), }, - "should return nil efs not specified": { + "should return nil when no fields are specified": { in: validateWindowsOpts{}, wantedError: nil, }, @@ -3250,9 +3250,9 @@ func TestValidateWindows(t *testing.T) { in: validateWindowsOpts{ readOnlyFS: aws.Bool(true), }, - wantedError: errors.New(`'ReadOnlyFS' can not be set to true when deploying a Windows container`), + wantedError: fmt.Errorf(`%q can not be set to 'true' when deploying a Windows container`, "readonly_fs"), }, - "should return nil readonlyfs is false or nil": { + "should return nil if readonly_fs is false": { in: validateWindowsOpts{ readOnlyFS: aws.Bool(false), }, diff --git a/internal/pkg/template/templates/workloads/services/backend/manifest.yml b/internal/pkg/template/templates/workloads/services/backend/manifest.yml index 1e644fdb871..d940937f551 100644 --- a/internal/pkg/template/templates/workloads/services/backend/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/backend/manifest.yml @@ -57,4 +57,6 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml b/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml index 7f0c4f4b7fe..b509a13b793 100644 --- a/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml @@ -53,3 +53,5 @@ exec: true # Enable running commands in your container. # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. # rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file diff --git a/internal/pkg/template/templates/workloads/services/worker/manifest.yml b/internal/pkg/template/templates/workloads/services/worker/manifest.yml index 7663c1d16aa..0f49710f468 100644 --- a/internal/pkg/template/templates/workloads/services/worker/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/worker/manifest.yml @@ -65,3 +65,5 @@ subscribe: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. # rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +storage: + readonly_fs: true \ No newline at end of file From 2b3fb871c8585edff32ac9cd8fbee1b2a954151d Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Thu, 6 Oct 2022 01:20:14 -0700 Subject: [PATCH 4/7] updated unit tests --- internal/pkg/manifest/validate_test.go | 51 +++++++++---------- .../partials/cf/workload-container.yml | 4 +- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 6267d6d7540..e16739168ce 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -211,18 +211,17 @@ func TestLoadBalancedWebService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ - Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), - }, + Storage: Storage{Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), }, }, }, + }, }, RoutingRule: RoutingRuleConfigOrBool{ RoutingRuleConfiguration: RoutingRuleConfiguration{ @@ -478,18 +477,17 @@ func TestBackendService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ - Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), - }, + Storage: Storage{Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), }, }, }, + }, }, }, }, @@ -1087,18 +1085,17 @@ func TestScheduledJob_validate(t *testing.T) { }, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ - Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), - }, + Storage: Storage{Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), }, }, }, + }, }, }, }, diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 976096d5516..8b1d8971d2b 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -48,11 +48,9 @@ StartPeriod: {{.HealthCheck.StartPeriod}} Timeout: {{.HealthCheck.Timeout}} {{- end}} -{{- if .Storage}} -{{- if .Storage.ReadonlyRootFS}} +{{- if and .Storage .Storage.ReadonlyRootFS}} ReadonlyRootFilesystem: {{.Storage.ReadonlyRootFS}} {{- end}} -{{- end}} {{- if .CredentialsParameter}} RepositoryCredentials: CredentialsParameter: {{.CredentialsParameter}} From 9244cbeba1f9c3e0a6fae4f516be061c8c84960e Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Thu, 6 Oct 2022 01:22:13 -0700 Subject: [PATCH 5/7] updated unit tests --- internal/pkg/manifest/validate_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index e16739168ce..ab252ddcbe0 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -869,18 +869,17 @@ func TestWorkerService_validate(t *testing.T) { ImageConfig: testImageConfig, TaskConfig: TaskConfig{ Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, - Storage: Storage{ - Volumes: map[string]*Volume{ - "foo": { - EFS: EFSConfigOrBool{ - Enabled: aws.Bool(true), - }, - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("mockPath"), - }, + Storage: Storage{Volumes: map[string]*Volume{ + "foo": { + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), }, }, }, + }, }, }, }, From 451871eeef9e6ed844a03e0cbeff4c08ba85d90f Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Sat, 8 Oct 2022 00:17:01 -0700 Subject: [PATCH 6/7] addressing feedback --- .../testdata/backend-svc-customhealthcheck.yml | 7 ++++--- .../manifest/testdata/backend-svc-nohealthcheck.yml | 7 ++++--- internal/pkg/manifest/testdata/lb-svc.yml | 7 ++++--- .../pkg/manifest/testdata/worker-svc-nosubscribe.yml | 8 +++++--- .../pkg/manifest/testdata/worker-svc-subscribe.yml | 8 +++++--- .../templates/workloads/services/backend/manifest.yml | 9 ++++++--- .../templates/workloads/services/lb-web/manifest.yml | 9 ++++++--- .../templates/workloads/services/worker/manifest.yml | 10 +++++++--- 8 files changed, 41 insertions(+), 24 deletions(-) diff --git a/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml b/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml index 3b25812169f..d6d74c13ff5 100644 --- a/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml +++ b/internal/pkg/manifest/testdata/backend-svc-customhealthcheck.yml @@ -26,6 +26,9 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. @@ -39,6 +42,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml b/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml index f76cfb7c823..e23f81f8337 100644 --- a/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml +++ b/internal/pkg/manifest/testdata/backend-svc-nohealthcheck.yml @@ -18,6 +18,9 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. @@ -31,6 +34,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/lb-svc.yml b/internal/pkg/manifest/testdata/lb-svc.yml index 478fb30d41a..bf8ea1bce0d 100644 --- a/internal/pkg/manifest/testdata/lb-svc.yml +++ b/internal/pkg/manifest/testdata/lb-svc.yml @@ -26,6 +26,9 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. @@ -39,6 +42,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml b/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml index 1ca90494481..47ab8e26710 100644 --- a/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml +++ b/internal/pkg/manifest/testdata/worker-svc-nosubscribe.yml @@ -16,6 +16,10 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + + # You can register to topics from other services. # The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. # subscribe: @@ -36,6 +40,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/manifest/testdata/worker-svc-subscribe.yml b/internal/pkg/manifest/testdata/worker-svc-subscribe.yml index b541dad3f11..45532afecd8 100644 --- a/internal/pkg/manifest/testdata/worker-svc-subscribe.yml +++ b/internal/pkg/manifest/testdata/worker-svc-subscribe.yml @@ -16,6 +16,10 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + + # The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. subscribe: topics: @@ -37,6 +41,4 @@ subscribe: # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/template/templates/workloads/services/backend/manifest.yml b/internal/pkg/template/templates/workloads/services/backend/manifest.yml index d940937f551..6b4be00d4d2 100644 --- a/internal/pkg/template/templates/workloads/services/backend/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/backend/manifest.yml @@ -44,6 +44,11 @@ count: {{.Count.Value}} # Number of tasks that should be running in your s exec: true # Enable running commands in your container. {{- end}} +storage: +{{- if not .TaskConfig.IsWindows}} + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. +{{- end}} + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. @@ -57,6 +62,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml b/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml index b509a13b793..8667f3574bb 100644 --- a/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/lb-web/manifest.yml @@ -39,6 +39,11 @@ count: {{.Count.Value}} # Number of tasks that should be running in your s exec: true # Enable running commands in your container. {{- end}} +storage: +{{- if not .TaskConfig.IsWindows}} + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. +{{- end}} + # Optional fields for more advanced use-cases. # #variables: # Pass environment variables as key value pairs. @@ -52,6 +57,4 @@ exec: true # Enable running commands in your container. # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file diff --git a/internal/pkg/template/templates/workloads/services/worker/manifest.yml b/internal/pkg/template/templates/workloads/services/worker/manifest.yml index 0f49710f468..9f9733b7141 100644 --- a/internal/pkg/template/templates/workloads/services/worker/manifest.yml +++ b/internal/pkg/template/templates/workloads/services/worker/manifest.yml @@ -34,6 +34,12 @@ count: {{.Count.Value}} # Number of tasks that should be running in your s {{- if not .TaskConfig.IsWindows }} exec: true # Enable running commands in your container. {{- end}} + +storage: +{{- if not .TaskConfig.IsWindows}} + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. +{{- end}} + {{if .Subscribe}}{{- if .Subscribe.Topics}} # The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. subscribe: @@ -64,6 +70,4 @@ subscribe: # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. -storage: - readonly_fs: true \ No newline at end of file +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file From 92ebe5cbea8f300b088eacfc30448bfd3b11fe8f Mon Sep 17 00:00:00 2001 From: Adithya Kolla Date: Thu, 13 Oct 2022 12:46:26 -0700 Subject: [PATCH 7/7] adressing merge conflicts --- .../testdata/worker-svc-with-default-fifo-queue.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml b/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml index 00e1da5e041..ad30eadcb99 100644 --- a/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml +++ b/internal/pkg/manifest/testdata/worker-svc-with-default-fifo-queue.yml @@ -16,6 +16,10 @@ memory: 512 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. exec: true # Enable running commands in your container. +storage: + readonly_fs: true # Limit to read-only access to mounted root filesystems by default. + + # The events can be be received from an SQS queue via the env var $COPILOT_QUEUE_URI. subscribe: topics: @@ -37,4 +41,4 @@ subscribe: # test: # count: 2 # Number of tasks to run for the "test" environment. # deployment: # The deployment strategy for the "test" environment. -# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. +# rolling: 'recreate' # Stops existing tasks before new ones are started for faster deployments. \ No newline at end of file