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

Add name_prefix to aws_iam_instance_profile and aws_iam_role #6939

Merged
merged 1 commit into from
Jun 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
43 changes: 39 additions & 4 deletions builtin/providers/aws/resource_aws_iam_instance_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"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"
)

Expand All @@ -23,18 +24,23 @@ func resourceAwsIamInstanceProfile() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},

"create_date": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},

"unique_id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},

"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshuaspence as you do not specify a name but do specify a name_prefix, you need to make name Computed. This means that when we set it in the Read, we will not get any alternative plan

Doing this locally gave me this:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMInstanceProfile'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMInstanceProfile -timeout 120m
=== RUN   TestAccAWSIAMInstanceProfile_basic
--- PASS: TestAccAWSIAMInstanceProfile_basic (16.34s)
=== RUN   TestAccAWSIAMInstanceProfile_namePrefix
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (17.01s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    33.367s

ConflictsWith: []string{"name_prefix"},
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
// https://github.com/boto/botocore/blob/2485f5c/botocore/data/iam/2010-05-08/service-2.json#L8196-L8201
value := v.(string)
Expand All @@ -49,12 +55,33 @@ func resourceAwsIamInstanceProfile() *schema.Resource {
return
},
},

"name_prefix": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
// https://github.com/boto/botocore/blob/2485f5c/botocore/data/iam/2010-05-08/service-2.json#L8196-L8201
value := v.(string)
if len(value) > 64 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value probably isn't correct... are there any guarantees made about the length of the string returned by the uuidV4 function?

errors = append(errors, fmt.Errorf(
"%q cannot be longer than 64 characters, name is limited to 128", k))
}
if !regexp.MustCompile("^[\\w+=,.@-]+$").MatchString(value) {
errors = append(errors, fmt.Errorf(
"%q must match [\\w+=,.@-]", k))
}
return
},
},

"path": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
},

"roles": &schema.Schema{
Type: schema.TypeSet,
Required: true,
Expand All @@ -67,7 +94,15 @@ func resourceAwsIamInstanceProfile() *schema.Resource {

func resourceAwsIamInstanceProfileCreate(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn
name := d.Get("name").(string)

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()
}

request := &iam.CreateInstanceProfileInput{
InstanceProfileName: aws.String(name),
Expand Down
112 changes: 112 additions & 0 deletions builtin/providers/aws/resource_aws_iam_instance_profile_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package aws

import (
"fmt"
"strings"
"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/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSIAMInstanceProfile_basic(t *testing.T) {
Expand All @@ -18,6 +24,100 @@ func TestAccAWSIAMInstanceProfile_basic(t *testing.T) {
})
}

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

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: "aws_iam_instance_profile.test",
IDRefreshIgnore: []string{"name_prefix"},
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSInstanceProfileDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSInstanceProfilePrefixNameConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSInstanceProfileExists("aws_iam_instance_profile.test", &conf),
testAccCheckAWSInstanceProfileGeneratedNamePrefix(
"aws_iam_instance_profile.test", "test-"),
),
},
},
})
}

func testAccCheckAWSInstanceProfileGeneratedNamePrefix(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 testAccCheckAWSInstanceProfileDestroy(s *terraform.State) error {
iamconn := testAccProvider.Meta().(*AWSClient).iamconn

for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_iam_instance_profile" {
continue
}

// Try to get role
_, err := iamconn.GetInstanceProfile(&iam.GetInstanceProfileInput{
InstanceProfileName: aws.String(rs.Primary.ID),
})
if err == nil {
return fmt.Errorf("still exist.")
}

// Verify the error is what we want
ec2err, ok := err.(awserr.Error)
if !ok {
return err
}
if ec2err.Code() != "NoSuchEntity" {
return err
}
}

return nil
}

func testAccCheckAWSInstanceProfileExists(n string, res *iam.GetInstanceProfileOutput) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No Instance Profile name is set")
}

iamconn := testAccProvider.Meta().(*AWSClient).iamconn

