Merge pull request #39328 from hashicorp/b-assume-role-empty-string

provider: Allow empty string in `assume_role.role_arn`
hashicorp · Sep 16, 2024 · 2d12e5f · 2d12e5f
2 parents dc25114 + 811afb5
commit 2d12e5f
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 5 deletions.
diff --git a/.changelog/39328.txt b/.changelog/39328.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+provider: Allows `assume_role.role_arn` to be an empty string when there is a single `assume_role` entry.
+```
diff --git a/internal/provider/provider.go b/internal/provider/provider.go
@@ -518,11 +518,27 @@ func configure(ctx context.Context, provider *schema.Provider, d *schema.Resourc
 	if v, ok := d.GetOk("assume_role"); ok {
 		path := cty.GetAttrPath("assume_role")
 		v := v.([]any)
-		if len(v) == 1 && v[0] == nil {
-			diags = append(diags,
-				errs.NewAttributeRequiredWillBeError(path.IndexInt(0), "role_arn"),
-			)
-		} else if len(v) > 0 {
+		if len(v) == 1 {
+			if v[0] == nil {
+				diags = append(diags,
+					errs.NewAttributeRequiredWillBeError(path.IndexInt(0), "role_arn"),
+				)
+			} else {
+				l := v[0].(map[string]any)
+				if s, ok := l["role_arn"]; !ok || s == "" {
+					diags = append(diags,
+						errs.NewAttributeRequiredWillBeError(path.IndexInt(0), "role_arn"),
+					)
+				} else {
+					ar, dg := expandAssumeRoles(ctx, path, v)
+					diags = append(diags, dg...)
+					if dg.HasError() {
+						return nil, diags
+					}
+					config.AssumeRole = ar
+				}
+			}
+		} else if len(v) > 1 {
 			ar, dg := expandAssumeRoles(ctx, path, v)
 			diags = append(diags, dg...)
 			if dg.HasError() {

diff --git a/internal/provider/provider_config_test.go b/internal/provider/provider_config_test.go
@@ -439,6 +439,36 @@ func TestProviderConfig_AssumeRole(t *testing.T) { //nolint:paralleltest
 			},
 		},
 
+		// For historical reasons, this is valid
+		"config null string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn": nil,
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredWillBeError(cty.GetAttrPath("assume_role").IndexInt(0), "role_arn"),
+			},
+		},
+
+		// For historical reasons, this is valid
+		"config empty string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn": "",
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredWillBeError(cty.GetAttrPath("assume_role").IndexInt(0), "role_arn"),
+			},
+		},
+
 		"config multiple first empty": {
 			Config: map[string]any{
 				"assume_role": []any{
@@ -455,6 +485,42 @@ func TestProviderConfig_AssumeRole(t *testing.T) { //nolint:paralleltest
 			},
 		},
 
+		"config multiple first null string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn": nil,
+					},
+					map[string]any{
+						"role_arn":     servicemocks.MockStsAssumeRoleArn2,
+						"session_name": servicemocks.MockStsAssumeRoleSessionName2,
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredError(cty.GetAttrPath("assume_role").IndexInt(0), "role_arn"),
+			},
+		},
+
+		"config multiple first empty string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn": nil,
+					},
+					map[string]any{
+						"role_arn":     servicemocks.MockStsAssumeRoleArn2,
+						"session_name": servicemocks.MockStsAssumeRoleSessionName2,
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredError(cty.GetAttrPath("assume_role").IndexInt(0), "role_arn"),
+			},
+		},
+
 		"config multiple last empty": {
 			Config: map[string]any{
 				"assume_role": []any{
@@ -473,6 +539,48 @@ func TestProviderConfig_AssumeRole(t *testing.T) { //nolint:paralleltest
 				servicemocks.MockStsAssumeRoleValidEndpoint,
 			},
 		},
+
+		"config multiple last null string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn":     servicemocks.MockStsAssumeRoleArn,
+						"session_name": servicemocks.MockStsAssumeRoleSessionName,
+					},
+					map[string]any{
+						"role_arn": nil,
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredError(cty.GetAttrPath("assume_role").IndexInt(1), "role_arn"),
+			},
+			MockStsEndpoints: []*servicemocks.MockEndpoint{
+				servicemocks.MockStsAssumeRoleValidEndpoint,
+			},
+		},
+
+		"config multiple last empty string": {
+			Config: map[string]any{
+				"assume_role": []any{
+					map[string]any{
+						"role_arn":     servicemocks.MockStsAssumeRoleArn,
+						"session_name": servicemocks.MockStsAssumeRoleSessionName,
+					},
+					map[string]any{
+						"role_arn": "",
+					},
+				},
+			},
+			ExpectedCredentialsValue: mockdata.MockStsAssumeRoleCredentials,
+			ExpectedDiags: diag.Diagnostics{
+				errs.NewAttributeRequiredError(cty.GetAttrPath("assume_role").IndexInt(1), "role_arn"),
+			},
+			MockStsEndpoints: []*servicemocks.MockEndpoint{
+				servicemocks.MockStsAssumeRoleValidEndpoint,
+			},
+		},
 	}
 
 	for name, tc := range testCases { //nolint:paralleltest