Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Location Should Not Be Hardcoded - Allowed Values - Question #711

Open
drewkg opened this issue Oct 24, 2022 · 3 comments
Open

Location Should Not Be Hardcoded - Allowed Values - Question #711

drewkg opened this issue Oct 24, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@drewkg
Copy link

drewkg commented Oct 24, 2022

First off, this is not a bug more a question / guidance request.

I have a param for resource location, that due to restrictions in Azure needs to limit the possible inputs. If I follow ARM-TTK best practice I should set the defaultValue to resourceGroup().location or similar and then the template passes ARM. However this could result in an incorrect deployment. However if I set the defaultValue to one of the allowedValues ARM-TTK will fail this test.

I can disable the Location test, but then I don't have the protection it provides in the rest of the template.

So my question is should the Location test, take into account allowed values?

PS This might be done for corporate policy, not just Azure limitations.

"location": {
"type": "string",
"defaultValue": "uksouth",
"metadata": {
"description": "The location of the resources created, this is limited to only the locations that support linked Log Analytics and Automation Account."
},
"allowedValues": [
"eastus",
"westus2",
"canadacentral",
"australiasoutheast",
"southeastasia",
"centralindia",
"japaneast",
"uksouth",
"westeurope",
"usgovvirginia"
]
},

  [-] Location Should Not Be Hardcoded 119ms
    RuntimeException: The defaultValue of the location parameter in the main template must not be a specific location. 
                             The default value must be [resourceGroup().location], [deployment().location] or 'global'. It is "uksouth"
@ghost ghost added the Needs: triage 🔍 label Oct 24, 2022
@bmoore-msft
Copy link
Contributor

The test is trying to avoid using a hardcoded location for a defaultValue as it tends to collect things all in one place (i.e. folks don't change the default). The test doesn't look to see whether the defaultValue are valid since the deployment engine will do that.

If you have allowedValues, the only "fix" the TTK won't complain about is removing the defaultValue. If you use [resourceGroup().location] there are experiences that will kick that out (e.g. portal) unless you explicitly add resourceGroup().location to the list. And that won't give you the result you want. For for the TTK scenario you'd need to remove the defaultValue.

Thoughts?

@drewkg
Copy link
Author

drewkg commented Nov 14, 2022

There used to be a rule in the ARM linter that would notice when allowed values were used and notify that resourceGroup().location was not in the allowed value lists.

For myself, the few templates that require a location limitation I just disable the particular test and make sure that I do not accidentally hard code the location into any resource, it just feel inelegant to have to do this.

The only resources I am aware of for this limitation is when linking a Log Analytics Workspace with an Automation Account, so really an edge case.

I guess in an ideal world the test would take into account the allowed values, and allow a hardcoded default value if present.

@bmoore-msft
Copy link
Contributor

I guess in an ideal world the test would take into account the allowed values, and allow a hardcoded default value if present.

This is not a bad way to approach it - i.e. if there are allowed values, then we know we're in a "limited region" type scenario and can be more forgiving... could even check allowedValues for the defaultValue.

@bmoore-msft bmoore-msft added the enhancement New feature or request label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants