From db2ff4e43206a2b98d5af84f64bde8332897da1b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 10:50:48 -0500 Subject: [PATCH 1/6] iam: ISO-friendly tagging --- internal/service/iam/consts.go | 5 ++ internal/service/iam/role.go | 118 ++++++++++++++++++++++++--------- 2 files changed, 91 insertions(+), 32 deletions(-) create mode 100644 internal/service/iam/consts.go diff --git a/internal/service/iam/consts.go b/internal/service/iam/consts.go new file mode 100644 index 00000000000..b6a409b420c --- /dev/null +++ b/internal/service/iam/consts.go @@ -0,0 +1,5 @@ +package iam + +const ( + ErrCodeAccessDenied = "AccessDenied" +) diff --git a/internal/service/iam/role.go b/internal/service/iam/role.go index bb866b5cc04..b73fd681f8c 100644 --- a/internal/service/iam/role.go +++ b/internal/service/iam/role.go @@ -8,6 +8,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/aws-sdk-go-base/tfawserr" awspolicy "github.com/hashicorp/awspolicyequivalence" @@ -199,25 +200,21 @@ func resourceRoleCreate(d *schema.ResourceData, meta interface{}) error { request.Tags = Tags(tags.IgnoreAWS()) } - outputRaw, err := tfresource.RetryWhen( - PropagationTimeout, - func() (interface{}, error) { - return conn.CreateRole(request) - }, - func(err error) (bool, error) { - if tfawserr.ErrMessageContains(err, iam.ErrCodeMalformedPolicyDocumentException, "Invalid principal in policy") { - return true, err - } + output, err := retryCreateRole(conn, request) - return false, err - }, - ) + // Some partitions (i.e., ISO) may not support tag-on-create + if request.Tags != nil && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] IAM Role (%s) create failed (%s) with tags. Trying create without tags.", d.Id(), err) + request.Tags = nil + + output, err = retryCreateRole(conn, request) + } if err != nil { return fmt.Errorf("error creating IAM Role (%s): %w", name, err) } - roleName := aws.StringValue(outputRaw.(*iam.CreateRoleOutput).Role.RoleName) + roleName := aws.StringValue(output.Role.RoleName) if v, ok := d.GetOk("inline_policy"); ok && v.(*schema.Set).Len() > 0 { policies := expandRoleInlinePolicies(roleName, v.(*schema.Set).List()) @@ -234,6 +231,22 @@ func resourceRoleCreate(d *schema.ResourceData, meta interface{}) error { } d.SetId(roleName) + + // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create + if request.Tags == nil && len(tags) > 0 && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID { + err := roleUpdateTags(conn, d.Id(), nil, tags) + + // If default tags only, log and continue. Otherwise, error. + if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] error adding tags after create for IAM Role (%s): %s", d.Id(), err) + return resourceRoleRead(d, meta) + } + + if err != nil { + return fmt.Errorf("error creating IAM Role (%s) tags: %w", d.Id(), err) + } + } + return resourceRoleRead(d, meta) } @@ -277,17 +290,6 @@ func resourceRoleRead(d *schema.ResourceData, meta interface{}) error { } d.Set("unique_id", role.RoleId) - tags := KeyValueTags(role.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) - - //lintignore:AWSR002 - if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %w", err) - } - - if err := d.Set("tags_all", tags.Map()); err != nil { - return fmt.Errorf("error setting tags_all: %w", err) - } - assumeRolePolicy, err := url.QueryUnescape(*role.AssumeRolePolicyDocument) if err != nil { return err @@ -318,6 +320,23 @@ func resourceRoleRead(d *schema.ResourceData, meta interface{}) error { } d.Set("managed_policy_arns", managedPolicies) + tags := KeyValueTags(role.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to list tags for IAM Role %s: %s", d.Id(), err) + return nil + } + + //lintignore:AWSR002 + if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { + return fmt.Errorf("error setting tags: %w", err) + } + + if err := d.Set("tags_all", tags.Map()); err != nil { + return fmt.Errorf("error setting tags_all: %w", err) + } + return nil } @@ -401,14 +420,6 @@ func resourceRoleUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") - - if err := roleUpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating IAM Role (%s) tags: %s", d.Id(), err) - } - } - if d.HasChange("inline_policy") && inlinePoliciesActualDiff(d) { roleName := d.Get("name").(string) @@ -475,6 +486,22 @@ func resourceRoleUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("tags_all") { + o, n := d.GetChange("tags_all") + + err := roleUpdateTags(conn, d.Id(), o, n) + + // Some partitions may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to update tags for IAM Role %s: %s", d.Id(), err) + return resourceRoleRead(d, meta) + } + + if err != nil { + return fmt.Errorf("error updating IAM Role (%s) tags: %w", d.Id(), err) + } + } + return resourceRoleRead(d, meta) } @@ -582,6 +609,33 @@ func deleteRoleInstanceProfiles(conn *iam.IAM, roleName string) error { return nil } +func retryCreateRole(conn *iam.IAM, input *iam.CreateRoleInput) (*iam.CreateRoleOutput, error) { + outputRaw, err := tfresource.RetryWhen( + PropagationTimeout, + func() (interface{}, error) { + return conn.CreateRole(input) + }, + func(err error) (bool, error) { + if tfawserr.ErrMessageContains(err, iam.ErrCodeMalformedPolicyDocumentException, "Invalid principal in policy") { + return true, err + } + + return false, err + }, + ) + + if err != nil { + return nil, err + } + + output, ok := outputRaw.(*iam.CreateRoleOutput) + if !ok || output == nil || aws.StringValue(output.Role.RoleName) == "" { + return nil, fmt.Errorf("create IAM role (%s) returned an empty result", aws.StringValue(input.RoleName)) + } + + return output, err +} + func readRolePolicyAttachments(conn *iam.IAM, roleName string) ([]*string, error) { managedPolicies := make([]*string, 0) input := &iam.ListAttachedRolePoliciesInput{ From 6fe479a90d2d780f0c5e276369c2fcc7bd776428 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 10:52:16 -0500 Subject: [PATCH 2/6] Add changelog --- .changelog/22544.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22544.txt diff --git a/.changelog/22544.txt b/.changelog/22544.txt new file mode 100644 index 00000000000..0edd0ccaa56 --- /dev/null +++ b/.changelog/22544.txt @@ -0,0 +1,3 @@ +```release-note:enhancement + resource/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) + ``` \ No newline at end of file From 88a20474f5b128e19ffb89e538a02ef7ed567329 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 12:00:33 -0500 Subject: [PATCH 3/6] ds/role: ISO-friendly tagging --- internal/service/iam/role_data_source.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/service/iam/role_data_source.go b/internal/service/iam/role_data_source.go index 206fef40b65..711166adb58 100644 --- a/internal/service/iam/role_data_source.go +++ b/internal/service/iam/role_data_source.go @@ -2,11 +2,14 @@ package iam import ( "fmt" + "log" "net/url" "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/iam" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" @@ -86,7 +89,6 @@ func dataSourceRoleRead(d *schema.ResourceData, meta interface{}) error { d.Set("permissions_boundary", output.Role.PermissionsBoundary.PermissionsBoundaryArn) } d.Set("unique_id", output.Role.RoleId) - d.Set("tags", KeyValueTags(output.Role.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map()) assumRolePolicy, err := url.QueryUnescape(aws.StringValue(output.Role.AssumeRolePolicyDocument)) if err != nil { @@ -96,6 +98,19 @@ func dataSourceRoleRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error setting assume_role_policy: %w", err) } + tags := KeyValueTags(output.Role.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to list tags for IAM Role %s: %s", d.Id(), err) + return nil + } + + //lintignore:AWSR002 + if err := d.Set("tags", tags.Map()); err != nil { + return fmt.Errorf("error setting tags: %w", err) + } + d.SetId(name) return nil From 77a3a8a920dcc1f76d4c7b826e505d37dc6f0479 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 12:03:16 -0500 Subject: [PATCH 4/6] Update changelog --- .changelog/22544.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.changelog/22544.txt b/.changelog/22544.txt index 0edd0ccaa56..53d87d09665 100644 --- a/.changelog/22544.txt +++ b/.changelog/22544.txt @@ -1,3 +1,7 @@ ```release-note:enhancement - resource/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) - ``` \ No newline at end of file +resource/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` + +```release-note:enhancement +data-source/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` \ No newline at end of file From 87cd1422f9451e7ede6d1acf53b0ca5dde28cf09 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 12:20:58 -0500 Subject: [PATCH 5/6] Edit changelog --- .changelog/22544.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/22544.txt b/.changelog/22544.txt index 53d87d09665..1c242d5146c 100644 --- a/.changelog/22544.txt +++ b/.changelog/22544.txt @@ -3,5 +3,5 @@ resource/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, a ``` ```release-note:enhancement -data-source/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +data-source/aws_iam_role: Allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) ``` \ No newline at end of file From 7ec34ac30aaa97193a05eb7bb6df306e41e8e391 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 12:33:45 -0500 Subject: [PATCH 6/6] iam/user: ISO-friendly tagging --- .changelog/22544.txt | 8 +++++ internal/service/iam/user.go | 43 ++++++++++++++++++++++-- internal/service/iam/user_data_source.go | 14 +++++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/.changelog/22544.txt b/.changelog/22544.txt index 1c242d5146c..892084a0088 100644 --- a/.changelog/22544.txt +++ b/.changelog/22544.txt @@ -4,4 +4,12 @@ resource/aws_iam_role: Attempt `tags`-on-create, fallback to tag after create, a ```release-note:enhancement data-source/aws_iam_role: Allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` + +```release-note:enhancement +resource/aws_iam_user: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` + +```release-note:enhancement +data-source/aws_iam_user: Allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) ``` \ No newline at end of file diff --git a/internal/service/iam/user.go b/internal/service/iam/user.go index 581ea751571..4b33063a7cc 100644 --- a/internal/service/iam/user.go +++ b/internal/service/iam/user.go @@ -6,6 +6,7 @@ import ( "regexp" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -98,12 +99,36 @@ func resourceUserCreate(d *schema.ResourceData, meta interface{}) error { log.Println("[DEBUG] Create IAM User request:", request) createResp, err := conn.CreateUser(request) + + // Some partitions (i.e., ISO) may not support tag-on-create + if request.Tags != nil && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] IAM User (%s) create failed (%s) with tags. Trying create without tags.", d.Get("name").(string), err) + request.Tags = nil + + createResp, err = conn.CreateUser(request) + } + if err != nil { return fmt.Errorf("Error creating IAM User %s: %s", name, err) } d.SetId(aws.StringValue(createResp.User.UserName)) + // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create + if request.Tags == nil && len(tags) > 0 && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID { + err := userUpdateTags(conn, d.Id(), nil, tags) + + // If default tags only, log and continue. Otherwise, error. + if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] error adding tags after create for IAM User (%s): %s", d.Id(), err) + return resourceUserRead(d, meta) + } + + if err != nil { + return fmt.Errorf("error creating IAM User (%s) tags: %w", d.Id(), err) + } + } + return resourceUserRead(d, meta) } @@ -162,6 +187,12 @@ func resourceUserRead(d *schema.ResourceData, meta interface{}) error { tags := KeyValueTags(output.User.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + // Some partitions (i.e., ISO) may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to list tags for IAM User %s: %s", d.Id(), err) + return nil + } + //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { return fmt.Errorf("error setting tags: %w", err) @@ -226,8 +257,16 @@ func resourceUserUpdate(d *schema.ResourceData, meta interface{}) error { if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") - if err := userUpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating IAM User (%s) tags: %s", d.Id(), err) + err := userUpdateTags(conn, d.Id(), o, n) + + // Some partitions may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to update tags for IAM User %s: %s", d.Id(), err) + return resourceUserRead(d, meta) + } + + if err != nil { + return fmt.Errorf("error updating IAM User (%s) tags: %w", d.Id(), err) } } diff --git a/internal/service/iam/user_data_source.go b/internal/service/iam/user_data_source.go index aa0713b5765..89b432256cc 100644 --- a/internal/service/iam/user_data_source.go +++ b/internal/service/iam/user_data_source.go @@ -5,7 +5,9 @@ import ( "log" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/iam" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" @@ -65,7 +67,17 @@ func dataSourceUserRead(d *schema.ResourceData, meta interface{}) error { d.Set("permissions_boundary", user.PermissionsBoundary.PermissionsBoundaryArn) } d.Set("user_id", user.UserId) - if err := d.Set("tags", KeyValueTags(user.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { + + tags := KeyValueTags(user.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && (tfawserr.ErrCodeContains(err, ErrCodeAccessDenied) || tfawserr.ErrCodeContains(err, iam.ErrCodeInvalidInputException) || tfawserr.ErrCodeContains(err, iam.ErrCodeServiceFailureException)) { + log.Printf("[WARN] Unable to list tags for IAM User %s: %s", d.Id(), err) + return nil + } + + //lintignore:AWSR002 + if err := d.Set("tags", tags.Map()); err != nil { return fmt.Errorf("error setting tags: %w", err) }