Fixing destroy when role scope is a Management Group

hashicorp · Mar 27, 2020 · a17632b · a17632b
1 parent 63e1613
commit a17632b
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 3 deletions.
diff --git a/azurerm/internal/services/authorization/resource_arm_role_definition.go b/azurerm/internal/services/authorization/resource_arm_role_definition.go
@@ -185,7 +185,7 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err
 	}
 
 	if id := resp.ID; id != nil {
-		roleDefinitionId, err := parseRoleDefinitionId(*id)
+		roleDefinitionId, err := parseRoleDefinitionId(*id, *resp.RoleDefinitionProperties.AssignableScopes)
 		if err != nil {
 			return fmt.Errorf("Error parsing Role Definition ID: %+v", err)
 		}
@@ -217,7 +217,9 @@ func resourceArmRoleDefinitionDelete(d *schema.ResourceData, meta interface{}) e
 	ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
 	defer cancel()
 
-	id, err := parseRoleDefinitionId(d.Id())
+	scope := d.Get("scope").(string)
+
+	id, err := parseRoleDefinitionId(d.Id(), []string{scope})
 	if err != nil {
 		return err
 	}
@@ -346,8 +348,19 @@ type roleDefinitionId struct {
 	roleDefinitionId string
 }
 
-func parseRoleDefinitionId(input string) (*roleDefinitionId, error) {
+func parseRoleDefinitionId(input string, scopes []string) (*roleDefinitionId, error) {
 	segments := strings.Split(input, "/providers/Microsoft.Authorization/roleDefinitions/")
+
+	// First check if role is scoped to a Management Group
+	if len(scopes) == 1 && strings.HasPrefix(scopes[0], "/providers/Microsoft.Management/managementGroups/") {
+		id := roleDefinitionId{
+			scope:            strings.TrimPrefix(scopes[0], "/"),
+			roleDefinitionId: segments[1],
+		}
+		return &id, nil
+	}
+
+	// Role is scoped to a Subscription
 	if len(segments) != 2 {
 		return nil, fmt.Errorf("Expected Role Definition ID to be in the format `{scope}/providers/Microsoft.Authorization/roleDefinitions/{name}` but got %q", input)
 	}

diff --git a/azurerm/internal/services/authorization/tests/resource_arm_role_definition_test.go b/azurerm/internal/services/authorization/tests/resource_arm_role_definition_test.go
@@ -170,6 +170,26 @@ func TestAccAzureRMRoleDefinition_emptyName(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMRoleDefinition_managementGroup(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_role_definition", "test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acceptance.PreCheck(t) },
+		Providers:    acceptance.SupportedProviders,
+		CheckDestroy: testCheckAzureRMRoleDefinitionDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMRoleDefinition_managementGroup(uuid.New().String(), data),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMRoleDefinitionExists(data.ResourceName),
+					resource.TestCheckResourceAttr(data.ResourceName, "scope", "/providers/Microsoft.Management/managementGroups/testMG"),
+					resource.TestCheckResourceAttr(data.ResourceName, "assignable_scopes.0", "/providers/Microsoft.Management/managementGroups/testMG"),
+				),
+			},
+		},
+	})
+}
+
 func testCheckAzureRMRoleDefinitionExists(resourceName string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		client := acceptance.AzureProvider.Meta().(*clients.Client).Authorization.RoleDefinitionsClient
@@ -374,3 +394,33 @@ resource "azurerm_role_definition" "test" {
 }
 `, data.RandomInteger)
 }
+
+func testAccAzureRMRoleDefinition_managementGroup(id string, data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+resource "azurerm_management_group" "test" {
+}
+
+locals {
+  scope = azurerm_management_group.test.id
+}
+
+resource "azurerm_role_definition" "test" {
+  role_definition_id = "%s"
+  name               = "acctestrd-%d"
+  scope              = local.scope
+
+  permissions {
+    actions     = ["*"]
+    not_actions = []
+  }
+
+  assignable_scopes = [
+    local.scope
+  ]
+}
+`, id, data.RandomInteger)
+}