`dependsOn` indexing is broken if it skips over an `existing` resource with a different `for` loop or indexing expression #15513

slavizh · 2024-11-07T09:42:33Z

Bicep version
Bicep CLI version 0.31.34 (ec82b47)

Describe the bug
The change in #13674 broke existing syntax for language version 2 in critical way. The example is using Graph but it applies to all resource types with existing. Note that I am also using user assigned identities which is also Azure resource.

To Reproduce
This template:

extension 'br:mcr.microsoft.com/bicep/extensions/microsoftgraph/v1.0:0.1.8-preview'  as microsoftGraphV1
extension 'br:mcr.microsoft.com/bicep/extensions/microsoftgraph/beta:0.1.8-preview'  as microsoftGraphBeta

param entraGroup object = {
  name: 'ExampleGroup2'
  type: 'Security'
  members: [
    {
      name: '{application name}'
      type: 'Application'
    }
  ]
  owners: [
    {
      name: '{user identity name}'
      resourceGroup: '{resource group name}'
      type: 'UserAssignedManagedIdentity'
    }
  ]
}

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

resource memberManagedIdentities 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-07-31-preview' existing = [
  for (member, i) in entraGroup.members: if (member.type =~ 'UserAssignedManagedIdentity') {
    //https://github.com/Azure/bicep/issues/13937
    name: empty(union(defaultMember, member).name) ? 'dummy${i}' : member.name
    scope: resourceGroup(union(defaultMember, member).subscriptionId, union(defaultMember, member).resourceGroup)
  }
]

resource memberApplications 'Microsoft.Graph/[email protected]' existing = [
  for (member, i) in entraGroup.members: if (member.type =~ 'Application') {
    //https://github.com/Azure/bicep/issues/13937
    uniqueName: empty(union(defaultMember, member).name) ? 'dummy${i}' : member.name
  }
]

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

resource memberServicePrincipalsStandalone 'Microsoft.Graph/[email protected]' existing = [
  for (member, i) in entraGroup.members: if (member.type =~ 'ServicePrincipal') {
    //https://github.com/Azure/bicep/issues/13937
    appId: empty(union(defaultMember, member).appId) ? 'dummy${i}' : member.appId
  }
]

resource memberGroups 'Microsoft.Graph/[email protected]' existing = [
  for (member, i) in entraGroup.members: if (member.type =~ 'Group') {
    //https://github.com/Azure/bicep/issues/13937
    uniqueName: empty(union(defaultMember, member).name) ? 'dummy${i}' : member.name
  }
]

resource ownerManagedIdentities 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-07-31-preview' existing = [
  for (owner, i) in entraGroup.owners: if (owner.type =~ 'UserAssignedManagedIdentity') {
    //https://github.com/Azure/bicep/issues/13937
    name: empty(union(defaultMember, owner).name) ? 'dummy${i}' : owner.name
    scope: resourceGroup(union(defaultMember, owner).subscriptionId, union(defaultMember, owner).resourceGroup)
  }
]

resource ownerApplications 'Microsoft.Graph/[email protected]' existing = [
  for (owner, i) in entraGroup.owners: if (owner.type =~ 'Application') {
    //https://github.com/Azure/bicep/issues/13937
    uniqueName: empty(union(defaultMember, owner).name) ? 'dummy${i}' : owner.name
  }
]

resource ownerServicePrincipals 'Microsoft.Graph/[email protected]' existing = [
  for (owner, i) in entraGroup.owners: if (owner.type =~ 'Application') {
    appId: ownerApplications[i].appId
  }
]

resource ownerServicePrincipalsStandalone 'Microsoft.Graph/[email protected]' existing = [
  for (owner, i) in entraGroup.owners: if (owner.type =~ 'ServicePrincipal') {
    //https://github.com/Azure/bicep/issues/13937
    appId: empty(union(defaultMember, owner).appId) ? 'dummy${i}' : owner.appId
  }
]

resource entraGroupRes 'Microsoft.Graph/[email protected]' = {
  uniqueName: entraGroup.name
  displayName: entraGroup.name
  mailEnabled: false
  mailNickname: entraGroup.name
  securityEnabled: true
  members: [
    for (member, i) in entraGroup.members: member.type =~ 'UserAssignedManagedIdentity'
      ? memberManagedIdentities[i].properties.principalId
      : member.type =~ 'Application'
          ? memberServicePrincipals[i].id
          : member.type =~ 'ServicePrincipal'
              ? memberServicePrincipalsStandalone[i].id
              : member.type =~ 'Group' ? memberGroups[i].id : member.type =~ 'PrincipalId' ? member.principalId : ''
  ]
  owners: [
    for (owner, i) in entraGroup.owners: owner.type =~ 'UserAssignedManagedIdentity'
      ? ownerManagedIdentities[i].properties.principalId
      : owner.type =~ 'Application'
          ? ownerServicePrincipals[i].id
          : owner.type =~ 'ServicePrincipal' ? ownerServicePrincipalsStandalone[i].id : owner.type =~ 'PrincipalId' ? owner.principalId : ''
  ]
}

