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

[Design Proposal] Modular Parameters #12019

Open
polatengin opened this issue Oct 2, 2023 · 21 comments · May be fixed by #13900
Open

[Design Proposal] Modular Parameters #12019

polatengin opened this issue Oct 2, 2023 · 21 comments · May be fixed by #13900
Assignees
Labels

Comments

@polatengin
Copy link
Member

polatengin commented Oct 2, 2023

Modular Parameters Design Proposal

As a Bicep developer, I want to be able to import parameters from other bicepparam files, so that I can reuse parameters across multiple deployments.

In most deployment scenarios, there are parameters that are common across multiple deployments. For example, a location parameter is often used in multiple deployments. It would be useful to be able to define the location parameter in a single file, and import it into other .bicepparam files.

Also it would be useful to be able to define parameters in environment specific files, and import them into the main.bicepparam file. For example, a location parameter can be defined in dev.bicepparam and prod.bicepparam files, and the main.bicepparam file can import the location parameter from the dev.bicepparam file when deploying to the dev environment, and import the location parameter from the prod.bicepparam file when deploying to the prod environment.

Goals

  • I can import parameter files into others with an import directive

Parent GitHub Issue: Bicep Parameters Upcoming Features #10777

Design Considerations

  • All parameters are exported by default

    No special syntax is needed to export parameters

  • Able to import .bicepparam files (more than one import is possible)

    This is to avoid having to define all parameters in a single file

  • No circular imports

    This is to avoid infinite loops

  • No duplicate parameters, name collision will result in an error

    This is to avoid ambiguity

  • No import all parameters from a file, only able to import specific parameters

    This is to avoid importing parameters that are not needed, to avoid name collisions, and avoid failing deployments because of future changes to the imported file

  • Able to import parameters from a file that is not in the same directory as the importing file

    This is to avoid having to define all parameters in a single directory, so parameters can be grouped by functionality and split into different directories

  • Able to import parameters from a file that is resolved using readEnvironmentVariable() function

    This is to allow importing parameters from environment specific files

Proposed Solution

Syntax

import { <param_name_1>, <param_name_2>, <param_name_3> as alias_name } from '<path_to_bicepparam_file>'

Scenario 1

Combine parameters from the main.bicepparam, and two different .bicepparam files;

// shared.bicepparam
param location = 'eastus'
param sqlPwd = 'p@55w0rd!'

// module_specific.bicepparam
param location = 'westeurope'
param storageName = 'importantstorage'

// main.bicepparam
using './main.bicep'
import { sqlPwd } from './shared.bicepparam'
import { storageName } from './module_specific.bicepparam'
param location = 'westus'
param name = 'bicep'

// compiled.json
{
  location: 'westus'
  sqlPwd: 'p@55w0rd!'
  storageName: 'importantstorage'
  name: 'bicep'
}

Scenario 2

main.bicepparam file imports parameters and alias one of the parameters, so it'll be renamed.

// shared.bicepparam
param appName = 'app_1'
param sqlPwd = 'p@55w0rd!'
param sqlServerName = 'sqlServer'

// main.bicepparam
import { appName, sqlPwd as sqlPassword } from './shared.bicepparam'
param name = 'bicep'

// compiled.json
{
  appName: 'app_1'
  sqlPassword: 'p@55w0rd!'
  name: 'bicep'
}

Scenario 3

Import parameters from a .bicepparam file, resolving the path of the .bicepparam using readEnvironmentVariable() function

// .env
env='dev'

// ./global/dev.bicepparam
param appName = 'app_1'
param sqlPwd = 'p@55w0rd!'

// main.bicepparam
import { appName, sqlPwd } from './global/${readEnvironmentVariable('env')}.bicepparam'

// compiled.json
{
  appName: 'app_1'
  sqlPwd: 'p@55w0rd!'
}

Scenario 4

Import parameters from multiuple .bicepparam file, resolving the path of the .bicepparam using readEnvironmentVariable() function

// .env
env='dev'

// ./global/dev.bicepparam
param appName = 'app_1'
param sqlPwd = 'p@55w0rd!'

// shared.bicepparam
param location = 'westus'
param name = 'bicep'

// main.dev.bicepparam
import { location, name } from './shared.bicepparam'
import { appName, sqlPwd } from './global.${readEnvironmentVariable('env', 'dev')}.bicepparam'

// compiled.json
{
  location: 'westus'
  name: 'bicep'
  appName: 'app_1'
  sqlPwd: 'p@55w0rd!'
}
@polatengin polatengin added the enhancement New feature or request label Oct 2, 2023
@polatengin polatengin self-assigned this Oct 2, 2023
@alex-frankel
Copy link
Collaborator

Will there by a specific syntax for importing all param values from a file?

@cwp-michaell
Copy link

I like these ideas
Perhaps something a little cleaner

param location = import(location, ./foo.bicepparam) // This imports a single value
import(./bar.bicepparam) // This imports all values

The env variable idea could be done by extending the current var usage to the above

var env = 'dev'

param location = import(location, ./foo${env}.bicepparam)
import(./bar${env}.bicepparam}) // This imports all values

I'm not sure the named import would be needed with the above approach as any imported param should just be able to be referenced from the .bicep file.

Alternatively perhaps an include in the .bicep file pointing to any required bicepparam files would be a better approach?

@tg123
Copy link

tg123 commented Oct 9, 2023

will this support shadow param?

for exmaple

default.bicep

param a int = 1
param b int = 2

eastus.bicep

import * from default.bicep

param a int = 3

eastus.json would be

{
a : 3
b : 2
}

@WhitWaldo
Copy link

I would like to see both a wildcard import to import all parameters from a given file in addition to selecting specific params.

How would name conflicts be handled between files? Personally, I'd prefer a preference towards specificity (e.g. import the specifically named import over a wildcard, but show a squiggly line and build warning indicating the conflict).

@glloyd2010f
Copy link

I would like to see both a wildcard import to import all parameters from a given file in addition to selecting specific params.

How would name conflicts be handled between files? Personally, I'd prefer a preference towards specificity (e.g. import the specifically named import over a wildcard, but show a squiggly line and build warning indicating the conflict).

Expanding on this, seen customers where they like the generate a parameter file for an entire environment which will support multiple deployments. Was wondering if we could then do selective filtering based on an initial import of parameters.

Rough code example of this idea

allParams = import { * } from './global/${readEnvironmentVariable('env')}.bicepparam
import { location, name } from allParams

@polatengin
Copy link
Member Author

Will there be a specific syntax for importing all param values from a file?

great question!

Importing everything from a separate file could be dangerous, since you have to provide only the parameters required in the deployment, and that separate file could be updated and have new parameters for totally different deployments.

This could make the deployments that have "everything" syntax fail.

We'd like to have a conservative approach here, to make sure if the deployment can be made with the current content of the files, it'll still be made even if you add new parameters into the other file.

But we're thinking to add some features into the VSCode Extension, so authors of bicepparam files will easily get the list of parameters that can be imported.

@polatengin
Copy link
Member Author

I like these ideas Perhaps something a little cleaner

param location = import(location, ./foo.bicepparam) // This imports a single value
import(./bar.bicepparam) // This imports all values

in different places in bicep, we already have import syntax. it'd be better to reuse that syntax, so developers (especially the ones who are learning bicep) don't have to learn and remember two similar import syntax.

@polatengin
Copy link
Member Author

will this support shadow param?

for exmaple

default.bicep

param a int = 1
param b int = 2

eastus.bicep

import * from default.bicep

param a int = 3

eastus.json would be

{
a : 3
b : 2
}

we intentionally won't support everything syntax, the main reason is, eastus.bicepparam in your example can get new parameter definitions over time, to support "other" deployments, but it could fail the deployments, that uses eastus.bicepparam

I also mentioned this issue here; #12019 (comment)

@polatengin
Copy link
Member Author

I would like to see both a wildcard import to import all parameters from a given file in addition to selecting specific params.

