diff --git a/internal/service/iam/find.go b/internal/service/iam/find.go index ab06e0bdd5c6..43f1258a7700 100644 --- a/internal/service/iam/find.go +++ b/internal/service/iam/find.go @@ -79,83 +79,6 @@ func FindUserAttachedPolicy(ctx context.Context, conn *iam.IAM, userName string, return result, nil } -// FindPolicyByARN returns the Policy corresponding to the specified ARN. -func FindPolicyByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.Policy, error) { - input := &iam.GetPolicyInput{ - PolicyArn: aws.String(arn), - } - - output, err := conn.GetPolicyWithContext(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 || output.Policy == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output.Policy, nil -} - -// FindPolicyByName returns the Policy corresponding to the name and/or path-prefix. -func FindPolicyByName(ctx context.Context, conn *iam.IAM, name, pathPrefix string) (*iam.Policy, error) { - input := &iam.ListPoliciesInput{} - if pathPrefix != "" { - input.PathPrefix = aws.String(pathPrefix) - } - - all, err := FindPolicies(ctx, conn, input) - if err != nil { - return nil, err - } - - var results []*iam.Policy - for _, p := range all { - if name != "" && name != aws.StringValue(p.PolicyName) { - continue - } - - results = append(results, p) - } - - if len(results) == 0 || results[0] == nil { - return nil, tfresource.NewEmptyResultError(nil) - } - if l := len(results); l > 1 { - return nil, tfresource.NewTooManyResultsError(1, nil) - } - - return results[0], nil -} - -// FindPolicies returns the Policies matching the parameters in the iam.ListPoliciesInput. -func FindPolicies(ctx context.Context, conn *iam.IAM, input *iam.ListPoliciesInput) ([]*iam.Policy, error) { - var results []*iam.Policy - err := conn.ListPoliciesPagesWithContext(ctx, input, func(page *iam.ListPoliciesOutput, lastPage bool) bool { - if page == nil { - return !lastPage - } - - for _, p := range page.Policies { - if p == nil { - continue - } - - results = append(results, p) - } - - return !lastPage - }) - - return results, err -} - func FindUsers(ctx context.Context, conn *iam.IAM, nameRegex, pathPrefix string) ([]*iam.User, error) { input := &iam.ListUsersInput{} @@ -188,60 +111,6 @@ func FindUsers(ctx context.Context, conn *iam.IAM, nameRegex, pathPrefix string) return results, err } -func FindRoleByName(ctx context.Context, conn *iam.IAM, name string) (*iam.Role, error) { - input := &iam.GetRoleInput{ - RoleName: aws.String(name), - } - - output, err := conn.GetRoleWithContext(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 || output.Role == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output.Role, nil -} - -func FindVirtualMFADevice(ctx context.Context, conn *iam.IAM, serialNum string) (*iam.VirtualMFADevice, error) { - input := &iam.ListVirtualMFADevicesInput{} - - output, err := conn.ListVirtualMFADevicesWithContext(ctx, input) - - if err != nil { - return nil, err - } - - if len(output.VirtualMFADevices) == 0 || output.VirtualMFADevices[0] == nil { - return nil, tfresource.NewEmptyResultError(output) - } - - var device *iam.VirtualMFADevice - - for _, dvs := range output.VirtualMFADevices { - if aws.StringValue(dvs.SerialNumber) == serialNum { - device = dvs - break - } - } - - if device == nil { - return nil, tfresource.NewEmptyResultError(device) - } - - return device, nil -} - func FindServiceSpecificCredential(ctx context.Context, conn *iam.IAM, serviceName, userName, credID string) (*iam.ServiceSpecificCredentialMetadata, error) { input := &iam.ListServiceSpecificCredentialsInput{ ServiceName: aws.String(serviceName), @@ -322,31 +191,6 @@ func FindSigningCertificate(ctx context.Context, conn *iam.IAM, userName, certId return cert, nil } -func FindSAMLProviderByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.GetSAMLProviderOutput, error) { - input := &iam.GetSAMLProviderInput{ - SAMLProviderArn: aws.String(arn), - } - - output, err := conn.GetSAMLProviderWithContext(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 FindAccessKey(ctx context.Context, conn *iam.IAM, username, id string) (*iam.AccessKeyMetadata, error) { accessKeys, err := FindAccessKeys(ctx, conn, username) if err != nil { diff --git a/internal/service/iam/instance_profile.go b/internal/service/iam/instance_profile.go index bc95092e30e7..3a73dcfc1310 100644 --- a/internal/service/iam/instance_profile.go +++ b/internal/service/iam/instance_profile.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" + "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" @@ -92,18 +93,16 @@ func resourceInstanceProfileCreate(ctx context.Context, d *schema.ResourceData, conn := meta.(*conns.AWSClient).IAMConn() name := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) - tags := GetTagsIn(ctx) input := &iam.CreateInstanceProfileInput{ InstanceProfileName: aws.String(name), Path: aws.String(d.Get("path").(string)), - Tags: tags, + Tags: GetTagsIn(ctx), } output, err := conn.CreateInstanceProfileWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM Instance Profile (%s) with tags: %s. Trying create without tags.", name, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = conn.CreateInstanceProfileWithContext(ctx, input) @@ -113,89 +112,87 @@ func resourceInstanceProfileCreate(ctx context.Context, d *schema.ResourceData, return sdkdiag.AppendErrorf(diags, "creating IAM Instance Profile (%s): %s", name, err) } - flattenInstanceProfile(ctx, d, output.InstanceProfile) // sets id + d.SetId(aws.StringValue(output.InstanceProfile.InstanceProfileName)) - waiterRequest := &iam.GetInstanceProfileInput{ - InstanceProfileName: aws.String(name), - } - // don't return until the IAM service reports that the instance profile is ready. - // this ensures that terraform resources which rely on the instance profile will 'see' - // that the instance profile exists. - err = conn.WaitUntilInstanceProfileExistsWithContext(ctx, waiterRequest) + _, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) { + return FindInstanceProfileByName(ctx, conn, d.Id()) + }) if err != nil { - return sdkdiag.AppendErrorf(diags, "timed out while waiting for instance profile %s: %s", name, err) + return sdkdiag.AppendErrorf(diags, "waiting for IAM Instance Profile (%s) create: %s", d.Id(), err) } - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 { - err := instanceProfileUpdateTags(ctx, conn, d.Id(), 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 Instance Profile (%s): %s", d.Id(), err) - return append(diags, resourceInstanceProfileUpdate(ctx, d, meta)...) - } + if v, ok := d.GetOk("role"); ok { + err := instanceProfileAddRole(ctx, conn, d.Id(), v.(string)) if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM Instance Profile (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } } - return append(diags, resourceInstanceProfileUpdate(ctx, d, meta)...) -} + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := instanceProfileCreateTags(ctx, conn, d.Id(), tags) -func instanceProfileAddRole(ctx context.Context, conn *iam.IAM, profileName, roleName string) error { - request := &iam.AddRoleToInstanceProfileInput{ - InstanceProfileName: aws.String(profileName), - RoleName: aws.String(roleName), - } - - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - _, err := conn.AddRoleToInstanceProfileWithContext(ctx, request) - // IAM unfortunately does not provide a better error code or message for eventual consistency - // InvalidParameterValue: Value (XXX) for parameter iamInstanceProfile.name is invalid. Invalid IAM Instance Profile name - // NoSuchEntity: The request was rejected because it referenced an entity that does not exist. The error message describes the entity. HTTP Status Code: 404 - if tfawserr.ErrMessageContains(err, "InvalidParameterValue", "Invalid IAM Instance Profile name") || tfawserr.ErrMessageContains(err, iam.ErrCodeNoSuchEntityException, "The role with name") { - return retry.RetryableError(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, resourceInstanceProfileRead(ctx, d, meta)...) } + if err != nil { - return retry.NonRetryableError(err) + return sdkdiag.AppendErrorf(diags, "setting IAM Instance Profile (%s) tags: %s", d.Id(), err) } - return nil - }) - if tfresource.TimedOut(err) { - _, err = conn.AddRoleToInstanceProfileWithContext(ctx, request) - } - if err != nil { - return fmt.Errorf("adding IAM Role %s to Instance Profile %s: %w", roleName, profileName, err) } - return err + return append(diags, resourceInstanceProfileRead(ctx, d, meta)...) } -func instanceProfileRemoveRole(ctx context.Context, conn *iam.IAM, profileName, roleName string) error { - request := &iam.RemoveRoleFromInstanceProfileInput{ - InstanceProfileName: aws.String(profileName), - RoleName: aws.String(roleName), +func resourceInstanceProfileRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).IAMConn() + + instanceProfile, err := FindInstanceProfileByName(ctx, conn, d.Id()) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] IAM Instance Profile (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags } - _, err := conn.RemoveRoleFromInstanceProfileWithContext(ctx, request) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return nil + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading IAM Instance Profile (%s): %s", d.Id(), err) } - return err -} -func instanceProfileRemoveAllRoles(ctx context.Context, d *schema.ResourceData, conn *iam.IAM) error { - if role, ok := d.GetOk("role"); ok { - err := instanceProfileRemoveRole(ctx, conn, d.Id(), role.(string)) + if len(instanceProfile.Roles) > 0 { + roleName := aws.StringValue(instanceProfile.Roles[0].RoleName) + _, err := FindRoleByName(ctx, conn, roleName) + if err != nil { - return fmt.Errorf("removing role (%s): %w", role, err) + if tfresource.NotFound(err) { + err := instanceProfileRemoveRole(ctx, conn, d.Id(), roleName) + + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + } + + return sdkdiag.AppendErrorf(diags, "reading IAM Role (%s) attached to IAM Instance Profile (%s): %s", roleName, d.Id(), err) } } - return nil + d.Set("arn", instanceProfile.Arn) + d.Set("create_date", instanceProfile.CreateDate.Format(time.RFC3339)) + d.Set("name", instanceProfile.InstanceProfileName) + d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(instanceProfile.InstanceProfileName))) + d.Set("path", instanceProfile.Path) + if len(instanceProfile.Roles) > 0 { + d.Set("role", instanceProfile.Roles[0].RoleName) //there will only be 1 role returned + } + d.Set("unique_id", instanceProfile.InstanceProfileId) + + SetTagsOut(ctx, instanceProfile.Tags) + + return diags } func resourceInstanceProfileUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { @@ -203,19 +200,21 @@ func resourceInstanceProfileUpdate(ctx context.Context, d *schema.ResourceData, conn := meta.(*conns.AWSClient).IAMConn() if d.HasChange("role") { - oldRole, newRole := d.GetChange("role") + o, n := d.GetChange("role") + + if o := o.(string); o != "" { + err := instanceProfileRemoveRole(ctx, conn, d.Id(), o) - if oldRole.(string) != "" { - err := instanceProfileRemoveRole(ctx, conn, d.Id(), oldRole.(string)) if err != nil { - return sdkdiag.AppendErrorf(diags, "removing role %s to IAM Instance Profile (%s): %s", oldRole.(string), d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } } - if newRole.(string) != "" { - err := instanceProfileAddRole(ctx, conn, d.Id(), newRole.(string)) + if n := n.(string); n != "" { + err := instanceProfileAddRole(ctx, conn, d.Id(), n) + if err != nil { - return sdkdiag.AppendErrorf(diags, "adding role %s to IAM Instance Profile (%s): %s", newRole.(string), d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } } } @@ -225,10 +224,9 @@ func resourceInstanceProfileUpdate(ctx context.Context, d *schema.ResourceData, err := instanceProfileUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM Instance Profile (%s): %s", d.Id(), err) - return diags + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { + return append(diags, resourceInstanceProfileRead(ctx, d, meta)...) } if err != nil { @@ -239,82 +237,103 @@ func resourceInstanceProfileUpdate(ctx context.Context, d *schema.ResourceData, return append(diags, resourceInstanceProfileRead(ctx, d, meta)...) } -func resourceInstanceProfileRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourceInstanceProfileDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - request := &iam.GetInstanceProfileInput{ - InstanceProfileName: aws.String(d.Id()), + if v, ok := d.GetOk("role"); ok { + err := instanceProfileRemoveRole(ctx, conn, d.Id(), v.(string)) + + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } } - result, err := conn.GetInstanceProfileWithContext(ctx, request) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM Instance Profile (%s) not found, removing from state", d.Id()) - d.SetId("") + log.Printf("[INFO] Deleting IAM Instance Profile: %s", d.Id()) + _, err := conn.DeleteInstanceProfileWithContext(ctx, &iam.DeleteInstanceProfileInput{ + InstanceProfileName: aws.String(d.Id()), + }) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { return diags } + if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Instance Profile (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting IAM Instance Profile (%s): %s", d.Id(), err) } - instanceProfile := result.InstanceProfile - if instanceProfile.Roles != nil && len(instanceProfile.Roles) > 0 { - roleName := aws.StringValue(instanceProfile.Roles[0].RoleName) - input := &iam.GetRoleInput{ - RoleName: aws.String(roleName), - } + return diags +} - _, err := conn.GetRoleWithContext(ctx, input) - if err != nil { - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - err := instanceProfileRemoveRole(ctx, conn, d.Id(), roleName) - if err != nil { - return sdkdiag.AppendErrorf(diags, "removing role %s to IAM Instance Profile (%s): %s", roleName, d.Id(), err) - } - } - return sdkdiag.AppendErrorf(diags, "reading IAM Role %s attached to IAM Instance Profile (%s): %s", roleName, d.Id(), err) - } +func instanceProfileAddRole(ctx context.Context, conn *iam.IAM, profileName, roleName string) error { + input := &iam.AddRoleToInstanceProfileInput{ + InstanceProfileName: aws.String(profileName), + RoleName: aws.String(roleName), } - flattenInstanceProfile(ctx, d, instanceProfile) + _, err := tfresource.RetryWhen(ctx, propagationTimeout, + func() (interface{}, error) { + return conn.AddRoleToInstanceProfileWithContext(ctx, input) + }, + func(err error) (bool, error) { + // IAM unfortunately does not provide a better error code or message for eventual consistency + // InvalidParameterValue: Value (XXX) for parameter iamInstanceProfile.name is invalid. Invalid IAM Instance Profile name + // NoSuchEntity: The request was rejected because it referenced an entity that does not exist. The error message describes the entity. HTTP Status Code: 404 + if tfawserr.ErrMessageContains(err, "InvalidParameterValue", "Invalid IAM Instance Profile name") || tfawserr.ErrMessageContains(err, iam.ErrCodeNoSuchEntityException, "The role with name") { + return true, err + } - return diags -} + return false, err + }, + ) -func resourceInstanceProfileDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).IAMConn() + if err != nil { + return fmt.Errorf("adding IAM Role (%s) to IAM Instance Profile (%s): %w", roleName, profileName, err) + } - if err := instanceProfileRemoveAllRoles(ctx, d, conn); err != nil { - return sdkdiag.AppendErrorf(diags, "deleting IAM Instance Profile (%s): %s", d.Id(), err) + return nil +} + +func instanceProfileRemoveRole(ctx context.Context, conn *iam.IAM, profileName, roleName string) error { + input := &iam.RemoveRoleFromInstanceProfileInput{ + InstanceProfileName: aws.String(profileName), + RoleName: aws.String(roleName), } - request := &iam.DeleteInstanceProfileInput{ - InstanceProfileName: aws.String(d.Id()), + _, err := conn.RemoveRoleFromInstanceProfileWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil } - _, err := conn.DeleteInstanceProfileWithContext(ctx, request) + if err != nil { - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return diags - } - return sdkdiag.AppendErrorf(diags, "deleting IAM Instance Profile (%s): %s", d.Id(), err) + return fmt.Errorf("removing IAM Role (%s) from IAM Instance Profile (%s): %w", roleName, profileName, err) } - return diags + return nil } -func flattenInstanceProfile(ctx context.Context, d *schema.ResourceData, result *iam.InstanceProfile) { - d.SetId(aws.StringValue(result.InstanceProfileName)) - d.Set("arn", result.Arn) - d.Set("create_date", result.CreateDate.Format(time.RFC3339)) - d.Set("name", result.InstanceProfileName) - d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(result.InstanceProfileName))) - d.Set("path", result.Path) - d.Set("unique_id", result.InstanceProfileId) - - if result.Roles != nil && len(result.Roles) > 0 { - d.Set("role", result.Roles[0].RoleName) //there will only be 1 role returned +func FindInstanceProfileByName(ctx context.Context, conn *iam.IAM, name string) (*iam.InstanceProfile, error) { + input := &iam.GetInstanceProfileInput{ + InstanceProfileName: aws.String(name), + } + + output, err := conn.GetInstanceProfileWithContext(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 || output.InstanceProfile == nil { + return nil, tfresource.NewEmptyResultError(input) } - SetTagsOut(ctx, result.Tags) + return output.InstanceProfile, nil } diff --git a/internal/service/iam/instance_profile_data_source.go b/internal/service/iam/instance_profile_data_source.go index 00cca5d10e25..3e148f91435f 100644 --- a/internal/service/iam/instance_profile_data_source.go +++ b/internal/service/iam/instance_profile_data_source.go @@ -3,10 +3,8 @@ package iam import ( "context" "fmt" - "log" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/iam" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -56,27 +54,16 @@ func dataSourceInstanceProfileRead(ctx context.Context, d *schema.ResourceData, conn := meta.(*conns.AWSClient).IAMConn() name := d.Get("name").(string) + instanceProfile, err := FindInstanceProfileByName(ctx, conn, name) - req := &iam.GetInstanceProfileInput{ - InstanceProfileName: aws.String(name), - } - - log.Printf("[DEBUG] Reading IAM Instance Profile: %s", req) - resp, err := conn.GetInstanceProfileWithContext(ctx, req) if err != nil { - return sdkdiag.AppendErrorf(diags, "getting instance profiles: %s", err) - } - if resp == nil { - return sdkdiag.AppendErrorf(diags, "no IAM instance profile found") + return sdkdiag.AppendErrorf(diags, "reading IAM Instance Profile (%s): %s", name, err) } - instanceProfile := resp.InstanceProfile - d.SetId(aws.StringValue(instanceProfile.InstanceProfileId)) d.Set("arn", instanceProfile.Arn) d.Set("create_date", fmt.Sprintf("%v", instanceProfile.CreateDate)) d.Set("path", instanceProfile.Path) - if len(instanceProfile.Roles) > 0 { role := instanceProfile.Roles[0] d.Set("role_arn", role.Arn) diff --git a/internal/service/iam/instance_profile_data_source_test.go b/internal/service/iam/instance_profile_data_source_test.go index bd36c4bd2a39..3e639c929827 100644 --- a/internal/service/iam/instance_profile_data_source_test.go +++ b/internal/service/iam/instance_profile_data_source_test.go @@ -12,10 +12,11 @@ import ( func TestAccIAMInstanceProfileDataSource_basic(t *testing.T) { ctx := acctest.Context(t) - resourceName := "data.aws_iam_instance_profile.test" - - roleName := fmt.Sprintf("tf-acc-ds-instance-profile-role-%d", sdkacctest.RandInt()) - profileName := fmt.Sprintf("tf-acc-ds-instance-profile-%d", sdkacctest.RandInt()) + dataSourceName := "data.aws_iam_instance_profile.test" + resourceName := "aws_iam_instance_profile.test" + roleResourceName := "aws_iam_role.test" + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -23,13 +24,13 @@ func TestAccIAMInstanceProfileDataSource_basic(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { - Config: testAccInstanceProfileDataSourceConfig_basic(roleName, profileName), + Config: testAccInstanceProfileDataSourceConfig_basic(rName1, rName2), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrPair(resourceName, "arn", "aws_iam_instance_profile.test", "arn"), - resource.TestCheckResourceAttr(resourceName, "path", "/testpath/"), - resource.TestCheckResourceAttrPair(resourceName, "role_arn", "aws_iam_role.test", "arn"), - resource.TestCheckResourceAttrPair(resourceName, "role_id", "aws_iam_role.test", "unique_id"), - resource.TestCheckResourceAttr(resourceName, "role_name", roleName), + resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), + resource.TestCheckResourceAttr(dataSourceName, "path", "/testpath/"), + resource.TestCheckResourceAttrPair(dataSourceName, "role_arn", roleResourceName, "arn"), + resource.TestCheckResourceAttrPair(dataSourceName, "role_id", roleResourceName, "unique_id"), + resource.TestCheckResourceAttr(dataSourceName, "role_name", rName1), ), }, }, @@ -39,12 +40,12 @@ func TestAccIAMInstanceProfileDataSource_basic(t *testing.T) { func testAccInstanceProfileDataSourceConfig_basic(roleName, profileName string) string { return fmt.Sprintf(` resource "aws_iam_role" "test" { - name = "%s" + name = %[1]q assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" } resource "aws_iam_instance_profile" "test" { - name = "%s" + name = %[2]q role = aws_iam_role.test.name path = "/testpath/" } diff --git a/internal/service/iam/instance_profile_test.go b/internal/service/iam/instance_profile_test.go index 9b72735efb4f..f9c0cbd7d854 100644 --- a/internal/service/iam/instance_profile_test.go +++ b/internal/service/iam/instance_profile_test.go @@ -3,25 +3,24 @@ package iam_test import ( "context" "fmt" - "strings" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/id" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMInstanceProfile_basic(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -33,9 +32,9 @@ func TestAccIAMInstanceProfile_basic(t *testing.T) { Config: testAccInstanceProfileConfig_basic(rName), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceProfileExists(ctx, resourceName, &conf), - acctest.CheckResourceAttrGlobalARN(resourceName, "arn", "iam", fmt.Sprintf("instance-profile/test-%s", rName)), + acctest.CheckResourceAttrGlobalARN(resourceName, "arn", "iam", fmt.Sprintf("instance-profile/%s", rName)), resource.TestCheckResourceAttrPair(resourceName, "role", "aws_iam_role.test", "name"), - resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("test-%s", rName)), + resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, @@ -50,9 +49,10 @@ func TestAccIAMInstanceProfile_basic(t *testing.T) { func TestAccIAMInstanceProfile_withoutRole(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), @@ -63,13 +63,13 @@ func TestAccIAMInstanceProfile_withoutRole(t *testing.T) { Config: testAccInstanceProfileConfig_noRole(rName), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceProfileExists(ctx, resourceName, &conf), + resource.TestCheckNoResourceAttr(resourceName, "role"), ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"name_prefix"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -77,9 +77,9 @@ func TestAccIAMInstanceProfile_withoutRole(t *testing.T) { func TestAccIAMInstanceProfile_tags(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -96,10 +96,9 @@ func TestAccIAMInstanceProfile_tags(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"name_prefix"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, { Config: testAccInstanceProfileConfig_tags2(rName, "key1", "value1updated", "key2", "value2"), @@ -122,11 +121,40 @@ func TestAccIAMInstanceProfile_tags(t *testing.T) { }) } +func TestAccIAMInstanceProfile_nameGenerated(t *testing.T) { + ctx := acctest.Context(t) + var conf iam.InstanceProfile + resourceName := "aws_iam_instance_profile.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckInstanceProfileDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccInstanceProfileConfig_nameGenerated(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceProfileExists(ctx, resourceName, &conf), + acctest.CheckResourceAttrNameGenerated(resourceName, "name"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", id.UniqueIdPrefix), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccIAMInstanceProfile_namePrefix(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput - rName := sdkacctest.RandString(5) + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -135,18 +163,17 @@ func TestAccIAMInstanceProfile_namePrefix(t *testing.T) { CheckDestroy: testAccCheckInstanceProfileDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccInstanceProfileConfig_prefixName(rName), + Config: testAccInstanceProfileConfig_namePrefix(rName, "tf-acc-test-prefix-"), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceProfileExists(ctx, resourceName, &conf), - testAccCheckInstanceProfileGeneratedNamePrefix( - resourceName, "test-"), + acctest.CheckResourceAttrNameFromPrefix(resourceName, "name", "tf-acc-test-prefix-"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", "tf-acc-test-prefix-"), ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"name_prefix"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -154,9 +181,9 @@ func TestAccIAMInstanceProfile_namePrefix(t *testing.T) { func TestAccIAMInstanceProfile_disappears(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -178,9 +205,9 @@ func TestAccIAMInstanceProfile_disappears(t *testing.T) { func TestAccIAMInstanceProfile_Disappears_role(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetInstanceProfileOutput + var conf iam.InstanceProfile resourceName := "aws_iam_instance_profile.test" - rName := sdkacctest.RandString(5) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -200,23 +227,6 @@ func TestAccIAMInstanceProfile_Disappears_role(t *testing.T) { }) } -func testAccCheckInstanceProfileGeneratedNamePrefix(resource, prefix string) resource.TestCheckFunc { - return func(s *terraform.State) error { - r, ok := s.RootModule().Resources[resource] - if !ok { - return fmt.Errorf("Resource not found") - } - name, ok := r.Primary.Attributes["name"] - if !ok { - return fmt.Errorf("Name attr not found: %#v", r.Primary.Attributes) - } - if !strings.HasPrefix(name, prefix) { - return fmt.Errorf("Name: %q, does not have prefix: %q", name, prefix) - } - return nil - } -} - func testAccCheckInstanceProfileDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() @@ -226,26 +236,24 @@ func testAccCheckInstanceProfileDestroy(ctx context.Context) resource.TestCheckF continue } - // Try to get role - _, err := conn.GetInstanceProfileWithContext(ctx, &iam.GetInstanceProfileInput{ - InstanceProfileName: aws.String(rs.Primary.ID), - }) - if err == nil { - return fmt.Errorf("still exist.") - } + _, err := tfiam.FindInstanceProfileByName(ctx, conn, rs.Primary.ID) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if tfresource.NotFound(err) { continue } - return err + if err != nil { + return err + } + + return fmt.Errorf("IAM Instance Profile %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckInstanceProfileExists(ctx context.Context, n string, res *iam.GetInstanceProfileOutput) resource.TestCheckFunc { +func testAccCheckInstanceProfileExists(ctx context.Context, n string, v *iam.InstanceProfile) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -253,28 +261,27 @@ func testAccCheckInstanceProfileExists(ctx context.Context, n string, res *iam.G } if rs.Primary.ID == "" { - return fmt.Errorf("No Instance Profile name is set") + return fmt.Errorf("No IAM Instance Profile ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - resp, err := conn.GetInstanceProfileWithContext(ctx, &iam.GetInstanceProfileInput{ - InstanceProfileName: aws.String(rs.Primary.ID), - }) + output, err := tfiam.FindInstanceProfileByName(ctx, conn, rs.Primary.ID) + if err != nil { return err } - *res = *resp + *v = *output return nil } } -func testAccInstanceProfileBaseConfig(rName string) string { +func testAccInstanceProfileConfig_base(rName string) string { return fmt.Sprintf(` resource "aws_iam_role" "test" { - name = "test-%s" + name = "%[1]s-role" assume_role_policy = < 0 { - err := openIDConnectProviderUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := openIDConnectProviderCreateTags(ctx, conn, d.Id(), 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 OIDC Provider (%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, resourceOpenIDConnectProviderRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM OIDC Provider (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM OIDC Provider (%s) tags: %s", d.Id(), err) } } @@ -118,23 +118,22 @@ func resourceOpenIDConnectProviderRead(ctx context.Context, d *schema.ResourceDa var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - input := &iam.GetOpenIDConnectProviderInput{ - OpenIDConnectProviderArn: aws.String(d.Id()), - } - output, err := conn.GetOpenIDConnectProviderWithContext(ctx, input) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + output, err := FindOpenIDConnectProviderByARN(ctx, conn, d.Id()) + + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] IAM OIDC Provider (%s) not found, removing from state", d.Id()) d.SetId("") return diags } + if err != nil { return sdkdiag.AppendErrorf(diags, "reading IAM OIDC Provider (%s): %s", d.Id(), err) } d.Set("arn", d.Id()) - d.Set("url", output.Url) d.Set("client_id_list", aws.StringValueSlice(output.ClientIDList)) d.Set("thumbprint_list", aws.StringValueSlice(output.ThumbprintList)) + d.Set("url", output.Url) SetTagsOut(ctx, output.Tags) @@ -152,6 +151,7 @@ func resourceOpenIDConnectProviderUpdate(ctx context.Context, d *schema.Resource } _, err := conn.UpdateOpenIDConnectProviderThumbprintWithContext(ctx, input) + if err != nil { return sdkdiag.AppendErrorf(diags, "updating IAM OIDC Provider (%s) thumbprint: %s", d.Id(), err) } @@ -162,9 +162,8 @@ func resourceOpenIDConnectProviderUpdate(ctx context.Context, d *schema.Resource err := openIDConnectProviderUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM OIDC Provider (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourceOpenIDConnectProviderRead(ctx, d, meta)...) } @@ -180,16 +179,42 @@ func resourceOpenIDConnectProviderDelete(ctx context.Context, d *schema.Resource var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - input := &iam.DeleteOpenIDConnectProviderInput{ + log.Printf("[INFO] Deleting IAM OIDC Provider: %s", d.Id()) + _, err := conn.DeleteOpenIDConnectProviderWithContext(ctx, &iam.DeleteOpenIDConnectProviderInput{ OpenIDConnectProviderArn: aws.String(d.Id()), - } - _, err := conn.DeleteOpenIDConnectProviderWithContext(ctx, input) + }) + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { return diags } + if err != nil { return sdkdiag.AppendErrorf(diags, "deleting IAM OIDC Provider (%s): %s", d.Id(), err) } return diags } + +func FindOpenIDConnectProviderByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.GetOpenIDConnectProviderOutput, error) { + input := &iam.GetOpenIDConnectProviderInput{ + OpenIDConnectProviderArn: aws.String(arn), + } + + output, err := conn.GetOpenIDConnectProviderWithContext(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 +} diff --git a/internal/service/iam/openid_connect_provider_data_source_test.go b/internal/service/iam/openid_connect_provider_data_source_test.go index a5af5eace5a4..abcc55231e72 100644 --- a/internal/service/iam/openid_connect_provider_data_source_test.go +++ b/internal/service/iam/openid_connect_provider_data_source_test.go @@ -25,7 +25,7 @@ func TestAccIAMOpenidConnectProviderDataSource_basic(t *testing.T) { { Config: testAccOpenIDConnectProviderDataSourceConfig_basic(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), resource.TestCheckResourceAttrPair(dataSourceName, "url", resourceName, "url"), resource.TestCheckResourceAttrPair(dataSourceName, "client_id_list", resourceName, "client_id_list"), @@ -52,7 +52,7 @@ func TestAccIAMOpenidConnectProviderDataSource_url(t *testing.T) { { Config: testAccOpenIDConnectProviderDataSourceConfig_url(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), resource.TestCheckResourceAttrPair(dataSourceName, "url", resourceName, "url"), resource.TestCheckResourceAttrPair(dataSourceName, "client_id_list", resourceName, "client_id_list"), @@ -79,7 +79,7 @@ func TestAccIAMOpenidConnectProviderDataSource_tags(t *testing.T) { { Config: testAccOpenIDConnectProviderDataSourceConfig_tags(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), resource.TestCheckResourceAttrPair(dataSourceName, "url", resourceName, "url"), resource.TestCheckResourceAttrPair(dataSourceName, "client_id_list", resourceName, "client_id_list"), diff --git a/internal/service/iam/openid_connect_provider_test.go b/internal/service/iam/openid_connect_provider_test.go index 114dad4ba867..da4f3596891d 100644 --- a/internal/service/iam/openid_connect_provider_test.go +++ b/internal/service/iam/openid_connect_provider_test.go @@ -5,15 +5,14 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMOpenIDConnectProvider_basic(t *testing.T) { @@ -31,7 +30,7 @@ func TestAccIAMOpenIDConnectProvider_basic(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_basic(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), acctest.CheckResourceAttrGlobalARN(resourceName, "arn", "iam", fmt.Sprintf("oidc-provider/%s", url)), resource.TestCheckResourceAttr(resourceName, "url", url), resource.TestCheckResourceAttr(resourceName, "client_id_list.#", "1"), @@ -49,7 +48,7 @@ func TestAccIAMOpenIDConnectProvider_basic(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_modified(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "url", url), resource.TestCheckResourceAttr(resourceName, "client_id_list.#", "1"), resource.TestCheckResourceAttr(resourceName, "client_id_list.0", @@ -78,7 +77,7 @@ func TestAccIAMOpenIDConnectProvider_tags(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_tags1(rString, "key1", "value1"), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), ), @@ -92,7 +91,7 @@ func TestAccIAMOpenIDConnectProvider_tags(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_tags2(rString, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), @@ -101,7 +100,7 @@ func TestAccIAMOpenIDConnectProvider_tags(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_tags1(rString, "key2", "value2"), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), @@ -124,7 +123,7 @@ func TestAccIAMOpenIDConnectProvider_disappears(t *testing.T) { { Config: testAccOpenIDConnectProviderConfig_basic(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceOpenIDConnectProvider(), resourceName), ), ExpectNonEmptyPlan: true, @@ -133,7 +132,7 @@ func TestAccIAMOpenIDConnectProvider_disappears(t *testing.T) { }) } -func TestAccIAMOpenIDConnectProvider_clientIdListOrder(t *testing.T) { +func TestAccIAMOpenIDConnectProvider_clientIDListOrder(t *testing.T) { ctx := acctest.Context(t) rString := sdkacctest.RandString(5) resourceName := "aws_iam_openid_connect_provider.test" @@ -145,15 +144,15 @@ func TestAccIAMOpenIDConnectProvider_clientIdListOrder(t *testing.T) { CheckDestroy: testAccCheckOpenIDConnectProviderDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccOpenIDConnectProviderConfig_clientIdList_first(rString), + Config: testAccOpenIDConnectProviderConfig_clientIDList_first(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), ), }, { - Config: testAccOpenIDConnectProviderConfig_clientIdList_second(rString), + Config: testAccOpenIDConnectProviderConfig_clientIDList_second(rString), Check: resource.ComposeTestCheckFunc( - testAccCheckOpenIDConnectProvider(ctx, resourceName), + testAccCheckOpenIDConnectProviderExists(ctx, resourceName), ), ExpectNonEmptyPlan: false, // Expect an empty plan as only the order has been changed PlanOnly: true, // Expect an empty plan as only the order has been changed @@ -171,51 +170,46 @@ func testAccCheckOpenIDConnectProviderDestroy(ctx context.Context) resource.Test continue } - input := &iam.GetOpenIDConnectProviderInput{ - OpenIDConnectProviderArn: aws.String(rs.Primary.ID), + _, err := tfiam.FindOpenIDConnectProviderByARN(ctx, conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue } - out, err := conn.GetOpenIDConnectProviderWithContext(ctx, input) + if err != nil { - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - // none found, that's good - return nil - } - return fmt.Errorf("Error reading IAM OpenID Connect Provider, out: %s, err: %w", out, err) + return err } - if out != nil { - return fmt.Errorf("Found IAM OpenID Connect Provider, expected none: %s", out) - } + return fmt.Errorf("IAM OIDC Provider %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckOpenIDConnectProvider(ctx context.Context, id string) resource.TestCheckFunc { +func testAccCheckOpenIDConnectProviderExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[id] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not Found: %s", id) + return fmt.Errorf("Not Found: %s", n) } if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") + return fmt.Errorf("No IAM OIDC Provider ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - _, err := conn.GetOpenIDConnectProviderWithContext(ctx, &iam.GetOpenIDConnectProviderInput{ - OpenIDConnectProviderArn: aws.String(rs.Primary.ID), - }) + + _, err := tfiam.FindOpenIDConnectProviderByARN(ctx, conn, rs.Primary.ID) return err } } -func testAccOpenIDConnectProviderConfig_basic(rString string) string { +func testAccOpenIDConnectProviderConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "266362248691-re108qaeld573ia0l6clj2i5ac7r7291.apps.testleusercontent.com", @@ -223,13 +217,13 @@ resource "aws_iam_openid_connect_provider" "test" { thumbprint_list = ["cf23df2207d99a74fbe169e3eba035e633b65d94"] } -`, rString) +`, rName) } -func testAccOpenIDConnectProviderConfig_modified(rString string) string { +func testAccOpenIDConnectProviderConfig_modified(rName string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "266362248691-re108qaeld573ia0l6clj2i5ac7r7291.apps.testleusercontent.com", @@ -237,13 +231,13 @@ resource "aws_iam_openid_connect_provider" "test" { thumbprint_list = ["cf23df2207d99a74fbe169e3eba035e633b65d94", "c784713d6f9cb67b55dd84f4e4af7832d42b8f55"] } -`, rString) +`, rName) } -func testAccOpenIDConnectProviderConfig_tags1(rString, tagKey1, tagValue1 string) string { +func testAccOpenIDConnectProviderConfig_tags1(rName, tagKey1, tagValue1 string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "266362248691-re108qaeld573ia0l6clj2i5ac7r7291.apps.testleusercontent.com", @@ -255,13 +249,13 @@ resource "aws_iam_openid_connect_provider" "test" { %[2]q = %[3]q } } -`, rString, tagKey1, tagValue1) +`, rName, tagKey1, tagValue1) } -func testAccOpenIDConnectProviderConfig_tags2(rString, tagKey1, tagValue1, tagKey2, tagValue2 string) string { +func testAccOpenIDConnectProviderConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "266362248691-re108qaeld573ia0l6clj2i5ac7r7291.apps.testleusercontent.com", @@ -274,13 +268,13 @@ resource "aws_iam_openid_connect_provider" "test" { %[4]q = %[5]q } } -`, rString, tagKey1, tagValue1, tagKey2, tagValue2) +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) } -func testAccOpenIDConnectProviderConfig_clientIdList_first(rString string) string { +func testAccOpenIDConnectProviderConfig_clientIDList_first(rName string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "abc.testle.com", @@ -290,13 +284,13 @@ resource "aws_iam_openid_connect_provider" "test" { thumbprint_list = ["oif8192f189fa2178f-testle.thumbprint.com"] } -`, rString) +`, rName) } -func testAccOpenIDConnectProviderConfig_clientIdList_second(rString string) string { +func testAccOpenIDConnectProviderConfig_clientIDList_second(rName string) string { return fmt.Sprintf(` resource "aws_iam_openid_connect_provider" "test" { - url = "https://accounts.testle.com/%s" + url = "https://accounts.testle.com/%[1]s" client_id_list = [ "def.testle.com", @@ -306,5 +300,5 @@ resource "aws_iam_openid_connect_provider" "test" { thumbprint_list = ["oif8192f189fa2178f-testle.thumbprint.com"] } -`, rString) +`, rName) } diff --git a/internal/service/iam/policy.go b/internal/service/iam/policy.go index 8abee2cf3ba1..961db0e4d571 100644 --- a/internal/service/iam/policy.go +++ b/internal/service/iam/policy.go @@ -16,7 +16,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" + "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/slices" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" @@ -106,43 +108,40 @@ func resourcePolicyCreate(ctx context.Context, d *schema.ResourceData, meta inte } name := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) - tags := GetTagsIn(ctx) input := &iam.CreatePolicyInput{ Description: aws.String(d.Get("description").(string)), Path: aws.String(d.Get("path").(string)), PolicyDocument: aws.String(policy), PolicyName: aws.String(name), - Tags: tags, + Tags: GetTagsIn(ctx), } output, err := conn.CreatePolicyWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM Policy (%s) with tags: %s. Trying create without tags.", name, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = conn.CreatePolicyWithContext(ctx, input) } if err != nil { - return sdkdiag.AppendErrorf(diags, "creating IAM Policy %s: %s", name, err) + return sdkdiag.AppendErrorf(diags, "creating IAM Policy (%s): %s", name, err) } d.SetId(aws.StringValue(output.Policy.Arn)) - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 { - err := policyUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := policyCreateTags(ctx, conn, d.Id(), 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 Policy (%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, resourcePolicyRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM Policy (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM Policy (%s) tags: %s", d.Id(), err) } } @@ -153,48 +152,40 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - input := &iam.GetPolicyInput{ - PolicyArn: aws.String(d.Id()), + type policyWithVersion struct { + policy *iam.Policy + policyVersion *iam.PolicyVersion } + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { + iamPolicy := &policyWithVersion{} - // Handle IAM eventual consistency - var getPolicyResponse *iam.GetPolicyOutput - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - getPolicyResponse, err = conn.GetPolicyWithContext(ctx, input) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) + if v, err := FindPolicyByARN(ctx, conn, d.Id()); err == nil { + iamPolicy.policy = v + } else { + return nil, err } - if err != nil { - return retry.NonRetryableError(err) + if v, err := findPolicyVersion(ctx, conn, d.Id(), aws.StringValue(iamPolicy.policy.DefaultVersionId)); err == nil { + iamPolicy.policyVersion = v + } else { + return nil, err } - return nil - }) - - if tfresource.TimedOut(err) { - getPolicyResponse, err = conn.GetPolicyWithContext(ctx, input) - } + return iamPolicy, nil + }, d.IsNewResource()) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] IAM Policy (%s) not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM policy %s: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s): %s", d.Id(), err) } - if !d.IsNewResource() && (getPolicyResponse == nil || getPolicyResponse.Policy == nil) { - log.Printf("[WARN] IAM Policy (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags - } - - policy := getPolicyResponse.Policy + output := outputRaw.(*policyWithVersion) + policy := output.policy d.Set("arn", policy.Arn) d.Set("description", policy.Description) @@ -205,51 +196,10 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf SetTagsOut(ctx, policy.Tags) - // Retrieve policy - - getPolicyVersionRequest := &iam.GetPolicyVersionInput{ - PolicyArn: aws.String(d.Id()), - VersionId: policy.DefaultVersionId, - } - - // Handle IAM eventual consistency - var getPolicyVersionResponse *iam.GetPolicyVersionOutput - err = retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - getPolicyVersionResponse, err = conn.GetPolicyVersionWithContext(ctx, getPolicyVersionRequest) - - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } - - return nil - }) - - if tfresource.TimedOut(err) { - getPolicyVersionResponse, err = conn.GetPolicyVersionWithContext(ctx, getPolicyVersionRequest) - } - - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM Policy (%s) version (%s) not found, removing from state", d.Id(), aws.StringValue(policy.DefaultVersionId)) - d.SetId("") - return diags - } + policyDocument, err := url.QueryUnescape(aws.StringValue(output.policyVersion.Document)) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM policy version %s: %s", d.Id(), err) - } - - var policyDocument string - if getPolicyVersionResponse != nil && getPolicyVersionResponse.PolicyVersion != nil { - var err error - policyDocument, err = url.QueryUnescape(aws.StringValue(getPolicyVersionResponse.PolicyVersion.Document)) - if err != nil { - return sdkdiag.AppendErrorf(diags, "parsing IAM policy (%s) document: %s", d.Id(), err) - } + return sdkdiag.AppendErrorf(diags, "parsing IAM Policy (%s) document: %s", d.Id(), err) } policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), policyDocument) @@ -267,8 +217,8 @@ func resourcePolicyUpdate(ctx context.Context, d *schema.ResourceData, meta inte conn := meta.(*conns.AWSClient).IAMConn() if d.HasChangesExcept("tags", "tags_all") { - if err := policyPruneVersions(ctx, d.Id(), conn); err != nil { - return sdkdiag.AppendErrorf(diags, "updating IAM policy %s: pruning versions: %s", d.Id(), err) + if err := policyPruneVersions(ctx, conn, d.Id()); err != nil { + return sdkdiag.AppendFromErr(diags, err) } policy, err := structure.NormalizeJsonString(d.Get("policy").(string)) @@ -276,14 +226,16 @@ func resourcePolicyUpdate(ctx context.Context, d *schema.ResourceData, meta inte return sdkdiag.AppendErrorf(diags, "policy (%s) is invalid JSON: %s", policy, err) } - request := &iam.CreatePolicyVersionInput{ + input := &iam.CreatePolicyVersionInput{ PolicyArn: aws.String(d.Id()), PolicyDocument: aws.String(policy), SetAsDefault: aws.Bool(true), } - if _, err := conn.CreatePolicyVersionWithContext(ctx, request); err != nil { - return sdkdiag.AppendErrorf(diags, "updating IAM policy %s: %s", d.Id(), err) + _, err = conn.CreatePolicyVersionWithContext(ctx, input) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating IAM Policy (%s): %s", d.Id(), err) } } @@ -292,9 +244,8 @@ func resourcePolicyUpdate(ctx context.Context, d *schema.ResourceData, meta inte err := policyUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM Policy (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourcePolicyRead(ctx, d, meta)...) } @@ -310,39 +261,52 @@ func resourcePolicyDelete(ctx context.Context, d *schema.ResourceData, meta inte var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - if err := policyDeleteNonDefaultVersions(ctx, d.Id(), conn); err != nil { - return sdkdiag.AppendErrorf(diags, "deleting IAM policy (%s): deleting non-default versions: %s", d.Id(), err) + // Delete non-default policy versions. + versions, err := findPolicyVersionsByARN(ctx, conn, d.Id()) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s) versions: %s", d.Id(), err) } - request := &iam.DeletePolicyInput{ - PolicyArn: aws.String(d.Id()), + for _, version := range versions { + if aws.BoolValue(version.IsDefaultVersion) { + continue + } + + if err := policyDeleteVersion(ctx, conn, d.Id(), aws.StringValue(version.VersionId)); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } } - _, err := conn.DeletePolicyWithContext(ctx, request) + log.Printf("[INFO] Deleting IAM Policy: %s", d.Id()) + _, err = conn.DeletePolicyWithContext(ctx, &iam.DeletePolicyInput{ + PolicyArn: aws.String(d.Id()), + }) if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting IAM policy (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting IAM Policy (%s): %s", d.Id(), err) } return diags } -// policyPruneVersions deletes the oldest versions. +// policyPruneVersions deletes the oldest version. // // Old versions are deleted until there are 4 or less remaining, which means at // least one more can be created before hitting the maximum of 5. // // The default version is never deleted. +func policyPruneVersions(ctx context.Context, conn *iam.IAM, arn string) error { + versions, err := findPolicyVersionsByARN(ctx, conn, arn) -func policyPruneVersions(ctx context.Context, arn string, conn *iam.IAM) error { - versions, err := policyListVersions(ctx, arn, conn) if err != nil { return err } + if len(versions) < 5 { return nil } @@ -350,57 +314,163 @@ func policyPruneVersions(ctx context.Context, arn string, conn *iam.IAM) error { var oldestVersion *iam.PolicyVersion for _, version := range versions { - if *version.IsDefaultVersion { + if aws.BoolValue(version.IsDefaultVersion) { continue } - if oldestVersion == nil || - version.CreateDate.Before(*oldestVersion.CreateDate) { + + if oldestVersion == nil || version.CreateDate.Before(aws.TimeValue(oldestVersion.CreateDate)) { oldestVersion = version } } - return policyDeleteVersion(ctx, arn, aws.StringValue(oldestVersion.VersionId), conn) + if oldestVersion == nil { + return nil + } + + return policyDeleteVersion(ctx, conn, arn, aws.StringValue(oldestVersion.VersionId)) } -func policyDeleteNonDefaultVersions(ctx context.Context, arn string, conn *iam.IAM) error { - versions, err := policyListVersions(ctx, arn, conn) +func policyDeleteVersion(ctx context.Context, conn *iam.IAM, arn, versionID string) error { + input := &iam.DeletePolicyVersionInput{ + PolicyArn: aws.String(arn), + VersionId: aws.String(versionID), + } + + _, err := conn.DeletePolicyVersionWithContext(ctx, input) + if err != nil { - return err + return fmt.Errorf("deleting IAM Policy (%s) version (%s): %w", arn, versionID, err) } - for _, version := range versions { - if aws.BoolValue(version.IsDefaultVersion) { - continue + return nil +} + +func FindPolicyByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.Policy, error) { + input := &iam.GetPolicyInput{ + PolicyArn: aws.String(arn), + } + + output, err := conn.GetPolicyWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, } - if err := policyDeleteVersion(ctx, arn, aws.StringValue(version.VersionId), conn); err != nil { - return err + } + + if err != nil { + return nil, err + } + + if output == nil || output.Policy == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.Policy, nil +} + +func findPolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, name, pathPrefix string) (*iam.Policy, error) { + input := &iam.ListPoliciesInput{} + if pathPrefix != "" { + input.PathPrefix = aws.String(pathPrefix) + } + + output, err := findPolicies(ctx, conn, input) + + if err != nil { + return nil, err + } + + if name != "" { + output = slices.Filter(output, func(v *iam.Policy) bool { + return aws.StringValue(v.PolicyName) == name + }) + } + + return tfresource.AssertSinglePtrResult(output) +} + +func findPolicies(ctx context.Context, conn *iam.IAM, input *iam.ListPoliciesInput) ([]*iam.Policy, error) { + var output []*iam.Policy + + err := conn.ListPoliciesPagesWithContext(ctx, input, func(page *iam.ListPoliciesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.Policies { + if v != nil { + output = append(output, v) + } } + + return !lastPage + }) + + if err != nil { + return nil, err } - return nil + return output, nil } -func policyDeleteVersion(ctx context.Context, arn, versionID string, conn *iam.IAM) error { - request := &iam.DeletePolicyVersionInput{ +func findPolicyVersion(ctx context.Context, conn *iam.IAM, arn, versionID string) (*iam.PolicyVersion, error) { + input := &iam.GetPolicyVersionInput{ PolicyArn: aws.String(arn), VersionId: aws.String(versionID), } - _, err := conn.DeletePolicyVersionWithContext(ctx, request) + output, err := conn.GetPolicyVersionWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + if err != nil { - return fmt.Errorf("deleting policy version (%s): %w", versionID, err) + return nil, err } - return nil + + if output == nil || output.PolicyVersion == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.PolicyVersion, nil } -func policyListVersions(ctx context.Context, arn string, conn *iam.IAM) ([]*iam.PolicyVersion, error) { - request := &iam.ListPolicyVersionsInput{ +func findPolicyVersionsByARN(ctx context.Context, conn *iam.IAM, arn string) ([]*iam.PolicyVersion, error) { + input := &iam.ListPolicyVersionsInput{ PolicyArn: aws.String(arn), } + var output []*iam.PolicyVersion + + err := conn.ListPolicyVersionsPagesWithContext(ctx, input, func(page *iam.ListPolicyVersionsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.Versions { + if v != nil { + output = append(output, v) + } + } + + return !lastPage + }) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } - response, err := conn.ListPolicyVersionsWithContext(ctx, request) if err != nil { - return nil, fmt.Errorf("Error listing versions for IAM policy %s: %w", arn, err) + return nil, err } - return response.Versions, nil + + return output, nil } diff --git a/internal/service/iam/policy_data_source.go b/internal/service/iam/policy_data_source.go index 0ec0c4aab137..3b7c93fbe29e 100644 --- a/internal/service/iam/policy_data_source.go +++ b/internal/service/iam/policy_data_source.go @@ -2,16 +2,11 @@ package iam import ( "context" - "errors" - "fmt" "net/url" - "strings" "github.com/aws/aws-sdk-go/aws" "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/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" @@ -75,35 +70,30 @@ func dataSourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta inte pathPrefix := d.Get("path_prefix").(string) if arn == "" { - raw, err := tfresource.RetryWhenNotFound(ctx, propagationTimeout, + outputRaw, err := tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) { - return FindPolicyByName(ctx, conn, name, pathPrefix) + return findPolicyByTwoPartKey(ctx, conn, name, pathPrefix) }, ) - if errors.Is(err, tfresource.ErrEmptyResult) { - return sdkdiag.AppendErrorf(diags, "no IAM policy found matching criteria (%s); try different search", PolicySearchDetails(name, pathPrefix)) - } - if errors.Is(err, tfresource.ErrTooManyResults) { - return sdkdiag.AppendErrorf(diags, "multiple IAM policies found matching criteria (%s); try different search. %s", PolicySearchDetails(name, pathPrefix), err) - } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM policy (%s): %s", PolicySearchDetails(name, pathPrefix), err) + return sdkdiag.AppendFromErr(diags, tfresource.SingularDataSourceFindError("IAM Policy", err)) } - arn = aws.StringValue((raw.(*iam.Policy)).Arn) + arn = aws.StringValue((outputRaw.(*iam.Policy)).Arn) } // We need to make a call to `iam.GetPolicy` because `iam.ListPolicies` doesn't return all values policy, err := FindPolicyByARN(ctx, conn, arn) + if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM policy (%s): %s", arn, err) + return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s): %s", arn, err) } - policyArn := aws.StringValue(policy.Arn) + arn = aws.StringValue(policy.Arn) - d.SetId(policyArn) - d.Set("arn", policyArn) + d.SetId(arn) + d.Set("arn", arn) d.Set("description", policy.Description) d.Set("name", policy.PolicyName) d.Set("path", policy.Path) @@ -113,65 +103,22 @@ func dataSourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta inte return sdkdiag.AppendErrorf(diags, "setting tags: %s", err) } - // Retrieve policy - policyVersionInput := &iam.GetPolicyVersionInput{ - PolicyArn: policy.Arn, - VersionId: policy.DefaultVersionId, - } - - // Handle IAM eventual consistency - var policyVersionOutput *iam.GetPolicyVersionOutput - err = retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - policyVersionOutput, err = conn.GetPolicyVersionWithContext(ctx, policyVersionInput) - - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } - - return nil - }) - - if tfresource.TimedOut(err) { - policyVersionOutput, err = conn.GetPolicyVersionWithContext(ctx, policyVersionInput) - } + outputRaw, err := tfresource.RetryWhenNotFound(ctx, propagationTimeout, + func() (interface{}, error) { + return findPolicyVersion(ctx, conn, arn, aws.StringValue(policy.DefaultVersionId)) + }, + ) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s) version: %s", policyArn, err) - } - - if policyVersionOutput == nil || policyVersionOutput.PolicyVersion == nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s) version: empty output", policyArn) + return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s) default version: %s", arn, err) } - policyVersion := policyVersionOutput.PolicyVersion - - var policyDocument string - if policyVersion != nil { - policyDocument, err = url.QueryUnescape(aws.StringValue(policyVersion.Document)) - if err != nil { - return sdkdiag.AppendErrorf(diags, "parsing IAM Policy (%s) document: %s", policyArn, err) - } + policyDocument, err := url.QueryUnescape(aws.StringValue(outputRaw.(*iam.PolicyVersion).Document)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "parsing IAM Policy (%s) document: %s", arn, err) } d.Set("policy", policyDocument) return diags } - -// PolicySearchDetails returns the configured search criteria as a printable string -func PolicySearchDetails(name, pathPrefix string) string { - var policyDetails []string - if name != "" { - policyDetails = append(policyDetails, fmt.Sprintf("Name: %s", name)) - } - if pathPrefix != "" { - policyDetails = append(policyDetails, fmt.Sprintf("PathPrefix: %s", pathPrefix)) - } - - return strings.Join(policyDetails, ", ") -} diff --git a/internal/service/iam/policy_data_source_test.go b/internal/service/iam/policy_data_source_test.go index c62bc76cb71b..9271c3232a35 100644 --- a/internal/service/iam/policy_data_source_test.go +++ b/internal/service/iam/policy_data_source_test.go @@ -9,53 +9,8 @@ import ( sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-provider-aws/internal/acctest" - tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" ) -func TestPolicySearchDetails(t *testing.T) { - t.Parallel() - - testCases := []struct { - Name string - PathPrefix string - Expected string - }{ - { - Name: "", - PathPrefix: "", - Expected: "", - }, - { - Name: "tf-acc-test-policy", - PathPrefix: "", - Expected: "Name: tf-acc-test-policy", - }, - { - Name: "", - PathPrefix: "/test-prefix/", - Expected: "PathPrefix: /test-prefix/", - }, - { - Name: "tf-acc-test-policy", - PathPrefix: "/test-prefix/", - Expected: "Name: tf-acc-test-policy, PathPrefix: /test-prefix/", - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.Name, func(t *testing.T) { - t.Parallel() - - got := tfiam.PolicySearchDetails(testCase.Name, testCase.PathPrefix) - - if got != testCase.Expected { - t.Errorf("got %s, expected %s", got, testCase.Expected) - } - }) - } -} - func TestAccIAMPolicyDataSource_arn(t *testing.T) { ctx := acctest.Context(t) datasourceName := "data.aws_iam_policy.test" @@ -237,7 +192,7 @@ func TestAccIAMPolicyDataSource_nonExistent(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccPolicyDataSourceConfig_nonExistent(policyName, policyPath), - ExpectError: regexp.MustCompile(`no IAM policy found matching criteria`), + ExpectError: regexp.MustCompile(`no matching IAM Policy found`), }, }, }) diff --git a/internal/service/iam/policy_test.go b/internal/service/iam/policy_test.go index 69ae314a8dae..5423a19a0a12 100644 --- a/internal/service/iam/policy_test.go +++ b/internal/service/iam/policy_test.go @@ -6,20 +6,19 @@ import ( "regexp" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMPolicy_basic(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" expectedPolicyText := `{"Statement":[{"Action":["ec2:Describe*"],"Effect":"Allow","Resource":"*"}],"Version":"2012-10-17"}` @@ -54,7 +53,7 @@ func TestAccIAMPolicy_basic(t *testing.T) { func TestAccIAMPolicy_description(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" @@ -82,7 +81,7 @@ func TestAccIAMPolicy_description(t *testing.T) { func TestAccIAMPolicy_tags(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" @@ -128,7 +127,7 @@ func TestAccIAMPolicy_tags(t *testing.T) { func TestAccIAMPolicy_disappears(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" @@ -152,7 +151,7 @@ func TestAccIAMPolicy_disappears(t *testing.T) { func TestAccIAMPolicy_namePrefix(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy resourceName := "aws_iam_policy.test" resource.ParallelTest(t, resource.TestCase{ @@ -180,7 +179,7 @@ func TestAccIAMPolicy_namePrefix(t *testing.T) { func TestAccIAMPolicy_path(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" @@ -208,7 +207,7 @@ func TestAccIAMPolicy_path(t *testing.T) { func TestAccIAMPolicy_policy(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" policy1 := `{"Statement":[{"Action":["ec2:Describe*"],"Effect":"Allow","Resource":"*"}],"Version":"2012-10-17"}` @@ -250,7 +249,7 @@ func TestAccIAMPolicy_policy(t *testing.T) { // https://github.com/hashicorp/terraform-provider-aws/issues/28833 func TestAccIAMPolicy_diffs(t *testing.T) { ctx := acctest.Context(t) - var out iam.GetPolicyOutput + var out iam.Policy rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy.test" @@ -336,27 +335,26 @@ func TestAccIAMPolicy_diffs(t *testing.T) { }) } -func testAccCheckPolicyExists(ctx context.Context, resource string, res *iam.GetPolicyOutput) resource.TestCheckFunc { +func testAccCheckPolicyExists(ctx context.Context, n string, v *iam.Policy) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resource] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", resource) + return fmt.Errorf("Not found: %s", n) } if rs.Primary.ID == "" { - return fmt.Errorf("No Policy name is set") + return fmt.Errorf("No IAM Policy ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - resp, err := conn.GetPolicyWithContext(ctx, &iam.GetPolicyInput{ - PolicyArn: aws.String(rs.Primary.Attributes["arn"]), - }) + output, err := tfiam.FindPolicyByARN(ctx, conn, rs.Primary.ID) + if err != nil { return err } - *res = *resp + *v = *output return nil } @@ -371,11 +369,9 @@ func testAccCheckPolicyDestroy(ctx context.Context) resource.TestCheckFunc { continue } - _, err := conn.GetPolicyWithContext(ctx, &iam.GetPolicyInput{ - PolicyArn: aws.String(rs.Primary.ID), - }) + _, err := tfiam.FindPolicyByARN(ctx, conn, rs.Primary.ID) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if tfresource.NotFound(err) { continue } @@ -383,7 +379,7 @@ func testAccCheckPolicyDestroy(ctx context.Context) resource.TestCheckFunc { return err } - return fmt.Errorf("IAM Policy (%s) still exists", rs.Primary.ID) + return fmt.Errorf("IAM Policy %s still exists", rs.Primary.ID) } return nil diff --git a/internal/service/iam/role.go b/internal/service/iam/role.go index c902f7fd4b0f..243b1a27624f 100644 --- a/internal/service/iam/role.go +++ b/internal/service/iam/role.go @@ -9,7 +9,6 @@ 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/v2/awsv1shim/v2/tfawserr" awspolicy "github.com/hashicorp/awspolicyequivalence" @@ -22,6 +21,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" + "github.com/hashicorp/terraform-provider-aws/internal/errs" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" @@ -203,12 +203,11 @@ func resourceRoleCreate(ctx context.Context, d *schema.ResourceData, meta interf } name := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) - tags := GetTagsIn(ctx) input := &iam.CreateRoleInput{ AssumeRolePolicyDocument: aws.String(assumeRolePolicy), Path: aws.String(d.Get("path").(string)), RoleName: aws.String(name), - Tags: tags, + Tags: GetTagsIn(ctx), } if v, ok := d.GetOk("description"); ok { @@ -225,9 +224,8 @@ func resourceRoleCreate(ctx context.Context, d *schema.ResourceData, meta interf output, err := retryCreateRole(ctx, conn, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM Role (%s) with tags: %s. Trying create without tags.", name, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = retryCreateRole(ctx, conn, input) @@ -255,18 +253,17 @@ func resourceRoleCreate(ctx context.Context, d *schema.ResourceData, meta interf d.SetId(roleName) - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID { - err := roleUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := roleCreateTags(ctx, conn, d.Id(), 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 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, resourceRoleRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM Role (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM Role (%s) tags: %s", d.Id(), err) } } @@ -509,9 +506,8 @@ func resourceRoleUpdate(ctx context.Context, d *schema.ResourceData, meta interf err := roleUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions may not support tagging, giving error - if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM Role %s: %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourceRoleRead(ctx, d, meta)...) } @@ -654,6 +650,31 @@ func retryCreateRole(ctx context.Context, conn *iam.IAM, input *iam.CreateRoleIn return output, err } +func FindRoleByName(ctx context.Context, conn *iam.IAM, name string) (*iam.Role, error) { + input := &iam.GetRoleInput{ + RoleName: aws.String(name), + } + + output, err := conn.GetRoleWithContext(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 || output.Role == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.Role, nil +} + func readRolePolicyAttachments(ctx context.Context, conn *iam.IAM, roleName string) ([]*string, error) { managedPolicies := make([]*string, 0) input := &iam.ListAttachedRolePoliciesInput{ diff --git a/internal/service/iam/saml_provider.go b/internal/service/iam/saml_provider.go index e32230cdef63..1ccc41086c84 100644 --- a/internal/service/iam/saml_provider.go +++ b/internal/service/iam/saml_provider.go @@ -12,9 +12,11 @@ import ( "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/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" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" @@ -66,18 +68,16 @@ func resourceSAMLProviderCreate(ctx context.Context, d *schema.ResourceData, met conn := meta.(*conns.AWSClient).IAMConn() name := d.Get("name").(string) - tags := GetTagsIn(ctx) input := &iam.CreateSAMLProviderInput{ Name: aws.String(name), SAMLMetadataDocument: aws.String(d.Get("saml_metadata_document").(string)), - Tags: tags, + Tags: GetTagsIn(ctx), } output, err := conn.CreateSAMLProviderWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM SAML Provider (%s) with tags: %s. Trying create without tags.", name, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = conn.CreateSAMLProviderWithContext(ctx, input) @@ -89,18 +89,17 @@ func resourceSAMLProviderCreate(ctx context.Context, d *schema.ResourceData, met d.SetId(aws.StringValue(output.SAMLProviderArn)) - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 { - err := samlProviderUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := samlProviderCreateTags(ctx, conn, d.Id(), 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 SAML Provider (%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 resourceSAMLProviderRead(ctx, d, meta) } if err != nil { - return diag.Errorf("adding tags after create for IAM SAML Provider (%s): %s", d.Id(), err) + return diag.Errorf("setting IAM SAML Provider (%s) tags: %s", d.Id(), err) } } @@ -151,7 +150,6 @@ func resourceSAMLProviderUpdate(ctx context.Context, d *schema.ResourceData, met SAMLMetadataDocument: aws.String(d.Get("saml_metadata_document").(string)), } - log.Printf("[DEBUG] Updating IAM SAML Provider: %s", input) _, err := conn.UpdateSAMLProviderWithContext(ctx, input) if err != nil { @@ -164,9 +162,8 @@ func resourceSAMLProviderUpdate(ctx context.Context, d *schema.ResourceData, met err := samlProviderUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM SAML Provider (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return resourceSAMLProviderRead(ctx, d, meta) } @@ -197,6 +194,31 @@ func resourceSAMLProviderDelete(ctx context.Context, d *schema.ResourceData, met return nil } +func FindSAMLProviderByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.GetSAMLProviderOutput, error) { + input := &iam.GetSAMLProviderInput{ + SAMLProviderArn: aws.String(arn), + } + + output, err := conn.GetSAMLProviderWithContext(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 nameFromSAMLProviderARN(v string) (string, error) { arn, err := arn.Parse(v) diff --git a/internal/service/iam/server_certificate.go b/internal/service/iam/server_certificate.go index 525650f0c3e1..4df084dcd7e2 100644 --- a/internal/service/iam/server_certificate.go +++ b/internal/service/iam/server_certificate.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" + "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" @@ -108,12 +109,11 @@ func resourceServerCertificateCreate(ctx context.Context, d *schema.ResourceData conn := meta.(*conns.AWSClient).IAMConn() sslCertName := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) - tags := GetTagsIn(ctx) input := &iam.UploadServerCertificateInput{ CertificateBody: aws.String(d.Get("certificate_body").(string)), PrivateKey: aws.String(d.Get("private_key").(string)), ServerCertificateName: aws.String(sslCertName), - Tags: tags, + Tags: GetTagsIn(ctx), } if v, ok := d.GetOk("certificate_chain"); ok { @@ -126,9 +126,8 @@ func resourceServerCertificateCreate(ctx context.Context, d *schema.ResourceData output, err := conn.UploadServerCertificateWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM Server Certificate (%s) with tags: %s. Trying create without tags.", sslCertName, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = conn.UploadServerCertificateWithContext(ctx, input) @@ -141,18 +140,17 @@ func resourceServerCertificateCreate(ctx context.Context, d *schema.ResourceData d.SetId(aws.StringValue(output.ServerCertificateMetadata.ServerCertificateId)) d.Set("name", sslCertName) // Required for resource Read. - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 { - err := serverCertificateUpdateTags(ctx, conn, sslCertName, nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := serverCertificateCreateTags(ctx, conn, sslCertName, 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 Server Certificate (%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, resourceServerCertificateRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM Server Certificate (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM Server Certificate (%s) tags: %s", d.Id(), err) } } @@ -208,9 +206,8 @@ func resourceServerCertificateUpdate(ctx context.Context, d *schema.ResourceData err := serverCertificateUpdateTags(ctx, conn, d.Get("name").(string), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM Server Certificate (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourceServerCertificateRead(ctx, d, meta)...) } diff --git a/internal/service/iam/service_linked_role.go b/internal/service/iam/service_linked_role.go index bee0c1700c90..6aab671cabb8 100644 --- a/internal/service/iam/service_linked_role.go +++ b/internal/service/iam/service_linked_role.go @@ -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) diff --git a/internal/service/iam/service_linked_role_test.go b/internal/service/iam/service_linked_role_test.go index ce5b42f37554..0700df33d2a2 100644 --- a/internal/service/iam/service_linked_role_test.go +++ b/internal/service/iam/service_linked_role_test.go @@ -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,7 +357,7 @@ 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) } @@ -362,8 +365,8 @@ resource "aws_iam_service_linked_role" "test" { 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) } @@ -371,9 +374,9 @@ resource "aws_iam_service_linked_role" "test" { 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) } diff --git a/internal/service/iam/sweep.go b/internal/service/iam/sweep.go index 96187e844d3e..0395fbb77136 100644 --- a/internal/service/iam/sweep.go +++ b/internal/service/iam/sweep.go @@ -404,11 +404,9 @@ func sweepServiceSpecificCredentials(region string) error { func sweepPolicies(region string) error { ctx := sweep.Context(region) client, err := sweep.SharedRegionalSweepClient(region) - if err != nil { return fmt.Errorf("error getting client: %w", err) } - conn := client.(*conns.AWSClient).IAMConn() input := &iam.ListPoliciesInput{ Scope: aws.String(iam.PolicyScopeTypeLocal), @@ -420,21 +418,13 @@ func sweepPolicies(region string) error { return !lastPage } - for _, policy := range page.Policies { - arn := aws.StringValue(policy.Arn) - input := &iam.DeletePolicyInput{ - PolicyArn: policy.Arn, - } - - log.Printf("[INFO] Deleting IAM Policy: %s", arn) - if err := policyDeleteNonDefaultVersions(ctx, arn, conn); err != nil { - sweeperErr := fmt.Errorf("error deleting IAM Policy (%s) non-default versions: %w", arn, err) - log.Printf("[ERROR] %s", sweeperErr) - sweeperErrs = multierror.Append(sweeperErrs, sweeperErr) - continue - } + for _, v := range page.Policies { + arn := aws.StringValue(v.Arn) + r := ResourcePolicy() + d := r.Data(nil) + d.SetId(arn) - _, err := conn.DeletePolicyWithContext(ctx, input) + err := sweep.NewSweepResource(r, d, client).Delete(ctx, sweep.ThrottlingRetryTimeout) // nosemgrep:ci.semgrep.migrate.direct-CRUD-calls // Treat this sweeper as best effort for now. There are a lot of edge cases // with lingering aws_iam_role resources in the HashiCorp testing accounts. @@ -443,10 +433,6 @@ func sweepPolicies(region string) error { continue } - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - continue - } - if tfawserr.ErrMessageContains(err, "AccessDenied", "with an explicit deny") { continue } diff --git a/internal/service/iam/tags.go b/internal/service/iam/tags.go index caf023eb6efe..a213d1f1cf41 100644 --- a/internal/service/iam/tags.go +++ b/internal/service/iam/tags.go @@ -16,76 +16,6 @@ import ( // Custom IAM tag service update functions using the same format as generated code. -// roleUpdateTags updates IAM role tags. -// The identifier is the role name. -func roleUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { - oldTags := tftags.New(ctx, oldTagsMap) - newTags := tftags.New(ctx, newTagsMap) - - if removedTags := oldTags.Removed(newTags).IgnoreSystem(names.IAM); len(removedTags) > 0 { - input := &iam.UntagRoleInput{ - RoleName: aws.String(identifier), - TagKeys: aws.StringSlice(removedTags.Keys()), - } - - _, err := conn.UntagRoleWithContext(ctx, input) - - if err != nil { - return fmt.Errorf("untagging resource (%s): %w", identifier, err) - } - } - - if updatedTags := oldTags.Updated(newTags).IgnoreSystem(names.IAM); len(updatedTags) > 0 { - input := &iam.TagRoleInput{ - RoleName: aws.String(identifier), - Tags: Tags(updatedTags), - } - - _, err := conn.TagRoleWithContext(ctx, input) - - if err != nil { - return fmt.Errorf("tagging resource (%s): %w", identifier, err) - } - } - - return nil -} - -// userUpdateTags updates IAM user tags. -// The identifier is the user name. -func userUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { - oldTags := tftags.New(ctx, oldTagsMap) - newTags := tftags.New(ctx, newTagsMap) - - if removedTags := oldTags.Removed(newTags).IgnoreSystem(names.IAM); len(removedTags) > 0 { - input := &iam.UntagUserInput{ - UserName: aws.String(identifier), - TagKeys: aws.StringSlice(removedTags.Keys()), - } - - _, err := conn.UntagUserWithContext(ctx, input) - - if err != nil { - return fmt.Errorf("untagging resource (%s): %w", identifier, err) - } - } - - if updatedTags := oldTags.Updated(newTags).IgnoreSystem(names.IAM); len(updatedTags) > 0 { - input := &iam.TagUserInput{ - UserName: aws.String(identifier), - Tags: Tags(updatedTags), - } - - _, err := conn.TagUserWithContext(ctx, input) - - if err != nil { - return fmt.Errorf("tagging resource (%s): %w", identifier, err) - } - } - - return nil -} - // instanceProfileUpdateTags updates IAM Instance Profile tags. // The identifier is the Instance Profile name. func instanceProfileUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { @@ -121,6 +51,14 @@ func instanceProfileUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identi return nil } +func instanceProfileCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return instanceProfileUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + // openIDConnectProviderUpdateTags updates IAM OpenID Connect Provider tags. // The identifier is the OpenID Connect Provider ARN. func openIDConnectProviderUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { @@ -156,6 +94,14 @@ func openIDConnectProviderUpdateTags(ctx context.Context, conn iamiface.IAMAPI, return nil } +func openIDConnectProviderCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return openIDConnectProviderUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + // policyUpdateTags updates IAM Policy tags. // The identifier is the Policy ARN. func policyUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { @@ -191,6 +137,57 @@ func policyUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier stri return nil } +func policyCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return policyUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + +// roleUpdateTags updates IAM role tags. +// The identifier is the role name. +func roleUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { + oldTags := tftags.New(ctx, oldTagsMap) + newTags := tftags.New(ctx, newTagsMap) + + if removedTags := oldTags.Removed(newTags).IgnoreSystem(names.IAM); len(removedTags) > 0 { + input := &iam.UntagRoleInput{ + RoleName: aws.String(identifier), + TagKeys: aws.StringSlice(removedTags.Keys()), + } + + _, err := conn.UntagRoleWithContext(ctx, input) + + if err != nil { + return fmt.Errorf("untagging resource (%s): %w", identifier, err) + } + } + + if updatedTags := oldTags.Updated(newTags).IgnoreSystem(names.IAM); len(updatedTags) > 0 { + input := &iam.TagRoleInput{ + RoleName: aws.String(identifier), + Tags: Tags(updatedTags), + } + + _, err := conn.TagRoleWithContext(ctx, input) + + if err != nil { + return fmt.Errorf("tagging resource (%s): %w", identifier, err) + } + } + + return nil +} + +func roleCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return roleUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + // samlProviderUpdateTags updates IAM SAML Provider tags. // The identifier is the SAML Provider ARN. func samlProviderUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { @@ -226,6 +223,14 @@ func samlProviderUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifie return nil } +func samlProviderCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return samlProviderUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + // serverCertificateUpdateTags updates IAM Server Certificate tags. // The identifier is the Server Certificate name. func serverCertificateUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { @@ -261,9 +266,60 @@ func serverCertificateUpdateTags(ctx context.Context, conn iamiface.IAMAPI, iden return nil } -// virtualMFAUpdateTags updates IAM Virtual MFA Device tags. +func serverCertificateCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return serverCertificateUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + +// userUpdateTags updates IAM user tags. +// The identifier is the user name. +func userUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { + oldTags := tftags.New(ctx, oldTagsMap) + newTags := tftags.New(ctx, newTagsMap) + + if removedTags := oldTags.Removed(newTags).IgnoreSystem(names.IAM); len(removedTags) > 0 { + input := &iam.UntagUserInput{ + UserName: aws.String(identifier), + TagKeys: aws.StringSlice(removedTags.Keys()), + } + + _, err := conn.UntagUserWithContext(ctx, input) + + if err != nil { + return fmt.Errorf("untagging resource (%s): %w", identifier, err) + } + } + + if updatedTags := oldTags.Updated(newTags).IgnoreSystem(names.IAM); len(updatedTags) > 0 { + input := &iam.TagUserInput{ + UserName: aws.String(identifier), + Tags: Tags(updatedTags), + } + + _, err := conn.TagUserWithContext(ctx, input) + + if err != nil { + return fmt.Errorf("tagging resource (%s): %w", identifier, err) + } + } + + return nil +} + +func userCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return userUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} + +// virtualMFADeviceUpdateTags updates IAM Virtual MFA Device tags. // The identifier is the Virtual MFA Device ARN. -func virtualMFAUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { +func virtualMFADeviceUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, oldTagsMap, newTagsMap any) error { oldTags := tftags.New(ctx, oldTagsMap) newTags := tftags.New(ctx, newTagsMap) @@ -295,3 +351,11 @@ func virtualMFAUpdateTags(ctx context.Context, conn iamiface.IAMAPI, identifier return nil } + +func virtualMFADeviceCreateTags(ctx context.Context, conn iamiface.IAMAPI, identifier string, tags []*iam.Tag) error { + if len(tags) == 0 { + return nil + } + + return virtualMFADeviceUpdateTags(ctx, conn, identifier, nil, KeyValueTags(ctx, tags)) +} diff --git a/internal/service/iam/user.go b/internal/service/iam/user.go index fcaebaffa8d7..1204e5e9fc82 100644 --- a/internal/service/iam/user.go +++ b/internal/service/iam/user.go @@ -7,7 +7,6 @@ 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/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/go-multierror" @@ -16,6 +15,7 @@ import ( "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" @@ -91,10 +91,9 @@ func resourceUserCreate(ctx context.Context, d *schema.ResourceData, meta interf name := d.Get("name").(string) path := d.Get("path").(string) - tags := GetTagsIn(ctx) input := &iam.CreateUserInput{ Path: aws.String(path), - Tags: tags, + Tags: GetTagsIn(ctx), UserName: aws.String(name), } @@ -104,9 +103,8 @@ func resourceUserCreate(ctx context.Context, d *schema.ResourceData, meta interf output, err := conn.CreateUserWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if input.Tags != nil && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM User (%s) with tags: %s. Trying create without tags.", name, err) + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { input.Tags = nil output, err = conn.CreateUserWithContext(ctx, input) @@ -118,18 +116,17 @@ func resourceUserCreate(ctx context.Context, d *schema.ResourceData, meta interf d.SetId(aws.StringValue(output.User.UserName)) - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if input.Tags == nil && len(tags) > 0 && meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID { - err := userUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := userCreateTags(ctx, conn, d.Id(), 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 User (%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, resourceUserRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM User (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM User (%s) tags: %s", d.Id(), err) } } @@ -140,33 +137,11 @@ func resourceUserRead(ctx context.Context, d *schema.ResourceData, meta interfac var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - request := &iam.GetUserInput{ - UserName: aws.String(d.Id()), - } - - var output *iam.GetUserOutput - - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - - output, err = conn.GetUserWithContext(ctx, request) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { + return FindUserByName(ctx, conn, d.Id()) + }, d.IsNewResource()) - return nil - }) - - if tfresource.TimedOut(err) { - output, err = conn.GetUserWithContext(ctx, request) - } - - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] IAM User (%s) not found, removing from state", d.Id()) d.SetId("") return diags @@ -176,19 +151,17 @@ func resourceUserRead(ctx context.Context, d *schema.ResourceData, meta interfac return sdkdiag.AppendErrorf(diags, "reading IAM User (%s): %s", d.Id(), err) } - if output == nil || output.User == nil { - return sdkdiag.AppendErrorf(diags, "reading IAM User (%s): empty response", d.Id()) - } + user := outputRaw.(*iam.User) - d.Set("arn", output.User.Arn) - d.Set("name", output.User.UserName) - d.Set("path", output.User.Path) - if output.User.PermissionsBoundary != nil { - d.Set("permissions_boundary", output.User.PermissionsBoundary.PermissionsBoundaryArn) + d.Set("arn", user.Arn) + d.Set("name", user.UserName) + d.Set("path", user.Path) + if user.PermissionsBoundary != nil { + d.Set("permissions_boundary", user.PermissionsBoundary.PermissionsBoundaryArn) } - d.Set("unique_id", output.User.UserId) + d.Set("unique_id", user.UserId) - SetTagsOut(ctx, output.User.Tags) + SetTagsOut(ctx, user.Tags) return diags } @@ -198,42 +171,42 @@ func resourceUserUpdate(ctx context.Context, d *schema.ResourceData, meta interf conn := meta.(*conns.AWSClient).IAMConn() if d.HasChanges("name", "path") { - on, nn := d.GetChange("name") - _, np := d.GetChange("path") - - request := &iam.UpdateUserInput{ - UserName: aws.String(on.(string)), - NewUserName: aws.String(nn.(string)), - NewPath: aws.String(np.(string)), + o, n := d.GetChange("name") + input := &iam.UpdateUserInput{ + UserName: aws.String(o.(string)), + NewUserName: aws.String(n.(string)), + NewPath: aws.String(d.Get("path").(string)), } - log.Println("[DEBUG] Update IAM User request:", request) - _, err := conn.UpdateUserWithContext(ctx, request) + _, err := conn.UpdateUserWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "updating IAM User %s: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "updating IAM User (%s): %s", d.Id(), err) } - d.SetId(nn.(string)) + d.SetId(n.(string)) } if d.HasChange("permissions_boundary") { - permissionsBoundary := d.Get("permissions_boundary").(string) - if permissionsBoundary != "" { + if v, ok := d.GetOk("permissions_boundary"); ok { input := &iam.PutUserPermissionsBoundaryInput{ - PermissionsBoundary: aws.String(permissionsBoundary), + PermissionsBoundary: aws.String(v.(string)), UserName: aws.String(d.Id()), } + _, err := conn.PutUserPermissionsBoundaryWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "updating IAM User permissions boundary: %s", err) + return sdkdiag.AppendErrorf(diags, "setting IAM User (%s) permissions boundary: %s", d.Id(), err) } } else { input := &iam.DeleteUserPermissionsBoundaryInput{ UserName: aws.String(d.Id()), } _, err := conn.DeleteUserPermissionsBoundaryWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting IAM User permissions boundary: %s", err) + return sdkdiag.AppendErrorf(diags, "deleting IAM User (%s) permissions boundary: %s", d.Id(), err) } } } @@ -243,9 +216,8 @@ func resourceUserUpdate(ctx context.Context, d *schema.ResourceData, meta interf err := userUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions may not support tagging, giving error - if meta.(*conns.AWSClient).Partition != endpoints.AwsPartitionID && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM User (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourceUserRead(ctx, d, meta)...) } @@ -297,24 +269,46 @@ func resourceUserDelete(ctx context.Context, d *schema.ResourceData, meta interf } } - deleteUserInput := &iam.DeleteUserInput{ + log.Println("[DEBUG] Deleting IAM User:", d.Id()) + _, err := conn.DeleteUserWithContext(ctx, &iam.DeleteUserInput{ UserName: aws.String(d.Id()), - } - - log.Println("[DEBUG] Delete IAM User request:", deleteUserInput) - _, err := conn.DeleteUserWithContext(ctx, deleteUserInput) + }) if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting IAM User %s: %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting IAM User (%s): %s", d.Id(), err) } return diags } +func FindUserByName(ctx context.Context, conn *iam.IAM, name string) (*iam.User, error) { + input := &iam.GetUserInput{ + UserName: aws.String(name), + } + + output, err := conn.GetUserWithContext(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 || output.User == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.User, nil +} + func DeleteUserGroupMemberships(ctx context.Context, conn *iam.IAM, username string) error { var groups []string listGroups := &iam.ListGroupsForUserInput{ diff --git a/internal/service/iam/user_test.go b/internal/service/iam/user_test.go index feb745b27237..715d1274aa88 100644 --- a/internal/service/iam/user_test.go +++ b/internal/service/iam/user_test.go @@ -8,19 +8,19 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/pquerna/otp/totp" ) func TestAccIAMUser_basic(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetUserOutput + var conf iam.User name1 := fmt.Sprintf("test-user-%d", sdkacctest.RandInt()) name2 := fmt.Sprintf("test-user-%d", sdkacctest.RandInt()) @@ -61,7 +61,7 @@ func TestAccIAMUser_basic(t *testing.T) { func TestAccIAMUser_disappears(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.user" @@ -76,7 +76,7 @@ func TestAccIAMUser_disappears(t *testing.T) { Config: testAccUserConfig_basic(rName, "/"), Check: resource.ComposeTestCheckFunc( testAccCheckUserExists(ctx, resourceName, &user), - testAccCheckUserDisappears(ctx, &user), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceUser(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -86,7 +86,7 @@ func TestAccIAMUser_disappears(t *testing.T) { func TestAccIAMUser_ForceDestroy_accessKey(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -117,7 +117,7 @@ func TestAccIAMUser_ForceDestroy_accessKey(t *testing.T) { func TestAccIAMUser_ForceDestroy_loginProfile(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -148,7 +148,7 @@ func TestAccIAMUser_ForceDestroy_loginProfile(t *testing.T) { func TestAccIAMUser_ForceDestroy_mfaDevice(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -179,7 +179,7 @@ func TestAccIAMUser_ForceDestroy_mfaDevice(t *testing.T) { func TestAccIAMUser_ForceDestroy_sshKey(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -209,7 +209,7 @@ func TestAccIAMUser_ForceDestroy_sshKey(t *testing.T) { func TestAccIAMUser_ForceDestroy_serviceSpecificCred(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -239,7 +239,7 @@ func TestAccIAMUser_ForceDestroy_serviceSpecificCred(t *testing.T) { func TestAccIAMUser_ForceDestroy_signingCertificate(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -270,7 +270,7 @@ func TestAccIAMUser_ForceDestroy_signingCertificate(t *testing.T) { func TestAccIAMUser_nameChange(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetUserOutput + var conf iam.User name1 := fmt.Sprintf("test-user-%d", sdkacctest.RandInt()) name2 := fmt.Sprintf("test-user-%d", sdkacctest.RandInt()) @@ -308,7 +308,7 @@ func TestAccIAMUser_nameChange(t *testing.T) { func TestAccIAMUser_pathChange(t *testing.T) { ctx := acctest.Context(t) - var conf iam.GetUserOutput + var conf iam.User name := fmt.Sprintf("test-user-%d", sdkacctest.RandInt()) path1 := "/" @@ -346,7 +346,7 @@ func TestAccIAMUser_pathChange(t *testing.T) { func TestAccIAMUser_permissionsBoundary(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.user" @@ -425,7 +425,7 @@ func TestAccIAMUser_permissionsBoundary(t *testing.T) { func TestAccIAMUser_tags(t *testing.T) { ctx := acctest.Context(t) - var user iam.GetUserOutput + var user iam.User rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_user.test" @@ -473,25 +473,24 @@ func testAccCheckUserDestroy(ctx context.Context) resource.TestCheckFunc { continue } - // Try to get user - _, err := conn.GetUserWithContext(ctx, &iam.GetUserInput{ - UserName: aws.String(rs.Primary.ID), - }) - if err == nil { - return fmt.Errorf("still exist.") + _, err := tfiam.FindUserByName(ctx, conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue } - // Verify the error is what we want - if !tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if err != nil { return err } + + return fmt.Errorf("IAM User %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckUserExists(ctx context.Context, n string, res *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserExists(ctx context.Context, n string, v *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -499,61 +498,43 @@ func testAccCheckUserExists(ctx context.Context, n string, res *iam.GetUserOutpu } if rs.Primary.ID == "" { - return fmt.Errorf("No User name is set") + return fmt.Errorf("No IAM User ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - resp, err := conn.GetUserWithContext(ctx, &iam.GetUserInput{ - UserName: aws.String(rs.Primary.ID), - }) + output, err := tfiam.FindUserByName(ctx, conn, rs.Primary.ID) + if err != nil { return err } - *res = *resp + *v = *output return nil } } -func testAccCheckUserAttributes(user *iam.GetUserOutput, name string, path string) resource.TestCheckFunc { +func testAccCheckUserAttributes(user *iam.User, name string, path string) resource.TestCheckFunc { return func(s *terraform.State) error { - if *user.User.UserName != name { - return fmt.Errorf("Bad name: %s", *user.User.UserName) + if *user.UserName != name { + return fmt.Errorf("Bad name: %s", *user.UserName) } - if *user.User.Path != path { - return fmt.Errorf("Bad path: %s", *user.User.Path) - } - - return nil - } -} - -func testAccCheckUserDisappears(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - - userName := aws.StringValue(getUserOutput.User.UserName) - - _, err := conn.DeleteUserWithContext(ctx, &iam.DeleteUserInput{ - UserName: aws.String(userName), - }) - if err != nil { - return fmt.Errorf("error deleting user %q: %s", userName, err) + if *user.Path != path { + return fmt.Errorf("Bad path: %s", *user.Path) } return nil } } -func testAccCheckUserPermissionsBoundary(getUserOutput *iam.GetUserOutput, expectedPermissionsBoundaryArn string) resource.TestCheckFunc { +func testAccCheckUserPermissionsBoundary(user *iam.User, expectedPermissionsBoundaryArn string) resource.TestCheckFunc { return func(s *terraform.State) error { actualPermissionsBoundaryArn := "" - if getUserOutput.User.PermissionsBoundary != nil { - actualPermissionsBoundaryArn = *getUserOutput.User.PermissionsBoundary.PermissionsBoundaryArn + if user.PermissionsBoundary != nil { + actualPermissionsBoundaryArn = *user.PermissionsBoundary.PermissionsBoundaryArn } if actualPermissionsBoundaryArn != expectedPermissionsBoundaryArn { @@ -564,23 +545,23 @@ func testAccCheckUserPermissionsBoundary(getUserOutput *iam.GetUserOutput, expec } } -func testAccCheckUserCreatesAccessKey(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserCreatesAccessKey(ctx context.Context, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() input := &iam.CreateAccessKeyInput{ - UserName: getUserOutput.User.UserName, + UserName: user.UserName, } if _, err := conn.CreateAccessKeyWithContext(ctx, input); err != nil { - return fmt.Errorf("error creating IAM User (%s) Access Key: %s", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error creating IAM User (%s) Access Key: %s", aws.StringValue(user.UserName), err) } return nil } } -func testAccCheckUserCreatesLoginProfile(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserCreatesLoginProfile(ctx context.Context, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() password, err := tfiam.GeneratePassword(32) @@ -589,29 +570,29 @@ func testAccCheckUserCreatesLoginProfile(ctx context.Context, getUserOutput *iam } input := &iam.CreateLoginProfileInput{ Password: aws.String(password), - UserName: getUserOutput.User.UserName, + UserName: user.UserName, } if _, err := conn.CreateLoginProfileWithContext(ctx, input); err != nil { - return fmt.Errorf("error creating IAM User (%s) Login Profile: %s", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error creating IAM User (%s) Login Profile: %s", aws.StringValue(user.UserName), err) } return nil } } -func testAccCheckUserCreatesMFADevice(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserCreatesMFADevice(ctx context.Context, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() createVirtualMFADeviceInput := &iam.CreateVirtualMFADeviceInput{ - Path: getUserOutput.User.Path, - VirtualMFADeviceName: getUserOutput.User.UserName, + Path: user.Path, + VirtualMFADeviceName: user.UserName, } createVirtualMFADeviceOutput, err := conn.CreateVirtualMFADeviceWithContext(ctx, createVirtualMFADeviceInput) if err != nil { - return fmt.Errorf("error creating IAM User (%s) Virtual MFA Device: %s", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error creating IAM User (%s) Virtual MFA Device: %s", aws.StringValue(user.UserName), err) } secret := string(createVirtualMFADeviceOutput.VirtualMFADevice.Base32StringSeed) @@ -628,11 +609,11 @@ func testAccCheckUserCreatesMFADevice(ctx context.Context, getUserOutput *iam.Ge AuthenticationCode1: aws.String(authenticationCode1), AuthenticationCode2: aws.String(authenticationCode2), SerialNumber: createVirtualMFADeviceOutput.VirtualMFADevice.SerialNumber, - UserName: getUserOutput.User.UserName, + UserName: user.UserName, } if _, err := conn.EnableMFADeviceWithContext(ctx, enableVirtualMFADeviceInput); err != nil { - return fmt.Errorf("error enabling IAM User (%s) Virtual MFA Device: %s", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error enabling IAM User (%s) Virtual MFA Device: %s", aws.StringValue(user.UserName), err) } return nil @@ -640,7 +621,7 @@ func testAccCheckUserCreatesMFADevice(ctx context.Context, getUserOutput *iam.Ge } // Creates an IAM User SSH Key outside of Terraform to verify that it is deleted when `force_destroy` is set -func testAccCheckUserUploadsSSHKey(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserUploadsSSHKey(ctx context.Context, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { publicKey, _, err := RandSSHKeyPairSize(2048, acctest.DefaultEmailAddress) if err != nil { @@ -650,13 +631,13 @@ func testAccCheckUserUploadsSSHKey(ctx context.Context, getUserOutput *iam.GetUs conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() input := &iam.UploadSSHPublicKeyInput{ - UserName: getUserOutput.User.UserName, + UserName: user.UserName, SSHPublicKeyBody: aws.String(publicKey), } _, err = conn.UploadSSHPublicKeyWithContext(ctx, input) if err != nil { - return fmt.Errorf("error uploading IAM User (%s) SSH key: %w", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error uploading IAM User (%s) SSH key: %w", aws.StringValue(user.UserName), err) } return nil @@ -664,25 +645,25 @@ func testAccCheckUserUploadsSSHKey(ctx context.Context, getUserOutput *iam.GetUs } // Creates an IAM User Service Specific Credential outside of Terraform to verify that it is deleted when `force_destroy` is set -func testAccCheckUserServiceSpecificCredential(ctx context.Context, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserServiceSpecificCredential(ctx context.Context, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() input := &iam.CreateServiceSpecificCredentialInput{ - UserName: getUserOutput.User.UserName, + UserName: user.UserName, ServiceName: aws.String("codecommit.amazonaws.com"), } _, err := conn.CreateServiceSpecificCredentialWithContext(ctx, input) if err != nil { - return fmt.Errorf("error uploading IAM User (%s) Service Specifc Credential: %w", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error uploading IAM User (%s) Service Specifc Credential: %w", aws.StringValue(user.UserName), err) } return nil } } -func testAccCheckUserUploadSigningCertificate(ctx context.Context, t *testing.T, getUserOutput *iam.GetUserOutput) resource.TestCheckFunc { +func testAccCheckUserUploadSigningCertificate(ctx context.Context, t *testing.T, user *iam.User) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() @@ -691,11 +672,11 @@ func testAccCheckUserUploadSigningCertificate(ctx context.Context, t *testing.T, input := &iam.UploadSigningCertificateInput{ CertificateBody: aws.String(certificate), - UserName: getUserOutput.User.UserName, + UserName: user.UserName, } if _, err := conn.UploadSigningCertificateWithContext(ctx, input); err != nil { - return fmt.Errorf("error uploading IAM User (%s) Signing Certificate : %s", aws.StringValue(getUserOutput.User.UserName), err) + return fmt.Errorf("error uploading IAM User (%s) Signing Certificate : %s", aws.StringValue(user.UserName), err) } return nil diff --git a/internal/service/iam/virtual_mfa_device.go b/internal/service/iam/virtual_mfa_device.go index daccd10027d3..008b5ca0b1af 100644 --- a/internal/service/iam/virtual_mfa_device.go +++ b/internal/service/iam/virtual_mfa_device.go @@ -9,9 +9,11 @@ import ( "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/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" @@ -74,45 +76,42 @@ func resourceVirtualMFADeviceCreate(ctx context.Context, d *schema.ResourceData, conn := meta.(*conns.AWSClient).IAMConn() name := d.Get("virtual_mfa_device_name").(string) - tags := GetTagsIn(ctx) - request := &iam.CreateVirtualMFADeviceInput{ + input := &iam.CreateVirtualMFADeviceInput{ Path: aws.String(d.Get("path").(string)), - Tags: tags, + Tags: GetTagsIn(ctx), VirtualMFADeviceName: aws.String(name), } - output, err := conn.CreateVirtualMFADeviceWithContext(ctx, request) + output, err := conn.CreateVirtualMFADeviceWithContext(ctx, input) - // Some partitions (i.e., ISO) may not support tag-on-create - if request.Tags != nil && verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed creating IAM Virtual MFA Device (%s) with tags: %s. Trying create without tags.", name, err) - request.Tags = nil + // Some partitions (e.g. ISO) may not support tag-on-create. + if input.Tags != nil && errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { + input.Tags = nil - output, err = conn.CreateVirtualMFADeviceWithContext(ctx, request) + output, err = conn.CreateVirtualMFADeviceWithContext(ctx, input) } if err != nil { - return sdkdiag.AppendErrorf(diags, "creating IAM Virtual MFA Device %s: %s", name, err) + return sdkdiag.AppendErrorf(diags, "creating IAM Virtual MFA Device (%s): %s", name, err) } - vMfa := output.VirtualMFADevice - d.SetId(aws.StringValue(vMfa.SerialNumber)) + vMFA := output.VirtualMFADevice + d.SetId(aws.StringValue(vMFA.SerialNumber)) - d.Set("base_32_string_seed", string(vMfa.Base32StringSeed)) - d.Set("qr_code_png", string(vMfa.QRCodePNG)) + d.Set("base_32_string_seed", string(vMFA.Base32StringSeed)) + d.Set("qr_code_png", string(vMFA.QRCodePNG)) - // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create - if request.Tags == nil && len(tags) > 0 { - err := virtualMFAUpdateTags(ctx, conn, d.Id(), nil, KeyValueTags(ctx, tags)) + // For partitions not supporting tag-on-create, attempt tag after create. + if tags := GetTagsIn(ctx); input.Tags == nil && len(tags) > 0 { + err := virtualMFADeviceCreateTags(ctx, conn, d.Id(), 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 Virtual MFA Device (%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, resourceVirtualMFADeviceRead(ctx, d, meta)...) } if err != nil { - return sdkdiag.AppendErrorf(diags, "adding tags after create for IAM Virtual MFA Device (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "setting IAM Virtual MFA Device (%s) tags: %s", d.Id(), err) } } @@ -123,7 +122,7 @@ func resourceVirtualMFADeviceRead(ctx context.Context, d *schema.ResourceData, m var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - output, err := FindVirtualMFADevice(ctx, conn, d.Id()) + vMFA, err := FindVirtualMFADeviceBySerialNumber(ctx, conn, d.Id()) if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] IAM Virtual MFA Device (%s) not found, removing from state", d.Id()) @@ -135,10 +134,10 @@ func resourceVirtualMFADeviceRead(ctx context.Context, d *schema.ResourceData, m return sdkdiag.AppendErrorf(diags, "reading IAM Virtual MFA Device (%s): %s", d.Id(), err) } - d.Set("arn", output.SerialNumber) + d.Set("arn", vMFA.SerialNumber) - // The call above returns empty tags - mfaTags, err := conn.ListMFADeviceTagsWithContext(ctx, &iam.ListMFADeviceTagsInput{ + // The call above returns empty tags. + output, err := conn.ListMFADeviceTagsWithContext(ctx, &iam.ListMFADeviceTagsInput{ SerialNumber: aws.String(d.Id()), }) @@ -146,7 +145,7 @@ func resourceVirtualMFADeviceRead(ctx context.Context, d *schema.ResourceData, m return sdkdiag.AppendErrorf(diags, "listing IAM Virtual MFA Device (%s) tags: %s", d.Id(), err) } - SetTagsOut(ctx, mfaTags.Tags) + SetTagsOut(ctx, output.Tags) return diags } @@ -157,11 +156,10 @@ func resourceVirtualMFADeviceUpdate(ctx context.Context, d *schema.ResourceData, o, n := d.GetChange("tags_all") - err := virtualMFAUpdateTags(ctx, conn, d.Id(), o, n) + err := virtualMFADeviceUpdateTags(ctx, conn, d.Id(), o, n) - // Some partitions (i.e., ISO) may not support tagging, giving error - if verify.ErrorISOUnsupported(conn.PartitionID, err) { - log.Printf("[WARN] failed updating tags for IAM Virtual MFA Device (%s): %s", d.Id(), err) + // Some partitions (e.g. ISO) may not support tagging. + if errs.IsUnsupportedOperationInPartitionError(conn.PartitionID, err) { return append(diags, resourceVirtualMFADeviceRead(ctx, d, meta)...) } @@ -176,15 +174,48 @@ func resourceVirtualMFADeviceDelete(ctx context.Context, d *schema.ResourceData, var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn() - request := &iam.DeleteVirtualMFADeviceInput{ + log.Printf("[INFO] Deleting IAM Virtual MFA Device: %s", d.Id()) + _, err := conn.DeleteVirtualMFADeviceWithContext(ctx, &iam.DeleteVirtualMFADeviceInput{ SerialNumber: aws.String(d.Id()), + }) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return diags } - if _, err := conn.DeleteVirtualMFADeviceWithContext(ctx, request); err != nil { - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return diags - } - return sdkdiag.AppendErrorf(diags, "deleting IAM Virtual MFA Device %s: %s", d.Id(), err) + if err != nil { + return sdkdiag.AppendErrorf(diags, "deleting IAM Virtual MFA Device (%s): %s", d.Id(), err) } + return diags } + +func FindVirtualMFADeviceBySerialNumber(ctx context.Context, conn *iam.IAM, serialNumber string) (*iam.VirtualMFADevice, error) { + input := &iam.ListVirtualMFADevicesInput{} + var output *iam.VirtualMFADevice + + err := conn.ListVirtualMFADevicesPagesWithContext(ctx, input, func(page *iam.ListVirtualMFADevicesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.VirtualMFADevices { + if v != nil && aws.StringValue(v.SerialNumber) == serialNumber { + output = v + return false + } + } + + return !lastPage + }) + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &retry.NotFoundError{} + } + + return output, nil +} diff --git a/internal/service/iam/virtual_mfa_device_test.go b/internal/service/iam/virtual_mfa_device_test.go index 8cba7d540430..43c8afec588c 100644 --- a/internal/service/iam/virtual_mfa_device_test.go +++ b/internal/service/iam/virtual_mfa_device_test.go @@ -131,7 +131,7 @@ func testAccCheckVirtualMFADeviceDestroy(ctx context.Context) resource.TestCheck continue } - output, err := tfiam.FindVirtualMFADevice(ctx, conn, rs.Primary.ID) + output, err := tfiam.FindVirtualMFADeviceBySerialNumber(ctx, conn, rs.Primary.ID) if tfresource.NotFound(err) { continue @@ -146,7 +146,7 @@ func testAccCheckVirtualMFADeviceDestroy(ctx context.Context) resource.TestCheck } } -func testAccCheckVirtualMFADeviceExists(ctx context.Context, n string, res *iam.VirtualMFADevice) resource.TestCheckFunc { +func testAccCheckVirtualMFADeviceExists(ctx context.Context, n string, v *iam.VirtualMFADevice) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -154,17 +154,18 @@ func testAccCheckVirtualMFADeviceExists(ctx context.Context, n string, res *iam. } if rs.Primary.ID == "" { - return errors.New("No Virtual MFA Device name is set") + return errors.New("No Virtual MFA Device ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn() - output, err := tfiam.FindVirtualMFADevice(ctx, conn, rs.Primary.ID) + output, err := tfiam.FindVirtualMFADeviceBySerialNumber(ctx, conn, rs.Primary.ID) + if err != nil { return err } - *res = *output + *v = *output return nil } diff --git a/internal/service/iam/wait.go b/internal/service/iam/wait.go index e180013a30fa..51f8acf6abc8 100644 --- a/internal/service/iam/wait.go +++ b/internal/service/iam/wait.go @@ -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 - } -}