Fixing destroy when role scope is a Management Group

[cmendible]

Fixes #5125: When using azurerm_role_definition to create a role scoped to a Management Group instead to a Subscription or Resource Group, terraform destroy failed cause the scope sent to the API was empty.

This was caused by the parseRoleDefinitionId function which asumes that every roleDefinitionId starts with the scope which is not the case for Roles scoped to a Management Group.

[katbyte]

Hi @cmendible, thank you for this fix

I was wondering if you could add a test that fails without it, and passes with the change so we can ensure it doesn't fail in the future.

Also what is the ID of the role definition when it is scoped to a management group? doesn't it start with the group and then finish with the scope?

[cmendible]

Hi @katbyte no problem I'll add the tests and come back to you with sample IDs

[cmendible]

@katbyte when roles are scoped to a management group, the ID takes the following form:

/providers/Microsoft.Authorization/roleDefinitions/<GUID>

so the scope can not be extracted from it and therefore we must be read the assignable_scopes property.

[katbyte]

Getting an error running the test @cmendible :

------ Stdout: -------
=== RUN   TestAccAzureRMRoleDefinition_managementGroup
=== PAUSE TestAccAzureRMRoleDefinition_managementGroup
=== CONT  TestAccAzureRMRoleDefinition_managementGroup
--- FAIL: TestAccAzureRMRoleDefinition_managementGroup (7.72s)
    testing.go:640: Step 0 error: errors during apply:
        
        Error: Error checking for presence of existing Role Definition ID for "acctestrd-200325225805475020" (Scope "/providers/Microsoft.Management/managementGroups/testMG")
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test132856266/main.tf line 10:
          (source code not available)
 

[cmendible]

Error checking for presence of

Sorry about that, the management group must exist before the test runs. Is there a place where we can create this dependency before running the tests or should I embed the management group creation inside the testAccAzureRMRoleDefinition_managementGroup method?

[cmendible]

@katbyte just added the management group to the test.

[cmendible]

@katbyte just added the management group to the test.

@katbyte can you force and e2e test?

[cmendible]

Hi @katbyte any news on this one?

[javierjeronimo]

Big organisations use Management-Groups to manage Azure Policy and Custom-Roles definitions, this is really needed. Thanks.

[Vrecuero]

I have the same problem. It would really be helpful for Azure-Terraform interactions regarding Management Groups.

[manicminer]

LGTM 👍

[tombuildsstuff]

Aside from needing to store/lookup the scope field somewhere this otherwise LGTM 👍

[tombuildsstuff]

LGTM 👍

[jackofallops]

Tests passing, (failures unrelated)

[ghost]

This has been released in version 2.27.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.27.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!