From dcbfaabb954c7b7184bd01ad873acca44d24592e Mon Sep 17 00:00:00 2001 From: Paul Stack Date: Tue, 28 Mar 2017 22:07:50 +0300 Subject: [PATCH] provider/aws: Deprecate roles in favour of role in iam_instance_profile (#13130) * provider/aws: Deprecate roles in favour of role in iam_instance_profile You can only specify a single role to an IAM Instance Profile. So having a slice of roles in the provider makes no sense. Therefore, we are going to deprecate this infavour of `role` ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMInstanceProfile_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/03/28 21:24:20 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMInstanceProfile_ -timeout 120m === RUN TestAccAWSIAMInstanceProfile_importBasic --- PASS: TestAccAWSIAMInstanceProfile_importBasic (25.08s) === RUN TestAccAWSIAMInstanceProfile_basic --- PASS: TestAccAWSIAMInstanceProfile_basic (22.40s) === RUN TestAccAWSIAMInstanceProfile_withRoleNotRoles --- PASS: TestAccAWSIAMInstanceProfile_withRoleNotRoles (22.63s) === RUN TestAccAWSIAMInstanceProfile_missingRoleThrowsError --- PASS: TestAccAWSIAMInstanceProfile_missingRoleThrowsError (4.02s) === RUN TestAccAWSIAMInstanceProfile_namePrefix --- PASS: TestAccAWSIAMInstanceProfile_namePrefix (22.18s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 96.349s ``` * Update iam_instance_profile.html.markdown --- .../aws/resource_aws_iam_instance_profile.go | 61 ++++++++++++++++--- .../resource_aws_iam_instance_profile_test.go | 58 ++++++++++++++++++ .../aws/r/iam_instance_profile.html.markdown | 6 +- 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/builtin/providers/aws/resource_aws_iam_instance_profile.go b/builtin/providers/aws/resource_aws_iam_instance_profile.go index 2e45a655f..8e1d2d68e 100644 --- a/builtin/providers/aws/resource_aws_iam_instance_profile.go +++ b/builtin/providers/aws/resource_aws_iam_instance_profile.go @@ -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"}, }, }, } @@ -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)), @@ -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 { @@ -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") } - return instanceProfileSetRoles(d, iamconn) + if d.HasChange("roles") { + return instanceProfileSetRoles(d, iamconn) + } + + d.Partial(false) + + return nil } func resourceAwsIamInstanceProfileRead(d *schema.ResourceData, meta interface{}) error { @@ -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) diff --git a/builtin/providers/aws/resource_aws_iam_instance_profile_test.go b/builtin/providers/aws/resource_aws_iam_instance_profile_test.go index 0bd3d5ffb..0de137102 100644 --- a/builtin/providers/aws/resource_aws_iam_instance_profile_test.go +++ b/builtin/providers/aws/resource_aws_iam_instance_profile_test.go @@ -2,6 +2,7 @@ package aws import ( "fmt" + "regexp" "strings" "testing" @@ -37,6 +38,8 @@ 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) }, @@ -44,6 +47,41 @@ func TestAccAWSIAMInstanceProfile_basic(t *testing.T) { 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"), }, }, }) @@ -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" { @@ -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) +} diff --git a/website/source/docs/providers/aws/r/iam_instance_profile.html.markdown b/website/source/docs/providers/aws/r/iam_instance_profile.html.markdown index 73b66e6df..9db9be485 100644 --- a/website/source/docs/providers/aws/r/iam_instance_profile.html.markdown +++ b/website/source/docs/providers/aws/r/iam_instance_profile.html.markdown @@ -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 ``` @@ -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. `WARNING: This will be deprecated in a future version of Terraform`. +* `role` - (Optional) The role name to include in the profile. This. ## Attribute Reference @@ -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