User Define types - Deployment template validation failed: 'The resource '' at line '' and column '' is defined multiple times in a template.

[slavizh]

Bicep version
Bicep CLI version 0.25.53 (c0ad57d)

Describe the bug
I have stumbled upon the following deployment error:

New-AzDeployment: 14:50:09 - Error: Code=InvalidTemplate; Message=Deployment template validation failed: 'The resource 'Microsoft.Authorization/policyDefinitions/96670d01-0a4d-4649-9c89-2d3abc0a5025' at line '8' and column '36' is defined multiple times in a template. Please see https://aka.ms/arm-syntax-resources for usage details.'.
New-AzDeployment: The deployment validation failed

I am trying to do two different policy assignments for the same policy definition. Interestingly the error appears only if there is schema (user defined types) defined and runs without issue if I remove the user defined types from code. The issue is also not present even with schema when the referenced policy definitions do not repeat.

To Reproduce
Steps to reproduce the behavior:

input-types.bicep

@export()
type policyAssignment = {
  @description('''The ID of the policy assignment.''')
  id: string
  @description('Display name for the policy assignment.')
  @maxLength(128)
  name: string
  @description('''The ID of the policy definition.''')
  policyDefinitionId: string
  @description('Pair of name and value to be added.')
  metadata: metadata?
  @description('''The policy definition parameters and values''')
  parameters: parametersPolicyDefinition?
}

type parametersPolicyDefinition = {
  @description('''The name of the parameter.''')
  *: {}
}

type metadata = {
  @description('The value for the metadata name.')
  *: string
}

@export()
type tagsType = {
  @description('''The tag name and value pair.''')
  *: string
}



@description('Deploys Policy Assignments for policy definitions.')
param policyAssignments policyAssignment[] = []

main.bicep

targetScope = 'subscription'

import * as inputSchema from 'input-types.bicep'

param policyAssignments inputSchema.policyAssignment[] = [
  {
    id: 'require-tag-one'
    name: 'Require a tag on resource groups: [tag-one]'
    policyDefinitionId: '96670d01-0a4d-4649-9c89-2d3abc0a5025'
    parameters: {
      tagName: {
        value: 'tag-one'
      }
    }
  }
  {
    id: 'require-tag-two'
    name: 'Require a tag on resource groups: [tag-two]'
    policyDefinitionId: '96670d01-0a4d-4649-9c89-2d3abc0a5025'
    parameters: {
      tagName: {
        value: 'tag-two'
      }
    }
  }
]


resource tenantPolicyDefinitions 'Microsoft.Authorization/policyDefinitions@2023-04-01' existing = [for (policyAssignment, i) in policyAssignments: {
  name: policyAssignment.policyDefinitionId
  scope: tenant()
}]

resource policyAssignmentsRes 'Microsoft.Authorization/policyAssignments@2023-04-01' = [for (policyAssignment, i) in policyAssignments: {
  name: policyAssignment.id
  location: deployment().location
  properties: {
    displayName: policyAssignment.name
    policyDefinitionId: tenantPolicyDefinitions[i].id
    metadata: {}
    parameters: policyAssignment.parameters
  }
}]

Additional context
Add any other context about the problem here.

[slavizh]

The only workaround if you still want to use user defined types is to enter the policy definition ID directly instead of using existing resource but obviously that should be temporary workaround as this is a bug.

[jeskew]

Tagging this one for team discussion. Everything here is working as intended, although a divergence in behavior between symbolic name and non-symbolic name templates was not foreseen.

@slavizh I'm guessing the example you shared has been simplified, but one workaround would be to use the tenantResourceId function instead of an existing resource:

resource policyAssignmentsRes 'Microsoft.Authorization/policyAssignments@2023-04-01' = [for (policyAssignment, i) in policyAssignments: {
  name: policyAssignment.id
  location: deployment().location
  properties: {
    displayName: policyAssignment.name
    policyDefinitionId: tenantResourceId('Microsoft.Authorization/policyDefinitions', policyAssignment.policyDefinitionId)
    metadata: {}
    parameters: policyAssignment.parameters
  }
}]

