bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Change userAssignedIdentities from object to array

Open dozer75 opened this issue 1 year ago • 2 comments

Is your feature request related to a problem? Please describe.

I have an AKS cluster that I want to have a user assigned identity. This user assigned identity is created using a helper module for creating managed identities. The problem is, that assigning the output id from this module to the userAssignedIdentities in identity causes bicep to issue the BCP120 error since it is an object where the key is the identity id rather than an array of objects (where the id could be a property), so it is simply not possible to use the output from my managed identity module as the value during AKS creation.

Example:

managedidentity.bicep

@description('The name of the managed identity to create.')
param name string

@description('The location of the managed identity')
param location string

resource mi 'Microsoft.ManagedIdentity/userAssignedIdentities@2018-11-30' = {
  name: name
  location: location
}

// Some more code here that does role assignments and so on

output id string = mi.id

aks.bicep


var idName = 'id-aks-${environment}-${location}'

module aksidentity 'managedidentity.bicep' = {
  name: 'aksidentity'
  params: {
    name: idName
    location: location
  }
}


resource aks 'Microsoft.ContainerService/managedClusters@2022-07-02-preview' = {
  name: 'aks-${environment}-${location}'
  location: location
  sku: {
    name: 'Basic'
    tier: 'Free'
  }
  identity: {
    type: 'UserAssigned'
    userAssignedIdentities: {
      '${aksidentity.outputs.id}': {} // this gives me BCP120
    }
  }
  // more configuration here
 }

The line that has the comment // this gives me BCP120 gives this complete error:

This expression is being used in an assignment to the "identity" property of the "Microsoft.ContainerService/managedClusters" type, which requires a value that can be calculated at the start of the deployment. Properties of aksidentity which can be calculated at the start include "name". bicep(BCP120)

While I can see the reason for this to happen it is very unfortunate that it isn't possible. I have found a couple of workarounds, but I think those makes the code a bit clumsy to work with, so an alternate solution would be nice to have.

One workaround I have found (and uses) is simply to use '${resourceGroup().id}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/${idName}': {} and it will work but it doesn't look smooth.

Describe the solution you'd like

I would like the userAssignedIdentities to be like other properties that takes multiple values; an array of objects. This will make it possible to have dynamic values and in addition is a smoother way and more consisting way to do it compared to other properties that works with arrays/collections. It will also be more future proof if neccessary.

Example:

resource aks 'Microsoft.ContainerService/managedClusters@2022-07-02-preview' = {
  name: 'aks-${environment}-${location}'
  location: location
  sku: {
    name: 'Basic'
    tier: 'Free'
  }
  identity: {
    type: 'UserAssigned'
    userAssignedIdentities: [
      {
        id: '${aksidentity.outputs.id}'
        properties: {} // if any and needed
      }
    ]
  }
  // more configuration here
 }

I can see that this may break with the ARM template way since that uses the same way as bicep today, but it should either be fixed in the ARM as well or mapped from array -> object in the bicep to arm transformation somehow.

dozer75 avatar Sep 26 '22 18:09 dozer75

Your best option here would be to declare the managed identity resource directly inside your aks.bicep template instead of using a module. By doing that the value can be calculated at the start of the deployment and your deployment will succeed. You will also end up with less code since Microsoft.ManagedIdentity/userAssignedIdentities only requires two properties.

var idName = 'id-aks-${environment}-${location}'

resource aksidentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2018-11-30' = {
  name: idName
  location: location
}

resource aks 'Microsoft.ContainerService/managedClusters@2022-07-02-preview' = {
  name: 'aks-${environment}-${location}'
  location: location
  sku: {
    name: 'Basic'
    tier: 'Free'
  }
  identity: {
    type: 'UserAssigned'
    userAssignedIdentities: {
      '${aksidentity.id}': {} // this gives me BCP120
    }
  }
  // more configuration here
 }

StefanIvemo avatar Sep 27 '22 19:09 StefanIvemo

@StefanIvemo Yes, I know that's possible, but that's not the point here and not desired way to do it. This was a simple example how to reproduce it, my real Bicep structure is way more complex than this and having the external module to create managed identities aids me in other circumstances creating managed identities in a generic way all over my Bicep IaC structure.

