Repository navigation
fix(foreman): reap audit ConfigMaps older than retention (#990) - #1027
Conversation
…#990) Per-task audit records are written as ConfigMaps labeled foreman.llmkube.dev/audit=true (see writer.go) and intentionally owner-unbound so they outlive AgenticTask GC for compliance. Without retention they accumulate unbounded in the task namespace — five foreman-audit-dispatch-892-* CMs were observed from repeated dispatches of one issue after every AgenticTask had been deleted. Add a periodic reaper registered with the controller manager: pkg/foreman/audit/reaper.go Sweep + Reaper (leader-aware) pkg/foreman/audit/reaper_test.go covers prune/keep/label-filter/disable cmd/foreman-operator/main.go --audit-retention (default 7d, 0 disables) + interval flag; registers the runnable so leader election prevents HA double-sweep charts/foreman/values.yaml operator.audit.{retention,interval} charts/foreman/templates/operator-deployment.yaml flags plumbed charts/foreman/templates/operator-rbac.yaml configmaps/delete Retention-by-age via metadata.creationTimestamp (k8s-immutable, no need to stamp our own timestamp). The label filter keeps the blast radius to CMs the reaper actually owns. NeedLeaderElection makes the reaper a no-op on standby replicas under --leader-elect. Signed-off-by: Jory Irving <jory@jory.dev>
a1308df to
6706732
Compare
Defilan
left a comment
There was a problem hiding this comment.
Approve. This is clean, and it got the parts that usually bite exactly right.
The reaper logic is sound: age cut on the immutable CreationTimestamp against now - retention, label-scoped List so the blast radius is audit CMs only, IsNotFound-on-delete treated as benign for the HA race, and retention <= 0 a genuine no-op in both Sweep and Start so --audit-retention=0s disables cleanly. NeedLeaderElection() = true plus the immediate-first-sweep and log-and-continue loop are the right calls.
Nice catch on the duration handling too: DurationVar won't parse "7d", and you used 7*24*time.Hour for the default and 168h in the values, so the operator won't crash on a bad parse.
The 5 unit tests are real (old deleted / recent kept / non-audit ignored / retention-zero-and-negative no-op / empty cluster / ticker ticks and stops), and the body matches the diff.
Two non-blocking nits, take or leave:
Sweep'sListisn't paginated. Fine given the label filter and self-limiting deletes, but worth a comment if audit volume ever gets large.- The RBAC grant is full CRUD on configmaps while the reaper only needs list+delete. Standard (RBAC can't label-scope), but a one-line note that the code's label filter is the real guard would be nice.
The failing "Package and Install Test" is a kind-cluster setup flake (connection refused to localhost:8080, missing kind-registry container), not this change. I re-ran it.
…wing intent (defilantech#990) Reviewer nits on defilantech#1027: - pkg/foreman/audit/reaper.go: explain why the Sweep List is not paginated (label filter + self-bounded by deletes) and what to do if audit volume ever outgrows --max-objects-per-list (default 500). - charts/foreman/templates/operator-rbac.yaml: note that the audit reaper only needs list + delete but Kubernetes RBAC verbs are per-resource, not per-label-selector, so the full CRUD set is granted and the in-code label filter is the real blast guard. Both are comment-only. No behavior change, no new tests. Signed-off-by: Jory Irving <jory@jory.dev>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…1 ➔ 0.9.3) (#1489) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/defilantech/charts/llmkube](https://github.com/defilantech/LLMKube) | patch | `0.9.1` → `0.9.3` | --- ### Release Notes <details> <summary>defilantech/LLMKube (ghcr.io/defilantech/charts/llmkube)</summary> ### [`v0.9.3`](https://github.com/defilantech/LLMKube/blob/HEAD/CHANGELOG.md#093-2026-07-09) [Compare Source](defilantech/LLMKube@v0.9.2...v0.9.3) ##### Bug Fixes - **controller:** drop invalid --enable-metrics flag from vLLM runtime ([#​1031](defilantech/LLMKube#1031)) ([9b9450d](defilantech/LLMKube@9b9450d)) ### [`v0.9.2`](https://github.com/defilantech/LLMKube/blob/HEAD/CHANGELOG.md#092-2026-07-09) [Compare Source](defilantech/LLMKube@v0.9.1...v0.9.2) ##### Features - **foreman:** coder grounding rail (flag doc-contradicting metric writes) ([#​1017](defilantech/LLMKube#1017)) ([aa969ed](defilantech/LLMKube@aa969ed)) - **foreman:** distinct ALREADY-RESOLVED coder outcome ([#​970](defilantech/LLMKube#970)) ([#​1011](defilantech/LLMKube#1011)) ([8e041e0](defilantech/LLMKube@8e041e0)) - **foreman:** flag a GO that changes no functional production code ([#​1024](defilantech/LLMKube#1024)) ([3e1a6fb](defilantech/LLMKube@3e1a6fb)) - **foreman:** MCP client for agents (Phase 1: HTTP transport, context7) ([#​1014](defilantech/LLMKube#1014)) ([d34343f](defilantech/LLMKube@d34343f)) - **foreman:** semantic issueAsk verification instead of verbatim substring ([#​1020](defilantech/LLMKube#1020)) ([85d92ee](defilantech/LLMKube@85d92ee)) - **foreman:** str\_replace escalates to write\_file after repeated failures ([#​1026](defilantech/LLMKube#1026)) ([de1b8cb](defilantech/LLMKube@de1b8cb)) ##### Bug Fixes - **foreman:** exempt REJECT from grounded-finding demotion ([#​1008](defilantech/LLMKube#1008)) ([18098cb](defilantech/LLMKube@18098cb)) - **foreman:** gate Job downloads the go.mod toolchain (GOTOOLCHAIN=auto) ([#​1019](defilantech/LLMKube#1019)) ([5f4c51c](defilantech/LLMKube@5f4c51c)) - **foreman:** guard file-less findings + resolve bare/absolute paths in grounding rail ([#​1009](defilantech/LLMKube#1009)) ([304a2b8](defilantech/LLMKube@304a2b8)) - **foreman:** reap audit ConfigMaps older than retention ([#​990](defilantech/LLMKube#990)) ([#​1027](defilantech/LLMKube#1027)) ([08d06d6](defilantech/LLMKube@08d06d6)) - **foreman:** reviewer diffs against the upstream base, not stale local fork main ([#​1006](defilantech/LLMKube#1006)) ([6ef7278](defilantech/LLMKube@6ef7278)) - **metrics:** remove dead metrics + phantom PrometheusRule error-rate rule ([#​1010](defilantech/LLMKube#1010)) ([7c92f3f](defilantech/LLMKube@7c92f3f)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/1489
…mkube (0.9.1 ➔ 0.9.3) (#497) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/home-operations/charts-mirror/llmkube](https://github.com/defilantech/LLMKube) | patch | `0.9.1` → `0.9.3` | --- ### Release Notes <details> <summary>defilantech/LLMKube (ghcr.io/home-operations/charts-mirror/llmkube)</summary> ### [`v0.9.3`](https://github.com/defilantech/LLMKube/blob/HEAD/CHANGELOG.md#093-2026-07-09) [Compare Source](defilantech/LLMKube@v0.9.2...v0.9.3) ##### Bug Fixes - **controller:** drop invalid --enable-metrics flag from vLLM runtime ([#​1031](defilantech/LLMKube#1031)) ([9b9450d](defilantech/LLMKube@9b9450d)) ### [`v0.9.2`](https://github.com/defilantech/LLMKube/blob/HEAD/CHANGELOG.md#092-2026-07-09) [Compare Source](defilantech/LLMKube@v0.9.1...v0.9.2) ##### Features - **foreman:** coder grounding rail (flag doc-contradicting metric writes) ([#​1017](defilantech/LLMKube#1017)) ([aa969ed](defilantech/LLMKube@aa969ed)) - **foreman:** distinct ALREADY-RESOLVED coder outcome ([#​970](defilantech/LLMKube#970)) ([#​1011](defilantech/LLMKube#1011)) ([8e041e0](defilantech/LLMKube@8e041e0)) - **foreman:** flag a GO that changes no functional production code ([#​1024](defilantech/LLMKube#1024)) ([3e1a6fb](defilantech/LLMKube@3e1a6fb)) - **foreman:** MCP client for agents (Phase 1: HTTP transport, context7) ([#​1014](defilantech/LLMKube#1014)) ([d34343f](defilantech/LLMKube@d34343f)) - **foreman:** semantic issueAsk verification instead of verbatim substring ([#​1020](defilantech/LLMKube#1020)) ([85d92ee](defilantech/LLMKube@85d92ee)) - **foreman:** str\_replace escalates to write\_file after repeated failures ([#​1026](defilantech/LLMKube#1026)) ([de1b8cb](defilantech/LLMKube@de1b8cb)) ##### Bug Fixes - **foreman:** exempt REJECT from grounded-finding demotion ([#​1008](defilantech/LLMKube#1008)) ([18098cb](defilantech/LLMKube@18098cb)) - **foreman:** gate Job downloads the go.mod toolchain (GOTOOLCHAIN=auto) ([#​1019](defilantech/LLMKube#1019)) ([5f4c51c](defilantech/LLMKube@5f4c51c)) - **foreman:** guard file-less findings + resolve bare/absolute paths in grounding rail ([#​1009](defilantech/LLMKube#1009)) ([304a2b8](defilantech/LLMKube@304a2b8)) - **foreman:** reap audit ConfigMaps older than retention ([#​990](defilantech/LLMKube#990)) ([#​1027](defilantech/LLMKube#1027)) ([08d06d6](defilantech/LLMKube@08d06d6)) - **foreman:** reviewer diffs against the upstream base, not stale local fork main ([#​1006](defilantech/LLMKube#1006)) ([6ef7278](defilantech/LLMKube@6ef7278)) - **metrics:** remove dead metrics + phantom PrometheusRule error-rate rule ([#​1010](defilantech/LLMKube#1010)) ([7c92f3f](defilantech/LLMKube@7c92f3f)) </details> --- ### Configuration 📅 **Schedule**: (in timezone America/New_York) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTIuMSIsInVwZGF0ZWRJblZlciI6IjQzLjI1Mi4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.greyrock.io/greyrock-labs/home-ops/pulls/497
What
Adds a periodic reaper to
foreman-operatorthat deletes audit-recordConfigMaps older than a configurable retention window (default 7d). Label-scoped toforeman.llmkube.dev/audit=trueso non-audit CMs are untouched.Why
pkg/foreman/audit/writer.gostampsforeman-audit-<task>ConfigMaps WITHOUTownerReferencesby design — the audit record must outlive theAgenticTaskto remain a compliance trail after task GC. Reported in this issue: fiveforeman-audit-dispatch-892-*CMs from repeated dispatches of a single issue, all surviving every AgenticTask deletion and growing unbounded. Without a reaper there is no way to bound the count.AI assistance disclosure: drafted with opencode using MiniMax M3, per
CONTRIBUTING.md.Fixes #990
How
pkg/foreman/audit/reaper.go—Sweep(ctx, client, retention)(pure) andReaper(amanager.LeaderElectionRunnablethat ticksSweep). Retention 0 disables the reaper cleanly (Startblocks onctx.Done()and returns nil). Usesmetadata.creationTimestamp(k8s-immutable) as the age source. The first sweep runs immediately on start so a backlog of stale CMs is cleaned without waiting an hour.pkg/foreman/audit/reaper_test.go— 5 unit tests via fake client: prunes old CMs, keeps recent CMs, ignores non-audit CMs by label filter, retention≤0 is a no-op (negative also), empty cluster is a clean 0, the periodic runner ticks and stops on ctx cancellation,Retention=0runner never deletes.cmd/foreman-operator/main.go—--audit-retention(default7d,0disables) and--audit-retention-interval(default1h) flags;mgr.Addregisters the reaper unconditionally.NeedLeaderElectionmakes it a no-op on standby replicas under--leader-elect, so HA cannot double-sweep.charts/foreman/values.yaml—operator.audit.{retention,interval}keys.charts/foreman/templates/operator-deployment.yaml— flags plumbed through.charts/foreman/templates/operator-rbac.yaml— addeddeletetoconfigmaps(the reaper's only new RBAC need).Checklist
make testpasses locallymake lintpasses locallygit commit -s) per DCO