Export lint output to file

[johnnyreilly]

Is your feature request related to a problem? Please describe.

It's very exciting to see linting become a first class citizen of Bicep with @johan-lindqvist's work on #10819

My understanding of this feature (which I hope to be able to use when it ships with the Azure CLI Azure/azure-cli#27378) is that it outputs errors and warnings to stdout, in two possible formats; the default one and Sarif.

Whilst this is great, it means there's a job of parsing work to do if you'd like to interrogate the results, perhaps to surface them in an Azure Pipeline. We tackle this with arm-ttk at the moment with this extension by @sam-cogan: https://github.com/sam-cogan/arm-ttk-extension-xplatform

Describe the solution you'd like

The idea I have in mind, is the ability to output lint results to a file. And possibly expand the formats available to include say JUnit. You can kind of see the scenarion that I have in mind in this blogpost that demos surfacing test results from Vitest (could be anything really) in Azure Pipelines

What do you think?

I can see the lint options currently look like this:

  {exeName} lint [options] <file>
    Lints a .bicep file.
    Arguments:
      <file>        The input file
    Options:
      --no-restore                   Skips restoring external modules.
      --diagnostics-format <format>  Sets the format with which diagnostics are displayed. Valid values are ( {string.Join(" | ", Enum.GetNames(typeof(DiagnosticsFormat)))} ).
    Examples:
      bicep lint file.bicep
      bicep lint file.bicep --no-restore
      bicep lint file.bicep --diagnostics-format sarif

Imagine an extra --outfile option as well, which if supplied passes the text output into that file?

[anthony-c-martin]

@johnnyreilly I believe SARIF was picked as a base format that can be converted. As a stopgap, have you tried converting from sarif -> junit xml format?

For example:

~/.azure/bin/bicep lint main.bicep --diagnostics-format sarif 2> lint.sarif
npx -y sarif-junit -i lint.sarif -o lint.xml

Gives me:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="My suite" tests="8" failures="0" errors="0" skipped="0">
    <testcase classname="artifacts-parameters" name="If an '_artifactsLocation' parameter is provided, an '_artifactsLocationSasToken' parameter must also be provided. [https://aka.ms/bicep/linter/artifacts-parameters]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="no-loc-expr-outside-params" name="Use a parameter here instead of 'resourceGroup().location'. 'resourceGroup().location' and 'deployment().location' should only be used as a default value for parameters. [https://aka.ms/bicep/linter/no-loc-expr-outside-params]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="simplify-interpolation" name="Remove unnecessary string interpolation. [https://aka.ms/bicep/linter/simplify-interpolation]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="no-loc-expr-outside-params" name="Use a parameter here instead of 'resourceGroup().location'. 'resourceGroup().location' and 'deployment().location' should only be used as a default value for parameters. [https://aka.ms/bicep/linter/no-loc-expr-outside-params]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="BCP036" name="The property &quot;logProgress&quot; expected a value of type &quot;bool | null&quot; but the provided value is of type &quot;'false'&quot;. If this is an inaccuracy in the documentation, please report it to the Bicep Team. [https://aka.ms/bicep-type-issues]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="BCP036" name="The property &quot;logVerbose&quot; expected a value of type &quot;bool | null&quot; but the provided value is of type &quot;'false'&quot;. If this is an inaccuracy in the documentation, please report it to the Bicep Team. [https://aka.ms/bicep-type-issues]" file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="BCP081" name="Resource type &quot;Microsoft.Automation/automationAccounts/webhooks@2018-06-30&quot; does not have types available." file="//Users/ant/Desktop/main.bicep"/>
    <testcase classname="simplify-interpolation" name="Remove unnecessary string interpolation. [https://aka.ms/bicep/linter/simplify-interpolation]" file="//Users/ant/Desktop/main.bicep"/>
  </testsuite>
</testsuites>

[johnnyreilly]

oh nice! let me try!

What do you think about the file output option?

I'm doing this right now:

az bicep build --file ${{ value.path }} > ${{ value.path }}.lint-output.txt 2>&1

[anthony-c-martin]

What do you think about the file output option?

I think that's a great suggestion - redirecting stderr has pitfalls as it's not straightforward to distinguish between success (sarif format output) and failure (for example, failing to find the .bicep file on disk).

[johnnyreilly]

BTW I'm using az bicep build as the az bicep lint isn't available yet (see Azure/azure-cli#27378)

In the interim I'm planning to build a mini parser which will read the build output - I can throw it away in future once lint lands.

[johnnyreilly]

Built a mini bicep build output -> JUnit JavaScript script that we're running in Azure Pipelines until az bicep lint lands later this year. Not perfect, but good enough for our needs.

//@ts-check
const fs = require("fs");

// This is necessary until direct lint support lands in the Azure CLI for Bicep
// See: https://github.com/Azure/bicep/issues/11960

/**
 * @typedef {Object} BicepLintWarningsOrError
 * @property {string} type
 * @property {string} typeAndMessage
 * @property {string} filePath
 * @property {string} lineNumber
 * @property {string} columnNumber
 * @property {string} line
 */

async function convertBicepOutputToJUnit() {
  // Check if there is at least two command-line arguments
  if (process.argv.length < 4) {
    console.error(
      "Example usage: node scripts/bicep-lint-to-junit.js containerapps.bicep.lint-output.txt containerapps.bicep.lint-output.junit.xml"
    );
    process.exit(1); // Exit with an error code
  }

  // Get the first command-line argument (index 2 because 0 and 1 are reserved for node and script file paths)
  const bicepOutputPath = process.argv[2];
  const bicepOutputJUnitPath = process.argv[3];

  const dirname = process.cwd();

  console.log("Loading:", bicepOutputPath, "from:", dirname);

  const bicepOutput = await fs.promises.readFile(bicepOutputPath, "utf-8");

  const bicepOutputLines = bicepOutput
    .replace(/\r/g, "")
    .split("\n")
    .filter((line) => line.trim().length > 0)
    .map((line) =>
      line
        .replace("WARNING: ", "")
        .replace("ERROR: ", "")
        .replace(dirname, "")
        .replace(/"/g, "")
    );

  if (bicepOutputLines.length === 0) {
    console.log("No issues found in:", bicepOutputPath);
    return;
  }

  /** @type {Map<string, BicepLintWarningsOrError[]>} */
  const bicepLintWarningsAndErrors = bicepOutputLines
    .map((line) => {
      try {
        const [filePathAndLine, typeAndMessage] = line.split(" : ");
        const [filePath, lineParts] = filePathAndLine.split("(");
        const [lineNumber, columnNumber] = lineParts
          ?.replace(")", "")
          .split(",");

        const type = typeAndMessage.startsWith("Error")
          ? "Error"
          : typeAndMessage.startsWith("Warning")
          ? "Warning"
          : "Unknown";

        return {
          type,
          typeAndMessage,
          filePath,
          lineNumber,
          columnNumber,
          line,
        };
      } catch (error) {
        console.error("Error parsing line:", line);
        console.error(error);
        return undefined;
      }
    })
    .reduce(
      (groupedByFilePath, curr) =>
        curr === undefined
          ? groupedByFilePath
          : groupedByFilePath.set(curr.filePath, [...(groupedByFilePath.get(curr.filePath) || []), curr]),
      new Map()
    );

  if (bicepLintWarningsAndErrors.size === 0) {
    console.log("No issues parsed in:", bicepOutputPath);
    return;
  }

  console.log("Parsed issues:", Array.from(bicepLintWarningsAndErrors.values()).flat());

  const entries = Array.from(bicepLintWarningsAndErrors.entries());

  const junit = `<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
${entries.map(([filePath, bicepLintWarningsAndErrors]) => { 
  return `    <testsuite name="${filePath}">
  ${bicepLintWarningsAndErrors
    .map((lintOrError) => lintOrError.type === "Error"
        ? `        <testcase name="${lintOrError.line.replace(filePath, "")}" classname="${filePath}" file="${lintOrError.filePath}" line="${lintOrError.lineNumber}">
            <failure message="${lintOrError.typeAndMessage}" />
          </testcase>`
        : `        <testcase name="${lintOrError.line.replace(filePath, "")}" classname="${filePath}" file="${lintOrError.filePath}" line="${lintOrError.lineNumber}" />`
    )
    .join("\n")}
    </testsuite>
`
}).join("\n")}
</testsuites>`;

  console.log(`
JUnit output:

${junit}`);

  console.log("Saving:", bicepOutputJUnitPath);

  await fs.promises.writeFile(bicepOutputJUnitPath, junit);

  console.log("Done!");
}

convertBicepOutputToJUnit().catch((error) => {
  console.error(error);
  process.exit(1);
});

// node scripts/bicep-lint-to-junit.js containerapps.bicep.lint-output.txt containerapps.bicep.lint-output.txt.junit.xml

[johnnyreilly]

Parking the junit conversation for a moment @anthony-c-martin, I've been having a quick gander at what an implementation might look like for a potential --outfile option.

By the looks of it, the relevant code sits in https://github.com/Azure/bicep/blob/main/src/Bicep.Cli/Logging/BicepDiagnosticLogger.cs

A simple implementation could take this code:

bicep/src/Bicep.Cli/Logging/BicepDiagnosticLogger.cs

Lines 130 to 132 in d7b6beb

	var sarifText = JsonConvert.SerializeObject(sarifLog, settings);
	io.Error.Write(sarifText);
	io.Error.Flush();

which turns the SARIF output into JSON and flushes it to the log, and also (if an --outfile option was provided) write output to that file as well.

Is something like that what you had in mind? Or something different? Of course with the current implementation that would only work if you had SARIF format selected; so that would have to change. But I wanted to get a sense of whether that would be the sort of thing you felt worked?

Also to add, I'm imagining that outputting to file will not replace outputting to console; it will be additional. So if --outfile was supplied, you would get results in the console and in a file. This would mean that in a CI context you'd see the linting failures in the output and then be able to do more specific visualisation things with them by interrogating the file.

What do you think?

[anthony-c-martin]

Sorry for the long delay in responding!

Is something like that what you had in mind? Or something different? Of course with the current implementation that would only work if you had SARIF format selected; so that would have to change. But I wanted to get a sense of whether that would be the sort of thing you felt worked?

Yeah, I believe this would work. However I'm wondering if we need it - instead could we just output to stdout instead of stderr? That way you'd be able to just to run: bicep lint main.bicep --diagnostics-format sarif > lint.sarif, and there wouldn't be a need for an --outfile option.

Also to add, I'm imagining that outputting to file will not replace outputting to console; it will be additional. So if --outfile was supplied, you would get results in the console and in a file. This would mean that in a CI context you'd see the linting failures in the output and then be able to do more specific visualisation things with them by interrogating the file.

My opinion is that it would give more flexibility to just log to file if the --outfile option is used; sarif isn't very human-readable, so logged to the CI output doesn't feel that helpful by default. However, if someone does want that behavior, they can always use cat:

bicep lint main.bicep --diagnostics-format sarif > lint.sarif
cat lint.sarif

[johnnyreilly]

I will respond to this soon - promise!

[anthony-c-martin]

FWIW here's how I've set this up with the latest Bicep release (0.24.24):
https://github.com/anthony-c-martin/bicep-on-k8s/blob/8281ba0fbb4efe42bcb6b282f925936ea468598d/.github/workflows/ci.yml#L36-L48

Example PR with warnings/errors raised by the linter:
anthony-c-martin/bicep-on-k8s#1

[johnnyreilly]

This looks quite promising! Let me give it a try on ADO pipelines

[johnnyreilly]

Just doing some experimentation now @anthony-c-martin. I'm not sure whether this should be regarded as a bug or something that makes the case for a dedicated output to process mechanism.

Consider a bicepconfig.json like this:

{
  // See https://aka.ms/bicep/config for more information on Bicep configuration options
  // Press CTRL+SPACE/CMD+SPACE at any location to see Intellisense suggestions
  "analyzers": {
    "core": {
      "rules": {
        "no-unused-params": {
          "level": "warning"
        },
        "no-unused-vars": {
          "level": "warning"
        },
        "secure-secrets-in-params": {
          "level": "error"
        }
      }
    }
  },
  "experimentalFeaturesEnabled": {
    "sourceMapping": true
  }
}

Running az bicep lint --file infra/main.bicep --diagnostics-format sarif > $(System.DefaultWorkingDirectory)/biceplint.sarif 2>&1 will result in output like this:

WARNING: WARNING: The following experimental Bicep features have been enabled: Source mapping. Experimental features should be enabled for testing purposes only, as there are no guarantees about the quality or stability of these features. Do not enable these settings for any production usage, or your production environment may be subject to breaking.

{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json",
  "version": "2.1.0",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "bicep"
        }
      },
      "results": [
        {
          "ruleId": "no-unused-vars",
          "message": {
            "text": "Variable \"unusedX\" is declared but never used. [https://aka.ms/bicep/linter/no-unused-vars]"
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "file:///home/vsts/work/1/s/infra/main.bicep"
                },
                "region": {
                  "startLine": 67,
                  "charOffset": 5
                }
              }
            }
          ]
        }
      ],
      "columnKind": "utf16CodeUnits"
    }
  ]
}

