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

[Story] Provide a linter for Bicep #428

Closed
tyconsulting opened this issue Sep 2, 2020 · 10 comments · Fixed by #2341
Closed

[Story] Provide a linter for Bicep #428

tyconsulting opened this issue Sep 2, 2020 · 10 comments · Fixed by #2341
Assignees
Labels
enhancement New feature or request story: linter
Milestone

Comments

@tyconsulting
Copy link

It's probably too early to ask, but please provide a linter for Bicep (just like what ARM-TTK does for ARM Template) so it's easier to leverage Bicep in a CI/CD pipeline in the future.

And also, when the linter is available, please make it available in the GitHub Super Linter container image.

@tyconsulting tyconsulting added the enhancement New feature or request label Sep 2, 2020
@ghost ghost added the Needs: Triage 🔍 label Sep 2, 2020
@alex-frankel alex-frankel added this to the Committed Backlog milestone Oct 7, 2020
This was referenced Oct 8, 2020
@alex-frankel
Copy link
Collaborator

If/when we do a linter, we'd like to have certain things show up as you are authoring the bicep file in VS code, but there may also be certain things that would only be checked at build time as well. And either way, this should be able to be run adhoc in a pipeline.

@MarcusFelling
Copy link
Collaborator

MVP Scope

  • Refined set of arm-ttk rules (see below) with auto-fixes (VS Code, CLI)
  • Configurable On/Off and Severity through config file
  • Baked into Bicep core (for now), rules defined in code

Initial rules:

  1. Parameters must exist
  2. Declared parameters must be used
  3. Secure parameters can't have hardcoded default
  4. Environment URLs can't be hardcoded
  5. Location uses parameter
  6. Declared variables must be used
  7. Dynamic variable should not use concat - encourage interpolation instead of concat
  8. dependsOn best practices - Unnecessary dependsOn

Phase 2

  • Add new rules based on feedback
  • Rules language to add extensibility for things like ARMory
  • Additional VS Code features
  • Open questions: How will this play with public modules? How will this work for things that need auth (like template specs).

@jabbera
Copy link
Contributor

jabbera commented Mar 31, 2021

Question: Location uses parameter. Typically I use var location = resourceGroup().location. Is this a poor practice?

@MarcusFelling
Copy link
Collaborator

Question: Location uses parameter. Typically I use var location = resourceGroup().location. Is this a poor practice?

@jabbera Using the resourceGroup().location function is certainly an improvement from hardcoding the region name. However, you can go a step further by adding a location parameter that defaults to resourceGroup().location. This way users can use the default value, but also have the option to specify a different region if needed. The arm-ttk test case Location uses parameter describes this in more detail.

@MarcusFelling MarcusFelling changed the title Provide a linter for Bicep [Story] Provide a linter for Bicep May 7, 2021
@StephenWeatherford StephenWeatherford self-assigned this May 23, 2021
@johndowns
Copy link
Contributor

Is the linter likely to be available in the CLI? e.g. bicep lint main.bicep?

@alex-frankel
Copy link
Collaborator

The linter rules will be evaluated on a bicep build or deploy via Az PoSH or Az CLI. @johndowns - are you looking to run the linter rules separate from the core validation warnings/errors?

@johndowns
Copy link
Contributor

johndowns commented May 24, 2021

@alex-frankel Cool. I'm thinking about how to run this in a CI/CD pipeline. The bicep build exit code seems to be 0 even when there are linter warnings. Also, that emits an output file, which I don't need. I was thinking it'd be nice to have a bicep lint command that does nothing except lint, and which I can use to quickly check the exit code != 0.

I guess the workaround for now would be to update the bicepconfig.json file to set the level to Error for all rules?

@alex-frankel
Copy link
Collaborator

I guess the workaround for now would be to update the bicepconfig.json file to set the level to Error for all rules?

Yep, exactly. I wouldn't necessarily say it's a workaround, since we chose to make linter violations warnings. That being said I can see a bicep lint command still being useful if you don't want to/can't modify that file.

@johndowns
Copy link
Contributor

johndowns commented May 24, 2021

@alex-frankel I've created a feature request: #2811

@StephenWeatherford
Copy link
Contributor

That also goes to my thoughts that people will want to disable lint or config it via command-line args and not a config file. For instance, in the quick-starts pipeline. For now I can add a config on disk.

@ghost ghost locked as resolved and limited conversation to collaborators May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request story: linter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants