search

microsoftgraph / msgraph-bicep-types

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

The template reference '...' is not valid: could not find template resource or resource copy with this name. #126

Closed slavizh closed 2 months ago

slavizh commented 6 months ago

Bicep version Bicep CLI version 0.27.1 (4b41cb6d4b)

Resource and API version in the reproduction below

Auth flow automated

Deployment details No correlation ID:

Error: Code=InvalidTemplate; Message=Deployment template
     | validation failed: 'The template reference 'memberApplications' is not
     | valid: could not find template resource or resource copy with this name.
     | Please see https://aka.ms/arm-function-reference for usage details.'. 
     | The deployment validation failed

Describe the bug It should not act this way. I think this is caused by Bicep issue due to how existing resources are referenced in language version 2 which is the one that is compiled if you have Graph resource provider. For Azure resources we can workaround this by using resourceId() function but with graph we can only use existing syntax. I think it might be related to these two issues: https://github.com/Azure/bicep/issues/13937 https://github.com/Azure/bicep/issues/12204 https://github.com/Azure/bicep/issues/13555

To Reproduce Steps to reproduce the behavior:

provider microsoftGraph

param entraGroup object = {
  name: 'ExampleGroup2'
  type: 'Security'
  displayName: 'Example Group 2'
  mailNickname: 'exampleGroup2'
  members: [
    {
      name: 'ExampleGroup'
      type: 'Group'
    }
    {
      name: '<some name>'
      resourceGroup: '<some name>'
      type: 'ManagedIdentity'
    }
  ]
}

var defaultMember = {
  subscriptionId: subscription().subscriptionId
  resourceGroup: ''
}

resource memberManagedIdentities 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-07-31-preview' existing = [for (member, i) in entraGroup.members: if (member.type =~ 'ManagedIdentity') {
  name: member.name
  scope: resourceGroup(union(defaultMember, member).subscriptionId, union(defaultMember, member).resourceGroup)
}]

resource memberApplications 'Microsoft.Graph/applications@v1.0' existing = [for (member, i) in entraGroup.members: if (member.type =~ 'Application') {
  uniqueName: member.name
}]

resource memberServicePrincipals 'Microsoft.Graph/servicePrincipals@v1.0' existing = [for (member, i) in entraGroup.members: if (member.type =~ 'Application') {
  appId: memberApplications[i].appId
}]

resource memberGroups 'Microsoft.Graph/groups@v1.0' existing = [for (member, i) in entraGroup.members: if (member.type =~ 'Group') {
  uniqueName: member.name
}]

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname:  entraGroup.mailNickname
  securityEnabled: entraGroup.securityEnabled
  members: [for (member, i) in entraGroup.members: member.type =~ 'ManagedIdentity'
    ? memberManagedIdentities[i].properties.principalId
    : member.type =~ 'Application'
    ? memberServicePrincipals[i].id
    : member.type =~ 'Group'
    ? memberGroups[i].id
    : ''
  ]
}

Strangely even this does not work so it might not be just Bicep issue:

provider microsoftGraph

param entraGroup object = {
  name: 'ExampleGroup2'
  type: 'Security'
  displayName: 'Example Group 2'
  mailNickname: 'exampleGroup2'
  members: [
    {
      name: 'ExampleGroup'
      type: 'Group'
    }
  ]
}

resource memberGroups 'Microsoft.Graph/groups@v1.0' existing = [for (member, i) in entraGroup.members: {
  uniqueName: member.name
}]

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname:  entraGroup.mailNickname
  securityEnabled: entraGroup.securityEnabled
  members: [for (member, i) in entraGroup.members: memberGroups[i].id]
}

The only thing that works if you define a single existing group and add it as member. Basically removing being able to add multiple members with for loops.

Additional context N/A

dkershaw10 commented 6 months ago

@jason-dou please investigate if this is a problem in the members/owners implementation, or if it's caused by something in the Bicep implementation.

shenglol commented 6 months ago

This seems to be related to https://github.com/Azure/bicep/issues/12906.

shenglol commented 6 months ago

@slavizh One workaround might be to put the existing resource declaration in a nested module, output the id array, and reference them in the parent template.

slavizh commented 6 months ago

