Block nested deployment resources with inner scoped evaluation and symbolic references to outer scope

[jeskew]

Resolves #11846

This PR updates EmitLimitationCalculator to emit an error when a resource of type Microsoft.Resources/deployments uses inner scoped expression evaluation but refers to symbols defined in the outer scope from within the template property.

This PR may be a breaking change for some templates, as the added check may catch scenarios that would not result in a runtime deployment failure, e.g., when a template has a symbol with the same name and type defined in both contexts:

var fizz = 'buzz'

resource nested 'Microsoft.Resources/deployments@2020-10-01' = {
  name: 'name'
  properties: {
    mode: 'Incremental'
    expressionEvaluationOptions: {
      scope: 'inner'
    }
    template: {
      '$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
      contentVersion: '1.0.0.0'
      variables: {
        fizz: 'pop'
      }
      resources: [
        {
          apiVersion: '2022-09-01'
          type: 'Microsoft.Resources/tags'
          name: 'default'
          properties: {
            tags: {
              tag1: fizz // <-- Not an error, but surprise! Its value is 'pop', not 'buzz'
            }
          }
        }
      ]
    }
  }
}

In that case, the user may be using a Bicep symbolic reference to save a few characters, but what the template will do (dereference the inner-scoped symbol) does not line up with the communicated intent of the syntax (i.e., to dereference the outer-scoped symbol), so I chose to flag that as an error, too, as this usage would almost certainly represent a logical error even if the compiled nested deployment would not raise a runtime error.

Microsoft Reviewers: Open in CodeFlow

[github-actions]

Test this change out locally with the following install scripts (Action run 6278976773)

VSCode

• Mac/Linux

  bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 6278976773

• Windows

  iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 6278976773"

Azure CLI

• Mac/Linux

  bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 6278976773

• Windows

  iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 6278976773"

[github-actions]

Test Results

     132 files  ±0       132 suites  ±0   3h 34m 28s ⏱️ + 4m 20s
10 553 tests +1  10 553 ✔️ +1  0 💤 ±0  0 ❌ ±0 
50 763 runs  +4  50 763 ✔️ +4  0 💤 ±0  0 ❌ ±0 

Results for commit 995d0fa. ± Comparison against base commit e964970.

♻️ This comment has been updated with latest results.

[majastrz]

[anthony-c-martin]

[nit] Shouldn't this be a linter rule rather than part of the emit limitation calculations?

[jeskew]

[nit] Shouldn't this be a linter rule rather than part of the emit limitation calculations?

I thought this fit better conceptually with other emit limitations, but moving this to a linter instead would give users who are impacted by the breaking change a way to disable the check if they want to.

[anthony-c-martin]

[nit] Shouldn't this be a linter rule rather than part of the emit limitation calculations?

I thought this fit better conceptually with other emit limitations, but moving this to a linter instead would give users who are impacted by the breaking change a way to disable the check if they want to.

Just realized I didn't explain my thinking, so wanted to note it down:

It's a bit of a grey area - my opinion is generally that the core compiler logic should not be resource-aware - that's more the job of the linter. We have chosen to not fully block Microsoft.Resources/deployments, but we also haven't special-cased it in any way, so although it is technically "our" resource type, I feel like we don't and shouldn't treat it as such in Bicep.

In general, I suspect that people are utilizing this functionality accidentally without realizing they should be instead using modules. I feel like we should strongly discourage unless we can see a compelling reason to use it instead of modules; it's full of footguns like this one.

[maskati]

FYI a use case of nested deployment template referencing outer scope is a native bicep implementation of #5805 (reply in thread). This is essentially a way of implementing role assignment extension deployments for generic parent resource types. Instead of loading an ARM JSON as in the linked comment, I define a Microsoft.Resources/deployments resource and parameterize this using outer scoped references to have the compiler generate ARM expressions for these within the template itself.

[anthony-c-martin]

@maskati - I believe there's a simpler solution to this today, as we now allow direct referencing of .json templates with the module syntax - see this comment: #5805 (comment)

[maskati]

Nice! I didn't know you could do that.

[StephenWeatherford]

@anthony-c-martin cc @puicchan I'll be doing a round of rule fixes/additions in the next few months. Do you think we should have a linter rule warning against Microsoft.Resources.deployments usage period?

[jeskew]

@StephenWeatherford we have one that was recently added: #11848