From 73f33259b8a73f657c08a2507454a3e8fcfa8df2 Mon Sep 17 00:00:00 2001 From: Janice Huang <60631893+huanjani@users.noreply.github.com> Date: Wed, 27 Oct 2021 20:14:00 +0000 Subject: [PATCH] fix: redirect AppRunner services on ARM architectures to AMD (#2955) Previously, ARM -> AMD redirection was failing because we don't surface the `platform` field in RDWS manifests. Now we hard code `linux/amd64` for empty `platform` fields. Fixes https://github.com/aws/copilot-cli/issues/2640. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/cli/svc_deploy.go | 10 +- internal/pkg/cli/svc_deploy_test.go | 2 +- internal/pkg/cli/svc_init.go | 2 +- internal/pkg/manifest/backend_svc.go | 20 --- internal/pkg/manifest/job.go | 16 -- internal/pkg/manifest/lb_web_svc.go | 18 --- internal/pkg/manifest/lb_web_svc_test.go | 87 ----------- internal/pkg/manifest/rd_web_svc.go | 28 +--- internal/pkg/manifest/rd_web_svc_test.go | 105 +++++-------- internal/pkg/manifest/validate.go | 69 ++++++++- internal/pkg/manifest/validate_test.go | 187 ++++++++++++++++++++++- internal/pkg/manifest/worker_svc.go | 18 --- internal/pkg/manifest/workload.go | 24 +-- 13 files changed, 309 insertions(+), 277 deletions(-) diff --git a/internal/pkg/cli/svc_deploy.go b/internal/pkg/cli/svc_deploy.go index 1ea45be275b..a2bc94de03e 100644 --- a/internal/pkg/cli/svc_deploy.go +++ b/internal/pkg/cli/svc_deploy.go @@ -396,28 +396,24 @@ func (o *deploySvcOpts) dfBuildArgs(svc interface{}) (*dockerengine.BuildArgumen func buildArgs(name, imageTag, copilotDir string, unmarshaledManifest interface{}) (*dockerengine.BuildArguments, error) { type dfArgs interface { BuildArgs(rootDirectory string) *manifest.DockerBuildArgs - TaskPlatform() (*string, error) + ContainerPlatform() string } mf, ok := unmarshaledManifest.(dfArgs) if !ok { - return nil, fmt.Errorf("%s does not have required methods BuildArgs() and TaskPlatform()", name) + return nil, fmt.Errorf("%s does not have required methods BuildArgs() and ContainerPlatform()", name) } var tags []string if imageTag != "" { tags = append(tags, imageTag) } args := mf.BuildArgs(filepath.Dir(copilotDir)) - platform, err := mf.TaskPlatform() - if err != nil { - return nil, fmt.Errorf("get platform for service: %w", err) - } return &dockerengine.BuildArguments{ Dockerfile: *args.Dockerfile, Context: *args.Context, Args: args.Args, CacheFrom: args.CacheFrom, Target: aws.StringValue(args.Target), - Platform: aws.StringValue(platform), + Platform: mf.ContainerPlatform(), Tags: tags, }, nil } diff --git a/internal/pkg/cli/svc_deploy_test.go b/internal/pkg/cli/svc_deploy_test.go index c38d828ebfa..7349ff49dbf 100644 --- a/internal/pkg/cli/svc_deploy_test.go +++ b/internal/pkg/cli/svc_deploy_test.go @@ -306,7 +306,7 @@ image: m.mockInterpolator.EXPECT().Interpolate(string(mockManifestWithBadPlatform)).Return(string(mockManifestWithBadPlatform), nil), ) }, - wantErr: errors.New("validate manifest against environment : validate \"platform\": platform linus/abc123 is invalid; valid platforms are: linux/amd64, linux/x86_64, windows/amd64 and windows/x86_64"), + wantErr: errors.New("validate manifest against environment : validate \"platform\": platform 'linus/abc123' is invalid; valid platforms are: linux/amd64, linux/x86_64, windows/amd64 and windows/x86_64"), }, "success with valid platform": { inputSvc: "serviceA", diff --git a/internal/pkg/cli/svc_init.go b/internal/pkg/cli/svc_init.go index ac332ac0718..81a1134667a 100644 --- a/internal/pkg/cli/svc_init.go +++ b/internal/pkg/cli/svc_init.go @@ -461,7 +461,7 @@ func legitimizePlatform(engine dockerEngine, wkldType string) (manifest.Platform if redirectedPlatform == "" { return "", nil } - if redirectedPlatform != detectedPlatform { + if redirectedPlatform != detectedPlatform && wkldType != manifest.RequestDrivenWebServiceType { log.Warningf("Your architecture type %s is currently unsupported. Setting platform %s instead.\n", color.HighlightCode(detectedArch), redirectedPlatform) } platform := manifest.PlatformString(redirectedPlatform) diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index 57c92081109..67064bedf1b 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -4,8 +4,6 @@ package manifest import ( - "errors" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/copilot-cli/internal/pkg/template" "github.com/imdario/mergo" @@ -127,24 +125,6 @@ func (s BackendService) ApplyEnv(envName string) (WorkloadManifest, error) { return &s, nil } -// windowsCompatibility disallows unsupported services when deploying Windows containers on Fargate. -func (s *BackendService) windowsCompatibility() error { - if !s.IsWindows() { - return nil - } - // Exec is not supported. - if aws.BoolValue(s.ExecuteCommand.Enable) { - return errors.New(`'exec' is not supported when deploying a Windows container`) - } - // EFS is not supported. - for _, volume := range s.Storage.Volumes { - if !volume.EmptyVolume() { - return errors.New(`'EFS' is not supported when deploying a Windows container`) - } - } - return nil -} - // newDefaultBackendService returns a backend service with minimal task sizes and a single replica. func newDefaultBackendService() *BackendService { return &BackendService{ diff --git a/internal/pkg/manifest/job.go b/internal/pkg/manifest/job.go index 92da610e20e..a3f125fae20 100644 --- a/internal/pkg/manifest/job.go +++ b/internal/pkg/manifest/job.go @@ -5,8 +5,6 @@ package manifest import ( - "errors" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/copilot-cli/internal/pkg/template" "github.com/imdario/mergo" @@ -123,20 +121,6 @@ func (j ScheduledJob) ApplyEnv(envName string) (WorkloadManifest, error) { return &j, nil } -// windowsCompatibility disallows unsupported services when deploying Windows containers on Fargate. -func (j *ScheduledJob) windowsCompatibility() error { - if !j.IsWindows() { - return nil - } - // EFS is not supported. - for _, volume := range j.Storage.Volumes { - if !volume.EmptyVolume() { - return errors.New(`'EFS' is not supported when deploying a Windows container`) - } - } - return nil -} - // Publish returns the list of topics where notifications can be published. func (j *ScheduledJob) Publish() []Topic { return j.ScheduledJobConfig.PublishConfig.Topics diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index d3dca108664..61a0eebaa55 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -192,24 +192,6 @@ type RoutingRule struct { AllowedSourceIps []IPNet `yaml:"allowed_source_ips"` } -// windowsCompatibility disallows unsupported services when deploying Windows containers on Fargate. -func (s *LoadBalancedWebService) windowsCompatibility() error { - if !s.IsWindows() { - return nil - } - // Exec is not supported. - if aws.BoolValue(s.ExecuteCommand.Enable) { - return errors.New(`'exec' is not supported when deploying a Windows container`) - } - // EFS is not supported. - for _, volume := range s.Storage.Volumes { - if !volume.EmptyVolume() { - return errors.New(`'EFS' is not supported when deploying a Windows container`) - } - } - return nil -} - // IPNet represents an IP network string. For example: 10.1.0.0/16 type IPNet string diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index 99b08049ebe..03da1c95b1d 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -4,7 +4,6 @@ package manifest import ( - "errors" "fmt" "io/ioutil" "path/filepath" @@ -1255,92 +1254,6 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) { } } -func TestLoadBalancedWebService_ValidateForWindows(t *testing.T) { - testCases := map[string]struct { - in *LoadBalancedWebService - wantedErr error - }{ - "returns nil if not windows": { - in: &LoadBalancedWebService{ - LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{ - PlatformString: (*PlatformString)(aws.String("doors/amp64")), - PlatformArgs: PlatformArgs{ - OSFamily: nil, - Arch: nil, - }, - }, - ExecuteCommand: ExecuteCommand{ - Config: ExecuteCommandConfig{ - Enable: aws.Bool(true)}, - }, - }, - }, - }, - wantedErr: nil, - }, - "throws error when windows and exec both present": { - in: &LoadBalancedWebService{ - LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{ - PlatformString: nil, - PlatformArgs: PlatformArgs{ - OSFamily: aws.String("Windows-Server-2019-Full"), - Arch: aws.String("amd64"), - }, - }, - ExecuteCommand: ExecuteCommand{ - Config: ExecuteCommandConfig{ - Enable: aws.Bool(true)}, - }, - }, - }, - }, - wantedErr: errors.New("'exec' is not supported when deploying a Windows container"), - }, - "throws error when windows and efs both present": { - in: &LoadBalancedWebService{ - LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{ - PlatformString: (*PlatformString)(aws.String("windows/amd64")), - }, - Storage: Storage{ - Volumes: map[string]*Volume{ - "myEFSVolume": { - MountPointOpts: MountPointOpts{ - ContainerPath: aws.String("/path/to/files"), - ReadOnly: aws.Bool(false), - }, - EFS: EFSConfigOrBool{ - Advanced: EFSVolumeConfiguration{ - FileSystemID: aws.String("fs-1234"), - }, - }, - }, - }, - }, - }, - }, - }, - wantedErr: errors.New("'EFS' is not supported when deploying a Windows container"), - }, - } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // WHEN - err := tc.in.windowsCompatibility() - - // THEN - if err != nil { - require.EqualError(t, err, tc.wantedErr.Error(), "errors should be returned when disallowed services present") - } - }) - } -} - func TestLoadBalancedWebService_Port(t *testing.T) { // GIVEN mft := LoadBalancedWebService{ diff --git a/internal/pkg/manifest/rd_web_svc.go b/internal/pkg/manifest/rd_web_svc.go index db91a23e5cf..913f61ed813 100644 --- a/internal/pkg/manifest/rd_web_svc.go +++ b/internal/pkg/manifest/rd_web_svc.go @@ -4,8 +4,6 @@ package manifest import ( - "fmt" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/copilot-cli/internal/pkg/template" "github.com/imdario/mergo" @@ -92,12 +90,12 @@ func (s *RequestDrivenWebService) BuildRequired() (bool, error) { return requiresBuild(s.ImageConfig.Image) } -// TaskPlatform returns the platform for the service. -func (s *RequestDrivenWebService) TaskPlatform() (*string, error) { - if s.InstanceConfig.Platform.PlatformString == nil { - return nil, nil +// ContainerPlatform returns the platform for the service. +func (s *RequestDrivenWebService) ContainerPlatform() string { + if s.InstanceConfig.Platform.IsEmpty() { + return platformString(OSLinux, ArchAMD64) } - return aws.String(platformString(s.InstanceConfig.Platform.OS(), s.InstanceConfig.Platform.Arch())), nil + return platformString(s.InstanceConfig.Platform.OS(), s.InstanceConfig.Platform.Arch()) } // BuildArgs returns a docker.BuildArguments object given a ws root directory. @@ -126,22 +124,6 @@ func (s RequestDrivenWebService) ApplyEnv(envName string) (WorkloadManifest, err return &s, nil } -// WindowsCompatibility disallows unsupported services when deploying Windows containers on Fargate. -// Here, this method is simply satisfying the WorkloadManifest interface. -func (s *RequestDrivenWebService) windowsCompatibility() error { - if s.InstanceConfig.Platform.IsEmpty() { - return nil - } - // Error out if user added Windows as platform in manifest. - if isWindowsPlatform(s.InstanceConfig.Platform) { - return errAppRunnerInvalidPlatformWindows - } - if s.InstanceConfig.Platform.Arch() != ArchAMD64 || s.InstanceConfig.Platform.Arch() != ArchX86 { - return fmt.Errorf("App Runner services can only build on %s and %s architectures", ArchAMD64, ArchX86) - } - return nil -} - // newDefaultRequestDrivenWebService returns an empty RequestDrivenWebService with only the default values set. func newDefaultRequestDrivenWebService() *RequestDrivenWebService { return &RequestDrivenWebService{ diff --git a/internal/pkg/manifest/rd_web_svc_test.go b/internal/pkg/manifest/rd_web_svc_test.go index b20c88cd5a2..af6a0cb2a87 100644 --- a/internal/pkg/manifest/rd_web_svc_test.go +++ b/internal/pkg/manifest/rd_web_svc_test.go @@ -316,6 +316,46 @@ func TestRequestDrivenWebService_Port(t *testing.T) { require.Equal(t, uint16(80), actual) } +func TestRequestDrivenWebService_ContainerPlatform(t *testing.T) { + t.Run("should return platform string with values found in args", func(t *testing.T) { + // GIVEN + mft := RequestDrivenWebService{ + RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ + InstanceConfig: AppRunnerInstanceConfig{ + Platform: PlatformArgsOrString{ + PlatformArgs: PlatformArgs{ + OSFamily: aws.String("ososos"), + Arch: aws.String("arch"), + }, + }, + }, + }, + } + // WHEN + actual := mft.ContainerPlatform() + + // THEN + require.Equal(t, "ososos/arch", actual) + }) + t.Run("should return default platform if platform field empty", func(t *testing.T) { + // GIVEN + mft := RequestDrivenWebService{ + RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ + InstanceConfig: AppRunnerInstanceConfig{ + Platform: PlatformArgsOrString{ + PlatformString: nil, + }, + }, + }, + } + // WHEN + actual := mft.ContainerPlatform() + + // THEN + require.Equal(t, "linux/amd64", actual) + + }) +} func TestRequestDrivenWebService_Publish(t *testing.T) { testCases := map[string]struct { mft *RequestDrivenWebService @@ -547,68 +587,3 @@ func TestRequestDrivenWebService_ApplyEnv(t *testing.T) { }) } } - -func TestRequestDrivenWebService_windowsCompatibility(t *testing.T) { - testCases := map[string]struct { - in *RequestDrivenWebService - - wantedErr error - }{ - "returns error if windows indicated as string": { - in: &RequestDrivenWebService{ - RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ - InstanceConfig: AppRunnerInstanceConfig{ - Platform: PlatformArgsOrString{ - PlatformString: (*PlatformString)(aws.String("windows/amd64")), - PlatformArgs: PlatformArgs{}, - }, - }, - }, - }, - wantedErr: errors.New("Windows is not supported for App Runner services"), - }, - "returns error if windows indicated as map": { - in: &RequestDrivenWebService{ - RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ - InstanceConfig: AppRunnerInstanceConfig{ - Platform: PlatformArgsOrString{ - PlatformString: nil, - PlatformArgs: PlatformArgs{ - OSFamily: stringP("windows_server_2019_core"), - Arch: stringP("arm64"), - }, - }, - }, - }, - }, - wantedErr: errors.New("Windows is not supported for App Runner services"), - }, - "returns error if arch is not valid": { - in: &RequestDrivenWebService{ - RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ - InstanceConfig: AppRunnerInstanceConfig{ - Platform: PlatformArgsOrString{ - PlatformString: nil, - PlatformArgs: PlatformArgs{ - OSFamily: stringP("linux"), - Arch: stringP("arm64"), - }, - }, - }, - }, - }, - wantedErr: errors.New("App Runner services can only build on amd64 and x86_64 architectures"), - }, - } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - // WHEN - err := tc.in.windowsCompatibility() - - // THEN - if err != nil { - require.EqualError(t, err, tc.wantedErr.Error()) - } - }) - } -} diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index f7b0a400e09..f9e9abf5da9 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -103,6 +103,14 @@ func (l LoadBalancedWebServiceConfig) Validate() error { return fmt.Errorf(`validate "taskdef_overrides[%d]": %w`, ind, err) } } + if l.TaskConfig.IsWindows() { + if err = validateWindows(validateWindowsOpts{ + execEnabled: aws.BoolValue(l.ExecuteCommand.Enable), + efsVolumes: l.Storage.Volumes, + }); err != nil { + return fmt.Errorf("validate Windows: %w", err) + } + } return nil } @@ -156,6 +164,14 @@ func (b BackendServiceConfig) Validate() error { return fmt.Errorf(`validate "taskdef_overrides[%d]": %w`, ind, err) } } + if b.TaskConfig.IsWindows() { + if err = validateWindows(validateWindowsOpts{ + execEnabled: aws.BoolValue(b.ExecuteCommand.Enable), + efsVolumes: b.Storage.Volumes, + }); err != nil { + return fmt.Errorf("validate Windows: %w", err) + } + } return nil } @@ -173,6 +189,9 @@ func (r RequestDrivenWebServiceConfig) Validate() error { if err = r.ImageConfig.Validate(); err != nil { return fmt.Errorf(`validate "image": %w`, err) } + if err = r.InstanceConfig.Validate(); err != nil { + return err + } if err = r.RequestDrivenWebServiceHttpConfig.Validate(); err != nil { return fmt.Errorf(`validate "http": %w`, err) } @@ -235,6 +254,14 @@ func (w WorkerServiceConfig) Validate() error { return fmt.Errorf(`validate "taskdef_overrides[%d]": %w`, ind, err) } } + if w.TaskConfig.IsWindows() { + if err = validateWindows(validateWindowsOpts{ + execEnabled: aws.BoolValue(w.ExecuteCommand.Enable), + efsVolumes: w.Storage.Volumes, + }); err != nil { + return fmt.Errorf(`validate Windows: %w`, err) + } + } return nil } @@ -294,6 +321,14 @@ func (s ScheduledJobConfig) Validate() error { return fmt.Errorf(`validate "taskdef_overrides[%d]": %w`, ind, err) } } + if s.TaskConfig.IsWindows() { + if err = validateWindows(validateWindowsOpts{ + execEnabled: aws.BoolValue(s.ExecuteCommand.Enable), + efsVolumes: s.Storage.Volumes, + }); err != nil { + return fmt.Errorf(`validate Windows: %w`, err) + } + } return nil } @@ -549,13 +584,18 @@ func (p PlatformArgs) Validate() error { return fmt.Errorf("platform pair %s is invalid: fields ('osfamily', 'architecture') must be one of %s", p.String(), prettyValidPlatforms) } +// Validate returns nil if PlatformString is configured correctly. func (p PlatformString) Validate() error { + args := strings.Split(string(p), "/") + if len(args) != 2 { + return fmt.Errorf("platform '%s' must be in the format [OS]/[Arch]", string(p)) + } for _, validPlatform := range ValidShortPlatforms { if strings.ToLower(string(p)) == validPlatform { return nil } } - return fmt.Errorf("platform %s is invalid; %s: %s", p, english.PluralWord(len(ValidShortPlatforms), "the valid platform is", "valid platforms are"), english.WordSeries(ValidShortPlatforms, "and")) + return fmt.Errorf("platform '%s' is invalid; %s: %s", p, english.PluralWord(len(ValidShortPlatforms), "the valid platform is", "valid platforms are"), english.WordSeries(ValidShortPlatforms, "and")) } // Validate returns nil if Count is configured correctly. @@ -902,6 +942,16 @@ func (r AppRunnerInstanceConfig) Validate() error { if err := r.Platform.Validate(); err != nil { return fmt.Errorf(`validate "platform": %w`, err) } + // Error out if user added Windows as platform in manifest. + if isWindowsPlatform(r.Platform) { + return errAppRunnerInvalidPlatformWindows + } + // This extra check is because ARM architectures won't work for App Runner services. + if !r.Platform.IsEmpty() { + if r.Platform.Arch() != ArchAMD64 || r.Platform.Arch() != ArchX86 { + return fmt.Errorf("App Runner services can only build on %s and %s architectures", ArchAMD64, ArchX86) + } + } return nil } @@ -1031,6 +1081,11 @@ type containerDependency struct { isEssential bool } +type validateWindowsOpts struct { + execEnabled bool + efsVolumes map[string]*Volume +} + func validateLoadBalancerTarget(opts validateLoadBalancerTargetOpts) error { if opts.routingRule.TargetContainer == nil && opts.routingRule.TargetContainerCamelCase == nil { return nil @@ -1178,3 +1233,15 @@ func isValidSubSvcName(name string) bool { return len(trailingMatch) == 0 } +func validateWindows(opts validateWindowsOpts) error { + if opts.execEnabled { + return errors.New(`'exec' is not supported 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`) + } + } + return nil +} + diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index fc3d696a561..2b108d46203 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -148,8 +148,21 @@ func TestLoadBalancedWebService_Validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate container dependencies: `, }, + "error if fail to validate windows": { + lbConfig: LoadBalancedWebService{ + Workload: Workload{Name: aws.String("mockName")}, + LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ + ImageConfig: testImageConfig, + TaskConfig: TaskConfig{ + Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, + ExecuteCommand: ExecuteCommand{Enable: aws.Bool(true)}, + }, + }, + }, + wantedErrorMsgPrefix: `validate Windows: `, + }, } - for name, tc := range testCases { + for name, tc := range testCases { t.Run(name, func(t *testing.T) { gotErr := tc.lbConfig.Validate() @@ -275,6 +288,19 @@ func TestBackendService_Validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate container dependencies: `, }, + "error if fail to validate Windows": { + config: BackendService{ + Workload: Workload{Name: aws.String("mockName")}, + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: testImageConfig, + TaskConfig: TaskConfig{ + Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, + ExecuteCommand: ExecuteCommand{Enable: aws.Bool(true)}, + }, + }, + }, + wantedErrorMsgPrefix: `validate Windows: `, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -298,8 +324,8 @@ func TestRequestDrivenWebService_Validate(t *testing.T) { testCases := map[string]struct { config RequestDrivenWebService - wantedErrorMsgPrefix string wantedError error + wantedErrorMsgPrefix string }{ "error if fail to validate image": { config: RequestDrivenWebService{ @@ -317,6 +343,29 @@ func TestRequestDrivenWebService_Validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate "image": `, }, + "error if fail to validate instance": { + config: RequestDrivenWebService{ + Workload: Workload{ + Name: aws.String("mockName"), + }, + RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ + ImageConfig: ImageWithPort{ + Image: Image{ + Build: BuildArgsOrString{BuildString: aws.String("mockBuild")}, + }, + Port: uint16P(80), + }, + InstanceConfig: AppRunnerInstanceConfig{ + CPU: nil, + Memory: nil, + Platform: PlatformArgsOrString{ + PlatformString: (*PlatformString)(aws.String("mockPlatform")), + }, + }, + }, + }, + wantedErrorMsgPrefix: `validate "platform": `, + }, "error if name is not set": { config: RequestDrivenWebService{ RequestDrivenWebServiceConfig: RequestDrivenWebServiceConfig{ @@ -334,7 +383,6 @@ func TestRequestDrivenWebService_Validate(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { gotErr := tc.config.Validate() - if tc.wantedError != nil { require.EqualError(t, gotErr, tc.wantedError.Error()) return @@ -468,6 +516,19 @@ func TestWorkerService_Validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate container dependencies: `, }, + "error if fail to validate windows": { + config: WorkerService{ + Workload: Workload{Name: aws.String("mockName")}, + WorkerServiceConfig: WorkerServiceConfig{ + ImageConfig: testImageConfig, + TaskConfig: TaskConfig{ + Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, + ExecuteCommand: ExecuteCommand{Enable: aws.Bool(true)}, + }, + }, + }, + wantedErrorMsgPrefix: `validate Windows: `, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -612,6 +673,22 @@ func TestScheduledJob_Validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate container dependencies: `, }, + "error if fail to validate windows": { + config: ScheduledJob{ + Workload: Workload{Name: aws.String("mockName")}, + ScheduledJobConfig: ScheduledJobConfig{ + ImageConfig: testImageConfig, + On: JobTriggerConfig{ + Schedule: aws.String("mockSchedule"), + }, + TaskConfig: TaskConfig{ + Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("windows/amd64"))}, + ExecuteCommand: ExecuteCommand{Enable: aws.Bool(true)}, + }, + }, + }, + wantedErrorMsgPrefix: `validate Windows: `, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -859,8 +936,12 @@ func TestPlatformArgsOrString_Validate(t *testing.T) { wanted error }{ "error if platform string is invalid": { - in: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("foobar"))}, - wanted: fmt.Errorf("platform foobar is invalid; valid platforms are: linux/amd64, linux/x86_64, windows/amd64 and windows/x86_64"), + in: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("foobar/amd64"))}, + wanted: fmt.Errorf("platform 'foobar/amd64' is invalid; valid platforms are: linux/amd64, linux/x86_64, windows/amd64 and windows/x86_64"), + }, + "error if only half of platform string is specified": { + in: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("linux"))}, + wanted: fmt.Errorf("platform 'linux' must be in the format [OS]/[Arch]"), }, "error if only osfamily is specified": { in: PlatformArgsOrString{ @@ -885,7 +966,7 @@ func TestPlatformArgsOrString_Validate(t *testing.T) { Arch: aws.String("amd64"), }, }, - wanted: fmt.Errorf("platform pair ('foo', 'amd64') is invalid: fields ('osfamily', 'architecture') must be one of ('linux', 'x86_64'), ('linux', 'amd64'), ('windows_server_2019_core', 'x86_64'), ('windows_server_2019_core', 'amd64'), ('windows_server_2019_full', 'x86_64'), ('windows_server_2019_full', 'amd64')"), + wanted: fmt.Errorf("platform pair ('foo', 'amd64') is invalid: fields ('osfamily', 'architecture') must be one of ('linux', 'x86_64'), ('linux', 'amd64'), ('windows', 'x86_64'), ('windows', 'amd64'), ('windows_server_2019_core', 'x86_64'), ('windows_server_2019_core', 'amd64'), ('windows_server_2019_full', 'x86_64'), ('windows_server_2019_full', 'amd64')"), }, "error if arch is invalid": { in: PlatformArgsOrString{ @@ -894,7 +975,7 @@ func TestPlatformArgsOrString_Validate(t *testing.T) { Arch: aws.String("bar"), }, }, - wanted: fmt.Errorf("platform pair ('linux', 'bar') is invalid: fields ('osfamily', 'architecture') must be one of ('linux', 'x86_64'), ('linux', 'amd64'), ('windows_server_2019_core', 'x86_64'), ('windows_server_2019_core', 'amd64'), ('windows_server_2019_full', 'x86_64'), ('windows_server_2019_full', 'amd64')"), + wanted: fmt.Errorf("platform pair ('linux', 'bar') is invalid: fields ('osfamily', 'architecture') must be one of ('linux', 'x86_64'), ('linux', 'amd64'), ('windows', 'x86_64'), ('windows', 'amd64'), ('windows_server_2019_core', 'x86_64'), ('windows_server_2019_core', 'amd64'), ('windows_server_2019_full', 'x86_64'), ('windows_server_2019_full', 'amd64')"), }, "return nil if platform string valid": { in: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("linux/amd64"))}, @@ -1574,8 +1655,8 @@ func TestPlacement_Validate(t *testing.T) { func TestAppRunnerInstanceConfig_Validate(t *testing.T) { testCases := map[string]struct { config AppRunnerInstanceConfig - wantedErrorPrefix string + wantedError error }{ "error if fail to validate platforms": { config: AppRunnerInstanceConfig{ @@ -1585,6 +1666,47 @@ func TestAppRunnerInstanceConfig_Validate(t *testing.T) { }, wantedErrorPrefix: `validate "platform": `, }, + "error if windows os in PlatformString": { + config: AppRunnerInstanceConfig{ + Platform: PlatformArgsOrString{ + PlatformString: (*PlatformString)(aws.String("windows/amd64")), + }, + }, + wantedError: fmt.Errorf("Windows is not supported for App Runner services"), + }, + "error if windows os in PlatformArgs": { + config: AppRunnerInstanceConfig{ + CPU: nil, + Memory: nil, + Platform: PlatformArgsOrString{ + PlatformArgs: PlatformArgs{ + OSFamily: aws.String("windows"), + Arch: aws.String("amd64"), + }, + }, + }, + wantedError: fmt.Errorf("Windows is not supported for App Runner services"), + }, + // TODO: after ARM architectures are added to the valid list, these will return the error "App Runner services can only build on amd64 and x86_64 architectures" + "error if invalid arch in PlatformString": { + config: AppRunnerInstanceConfig{ + Platform: PlatformArgsOrString{ + PlatformString: (*PlatformString)(aws.String("linux/arm64")), + }, + }, + wantedError: fmt.Errorf("validate \"platform\": platform 'linux/arm64' is invalid; valid platforms are: linux/amd64, linux/x86_64, windows/amd64 and windows/x86_64"), + }, + "error if invalid arch in PlatformArgs": { + config: AppRunnerInstanceConfig{ + Platform: PlatformArgsOrString{ + PlatformArgs: PlatformArgs{ + OSFamily: aws.String("linux"), + Arch: aws.String("arm64"), + }, + }, + }, + wantedError: fmt.Errorf("validate \"platform\": platform pair ('linux', 'arm64') is invalid: fields ('osfamily', 'architecture') must be one of ('linux', 'x86_64'), ('linux', 'amd64'), ('windows', 'x86_64'), ('windows', 'amd64'), ('windows_server_2019_core', 'x86_64'), ('windows_server_2019_core', 'amd64'), ('windows_server_2019_full', 'x86_64'), ('windows_server_2019_full', 'amd64')"), + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -1592,6 +1714,8 @@ func TestAppRunnerInstanceConfig_Validate(t *testing.T) { if tc.wantedErrorPrefix != "" { require.Contains(t, gotErr.Error(), tc.wantedErrorPrefix) + } else if tc.wantedError != nil{ + require.EqualError(t, gotErr, tc.wantedError.Error()) } else { require.NoError(t, gotErr) } @@ -1937,3 +2061,50 @@ func TestValidateContainerDeps(t *testing.T) { }) } } + +func TestValidateWindows(t *testing.T) { + testCases := map[string]struct { + in validateWindowsOpts + wantedError error + }{ + "should return an error if efs specified": { + in: validateWindowsOpts{ + execEnabled: true, + }, + wantedError: fmt.Errorf(`'exec' is not supported when deploying a Windows container`), + }, + "error if efs specified": { + in: validateWindowsOpts{ + efsVolumes: map[string]*Volume{ + "someVolume": &Volume{ + EFS: EFSConfigOrBool{ + Enabled: aws.Bool(true), + }, + MountPointOpts: MountPointOpts{ + ContainerPath: aws.String("mockPath"), + }, + }, + }, + }, + wantedError: errors.New(`'EFS' is not supported when deploying a Windows container`), + }, + "should return nil if neither efs nor exec specified": { + in: validateWindowsOpts{ + execEnabled: false, + }, + wantedError: nil, + + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + err := validateWindows(tc.in) + + if tc.wantedError != nil { + require.EqualError(t, err, tc.wantedError.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/pkg/manifest/worker_svc.go b/internal/pkg/manifest/worker_svc.go index 03252f26b43..9fbb15c50b7 100644 --- a/internal/pkg/manifest/worker_svc.go +++ b/internal/pkg/manifest/worker_svc.go @@ -208,24 +208,6 @@ func (s WorkerService) ApplyEnv(envName string) (WorkloadManifest, error) { return &s, nil } -// windowsCompatibility disallows unsupported when deploying Windows containers on Fargate. -func (s *WorkerService) windowsCompatibility() error { - if !s.IsWindows() { - return nil - } - // Exec is not supported. - if aws.BoolValue(s.ExecuteCommand.Enable) { - return errors.New(`'exec' is not supported when deploying a Windows container`) - } - // EFS is not supported. - for _, volume := range s.Storage.Volumes { - if !volume.EmptyVolume() { - return errors.New(`'EFS' is not supported when deploying a Windows container`) - } - } - return nil -} - // newDefaultWorkerService returns a Worker service with minimal task sizes and a single replica. func newDefaultWorkerService() *WorkerService { return &WorkerService{ diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index a79ea18bfcb..684886d4843 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -66,6 +66,8 @@ var ( validAdvancedPlatforms = []PlatformArgs{ {OSFamily: aws.String(OSLinux), Arch: aws.String(ArchX86)}, {OSFamily: aws.String(OSLinux), Arch: aws.String(ArchAMD64)}, + {OSFamily: aws.String(OSWindows), Arch: aws.String(ArchX86)}, + {OSFamily: aws.String(OSWindows), Arch: aws.String(ArchAMD64)}, {OSFamily: aws.String(OSWindowsServer2019Core), Arch: aws.String(ArchX86)}, {OSFamily: aws.String(OSWindowsServer2019Core), Arch: aws.String(ArchAMD64)}, {OSFamily: aws.String(OSWindowsServer2019Full), Arch: aws.String(ArchX86)}, @@ -502,13 +504,15 @@ type TaskConfig struct { Storage Storage `yaml:"storage"` } -// TaskPlatform returns the platform for the service. -func (t *TaskConfig) TaskPlatform() (*string, error) { - if t.Platform.PlatformString == nil { - return nil, nil +// ContainerPlatform returns the platform for the service. +func (t *TaskConfig) ContainerPlatform() string { + if t.Platform.IsEmpty() { + return "" + } + if t.IsWindows() { + return platformString(OSWindows, t.Platform.Arch()) } - val := string(*t.Platform.PlatformString) - return &val, nil + return platformString(t.Platform.OS(), t.Platform.Arch()) } // IsWindows returns whether or not the service is building with a Windows OS. @@ -576,7 +580,6 @@ func (c *vpcConfig) isEmpty() bool { func UnmarshalWorkload(in []byte) (WorkloadManifest, error) { type manifest interface { WorkloadManifest - windowsCompatibility() error } am := Workload{} if err := yaml.Unmarshal(in, &am); err != nil { @@ -602,9 +605,6 @@ func UnmarshalWorkload(in []byte) (WorkloadManifest, error) { if err := yaml.Unmarshal(in, m); err != nil { return nil, fmt.Errorf("unmarshal manifest for %s: %w", typeVal, err) } - if err := m.windowsCompatibility(); err != nil { - return nil, err - } return m, nil } @@ -687,7 +687,7 @@ func (p *PlatformArgsOrString) UnmarshalYAML(value *yaml.Node) error { // OS returns the operating system family. func (p *PlatformArgsOrString) OS() string { if p := aws.StringValue((*string)(p.PlatformString)); p != "" { - args := strings.Split(p, "/") // There are always at least two elements because of validateShortPlatform. + args := strings.Split(p, "/") return strings.ToLower(args[0]) } return strings.ToLower(aws.StringValue(p.PlatformArgs.OSFamily)) @@ -696,7 +696,7 @@ func (p *PlatformArgsOrString) OS() string { // Arch returns the architecture. func (p *PlatformArgsOrString) Arch() string { if p := aws.StringValue((*string)(p.PlatformString)); p != "" { - args := strings.Split(p, "/") // There are always at least two elements because of validateShortPlatform. + args := strings.Split(p, "/") return strings.ToLower(args[1]) } return strings.ToLower(aws.StringValue(p.PlatformArgs.Arch))