@shenglol can I just say no to that? That is not good workaround. As this is preview it should just be fixed. I have encountered too many issues currently to put it in production so I would rather wait to be fixed. Creating additional deployments increases deployment time and makes the code harder to maintain. Also reduces scale as you cannot go beyond certain number of deployments.

dkershaw10 commented 6 months ago

@slavizh does your second example fail with a different error, or with the same "Template reference is not valid"? Will try to repo myself, later today.

slavizh commented 6 months ago

@dkershaw10 same just the referenced name is different. As soon as you add loop you get the error. I do not get error if members is empty array but of course than the code is not executed at all.

shenglol commented 6 months ago

@slavizh does your second example fail with a different error, or with the same "Template reference is not valid"? Will try to repo myself, later today.

@dkershaw10 Just want to let you know that this highly likely a general ARM deployment engine issue (the issue I linked above), not something specific to the Graph extension. I suggested @jason-dou to transfer the issue to the Bicep repo but GitHub does not support cross-org transfer.

@shenglol can I just say no to that? That is not good workaround. As this is preview it should just be fixed. I have encountered too many issues currently to put it in production so I would rather wait to be fixed. Creating additional deployments increases deployment time and makes the code harder to maintain. Also reduces scale as you cannot go beyond certain number of deployments.

@slavizh I understand your concerns. We will make sure this issue gets addressed.

slavizh commented 6 months ago

@shenglol there is no workaround to this. I have tried this: https://github.com/microsoftgraph/msgraph-bicep-types/issues/131 where I loop over deployment and basically have the same definitions for existing resource but the existing resources are single one not loops and I got the error in the issue.

I also have tried with separate template for each resource type:

module managedIdentityIds 'resource-id.bicep' = [for (member, i) in entraGroup.members: if (member.type =~ 'ManagedIdentity') {
  name: 'memberManagedIdentityId-${uniqueString(entraGroup.name)}-${i}'
  params: {
    member: member
  }
}]

module applicationIds 'resource-id.bicep' = [for (member, i) in entraGroup.members: if (member.type =~ 'Application') {
  name: 'memberApplicationId-${uniqueString(entraGroup.name)}-${i}'
  params: {
    member: member
  }
}]

module servicePrincipalIds 'resource-id.bicep' = [for (member, i) in entraGroup.members: if (member.type =~ 'ServicePrincipal') {
  name: 'memberServicePrincipalId-${uniqueString(entraGroup.name)}-${i}'
  params: {
    member: member
  }
}]

module groupIds 'resource-id.bicep' = [for (member, i) in entraGroup.members: if (member.type =~ 'Group') {
  name: 'memberGroupId-${uniqueString(entraGroup.name)}-${i}'
  params: {
    member: member
  }
}]

but still I get error:

{
  "code": "InvalidTemplateDeployment",
  "message": "The template deployment 'memberGroupId-6r7qavqqjkwe4-0' is not valid according to the validation procedure. The tracking id is 'f567f33e-384a-41f4-ac48-cfc48ee34418'. See inner errors for details.",
  "details": [
    {
      "code": "Forbidden",
      "target": "/resources/memberApplication",
      "message": "Insufficient privileges to complete the operation. Graph client request id: c3792df7-232d-432c-99c4-47e162ca423b. Graph request timestamp: Fri, 31 May 2024 11:59:12 GMT."
    },
    {
      "code": "Forbidden",
      "target": "/resources/memberGroup",
      "message": "Insufficient privileges to complete the operation. Graph client request id: 9c065d57-f4c1-493b-8890-3da0752df957. Graph request timestamp: Fri, 31 May 2024 11:59:13 GMT."
    }
  ]
}

Most likely this can only work if I am not putting all types into members but each into their own parameter which is nightmare experience for those who will use the template and nightmare experience for those who will maintain it. It is like if conditions are not taken into account even with deployments.

slavizh commented 6 months ago

@shenglol @dkershaw10 I think I have nailed it down to conditions not working at all. If I not mistaken even in v2 they should work to some extent so may be the issue is a mix of v2 and graph.

provider microsoftGraph

param entraGroup object = {
        name: 'ExampleGroup'
        displayName: 'Example Group 1'
        mailNickname: 'exampleGroup'
      }

var group = {
  name: 'dummy'
  type: ''
}
resource oneMemberGroup 'Microsoft.Graph/groups@v1.0' existing = if (!empty(group.type)) {
  uniqueName: group.name
}

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname: entraGroup.mailNickname
  securityEnabled: true
  members: (!empty(group.type) ? [
    oneMemberGroup.id
  ] : []
}
slavizh commented 5 months ago

