feat: Comment Audit result on PR (only on errors)

[eduardo-getpassport]

What's on this PR

• Changes on pr-change-set.yaml:
  • Added step to download the vulnerabilities artifact from build step and post a comment with the output.
• Changes on build.yaml:
  • Added continue-on-error: true in Audit installed packages step . In case of an audit error, the next step will run
  • Added the id audit-packages in Audit installed packages step. This is to reference this step in the later one.
  • Changed the run command to export the output.
  • Upload the output artifact
• Updated .gitignore with vulnerabilities file to avoid issues.

What Comment vulnerabilities on PR step does:

Takes the file generated in the previous step in case of an error and posts the content inside the PR.

Example:

[jenstroeger]

Looks good to me, thank you @eduardo-getpassport! I have only three minor notes below that you can ignore if you’d like 😉

[jenstroeger]

I wonder if steps.audit-packages.conclusion == 'failure' is more reliable because any non-zero exit code is considered a failure (and we check only for 1^*)?

—————
^* At first glance, 1 seems to be the only exit code that pip-audit returns.

[jenstroeger]

@behnazh I’m actually surprised that referencing workflow steps works across different workflows (src) 🤔 I wasn’t able to find that documented, so hopefully that handy feature isn’t a bug.

For example, in pr-change-set.yaml we have:

if: steps.audit-packages.outputs.exit_code == 1

which references the audit-packages step in the reusable workflow build.yaml

[behnazh]

Thanks, looks great! But the pipelines are failing. I think the problem is storing the result file at the root directory, which flit is not tracking and therefore it complains. You can either store it at dist/ or /tmp?

[behnazh]

@behnazh I’m actually surprised that referencing workflow steps works across different workflows (src) thinking I wasn’t able to find that documented, so hopefully that handy feature isn’t a bug.

Good point. I had missed it. Actually I doubt it works and will probably fail at runtime.

@eduardo-getpassport have you tested these workflows?

[eduardo-getpassport]

@behnazh I’m actually surprised that referencing workflow steps works across different workflows (src) thinking I wasn’t able to find that documented, so hopefully that handy feature isn’t a bug.

Good point. I had missed it. Actually I doubt it works and will probably fail at runtime.

@eduardo-getpassport have you tested these workflows?

Yes! I just tested again.

I commented on the if: steps.audit-packages.outputs.exit_code == 1 from build and let the two others in the PR file and work as expected (didn't skip the post artifact on the build and skipped in the PR file).

After that I commenter the PR ones and works. I think since we are using the needs: build, the step can take the ID from another workflow.

This is the first run with only the build commented: Link

The second run with the if commented in both files: Link

PS: Only on my runs, I commented the Mac and Windows jobs since we are expensive 😱

[eduardo-getpassport]

Thanks, looks great! But the pipelines are failing. I think the problem is storing the result file at the root directory, which flit is not tracking and therefore it complains. You can either store it at dist/ or /tmp?

Fixed on separate branch, pushing the changes rn

[behnazh]

Yes! I just tested again.

I commented on the if: steps.audit-packages.outputs.exit_code == 1 from build and let the two others in the PR file and work as expected (didn't skip the post artifact on the build and skipped in the PR file).

After that I commenter the PR ones and works. I think since we are using the needs: build, the step can take the ID from another workflow.

@eduardo-getpassport I'm trying to debug this. The outputs.exit_code is not defined anywhere in build.yaml and I can't find any documentation for it. Why are you using this variable? When I run echo ${{ steps.audit-packages.outputs.exit_code }} it's empty.

[jenstroeger]

With PR #551 in mind I wonder if it would make sense to post an informative message on a PR if package auditing is disabled?