azurerm_role_definition - enture role_definition_id is correctl… (#3913)

We encountered the issue where role_definition_id remained nil after create, returning 409s on RoleDefinitionsClient.CreateOrUpdate with code RoleDefinitionWithSameNameExists. Updating role definition resource was failing because resourceArmRoleDefinitionCreateUpdate checks if role_definition_id is nil and, if so, creates a new uuid and calls client.CreateOrUpdate which attempts to create a new resource instead of updating.
hashicorp · Aug 2, 2019 · a5b6317 · a5b6317
1 parent 532334d
commit a5b6317
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 0 deletions.
diff --git a/azurerm/resource_arm_role_definition.go b/azurerm/resource_arm_role_definition.go
@@ -172,6 +172,16 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err
 		return fmt.Errorf("Error loading Role Definition %q: %+v", d.Id(), err)
 	}
 
+	if id := resp.ID; id != nil {
+		roleDefinitionId, err := parseRoleDefinitionId(*id)
+		if err != nil {
+			return fmt.Errorf("Error parsing Role Definition ID: %+v", err)
+		}
+		if roleDefinitionId != nil {
+			d.Set("role_definition_id", roleDefinitionId.roleDefinitionId)
+		}
+	}
+
 	if props := resp.RoleDefinitionProperties; props != nil {
 		d.Set("name", props.RoleName)
 		d.Set("description", props.Description)

diff --git a/azurerm/resource_arm_role_definition_test.go b/azurerm/resource_arm_role_definition_test.go
@@ -125,6 +125,40 @@ func TestAccAzureRMRoleDefinition_update(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMRoleDefinition_updateEmptyId(t *testing.T) {
+	resourceName := "azurerm_role_definition.test"
+	ri := tf.AccRandTimeInt()
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMRoleDefinitionDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMRoleDefinition_emptyId(ri),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMRoleDefinitionExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "permissions.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.actions.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.actions.0", "*"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.not_actions.#", "0"),
+				),
+			},
+			{
+				Config: testAccAzureRMRoleDefinition_updateEmptyId(ri),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMRoleDefinitionExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "permissions.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.actions.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.actions.0", "*"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.not_actions.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "permissions.0.not_actions.0", "Microsoft.Authorization/*/read"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAzureRMRoleDefinition_emptyName(t *testing.T) {
 	resourceName := "azurerm_role_definition.test"
 	ri := tf.AccRandTimeInt()
@@ -303,3 +337,23 @@ resource "azurerm_role_definition" "test" {
 }
 `, rInt)
 }
+
+func testAccAzureRMRoleDefinition_updateEmptyId(rInt int) string {
+	return fmt.Sprintf(`
+data "azurerm_subscription" "primary" {}
+
+resource "azurerm_role_definition" "test" {
+  name  = "acctestrd-%d"
+  scope = "${data.azurerm_subscription.primary.id}"
+
+  permissions {
+    actions     = ["*"]
+    not_actions = ["Microsoft.Authorization/*/read"]
+  }
+
+  assignable_scopes = [
+    "${data.azurerm_subscription.primary.id}",
+  ]
+}
+`, rInt)
+}