Nested loops

[afscrome]

Bicep version
0.3.255 (VS Code Extension)

Describe the bug

In the following example I'm trying to assign a number of RBAC roles within a keyvault. I have a set of roles, and for each of these roles, a set of principals to add to that role. So I want to loop over the roles, and then the principals within those roles to do a role assignment. I've tried the below, but I can't get the loop syntax correct for such a nested loop.

Whilst the Loops spec does say that nested loops are supported, none of the examples cover multiple nesting at the same level, so I'm not entirely sure if I've got the syntax wrong, of if this form of nested loops is not supported.

To Reproduce

param keyvaultReaders array
param keyvaultSecretsUsers array

var roleAssignments = [
  {
    roleId: '21090545-7ca7-4776-b22c-e363652d74d2'
    principalIds: keyvaultReaders
  }
  {
    roleId: '4633458b-17de-408a-b874-0445c86b69e6'
    principalIds: keyvaultSecretsUsers
  }
]

resource rbacRoleAssignments 'Microsoft.Authorization/roleAssignments@2020-04-01-preview' = [for role in roleAssignments
  for principalId in role.principalIds: {
    name: guid(role.roleId, principalId, resourceGroup().id)
    properties: {
      principalId: principalId
      roleDefinitionId: resourceId('Microsoft.Authorization/roleDefinitions', role.roleId)
    }
    scope: keyVault
}]

Additional context

As a work around, I have been able to use a combination of the recent loop variable support and array concatenation to flat the 2 level object down to a single level, and then loop over that. It works, but it is a bit messier than using nested loops.

param keyvaultReaders array
param keyvaultSecretsUsers array

var keyvaultReaderRoleAssignments = [for principalId in keyvaultReaders: {
  roleId: '21090545-7ca7-4776-b22c-e363652d74d2'
  principalId: principalId
}]
var keyvaultSecretsUsersRoleAssignments = [for principalId in keyvaultSecretsUsers: {
  roleId: '4633458b-17de-408a-b874-0445c86b69e6'
  principalId: principalId
}]

var allRoleAssignments = concat(keyvaultReaderRoleAssignments, keyvaultSecretsUsersRoleAssignments)

resource rbacRoleAssignments 'Microsoft.Authorization/roleAssignments@2020-04-01-preview' = [for assignment in allRoleAssignments: {
  name: guid(assignment.roleId, assignment.principalId, resourceGroup().id)
  properties: {
    principalId: assignment.principalId
    roleDefinitionId: resourceId('Microsoft.Authorization/roleDefinitions', assignment.roleId)
  }
  scope: keyVault
}]

[alex-frankel]

The issue is that the roleAssignment resource type can only create one role assignment at a time. It doesn't know how to work with a set of principal IDs, it can only work with one in each instance.

I kind of like how you solved it tbh. The other way to do it is to create a module that when given a set of N principal IDs and one role Id, it can create N role assignments. Then call the module with a loop like this:

param keyvaultReaders array
param keyvaultSecretsUsers array

var roleAssignments = [
  {
    roleId: '21090545-7ca7-4776-b22c-e363652d74d2'
    principalIds: keyvaultReaders
  }
  {
    roleId: '4633458b-17de-408a-b874-0445c86b69e6'
    principalIds: keyvaultSecretsUsers
  }
]

module roleAssignmentCreator 'roleAssignmentCreator.bicep' = [item in roleAssignments: {
  name: 'roleassignmentcreator'
  params: {
    roleId: item.roleId
    principalIds: item.principalIds
  }
}

[afscrome]

Thanks @alex-frankel . Whilst my work around isn't terrible, I still prefer the original as you can add a new role in just one place and it works. With my work around you have to create a new variable, then remember to add that to the concat, as well as opening the possibility for hard to spot bugs to creep in if the intermediate variable name and the variable it's looping over don't match up. If I could get my initial aproach to work, then I'd just add a new element to the roleAssignments array, and you're off.

I get that the role assignment resource has to work with a single principal, role at a time, hence the attempt to use nested loops. Based on the documentation saying that nested loops are supported I was expecting to get something like the following to work, but it seems like there are some caveats to nested loops.

foreach(role in roleAssignments) {
    foreach(principal in role.Principals)
    {
        resource roleAssignment(roleId: role.roleId, principalId: principal)
    }
}

[alex-frankel]

Gotcha - yes you are right that this is not possible today. Out of curiosity, is the module-based solution a less error prone way of handling in than flattening the list yourself?

[AlexanderSehr]

@alex-frankel I ran some tests with a nested module approach myself that worked like a charm:

// input in parameter.json
{
  "roleAssignments": {
      "value": [
          {
              "roleDefinitionIdOrName": "4633458b-17de-408a-b874-0445c86b69e6",
              "principalIds": [
                  "999999-17de-408a-b874-0445c86b69e6"
              ]
          }
      ]
  }
}

// deploy.bicep
// ========
module name_location_Storage_Rbac './nested_rbac.bicep' = [for (roleassignment, index) in roleAssignments: {
  name: '${uniqueString(deployment().name, location)}-Storage-Rbac-${index}'
  // scope: storageAccount // module scopes are not yet supported besides subscription & up
  params: {
    roleAssignment: roleassignment
    builtInRoleNames: builtInRoleNames
    storageAccountName: storageAccountName
  }
  dependsOn: [
    storageAccount
  ]
}]

// nested_rbac.bicep
// ========
param roleAssignment object
param builtInRoleNames object
param storageAccountName string

resource nested_rbac 'Microsoft.Storage/storageAccounts/providers/roleAssignments@2020-04-01-preview' = [for principalId in roleAssignment.principalIds: {
  name: '${storageAccountName}/Microsoft.Authorization/${guid(storageAccountName, principalId, roleAssignment.roleDefinitionIdOrName)}'
  properties: {
    roleDefinitionId: (contains(builtInRoleNames, roleAssignment.roleDefinitionIdOrName) ? builtInRoleNames[roleAssignment.roleDefinitionIdOrName] : roleAssignment.roleDefinitionIdOrName)
    principalId: principalId
  }
  dependsOn: []
}]

I'd just wish the nested file would stop complaining about the resource name as I don't leverage the non-supported 'scope' parameter ;)