dozer75 avatar Sep 29 '22 15:09 dozer75

In my deployment, I have a generic module for creating my identities but I stick to a rule of deploying only one resource and its children per module. As such, I'd recommend that in whatever you're referencing the AKS module from, deploy your UAI there and pass its name as a parameter into the AKS module, then do a resource existing to reference in the identity field.

WhitWaldo avatar Oct 04 '22 08:10 WhitWaldo

I once raised the question about why identities was an object and not an array at one point and it was dismissed as being by design since an array can't be easily updated via PATCH as a dictionary can. While I can't find the thread or the link I was provided, you can see on this blog entry (towards the top) that it once was an array, but was changed for that reason.

WhitWaldo avatar Oct 04 '22 08:10 WhitWaldo

@WhitWaldo While I do agree in your method in the first post, I'm still a bit curios why that changed and on this place only...

You could say the same argument as they had on identity on almost everything that array of objects are used for today in ARM/Bicep (may it be identities, various network configurations like e.g. subnet in virtual networks, valid IP address exceptions in network configuration on almost all servies that has this in Azure and more), so I really struggle with a (in my opinion) short-thinked solution like this since it doesn't align with how it works everywhere else.

I can to some extent see the point having it as a dictionary when it comes to PATCH related operations, but then it should be possible to have the key dynamic as well... If that worked it would be a nice feature almost everywhere we're working with arrays of objects today.

dozer75 avatar Oct 05 '22 13:10 dozer75

I don't think anyone believes that the virtual network approach as a whole is desirable given its general pain in pushing updates at all, but it's also generally accepted that's a historical limitation since it's such a relatively older API.

At least in my experience, a vast number of the modern ARM configurations do prefer maps over arrays, but it's more "understandable" since they expect arrays of whole objects instead of only settings the keys. I agree - that part does feel weird about Identity, but again, it makes sense why they do that. I'd rather have the opportunity for an update that retains unchanged properties than have to go the Vnet route and constantly re-specify everything for minor changes.

WhitWaldo avatar Oct 06 '22 07:10 WhitWaldo

~~The original change was reverted with the 0.11 release, so this should no longer be occurring.~~

Commented on the wrong issue. Time to go to bed...

anthony-c-martin avatar Oct 07 '22 02:10 anthony-c-martin

Here's a potential workaround that's generic; meaning that it allows for zero, one, or many user assigned identities and/or the assignment of the system-assigned identity. The sort is somewhat arbitrary and is there to allow one to consistently treat the first entry in the list as the primary identity, in the few situations where such behavior is desired. Adjust as you see fit.

param identity object = {}

var userAssignedIdentities = sort(map(range(0, length(union({ userAssignedIdentities: []}, identity).userAssignedIdentities)), i => {
  id: resourceId(union({
    subscriptionId: subscription().subscriptionId
  }, identity.userAssignedIdentities[i]).subscriptionId, union({
    resourceGroupName: resourceGroup().name
  }, identity.userAssignedIdentities[i]).resourceGroupName, 'Microsoft.ManagedIdentity/userAssignedIdentities', identity.userAssignedIdentities[i].name)
  index: i
}), (x, y) => (x.index < y.index))

resource symbolicName '...' = {
  identity: (empty(identity) ? null : {
    type: union({ type: (empty(userAssignedIdentities) ? 'SystemAssigned' : 'UserAssigned') }, identity).type
    userAssignedIdentities: (empty(userAssignedIdentities) ? null : reduce(userAssignedIdentities, {}, (x, y) => union(x, { '${y.id}': {} })))
  })
}

Example that assigns a single user-assigned identity:

{
    userAssignedIdentities: [
        {
            name: 'SomeManagedIdentity'
            resourceGroupName: 'SomeResourceGroup' // optional, defaults to current scope
            subscriptionId: 'SomeScriptionId' // optional, defaults to current scope
        }
    ]
}

Kittoes0124 avatar Feb 03 '23 21:02 Kittoes0124