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 #2640. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
aws · Oct 27, 2021 · 73f3325 · 73f3325
1 parent 4a8f9cd
commit 73f3325
Show file tree

Hide file tree

Showing 13 changed files with 309 additions and 277 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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{