Skip to content

Add REVIEWERS.md for mTLS spec #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
varshaprasad96 merged 1 commit into from
Jun 8, 2026
Merged

Add REVIEWERS.md for mTLS spec #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions specs/003-mtls-transport-security/REVIEWERS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Review Guide: mTLS Transport Security for Agent Communication

**Generated**: 2026-06-08 | **Spec**: [spec.md](spec.md)

## Why This Change

Agent-to-agent and controller-to-agent communication in kagenti currently runs over plaintext HTTP by default. While authbridge already has full mTLS support implemented (permissive/strict modes, SPIRE-based SVIDs, per-handshake cert rotation), the operator doesn't activate it by default. Operators must manually set flags and configure mTLS mode. This spec makes mTLS the default transport security, with clear error conditions when SPIRE is unavailable.

## What Changes

1. **mTLS enabled by default**: `mTLSMode` defaults to `permissive` (was implicitly `disabled`). Agents communicate over mTLS automatically when SPIRE is deployed.
2. **MTLSReady condition**: New status condition on AgentRuntime showing whether mTLS infrastructure (SPIRE) is available, with actionable error messages when it's not.
3. **Controller uses mTLS by default**: `--enable-verified-fetch` and `--enable-card-discovery` flags flip to `true`. SpiffeFetcher becomes the default card fetcher.
4. **JWS signing deprecation**: Legacy signing flags (`--require-a2a-signature`, `--signature-audit-mode`, `--enforce-network-policies`) emit deprecation warnings.
5. **ConfigMap mtls block**: Operator injects `mtls: mode: <value>` into authbridge ConfigMap based on AgentRuntime spec.

No breaking changes. Existing deployments without SPIRE get a clear `MTLSReady=False` condition and can opt out with `mTLSMode: disabled`.

## How It Works

The implementation leverages heavily what's already built:

- **Authbridge** (kagenti-extensions): mTLS is fully implemented across all proxy modes. No changes needed, only verification.
- **Operator**: The main work is (a) changing the `mTLSMode` kubebuilder default to `permissive`, (b) injecting the `mtls:` block into the authbridge ConfigMap, (c) adding `MTLSReady` condition logic, and (d) flipping flag defaults.
- **SPIRE detection**: The controller checks whether spiffe-helper volume mounts exist in the workload's pod template. If absent while mTLS is enabled, `MTLSReady=False/SPIREUnavailable`.
- **Config hash**: `resolvedConfig.MTLSMode` is already included in the config hash, so mTLSMode changes trigger rolling restarts automatically.

Of 24 tasks, 2 are already done (`[DONE]`), 1 is partial (`[PARTIAL]`), and the remaining 21 are new work, mostly focused on the operator side.

## When It Applies

**Applies when**:
- Deploying agents in clusters with SPIRE
- Controller fetching agent cards from live workloads
- Setting or changing `mTLSMode` on AgentRuntime CRs
- Migrating from JWS signing to mTLS-based identity

**Does not apply when**:
- Using Istio service mesh for mTLS (explicitly out of scope, separate effort in [#399](https://github.com/kagenti/kagenti-operator/issues/399))
- Working with user-supplied certificates or cert-manager (future iteration)
- Authbridge plugin changes (orthogonal)
- Cross-cluster agent federation (future work)

## Key Decisions

1. **SPIRE only, no Istio dependency**: The spec explicitly scopes to SPIRE-based mTLS. Istio service mesh mTLS (L4, pod-to-pod) is a separate effort tracked in [#399](https://github.com/kagenti/kagenti-operator/issues/399) and [PR #383](https://github.com/kagenti/kagenti-operator/pull/383). These are complementary (SPIRE = application-layer identity, Istio = infrastructure-layer encryption), not competing.

2. **Permissive as default, not strict**: Accepts both TLS and plaintext inbound. This allows gradual rollout without breaking existing agents that haven't enabled SPIRE yet.

3. **Controller uses go-spiffe SDK directly**: SpiffeFetcher uses `go-spiffe/v2` X509Source in-process, not file-based certificates. This is different from the data-plane approach (spiffe-helper sidecar writing PEM files).

4. **Feature-gated via existing CRD field**: No new CLI flag for mTLS enablement. The `mTLSMode` field on AgentRuntimeSpec is the control surface. `--enable-verified-fetch` is retained as a kill switch only.

5. **JWS signing soft-deprecated**: Flags default to false (already on main), warnings added. No code removal yet, just signaling the migration path.

## Areas Needing Attention

- **Overlap with Istio mTLS work**: [PR #383](https://github.com/kagenti/kagenti-operator/pull/383) (SharedTrust controller, already merged) and [Issue #399](https://github.com/kagenti/kagenti-operator/issues/399) (Istio auto-labeling) introduce Istio-based mTLS at the infrastructure layer. This spec operates at the application layer (SPIRE). Reviewers should verify these don't conflict at the configuration level (e.g., what happens when both SPIRE mTLS and Istio mTLS are active on the same workload).

- **ConfigMap schema contract**: T005 (injecting `mtls:` block) and T020 (verifying authbridge expects this shape) are the critical integration point. If the authbridge config schema doesn't match what the operator generates, mTLS silently fails.

- **MTLSReady condition gating Ready**: T016 proposes that `MTLSReady=False` should affect the overall `Ready` condition. The exact behavior (block Ready entirely vs. add a warning) needs careful design, since it changes the controller's availability semantics.

- **SPIRE detection heuristic**: Using spiffe-helper volume mount presence as a proxy for "SPIRE is available" may not cover all deployment patterns (e.g., SPIRE with CSI driver instead of init container).

## Open Questions

- How do SPIRE-based mTLS (this spec) and Istio-based mTLS ([PR #383](https://github.com/kagenti/kagenti-operator/pull/383), [Issue #399](https://github.com/kagenti/kagenti-operator/issues/399)) coexist? Is double encryption acceptable, or should one disable when the other is active?
- Should `MTLSReady=False` block the overall `Ready=True` condition, or just add a warning?
- Does the SPIRE detection heuristic (spiffe-helper volume check) cover SPIRE CSI driver deployments?

## Review Checklist

- [ ] Key decisions are justified (especially SPIRE-only vs Istio interaction)
- [ ] Scope matches the stated boundaries (no Istio, no user certs, no cross-cluster)
- [ ] Constitution compliance verified (all 5 principles addressed in plan.md)
- [ ] ConfigMap `mtls:` block schema matches authbridge expectations
- [ ] No conflict with existing SharedTrust controller ([PR #383](https://github.com/kagenti/kagenti-operator/pull/383))
- [ ] Task reconciliation against `main` is accurate ([DONE]/[PARTIAL] markers)
- [ ] Success criteria are achievable and testable
- [ ] Deprecation warnings are clear and actionable

---

<!-- Code phase sections are appended below this line by the phase-manager command -->
Loading