Add meta keyword

[SimonWahlin]

Disclaimer: This is my first time doing anything serious in C# so don't hesitate to critique my code :)

I think this is now working as expected.

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

Description

I have added the keyword meta to be able to add metadata to ARM templates. Both to create them in bicep but also to not loose any metadata defined in an ARM template when decompiling.

Metadata for _generator will be ignored on decompile since it is not relevant and would cause a conflict.

Example bicep:

meta metaSimon = 'metaSimonvalue'
meta bicepRegistry = {
  name: 'sampleModule'
  description: 'Sample module description'
  owner: 'Sampleusername'
  version: '1.0.0'
}

This will compile into:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.4.1386.19018",
      "templateHash": "18051444936684385945"
    },
    "metaSimon": "metaSimonvalue",
    "bicepRegistry": {
      "name": "sampleModule",
      "description": "Sample module description",
      "owner": "Sampleusername",
      "version": "1.0.0"
    }
  },
  "resources": []
}

Fixes #6126

[SimonWahlin]

I've been troubleshooting this for a while now and it seems to be working using the CLI, but with the vscode extension, when I hover I get a message saying: templateMetadata myMetadata: error instead of templateMetadata myMetadata: 'myValue'

My bicep contains: templateMetadata myMetadata = 'myValue'

What am I doing wrong?

[SimonWahlin]

I fully respect if this is something you don't want, but I would highly appreciate some kind of feedback or response, even if it is just a reject.

Ping @shenglol @alex-frankel @anthony-c-martin

[alex-frankel]

Apologies @simonwahlin, I'm going to cue this up for our internal weekly discussions which will take place tomorrow and we will get you an update.

[SimonWahlin]

Thanks a lot!

[SimonWahlin]

@alex-frankel I think this is working now. I do however need some guidance on how/where to add check for reserved metadata (starting with _)

[alex-frankel]

Running the CI now, but I might recommend holding off on making changes until all the discussions are finalized. I want to avoid you having to do any re-work if possible.

[SimonWahlin]

Thanks!

[SimonWahlin]

@alex-frankel I think this is working now. I do however need some guidance on how/where to add check for reserved metadata (starting with _)

@alex-frankel do you have anyone that can give me some advice here?

[anthony-c-martin]

@alex-frankel I think this is working now. I do however need some guidance on how/where to add check for reserved metadata (starting with _)

@SimonWahlin - I'd think either in the Binder: https://github.com/Azure/bicep/blob/72764e3437816e51d96c33f0c02aa9210783e5f0/src/Bicep.Core/Semantics/FileSymbol.cs#L93 Or in the EmitLimitationCalculator: https://github.com/Azure/bicep/blob/72764e3437816e51d96c33f0c02aa9210783e5f0/src/Bicep.Core/Emit/EmitLimitationCalculator.cs#L34-L41

[SimonWahlin]

@anthony-c-martin not sure if this was how you expected me to do it, but it seems to be working. Thanks for the help!

[SimonWahlin]

I've run the tests several times on my machine but I cannot reproduce the failing result. @anthony-c-martin @alex-frankel would you mind helping me understand what to do next?

[alex-frankel]

If tests are passing locally, it may be an intermittent issue, so I am re-running the tests.

[SimonWahlin]

Thanks @alex-frankel!

This time it is failing on the step Build Bicep In Visual Studio which ran successful on the last try. I'm not sure I understand what actually fails, can you please help me?

[shenglol]

Thanks @alex-frankel!

This time it is failing on the step Build Bicep In Visual Studio which ran successful on the last try. I'm not sure I understand what actually fails, can you please help me?

You may ignore that check since it's not required.

[shenglol]

With the current implementation, metadata accepts anything, including resource references, variables or parameters, for example: However, it probably doesn't make sense to allow metadata to contain runtime values / deploy-time constants, since they'll get ignored by the ARM deployments engine. I think we might want to add a validation to ensure that metadata only contains literal values (a.k.a. compile-time constants), which can be done by calling TypeValidator.GetCompileTimeConstantViolation.

[shenglol]

We'll need to update the error message of BCP007 to include metadata declaration:

[SimonWahlin]

@anthony-c-martin I think I got all things covered now. I ran the test workflow in my fork, results here: https://github.com/SimonWahlin/bicep/runs/8043631142

[majastrz]

There's something wrong with binding. The below doesn't compile:

metadata test = 2
param test string
output test string = 's'
var foo = test

Metadata should behave like outputs where they do not occupy any space in the global namespace. However, it appears that the VariableAccessSyntax is bound to the metadata rather than the parameter (see the highlight in the screenshot). If I reorder the lines so the parameter is first, the file compiles just fine.

[SimonWahlin]

There's something wrong with binding. The below doesn't compile:

metadata test = 2
param test string
output test string = 's'
var foo = test

Metadata should behave like outputs where they do not occupy any space in the global namespace. However, it appears that the VariableAccessSyntax is bound to the metadata rather than the parameter (see the highlight in the screenshot). If I reorder the lines so the parameter is first, the file compiles just fine.