If you do need to work with other properties of the policy definition resource, you can use an expression to get the distinct policy definition IDs (although this could cause the index of a policy assignment in policyAssignmentsRes not to be aligned with the index of the corresponding definition ID in tenantPolicyDefinitions). The union function applied to arrays will prune duplicates:

resource tenantPolicyDefinitions 'Microsoft.Authorization/policyDefinitions@2023-04-01' existing = [for policyDefinitionId in union(map(policyAssignments, a => a.policyDefinitionId), []): {
  name: policyDefinitionId
  scope: tenant()
}]

[slavizh]

I prefer using concat as it feels better for me. Thankfully I do not need the other properties in this case. I am hoping that it could be fixed though as it is really that is something allowed by ARM and it should not be limit with bicep when using user define types.

[jeskew]

The error you're seeing is coming from ARM and reflects behavior of the deployment engine. The reason this only comes up when using user-defined types is because that is one of the triggers that will cause Bicep compilation to target "languageVersion 2.0" in the ARM JSON it outputs, and one of the primary differences between that template language version and the one that preceded it is how existing resources are handled.

In a standard ARM template, existing resources don't have a concrete representation in the ARM JSON. Given a template like the following:

param storageAccountName string

resource sa 'Microsoft.Storage/storageAccounts@2023-01-01' existing = {
  name: storageAccountName
}

output storageAccountAccessTier string = sa.properties.accessTier

Bicep will compile it to either:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.25.53.49325",
      "templateHash": "7769578801167663262"
    }
  },
  "parameters": {
    "storageAccountName": {
      "type": "string"
    }
  },
  "resources": [],
  "outputs": {
    "storageAccountAccessTier": {
      "type": "string",
      "value": "[reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-01-01').accessTier]"
    }
  }
}

or

{
  "$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.25.53.49325",
      "templateHash": "12793356154020495492"
    }
  },
  "parameters": {
    "storageAccountName": {
      "type": "string"
    }
  },
  "resources": {
    "sa": {
      "existing": true,
      "type": "Microsoft.Storage/storageAccounts",
      "apiVersion": "2023-01-01",
      "name": "[parameters('storageAccountName')]"
    }
  },
  "outputs": {
    "storageAccountAccessTier": {
      "type": "string",
      "value": "[reference('sa').accessTier]"
    }
  }
}

The main difference is that in the first example, resources is an empty array, whereas in the second, there is an entry for the sa existing resource. Because sa appears as a resource declaration, ARM allows you to manipulate when during the deployment its data will be fetched via "dependsOn" properties. For example, if the storage account will be created in a module, you can force ARM to delay the GET call until after the module is complete:

{
  "$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.25.53.49325",
      "templateHash": "11667410164823205578"
    }
  },
  "parameters": {
    "storageAccountName": {
      "type": "string"
    }
  },
  "resources": {
    "sa": {
      "existing": true,
      "type": "Microsoft.Storage/storageAccounts",
      "apiVersion": "2023-01-01",
      "name": "[parameters('storageAccountName')]",
      "dependsOn": [
        "mod"
      ]
    },
    "mod": {
      "type": "Microsoft.Resources/deployments",
      "apiVersion": "2022-09-01",
      "name": "mod",
      "properties": {...}
    }
  },
  "outputs": {
    "storageAccountAccessTier": {
      "type": "string",
      "value": "[reference('sa').accessTier]"
    }
  }
}

There's no good way for ARM to support multiple resource declarations targeting the same resource ID while allowing the order of calls to be controlled via dependsOn. I opened a couple of issues (#13595 and #13596) to make sure Bicep is handling this correctly, but the ARM behavior is what is leading to the error message you encountered.

[slavizh]

ok. so it cannot be fixed due to being ARM issue? If it cannot be fixed than the warning of using resourceId() instead of existing becomes useless and we have to suppress it. Additionally every time we use existing somewhere in loop we have to think if the reference resource will repeat or not and put different code based on that. Also when we add schema to existing templates we need to figure out if we have case like that as well. Another scenario that comes to my mind if you have the same type of existing resource defined twice with different names (no loops). If the parameters input results in the same resource name provided would that result also in error? Typical case is where you have resource with password and encryption. For both you can define different existing Key Vault statement but the end user providing the parameters can use the same key vault.

