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.

Sign up for GitHub

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

resource/aws_iam_user: Add permissions_boundary argument #5183

Merged
merged 2 commits into from
Jul 30, 2018
Merged

resource/aws_iam_user: Add permissions_boundary argument #5183

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bef84f
resource/aws_iam_user: Add permissions_boundary argument
bflad Jul 13, 2018
c8c9ff7
tests/resource/aws_iam_user: Add TestAccAWSUser_disappears
bflad Jul 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 78 additions & 30 deletions aws/resource_aws_iam_user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/iam"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
)

func resourceAwsIamUser() *schema.Resource {
Expand All @@ -25,7 +25,7 @@ func resourceAwsIamUser() *schema.Resource {
},

Schema: map[string]*schema.Schema{
"arn": &schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},
Expand All @@ -37,21 +37,26 @@ func resourceAwsIamUser() *schema.Resource {
and inefficient. Still, there are other reasons one might want
the UniqueID, so we can make it available.
*/
"unique_id": &schema.Schema{
"unique_id": {
Type: schema.TypeString,
Computed: true,
},
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateAwsIamUserName,
},
"path": &schema.Schema{
"path": {
Type: schema.TypeString,
Optional: true,
Default: "/",
},
"force_destroy": &schema.Schema{
"permissions_boundary": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringLenBetween(20, 2048),
},
"force_destroy": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Expand All @@ -71,13 +76,19 @@ func resourceAwsIamUserCreate(d *schema.ResourceData, meta interface{}) error {
UserName: aws.String(name),
}

if v, ok := d.GetOk("permissions_boundary"); ok && v.(string) != "" {
request.PermissionsBoundary = aws.String(v.(string))
}

log.Println("[DEBUG] Create IAM User request:", request)
createResp, err := iamconn.CreateUser(request)
if err != nil {
return fmt.Errorf("Error creating IAM User %s: %s", name, err)
}
d.SetId(*createResp.User.UserName)
return resourceAwsIamUserReadResult(d, createResp.User)

d.SetId(aws.StringValue(createResp.User.UserName))

return resourceAwsIamUserRead(d, meta)
}

func resourceAwsIamUserRead(d *schema.ResourceData, meta interface{}) error {
Expand All @@ -87,37 +98,37 @@ func resourceAwsIamUserRead(d *schema.ResourceData, meta interface{}) error {
UserName: aws.String(d.Id()),
}

getResp, err := iamconn.GetUser(request)
output, err := iamconn.GetUser(request)
if err != nil {
if iamerr, ok := err.(awserr.Error); ok && iamerr.Code() == "NoSuchEntity" { // XXX test me
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") { // XXX test me
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been tested? Do we still need this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test added 👍

make testacc TEST=./aws TESTARGS='-run=TestAccAWSUser_disappears'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSUser_disappears -timeout 120m
=== RUN   TestAccAWSUser_disappears
--- PASS: TestAccAWSUser_disappears (8.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	9.475s

log.Printf("[WARN] No IAM user by name (%s) found", d.Id())
d.SetId("")
return nil
}
return fmt.Errorf("Error reading IAM User %s: %s", d.Id(), err)
}
return resourceAwsIamUserReadResult(d, getResp.User)
}

func resourceAwsIamUserReadResult(d *schema.ResourceData, user *iam.User) error {
if err := d.Set("name", user.UserName); err != nil {
return err
}
if err := d.Set("arn", user.Arn); err != nil {
return err
}
if err := d.Set("path", user.Path); err != nil {
return err
if output == nil || output.User == nil {
log.Printf("[WARN] No IAM user by name (%s) found", d.Id())
d.SetId("")
return nil
}
if err := d.Set("unique_id", user.UserId); err != nil {
return err

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("unique_id", output.User.UserId)

return nil
}

func resourceAwsIamUserUpdate(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn

if d.HasChange("name") || d.HasChange("path") {
iamconn := meta.(*AWSClient).iamconn
on, nn := d.GetChange("name")
_, np := d.GetChange("path")

Expand All @@ -130,7 +141,7 @@ func resourceAwsIamUserUpdate(d *schema.ResourceData, meta interface{}) error {
log.Println("[DEBUG] Update IAM User request:", request)
_, err := iamconn.UpdateUser(request)
if err != nil {
if iamerr, ok := err.(awserr.Error); ok && iamerr.Code() == "NoSuchEntity" {
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
log.Printf("[WARN] No IAM user by name (%s) found", d.Id())
d.SetId("")
return nil
Expand All @@ -139,9 +150,31 @@ func resourceAwsIamUserUpdate(d *schema.ResourceData, meta interface{}) error {
}

d.SetId(nn.(string))
return resourceAwsIamUserRead(d, meta)
}
return nil

if d.HasChange("permissions_boundary") {
permissionsBoundary := d.Get("permissions_boundary").(string)
if permissionsBoundary != "" {
input := &iam.PutUserPermissionsBoundaryInput{
PermissionsBoundary: aws.String(permissionsBoundary),
UserName: aws.String(d.Id()),
}
_, err := iamconn.PutUserPermissionsBoundary(input)
if err != nil {
return fmt.Errorf("error updating IAM User permissions boundary: %s", err)
}
} else {
input := &iam.DeleteUserPermissionsBoundaryInput{
UserName: aws.String(d.Id()),
}
_, err := iamconn.DeleteUserPermissionsBoundary(input)
if err != nil {
return fmt.Errorf("error deleting IAM User permissions boundary: %s", err)
}
}
}

return resourceAwsIamUserRead(d, meta)
}

func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -242,14 +275,29 @@ func resourceAwsIamUserDelete(d *schema.ResourceData, meta interface{}) error {
}
}

request := &iam.DeleteUserInput{
deleteUserPermissionsBoundaryInput := &iam.DeleteUserPermissionsBoundaryInput{
UserName: aws.String(d.Id()),
}

log.Println("[DEBUG] Delete IAM User request:", request)
if _, err := iamconn.DeleteUser(request); err != nil {
log.Println("[DEBUG] Delete IAM User Permissions Boundary request:", deleteUserPermissionsBoundaryInput)
_, err = iamconn.DeleteUserPermissionsBoundary(deleteUserPermissionsBoundaryInput)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd that we'd have to delete the boundary before we delete the resource. I'm guessing that deleting a user before the boundary will cause issues but it seems like you'd want two resources here instead of one.

if err != nil && !isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
return fmt.Errorf("Error deleting IAM User %s Permissions Boundary: %s", d.Id(), err)
}

deleteUserInput := &iam.DeleteUserInput{
UserName: aws.String(d.Id()),
}

log.Println("[DEBUG] Delete IAM User request:", deleteUserInput)
_, err = iamconn.DeleteUser(deleteUserInput)
if err != nil {
if isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
return nil
}
return fmt.Errorf("Error deleting IAM User %s: %s", d.Id(), err)
}

return nil
}

Expand Down
82 changes: 72 additions & 10 deletions aws/resource_aws_iam_user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
Expand Down Expand Up @@ -136,6 +135,63 @@ func TestAccAWSUser_pathChange(t *testing.T) {
})
}

func TestAccAWSUser_permissionsBoundary(t *testing.T) {
var user iam.GetUserOutput

rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_iam_user.user"

permissionsBoundary1 := fmt.Sprintf("arn:%s:iam::aws:policy/AdministratorAccess", testAccGetPartition())
permissionsBoundary2 := fmt.Sprintf("arn:%s:iam::aws:policy/ReadOnlyAccess", testAccGetPartition())

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSUserDestroy,
Steps: []resource.TestStep{
// Test creation
{
Config: testAccAWSUserConfig_permissionsBoundary(rName, permissionsBoundary1),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "permissions_boundary", permissionsBoundary1),
),
},
// Test update
{
Config: testAccAWSUserConfig_permissionsBoundary(rName, permissionsBoundary2),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "permissions_boundary", permissionsBoundary2),
),
},
// Test import
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"force_destroy"},
},
// Test removal
{
Config: testAccAWSUserConfig(rName, "/"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "permissions_boundary", ""),
),
},
// Test addition
{
Config: testAccAWSUserConfig_permissionsBoundary(rName, permissionsBoundary1),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "permissions_boundary", permissionsBoundary1),
),
},
},
})
}

