From 8e00a77b0f2eafeee6875479b91eb0eb2942bc33 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 13 Sep 2024 14:03:31 -0700 Subject: [PATCH 1/2] Adds check for record with empty string `role_arn` --- internal/provider/provider.go | 26 +++++- internal/provider/provider_config_test.go | 108 ++++++++++++++++++++++ 2 files changed, 129 insertions(+), 5 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index dd2f71e069b5..709f47a26469 100644 --- 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 index 458c7bae2736..ca31f974b141 100644 --- 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 From 811afb5b07cb37d3f96cff19bb8ff81fed7caabc Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 13 Sep 2024 14:47:52 -0700 Subject: [PATCH 2/2] Adds CHANGELOG entry --- .changelog/39328.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/39328.txt diff --git a/.changelog/39328.txt b/.changelog/39328.txt new file mode 100644 index 000000000000..c6bd91f3d144 --- /dev/null +++ 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. +```