search

Azure / Enterprise-Scale

The Azure Landing Zones (Enterprise-Scale) architecture provides prescriptive guidance coupled with Azure best practices, and it follows design principles across the critical design areas for organizations to define their Azure architecture
https://aka.ms/alz
MIT License
1.72k stars 978 forks source link

fix: [[, double effect, empty default #1705

Closed SvenSowa closed 4 months ago

SvenSowa commented 4 months ago

Fixed double square brackets "[[parameters". Fixed double "effect, effect1", removed "effect1". Fixed "defaultValue": "" and removed it, since all parameters are really required.

Overview/Summary

The updated template can be deployed via the REST API without an error. Also, it will show the parameters correctly in the Azure portal.

This PR fixes/adds/changes/removes

  1. Fixes syntax error with "[["
  2. Removed "effect1" since it is defined twice as "effect" and "effect1"
  3. Removed defaultValue:"" since it leads to a faulty deployment, if one ignores to set all parameters, when assigning the initiative

Breaking Changes

  1. No assignment of initiative with default values possible. It would be a broken assignment anyhow.

Testing Evidence

When deployed with "[[parameters" syntax error, the REST API throws this error: url: https://learn.microsoft.com/en-us/rest/api/policy/policy-set-definitions/create-or-update?view=rest-policy-2023-04-01&tabs=HTTP&tryIt=true&source=docs#code-try-0 body: { "error": { "code": "UnusedPolicyParameters", "message": "The policy set '' has defined parameters 'azureFilePrivateDnsZoneId,azureAutomationWebhookPrivateDnsZoneId,azureAutomationDSCHybridPrivateDnsZoneId,azureCosmosSQLPrivateDnsZoneId,azureCosmosMongoPrivateDnsZoneId,azureIotCentralPrivateDnsZoneId,azureStorageTablePrivateDnsZoneId,azureStorageTableSecondaryPrivateDnsZoneId,azureSiteRecoveryBackupPrivateDnsZoneID,azureSiteRecoveryBlobPrivateDnsZoneID,azureSiteRecoveryQueuePrivateDnsZoneID' which are not used in referenced policy definitions. Please either remove these parameters from the definition or ensure that they are used." } }

When deployed with "effect" and "effect1", it will show double in the Azure portal: image

When deployed with "defaultValue": "", it will not show the parameters in the Azure portal, even though they are really required: image

The fixed version can be deployed trough the REST API, plus it will show the parameters correctly and the effect shows up only once: image

image

Azure Public

[Deploy To Azure](https://portal.azure.com/#blade/Microsoft_Azure_CreateUIDef/CustomDeploymentBlade/uri/https%3A%2F%2Fraw.githubusercontent.com%2F{YOUR GITHUB ORG/ACCOUNT HERE - Remove Curly Brackets Also}%2FEnterprise-Scale%2F{YOUR GITHUB BRANCH NAME HERE - Remove Curly Brackets Also}%2FeslzArm%2FeslzArm.json/uiFormDefinitionUri/https%3A%2F%2Fraw.githubusercontent.com%2F{YOUR GITHUB ORG/ACCOUNT HERE - Remove Curly Brackets Also}%2FEnterprise-Scale%2F{YOUR GITHUB BRANCH NAME HERE - Remove Curly Brackets Also}%2FeslzArm%2Feslz-portal.json)

Azure US Gov (Fairfax)

[Deploy To Azure](https://portal.azure.us/#blade/Microsoft_Azure_CreateUIDef/CustomDeploymentBlade/uri/https%3A%2F%2Fraw.githubusercontent.com%2F{YOUR GITHUB ORG/ACCOUNT HERE - Remove Curly Brackets Also}%2FEnterprise-Scale%2F{YOUR GITHUB BRANCH NAME HERE - Remove Curly Brackets Also}%2FeslzArm%2FeslzArm.json/uiFormDefinitionUri/https%3A%2F%2Fraw.githubusercontent.com%2F{YOUR GITHUB ORG/ACCOUNT HERE - Remove Curly Brackets Also}%2FEnterprise-Scale%2F{YOUR GITHUB BRANCH NAME HERE - Remove Curly Brackets Also}%2FeslzArm%2Ffairfaxeslz-portal.json)

As part of this Pull Request I have

Springstone commented 4 months ago

Hi @SvenSowa, many thanks for your submission. I delighted to see engagement from our community! Regrettably though, we cannot merge this due to several reason relating to how Azure Policy and ALZ (this repo) handles policy.

  1. Policies in this repo are not intended to be deployed as is. If you would like to deploy the policy, I would highly recommend looking it up on AzAdvertizer and copying the definition from there into a new policy, or use other tools like Enterprise Policy-as-Code (https://aka.ms/epac). Policies in this repo are authored to support aggregation into a single ARM template (so we deploy one template with all policies), and in order to do this, we need to escape template functions which include parameters hence the double [[. This is required for ALZ purposes, and you will see this in every policy/initiative in this repo.
  2. The Azure Policy engine is case sensitive, which is why we have two versions of the effect parameter, one with upper case DeployIfNotExists and one with lower d deployIfNotExists. These are not evaluated the same and is related to how the built-in policy defined the allowed effect parameter.
  3. For defaultValue="", this is needed, as when you deploy Private DNS Zones, you may not want to deploy all of them (it happens). We need to pass through a value in the event it's not specified, e.g. you only want private DNS for storage - then we need that default value for all the other options.

I hope this makes sense, and feel free to comment on this, I really don't want to discourage you from contributing! I'll close the PR for now, as we can't proceed.