Skip to content

Commit

Permalink
Merge pull request #38622 from hashicorp/b-aws_ecs_task_definition.co…
Browse files Browse the repository at this point in the history
…ntainer_definitions-json-uppercase

r/aws_ecs_task_definition: Ensure JSON keys in `container_definitions` have leading lowercase
  • Loading branch information
ewbankkit authored Jul 31, 2024
2 parents 1b7bc29 + 7c7d6bd commit 37bd1c1
Show file tree
Hide file tree
Showing 8 changed files with 291 additions and 80 deletions.
3 changes: 3 additions & 0 deletions .changelog/38622.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ecs_task_definition: Ensure that JSON keys in `container_definitions` start with a lowercase letter
```
41 changes: 41 additions & 0 deletions internal/acctest/acctest.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,47 @@ func CheckResourceAttrJMESPair(nameFirst, keyFirst, jmesPath, nameSecond, keySec
}
}

func CheckResourceAttrJMESNotExists(name, key, jmesPath string) resource.TestCheckFunc {
return func(s *terraform.State) error {
is, err := PrimaryInstanceState(s, name)
if err != nil {
return err
}

attr, ok := is.Attributes[key]
if !ok {
return fmt.Errorf("%s: Attribute %q not set", name, key)
}

var jsonData any
err = json.Unmarshal([]byte(attr), &jsonData)
if err != nil {
return fmt.Errorf("%s: Expected attribute %q to be JSON: %w", name, key, err)
}

result, err := jmespath.Search(jmesPath, jsonData)
if err != nil {
return fmt.Errorf("invalid JMESPath %q: %w", jmesPath, err)
}

var v string
switch x := result.(type) {
case nil:
return nil
case string:
v = x
case float64:
v = strconv.FormatFloat(x, 'f', -1, 64)
case bool:
v = fmt.Sprint(x)
default:
return fmt.Errorf(`%[1]s: Attribute %[2]q, JMESPath %[3]q got "%#[4]v" (%[4]T), expected no attribute`, name, key, jmesPath, result)
}

return fmt.Errorf("%s: Attribute %q, JMESPath %q expected no attribute, got %#v", name, key, jmesPath, v)
}
}

// CheckResourceAttrContains ensures the Terraform state value contains the specified substr.
func CheckResourceAttrContains(name, key, substr string) resource.TestCheckFunc {
return resource.TestCheckResourceAttrWith(name, key, func(value string) error {
Expand Down
51 changes: 51 additions & 0 deletions internal/json/transform.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package json

import (
"unicode"
"unicode/utf8"

"github.com/hashicorp/terraform-provider-aws/internal/json/ujson"
)

// KeyFirstLower converts the first letter of each key to lowercase.
func KeyFirstLower(in []byte) []byte {
out := make([]byte, 0, len(in))

err := ujson.Walk(in, func(_ int, key, value []byte) bool {
// Write to output.
if len(out) != 0 && ujson.ShouldAddComma(value, out[len(out)-1]) {
out = append(out, ',')
}
// key is the raw key of the current object or empty otherwise.
// It can be a double-quoted string or empty.
switch len(key) {
case 0:
case 1, 2:
// Empty key.
out = append(out, key...)
default:
}
if len(key) > 0 {
out = append(out, '"')
r, n := utf8.DecodeRune(key[1:])
r = unicode.ToLower(r)
low := make([]byte, utf8.RuneLen(r))
utf8.EncodeRune(low, r)
out = append(out, low...)
out = append(out, key[n+1:]...)
out = append(out, ':')
}
out = append(out, value...)

return true
})

if err != nil {
return nil
}

return out
}
68 changes: 68 additions & 0 deletions internal/json/transform_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package json_test

import (
"testing"

"github.com/hashicorp/terraform-provider-aws/internal/json"
)

func TestKeyFirstLower(t *testing.T) {
t.Parallel()

testCases := []struct {
testName string
input string
want string
}{
{
testName: "empty JSON",
input: "{}",
want: "{}",
},
{
testName: "single field, lowercase",
input: `{ "key": 42 }`,
want: `{"key":42}`,
},
{
testName: "single field, uppercase",
input: `{ "Key": 42 }`,
want: `{"key":42}`,
},
{
testName: "multiple fields",
input: `
[
{
"Name": "FIRST",
"Image": "alpine",
"Cpu": 10,
"Memory": 512,
"Essential": true,
"PortMappings": [
{
"ContainerPort": 80,
"HostPort": 80
}
]
}
]
`,
want: `[{"name":"FIRST","image":"alpine","cpu":10,"memory":512,"essential":true,"portMappings":[{"containerPort":80,"hostPort":80}]}]`,
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.testName, func(t *testing.T) {
t.Parallel()

if got, want := string(json.KeyFirstLower([]byte(testCase.input))), testCase.want; got != want {
t.Errorf("KeyFirstLower(%q) = %q, want %q", testCase.input, got, want)
}
})
}
}
11 changes: 10 additions & 1 deletion internal/service/ecs/task_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,16 @@ func flattenFSxWinVolumeAuthorizationConfig(config *awstypes.FSxWindowsFileServe
}

func flattenContainerDefinitions(apiObjects []awstypes.ContainerDefinition) (string, error) {
return tfjson.EncodeToString(apiObjects)
json, err := tfjson.EncodeToBytes(apiObjects)
if err != nil {
return "", err
}

// Remove empty fields and convert first character of keys to lowercase.
json = tfjson.RemoveEmptyFields(json)
json = tfjson.KeyFirstLower(json)

return string(json), nil
}

func expandContainerDefinitions(tfString string) ([]awstypes.ContainerDefinition, error) {
Expand Down
116 changes: 116 additions & 0 deletions internal/service/ecs/task_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,99 @@ func TestAccECSTaskDefinition_unknownContainerDefinitions(t *testing.T) {
})
}

// https://github.com/hashicorp/terraform-provider-aws/issues/38543.
func TestAccECSTaskDefinition_v5590ContainerDefinitionsRegression(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),
CheckDestroy: testAccCheckTaskDefinitionDestroy(ctx),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: "5.58.0",
},
},
Config: testAccTaskDefinitionConfig_v5590ContainerDefinitionsRegression(rName, "alpine"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckTaskDefinitionExists(ctx, resourceName, &def),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length(@)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].cpu", acctest.Ct10),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].essential", acctest.CtTrue),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].image", "alpine"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].memory", "512"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].name", "first"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length([0].portMappings)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].portMappings[0].containerPort", "80"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].portMappings[0].hostPort", "80"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Cpu"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Essential"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Image"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Memory"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Name"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].PortMappings"),
),
},
// At v5.59.0 and v5.60.0, JSON keys were returned with leading capital letters.
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: "5.60.0",
},
},
Config: testAccTaskDefinitionConfig_v5590ContainerDefinitionsRegression(rName, "jenkins"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckTaskDefinitionExists(ctx, resourceName, &def),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length(@)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].Cpu", acctest.Ct10),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].Essential", acctest.CtTrue),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].Image", "jenkins"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].Memory", "512"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].Name", "first"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length([0].PortMappings)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].PortMappings[0].ContainerPort", "80"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].PortMappings[0].HostPort", "80"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].cpu"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].essential"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].image"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].memory"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].name"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].portMappings"),
),
},
{
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Config: testAccTaskDefinitionConfig_v5590ContainerDefinitionsRegression(rName, "nginx"),
Check: resource.ComposeTestCheckFunc(
testAccCheckTaskDefinitionExists(ctx, resourceName, &def),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length(@)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].cpu", acctest.Ct10),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].essential", acctest.CtTrue),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].image", "nginx"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].memory", "512"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].name", "first"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "length([0].portMappings)", acctest.Ct1),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].portMappings[0].containerPort", "80"),
acctest.CheckResourceAttrJMES(resourceName, "container_definitions", "[0].portMappings[0].hostPort", "80"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Cpu"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Essential"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Image"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Memory"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].Name"),
acctest.CheckResourceAttrJMESNotExists(resourceName, "container_definitions", "[0].PortMappings"),
),
},
},
})
}

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 @@ -3145,3 +3238,26 @@ resource "aws_ecs_task_definition" "test" {
}
`, rName)
}

func testAccTaskDefinitionConfig_v5590ContainerDefinitionsRegression(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 = [
{
containerPort = 80
hostPort = 80
}
]
}
])
}
`, rName, image)
}
Loading

0 comments on commit 37bd1c1

Please sign in to comment.