[Bug Report]: v0.10.0 breaks deployments with existing Resource Groups (deployed with CARML <= 0.9.0)

[zydio]

Describe the bug

Release 0.10.0 shows the following entry in the Fixes section:

• Update Resource Group roleAssignments name (GUID) to use Resource Group resourceID instead of Resource Group name

There are two problems:

1. This is a Breaking Change and this is not advertised in the release notes
2. This is a Breaking Change and makes it impossible to successfully run a deployment including existing Resource Groups that were first deployed with BICEPs using CARML <= 0.9.0

Looking in the repo history, this change comes from PR #2642 , which is actually marked as containing Breaking change

Regarding point 2, if you have deployed resource groups with role assignments with CARML <= 0.9.0, and you try to run a deployment again of the same BICEP with CARML 0.10.0 you get a conflict with error The role assignment already exists. and the deployment fails.
ARM is right to complain, because the assignment exists but with a different GUID as a name.

New BICEP compile versions (for sure from 0.15.31 onwards) don't complain anymore for the error shown in the ticket.
That line should be changed to something like:
useLegacyRoleAssignmentNaming ? guid(last(split(resourceId, '/')), principalId, roleDefinitionIdOrName) : guid(resourceId, principalId, roleDefinitionIdOrName)

And a useLegacyRoleAssignmentNaming param should be added to modules\Microsoft.Resources\resourceGroups.bicep\nested_roleAssignments.bicep and surfaced in modules\Microsoft.Resources\resourceGroups\deploy.bicep so that ARM can handle new deployments on existing resources deployed with old versions.

To reproduce

Deploy a Resource Group with the CARML module <= 0.9.0 and specify a Role Assignment on the Resource Group.

Update CARML to 0.10.0 and run the same deployment: it will fail with error The role assignment already exists.

Code snippet

module resourceGroups 'CARML/modules/Microsoft.Resources/resourceGroups/deploy.bicep' = {
  name: '${uniqueString(deployment().name)}-ResourceGroup-test'
  params: {
    name: 'breaking-change-test-rg'
    location: 'westeurope'
    roleAssignments: [
        {
          principalIds: [
             // ..Write here a principalId..
          ]
          roleDefinitionIdOrName: 'Reader'
        }
      ]
    }
  }
  
}]

Relevant log output

The role assignment already exists.

[eriqua]

Hi @zydio, thanks for pointing this out.

While breaking changes are not ideal, we had to introduce this one for three main reasons:

1. Following RBAC naming best practices (ref Create Azure RBAC resources by using Bicep )
2. Consistency with nested RBAC implementation at resource level for other modules
3. Fixing an error thrown by the Bicep compiler once upgrading Bicep to release 0.14.6
   Argument of type "null | string" is not assignable to parameter of type "string".bicep(BCP070)

You may ref to PR #2642 introducing this change, for additional context.

To solve the already existing role assignment, a possible workaround would be to delete the Azure role assignment and redeploy the solution using the new module. Would that be feasible in your scenario?

Please let us know if that helps!

[zydio]

Hi @eriqua, I know bout the issue introduced by bicep 0.14.6, that's what prevented us from updating bicep itself for a while, and also CARML modules as well.

I do understand and see the reasons behind the change itself, what I'm questioning is the lack of configurability of this breaking change.
It would have been easy to add a parameter and generate a "legacy" RBAC name or a "new" RBAC name compliant to the best practices.

I'd rather see this parameter added in this case, but I would also expect the same retro-compatible approach to be added to CARML contribution guidelines to safeguard all users in similar cases in the future.
The CARML team can't expect everybody to delete and fix things manually around tens or hundreds of resources for something that could be dealt with a simple parameter in the module. This defeats the purpose of using IaC and BICEP.

In this specific case removing role assignments around and having to re-deploy with BICEP using new CARML version in order to re-align everything means to us providing hours long unavailability to the system, because everything revolves around RBAC grants obviously.

The lack of care in dealing with breaking changes is something that can only strongly discourage the use of CARML in real life, because everyone in a couple of releases will see what we have seen: the havoc created by CARML upgrades and the effort and time required to understand the root cause and fix it is far more than the benefit of using CARML in the first place.

[AlexanderSehr]

I'll close this issue in favor of Azure/bicep-registry-modules#2008 as it's implementation will provide a workaround via a configurable role assignment GUID. Truth be told, it's not comprehensable why the role assignment resource provider would have this limitation to begin with - and I sincerely hope it will be addressed in the future.

That being said, to also address CARML's tendency to introduce breaking changes without adding backwards-compatibility flags - this is 'somewhat' by design. The module's in CARML were always intended to shared the latest version of modules that we and others may be using when deploying infrastructure. The version.json file alongside the CI where hence intended to inner-source the library and use it from there on out - and by being open-source, allowing users to modify the module source-code as they see fit.
I know, that makes the 'issue' really easy for us to ignore, but considering the opposite - that is, to provide backwards compatibility for every breaking change for an abstraction layer that is built on top of an Infrastructure-as-Code language can render the code eventually extremly hard to maintain / too complex to use. Can't say I see a clear good solution for the matter.

PS: With the move of the module code into AVM / BRM (see linked issue) the CI will continue as its own standalone product that will remain focused on providing an inner-source environment to deploy if one is needed.