diff --git a/charts/nudgebee-agent/values.yaml b/charts/nudgebee-agent/values.yaml index 0ec0a3e9..337e1ba6 100644 --- a/charts/nudgebee-agent/values.yaml +++ b/charts/nudgebee-agent/values.yaml @@ -65,7 +65,7 @@ runnerServiceAccount: runner: image: repository: ghcr.io/nudgebee/nudgebee-agent - tag: 2026-08-12T08-30-26_13e26b9dce6e3419f5e1ac52836c773a95098fde + tag: 2026-08-19T15-28-06_5a862ee549e8f98366ea71c01a7d07a2a151addd # Image template the pod_profiler action launches debugger pods from. # The agent substitutes `{}` for the variant (bpf, jvm, python, perf, ruby). # Surfaces as PROFILER_IMAGE; leave empty to fall back to the binary default. diff --git a/runner/pkg/triggers/diff_test.go b/runner/pkg/triggers/diff_test.go index 1af53dd4..2201af69 100644 --- a/runner/pkg/triggers/diff_test.go +++ b/runner/pkg/triggers/diff_test.go @@ -164,15 +164,39 @@ func TestBabysitter_EnrichBlocksAttachesDiffBlock(t *testing.T) { } } -func TestBabysitter_FingerprintIncludesResourceVersion(t *testing.T) { - // Two distinct spec changes (different resourceVersions) → two - // distinct fingerprints → two Findings, not deduped to one. +func TestBabysitter_FingerprintIsPerResourceNotPerChange(t *testing.T) { + // Repeated changes to the SAME resource must share a fingerprint so the + // server's occurrence chain (event_duplicates, keyed on fingerprint) can + // collapse them into one recurring entry with a repeat count. Keying on + // resourceVersion made every change a fresh chain of length 1 (#36647). mk := func(rv string, replicas int) map[string]any { return mustObj(t, `{"metadata":{"name":"d","namespace":"prod","resourceVersion":"`+rv+`"},"spec":{"replicas":`+itoa(replicas)+`}}`) } m := babysitterChangeMatcher("Deployment") - if m.FingerprintFn(mk("100", 2)) == m.FingerprintFn(mk("101", 5)) { - t.Error("different resourceVersions must produce distinct fingerprints") + if m.FingerprintFn(mk("100", 2)) != m.FingerprintFn(mk("101", 5)) { + t.Error("changes to the same resource must share a fingerprint regardless of resourceVersion") + } +} + +func TestBabysitter_FingerprintSeparatesResources(t *testing.T) { + mk := func(kind, ns, name string) string { + obj := mustObj(t, `{"metadata":{"name":"`+name+`","namespace":"`+ns+`","resourceVersion":"1"},"spec":{"replicas":1}}`) + return babysitterChangeMatcher(kind).FingerprintFn(obj) + } + base := mk("Deployment", "prod", "api") + for _, tc := range []struct { + desc string + kind, ns, name string + }{ + {"different name", "Deployment", "prod", "web"}, + {"different namespace", "Deployment", "staging", "api"}, + // A Deployment and a Service can share a name in one namespace; they + // are different resources and must not chain together. + {"different kind", "Service", "prod", "api"}, + } { + if got := mk(tc.kind, tc.ns, tc.name); got == base { + t.Errorf("%s: expected a distinct fingerprint, got the same one", tc.desc) + } } } diff --git a/runner/pkg/triggers/owner.go b/runner/pkg/triggers/owner.go index 9e2a044c..5d23fe98 100644 --- a/runner/pkg/triggers/owner.go +++ b/runner/pkg/triggers/owner.go @@ -95,6 +95,50 @@ func stripPodTemplateHash(name string) string { return podTemplateHashSuffix.ReplaceAllString(name, "") } +// generatedJobSuffix matches the per-run suffix appended to Job names, in the +// two shapes we actually observe in the field: +// +// - `-` — what the CronJob controller appends (a unix-minute stamp, +// e.g. `blinq-api-healthchecks-29764215`), and what indexed batch creators +// use (`grade-astropy--astropy-13398`). +// - `-<8+ hex>` — what our own one-Job-per-unit creators append +// (`trivy-image-scan-24e032a5`, `kube-bench-scan-177dde3a`). +// +// 5 digits is the floor because a shorter numeric tail is more likely to be a +// meaningful part of the name (`postgres-15`) than a generated one. +var generatedJobSuffix = regexp.MustCompile(`-([0-9]{5,}|[0-9a-f]{8,})$`) + +// generatedUUIDSuffix matches a full UUID tail (`nb-llm-ct-7e4998e5-2e91-4579- +// 989e-16926b6158e1`). It has to run before generatedJobSuffix, which would +// otherwise strip only the final hex group and leave a still-unique partial +// UUID behind. +var generatedUUIDSuffix = regexp.MustCompile(`-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) + +// JobFamily strips the generated per-run suffix from a Job name so every run +// of the same logical job shares one identity. +// +// Why this is needed on top of ResolveOwner: a Job created directly (not by a +// CronJob) has no ownerReferences to walk, so ResolveOwner returns nothing and +// the Job name itself is the only identity available — and that name is unique +// per run. Fingerprinting on it makes every run a brand-new problem, so the +// occurrence chain in event_duplicates never forms (issue #36647). CronJob-owned +// Jobs are unaffected: ResolveOwner already lifts those to the CronJob, and +// callers should prefer the resolved owner and fall back to this. +// +// This is a heuristic, in the same spirit as stripPodTemplateHash above, and it +// errs toward collapsing: two genuinely different Jobs that differ only by a +// generated-looking tail become one family, which is the grouping we want. +func JobFamily(name string) string { + stripped := generatedUUIDSuffix.ReplaceAllString(name, "") + stripped = generatedJobSuffix.ReplaceAllString(stripped, "") + // Never strip the name down to nothing — a Job literally named `12345678` + // keeps its name rather than becoming an empty fingerprint component. + if stripped == "" { + return name + } + return stripped +} + // SubjectFromObj extracts the (name, namespace, lowercased-kind, node) // for an obj. node is "" when not a Pod or when not yet scheduled. // Used by Engine.Match to populate Match.Subject* fields uniformly. diff --git a/runner/pkg/triggers/predicates.go b/runner/pkg/triggers/predicates.go index 57e06725..e693203d 100644 --- a/runner/pkg/triggers/predicates.go +++ b/runner/pkg/triggers/predicates.go @@ -309,10 +309,20 @@ func imagePullBackoffMatcher() MatcherSpec { owner := ResolveOwner(obj) if owner.Name != "" { name = owner.Name + // A Pod owned by a Job resolves to that Job, whose name + // carries a per-run suffix — so a creator that runs one Job + // per unit of work (our image scanner: one Job per image) + // produced a brand-new fingerprint every run and never + // chained. Collapse to the job family (#36647). + if owner.Kind == "job" { + name = JobFamily(name) + } } // Include the bad image — different bad images on the same // workload should be distinct Findings (operator just typo'd - // one container, the rest are fine). + // one container, the rest are fine). This also keeps per-image + // resolution after the job-family collapse above: one scan Job + // family with two bad images is still two Findings. image := firstFailingImage(obj) return fp("image_pull_backoff_reporter", ns, name, image) }, @@ -391,14 +401,34 @@ func jobFailureMatcher() MatcherSpec { AggregationKey: "job_failure", Priority: "MEDIUM", FindingType: "issue", - RateLimit: 0, // terminal — fingerprint by UID is enough + // Was 0 ("terminal — fingerprint by UID is enough"). That reasoning + // depended on the UID making every run unique; now that runs of the + // same job share a fingerprint, leaving this unlimited would re-emit + // for as long as the failed Job lingers. The transition gate cannot be + // relied on to prevent that either — same kubewatch pointer-aliasing + // caveat as pod_crash_loop, and prod shows single failed Jobs emitting + // ~68 times. The occurrence chain still counts every repeat; this only + // bounds emission rate. + RateLimit: 10 * time.Minute, Predicate: func(obj, oldObj map[string]any) bool { return jobHasFailedCondition(obj) && !jobHasFailedCondition(oldObj) }, FingerprintFn: func(obj map[string]any) string { ns := metaNS(obj) - uid := metaUID(obj) - return fp("job_failure", ns, uid) + // Prefer the CronJob parent — the watched object here is the Job + // itself, so its ownerReferences carry the CronJob when there is + // one. A directly-created Job has no owner to walk, and its own + // name is unique per run, so fall back to the job family. + // Keying on metadata.uid (the previous behaviour) made every run + // a distinct problem and stopped the occurrence chain from ever + // forming (#36647). + name := metaName(obj) + if owner := ResolveOwner(obj); owner.Name != "" { + name = owner.Name + } else { + name = JobFamily(name) + } + return fp("job_failure", ns, name) }, } } @@ -586,10 +616,11 @@ func babysitterChangeMatcher(kind string) MatcherSpec { AggregationKey: "ConfigurationChange/KubernetesResource/Change", Priority: "INFO", FindingType: "configuration_change", - // Each spec change has its own resourceVersion → its own - // fingerprint → no rate-limit needed for dedup. We do set a - // short rate-limit to absorb the rare spurious double-fire - // (kubewatch occasionally re-delivers the same event). + // Short rate-limit absorbs the rare spurious double-fire + // (kubewatch occasionally re-delivers the same event). A real + // second edit within the window is not lost to it in practice: + // the predicate requires an actual spec diff, and the status-only + // updates that dominate a rollout are excluded by diffOpt. RateLimit: 30 * time.Second, Predicate: func(obj, oldObj map[string]any) bool { if oldObj == nil { @@ -601,13 +632,17 @@ func babysitterChangeMatcher(kind string) MatcherSpec { FingerprintFn: func(obj map[string]any) string { ns := metaNS(obj) name := metaName(obj) - // Include resourceVersion so each distinct spec change gets - // its own Finding (Plan agent: "include obj.metadata. - // resourceVersion so each spec change is a distinct - // finding"). - meta, _ := obj["metadata"].(map[string]any) - rv, _ := meta["resourceVersion"].(string) - return fp("ConfigurationChange/KubernetesResource/Change", ns, name, rv) + // Identity is the resource that changed, NOT the individual + // change. This previously mixed in metadata.resourceVersion so + // "each spec change is a distinct finding" — but resourceVersion + // advances on every write, so no two changes to the same resource + // ever shared a fingerprint and the occurrence chain never formed: + // all 8824 of these in 30d of prod had occurrence_number = 1 + // (#36647). Each change is still its own event row carrying its + // own diff evidence; they now chain into one recurring entry with + // a repeat count. Kind is included so a Deployment and a Service + // of the same name in one namespace stay distinct. + return fp("ConfigurationChange/KubernetesResource/Change", ns, strings.ToLower(kind), name) }, EnrichBlocks: func(obj, oldObj map[string]any, _ EnrichContext) []EvidenceBlock { diffs := ComputeSpecDiff(obj, oldObj, diffOpt) @@ -825,12 +860,6 @@ func metaNS(obj map[string]any) string { return n } -func metaUID(obj map[string]any) string { - m, _ := obj["metadata"].(map[string]any) - u, _ := m["uid"].(string) - return u -} - // fp produces a stable sha256 hex of joined fields. Used by FingerprintFn // so all matchers produce same-shape fingerprints (inspectable, hex-safe). func fp(parts ...string) string { diff --git a/runner/pkg/triggers/predicates_test.go b/runner/pkg/triggers/predicates_test.go index 26cacb9f..377e0dc9 100644 --- a/runner/pkg/triggers/predicates_test.go +++ b/runner/pkg/triggers/predicates_test.go @@ -709,6 +709,89 @@ func TestJobFailure_DoesNotRefireWhilePersistentlyFailed(t *testing.T) { } } +func TestJobFailure_RunsOfTheSameJobShareAFingerprint(t *testing.T) { + // Every run of a directly-created Job gets a fresh name and UID. Keying + // on the UID made each run its own problem, so the server's occurrence + // chain never formed and a job failing all day looked like N unrelated + // failures (#36647). + mk := func(name, uid string) map[string]any { + return asObj(t, `{ + "metadata":{"name":"`+name+`","namespace":"scan","uid":"`+uid+`"}, + "status":{"conditions":[{"type":"Failed","status":"True"}]} + }`) + } + m := jobFailureMatcher() + first := m.FingerprintFn(mk("trivy-image-scan-24e032a5", "u-1")) + second := m.FingerprintFn(mk("trivy-image-scan-ce3efcc1", "u-2")) + if first != second { + t.Error("two runs of the same job family must share a fingerprint") + } + if other := m.FingerprintFn(mk("kube-bench-scan-177dde3a", "u-3")); other == first { + t.Error("a different job family must not share the fingerprint") + } +} + +func TestJobFailure_PrefersCronJobOwnerOverName(t *testing.T) { + // A CronJob-created Job carries the CronJob in ownerReferences, and the + // generated name suffix is a unix-minute stamp. Both routes must land on + // the same identity: the CronJob. + mk := func(name string) map[string]any { + return asObj(t, `{ + "metadata":{ + "name":"`+name+`","namespace":"ns","uid":"u-`+name+`", + "ownerReferences":[{"kind":"CronJob","name":"healthchecks","controller":true}] + }, + "status":{"conditions":[{"type":"Failed","status":"True"}]} + }`) + } + m := jobFailureMatcher() + if m.FingerprintFn(mk("healthchecks-29764215")) != m.FingerprintFn(mk("healthchecks-29764220")) { + t.Error("consecutive runs of one CronJob must share a fingerprint") + } +} + +func TestImagePullBackoff_JobOwnedPodsCollapseToJobFamily(t *testing.T) { + // One-Job-per-image scanners produced a new fingerprint for every scan + // because the Pod's owner is the per-run Job (#36647). + mk := func(job string) map[string]any { + return asObj(t, `{ + "metadata":{ + "name":"`+job+`-p9x2","namespace":"scan", + "ownerReferences":[{"kind":"Job","name":"`+job+`","controller":true}] + }, + "status":{"containerStatuses":[ + {"name":"scan","image":"registry/trivy:v1", + "state":{"waiting":{"reason":"ImagePullBackOff"}}} + ]} + }`) + } + m := imagePullBackoffMatcher() + if m.FingerprintFn(mk("trivy-image-scan-24e032a5")) != m.FingerprintFn(mk("trivy-image-scan-ce3efcc1")) { + t.Error("pods from two runs of the same job family must share a fingerprint") + } +} + +func TestImagePullBackoff_DeploymentReplicasStillCollapse(t *testing.T) { + // Guards the behaviour that already worked, so the job-family change + // above cannot regress it. + mk := func(pod string) map[string]any { + return asObj(t, `{ + "metadata":{ + "name":"`+pod+`","namespace":"prod", + "ownerReferences":[{"kind":"ReplicaSet","name":"api-7f9d8c5b6d","controller":true}] + }, + "status":{"containerStatuses":[ + {"name":"app","image":"registry/api:bad", + "state":{"waiting":{"reason":"ImagePullBackOff"}}} + ]} + }`) + } + m := imagePullBackoffMatcher() + if m.FingerprintFn(mk("api-7f9d8c5b6d-aaaaa")) != m.FingerprintFn(mk("api-7f9d8c5b6d-bbbbb")) { + t.Error("replicas of one deployment must share a fingerprint") + } +} + // ---------- node_not_ready ---------- // notReadyNode builds a Node fixture whose Ready condition has been False @@ -995,6 +1078,28 @@ func TestEngine_ReturnsEmptyForNoMatch(t *testing.T) { // ---------- owner-walk ---------- +func TestJobFamily(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + // Generated per-run suffixes we actually see in the field. + {"trivy-image-scan-24e032a5", "trivy-image-scan"}, // 8-hex, our scanner + {"kube-bench-scan-177dde3a", "kube-bench-scan"}, // 8-hex + {"blinq-api-healthchecks-29764215", "blinq-api-healthchecks"}, // CronJob unix-minute stamp + {"grade-astropy--astropy-13398", "grade-astropy--astropy"}, // indexed batch creator + // Full UUID tail must strip whole, not just its last hex group. + {"nb-llm-ct-7e4998e5-2e91-4579-989e-16926b6158e1", "nb-llm-ct"}, + // Names that must survive untouched. + {"nightly-backup", "nightly-backup"}, + {"postgres-15", "postgres-15"}, // short numeric tail is meaningful + {"migrate-v2", "migrate-v2"}, + {"12345678", "12345678"}, // never strip to empty + {"", ""}, + } { + if got := JobFamily(tc.in); got != tc.want { + t.Errorf("JobFamily(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + func TestResolveOwner_ReplicaSetStripsHash(t *testing.T) { pod := asObj(t, `{ "metadata":{"ownerReferences":[