func testAccCheckAWSUserDestroy(s *terraform.State) error {
iamconn := testAccProvider.Meta().(*AWSClient).iamconn

Expand All @@ -153,11 +209,7 @@ func testAccCheckAWSUserDestroy(s *terraform.State) error {
}

// Verify the error is what we want
ec2err, ok := err.(awserr.Error)
if !ok {
return err
}
if ec2err.Code() != "NoSuchEntity" {
if !isAWSErr(err, iam.ErrCodeNoSuchEntityException, "") {
return err
}
}
Expand Down Expand Up @@ -205,10 +257,20 @@ func testAccCheckAWSUserAttributes(user *iam.GetUserOutput, name string, path st
}
}

func testAccAWSUserConfig(r, p string) string {
func testAccAWSUserConfig(rName, path string) string {
return fmt.Sprintf(`
resource "aws_iam_user" "user" {
name = "%s"
path = "%s"
}`, r, p)
name = %q
path = %q
}
`, rName, path)
}

func testAccAWSUserConfig_permissionsBoundary(rName, permissionsBoundary string) string {
return fmt.Sprintf(`
resource "aws_iam_user" "user" {
name = %q
permissions_boundary = %q
}
`, rName, permissionsBoundary)
}
1 change: 1 addition & 0 deletions website/docs/r/iam_user.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ The following arguments are supported:

* `name` - (Required) The user's name. The name must consist of upper and lowercase alphanumeric characters with no spaces. You can also include any of the following characters: `=,.@-_.`. User names are not distinguished by case. For example, you cannot create users named both "TESTUSER" and "testuser".
* `path` - (Optional, default "/") Path in which to create the user.
* `permissions_boundary` - (Optional) The ARN of the policy that is used to set the permissions boundary for the user.
* `force_destroy` - (Optional, default false) When destroying this user, destroy even if it
has non-Terraform-managed IAM access keys, login profile or MFA devices. Without `force_destroy`
a user with non-Terraform-managed access keys and login profile will fail to be destroyed.
Expand Down