That is interesting! I tried removing metadata and putting output on top of the file, this gave me the error Error BCP058: The name "test" is an output. Outputs cannot be referenced in expressions.

It seems to me that the problem is with GetUniqueDeclarations in Binder.cs, which will only return the first declaration of each identifier which makes us bind to that.

I tried adding a filter in the method GetUniqueDeclarations in Binder.cs to only return declarations that can be referenced (anything but metadata and output) and it seems to do the trick. However, that breaks BCP058 "The name \"{name}\" is an output. Outputs cannot be referenced in expressions." that we get when trying to reference output or metadata since those identifiers are no longer included. Instead we get BCP057 "The name \"{name}\" does not exist in the current context." which does not seem like the best solution to me.

Then I tried to sort the outermostDeclarations by putting anything that is not metadata or output first. This seems to successfully pass the test baseline and will only prioritize by type when we have more than one declaration with the same identifier.

I am still not fully comfortable with the codebase (or C# tbh) so please let me know if you think my solution might break something else or if there is a better solution.

[majastrz]

There's something wrong with binding. The below doesn't compile:

metadata test = 2
param test string
output test string = 's'
var foo = test

Metadata should behave like outputs where they do not occupy any space in the global namespace. However, it appears that the `VariableAccessSyntax` is bound to the metadata rather than the parameter (see the highlight in the screenshot). If I reorder the lines so the parameter is first, the file compiles just fine.

That is interesting! I tried removing metadata and putting output on top of the file, this gave me the error Error BCP058: The name "test" is an output. Outputs cannot be referenced in expressions.

It seems to me that the problem is with GetUniqueDeclarations in Binder.cs, which will only return the first declaration of each identifier which makes us bind to that.

I tried adding a filter in the method GetUniqueDeclarations in Binder.cs to only return declarations that can be referenced (anything but metadata and output) and it seems to do the trick. However, that breaks BCP058 "The name \"{name}\" is an output. Outputs cannot be referenced in expressions." that we get when trying to reference output or metadata since those identifiers are no longer included. Instead we get BCP057 "The name \"{name}\" does not exist in the current context." which does not seem like the best solution to me.

Then I tried to sort the outermostDeclarations by putting anything that is not metadata or output first. This seems to successfully pass the test baseline and will only prioritize by type when we have more than one declaration with the same identifier.

I am still not fully comfortable with the codebase (or C# tbh) so please let me know if you think my solution might break something else or if there is a better solution.

Yeah, it definitely works better. I also realized that the issue was present with outputs as well, so it wasn't specific to just metadata. But you fixed it 🙂

[SimonWahlin]

Adding | MetadataDecorator to FunctionFlags.AnyDecorator breaks the test ParameterDecorator_MissingDeclaration_ExpectedParameterDeclaration

The test builds a bicep containing only @secure() and expects the output Error BCP147: Expected a parameter declaration after the decorator.

As soon as I modify this line the output is instead:

Error BCP132: Expected a declaration after the decorator.
Error BCP126: Function "secure" cannot be used as a variable decorator.

That is also why the baseline diagnostic files has some weird output like @majastrz commented on above.

I can't seem to figure out where this breaks, any pointers appreciated. Ping @anthony-c-martin @shenglol

Edit: I reposted this here after realizing my comment on the code itself was not very visible.

[majastrz]

I looked into it a little bit. In main it's going through TypeAssignmentVisitor.VisitMissingDeclarationSyntax and finding a binding for the decorator function. In your branch it's not finding a binding for the decorator function for some reason. I'll dig more tomorrow evening...

[anthony-c-martin]

@SimonWahlin I believe you just need to add your new FunctionFlags.MetadataDecorator here.

MissingDeclarationSyntax is used when you have a decorator with no declaration statement underneath.

[SimonWahlin]

Thank you @anthony-c-martin ! I think that did the trick! I'll run the tests and push a new commit

[asensionacher]

I'm having problems when adding metadata description on module's top level and also using @description decorator on parameters

// =========== self-hosted-devops-agent-extension-deploy.bicep ===========
targetScope = 'resourceGroup'

metadata name = 'self-hosted-devops-agent-extension-deploy.bicep'
metadata description = 'Deploys a self hosted extension on a Windows Virtual Machine'
// Parameters

@description('Name of Virtual Machine')
param vmName string

Any idea? I troubleshoot this by using @metadata decorator, but I think would not be the best approach.

[SimonWahlin]

Please post this kind of questions as a new issue or as a discussion in the future, there is a high risk of it being missed when written as a comment to an old and closed pullrequest.

This is a problem with identifier namespaces, if you have a metadata entry that is called description, it collides with the name of the function/decorator description. You can solve this by either calling your metadata entry something else or using the full name of sys.description on your decorator.

Here are two possible sollutions:

metadata templateDescription = 'Deploys a thing'

@description('Name of VM')
param vmName string

metadata description = 'Deploys a thing'

@sys.description('Name of VM')
param vmName string