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

Provide an output that can be used in later steps to avoid duplicate runs #412

Open
erzz opened this issue Oct 14, 2024 · 3 comments · May be fixed by #419
Open

Provide an output that can be used in later steps to avoid duplicate runs #412

erzz opened this issue Oct 14, 2024 · 3 comments · May be fixed by #419

Comments

@erzz
Copy link

erzz commented Oct 14, 2024

The request

As another step towards reducing load on the rate limiting - could you provide an action output that we could use like result=fail. This would be decoupled completely from exit code (for which the behavior would remain the same as it is today, depending on the exit-code flag)

The background

To explain where this would be useful - our typical flow up to now has been to run Trivy twice in a workflow

  • once to produce the nice readable table view (with exit-code 1) so engineers can see the feedback direct in the workflow if it should fail a scan and we can "break the build"
  • then immediately again (if: always())to get in SARIF format for upload to GHAS

What would an output solve?

Mainly that we can just start to use convert instead of a second run whilst presenting a workflow output that is easy and obvious to an engineer to see their results in a workflow when it fails.

To do this now in a less obvious format for the consumer of the workflow I have something like:

- name: Trivy Image Scan
  id: scan
  uses: aquasecurity/[email protected]
  with:
      image-ref: some-image
      exit-code: 1
      format: json
      output: "report.json"

- name: Setup trivy for SARIF
  if: always()
  uses: aquasecurity/[email protected]

- name: Convert to SARIF
  if: always()
  run: |
      trivy convert --format=sarif --output="results.sarif" "results.json"

- name: Upload results to GitHub Security
  if: always()
  uses: github/codeql-action/upload-sarif@v3
  with:
      sarif_file: "results.sarif"

- name: View readable report
  if: steps.scan.outcome == 'failure'
  run: |
      trivy convert --format=table results.json && exit 1

The problems with this are:

  1. there will be two failed steps. One of which is the original scan step with no output because its written to file (confusing to engineer and they may assume it was a runtime error) and then a second step further down the job log to find the actual results in a readable format.
  2. I cannot trigger the readable report without also failing the scan step
  3. Its all a lot of faff :)

What would the solution look like

- name: Trivy Image Scan
  id: scan
  uses: aquasecurity/[email protected]
  with:
      image-ref: some-image
      exit-code: 0                                               <<<------ engineer will not look here
      format: json
      output: "report.json"

- name: Setup trivy for SARIF
  if: always()
  uses: aquasecurity/[email protected]

- name: Convert to SARIF
  if: always()
  run: |
      trivy convert --format=sarif --output="results.sarif" "results.json"

- name: Upload results to GitHub Security
  if: always()
  uses: github/codeql-action/upload-sarif@v3
  with:
      sarif_file: results.sarif

- name: View results
  if: steps.scan.outputs.result == 'fail'
  run: |
      trivy convert --format=table "results.json && exit 1       <<<----- nice obvious end of job view if there are failures

So now the workflow output makes total sense to a consumer when there are findings, you get less requests for the DB because we arent running it twice.

Alternative suggestions

Adding the convert command to the action which would also be nice - but I suspect much more work than a simple addition of an output?

Allow multiple outputs in a single execution - but you guys already rejected that many times and is the reason you added the excellent convert command :)

Add a upload-sarif flag and you handle the creation of and upload of the sarif directly in the initial execution - also a lot more work but would be nice

You could go bananas with outputs like total-findings, critical-findings, high-findings etc etc

@AshishBhadouria
Copy link

As a consumer of the trivy findings, I endorse this request.

@simar7
Copy link
Member

simar7 commented Oct 15, 2024

Maybe I'm misunderstanding something here but running trivy twice in the same pipeline job doesn't request a DB twice, does it?

@erzz
Copy link
Author

erzz commented Oct 16, 2024

@simar7 It shouldn't (though it has in recentish runs I think because of now-resolved bug)

I try to be a good citizen and have 2 steps in the same job - I have seen many having parallel jobs (for speed).

But pushing convert as the correct method to approach multiple and/or dependent/conditional formats requires either the output(s), or convert, to be better exposed by the action. I just proposed the lowest hanging fruit to achieve it as the main suggestion :)

I am pretty close to throwing out the action anyway now given we have the new setup-trivy action.... its a lot of ugly flag management in yaml-wrapped shell to just use native trivy... but most of my suggestions those suggestions would alleviate the need for that

@erzz erzz linked a pull request Oct 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants