[Bug Report]: Key Vault Role assignments should use more inputs to guid

[jongio]

Module path

avm/res/key-vault/vault

Bicep version

0.21.1.54444

Describe the bug

If I re-run a deployment I'm getting a conflict on the role assignment used here: https://github.com/Azure/bicep-registry-modules/blob/main/avm/res/key-vault/vault/main.bicep#L267C14-L267C92

One way around that is to use more inputs to the guid parameter like we do here: https://github.com/Azure/azure-dev/blob/main/templates/common/infra/bicep/core/security/role.bicep#L15

To reproduce

Run a template with the following, multiple times.

module keyVault 'br/public:avm-res-keyvault-vault:0.1.0' = {
  name: 'keyvault'
  scope: rg
  params: {
    name: !empty(keyVaultName) ? keyVaultName : '${abbrs.keyVaultVaults}${resourceToken}'
    location: location
    tags: tags
    roleAssignments: !empty(principalId) ? [
      {
        principalId: principalId
        principalType: 'User'
        roleDefinitionIdOrName: '4633458b-17de-408a-b874-0445c86b69e6'
      }
    ] : []
  }
}

Code snippet

.

Relevant log output

.

[AlexanderSehr]

Unfortunately, if there is a pre-existing role assignment with a different name, there is not much we can do from a module perspective. This is an unfortunate "design" of role assignments in Azure. The only solution I know of is to remove the role assignment in Azure once and redeploy. Adding more inputs doesn't solve the issue (the ID is already unique thanks to its elements). If you have a pre-existing role assignment and change the name of the role assignment in any way (e.g., by adding different segments) the effect is simply that ARM will tell you that there already is a role assignment with a different name.

[AlexanderSehr]

Hey @jongio ,

I've now been deploying the same template 3 times and cannot reproduce the issue.

Instead of conflicts the only issue that I got was that the provided roleDefinitionId is in an incorrect format. RoleDefinitionIds are not just GUIDs but look like this:

roleDefinitionIdOrName: '/subscriptions/{subscriptionId}/providers/Microsoft.Authorization/roleDefinitions/14b46e9e-c2b7-41b4-b07b-48a6ebf60603'

or

roleDefinitionIdOrName: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '14b46e9e-c2b7-41b4-b07b-48a6ebf60603')

for short.

Another alternative is the reference:

resource roleDef 'Microsoft.Authorization/roleDefinitions@2022-04-01' existing = {
  name: '14b46e9e-c2b7-41b4-b07b-48a6ebf60603'
  scope: tenant()
}

(...)
   roleDefinitionIdOrName: roleDef.id

For reference: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles#contributor

The template variant of yours that I did use for testing, was this one

targetScope = 'subscription'

param location string = deployment().location
param keyVaultName string = 'alsehrkvtestvault'
param principalId string = '<myUserId>'

resource rg 'Microsoft.Resources/resourceGroups@2023-07-01' = {
  name: 'rg-alsehr'
  location: location
}

module keyVault 'br/public:avm-res-keyvault-vault:0.1.0' = {
  name: 'keyvault'
  scope: rg
  params: {
    name: keyVaultName
    location: location
    tags: {}
    roleAssignments: !empty(principalId) ? [
      {
        principalId: principalId
        principalType: 'User'
        roleDefinitionIdOrName: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '14b46e9e-c2b7-41b4-b07b-48a6ebf60603')
      }
    ] : []
  }
}

[jongio]

I switched to

        roleDefinitionIdOrName: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '4633458b-17de-408a-b874-0445c86b69e6')

And it works.

Maybe we need to more clear that this is "resource id" not the "Role id"

[AlexanderSehr]

I switched to

        roleDefinitionIdOrName: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '4633458b-17de-408a-b874-0445c86b69e6')

And it works.

Maybe we need to more clear that this is "resource id" not the "Role id"

I agree @jongio . Would could also add an example in the test files to make it a bit more obvious.

It is actually described in the parameter description for role assignments:

Optional. Array of role assignment objects that contain the 'roleDefinitionIdOrName' and 'principalId' to define RBAC role assignments on this resource. In the roleDefinitionIdOrName attribute, you can provide either the display name of the role definition, or its fully qualified ID in the following format: '/providers/Microsoft.Authorization/roleDefinitions/c2f4ef07-c644-48eb-af81-4b1b4947fb11'.

But I'll suggest to move that to the 'roleDefinitionIdOrName' parameter metadata.

@eriqua , any thoughts on the matter? I'd suggest we do both:

• Update the parameter descriptions (and in turn the AVM spec)
• Update the test cases of the existing modules to also have a role assignment using a role definition id

[jongio]

What about calling the param roleDefinitionResourceId, there would have been no confusion for me if resourceid was in the name

[AlexanderSehr]

What about calling the param roleDefinitionResourceId, there would have been no confusion for me if resourceid was in the name

Fundamentally I have no objection to that, but it should / would need to be updated in the Bicep & Terraform schema. I assume the reason it's not called a resource ID is that it's not a resource? Just a guess 😄

Let's see, @eriqua & @jtracey93 what's your take on this?

[AlexanderSehr]

We opted to not diviate from the specs for this (name-wise) - however, we did implement a PR that not only enables all schemas possible (name / guid / full ID ref), but also updated the tests accordingly to show the different options in #663 . This should resolve also the original issue. Hence I'll go ahead and close it. If there are any further questions, please let me know.