Group members are append-only and existing members can't be read

[mattias-fjellstrom]

Bicep version Bicep CLI version 0.27.1 (4b41cb6d4b)

Resource and API version Microsoft.Graph/groups@v1.0 and Microsoft.Graph/groups@beta

Auth flow Interactive

Describe the bug

• Creating a group with members only allow adding new members. Removing a member from the list of members and re-deploying does not remove the member from the actual group.
• It is not possible to read group members from a group resource

To Reproduce Create a resource

resource group 'Microsoft.Graph/groups@v1.0' = {
  displayName: 'Team Fabulous Unicorns'
  mailEnabled: false
  mailNickname: 'teamFabulousUnicorns'
  securityEnabled: true
  uniqueName: 'team-fabulous-unicorns'
  members: [
    'member-guid-1' // replace with actual user guid
    'member-guid-2' // replace with actual user guid
  ]
}

Deploy the resource. Update to the following:

resource group 'Microsoft.Graph/groups@v1.0' = {
  displayName: 'Team Fabulous Unicorns'
  mailEnabled: false
  mailNickname: 'teamFabulousUnicorns'
  securityEnabled: true
  uniqueName: 'team-fabulous-unicorns'
  members: [
    'member-guid-1' // replace with actual user guid
  ]
}

Re-deploy. Look at the group in the Azure portal, it still contains both users.

Also, the following will not work:

resource group 'Microsoft.Graph/groups@v1.0' = {
  displayName: 'Team Fabulous Unicorns'
  mailEnabled: false
  mailNickname: 'teamFabulousUnicorns'
  securityEnabled: true
  uniqueName: 'team-fabulous-unicorns'
  members: [
    'member-guid-1' // replace with actual user guid
    'member-guid-2' // replace with actual user guid
  ]
}

output members string[] = group.members // allowed in editor, but not at deploy time

[slavizh]

If I am not mistaken this is a know issue that I think hasn't be documented. Also if I remember correctly there is limit of how many members can be added at single deployment. Most likely this is the same behavior for owners as well?

[dkershaw10]

@mattias-fjellstrom and @slavizh - yes this is a known issue. I will update the known issues topic with this information. Updating the reference docs with this info is tricky, because the reference is auto-generated based on some cleaned up schema that sources the property descriptions from existing REST API documentation. Will talk to engineering about possible workarounds there.

Just to summarize:

1. Group membership/ownership is additive only (i.e. non-destructive).
2. We have discussed adding a facility/option for making groups membership/ownership updates with "replace" nature.
3. Groups create/update of membership+ownership is limited to 20 total (i.e. 16 members and 4 owners) in a single "request" (single Bicep resource declaration). Any "true" declarative IaC for group member/owner-ships for > 20 "relationships" would be tricky (with current API implementation).

One thing definitely worth investigating is the output/reference of group members.

[dkershaw10]

Known issues will be updated soon with this info. Need to investigate output of group members.

[petersgiles]

Just thoughts

Can we have a construct that says something like this for now

replaceMembers: bool // default false, true = the group will only have whats in the members list
 members: [
    'string'
  ]

In terms of Configuration as Code philosophy I don't see how it is useful to have add and remove constructs in the long term as, to my mind, the whole point of bicep is to document the intended state of the infrastructure as living code. If you want something else you may as well just do clickOps or adhoc powershell/az calls.

I know the graph API is a bit tricky when it comes to volume requests so perhaps introducing

resource groupmembers 'Microsoft.Graph/groupmember@v1.0' = [for member in memberlist {
 group: mygroup
 member: member
}]

this would compare against the current state and add and remove as required.

[alex-frankel]

+1 - we should avoid having to declare items to be explicitly removed or anything of that sort. Having a property to switch between additive (PUT-AS-PATCH in ARM parlance) vs replace (strict PUT) semantics is a reasonable way to do it if we need to allow users to make this choice.

[slavizh]

switch is not needed if it is fixed. There shouldn't be two modes one if which is not idempotent. Bicep should be idempotent so the RPs. As this is known issue it is better to just be fixed and do not allow for the current behavior.

[dkershaw10]

Known issues has been updated

[dkershaw10]

