From 15c61db69452f646811a357947c52b1b870fdf31 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 09:42:06 -0400 Subject: [PATCH 01/10] Add 'flex.ExpandStringListKeepEmpty'. --- internal/flex/flex.go | 21 ++++++++++---- internal/flex/flex_test.go | 59 ++++++++++++++++++++++++++------------ 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/internal/flex/flex.go b/internal/flex/flex.go index fb13d15d5831..ca86dbf31207 100644 --- a/internal/flex/flex.go +++ b/internal/flex/flex.go @@ -18,14 +18,25 @@ const ( ResourceIdSeparator = "," ) -// Takes the result of flatmap.Expand for an array of strings -// and returns a []*string +// ExpandStringList the result of flatmap.Expand for an array of strings +// and returns a []*string. Empty strings are skipped. func ExpandStringList(configured []interface{}) []*string { vs := make([]*string, 0, len(configured)) for _, v := range configured { - val, ok := v.(string) - if ok && val != "" { - vs = append(vs, aws.String(v.(string))) + if v, ok := v.(string); ok && v != "" { + vs = append(vs, aws.String(v)) + } + } + return vs +} + +// ExpandStringListKeepEmpty the result of flatmap.Expand for an array of strings +// and returns a []*string. Empty strings are kept. +func ExpandStringListKeepEmpty(configured []interface{}) []*string { + vs := make([]*string, 0, len(configured)) + for _, v := range configured { + if v, ok := v.(string); ok { + vs = append(vs, aws.String(v)) } } return vs diff --git a/internal/flex/flex_test.go b/internal/flex/flex_test.go index 8c76a17871d7..ac79bfd53d87 100644 --- a/internal/flex/flex_test.go +++ b/internal/flex/flex_test.go @@ -15,31 +15,54 @@ import ( func TestExpandStringList(t *testing.T) { t.Parallel() - configured := []interface{}{"abc", "xyz123"} - got := ExpandStringList(configured) - want := []*string{ - aws.String("abc"), - aws.String("xyz123"), + testCases := []struct { + configured []interface{} + want []*string + }{ + { + configured: []interface{}{"abc", "xyz123"}, + want: []*string{aws.String("abc"), aws.String("xyz123")}, + }, + { + configured: []interface{}{"abc", 123, "xyz123"}, + want: []*string{aws.String("abc"), aws.String("xyz123")}, + }, + { + configured: []interface{}{"foo", "bar", "", "baz"}, + want: []*string{aws.String("foo"), aws.String("bar"), aws.String("baz")}, + }, } - - if !cmp.Equal(got, want) { - t.Errorf("expanded = %v, want = %v", got, want) + for _, testCase := range testCases { + if got, want := ExpandStringList(testCase.configured), testCase.want; !cmp.Equal(got, want) { + t.Errorf("ExpandStringList(%v) = %v, want %v", testCase.configured, got, want) + } } } -func TestExpandStringListEmptyItems(t *testing.T) { +func TestExpandStringListKeepEmpty(t *testing.T) { t.Parallel() - configured := []interface{}{"foo", "bar", "", "baz"} - got := ExpandStringList(configured) - want := []*string{ - aws.String("foo"), - aws.String("bar"), - aws.String("baz"), + testCases := []struct { + configured []interface{} + want []*string + }{ + { + configured: []interface{}{"abc", "xyz123"}, + want: []*string{aws.String("abc"), aws.String("xyz123")}, + }, + { + configured: []interface{}{"abc", 123, "xyz123"}, + want: []*string{aws.String("abc"), aws.String("xyz123")}, + }, + { + configured: []interface{}{"foo", "bar", "", "baz"}, + want: []*string{aws.String("foo"), aws.String("bar"), aws.String(""), aws.String("baz")}, + }, } - - if !cmp.Equal(got, want) { - t.Errorf("expanded = %v, want = %v", got, want) + for _, testCase := range testCases { + if got, want := ExpandStringListKeepEmpty(testCase.configured), testCase.want; !cmp.Equal(got, want) { + t.Errorf("ExpandStringListKeepEmpty(%v) = %v, want %v", testCase.configured, got, want) + } } } From 296f7e02cf77af6461aba577a469ffe00e096414 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 09:54:11 -0400 Subject: [PATCH 02/10] r/aws_emr_cluster: Keep empty strings in 'bootstrap_action.args'. --- internal/service/emr/cluster.go | 2 +- internal/service/emr/cluster_test.go | 51 ++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/internal/service/emr/cluster.go b/internal/service/emr/cluster.go index faac7a6412fe..6fe6fbf52564 100644 --- a/internal/service/emr/cluster.go +++ b/internal/service/emr/cluster.go @@ -1740,7 +1740,7 @@ func expandBootstrapActions(bootstrapActions []interface{}) []*emr.BootstrapActi Name: aws.String(actionName), ScriptBootstrapAction: &emr.ScriptBootstrapActionConfig{ Path: aws.String(actionPath), - Args: flex.ExpandStringList(actionArgs), + Args: flex.ExpandStringListKeepEmpty(actionArgs), }, } actionsOut = append(actionsOut, action) diff --git a/internal/service/emr/cluster_test.go b/internal/service/emr/cluster_test.go index d3801b26af26..0cb038d76f22 100644 --- a/internal/service/emr/cluster_test.go +++ b/internal/service/emr/cluster_test.go @@ -1040,7 +1040,18 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.args.1", "echo running on master node"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "11"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.1", "1"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.3", "3"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.4", "4"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.5", "5"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.6", "6"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.7", "7"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.8", "8"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "9"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "10"), ), }, { @@ -1065,7 +1076,18 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.args.1", "echo running on master node"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "11"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.1", "1"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.3", "3"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.4", "4"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.5", "5"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.6", "6"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.7", "7"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.8", "8"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "9"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "10"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.name", "runif-2"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.path", "s3://elasticmapreduce/bootstrap-actions/run-if"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.#", "2"), @@ -1095,7 +1117,18 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.args.1", "echo running on master node"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.#", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.#", "11"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.1", "1"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.3", "3"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.4", "4"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.5", "5"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.6", "6"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.7", "7"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.8", "8"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.9", "9"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.9", "10"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "runif-2"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", "s3://elasticmapreduce/bootstrap-actions/run-if"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "2"), @@ -3059,7 +3092,9 @@ resource "aws_emr_cluster" "test" { path = "s3://${aws_s3_object.testobject.bucket}/${aws_s3_object.testobject.key}" name = "test" - args = ["1", + args = [ + "", + "1", "2", "3", "4", @@ -3123,7 +3158,9 @@ resource "aws_emr_cluster" "test" { path = "s3://${aws_s3_object.testobject.bucket}/${aws_s3_object.testobject.key}" name = "test" - args = ["1", + args = [ + "", + "1", "2", "3", "4", @@ -3199,7 +3236,9 @@ resource "aws_emr_cluster" "test" { path = "s3://${aws_s3_object.testobject.bucket}/${aws_s3_object.testobject.key}" name = "test" - args = ["1", + args = [ + "", + "1", "2", "3", "4", From d50050893372d7ad4e1cc2edad47c2c65d9b8883 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 09:55:32 -0400 Subject: [PATCH 03/10] Add CHANGELOG entry. --- .changelog/#####.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/#####.txt diff --git a/.changelog/#####.txt b/.changelog/#####.txt new file mode 100644 index 000000000000..dc27171bc330 --- /dev/null +++ b/.changelog/#####.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_emr_cluster: Keep empty string values in `bootstrap_action.args` +``` \ No newline at end of file From 3d57caf604385165e3746218ee7c4b71b87720ce Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 09:57:49 -0400 Subject: [PATCH 04/10] Correct CHANGELOG entry file name. --- .changelog/{#####.txt => 32738.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{#####.txt => 32738.txt} (100%) diff --git a/.changelog/#####.txt b/.changelog/32738.txt similarity index 100% rename from .changelog/#####.txt rename to .changelog/32738.txt From d791cdb1d9ddd3c8ab656e72535bd5d5a17db62d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 10:12:43 -0400 Subject: [PATCH 05/10] Fix 'Error: creating S3 bucket ACL for tf-acc-test-6517809390050450753: AccessDenied: Access Denied'. --- internal/service/emr/cluster_test.go | 36 +++++++++++++++++++++------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/internal/service/emr/cluster_test.go b/internal/service/emr/cluster_test.go index 0cb038d76f22..7e6860e0b849 100644 --- a/internal/service/emr/cluster_test.go +++ b/internal/service/emr/cluster_test.go @@ -2075,13 +2075,34 @@ resource "aws_iam_role_policy_attachment" "emr_autoscaling_role" { `, rName) } -func testAccClusterBootstrapActionBucketConfig(rName string) string { +func testAccClusterConfig_baseBootstrapActionBucket(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "tester" { bucket = %[1]q } +resource "aws_s3_bucket_public_access_block" "tester" { + bucket = aws_s3_bucket.tester.id + + block_public_acls = false + block_public_policy = false + ignore_public_acls = false + restrict_public_buckets = false +} + +resource "aws_s3_bucket_ownership_controls" "tester" { + bucket = aws_s3_bucket.tester.id + rule { + object_ownership = "BucketOwnerPreferred" + } +} + resource "aws_s3_bucket_acl" "tester" { + depends_on = [ + aws_s3_bucket_public_access_block.tester, + aws_s3_bucket_ownership_controls.tester, + ] + bucket = aws_s3_bucket.tester.id acl = "public-read" } @@ -2094,7 +2115,6 @@ resource "aws_s3_object" "testobject" { echo $@ EOF - acl = "public-read" } `, rName) @@ -3049,7 +3069,7 @@ func testAccClusterConfig_bootstrap(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} @@ -3115,7 +3135,7 @@ func testAccClusterConfig_bootstrapAdd(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} @@ -3187,7 +3207,7 @@ func testAccClusterConfig_bootstrapReorder(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} @@ -3822,7 +3842,7 @@ func testAccClusterConfig_instanceFleets(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} @@ -3910,7 +3930,7 @@ func testAccClusterConfig_instanceFleetMultipleSubnets(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} @@ -4010,7 +4030,7 @@ func testAccClusterConfig_instanceFleetsMasterOnly(rName string) string { testAccClusterConfig_baseVPC(rName, false), testAccClusterConfig_baseIAMServiceRole(rName), testAccClusterConfig_baseIAMInstanceProfile(rName), - testAccClusterBootstrapActionBucketConfig(rName), + testAccClusterConfig_baseBootstrapActionBucket(rName), fmt.Sprintf(` data "aws_partition" "current" {} From 5bccd5d87cf4e112db2aa932b806fb971149eaf7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 11:38:36 -0400 Subject: [PATCH 06/10] Fix terrafmt errors. --- internal/service/emr/cluster_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/emr/cluster_test.go b/internal/service/emr/cluster_test.go index 7e6860e0b849..6ad4d8741051 100644 --- a/internal/service/emr/cluster_test.go +++ b/internal/service/emr/cluster_test.go @@ -2099,8 +2099,8 @@ resource "aws_s3_bucket_ownership_controls" "tester" { resource "aws_s3_bucket_acl" "tester" { depends_on = [ - aws_s3_bucket_public_access_block.tester, - aws_s3_bucket_ownership_controls.tester, + aws_s3_bucket_public_access_block.tester, + aws_s3_bucket_ownership_controls.tester, ] bucket = aws_s3_bucket.tester.id @@ -2108,7 +2108,7 @@ resource "aws_s3_bucket_acl" "tester" { } resource "aws_s3_object" "testobject" { - bucket = aws_s3_bucket.tester.bucket + bucket = aws_s3_bucket_acl.tester.bucket key = "testscript.sh" content = < Date: Fri, 28 Jul 2023 13:14:35 -0400 Subject: [PATCH 07/10] Revert "Correct CHANGELOG entry file name." This reverts commit 3d57caf604385165e3746218ee7c4b71b87720ce. --- .changelog/{32738.txt => #####.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{32738.txt => #####.txt} (100%) diff --git a/.changelog/32738.txt b/.changelog/#####.txt similarity index 100% rename from .changelog/32738.txt rename to .changelog/#####.txt From 934e63bc26271578ff9c7c1c3a084df47d26e797 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 13:14:53 -0400 Subject: [PATCH 08/10] Revert "Add CHANGELOG entry." This reverts commit d50050893372d7ad4e1cc2edad47c2c65d9b8883. --- .changelog/#####.txt | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .changelog/#####.txt diff --git a/.changelog/#####.txt b/.changelog/#####.txt deleted file mode 100644 index dc27171bc330..000000000000 --- a/.changelog/#####.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:bug -resource/aws_emr_cluster: Keep empty string values in `bootstrap_action.args` -``` \ No newline at end of file From 40892ef40692ba6b429b0ef4845e6835b7eaa981 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 13:16:52 -0400 Subject: [PATCH 09/10] 'TestAccEMRCluster_Bootstrap_ordering' test with quoted empty string. --- internal/service/emr/cluster_test.go | 36 ++++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/service/emr/cluster_test.go b/internal/service/emr/cluster_test.go index 6ad4d8741051..8ee02c870275 100644 --- a/internal/service/emr/cluster_test.go +++ b/internal/service/emr/cluster_test.go @@ -1030,7 +1030,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccClusterConfig_bootstrap(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckClusterExists(ctx, resourceName, &cluster), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.#", "2"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.name", "runif"), @@ -1041,9 +1041,9 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "11"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", "0"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.1", "1"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "\"\""), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.3", "3"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.4", "4"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.5", "5"), @@ -1051,7 +1051,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.7", "7"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.8", "8"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "9"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.10", "10"), ), }, { @@ -1066,7 +1066,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { }, { Config: testAccClusterConfig_bootstrapAdd(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckClusterExists(ctx, resourceName, &cluster), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.#", "3"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.name", "runif"), @@ -1077,9 +1077,9 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "11"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.0", "0"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.1", "1"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.2", "\"\""), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.3", "3"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.4", "4"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.5", "5"), @@ -1087,7 +1087,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.7", "7"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.8", "8"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "9"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.9", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.10", "10"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.name", "runif-2"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.path", "s3://elasticmapreduce/bootstrap-actions/run-if"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.#", "2"), @@ -1107,7 +1107,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { }, { Config: testAccClusterConfig_bootstrapReorder(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckClusterExists(ctx, resourceName, &cluster), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.#", "3"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.0.name", "runif"), @@ -1118,9 +1118,9 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.name", "test"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.path", fmt.Sprintf("s3://%s/testscript.sh", rName)), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.#", "11"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.0", ""), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.0", "0"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.1", "1"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.2", "2"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.2", "\"\""), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.3", "3"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.4", "4"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.5", "5"), @@ -1128,7 +1128,7 @@ func TestAccEMRCluster_Bootstrap_ordering(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.7", "7"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.8", "8"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.9", "9"), - resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.9", "10"), + resource.TestCheckResourceAttr(resourceName, "bootstrap_action.2.args.10", "10"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.name", "runif-2"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.path", "s3://elasticmapreduce/bootstrap-actions/run-if"), resource.TestCheckResourceAttr(resourceName, "bootstrap_action.1.args.#", "2"), @@ -3113,9 +3113,9 @@ resource "aws_emr_cluster" "test" { name = "test" args = [ - "", + "0", "1", - "2", + "\"\"", "3", "4", "5", @@ -3179,9 +3179,9 @@ resource "aws_emr_cluster" "test" { name = "test" args = [ - "", + "0", "1", - "2", + "\"\"", "3", "4", "5", @@ -3257,9 +3257,9 @@ resource "aws_emr_cluster" "test" { name = "test" args = [ - "", + "0", "1", - "2", + "\"\"", "3", "4", "5", From d452e6d89c14a0771a5b488520662674e9dad95a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 28 Jul 2023 13:17:13 -0400 Subject: [PATCH 10/10] Remove 'flex.ExpandStringListKeepEmpty'. --- internal/flex/flex.go | 12 ------------ internal/flex/flex_test.go | 27 --------------------------- internal/service/emr/cluster.go | 2 +- 3 files changed, 1 insertion(+), 40 deletions(-) diff --git a/internal/flex/flex.go b/internal/flex/flex.go index ca86dbf31207..43005cd29db7 100644 --- a/internal/flex/flex.go +++ b/internal/flex/flex.go @@ -30,18 +30,6 @@ func ExpandStringList(configured []interface{}) []*string { return vs } -// ExpandStringListKeepEmpty the result of flatmap.Expand for an array of strings -// and returns a []*string. Empty strings are kept. -func ExpandStringListKeepEmpty(configured []interface{}) []*string { - vs := make([]*string, 0, len(configured)) - for _, v := range configured { - if v, ok := v.(string); ok { - vs = append(vs, aws.String(v)) - } - } - return vs -} - // Takes the result of flatmap.Expand for an array of strings // and returns a []*time.Time func ExpandStringTimeList(configured []interface{}, format string) []*time.Time { diff --git a/internal/flex/flex_test.go b/internal/flex/flex_test.go index ac79bfd53d87..5f24939207ea 100644 --- a/internal/flex/flex_test.go +++ b/internal/flex/flex_test.go @@ -39,33 +39,6 @@ func TestExpandStringList(t *testing.T) { } } -func TestExpandStringListKeepEmpty(t *testing.T) { - t.Parallel() - - testCases := []struct { - configured []interface{} - want []*string - }{ - { - configured: []interface{}{"abc", "xyz123"}, - want: []*string{aws.String("abc"), aws.String("xyz123")}, - }, - { - configured: []interface{}{"abc", 123, "xyz123"}, - want: []*string{aws.String("abc"), aws.String("xyz123")}, - }, - { - configured: []interface{}{"foo", "bar", "", "baz"}, - want: []*string{aws.String("foo"), aws.String("bar"), aws.String(""), aws.String("baz")}, - }, - } - for _, testCase := range testCases { - if got, want := ExpandStringListKeepEmpty(testCase.configured), testCase.want; !cmp.Equal(got, want) { - t.Errorf("ExpandStringListKeepEmpty(%v) = %v, want %v", testCase.configured, got, want) - } - } -} - func TestExpandStringTimeList(t *testing.T) { t.Parallel() diff --git a/internal/service/emr/cluster.go b/internal/service/emr/cluster.go index 6fe6fbf52564..faac7a6412fe 100644 --- a/internal/service/emr/cluster.go +++ b/internal/service/emr/cluster.go @@ -1740,7 +1740,7 @@ func expandBootstrapActions(bootstrapActions []interface{}) []*emr.BootstrapActi Name: aws.String(actionName), ScriptBootstrapAction: &emr.ScriptBootstrapActionConfig{ Path: aws.String(actionPath), - Args: flex.ExpandStringListKeepEmpty(actionArgs), + Args: flex.ExpandStringList(actionArgs), }, } actionsOut = append(actionsOut, action)