microsoftgraph / msgraph-bicep-types

Repo contains Microsoft Graph resource types to integrate with bicep templates.
MIT License
38 stars 6 forks source link

Microsoft.Graph/groups@v1.0 fails to PATCH a group #171

Open jannef opened 1 week ago

jannef commented 1 week ago

Bicep version

az bicep version
Bicep CLI version 0.29.47 (132ade51bc)

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

Auth flow Interactive, az cli credentials.

Deployment details Start time: 9/3/2024, 2:49:11 PM Correlation ID: f07c3c47-2b07-43d0-8f1c-cee3379419a9 Graph client request id: 79e9a036-7856-485a-be8e-c0c5eb19f282 Graph request timestamp: 2024-09-03T11:49:11Z

Describe the bug Graph api call the resource provider in question fails with status code 400. Graph documentation states that this means the generated request is malformed. The same request used to work 8/30/2024, 12:57:43.709 PM. See attached picture. The bicep code responsible for generating the request is attached below.

"details":[
   {
      "code":"",
      "message":"{\"error\":{\"code\":\"BadRequest\",\"target\":\"/resources/adminGroup\",\"message\":\"Unexpected segment DynamicPathSegment. Expected property/$value. Graph client request id: 79e9a036-7856-485a-be8e-c0c5eb19f282. Graph request timestamp: 2024-09-03T11:49:11Z.\"}}"
   }
]
kuva

Failing bicep resource definition:

var adminUniqueName = 'my-fake-group-name'
resource adminGroup 'Microsoft.Graph/groups@v1.0' = {
  description: 'Administrators of the ${organisation} ${environment} database'
  displayName: '${organisation} ${environment} Database Admins'
  isAssignableToRole: false
  securityEnabled: true
  uniqueName: adminUniqueName
  mailEnabled: false
  mailNickname: adminUniqueName
  members: [
    applicationIdentityId
    '93a2f6e5-3f48-476d-a685-e9136a5e6cd2'
  ]
}

Generated ARM:

"adminGroup": {
    "import": "microsoftGraph",
    "type": "Microsoft.Graph/groups@v1.0",
    "properties": {
        "description": "[format('Administrators of the {0} {1} database', parameters('organisation'), parameters('environment'))]",
        "displayName": "[format('{0} {1} Database Admins', parameters('organisation'), parameters('environment'))]",
        "isAssignableToRole": false,
        "securityEnabled": true,
        "uniqueName": "[variables('adminUniqueName')]",
        "mailEnabled": false,
        "mailNickname": "[variables('adminUniqueName')]",
        "members": [
            "[parameters('applicationIdentityId')]",
            "93a2f6e5-3f48-476d-a685-e9136a5e6cd2"
        ]
    }
}

To Reproduce Deploy a group several times several times (might work on first deploy, it's a graph PATCH that fails.

Additional context Azure Portal deployments UI cannot handle this situation. It fails to load any child resources for the failed deployment. Crash happens in react, there is no failed network calls.

kuva
dkershaw10 commented 1 week ago

@jannef - thanks for reporting this. We'll investigate and come back to you. FYI: All requests to create/update group resources use the PATCH (upsert) mechanism.

For your additional context item, this is a known issue that the deployments UI does not support Bicep extensibility (and extensible resources like Graph).

jannef commented 1 week ago

@dkershaw10 This is an user error, i've just discovered we are now passing in resourceId instead of principalId to our module. It might still be worth it to improve validation, instead of crashing like it currently is.

dkershaw10 commented 1 week ago

@jannef can you be a little more specific please? Or can you confirm that you are passing in an invalid value to the members property?

jannef commented 1 week ago

Essentially, a resource id of a managed identity like '/subscriptions/365d1707-aaaa-aaaa-aaaa-919e5240ef03/resourcegroups/rg-my-fake-we-001/providers/microsoft.managedidentity/userassignedidentities/mi-my-fake-we-001' was passed in to parameter applicationIdentityId. This seems to bypass validation and get posted to Graph.

param applicationIdentityId string

var adminUniqueName = 'my-fake-group-name'
resource adminGroup 'Microsoft.Graph/groups@v1.0' = {
  description: 'Administrators of the ${organisation} ${environment} database'
  displayName: '${organisation} ${environment} Database Admins'
  isAssignableToRole: false
  securityEnabled: true
  uniqueName: adminUniqueName
  mailEnabled: false
  mailNickname: adminUniqueName
  members: [
    applicationIdentityId
    '93a2f6e5-3f48-476d-a685-e9136a5e6cd2'
  ]
}
dkershaw10 commented 4 days ago

@jannef Sorry for the late response. Not sure if there's a way to do any validation here through linting rules to ensure that only valid referenced properties can be added to members/owners, if that's what you are asking for.

@jason-dou @eketo-msft is there a way to check via linting, that the following are valid references for the members/owners lists:

This may be an interesting validation use case for preview/whatif, when we support it, where we would read all member/owner references to make sure they are valid/exist.