feat: Target Pods at CREATE time instead of workload objects#183
Conversation
Change the mutating webhook from intercepting Deployments, StatefulSets, DaemonSets, Jobs, and CronJobs to intercepting Pods at CREATE time. This follows the pattern used by Istio, Linkerd, and Vault Agent Injector, and eliminates GitOps drift caused by mutating workload objects in etcd. Key changes: - Webhook handler decodes corev1.Pod directly (no workload switch/case) - deriveWorkloadName() extracts stable name from GenerateName - MutatingWebhookConfiguration targets pods in core API group - objectSelector pre-filters on kagenti.io/type + kagenti.io/inject - namespaceSelector requires kagenti-enabled=true label - reinvocationPolicy set to IfNeeded for multi-webhook compatibility - ClusterRole updated: removed apps/batch perms, added replicasets - Rollout script applies webhook config before image rollout (race fix) - 5 Ginkgo webhook handler tests added Closes rossoctl#176 (Phase 1) Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: cwiklik <cwiklikj@gmail.com>
- Add comment to deriveWorkloadName clarifying it returns the ReplicaSet name (not Deployment); Deployment-level resolution is Phase 2 (rossoctl#177) - Handle edge case where both GenerateName and Name are empty (fall back to UID with warning log) - Add table-driven tests for deriveWorkloadName (5 edge cases) - Fix copyright year (2025 -> 2025-2026) - Add nolint directives for standard Ginkgo/Gomega dot imports Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: cwiklik <cwiklikj@gmail.com>
cwiklik
left a comment
cwiklik
left a comment
There was a problem hiding this comment.
Review Summary
Well-structured architectural change moving the mutating webhook from workload-level to Pod-level interception. Removes significant complexity (multi-Kind switch), adds proper safety guards (Kind check, namespace opt-in, object selector), and includes 5 meaningful Ginkgo tests. Follows the proven pattern used by Istio, Linkerd, and Vault Agent Injector.
No blocking issues found. Suggestions below are non-blocking improvements.
Areas reviewed: Go, Helm/K8s, Shell, Docs, Security
Commits: 1 (signed off, Assisted-By correct)
CI status: pending at time of review
Verdict: APPROVE (submitted as COMMENT since author cannot self-approve)
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Review Summary
Well-structured PR that follows the Istio/Linkerd pattern of mutating Pods at CREATE instead of workload objects. The code is significantly simpler, RBAC surface is reduced, and the rationale (GitOps drift elimination) is sound.
Areas reviewed: Go, Helm/K8s, Shell, Tests, Docs
Commits: 2 — both signed-off with Assisted-By trailer ✓
CI: Passing (CI Checks ✓, DCO ✓)
The main concern is the kagenti-enabled: "true" namespace selector being a breaking change for existing deployments — operators need a documented migration step. The test suite is a good start but the positive injection assertion may need a Get() re-fetch to be reliable against envtest behavior.
Verdict: COMMENT — one item needs attention (namespace migration path) but the design is solid.
- Re-fetch Pods after Create in all tests to assert on server-side mutations rather than in-memory state (false-pass prevention) - Add UID fallback test entry for deriveWorkloadName - Use structured logging (V(0).Info) instead of embedding severity in message string - Document BREAKING CHANGE: namespaces now require kagenti-enabled=true label for injection — add migration step to README - Rollout script labels AuthBridge demo namespace with kagenti-enabled=true Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: cwiklik <cwiklikj@gmail.com>
github-project-automation
Bot
moved this from Backlog
to Done
in Kagenti Issue Prioritization
2 participants
Summary
Change the mutating webhook from intercepting workload objects (Deployments, StatefulSets, DaemonSets, Jobs, CronJobs) to intercepting Pods at CREATE time. This follows the pattern used by Istio, Linkerd, and Vault Agent Injector, and eliminates GitOps drift caused by mutating workload objects stored in etcd.
corev1.Poddirectly — no workloadswitch/casederiveWorkloadName()extracts a stable workload name fromGenerateNameMutatingWebhookConfigurationtargets pods in the core API group withCREATEonlyobjectSelectorpre-filters onkagenti.io/typein [agent, tool] +kagenti.io/injectNotIn [disabled]namespaceSelectorrequireskagenti-enabled: "true"labelreinvocationPolicy: IfNeededfor multi-webhook compatibilityCloses #176 (Phase 1 of 2 — Phase 2 is AgentRuntime CR integration, tracked in #177)
Test plan
make buildpassesmake testpasses (5 new webhook handler tests + existing precedence tests)webhook-rollout.sh— clean startup logskagenti.io/type=agentreceives sidecarshelm template)