Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tech debt: Reduce tags boilerplate code - Use createTags; IAM #31341

Merged
merged 30 commits into from
May 11, 2023
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5731e01
IAM: Alphabetize tag update functions.
ewbankkit May 9, 2023
5dadd75
Add 'instanceProfileCreateTags'.
ewbankkit May 9, 2023
430cc14
r/aws_iam_instance_profile: Use 'instanceProfileCreateTags'.
ewbankkit May 9, 2023
e930e30
r/aws_iam_instance_profile: CRUD handlers in the correct order.
ewbankkit May 9, 2023
7253b41
r/aws_iam_instance_profile: Modernize.
ewbankkit May 9, 2023
dcbf4c9
d/aws_iam_instance_profile: Modernize.
ewbankkit May 10, 2023
eb1c5fa
Add 'openIDConnectProviderCreateTags'.
ewbankkit May 10, 2023
65dd07b
r/aws_iam_openid_connect_provider: Use 'openIDConnectProviderCreateTa…
ewbankkit May 10, 2023
7cd6a73
r/aws_iam_instance_profile: Correction.
ewbankkit May 10, 2023
2ab58fe
r/aws_iam_instance_profile: Modernize.
ewbankkit May 10, 2023
e4d1f93
Add 'policyCreateTags'.
ewbankkit May 10, 2023
4b58125
r/aws_iam_policy: Use 'policyCreateTags'.
ewbankkit May 10, 2023
7d04213
r/aws_iam_policy: Modernize.
ewbankkit May 10, 2023
ceb6e64
d/aws_iam_policy: Modernize.
ewbankkit May 10, 2023
8544c79
Add 'roleCreateTags'.
ewbankkit May 10, 2023
0dbed88
r/aws_iam_role: Use 'roleCreateTags'.
ewbankkit May 10, 2023
e97cb72
Add 'samlProviderCreateTags'.
ewbankkit May 10, 2023
b065340
r/aws_iam_saml_provider: Use 'samlProviderCreateTags'.
ewbankkit May 10, 2023
3b3b853
Add 'serverCertificateCreateTags'.
ewbankkit May 11, 2023
6fb652f
r/aws_iam_server_certificate: Use 'serverCertificateCreateTags'.
ewbankkit May 11, 2023
cca7c9b
Add 'userCreateTags'.
ewbankkit May 11, 2023
0dd83b7
r/aws_iam_user: Use 'userCreateTags'.
ewbankkit May 11, 2023
3bd971b
r/aws_iam_user: Modernize.
ewbankkit May 11, 2023
4ed214f
Add 'virtualMFADeviceCreateTags'.
ewbankkit May 11, 2023
e7cecf1
r/aws_iam_virtual_mfa_device: Use 'virtualMFADeviceCreateTags'.
ewbankkit May 11, 2023
9df950e
r/aws_iam_service_linked_role: Better error handling.
ewbankkit May 11, 2023
3d591b3
Fix sweeper compilation error.
ewbankkit May 11, 2023
7a34ca3
Fix semgrep 'ci.semgrep.migrate.direct-CRUD-calls'.
ewbankkit May 11, 2023
2b7f1e1
Revert "Fix semgrep 'ci.semgrep.migrate.direct-CRUD-calls'."
ewbankkit May 11, 2023
52ca245
Fix semgrep 'ci.semgrep.migrate.direct-CRUD-calls'.
ewbankkit May 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/service/iam/instance_profile.go
Original file line number Diff line number Diff line change
@@ -326,6 +326,7 @@ func FindInstanceProfileByName(ctx context.Context, conn *iam.IAM, name string)
LastRequest: input,
}
}

if err != nil {
return nil, err
}
102 changes: 84 additions & 18 deletions internal/service/iam/service_linked_role.go
Original file line number Diff line number Diff line change
@@ -12,10 +12,13 @@ import (
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
multierror "github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
@@ -90,7 +93,6 @@ func resourceServiceLinkedRoleCreate(ctx context.Context, d *schema.ResourceData
conn := meta.(*conns.AWSClient).IAMConn()

serviceName := d.Get("aws_service_name").(string)
tags := GetTagsIn(ctx)
input := &iam.CreateServiceLinkedRoleInput{
AWSServiceName: aws.String(serviceName),
}
@@ -111,23 +113,22 @@ func resourceServiceLinkedRoleCreate(ctx context.Context, d *schema.ResourceData

d.SetId(aws.StringValue(output.Role.Arn))

if len(tags) > 0 {
if tags := GetTagsIn(ctx); len(tags) > 0 {
_, roleName, _, err := DecodeServiceLinkedRoleID(d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "decoding resource ID: %s", err)
return sdkdiag.AppendFromErr(diags, err)
}

err = roleUpdateTags(ctx, conn, roleName, nil, KeyValueTags(ctx, tags))

// If default tags only, log and continue. Otherwise, error.
if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && verify.ErrorISOUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] failed adding tags after create for IAM Service Linked Role (%s): %s", d.Id(), err)
// If default tags only, continue. Otherwise, error.
if v, ok := d.GetOk(names.AttrTags); (!ok || len(v.(map[string]interface{})) == 0) && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) {
return append(diags, resourceServiceLinkedRoleRead(ctx, d, meta)...)
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "creating IAM Service Linked Role (%s): adding tags: %s", serviceName, err)
return sdkdiag.AppendErrorf(diags, "setting IAM Service Linked Role (%s) tags: %s", d.Id(), err)
}
}

