bicep icon indicating copy to clipboard operation
bicep copied to clipboard

`Extendable Param Files` feature

Open polatengin opened this issue 1 year ago • 8 comments

With this PR

  • we introduce a new keyword extends
  • using none is allowed

extends keyword make bicepparam file inherits from another bicepparam

using none is valid only on shared bicepparam files and it turns off the parameter validation process on the file

How does it work

Files:

// shared.bicepparam
using none
param location = 'westus'
param name = 'default_name'
// main.bicepparam
using './main.bicep'
extends 'shared.bicepparam'
param vm_sku = 'Standard_D4_v4'

Compiled output:

{
  "location": "westus",
  "name": "default_name",
  "vm_sku": "Standard_D4_v4"
}

Fixes #12019

Contributing a feature

  • [x] I have opened a new issue for the proposal, or commented on an existing one, and ensured that the Bicep maintainers are good with the design of the feature being implemented
  • [x] I have included "Fixes #{issue_number}" in the PR description, so GitHub can link to the issue and close it when the PR is merged
  • [x] I have appropriate test coverage of my new feature
Microsoft Reviewers: Open in CodeFlow

polatengin avatar Apr 18 '24 22:04 polatengin

Test this change out locally with the following install scripts (Action run 9843636081)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 9843636081
    
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 9843636081"
    
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 9843636081
    
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 9843636081"
    

github-actions[bot] avatar Apr 18 '24 22:04 github-actions[bot]

Test Results

    66 files   -     33      66 suites   - 33   23m 48s :stopwatch: - 7m 57s 10 897 tests  -      6  10 895 :white_check_mark:  -      7  1 :zzz: ±0  1 :x: +1  25 690 runs   - 12 799  25 686 :white_check_mark:  - 12 800  2 :zzz:  - 1  2 :x: +2 

For more details on these failures, see this check.

Results for commit 2b30da25. ± Comparison against base commit a8eff9c8.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 18 '24 22:04 github-actions[bot]

What happens when main.bicepparam contains duplicate params as the shared param file?

ngetchell-pi avatar Apr 22 '24 13:04 ngetchell-pi

A couple of comments, some which should be addressed now and something in the future.

Understanding the override characteristics would be important -

  1. does the "primary" value override an extend or does the extend override the primary?
  2. Does a command line parameter override merged parameters?

Could we possibly get a decorator such as @MergeCharacteristcs with values:

  1. NeverOverride - this parameter will never be overridden with either an extend or CLI param
  2. Override - this parameter will be overridden by the extend (useful for arrays/objects)
  3. MergeObject - union arrays/objects rather than overriding. Allows us to define tags at a hierarchy level and build up tags via multiple extends

Can we support chained extends?
Given the tree:

C:.
+---dev
|   |   env.bicepparam
|   |
|   +---eastus
|   |       region.bicepparam
|   |
|   \---westus
|           region.bicepparam
|
\---prod
    |   env.bicepparam
    |
    +---eastus
    |       region.bicepparam
    |
    \---westus
            region.bicepparam

It would be great if our main.bicepparam would extend region.bicepparam and region extent env.bicepparm. That would give us a nice changed hierarchy.

Finally, can the extend use paths that are dynamic?

Given the above hierarchy, when I deploy an object using a main.bicepparam, I'd like to have the main have the following

main.bicepparam

using './main.bicep'
extends '../${readEnvironmentVariable('env')}/${readEnvironmentVariable('region')}/region.bicepparam'
param vm_sku = 'Standard_D4_v4'

region.bicepparam

using null
extends '../${readEnvironmentVariable('env')}/env.bicepparam'
param region = 'eastus'

env.bicepparam

using null
param env = 'prod'

ChristopherGLewis avatar May 02 '24 16:05 ChristopherGLewis

I find the proposed design very limited. As far as I understood from the meeting the following things are not possible:

  • Intelisense for shared bicep parameters files
  • multiple extends statements
  • ability to nest shared bicep parameters files
  • ability to merge (union) parameters of type object and array. Merge tags scenarios
  • support for variables

I think better approach would be to have something like:

// shared.bicepparam
using 'shared'
using 'main1.bicep'
using 'main2.bicep'

In this scenario you are clearly defining that this is shared bicep parameters file. Also additionally you provide two (or more if you want, these are optional statements) bicep files to serve as intelisense for the shared file. In this scenario when you define that this is shared bicep parameters files any errors related to required parameters are turned off. In VSC you can still see from intelisense if specific parameter is required or not. You can also see the description for those parameters. In case main1.bicep and main2.bicep contains the same parameter let's say location and that contains two different descriptions upon hovering the parameter when added you can see the description for both. In summary you still get intelisense and other information like description, allowed values, etc, but without the errors you would get in regular bicep parameters files.

With that said I would like to see support for the rest of the missing features as well:

  • multiple extends statements
  • ability to nest shared bicep parameters files
  • ability to merge (union) parameters of type object and array. Merge tags scenarios
  • support for variables

Would also be nice that if you can declare which parameters from extends statement you would like to add instead of adding everything. Similar to how it is in import statements. With the support for variables would be good to see their value when you use them on other parameters when you hover over the variable.

It would really make sense if all these features are released at once otherwise it is best to be preview feature until every feature is added.

slavizh avatar May 03 '24 08:05 slavizh

I dont like the idea of typing "using null" as it seems a little clunky. I would rather it be some other form of MS decorator or keyword such as "using sharedbicepparam" which can imply that the bicepparam file can be extended to other files.

One other point with this, if this gets implemented, could this lead to a future development whereby we can use the "extends" command direct from a main.bicep file rather than a bicepparam file? This could lead to the possibility of having one global variables file whereby individual main files could 'import' selected variables from the sharedbicepparam file when needed. This could also be through the form of a function, like loadJsonContext(), but maybe something like loadBicepparamFile() ?

mmassey1993 avatar May 03 '24 09:05 mmassey1993

This pr doesn't explain nor solve how we can decouple the bicepparam files between modules and main bicep files. The biggest problem right now is you need to define all the parameters in the main bicep files. I have an extended explanation here on why this is an issue and how i think this could be tackled: https://github.com/Azure/bicep/issues/12200

A main bicep file should be rather clean of everything. If we have a look at where Microsoft is heading with the bicep verified modules, i believe Microsoft needs an implementation where they can host modules with their own bicep param files as a blueprint. Only the last part is now solved with the extend, where you can override the definition of the bicep files.

mennolaan avatar May 08 '24 07:05 mennolaan

Dotnet Test Results

    72 files   -     36      72 suites   - 36   23m 12s :stopwatch: - 9m 50s 10 949 tests  -      3  10 949 :white_check_mark:  -      3  0 :zzz: ±0  0 :x: ±0  25 794 runs   - 12 842  25 794 :white_check_mark:  - 12 842  0 :zzz: ±0  0 :x: ±0 

Results for commit 0038c1fc. ± Comparison against base commit 9864b148.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Jun 09 '24 20:06 github-actions[bot]

I have watched the last meeting but did not see any of the feedback given before implemented. In fact it seems that everything was the same and the only small change is that using is having none for value instead of null. That is only cosmetic change and it is irrelevant for the functionality. Kind of disappointed due to that.

slavizh avatar Jul 05 '24 05:07 slavizh

@polatengin : Please note that using parameters of type Object in shared.bicepparam file is currently throwing error in main param files. I think it is easy for you to reproduce

m-soltani avatar Sep 04 '24 15:09 m-soltani