[jeskew]

There are a few things we can do here to improve the experience. Please let me know if you would have any preference between them and the team can prioritize accordingly:

1. The warning you are seeing when using resourceId() should be adjusted so that it allows legitimate uses of that function. I didn't see any warnings with the snippet using tenantResourceId() from a previous comment, but if you can share a sample that produces this warning, we can try to make the linter more context-aware. Honestly, tenantResourceId() feels like the most "correct" option in this particular scenario, so any linters we have that push users away from that should be looked at.
2. There's a long-standing proposal to add a function to create inline existing resource references that could be used as a substitute for top-level existing resource declarations. For your sample, this would look something like:

   resource policyAssignmentsRes 'Microsoft.Authorization/policyAssignments@2023-04-01' = [for (policyAssignment, i) in policyAssignments: {
     name: policyAssignment.id
     location: deployment().location
     properties: {
       displayName: policyAssignment.name
       policyDefinitionId: tenantResource('Microsoft.Authorization/policyDefinitions@2023-04-01', policyAssignment.policyDefinitionId).id
       metadata: {}
       parameters: policyAssignment.parameters
     }
   }]

3. Bicep needs a more ergonomic way to remove duplicates from a list. I suggested union(map(policyAssignments, a => a.policyDefinitionId), []) above, and @anthony-c-martin is working on adding functions to support an expression like objectKeys(groupBy(policyAssignments, x => x.policyDefinitionId)). But maybe we need a simple distinct function (looks like there is an open issue for this: Built-in function for creating distinct "sets" from an array #2082).

[anthony-c-martin]

@jeskew should add "support deduplication of existing symbolic name resources to deployment engine" (I think what you were suggesting earlier) to that list? With the caveat that it's probably going to be tricky to implement and would likely take a long time.

[jeskew]

@anthony-c-martin Per the team discussion, that's not something we can do. The fundamental affordance in ARM of "existing": true resource declarations over reference() functions is that template authors can control where in the deployment graph the corresponding GET request is executed. A necessary consequence of this affordance is that "existing": true declarations are not fungible in the way reference() functions are.

We could expose some mechanism to control whether an existing resource in a Bicep template would be compiled to an "existing": true resource declaration or just a reference() function, but it would come with caveats and is something we would probably want to see used only rarely. Something like:

@inlineOnly()
resource r '<type>@<version>' existing = {
  name: 'name'
}

Where the @inlineOnly() decorator would prevent an "existing": true declaration from being generated in the compiled JSON template. This would need to block the use of dependsOn in the targeted existing resource and block using the targeted resource in the dependsOn property of any other resource:

@inlineOnly()
resource r '<type>@<version>' existing = {
  name: 'name'
  dependsOn: [ // <-- error-level diagnostic because a `reference()` function can't depend on anything
    other
  ] 
}

resource other '<type>@<version>' = {
  dependsOn: [
    r // <-- error-level diagnostic because `r` isn't part of the deployment graph
  ]
}

[anthony-c-martin]

The fundamental affordance in ARM of "existing": true resource declarations over reference() functions is that template authors can control where in the deployment graph the corresponding GET request is executed. A necessary consequence of this affordance is that "existing": true declarations are not fungible in the way reference() functions are.

Is this desirable though? Mostly we want users thinking declaratively, not imperatively. I'd have thought collapsing the references and then scheduling it as late as possible (based on dependencies) in the job graph would allow us to be at parity with the reference() function.

Alternatively, if we want the opposite behavior (independent GETs for each resource), I'd have thought we could fix this by adding entropy (e.g. the symbolic name of the resource declaration) to the set of identifiers used to deduplicate references in the job graph.

[jeskew]

Is this desirable though? Mostly we want users thinking declaratively, not imperatively. I'd have thought collapsing the references and then scheduling it as late as possible (based on dependencies) in the job graph would allow us to be at parity with the reference() function.

I would push back gently on the idea that specifying dependencies on existing resources is imperative rather than declarative. The only control lever exposed by ARM is "dependsOn", and then ARM assembles the graph based on declared dependencies. This is also how ARM chose to solve #7402, which is still an open issue for non-symbolic name templates, as they offer no way to declare that a reference() or list*() function call depends on a different resource having completed its deployment.

Alternatively, if we want the opposite behavior (independent GETs for each resource), I'd have thought we could fix this by adding entropy (e.g. the symbolic name of the resource declaration) to the set of identifiers used to deduplicate references in the job graph.

This may be possible, although I suspect it would break whatever makes #7402 avoidable in symbolic name templates.

[slavizh]

Looking at the proposed methods I think they are all some workarounds with the exception of this being fixed somehow in ARM so the developer can just do not have to care and use only existing as syntax. With that said I think it is better for you to decide what workaround to implement that fits mostly into the spirit of Bicep language. This workaround must come up with some good documentation when existing should be used and when the other thing and if in doubt which one should be chosen as default. I have noticed this problem when when it was reported by end user who tried to assign the same policy definition more than once. Initially I thought it was some kind of bug in code or end user issue even. Also I am still not sure if I define two different existing resources but because the end user put the same resource would end up in error or not.

[Adunaphel]

Running into this issue with a template that has several secrets passed to it from a parent template. In this case, using resourceId() as a workaround is not an option, since getSecret() can only be used in conjunction with the existing keyword, and since all of these are conditional based on parameters passed to the template, deduplicating them will become a challenge.

Snippet in question

resource runAsPasswordKeyVaults 'Microsoft.KeyVault/vaults@2023-07-01' existing = [
  for runCommand in virtualMachine.?runCommands ?? []: if (!empty(runCommand.?runAsPassword)) {
    name: runCommand.?runAsPassword.name ?? 'dummy'
    scope: resourceGroup(
      runCommand.?runAsPassword.?subscriptionId ?? subscription().subscriptionId,
      runCommand.?runAsPassword.?resourceGroup ?? resourceGroup().name
    )
  }
]

resource secureParamKeyVaults 'Microsoft.KeyVault/vaults@2023-07-01' existing = [
  for (runCommand, i) in virtualMachine.?runCommands ?? []: if (!empty(runCommand.?secureParameters)) {
    name: runCommand.?secureParameters.name ?? 'dummy'
    scope: resourceGroup(
      runCommand.?secureParameters.?subscriptionId ?? subscription().subscriptionId,
      runCommand.?secureParameters.?resourceGroup ?? resourceGroup().name
    )
  }
]

resource sasTokenKeyVaults 'Microsoft.KeyVault/vaults@2023-07-01' existing = [
  for (runCommand, i) in virtualMachine.?runCommands ?? []: if (!empty(runCommand.source.?sasToken)) {
    name: runCommand.source.?sasToken.?name ?? 'dummy'
    scope: resourceGroup(
      runCommand.source.?sasToken.?subscriptionId ?? subscription().subscriptionId,
      runCommand.source.?sasToken.?resourceGroup ?? resourceGroup().name
    )
  }
]

@batchSize(1)
module runCommandsDeploy 'virtual-machine-run-command.bicep' = [
  for (runCommand, i) in virtualMachine.?runCommands ?? []: {
    name: 'runCommand-${uniqueString(virtualMachineRes.name)}-${i}'
    params: {
      location: location
      tags: union(tags, runCommand.?tags ?? {})
      runCommand: runCommand
      virtualMachineName: virtualMachineRes.name
      runAsUserPassword: !empty(runCommand.?runAsPassword)
        ? runAsPasswordKeyVaults[i].getSecret(runCommand.?runAsPassword.secretName)
        : ''
      secureParams: !empty(runCommand.?secureParameters)
        ? secureParamKeyVaults[i].getSecret(runCommand.?secureParameters.secretName)
        : ''
      sasToken: !empty(runCommand.source.?sasToken)
        ? sasTokenKeyVaults[i].getSecret(runCommand.source.?sasToken.?secretName)
        : ''
    }
  }
]

[slavizh]

agree. The key vault part is big problem as many times you need to define the same resource to get different secrets. I think we need resolution sooner on this.

[jeskew]

The cases discussed so far in this issue would be addressed by #13674. This is an imperfect solution, as template authors would still need to prune duplicate existing resources if they refer to runtime properties of the resource.

[slavizh]

glad that there will be fix for get secret scenario as well.