bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Deployment should fail for `existing` resources that do not exist, even when the `reference()` call is not required for transpile

Open mtessier84 opened this issue 2 years ago • 22 comments

Bicep version v0.14.85

Describe the bug The documentation says: If you attempt to reference a resource that doesn't exist, you get the NotFound error and your deployment fails. Check the name and scope of the resource you're trying to reference. Link

This is not true. The deployment will not fail if only referring to the name or id of the referenced existing resource. In this case, Bicep does not "resolve" the resource from Azure Resource Manager because Bicep can construct the resource id and get the name from the resource existing block without having to fetch the properties from Azure.

To Reproduce Steps to reproduce the behavior:

Additional context The resource existing block should allow enforcing validation or documentation should be updated to explain better the behavior.

mtessier84 avatar Mar 13 '23 05:03 mtessier84

I will close it because I found the documentation under "troubleshooting", but good to keep for visibility.

mtessier84 avatar Mar 13 '23 05:03 mtessier84

@mtessier84 , what documentation did you find ? Can you post the link? I also need to fail the deployment if the resource doesn't exist, but didn't find how yet.

arsenmk avatar Mar 17 '23 17:03 arsenmk

Sorry I was not clear. I could not find any documentation that says that the intended behavior is that it should fail the deployment. The documentation that I found and shared in the original post is under "Troubleshooting".

I dont have a solution for this. I could be reported as a feature request instead of a bug

mtessier84 avatar Mar 18 '23 12:03 mtessier84

I think the deployment will fail when at least when that resource is referenced somewhere (beyond the initial declaration). Otherwise, maybe we do not make the underlying reference() call?

alex-frankel avatar Mar 20 '23 21:03 alex-frankel

The following code does not fail if the storage account does not exist.

param Location string
param StorageAccountName string

resource StorageAccount 'Microsoft.Storage/storageAccounts@2022-09-01' existing = {
  name: StorageAccountName
}

resource ManagedIdentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
  name: 'MID${StorageAccount.name}'
  location: Location
}

There are cases when I wish the deployment would fail in such scenarios.

mtessier84 avatar Apr 03 '23 08:04 mtessier84

I see now -- thank you for sharing this example. This is in fact a case where we won't even attempt to reference the storage account because the storage account name can be calculated without it. Here is what the compiled Template looks like for that sample:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.15.164.21910",
      "templateHash": "8213433503989717287"
    }
  },
  "parameters": {
    "Location": {
      "type": "string"
    },
    "StorageAccountName": {
      "type": "string"
    }
  },
  "resources": [
    {
      "type": "Microsoft.ManagedIdentity/userAssignedIdentities",
      "apiVersion": "2023-01-31",
      "name": "[format('MID{0}', parameters('StorageAccountName'))]",
      "location": "[parameters('Location')]"
    }
  ]
}

I will re-open the issue.

alex-frankel avatar Apr 03 '23 12:04 alex-frankel

@mtessier84 if you have an existing resource and you want to ensure it gets added to the template, you can use an output for this reference. That will ensure it is included in the compiled template and the resourceid reference expression will be evaluated in the deployment.

param plan string = 'StorageAccounts'
targetScope = 'subscription'
resource PRICING 'Microsoft.Security/pricings@2022-03-01' existing =  {
    name: plan
}
output storage string = PRICING.id // <-- output referencing existing resource
  "outputs": {
    "storage": {
      "type": "string",
      "value": "[subscriptionResourceId('Microsoft.Security/pricings', parameters('plan'))]"
    }
  }

