Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

aws_iam_role name_prefix size #20785

Merged
3 changes: 3 additions & 0 deletions .changelog/12436.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_iam_role: Retry `assume_role_policy` updates for IAM eventual consistency
```
3 changes: 3 additions & 0 deletions .changelog/19532.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_iam_role: Add plan time validation for `path`, `permissions_boundary`, `managed_policy_arns`.
```
7 changes: 7 additions & 0 deletions .changelog/20785.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_iam_role: Change `name_prefix` validation to a range of 1 to 38 characters
```

```release-note:enhancement
resource/aws_iam_role: `name_prefix` is now Computed
```
7 changes: 3 additions & 4 deletions aws/data_source_aws_iam_session_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/finder"
Expand Down Expand Up @@ -70,9 +69,9 @@ func dataSourceAwsIAMSessionContextRead(d *schema.ResourceData, meta interface{}
err = resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
var err error

role, err = finder.Role(conn, roleName)
role, err = finder.RoleByName(conn, roleName)

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
if !d.IsNewResource() && tfresource.NotFound(err) {
return resource.RetryableError(err)
}

Expand All @@ -84,7 +83,7 @@ func dataSourceAwsIAMSessionContextRead(d *schema.ResourceData, meta interface{}
})

if tfresource.TimedOut(err) {
role, err = finder.Role(conn, roleName)
role, err = finder.RoleByName(conn, roleName)
}

if err != nil {
Expand Down
19 changes: 13 additions & 6 deletions aws/internal/service/iam/finder/finder.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package finder

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

// GroupAttachedPolicy returns the AttachedPolicy corresponding to the specified group and policy ARN.
Expand Down Expand Up @@ -112,20 +113,26 @@ func Policies(conn *iam.IAM, arn, name, pathPrefix string) ([]*iam.Policy, error
return results, err
}

// Role returns a role's ARN given the role name
func Role(conn *iam.IAM, name string) (*iam.Role, error) {
func RoleByName(conn *iam.IAM, name string) (*iam.Role, error) {
input := &iam.GetRoleInput{
RoleName: aws.String(name),
}

output, err := conn.GetRole(input)

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

if err != nil {
return nil, fmt.Errorf("getting IAM Role (%s): %w", name, err)
return nil, err
}

if output == nil || output.Role == nil {
return nil, fmt.Errorf("getting IAM Role (%s): empty response", name)
return nil, tfresource.NewEmptyResultError(input)
}

return output.Role, nil
Expand Down
152 changes: 67 additions & 85 deletions aws/resource_aws_iam_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/naming"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/finder"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)
Expand Down Expand Up @@ -54,25 +56,27 @@ func resourceAwsIamRole() *schema.Resource {
"name_prefix": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"name"},
ValidateFunc: validation.All(
validation.StringLenBetween(1, 32),
validation.StringLenBetween(1, 64-resource.UniqueIDSuffixLength),
validation.StringMatch(regexp.MustCompile(`^[\w+=,.@-]*$`), "must match [\\w+=,.@-]"),
),
},

"path": {
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
ValidateFunc: validation.StringLenBetween(0, 512),
},

"permissions_boundary": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringLenBetween(0, 2048),
ValidateFunc: validateArn,
},

"description": {
Expand Down Expand Up @@ -141,8 +145,10 @@ func resourceAwsIamRole() *schema.Resource {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validateArn,
},
},
},

Expand All @@ -161,15 +167,7 @@ func resourceAwsIamRoleCreate(d *schema.ResourceData, meta interface{}) error {
defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig
tags := defaultTagsConfig.MergeTags(keyvaluetags.New(d.Get("tags").(map[string]interface{})))

var name string
if v, ok := d.GetOk("name"); ok {
name = v.(string)
} else if v, ok := d.GetOk("name_prefix"); ok {
name = resource.PrefixedUniqueId(v.(string))
} else {
name = resource.UniqueId()
}

name := naming.Generate(d.Get("name").(string), d.Get("name_prefix").(string))
request := &iam.CreateRoleInput{
Path: aws.String(d.Get("path").(string)),
RoleName: aws.String(name),
Expand All @@ -192,28 +190,25 @@ func resourceAwsIamRoleCreate(d *schema.ResourceData, meta interface{}) error {
request.Tags = tags.IgnoreAws().IamTags()
}

var createResp *iam.CreateRoleOutput
err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
var err error
createResp, err = iamconn.CreateRole(request)
// IAM users (referenced in Principal field of assume policy)
// can take ~30 seconds to propagate in AWS
if isAWSErr(err, "MalformedPolicyDocument", "Invalid principal in policy") {
return resource.RetryableError(err)
}
if err != nil {
return resource.NonRetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {
createResp, err = iamconn.CreateRole(request)
}
outputRaw, err := tfresource.RetryWhen(
waiter.PropagationTimeout,
func() (interface{}, error) {
return iamconn.CreateRole(request)
},
func(err error) (bool, error) {
if tfawserr.ErrMessageContains(err, iam.ErrCodeMalformedPolicyDocumentException, "Invalid principal in policy") {
return true, err
}

return false, err
},
)

if err != nil {
return fmt.Errorf("Error creating IAM Role %s: %s", name, err)
return fmt.Errorf("error creating IAM Role (%s): %w", name, err)
}

roleName := aws.StringValue(createResp.Role.RoleName)
roleName := aws.StringValue(outputRaw.(*iam.CreateRoleOutput).Role.RoleName)

if v, ok := d.GetOk("inline_policy"); ok && v.(*schema.Set).Len() > 0 {
policies := expandIamInlinePolicies(roleName, v.(*schema.Set).List())
Expand All @@ -238,33 +233,11 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error {
defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

request := &iam.GetRoleInput{
RoleName: aws.String(d.Id()),
}

var getResp *iam.GetRoleOutput

err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
var err error

getResp, err = iamconn.GetRole(request)

if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

return nil
})
outputRaw, err := tfresource.RetryWhenNewResourceNotFound(waiter.PropagationTimeout, func() (interface{}, error) {
return finder.RoleByName(iamconn, d.Id())
}, d.IsNewResource())

if tfresource.TimedOut(err) {
getResp, err = iamconn.GetRole(request)
}

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
if !d.IsNewResource() && tfresource.NotFound(err) {
log.Printf("[WARN] IAM Role (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
Expand All @@ -274,11 +247,7 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error reading IAM Role (%s): %w", d.Id(), err)
}

if getResp == nil || getResp.Role == nil {
return fmt.Errorf("error reading IAM Role (%s): empty response", d.Id())
}

role := getResp.Role
role := outputRaw.(*iam.Role)

d.Set("arn", role.Arn)
if err := d.Set("create_date", role.CreateDate.Format(time.RFC3339)); err != nil {
Expand All @@ -287,6 +256,7 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error {
d.Set("description", role.Description)
d.Set("max_session_duration", role.MaxSessionDuration)
d.Set("name", role.RoleName)
d.Set("name_prefix", naming.NamePrefixFromName(aws.StringValue(role.RoleName)))
d.Set("path", role.Path)
if role.PermissionsBoundary != nil {
d.Set("permissions_boundary", role.PermissionsBoundary.PermissionsBoundaryArn)
Expand Down Expand Up @@ -337,13 +307,23 @@ func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error {
RoleName: aws.String(d.Id()),
PolicyDocument: aws.String(d.Get("assume_role_policy").(string)),
}
_, err := iamconn.UpdateAssumeRolePolicy(assumeRolePolicyInput)

_, err := tfresource.RetryWhen(
waiter.PropagationTimeout,
func() (interface{}, error) {
return iamconn.UpdateAssumeRolePolicy(assumeRolePolicyInput)
},
func(err error) (bool, error) {
if tfawserr.ErrMessageContains(err, iam.ErrCodeMalformedPolicyDocumentException, "Invalid principal in policy") {
return true, err
}

return false, err
},
)

if err != nil {
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
d.SetId("")
return nil
}
return fmt.Errorf("Error Updating IAM Role (%s) Assume Role Policy: %s", d.Id(), err)
return fmt.Errorf("error updating IAM Role (%s) assume role policy: %w", d.Id(), err)
}
}

Expand All @@ -352,13 +332,11 @@ func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error {
RoleName: aws.String(d.Id()),
Description: aws.String(d.Get("description").(string)),
}

_, err := iamconn.UpdateRoleDescription(roleDescriptionInput)

if err != nil {
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
d.SetId("")
return nil
}
return fmt.Errorf("Error Updating IAM Role (%s) Assume Role Policy: %s", d.Id(), err)
return fmt.Errorf("error updating IAM Role (%s) description: %w", d.Id(), err)
}
}

Expand All @@ -367,13 +345,11 @@ func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error {
RoleName: aws.String(d.Id()),
MaxSessionDuration: aws.Int64(int64(d.Get("max_session_duration").(int))),
}

_, err := iamconn.UpdateRole(roleMaxDurationInput)

if err != nil {
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
d.SetId("")
return nil
}
return fmt.Errorf("Error Updating IAM Role (%s) Max Session Duration: %s", d.Id(), err)
return fmt.Errorf("error updating IAM Role (%s) MaxSessionDuration: %s", d.Id(), err)
}
}

Expand All @@ -384,17 +360,21 @@ func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error {
PermissionsBoundary: aws.String(permissionsBoundary),
RoleName: aws.String(d.Id()),
}

_, err := iamconn.PutRolePermissionsBoundary(input)

if err != nil {
return fmt.Errorf("error updating IAM Role permissions boundary: %s", err)
return fmt.Errorf("error updating IAM Role (%s) permissions boundary: %w", d.Id(), err)
}
} else {
input := &iam.DeleteRolePermissionsBoundaryInput{
RoleName: aws.String(d.Id()),
}

_, err := iamconn.DeleteRolePermissionsBoundary(input)

if err != nil {
return fmt.Errorf("error deleting IAM Role permissions boundary: %s", err)
return fmt.Errorf("error deleting IAM Role (%s) permissions boundary: %w", d.Id(), err)
}
}
}
Expand Down Expand Up @@ -486,9 +466,11 @@ func resourceAwsIamRoleDelete(d *schema.ResourceData, meta interface{}) error {
}

err := deleteIamRole(conn, d.Id(), d.Get("force_detach_policies").(bool), hasInline, hasManaged)

if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return nil
}

if err != nil {
return fmt.Errorf("error deleting IAM Role (%s): %w", d.Id(), err)
}
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_iam_role_policy_attachment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestAccAWSRolePolicyAttachment_disappears(t *testing.T) {

func TestAccAWSRolePolicyAttachment_disappears_Role(t *testing.T) {
var attachedRolePolicies iam.ListAttachedRolePoliciesOutput
var role iam.GetRoleOutput
var role iam.Role

rName := acctest.RandomWithPrefix("tf-acc-test")
iamRoleResourceName := "aws_iam_role.test"
Expand All @@ -109,7 +109,7 @@ func TestAccAWSRolePolicyAttachment_disappears_Role(t *testing.T) {
testAccCheckAWSRolePolicyAttachmentExists(resourceName, 1, &attachedRolePolicies),
// DeleteConflict: Cannot delete entity, must detach all policies first.
testAccCheckAWSIAMRolePolicyAttachmentDisappears(resourceName),
testAccCheckAWSRoleDisappears(&role),
testAccCheckResourceDisappears(testAccProvider, resourceAwsIamRole(), iamRoleResourceName),
),
ExpectNonEmptyPlan: true,
},
Expand Down
Loading