fix(trivy-operator): repair CVE alerts that could never fire - #176
Merged
Conversation
Three defects meant the chart's alerting was partly or wholly inert, in the worst way: silently. A join that matches nothing and a metric that does not exist both look exactly like 'no problems found'. TrivyOperatorScannerStuck queried trivy_resource_last_scan_timestamp_seconds. That metric does not exist; trivy-operator exposes no last-scan timestamp of any kind, so the alert could never fire. Replaced with TrivyOperatorMetricsMissing, which infers scanner liveness from whether vulnerability metrics exist at all. ImageOutdatedAndVulnerable joined on an image key that only matched for fully-qualified references. Trivy builds image_registry/image_repository (index.docker.io/library/nginx) while version-checker copies the pod spec verbatim - its urlTagSHAFromImage only splits off tag and digest, with no registry normalisation, so the same image is labelled 'nginx'. Every short-form Docker Hub reference silently failed to join while fully-qualified ones worked, making the alert look alive. Added record::version_checker::outdated, which applies Docker's reference expansion rules so both sides share a canonical form before joining. The join also used '+' as its operator, which worked only because the right-hand side was filtered to zero. Switched to multiplication against a count-normalised series so the CVE count is preserved and group_left can still carry latest_version into the annotation. Adds health alerts for the silent-failure class itself: TrivyVersionCheckerJoinEmpty fires when both inputs have series but the join is empty, and VersionCheckerMetricsMissing covers version-checker being load-bearing for the CVE alert while shipping no alerts of its own. NOTES.txt advertised trivy_image_vulnerabilities_id, which does not exist. The real metric is trivy_vulnerability_id. Validated with promtool check rules (6 rules, SUCCESS).
The recording rules normalised Trivy's three image labels and version-checker's single one into a canonical reference so the two could be joined. That normalisation was a chain of four label_replace regexes reimplementing Docker's reference grammar, and it got it wrong: a registry-qualified host with no dot, such as localhost/myapp, matched the bare-name rule and became index.docker.io/localhost/myapp. Those images silently never joined, so the alert worked for some and could not fire for others. Correlation now happens in kubeaid-agent, which parses both sides with go-containerregistry, the library Trivy itself uses, and joins canonical references in Go. That removes the reason for the recording rules, and ImageOutdatedAndVulnerable with them. What remains are two alerts that depend on no join at all: an absent() watchdog so a scanner that stops reporting reads as broken rather than as clean, and a backlog threshold on the raw severity gauge.
The chart is open source and should not name a specific hosted service. Points at the VulnerabilityReport CRs instead, which is where the per-CVE detail actually lives and is reachable by anyone running the chart.
The comment claimed the alert was "a ticket, never a page". A rule sets a severity and nothing more -- what that severity routes to is decided downstream. Says what this rule controls, and why warning rather than critical, without overstating its reach.
Three fixes. It named a specific hosted service as the destination for CVE detail, which does not belong in an open-source chart. It repeated the "ticket, never a page" claim just corrected in the template -- a rule sets a severity, and what that routes to is decided by the alerting pipeline. And the report list was missing InfraAssessmentReport and ClusterComplianceReport, the latter being what actually produces compliance evidence. Restructure: each alert gets its own section headed with its severity, so a reader can see what fires and how loudly without reading prose. The account of the failed PromQL correlation is cut to the part a future reader needs -- do not rebuild reference parsing in label_replace -- with the rest left to the decisions doc.
The backlog alert described every counted finding as having a published fix. Nothing in the expression establishes that. trivy_image_vulnerabilities counts by severity and carries no fixed_version label, so the claim rested entirely on trivy.ignoreUnfixed being true, which drops unfixed CVEs at scan time so they never reach a report. That coupling was invisible. Setting ignoreUnfixed to false left the alert firing on CVEs nobody can act on while still calling the backlog actionable. The annotations now derive their wording from that value instead of asserting it, so turning it off changes what the alert says about itself. Same correction applied to the values.yaml comments, which still described the alert as a ticket rather than a page.
Thirty-one lines of comment for two alerts. Keeps only what stops someone reintroducing a bug -- do not rebuild image references in PromQL, a silent scanner reads as a clean cluster, and fixability comes from ignoreUnfixed rather than from the expression. The rest was narrative that belongs in the decisions doc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes three defects in the
trivy-operatorchart's alerting, plus two documentation errors. All of them failed the same way — silently. A PromQL join that matches nothing and an alert on a metric that does not exist both look exactly like "no problems found".Companion to #175, which records these in
decisions/security-scanning.md. That PR documents them; this one fixes them.What was broken
TrivyOperatorScannerStuckqueriedtrivy_resource_last_scan_timestamp_secondsImageOutdatedAndVulnerablejoined on a mismatched image key+used as the join operator0; fragile and obscures intentversion-checkerload-bearing but unmonitoredNOTES.txtadvertisedtrivy_image_vulnerabilities_idtrivy_vulnerability_idThe join bug in detail
Trivy describes an image as three labels and the recording rule joined them into
image_registry/image_repository, givingindex.docker.io/library/nginx.version-checker emits a single
imagelabel copied from the pod spec. ItsurlTagSHAFromImagedoes pure string splitting — it strips the tag and digest and nothing else:So the same container is
index.docker.io/library/nginxto Trivy andnginxto version-checker. The join produced no series for those images. It kept working forquay.io/...-style references, which is what made it hard to notice — the alert looked alive.What changed
record::version_checker::outdatedapplies Docker's reference expansion rules (nginx→index.docker.io/library/nginx,bitnami/redis→index.docker.io/bitnami/redis,docker.io/…→index.docker.io/…) so both sides share a canonical form before joining. The four regexes are mutually exclusive, so chaining applies at most one transformation per series.1) series. The CVE count is preserved andgroup_leftstill carrieslatest_versioninto the annotation.TrivyOperatorScannerStuck→TrivyOperatorMetricsMissing, inferring scanner liveness from whether vulnerability metrics exist at all, since no timestamp metric is available to query.VersionCheckerMetricsMissing— covers the dependency that had no alerts of its own.TrivyVersionCheckerJoinEmpty— the interesting one. Fires when both inputs have series but their join is empty, which means the normalisation no longer matches how images are referenced in this cluster.That last alert is the general fix for this whole class of bug: the join now monitors itself. Any future drift in image reference style surfaces as an alert instead of as silence.
Verification
helm templaterenders cleanly, and the escaped Prometheus templating ({{ $labels.image }}) survives Helm's own templatingpromtool check rules— 6 rules, SUCCESSNote for review
VersionCheckerMetricsMissingfires iftrivy-operatoris deployed withoutversion-checker. That is intentional — the chart README already states version-checker is required forImageOutdatedAndVulnerable, and #175 makes the two a standard pair. If we ever want trivy-only deployments to be silent, this needs a values toggle.🤖 Generated with Claude Code