@dkershaw10 I think this should be added as known issues as it is pretty significant to me.

shenglol commented 5 months ago

@shenglol @dkershaw10 I think I have nailed it down to conditions not working at all. If I not mistaken even in v2 they should work to some extent so may be the issue is a mix of v2 and graph.

provider microsoftGraph

param entraGroup object = {
        name: 'ExampleGroup'
        displayName: 'Example Group 1'
        mailNickname: 'exampleGroup'
      }

var group = {
  name: 'dummy'
  type: ''
}
resource oneMemberGroup 'Microsoft.Graph/groups@v1.0' existing = if (!empty(group.type)) {
  uniqueName: group.name
}

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname: entraGroup.mailNickname
  securityEnabled: true
  members: (!empty(group.type) ? [
    oneMemberGroup.id
  ] : []
}

@slavizh I've identified the root cause for this one. This is due to us performing preflight validation for the existing resource oneMemberGroup while its condition is false. I'm working on a fix for it.

We are still investigating the other issues.

shenglol commented 4 months ago

Found the bug that blocks the example blow. Working on a fix now.

provider microsoftGraph

param entraGroup object = {
  name: 'ExampleGroup2'
  type: 'Security'
  displayName: 'Example Group 2'
  mailNickname: 'exampleGroup2'
  members: [
    {
      name: 'ExampleGroup'
      type: 'Group'
    }
  ]
}

resource memberGroups 'Microsoft.Graph/groups@v1.0' existing = [for (member, i) in entraGroup.members: {
  uniqueName: member.name
}]

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname:  entraGroup.mailNickname
  securityEnabled: entraGroup.securityEnabled
  members: [for (member, i) in entraGroup.members: memberGroups[i].id]
}
prittelm commented 2 months ago

@shenglol Do you have any updates on this? We are currently running into a very similar issue with the "existing" keyword.

shenglol commented 2 months ago

Found the bug that blocks the example blow. Working on a fix now.

provider microsoftGraph

param entraGroup object = {
  name: 'ExampleGroup2'
  type: 'Security'
  displayName: 'Example Group 2'
  mailNickname: 'exampleGroup2'
  members: [
    {
      name: 'ExampleGroup'
      type: 'Group'
    }
  ]
}

resource memberGroups 'Microsoft.Graph/groups@v1.0' existing = [for (member, i) in entraGroup.members: {
  uniqueName: member.name
}]

resource entraGroupRes 'Microsoft.Graph/groups@v1.0' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.displayName
  mailEnabled: false
  mailNickname:  entraGroup.mailNickname
  securityEnabled: entraGroup.securityEnabled
  members: [for (member, i) in entraGroup.members: memberGroups[i].id]
}

The fix for this should be deployed recently.

@prittelm Do you mind sharing your code? I wonder if it's the same issue or a different issue.

@slavizh Could you check if this works for you now?

slavizh commented 2 months ago

@shenglol I can confirm that it works now. Thank you a lot Shenglong! This was one of the critical bugs for us. Unfortunately I think I have encountered another two bugs:

Should I log separate issues for problem 1 or not? Should I log separate issue for problem 2 or not?

slavizh commented 2 months ago

@shenglol another interesting behavior is that if you try to add the same Entra group twice in single deployment by defining it existing loop syntax you also get Insufficient privileges error. Overall that scenario is something someone would not do as it does not makes sense to add the same group twice in members but the error message is not correct. For example if you try that scenario with user assigned identity you get error that the resource is defined multiple times.

dkershaw10 commented 2 months ago

@slavizh Thanks for validating the fix, and please can you log separate issues for 1 and 2. I'm not sure I fully follow issue 2, but if you create a new issue with repro steps then I'm sure I'll understand :).

Doe the "interesting behavior", can you provide error message for insufficient privileges (when adding the same group twice in members please)? I'll try to repro that as well. Seems like this might be issue 3 ;)

slavizh commented 2 months ago

@dkershaw10 done.

dkershaw10 commented 2 months ago

Given the other 2 issues (thanks for filing those), can we close this issue now @slavizh?

slavizh commented 2 months ago

@dkershaw10 yes