@@ -141,7 +142,7 @@ func resourceServiceLinkedRoleRead(ctx context.Context, d *schema.ResourceData,
serviceName, roleName, customSuffix, err := DecodeServiceLinkedRoleID(d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "decoding resource ID: %s", err)
return sdkdiag.AppendFromErr(diags, err)
}

outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) {
@@ -181,7 +182,7 @@ func resourceServiceLinkedRoleUpdate(ctx context.Context, d *schema.ResourceData
_, roleName, _, err := DecodeServiceLinkedRoleID(d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "decoding resource ID: %s", err)
return sdkdiag.AppendFromErr(diags, err)
}

if d.HasChangesExcept("tags_all", "tags") {
@@ -190,7 +191,6 @@ func resourceServiceLinkedRoleUpdate(ctx context.Context, d *schema.ResourceData
RoleName: aws.String(roleName),
}

log.Printf("[DEBUG] Updating IAM Service Linked Role: %s", input)
_, err = conn.UpdateRoleWithContext(ctx, input)

if err != nil {
@@ -203,9 +203,8 @@ func resourceServiceLinkedRoleUpdate(ctx context.Context, d *schema.ResourceData

err := roleUpdateTags(ctx, conn, roleName, o, n)

// If default tags only, log and continue. Otherwise, error.
if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && verify.ErrorISOUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] failed updating tags for IAM Service Linked Role (%s): %s", d.Id(), err)
// Some partitions (e.g. ISO) may not support tagging.
if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) {
return append(diags, resourceServiceLinkedRoleRead(ctx, d, meta)...)
}

@@ -224,10 +223,10 @@ func resourceServiceLinkedRoleDelete(ctx context.Context, d *schema.ResourceData
_, roleName, _, err := DecodeServiceLinkedRoleID(d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "decoding resource ID: %s", err)
return sdkdiag.AppendFromErr(diags, err)
}

log.Printf("[DEBUG] Deleting IAM Service Linked Role: (%s)", d.Id())
log.Printf("[DEBUG] Deleting IAM Service Linked Role: %s", d.Id())
output, err := conn.DeleteServiceLinkedRoleWithContext(ctx, &iam.DeleteServiceLinkedRoleInput{
RoleName: aws.String(roleName),
})
@@ -246,15 +245,82 @@ func resourceServiceLinkedRoleDelete(ctx context.Context, d *schema.ResourceData
return diags
}

err = waitDeleteServiceLinkedRole(ctx, conn, deletionTaskID)

if err != nil {
if err := waitServiceLinkedRoleDeleted(ctx, conn, deletionTaskID); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for IAM Service Linked Role (%s) delete: %s", d.Id(), err)
}

return diags
}

func waitServiceLinkedRoleDeleted(ctx context.Context, conn *iam.IAM, id string) error {
stateConf := &retry.StateChangeConf{
Pending: []string{iam.DeletionTaskStatusTypeInProgress, iam.DeletionTaskStatusTypeNotStarted},
Target: []string{iam.DeletionTaskStatusTypeSucceeded},
Refresh: statusServiceLinkedRoleDeletion(ctx, conn, id),
Timeout: 5 * time.Minute,
Delay: 10 * time.Second,
}

outputRaw, err := stateConf.WaitForStateContext(ctx)

if output, ok := outputRaw.(*iam.GetServiceLinkedRoleDeletionStatusOutput); ok {
if reason := output.Reason; reason != nil {
var errs *multierror.Error

for _, v := range reason.RoleUsageList {
errs = multierror.Append(errs, fmt.Errorf("%s: %s", aws.StringValue(v.Region), strings.Join(aws.StringValueSlice(v.Resources), ", ")))
}

tfresource.SetLastError(err, fmt.Errorf("%s: %w", aws.StringValue(reason.Reason), errs.ErrorOrNil()))
}

return err
}

return err
}

func statusServiceLinkedRoleDeletion(ctx context.Context, conn *iam.IAM, id string) retry.StateRefreshFunc {
return func() (interface{}, string, error) {
output, err := findServiceLinkedRoleDeletionStatusByID(ctx, conn, id)

if tfresource.NotFound(err) {
return nil, "", nil
}

if err != nil {
return nil, "", err
}

return output, aws.StringValue(output.Status), nil
}
}

func findServiceLinkedRoleDeletionStatusByID(ctx context.Context, conn *iam.IAM, id string) (*iam.GetServiceLinkedRoleDeletionStatusOutput, error) {
input := &iam.GetServiceLinkedRoleDeletionStatusInput{
DeletionTaskId: aws.String(id),
}

output, err := conn.GetServiceLinkedRoleDeletionStatusWithContext(ctx, input)

if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return nil, &retry.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

if output == nil {
return nil, tfresource.NewEmptyResultError(input)
}

return output, nil
}

func DecodeServiceLinkedRoleID(id string) (serviceName, roleName, customSuffix string, err error) {
idArn, err := arn.Parse(id)

15 changes: 9 additions & 6 deletions internal/service/iam/service_linked_role_test.go
Original file line number Diff line number Diff line change
@@ -311,6 +311,7 @@ func testAccCheckServiceLinkedRoleDestroy(ctx context.Context) resource.TestChec
}

_, roleName, _, err := tfiam.DecodeServiceLinkedRoleID(rs.Primary.ID)

if err != nil {
return err
}
@@ -340,7 +341,9 @@ func testAccCheckServiceLinkedRoleExists(ctx context.Context, n string) resource
}

conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn()

_, roleName, _, err := tfiam.DecodeServiceLinkedRoleID(rs.Primary.ID)

if err != nil {
return err
}
@@ -354,26 +357,26 @@ func testAccCheckServiceLinkedRoleExists(ctx context.Context, n string) resource
func testAccServiceLinkedRoleConfig_basic(awsServiceName string) string {
return fmt.Sprintf(`
resource "aws_iam_service_linked_role" "test" {
aws_service_name = "%s"
aws_service_name = %[1]q
}
`, awsServiceName)
}

func testAccServiceLinkedRoleConfig_customSuffix(awsServiceName, customSuffix string) string {
return fmt.Sprintf(`
resource "aws_iam_service_linked_role" "test" {
aws_service_name = "%s"
custom_suffix = "%s"
aws_service_name = %[1]q
custom_suffix = %[2]q
}
`, awsServiceName, customSuffix)
}

func testAccServiceLinkedRoleConfig_description(awsServiceName, customSuffix, description string) string {
return fmt.Sprintf(`
resource "aws_iam_service_linked_role" "test" {
aws_service_name = "%s"
custom_suffix = "%s"
description = "%s"
aws_service_name = %[1]q
custom_suffix = %[2]q
description = %[3]q
}
`, awsServiceName, customSuffix, description)
}
36 changes: 0 additions & 36 deletions internal/service/iam/wait.go
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
)
@@ -67,38 +66,3 @@ func statusRoleCreate(ctx context.Context, conn *iam.IAM, id string) retry.State
return role, RoleStatusARNIsUniqueID, nil
}
}

func waitDeleteServiceLinkedRole(ctx context.Context, conn *iam.IAM, deletionTaskID string) error {
stateConf := &retry.StateChangeConf{
Pending: []string{iam.DeletionTaskStatusTypeInProgress, iam.DeletionTaskStatusTypeNotStarted},
Target: []string{iam.DeletionTaskStatusTypeSucceeded},
Refresh: statusDeleteServiceLinkedRole(ctx, conn, deletionTaskID),
Timeout: 5 * time.Minute,
Delay: 10 * time.Second,
}

_, err := stateConf.WaitForStateContext(ctx)
if err != nil {
if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return nil
}
return err
}

return nil
}

func statusDeleteServiceLinkedRole(ctx context.Context, conn *iam.IAM, deletionTaskId string) retry.StateRefreshFunc {
return func() (interface{}, string, error) {
params := &iam.GetServiceLinkedRoleDeletionStatusInput{
DeletionTaskId: aws.String(deletionTaskId),
}

resp, err := conn.GetServiceLinkedRoleDeletionStatusWithContext(ctx, params)
if err != nil {
return nil, "", err
}

return resp, aws.StringValue(resp.Status), nil
}
}