Skip to content

Commit

Permalink
Merge pull request #38870 from hashicorp/issue-38779
Browse files Browse the repository at this point in the history
r/aws_ecs_task_definition: Remove zeros from `container_definitions` object arrays
  • Loading branch information
ewbankkit authored Aug 14, 2024
2 parents 26accfa + 6e42270 commit 6fa269f
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 1 deletion.
2 changes: 2 additions & 0 deletions .changelog/38870.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
```release-note:bug
resource/aws_ecs_task_definition: Remove `null`s from `container_definition` array fields
33 changes: 33 additions & 0 deletions internal/service/ecs/container_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -179,5 +210,7 @@ func expandContainerDefinitions(tfString string) ([]awstypes.ContainerDefinition
}
}

containerDefinitions(apiObjects).compactArrays()

return apiObjects, nil
}
48 changes: 48 additions & 0 deletions internal/service/ecs/container_definitions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
}
1 change: 1 addition & 0 deletions internal/service/ecs/task_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 48 additions & 1 deletion internal/service/ecs/task_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

0 comments on commit 6fa269f

Please sign in to comment.