Feat: Add egressEnforcement opt-out for proxy-sidecar mode#436
Conversation
a1693ad to
07ec6f7
Compare
huang195
left a comment
There was a problem hiding this comment.
Well-structured, security-conscious feature with a correct fail-closed resolution chain (CR > namespace > enforce-redirect default; unknown values fail closed; envoy-sidecar correctly unaffected) and strong unit coverage. One blocking issue: the CRD manifests were not regenerated, so the marquee per-workload .spec.egressEnforcement field is pruned by the API server and silently no-ops on a real cluster — only the namespace-ConfigMap and platform-policy paths actually work. Details inline. Two design suggestions and one nit round it out.
Areas reviewed: Go (CRD type, webhook resolution + injection, platform policy), tests, CRD manifests, Helm values
Author: akram (MEMBER — maintainer)
Commits: 2, both signed-off (DCO passing)
CI: Build / Unit / Integration / Lint / CodeQL / Trivy green; E2E pending (PR notes full e2e blocked by an unrelated ClientRegistrationReconciler issue — so the CRD-pruning bug isn't caught by automated e2e either).
Once make manifests is committed (and a quick check that kubectl apply of an AgentRuntime preserves egressEnforcement), this is an easy approve.
Assisted-By: Claude Code
On managed OpenShift platforms (ROSA HCP), iptables is unavailable in init containers due to missing kernel modules (iptable_nat) and SELinux restrictions. PR rossoctl#407 made enforce-redirect always-on in proxy-sidecar mode, which breaks pod creation on these platforms. Add an egressEnforcement field (enforce-redirect | none) to the AgentRuntime CR and namespace authbridge-runtime-config ConfigMap, following the same resolution chain as authBridgeMode and mtlsMode: 1. AgentRuntime CR .spec.egressEnforcement (per-workload) 2. Namespace authbridge-runtime-config egressEnforcement (namespace) 3. "enforce-redirect" (cluster default, preserves current behavior) When set to "none", proxy-init is not injected in proxy-sidecar/lite modes. Egress relies on HTTP_PROXY (cooperative) + inbound AuthBridge on destinations + NetworkPolicy. Unknown values fail closed to enforce-redirect. envoy-sidecar mode is unaffected — its proxy-init (redirect mode) is structural and always injected regardless of this setting. Ref: RHAIENG-5702 Ref: rossoctl/cortex#502 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Akram <akram.benaissi@gmail.com>
Adds proxy.allowedEgressEnforcement to the platform config (Helm values → kagenti-platform-config ConfigMap). The webhook validates the resolved egressEnforcement value against this list and overrides it to the first allowed value when not permitted. This gives platform admins governance over egress enforcement modes: - ["enforce-redirect"] → no opt-out allowed - ["none"] → iptables disabled cluster-wide (ROSA HCP) - ["enforce-redirect", "none"] → workloads choose (default) Default allows both modes for backward compatibility. Ref: RHAIENG-5702 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Akram <akram.benaissi@gmail.com>
- Regenerate CRD manifests (make manifests) — egressEnforcement field now appears in agent.kagenti.dev_agentruntimes.yaml with enum validation - Replace stringInSlice helper with slices.Contains (stdlib) - Reject empty allowedEgressEnforcement in Validate() — empty list was a footgun (bypass governance); must be explicitly set - Add allowedEgressEnforcement to charts/kagenti-operator/values.yaml with doc comment for discoverability Ref: RHAIENG-5702 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Akram <akram.benaissi@gmail.com>
305021e to
ecea178
Compare
huang195
left a comment
There was a problem hiding this comment.
Re-review after the rebase (HEAD ecea178). All feedback from both prior rounds is resolved:
- CRD regenerated —
egressEnforcementis now present withenum: [enforce-redirect, none]in bothcharts/...agentruntimes.yamlandconfig/crd/bases/...agentruntimes.yaml, so the per-workload.spec.egressEnforcementfield is no longer pruned by the API server. This was the round-1 blocker. - gofmt fixed — the leftover double blank line from removing
stringInSliceis gone; Lint is green. (round-2 blocker) allowedEgressEnforcementsurfaced and documented incharts/kagenti-operator/values.yaml.- Empty-list hardened —
Validate()now rejects an emptyallowedEgressEnforcementwith an actionable error, so theallowed[0]override is safe. slices.Containsreplaces the hand-rolled helper.
Clean implementation overall: secure-by-default (enforce-redirect), fail-closed on unknown values, envoy-sidecar correctly unaffected, good unit coverage (10 tests).
Author: akram (MEMBER — maintainer)
CI: Build / Unit / Integration / Lint / Helm / YAML green; E2E pending (PR notes full e2e is blocked by an unrelated ClientRegistrationReconciler issue).
Agent/IDE config (.claude/.vscode): none
LGTM.
Assisted-By: Claude Code
Add missing workloadKind parameter to 10 InjectAuthBridge test calls introduced by upstream PRs (rossoctl#436 egress enforcement, rossoctl#434 namespace realm, rossoctl#432 iptables backend) that merged after this branch diverged. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Summary
On managed OpenShift platforms (ROSA HCP), iptables is unavailable in init containers due to missing kernel modules (
iptable_nat) and SELinux restrictions (Red Hat KB 5990311). PR #407 madeenforce-redirectproxy-init always-on in proxy-sidecar mode, which breaks pod creation on these platforms — pods are stuck inInit:CrashLoopBackOff.This PR adds an
egressEnforcementfield to the AgentRuntime CR and namespaceauthbridge-runtime-configConfigMap, allowing platforms without iptables to opt out of proxy-init injection.Changes
Per-workload / per-namespace opt-out
egressEnforcementfield with enum validation (enforce-redirect|none)authBridgeModeandmtlsMode):.spec.egressEnforcement(per-workload)authbridge-runtime-configConfigMapegressEnforcementfieldenforce-redirect(cluster default — preserves current behavior)enforce-redirectPlatform-level governance
proxy.allowedEgressEnforcementin platform config (Helm values →kagenti-platform-config): restricts which modes workloads may select["enforce-redirect"]— no opt-out allowed["none"]— iptables disabled cluster-wide (ROSA HCP)["enforce-redirect", "none"]— workloads choose (default)Security model when
egressEnforcement: noneHTTP_PROXY) → authenticated through authbridge ✓Files changed
api/v1alpha1/agentruntime_types.go— CRD fieldinternal/webhook/injector/constants.go— enforcement mode constantsinternal/webhook/injector/agentruntime_config.go— CR extractioninternal/webhook/injector/namespace_config.go—ExtractEgressEnforcement()internal/webhook/injector/pod_mutator.go— resolution chain + conditional injection + platform policyinternal/webhook/config/types.go—AllowedEgressEnforcementfield + validationinternal/webhook/config/defaults.go— default allows both modesinternal/webhook/injector/pod_mutator_test.go— 10 testsTest plan
go build ./...passesegressEnforcement: none— pods start without proxy-init, authbridge-proxy sidecar injected, no iptables crashContext
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com