The presence of WARNING: WARNING: The following experimental Bicep features have been enabled... at the start of the biceplint.sarif file breaks usage of the output.

What would the advice be on how to handle this? As I said at the top, I'm not sure whether this should be regarded as a bug or something that makes the case for a dedicated output to process mechanism. What do you reckon?

If I turn off experimental features, this issue goes away. But generally there's probably a need to handle this in a way that affords consistent output?

[johnnyreilly]

Additional thoughts: I took a look at the linting in GitHub Actions here: https://github.com/anthony-c-martin/bicep-on-k8s/actions/runs/7224883231/job/19687151956?pr=1

By the looks of it, in the case of an error you just get a hard fail with no output. The pull request: anthony-c-martin/bicep-on-k8s#1 initially got me very excited, as I thought it was the lint failures being surfaced into the PR. Unless I'm reading it wrong, it seems like the messages in the PR like this:

are actually coming from somewhere else. Certainly if I fail a lint with an error using Bicep 0.24.24, the bicep.sarif is as simple as:

ERROR: 

Given you'll generally get the full error later when a bicep build takes place, it's still relatively straightforward to get to the reason for a build failure. It does feel unfortunate that lint fails without revealing too much though.

[anthony-c-martin]

@johnnyreilly the difference is that I'm using the bicep CLI directly - it looks like the AzureCLI implementation has a bug if Bicep returns a non-0 exit code (e.g. there are any build errors). Would you mind raising an issue in https://github.com/Azure/azure-cli for this?

