[Bicep CLI] Linting should provide structured output

rchaganti commented 3 years ago

Is your feature request related to a problem? Please describe. In a CI/CD pipeline, when we want to use the linter, we invoke the build command. If the build fails because of linter error, the pipeline will report it as a failure. However, the output from the build process is not actionable as it is plain-text. A more structured output from the linter will be helpful.

Describe the solution you'd like A command-line option to retrieve linter output in a structured format will be helpful. For example, bicep build main.bicep --o json or bicep lint main.bicep --o json

nicktolhurst commented 3 years ago

Is this up for grabs? I don't mind taking a look at this.

bicep lint main.bicep --output json seems like the sensible option here.

majastrz commented 3 years ago

I think we need to decide first whether we reuse an existing result format (perhaps something from one of the more popular test frameworks?) or invent our own.

There may be some benefit to reusing an existing format (as long as it fits well) because various CI providers might have good support for them. (For example test result parsers in GH actions or the tasks to upload test results in Azure DevOps.)

nicktolhurst commented 3 years ago

I think the eslint examples are worth consideration.

Relates to #3118

nicktolhurst commented 3 years ago

I had a quick look at what it would look like adding a new command like bicep lint to the CLI. I think, before adding one, it would be best to refactor Bicep.Cli.Program into something a bit more structured.

I've put together a proposed PR in my fork, over at https://github.com/nicktolhurst/bicep/pull/1 - might be worth having a look?

alex-frankel commented 3 years ago

@nicktolhurst - any chance you can create an issue in this repo describing the refactor in more detail? Include a link the the PR in your fork as well.

nicktolhurst commented 3 years ago

I foresee the most useful output types to be:

XML (one of JUnit, NUnit, VSTest, XUnit, CUnit)
JSON (this could be a custom format that matches the standard XML formats.
VisualStudio (the current output format)

XML (JUnit)

<?xml version="1.0" encoding="utf-8"?>
<testsuites>
    <testsuite package="bicep.cli.lint" time="0" result="failed" tests="1" errors="1" name="">
        <testcase time="0" name="BCP091" classname="c:\\temp\\main.bicep">
            <failure message="An error occurred reading file. Could not find a part of the path &apos;c:\\temp\\modules\\managementGroup.bicep&apos;">
                <![CDATA[line 8, col 44, Error - An error occurred reading file. Could not find a part of the path &apos;c:\\temp\\modules\\managementGroup.bicep&apos;. (BCP091)]]>
            </failure>
        </testcase>
    </testsuite>
</testsuites>

JSON (Custom/TBD)

{
    "summary": {
        "status": "failed",
        "errors": 1,
        "warnings": 0,
        "total": 1
    },
    "files": [
        {
            "path": "c:\\temp\\main.bicep",
            "lints": [
                {
                    "position": {
                        "line": 2,
                        "col": 44
                    },
                    "level": "Error",
                    "code": "BCP091",
                    "message": "An error occurred reading file. Could not find a part of the path 'c:\\temp\\modules\\managementGroup.bicep'."
                }
            ]
        }
    ]
}

VisualStudio/MSBuild (Current)

c:\temp\main.bicep(3,44) : Error BCP091: An error occurred reading file. Could not find a part of the path 'c:\temp\modules\managementGroup.bicep'.

The tool format could simply be

Usage:
  bicep lint [options] <file>
    Lints a .bicep file

    Arguments:
      <file>        The input file.

    Options:
      --outdir <dir>    Saves the output at the specified directory.
      --outfile <file>  Saves the output as the specified file path.
      --stdout          Prints the output to stdout.
      --format <json|jlint|visualstudio> Specifies the output format. default: visualstudio

    Examples:
      bicep lint file.bicep --format json
      bicep lint file.bicep --stdout --format json
      bicep lint file.bicep --outdir dir1 --format.json
      bicep lint file.bicep --outfile file.json --format.json

lr90 commented 2 years ago

As our Bicep deployment grow I'm bringing this back to the top in the hope we can switch to using the Bicep linter in our ADO pipeline capturing the results in XML/NUnit format as an alternative to ARM-TTK

ReneRebsdorf commented 2 years ago

Just wanted to weigh in after a conversation with @alex-frankel

I agree on the formats described by @nicktolhurst Our use case sounds similar to @lr90

We currently download the latest release from arm-ttk, and import the module. perform the tests and store it in a hashtable, which we convert to NUnit to provide test code coverage.

It seems there are so many initiatives going on, relating to how to do best practice tests, and I think our lives would be simpler if there were fewer options, which could hopefully allow for more contribution on the issue tracker for the repos.

On top of my head I can think of:

ARMTTK
Bicep Lint
PSRule
Template-Analyzer

Key features in my opinion for these frameworks are:

Should be easy to contribute to test cases
Should contain unit tests to ensure these tests work as intended after they are initially created
Should be outputable in formats fitting a terminal screen, as well as one or more common test formats (JUnit or NUnit comes to mind)
The output should include all pass and failing tests
It should be possible to exclude tests
It should be possible to include 'custom tests', e.g. ones not approved to the common bpa's provided by the repository.

craigofnz commented 2 years ago

I would like to see consideration been given to adopting SARIF as an output format from the linter?

It is well supported by CI/CD platforms (sometimes via extensions) and is adopted by other common tools such as gitleaks, codeql, snyk etc.

By output from the Bicep linter itself, the related error or warning messages will directly relate to the relevant code and line-numbers without needing to reverse the compilation from .bicep to .json to correct identified issues.

If the same rules are available via the VSCode extension, command-line and CI/CD systems, it could also provide a focal point for defining best practice rulesets for Azure resources defined in bicep.

lakshmojirao999 commented 2 years ago

Hi Team

In my case i created repo name as POC Application the space in the repo name causing this kind of issue An error occurred reading file. Could not find a part of the path '/Users/lakshmoji/office/zelarsoft/celito/POC Applications/main.bicep'

Then I just simply renamed the repo and the executed the bicep build --file file.bicep

StephenWeatherford commented 2 years ago

@lakshmojirao999 I can't repro this issue. If you are still seeing it, could you please create a new issue with repro steps? For instance, are you using "az bicep" or "bicep"? Is this Mac or Windows? Etc. Thanks!

ucheNkadiCode commented 2 years ago

@rchaganti @nicktolhurst @lakshmojirao999 @craigofnz @lr90 Thank you for being so active on this thread. We were able to come up with a design draft, but could you give us a few examples of how you intent to use a structured linter output? Some of you mentioned "using tests", but to our understanding, this occurs at Runtime while the linter is a compile time tool. A bit of clarity will help.

Also, what output formats are you expecting? Please select 1-2 formats.

Current Design Draft:

Formats

Human Readable as a default (example below)

JSON
SARIF? (Not sure about this one)

The command would likely be a standalone lint command.

bicep lint <filename.bicep> -> this would default to the human readable format above. Are you a fan of defaulting to human readable? AND you could declare bicep lint <filename.bicep> --json as an optional flag. Could also be --sarif, --xml depending on your feedback here.

ReneRebsdorf commented 2 years ago

@ucheNkadiCode I would argue that at least one human readable format, and one test framework output format should be supported. Given the choices in your list that means your screenshot plus SARIF.

Speaking for myself, I have little preference in which output standard is supported for test outputs.

Regarding test usage at build:

We have a lot of bicep templates, which we want to continuously scan to make sure newer API versions are used, and new best practices are adopted as they are being made. The bicep linter provides all of this when building to arm, but doesn't output it is a very useful way for test reports. Currently we concert bicep to arm, capture test output using arm-ttk, then convert that to NUnit (xmll) and publish that as a test result. Supporting something like 'az bicep lint main.bicep -outfile X -outformat sarif' would be huge.

johndowns commented 2 years ago

@ucheNkadiCode I like the proposal. I think it's important that we can lint our Bicep files from pipelines - ideally the linter is used interactively (e.g. in Visual Studio Code) while someone writes Bicep code, but if they make a change and don't see or deal with the linter's recommendations then the pipeline should be a backstop to scan the files before they proceed to the deployment process.

Regarding the output format, I'd suggest it's important to emit a format that can be read by both the Azure Pipelines Publish Test Results task and the GitHub Actions Test Reporter action. Both of these are used within pipelines to surface test results, and I think linter output is a natural fit for this.

Here are the file formats supported by the Azure Pipelines Publish Test Results task:

CTest
JUnit
NUnit 2, NUnit 3
Visual Studio Test (TRX)
xUnit 2

And by the GitHub Actions Test Reporter action:

dart-json
dotnet-trx
flutter-json
java-junit
jest-junit
mocha-json

The overlap there appears to be TRX and JUnit, so if the linter could emit one of those (maybe in addition to other formats?) then I think that would cover the pipeline use case well.

lr90 commented 2 years ago

What @johndowns says ^^ :)

As per earlier in the thread we are looking for consistent linting as part of our ADO pipelines (ultimately to replace ARM-TTK) which we report through the ADO Test panel:

So, its not just about failing the pipeline on warnings/errors but reporting the coverage and being able to drill down to find the offending modules.

nicktolhurst commented 2 years ago

☝️ I haven't much more to add to the above.

I'd expect to lint out to a file supported by a test report task / action, such as Azure Pipelines Publish Test Results task.

A JSON output could be useful, but I don't actually have a use case for this.

As for the default behaviour? I think the default should be human readable, the UX would be better for humans who care about such things! 🤖

Azure / bicep