Merge pull request #2565 from terraform-providers/roles/assingment-by…

…-name azurerm_role_assignment: fix import when use role name instead of id
hashicorp · Dec 23, 2018 · cb345fb · cb345fb
2 parents 0a4ef4b + 515c44e
commit cb345fb
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 17 deletions.
diff --git a/azurerm/resource_arm_role_assignment.go b/azurerm/resource_arm_role_assignment.go
@@ -6,6 +6,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress"
+
 	"github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2018-01-01-preview/authorization"
 	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/terraform/helper/resource"
@@ -42,12 +44,13 @@ func resourceArmRoleAssignment() *schema.Resource {
 				Computed:         true,
 				ForceNew:         true,
 				ConflictsWith:    []string{"role_definition_name"},
-				DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
+				DiffSuppressFunc: suppress.CaseDifference,
 			},
 
 			"role_definition_name": {
 				Type:          schema.TypeString,
 				Optional:      true,
+				Computed:      true,
 				ForceNew:      true,
 				ConflictsWith: []string{"role_definition_id"},
 				ValidateFunc:  validateRoleDefinitionName,
@@ -74,14 +77,13 @@ func resourceArmRoleAssignmentCreate(d *schema.ResourceData, meta interface{}) e
 	if v, ok := d.GetOk("role_definition_id"); ok {
 		roleDefinitionId = v.(string)
 	} else if v, ok := d.GetOk("role_definition_name"); ok {
-		value := v.(string)
-		filter := fmt.Sprintf("roleName eq '%s'", value)
-		roleDefinitions, err := roleDefinitionsClient.List(ctx, "", filter)
+		roleName := v.(string)
+		roleDefinitions, err := roleDefinitionsClient.List(ctx, "", fmt.Sprintf("roleName eq '%s'", roleName))
 		if err != nil {
 			return fmt.Errorf("Error loading Role Definition List: %+v", err)
 		}
 		if len(roleDefinitions.Values()) != 1 {
-			return fmt.Errorf("Error loading Role Definition List: could not find role '%s'", value)
+			return fmt.Errorf("Error loading Role Definition List: could not find role '%s'", roleName)
 		}
 		roleDefinitionId = *roleDefinitions.Values()[0].ID
 	} else {
@@ -125,6 +127,7 @@ func resourceArmRoleAssignmentCreate(d *schema.ResourceData, meta interface{}) e
 
 func resourceArmRoleAssignmentRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*ArmClient).roleAssignmentsClient
+	roleDefinitionsClient := meta.(*ArmClient).roleDefinitionsClient
 	ctx := meta.(*ArmClient).StopContext
 
 	resp, err := client.GetByID(ctx, d.Id())
@@ -144,6 +147,18 @@ func resourceArmRoleAssignmentRead(d *schema.ResourceData, meta interface{}) err
 		d.Set("scope", props.Scope)
 		d.Set("role_definition_id", props.RoleDefinitionID)
 		d.Set("principal_id", props.PrincipalID)
+
+		//allows for import when role name is used (also if the role name changes a plan will show a diff)
+		if roleId := props.RoleDefinitionID; roleId != nil {
+			roleResp, err := roleDefinitionsClient.GetByID(ctx, *roleId)
+			if err != nil {
+				return fmt.Errorf("Error loading Role Definition %q: %+v", *roleId, err)
+			}
+
+			if roleProps := roleResp.RoleDefinitionProperties; props != nil {
+				d.Set("role_definition_name", roleProps.RoleName)
+			}
+		}
 	}
 
 	return nil

diff --git a/azurerm/resource_arm_role_assignment_test.go b/azurerm/resource_arm_role_assignment_test.go
@@ -42,15 +42,14 @@ func TestAccAzureRMRoleAssignment(t *testing.T) {
 
 func testAccAzureRMRoleAssignment_emptyName(t *testing.T) {
 	resourceName := "azurerm_role_assignment.test"
-	config := testAccAzureRMRoleAssignment_emptyNameConfig()
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMRoleAssignmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMRoleAssignment_emptyNameConfig(),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMRoleAssignmentExists(resourceName),
 					resource.TestCheckResourceAttrSet(resourceName, "name"),
@@ -68,18 +67,18 @@ func testAccAzureRMRoleAssignment_emptyName(t *testing.T) {
 func testAccAzureRMRoleAssignment_roleName(t *testing.T) {
 	resourceName := "azurerm_role_assignment.test"
 	id := uuid.New().String()
-	config := testAccAzureRMRoleAssignment_roleNameConfig(id)
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMRoleAssignmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMRoleAssignment_roleNameConfig(id),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMRoleAssignmentExists(resourceName),
 					resource.TestCheckResourceAttrSet(resourceName, "role_definition_id"),
+					resource.TestCheckResourceAttr(resourceName, "role_definition_name", "Log Analytics Reader"),
 				),
 			},
 			{
@@ -94,15 +93,14 @@ func testAccAzureRMRoleAssignment_roleName(t *testing.T) {
 func testAccAzureRMRoleAssignment_dataActions(t *testing.T) {
 	id := uuid.New().String()
 	resourceName := "azurerm_role_assignment.test"
-	config := testAccAzureRMRoleAssignment_dataActionsConfig(id)
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMRoleAssignmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMRoleAssignment_dataActionsConfig(id),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMRoleAssignmentExists(resourceName),
 					resource.TestCheckResourceAttrSet(resourceName, "role_definition_id"),
@@ -120,15 +118,14 @@ func testAccAzureRMRoleAssignment_dataActions(t *testing.T) {
 func testAccAzureRMRoleAssignment_builtin(t *testing.T) {
 	resourceName := "azurerm_role_assignment.test"
 	id := uuid.New().String()
-	config := testAccAzureRMRoleAssignment_builtinConfig(id)
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMRoleAssignmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMRoleAssignment_builtinConfig(id),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMRoleAssignmentExists(resourceName),
 				),
@@ -147,15 +144,14 @@ func testAccAzureRMRoleAssignment_custom(t *testing.T) {
 	roleDefinitionId := uuid.New().String()
 	roleAssignmentId := uuid.New().String()
 	rInt := acctest.RandInt()
-	config := testAccAzureRMRoleAssignment_customConfig(roleDefinitionId, roleAssignmentId, rInt)
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMRoleAssignmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMRoleAssignment_customConfig(roleDefinitionId, roleAssignmentId, rInt),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMRoleAssignmentExists(resourceName),
 				),
@@ -226,15 +222,14 @@ func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(t *testing.T)
 
 	ri := acctest.RandInt()
 	id := uuid.New().String()
-	config := testAccAzureRMActiveDirectoryServicePrincipal_roleAssignmentConfig(ri, id)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMActiveDirectoryServicePrincipalDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: config,
+				Config: testAccAzureRMActiveDirectoryServicePrincipal_roleAssignmentConfig(ri, id),
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMActiveDirectoryServicePrincipalExists(resourceName),
 					testCheckAzureRMRoleAssignmentExists("azurerm_role_assignment.test"),