dependsOn indexing is broken if it skips over an existing resource with a different for loop or indexing expression
#15513
Comments
|
@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. |
|
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. |
slavizh
changed the title
|
@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 {
"$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 This did not surface for symbolic name templates in v0.30 and below because {
"$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 In v0.31, the graph builder tries to be a bit smarter and skip over the existing resource since {
"$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. |
|
@slavizh we're working on a fix for this now - I think we'll need to do a new Bicep release for it. |
|
@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)
|
@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 |
jeskew
changed the title
|
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 and the corresponding code is I changed to |
|
@scrocquesel-ml150 There's some discussion on that in #15517, and it should be fixed in 0.31.92 |
jeskew
changed the title
dependsOn indexing is broken if it skips over an existing resource with a different for loop or indexing expression
#15580) Resolves #15513 ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/15580) Bicep will normally generate an explicit dependency when one resource refers to another. For example, if the body of `b` includes a symbolic reference to `a`, then in the compiled JSON template, the declaration for `b` will have a `dependsOn` property that includes `a`. However, if `a` is an `existing` resource and the template is not being compiled to language version 2.0, then the compiler will "skip over" `a` and have `b` depend on whatever `a` depends on. For example, for the following template: ```bicep resource a 'type@version' existing = { name: 'a' dependsOn: [ c ] } resource b 'type@version' = { name: 'b' properties: { foo: a.properties.bar } } resource c 'type@version' = { name: 'c' } ``` the non-symbolic name output will have `b` depend on `c`. #15447 added a couple of scenarios in which Bicep would skip over an existing resource even if compiling with symbolic name support. This was done because the ARM backend will perform a `GET` request on any `existing` resource in the template _unless_ its body properties are never read and no deployed resource explicitly depends on it. The extra `GET` requests could sometimes cause template deployments to fail, for example if the deploying principal had permission to use secrets from a key vault as part of a deployment but did not have more generic /read permissions on the vault. #15447 reused some existing logic for skipping over an intermediate existing dependency that unfortunately had an underlying bug that manifested when the skipped over resource was looped and used its loop iterator to index into the dependency once removed. For example, if we modify the earlier example slightly: ```bicep resource a 'type@version' existing = [for i in range(0, 10): { name: 'a${i}' dependsOn: [ c[i] ] }] resource b 'type@version' = { name: 'b' properties: { foo: [for i in range(0, 10): a[i].properties.bar] } } resource c 'type@version' = [for i in range(0, 10): { name: 'c${i}' }] ``` Then in the compiled output, `b` will have an explicit dependency on `[resourceId('type', format('c[{0}]', copyIndex()))]`. Because `b` is not looped, the deployment will fail. Related issues will occur if `b` indexes into `a` with a more complex expression or if there is an intervening variable. This PR updates explicit dependency generation to take all steps between a depending resource and its dependency into account when generating index expressions. For example, in the following template: ```bicep resource vnets 'Microsoft.Network/virtualNetworks@2024-03-01' = [for i in range(0, 2): { name: 'vnet${i}' }] resource subnets 'Microsoft.Network/virtualNetworks/subnets@2024-03-01' existing = [for j in range(0, 10): { parent: vnets[j % 2] name: 'subnet' }] resource vault 'Microsoft.KeyVault/vaults@2023-07-01' = [for k in range(11, 10): { name: 'vault${k}' location: resourceGroup().location properties: { sku: { name: 'standard' family: 'A' } tenantId: subscription().tenantId networkAcls: { virtualNetworkRules: [{ id: subnetIds[k - 11] }] } } }] var subnetIds = [for l in range(20, 10): subnets[l - 20].id] ``` `vault` will depend on `vnets[(range(20, 10)[k - 11] - 20) % 2]`. Prior to this PR, `vault` will instead depend on `vnets[k % 2]`, which is the wrong vnet. This PR does **not** reapply the change from #15447 but only addresses the issue described above. #15447 is reapplied in #15693
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:
Works fine with 0.30.23 (ec3612e) but it fails with 0.31.34 (ec82b47).
Error:
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: