diff --git a/docs/proposals/cli-mcp-operator-design.md b/docs/proposals/cli-mcp-operator-design.md new file mode 100644 index 0000000..b33bb4b --- /dev/null +++ b/docs/proposals/cli-mcp-operator-design.md @@ -0,0 +1,533 @@ +# CLI MCP Operator + +**Status:** Final + +**Related:** [cli-mcp-operator-questions.md](cli-mcp-operator-questions.md) · [As-built design](../design.md) · [Credential proxy WHAT](credential-proxy-design.md) · [Umbrella proxy analysis](../../../docs/proposals/cli-mcp-credential-proxy.md) + +This document is the **HOW** for instance infrastructure: a Kubernetes operator that owns one MCP instance per CR. It is **not** the proxy design. This phase is for **building and testing** that operator (bash + session pods + investigation kubeconfig still mounted so `oc` works). Nothing is in production; the MCP is not used until the proxy exists. The next design pass adds proxy children to the same operator. + +This is an **open-source Kubernetes operator**. Docs describe the product any cluster can install. First-party internal deploy is one consumer of the OLM catalog, not part of the operator API. + +## Overview + +CLI MCP is a stateless, multi-replica MCP server (`cmd/server`) that creates per-session sandbox pods and proxies `bash` to them. Session pods and HMAC auth Secrets are already created in-process. Everything *around* that data plane — the MCP Deployment, Service, kube-rbac-proxy sidecar, NetworkPolicies, ServiceAccounts — would otherwise be a growing pile of YAML that cannot derive later objects (dummy kubeconfig, proxy CA, route list) from live Secrets. + +[Proxy Q1](credential-proxy-questions.md) already decided: a **real operator**, not an ensure-loop inside `cmd/server`. This design is that operator, implemented **before** the proxy: + +1. Convert today’s MCP into an operator-managed instance (same bash/session contract). +2. Implement and deploy that (no proxy components). +3. Return to the proxy design and add proxy children to this operator. + +``` +Cluster admin Cluster +───────────── ─────── +OLM catalog / kustomize ───────────► cli-mcp-operator (leader-elected) +CliMcpInstance CR ───────────► reconciler +admin Secrets (not children): ├── HMAC Secret (generate-once) + cli-mcp--kubeconfig ├── MCP Deployment (kube-rbac-proxy + server) + cli-mcp--tls (non-OpenShift) ├── Service (ClusterIP) + ├── MCP SA + Role/RoleBinding (pods; secret create/delete) + ├── sandbox SA (no RoleBindings, automount false) + └── NetworkPolicy (sandbox :8090 from this MCP) + │ + ▼ + cli-mcp-server × N (flags only; no CR watch) + ├── always claim unassigned or create on demand + └── session HMAC Secrets + operator also: warm pool + idle GC +``` + +An MCP client calls `/mcp` with `X-Session-ID` and `DELETE /sessions/{id}`. That path does not change. + +## Design Principles + +1. **Operator is the singleton; MCP is the data plane.** Leader-elected controller. MCP replicas stay stateless and horizontally scaled. Sessions are **not** CRs. +2. **MCP does not reconcile infrastructure, pool, or idle GC.** No Deployment/NP/CA ensure-loop in `cmd/server`. The server claims or creates session pods on the bash hot path and deletes on `DELETE /sessions/{id}` only. +3. **Testable bash contract before proxy.** One tool, HMAC `/exec`, real investigation kubeconfig mounted so operator tests can run `oc`. Isolation (dummy kubeconfig, egress lock) is the next design. Warm pool and idle GC move to the operator (Q5). +4. **CR + labels + ownership must be proxy-ready.** Instance identity, ownerRefs, and “admin provides investigation kubeconfig” are the extension points. Do **not** implement proxy children here, and do **not** freeze a `spec.proxy` API before that design. +5. **Operator does not mint investigation tokens.** That kubeconfig Secret is provided (GitOps, External Secrets, or `kubectl`) under the conventional name `cli-mcp--kubeconfig`. The operator does not put a secret ref on the CR (same as TLS). HMAC is an internal MCP↔agent shared secret: the operator **generate-once**s it (same pattern as the later proxy CA). The operator does not rotate HMAC on reconcile. +6. **`cmd/server` remains runnable without the operator.** Local stdio, unit tests, and `go run ./cmd/server` stay flag-driven. After Phase 3 that means claim + on-demand create only — not pool replenish or idle GC (operator: idle GC in Phase 4, pool in Phase 5). +7. **Fail closed on instance delete.** Removing the CR must not leave sandbox pods as unlabeled orphans forever. +8. **Portable Kubernetes, optional OpenShift.** The operator must install and reconcile on generic Kubernetes. OpenShift-only behavior (serving-cert annotation, SCCs) is detected or left to the admin, not required. (Q2) +9. **One repo, renamed to `cli-mcp-operator`, multiple images (Q1).** Kubebuilder go/v4 + operator-sdk in this module; `cmd/server` stays a data-plane binary. First-party GitOps is a consumer of the catalog, documented separately when we deploy it. + +## Architecture / How It Works + +### As-built (today) + +`cli-mcp-server` runs with flags. The process uses an in-cluster (or `--kubeconfig` path) **client-go** config to create sandbox pods in `--namespace` (flag default `tarsy`). The flag’s help text today says “for sandbox pods”; that is wrong — `--kubeconfig` is only `buildClientset`. The investigation Secret name is **not** that flag; it is hardcoded in `pkg/session.DefaultConfig` as `cli-mcp-investigation-kubeconfig`. Also hardcoded: SA `cli-mcp-investigation-sa`, agent port `8090`, CPU/memory requests+limits (`100m`/`500m`/`128Mi`/`512Mi`). `buildBasePodSpec` does **not** set `automountServiceAccountToken` (Kubernetes defaults **true**). Labels/annotations are `tarsy.redhat.com/{session-id,component,created-at,last-activity}` with `component=cli-mcp-sandbox`. Claim, discover, idle GC, and `unassignedSelector` are **component-only** (no instance id). + +HTTP mux (`pkg/server.NewMux`): `/mcp`, `/metrics`, `/live`, `/health`, `DELETE /sessions/{id}`. `/mcp` already sets `DisableLocalhostProtection: true` (loopback bind is still required). `/health` today does a **cluster-scoped** `get` on the Namespace object (`CoreV1().Namespaces().Get`) — that must not become MCP Role RBAC (Q13). Idle GC (`CleanupStale` / `startCleanupLoop`) uses `last-activity` (else `created-at`) and skips pods with no session-id. Warm pool: unassigned pods have no session-id and no auth Secret; claim is a label patch + Secret + `POST /assign`. `WarmPool` exists only when `--warm-pool-size > 0`. As-built `ReconcilePool` **creates on deficit only** — it does not trim surplus — and deletes unassigned pods older than **2× idle timeout** (drain extras when every replica replenished). The operator does **not** copy that timer; it trims surplus immediately. Sandbox readiness is an **exec** curl to loopback `/health` so kubelet does not need NP ingress. There is no egress NetworkPolicy in-tree (as-built `design.md` “egress limited” is aspirational). + +### This phase + +```mermaid +flowchart TB + Admin["Cluster admin: OLM or kustomize + CR + Secrets"] + Op["cli-mcp-operator"] + CR["CliMcpInstance"] + MCP["cli-mcp-server Deployment"] + Sandbox["sandbox pods + session Secrets"] + Client["MCP client"] + + Admin --> Op + Admin --> CR + Op -->|"reconcile children"| MCP + CR --> Op + Client -->|"HTTPS /mcp + X-Session-ID"| MCP + MCP -->|"claim or create; POST /exec"| Sandbox + Op -->|"warm pool + idle GC"| Sandbox +``` + +OLM is the OOTB install (bundle + catalog + CD, like claw-operator). `config/` kustomize remains the source of truth and a supported non-OLM path (`make deploy`). Helm can wrap the same manifests later; it is not v1. + +CSV install modes (Q4), same as claw-operator: **OwnNamespace** and **SingleNamespace**. Not MultiNamespace, not AllNamespaces. Instance children always live in the CR’s namespace. Watch scope is OLM OperatorGroup `targetNamespaces` (controller-runtime cache), **not** a product namespace in the binary. Do not copy claw’s `WATCH_NAMESPACE` meaning (that env is claw’s operator-config singleton lookup). We have no operator-config CR. + +**Ownership (Q5):** admin provides CR + investigation kubeconfig Secret + cluster RBAC (+ TLS Secret on generic Kubernetes). Operator reconciles instance infrastructure (including HMAC Secret generate-once), **warm pool**, **idle GC**, and instance teardown. MCP replicas **always try claim** of instance-labeled unassigned pods, else **create** on demand, wait Ready, HMAC `/exec`; `DELETE /sessions/{id}` deletes that session. Claim is **not** gated on `--warm-pool-size` (Q9: pool size must not roll MCP). No session CRD. + +**Two Pod writers:** MCP (claim / on-demand create / last-activity patch / explicit session delete) and operator (unassigned pool create/surplus delete, idle assigned GC, finalizer). Coordinate only via labels. Claim remains a resourceVersion label patch (first writer wins). After a successful claim, MCP must **not** signal replenish (`TriggerReplenish` goes away with `ReconcilePool`). The operator **Watches** instance sandbox Pods with a predicate: enqueue Create, Delete, and `session-id` appearing (claim refills the pool; a new assigned pod arms idle GC). Drop last-activity-only annotation patches and routine kubelet status (exec readiness probe is 10s; MCP patches last-activity on every `bash`). Ready/Failed/backoff updates enqueue when pool Ready cares (Phase 5). Session Secrets are listed/deleted with the pod; they are not the activity signal. Operator must **not** server-side-apply or delete pods that have `session-id` except idle GC / finalizer. Operator must **not** treat MCP-created assigned pods as drift to delete during SSA of “children.” Before deleting an unassigned surplus/hash-rebuild pod, **re-get** it and skip if `session-id` appeared (stale list vs claim race). Overlay/image change: recreate **unassigned** pods only; assigned sessions keep the old spec until DELETE / idle GC / CR delete. + +**Idle timer:** each instance reconcile lists assigned pods, deletes those past `idleTimeout` (`last-activity`, else `created-at`), and returns `requeueAfter` = time until the soonest remaining expiry (CR spec / `idleTimeout` changes also recompute). A quiet session still GCs when that delay fires. Do **not** enqueue or `AddAfter` on every activity patch — `requeueAfter` is a Reconcile result; the next timer fire rereads annotations and reschedules. Create and claim **must** enqueue; if those and last-activity are all filtered, a session can sit with no timer. + +### What the operator reconciles + +Namespaced children of a CR `metadata.name=oc` in the CR’s namespace (`cli-mcp-`): + +| Child | Role | +|---|---| +| Deployment `cli-mcp-oc` | kube-rbac-proxy `:8443` → MCP `127.0.0.1:8080`; N replicas | +| Service `cli-mcp-oc` | ClusterIP `:8443`; OpenShift serving-cert annotation **when on OpenShift** (Q2, Q13) | +| ServiceAccount `cli-mcp-oc` | MCP pod identity (in-cluster client for session objects) | +| Role + RoleBinding | MCP SA: pods create/get/list/watch/update/patch/delete (claim, last-activity). Secrets **create/delete** only (session auth Secrets). No secret get/list/watch. | +| ServiceAccount `cli-mcp-oc-sandbox` | Sandbox pods. No RoleBindings. Q12 | +| Secret `cli-mcp-oc-hmac` | MCP↔agent HMAC key (data key `key`). Generate-once; `ownerRef` → CR. Never overwrite if present. | +| NetworkPolicy sandbox ingress | `:8090` from pods labeled this instance + `component=server` (not caller identity). Q11 | + +**Secret RBAC:** HMAC is a file mount; investigation kubeconfig is a sandbox volume — MCP must not get/list/watch Secrets. Session Secret **create/delete** is namespace-wide (RBAC cannot prefix-limit `cli-mcp-sandbox-auth-*`). Operator `manager-role` keeps namespaced secrets get/list/watch/create/update/patch/delete (HMAC, Ready keys, idle GC/finalizer). That SA is the OperatorGroup target-namespace **secret trust boundary**; do not co-locate unrelated tenant Secrets. Do not `ownerRef` or delete admin kubeconfig/TLS. + +TLS for kube-rbac-proxy: mount Secret `cli-mcp--tls`. On OpenShift the operator sets the Service serving-cert annotation (platform creates the Secret). On generic Kubernetes the **admin** creates that Secret. The operator does not generate certs and does not `ownerRef` this Secret. + +Investigation kubeconfig: admin creates Secret `cli-mcp--kubeconfig` (key `kubeconfig`). This phase it is mounted on sandbox pods. No spec field, no `ownerRef` (same as TLS). Proxy pass unmounts it from the sandbox and mounts it on the proxy. + +All operator-owned objects get `ownerRef` → the CR and instance labels. Pool pods the operator creates get `ownerRef`; MCP on-demand session pods do not. + +**Not** created as CRs: session pods and per-session `cli-mcp-sandbox-auth-*` Secrets. MCP creates/claims them on the hot path. Operator keeps **unassigned** count equal to `warmPoolSize` (same pod spec as MCP): create on deficit, delete surplus immediately (oldest first; re-get and skip a pod that just gained `session-id`). Recreate unassigned pods when the desired sandbox spec/image/env/resources changes (hash), not on a timer. Do not copy as-built 2× idle age-drain (that existed because MCP replicas overshot and `ReconcilePool` would not trim extras). Idle-GC **assigned** sessions via `last-activity` at 1× `idleTimeout` (Q5). + +**Not** created by the operator: CRD (OLM/kustomize), operator Deployment, cluster-scoped RBAC, investigation kubeconfig Secret, MCP client SA, TLS Secret on non-OpenShift, OpenShift SCCs / namespace PSA, extra Secrets referenced from `spec.sandbox.env`. + +Q13: kube-rbac-proxy sidecar is always part of the MCP Deployment. Image from `RELATED_IMAGE_KUBE_RBAC_PROXY` (OLM `relatedImages`; not an instance workload). `--allow-paths` must cover the mux: `/mcp`, `/metrics`, `/live`, `/health`, `/sessions`. Admin owns `system:auth-delegator` on the MCP SA, the client SA, and the `/mcp` ClusterRole/Binding (plus `/sessions` if that is a separate nonResource URL). Repo ships sample cluster RBAC for operator tests. Clients are not required to carry a special pod label; kube-rbac-proxy is the MCP front door (Q11). + +### What the cluster admin applies + +``` +OLM: CatalogSource + OperatorGroup + Subscription + plus: + CliMcpInstance CR + investigation kubeconfig Secret `cli-mcp--kubeconfig` (key `kubeconfig`) + TLS Secret `cli-mcp--tls` (generic Kubernetes only; OpenShift serving-cert) + cluster-scoped RBAC for kube-rbac-proxy / MCP client (Q5, Q13) + OpenShift SCC / namespace PSA as needed (sandbox is non-root, drop ALL caps; Q2) + if the namespace is default-deny ingress: allow clients → MCP Service :8443 + (operator does not create an MCP ingress NP; kube-rbac-proxy is the front door) +``` + +Without OLM: `make deploy` / `kubectl apply -k config/default`, then the same CR + Secrets. Helm is a later optional wrapper, not a second source of truth. + +Investigation tokens in the kubeconfig Secret are provisioned outside this operator (the operator does not mint cluster identities on this or other clusters). + +### How the MCP process is configured + +The operator **renders Deployment args/env from the CR**. The MCP binary does **not** watch the CR (Q9). Spec changes that affect the MCP process update the Deployment; kube rolls replicas. Pool/idle fields are consumed by the operator in place (no MCP restart). + +Flags the operator sets (existing + small additions): + +| Flag | Source | +|---|---| +| `--transport http --stateless --address 127.0.0.1:8080` | fixed for in-cluster | +| `--namespace` | CR namespace | +| `--sandbox-image` | resolved `spec.sandbox.image` or `RELATED_IMAGE_SANDBOX` (Q7, Q16) | +| `--hmac-key-file` | mount of operator-generated Secret `cli-mcp--hmac` (file is the `key` entry) | +| `--idle-timeout`, `--warm-pool-size` | **not** passed in-cluster — `spec.sandbox` is operator-only (Q5, Q6, Q9). After Phase 3 they must **not** start MCP replenish or idle GC (no dual path). Keep the flags so old CLIs still parse if useful; they have no in-cluster effect. | +| `--instance-name` | **new** — CR `metadata.name` (labels). Required; no default. | +| `--kubeconfig-secret` | **new** — always `cli-mcp--kubeconfig` (not a spec field; not `--kubeconfig`). Required; no default. | +| `--sandbox-service-account` | **new** — operator-owned `cli-mcp--sandbox` (not a spec field). Required; do **not** default to `cli-mcp-investigation-sa` (Q12). | +| `--sandbox-cpu-request`, `--sandbox-cpu-limit`, `--sandbox-memory-request`, `--sandbox-memory-limit` | **new** — `spec.sandbox.resources`. Empty/omitted → DefaultConfig `100m`/`500m`/`128Mi`/`512Mi`. | +| `--sandbox-image-pull-policy` | **new** — `spec.sandbox.imagePullPolicy`. | +| `--sandbox-env` | **new** — JSON `[]corev1.EnvVar` (includes `valueFrom`). Local may omit. Overlay change updates these args and rolls MCP (Q9). Operator maps spec → `SandboxConfig` in-process for pool; MCP fills the same struct from flags. No overlay ConfigMap. | + +`--kubeconfig` stays the MCP process’s client-go config (empty = in-cluster). It is not the investigation Secret. + +`/mcp` already sets `DisableLocalhostProtection: true` in `NewMux`. Do not rely on `MCPGODEBUG`. Loopback `--address` stays mandatory (`ValidateTransportFlags`). + +**Claim vs pool size:** in-cluster MCP **always** lists/claims instance-labeled unassigned pods, then on-demand creates if none. Do not gate that on `--warm-pool-size` (today `NewSessionManager` only builds `WarmPool` when the flag is `> 0` — that would skip claim unless the operator passed the flag and rolled MCP on `0 ↔ N`, which Q9 forbids). With `warmPoolSize: 0` the list is empty and create runs as today. + +Local/dev: `cmd/server` stays flag-only. Drop the `tarsy` namespace default (operator always passes `--namespace`). Tests and the operator pass namespace / SA / kubeconfig Secret / instance name **explicitly**. After Phase 3, `go run ./cmd/server` still does claim + on-demand create; it does **not** replenish a pool or idle-GC (run the operator for that, or DELETE sessions yourself). + +Images (Q7, Q16): OLM `relatedImages` → `RELATED_IMAGE_SERVER` / `RELATED_IMAGE_SANDBOX` / `RELATED_IMAGE_KUBE_RBAC_PROXY` on the operator. MCP container **always** uses `RELATED_IMAGE_SERVER` (no `spec.serverImage`; local/dev overlays that env on the operator). Empty `spec.sandbox.image` → our default sandbox class; **set `spec.sandbox.image` to run a different class**. No CRD-baked default tags. Status records `resolvedSandboxImage` only (two sources). MCP image is on the Deployment / operator env, not CR status. + +**Shared pod spec:** operator pool pods and MCP on-demand pods must call the same builder in `pkg/session` (export today’s `buildBasePodSpec`). The builder takes operator-owned base (SA, automount false, instance/component labels, probes, kubeconfig mount this phase, today’s non-root / drop-caps security context) plus the class overlay from `SandboxConfig` (image, resources, env, imagePullPolicy). Session token env (`SANDBOX_AUTH_TOKEN`) is **assigned / on-demand only**; unassigned pool pods get the token via `POST /assign`, not that env. The operator image may import `pkg/session`. `cmd/server` must not import `internal/controller`. `pkg/session` must not import `api/`. Empty `spec.sandbox.resources` → **today’s DefaultConfig requests/limits**, not BestEffort. Pool recreate hash includes the overlay, not only the image tag. A CR `securityContext` / extra volume field is later (do not stub); the builder still ships today’s pod security context now. + +**Investigation kubeconfig Secret** is admin-provided, name `cli-mcp--kubeconfig`, key `kubeconfig` (as-built: `KUBECONFIG=/config/kubeconfig`). Same convention as TLS (`cli-mcp--tls`): no spec field, no `ownerRef`. This phase the operator **mounts it on sandbox pods** so tests can run `oc`. Ready requires that key present and non-empty (`SecretKeysInvalid` if not); it does not parse the kubeconfig. Proxy pass: **unmount from sandbox**, keep the Secret, mount it on the proxy, derive dummy + routes. Do not delete the Secret when the proxy lands. + +**HMAC Secret:** operator creates `cli-mcp--hmac` if missing (random bytes, key `key`), `ownerRef` → CR, mounts into every MCP replica. Do not overwrite an existing Secret (generate-once). Do not rotate on reconcile — that would invalidate live session tokens. If the Secret is deleted, the operator recreates it and must roll the MCP Deployment (stamp the Secret hash/resourceVersion on the pod template). If it exists but `key` is missing or empty, Ready is `SecretKeysInvalid` (do not fill it in). Local `cmd/server` still uses `--hmac-key-file`. No `spec.hmacKeySecretRef`. + +### Instance identity + +v1 of a given install may be a single CR. The **API** is already multi-class: a second CR in the same namespace is another sandbox class (different image/env), not a snowflake Deployment. Selectors must stay instance-specific so those CRs do not share NetworkPolicies or session GC. + +Q8: CR `metadata.name` is the instance id. Labels **and** annotations live under `cli-mcp.redhat.com`. No `tarsy.redhat.com` keys (nothing in production; no migration). + +- `cli-mcp.redhat.com/instance=` on MCP pods, sandbox pods, session Secrets, and later proxy pods. +- `cli-mcp.redhat.com/component=sandbox` \| `server` — replace as-built `component=cli-mcp-sandbox`. +- `cli-mcp.redhat.com/session-id` on assigned sandbox pods and their auth Secrets. +- Annotations `cli-mcp.redhat.com/created-at` and `cli-mcp.redhat.com/last-activity` (RFC3339). MCP still patches last-activity on `bash`. Operator idle GC **reads** them on reconcile; the Pod watch does **not** enqueue on those patches. Unassigned pool pods have created-at and **no** session-id (idle GC skips them). +- Children named `cli-mcp-` (SA `cli-mcp--sandbox`, Secret `cli-mcp--kubeconfig`). CEL: CR name must leave room for the longest child (`cli-mcp-` + name + `-kubeconfig` ≤ 63 → name ≤ 44). Sample name `oc` is fine. +- `app.kubernetes.io/name=cli-mcp-server` may stay on MCP pods as a secondary label; sandbox NP selectors use instance/component labels above. + +Pool/claim/GC selectors are **component + instance**, plus `!session-id` for unassigned. As-built selectors are component-only and would mix two CRs in one namespace. + +### CR delete and session pods + +Delete CR destroys the instance. Rolling the MCP Deployment does not. + +Q10: **finalizer** `cli-mcp.redhat.com/finalizer` on `CliMcpInstance`. On delete, do not re-ensure MCP replicas. Scale the MCP Deployment to 0, wait until this instance’s `component=server` pods are gone, then list instance-labeled sandbox pods and session Secrets, delete them, wait until gone, then remove the finalizer (name stays taken until then). **`ownerRef` → CR** on operator-created children (MCP Deployment, Service, NPs, SAs, Role/RB, HMAC Secret, pool pods). After the finalizer drops, Kubernetes GCs these — do not wait for them in the finalizer. MCP does not set `ownerRef` on on-demand session pods. Never `ownerRef` session pods to the MCP Deployment. Do not delete the admin kubeconfig or TLS Secrets. + +### NetworkPolicy this phase (no proxy) + +As-built security, not the proxy topology: + +- MCP `:8443` has **no** client-label NetworkPolicy. kube-rbac-proxy (SA token + `/mcp` RBAC) is the front door. Clients are not required to set a pod label (many cannot). +- Sandbox `:8090` from pods labeled this instance’s `component=server`. That selector is not identity (spoofable in-namespace). Covers `/exec` and warm-pool `POST /assign`. `/assign` stays once-unauthenticated; HMAC is not kube RBAC. Same namespace trust boundary as secrets: do not co-locate untrusted pods. +- Sandbox **egress stays unrestricted** (today’s token-replay gap). Closing it without a proxy needs either a kube-API allowlist (a stopgap that must go away when the proxy exists) or the MITM proxy. **Do not** pretend to fix token-replay in this phase. + +Q11: operator creates the sandbox ingress NP only. No MCP ingress NP, no egress policy, no EgressFirewall in this phase. + +### Sandbox identity this phase + +Q12: operator creates a dedicated sandbox SA (`cli-mcp--sandbox`) with **no RoleBindings**. MCP sets that SA on sandbox pods and `automountServiceAccountToken: false`. The admin-provided investigation kubeconfig Secret **stays mounted** so `oc` works when testing the operator before the proxy exists. + +Dummy kubeconfig is proxy work — without a proxy it would make `oc` fail and block operator tests. The investigation SA is **not** the sandbox pod identity (that name would imply the pod *is* the investigation subject; accidental automount would project a useful host-cluster token). + +### Later: proxy (out of scope, extension point) + +The same CR and reconciler grow children: proxy Deployment+Service, CA Secret, dummy kubeconfig ConfigMap, route ConfigMap, four NPs (sandbox egress, proxy ingress/egress, …). MCP flags gain `HTTPS_PROXY` / dummy mount via pod-spec changes in `pkg/session`. The admin still provides the **real** kubeconfig Secret (`cli-mcp--kubeconfig`); the operator derives dummy + routes and **stops mounting the real Secret on sandbox pods**. + +Do not add `spec.proxy` in this CRD (Q14). Same Kind later; additive fields and children in the proxy pass. + +## Core Concepts + +| Concept | Role | +|---|---| +| **CliMcpInstance** | CRD `cli-mcp.redhat.com/v1alpha1`. One MCP **class/instance** (one sandbox image + config, one MCP Deployment). Sample name `oc` is the class we ship; `curl` / BYO is another CR (Q16). Group stays on `redhat.com` until real external community justifies a new domain (Q2). | +| **Operator** | Leader-elected controller. Owns instance infrastructure. Does not serve `/mcp`. | +| **MCP server** | Existing `cmd/server`. Bash, session claim/create, `/exec`. Flag-driven. Multi-replica. Not the pool or idle GC. | +| **Sandbox agent** | Existing `cmd/agent`. Unchanged. | +| **Warm pool** | Operator-maintained unassigned pods for the instance. MCP claims; does not replenish. | +| **Instance label** | Discriminator for NPs, session list/GC, and later proxy ingress. | +| **Admin-provided Secrets** | Investigation kubeconfig `cli-mcp--kubeconfig` and TLS on generic Kubernetes (`cli-mcp--tls`). Conventional names; not spec fields. Operator does not generate. | +| **HMAC Secret** | Operator generate-once per instance. Internal MCP↔agent key, not a cluster identity. | +| **Session objects** | Pods + auth Secrets. Not CRs. MCP creates/claims on the hot path; operator pool/GC/finalizer. | + +## Repository layout (target) + +GitHub repo and Go module: `github.com/codeready-toolchain/cli-mcp-operator` (rename of this repo; Q1). Images stay independently named. + +This is a **Kubebuilder `go.kubebuilder.io/v4` project that also ships data-plane binaries**, not “add a controller to the current Makefile.” Types live **in this module** (`api/v1alpha1`). Do **not** put `CliMcpInstance` in `codeready-toolchain/api` and do **not** copy host/member’s root `controllers/` + CRD-dispatch layout (that exists because two operators share one CRD set; we have one operator and one CRD). + +### Tree + +``` +cli-mcp-operator/ + PROJECT # go.kubebuilder.io/v4 + operator-sdk manifests/scorecard plugins + Makefile # Kubebuilder/claw spine; port server/agent targets (see Make targets) + hack/boilerplate.go.txt # controller-gen header + api/v1alpha1/ # CliMcpInstance types + generated deepcopy + cmd/operator/main.go # manager (see Manager entrypoint) + cmd/server/ # existing MCP; must not import internal/controller + cmd/agent/ # existing sandbox agent + # later: cmd/proxy/ + internal/controller/ # reconciler, Go child builders, pool, idle GC, status, envtest + pkg/ # existing session/server/agent/tools — shared pod spec lives here + config/ # kustomize source of truth (install the *operator*, not instance children) + crd/bases/ # generated CRD YAML + default/ + manager/ # operator Deployment; command /manager; RELATED_IMAGE_* env (Q7) + rbac/ # operator manager-role (not the MCP instance Role) + samples/ # CliMcpInstance sample + manifests/ # CSV base for operator-sdk generate bundle + scorecard/ + test/e2e/ # later; envtest stays in internal/controller + Containerfile.operator # new; do not use an unsuffixed Containerfile (repo already has two) + Containerfile.server # keep + Containerfile.agent # keep + bundle.Dockerfile # operator-sdk generate bundle + bundle/ # generated; commit if CD diffs it (claw pattern) +``` + +| Image | Source | Built binary (in image) | +|---|---|---| +| `cli-mcp-operator` | `cmd/operator` | `/manager` | +| `cli-mcp-server` | `cmd/server` | server binary (unchanged) | +| `cli-mcp-sandbox` | `cmd/agent` | agent binary (unchanged) | +| `cli-mcp-proxy` | later `cmd/proxy` | later | + +Operator and server are **separate images**. The server binary must not link controller-runtime. OpenShift is not required (Q2). OLM is the default install; `config/` kustomize remains usable without it (`make deploy`). + +### Manager entrypoint (`cmd/operator`, binary `manager`) + +Kubebuilder scaffolds `cmd/main.go`. This repo already has `cmd/server` and `cmd/agent`, so **move** the manager to `cmd/operator/main.go` after init (one-time Makefile/Containerfile patch). Keep the **binary name `manager`** and Containerfile `ENTRYPOINT ["/manager"]` so `config/manager/manager.yaml` stays on the Kubebuilder rails (`command: ["/manager"]`). + +`go build -o bin/manager ./cmd/operator` matches `go build ./cmd/server` and `./cmd/agent`. Later proxy is `cmd/proxy/`. Do not leave a second `cmd/main.go` that means “the operator.” + +`kubebuilder create api` / `create webhook` look for `cmd/main.go`. This phase has one CRD and no webhooks — scaffold once, then do not re-run those commands without updating the path. + +### Scaffolding (Phase 2) + +Do not grow the current ~120-line Makefile plus `make/git.mk`. Init in a throwaway directory (same module path), then merge: + +``` +operator-sdk init \ + --plugins=go.kubebuilder.io/v4 \ + --domain redhat.com \ + --repo github.com/codeready-toolchain/cli-mcp-operator \ + --project-name cli-mcp-operator + +operator-sdk create api \ + --group cli-mcp --version v1alpha1 --kind CliMcpInstance \ + --resource --controller +``` + +That yields group `cli-mcp.redhat.com`. Merge `PROJECT`, `config/`, `hack/`, and the Kubebuilder Makefile into this repo; move `cmd/main.go` → `cmd/operator/main.go`; port existing server/agent image targets. Copy claw-operator’s **bundle/CD Makefile patterns** (kustomize overlays so `make deploy` does not mutate committed files; CSV `REPLACE_*` relatedImages; `opm` catalog). Do **not** copy claw’s `internal/assets` kustomize-in-operator, `WATCH_NAMESPACE` operator-config lookup, or host/member `make/*.mk` + `build/Dockerfile`. + +Pin tool versions the same way claw does (`LOCALBIN` + `go-install-tool` / download): `controller-gen`, `kustomize`, `setup-envtest`, `operator-sdk`, `opm`, `golangci-lint`. Start from current claw pins and bump only if the scaffolded `controller-runtime` requires it. + +### Import and generate boundaries + +``` +cmd/server, cmd/agent → pkg/* only +cmd/operator → internal/controller + pkg/session (pod spec / class overlay) +cmd/server ✗ internal/controller +pkg/session ✗ api/ (CRD-agnostic SandboxConfig) +``` + +- **`pkg/session`:** CRD-agnostic. Export today’s `buildBasePodSpec` (instance+component labels, dedicated sandbox SA, `automountServiceAccountToken: false`, kubeconfig mount, today’s security context) and merge `SandboxConfig` overlay (image, resources, env, imagePullPolicy). Operator pool pods and MCP on-demand pods call this builder. **Do not** import `api/v1alpha1` from here. **Do not** leave warm-pool replenish or idle-GC tickers here for the operator to call — those move to `internal/controller` (Phase 3 removes `StartPool` / `StartReconciler` / `ReconcilePool` / `TriggerReplenish` / `startCleanupLoop` / `CleanupStale` from the MCP process). Keep `ClaimPod` (always available, not gated on `WarmPoolSize`). +- **`internal/controller`:** Reconcile, finalizer, HMAC generate-once, child Apply, warm pool, idle GC, status. MCP namespaced Role (pods + secret create/delete) is a **child the reconciler applies**, not `+kubebuilder:rbac` on the manager. Start small (`climcpinstance_controller.go`, `children.go`, `idle.go` in Phase 4, `pool.go` / `status.go` as they grow in Phase 5, `suite_test.go` for envtest). Do not clone claw’s large `claw_*.go` surface up front. +- **`controller-gen` paths:** `./api/...` and `./internal/...` (and `./cmd/operator/...` if markers land there). Do **not** scan `pkg/` for RBAC. Do **not** copy claw’s `paths="./cmd/..."` if that would imply `cmd/server` is a controller. Operator `manager-role` is cluster/watch RBAC for the controller; it is not the MCP SA Role. +- **Instance children are Go builders**, not embedded kustomize. `config/` installs the operator. Claw’s `internal/assets` + krusty pattern fits a large third-party operand YAML graph; HMAC, `RELATED_IMAGE_*`, instance labels, and two Pod writers do not. + +### Containerfiles + +Keep suffixed names: `Containerfile.operator`, `Containerfile.server`, `Containerfile.agent` (later `Containerfile.proxy`). Makefile uses `-f`. + +| Image | COPY into build | Must not COPY | +|---|---|---| +| operator | `go.mod`/`go.sum`, `cmd/operator/`, `api/`, `internal/`, **`pkg/`** (operator imports `pkg/session`) | `cmd/server`, `cmd/agent` | +| server | `go.mod`/`go.sum`, `cmd/server/`, `pkg/` | `internal/`, `api/` (keeps the import boundary honest) | +| agent | `go.mod`/`go.sum`, `cmd/agent/`, `pkg/` as needed | `internal/`, `api/` | + +Today’s server/agent Containerfiles `COPY . .` — tighten them in Phase 2 so `cmd/server` cannot accidentally compile `internal/controller`. + +Operator Deployment env (`config/manager`): `RELATED_IMAGE_SERVER`, `RELATED_IMAGE_SANDBOX`, `RELATED_IMAGE_KUBE_RBAC_PROXY` (Q7). Do not add claw’s `WATCH_NAMESPACE` as an operator-config singleton lookup. OLM OperatorGroup still scopes the cache via the usual operator-sdk/controller-runtime watch-namespace mechanism (Q4). + +### Make targets + +Replace the current Makefile with the Kubebuilder/claw Makefile, then **extend** it. Today `make build` means server+agent; after this it must still compile every binary CI cares about. Port `make/git.mk` `GIT_COMMIT_ID` / `BUILD_TIME` into operator+server+agent ldflags (Containerfiles today bake `github.com/codeready-toolchain/cli-mcp-server/pkg/version` — update that path in Phase 2). Do **not** copy claw’s `go test … -coverpkg=./internal/...` — this module’s tests live in `pkg/` as well as `internal/`. + +Today `make run` is `go run ./cmd/server`. After Phase 2, Kubebuilder `make run` is the **operator**. Keep `make run-server` / `run-agent` so the data plane is still one target away. + +| Target | Meaning | +|---|---| +| `make generate` | deepcopy (`controller-gen object`) | +| `make manifests` | CRDs + operator `manager-role` into `config/` | +| `make build` | **all** binaries: `bin/manager` from `./cmd/operator`, plus server and agent | +| `make build-operator` / `build-server` / `build-agent` | one binary each (`-o bin/manager` for the operator) | +| `make test` | unit (`pkg/`, `cmd/…`) + envtest; exclude `test/e2e`. `KUBEBUILDER_ASSETS` from `setup-envtest`. Cover `pkg/` **and** `internal/` — not claw’s `coverpkg=./internal/...` only | +| `make run` | `go run ./cmd/operator` (Kubebuilder’s “run manager on the host”) | +| `make run-server` / `run-agent` | existing data-plane binaries (rename of today’s `make run`) | +| `make install` / `uninstall` | CRDs only (`config/crd`) | +| `make deploy` / `undeploy` | operator from `config/default` via a **temporary overlay** (claw: do not mutate committed `kustomization.yaml` image tags) | +| `make container-build` | operator image (`-f Containerfile.operator`, `IMG ?= cli-mcp-operator:latest`) | +| `make container-build-server` / `container-build-agent` | existing images (`SERVER_IMG` / `SANDBOX_IMG`). Keep `image-server` / `image-agent` as aliases if useful | +| `make bundle` | `operator-sdk generate kustomize manifests` + `generate bundle` from `config/`; CSV `relatedImages` for operator, server, sandbox, kube-rbac-proxy (placeholders `REPLACE_*` like claw) | +| `make bundle-build` / `bundle-push` | bundle image | +| CD catalog | claw-equivalent `opm` render + catalog image + `relatedImages` substitution for all four images | + +`make deploy` without OLM remains supported. Helm is not a Makefile target in v1. + +### CR API (v1) + +Typed instance spec (Q6, Q16): one CR = one **sandbox class** (image + config) + one MCP Deployment. `spec.replicas` (default **1**, minimum 1; sample may use 2). `spec.sandbox` (image, idle default **30m**, pool default **0**, resources, env, imagePullPolicy). Optional `spec.serverContainer` (MCP container resources / imagePullPolicy only — image is `RELATED_IMAGE_SERVER`, not a spec field). No HMAC secret ref (operator-owned). No investigation kubeconfig secret ref (conventional name `cli-mcp--kubeconfig`). No `spec.args` passthrough. No `spec.proxy` in this revision (Q14). No `spec.sandbox.type` enum and no `PodTemplateSpec`. + +**Sandbox image contract:** the container must run a compatible agent (`/health`, `/exec`, `/assign` on the agent port, HMAC) and still have `curl` for the as-built exec readiness probe. Typical custom image: `FROM` our sandbox image or COPY `cmd/agent`. This operator does not run arbitrary pods. + +**Operator-owned on every sandbox pod** (not spec fields): dedicated SA, `automountServiceAccountToken: false`, instance/component labels, probes, agent port, kubeconfig mount **this phase** (Q12), today’s non-root / drop-caps security context. Session token env only when assigned (on-demand create) or via `/assign` (claimed pool). User `env` entries for `KUBECONFIG`, `HOME`, `SANDBOX_AUTH_TOKEN` are ignored (operator wins). + +**User-mergeable now** on `spec.sandbox`: `image`, `resources`, `env` (`[]corev1.EnvVar`, including `valueFrom`), `imagePullPolicy`. Empty `image` → `RELATED_IMAGE_SANDBOX` (the class we ship). Set `image` for another class (first-class, not a test pin). Empty `resources` → as-built DefaultConfig requests/limits (`100m`/`500m`/`128Mi`/`512Mi`), not an empty ResourceRequirements. Extra Secrets in `valueFrom` are admin-owned; they are **not** Ready gates (a missing one shows up as a non-Ready sandbox pod). + +**Later, same object** (do not stub now): extra volumes/mounts, `imagePullSecrets`, args, `securityContext` override, optional kubeconfig mount, agent port. A class that does not need kubeconfig is an additive change (skip mount if Secret absent), not a new Kind. + +```yaml +apiVersion: cli-mcp.redhat.com/v1alpha1 +kind: CliMcpInstance +metadata: + name: oc # instance / class id; another CR (e.g. aws) is another class + namespace: cli-mcp +spec: + replicas: 2 # CRD default 1; sample uses 2 + sandbox: + # image omitted → RELATED_IMAGE_SANDBOX (agent + oc/jq we ship) + # image: quay.io/example/cli-mcp-sandbox-aws:1.2.3 # BYO class; must speak the agent contract + idleTimeout: 30m + warmPoolSize: 0 + # resources: {} # omitted/empty → DefaultConfig 100m/500m/128Mi/512Mi + # imagePullPolicy: IfNotPresent + # env: + # - name: AWS_REGION + # value: us-east-1 + # - name: AWS_SHARED_CREDENTIALS_FILE + # valueFrom: + # secretKeyRef: + # name: aws-cli-creds + # key: path + # serverContainer: optional resources / imagePullPolicy for the MCP container +status: + warmPoolReady: 0 + warmPoolDesired: 0 + resolvedSandboxImage: "" # spec.sandbox.image or RELATED_IMAGE_SANDBOX + conditions: + - type: Ready # aggregate: infra + MCP Available + pool init; does not flap on claim (Q15) + - type: WarmPoolReady # optional; strict unassigned Ready count +``` + +Q15: `Ready` is investigation kubeconfig Secret `cli-mcp--kubeconfig` present with non-empty `kubeconfig` (TLS Secret on generic Kubernetes with `tls.crt`/`tls.key`; HMAC with non-empty `key`), other children applied, and MCP Deployment Available (kube-rbac-proxy + server). Missing object → `SecretsNotFound`; missing/empty required key → `SecretKeysInvalid`; do not parse kubeconfig. Extra Secrets referenced from `spec.sandbox.env` are **not** Ready gates. If `warmPoolSize > 0`, first Ready (and a pool-size increase) waits until `warmPoolReady >= warmPoolDesired`. After that, claim/replenish does not clear `Ready` unless a pool pod is Failed/backoff or the shortfall lasts past a replenish deadline. Assigned sessions are not part of Ready. Always publish `warmPoolReady` / `warmPoolDesired`. + +## Implementation Plan + +Do **not** start the paused proxy work. This plan is operator + current MCP only. The MCP is not in production; **later phases may break earlier MCP flag defaults, labels, and deploy YAML.** Prefer that over a dual code path. + +Phase 3 removes MCP `startCleanupLoop` / `CleanupStale`. **Idle GC of assigned sessions lands in Phase 4** with instance children (same label list as the finalizer, plus `last-activity`; not the two-writer pool). Phase 5 is warm pool + Ready pool-init / no-flap-on-claim only. The first catalog therefore has a janitor. + +A **phase is a milestone**. It does not always produce a PR (rename, GitOps, design). **Code phases: exactly one PR.** Do not merge Phase 4+5 (children + idle GC vs two-writer pool are different reviews). Do not split labels / `/health` / drop-pool into their own PRs (too small; they are one MCP contract). + +### Testing (all code phases) + +**Tests are part of each code PR, not a cleanup phase at the end.** The goal after Phase 5 is **strong, practical coverage** of the operator and the MCP hot path — not every theoretical branch. + +- Cover **this phase’s behavior** with automated tests that are cheap and stable to write *now*. Prefer unit tests and envtest. Do not skip tests because “e2e will catch it later.” +- **Defer** a test to a later phase when it is clearly easier or only meaningful once that code exists (example: kind e2e of a full instance waits until Phase 4; pool/claim/Ready-no-flap waits until Phase 5; idle assigned GC is Phase 4 with children). Note the deferral in the PR, do not drop it. +- Do **not** add brittle, duplicative, or scenario-fiction tests. If a check is painful to automate and low value, skip it and say so. +- **Kind e2e** (same idea as claw-operator: `test/e2e`, `make test-e2e`, local Kind cluster, load images, deploy, assert instance behavior) is a plan goal. Phase 2 may only wire the harness; the first real e2e belongs when an instance can come Ready (Phase 4+). Grow it as features land. Do not design the cases in this document. +- CI runs whatever automated tests exist at that phase (`make test`; e2e when the target exists). + +**Coverage check (required on every code PR).** Before calling the phase done, the implementer (human or agent) must answer, against **this section** and the **diff they actually wrote**, not against a closed list of cases: + +1. Did I do my best to cover the changes in this PR with practical automated tests? +2. What did I defer, and why is a later phase the better home? +3. What did I skip as not practical, and why is that acceptable? + +Per-phase **Test tips** below are hints for the implementer, not an exhaustive suite and not a pass/fail checklist. Missing a tip is fine if the coverage check still holds; inventing tests the tip never mentioned is also fine if they are practical. + +### Phase 0 — Decisions (done) — **no PR** + +Walked [cli-mcp-operator-questions.md](cli-mcp-operator-questions.md). This document is Final. + +### Phase 1 — GitHub rename — **no PR** + +Rename the GitHub repo `cli-mcp-server` → `cli-mcp-operator` (settings; issues/PRs/redirects kept). No code change in this phase. Phase 2’s first commit sets the Go module path to match. + +- **Verify:** repo URL is `codeready-toolchain/cli-mcp-operator`; old URL redirects. + +### Phase 2 — Scaffold the operator repo — **PR** + +Follow [Repository layout](#repository-layout-target). **No reconciler product logic** (stub from `create api` only). **MCP behavior stays as-built** except import-path churn from the module rename. + +- Module path `github.com/codeready-toolchain/cli-mcp-operator`; fix imports. +- `operator-sdk init` + `create api` in a throwaway dir; merge `PROJECT`, `config/`, `hack/`, Kubebuilder Makefile. Move `cmd/main.go` → `cmd/operator/main.go`; `go build -o bin/manager ./cmd/operator`. +- Replace the current Makefile; port `build-server` / `build-agent` / image targets **and** `make/git.mk` ldflags. `make build` compiles manager + server + agent. `make run` is the operator; add `run-server` / `run-agent`. `LOCALBIN`: controller-gen, kustomize, setup-envtest, operator-sdk, opm. +- `Containerfile.operator`; operator image COPYs `pkg/`. Tighten server/agent Containerfiles (no `internal/` / `api/`). Update ldflags module path in both existing Containerfiles. +- `config/manager`: `RELATED_IMAGE_*`. No claw operator-config `WATCH_NAMESPACE`. +- **OLM artifacts, not catalog CD:** CSV base, `make bundle` with `REPLACE_*` relatedImages, `bundle.Dockerfile`. Copy claw overlay/`opm` **Makefile** patterns. Do **not** turn on master catalog publish yet (that would ship a no-op operator). +- **CI/CD:** keep the required check job id `build-test-coverage`. Add the operator image to the CI build matrix. Quay push of `cli-mcp-operator` on master may start here (image only). Catalog publish waits for Phase 4. `make test` must still run `pkg/` tests (do not copy claw `coverpkg=./internal/...`). +- envtest wired (`make test`); empty reconciler is enough. +- **Test tips:** keep existing MCP tests green after the module rename. Kind e2e harness may be stubbed (claw-style `test/e2e`); no instance to assert yet. +- **Done when:** `make generate` / `manifests` clean; `make build` → `bin/manager` + server + agent; `go build ./cmd/server` does not type-check `internal/controller`; operator image builds; `make bundle` validates; `make deploy` works without OLM; `make run-server` still runs the MCP. +- **Coverage check:** [Testing](#testing-all-code-phases) against this PR’s diff. +- **Out of this PR:** CR field logic, HMAC, children, pool, MCP label/flag changes, GitHub CD catalog push. + +### Phase 3 — MCP contract (hot path only) — **PR** + +Depends on Phase 2 (shared module / `pkg/session` layout). **Breaks** as-built labels, `tarsy` default namespace, and MCP-side pool/GC. Flag-driven `cmd/server` still runs without the operator. + +- Export `buildBasePodSpec`; extend `SandboxConfig` (instance name, env, imagePullPolicy, automount, `ResourceRequirements` or equivalent). Merge class overlay there — **not** by importing `api/v1alpha1`. +- Labels/annotations `cli-mcp.redhat.com` (drop `tarsy.redhat.com`). `--instance-name`, `--kubeconfig-secret`, `--sandbox-service-account` — all required, no production defaults. `--sandbox-cpu-request` / `--sandbox-cpu-limit` / `--sandbox-memory-request` / `--sandbox-memory-limit` (DefaultConfig if empty), `--sandbox-image-pull-policy`, `--sandbox-env` as JSON `[]corev1.EnvVar` (local may omit). Fix `--kubeconfig` help text (client-go, not the investigation Secret). +- Discover / claim / cleanup / `unassignedSelector` must include **instance + component** (as-built is component-only and would mix two CRs). +- Sandbox pods: dedicated SA name from flags, `automountServiceAccountToken: false`, still mount the real kubeconfig Secret (Q12). +- **Always claim then create** (`ClaimPod` even when `WarmPoolSize == 0`). Remove `StartPool`, `StartReconciler`, `ReconcilePool`, `TriggerReplenish`, `startCleanupLoop`, `CleanupStale`. Claim + `POST /assign` + on-demand create remain. +- `/health` lists pods in the process namespace (not `get` Namespace) so later MCP Role can stay namespaced (Q13). +- Drop `--namespace` default `tarsy`. Tests pass namespace/SA/secret/instance explicitly (today `testNamespace = "tarsy"` in `pkg/session`). +- **Test tips:** update or drop tests that assumed MCP pool replenish / `CleanupStale` / component-only selectors; exercise the new contract (labels, instance isolation, automount, always-claim, last-activity) where unit tests already live. Kind e2e of a live instance waits for Phase 4. +- **Done when:** `cmd/server` is flag-driven without operator packages; claim + on-demand create still work; MCP no longer replenishes the pool or runs idle GC. +- **Coverage check:** [Testing](#testing-all-code-phases) against this PR’s diff. +- **Out of this PR:** operator reconciler, HMAC Secret generate, Deployment children, operator idle GC (Phase 4), warm pool (Phase 5). After this PR, local/dev MCP has **no idle janitor** until Phase 4. + +### Phase 4 — Instance children + Ready + idle GC (pool size 0) — **PR** + +Depends on Phase 3 (flags + builder + labels the Deployment will inject). Operator **does not** create warm-pool pods yet. `warmPoolSize: 0` (default) + MCP on-demand create is enough to test an instance. + +- Types + CEL (CR name ≤ 44 chars so `cli-mcp--kubeconfig` fits). `replicas` default 1, minimum 1. `idleTimeout` default 30m (consumed here; not passed as an MCP flag). +- Go child builders: Deployment (server + kube-rbac-proxy, flags from Phase 3 including always-claim — do **not** pass `--warm-pool-size` / `--idle-timeout`), Service, MCP SA + Role/RoleBinding (pods + secret create/delete; no secret get/list/watch), sandbox SA, sandbox ingress NP (instance **and** `component=server`), HMAC Secret generate-once; ownerRefs; instance labels. +- Operator `manager-role` includes namespaced pods and secrets get/list/watch/create/update/patch/delete in this phase (HMAC, Ready keys, finalizer + idle GC), not only Deployments/Services. +- Ready per Q15 **without pool init** (`warmPoolSize == 0` skips that clause). Missing `cli-mcp--kubeconfig` or TLS (non-OpenShift) → `SecretsNotFound`. Missing or empty required keys (`kubeconfig`, HMAC `key`, TLS `tls.crt`/`tls.key`) → `SecretKeysInvalid`. Extra `spec.sandbox.env` Secrets are not Ready gates. +- Finalizer (Q10): on delete, do not re-ensure MCP replicas; scale MCP Deployment to 0; wait until `component=server` pods are gone; then delete instance-labeled sandbox pods/secrets; wait; remove finalizer. +- **Idle GC:** delete **assigned** session pods/secrets past `idleTimeout` via `last-activity` (else `created-at`), selecting instance + component + session-id. Skip unassigned (no session-id). Do not use as-built component-only `CleanupStale`. Same predicated Pod watch as Two Pod writers: enqueue Create / Delete / `session-id` assignment, **not** last-activity patches or probe status. On reconcile, GC due sessions and `requeueAfter` the soonest remaining expiry so a quiet session still GCs. Do **not** `AddAfter` per bash. Do **not** create or trim unassigned pool pods. +- Sample cluster RBAC for kube-rbac-proxy / MCP client (Q13). Sample notes for default-deny / OpenShift SCC as needed. +- **OLM CD on:** master catalog publish, PR check `bundle/` matches `config/`, relatedImages for operator/server/sandbox/kube-rbac-proxy. First catalog is a working instance (on-demand sessions + idle janitor), not a stub manager. +- **Test tips:** envtest is the natural home for children, Ready (`warmPoolSize == 0`, including `SecretKeysInvalid` on empty/wrong keys), HMAC generate-once, finalizer (MCP scaled to 0 and server pods gone before session delete), idle assigned GC (`requeueAfter` + Pod predicate: enqueue create/claim/delete, not last-activity or status-only), and MCP Role verbs (secrets create/delete only). First Kind e2e when practical (instance Ready; optional idle assertion if cheap). Pool assertions wait for Phase 5. +- **Done when:** a CR with `warmPoolSize: 0` gets MCP + HMAC + NP children, goes Ready when admin Secrets have the required keys, GCs idle assigned sessions, and tears down sandboxes on delete. Catalog CD publishes that operator. +- **Coverage check:** [Testing](#testing-all-code-phases) against this PR’s diff. +- **Out of this PR:** operator warm pool, Ready pool-init / no-flap-on-claim (nothing to flap yet). + +### Phase 5 — Warm pool + Ready (no flap on claim) — **PR** + +Depends on Phase 4 (instance exists; builder and idle GC already shipped). This is the two-writer pool contract. + +- Operator **Watches** instance sandbox Pods (claim does not call `TriggerReplenish`; same predicated watch as Phase 4 — assignment enqueues, last-activity does not). Keep unassigned count == `spec.sandbox.warmPoolSize`; surplus deleted immediately (re-get; skip if `session-id` appeared); recreate on spec/image/env/resources hash (no 2× age-drain). Assigned pods are left on overlay change. Enqueue Ready/Failed/backoff here so pool Ready can see them. +- Ready: first Ready / pool-size increase waits for full pool; claim does not flap (Q15). Idle GC already in Phase 4; do not treat assigned session count as Ready. +- Operator must not SSA/delete a pod that just gained `session-id` except idle GC / finalizer. +- **Test tips:** envtest for the two-writer contract (pool size, surplus trim, stale-list vs claim, Ready must not flap on claim). Extend Kind e2e for warm pool if that is the cheap place. +- **Done when:** the operator maintains `warmPoolSize` and Ready follows Q15 (including no flap on claim). +- **Coverage check:** [Testing](#testing-all-code-phases) against this PR’s diff. +- **Out of this PR:** proxy children, extra sandbox volume knobs / `imagePullSecrets`. + +### Phase 6 — First-party catalog consume (test/validation) — **no PR in this repo** + +Other repo / GitOps: CatalogSource + OperatorGroup + Subscription + one `CliMcpInstance` + `cli-mcp--kubeconfig` + TLS (non-OpenShift) + cluster RBAC (+ SCC/PSA as needed). After Phase 4 at the earliest (pool 0, idle GC on); Phase 5 if that environment wants a warm pool. + +This path is **test/validation only** (dev cluster, kind, a non-prod overlay). Verify `bash` creates a sandbox and other pods cannot hit `:8090`. **Do not** wire TARSy or any production/stage MCP client — sandbox still mounts the real kubeconfig and egress is unrestricted. First-party production/stage client wiring waits until proxy children and sandbox egress lock exist and pass the isolation checks in the proxy HOW. That is a **later implementation plan**, not Phase 7’s doc rewrite. + +### Phase 7 — Return to proxy design — **no PR in this repo (docs / later PRs)** + +Rewrite [credential-proxy-design.md](credential-proxy-design.md) HOW against this operator. Resume proxy Q2–Q12. Then a **new** implementation plan for proxy children — not more phases of 1–5. Shipping that plan (not this rewrite) is what unlocks production/stage first-party MCP client wiring. + +## Out of scope / non-goals + +- Credential-isolating proxy, dummy kubeconfig, proxy CA, proxy NetworkPolicies (next design). +- Session CRs, MCP leader election, command allowlists. +- Putting CLI MCP types in `codeready-toolchain/api`, host/member root `controllers/`, or CRD dispatch from a sibling repo. +- Changing any first-party MCP client wiring in this phase. +- Namespace EgressFirewall to kube API IPs (stopgap rejected once a proxy exists; not a substitute for the operator). +- Helm chart in v1 (kustomize + OLM cover install; Helm can wrap the same manifests later). +- First-party production/stage install or MCP client wiring until proxy children and sandbox egress lock exist (operator catalog consume in test/dev is Phase 6). +- Operator-generated TLS certificates. Operator-minted investigation tokens. +- Copying claw’s `WATCH_NAMESPACE` / operator-config CR, or claw `internal/assets` kustomize-in-operator for instance children (Go builders instead). +- Unsuffixed `Containerfile` for the operator (keep `Containerfile.operator` / `.server` / `.agent`). +- Growing the current non-Kubebuilder Makefile instead of replacing it with the go/v4 spine. +- Closed `spec.sandbox.type` enum or extra `RELATED_IMAGE_SANDBOX_*` keys per class. Full `spec.sandbox.template` PodSpec. +- Stubbing later `spec.sandbox` fields now (`imagePullSecrets`, extra volumes, `securityContext` override, agent port). +- `spec.serverImage` / `status.resolvedServerImage`. MCP image is `RELATED_IMAGE_SERVER` on the operator; look at the Deployment. Dev/test overlays that env. +- `spec.investigationKubeconfigSecretRef`. Admin Secret is `cli-mcp--kubeconfig`. Proxy unmounts it from the sandbox; does not delete it. +- Gating MCP claim on `--warm-pool-size` (would roll MCP on pool `0 ↔ N` and contradict Q9). +- Enqueueing the instance reconciler on every last-activity patch, or a custom `AddAfter` per bash (predicate + `requeueAfter` reread). +- HMAC or mTLS on `/assign`, or a VAP that only the MCP SA may set `component=server` (Q11: labels are not identity; the namespace is the trust boundary). +- MCP Secret get/list/watch, or RBAC `resourceNames` / label selectors for session Secrets (API cannot). ValidatingAdmissionPolicy to restrict MCP secret delete to `cli-mcp-sandbox-auth-*` (later; residual delete-by-name). +- Copying claw’s `make test` `coverpkg=./internal/...` (this repo must keep `pkg/` coverage). diff --git a/docs/proposals/cli-mcp-operator-questions.md b/docs/proposals/cli-mcp-operator-questions.md new file mode 100644 index 0000000..c0ea766 --- /dev/null +++ b/docs/proposals/cli-mcp-operator-questions.md @@ -0,0 +1,328 @@ +# CLI MCP Operator — design questions + +**Status:** Decisions recorded +**Related:** [Design document](cli-mcp-operator-design.md) + +This is the **HOW** (operator) for an **open-source Kubernetes operator**. First-party internal deploy is one catalog consumer; do not bake that environment into the API. Proxy WHAT stays in [credential-proxy-design.md](credential-proxy-design.md); proxy Q2–Q12 stay paused until this operator is implemented. + +--- + +## Q1: Where does the operator live? + +We are converting CLI MCP into an operator, then adding proxy children later. Layout now determines module boundaries, CD, and whether `cmd/server` stays a thin data plane. One repo should hold all components (controllers, MCP server, sandbox, later proxy) as multiple images — same shape as `claw-operator`. + +### Option A: Same git repo, rename to `cli-mcp-operator`, multiple images + +GitHub rename `cli-mcp-server` → `cli-mcp-operator` (issues/PRs/redirects kept). Go module path follows: `github.com/codeready-toolchain/cli-mcp-operator`. Add `cmd/operator` + `api/v1alpha1` beside existing `cmd/server` and `cmd/agent`. Separate images: `cli-mcp-operator`, `cli-mcp-server`, `cli-mcp-sandbox`, later `cli-mcp-proxy`. Image names do not have to match the repo (claw-operator already ships `claw-proxy`). Server must not import `internal/controller`. + +- **Pro:** One PR surface for CRD + flag changes + later proxy. Matches claw-operator. No GitOps consumer yet, so rename is cheap. History stays one timeline. +- **Con:** `go.mod` gains controller-runtime/envtest (server binary still won’t link them if imports stay clean). One-time import-path churn. `pkg/session` lives under an `-operator` module (same as claw’s `internal/proxy`). + +**Decision:** Option A — rename this repo in place to `cli-mcp-operator`; keep multiple images in one module. Do not create a second clone. Concrete tree, Makefile, and scaffold steps: [Repository layout](cli-mcp-operator-design.md#repository-layout-target) in the design (Kubebuilder go/v4 + `cmd/operator` building binary `manager`; types in-repo; do not copy host/member’s shared `api` repo). + +_Considered and rejected: Option B (new `cli-mcp-operator` repo + move/archive old — extra remotes and broken links; nothing consumes this module yet), keep the name `cli-mcp-server` (repo would still say “server”), shorter name `cli-mcp` (vaguer in this org next to other MCP repos)._ + +--- + +## Q2: API group and Kind name? + +The CR is the GitOps API for “one MCP instance / class.” It must not collide with `toolchain.dev.openshift.com` (UserSignup/Space) or `claw.sandbox.redhat.com`. + +The operator should be a **generic Kubernetes operator** (installable on any cluster), not OpenShift-only like claw-operator. OpenShift-only features (serving certs, SCCs) may be enabled when the cluster is OpenShift. A dedicated public domain for the API group is not justified until there is real external community use; start on `redhat.com` and switch the group later if that appears. + +### Option A: `cli-mcp.redhat.com` / `CliMcpInstance` + +Group matches the instance label draft (`cli-mcp.redhat.com/instance`). Kind states “one instance,” which is what a later second class is. CRD lives in this repo (not `codeready-toolchain/api`). + +- **Pro:** Stable, specific, not tied to one internal consumer. Kind matches proxy-Q1 language. No domain purchase until community demand is real. +- **Con:** Slightly long Kind. `redhat.com` without `sandbox.` unlike claw. A later group rename is a CRD conversion / bump if community appears. + +**Decision:** Option A — `cli-mcp.redhat.com/v1alpha1`, Kind `CliMcpInstance`. Portable Kubernetes operator; OpenShift extras are optional detections, not requirements. Keep this group unless/until external adopters justify a new domain and group. + +_Considered and rejected: Option B (`tarsy.redhat.com` / `CliMcpServer` — couples the CRD to one internal consumer and names the Kind after the Deployment), Option C (types in `codeready-toolchain/api` under `toolchain.dev.openshift.com` — that repo is an unrelated product API)._ + +--- + +## Q3: How is the operator installed? + +Q2: this is a generic Kubernetes operator, not OpenShift-only. Claw-operator is OLM. Install artifacts in *this* repo are what any cluster uses. + +OLM does **not** replace kustomize: the bundle is generated *from* `config/` (same as claw-operator ADR-0010). GitOps can subscribe to the catalog **or** apply kustomize. Helm can wrap the same manifests later. + +### Option B: OLM OOTB like claw-operator; kustomize remains a supported install + +Ship CatalogSource + bundle + catalog images and CD (`make bundle`, bundle/catalog push, PR `git diff` on `bundle/`) following claw-operator. `config/` kustomize is the source of truth. `make deploy` / `make dev-deploy` stay for local/dev and for clusters without OLM. Helm chart and extra docs are later, not v1. + +- **Pro:** Same OOTB story as claw (OperatorHub-style install, channels, `relatedImages` for server/sandbox/later proxy). CI/CD already has a template in-tree next door. Does not block GitOps or Helm. +- **Con:** CSV, catalog image, scorecard, versioning — real tax. Vanilla Kubernetes users who do not run OLM use kustomize instead (documented escape hatch). + +**Decision:** Option B — OLM is the default/OOTB install, including CI/CD like claw-operator. Kustomize stays supported (`config/`, `make deploy`). Helm is optional later documentation/chart, not a v1 deliverable. Supporting OLM does not contradict GitOps or Helm. + +_Considered and rejected: Option A (kustomize-only / no OLM in v1 — user wants claw-parity OOTB including catalog CD), Option C (Helm as the primary portable install — can wrap the same manifests later if someone asks)._ + +--- + +## Q4: Which OLM install modes does the operator support? + +Watch scope is an OLM **install mode**, not a hardcoded namespace. Children of a `CliMcpInstance` always live in **the CR’s namespace**. + +Claw-operator’s CSV: + +| Mode | Supported | +|---|---| +| OwnNamespace | yes | +| SingleNamespace | yes | +| MultiNamespace | no | +| AllNamespaces | no | + +`WATCH_NAMESPACE` on the claw manager pod is the operator’s **own** namespace (downward API), used for operator-config lookup — not “only watch this one app namespace.” OLM RBAC (OperatorGroup `targetNamespaces`) is what limits which namespaces the operator can touch. + +### Option A: Match claw — OwnNamespace + SingleNamespace + +- **OwnNamespace:** operator Deployment and `CliMcpInstance` in the same namespace. Typical community install: one ns, CR there, MCP + sandbox pods there. +- **SingleNamespace:** operator in namespace A, reconcile CRs in one other namespace B (OperatorGroup `targetNamespaces: [B]`). +- Not MultiNamespace, not AllNamespaces. + +- **Pro:** Same OLM story as claw. Tight RBAC. Portable: admin picks the namespace(s) at install time. No name baked into the binary. +- **Con:** One operator install cannot watch every namespace. A CR in a third namespace needs another Subscription or a mode we do not enable. + +**Decision:** Option A — OwnNamespace + SingleNamespace, matching claw-operator. Children always live in the CR’s namespace. No product namespace in the binary. + +_Considered and rejected: Option B (AllNamespaces — wide RBAC; claw disabled it), Option C (OwnNamespace only — drops SingleNamespace, which claw keeps for operator-in-one-ns / CR-in-another)._ + +--- + +## Q5: What does the admin own vs the operator vs the MCP process? + +Wrong boundaries either put an ensure-loop for Deployments/NPs inside `cmd/server`, freeze derived objects in git, put the bash hot path through a Session CR, or leave multi-replica pool replenishment in every MCP replica. + +Infrastructure (Deployment, Service, NPs) is rare and must reconverge. Session **create/claim** is synchronous with `bash`. Pool desired-count and idle janitor are not. + +### Option A: Operator owns instance infrastructure + pool + idle GC; admin owns install and secrets; MCP owns the bash hot path + +| Owner | Objects / jobs | +|---|---| +| **Admin / OLM** | CRD, operator Deployment/SA/RBAC, `CliMcpInstance` CR, investigation kubeconfig Secret `cli-mcp--kubeconfig`, TLS Secret `cli-mcp--tls` on generic Kubernetes, cluster-scoped RBAC (`system:auth-delegator`, MCP client `/mcp` access), OpenShift SCC / PSA as needed | +| **Operator** | MCP Deployment+Service, MCP SA, Role/RoleBinding (pods + secret create/delete), sandbox SA (Q12), NetworkPolicies (Q11), serving-cert annotation when on OpenShift, HMAC generate-once. **manager-role** namespaced secrets (HMAC, Ready, idle GC/finalizer) — OperatorGroup target namespace is the **secret trust boundary**. Warm pool, idle GC, teardown (Q10). Later: proxy children. | +| **MCP** | On `bash`: cache → discover → **always claim** a warm pod if one exists, else **create** on demand → wait Ready → HMAC `/exec`. Per-session auth Secret **create/delete only** (HMAC from file mount; no secret get/list/watch). `DELETE /sessions/{id}` deletes that session’s pod+secret. Does **not** replenish the pool or run idle GC. Claim is **not** gated on `--warm-pool-size` (Q9). | + +No `Sandbox` / session CRD. Pool pods are ordinary Pods (no session-id). Claim stays a label patch (first writer wins). On-demand create stays in the MCP so an empty pool does not wait on the next operator reconcile for the first command. + +- **Pro:** Request-synchronous work stays in the multi-replica MCP. Desired-count / janitor stay in the leader-elected operator (no replica pool stampede). Operator can later derive dummy kubeconfig/CA/routes. Admin does not hand-maintain the MCP Deployment. Tokens stay outside the operator. No second CRD. +- **Con:** Two writers of Pods (MCP create/claim, operator pool/GC) — contract must be label-based and idempotent. MCP still needs pod RBAC plus secret create/delete (not get/list) for the hot path. + +**Decision:** Option A — this hybrid. Instance infrastructure + HMAC generate-once + warm pool + idle GC + instance teardown on the operator. MCP keeps claim / on-demand create / exec / explicit session delete. No session CRD. Investigation kubeconfig stays admin-provided. + +_Considered and rejected: MCP owns pool and idle GC as well (replica stampede; janitor dies with the Deployment), `Sandbox` CRD (MCP apply CR, operator creates pods — extra hop, second API, HMAC on both sides, pool-as-CR awkward; same blast radius as `sandboxes create`), admin still owns the MCP Deployment (two owners; proxy would redo it), operator mints investigation tokens / cluster RBAC (identity provider), admin provides HMAC (internal MCP↔agent secret; ceremony with no admin-chosen value), copy as-built 2× idle age-drain of unassigned pool pods (MCP-replica overshoot leftover; operator trims surplus immediately), enqueue every last-activity patch or custom `AddAfter` per bash (hammers the instance reconciler; a later `requeueAfter` reread is enough), MCP secret get/list/watch (would read other CRs’ kubeconfigs and HMAC in the same namespace), RBAC `resourceNames` or label selectors for dynamic session Secrets (API cannot)._ + +--- + +## Q6: How much of today’s server flags belong on the CR spec? + +`cmd/server` flags today: transport, address, stateless, namespace, sandbox-image, kubeconfig (MCP’s client), hmac-key-file, idle-timeout, warm-pool-size. In-cluster, transport/address/stateless/namespace are determined by the operator. + +### Option A: Spec is the instance API — replicas, sandbox class (idle/pool/resources/image/env) + +In-cluster-only flags are not on the spec (operator hardcodes HTTP/loopback/stateless). Namespace is `metadata.namespace`. Optional `spec.serverContainer` extras (resources, imagePullPolicy); not a free-form args array. `spec.sandbox.idleTimeout` and `warmPoolSize` are what the operator reconciles (Q5). No `spec.hmacKeySecretRef` — HMAC Secret is operator-owned. + +- **Pro:** Reviewable, validated, matches claw. A second instance can differ. Pool/GC have a real API. Room to add proxy fields later without a junk drawer of CLI flags. +- **Con:** A new MCP flag needs a CRD field (or an escape hatch). + +**Decision:** Option A — typed instance spec. No `spec.args` passthrough. Local `cmd/server` flags remain for running without the operator. Sandbox image lives at `spec.sandbox.image` (Q16), not a top-level `spec.sandboxImage`. No `spec.investigationKubeconfigSecretRef` — admin Secret name is `cli-mcp--kubeconfig` (same convention as TLS). This phase the operator mounts it on sandbox pods; proxy pass unmounts it from the sandbox and keeps the Secret for the proxy. + +_Considered and rejected: Option B (opaque args/env — no validation, pool size is not a typed field), Option C (secret refs only — pool/idle would live on the operator Deployment or in code, so instances cannot differ), `spec.investigationKubeconfigSecretRef` (ceremony; TLS already uses a conventional name; two CRs get `cli-mcp--kubeconfig` without a spec knob)._ + +--- + +## Q7: How are MCP and sandbox images supplied? + +All images (`cli-mcp-operator`, `cli-mcp-server`, `cli-mcp-sandbox`, later `cli-mcp-proxy`) are **ours** by default: same repo, same CD, Quay. The MCP server and operator stay that way. The **sandbox** image is the default class we ship; Q16 makes a different image a first-class instance setting, not a test-only pin. They are not an upstream workload like OpenClaw’s `spec.image` (the operator still supplies HMAC, labels, SA). Operator upgrades should pick up new **default** image tags automatically via OLM `relatedImages`. Do not bake `+kubebuilder:default` image tags into the CRD. + +Operator-owned Deployments **fight** ImageStream triggers (the operator reverts the mutated image). + +### Option A: OLM `relatedImages` → operator env defaults; sandbox class on the CR + +CSV `relatedImages` stamp `RELATED_IMAGE_SERVER` / `RELATED_IMAGE_SANDBOX` / `RELATED_IMAGE_KUBE_RBAC_PROXY` (and later proxy) on the operator Deployment, same as claw’s `PROXY_IMAGE`. MCP server image is **always** `RELATED_IMAGE_SERVER` — no `spec.serverImage`. Local/dev points the operator at a locally built server image by overlaying that env (`make deploy`), so every CR follows. Empty `spec.sandbox.image` means `RELATED_IMAGE_SANDBOX`; set it for another class (Q16). Status records `resolvedSandboxImage` (spec vs default). MCP image is not on CR status. kube-rbac-proxy is a sidecar image, not an instance override field. + +- **Pro:** Catalog/operator bump rolls every instance’s MCP together. No per-CR pin that survives upgrades. Sandbox class still differs per CR. +- **Con:** Cannot run two MCP *server* versions in one operator install (not needed; classes differ by sandbox, not `cmd/server`). + +**Decision:** Option A — MCP image from operator env only. Not claw’s upstream-gateway `spec.image` + kubebuilder default. + +**Revised by [Q16](#q16-how-does-the-crd-support-more-than-the-default-oc-sandbox):** `spec.sandbox.image` is a **first-class sandbox class** (BYO image that speaks the agent contract). Empty still means `RELATED_IMAGE_SANDBOX`. Do not add a closed enum of sandbox types or extra `RELATED_IMAGE_SANDBOX_*` keys to express classes. + +**Revised:** drop `spec.serverImage`. Per-CR MCP pin blocks operator upgrades, and two CRs do not need different `cmd/server` images. Dev/test uses `RELATED_IMAGE_SERVER` on the operator Deployment. + +_Considered and rejected: Option B (images always required on the CR — every bump edits every CR; operator upgrade would not move instances), Option C (ImageStream triggers on the MCP Deployment — operator and trigger both write the image), `spec.serverImage` optional pin (escape hatch that fights Q7’s upgrade story; local/dev already has operator-env overlay)._ + +--- + +## Q8: Instance identity — labels and child resource names? + +NPs, pool, claim, and idle GC need a stable instance id. As-built sandbox pods use `tarsy.redhat.com/component` and `tarsy.redhat.com/session-id` in `pkg/session`. Nothing is in production; those keys can change in place (no migration). + +Kubernetes already makes `metadata.name` immutable and DNS-1123. Child names `cli-mcp-` must still fit 63 characters. + +### Option A: CR `metadata.name` is the instance id; labels under `cli-mcp.redhat.com`; children named `cli-mcp-` + +Labels: `cli-mcp.redhat.com/instance=`, `cli-mcp.redhat.com/component` (`sandbox` | `server`), `cli-mcp.redhat.com/session-id` when assigned. Annotations: `cli-mcp.redhat.com/created-at`, `cli-mcp.redhat.com/last-activity`. Session manager and operator pool/GC select **component + instance** (and session-id when assigned). Drop `tarsy.redhat.com/*`. No client-pod label. + +- **Pro:** One id. NP/pool selectors are obvious. A second CR cannot claim the first’s pods. Label domain matches the API group. No leftover internal domain in the open-source API. +- **Con:** Long CR names plus the `cli-mcp-` prefix can hit the 63-char Deployment name limit (use short CR names; `oc` is fine). + +**Decision:** Option A — instance id is the CR name; `cli-mcp.redhat.com` labels; children `cli-mcp-`. Replace as-built `tarsy.redhat.com` labels in `pkg/session` in the same change. No migration. + +_Considered and rejected: Option B (encode instance only in a component label value — breaks current selectors and keeps a single overloaded key), Option C (random child names — worse UX; ownerRefs already GC)._ + +--- + +## Q9: Does the MCP process read the CR, or only flags the operator injects? + +If `CliMcpInstance` spec changes, something has to pick that up. The MCP must **not** watch the CR. + +### Option A: Operator patches the MCP Deployment; kube rolls the replicas + +Operator translates spec → Deployment args/env/mounts. Kubernetes restarts MCP pods with the new flags. MCP never gets/watches `CliMcpInstance`. Local stdio stays flag-only. + +Not every spec field needs a rollout: `spec.sandbox.warmPoolSize` / `idleTimeout` are consumed by the operator’s pool/GC (Q5). Fields that affect the MCP process (sandbox image/env/resources overlay, HMAC mount, SA, instance name, replicas) go on the Deployment. MCP **always** attempts claim then on-demand create; do not pass `--warm-pool-size` to gate claim (that would roll MCP on `0 ↔ N`). + +- **Pro:** One control loop. Standard Deployment rollout. MCP SA needs no get-CR. Tests stay fake client-go. +- **Con:** Each new MCP knob is a flag + CR field + operator wiring. Secret *data* rotation (same Secret name) does not roll pods unless the operator stamps a hash/annotation (same as claw). + +**Decision:** Option A — CR changes that affect the MCP go through the Deployment; kube rolls replicas. MCP does not watch the CR. + +_Considered and rejected: Option B (MCP watches `CliMcpInstance` and self-configures — second reconciler in the data plane)._ + +--- + +## Q10: What happens to sandbox pods when the CR is deleted? + +Delete CR must destroy that instance (including sandbox pods). Rolling the MCP Deployment must **not**. Background `ownerRef` GC alone can drop the CR while pods are still Terminating, so a same-name recreate can overlap. + +### Option A: Finalizer quiesces MCP, then waits until session pods/secrets are gone; operator children keep `ownerRef` → CR + +- **Finalizer:** while `deletionTimestamp` is set, do **not** re-ensure MCP `replicas`. Scale the MCP Deployment to 0, **wait until this instance’s `component=server` pods are gone**, then list instance-labeled sandbox pods and session Secrets, delete, **wait until gone**, then remove the finalizer. The CR name stays taken until session cleanup finishes. +- **`ownerRef` → CR:** MCP Deployment, Service, NPs, SAs, Role/RoleBinding, HMAC Secret, warm-pool pods. After the finalizer drops, Kubernetes GCs these. Do not wait for those objects in the finalizer. +- **MCP** does not set `ownerRef` on on-demand session pods and does not get the CR. Idle GC (Q5) is the same label list in steady state. + +- **Pro:** Name reuse cannot overlap Terminating session pods. MCP is not still creating sessions while the finalizer waits. MCP Deployment rollout does not kill sessions. Standard `ownerRef` for operator children. MCP stays flag-only (Q9). +- **Con:** A stuck finalizer if the operator is down (don’t remove it unless you intend to orphan). Operator must not SSA-delete assigned session pods as “unexpected children.” + +**Decision:** Option A — finalizer is the teardown guarantee (quiesce MCP, then session objects); `ownerRef` on operator-created children; MCP session pods are not owned by the Deployment. + +_Considered and rejected: ownerRef-only on session pods (background GC + same-name overlap), MCP ownerRef to the CR as the sole mechanism (still need the finalizer), orphan / idle-only (after CR delete nothing reconciles those pods), delete session pods while MCP is still Running (MCP recreates; a create after the list goes empty orphans a pod with no ownerRef), wait for every ownerRef child in the finalizer (ownerRef GC after the finalizer drops)._ + +--- + +## Q11: Which NetworkPolicies does this phase create? + +Proxy NPs (sandbox egress-to-proxy-only, proxy ingress/egress) are the next design. This phase still needs the as-built ingress rules or `/exec` is open inside the namespace. + +### Option A: Ingress only — MCP `:8443` from configured clients; sandbox `:8090` from this instance’s MCP pods. No sandbox egress policy + +- **Pro:** Same security as the current design docs. Instance label on sandbox ingress so a later second MCP cannot hit these agents. Does not fake a token-replay fix. +- **Con:** Sandbox egress stays open. Explicitly accepted until proxy. + +**Decision:** Option A, amended — operator-owned **sandbox** ingress NP only (`:8090` from this instance’s `component=server` pods). That selector is labels, not identity; `/assign` stays once-unauthenticated. The OperatorGroup target namespace is the trust boundary (same as secrets). No MCP `:8443` NetworkPolicy and no client pod label; kube-rbac-proxy is the MCP front door (clients often cannot set labels). Egress lock waits for proxy. + +_Considered and rejected: egress NP / EgressFirewall to kube API IPs now (stopgap that must be removed when the proxy exists; OpenShift API IPs are painful), admin-applied NPs (cannot select MCP-created sandbox pods without the same labels/selectors the operator should own), MCP ingress NP requiring `cli-mcp.redhat.com/mcp-client` on callers (impractical; kube-rbac-proxy already authenticates `/mcp`), HMAC or mTLS on `/assign` or a VAP that only the MCP SA may set `component=server` (namespace is the trust boundary; identity-aware assign is proxy work)._ + +--- + +## Q12: Sandbox pod identity in this phase? + +Nothing is in production. The MCP will not be used until the proxy exists. This phase still has to be **testable**: `oc` in a session pod must work so we can verify the operator (pool, claim, HMAC `/exec`, idle GC, CR delete) before the proxy pass. + +Dummy kubeconfig without a proxy makes `oc` fail and blocks that testing. Keeping the as-built investigation SA + automount-true is not “preserving a product”; it is carrying a footgun into the first operator tests. + +### Option B: Dedicated sandbox SA + `automountServiceAccountToken: false` now; still mount the real kubeconfig + +- Operator creates `cli-mcp--sandbox` with **no RoleBindings** (OpenShift needs an SA; this is the proxy-ready identity). +- MCP sets that SA on sandbox pods and `automountServiceAccountToken: false`. +- Admin-provided investigation kubeconfig Secret stays mounted so `oc` works in tests. +- Proxy pass later swaps the mount for dummy kubeconfig + `HTTPS_PROXY`; SA and automount already match. + +- **Pro:** Operator tests still cover real `oc`. Pod identity is already the proxy shape. Accidental automount of an investigation SA never exists in this operator. +- **Con:** Sandbox disk still holds real tokens until proxy (accepted; this phase is not the isolation ship). + +**Decision:** Option B — proxy-ready pod identity now; real kubeconfig mount until proxy so the operator is testable. + +_Considered and rejected: keep investigation SA and default automount (no product to preserve; extra projected token on every test pod), dummy kubeconfig now (breaks `oc`, cannot test the operator before proxy)._ + +--- + +## Q13: Who owns MCP client authentication (kube-rbac-proxy extras)? + +In-cluster MCP is typically: kube-rbac-proxy sidecar, TLS on the Service, `system:auth-delegator` on the MCP SA, a **client** SA + token, and RBAC allowing that client to call `/mcp`. The MCP client uses that token. Loopback-only bind in `cmd/server` exists for this sidecar. + +### Option A: Operator owns sidecar + Service TLS (serving-cert when on OpenShift); admin owns cluster-scoped auth-delegator, client SA, `/mcp` ClusterRole/Binding + +Operator always injects kube-rbac-proxy (not optional). Admin (or `config/samples` in tests) creates the client SA and points the MCP client at it. + +- **Pro:** Operator stays mostly namespaced. ClusterRole for `/mcp` is a cluster API grant — belongs to the admin. Sidecar is an implementation detail of the MCP Deployment the operator already owns. +- **Con:** Standing up an instance is “CR + a few cluster RBAC YAMLs,” not CR-only. + +**Decision:** Option A — kube-rbac-proxy is a fixed part of the MCP Deployment (`RELATED_IMAGE_KUBE_RBAC_PROXY`). Cluster-scoped client auth stays with the admin; ship sample YAMLs so operator tests can call `/mcp`. `--allow-paths`: `/mcp`, `/metrics`, `/live`, `/health`, `/sessions`. TLS Secret `cli-mcp--tls`: serving-cert on OpenShift, admin-provided on generic Kubernetes. + +_Considered and rejected: operator creates auth-delegator, client SA, and `/mcp` ClusterRole (operator ClusterRole too wide; cluster-scoped GC on CR delete is awkward), no kube-rbac-proxy (no TLS + SA-token front door; breaks loopback-only bind)._ + +--- + +## Q14: Put `spec.proxy` on the CR now (disabled), or omit until the proxy design? + +### Option A: Omit. Extension point is “same CR, new fields + children later” + +- **Pro:** Does not freeze injector types, route lists, or CA knobs before proxy Q2–Q12. CRD additive changes are normal for v1alpha1. +- **Con:** First CRD bump in the proxy PR (expected). + +**Decision:** Option A — document the extension point; do not stub `spec.proxy` on this CRD. + +_Considered and rejected: `spec.proxy.enabled: false` and empty structs now (speculative API; proxy questions will reshape the field; dead validation)._ + +--- + +## Q15: What does `status.Ready` mean in this phase? + +`Ready` is how we notice that the instance did **not** come up — including a warm pool that cannot initialize. The operator owns the pool (Q5), so pool health belongs on this condition. Assigned session pods (on-demand / claimed) are not part of Ready. A claim is success, not an outage: `Ready` must not flicker every time the pool is replenished. + +### Decision + +`Ready` requires **all** of: + +1. **Required Secrets exist with the conventional non-empty keys** (no kubeconfig/YAML/cert parse). `cli-mcp--kubeconfig` data `kubeconfig`; HMAC `cli-mcp--hmac` data `key`; on generic Kubernetes also `cli-mcp--tls` data `tls.crt` and `tls.key`. Missing object → not Ready, reason `SecretsNotFound`. Missing or empty required key → not Ready, reason `SecretKeysInvalid`. HMAC is operator-created (part of (2)); generate-once does **not** overwrite a pre-existing Secret, so an empty/wrong-key HMAC stays `SecretKeysInvalid`. Extra Secrets referenced only from `spec.sandbox.env` are **not** this check. OpenShift serving-cert TLS is platform-filled; Deployment Available still waits on the sidecar mount. +2. **Every operator-managed namespaced child is present and matches spec** (SAs, Role/RoleBinding, Service, NetworkPolicies, sandbox SA, HMAC Secret). +3. **MCP is ready to receive requests:** Deployment Available — desired replicas ready, including the kube-rbac-proxy sidecar (readiness probes on both containers). +4. **Warm pool, if `warmPoolSize > 0`:** + - **First Ready** (and after `warmPoolSize` increases): wait until there are `warmPoolSize` unassigned **Ready** pods. + - **After that:** a claim/replenish dip does **not** clear `Ready` unless a pool pod is Failed / ImagePullBackOff / CrashLoopBackOff, or the pool is still short of desired past a **replenish deadline** (operator constant, not a spec field). Decreasing `warmPoolSize` does not wait. + - Always publish `status.warmPoolReady` / `status.warmPoolDesired`. Optional condition `WarmPoolReady` is the strict count (may flap); aggregate `Ready` does not flap on claim. + +`warmPoolSize: 0` skips (4). Later proxy children fold into the same `Ready`. No separate `ProxyReady` until that pass needs it. + +_Considered and rejected: Ready = MCP Deployment + Secrets only (stuck pool looks Ready), Ready tracks unassigned Ready count every second (claim looks like an outage), Ready after only one pool pod, no status beyond observed generation, parse the kubeconfig or TLS certs (key presence is enough), a separate `InvalidSecret` condition (same gate as Ready; use reason `SecretKeysInvalid`)._ + +--- + +## Q16: How does the CRD support more than the default oc sandbox? + +The sample CR is named `oc` and Q7’s `RELATED_IMAGE_SANDBOX` is the agent+oc image we ship. That must not freeze the API as “one kubectl sandbox.” A second class (curl, aws-cli, a user’s image) is another `CliMcpInstance` in the same namespace (Q8: CR name is the instance id). The MCP tool is still `bash`; what varies is the **pod image and its config**. + +The constraint: session pods must speak the **agent HTTP contract** (`/health`, `/exec`, `/assign` on the agent port, HMAC). A custom image typically `FROM` our sandbox image or copies `cmd/agent`. This is not a generic Job/Pod operator. + +### Option A: Nested `spec.sandbox` is the class — BYO `image`, additive knobs, operator-owned base + +- `spec.sandbox.image` omitted → `RELATED_IMAGE_SANDBOX` (the class we ship). Set → that class’s image. First-class, not a test escape hatch (revises Q7). +- v1 mergeable knobs on `spec.sandbox`: `resources`, `env` (`[]corev1.EnvVar`, including `valueFrom` for extra Secrets), `imagePullPolicy`. Empty `resources` → as-built DefaultConfig requests/limits, not BestEffort. Extra `valueFrom` Secrets are not Ready gates. +- Operator-owned (not in spec, not overridable): sandbox SA, automount, session token env (assigned/on-demand; pool via `/assign`), instance/component labels, probes, agent port, kubeconfig mount **in this phase** (Q12), today’s non-root / drop-caps security context. Reserved env names win if the user sets them: `KUBECONFIG`, `HOME`, `SANDBOX_AUTH_TOKEN`. +- No `spec.sandbox.type` enum. No full `PodTemplateSpec` (would fight HMAC, labels, SA, two Pod writers). +- Later additive fields on the same object: extra volumes/mounts, `imagePullSecrets`, args, `securityContext` override, optional kubeconfig mount, agent port. Same rule as Q14: do not stub them empty now. +- Investigation kubeconfig Secret `cli-mcp--kubeconfig` (key `kubeconfig`) is **required this phase** so operator tests can run `oc` (mounted on sandbox pods). No spec ref — conventional name, same as TLS. Proxy pass **unmounts from sandbox** and keeps the Secret for the proxy. A future class that does not need kube is an additive change (skip mount if absent), not a new Kind. +- Pool recreate hash includes the resolved sandbox spec (image + env + resources), not only the image tag. Assigned sessions keep the old spec until they end. +- Operator maps CR spec → `SandboxConfig` in-process for pool pods. MCP gets the same overlay as Deployment flags (Q9). Overlay change rolls MCP. No overlay ConfigMap. `pkg/session` must not import `api/v1alpha1`. + +- **Pro:** Second CR `curl` / `aws` is a different image+env, same operator. Users bring an image without a code change. CRD stays typed. Room to add mounts later without a rewrite. +- **Con:** Custom images must include a compatible agent (and `curl` for the exec readiness probe, as-built). Env merge cannot override operator-owned names. + +**Decision:** Option A — one CR = one sandbox class; `spec.sandbox.image` + additive `env`/`resources`/`imagePullPolicy`; shipped image is the default, not the only type. Extend `spec.sandbox` later; do not add a type enum or a pod template. + +_Considered and rejected: closed `type: oc|curl` + `RELATED_IMAGE_SANDBOX_*` (not universal; every new class is an operator release), `spec.sandbox.template` PodSpec (operator cannot own HMAC/labels/SA safely), keeping sandbox image as tests-only (Q7 as originally written), a separate SandboxClass CRD (second API for one nested object), overlay ConfigMap or a versioned SandboxConfig wire format (Deployment flags; pool maps spec in-process)._ diff --git a/docs/proposals/credential-proxy-design.md b/docs/proposals/credential-proxy-design.md new file mode 100644 index 0000000..5a31a88 --- /dev/null +++ b/docs/proposals/credential-proxy-design.md @@ -0,0 +1,350 @@ +# CLI MCP — credential-isolating proxy + +**Status:** Paused — Q1 decided (real operator); remaining questions and this doc’s HOW wait on the operator **implementation** + +**Related:** [Questions](credential-proxy-questions.md) · [Operator HOW](cli-mcp-operator-design.md) · [As-built design](../design.md) · [Architecture overview](../architecture-overview.md) · [Umbrella analysis](../../../docs/proposals/cli-mcp-credential-proxy.md) + +> **Paused.** Q1: instance infrastructure (proxy Deployment, CA, dummy kubeconfig, NetworkPolicies, MCP Deployment) will be managed by a **CLI MCP Operator** (CR per instance), not by an ensure-loop in the MCP server. The MCP process stays the stateless `bash` / session data plane. +> +> This document is the **WHAT**: components, security invariants, network topology. Treat HOW (who applies objects, flags, in-process reconcile, implementation phases below) as **stale until rewritten against the operator**. The operator HOW is [Final](cli-mcp-operator-design.md). Q2–Q12 in the [questions doc](credential-proxy-questions.md) stay paused until that operator is implemented. Do not implement the proxy stack in `cmd/server`. + +This is proxy design for an **open-source Kubernetes operator**. Docs describe the product any cluster can install. First-party internal deploy is one catalog consumer, not part of the operator API. + +v1 gives an MCP client a per-session `oc`/`kubectl` bash sandbox whose **effective** cluster identity is always the investigation ServiceAccount, even if the model passes `--token`, `--server`, `--kubeconfig`, or a token copied from another MCP tool. + +## Overview + +CLI MCP is a stateless control plane plus per-session sandbox pods (`bash` over HMAC-authenticated `/exec`). Today the investigation kubeconfig Secret is mounted into every sandbox (`pkg/session/manager.go`). The sandbox image includes `oc`, `kubectl`, and `curl`. There is no command allowlist. That is a token-replay path: + +1. Another MCP tool, or anything the sandbox can read, can yield a projected SA token or a kubeconfig-like file. The model can feed that into the **same** bash session. +2. Client-side redaction of tool output sent back to the LLM does not stop a **same-sandbox** pipeline (`cat … | oc --token …`) where the token never returns to the client. +3. `oc --token ` or `curl -H Authorization:` talks to the API with that identity. Read-only investigation RBAC on the **mounted** kubeconfig is bypassed. + +NetworkPolicy and a dummy kubeconfig are not enough on their own: `unset HTTPS_PROXY` / `--server` must fail at the network, and any `Authorization` that does reach the API must be **stripped and replaced** with the investigation token. + +This design adds a third binary — **`cli-mcp-proxy`** — a MITM forward proxy copied from `claw-operator` (own image, own lifecycle). Sandbox pods receive a dummy kubeconfig and can reach **only** that proxy. The proxy holds the real kubeconfig and injects tokens. + +**v1 scope:** one MCP instance, one sandbox image (`oc`/`kubectl`), one proxy (kube API hosts only). + +A second class (illustrated as **curl**: no `oc`, HTTP(S) allowlist, must not reach kube APIs) is **architecture-only and out of scope for v1**. It exists so v1 labels, NetworkPolicies, and “one proxy per MCP instance” do not paint us into a shared-proxy corner. + +## Design Principles + +1. **Sandbox remains the security boundary** — no bash command allowlists. Capability is image + investigation RBAC + proxy L7 + NetworkPolicy. +2. **The sandbox never holds a useful cluster credential** — dummy kubeconfig token, `automountServiceAccountToken: false`, sandbox pod SA has no RoleBindings. +3. **Strip then inject** — client `Authorization` / impersonation headers are discarded; the proxy injects the configured credential for that host. Stolen tokens in the sandbox cannot be replayed through the proxy. +4. **NetworkPolicy is what makes the proxy mandatory** — dummy kubeconfig is convenience; egress-to-proxy-only is the control. Direct API, `--server`, and `unset HTTPS_PROXY` fail closed. +5. **One proxy per MCP instance / class, shared by all sessions of that class** — not a sidecar (shared netns = bypass), not per-session (same identity anyway). +6. **Do not share a proxy across classes** — a union allowlist would let the `oc` sandbox `CONNECT` to whatever a later class may reach. Curl in this doc is only that illustration. +7. **Do not consume the claw-operator proxy image** — copy the MITM + kubernetes injector into this repo and ship `cli-mcp-proxy`. +8. **No namespace EgressFirewall to kube API IPs once the proxy is in place** — that EF was a no-proxy stopgap and would reopen direct API from sandbox pods. +9. **MCP stays a dumb shell proxy** — it still does not parse bash. Credential isolation is a network/identity feature, not a command filter. +10. **Fail closed** — if the proxy, dummy kubeconfig, or NetworkPolicies are not ready, do not create sandbox pods that could egress more freely. + +Q1 (ownership): **CLI MCP Operator** — see [questions](credential-proxy-questions.md). The operator is HOW; this list is WHAT it must be able to represent. + +## Architecture / How It Works + +### As-built (today) + +``` +MCP client ──bash + X-Session-ID──► cli-mcp-server + │ POST /exec (HMAC) + ▼ + sandbox pod + KUBECONFIG=/config/kubeconfig ← real Secret + automount SA token: default true + │ + ▼ + kube API (unrestricted egress) +``` + +### v1 (`oc` class) + +``` +MCP client + │ bash + X-Session-ID + ▼ +cli-mcp-server (instance=oc) + │ POST /exec (HMAC) to sandbox :8090 + ▼ +oc sandbox pods + dummy KUBECONFIG (real server URLs, placeholder token, proxy CA) + HTTPS_PROXY=http://cli-mcp-proxy-oc:8080 + automountServiceAccountToken: false + │ CONNECT api.:6443 + ▼ +cli-mcp-proxy (MITM) + real kubeconfig Secret (tokens) + strip Authorization + Impersonate-* + inject investigation Bearer for that hostname:port + │ + ▼ +kube API server(s) RBAC = investigation SA (get/list/watch, no exec) +``` + +```mermaid +flowchart TB + Client["MCP client"] + MCP["cli-mcp-server"] + Sandbox["oc sandbox pods"] + Proxy["cli-mcp-proxy"] + API["Kube API servers"] + + Client -->|"bash + X-Session-ID"| MCP + MCP -->|"POST /exec HMAC"| Sandbox + Sandbox -->|"CONNECT host:port"| Proxy + Proxy -->|"Bearer investigation token"| API +``` + +Later class (**example only, not v1**) — same MITM binary, different instance / image / route ConfigMap / NPs: + +``` +cli-mcp-server (instance=curl) out of scope for v1 + ▼ +curl sandbox pods no kubeconfig / no oc + HTTPS_PROXY=http://cli-mcp-proxy-curl:8080 + │ CONNECT + ▼ +cli-mcp-proxy-curl routes = curl allowlist; not kube API +``` + +### Traffic rules the proxy enforces + +On `CONNECT` and on each MITM’d request: + +| Step | Behavior | +|---|---| +| Host allowlist | `MatchRoute` on `hostname:port` from the route list (derived from kubeconfig cluster servers). Unknown host → 403, no tunnel. | +| MITM | Kubernetes routes always MITM (credential injection). Leaf certs signed by the proxy CA. Dummy kubeconfig’s `certificate-authority-data` is that CA so `oc`/`kubectl` trust the intercept. | +| Strip | Remove `Authorization`, `Impersonate-*`, `X-Api-Key`, `Proxy-Authorization` on the **tunneled** request (same list as claw `StripAuthHeaders`). | +| Inject | `kubernetes` injector maps `hostname:port` → token from the **real** kubeconfig. | +| Upstream TLS | Proxy verifies the real API server using each cluster’s original CA (`caCert` on the route), not `InsecureSkipVerify`. | + +`oc --token `, `oc --kubeconfig /workspace/leaked`, and `curl -H 'Authorization: Bearer …'` that still go through `HTTPS_PROXY` therefore authenticate as the investigation SA. + +### What NetworkPolicy does (both directions) + +NetworkPolicy is **not** optional. Without it the model unsets `HTTPS_PROXY` and talks to the API with any token. + +| Policy | Selects | Allows | +|---|---| +| **Sandbox egress** | This instance’s sandbox pods | TCP `:8080` to **this** instance’s proxy pods; DNS (UDP/TCP 53 and 5353, OpenShift DNS in `openshift-dns` when on OpenShift) | +| **Sandbox ingress** | This instance’s sandbox pods | TCP `:8090` only from this instance’s MCP server pods (already in the as-built design) | +| **Proxy ingress** | This instance’s proxy pods | TCP `:8080` only from this instance’s sandbox pods. **This stops unauthorized clients** (the MCP client, other MCP servers, a later curl sandbox, a random pod in the instance namespace). | +| **Proxy egress** | This instance’s proxy pods | DNS + kube API ports. Tightness of destinations is [Q4](credential-proxy-questions.md). | + +A NetworkPolicy with `policyTypes: [Egress]` on sandbox pods makes those pods default-deny egress except the listed rules. A namespace-wide default-deny is not required. + +Selectors must be **instance-specific** (`cli-mcp.redhat.com/instance=` plus `component`), not a shared `component=sandbox` alone. Same namespace, two future instances: `oc` sandboxes must not reach the curl proxy (and vice versa). Curl = example, not v1. (Operator Q8 already dropped as-built `tarsy.redhat.com/*` keys.) + +Proxy Service is **ClusterIP only** — no Route, NodePort, or LoadBalancer. + +Do **not** add an EgressFirewall that allows sandbox pods to kube API IPs. + +> **Open question:** optional `Proxy-Authorization` on CONNECT as a belt beyond NP — see [Q3](credential-proxy-questions.md). +> +> **Open question:** CONNECT to raw IPs — see [Q5](credential-proxy-questions.md). +> +> **Open question:** L7 denylist of `pods/exec` and related subresources — see [Q6](credential-proxy-questions.md). + +### Dummy kubeconfig + +Copied from claw `sanitizeKubeconfig`: + +- Preserve clusters (real `server` URLs), contexts, namespaces. +- Replace every user token with `proxy-managed-token`. Clear `tokenFile`. +- Reject kubeconfigs that use client certs, exec, auth-provider, or basic auth (token-only, same as claw ADR-0003). +- Set each cluster’s `certificate-authority-data` to the **proxy CA** (not the real API CA). Clear `insecure-skip-tls-verify`. +- Real API CAs go on the proxy **route** `caCert` so the proxy can verify upstream. + +Delivery: ConfigMap (no real credentials), mounted read-only at `/config` with `KUBECONFIG=/config/kubeconfig`. The real kubeconfig Secret is mounted **only** on the proxy. + +Today `buildBasePodSpec` mounts Secret `cli-mcp-investigation-kubeconfig` into every sandbox, including the warm pool. That mount becomes the dummy ConfigMap. Warm pool pods get the same dummy + `HTTPS_PROXY`; they have nothing useful to steal. + +### Identities (three, not two) + +| Identity | Where | Purpose | +|---|---|---| +| `cli-mcp-server` SA | MCP server pod | Sandbox pods plus session auth Secret **create/delete** (no secret get/list/watch). NPs, CA, dummy kubeconfig are the operator (Q1). In-cluster client. **Not** used for investigation API calls. | +| Investigation tokens | Real kubeconfig Secret on the **proxy** | get/list/watch on every cluster `server` in that kubeconfig. **No `pods/exec`**, no secrets, no impersonate, no VM start/stop, no `nodes/proxy`. Dedicated Secret — do **not** reuse an SA or kubeconfig that already has exec, VM mutate, or `nodes/proxy`. | +| Sandbox pod SA | Sandbox pods | Exists because OpenShift requires an SA. **No RoleBindings. `automountServiceAccountToken: false`.** Avoids a second in-cluster identity beside the dummy kubeconfig. | + +> **Open question:** exact investigation ClusterRole — see [Q2](credential-proxy-questions.md). +> +> **Open question:** sandbox SA name — see [Q9](credential-proxy-questions.md). Operator Q12 already chose a dedicated sandbox SA with automount false; proxy Q9 should not re-litigate that. + +Current as-built sets `ServiceAccountName: cli-mcp-investigation-sa` and does **not** set `automountServiceAccountToken`. Kubernetes defaults that to **true**, so today a sandbox would also get a projected token for that SA. That must not survive this change. (Operator phase already switches the pod SA and automount; this pass swaps the mounted kubeconfig for dummy.) + +### Images (v1) + +| Image | Binary | Role | +|---|---|---| +| `cli-mcp-server` | `cmd/server` | MCP `bash`, session lifecycle, (Q1) reconcile derived objects | +| `cli-mcp-sandbox` | `cmd/agent` + `oc`/`kubectl`/`jq`/`yq`/`curl` | Per-session bash. Unchanged CLIs; env/mounts change. | +| `cli-mcp-proxy` | `cmd/proxy` | MITM forward proxy | + +CD (`.github/workflows/cd.yml`) gains a third matrix entry. `Containerfile.proxy` is a minimal UBI image like the server. + +### Package layout (target) + +``` +cmd/server/ MCP server (existing) + startup reconcile +cmd/agent/ sandbox agent (unchanged) +cmd/proxy/ new — claw-style flags: --config, --ca-cert, --ca-key, --listen +pkg/session/ pod spec: dummy CM, HTTPS_PROXY, labels, automount false +pkg/proxy/ MITM server + kubernetes injector (copied/adapted from claw) +pkg/kubeconfig/ validate token-only kubeconfig, sanitize dummy, build route JSON +pkg/infra/ CA ensure, dummy ConfigMap, NetworkPolicies (shape depends on Q1) +``` + +> **Open question:** how much of claw’s injector surface to copy — see [Q7](credential-proxy-questions.md). + +`pkg/infra` / “startup reconcile” above is **stale HOW** (Q1: operator owns those objects, not `cmd/server`). + +### Sandbox pod spec changes + +On top of the as-built spec (`buildBasePodSpec` / warm pool): + +| Field | v1 | +|---|---| +| Labels | Operator Q8: `cli-mcp.redhat.com/component=sandbox`, `cli-mcp.redhat.com/session-id` when assigned, `cli-mcp.redhat.com/instance=`. Replace as-built `tarsy.redhat.com/*`. Proxy pods: `component=proxy` + same instance. | +| `automountServiceAccountToken` | `false` | +| `serviceAccountName` | Dedicated sandbox SA (operator Q12 / [Q9](credential-proxy-questions.md)), not the investigation SA | +| kubeconfig volume | ConfigMap dummy, not the real Secret | +| Env | `KUBECONFIG=/config/kubeconfig` (unchanged path); `HTTP_PROXY` + `HTTPS_PROXY` = `http://:8080`; `NO_PROXY=127.0.0.1,localhost,::1` so the agent loopback readiness probe and local curl do not go through the proxy. **Do not** put `.svc`, `.cluster.local`, or API hostnames in `NO_PROXY` — that would bypass the proxy for in-cluster API. | +| Readiness | Unchanged: exec `curl` to `127.0.0.1:8090/health` (loopback; works with server-only ingress NP) | + +`NO_PROXY` is a load-bearing footgun: a broad cluster-local list would let `oc` reach `kubernetes.default.svc` directly. + +### Proxy configuration + +Route list JSON (claw format), one route per kubeconfig cluster server: + +```json +{ + "routes": [ + { + "domain": "api.host.example.com:6443", + "injector": "kubernetes", + "kubeconfigPath": "/etc/kube/config", + "caCert": "" + } + ] +} +``` + +v1 enables only kubernetes routes. The binary stays route-list driven so a later class is a new instance + ConfigMap, not a new architecture. + +Real kubeconfig: admin-provided Secret (GitOps, External Secrets, or `kubectl`). Cluster admin provisions investigation SAs and tokens; this operator does not mint tokens. + +> **Open question:** proxy restart when that Secret rotates — see [Q11](credential-proxy-questions.md). +> +> **Open question:** CA create-if-missing vs rotation — see [Q10](credential-proxy-questions.md). + +### MCP server flags (additive) + +Existing flags stay. Additions (names indicative): + +| Flag | Role | +|---|---| +| `--instance-name` | Label + resource name suffix (v1 default `oc`) | +| `--investigation-kubeconfig-secret` | Real kubeconfig (proxy mount / dummy source) | +| `--proxy-service` | DNS name used in `HTTPS_PROXY` | +| `--dummy-kubeconfig-configmap` | Well-known dummy ConfigMap name | +| `--proxy-ca-secret` | Proxy CA Secret name | + +Exact reconcile loop vs “GitOps already created it” depends on Q1 (decided: **operator** renders these onto the MCP Deployment; MCP does not watch the CR). + +`--kubeconfig` on the server remains the **MCP’s** client-go config for managing sandbox resources (in-cluster in production). It is not the investigation kubeconfig. + +### What does not change + +- Single MCP tool: `bash`. No command filter. +- `X-Session-ID`, HMAC `/exec`, warm pool, idle GC, `DELETE /sessions/{id}`. +- Namespace-pinned `exec` / file forensics, if needed, stay on a **different** tool. This proxy does not provide `oc exec`. +- Client-side data masking is still useful for tokens that **do** return to the LLM; it is not a replay control. + +## Core Concepts + +| Concept | Role | +|---|---| +| **Instance / class** | One MCP Deployment + one sandbox image + one proxy + one NP set. v1: `oc`. A later curl instance is a second CR of the same operator with different flags/image — **not v1**. | +| **Dummy kubeconfig** | What `oc`/`kubectl` read in the sandbox. Real hosts, fake token, proxy CA. | +| **Real kubeconfig** | Tokens + real API CAs. Proxy only. Token-only auth. | +| **MITM forward proxy** | `HTTPS_PROXY` + CONNECT. Clients keep real hostnames. A kube-API reverse proxy would not extend to a later non-kube class. | +| **Strip-then-inject** | Stolen `Authorization` cannot survive the hop to the API. | +| **Proxy ingress NP** | Only this instance’s sandbox pods may use the proxy. | +| **Sandbox egress NP** | Sandboxes cannot skip the proxy. | +| **Investigation RBAC** | Last line if injection works as designed: even “successful” API calls are view-only, no exec. | + +## Implementation Plan + +**Paused.** Do not execute these phases as written. They assume in-process MCP ownership of child objects (pre-Q1). After the operator is implemented, rewrite this plan as operator reconcilers + a later proxy pass. + +### Phase 0 — Decisions + +Q1 done (operator). Q2–Q12 paused. Operator HOW: [cli-mcp-operator-design.md](cli-mcp-operator-design.md) (Final). + +### Phase 1 — Proxy binary in this repo + +- Add `cmd/proxy` + `pkg/proxy` (goproxy MITM, route JSON, `StripAuthHeaders`, kubernetes injector, upstream CA pool). +- Unit tests: route match (host:port), unknown host CONNECT rejected, Authorization stripped and replaced, token-only kubeconfig validation, sanitize swaps CA + dummy token. +- `Containerfile.proxy`, `make build-proxy` / `image-proxy`, CD matrix entry. +- **Verify:** `go test ./pkg/proxy/... ./pkg/kubeconfig/...`; local CONNECT to a fake API with dummy vs real token. + +### Phase 2 — Sandbox pod contract + +- Dummy ConfigMap mount; `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY`; `automountServiceAccountToken: false`; instance label; sandbox SA. +- Update `pkg/session` tests (`buildBasePodSpec`, warm pool spec). +- **Verify:** existing session/pool tests; golden pod spec assertions for env, volumes, automount, labels. + +### Phase 3 — Derived cluster objects + fail-closed + +- CA ensure, dummy ConfigMap, four NetworkPolicies, route ConfigMap — **owned per Q1** (operator). +- MCP RBAC in GitOps for whatever the server applies. +- Startup: if proxy/dummy/NPs are not ready, do not create sandbox pods ([Q12](credential-proxy-questions.md)). +- **Verify:** fake client-go tests for reconcile; NP selectors cannot match a different instance label. + +### Phase 4 — First-party install (separate from this repo) + +- Catalog consumer: `CliMcpInstance` + Secrets + investigation ClusterRole(s)+bindings + cluster RBAC, in whatever namespace that environment uses. Operator already owns MCP Deployment (kube-rbac-proxy sidecar), and in this pass proxy Deployment+ClusterIP Service, SAs, dummy/routes/NPs. +- Wire the MCP client to the instance only after it is Ready (including proxy children). +- **Do not** add namespace EgressFirewall to API IPs. +- **Verify:** `kubectl auth can-i` as investigation SA (no exec, no secrets get, no impersonate); NetworkPolicy from a throwaway pod in the instance namespace cannot CONNECT to the proxy; sandbox cannot reach API IPs. + +### Phase 5 — Enablement + +- Confirm `oc --token` / `curl -H Authorization` through the sandbox still only has investigation RBAC. Confirm `unset HTTPS_PROXY` / `oc --server` fail. Then production. + +## Open Questions + +Full options: [credential-proxy-questions.md](credential-proxy-questions.md). **Q2–Q12 paused** pending operator implementation. + +| # | Topic | +|---|---| +| [Q1](credential-proxy-questions.md) | **Decided:** CLI MCP Operator (not in-process MCP, not GitOps-static) | +| [Q2](credential-proxy-questions.md) | Investigation ClusterRole scope | +| [Q3](credential-proxy-questions.md) | Proxy-Authorization belt | +| [Q4](credential-proxy-questions.md) | Proxy egress tightness | +| [Q5](credential-proxy-questions.md) | CONNECT to raw IPs | +| [Q6](credential-proxy-questions.md) | L7 denylist for kube subresources | +| [Q7](credential-proxy-questions.md) | How much claw proxy code to copy | +| [Q8](credential-proxy-questions.md) | Instance label / naming — **constrained by operator Q8** (`cli-mcp.redhat.com`; drop `tarsy.redhat.com`) | +| [Q9](credential-proxy-questions.md) | Sandbox pod ServiceAccount — **constrained by operator Q12** (dedicated SA, automount false) | +| [Q10](credential-proxy-questions.md) | Proxy CA lifecycle | +| [Q11](credential-proxy-questions.md) | Investigation kubeconfig rotation | +| [Q12](credential-proxy-questions.md) | Fail-closed until proxy is ready | + +## Out of scope / non-goals + +- Command allowlists on `bash`. +- `oc exec` / port-forward / attach through the proxy (use a namespace-pinned tool if you need that). +- Sharing the claw-operator proxy image or extracting a new common proxy repo. +- Namespace EgressFirewall to kube API IPs **together with** the proxy. +- Per-session proxy pods or proxy sidecars. +- One shared proxy for multiple classes (union allowlist). +- **A curl (or any non-oc) MCP instance, sandbox image, or proxy — v1 is oc/kubectl + kube-API proxy only.** Curl in this doc is only to illustrate a second class. +- Automatic proxy CA rotation (Q10 may confirm generate-once). +- Per-user credentials (shared investigation kubeconfig, unchanged from as-built). +- Interactive stdin (`oc edit`, `oc exec -it`). diff --git a/docs/proposals/credential-proxy-questions.md b/docs/proposals/credential-proxy-questions.md new file mode 100644 index 0000000..cee6d20 --- /dev/null +++ b/docs/proposals/credential-proxy-questions.md @@ -0,0 +1,286 @@ +# CLI MCP — credential-isolating proxy — design questions + +**Status:** Paused after Q1 — remaining questions wait on the operator **implementation** +**Related:** [Design document](credential-proxy-design.md) · [Operator HOW](cli-mcp-operator-design.md) · [Operator questions](cli-mcp-operator-questions.md) + +Q1 is decided. **Q2–Q12 are on pause** until the CLI MCP Operator is implemented. This proxy document is the component/security inventory (WHAT). The operator is HOW those objects are managed. After that implementation, return here and finish these questions with controller ownership in mind. + +This is an **open-source Kubernetes operator**. First-party internal deploy is one catalog consumer; do not bake that environment into the API. + +--- + +## Q1: Who owns the proxy stack vs derived objects? + +The umbrella analysis said the MCP server would create the proxy Deployment, route ConfigMap, CA, dummy kubeconfig, and NetworkPolicies. That is a mini-operator inside a process that must stay **stateless and multi-replica** for `bash`. Traditional operators are leader-elected singletons. Dummy kubeconfig and routes **must** be derived from the live investigation Secret + proxy CA (not frozen in git). Child objects also need watches, ownerRefs, and status — which a 30s ensure loop in `cmd/server` would re-invent. + +### Option D: Real Kubernetes operator (CLI MCP Operator) + +A CRD instance (one CR = one MCP instance / class) is the API. A leader-elected operator Deployment watches it and bootstraps instance infrastructure: MCP server Deployment, proxy Deployment+ClusterIP Service, CA, route ConfigMap, dummy kubeconfig, NetworkPolicies, sandbox SA, and related RBAC wiring. + +The MCP server stays a horizontally scaled data plane: `bash`, per-session pods, HMAC claim/create/`/exec`. It does **not** reconcile the proxy stack, the MCP Deployment, warm-pool size, or idle GC (those are the operator — see operator Q5). Sessions are not CRs. + +GitOps installs the operator once and applies `CliMcpInstance` (name TBD) objects. It does not hand-maintain the proxy Deployment. + +- **Pro:** Proper watches (deleted/edited proxy comes back), ownerRefs/GC, status (`ProxyReady`), validation on spec (sandbox image, proxy config, warm pool). A second class later is another CR, not a snowflake Deployment. Matches claw-operator’s split (operator owns children; workload process is not the controller). Resolves singleton-vs-stateless: operator is the singleton; MCP replicas stay stateless. +- **Con:** CRD, manager, envtest, operator RBAC — larger than “apply() in `cmd/server`.” Two Deployments to run (operator + MCP). Operator design must reserve spec/status for proxy children even if proxy ships in a later phase. + +**Decision:** Option D — real operator. This proxy design keeps the security/topology WHAT; operator design will be HOW. Do not put an ensure-loop mini-operator in the MCP process. + +_Considered and rejected: Option A (in-process MCP reconcile of the proxy Deployment — re-invents operator watches/ownerRefs/status and collides with stateless MCP replicas), Option B (GitOps-static proxy/dummy/routes — cannot derive dummy kubeconfig and CA at runtime; NP drift is the token-replay footgun), Option C (GitOps Deployments + MCP-derived CA/dummy/NPs — splits the security boundary across two owners and still needs a half-written reconciler in the MCP)._ + +--- + +**Pause:** Q2–Q12 below are unchanged in *decision status* and **not** being walked through until the operator is implemented. Wording is aligned with the operator HOW (Final): `cli-mcp.redhat.com` labels, dedicated sandbox SA, operator owns children. + +## Q2: How broad is the investigation ClusterRole? + +This is the identity the proxy injects. It must be a **read-only investigation** surface, not an SA that already has `pods/exec`, VM start/stop, or `nodes/proxy`. Tokens live in a **new** Secret. The operator does not mint those tokens (admin-provided kubeconfig). + +### Option A: Bind `view` + a dedicated `cli-mcp-investigation-readonly` ClusterRole + +`view` for namespaced get/list/watch. Extra ClusterRole for cluster-scoped reads investigators actually use (namespaces, PVs, CRDs, ClusterRoles, storageclasses, metrics, and on OpenShift e.g. clusteroperators) **minus** `nodes/proxy`, **minus** any exec/attach/portforward, **no secrets**. + +- **Pro:** Close to a typical cluster-wide `oc` investigation view; a client replacing a broader kubernetes MCP does not silently lose cluster-scoped reads. +- **Con:** `view` is broad (all namespaced resources including user Secrets **in every namespace**). `view` includes `get` on `secrets`. That is a real data leak via `oc get secret` even with no exec. + +### Option B: Custom ClusterRole only — no `view`; explicit resource list; no secrets + +Hand-maintained rules: pods, logs, events, controllers, routes, networkpolicies, CRs needed for investigations, cluster-scoped reads from Option A, **no `secrets`**, no `pods/exec`, no `nodes/proxy`. + +- **Pro:** Closes `oc get secret -A` / `oc get secret … -o yaml`. Least privilege vs `view`. +- **Con:** Will miss resources until someone adds them; more GitOps churn. Investigations that today `oc get secret` would break — that is intended. If a specific namespace’s files must be read, use a **separate**, namespace-pinned tool, not this proxy. + +### Option C: Reuse an existing privileged SA / kubeconfig but rely on no-exec + proxy + +- **Pro:** No new tokens. +- **Con:** If that SA has exec or VM power, proxy injection would give the bash sandbox **exec**. Violates the stated no-exec requirement. + +**Recommendation:** Option B. Do not bind `view` (it includes secrets). Do not reuse an SA that already has exec / VM mutate / `nodes/proxy`. Start from the *effective* investigation surface you need, strip secrets/exec/`nodes/proxy`/impersonate/VM mutate, then add resources only when a real investigation hits a gap. Verify with `kubectl auth can-i --list`. + +--- + +## Q3: Require HTTP `Proxy-Authorization` in addition to NetworkPolicy? + +kubectl honors `HTTPS_PROXY=http://user:pass@host:8080` and sends `Proxy-Authorization` on CONNECT. NetworkPolicy (proxy ingress) is the primary control so only sandbox pods can reach `:8080`. This would be a second factor if something in the **instance namespace** can spoof sandbox labels or if NP is mis-applied. + +### Option A: NetworkPolicy only (v1) + +- **Pro:** Matches claw-operator today (claw has **no** proxy-ingress NP and no proxy basic auth — we are already stricter on NP). Fewer secrets. kubectl/`curl` keep a simple `HTTPS_PROXY`. +- **Con:** Anyone who can run a pod with this instance’s sandbox labels in the CR namespace can use the proxy and thus the investigation token. That already implies they can create pods in that namespace (high privilege). + +### Option B: NP + proxy basic auth (secret in sandbox env) + +- **Pro:** Defense in depth if labels are copied or NP selectors go wrong. +- **Con:** Secret mounted in every sandbox (readable by bash — but it only authorizes *use of the already-dummy path*, not a cluster token). goproxy must enforce CONNECT auth; claw does not. More moving parts for v1. + +**Recommendation:** Option A for v1. Proxy ingress NP + ClusterIP + instance labels. Revisit auth if we ever run untrusted workloads in the instance namespace that can set arbitrary pod labels. + +--- + +## Q4: How tight is proxy egress NetworkPolicy? + +Sandbox egress is proxy+DNS only. Proxy egress must reach every API server listed in the investigation kubeconfig (often `:6443`, in-cluster `:443`). Claw’s kube path adds those ports to `0.0.0.0/0` and treats L7 as the real allowlist. + +### Option A: DNS + TCP 443 and 6443 to `0.0.0.0/0` + +- **Pro:** API load-balancer and PrivateLink IPs can change without NP edits. Same as claw. L7 host allowlist still 403s unknown CONNECT. +- **Con:** If L7 is buggy, the proxy pod can speak HTTPS to the internet. `0.0.0.0/0` does not cover IPv6. + +### Option B: Resolve kubeconfig hostnames at reconcile time; NP `ipBlock` CIDRs + +- **Pro:** Proxy cannot talk to arbitrary IPs even if L7 fails. +- **Con:** DNS TTL / NLB change → outage until re-reconcile. Need to handle multiple A records, IPv6, and `kubernetes.default.svc` cluster IPs. Painful on OpenShift. + +### Option C: OpenShift EgressFirewall / DNSNames on the **proxy** namespace or pod + +- **Pro:** Hostname-level egress at CNI. +- **Con:** EF is namespace-scoped on OpenShift, not per-pod. Would affect the MCP server and any other workloads in the instance namespace if applied to the whole namespace. Per-pod FQDN policy needs Cilium (not the cluster default). Easy to get wrong; the umbrella analysis already rejected EF on **sandbox** pods together with the proxy. + +**Recommendation:** Option A for v1, plus IPv6 `::/0` on the same ports if the cluster is dual-stack. Document L7 as the real host allowlist. Do not put EF on sandbox pods. + +--- + +## Q5: Allow CONNECT to raw IP addresses? + +`MatchRoute` is hostname-based. `oc` uses the kubeconfig `server` URL (usually a hostname). An attacker in the sandbox can `curl -x $HTTPS_PROXY https://:6443` with a stolen token. If we also inject by IP, that becomes a replay path unless strip-then-inject still replaces Authorization (it would — injection is by host key). If the IP is **not** in the token map, inject fails closed (good) but CONNECT might still be allowed as a tunnel. + +### Option A: Reject CONNECT unless the host matches a kubeconfig server host:port (IPs only if the kubeconfig server is an IP) + +- **Pro:** No extra tunnel to “something on 6443.” Matches strip-then-inject: unknown host is 403. +- **Con:** If a cluster is only reachable by IP and kubeconfig uses a hostname, `oc` still uses the hostname (fine). Unusual kubeconfigs that mix IP and hostname need the IP as a cluster server URL. + +### Option B: Also map resolved IPs to the same token (inject on IP CONNECT) + +- **Pro:** `curl https://` works like `oc`. +- **Con:** DNS/IP drift; easier to accidentally allow CONNECT to a shared LB IP that fronts more than the API. More code. + +**Recommendation:** Option A. Allow IP CONNECT only when that `ip:port` is literally a kubeconfig `server`. Do not DNS-resolve and add IPs. + +--- + +## Q6: Deny kube subresources at L7 (`exec` / `attach` / `portforward` / `proxy`)? + +Investigation RBAC should already deny these. Bash can still *attempt* them. Claw kubernetes routes do not path-filter; `AllowedPaths` exists on the proxy for other injectors. + +### Option A: RBAC only + +- **Pro:** One source of truth. No denylist to maintain (`pods/ephemeralcontainers`, impersonate already stripped as headers, `nodes/proxy`, …). +- **Con:** Mis-bound ClusterRole + `oc exec` is instant cluster-admin-adjacent in user namespaces. + +### Option B: Deny-list well-known mutating subresource path suffixes on kubernetes routes + +Reject paths matching `…/exec`, `…/attach`, `…/portforward`, `…/proxy` (and impersonate is already header-stripped). + +- **Pro:** Cheap belt given unconstrained bash. Survives a RoleBinding mistake. +- **Con:** Path matching on the kube API is annoying (query strings, `?command=`, SPDY). False positives possible; must test `oc logs`, `oc get --watch`, `oc explain`. + +**Recommendation:** Option B as a small denylist with tests for `logs`/`watch` still allowed. RBAC remains authoritative; this is belt-and-suspenders for the exact bypass we are designing against (`exec`). + +--- + +## Q7: How much claw-operator proxy code to copy? + +Goal: own image, no claw-operator release coupling. claw’s `internal/proxy` also has gateway/pathPrefix reverse-proxy mode, Slack body rewrite, GCP token vending, oauth2, path_token, api_key. + +### Option A: Minimal — MITM CONNECT + kubernetes injector + `none` + StripAuthHeaders + route matching + CA pool + +- **Pro:** Smallest attack surface and test matrix. Enough for v1 kube and for a later curl class (`none` or `bearer` can be added then). +- **Con:** Harder to diff against claw later; a later curl class may need `bearer` immediately. + +### Option B: Minimal + `bearer` injector now, still no gateway/Slack/GCP/oauth2 + +- **Pro:** Route-list architecture is real in v1 (kubernetes + bearer types exist; v1 ConfigMap only enables kubernetes). Curl illustration stays honest. +- **Con:** A few more files/tests unused in production v1. + +### Option C: Copy the claw package almost whole, delete Slack rewrite only + +- **Pro:** Easier to pull claw bugfixes. +- **Con:** Dead injectors, gateway mode we do not want (clients would skip CONNECT), GCP dummy-token behavior is confusing in this threat model. + +**Recommendation:** Option B. Copy MITM + kubernetes + bearer + none. Do not copy gateway mode, Slack, GCP, oauth2, path_token, api_key. Keep claw’s CONNECT allow/deny and upstream TLS verification (never goproxy’s default `InsecureSkipVerify`). + +--- + +## Q8: Instance identity for labels and resource names? + +**Constrained by operator Q8** (decided): CR `metadata.name` is the instance id. Labels and annotations live under `cli-mcp.redhat.com`. As-built `tarsy.redhat.com/*` is dropped (nothing in production; no migration). Children named `cli-mcp-`. + +v1 is one instance. Selectors must not be a single shared `component=sandbox` so a later instance in the **same namespace** does not share NPs. + +When this question is resumed, the remaining work is proxy-specific labels, not a new domain: + +### Option A: Follow operator Q8; proxy pods get `component=proxy` + +- `cli-mcp.redhat.com/instance=` on MCP, sandbox, session Secrets, and proxy pods. +- `cli-mcp.redhat.com/component=sandbox` \| `server` \| `proxy`. +- Proxy Service / Deployment named `cli-mcp-proxy-` or `cli-mcp--proxy` (pick one when implementing; must fit 63 chars with the sandbox SA suffix already reserved). +- NPs and proxy ingress select **instance + component**. Session list/GC stays component+instance as the operator phase. + +- **Pro:** One label domain. NP podSelectors are obvious. Two CRs in one namespace cannot share proxies. +- **Con:** None beyond the operator phase already accepted. + +### Option B: Reuse a single `component=cli-mcp-sandbox` value and encode class in the value + +- **Pro:** No extra instance key. +- **Con:** Breaks operator-phase meaning of `component=sandbox` (session manager, warm pool, idle GC). Rejected by operator Q8. + +### Option C: A separate `cli-mcp-class` label besides instance + +- **Pro:** Could distinguish class vs instance if we ever run two `oc` CRs. +- **Con:** Two labels to document; CR name already is the instance id. + +**Recommendation:** Option A. Do not re-open `tarsy.redhat.com` or a shared component-only selector. + +--- + +## Q9: What ServiceAccount do sandbox pods run as? + +**Constrained by operator Q12** (decided): dedicated sandbox SA, no RoleBindings, `automountServiceAccountToken: false`. Investigation tokens must not be the pod’s projected SA token. This question is kept so the proxy pass can confirm the SA name and that the investigation subject exists **only** as ClusterRoleBinding subjects whose tokens are minted into the proxy’s kubeconfig Secret. + +### Option A: Dedicated `cli-mcp--sandbox` SA, no RoleBindings, `automountServiceAccountToken: false` + +- **Pro:** Clear split. Compromised sandbox gets no in-cluster identity. OpenShift still has an SA for SCC. +- **Con:** One more object (already created in the operator phase). + +### Option B: Investigation SA on the pod with automount false; tokens only in the proxy kubeconfig + +- **Pro:** Fewer SAs. +- **Con:** Name implies the sandbox *is* the investigation identity. Accidental automount true (revert/bug) immediately projects a useful in-cluster token, bypassing the dummy kubeconfig. Operator Q12 already rejected this. + +### Option C: `automountServiceAccountToken: false` and empty `serviceAccountName` (default SA in namespace) + +- **Pro:** No extra SA. +- **Con:** Namespace `default` SA is a footgun if anyone binds it. Less explicit. Operator Q12 already rejected this. + +**Recommendation:** Option A — same as operator Q12. Investigation SA exists only as the subject of ClusterRoleBindings whose tokens are minted into the proxy’s kubeconfig Secret (typically via External Secrets or equivalent), never mounted on sandbox pods. + +--- + +## Q10: Proxy CA lifecycle? + +MITM requires a CA the dummy kubeconfig trusts. Claw generates a P-256 ECDSA CA once, stores it in a Secret, and never rotates unless the Secret is deleted. + +### Option A: Generate-once (create-if-not-exists), 10-year lifetime, no automatic rotation + +- **Pro:** Same as claw. Dummy kubeconfig and running sandboxes stay valid. MCP replicas do not flip-flop CAs. +- **Con:** Compromise of `ca.key` means forging API-looking certs to sandboxes (they can only talk to the proxy anyway). Rotation is a documented break-glass: delete CA Secret + dummy CM, bounce proxy, idle-GC sandboxes. + +### Option B: cert-manager (or OpenShift service CA) + +- **Pro:** Rotation/policy exists in platform. +- **Con:** Service CA cannot sign arbitrary MITM leafs for `api.`. cert-manager is another dependency in the instance namespace for one Secret. + +### Option C: New CA every MCP restart + +- **Pro:** Short-lived. +- **Con:** Breaks warm pool and live sessions; multi-replica races. + +**Recommendation:** Option A. Generate-once in the CA Secret (**operator**, Q1). Copy claw’s CA template (IsCA, KeyUsageCertSign, ECDSA P-256). + +--- + +## Q11: How does the proxy pick up investigation kubeconfig rotation? + +An ExternalSecret (or equivalent) may rotate tokens. Claw stamps the Secret `resourceVersion` on the proxy Deployment to force a rollout; the proxy reads kubeconfig at **startup** only (no file watch). + +### Option A: Reloader / `secret.reloader.stakater.com` annotation in GitOps + +- **Pro:** No operator code. Common on OpenShift. +- **Con:** Depends on Reloader being installed in the cluster (confirm). Dummy kubeconfig cluster *list* also needs refresh if servers were added — operator reconcile on an interval can rewrite dummy/routes; proxy still needs restart to reload tokens. + +### Option B: Operator watches the Secret and patches the proxy Deployment annotation + +- **Pro:** Self-contained. Dummy + routes + proxy stay in lockstep. +- **Con:** Operator needs patch on the proxy Deployment. More controller behavior (acceptable: Q1 already made the operator the owner). + +### Option C: Proxy watches the kubeconfig file (inotify) and reloads + +- **Pro:** No rollout. +- **Con:** Reload races, token map mutex, not how claw works; more proxy complexity. + +**Recommendation:** Option B if Reloader is not already a standard in the target cluster; otherwise Option A plus the operator periodically rewriting dummy/routes. Default to **Option B** unless GitOps owners confirm Reloader. Tokens must not stay stale after rotation. + +--- + +## Q12: Fail-closed if proxy, dummy kubeconfig, or NPs are not ready? + +If sandbox pods are created before egress NP exists, they have unrestricted egress (the instance namespace has no default-deny today). That window is the original bug. + +Operator Q5 moved warm pool to the operator; MCP still creates on-demand session pods. Both paths must honor this gate. Operator Q15 `Ready` should include proxy children in the proxy pass. + +### Option A: Do not create/claim sandbox pods until Ready proxy endpoints, dummy ConfigMap, and the four NPs are observed + +- **Pro:** No open-egress sandbox even on first deploy / MCP crashloop. +- **Con:** First `bash` call waits on proxy readiness (acceptable). Warm pool must not pre-create pods either until the same gate passes. + +### Option B: GitOps ordering only (proxy+NPs in the same Argo app, hope apply order is enough) + +- **Pro:** No extra code. +- **Con:** Kubernetes does not give you transactional multi-resource apply. A race on first rollout is likely. + +### Option C: Namespace default-deny NetworkPolicy in GitOps, then allow-lists + +- **Pro:** Even a buggy operator/MCP cannot create a sandbox with open egress. +- **Con:** Default-deny in the instance namespace would break the MCP client, other MCP servers, observability, and anything else in that namespace unless carefully namespaced by podSelector. Easy to outage the whole namespace. + +**Recommendation:** Option A. Gate both on-demand create and warm-pool replenish. Do not namespace-wide default-deny the instance namespace.