revert implicit deletion of disappeared optional slice

Author: keroxp

rollout/executor_test.go

@@ -99,22 +99,22 @@ func TestExecutor_Rollout(t *testing.T) {
 			taskMock.EXPECT().Wait(gomock.Any()).Return(nil),
 			ecsMock.EXPECT().UpdateService(gomock.Any(), &ecs.UpdateServiceInput{
 				Cluster:                       &envars.Cluster,
-				CapacityProviderStrategy:      []ecstypes.CapacityProviderStrategyItem{},
+				CapacityProviderStrategy:      envars.ServiceDefinitionInput.CapacityProviderStrategy,
 				Service:                       &envars.Service,
 				TaskDefinition:                td.TaskDefinitionArn,
 				ServiceConnectConfiguration:   envars.ServiceDefinitionInput.ServiceConnectConfiguration,
 				LoadBalancers:                 envars.ServiceDefinitionInput.LoadBalancers,
 				NetworkConfiguration:          envars.ServiceDefinitionInput.NetworkConfiguration,
 				PlatformVersion:               envars.ServiceDefinitionInput.PlatformVersion,
-				VolumeConfigurations:          []ecstypes.ServiceVolumeConfiguration{},
-				ServiceRegistries:             []ecstypes.ServiceRegistry{},
+				VolumeConfigurations:          envars.ServiceDefinitionInput.VolumeConfigurations,
+				ServiceRegistries:             envars.ServiceDefinitionInput.ServiceRegistries,
 				DeploymentConfiguration:       envars.ServiceDefinitionInput.DeploymentConfiguration,
 				HealthCheckGracePeriodSeconds: envars.ServiceDefinitionInput.HealthCheckGracePeriodSeconds,
 				EnableECSManagedTags:          &envars.ServiceDefinitionInput.EnableECSManagedTags,
-				PlacementConstraints:          []ecstypes.PlacementConstraint{},
-				PlacementStrategy:             []ecstypes.PlacementStrategy{},
+				PlacementConstraints:          envars.ServiceDefinitionInput.PlacementConstraints,
+				PlacementStrategy:             envars.ServiceDefinitionInput.PlacementStrategy,
 				PropagateTags:                 envars.ServiceDefinitionInput.PropagateTags,
-				VpcLatticeConfigurations:      []ecstypes.VpcLatticeConfiguration{},
+				VpcLatticeConfigurations:      envars.ServiceDefinitionInput.VpcLatticeConfigurations,
 			}).
 				DoAndReturn(mocker.Ecs.UpdateService),
 			ecsMock.EXPECT().DescribeServices(gomock.Any(), gomock.Any(), gomock.Any()).

rollout/service_sync.go

@@ -5,27 +5,19 @@ import (
 )
 
 func applyServiceDefinitionToUpdateInput(updateInput *ecs.UpdateServiceInput, serviceInput *ecs.CreateServiceInput) {
-	// UpdateService treats omitted slice fields as "keep current", so explicitly
-	// send empty slices for settings removed from service.json.
-	updateInput.CapacityProviderStrategy = emptySliceIfNil(serviceInput.CapacityProviderStrategy)
-	updateInput.LoadBalancers = emptySliceIfNil(serviceInput.LoadBalancers)
+	// Preserve nil and empty slice distinctions from the service definition.
+	updateInput.CapacityProviderStrategy = serviceInput.CapacityProviderStrategy
+	updateInput.LoadBalancers = serviceInput.LoadBalancers
 	updateInput.NetworkConfiguration = serviceInput.NetworkConfiguration
 	updateInput.ServiceConnectConfiguration = serviceInput.ServiceConnectConfiguration
-	updateInput.ServiceRegistries = emptySliceIfNil(serviceInput.ServiceRegistries)
+	updateInput.ServiceRegistries = serviceInput.ServiceRegistries
 	updateInput.PlatformVersion = serviceInput.PlatformVersion
-	updateInput.VolumeConfigurations = emptySliceIfNil(serviceInput.VolumeConfigurations)
+	updateInput.VolumeConfigurations = serviceInput.VolumeConfigurations
 	updateInput.DeploymentConfiguration = serviceInput.DeploymentConfiguration
 	updateInput.HealthCheckGracePeriodSeconds = serviceInput.HealthCheckGracePeriodSeconds
 	updateInput.EnableECSManagedTags = &serviceInput.EnableECSManagedTags
-	updateInput.PlacementConstraints = emptySliceIfNil(serviceInput.PlacementConstraints)
-	updateInput.PlacementStrategy = emptySliceIfNil(serviceInput.PlacementStrategy)
+	updateInput.PlacementConstraints = serviceInput.PlacementConstraints
+	updateInput.PlacementStrategy = serviceInput.PlacementStrategy
 	updateInput.PropagateTags = serviceInput.PropagateTags
-	updateInput.VpcLatticeConfigurations = emptySliceIfNil(serviceInput.VpcLatticeConfigurations)
-}
-
-func emptySliceIfNil[T any](s []T) []T {
-	if s != nil {
-		return s
-	}
-	return []T{}
+	updateInput.VpcLatticeConfigurations = serviceInput.VpcLatticeConfigurations
 }

rollout/service_sync_test.go

@@ -9,21 +9,45 @@ import (
 )
 
 func TestApplyServiceDefinitionToUpdateInput(t *testing.T) {
-	updateInput := &ecs.UpdateServiceInput{}
-	applyServiceDefinitionToUpdateInput(updateInput, &ecs.CreateServiceInput{})
+	t.Run("preserves nil slice fields", func(t *testing.T) {
+		updateInput := &ecs.UpdateServiceInput{}
+		applyServiceDefinitionToUpdateInput(updateInput, &ecs.CreateServiceInput{})
 
-	assert.Equal(t, []ecstypes.CapacityProviderStrategyItem{}, updateInput.CapacityProviderStrategy)
-	assert.Equal(t, []ecstypes.LoadBalancer{}, updateInput.LoadBalancers)
-	assert.Equal(t, []ecstypes.ServiceRegistry{}, updateInput.ServiceRegistries)
-	assert.Equal(t, []ecstypes.ServiceVolumeConfiguration{}, updateInput.VolumeConfigurations)
-	assert.Equal(t, []ecstypes.PlacementConstraint{}, updateInput.PlacementConstraints)
-	assert.Equal(t, []ecstypes.PlacementStrategy{}, updateInput.PlacementStrategy)
-	assert.Equal(t, []ecstypes.VpcLatticeConfiguration{}, updateInput.VpcLatticeConfigurations)
+		assert.Nil(t, updateInput.CapacityProviderStrategy)
+		assert.Nil(t, updateInput.LoadBalancers)
+		assert.Nil(t, updateInput.ServiceRegistries)
+		assert.Nil(t, updateInput.VolumeConfigurations)
+		assert.Nil(t, updateInput.PlacementConstraints)
+		assert.Nil(t, updateInput.PlacementStrategy)
+		assert.Nil(t, updateInput.VpcLatticeConfigurations)
 
-	assert.Nil(t, updateInput.NetworkConfiguration)
-	assert.Nil(t, updateInput.ServiceConnectConfiguration)
-	assert.Nil(t, updateInput.PlatformVersion)
-	assert.Nil(t, updateInput.DeploymentConfiguration)
-	assert.Nil(t, updateInput.HealthCheckGracePeriodSeconds)
-	assert.Equal(t, ecstypes.PropagateTags(""), updateInput.PropagateTags)
+		assert.Nil(t, updateInput.NetworkConfiguration)
+		assert.Nil(t, updateInput.ServiceConnectConfiguration)
+		assert.Nil(t, updateInput.PlatformVersion)
+		assert.Nil(t, updateInput.DeploymentConfiguration)
+		assert.Nil(t, updateInput.HealthCheckGracePeriodSeconds)
+		assert.Equal(t, ecstypes.PropagateTags(""), updateInput.PropagateTags)
+	})
+
+	t.Run("preserves explicit empty slice fields", func(t *testing.T) {
+		serviceInput := &ecs.CreateServiceInput{
+			CapacityProviderStrategy: []ecstypes.CapacityProviderStrategyItem{},
+			LoadBalancers:            []ecstypes.LoadBalancer{},
+			ServiceRegistries:        []ecstypes.ServiceRegistry{},
+			VolumeConfigurations:     []ecstypes.ServiceVolumeConfiguration{},
+			PlacementConstraints:     []ecstypes.PlacementConstraint{},
+			PlacementStrategy:        []ecstypes.PlacementStrategy{},
+			VpcLatticeConfigurations: []ecstypes.VpcLatticeConfiguration{},
+		}
+		updateInput := &ecs.UpdateServiceInput{}
+		applyServiceDefinitionToUpdateInput(updateInput, serviceInput)
+
+		assert.Equal(t, serviceInput.CapacityProviderStrategy, updateInput.CapacityProviderStrategy)
+		assert.Equal(t, serviceInput.LoadBalancers, updateInput.LoadBalancers)
+		assert.Equal(t, serviceInput.ServiceRegistries, updateInput.ServiceRegistries)
+		assert.Equal(t, serviceInput.VolumeConfigurations, updateInput.VolumeConfigurations)
+		assert.Equal(t, serviceInput.PlacementConstraints, updateInput.PlacementConstraints)
+		assert.Equal(t, serviceInput.PlacementStrategy, updateInput.PlacementStrategy)
+		assert.Equal(t, serviceInput.VpcLatticeConfigurations, updateInput.VpcLatticeConfigurations)
+	})
 }

rollout_test.go

@@ -108,7 +108,7 @@ func integrationTest(t *testing.T, env *env.Envars, lbcount int, input *types.Ro
 	}
 }
 