Here's what the relevant part of my CI pipeline looks like:
https://github.com/anthony-c-martin/bicep-on-k8s/blob/e6dfa61fe7eae6fd6b148670f940041f3e294b20/.github/workflows/ci.yml#L36-L50

By the looks of it, in the case of an error you just get a hard fail with no output. The pull request: anthony-c-martin/bicep-on-k8s#1 initially got me very excited, as I thought it was the lint failures being surfaced into the PR. Unless I'm reading it wrong, it seems like the messages in the PR like this:

The lint issues are indeed being surfaced into the PR!

[anthony-c-martin]

The presence of WARNING: WARNING: The following experimental Bicep features have been enabled... at the start of the biceplint.sarif file breaks usage of the output.

What would the advice be on how to handle this? As I said at the top, I'm not sure whether this should be regarded as a bug or something that makes the case for a dedicated output to process mechanism. What do you reckon?

The problem here is the stderr redirect - as of version 0.24.24 of Bicep, you want to make sure you're not doing 2>&1 (which redirects stderr to stdout, and causes errors + sarif output to get mixed up). This also has the added benefit of making sure errors are logged properly to your console or CLI.

The fix I made was to solve exactly this problem - giving the caller a way to accurately read sarif output whilst being able to accurately differentiate it from errors.

