Feat: default mTLS to permissive and enable card discovery#408
Conversation
cwiklik
left a comment
There was a problem hiding this comment.
Well-executed default flip implementing Phase 1 of the mTLS spec (#401): permissive becomes the default mTLS posture (kubebuilder marker + consistent empty→permissive handling across the envoy template, pod mutator, and resolved config), card-discovery and verified-fetch default on (with documented kill switches), plus deprecation warnings for legacy JWS flags and upgrade-visibility startup logs. CRD manifests are regenerated in both config/crd/bases/ and charts/crds/ identically — good hygiene. Code logic is correct and the doc comments accurately describe the new-CR vs existing-CR resolution.
Security: net improvement (mTLS on by default). The main risk is operational (upgrade rollout + SPIRE dependency), not a vulnerability — see comment #1.
Tests: pass, but missing a positive assertion for the new empty→permissive default (comment #2).
CI: 15 green; E2E still running, and the two new E2E scenarios (with/without SPIRE) are unchecked — worth landing those before merge given the fleet-wide rollout.
Commits: signed-off. Nits: subject lacks the emoji prefix the convention favors; and the PR body uses "🤖 Generated with Claude Code" — Kagenti's convention is Assisted-By: Claude Code, never "Generated with."
Approving — the code is correct; the upgrade-impact verification (comment #1) is strongly recommended before merge.
| // process start). | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default=permissive |
There was a problem hiding this comment.
suggestion: This default flip has a notable upgrade impact worth surfacing. Per the field doc, mtlsMode != disabled auto-enables SPIRE and changing the mode triggers a pod rollout. So on operator upgrade, every existing AgentRuntime with an unset mtlsMode flips empty→permissive → SPIRE auto-enabled + pod rollout, fleet-wide. permissive is designed to be safe (accepts/falls back to plaintext), but on clusters without SPIRE this relies entirely on graceful fallback — and the "deploy without SPIRE, verify graceful fallback" E2E box is still unchecked in the test plan. Recommend verifying the no-SPIRE path before merge and calling out the rollout + SPIRE auto-enable in release notes / a prominent startup log (the existing startup logs cover card-discovery/verified-fetch but not the mTLS default change).
| "namespace", namespace, "crName", crName, | ||
| "unrecognized", mtlsMode, "source", mtlsSource) | ||
| mtlsMode = MTLSModeDisabled | ||
| mtlsMode = MTLSModePermissive |
There was a problem hiding this comment.
nit: The defense-in-depth fallback for an unrecognized mtlsMode flips from disabled to permissive. Reasonable under the "mTLS by default" theme (fails secure rather than open), but note a garbage value now also pulls in SPIRE — fine given the CRD enum rejects unknowns at admission, just a conscious posture change worth a line in the commit/PR rationale.
|
let's set it to draft until ready to merge |
Change mTLS default from disabled to permissive so agents get mTLS automatically when SPIRE is available. Enable card discovery and verified fetch by default. Add deprecation warnings for legacy JWS signing flags. Changes: - Add +kubebuilder:default=permissive on MTLSMode field - Add ConditionTypeMTLSReady and AnnotationMTLSMode constants - Set kagenti.io/mtls-mode annotation on pod template in controller - Set MTLS_MODE env var on authbridge containers in webhook - Treat empty MTLSMode as permissive across all resolution paths (controller, pod mutator, envoy template, resolved config) - Flip --enable-card-discovery and --enable-verified-fetch to true - Add startup deprecation warnings for legacy signing flags - Clean up mtls-mode annotation on AgentRuntime deletion - Update tests for new default behavior Jira: RHAIENG-4944 Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
c3aca86 to
e3a0491
Compare
E2E authbridge tests were failing because the mTLS default changed from disabled to permissive. The envoy-proxy container crash-looped as the envoy template now renders TLS contexts by default, but the E2E test fixtures aren't configured for full mTLS infrastructure. Set mtlsMode: disabled explicitly on both authbridge-agent and authbridge-disabled-agent AgentRuntime fixtures to preserve the existing test behavior. These tests validate sidecar injection, not mTLS transport. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
|
The PR has been updated with changes in #405 and is ready to be reviewed. |
pdettori
left a comment
There was a problem hiding this comment.
Approving — code is correct, CI is fully green, CRD regenerated identically in both config/crd/bases/ and charts/crds/, and the dependency spec (#401) and config-merge update (#405) are both merged.
Follow-up (non-blocking): The no-SPIRE graceful-fallback E2E path is still unverified. The default flip means existing AgentRuntimes with an unset mtlsMode go empty→permissive on upgrade, auto-enabling SPIRE + a pod rollout fleet-wide. On clusters without SPIRE this relies entirely on graceful fallback (per @cwiklik's comment #1, and the test-plan box deploy without SPIRE, verify graceful fallback is still unchecked). Please track this as a follow-up — verify the no-SPIRE path and/or call out the upgrade rollout + SPIRE auto-enable in release notes.
Minor cleanups also worth a follow-up: PR body uses 🤖 Generated with Claude Code (Kagenti convention is Assisted-By: Claude Code), and there is no startup log specifically for the mTLS default flip (only card-discovery / verified-fetch are logged).
- Clarify kagenti backend/UI is upstream-only, not midstream product - Note RHOAI agent views are built by dashboard team (ODH Dashboard AAA page) - Update AgentCard flow: JWS verification deprecated in favor of mTLS transport security (rossoctl/operator#408) - Clarify which repos are relevant for midstream strategies Co-reviewed-by: Varsha Prasad Narsing Assisted-By: 🤖 Claude Code Signed-off-by: Roland Huß <rhuss@redhat.com>
Summary
mTLSModetopermissiveso agents get mTLS automatically when SPIRE is available--enable-card-discoveryand--enable-verified-fetchby defaultmTLSModeaspermissiveacross all resolution paths (envoy template, pod mutator, resolved config) for consistency with existing CRsSpec
Implements Phase 1 tasks (T001-T004) from spec: mTLS transport security (#401)
Changes
api/v1alpha1/agentruntime_types.go+kubebuilder:default=permissiveon MTLSMode,ConditionTypeMTLSReadyconstantcmd/main.gointernal/webhook/injector/envoy_template.gointernal/webhook/injector/pod_mutator.gointernal/webhook/injector/resolved_config.gointernal/webhook/injector/*_test.goconfig/crd/bases/,charts/Test plan
make test)default: permissivefor mtlsModeJira: RHAIENG-4944
🤖 Generated with Claude Code