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

Setup Trivy caching of config file #764

Closed
wants to merge 1 commit into from

Conversation

Nava-JoshLong
Copy link
Contributor

Ticket

Resolves #{TICKET NUMBER OR URL}

Changes

  • Setup caching for the vulnerability scanner config files
  • Configure trivy to use the cached config file

Context for reviewers

The trivy action has a known issue where the host of the config file for the action is overwhelmed and causing timeout and failures. There was an update, and their official documentation recommend to use caching of the file, at least at daily, and use that cached file to reduce the load on the hosts.

This PR introduces a new cron workflow to cache that file (and any other scanner files we want to cache in the future), and then updates the trivy action to use that cached file. The cache needs to be used once a week, or the cache needs to have less than 5GB stored, or the older files will be removed, so we want to keep the cache warm for this file

Testing

Testing will be ran from the workflow once the PR is opened (looks like I need to trigger it manually). I'll link those runs once done. I'm looking at the trivy code to see if it will cache for us on first run, or if we need to run the cache workflow first

@Nava-JoshLong Nava-JoshLong requested a review from lorenyu October 9, 2024 20:43
@Nava-JoshLong Nava-JoshLong self-assigned this Oct 9, 2024
@lorenyu
Copy link
Contributor

lorenyu commented Oct 9, 2024

@Nava-JoshLong

The cache needs to be used once a week, or the cache needs to have less than 5GB stored, or the older files will be removed, so we want to keep the cache warm for this file

I don't understand the logic for why we need to keep the cache warm. If the Trivy scan isn't used more than once a week and the cache is cleared resulting in a cache miss on the next run, what's the problem with that?

Copy link
Contributor

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nava-JoshLong
Copy link
Contributor Author

Looks like they beat you to it: https://github.com/aquasecurity/trivy-action/pull/399/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

Seems like we can close this

For anyone reviewing, this is the PR I based this off of, we need to setup caching in the action. From the conversation in Slack:

  • We need to either create the cache on main, or share a branch cache to other branches
    • The new workflow creates this cache on main
  • Why do we want a warm cache via this new workflow?
    • The cache action saves date up to 10GB in the stash, or used in the last 7 days. If the stash is larger than 10GB, or the file is older than a week, the file is deleted from the cache
    • This workflow creates the cache in main in case this every happens, and we can use that on all branches and not need to worry about sharing it
  • The cache is pulled from the trivy action itself
    • Link to code
    • We need to have the cache be from today for it to be hit from this action
    • Otherwise, it will pull from the config host and then stash it for us

Comment on lines +67 to +68
TRIVY_SKIP_DB_UPDATE: true
TRIVY_SKIP_JAVA_DB_UPDATE: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this will fail if the cache is missing. This could block PRs or any updates to child repos until the cache workflow is merged and ran

@lorenyu
Copy link
Contributor

lorenyu commented Oct 9, 2024

@Nava-JoshLong re-reviewing the code from https://github.com/aquasecurity/trivy-action/pull/399/files it still seems like we can just not do anything and have things cached to the day by default. Are you seeing anything different?

Is there a way we can just test this hypothesis without any custom code changes? i.e. just open a PR, run trivy scan on main, then run trivy scan on the PR and check the logs in the second run to see that it was a cache hit

@Nava-JoshLong
Copy link
Contributor Author

We can't run workflows on main unless they are merged into main, we'd have to open a PR with just the new workflow to cache to main. I'm working on some logic that'd check if that cache exists, then sets the env variables so we are catching the error when it is a first run

@Nava-JoshLong
Copy link
Contributor Author

Logic that handles setting env var when the cache is missing

      - name: Get current date
        id: date
        run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT

      - name: Check for Trivy cache
        id: cache-check
        uses: actions/cache@v4
        with:
          path: ${{ github.workspace }}/.cache/trivy
          key: cache-trivy-${{ steps.date.outputs.date }}
          restore-keys: cache-trivy-
          lookup-only: true

      - name: Run Trivy vulnerability scan
        uses: aquasecurity/trivy-action@master
        with:
          scan-type: image
          image-ref: ${{ steps.build-image.outputs.image }}
          format: table
          exit-code: 1
          ignore-unfixed: true
          vuln-type: os
          scanners: vuln,secret
          # Enables caching of trivy config file that is used to search for the
          # vulnerabilities. We cache it since it only updates once every 6 hours,
          # and the confg hosts can be down, causing failures
          cache: 'true'
        env:
          # Set to skip since we cache the files in the vulnerability-update-cache
          # workflow. The if else logic handles when the cache doesn't exist, it
          # will cache the config file so we can skip in the future
          TRIVY_SKIP_DB_UPDATE: ${{ steps.cache-check.outputs.cache-hit != 'true' && 'false' || 'true' }}
          TRIVY_SKIP_JAVA_DB_UPDATE: ${{ steps.cache-check.outputs.cache-hit != 'true' && 'false' || 'true' }}

@Nava-JoshLong
Copy link
Contributor Author

Closing for now, the default behavior of trivy caching will help 90% of repos as is

@Nava-JoshLong Nava-JoshLong deleted the cache-trivy-config-file branch October 10, 2024 18:09
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 this pull request may close these issues.

2 participants