[johnnyreilly]

Would you mind raising an issue in https://github.com/Azure/azure-cli for this?

Yup - I'm travelling right now, but I'll try and do this when I get to my destination. I'll link back to this and @ you

The lint issues are indeed being surfaced into the PR!

You're saying the code scanning messages on the PR come from the bicep task? And that if the Azure CLI didn't have a bug, I could expect similar behaviour?

[anthony-c-martin]

You're saying the code scanning messages on the PR come from the bicep task? And that if the Azure CLI didn't have a bug, I could expect similar behaviour?

Yes, exactly. The github/codeql-action/upload-sarif@v3 action in the code link I shared above is responsible for converting the .sarif file into the user-visible messages on the PR.

[johnnyreilly]

That is cool!

[johnnyreilly]

I realised that in the other discussion we missed the potential other bug @anthony-c-martin. I'm not sure whether this should be regarded as a bug or something that makes the case for a dedicated output to process mechanism.

Consider a bicepconfig.json like this:

{
  // See https://aka.ms/bicep/config for more information on Bicep configuration options
  // Press CTRL+SPACE/CMD+SPACE at any location to see Intellisense suggestions
  "analyzers": {
    "core": {
      "rules": {
        "no-unused-params": {
          "level": "warning"
        },
        "no-unused-vars": {
          "level": "warning"
        },
        "secure-secrets-in-params": {
          "level": "error"
        }
      }
    }
  },
  "experimentalFeaturesEnabled": {
    "sourceMapping": true
  }
}

Running az bicep lint --file infra/main.bicep --diagnostics-format sarif > $(System.DefaultWorkingDirectory)/biceplint.sarif 2>&1 will result in output like this:

WARNING: WARNING: The following experimental Bicep features have been enabled: Source mapping. Experimental features should be enabled for testing purposes only, as there are no guarantees about the quality or stability of these features. Do not enable these settings for any production usage, or your production environment may be subject to breaking.

{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json",
  "version": "2.1.0",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "bicep"
        }
      },
      "results": [
        {
          "ruleId": "no-unused-vars",
          "message": {
            "text": "Variable \"unusedX\" is declared but never used. [https://aka.ms/bicep/linter/no-unused-vars]"
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "file:///home/vsts/work/1/s/infra/main.bicep"
                },
                "region": {
                  "startLine": 67,
                  "charOffset": 5
                }
              }
            }
          ]
        }
      ],
      "columnKind": "utf16CodeUnits"
    }
  ]
}