There is a current linter rule, that should pickup unused resources (when you don't reference it).

        "no-unused-existing-resources": {
          "level": "warning"
        }

image

brwilkinson avatar Apr 05 '23 19:04 brwilkinson

Thank you Ben, I think that would be a good workaround for my case!

mtessier84 avatar Apr 06 '23 00:04 mtessier84

Thank you @mtessier84

Given we have the linter rule (to catch this) and an appropriate way to handle the requirement I will "close as - by design".

brwilkinson avatar Apr 06 '23 00:04 brwilkinson

I'm not sure I follow the resolution. My understanding there's a desire to fail the deployment if the existing resource does not exist. @brwilkinson -- I think even for the case of emitting the id of the resource, there won't ever be a reference() call so the deployment will still succeed even if the resource exists. Is that right?

If so, then a workaround could be to reference a non-compile-time property like so:

targetScope = 'subscription'

param plan string = 'StorageAccounts'

resource PRICING 'Microsoft.Security/pricings@2022-03-01' existing =  {
    name: plan
}
output storage object = PRICING.properties // <-- output referencing existing resource

If that workaround is good enough, we can keep the issue closed, but I wonder if we should somehow emit a dummy reference() for all existing resources so that this would not be required. That would be a breaking change, so we would need to really think through it if it's something we wanted to do.

alex-frankel avatar Apr 06 '23 16:04 alex-frankel

@alex-frankel got it yes, without the reference there is no validation, I missed this one.

would still need an explicit reference for this to work in bicep.

resource storage 'Microsoft.Storage/storageAccounts@2022-09-01' existing = {
  name: 'doesnotexist'
}
output storage object = reference(storage.id)

Error: Code=InvalidTemplate; Message=Deployment template validation failed: 'The resource 'Microsoft.Storage/storageAccounts/doesnotexist' referenced in output is not defined in the template.

<edit, let me test this out some more>

brwilkinson avatar Apr 06 '23 18:04 brwilkinson

re-opening, since above does not cover the initial ask. Thanks @alex-frankel

brwilkinson avatar Apr 06 '23 18:04 brwilkinson

seems like this maybe related to below, in functionality.

  • https://github.com/Azure/bicep/issues/4023

brwilkinson avatar Apr 06 '23 18:04 brwilkinson

Looping back on the workaround scenario.

  • Need to add api version to the reference since resource is not defined in current template
resource storage 'Microsoft.Storage/storageAccounts@2022-09-01' existing = {
  name: 'doesnotexist'
}
output storage object = reference(storage.id, storage.apiVersion)

correctly fails if resource does not exist

The Resource 'Microsoft.Storage/storageAccounts/doesnotexist' under resource group 'AEU1-PE-CTL-RG-D1' was not found

resource storage 'Microsoft.Storage/storageAccounts@2022-09-01' existing = {
  name: 'storageaccountexists'
}
output storage object = reference(storage.id, storage.apiVersion)

correctly deploys, if resources exists

image

brwilkinson avatar Apr 07 '23 04:04 brwilkinson

Okay I found the inconsistency on the behavior ... worth adding ... relating to extensibility or symbolicNameCodegen

param vNETName string = 'doesnotexist'

resource VNET 'Microsoft.Network/virtualNetworks@2022-09-01' existing = {
  name: vNETName
}

output vnetid string = VNET.id
{
  "experimentalFeaturesEnabled": {
      // "extensibility": false
       "symbolicNameCodegen": false
  }

with extensibility|symbolicNameCodegen turned off, this does no evaluate the reference deployment has no error 🟩

Outputs : Name Type Value =============== ========================= ========== vnetid String "/subscriptions/4185fa9b-f470-466a-b3ae-8e6c3314a543/resourceGroups/AEU1-PE-CTL-RG-D1/providers/Microsoft.Network/virtualNetworks/doesnotexist"

image


param vNETName string = 'doesnotexist'

resource VNET 'Microsoft.Network/virtualNetworks@2022-09-01' existing = {
  name: vNETName
}
// even without any output

with extensibility|symbolicNameCodegen turned on, this always evaluates the reference so deployment has an error 🟥

{
  "experimentalFeaturesEnabled": {
    //"extensibility": true
    "symbolicNameCodegen": true
  }

The deployment 'temp' failed with error(s). Showing 1 out of 1 error(s). Status Message: The Resource 'Microsoft.Network/virtualNetworks/doesnotexist' under resource group 'AEU1-PE-CTL-RG-D1' was not found.

image

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "languageVersion": "1.10-experimental",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_EXPERIMENTAL_WARNING": "Symbolic name support in ARM is experimental, and should be enabled for testing purposes only. Do not enable this setting for any production usage, or you may be unexpectedly broken at any time!",
    "_generator": {
      "name": "bicep",
      "version": "0.16.1.55165",
      "templateHash": "9658580584476014833"
    }
  },
  "parameters": {
    "vNETName": {
      "type": "string",
      "defaultValue": "doesnotexist"
    }
  },
  "resources": {
    "VNET": {
      "existing": true,
      "type": "Microsoft.Network/virtualNetworks",
      "apiVersion": "2022-09-01",
      "name": "[parameters('vNETName')]"
    }
  }
}

brwilkinson avatar Apr 08 '23 23:04 brwilkinson

@jeskew I was meaning to ask if above was expected behavior?

brwilkinson avatar Apr 13 '23 07:04 brwilkinson

@brwilkinson The symbolic name template behavior is expected, though I'll have to tag in @anthony-c-martin as to whether this is an intentional behavioral difference between symbolic name and "classic" templates.

FWIW, though, that the symbolic name template behavior is I would expect from the Bicep syntax. The "classic" template behavior makes sense, but only if you know how existing resources get compiled to JSON template expressions (and are also familiar with how those expressions get processed at deploy time). From that perspective, I would say the issue is fixed in symbolic name templates.

jeskew avatar Apr 13 '23 13:04 jeskew

Thank you @jeskew

I am okay either way, although I will mention 4 things:

  1. I opened below where using an existing reference may be conditional
    • https://github.com/Azure/bicep/issues/10410
    • I didn't give a specific example there, however I can provide one here: for a Functions app, I might use a connection string OR a managed identity which I never need to use the connection string.
      • https://techcommunity.microsoft.com/t5/apps-on-azure-blog/use-managed-identity-instead-of-azurewebjobsstorage-to-connect-a/ba-p/3657606
  2. It appears that people like conditional use case for existing resources
    • https://github.com/Azure/bicep/issues/4023#issuecomment-1506743444
    • "Such kind of functionality is needed like water for flowers :)"
  3. It would be a breaking change.
  4. Obviously this issue was opened in favor of the behavior

brwilkinson avatar Apr 13 '23 18:04 brwilkinson

I think changing this behaviour if we introduce symbolic templates as a "default" can be a big breaking change. sometimes we use existing resources to construct an id (no reference call, just resoruceId). We might also not have permission to read the resource but we know it exists (i.e. creating private endpoints).

miqm avatar Apr 19 '23 19:04 miqm

@jeskew you mentioned below, I was wondering that also relates to this or was it a separate issue?

https://github.com/Azure/bicep/issues/9457#issuecomment-1518308832

The fix for this is in the w16 ARM release.

brwilkinson avatar Apr 21 '23 23:04 brwilkinson

@brwilkinson The fix in w16 only addresses what ARM does when there is more than one declared resource targeting the same resource ID but only one is active due to conditions. That manifested as a behavioral difference for templates that used a pattern like the following (which would not cause any issues in a non-symbolic name template):

param redeploy bool

resource alreadyThere 'provider/kind@version' existing = if (!redeploy) {
  ...
}

resource new 'provider/kind@version' = if (redeploy) {
  ...
}

This issue is specifically for unreachable/nonexistent existing resources that aren't deactivated via an if clause.

jeskew avatar Apr 23 '23 14:04 jeskew

The consensus of a team discussion was that there shouldn't be any divergence between the behavior of a symbolic name template and a template not using symbolic name syntax. The ARM runtime should be updated to allow existing resources that don't exist so long as no reference function invocation refers to those resources.

If we do add a mechanism to make sure an existing resource actually does exist, it should work regardless of whether the template was compiled with symbolic name codegen enabled.

jeskew avatar May 15 '23 22:05 jeskew