resp, err := iamconn.GetInstanceProfile(&iam.GetInstanceProfileInput{
InstanceProfileName: aws.String(rs.Primary.ID),
})
if err != nil {
return err
}

*res = *resp

return nil
}
}

const testAccAwsIamInstanceProfileConfig = `
resource "aws_iam_role" "test" {
name = "test"
Expand All @@ -29,3 +129,15 @@ resource "aws_iam_instance_profile" "test" {
roles = ["${aws_iam_role.test.name}"]
}
`

const testAccAWSInstanceProfilePrefixNameConfig = `
resource "aws_iam_role" "test" {
name = "test"
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-"
roles = ["${aws_iam_role.test.name}"]
}
`
43 changes: 39 additions & 4 deletions builtin/providers/aws/resource_aws_iam_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"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"
)

Expand All @@ -23,14 +24,18 @@ func resourceAwsIamRole() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},

"unique_id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},

"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Again this needs to be computed so that when we don't set it as part of our config, the Read can still set it and allow it to not cause Updates :)

Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"name_prefix"},
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
// https://github.com/boto/botocore/blob/2485f5c/botocore/data/iam/2010-05-08/service-2.json#L8329-L8334
value := v.(string)
Expand All @@ -45,12 +50,33 @@ func resourceAwsIamRole() *schema.Resource {
return
},
},

"name_prefix": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
// https://github.com/boto/botocore/blob/2485f5c/botocore/data/iam/2010-05-08/service-2.json#L8329-L8334
value := v.(string)
if len(value) > 32 {
errors = append(errors, fmt.Errorf(
"%q cannot be longer than 32 characters, name is limited to 64", k))
}
if !regexp.MustCompile("^[\\w+=,.@-]*$").MatchString(value) {
errors = append(errors, fmt.Errorf(
"%q must match [\\w+=,.@-]", k))
}
return
},
},

"path": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "/",
ForceNew: true,
},

"assume_role_policy": &schema.Schema{
Type: schema.TypeString,
Required: true,
Expand All @@ -61,7 +87,15 @@ func resourceAwsIamRole() *schema.Resource {

func resourceAwsIamRoleCreate(d *schema.ResourceData, meta interface{}) error {
iamconn := meta.(*AWSClient).iamconn
name := d.Get("name").(string)

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()
}

request := &iam.CreateRoleInput{
Path: aws.String(d.Get("path").(string)),
Expand Down Expand Up @@ -93,6 +127,7 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error {
}
return resourceAwsIamRoleReadResult(d, getResp.Role)
}

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

Expand Down
48 changes: 48 additions & 0 deletions builtin/providers/aws/resource_aws_iam_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"strings"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -30,6 +31,28 @@ func TestAccAWSRole_basic(t *testing.T) {
})
}

func TestAccAWSRole_namePrefix(t *testing.T) {
var conf iam.GetRoleOutput

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: "aws_iam_role.role",
IDRefreshIgnore: []string{"name_prefix"},
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSRolePrefixNameConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
testAccCheckAWSRoleGeneratedNamePrefix(
"aws_iam_role.role", "test-role-"),
),
},
},
})
}

func TestAccAWSRole_testNameChange(t *testing.T) {
var conf iam.GetRoleOutput

Expand Down Expand Up @@ -110,6 +133,23 @@ func testAccCheckAWSRoleExists(n string, res *iam.GetRoleOutput) resource.TestCh
}
}

func testAccCheckAWSRoleGeneratedNamePrefix(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 testAccCheckAWSRoleAttributes(role *iam.GetRoleOutput) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *role.Role.RoleName != "test-role" {
Expand All @@ -131,6 +171,14 @@ resource "aws_iam_role" "role" {
}
`

const testAccAWSRolePrefixNameConfig = `
resource "aws_iam_role" "role" {
name_prefix = "test-role-"
path = "/"
assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}"
}
`

const testAccAWSRolePre = `
resource "aws_iam_role" "role_update_test" {
name = "tf_old_name"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ EOF

The following arguments are supported:

* `name` - (Required) The profile's name.
* `name` - (Optional, Forces new resource) The profile's 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.

Expand Down
Loading