From 0ef68da333ae24b726f637edbb4640582abbf8ee Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 20 Dec 2018 18:15:46 -0800 Subject: [PATCH 1/2] azurerm_role_assignment: fix import when use role name instead of id --- azurerm/resource_arm_role_assignment.go | 24 ++++++++++++++++---- azurerm/resource_arm_role_assignment_test.go | 19 ++++++---------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/azurerm/resource_arm_role_assignment.go b/azurerm/resource_arm_role_assignment.go index f39d386aa4ca..73139a2e24f8 100644 --- a/azurerm/resource_arm_role_assignment.go +++ b/azurerm/resource_arm_role_assignment.go @@ -2,6 +2,7 @@ package azurerm import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "log" "strings" "time" @@ -42,12 +43,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 +76,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 +126,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 +146,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 index 3d051b795949..aa785108d5de 100644 --- a/azurerm/resource_arm_role_assignment_test.go +++ b/azurerm/resource_arm_role_assignment_test.go @@ -42,7 +42,6 @@ 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) }, @@ -50,7 +49,7 @@ func testAccAzureRMRoleAssignment_emptyName(t *testing.T) { CheckDestroy: testCheckAzureRMRoleAssignmentDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMRoleAssignment_emptyNameConfig(), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleAssignmentExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "name"), @@ -68,7 +67,6 @@ 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) }, @@ -76,10 +74,11 @@ func testAccAzureRMRoleAssignment_roleName(t *testing.T) { 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,7 +93,6 @@ 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) }, @@ -102,7 +100,7 @@ func testAccAzureRMRoleAssignment_dataActions(t *testing.T) { CheckDestroy: testCheckAzureRMRoleAssignmentDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMRoleAssignment_dataActionsConfig(id), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleAssignmentExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "role_definition_id"), @@ -120,7 +118,6 @@ 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) }, @@ -128,7 +125,7 @@ func testAccAzureRMRoleAssignment_builtin(t *testing.T) { CheckDestroy: testCheckAzureRMRoleAssignmentDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMRoleAssignment_builtinConfig(id), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleAssignmentExists(resourceName), ), @@ -147,7 +144,6 @@ 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) }, @@ -155,7 +151,7 @@ func testAccAzureRMRoleAssignment_custom(t *testing.T) { CheckDestroy: testCheckAzureRMRoleAssignmentDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMRoleAssignment_customConfig(roleDefinitionId, roleAssignmentId, rInt), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleAssignmentExists(resourceName), ), @@ -226,7 +222,6 @@ 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) }, @@ -234,7 +229,7 @@ func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(t *testing.T) CheckDestroy: testCheckAzureRMActiveDirectoryServicePrincipalDestroy, Steps: []resource.TestStep{ { - Config: config, + Config: testAccAzureRMActiveDirectoryServicePrincipal_roleAssignmentConfig(ri, id), Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryServicePrincipalExists(resourceName), testCheckAzureRMRoleAssignmentExists("azurerm_role_assignment.test"), From 515c44e5a7212762114d16f92ea997a510daeeb8 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 20 Dec 2018 18:30:51 -0800 Subject: [PATCH 2/2] make fmt test build goimport lint --- azurerm/resource_arm_role_assignment.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/azurerm/resource_arm_role_assignment.go b/azurerm/resource_arm_role_assignment.go index 73139a2e24f8..57d08dd8d957 100644 --- a/azurerm/resource_arm_role_assignment.go +++ b/azurerm/resource_arm_role_assignment.go @@ -2,11 +2,12 @@ package azurerm import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "log" "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"