fix(manifest): preserve default entrypoint and command on empty env o…

…verrides (#2490) This fixes the bug where an empty environment `entrypoint`/`command` will replace the default `entrypoint`/`command` in manifest if they are specified as list. This is because an empty list `[]string` isn't considered a zero value, hence `mergo` will use the empty list to replace the original list. This is the same fix as #2442, only it's on entrypoint and command overrides. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
aws · Jun 21, 2021 · ecffb5e · ecffb5e
1 parent b140070
commit ecffb5e
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 24 deletions.
diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go
@@ -105,18 +105,17 @@ func (s *BackendService) Template() (string, error) {
 		desiredCountOnSpot = advancedCount.Spot
 		capacityProviders = advancedCount.Cps
 	}
-
 	storage, err := convertStorageOpts(s.manifest.Name, s.manifest.Storage)
 	if err != nil {
 		return "", fmt.Errorf("convert storage options for service %s: %w", s.name, err)
 	}
-	entrypoint, err := s.manifest.EntryPoint.ToStringSlice()
+	entrypoint, err := convertEntryPoint(s.manifest.EntryPoint)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'entrypoint' to string slice: %w`, err)
+		return "", err
 	}
-	command, err := s.manifest.Command.ToStringSlice()
+	command, err := convertCommand(s.manifest.Command)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'command' to string slice: %w`, err)
+		return "", err
 	}
 	content, err := s.parser.ParseBackendService(template.WorkloadOpts{
 		Variables:           s.manifest.BackendServiceConfig.Variables,

diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go
@@ -169,11 +169,11 @@ Outputs:
 						StartPeriod: &testStartPeriod,
 					},
 				})
-				svc.manifest.EntryPoint = manifest.EntryPointOverride{
+				svc.manifest.EntryPoint = &manifest.EntryPointOverride{
 					String:      nil,
 					StringSlice: []string{"enter", "from"},
 				}
-				svc.manifest.Command = manifest.CommandOverride{
+				svc.manifest.Command = &manifest.CommandOverride{
 					String:      nil,
 					StringSlice: []string{"here"},
 				}

diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go
@@ -135,15 +135,15 @@ func (s *LoadBalancedWebService) Template() (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("convert storage options for service %s: %w", s.name, err)
 	}
-
-	entrypoint, err := s.manifest.EntryPoint.ToStringSlice()
+	entrypoint, err := convertEntryPoint(s.manifest.EntryPoint)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'entrypoint' to string slice: %w`, err)
+		return "", err
 	}
-	command, err := s.manifest.Command.ToStringSlice()
+	command, err := convertCommand(s.manifest.Command)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'command' to string slice: %w`, err)
+		return "", err
 	}
