diff --git a/builtin/providers/aws/import_aws_iam_role_test.go b/builtin/providers/aws/import_aws_iam_role_test.go index ea172721b..37aaace82 100644 --- a/builtin/providers/aws/import_aws_iam_role_test.go +++ b/builtin/providers/aws/import_aws_iam_role_test.go @@ -3,22 +3,24 @@ package aws import ( "testing" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" ) func TestAccAWSIAMRole_importBasic(t *testing.T) { resourceName := "aws_iam_role.role" + rName := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSRoleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSRoleConfig, + { + Config: testAccAWSRoleConfig(rName), }, - resource.TestStep{ + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, diff --git a/builtin/providers/aws/resource_aws_iam_role.go b/builtin/providers/aws/resource_aws_iam_role.go index f79b5a360..c2636ef50 100644 --- a/builtin/providers/aws/resource_aws_iam_role.go +++ b/builtin/providers/aws/resource_aws_iam_role.go @@ -83,8 +83,9 @@ func resourceAwsIamRole() *schema.Resource { }, "description": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateIamRoleDescription, }, "assume_role_policy": { @@ -138,7 +139,8 @@ func resourceAwsIamRoleCreate(d *schema.ResourceData, meta interface{}) error { if err != nil { return fmt.Errorf("Error creating IAM Role %s: %s", name, err) } - return resourceAwsIamRoleReadResult(d, createResp.Role) + d.SetId(*createResp.Role.RoleName) + return resourceAwsIamRoleRead(d, meta) } func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error { @@ -156,7 +158,40 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error { } return fmt.Errorf("Error reading IAM Role %s: %s", d.Id(), err) } - return resourceAwsIamRoleReadResult(d, getResp.Role) + + role := getResp.Role + + if err := d.Set("name", role.RoleName); err != nil { + return err + } + if err := d.Set("arn", role.Arn); err != nil { + return err + } + if err := d.Set("path", role.Path); err != nil { + return err + } + if err := d.Set("unique_id", role.RoleId); err != nil { + return err + } + if err := d.Set("create_date", role.CreateDate.Format(time.RFC3339)); err != nil { + return err + } + + if role.Description != nil { + // the description isn't present in the response to CreateRole. + if err := d.Set("description", role.Description); err != nil { + return err + } + } + + assumRolePolicy, err := url.QueryUnescape(*role.AssumeRolePolicyDocument) + if err != nil { + return err + } + if err := d.Set("assume_role_policy", assumRolePolicy); err != nil { + return err + } + return nil } func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error { @@ -188,44 +223,10 @@ func resourceAwsIamRoleUpdate(d *schema.ResourceData, meta interface{}) error { d.SetId("") return nil } - return fmt.Errorf("Error Updating IAM Role (%s) Description: %s", d.Id(), err) - } - } - return nil -} - -func resourceAwsIamRoleReadResult(d *schema.ResourceData, role *iam.Role) error { - d.SetId(*role.RoleName) - if err := d.Set("name", role.RoleName); err != nil { - return err - } - if err := d.Set("arn", role.Arn); err != nil { - return err - } - if err := d.Set("path", role.Path); err != nil { - return err - } - if err := d.Set("unique_id", role.RoleId); err != nil { - return err - } - if err := d.Set("create_date", role.CreateDate.Format(time.RFC3339)); err != nil { - return err - } - - if role.Description != nil { - // the description isn't present in the response to CreateRole. - if err := d.Set("description", role.Description); err != nil { - return err + return fmt.Errorf("Error Updating IAM Role (%s) Assume Role Policy: %s", d.Id(), err) } } - assumRolePolicy, err := url.QueryUnescape(*role.AssumeRolePolicyDocument) - if err != nil { - return err - } - if err := d.Set("assume_role_policy", assumRolePolicy); err != nil { - return err - } return nil } diff --git a/builtin/providers/aws/resource_aws_iam_role_test.go b/builtin/providers/aws/resource_aws_iam_role_test.go index b6a3eb293..4bc5b844a 100644 --- a/builtin/providers/aws/resource_aws_iam_role_test.go +++ b/builtin/providers/aws/resource_aws_iam_role_test.go @@ -10,12 +10,14 @@ import ( "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" "github.com/hashicorp/terraform/terraform" ) func TestAccAWSRole_basic(t *testing.T) { var conf iam.GetRoleOutput + rName := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -23,13 +25,48 @@ func TestAccAWSRole_basic(t *testing.T) { CheckDestroy: testAccCheckAWSRoleDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRoleConfig, + Config: testAccAWSRoleConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSRoleExists("aws_iam_role.role", &conf), - testAccCheckAWSRoleAttributes(&conf), - resource.TestCheckResourceAttrSet( - "aws_iam_role.role", "create_date", - ), + resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"), + resource.TestCheckResourceAttrSet("aws_iam_role.role", "create_date"), + ), + }, + }, + }) +} + +func TestAccAWSRole_basicWithDescription(t *testing.T) { + var conf iam.GetRoleOutput + rName := acctest.RandString(10) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSRoleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSRoleConfigWithDescription(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSRoleExists("aws_iam_role.role", &conf), + resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"), + resource.TestCheckResourceAttr("aws_iam_role.role", "description", "This 1s a D3scr!pti0n with weird content: &@90ë“‘{«¡Çø}"), + ), + }, + { + Config: testAccAWSRoleConfigWithUpdatedDescription(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSRoleExists("aws_iam_role.role", &conf), + resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"), + resource.TestCheckResourceAttr("aws_iam_role.role", "description", "This 1s an Upd@ted D3scr!pti0n with weird content: &90ë“‘{«¡Çø}"), + ), + }, + { + Config: testAccAWSRoleConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSRoleExists("aws_iam_role.role", &conf), + resource.TestCheckResourceAttrSet("aws_iam_role.role", "create_date"), + resource.TestCheckResourceAttr("aws_iam_role.role", "description", ""), ), }, }, @@ -38,6 +75,7 @@ func TestAccAWSRole_basic(t *testing.T) { func TestAccAWSRole_namePrefix(t *testing.T) { var conf iam.GetRoleOutput + rName := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -47,7 +85,7 @@ func TestAccAWSRole_namePrefix(t *testing.T) { CheckDestroy: testAccCheckAWSRoleDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRolePrefixNameConfig, + Config: testAccAWSRolePrefixNameConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSRoleExists("aws_iam_role.role", &conf), testAccCheckAWSRoleGeneratedNamePrefix( @@ -60,6 +98,7 @@ func TestAccAWSRole_namePrefix(t *testing.T) { func TestAccAWSRole_testNameChange(t *testing.T) { var conf iam.GetRoleOutput + rName := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -67,14 +106,14 @@ func TestAccAWSRole_testNameChange(t *testing.T) { CheckDestroy: testAccCheckAWSRoleDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRolePre, + Config: testAccAWSRolePre(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSRoleExists("aws_iam_role.role_update_test", &conf), ), }, { - Config: testAccAWSRolePost, + Config: testAccAWSRolePost(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSRoleExists("aws_iam_role.role_update_test", &conf), ), @@ -84,13 +123,15 @@ func TestAccAWSRole_testNameChange(t *testing.T) { } func TestAccAWSRole_badJSON(t *testing.T) { + rName := acctest.RandString(10) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSRoleDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRoleConfig_badJson, + Config: testAccAWSRoleConfig_badJson(rName), ExpectError: regexp.MustCompile(`.*contains an invalid JSON:.*`), }, }, @@ -169,43 +210,52 @@ func testAccCheckAWSRoleGeneratedNamePrefix(resource, prefix string) resource.Te } } -func testAccCheckAWSRoleAttributes(role *iam.GetRoleOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - if *role.Role.RoleName != "test-role" { - return fmt.Errorf("Bad name: %s", *role.Role.RoleName) - } - - if *role.Role.Path != "/" { - return fmt.Errorf("Bad path: %s", *role.Role.Path) - } - - if *role.Role.Description != "Test Role" { - return fmt.Errorf("Bad description: %s", *role.Role.Description) - } - return nil - } -} - -const testAccAWSRoleConfig = ` +func testAccAWSRoleConfig(rName string) string { + return fmt.Sprintf(` resource "aws_iam_role" "role" { - name = "test-role" - path = "/" - description = "Test Role" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" -} -` - -const testAccAWSRolePrefixNameConfig = ` -resource "aws_iam_role" "role" { - name_prefix = "test-role-" + name = "test-role-%s" path = "/" assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" } -` +`, rName) +} -const testAccAWSRolePre = ` +func testAccAWSRoleConfigWithDescription(rName string) string { + return fmt.Sprintf(` +resource "aws_iam_role" "role" { + name = "test-role-%s" + description = "This 1s a D3scr!pti0n with weird content: &@90ë“‘{«¡Çø}" + path = "/" + assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" +} +`, rName) +} + +func testAccAWSRoleConfigWithUpdatedDescription(rName string) string { + return fmt.Sprintf(` +resource "aws_iam_role" "role" { + name = "test-role-%s" + description = "This 1s an Upd@ted D3scr!pti0n with weird content: &90ë“‘{«¡Çø}" + path = "/" + assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" +} +`, rName) +} + +func testAccAWSRolePrefixNameConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_iam_role" "role" { + name_prefix = "test-role-%s" + path = "/" + assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"ec2.amazonaws.com\"]},\"Action\":[\"sts:AssumeRole\"]}]}" +} +`, rName) +} + +func testAccAWSRolePre(rName string) string { + return fmt.Sprintf(` resource "aws_iam_role" "role_update_test" { - name = "tf_old_name" + name = "tf_old_name_%s" path = "/test/" assume_role_policy = < 1000 { + errors = append(errors, fmt.Errorf("%q cannot be longer than 1000 caracters", k)) + } + + if !regexp.MustCompile(`[\p{L}\p{M}\p{Z}\p{S}\p{N}\p{P}]*`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "Only alphanumeric & accented characters allowed in %q: %q (Must satisfy regular expression pattern: [\\p{L}\\p{M}\\p{Z}\\p{S}\\p{N}\\p{P}]*)", + k, value)) + } + return +} diff --git a/builtin/providers/aws/validators_test.go b/builtin/providers/aws/validators_test.go index b344f206d..3d323ca51 100644 --- a/builtin/providers/aws/validators_test.go +++ b/builtin/providers/aws/validators_test.go @@ -2209,3 +2209,26 @@ func TestValidateWafMetricName(t *testing.T) { } } } + +func TestValidateIamRoleDescription(t *testing.T) { + validNames := []string{ + "This 1s a D3scr!pti0n with weird content: @ #^ù£ê®æ ø]ŒîÏî~ÈÙ£÷=,ë", + strings.Repeat("W", 1000), + } + for _, v := range validNames { + _, errors := validateIamRoleDescription(v, "description") + if len(errors) != 0 { + t.Fatalf("%q should be a valid IAM Role Description: %q", v, errors) + } + } + + invalidNames := []string{ + strings.Repeat("W", 1001), // > 1000 + } + for _, v := range invalidNames { + _, errors := validateIamRoleDescription(v, "description") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid IAM Role Description", v) + } + } +}