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

File reference for "allowed" decorator #10490

Closed
D-Mellenthin opened this issue Apr 21, 2023 · 4 comments
Closed

File reference for "allowed" decorator #10490

D-Mellenthin opened this issue Apr 21, 2023 · 4 comments
Labels
enhancement New feature or request Needs: Author Feedback Awaiting feedback from the author of the issue Needs: Triage 🔍

Comments

@D-Mellenthin
Copy link

Is your feature request related to a problem? Please describe.

I'm working in a team that sets up the core infrastructure for a bigger project in Azure and also provides Bicep modules for several teams of developers to include in the deployment of their respective services in the project. In nearly all our modules, we have a few parameters like the location (everyone can relate to that) and the domain which have a defined set of allowed values.

Currently, the allowed decorator only allows a static array of values directly at the decorator position. These arrays are duplicated (big code smell) in every module, because they are part of the name of every Azure resource that's deployed in our environment.

When we add a new domain or a location where the services can be deployed, we have to update every single file and every single repository for the services to support the new value. This a pain and a chore that could be avoided with a single point of truth. It's the same for several other parameters who have allowed values which can change or extend over time, even though they are used less often, like allowed SKUs etc.

I searched if this is already addressed somewhere, but didn't find this exact same issue.

This is related, but not identical to #2481 and #8448 . It's very close to #10121 , but specifically for the allowed decorator instead of variables/constants in general. This here should also be far easier to solve, but I might overlook something.

Describe the solution you'd like
A solution for this would be a centralized file where the allowed parameter values are defined and which can be referenced in the allowed decorator. When a new value is allowed, during a deployment, the ARM checks the allowed values against the file, which can either be available at a local path or remotely.

The structure or file type of the value definition is up to discussion, I don't mind if it's YAML, JSON or Bicep syntax or something completely different. Here are two suggestions how to use a possible bicep file solution:

@allowed("./locationValues.bicep")
param location

or as a remote file in ACR, where always the latest version is used, if not defined otherwise:

@allowed("br:<acrname>.azurecr.io/bicep/values/locationValues")
param location

But maybe JSON would be easier to distinguish from module files or invent a new Bicep syntax for it. In the background, the functionality of #471 could be used to load the files.
The syntax switch in the allowed decorator parenthesis to load the files seems to be simple since it's not an array anymore (no [ ] present), and afaik an array is currently the only allowed syntax inside the parenthesis.

@D-Mellenthin D-Mellenthin added the enhancement New feature or request label Apr 21, 2023
@ghost ghost added the Needs: Triage 🔍 label Apr 21, 2023
@jeskew
Copy link
Contributor

jeskew commented Apr 21, 2023

Would a solution that uses Bicep type syntax be acceptable?

For example, let's say you had the following locationName.bicep file:

type locationName = 'eastus' | 'eastus2' | 'westus' | 'westus2'

And you then used the proposed syntax from #10121 in a template:

import {locationName} from 'locationName.bicep'

param location locationName

When deployed, that template would validate that the location parameter was one of the values specified for locationName, which would have the effect I think you're asking for.

@D-Mellenthin
Copy link
Author

I think that would be a feasible solution, and that also makes the parameter typed, which is good and makes it clear imho.
The syntax is not the most readable/common, if we have long lists of allowed values, but I think it's fine.

@jeskew
Copy link
Contributor

jeskew commented Apr 24, 2023

You can split the union over multiple lines if that helps with readability:

type location = 'eastus' |
    'westus' |
    'westus2'

@jeskew
Copy link
Contributor

jeskew commented Apr 26, 2023

Tracking this request in #10121 since type syntax is an acceptable substitute.

@jeskew jeskew closed this as completed Apr 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 27, 2023
@StephenWeatherford StephenWeatherford added Needs: Author Feedback Awaiting feedback from the author of the issue and removed awaiting response labels Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request Needs: Author Feedback Awaiting feedback from the author of the issue Needs: Triage 🔍
Projects
Archived in project
Development

No branches or pull requests

3 participants