Repository navigation
Commit 5cfffaa
authored
🤖 feat: activate the CoderMCPServer controller (#381)
## Summary
The `CoderMCPServer` controller now runs. A `CoderMCPServer` creates and
manages one MCP server config in a Coder organization, which Coder chats
offer to the model. Before this PR the kind was dormant: its controller
was not registered and its CRD was not installed.
This is plan step M9 of #155. Steps M1 to M8 landed in earlier PRs, the
last ones as #368 (delete path), #371 (E2E stub), #376 (test S9) and
#377 (the opt-in `/status` admission policy).
## What changes
1. `internal/app/controllerapp/controllerapp.go` registers
`CoderMCPServerReconciler`.
2. The CRD moves from `config/crd/dormant/` to `config/crd/bases/`, into
`config/default/kustomization.yaml` and `dist/install.yaml`.
`hack/update-manifests.sh` drops it from `DORMANT_CRDS`, and the CI
dormancy guard step for the kind goes.
3. RBAC markers: `get`, `list`, `watch`, `patch` on `codermcpservers`,
`get` and `update` on `codermcpservers/status`, `update` on
`codermcpservers/finalizers`. The `secrets` rule does not change (test
S9, #376, passes).
4. A sample with its credential Secret:
`config/samples/coder_v1alpha1_codermcpserver.yaml`.
5. The how-to `docs/how-to/manage-mcp-servers.md`, plus the nav, the
index, the architecture page, the controller deploy page and the
getting-started cleanup.
6. The API reference page `docs/reference/api/codermcpserver.md`. The
"no effect yet" sentences leave the `spec.mcpServers` field docs of
`CoderControlPlane` and the `CoderMCPServer` types.
7. The E2E path filter gains `internal/controller/codermcpserver*`.
## What the how-to documents
- The control plane opt-in (`spec.mcpServers.organizations`,
`allowForceOn`): organization names resolve to IDs, and a withdrawn
opt-in disables each affected server with one `enabled: false` update
and no credential read. The object then reports `DisabledExternally`
until a spec edit.
- The credential Secret contract (label, destinations annotation, value
rules), and that Coder sends the credential again after a redirect.
- Access: `spec.access` lists groups only, so every sync removes user
grants added in the dashboard. With `access` omitted, Coder's default
lets the organization's Everyone group read the server.
- The #313 fail-closed behavior on a missing group: remove unnamed
entries, add nothing, send `enabled: false` if the generation disables
the server, and add the groups once they exist.
- The #365 disable window and its known limit.
- Rotation, URL changes (and their order), dashboard kill switches, the
credential kill switch, the emergency path, and deletion (from #368).
- The Coder-side limits (plan 5.5): access checked only at chat
selection, tool lists by exact name, OAuth2 tokens deleted on a URL
change, encryption at rest only with enterprise keys, no address guard.
- RBAC guidance: what each permission means in Coder, a viewer Role, an
editor Role, the wildcard warning, a `ResourceQuota` note.
- `config/admission/`: opt-in (the install bundle does not apply it),
needs `ValidatingAdmissionPolicy` (GA in Kubernetes 1.30), and
hard-codes the ServiceAccount
`system:serviceaccount:coder-system:coder-k8s`. Edit it when the
operator runs as another ServiceAccount.
## Dogfood
A full run of plan section 8 on a private Kind cluster (kind v0.26.0,
kindest/node v1.32.0, Coder v2.37.2, no license), with images built from
`091c7fcf`. That commit has the same controller code as this head. Later
commits changed only docs, the CI path filter, and moved the GitOps
health rules out. Every step passed:
| Step | Result |
|---|---|
| Opt-in, Secret, apply | `Ready` 233 ms after apply. Coder holds the
slug, URL, `enabled`, `has_api_key`, and an ACL of Everyone read only.
The key appears 0 times in the CR JSON and the operator logs. |
| Dashboard and chat | The server shows in **AI settings > MCP
servers**. A chat that selects it gets the stub's answer. The stub got a
matching key hash, and the LLM got `e2e-stub__alpha`, `e2e-stub__beta`
and `e2e-stub__Gamma`. |
| Rotation | Coder got the new key 99 ms after the Secret change, and
the next turn showed the new hash. An annotation-only change left
`updated_at` unchanged. |
| Refusals (P4 a to f) | `CredentialNotOptedIn`, `CredentialRefused`,
`CredentialDestinationNotAllowed`, `SlugConflict` (the other config
unchanged), `MCPServersNotAllowed`, `ForceOnNotAllowed`. No Coder config
for a, b, c, e, f. |
| Kill switch | A dashboard disable gives `DisabledExternally`, and
Coder stays disabled. A `displayName` edit enables it again. |
| URL change, both orders | The chat reached only the intended Service,
with a matching key hash. |
| Access | `groups: []`: a member's chat gets 400. `[Everyone]`: 201. |
| Admission policy | Status patches by an impersonated `system:masters`
user and by the Kind admin are denied with the policy message. The
controller still reconciles. |
| Delete, namespace delete | Coder answers 404 after the delete. In a
namespace deletion, a second CR releases with `ControlPlaneGone` (7 s),
and the namespace goes (17 s). |
For P4 (e), unlicensed Coder allows one organization, so the run listed
`[no-such-org]` instead. The dogfood found one how-to mismatch (the
reason after a restored opt-in), which this PR fixes. The screenshots
and the recording are in the evidence comment below.
## Validation
- Local gates on this head: `make verify-vendor`, `make build`, `make
lint`, `GOFLAGS=-mod=vendor go tool govulncheck ./...`, `make test`,
`make test-integration`, `go test -race ./internal/controller/`, and
`make docs-check` pass. `make codegen`, `make manifests`, `make
build-installer` and `make docs-reference` leave no diff. actionlint
passes.
- `TestSetupControllersStartsEveryController` (S18) now also needs a
`CoderMCPServer` to get its finalizer and `ControlPlaneNotReady`, which
proves the registration. A mutation that removes the registration fails
it.
- A new case in `TestCoderMCPServerControlPlaneAndOptIn`: a listed name
that resolves to the object's organization ID is allowed (IDs are
compared, not names). A mutation to a name compare fails it.
- `bash ./hack/e2e-workspace-lifecycle_test.sh` times out locally under
host load, also on unchanged `main`. CI runs it.
## Size
451 hand-written changed lines (300 of them docs) and 740 generated
(`dist/install.yaml`, the CRDs, `role.yaml`, the kustomization, the
reference pages).
## Follow-ups
- The Argo CD and Flux health rules for `CoderMCPServer` come in the
next PR, as #212 did for `CoderTemplateTest`.
- #362 (part 1 of #353) holds a deleted `CoderControlPlane` while
`CoderAIGatewayKey` and `CoderAIProvider` objects still use it. It does
not cover `CoderMCPServer`. A control plane deletion can therefore leave
MCP server configs in Coder. The how-to says to delete `CoderMCPServer`
objects first. Follow-up to #353: add `CoderMCPServer` to
`controlPlaneDependents`.
- #372: an ownership proof for Coder IDs in controller status (plan D11
B).
- #378: delete path and RBAC test follow-ups from the M7, S9 and S11
assessments.
- Each new CR logs one "Operation cannot be fulfilled" error when it
adds its finalizer. The controller retries it with no effect.
- Plan step M10 adds the Kind E2E phases, and M11 the tool lists. #155
stays open until then.
Refs #155
---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$220.71`_
<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high
costs=220.71 -->
---------
Signed-off-by: Thomas Kosiewski <tk@coder.com>1 parent 6dd8a6f commit 5cfffaa
24 files changed
Lines changed: 1116 additions & 75 deletions
File tree
- .github/workflows
- api/v1alpha1
- config
- admission
- crd/bases
- default
- rbac
- samples
- dist
- docs
- explanation
- how-to
- reference/api
- tutorials
- hack
- internal
- app/controllerapp
- controller
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| 54 | + | |
| 55 | + | |
54 | 56 | | |
55 | 57 | | |
56 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
| 90 | + | |
90 | 91 | | |
91 | 92 | | |
92 | 93 | | |
| |||
172 | 173 | | |
173 | 174 | | |
174 | 175 | | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
191 | 176 | | |
192 | 177 | | |
193 | 178 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
127 | 127 | | |
128 | 128 | | |
129 | 129 | | |
130 | | - | |
131 | | - | |
| 130 | + | |
132 | 131 | | |
133 | 132 | | |
134 | 133 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | 37 | | |
41 | 38 | | |
42 | 39 | | |
| |||
50 | 47 | | |
51 | 48 | | |
52 | 49 | | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | 50 | | |
57 | 51 | | |
58 | 52 | | |
| |||
76 | 70 | | |
77 | 71 | | |
78 | 72 | | |
79 | | - | |
| 73 | + | |
80 | 74 | | |
81 | 75 | | |
82 | 76 | | |
| |||
175 | 169 | | |
176 | 170 | | |
177 | 171 | | |
178 | | - | |
| 172 | + | |
179 | 173 | | |
180 | 174 | | |
181 | 175 | | |
| |||
277 | 271 | | |
278 | 272 | | |
279 | 273 | | |
280 | | - | |
| 274 | + | |
281 | 275 | | |
282 | 276 | | |
283 | 277 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1400 | 1400 | | |
1401 | 1401 | | |
1402 | 1402 | | |
1403 | | - | |
1404 | | - | |
| 1403 | + | |
1405 | 1404 | | |
1406 | 1405 | | |
1407 | 1406 | | |
| |||
1410 | 1409 | | |
1411 | 1410 | | |
1412 | 1411 | | |
1413 | | - | |
1414 | | - | |
1415 | | - | |
1416 | 1412 | | |
1417 | 1413 | | |
1418 | 1414 | | |
| |||
1427 | 1423 | | |
1428 | 1424 | | |
1429 | 1425 | | |
1430 | | - | |
1431 | | - | |
1432 | | - | |
1433 | 1426 | | |
1434 | 1427 | | |
1435 | 1428 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
382 | 382 | | |
383 | 383 | | |
384 | 384 | | |
385 | | - | |
| 385 | + | |
386 | 386 | | |
387 | 387 | | |
388 | 388 | | |
| |||
513 | 513 | | |
514 | 514 | | |
515 | 515 | | |
516 | | - | |
| 516 | + | |
517 | 517 | | |
518 | 518 | | |
519 | 519 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
| 89 | + | |
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| |||
97 | 98 | | |
98 | 99 | | |
99 | 100 | | |
| 101 | + | |
100 | 102 | | |
101 | 103 | | |
102 | 104 | | |
| |||
107 | 109 | | |
108 | 110 | | |
109 | 111 | | |
| 112 | + | |
110 | 113 | | |
111 | 114 | | |
112 | 115 | | |
| |||
135 | 138 | | |
136 | 139 | | |
137 | 140 | | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | 141 | | |
145 | 142 | | |
146 | 143 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
0 commit comments