The presence of WARNING: WARNING: The following experimental Bicep features have been enabled... at the start of the biceplint.sarif file breaks usage of the output.

What would the advice be on how to handle this? As I said at the top, I'm not sure whether this should be regarded as a bug or something that makes the case for a dedicated output to process mechanism. What do you reckon?

If I turn off experimental features, this issue goes away. But generally there's probably a need to handle this in a way that affords consistent output?

I'm guessing this is unrelated to the az bicep bug already discussed?

[anthony-c-martin]

The problem here is the stderr redirect - as of version 0.24.24 of Bicep, you want to make sure you're not doing 2>&1 (which redirects stderr to stdout, and causes errors + sarif output to get mixed up). This also has the added benefit of making sure errors are logged properly to your console or CLI.

The fix I made was to solve exactly this problem - giving the caller a way to accurately read sarif output whilst being able to accurately differentiate it from errors.

Take a look at this comment to resolve the issue you're describing. This should work with Bicep CLI, and should be unrelated to the AzCLI bug.

[johnnyreilly]

Okay cool, so I think we go from:

az bicep lint --file infra/main.bicep --diagnostics-format sarif > $(System.DefaultWorkingDirectory)/biceplint.sarif 2>&1 

to:

az bicep lint --file infra/main.bicep --diagnostics-format sarif > $(System.DefaultWorkingDirectory)/biceplint.sarif

Will experiment

[johnnyreilly]

Looks good!

[johnnyreilly]

hey @anthony-c-martin!

I'm back from travelling now and just catching up on this. I think there's a few separate things happening in this discussion and I realise from reading it back it's a little confusing. Herefollows an attempt by me to disambiguate the different topics under discussion.

Bug in Azure CLI around surfacing output when exit code is non-0

From here: #11960 (comment)

You asked if I could raise an issue with the Azure CLI project. Raised here: Azure/azure-cli#28259

Exporting lint output to file

You've outlined that it's possible to output lints to a file using > bicep.sarif:

az bicep lint --file ./infra/main.bicep --diagnostics-format sarif > bicep.sarif

This works. But there are gotchas. Consider the following from a GitHub Action:

      - name: Lint Bicep
        uses: azure/CLI@v1
        with:
          inlineScript: |
            az bicep lint --file ./infra/main.bicep --diagnostics-format sarif > bicep.sarif

Ideally this would run and output lints to bicep.sarif format. However, if other output comes along for the ride, this will also be included. Consider this GitHub action: https://github.com/johnnyreilly/blog.johnnyreilly.com/actions/runs/7616182236/job/20742359365?pr=817

The contents of bicep.sarif in this case were:

Installing Bicep CLI v0.24.24...
Successfully installed Bicep CLI to "/root/.azure/bin/bicep".
{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json",
  "version": "2.1.0",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "bicep"
        }
      },
      "results": [],
      "columnKind": "utf16CodeUnits"
    }
  ]
}

So because Bicep wasn't initially installed on the GitHub Actions server, the bicep lint output was prefixed by:

Installing Bicep CLI v0.24.24...
Successfully installed Bicep CLI to "/root/.azure/bin/bicep".

Which means that the subsequent upload task fails like this:

It's possible to work around this initial issue by say running az bicep install before you run az bicep lint, like so:

      - name: Lint Bicep
        uses: azure/CLI@v1
        with:
          inlineScript: |
            az bicep version
            az bicep lint --file ./infra/main.bicep --diagnostics-format sarif > bicep.sarif

But, whilst this works around the particular "bicep was not installed" issue that surfaced in this case, it doesn't solve the "there is other output coming from az bicep lint besides the lint output" issue in general.

Maybe that's okay; maybe it's fine to tackle each other issue that might present in the same way as it comes. My own instinct is that the redirect to file output approach isn't as reliable as I'd like for this reason. But maybe you feel it's good enough? What do you think?

[anthony-c-martin]

Maybe that's okay; maybe it's fine to tackle each other issue that might present in the same way as it comes. My own instinct is that the redirect to file output approach isn't as reliable as I'd like for this reason. But maybe you feel it's good enough? What do you think?

I think this is another AzCLI bug - Bicep is very strict about only writing the SARIF output to stdout for this exact reason. It looks like there's actually a PR open to fix this: Azure/azure-cli#28188.

[johnnyreilly]

okay cool - fingers crossed it lands!