diff --git a/.changelog/38870.txt b/.changelog/38870.txt new file mode 100644 index 00000000000..0d2d1b938fb --- /dev/null +++ b/.changelog/38870.txt @@ -0,0 +1,2 @@ +```release-note:bug +resource/aws_ecs_task_definition: Remove `null`s from `container_definition` array fields \ No newline at end of file diff --git a/internal/service/ecs/container_definitions.go b/internal/service/ecs/container_definitions.go index 1dbcbd2bc5d..7c71b88841d 100644 --- a/internal/service/ecs/container_definitions.go +++ b/internal/service/ecs/container_definitions.go @@ -13,6 +13,7 @@ import ( awstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types" smithyjson "github.com/aws/smithy-go/encoding/json" tfjson "github.com/hashicorp/terraform-provider-aws/internal/json" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" itypes "github.com/hashicorp/terraform-provider-aws/internal/types" ) @@ -50,6 +51,9 @@ func (cd containerDefinitions) reduce(isAWSVPC bool) { cd.orderEnvironmentVariables() cd.orderSecrets() + // Compact any sparse lists. + cd.compactArrays() + for i, def := range cd { // Deal with special fields which have defaults. if def.Essential == nil { @@ -149,6 +153,33 @@ func (cd containerDefinitions) orderContainers() { }) } +// compactArrays removes any zero values from the object arrays in the container definitions. +func (cd containerDefinitions) compactArrays() { + for i, def := range cd { + cd[i].DependsOn = compactArray(def.DependsOn) + cd[i].Environment = compactArray(def.Environment) + cd[i].EnvironmentFiles = compactArray(def.EnvironmentFiles) + cd[i].ExtraHosts = compactArray(def.ExtraHosts) + cd[i].MountPoints = compactArray(def.MountPoints) + cd[i].PortMappings = compactArray(def.PortMappings) + cd[i].ResourceRequirements = compactArray(def.ResourceRequirements) + cd[i].Secrets = compactArray(def.Secrets) + cd[i].SystemControls = compactArray(def.SystemControls) + cd[i].Ulimits = compactArray(def.Ulimits) + cd[i].VolumesFrom = compactArray(def.VolumesFrom) + } +} + +func compactArray[S ~[]E, E any](s S) S { + if len(s) == 0 { + return s + } + + return tfslices.Filter(s, func(e E) bool { + return !itypes.IsZero(&e) + }) +} + // Dirty hack to avoid any backwards compatibility issues with the AWS SDK for Go v2 migration. // Reach down into the SDK and use the same serialization function that the SDK uses. // @@ -179,5 +210,7 @@ func expandContainerDefinitions(tfString string) ([]awstypes.ContainerDefinition } } + containerDefinitions(apiObjects).compactArrays() + return apiObjects, nil } diff --git a/internal/service/ecs/container_definitions_test.go b/internal/service/ecs/container_definitions_test.go index 2e75d80a1c6..abd43d8de9c 100644 --- a/internal/service/ecs/container_definitions_test.go +++ b/internal/service/ecs/container_definitions_test.go @@ -576,3 +576,51 @@ func TestContainerDefinitionsAreEquivalent_missingEnvironmentName(t *testing.T) t.Fatal("Expected definitions to be equal.") } } + +func TestContainerDefinitionsAreEquivalent_sparseArrays(t *testing.T) { + t.Parallel() + + cfgRepresention := ` +[ + { + "name": "wordpress", + "links": [ + "mysql" + ], + "image": "wordpress", + "essential": true, + "portMappings": [ + {} + ], + "memory": 500, + "cpu": 10, + "environment": [null], + "mountPoints": [{"containerPath": null}], + "command": [""] + } +]` + + apiRepresentation := ` +[ + { + "name": "wordpress", + "image": "wordpress", + "cpu": 10, + "memory": 500, + "links": [ + "mysql" + ], + "portMappings": [], + "essential": true, + "command": [""] + } +]` + + equal, err := containerDefinitionsAreEquivalent(cfgRepresention, apiRepresentation, false) + if err != nil { + t.Fatal(err) + } + if !equal { + t.Fatal("Expected definitions to be equal.") + } +} diff --git a/internal/service/ecs/task_definition.go b/internal/service/ecs/task_definition.go index 88698563c2c..d7e5ca88066 100644 --- a/internal/service/ecs/task_definition.go +++ b/internal/service/ecs/task_definition.go @@ -93,6 +93,7 @@ func resourceTaskDefinition() *schema.Resource { containerDefinitions(orderedCDs).orderContainers() containerDefinitions(orderedCDs).orderEnvironmentVariables() containerDefinitions(orderedCDs).orderSecrets() + containerDefinitions(orderedCDs).compactArrays() unnormalizedJson, _ := flattenContainerDefinitions(orderedCDs) json, _ := structure.NormalizeJsonString(unnormalizedJson) return json diff --git a/internal/service/ecs/task_definition_test.go b/internal/service/ecs/task_definition_test.go index a8351554c32..3ba586fe045 100644 --- a/internal/service/ecs/task_definition_test.go +++ b/internal/service/ecs/task_definition_test.go @@ -609,7 +609,7 @@ func TestAccECSTaskDefinition_fsxWinFileSystem(t *testing.T) { t.Skip("skipping long-running test in short mode") } - if acctest.Partition() == "aws-us-gov" { + if acctest.Partition() == names.USGovCloudPartitionID { t.Skip("Amazon FSx for Windows File Server volumes for ECS tasks are not supported in GovCloud partition") } @@ -1464,6 +1464,32 @@ func TestAccECSTaskDefinition_containerDefinitionDockerLabels(t *testing.T) { }) } +// https://github.com/hashicorp/terraform-provider-aws/issues/38779. +func TestAccECSTaskDefinition_containerDefinitionNullPortMapping(t *testing.T) { + ctx := acctest.Context(t) + var def awstypes.TaskDefinition + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_task_definition.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ECSServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckTaskDefinitionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccTaskDefinitionConfig_containerDefinitionNullPortMapping(rName, "alpine"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckTaskDefinitionExists(ctx, resourceName, &def), + acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length(@)", acctest.Ct1), + acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length([0].portMappings)", acctest.Ct0), + acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].image", "alpine"), + ), + }, + }, + }) +} + func testAccCheckTaskDefinitionProxyConfiguration(after *awstypes.TaskDefinition, containerName string, proxyType string, ignoredUid string, ignoredGid string, appPorts string, proxyIngressPort string, proxyEgressPort string, egressIgnoredPorts string, egressIgnoredIPs string) resource.TestCheckFunc { @@ -3437,3 +3463,24 @@ resource "aws_ecs_task_definition" "test" { } `, rName, image) } + +func testAccTaskDefinitionConfig_containerDefinitionNullPortMapping(rName, image string) string { + return fmt.Sprintf(` +resource "aws_ecs_task_definition" "test" { + family = %[1]q + container_definitions = jsonencode([ + { + name = "first" + image = %[2]q + cpu = 10 + memory = 512 + essential = true + + portMappings = [ + null + ] + } + ]) +} +`, rName, image) +}