Wildcard imports could be dangerous and can make the deployments that have no change, fail. (as mentioned here; #12019 (comment))

I'd like to understand better, what's the use-case of a wildcard import, if there is an editor support and you can easily import required parameters?

I see wildcard import syntax is to make importing several parameters easily. is it still the case, if the editor helps you to import required parameters easily?

@polatengin
Copy link
Member Author

How would name conflicts be handled between files? Personally, I'd prefer a preference towards specificity (e.g. import the specifically named import over a wildcard, but show a squiggly line and build warning indicating the conflict).

bicep (vscode and vs extensions, language server, cli, etc.) parse bicep and bicepparam files not order-dependent ( at least, so far 😄 )

and we're not going to support wildcard import syntax.

so, developers should import parameters by their names, and if there is a conflict, developer should resolve the conflict.

alias syntax ( import { appName as name } from 'foo.bicepparam' ) can be used to resolve this kind of issues

@polatengin
Copy link
Member Author

Expanding on this, seen customers where they like the generate a parameter file for an entire environment which will support multiple deployments. Was wondering if we could then do selective filtering based on an initial import of parameters.

Rough code example of this idea

allParams = import { * } from './global/${readEnvironmentVariable('env')}.bicepparam
import { location, name } from allParams

I think it's a very interesting idea. Although how much I like the idea, I don't see better developer experience between these two code blocks;

allParams = import { * } from './global/${readEnvironmentVariable('env')}.bicepparam
import { location, name } from allParams
import { location, name } from './global/${readEnvironmentVariable('env')}.bicepparam

in fact, the first proposal seems more complex, and, longer to type 😄

two import syntax does two different things (import but don't add parameters to context, and import and add parameters to context) could be confusing, too.

@WhitWaldo
Copy link

WhitWaldo commented Oct 26, 2023

I'd like to understand better, what's the use-case of a wildcard import, if there is an editor support and you can easily import required parameters?

  1. Bicep is already subject to a wall of parameters with their comments and decorators, and large multi-line object declarations due to its aversion to commas. I'd really rather avoid further polluting the top of my file with additional lengthy imports that would effectively do the same thing as a wildcard import.

  2. User-defined data types support wildcard imports and a mix/match style. It doesn't feel like a great future developer experience to have different (but near-identical-looking) import syntaxes based on what's being imported.

More to the point, developers have been spoiled and are fairly used to having the option of wildcard imports. They're extremely common in other languages. I've got to imagine there'd be a sizable number of issues over time of new Bicep developers inquiring as to what they're doing wrong in that their wildcard import statements don't work for parameters, but they do for user-defined types.

  1. If I'm dropping all my parameters in some file to avoid some typing, I'd like to save myself additional time and just insert an import * from '../my.params at the top of all of my files. It seems like an unnecessarily ceremonial step to narrow the imports from all to a select few.

Even with some editor support, it's a poor experience in Resharper already when I drop in a wall of new types into a class and inevitably have to go one by one through the new types to have it locate and add the correct using statement for each. Given the opportunity, I'd rather avoid a repeat of that poor UX here as well.

we intentionally won't support everything syntax, the main reason is, eastus.bicepparam in your example can get new parameter definitions over time, to support "other" deployments, but it could fail the deployments, that uses eastus.bicepparam

I've been an advocate for constant-folding + reliable what-if local simulated deployments and this just feels like another excellent use case. Even today absent parameter files, a change to a parameter can break a deployment and there's no way to know about it absent repeated real deployments to Azure in temporary resource groups (which costs real money).

I personally wouldn't rely on re-using parameter files in the short term because the tooling simply isn't here to fully validate complex deployments with anywhere near the accuracy of a real deployment. I mean, to your point, the parameters you started with in eastus.bicepparam could easily change over time simply because someone forgot they were already used elsewhere... but wildcard or specific import statements don't help there either. Your objection is little different from user types - those might change over time and have non-default parameters added which would break existing uses of them.. and as I mentioned above, they support wildcard imports.

@dstarkowski
Copy link

We'd like to have a conservative approach here

That seems more opinionated than conservative. Why go out of the way to deliberately take away the choice from developers and make the language less consistent?

Without wildcard, adding a shared parameter results in modifying all parameter files that use it. Whether I have editor support or not, every file needs to be modified, every file is changed in git, and every file needs to be reviewed. And probably doing the change without editor help will be quicker.

@brwilkinson
Copy link
Collaborator

Just catching up on the community calls. Are you still looking for feedback?

Is this work already underway?

@evkaky
Copy link

evkaky commented Mar 14, 2024

Would love to see this feature. There is so much copy and paste params across my .bicepparam files

@mrkesu
Copy link

mrkesu commented Apr 15, 2024

Is this far from finishing? This would finally let me move away from having to use CUE lang for parameters...

@polatengin polatengin linked a pull request Apr 18, 2024 that will close this issue
3 tasks
@polatengin polatengin linked a pull request Apr 18, 2024 that will close this issue
3 tasks
@alex-frankel
Copy link
Collaborator

I believe we are getting close to getting this done and released as an experimental feature. @stephaniezyen / @polatengin can provide a more precise ETA.

@polatengin
Copy link
Member Author

we have a draft PR is up (#13900)

our plan is to make it to the next release, behind a feature flag.

@ChristopherGLewis
Copy link
Contributor

ChristopherGLewis commented May 2, 2024

Some comments here: #13900 (comment)

@ryancolley
Copy link

Are we any closer to this being merged in yet?
I have been eagerly watching the releases and its not appeared yet :(

@stephaniezyen
Copy link
Contributor

@ryancolley Unfortunately, we've had a longer delay than anticipated due to design decisions and changes. However, we're hoping to have it out as an experimental feature by the end of this month (hopefully)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.