chore(manifest): upgrade UnmarshalYAML to v3 and add for Image (#2861)

<!-- Provide summary of changes -->
This PR
- upgrade `UnmarshalYAML` to yaml v3 because of go-yaml/yaml#125 (comment)
- add `UnmarshalYAML` for `Image` to validate mutual exclusive fields before applying env override.
- Revert `Image` struct [embedding](https://golang.org/doc/effective_go#embedding) in `ImageWithHealthcheck` and `ImageWithPort`, as `UnmarshalYAML` would be wrongly taken as the method of its parent, causing the other fields in its parent struct (e.g., `Port` and `HealthCheck`) to be set `nil` unexpectedly. go-yaml/yaml#263 (comment)
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

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/deploy/cloudformation/stack/backend_svc.go

@@ -52,7 +52,7 @@ func NewBackendService(mft *manifest.BackendService, env, app string, rc Runtime
 				env:    env,
 				app:    app,
 				rc:     rc,
-				image:  mft.ImageConfig,
+				image:  mft.ImageConfig.Image,
 				parser: parser,
 				addons: addons,
 			},
@@ -136,15 +136,15 @@ func (s *BackendService) Template() (string, error) {
 		WorkloadType:             manifest.BackendServiceType,
 		HealthCheck:              convertContainerHealthCheck(s.manifest.BackendServiceConfig.ImageConfig.HealthCheck),
 		LogConfig:                convertLogging(s.manifest.Logging),
-		DockerLabels:             s.manifest.ImageConfig.DockerLabels,
+		DockerLabels:             s.manifest.ImageConfig.Image.DockerLabels,
 		DesiredCountLambda:       desiredCountLambda.String(),
 		EnvControllerLambda:      envControllerLambda.String(),
 		Storage:                  storage,
 		Network:                  convertNetworkConfig(s.manifest.Network),
 		EntryPoint:               entrypoint,
 		Command:                  command,
 		DependsOn:                dependencies,
-		CredentialsParameter:     aws.StringValue(s.manifest.ImageConfig.Credentials),
+		CredentialsParameter:     aws.StringValue(s.manifest.ImageConfig.Image.Credentials),
 		ServiceDiscoveryEndpoint: s.rc.ServiceDiscoveryEndpoint,
 		Publish:                  publishers,
 	})

internal/pkg/deploy/cloudformation/stack/lb_web_svc.go

@@ -60,7 +60,7 @@ func NewLoadBalancedWebService(mft *manifest.LoadBalancedWebService, env, app st
 				env:    env,
 				app:    app,
 				rc:     rc,
-				image:  mft.ImageConfig,
+				image:  mft.ImageConfig.Image,
 				parser: parser,
 				addons: addons,
 			},
@@ -174,7 +174,7 @@ func (s *LoadBalancedWebService) Template() (string, error) {
 		NestedStack:              outputs,
 		Sidecars:                 sidecars,
 		LogConfig:                convertLogging(s.manifest.Logging),
-		DockerLabels:             s.manifest.ImageConfig.DockerLabels,
+		DockerLabels:             s.manifest.ImageConfig.Image.DockerLabels,
 		Autoscaling:              autoscaling,
 		CapacityProviders:        capacityProviders,
 		DesiredCountOnSpot:       desiredCountOnSpot,
@@ -192,7 +192,7 @@ func (s *LoadBalancedWebService) Template() (string, error) {
 		EntryPoint:               entrypoint,
 		Command:                  command,
 		DependsOn:                dependencies,
-		CredentialsParameter:     aws.StringValue(s.manifest.ImageConfig.Credentials),
+		CredentialsParameter:     aws.StringValue(s.manifest.ImageConfig.Image.Credentials),
 		ServiceDiscoveryEndpoint: s.rc.ServiceDiscoveryEndpoint,
 		Publish:                  publishers,
 	})

internal/pkg/deploy/cloudformation/stack/rd_web_svc.go

@@ -78,7 +78,7 @@ func NewRequestDrivenWebService(mft *manifest.RequestDrivenWebService, env strin
 				env:    env,
 				app:    app.Name,
 				rc:     rc,
-				image:  mft.ImageConfig,
+				image:  mft.ImageConfig.Image,
 				addons: addons,
 				parser: parser,
 			},

internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go

@@ -87,7 +87,7 @@ func TestRequestDrivenWebService_NewRequestDrivenWebService(t *testing.T) {
 						env:   testEnvName,
 						app:   testAppName,
 						rc:    RuntimeConfig{},
-						image: testRDWebServiceManifest.ImageConfig,
+						image: testRDWebServiceManifest.ImageConfig.Image,
 					},
 					instanceConfig: testRDWebServiceManifest.InstanceConfig,
 					imageConfig:    testRDWebServiceManifest.ImageConfig,
@@ -165,7 +165,7 @@ func TestRequestDrivenWebService_NewRequestDrivenWebServiceWithAlias(t *testing.
 						env:   testEnvName,
 						app:   testAppName,
 						rc:    RuntimeConfig{},
-						image: testRDWebServiceManifest.ImageConfig,
+						image: testRDWebServiceManifest.ImageConfig.Image,
 					},
 					instanceConfig: testRDWebServiceManifest.InstanceConfig,
 					imageConfig:    testRDWebServiceManifest.ImageConfig,
@@ -459,7 +459,7 @@ func TestRequestDrivenWebService_Parameters(t *testing.T) {
 						name:  aws.StringValue(testRDWebServiceManifest.Name),
 						env:   testEnvName,
 						app:   testAppName,
-						image: tc.imageConfig,
+						image: tc.imageConfig.Image,
 					},
 					instanceConfig: tc.instanceConfig,
 					imageConfig:    tc.imageConfig,

internal/pkg/deploy/cloudformation/stack/scheduled_job.go

@@ -103,7 +103,7 @@ func NewScheduledJob(mft *manifest.ScheduledJob, env, app string, rc RuntimeConf
 				env:    env,
 				app:    app,
 				rc:     rc,
-				image:  mft.ImageConfig,
+				image:  mft.ImageConfig.Image,
 				parser: parser,
 				addons: addons,
 			},
@@ -178,13 +178,13 @@ func (j *ScheduledJob) Template() (string, error) {
 		StateMachine:             stateMachine,
 		HealthCheck:              convertContainerHealthCheck(j.manifest.ImageConfig.HealthCheck),
 		LogConfig:                convertLogging(j.manifest.Logging),
-		DockerLabels:             j.manifest.ImageConfig.DockerLabels,
+		DockerLabels:             j.manifest.ImageConfig.Image.DockerLabels,
 		Storage:                  storage,
 		Network:                  convertNetworkConfig(j.manifest.Network),
 		EntryPoint:               entrypoint,
 		Command:                  command,
 		DependsOn:                dependencies,
-		CredentialsParameter:     aws.StringValue(j.manifest.ImageConfig.Credentials),
+		CredentialsParameter:     aws.StringValue(j.manifest.ImageConfig.Image.Credentials),
 		ServiceDiscoveryEndpoint: j.rc.ServiceDiscoveryEndpoint,
 		Publish:                  publishers,
 

internal/pkg/deploy/cloudformation/stack/worker_svc.go

@@ -47,7 +47,7 @@ func NewWorkerService(mft *manifest.WorkerService, env, app string, rc RuntimeCo
 				env:    env,
 				app:    app,
 				rc:     rc,
-				image:  mft.ImageConfig,
+				image:  mft.ImageConfig.Image,
 				parser: parser,
 				addons: addons,
 			},
@@ -135,7 +135,7 @@ func (s *WorkerService) Template() (string, error) {
 		WorkloadType:                   manifest.WorkerServiceType,
 		HealthCheck:                    convertContainerHealthCheck(s.manifest.WorkerServiceConfig.ImageConfig.HealthCheck),
 		LogConfig:                      convertLogging(s.manifest.Logging),
-		DockerLabels:                   s.manifest.ImageConfig.DockerLabels,
+		DockerLabels:                   s.manifest.ImageConfig.Image.DockerLabels,
 		DesiredCountLambda:             desiredCountLambda.String(),
 		EnvControllerLambda:            envControllerLambda.String(),
 		BacklogPerTaskCalculatorLambda: backlogPerTaskLambda.String(),
@@ -144,7 +144,7 @@ func (s *WorkerService) Template() (string, error) {
 		EntryPoint:                     entrypoint,
 		Command:                        command,
 		DependsOn:                      dependencies,
-		CredentialsParameter:           aws.StringValue(s.manifest.ImageConfig.Credentials),
+		CredentialsParameter:           aws.StringValue(s.manifest.ImageConfig.Image.Credentials),
 		ServiceDiscoveryEndpoint:       s.rc.ServiceDiscoveryEndpoint,
 		Subscribe:                      subscribe,
 	})

internal/pkg/initialize/workload_test.go

@@ -85,7 +85,7 @@ func TestWorkloadInitializer_Job(t *testing.T) {
 			mockWriter: func(m *mocks.MockWorkspace) {
 				m.EXPECT().WriteJobManifest(gomock.Any(), "resizer").Do(func(m *manifest.ScheduledJob, _ string) {
 					require.Equal(t, *m.Workload.Type, manifest.ScheduledJobType)
-					require.Equal(t, *m.ImageConfig.Location, "mockImage")
+					require.Equal(t, *m.ImageConfig.Image.Location, "mockImage")
 				}).Return("/resizer/manifest.yml", nil)
 			},
 			mockstore: func(m *mocks.MockStore) {
@@ -363,7 +363,7 @@ func TestAppInitOpts_createLoadBalancedAppManifest(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, tc.inSvcName, aws.StringValue(manifest.Workload.Name))
 				require.Equal(t, tc.inSvcPort, aws.Uint16Value(manifest.ImageConfig.Port))
-				require.Contains(t, tc.inDockerfilePath, aws.StringValue(manifest.ImageConfig.Build.BuildArgs.Dockerfile))
+				require.Contains(t, tc.inDockerfilePath, aws.StringValue(manifest.ImageConfig.Image.Build.BuildArgs.Dockerfile))
 				require.Equal(t, tc.wantedPath, aws.StringValue(manifest.Path))
 			} else {
 				require.EqualError(t, err, tc.wantedErr.Error())
@@ -418,10 +418,10 @@ func TestAppInitOpts_createRequestDrivenWebServiceManifest(t *testing.T) {
 			require.Equal(t, tc.inSvcName, *manifest.Name)
 			require.Equal(t, tc.inSvcPort, *manifest.ImageConfig.Port)
 			if tc.inImage != "" {
-				require.Equal(t, tc.inImage, *manifest.ImageConfig.Location)
+				require.Equal(t, tc.inImage, *manifest.ImageConfig.Image.Location)
 			}
 			if tc.inDockerfilePath != "" {
-				require.Equal(t, tc.inDockerfilePath, *manifest.ImageConfig.Build.BuildArgs.Dockerfile)
+				require.Equal(t, tc.inDockerfilePath, *manifest.ImageConfig.Image.Build.BuildArgs.Dockerfile)
 			}
 		})
 	}
@@ -588,7 +588,7 @@ func TestWorkloadInitializer_Service(t *testing.T) {
 				m.EXPECT().WriteServiceManifest(gomock.Any(), "backend").
 					Do(func(m *manifest.BackendService, _ string) {
 						require.Equal(t, *m.Workload.Type, manifest.BackendServiceType)
-						require.Equal(t, *m.ImageConfig.Location, "mockImage")
+						require.Equal(t, *m.ImageConfig.Image.Location, "mockImage")
 						require.Empty(t, m.ImageConfig.HealthCheck)
 					}).Return("/backend/manifest.yml", nil)
 			},

internal/pkg/manifest/applyenv_test.go

@@ -475,28 +475,28 @@ func TestApplyEnv_String(t *testing.T) {
 	}{
 		"string overridden": {
 			inSvc: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("cairo")
-				svc.Environments["test"].ImageConfig.Location = aws.String("nerac")
+				svc.ImageConfig.Image.Location = aws.String("cairo")
+				svc.Environments["test"].ImageConfig.Image.Location = aws.String("nerac")
 			},
 			wanted: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("nerac")
+				svc.ImageConfig.Image.Location = aws.String("nerac")
 			},
 		},
 		"string overridden by zero value": {
 			inSvc: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("cairo")
-				svc.Environments["test"].ImageConfig.Location = aws.String("")
+				svc.ImageConfig.Image.Location = aws.String("cairo")
+				svc.Environments["test"].ImageConfig.Image.Location = aws.String("")
 			},
 			wanted: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("")
+				svc.ImageConfig.Image.Location = aws.String("")
 			},
 		},
 		"string not overridden": {
 			inSvc: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("cairo")
+				svc.ImageConfig.Image.Location = aws.String("cairo")
 			},
 			wanted: func(svc *LoadBalancedWebService) {
-				svc.ImageConfig.Location = aws.String("cairo")
+				svc.ImageConfig.Image.Location = aws.String("cairo")
 			},
 		},
 	}

internal/pkg/manifest/backend_svc.go

@@ -50,7 +50,7 @@ func NewBackendService(props BackendServiceProps) *BackendService {
 	// Apply overrides.
 	svc.Name = stringP(props.Name)
 	svc.BackendServiceConfig.ImageConfig.Image.Location = stringP(props.Image)
-	svc.BackendServiceConfig.ImageConfig.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
+	svc.BackendServiceConfig.ImageConfig.Image.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
 	svc.BackendServiceConfig.ImageConfig.Port = uint16P(props.Port)
 	svc.BackendServiceConfig.ImageConfig.HealthCheck = props.HealthCheck
 	svc.BackendServiceConfig.Platform = props.Platform
@@ -93,7 +93,7 @@ func (s *BackendService) BuildRequired() (bool, error) {
 
 // BuildArgs returns a docker.BuildArguments object for the service given a workspace root directory.
 func (s *BackendService) BuildArgs(wsRoot string) *DockerBuildArgs {
-	return s.ImageConfig.BuildConfig(wsRoot)
+	return s.ImageConfig.Image.BuildConfig(wsRoot)
 }
 
 // ApplyEnv returns the service manifest with environment overrides.

internal/pkg/manifest/job.go

@@ -74,8 +74,8 @@ func NewScheduledJob(props *ScheduledJobProps) *ScheduledJob {
 	job := newDefaultScheduledJob()
 	// Apply overrides.
 	job.Name = stringP(props.Name)
-	job.ImageConfig.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
-	job.ImageConfig.Location = stringP(props.Image)
+	job.ImageConfig.Image.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
+	job.ImageConfig.Image.Location = stringP(props.Image)
 	job.ImageConfig.HealthCheck = props.HealthCheck
 	job.On.Schedule = stringP(props.Schedule)
 	if props.Retries != 0 {
@@ -124,7 +124,7 @@ func (j *ScheduledJob) Publish() []Topic {
 
 // BuildArgs returns a docker.BuildArguments object for the job given a workspace root.
 func (j *ScheduledJob) BuildArgs(wsRoot string) *DockerBuildArgs {
-	return j.ImageConfig.BuildConfig(wsRoot)
+	return j.ImageConfig.Image.BuildConfig(wsRoot)
 }
 
 // BuildRequired returns if the service requires building from the local Dockerfile.

internal/pkg/manifest/lb_web_svc.go

@@ -10,6 +10,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/copilot-cli/internal/pkg/template"
 	"github.com/imdario/mergo"
+	"gopkg.in/yaml.v3"
 )
 
 const (
@@ -72,7 +73,7 @@ func NewLoadBalancedWebService(props *LoadBalancedWebServiceProps) *LoadBalanced
 	// Apply overrides.
 	svc.Name = stringP(props.Name)
 	svc.LoadBalancedWebServiceConfig.ImageConfig.Image.Location = stringP(props.Image)
-	svc.LoadBalancedWebServiceConfig.ImageConfig.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
+	svc.LoadBalancedWebServiceConfig.ImageConfig.Image.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
 	svc.LoadBalancedWebServiceConfig.ImageConfig.Port = aws.Uint16(props.Port)
 	svc.LoadBalancedWebServiceConfig.ImageConfig.HealthCheck = props.HealthCheck
 	svc.LoadBalancedWebServiceConfig.Platform = props.Platform
@@ -144,7 +145,7 @@ func (s *LoadBalancedWebService) BuildRequired() (bool, error) {
 
 // BuildArgs returns a docker.BuildArguments object given a ws root directory.
 func (s *LoadBalancedWebService) BuildArgs(wsRoot string) *DockerBuildArgs {
-	return s.ImageConfig.BuildConfig(wsRoot)
+	return s.ImageConfig.Image.BuildConfig(wsRoot)
 }
 
 // ApplyEnv returns the service manifest with environment overrides.
@@ -201,9 +202,9 @@ func (e *Alias) IsEmpty() bool {
 
 // UnmarshalYAML overrides the default YAML unmarshaling logic for the Alias
 // struct, allowing it to perform more complex unmarshaling behavior.
-// This method implements the yaml.Unmarshaler (v2) interface.
-func (e *Alias) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	if err := unmarshalYAMLToStringSliceOrString((*stringSliceOrString)(e), unmarshal); err != nil {
+// This method implements the yaml.Unmarshaler (v3) interface.
+func (e *Alias) UnmarshalYAML(value *yaml.Node) error {
+	if err := unmarshalYAMLToStringSliceOrString((*stringSliceOrString)(e), value); err != nil {
 		return errUnmarshalEntryPoint
 	}
 	return nil

internal/pkg/manifest/rd_web_svc.go

@@ -57,7 +57,7 @@ func NewRequestDrivenWebService(props *RequestDrivenWebServiceProps) *RequestDri
 	svc := newDefaultRequestDrivenWebService()
 	svc.Name = aws.String(props.Name)
 	svc.RequestDrivenWebServiceConfig.ImageConfig.Image.Location = stringP(props.Image)
-	svc.RequestDrivenWebServiceConfig.ImageConfig.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
+	svc.RequestDrivenWebServiceConfig.ImageConfig.Image.Build.BuildArgs.Dockerfile = stringP(props.Dockerfile)
 	svc.RequestDrivenWebServiceConfig.ImageConfig.Port = aws.Uint16(props.Port)
 	svc.RequestDrivenWebServiceConfig.InstanceConfig.Platform = props.Platform
 	svc.parser = template.New()
@@ -101,7 +101,7 @@ func (s *RequestDrivenWebService) TaskPlatform() (*string, error) {
 
 // BuildArgs returns a docker.BuildArguments object given a ws root directory.
 func (s *RequestDrivenWebService) BuildArgs(wsRoot string) *DockerBuildArgs {
-	return s.ImageConfig.BuildConfig(wsRoot)
+	return s.ImageConfig.Image.BuildConfig(wsRoot)
 }
 
 // ApplyEnv returns the service manifest with environment overrides.

internal/pkg/manifest/storage.go

@@ -81,10 +81,10 @@ func (e *EFSConfigOrBool) IsEmpty() bool {
 	return e.Advanced.IsEmpty() && e.Enabled == nil
 }
 
-// UnmarshalYAML implements the yaml(v2) interface. It allows EFS to be specified as a
+// UnmarshalYAML implements the yaml(v3) interface. It allows EFS to be specified as a
 // string or a struct alternately.
-func (e *EFSConfigOrBool) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	if err := unmarshal(&e.Advanced); err != nil {
+func (e *EFSConfigOrBool) UnmarshalYAML(value *yaml.Node) error {
+	if err := value.Decode(&e.Advanced); err != nil {
 		switch err.(type) {
 		case *yaml.TypeError:
 			break
@@ -102,7 +102,7 @@ func (e *EFSConfigOrBool) UnmarshalYAML(unmarshal func(interface{}) error) error
 		return nil
 	}
 
-	if err := unmarshal(&e.Enabled); err != nil {
+	if err := value.Decode(&e.Enabled); err != nil {
 		return errUnmarshalEFSOpts
 	}
 	return nil