Azure / deployment-stacks

Contains Deployment Stacks CLI scripts and releases
MIT License
87 stars 7 forks source link

SubscriptionDeploymentStack does not verify template schema #33

Open miqm opened 2 years ago

miqm commented 2 years ago

Describe the bug I took a resource group deployment and used SubscriptionDeploymentStack command to deploy. It was allowed but I think some basic verification should be done to avoid misuse.

To Reproduce Steps to reproduce the behavior:

  1. Take a RG template
  2. Create a deployment stack on subscription level.

Expected behavior Creating deployment stack should be blocked.

bmoore-msft commented 2 years ago

The ARM deployment engine (which stacks uses) doesn't do any schema validation... that's mostly historical but there are scenarios where a template may be valid at multiple scopes, so you'd have to change the source to use it at other scopes. It might be interesting to poke what kind of validation could reasonably be done given the schemas are never 100% complete, could be missing, etc.

What are your thoughts on the types of validation expected with having a given schema?

slavizh commented 2 years ago

@bmoore-msft don't do it. I have templates that deploy perfectly fine at Sub and MG scope even though the schema is only for sub.

bmoore-msft commented 2 years ago

;) - good feedback. What's in the templates? Policies? Roles?

miqm commented 2 years ago

perhaps just a warning? to validate $schema property as we do in bicep

bmoore-msft commented 2 years ago

Clients could certainly warn - the API doesn't have the capability to warn, at least not in a standard way in ARM.

Where do you see the warning in bicep? I just tried crossing deployment & targetScope in bicep and did not get a warning...

miqm commented 2 years ago

in bicep when you import template as a module schema is used to determine the target scope type.

bmoore-msft commented 2 years ago

Ah got it... so in VS Code?

slavizh commented 2 years ago

@bmoore-msft yes, we have solutions policies, custom rbac roles and role assignments that work perfectly fine no matter the scope. Overall the biggest problem was that policy names on MG scope are 24 characters max compared to Sub which is higher but that is RP problem not ARM.

slavizh commented 2 years ago

Bicep does not care about the target deployment as well. target is only useful for VSC. Basically if ARM does not care why Bicep or Deployment Stacks should.

miqm commented 2 years ago

in deployments it doesn't care (as this is CLI/pwsh job, not bicep), but when you include ARM template as a module (nested deployment) it does, not only in VSCode but also during build.

alex-frankel commented 2 years ago

Az CLI verifies that the deployment target matches the targetScope property. If you try to deploy a bicep file with targetScope = 'subscription' and then run az deployment group create ..., we will emit an error.

IMO, it is the right thing to do to be strict here, but in order to do this properly, we need to add the ability to set the targetScope to multiple scopes. i.e. `targetScope = [ 'resourceGroup', 'subscription', 'managementGroup']. I might recommend not doing any additional verification in stacks until we have that sorted out.