-func TestCage_RollOut_UpdateServiceRemovesOptionalSliceSettings(t *testing.T) {
+func TestCage_RollOut_UpdateServicePreservesNilOptionalSliceSettings(t *testing.T) {
 	ctx := context.TODO()
 	envars := test.DefaultEnvars()
 	mocker := test.NewMockContext()
@@ -163,16 +163,11 @@ func TestCage_RollOut_UpdateServiceRemovesOptionalSliceSettings(t *testing.T) {
 
 	updatedService, _ := mocker.GetEcsService(envars.Service)
 	assert.True(t, result.ServiceUpdated)
-	assert.NotNil(t, updatedService.CapacityProviderStrategy)
-	assert.Len(t, updatedService.CapacityProviderStrategy, 0)
-	assert.NotNil(t, updatedService.LoadBalancers)
-	assert.Len(t, updatedService.LoadBalancers, 0)
-	assert.NotNil(t, updatedService.ServiceRegistries)
-	assert.Len(t, updatedService.ServiceRegistries, 0)
-	assert.NotNil(t, updatedService.PlacementConstraints)
-	assert.Len(t, updatedService.PlacementConstraints, 0)
-	assert.NotNil(t, updatedService.PlacementStrategy)
-	assert.Len(t, updatedService.PlacementStrategy, 0)
+	assert.Nil(t, updatedService.CapacityProviderStrategy)
+	assert.Nil(t, updatedService.LoadBalancers)
+	assert.Nil(t, updatedService.ServiceRegistries)
+	assert.Nil(t, updatedService.PlacementConstraints)
+	assert.Nil(t, updatedService.PlacementStrategy)
 }
 
 func TestCage_Rollout_Failure(t *testing.T) {