feat: cache the trivy db and skip duplicate image scans

[stevegerrits]

TL;DR — Trivy scans get two speedups: the vulnerability DB is cached in a per-job docker volume (no re-download per scan), and identical images published under multiple names are scanned once per build (the report and verdict are reused). Scan findings (exit 2) are cleanly separated from operational errors (any other non-zero exit). No breaking change: findings keep their existing allowFailure behaviour, and operational errors are non-blocking by design (see below). Everything is opt-out (cacheVolume: '', skipDuplicates: false).

Summary

Two efficiency improvements to TrivyPlugin, both on by default and individually opt-out:

• Persistent vulnerability DB cache — the scan container mounts a named docker volume at trivy's cache path, so the DB is downloaded once per agent instead of once per scan (the download often takes longer than the scan itself). The volume defaults to a per-job name (jpipe-trivy-cache-<job>): trivy's cache DB takes an exclusive file lock, so a single cluster-wide volume could make concurrent builds of unrelated jobs contend. Pass a fixed cacheVolume name to deliberately share, or '' to disable the mount.
• Duplicate-image detection — pipelines that publish one built image under several names currently scan identical bits once per name. The plugin now resolves the image's docker image ID before scanning; if an image with the same ID was already scanned in the same build with the same scan configuration (severity, ignoreUnfixed, extraFlags, report, trivyVersion — folded into the marker key as a SHA-256 digest), it skips the scan, reuses the first report for publishing, and re-signals the first scan's verdict through its own allowFailure, so a strict gate never silently inherits a relaxed instance's pass. Detection degrades to a normal scan if the image ID can't be resolved; skipDuplicates: false disables it.

Findings vs. operational errors — and why operational errors never block

The scan runs with --exit-code=2 via returnStatus, which lets us tell two things apart that trivy's default --exit-code=1 conflates:

• exit 0 — clean.
• exit 2 — vulnerabilities found. This is a real verdict and keeps its existing behaviour: subject to allowFailure (UNSTABLE when tolerated, failing when not). Unchanged from today.
• any other non-zero exit — an operational error (cache DB lock, image pull failure, DB mirror down, …); the scan produced no verdict.

Deliberate product decision: operational errors are downgraded to an UNSTABLE stage and never block the build, independent of allowFailure. Rationale: a developer's build must not be held hostage to trivy scan-infrastructure flakiness — a down DB mirror or a transient lock should not turn into a red, blocking pipeline. It is not swallowed silently either (the old behaviour, where a broken scan looked identical to a clean one): the stage goes UNSTABLE with a distinct message, and no dedup marker is written, so a retry re-scans from scratch and duplicates never inherit a transient error.

Reviewer note: an earlier revision of this PR failed the build hard on operational errors. That was intentionally reverted — non-blocking-for-developers is the chosen trade-off here. allowFailure continues to govern findings only.

Also: report directories are keyed on the full sanitized image path, so distinct images sharing a basename (repo/app vs other/app) no longer overwrite each other's reports.

Testing

• TrivyPluginSpec (19 cases): per-job/explicit/disabled cache volume, single scan + outcome marker, duplicate skip + report reuse, config-mismatch and report-format-mismatch rescans, distinct image IDs, basename collision, skipDuplicates: false, image-ID fallback, findings with/without allowFailure, failed-verdict re-signaling (lenient + strict duplicates), --exit-code=2 wiring, and operational errors going UNSTABLE + non-blocking (with and without allowFailure).
• mvn clean test via make test: the new spec passes; the one pre-existing basePipelineSpec failure also fails on a clean main checkout and is unrelated.

[stevegerrits]

Review follow-ups, all pushed:

1. Operational errors masked as verdicts — fixed. The scan now runs with --exit-code=2 and returnStatus: exit 2 (findings) stays subject to allowFailure; any other non-zero exit fails the build outright and writes no marker, so a cache-lock or pull failure can't pass as UNSTABLE nor propagate to duplicates. Covered by two new spec cases.
2. hashCode() collision — replaced with a SHA-256 digest of the config string (first 16 hex chars) in the marker key.
3. Basename report-dir collision — report dirs are now keyed on the full sanitized image path (.trivy-report-repo_app vs .trivy-report-other_app); the published report name keeps the pre-existing basename convention. Spec case added.
4. Marker asymmetry on hard failure — left as-is by design (the build aborts, so duplicates never consult a marker); now documented at the write site.

On the per-job vs per-build volume question: kept per-job. A per-build volume would eliminate same-job contention but also the cache hit itself — every build would re-download the DB, which is the cost this feature removes. Same-job concurrent builds can still contend on the lock, but with the exit-code split that contention now surfaces as a loud operational failure rather than a silent UNSTABLE, which makes the trade-off observable. If it bites in practice, switching the default to include the executor/build number is a one-liner.

[stevegerrits]

Final review round, addressed:

• allowFailure behavior change now visible for release notes — the latest commit carries a BREAKING CHANGE: footer describing the new semantics (operational scan errors fail the build even with allowFailure: true; only findings are tolerated), and the PR body has a dedicated breaking-change section so a squash merge preserves it either way.
• Image reference quoted in docker inspect — a stray shell metacharacter in a tag derived from git describe would previously break the inspect and be masked by the "could not resolve" fallback.
• recordIssues on reused JSON reports — intentional: each published image name keeps its own report/issue entry so per-name dashboards stay complete; the duplicate skip changes where the data comes from, not what each name publishes. Aggregation across names double-counting one built image is the pre-existing semantics of scanning per name.
• The per-scan find cleanup noise is accepted as-is per the review.

[stevegerrits]

Update on the operational-error handling: reverted the hard-fail from the previous round.

The earlier revision failed the build on any non-vulnerability trivy exit (cache lock, DB mirror down, pull failure). On reflection that's the wrong trade-off for us — it would let trivy infrastructure flakiness block developers' builds. New behaviour:

• Findings (exit 2): unchanged — allowFailure governs them exactly as before. No breaking change anymore.
• Operational errors (other non-zero): downgraded to an UNSTABLE stage with a distinct message, never blocking, independent of allowFailure. Still not silent (the old bug where a broken scan looked clean is fixed) and still writes no dedup marker, so a retry re-scans.

PR title/body updated (dropped the BREAKING CHANGE note), and there's an explicit reviewer note documenting the decision so it isn't "corrected" back to hard-fail later. TrivyPluginSpec now covers both the UNSTABLE-non-blocking paths.

[Copilot]

Pull request overview

This PR enhances the TrivyPlugin to reduce CI time spent on repeated Trivy work by introducing a persistent vulnerability DB cache (via a Docker volume) and by skipping duplicate scans of identical images within the same build while preserving the correct pass/fail/unstable semantics.

Changes:

• Add a configurable Docker volume mount to persist Trivy’s DB/cache across scans (defaulting to a per-job volume; opt-out supported).
• Add duplicate-image detection (by Docker image ID + scan-config digest) to reuse prior scan report/verdict within the same build (opt-out supported).
• Refine exit-code handling to distinguish vulnerability findings (--exit-code=2) from operational errors, treating operational errors as UNSTABLE/non-blocking by design.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
test/io/stenic/jpipe/plugin/TrivyPluginSpec.groovy	Adds comprehensive Spock coverage for cache volume behavior, duplicate-scan skipping, report dir naming, and findings vs operational-error behavior.
src/io/stenic/jpipe/plugin/TrivyPlugin.groovy	Implements cache volume mounting, duplicate-scan markers keyed by image ID + config digest, distinct report directories, and exit-code based error handling.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.