[afscrome]

The module could work - it solves the error prone part, but brings it's own complexity as it now spreads the work out across two files rather than one, so it's a case of pick your poison.

Right now I'm only setting rbac roles on one resource so the extra file from the seperate module doesn't feel like a great trade off. But I will eventually be adding rbac to other resources which may flip the equation on it's head.

[afscrome]

@alex-frankel I've tried the module approach and it works if I want to assign roles at the resource group level, but I'm struggling to get it to work against a specific resource rather than the whole resource group - I can't work out how to pass a resource in as a param to the module in a way that the roleAssignment's scope will accept it the scope into the template (See one such failed attempt at #2281).

Following @MrMCake's example I can get it to work if I tightly couple the module to a specific resource type, but it feels like it should be possible to make it work generically across all resource types

@MrMCake I was able to get rid of the warning up by passing in the resource name, using an existing resource reference then using that as the roleAssignment's scope

param roleNameOrId string
param principalIds array
param keyVaultName string

// https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
var wellKnownRoles = {
  'Key Vault Reader': '21090545-7ca7-4776-b22c-e363652d74d2'
  'Key Vault Secrets User': '4633458b-17de-408a-b874-0445c86b69e6'
}

var roleId = contains(wellKnownRoles, roleNameOrId) ? wellKnownRoles[roleNameOrId] : roleNameOrId

resource keyVault 'Microsoft.KeyVault/vaults@2021-04-01-preview' existing = {
  name: keyVaultName
}

resource rbacRoleAssignments 'Microsoft.Authorization/roleAssignments@2020-04-01-preview' = [for principal in principalIds: {
  name: guid(keyVault.id, principal, roleId)
  properties: {
    principalId: principal
    roleDefinitionId: resourceId('Microsoft.Authorization/roleDefinitions', roleId)
  }
  scope: keyVault
}]

[alex-frankel]

Following @MrMCake's example I can get it to work if I tightly couple the module to a specific resource type, but it feels like it should be possible to make it work generically across all resource types

This is not possible today because in order to get a resource reference you need to provide a specific type (either creating the resource or establishing a reference with existing as you've done). We are working on a proposal to allow a more direct way of passing resource references between modules that will solve this. You'll be able to have a param of type resource like so:

param genericResource resource // pass a resource of any type as a parameter

...

resource rbacRoleAssignments 'Microsoft.Authorization/roleAssignments@2020-04-01-preview' = [for principal in principalIds: {
  name: guid(genericResource.id, principal, roleId)
  properties: {
    principalId: principal
    roleDefinitionId: resourceId('Microsoft.Authorization/roleDefinitions', roleId)
  }
  scope: genericResource
}]

[afscrome]

Cheers @alex-frankel. I'll follow that proposal with interest.

Back to the original nested loop question. Is this a case of this not being implemented yet, or no plans to do so? If the latter, I think the Loops spec / documentation should be updated to clarify exactly what types of nested loops are supported (and potentially highlight the nested module approach as a work around to achieve similar results).

[alex-frankel]

There's no concept today of authoring a loop outside of a value of a resource, module or property, so we support nested loops in the sense that you can loop over a resource, then nest another loop inside as the value of a property. There are no plans to support a generic loop in the way you are looking for.

I will create an issue to clarify this in docs a bit more. Good if I close this one in the meantime?

[afscrome]

Thanks @alex-frankel