r/aws_iam_role: Use internal naming package.

coveooss · Sep 10, 2021 · 83985f6 · 83985f6
1 parent a6c28f4
commit 83985f6
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 39 deletions.
diff --git a/.changelog/20785.txt b/.changelog/20785.txt
@@ -1,3 +1,7 @@
 ```release-note:bug
-resource_aws_iam_role: Changes `name_prefix` validation to a range of 1 ~ 64 characters to match the AWS documentation
+resource/aws_iam_role: Change `name_prefix` validation to a range of 1 to 38 characters
 ```
+
+```release-note:enhancement
+resource/aws_iam_role: `name_prefix` is now Computed 
+```
diff --git a/aws/resource_aws_iam_role.go b/aws/resource_aws_iam_role.go
@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/naming"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
 )
@@ -54,10 +55,11 @@ func resourceAwsIamRole() *schema.Resource {
 			"name_prefix": {
 				Type:          schema.TypeString,
 				Optional:      true,
+				Computed:      true,
 				ForceNew:      true,
 				ConflictsWith: []string{"name"},
 				ValidateFunc: validation.All(
-					validation.StringLenBetween(1, 64),
+					validation.StringLenBetween(1, 64-resource.UniqueIDSuffixLength),
 					validation.StringMatch(regexp.MustCompile(`^[\w+=,.@-]*$`), "must match [\\w+=,.@-]"),
 				),
 			},
@@ -161,15 +163,7 @@ func resourceAwsIamRoleCreate(d *schema.ResourceData, meta interface{}) error {
 	defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig
 	tags := defaultTagsConfig.MergeTags(keyvaluetags.New(d.Get("tags").(map[string]interface{})))
 
-	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()
-	}
-
+	name := naming.Generate(d.Get("name").(string), d.Get("name_prefix").(string))
 	request := &iam.CreateRoleInput{
 		Path:                     aws.String(d.Get("path").(string)),
 		RoleName:                 aws.String(name),
@@ -287,6 +281,7 @@ func resourceAwsIamRoleRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("description", role.Description)
 	d.Set("max_session_duration", role.MaxSessionDuration)
 	d.Set("name", role.RoleName)
+	d.Set("name_prefix", naming.NamePrefixFromName(aws.StringValue(role.RoleName)))
 	d.Set("path", role.Path)
 	if role.PermissionsBoundary != nil {
 		d.Set("permissions_boundary", role.PermissionsBoundary.PermissionsBoundaryArn)

diff --git a/aws/resource_aws_iam_role_test.go b/aws/resource_aws_iam_role_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/naming"
 )
 
 func init() {
@@ -225,9 +226,8 @@ func TestAccAWSIAMRole_basicWithDescription(t *testing.T) {
 	})
 }
 
-func TestAccAWSIAMRole_namePrefix(t *testing.T) {
+func TestAccAWSIAMRole_NameGenerated(t *testing.T) {
 	var conf iam.GetRoleOutput
-	rName := acctest.RandString(10)
 	resourceName := "aws_iam_role.test"
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -237,18 +237,45 @@ func TestAccAWSIAMRole_namePrefix(t *testing.T) {
 		CheckDestroy: testAccCheckAWSRoleDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSIAMRolePrefixNameConfig(rName),
+				Config: testAccAWSIAMRoleConfigNameGenerated(),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSRoleExists(resourceName, &conf),
-					testAccCheckAWSRoleGeneratedNamePrefix(
-						resourceName, "test-role-"),
+					naming.TestCheckResourceAttrNameGenerated(resourceName, "name"),
+					resource.TestCheckResourceAttr(resourceName, "name_prefix", "terraform-"),
 				),
 			},
 			{
-				ResourceName:            resourceName,
-				ImportState:             true,
-				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"name_prefix"},
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSIAMRole_NamePrefix(t *testing.T) {
+	var conf iam.GetRoleOutput
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	resourceName := "aws_iam_role.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, iam.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRoleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSIAMRoleConfigNamePrefix(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRoleExists(resourceName, &conf),
+					naming.TestCheckResourceAttrNameFromPrefix(resourceName, "name", rName),
+					resource.TestCheckResourceAttr(resourceName, "name_prefix", rName),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
 			},
 		},
 	})
@@ -970,23 +997,6 @@ func testAccCheckAWSRoleDisappears(getRoleOutput *iam.GetRoleOutput) resource.Te
 	}
 }
 
-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
-	}
-}
-
 // Attach inline policy out of band (outside of terraform)
 func testAccAddAwsIAMRolePolicy(n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
@@ -1298,12 +1308,41 @@ EOF
 `, rName)
 }
 
-func testAccAWSIAMRolePrefixNameConfig(rName string) string {
+func testAccAWSIAMRoleConfigNameGenerated() string {
+	return `
+data "aws_partition" "current" {}
+
+resource "aws_iam_role" "test" {
+  path = "/"
+
+  assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "ec2.${data.aws_partition.current.dns_suffix}"
+        ]
+      },
+      "Action": [
+        "sts:AssumeRole"
+      ]
+    }
+  ]
+}
+EOF
+}
+`
+}
+
+func testAccAWSIAMRoleConfigNamePrefix(rName string) string {
 	return fmt.Sprintf(`
 data "aws_partition" "current" {}
 
 resource "aws_iam_role" "test" {
-  name_prefix = "test-role-%s"
+  name_prefix = %[1]q
   path        = "/"
 
   assume_role_policy = <<EOF