+
 	var aliases []string
 	if s.httpsEnabled {
 		albAlias := aws.StringValue(s.manifest.Alias)

diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
@@ -106,11 +106,11 @@ func TestLoadBalancedWebService_Template(t *testing.T) {
 		Retries: aws.Int(5),
 	}
 	testLBWebServiceManifest.Alias = aws.String("mockAlias")
-	testLBWebServiceManifest.EntryPoint = manifest.EntryPointOverride{
+	testLBWebServiceManifest.EntryPoint = &manifest.EntryPointOverride{
 		String:      nil,
 		StringSlice: []string{"/bin/echo", "hello"},
 	}
-	testLBWebServiceManifest.Command = manifest.CommandOverride{
+	testLBWebServiceManifest.Command = &manifest.CommandOverride{
 		String:      nil,
 		StringSlice: []string{"world"},
 	}

diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job.go
@@ -153,14 +153,15 @@ func (j *ScheduledJob) Template() (string, error) {
 		return "", fmt.Errorf("read env controller lambda: %w", err)
 	}
 
-	entrypoint, err := j.manifest.EntryPoint.ToStringSlice()
+	entrypoint, err := convertEntryPoint(j.manifest.EntryPoint)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'entrypoint' to string slice: %w`, err)
+		return "", err
 	}
-	command, err := j.manifest.Command.ToStringSlice()
+	command, err := convertCommand(j.manifest.Command)
 	if err != nil {
-		return "", fmt.Errorf(`convert 'command' to string slice: %w`, err)
+		return "", err
 	}
+
 	content, err := j.parser.ParseScheduledJob(template.WorkloadOpts{
 		Variables:          j.manifest.Variables,
 		Secrets:            j.manifest.Secrets,

diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job_test.go
@@ -37,11 +37,11 @@ func TestScheduledJob_Template(t *testing.T) {
 		Timeout:  "1h30m",
 		Retries:  3,
 	})
-	testScheduledJobManifest.EntryPoint = manifest.EntryPointOverride{
+	testScheduledJobManifest.EntryPoint = &manifest.EntryPointOverride{
 		String:      nil,
 		StringSlice: []string{"/bin/echo", "hello"},
 	}
-	testScheduledJobManifest.Command = manifest.CommandOverride{
+	testScheduledJobManifest.Command = &manifest.CommandOverride{
 		String:      nil,
 		StringSlice: []string{"world"},
 	}

diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go
@@ -551,3 +551,26 @@ func convertNetworkConfig(network *manifest.NetworkConfig) *template.NetworkOpts
 	}
 	return opts
 }
+
+func convertEntryPoint(entrypoint *manifest.EntryPointOverride) ([]string, error) {
+	if entrypoint == nil {
+		return nil, nil
+	}
+
+	out, err := entrypoint.ToStringSlice()
+	if err != nil {
+		return nil, fmt.Errorf(`convert 'entrypoint' to string slice: %w`, err)
+	}
+	return out, nil
+}
+
+func convertCommand(command *manifest.CommandOverride) ([]string, error) {
+	if command == nil {
+		return nil, nil
+	}
+	out, err := command.ToStringSlice()
+	if err != nil {
+		return nil, fmt.Errorf(`convert 'command' to string slice: %w`, err)
+	}
+	return out, nil
+}
diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go
@@ -543,6 +543,14 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
 							},
 						},
 					},
+					ImageOverride: ImageOverride{
+						Command: &CommandOverride{
+							StringSlice: []string{"command", "default"},
+						},
+						EntryPoint: &EntryPointOverride{
+							StringSlice: []string{"entrypoint", "default"},
+						},
+					},
 				},
 				Environments: map[string]*LoadBalancedWebServiceConfig{
 					"prod-iad": nil,
@@ -561,6 +569,14 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
 							},
 						},
 					},
+					ImageOverride: ImageOverride{
+						Command: &CommandOverride{
+							StringSlice: []string{"command", "default"},
+						},
+						EntryPoint: &EntryPointOverride{
+							StringSlice: []string{"entrypoint", "default"},
+						},
+					},
 				},
 				Environments: map[string]*LoadBalancedWebServiceConfig{
 					"prod-iad": nil,
@@ -984,6 +1000,45 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
 				},
 			},
 		},
+		"with command and entrypoint overridden": {
+			in: &LoadBalancedWebService{
+				Workload: Workload{
+					Name: aws.String("phonetool"),
+					Type: aws.String(LoadBalancedWebServiceType),
+				},
+				LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{
+					ImageOverride: ImageOverride{
+						Command: &CommandOverride{
+							StringSlice: []string{"command", "default"},
+						},
+					},
+				},
+				Environments: map[string]*LoadBalancedWebServiceConfig{
+					"prod-iad": {
+						ImageOverride: ImageOverride{
+							Command: &CommandOverride{
+								StringSlice: []string{"command", "prod"},
+							},
+						},
+					},
+				},
+			},
+			envToApply: "prod-iad",
+
+			wanted: &LoadBalancedWebService{
+				Workload: Workload{
+					Name: aws.String("phonetool"),
+					Type: aws.String(LoadBalancedWebServiceType),
+				},
+				LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{
+					ImageOverride: ImageOverride{
+						Command: &CommandOverride{
+							StringSlice: []string{"command", "prod"},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for name, tc := range testCases {

diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go
@@ -211,8 +211,8 @@ func (i *Image) cacheFrom() []string {
 
 // ImageOverride holds fields that override Dockerfile image defaults.
 type ImageOverride struct {
-	EntryPoint EntryPointOverride `yaml:"entrypoint"`
-	Command    CommandOverride    `yaml:"command"`
+	EntryPoint *EntryPointOverride `yaml:"entrypoint"`
+	Command    *CommandOverride    `yaml:"command"`
 }
 
 // EntryPointOverride is a custom type which supports unmarshaling "entrypoint" yaml which

diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go
@@ -47,7 +47,7 @@ func TestEntryPointOverride_UnmarshalYAML(t *testing.T) {
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
 			e := ImageOverride{
-				EntryPoint: EntryPointOverride{
+				EntryPoint: &EntryPointOverride{
 					String: aws.String("wrong"),
 				},
 			}
@@ -138,7 +138,7 @@ func TestCommandOverride_UnmarshalYAML(t *testing.T) {
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
 			e := ImageOverride{
-				Command: CommandOverride{
+				Command: &CommandOverride{
 					String: aws.String("wrong"),
 				},
 			}