azurerm_role_assignment - Normalize case of the scope property

[rifelpet]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave "+1" or "me too" comments, they generate extra noise for PR followers and do not help prioritize for review

Description

This relates to #20138

The azurerm_role_assignment resource can be incorrectly replaced because of a casing change in the scope field:

  # azurerm_role_assignment.foo must be replaced
-/+ resource "azurerm_role_assignment" "doo" {
      ~ id                               = "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/example/providers/Microsoft.Authorization/roleAssignments/11111111-1111-1111-1111-111111111111" -> (known after apply)
      ~ name                             = "11111111-1111-1111-1111-111111111111" -> (known after apply)
      ~ principal_type                   = "Group" -> (known after apply)
      ~ role_definition_id               = "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/22222222-2222-2222-2222-222222222222" -> (known after apply)
      ~ scope                            = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/example" -> "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/example" # forces replacement
      + skip_service_principal_aad_check = (known after apply)
        # (2 unchanged attributes hidden)
    }

Notice that the old scope contained resourceGroups and the new scope contains resourcegroups. I confirmed that removing and reimporting the resource in the terraform state does not fix the issue. Instead we add a DiffSuppressFunc on scope to match other fields on this resource.

PR Checklist

• I have followed the guidelines in our Contributing Documentation.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
• I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
• I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
  For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevent documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

The following tests passed. I dont have access to an environment with the necessary permissions to run the remaining acceptance tests.

TF_ACC=1 go test -v ./internal/services/authorization -run=TestAccRoleAssignment -timeout 180m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccRoleAssignment_emptyName
=== PAUSE TestAccRoleAssignment_emptyName
=== RUN   TestAccRoleAssignment_roleName
--- PASS: TestAccRoleAssignment_roleName (44.46s)
=== RUN   TestAccRoleAssignment_requiresImport
--- PASS: TestAccRoleAssignment_requiresImport (46.96s)
=== RUN   TestAccRoleAssignment_dataActions
=== PAUSE TestAccRoleAssignment_dataActions
=== RUN   TestAccRoleAssignment_builtin
=== PAUSE TestAccRoleAssignment_builtin
=== RUN   TestAccRoleAssignment_custom
=== PAUSE TestAccRoleAssignment_custom
=== RUN   TestAccRoleAssignment_ServicePrincipal
=== PAUSE TestAccRoleAssignment_ServicePrincipal
=== RUN   TestAccRoleAssignment_ServicePrincipalWithType
=== PAUSE TestAccRoleAssignment_ServicePrincipalWithType
=== RUN   TestAccRoleAssignment_ServicePrincipalGroup
=== PAUSE TestAccRoleAssignment_ServicePrincipalGroup
=== RUN   TestAccRoleAssignment_managementGroup
=== PAUSE TestAccRoleAssignment_managementGroup
=== RUN   TestAccRoleAssignment_condition
=== PAUSE TestAccRoleAssignment_condition
=== RUN   TestAccRoleAssignment_resourceScoped
=== PAUSE TestAccRoleAssignment_resourceScoped
=== RUN   TestAccRoleAssignment_subscriptionScoped
    role_assignment_resource_test.go:237: Skipping this test as only elevated user account is able to run the test (i.e. via CLI auth)
--- SKIP: TestAccRoleAssignment_subscriptionScoped (0.00s)
=== RUN   TestAccRoleAssignment_resourceGroupScoped
=== PAUSE TestAccRoleAssignment_resourceGroupScoped
=== CONT  TestAccRoleAssignment_custom
=== CONT  TestAccRoleAssignmentMarketplace_emptyName
=== CONT  TestAccRoleAssignmentMarketplace_ServicePrincipalGroup
=== CONT  TestAccRoleAssignmentMarketplace_ServicePrincipal
=== CONT  TestAccRoleAssignmentMarketplace_ServicePrincipalWithType
=== CONT  TestAccRoleAssignment_dataActions
=== CONT  TestAccRoleAssignment_builtin
=== CONT  TestAccRoleAssignment_emptyName
=== CONT  TestAccRoleAssignmentMarketplace_builtin
=== CONT  TestAccRoleAssignment_managementGroup
--- PASS: TestAccRoleAssignment_dataActions (50.83s)
--- PASS: TestAccRoleAssignment_emptyName (51.61s)
--- PASS: TestAccRoleAssignment_builtin (52.52s)
--- PASS: TestAccRoleAssignment_resourceGroupScoped (134.27s)
--- PASS: TestAccRoleAssignment_custom (746.52s)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azurerm_role_assignment - normalize case of the scope property during reads [azurerm_role_assignment - Normalize case of the scope property #26086]

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

This relates to #20138

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

[jackofallops]

Hi @rifelpet - Thanks for this PR. Unfortunately I think this is the wrong way to fix this, CaseDifference frequently results in bugs of other flavours in resource IDs. The normalizeScopeValue func I think should be extended to support all potential ID types (as defined in the Validation function) and provide the correct returned value accordingly. Could you take a look at this?

[rifelpet]

Hi @rifelpet - Thanks for this PR. Unfortunately I think this is the wrong way to fix this, CaseDifference frequently results in bugs of other flavours in resource IDs. The normalizeScopeValue func I think should be extended to support all potential ID types (as defined in the Validation function) and provide the correct returned value accordingly. Could you take a look at this?

@jackofallops I can extend normalizeScopeValue to support all potential ID types, but the function already uses commonids.ParseResourceGroupIDInsensitively() and the drift I'm experiencing is for a ResourceGroup ID as shown in the example plan output in the PR description.

Does normalizeScopeValue() need to be called from more than just d.Set("scope", normalizeScopeValue(pointer.From(props.Scope))), or does anything else need to change around how it is called?

[rifelpet]

Quick ping on this @jackofallops any thoughts on how to proceed?

[mbfrahry]

Hey @rifelpet, apologies for the delay on getting this looked at again. From my understanding of this problem, it looks to be a direct result of what @jackofallops was alluding to in his review that having a resource group id with inconsistent casing results in down stream resources wanting to be recreated.

From the plan you've shown, we have a resource group id with the correct casing in state but the config file has a resource group id with the incorrect casing which is causing Terraform to want to recreate the resource.

I think instead of adding suppress.CaseDifference, we need to figure out where the inconsistent casing is coming from. Do you mind sharing your config file or where the resource group id is coming from so we can try and fix the casing there?

[rifelpet]

Hi @mbfrahry

Here is my terraform configuration. It is in a shared module where only some of the root modules exhibit the behavior reported here, others have a no-op plan as expected. All of the actual role assignments in azure were created from this shared module over the past few years.

resource "azurerm_role_assignment" "foo" {
  scope              = "/subscriptions/${data.azurerm_client_config.current.subscription_id}/resourcegroups/${var.resource_group_name}"
  role_definition_id = azurerm_role_definition.foo.role_definition_resource_id
  principal_id       = azurerm_user_assigned_identity.foo.principal_id
}

Changing the casing from /resourcegroups/ to /resourceGroups/ in this shared module results in similar recreations in the other root modules that currently report a no-op. The fact that some resources created by the provider with the same terraform configuration have different casing returned in the Azure API response leads me to believe the issue is not in my terraform configuration.

we have a resource group id with the correct casing in state

The Azure documentation mentions that resource group names are case insensitive, leading me to also believe that there is no "correct casing" which is why the provider should also treat it as case insensitive.

There are many similar reports in #21085, perhaps this can be fixed for all resources that accept a /subscriptions/.../resourceGroups/... values?

[katbyte]

@rifelpet - while azure will generally treat inputs as insensitive there still is a "correct" or "true" casing for each resource ID as well as standards for what the casing of each resource ID segment should be.

the resourceGroups segment's correct casing is resourceGroups and if changing it to resourcegroups is causing problems elsewhere those are the bugs we should be looking into. could you share what resources are having problems with resourceGroups?