Works fine with 0.30.23 (ec3612e) but it fails with 0.31.34 (ec82b47).

Error:

  "properties": {
        "statusCode": "InternalServerError",
        "serviceRequestId": null,
        "statusMessage": "{\"error\":{\"code\":\"MultipleErrorsOccurred\",\"message\":\"Multiple error occurred: . Please see details.\",\"details\":[{\"code\":\"InternalServerError\",\"message\":\"Encountered internal server error. Diagnostic information: timestamp '20241107T093958Z', subscription id '{redacted}', tracking id '8290acda-6a98-4c58-834f-a6056ede609e', request correlation id '8290acda-6a98-4c58-834f-a6056ede609e'.\"},{\"code\":\"InternalServerError\",\"message\":\"Encountered internal server error. Diagnostic information: timestamp '20241107T093958Z', subscription id '{redacted}', tracking id '8290acda-6a98-4c58-834f-a6056ede609e', request correlation id '8290acda-6a98-4c58-834f-a6056ede609e'.\"},{\"code\":\"InternalServerError\",\"message\":\"Encountered internal server error. Diagnostic information: timestamp '20241107T093958Z', subscription id '{redacted}', tracking id '8290acda-6a98-4c58-834f-a6056ede609e', request correlation id '8290acda-6a98-4c58-834f-a6056ede609e'.\"},{\"code\":\"InternalServerError\",\"message\":\"Encountered internal server error. Diagnostic information: timestamp '20241107T093958Z', subscription id '{redacted}', tracking id '8290acda-6a98-4c58-834f-a6056ede609e', request correlation id '8290acda-6a98-4c58-834f-a6056ede609e'.\"}]}}",

Additional context
Add any other context about the problem here.

The text was updated successfully, but these errors were encountered:

slavizh · 2024-11-07T09:44:37Z

@jeskew @alex-frankel no idea how many people write the code like me and use language version 2 but I think this is pretty critical thus opted to mention you.

slavizh · 2024-11-07T11:21:21Z

Actually managed to scope the issue to the error generated when only Graph resources are referenced as existing. If I comment all the code around existing for Microsoft.Graph/applications, Microsoft.Graph/servicePrincipals and Microsoft.Graph/groups and their references the template deploys successfully. This is good as it seems affects only the Graph resources which is still in preview.

jeskew · 2024-11-07T17:47:05Z

@slavizh It looks like this isn't related to extensibility but happens when a non-copy resource depends on an existing copy resource that in turn depends on a non-existing copy resource. In that case, Bicep may sometimes try to skip over the existing resource in the dependency graph. The following template reproduces the issue without extensibility resources:

resource vnets 'Microsoft.Network/virtualNetworks@2024-03-01' = [for i in range(0, 10): {
  name: 'vnet${i}'
}]

resource subnets 'Microsoft.Network/virtualNetworks/subnets@2024-03-01' existing = [for i in range(0, 10): {
  parent: vnets[i]
  name: 'subnet'
}]

resource vault 'Microsoft.KeyVault/vaults@2023-07-01' = {
  name: 'vault'
  location: resourceGroup().location
  properties: {
    sku: {
      name: 'standard'
      family: 'A'
    }
    tenantId: subscription().tenantId
    networkAcls: {
      virtualNetworkRules: [for i in range(0, 10): {
        id: subnets[i].id
      }]
    }
  }
}

When compiled without symbolic name support (and therefore no existing resources in the compiled template), the generated template is invalid:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.30.23.60470",
      "templateHash": "1546815409293910064"
    }
  },
  "resources": [
    {
      "copy": {
        "name": "vnets",
        "count": "[length(range(0, 10))]"
      },
      "type": "Microsoft.Network/virtualNetworks",
      "apiVersion": "2024-03-01",
      "name": "[format('vnet{0}', range(0, 10)[copyIndex()])]"
    },
    {
      "type": "Microsoft.KeyVault/vaults",
      "apiVersion": "2023-07-01",
      "name": "vault",
      "location": "[resourceGroup().location]",
      "properties": {
        "sku": {
          "name": "standard",
          "family": "A"
        },
        "tenantId": "[subscription().tenantId]",
        "networkAcls": {
          "copy": [
            {
              "name": "virtualNetworkRules",
              "count": "[length(range(0, 10))]",
              "input": {
                "id": "[resourceId('Microsoft.Network/virtualNetworks/subnets', format('vnet{0}', range(0, 10)[range(0, 10)[range(0, 10)[copyIndex('virtualNetworkRules')]]]), 'subnet')]"
              }
            }
          ]
        }
      },
      "dependsOn": [
        "[resourceId('Microsoft.Network/virtualNetworks', format('vnet{0}', range(0, 10)[range(0, 10)[copyIndex()]]))]"
      ]
    }
  ]
}

(Note the copyIndex() expression within the vault resource's dependsOn property, even though vault is not a copy resource.)

This did not surface for symbolic name templates in v0.30 and below because vault would take a dependency on the existing resource:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "languageVersion": "2.0",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.30.23.60470",
      "templateHash": "13202434162406894311"
    }
  },
  "resources": {
    "vnets": {
      "copy": {
        "name": "vnets",
        "count": "[length(range(0, 10))]"
      },
      "type": "Microsoft.Network/virtualNetworks",
      "apiVersion": "2024-03-01",
      "name": "[format('vnet{0}', range(0, 10)[copyIndex()])]"
    },
    "subnets": {
      "copy": {
        "name": "subnets",
        "count": "[length(range(0, 10))]"
      },
      "existing": true,
      "type": "Microsoft.Network/virtualNetworks/subnets",
      "apiVersion": "2024-03-01",
      "name": "[format('{0}/{1}', format('vnet{0}', range(0, 10)[range(0, 10)[copyIndex()]]), 'subnet')]",
      "dependsOn": [
        "[format('vnets[{0}]', range(0, 10)[copyIndex()])]"
      ]
    },
    "vault": {
      "type": "Microsoft.KeyVault/vaults",
      "apiVersion": "2023-07-01",
      "name": "vault",
      "location": "[resourceGroup().location]",
      "properties": {
        "sku": {
          "name": "standard",
          "family": "A"
        },
        "tenantId": "[subscription().tenantId]",
        "networkAcls": {
          "copy": [
            {
              "name": "virtualNetworkRules",
              "count": "[length(range(0, 10))]",
              "input": {
                "id": "[resourceId('Microsoft.Network/virtualNetworks/subnets', format('vnet{0}', range(0, 10)[range(0, 10)[range(0, 10)[copyIndex('virtualNetworkRules')]]]), 'subnet')]"
              }
            }
          ]
        }
      },
      "dependsOn": [
        "subnets"
      ]
    }
  }
}

The dependency graph builder recognizes that vault is not a copy resource and just takes a dependency on all of the subnet resources.

In v0.31, the graph builder tries to be a bit smarter and skip over the existing resource since vault is only using the subnets' ID, but this makes it run headfirst into the error that affects non-symbolic name templates:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "languageVersion": "2.0",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.31.34.60546",
      "templateHash": "1523914663347343621"
    }
  },
  "resources": {
    "vnets": {
      "copy": {
        "name": "vnets",
        "count": "[length(range(0, 10))]"
      },
      "type": "Microsoft.Network/virtualNetworks",
      "apiVersion": "2024-03-01",
      "name": "[format('vnet{0}', range(0, 10)[copyIndex()])]"
    },
    "subnets": {
      "copy": {
        "name": "subnets",
        "count": "[length(range(0, 10))]"
      },
      "existing": true,
      "type": "Microsoft.Network/virtualNetworks/subnets",
      "apiVersion": "2024-03-01",
      "name": "[format('{0}/{1}', format('vnet{0}', range(0, 10)[range(0, 10)[copyIndex()]]), 'subnet')]",
      "dependsOn": [
        "[format('vnets[{0}]', range(0, 10)[copyIndex()])]"
      ]
    },
    "vault": {
      "type": "Microsoft.KeyVault/vaults",
      "apiVersion": "2023-07-01",
      "name": "vault",
      "location": "[resourceGroup().location]",
      "properties": {
        "sku": {
          "name": "standard",
          "family": "A"
        },
        "tenantId": "[subscription().tenantId]",
        "networkAcls": {
          "copy": [
            {
              "name": "virtualNetworkRules",
              "count": "[length(range(0, 10))]",
              "input": {
                "id": "[resourceId('Microsoft.Network/virtualNetworks/subnets', format('vnet{0}', range(0, 10)[range(0, 10)[range(0, 10)[copyIndex('virtualNetworkRules')]]]), 'subnet')]"
              }
            }
          ]
        }
      },
      "dependsOn": [
        "[format('vnets[{0}]', range(0, 10)[copyIndex()])]"
      ]
    }
  }
}

The graph builder needs to be aware of when it is skipping over a looped existing resource.