@slavizh @petersgiles I like the idea of fully adopting IaC/config-as-code philosophy (and going with your second option Pete, to compare against the current state and add and remove as required). However, the one thing that gives me pause here, is that I could reference a pre-existing group that has 10s of thousands of members. And through the template, if we add idempotent group membership capability without any guard-rails, I could accidentally remove all existing members. That seems very scary to me - especially if this group was being used to provide access to a high-value application.

[slavizh]

@dkershaw10 true but that could be said for any other critical property. People should educate themselves in advance before using it. One thing that could help is What-If functionality so you can see the results before applying them. In Azure what you usually have is things like two APIs. For example let's take Virtual networks. You can define the subnets as part of the virtual networks resource or define subnets as its own resource. In the first case any subnet that is no longer in the configuration will be removed. In the second case you just add additional subnets to those that are part of the virtual network already. I am not sure if you can do the same thing for graph but it is just in the nature of IaC to have proper idempotency.

[dkershaw10]

Thanks. Yes - we plan to add what-if functionality once extensibility is updated to support it.

Maybe I just struggle to understand why group memberships would be managed through Bicep, or IaC in general. I get managing groups and maybe even group owners. But members is such a dynamic element, where memberships change often, and updates to memberships are expected to happen quickly to give or remove access to resources/apps. Deploying Bicep files that may "redeploy" a lot of infrastructure, could take hours, just to add a new member... I actually have a similar question around Privileged Identity Management for Groups - in whether that is a scenario that makes sense for IaC.

[petersgiles]

@dkershaw10 I understand your hesitation and to that, I would say bicep/terraform isn't for your situation.

Strict Config as Code is a journey that is an attempt to stop configuration drift and in some cases, it is an impossible ideal. Everyone needs to be on board with it and embrace what it means. it doesn't help that the tooling we are offered isn't complete and perhaps the documentation should reflect some of the ideology around why you would even bother with it.

So in circumstances where I want to be strict and make all group membership through bicep and git pipelines, I need this to be the authoritative statement of what should be in my environment. Any changes not managed by my DevOps process should be overwritten the next time my pipeline runs otherwise I can't trust that my environment is as I intended.

In my view without being able to explicitly describe what I want, there's very little point in embarking on the exercise.

[slavizh]

@dkershaw10 The way we structure our Bicep templates is like micro-services. For example we will have a template just for deploying entra groups. That template can deploy groups with different configurations based on different Bicep parameters file. That allows doing separate deployments. These separate deployments could be due to environment - prod, dev, etc.; application, etc. This I think answers you questions of deployment taking hours. If we have groups for example where there is delegation to the owners to manage the members we either will not managed those groups in IaC or not manage the members of those groups, only the owners, Not emitting members property is also possible via Bicep with spread operator.

[dkershaw10]

@mattias-fjellstrom @petersgiles @slavizh We wanted to share a speclet on how we hope to address this issue. Please take a look and let us know what you think.

Modelling relationships in Graph Bicep

Examples of "relationships" in Microsoft Graph are the group members and owners properties.

Current challenges

1. Relationships are additive only. Removing relationship items from the resource definition does not delete those relationship items in the service. Many/most customers would expect the deployment operation to use replace semantics.
2. A relationship definition in a single Bicep file may have an unlimited number of items in the array. However, declaring more than 20 items in a Bicep file results in a 400 error during deployment.

Proposal

We propose to introduce:

1. a deployment level setting to choose replace semantics for relationship properties
2. a new type that models relationships, so that these can be differentiated from regular string array properties
3. changes to the extension, so that deployments handle:
   • an unlimited number of items in a relationship, and
   • replace semantics, by first reading and then based on the diff, create/update or delete resources.

Deployment level setting

provider microsoftGraph with {
   // By default, the "safe" additive semantics is used (i.e. false) 
   useReplaceSemanticsForRelationships: bool
}

If set to true, all relationships defined in the Bicep file will be deployed using replacement semantics.

New relationships type

We'll introduce a new type that will indicate that the property is a relationship. This will directly map to Microsoft Graph's relationship properties (modelled as OData non-contained navigation properties). This concept is not generally present in ARM APIs. This example shows usage of the new type (as opposed to the current string array).

resource group 'Microsoft.Graph/groups@v1.0' = {
  displayName: groupName
  mailEnabled: false
  mailNickname: 'myGroup-2024-06-11'
  securityEnabled: true
  uniqueName: groupName
  owners: {
    relationships: [
        'ae1dbad7-a45f-4b68-b387-d005c458e33d'
        'ae1dbad7-a45f-4b68-b387-d005c458e331'
    ]
  }
  members: {
    relationships: [
        'ae1dbad7-a45f-4b68-b387-d005c458e33d'
        'f0891b1b-cefe-4f15-a1f1-6ca6cc71d23a'
        'b7436f41-4977-4c76-b737-0a551e95a562'
    ]
  }
}

Other options considered

Relationships as resources

If we modelled a single relationship as an individual resource, it provides some advantages. If you're using regular deployments, then you'll get update semantics; if you're using Deployment Stacks (once supported for extensible resources) then you'll get replace semantics.

However, we decided against this option (in favor of putting relationship replace semantics in the extension):

1. We won't be able to provide a human-readable client provided name for any new relationship resource type.
2. We will not be able to batch requests to Microsoft Graph, which would impact performance.
3. Using relationship resource types will lead to a more verbose Bicep file, although using a for-loop would remove much of this verbosity.

[petersgiles]

@dkershaw10 Thanks for this, it certainly is heading in a good direction.

My thoughts

Proposal:

• all good, addresses the concerns we are having

Deployment level setting:

• seems reasonable, setting replace semantics at the top level is a good solution, one in all in, so to speak.
• as mentioned before some doco on the philosophy of Config as Code, why replace semantics are critical and ultimately why this setting should default to true in the future

New relationships type and the other option:

• your proposal definitely better than what is currently there so on a pragmatic level I'll like to see this sooner than later However
• I don't really understand your hesitation with relationship as resources. IMHO it would be way more flexible. especially as group membership will change more often than the existence of a given group. I'd prefer to manage a separate bicep file just for these relationships with its own pipeline. One thing I do when managing change is to establish my repos of config to match the cadence of environmental change or in my team's parlance according to their perceived volatility. This also addresses your concern about performance because I can run the more volatile things at an appropriate cadence without having to rerun things I know don't change often.

My two cents, I'd prefer something like this.

resource group 'Microsoft.Graph/groups@v1.0' existing = { ... }

// excuse syntax taking an educated guess here 😄 
// mymemberlist is human readable
resource members 'Microsoft.Graph/members@v1.0' existing = [for m in mymemberlist: {
  name: m.name
  ...
}]
var memberIds array = [for i in range(0, length(members)): {
  id: members [i].id
}]

resource ownerelationships 'Microsoft.Graph/groupmembers@v1.0' = { // batch-able
 type : <'owner' | 'member' >
 group: group
 members: memberIds 
}

[slavizh]

My take on this: Having a setting that is in the provider definition seems too static to me and not flexible at all. Basically removes any scenarios where you want to deploy multiple groups with same template and have different behavior based on a choice from end user. The way we develop templates is making them general and than distribute to different end users for usage. Based on their different needs they can provide different parameters and deploy different configurations.

I liked the idea of deployment stacks and regular deployments. If deployment stacks support is added sooner than later may be it is better choice to make this happen without requiring any changes. Meaning the syntax stays the same and if you want to add members without removing existing deployments - use regular deployments. If you want to have full idempotency and only have the members that are in the configuration and remove any that does not exists - use deployment stacks.

Not being able to use existing syntax puts are two steps behind. The existing syntax is one of the main features for MSGraph bicep. When you apply configuration with members you want to know which these members are. Using uniqueName can tell you that in human readable way. GUIDS cannot do that.

Unfortunately, to me the current proposal does makes things worse with this different syntax that needs to be set on the template and the different syntax that needs to be used for existing resources. I can propose of having child members resource but I am not so familiar in depth of Graph to know it is suitable.

[dkershaw10]

Thanks @petersgiles and @slavizh for taking the time to provide your feedback, which is super valuable. We've been discussing your feedback and thinking about some other options. We'll be working on a couple of proposals and a series of questions (in the form of a poll), which we'll present at the upcoming monthly Bicep Community call. Hopefully this will give more folks an opportunity to chime in with their feedback and help guide us to a good solution here.

[shenglol]

@dkershaw10 I mentioned the proposal to @majastrz and he suggested that we change useReplaceSemanticsForRelationships to an enum to make it more extensible, e.g.:

provider microsoftGraph with {
   relationshipSemantics: 'append' | 'replace' // Default is 'append'
}