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

provider/aws: Deprecate roles in favour of role in iam_instance_profile #13130

Merged
merged 2 commits into from
Mar 28, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
61 changes: 53 additions & 8 deletions builtin/providers/aws/resource_aws_iam_instance_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,20 @@ func resourceAwsIamInstanceProfile() *schema.Resource {
},

"roles": {
Type: schema.TypeSet,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
Type: schema.TypeSet,
Optional: true,
Computed: true,
ConflictsWith: []string{"role"},
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
Deprecated: "Use `role` instead. Only a single role can be passed to an IAM Instance Profile",
},

"role": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ConflictsWith: []string{"roles"},
},
},
}
Expand All @@ -107,6 +117,13 @@ func resourceAwsIamInstanceProfileCreate(d *schema.ResourceData, meta interface{
name = resource.UniqueId()
}

_, hasRoles := d.GetOk("roles")
_, hasRole := d.GetOk("role")

if hasRole == false && hasRoles == false {
return fmt.Errorf("Either `roles` or `role` must be specified when creating an IAM Instance Profile")
}

request := &iam.CreateInstanceProfileInput{
InstanceProfileName: aws.String(name),
Path: aws.String(d.Get("path").(string)),
Expand All @@ -132,7 +149,7 @@ func resourceAwsIamInstanceProfileCreate(d *schema.ResourceData, meta interface{
return fmt.Errorf("Timed out while waiting for instance profile %s: %s", name, err)
}

return instanceProfileSetRoles(d, iamconn)
return resourceAwsIamInstanceProfileUpdate(d, meta)
}

func instanceProfileAddRole(iamconn *iam.IAM, profileName, roleName string) error {
Expand Down Expand Up @@ -205,11 +222,35 @@ func instanceProfileRemoveAllRoles(d *schema.ResourceData, iamconn *iam.IAM) err
func resourceAwsIamInstanceProfileUpdate(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn

if !d.HasChange("roles") {
return nil
d.Partial(true)

if d.HasChange("role") {
oldRole, newRole := d.GetChange("role")

if oldRole.(string) != "" {
err := instanceProfileRemoveRole(iamconn, d.Id(), oldRole.(string))
if err != nil {
return fmt.Errorf("Error adding role %s to IAM instance profile %s: %s", oldRole.(string), d.Id(), err)
}
}

if newRole.(string) != "" {
err := instanceProfileAddRole(iamconn, d.Id(), newRole.(string))
if err != nil {
return fmt.Errorf("Error adding role %s to IAM instance profile %s: %s", newRole.(string), d.Id(), err)
}
}

d.SetPartial("role")
}

if d.HasChange("roles") {
return instanceProfileSetRoles(d, iamconn)
}

return instanceProfileSetRoles(d, iamconn)
d.Partial(false)

return nil
}

func resourceAwsIamInstanceProfileRead(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -262,6 +303,10 @@ func instanceProfileReadResult(d *schema.ResourceData, result *iam.InstanceProfi
}
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
}

roles := &schema.Set{F: schema.HashString}
for _, role := range result.Roles {
roles.Add(*role.RoleName)
Expand Down
58 changes: 58 additions & 0 deletions builtin/providers/aws/resource_aws_iam_instance_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -37,13 +38,50 @@ func TestAccAWSIAMInstanceProfile_importBasic(t *testing.T) {
}

func TestAccAWSIAMInstanceProfile_basic(t *testing.T) {
var conf iam.GetInstanceProfileOutput

rName := acctest.RandString(5)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccAwsIamInstanceProfileConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSInstanceProfileExists("aws_iam_instance_profile.test", &conf),
),
},
},
})
}

func TestAccAWSIAMInstanceProfile_withRoleNotRoles(t *testing.T) {
var conf iam.GetInstanceProfileOutput

rName := acctest.RandString(5)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccAWSInstanceProfileWithRoleSpecified(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSInstanceProfileExists("aws_iam_instance_profile.test", &conf),
),
},
},
})
}

func TestAccAWSIAMInstanceProfile_missingRoleThrowsError(t *testing.T) {
rName := acctest.RandString(5)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccAwsIamInstanceProfileConfigMissingRole(rName),
ExpectError: regexp.MustCompile("Either `roles` or `role` must be specified when creating an IAM Instance Profile"),
},
},
})
Expand Down Expand Up @@ -157,6 +195,13 @@ resource "aws_iam_instance_profile" "test" {
}`, rName)
}

func testAccAwsIamInstanceProfileConfigMissingRole(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_instance_profile" "test" {
name = "test-%s"
}`, rName)
}

func testAccAWSInstanceProfilePrefixNameConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "test" {
Expand All @@ -169,3 +214,16 @@ resource "aws_iam_instance_profile" "test" {
roles = ["${aws_iam_role.test.name}"]
}`, rName)
}

func testAccAWSInstanceProfileWithRoleSpecified(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "test" {
name = "test-%s"
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_prefix = "test-"
role = "${aws_iam_role.test.name}"
}`, rName)
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ description: |-

Provides an IAM instance profile.

~> **NOTE:** Either `roles` or `role` must be specified in the IAM Instance Profile.

## Example Usage

```
Expand Down Expand Up @@ -47,7 +49,8 @@ The following arguments are supported:
* `name` - (Optional, Forces new resource) The profile's name. If omitted, Terraform will assign a random, unique name.
* `name_prefix` - (Optional, Forces new resource) Creates a unique name beginning with the specified prefix. Conflicts with `name`.
* `path` - (Optional, default "/") Path in which to create the profile.
* `roles` - (Required) A list of role names to include in the profile. The current default is 1. If you see an error message similar to `Cannot exceed quota for InstanceSessionsPerInstanceProfile: 1`, then you must contact AWS support and ask for a limit increase.
* `roles` - (Optional) A list of role names to include in the profile. The current default is 1. If you see an error message similar to `Cannot exceed quota for InstanceSessionsPerInstanceProfile: 1`, then you must contact AWS support and ask for a limit increase.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a deprecation notice here?

* `role` - (Optional) The role name to include in the profile. This.

## Attribute Reference

Expand All @@ -57,6 +60,7 @@ The following arguments are supported:
* `name` - The instance profile's name.
* `path` - The path of the instance profile in IAM.
* `roles` - The list of roles assigned to the instance profile.
* `role` - The role assigned to the instance profile
* `unique_id` - The [unique ID][1] assigned by AWS.

[1]: https://docs.aws.amazon.com/IAM/latest/UserGuide/Using_Identifiers.html#GUIDs
Expand Down