anthony-c-martin · 2024-11-07T20:08:13Z

@slavizh we're working on a fix for this now - I think we'll need to do a new Bicep release for it.

slavizh · 2024-11-08T08:13:47Z

@anthony-c-martin great! I am happy that the root cause was found so fast.

…the deployments engine will use the GET response" (#15524) Reverts #15447 to address #15513 There is a subtle bug in the way `dependsOn` is generated when a non-copy resource (_r_) depends on a copy resource (_parent_) which depends on a copy resource (_grandparent_) when _parent_ is not included in the calculated dependency graph. Note that even with this reversion, this scenario is still broken for non-symbolic name templates, but this is not unique to v0.31. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/15524)

jeskew · 2024-11-14T20:06:03Z

@slavizh The change included in v0.31 was reverted in the latest version. Leaving this open while we work on fixing the underlying issue for 0.32

scrocquesel-ml150 · 2024-11-18T15:45:36Z

Hi, could be the same root cause. Azure Devops agent have recently been updated to 0.31.34 and we now have build issues. Note that when using version v0.31.92 locally, we have no issue.

Error BCP036: The property "scope" expected a value of type "resourceGroup" but the provided value is of type "{ }". [https://aka.ms/bicep/core-diagnostics#BCP036]

and the corresponding code is

resource serviceBusResource 'Microsoft.ServiceBus/namespaces@2024-01-01' existing = {
  name: '${env}-svcbus'
  scope: env == 'dev' ? resourceGroup('x-${env}') : resourceGroup('x-${env}-rg')
}

I changed to scope: resourceGroup(env == 'dev' ? 'x-${env}' : 'x-${env}-rg') but I have other build errors.

jeskew · 2024-11-18T15:49:25Z

@scrocquesel-ml150 There's some discussion on that in #15517, and it should be fixed in 0.31.92

github-project-automation bot added this to Bicep Nov 7, 2024

github-project-automation bot moved this to Todo in Bicep Nov 7, 2024

microsoft-github-policy-service bot added the Needs: Triage 🔍 label Nov 7, 2024

slavizh changed the title ~~#13674 made thngs worse for existing syntax in language version 2~~ #13674 borke existing syntax when extensibility is enabled Nov 7, 2024

jeskew mentioned this issue Nov 8, 2024

Revert "Only add an explicit dependency on an existing resource when the deployments engine will use the GET response" #15524

Merged

stephaniezyen added this to the v0.32 milestone Nov 13, 2024

stephaniezyen assigned jeskew Nov 13, 2024

stephaniezyen added bug Something isn't working and removed Needs: Triage 🔍 labels Nov 13, 2024

jeskew linked a pull request Nov 14, 2024 that will close this issue

Move all indexing expressions when calculating dependsOn expressions #15580

Open

jeskew changed the title ~~#13674 borke existing syntax when extensibility is enabled~~ #13674 broke existing syntax when extensibility is enabled Nov 14, 2024

jeskew changed the title ~~#13674 broke existing syntax when extensibility is enabled~~ dependsOn indexing is broken if it skips over an existing resource with a different for loop or indexing expression Nov 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`dependsOn` indexing is broken if it skips over an `existing` resource with a different `for` loop or indexing expression #15513

`dependsOn` indexing is broken if it skips over an `existing` resource with a different `for` loop or indexing expression #15513

slavizh commented Nov 7, 2024

slavizh commented Nov 7, 2024

slavizh commented Nov 7, 2024

jeskew commented Nov 7, 2024

anthony-c-martin commented Nov 7, 2024

slavizh commented Nov 8, 2024

jeskew commented Nov 14, 2024

scrocquesel-ml150 commented Nov 18, 2024 •

edited

Loading

jeskew commented Nov 18, 2024

dependsOn indexing is broken if it skips over an existing resource with a different for loop or indexing expression #15513

dependsOn indexing is broken if it skips over an existing resource with a different for loop or indexing expression #15513

Comments

slavizh commented Nov 7, 2024

slavizh commented Nov 7, 2024

slavizh commented Nov 7, 2024

jeskew commented Nov 7, 2024

anthony-c-martin commented Nov 7, 2024

slavizh commented Nov 8, 2024

jeskew commented Nov 14, 2024

scrocquesel-ml150 commented Nov 18, 2024 • edited Loading

jeskew commented Nov 18, 2024

`dependsOn` indexing is broken if it skips over an `existing` resource with a different `for` loop or indexing expression #15513

`dependsOn` indexing is broken if it skips over an `existing` resource with a different `for` loop or indexing expression #15513

scrocquesel-ml150 commented Nov 18, 2024 •

edited

Loading