From a7c43205fca7489c2a8f331625757e2f44dd4de5 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 14:54:41 +0000 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=A4=96=20feat:=20delete=20the=20Cod?= =?UTF-8?q?er=20MCP=20server=20config=20of=20a=20deleted=20CoderMCPServer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deleted CoderMCPServer now deletes the config that it created and then releases its finalizer (plan 4.10): - With a config ID, one DELETE by ID. Coder's own 404 also releases. Any other answer keeps the finalizer and retries every 60 s. - With only a create marker, the slug read decides: only the nonce proves that the config is this object's. A missing config waits until a late create cannot commit any more (2 minutes), and needs a site owner. - A deleted control plane releases the finalizer without a Coder request. A control plane that is being deleted gets one DELETE attempt bounded at 10 s, then the finalizer goes. - The scope gate never blocks the delete. - The released object's credential hashes leave the reconciler's map. Refs #155 Change-Id: Iaa3d096523bb12408656377958c20563409982f3 Signed-off-by: Thomas Kosiewski --- .../controller/codermcpserver_controller.go | 15 +- .../codermcpserver_controller_test.go | 20 +- internal/controller/codermcpserver_delete.go | 138 ++++++++++ .../controller/codermcpserver_delete_test.go | 242 ++++++++++++++++++ internal/controller/export_test.go | 8 + 5 files changed, 407 insertions(+), 16 deletions(-) create mode 100644 internal/controller/codermcpserver_delete.go create mode 100644 internal/controller/codermcpserver_delete_test.go diff --git a/internal/controller/codermcpserver_controller.go b/internal/controller/codermcpserver_controller.go index 6a5c4a184..11723133b 100644 --- a/internal/controller/codermcpserver_controller.go +++ b/internal/controller/codermcpserver_controller.go @@ -163,15 +163,16 @@ func (r *CoderMCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Reque if !controllerutil.ContainsFinalizer(server, coderv1alpha1.CoderMCPServerCleanupFinalizer) { return ctrl.Result{}, nil } - if server.Status.ConfigID == "" && server.Status.CreateAttemptTime == nil { - // Nothing of this object can exist in Coder. + release, step, err := r.deleteStep(ctx, server) + if err != nil { + return ctrl.Result{}, err + } + if release { + r.sentCredentials.Delete(server.UID) return ctrl.Result{}, r.patchFinalizer(ctx, server, false) } - // Keep the finalizer: this controller version cannot prove what Coder - // holds, so it neither deletes nor forgets the config. - applyMCPServerStep(server, mcpServerWait(0, "DeletePending", - "MCP server config %s can exist in Coder. This controller version keeps the finalizer and deletes nothing.", server.Spec.Slug)) - return ctrl.Result{}, r.writeStatus(ctx, server, before) + applyMCPServerStep(server, step) + return ctrl.Result{RequeueAfter: step.requeue}, r.writeStatus(ctx, server, before) } if err := r.patchFinalizer(ctx, server, true); err != nil { diff --git a/internal/controller/codermcpserver_controller_test.go b/internal/controller/codermcpserver_controller_test.go index e07666d68..142f9c3aa 100644 --- a/internal/controller/codermcpserver_controller_test.go +++ b/internal/controller/codermcpserver_controller_test.go @@ -543,15 +543,17 @@ func TestCoderMCPServerFinalizer(t *testing.T) { require.NoError(t, err) require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(got), got))) - // A config with a Coder ID keeps its finalizer: this version deletes nothing. - kept := env.create(ctx, t, nil) - kept, _ = env.reconcile(ctx, t, kept) - kept.Status.ConfigID = "5d0c6c2e-3a4d-4c55-9d2f-3f1b1f0c9a11" - require.NoError(t, k8sClient.Status().Update(ctx, kept)) - require.NoError(t, k8sClient.Delete(ctx, kept)) - kept, _ = env.reconcile(ctx, t, kept) - require.Equal(t, []string{coderv1alpha1.CoderMCPServerCleanupFinalizer}, kept.Finalizers) - requireMCPServerStep(t, kept, false, "DeletePending") + // A config with a Coder ID whose control plane is gone: nothing can + // delete it any more, so the finalizer goes without a Coder request + // (plan 4.10 item 5). + gone := env.create(ctx, t, nil) + gone, _ = env.reconcile(ctx, t, gone) + gone.Status.ConfigID, gone.Status.OrganizationID = "5d0c6c2e-3a4d-4c55-9d2f-3f1b1f0c9a11", env.orgID.String() + require.NoError(t, k8sClient.Status().Update(ctx, gone)) + require.NoError(t, k8sClient.Delete(ctx, gone)) + _, err = env.r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gone)}) + require.NoError(t, err) + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(gone), gone))) require.Zero(t, env.fake.totalRequests()) } diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go new file mode 100644 index 000000000..072364af8 --- /dev/null +++ b/internal/controller/codermcpserver_delete.go @@ -0,0 +1,138 @@ +package controller + +import ( + "context" + "fmt" + "time" + + "github.com/coder/coder/v2/codersdk" + "github.com/google/uuid" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/log" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" +) + +const ( + // mcpServerDeletePoll is the retry while a delete cannot reach Coder + // (plan 4.10 item 4). + mcpServerDeletePoll = 60 * time.Second + // mcpServerBoundedDeleteTimeout bounds the one delete attempt while the + // control plane is being deleted (plan 4.10 item 5). + mcpServerBoundedDeleteTimeout = 10 * time.Second + // mcpServerCreateSettle is how long after a create request a missing + // config does not yet prove absence: the request can still be in flight + // in Coder. + mcpServerCreateSettle = 2 * time.Minute +) + +// deleteStep handles an object that is being deleted (plan 4.10). It +// deletes in Coder only a config that this object proved it created, and it +// returns release when the finalizer can go. Otherwise the step says why the +// finalizer stays. The scope gate never blocks it. +func (r *CoderMCPServerReconciler) deleteStep(ctx context.Context, server *coderv1alpha1.CoderMCPServer) (release bool, step *mcpServerStep, err error) { + if server.Status.ConfigID == "" && server.Status.CreateAttemptTime == nil { + // Nothing of this object can exist in Coder. + return true, nil, nil + } + orgID, err := uuid.Parse(server.Status.OrganizationID) + if err != nil || orgID == uuid.Nil { + // The organization is pinned before the first MCP write. + return false, nil, fmt.Errorf("assertion failed: MCP server %s/%s can exist in Coder but has status.organizationID %q", server.Namespace, server.Name, server.Status.OrganizationID) + } + ref := types.NamespacedName{Namespace: server.Namespace, Name: server.Spec.ControlPlaneRef.Name} + controlPlane := &coderv1alpha1.CoderControlPlane{} + if err := r.Get(ctx, ref, controlPlane); apierrors.IsNotFound(err) { + return r.releaseWithoutDelete(ctx, server, "CoderControlPlane is gone"), nil, nil + } else if err != nil { + return false, nil, err + } + deleting := !controlPlane.DeletionTimestamp.IsZero() + sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, deleting) + switch { + case err != nil && deleting: + // One attempt only, also when the access setup fails. + return r.releaseWithoutDelete(ctx, server, "the Coder access of CoderControlPlane, which is being deleted, failed: "+err.Error()), nil, nil + case err != nil: + return false, nil, err + case wait != nil && deleting: + return r.releaseWithoutDelete(ctx, server, "CoderControlPlane is being deleted without operator access"), nil, nil + case wait != nil: + return false, mcpServerWait(mcpServerDeletePoll, wait.reason, "%s The controller deletes MCP server config %s once it can call Coder.", wait.message, server.Spec.Slug), nil + } + if deleting { + // One bounded attempt, then release, so the namespace deletion + // finishes. + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, mcpServerBoundedDeleteTimeout) + defer cancel() + } + step, err = deleteMCPServerInCoder(ctx, sdk, server, orgID) + switch { + case err != nil && deleting: + // The attempt ended with an impossible answer. Log it, and still + // release: the one attempt is over. + log.FromContext(ctx).Error(err, "The one delete attempt while CoderControlPlane is being deleted got an impossible answer") + return r.releaseWithoutDelete(ctx, server, "the one delete attempt while CoderControlPlane is being deleted failed: "+err.Error()), nil, nil + case err != nil: + return false, nil, err + case step != nil && deleting: + return r.releaseWithoutDelete(ctx, server, "the one delete attempt while CoderControlPlane is being deleted failed: "+step.message), nil, nil + } + return step == nil, step, nil +} + +// deleteMCPServerInCoder deletes the config of this object in Coder and +// returns nil when Coder holds none. With only a create marker, a read +// decides first: only the nonce proves that the config is this object's. +// Only Coder's own 404 proves absence (#337). +func deleteMCPServerInCoder(ctx context.Context, sdk *codersdk.Client, server *coderv1alpha1.CoderMCPServer, orgID uuid.UUID) (*mcpServerStep, error) { + var id uuid.UUID + if server.Status.ConfigID != "" { + parsed, err := uuid.Parse(server.Status.ConfigID) + if err != nil || parsed == uuid.Nil || parsed.String() != server.Status.ConfigID { + return nil, fmt.Errorf("assertion failed: MCP server %s/%s has status.configID %q, which is not a canonical UUID", server.Namespace, server.Name, server.Status.ConfigID) + } + id = parsed + } else { + found, step := findMCPServerSlug(ctx, sdk, server, orgID) + switch { + case step != nil: + step.requeue = max(step.requeue, mcpServerDeletePoll) + return step, nil + case found == nil: + // A missing config proves absence only for a site owner. A role + // change in Coder has no watch, so the wait uses the delete poll. + if step := mcpServerRequireSiteOwner(ctx, sdk); step != nil { + step.requeue = mcpServerDeletePoll + return step, nil + } + if wait := mcpServerCreateSettle - time.Since(server.Status.CreateAttemptTime.Time); wait > 0 { + // A create sent moments ago can still commit after this read. + return mcpServerWait(wait, "ConfirmingCreate", + "MCP server config %s is not in Coder, but its create request was sent at %s and can still commit. The controller reads again before it releases the finalizer.", + server.Spec.Slug, server.Status.CreateAttemptTime.UTC().Format(time.RFC3339)), nil + } + return nil, nil + case found.DisplayName != mcpServerNonce(server): + // Someone else's config holds the slug, so no create of this + // object committed. The controller never deletes it. + return nil, nil + } + id = found.ID + } + if err := sdk.DeleteMCPServerConfig(ctx, orgID, id); err != nil && !isCoderResourceNotFound(err) { + return mcpServerWait(mcpServerDeletePoll, "CoderUnavailable", "delete MCP server config %s (ID %s): %s", server.Spec.Slug, id, coderErrorSummary(err)), nil + } + return nil, nil +} + +// releaseWithoutDelete logs what can stay in Coder, because nothing can +// delete it any more, and returns true (ControlPlaneGone). +func (r *CoderMCPServerReconciler) releaseWithoutDelete(ctx context.Context, server *coderv1alpha1.CoderMCPServer, why string) bool { + log.FromContext(ctx).Info("Releasing the finalizer without a delete in Coder: the MCP server config can stay in the Coder database", + "reason", "ControlPlaneGone", "why", why, "slug", server.Spec.Slug, "configID", server.Status.ConfigID, + "createPending", server.Status.CreateAttemptTime != nil) + return true +} diff --git a/internal/controller/codermcpserver_delete_test.go b/internal/controller/codermcpserver_delete_test.go new file mode 100644 index 000000000..f6a3e76b7 --- /dev/null +++ b/internal/controller/codermcpserver_delete_test.go @@ -0,0 +1,242 @@ +package controller_test + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/controller" +) + +// deleteMCPServer deletes the object and runs one reconcile. It returns the +// object, or nil when the finalizer went and the object is gone. +func (env mcpServerEnv) deleteMCPServer(ctx context.Context, t *testing.T, server *coderv1alpha1.CoderMCPServer) (*coderv1alpha1.CoderMCPServer, ctrl.Result) { + t.Helper() + require.NoError(t, client.IgnoreNotFound(k8sClient.Delete(ctx, server))) + return env.reconcileDeleting(ctx, t, server) +} + +func (env mcpServerEnv) reconcileDeleting(ctx context.Context, t *testing.T, server *coderv1alpha1.CoderMCPServer) (*coderv1alpha1.CoderMCPServer, ctrl.Result) { + t.Helper() + result, err := env.r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(server)}) + require.NoError(t, err) + got := &coderv1alpha1.CoderMCPServer{} + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(server), got) + if apierrors.IsNotFound(err) { + return nil, result + } + require.NoError(t, err) + return got, result +} + +// Plan 4.10 items 3 and 6: deleting the object deletes the config that it +// created, also when the control plane no longer allows the organization, +// and then releases the finalizer. +func TestCoderMCPServerDeleteDeletesTheConfig(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + withdraw bool + }{{name: "opted in"}, {name: "opt-in withdrawn", withdraw: true}} { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, id := env.applied(ctx, t, nil) + if tc.withdraw { + env.setOptIn(ctx, t, nil) + } + mark := len(env.writeRoutes()) + gone, _ := env.deleteMCPServer(ctx, t, got) + require.Nil(t, gone, "the finalizer went") + require.Empty(t, env.fake.mcpServers()) + require.Equal(t, 1, env.fake.requestCount(routeDeleteMCPServer)) + require.Empty(t, env.writesSince(mark), "no write other than the DELETE") + require.NotContains(t, configIDs(env), id) + }) + } +} + +// Plan 4.10 item 3: a config that is already gone (Coder's own 404) releases +// the finalizer. Any other 404 keeps it and retries (#337), as do other +// errors (plan 4.10 item 4). +func TestCoderMCPServerDeleteAnswers(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + prepare func(context.Context, *testing.T, mcpServerEnv, *coderv1alpha1.CoderMCPServer) + fault *fakeFault + release bool + }{ + {name: "gone in Coder", release: true, prepare: func(ctx context.Context, t *testing.T, env mcpServerEnv, got *coderv1alpha1.CoderMCPServer) { + require.NoError(t, env.dashboard(t).DeleteMCPServerConfig(ctx, env.orgID, env.requireCreated(t, got).view.ID)) + }}, + {name: "proxy 404", fault: ptrTo(notCoder404s["HTML page"])}, + {name: "route not found", fault: ptrTo(notCoder404s["route not found"])}, + {name: "Coder unavailable", fault: &fakeFault{Status: http.StatusBadGateway}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + if tc.prepare != nil { + tc.prepare(ctx, t, env, got) + } + if tc.fault != nil { + env.fake.failNext(routeDeleteMCPServer, *tc.fault) + } + kept, result := env.deleteMCPServer(ctx, t, got) + if tc.release { + require.Nil(t, kept) + return + } + require.NotNil(t, kept) + require.Equal(t, []string{coderv1alpha1.CoderMCPServerCleanupFinalizer}, kept.Finalizers) + requireMCPServerStep(t, kept, false, "CoderUnavailable") + require.Equal(t, 60*time.Second, result.RequeueAfter) + require.Len(t, env.fake.mcpServers(), 1, "the config stays") + + gone, _ := env.reconcileDeleting(ctx, t, kept) + require.Nil(t, gone, "the retry deletes it") + require.Empty(t, env.fake.mcpServers()) + }) + } +} + +// Plan 4.10 item 2: with only a create marker, the slug read decides. A +// config with this object's nonce is deleted. Someone else's config with the +// slug is never touched. With no config, the controller waits until a late +// create cannot commit any more, then releases. +func TestCoderMCPServerDeletePendingCreate(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + create func(context.Context, *testing.T, mcpServerEnv, *coderv1alpha1.CoderMCPServer) // nil: no config in Coder. + age time.Duration // Age of the create marker. + release bool + deletes int + }{ + {name: "nonce found", create: createWithNonce, release: true, deletes: 1}, + {name: "someone else's config", create: createOther, release: true}, + {name: "no config, recent create", age: time.Second}, + {name: "no config, old create", age: 3 * time.Minute, release: true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + server := env.create(ctx, t, nil) + server, _ = env.reconcile(ctx, t, server) + for _, s := range env.fake.mcpServers() { + require.NoError(t, env.dashboard(t).DeleteMCPServerConfig(ctx, env.orgID, s.view.ID)) + } + if tc.create != nil { + tc.create(ctx, t, env, server) + } + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(server), server)) + server.Status.ConfigID = "" + server.Status.CreateAttemptTime = &metav1.Time{Time: time.Now().Add(-max(tc.age, time.Second))} + require.NoError(t, k8sClient.Status().Update(ctx, server)) + before, deletes := len(env.fake.mcpServers()), env.fake.requestCount(routeDeleteMCPServer) + + kept, result := env.deleteMCPServer(ctx, t, server) + require.Equal(t, tc.deletes, env.fake.requestCount(routeDeleteMCPServer)-deletes) + if tc.release { + require.Nil(t, kept) + require.Len(t, env.fake.mcpServers(), before-tc.deletes) + return + } + require.NotNil(t, kept) + requireMCPServerStep(t, kept, false, "ConfirmingCreate") + require.Positive(t, result.RequeueAfter) + require.LessOrEqual(t, result.RequeueAfter, 2*time.Minute) + }) + } +} + +func createWithNonce(ctx context.Context, t *testing.T, env mcpServerEnv, server *coderv1alpha1.CoderMCPServer) { + t.Helper() + req := apiKeyMCPRequest(server.Spec.Slug) + req.DisplayName = "coder-k8s:" + string(server.UID) + _, err := env.dashboard(t).CreateMCPServerConfig(ctx, env.orgID, req) + require.NoError(t, err) +} + +func createOther(ctx context.Context, t *testing.T, env mcpServerEnv, server *coderv1alpha1.CoderMCPServer) { + t.Helper() + _, err := env.dashboard(t).CreateMCPServerConfig(ctx, env.orgID, apiKeyMCPRequest(server.Spec.Slug)) + require.NoError(t, err) +} + +// Plan 4.10 item 5: while the control plane is being deleted (for example +// with its namespace), the controller makes one bounded DELETE attempt and +// then releases the finalizer, also when that attempt fails. +func TestCoderMCPServerDeleteWhileControlPlaneDeletes(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + fault *fakeFault + }{ + {name: "attempt succeeds"}, + {name: "attempt fails", fault: &fakeFault{Status: http.StatusBadGateway}}, + // The attempt is bounded at 10 s, below the 30 s request timeout. + {name: "attempt hangs", fault: &fakeFault{Hang: true}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + holdControlPlane(ctx, t, env.namespace) + if tc.fault != nil { + env.fake.failNext(routeDeleteMCPServer, *tc.fault) + } + start := time.Now() + gone, _ := env.deleteMCPServer(ctx, t, got) + require.Less(t, time.Since(start), 20*time.Second, "the attempt is bounded") + require.Nil(t, gone, "released after one attempt") + require.Equal(t, 1, env.fake.requestCount(routeDeleteMCPServer)) + if tc.fault != nil && tc.fault.Hang { + return + } + require.Equal(t, tc.fault == nil, len(env.fake.mcpServers()) == 0) + }) + } +} + +// From the #329 final assessment: a released object's entry in the +// reconciler's credential hash map goes, so the map does not grow without +// bound. +func TestCoderMCPServerDeleteForgetsSentCredentials(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + require.True(t, controller.HasSentCredentials(env.r, got.UID)) + gone, _ := env.deleteMCPServer(ctx, t, got) + require.Nil(t, gone) + require.False(t, controller.HasSentCredentials(env.r, got.UID)) +} + +func configIDs(env mcpServerEnv) []uuid.UUID { + var out []uuid.UUID + for _, s := range env.fake.mcpServers() { + out = append(out, s.view.ID) + } + return out +} diff --git a/internal/controller/export_test.go b/internal/controller/export_test.go index 87d91a6eb..c57ffdf75 100644 --- a/internal/controller/export_test.go +++ b/internal/controller/export_test.go @@ -6,6 +6,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/google/uuid" + "k8s.io/apimachinery/pkg/types" coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" ) @@ -32,3 +33,10 @@ func SyncMCPServerACL( } return MCPServerACLResult{Reason: step.reason, Stalled: step.stalled, Message: step.message, Requeue: step.requeue}, err } + +// HasSentCredentials reports whether the reconciler keeps credential hashes +// for the object uid. +func HasSentCredentials(r *CoderMCPServerReconciler, uid types.UID) bool { + _, ok := r.sentCredentials.Load(uid) + return ok +} From 32865530cc0ae213be9460c0ae83274c24a33da1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 15:04:41 +0000 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=A4=96=20feat:=20add=20the=20CoderM?= =?UTF-8?q?CPServer=20E2E=20stub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #155 Change-Id: Ia6b1789a575744cc04e056fd4cfbd20e4ff6b057 Signed-off-by: Thomas Kosiewski --- .github/workflows/ci.yaml | 16 ++ hack/e2e/mcpstub/Dockerfile | 7 + hack/e2e/mcpstub/main.go | 269 ++++++++++++++++++++++++++++++++++ hack/e2e/mcpstub/main_test.go | 222 ++++++++++++++++++++++++++++ 4 files changed, 514 insertions(+) create mode 100644 hack/e2e/mcpstub/Dockerfile create mode 100644 hack/e2e/mcpstub/main.go create mode 100644 hack/e2e/mcpstub/main_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 645ec1401..b427509c5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -102,6 +102,7 @@ jobs: - 'deploy/**' - 'examples/cloudnativepg/**' - 'hack/e2e-workspace-lifecycle*.sh' + - 'hack/e2e/mcpstub/**' - '.github/workflows/ci.yaml' # Coder SDK bumps change how every aggregated-API call reaches the backend. - 'vendor/github.com/coder/coder/v2/**' @@ -379,6 +380,21 @@ jobs: echo "image_id=${image_id}" >> "$GITHUB_OUTPUT" kind load docker-image ghcr.io/coder/coder-k8s:e2e --name e2e + # The CoderMCPServer E2E stub (#155): an MCP server and an OpenAI-compatible LLM, standard library only. + - name: Build and load the MCP stub image + if: env.E2E_FULL == 'true' + env: + GOFLAGS: -mod=vendor + CGO_ENABLED: "0" + GOOS: linux + GOARCH: amd64 + run: | + stub_dir="${RUNNER_TEMP}/mcpstub" + mkdir -p "$stub_dir" + go build -o "${stub_dir}/mcpstub" ./hack/e2e/mcpstub + docker build -f hack/e2e/mcpstub/Dockerfile -t e2e.local/mcpstub:e2e "$stub_dir" + kind load docker-image e2e.local/mcpstub:e2e --name e2e + # Workspace pod image of config/e2e/codertemplate-agent.yaml (imagePullPolicy Never): pulled once by # its linux/amd64 manifest digest, so workspace builds never contact a registry. - name: Load the E2E workspace image diff --git a/hack/e2e/mcpstub/Dockerfile b/hack/e2e/mcpstub/Dockerfile new file mode 100644 index 000000000..917ac9986 --- /dev/null +++ b/hack/e2e/mcpstub/Dockerfile @@ -0,0 +1,7 @@ +# The Kind E2E stub image (#155). CI builds the static binary first and runs +# docker build with the binary's directory as the context. +FROM gcr.io/distroless/static:nonroot + +ENTRYPOINT ["/mcpstub"] +COPY mcpstub /mcpstub +USER nonroot:nonroot diff --git a/hack/e2e/mcpstub/main.go b/hack/e2e/mcpstub/main.go new file mode 100644 index 000000000..3426e24fc --- /dev/null +++ b/hack/e2e/mcpstub/main.go @@ -0,0 +1,269 @@ +// Command mcpstub is the Kind E2E stub of the CoderMCPServer plan (#155, +// section 7.2). It serves a minimal MCP server (streamable HTTP with JSON +// answers) on /mcp, a minimal OpenAI-compatible LLM on /v1, and /report, +// which lists for each MCP request the Host header and SHA-256 hashes of +// two credential headers. It never logs or reports a header value. +// +// It uses only the standard library, so go.mod and vendor/ do not change. +package main + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "log" + "net/http" + "slices" + "sync" + "time" +) + +// Header names whose values the report hashes. The E2E driver writes the +// matching synthetic keys into credential Secrets. +const ( + apiKeyHeader = "X-Api-Key" //nolint:gosec // G101: a header name, not a credential. + e2eHeader = "X-E2E-Header" + // maxBody bounds a request body. Coder sends small JSON-RPC requests. + maxBody = 1 << 20 +) + +// toolNames are the stub's MCP tools. Gamma checks that tool names keep +// their case. +var toolNames = []string{"alpha", "beta", "Gamma"} + +// mcpRecord is one MCP request in the report. Hashes are hex SHA-256 of the +// header value, or "" when the header is absent. +type mcpRecord struct { + Host string `json:"host"` + Method string `json:"method"` + APIKeyHash string `json:"apiKeyHash"` + HeaderHash string `json:"headerHash"` +} + +// llmRecord is one LLM request in the report: the tool names that Coder +// offered the model, sorted. +type llmRecord struct { + Stream bool `json:"stream"` + Tools []string `json:"tools"` +} + +type report struct { + MCP []mcpRecord `json:"mcp"` + LLM []llmRecord `json:"llm"` +} + +// stub holds the report. It is safe for concurrent use. +type stub struct { + mu sync.Mutex + report report + now func() time.Time +} + +func newStub() *stub { + return &stub{report: report{MCP: []mcpRecord{}, LLM: []llmRecord{}}, now: time.Now} +} + +func hashHeader(r *http.Request, name string) string { + v := r.Header.Get(name) + if v == "" { + return "" + } + sum := sha256.Sum256([]byte(v)) + return hex.EncodeToString(sum[:]) +} + +func (s *stub) handler() http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("/mcp", s.mcp) + mux.HandleFunc("POST /v1/chat/completions", s.chat) + mux.HandleFunc("GET /v1/models", s.models) + mux.HandleFunc("GET /report", s.serveReport) + mux.HandleFunc("GET /healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + return mux +} + +type rpcRequest struct { + JSONRPC string `json:"jsonrpc"` + ID json.RawMessage `json:"id,omitempty"` + Method string `json:"method"` + Params json.RawMessage `json:"params,omitempty"` +} + +func (s *stub) mcp(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + // No server-sent event stream: Coder then uses the POST answers. + w.WriteHeader(http.StatusMethodNotAllowed) + return + case http.MethodDelete: + w.WriteHeader(http.StatusNoContent) + return + case http.MethodPost: + default: + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + var req rpcRequest + if err := json.NewDecoder(io.LimitReader(r.Body, maxBody)).Decode(&req); err != nil { + http.Error(w, "invalid JSON-RPC request", http.StatusBadRequest) + return + } + s.mu.Lock() + s.report.MCP = append(s.report.MCP, mcpRecord{ + Host: r.Host, Method: req.Method, APIKeyHash: hashHeader(r, apiKeyHeader), HeaderHash: hashHeader(r, e2eHeader), + }) + s.mu.Unlock() + log.Printf("mcp %s", req.Method) + if len(req.ID) == 0 { + // A notification, such as notifications/initialized. + w.WriteHeader(http.StatusAccepted) + return + } + result, rpcErr := mcpResult(req) + answer := map[string]any{"jsonrpc": "2.0", "id": req.ID} + if rpcErr != nil { + answer["error"] = rpcErr + } else { + answer["result"] = result + } + w.Header().Set("Mcp-Session-Id", "mcpstub-session") + writeJSON(w, answer) +} + +func mcpResult(req rpcRequest) (any, map[string]any) { + switch req.Method { + case "initialize": + var p struct { + ProtocolVersion string `json:"protocolVersion"` + } + _ = json.Unmarshal(req.Params, &p) + if p.ProtocolVersion == "" { + p.ProtocolVersion = "2025-06-18" + } + return map[string]any{ + "protocolVersion": p.ProtocolVersion, + "capabilities": map[string]any{"tools": map[string]any{}}, + "serverInfo": map[string]any{"name": "mcpstub", "version": "1"}, + }, nil + case "tools/list": + tools := make([]map[string]any, 0, len(toolNames)) + for _, name := range toolNames { + tools = append(tools, map[string]any{ + "name": name, "description": "tool " + name, + "inputSchema": map[string]any{"type": "object", "properties": map[string]any{}}, + }) + } + return map[string]any{"tools": tools}, nil + case "tools/call": + var p struct { + Name string `json:"name"` + } + _ = json.Unmarshal(req.Params, &p) + if !slices.Contains(toolNames, p.Name) { + return nil, map[string]any{"code": -32602, "message": "unknown tool"} + } + return map[string]any{"content": []map[string]any{{"type": "text", "text": "called " + p.Name}}}, nil + case "ping": + return map[string]any{}, nil + } + return nil, map[string]any{"code": -32601, "message": "method not found"} +} + +const answerText = "stub answer" + +func (s *stub) chat(w http.ResponseWriter, r *http.Request) { + var req struct { + Stream bool `json:"stream"` + Tools []struct { + Name string `json:"name"` + Function struct { + Name string `json:"name"` + } `json:"function"` + } `json:"tools"` + } + if err := json.NewDecoder(io.LimitReader(r.Body, maxBody)).Decode(&req); err != nil { + http.Error(w, "invalid chat request", http.StatusBadRequest) + return + } + tools := []string{} + for _, t := range req.Tools { + tools = append(tools, cmpOr(t.Function.Name, t.Name)) + } + slices.Sort(tools) + s.mu.Lock() + s.report.LLM = append(s.report.LLM, llmRecord{Stream: req.Stream, Tools: tools}) + s.mu.Unlock() + log.Printf("llm stream=%v tools=%d", req.Stream, len(tools)) + created := s.now().Unix() + usage := map[string]any{"prompt_tokens": 5, "completion_tokens": 2, "total_tokens": 7} + if !req.Stream { + writeJSON(w, map[string]any{ + "id": "c1", "object": "chat.completion", "created": created, "model": "stub-model", + "choices": []map[string]any{{"index": 0, "message": map[string]any{"role": "assistant", "content": answerText}, "finish_reason": "stop"}}, + "usage": usage, + }) + return + } + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + chunks := []map[string]any{ + { + "id": "c1", "object": "chat.completion.chunk", "created": created, "model": "stub-model", + "choices": []map[string]any{{"index": 0, "delta": map[string]any{"role": "assistant", "content": answerText}, "finish_reason": nil}}, + }, + { + "id": "c1", "object": "chat.completion.chunk", "created": created, "model": "stub-model", + "choices": []map[string]any{{"index": 0, "delta": map[string]any{}, "finish_reason": "stop"}}, "usage": usage, + }, + } + flusher, _ := w.(http.Flusher) + for _, c := range chunks { + data, _ := json.Marshal(c) + _, _ = fmt.Fprintf(w, "data: %s\n\n", data) + if flusher != nil { + flusher.Flush() + } + } + _, _ = io.WriteString(w, "data: [DONE]\n\n") +} + +func (s *stub) models(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, map[string]any{"object": "list", "data": []map[string]any{{"id": "stub-model", "object": "model", "owned_by": "mcpstub"}}}) +} + +func (s *stub) serveReport(w http.ResponseWriter, _ *http.Request) { + s.mu.Lock() + defer s.mu.Unlock() + writeJSON(w, s.report) +} + +func writeJSON(w http.ResponseWriter, v any) { + data, err := json.Marshal(v) + if err != nil { + http.Error(w, "encode answer", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(data) +} + +func cmpOr(a, b string) string { + if a != "" { + return a + } + return b +} + +func main() { + addr := flag.String("addr", ":8080", "listen address") + flag.Parse() + srv := &http.Server{Addr: *addr, Handler: newStub().handler(), ReadHeaderTimeout: 10 * time.Second} + log.Printf("mcpstub listening on %s", *addr) + if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatal(err) + } +} diff --git a/hack/e2e/mcpstub/main_test.go b/hack/e2e/mcpstub/main_test.go new file mode 100644 index 000000000..f0534b390 --- /dev/null +++ b/hack/e2e/mcpstub/main_test.go @@ -0,0 +1,222 @@ +package main + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// The synthetic keys never appear in a log line or in the report. +const ( + testAPIKey = "synthetic-api-key-0123456789" + testHeaderVal = "synthetic-header-abcdef" +) + +func sha(v string) string { + sum := sha256.Sum256([]byte(v)) + return hex.EncodeToString(sum[:]) +} + +// captureLog sends the standard logger to a buffer for one test. +func captureLog(t *testing.T) *bytes.Buffer { + t.Helper() + var buf bytes.Buffer + prevOut, prevFlags := log.Writer(), log.Flags() + log.SetOutput(&buf) + t.Cleanup(func() { + log.SetOutput(prevOut) + log.SetFlags(prevFlags) + }) + return &buf +} + +func do(t *testing.T, h http.Handler, method, path, body string, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Host = "mcp-stub.coder.svc:8080" + for k, v := range headers { + req.Header.Set(k, v) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +func rpc(t *testing.T, h http.Handler, body string, headers map[string]string) map[string]any { + t.Helper() + rec := do(t, h, http.MethodPost, "/mcp", body, headers) + if rec.Code != http.StatusOK { + t.Fatalf("POST /mcp %s: status %d, want 200", body, rec.Code) + } + var answer map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &answer); err != nil { + t.Fatalf("decode answer to %s: %v", body, err) + } + if answer["jsonrpc"] != "2.0" { + t.Fatalf("answer to %s: jsonrpc %v, want 2.0", body, answer["jsonrpc"]) + } + return answer +} + +func getReport(t *testing.T, h http.Handler) report { + t.Helper() + rec := do(t, h, http.MethodGet, "/report", "", nil) + if rec.Code != http.StatusOK { + t.Fatalf("GET /report: status %d", rec.Code) + } + var got report + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode report: %v", err) + } + return got +} + +func TestMCPAnswers(t *testing.T) { + h := newStub().handler() + creds := map[string]string{apiKeyHeader: testAPIKey, e2eHeader: testHeaderVal} + + initAnswer := rpc(t, h, `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26"}}`, creds) + result, _ := initAnswer["result"].(map[string]any) + if result["protocolVersion"] != "2025-03-26" { + t.Fatalf("initialize protocolVersion = %v, want the client's version", result["protocolVersion"]) + } + if _, ok := result["capabilities"].(map[string]any)["tools"]; !ok { + t.Fatalf("initialize capabilities lack tools: %v", result) + } + + if rec := do(t, h, http.MethodPost, "/mcp", `{"jsonrpc":"2.0","method":"notifications/initialized"}`, creds); rec.Code != http.StatusAccepted { + t.Fatalf("notification: status %d, want 202", rec.Code) + } + + list := rpc(t, h, `{"jsonrpc":"2.0","id":2,"method":"tools/list"}`, creds) + tools, _ := list["result"].(map[string]any)["tools"].([]any) + var names []string + for _, tool := range tools { + names = append(names, tool.(map[string]any)["name"].(string)) + } + if strings.Join(names, ",") != "alpha,beta,Gamma" { + t.Fatalf("tools/list names = %v, want alpha,beta,Gamma", names) + } + + call := rpc(t, h, `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"Gamma"}}`, creds) + if !strings.Contains(string(mustJSON(t, call["result"])), "called Gamma") { + t.Fatalf("tools/call result = %v", call["result"]) + } + unknown := rpc(t, h, `{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"gamma"}}`, creds) + if unknown["error"] == nil { + t.Fatalf("tools/call of an unknown tool returned no error: %v", unknown) + } + if ping := rpc(t, h, `{"jsonrpc":"2.0","id":5,"method":"ping"}`, creds); ping["result"] == nil { + t.Fatalf("ping returned no result: %v", ping) + } + if missing := rpc(t, h, `{"jsonrpc":"2.0","id":6,"method":"resources/list"}`, creds); missing["error"] == nil { + t.Fatalf("an unknown method returned no error: %v", missing) + } + + if rec := do(t, h, http.MethodGet, "/mcp", "", nil); rec.Code != http.StatusMethodNotAllowed { + t.Fatalf("GET /mcp: status %d, want 405", rec.Code) + } + if rec := do(t, h, http.MethodDelete, "/mcp", "", nil); rec.Code != http.StatusNoContent { + t.Fatalf("DELETE /mcp: status %d, want 204", rec.Code) + } + if rec := do(t, h, http.MethodPost, "/mcp", "not json", nil); rec.Code != http.StatusBadRequest { + t.Fatalf("POST /mcp with bad JSON: status %d, want 400", rec.Code) + } +} + +func mustJSON(t *testing.T, v any) []byte { + t.Helper() + data, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return data +} + +func TestReportHashesHeadersAndLogsNoValue(t *testing.T) { + logs := captureLog(t) + h := newStub().handler() + + rpc(t, h, `{"jsonrpc":"2.0","id":1,"method":"initialize"}`, map[string]string{apiKeyHeader: testAPIKey, e2eHeader: testHeaderVal}) + rpc(t, h, `{"jsonrpc":"2.0","id":2,"method":"tools/list"}`, nil) + + got := getReport(t, h) + want := []mcpRecord{ + {Host: "mcp-stub.coder.svc:8080", Method: "initialize", APIKeyHash: sha(testAPIKey), HeaderHash: sha(testHeaderVal)}, + {Host: "mcp-stub.coder.svc:8080", Method: "tools/list"}, + } + if string(mustJSON(t, got.MCP)) != string(mustJSON(t, want)) { + t.Fatalf("report MCP = %+v, want %+v", got.MCP, want) + } + + raw := do(t, h, http.MethodGet, "/report", "", nil).Body.String() + for _, secret := range []string{testAPIKey, testHeaderVal} { + if strings.Contains(logs.String(), secret) { + t.Fatalf("the log holds a header value: %q", logs.String()) + } + if strings.Contains(raw, secret) { + t.Fatalf("the report holds a header value: %s", raw) + } + } + if !strings.Contains(logs.String(), "mcp initialize") { + t.Fatalf("the log lacks the method line: %q", logs.String()) + } +} + +func TestChatCompletions(t *testing.T) { + h := newStub().handler() + toolsBody := `"tools":[{"type":"function","function":{"name":"srv__beta"}},{"type":"function","function":{"name":"srv__alpha"}}]` + + plain := do(t, h, http.MethodPost, "/v1/chat/completions", `{"model":"stub-model","messages":[],`+toolsBody+`}`, nil) + if plain.Code != http.StatusOK { + t.Fatalf("plain chat: status %d", plain.Code) + } + var completion struct { + Choices []struct { + Message struct { + Content string `json:"content"` + } `json:"message"` + } `json:"choices"` + } + if err := json.Unmarshal(plain.Body.Bytes(), &completion); err != nil || len(completion.Choices) != 1 || completion.Choices[0].Message.Content != answerText { + t.Fatalf("plain chat answer = %s (err %v)", plain.Body.String(), err) + } + + stream := do(t, h, http.MethodPost, "/v1/chat/completions", `{"model":"stub-model","stream":true,"messages":[]}`, nil) + if ct := stream.Header().Get("Content-Type"); ct != "text/event-stream" { + t.Fatalf("stream Content-Type = %q", ct) + } + events := strings.Split(strings.TrimSpace(stream.Body.String()), "\n\n") + if len(events) != 3 || events[2] != "data: [DONE]" { + t.Fatalf("stream events = %q, want two chunks and [DONE]", events) + } + var chunk map[string]any + if err := json.Unmarshal([]byte(strings.TrimPrefix(events[0], "data: ")), &chunk); err != nil { + t.Fatalf("decode the first chunk: %v", err) + } + if !strings.Contains(events[0], answerText) { + t.Fatalf("the first chunk lacks the answer: %s", events[0]) + } + + got := getReport(t, h) + want := []llmRecord{{Stream: false, Tools: []string{"srv__alpha", "srv__beta"}}, {Stream: true, Tools: []string{}}} + if string(mustJSON(t, got.LLM)) != string(mustJSON(t, want)) { + t.Fatalf("report LLM = %+v, want %+v", got.LLM, want) + } + + models := do(t, h, http.MethodGet, "/v1/models", "", nil) + body, _ := io.ReadAll(models.Body) + if models.Code != http.StatusOK || !strings.Contains(string(body), `"stub-model"`) { + t.Fatalf("GET /v1/models: %d %s", models.Code, body) + } + if rec := do(t, h, http.MethodGet, "/healthz", "", nil); rec.Code != http.StatusOK { + t.Fatalf("GET /healthz: status %d", rec.Code) + } +} From cd6d3fba308c54f587b7579363e8dbec7dfc539c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 16:41:23 +0000 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=A4=96=20test:=20pin=20the=20Secret?= =?UTF-8?q?=20RBAC=20rules=20and=20the=20standalone=20API=20server=20RBAC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #155 Change-Id: I87e3d28b11cffb1a553df0ed73072ac3bf8b59a7 Signed-off-by: Thomas Kosiewski --- config/rbac/rbac_test.go | 99 +++++++++++++++++++ .../apiservice-cabundle-binding.yaml | 13 +++ .../auth-delegator-binding.yaml | 12 +++ .../authentication-reader-binding.yaml | 13 +++ .../apiserver-standalone/serviceaccount.yaml | 8 ++ .../apiserver-standalone/serving-ca-role.yaml | 31 ++++++ .../serving-ca-secret.yaml | 13 +++ config/rbac/testdata/role-secrets-rules.yaml | 17 ++++ 8 files changed, 206 insertions(+) create mode 100644 config/rbac/rbac_test.go create mode 100644 config/rbac/testdata/apiserver-standalone/apiservice-cabundle-binding.yaml create mode 100644 config/rbac/testdata/apiserver-standalone/auth-delegator-binding.yaml create mode 100644 config/rbac/testdata/apiserver-standalone/authentication-reader-binding.yaml create mode 100644 config/rbac/testdata/apiserver-standalone/serviceaccount.yaml create mode 100644 config/rbac/testdata/apiserver-standalone/serving-ca-role.yaml create mode 100644 config/rbac/testdata/apiserver-standalone/serving-ca-secret.yaml create mode 100644 config/rbac/testdata/role-secrets-rules.yaml diff --git a/config/rbac/rbac_test.go b/config/rbac/rbac_test.go new file mode 100644 index 000000000..db2c7bc6a --- /dev/null +++ b/config/rbac/rbac_test.go @@ -0,0 +1,99 @@ +// Package rbac_test guards the operator's Secret access (plan #155 G2, S9): +// the rules of config/rbac/role.yaml that can grant access to core Secrets, +// and every file in config/apiserver-standalone/, must equal the golden +// copies in testdata/. The manifests are data files, so this package has no +// production code. +package rbac_test + +import ( + "os" + "path/filepath" + "reflect" + "slices" + "strings" + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/yaml" +) + +const ( + rolePath = "role.yaml" + goldenSecretRules = "testdata/role-secrets-rules.yaml" //nolint:gosec // A file path, not a credential. + standaloneDir = "../apiserver-standalone" + goldenStandalone = "testdata/apiserver-standalone" +) + +func readFile(t *testing.T, path string) []byte { + t.Helper() + data, err := os.ReadFile(path) //nolint:gosec // Tests read fixed files of this repository. + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return data +} + +// secretRules returns the rules that grant something on core Secrets: the +// core group or "*", and the resource secrets, a secrets subresource, or "*". +func secretRules(rules []rbacv1.PolicyRule) []rbacv1.PolicyRule { + var out []rbacv1.PolicyRule + for _, rule := range rules { + if !slices.Contains(rule.APIGroups, "") && !slices.Contains(rule.APIGroups, "*") { + continue + } + if slices.ContainsFunc(rule.Resources, func(r string) bool { + return r == "secrets" || r == "*" || strings.HasPrefix(r, "secrets/") + }) { + out = append(out, rule) + } + } + return out +} + +func TestRoleSecretRulesMatchGolden(t *testing.T) { + t.Parallel() + var role rbacv1.ClusterRole + if err := yaml.UnmarshalStrict(readFile(t, rolePath), &role); err != nil { + t.Fatalf("parse %s: %v", rolePath, err) + } + if role.Kind != "ClusterRole" || role.Name != "manager-role" { + t.Fatalf("assertion failed: %s holds %s %q, want ClusterRole manager-role", rolePath, role.Kind, role.Name) + } + var golden []rbacv1.PolicyRule + if err := yaml.UnmarshalStrict(readFile(t, goldenSecretRules), &golden); err != nil { + t.Fatalf("parse %s: %v", goldenSecretRules, err) + } + if len(golden) == 0 { + t.Fatalf("assertion failed: %s holds no rule", goldenSecretRules) + } + if got := secretRules(role.Rules); !reflect.DeepEqual(got, golden) { + t.Fatalf("the Secret rules of %s changed (plan #155 G2).\ngot: %+v\nwant: %+v", rolePath, got, golden) + } +} + +func TestAPIServerStandaloneMatchesGolden(t *testing.T) { + t.Parallel() + names := func(dir string) []string { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read %s: %v", dir, err) + } + var out []string + for _, e := range entries { + out = append(out, e.Name()) + } + return out + } + got, want := names(standaloneDir), names(goldenStandalone) + if len(want) == 0 { + t.Fatalf("assertion failed: %s holds no file", goldenStandalone) + } + if !slices.Equal(got, want) { + t.Fatalf("the files of %s changed (plan #155 G2): got %v, want %v", standaloneDir, got, want) + } + for _, name := range want { + if string(readFile(t, filepath.Join(standaloneDir, name))) != string(readFile(t, filepath.Join(goldenStandalone, name))) { + t.Errorf("%s/%s differs from its golden copy (plan #155 G2)", standaloneDir, name) + } + } +} diff --git a/config/rbac/testdata/apiserver-standalone/apiservice-cabundle-binding.yaml b/config/rbac/testdata/apiserver-standalone/apiservice-cabundle-binding.yaml new file mode 100644 index 000000000..237686500 --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/apiservice-cabundle-binding.yaml @@ -0,0 +1,13 @@ +# Binds the ClusterRole from config/rbac/apiservice-cabundle-role.yaml; apply that file too. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: coder-k8s-apiserver-apiservice-cabundle +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: coder-k8s-apiservice-cabundle +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/rbac/testdata/apiserver-standalone/auth-delegator-binding.yaml b/config/rbac/testdata/apiserver-standalone/auth-delegator-binding.yaml new file mode 100644 index 000000000..bdf9722cc --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/auth-delegator-binding.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: coder-k8s-apiserver-auth-delegator +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:auth-delegator +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/rbac/testdata/apiserver-standalone/authentication-reader-binding.yaml b/config/rbac/testdata/apiserver-standalone/authentication-reader-binding.yaml new file mode 100644 index 000000000..1aecfbcc9 --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/authentication-reader-binding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: coder-k8s-apiserver-authentication-reader + namespace: kube-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: extension-apiserver-authentication-reader +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/rbac/testdata/apiserver-standalone/serviceaccount.yaml b/config/rbac/testdata/apiserver-standalone/serviceaccount.yaml new file mode 100644 index 000000000..da57aadc6 --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/serviceaccount.yaml @@ -0,0 +1,8 @@ +# Identity for a standalone aggregated API server (--app=aggregated-apiserver). It gets only what +# that mode uses: its own serving-CA Secret, delegated authentication and authorization, and its +# own APIService. Not part of dist/install.yaml, which installs the controller only. +apiVersion: v1 +kind: ServiceAccount +metadata: + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/rbac/testdata/apiserver-standalone/serving-ca-role.yaml b/config/rbac/testdata/apiserver-standalone/serving-ca-role.yaml new file mode 100644 index 000000000..3b61e5012 --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/serving-ca-role.yaml @@ -0,0 +1,31 @@ +# Read, fill and renew the serving-CA Secret, and nothing else. No create: see +# serving-ca-secret.yaml. +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: coder-k8s-apiserver-serving-ca + namespace: coder-system +rules: + - apiGroups: + - "" + resources: + - secrets + resourceNames: + - coder-k8s-apiserver-tls + verbs: + - get + - update +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: coder-k8s-apiserver-serving-ca + namespace: coder-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: coder-k8s-apiserver-serving-ca +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/rbac/testdata/apiserver-standalone/serving-ca-secret.yaml b/config/rbac/testdata/apiserver-standalone/serving-ca-secret.yaml new file mode 100644 index 000000000..b4eb05ec1 --- /dev/null +++ b/config/rbac/testdata/apiserver-standalone/serving-ca-secret.yaml @@ -0,0 +1,13 @@ +# Empty placeholder for the serving CA. The server fills it on first start, so its identity needs +# no create on Secrets: namespace-wide create would let this identity mint tokens for other +# ServiceAccounts in the namespace. Applying this file again keeps the filled data. +apiVersion: v1 +kind: Secret +metadata: + name: coder-k8s-apiserver-tls + namespace: coder-system + labels: + app.kubernetes.io/name: coder-k8s + app.kubernetes.io/component: aggregated-apiserver-serving-ca + app.kubernetes.io/managed-by: coder-k8s +type: coder.com/aggregated-apiserver-serving-ca diff --git a/config/rbac/testdata/role-secrets-rules.yaml b/config/rbac/testdata/role-secrets-rules.yaml new file mode 100644 index 000000000..13ebd14b1 --- /dev/null +++ b/config/rbac/testdata/role-secrets-rules.yaml @@ -0,0 +1,17 @@ +# Golden copy of every rule in config/rbac/role.yaml that can grant access to +# core Secrets (plan #155 G2, S9). rbac_test.go compares the role with it. +# A change here needs a security review of the operator's Secret access. +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + - services + verbs: + - create + - delete + - get + - list + - patch + - update + - watch From 7bf3b1c2bda1b50d75b5595a15e1de001ea6eae4 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 16:41:55 +0000 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=A4=96=20feat:=20add=20the=20opt-in?= =?UTF-8?q?=20CoderMCPServer=20status=20admission=20policy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #155 Change-Id: If4f948b82f1d598d4b49b3f861379b6c87001937 Signed-off-by: Thomas Kosiewski --- config/admission/status-operator-only.yaml | 33 +++++ .../codermcpserver_admission_test.go | 122 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 config/admission/status-operator-only.yaml create mode 100644 internal/controller/codermcpserver_admission_test.go diff --git a/config/admission/status-operator-only.yaml b/config/admission/status-operator-only.yaml new file mode 100644 index 000000000..6f75298ac --- /dev/null +++ b/config/admission/status-operator-only.yaml @@ -0,0 +1,33 @@ +# Opt-in admission policy (plan #155 D11 A): only the operator ServiceAccount +# can write the status of CoderMCPServer objects. status.configID tells the +# controller which Coder config an object owns, so a subject that writes the +# status can point an object at a config that it did not create. +# config/default does not apply this file. Apply it with +# `kubectl apply -f config/admission/`. If the operator runs as another +# ServiceAccount, change the user name in the expression first. +# docs/how-to/manage-mcp-servers.md describes the policy. +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: coder-k8s-status-operator-only +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: ["coder.com"] + apiVersions: ["*"] + # A status PATCH reaches admission as UPDATE. + operations: ["UPDATE"] + resources: ["codermcpservers/status"] + validations: + - expression: "request.userInfo.username == 'system:serviceaccount:coder-system:coder-k8s'" + message: "Only the coder-k8s operator ServiceAccount can write the status of CoderMCPServer objects." + reason: Forbidden +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: coder-k8s-status-operator-only +spec: + policyName: coder-k8s-status-operator-only + validationActions: [Deny] diff --git a/internal/controller/codermcpserver_admission_test.go b/internal/controller/codermcpserver_admission_test.go new file mode 100644 index 000000000..f5abaaad1 --- /dev/null +++ b/internal/controller/codermcpserver_admission_test.go @@ -0,0 +1,122 @@ +package controller_test + +import ( + "context" + "os" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" +) + +const ( + statusPolicyManifest = "../../config/admission/status-operator-only.yaml" + operatorUser = "system:serviceaccount:coder-system:coder-k8s" + statusPolicyDenial = "Only the coder-k8s operator ServiceAccount can write the status of CoderMCPServer objects." +) + +// The opt-in /status policy in config/admission/ (plan S11, D11 A) denies a +// status write by a subject that RBAC allows, and allows the same write by +// the operator ServiceAccount. Both subjects are impersonated, so the +// envtest API server applies RBAC and the policy to them. The policy is +// cluster-wide and denies the status writes of every other test, so this +// test runs serially and waits until the policy is gone. +func TestCoderMCPServerStatusPolicy(t *testing.T) { + ctx := context.Background() + namespace := createTestNamespace(ctx, t, "admission") + server := &coderv1alpha1.CoderMCPServer{ObjectMeta: metav1.ObjectMeta{Name: "github", Namespace: namespace}, Spec: validMCPServerSpec()} + require.NoError(t, k8sClient.Create(ctx, server)) + + // RBAC lets both subjects write the status, so a denial comes from the + // policy. + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Name: "status-writer", Namespace: namespace}, + Rules: []rbacv1.PolicyRule{ + {APIGroups: []string{"coder.com"}, Resources: []string{"codermcpservers"}, Verbs: []string{"get"}}, + {APIGroups: []string{"coder.com"}, Resources: []string{"codermcpservers/status"}, Verbs: []string{"update", "patch"}}, + }, + } + require.NoError(t, k8sClient.Create(ctx, role)) + require.NoError(t, k8sClient.Create(ctx, &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "status-writer", Namespace: namespace}, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: role.Name}, + Subjects: []rbacv1.Subject{ + {APIGroup: rbacv1.GroupName, Kind: rbacv1.UserKind, Name: "alice"}, + {Kind: rbacv1.ServiceAccountKind, Name: "coder-k8s", Namespace: "coder-system"}, + }, + })) + alice := impersonatingClient(t, rest.ImpersonationConfig{UserName: "alice", Groups: []string{"system:authenticated"}}) + operator := impersonatingClient(t, rest.ImpersonationConfig{ + UserName: operatorUser, + Groups: []string{"system:serviceaccounts", "system:serviceaccounts:coder-system", "system:authenticated"}, + }) + writeStatus := func(c client.Client) error { + got := &coderv1alpha1.CoderMCPServer{} + if err := c.Get(ctx, client.ObjectKeyFromObject(server), got); err != nil { + return err + } + base := got.DeepCopy() + got.Status.ObservedGeneration++ + return c.Status().Patch(ctx, got, client.MergeFrom(base)) + } + require.NoError(t, writeStatus(alice), "RBAC allows alice before the policy exists") + + t.Cleanup(func() { + // The API server drops the policy a moment after the delete. Cleanups run last + // first, so this wait runs after the deletes below. + require.Eventually(t, func() bool { return writeStatus(k8sClient) == nil }, 30*time.Second, 100*time.Millisecond, "the policy is gone") + }) + for _, obj := range statusPolicyObjects(t) { + require.NoError(t, k8sClient.Create(ctx, obj)) + t.Cleanup(func() { + require.NoError(t, client.IgnoreNotFound(k8sClient.Delete(context.Background(), obj))) + }) + } + + // The API server loads a new policy a moment after its create. + var denial error + require.Eventually(t, func() bool { + denial = writeStatus(alice) + return denial != nil && strings.Contains(denial.Error(), statusPolicyDenial) + }, 30*time.Second, 100*time.Millisecond, "the policy denies alice") + require.True(t, apierrors.IsForbidden(denial), "the denial is 403 Forbidden: %v", denial) + require.NoError(t, writeStatus(operator), "the operator ServiceAccount writes the status") + err := writeStatus(k8sClient) + require.ErrorContains(t, err, statusPolicyDenial, "a cluster admin is denied too") +} + +func impersonatingClient(t *testing.T, as rest.ImpersonationConfig) client.Client { + t.Helper() + impersonated := rest.CopyConfig(cfg) + impersonated.Impersonate = as + c, err := client.New(impersonated, client.Options{Scheme: scheme}) + require.NoError(t, err) + return c +} + +// statusPolicyObjects reads the shipped policy and binding unchanged. +func statusPolicyObjects(t *testing.T) []*unstructured.Unstructured { + t.Helper() + data, err := os.ReadFile(statusPolicyManifest) + require.NoError(t, err) + var objects []*unstructured.Unstructured + for _, doc := range strings.Split(string(data), "\n---\n") { + obj := map[string]any{} + require.NoError(t, yaml.Unmarshal([]byte(doc), &obj)) + objects = append(objects, &unstructured.Unstructured{Object: obj}) + } + require.Len(t, objects, 2, "assertion failed: %s holds a policy and its binding", statusPolicyManifest) + require.Equal(t, "ValidatingAdmissionPolicy", objects[0].GetKind()) + require.Equal(t, "ValidatingAdmissionPolicyBinding", objects[1].GetKind()) + return objects +} From 091c7fcf9a0f8f75867545a8c2a53558b2715417 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 16:43:14 +0000 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=A4=96=20feat:=20activate=20the=20C?= =?UTF-8?q?oderMCPServer=20controller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #155 Change-Id: I4559e944c3ea6cf0e0d1506234ae964bd089cac8 Signed-off-by: Thomas Kosiewski --- .cspell.json | 2 + .github/workflows/ci.yaml | 19 +- api/v1alpha1/codercontrolplane_types.go | 3 +- api/v1alpha1/codermcpserver_types.go | 12 +- .../bases/coder.com_codercontrolplanes.yaml | 9 +- .../coder.com_codermcpservers.yaml | 4 +- config/default/kustomization.yaml | 1 + .../gitops/argocd-health-codermcpserver.lua | 28 + .../flux-healthcheck-codermcpserver.yaml | 9 + config/gitops/health_test.go | 154 +++-- config/rbac/role.yaml | 9 +- .../coder_v1alpha1_codermcpserver.yaml | 46 ++ dist/install.yaml | 575 +++++++++++++++++- docs/explanation/architecture.md | 5 +- docs/how-to/deploy-controller.md | 2 +- docs/how-to/manage-mcp-servers.md | 346 +++++++++++ docs/index.md | 2 +- docs/reference/api/codercontrolplane.md | 6 +- docs/reference/api/codermcpserver.md | 136 +++++ docs/tutorials/getting-started.md | 5 +- hack/update-manifests.sh | 2 +- internal/app/controllerapp/controllerapp.go | 8 + .../controller/codermcpserver_controller.go | 18 +- .../codermcpserver_controller_test.go | 13 + .../controller/controllerapp_setup_test.go | 27 +- mkdocs.yml | 2 + 26 files changed, 1331 insertions(+), 112 deletions(-) rename config/crd/{dormant => bases}/coder.com_codermcpservers.yaml (99%) create mode 100644 config/gitops/argocd-health-codermcpserver.lua create mode 100644 config/gitops/flux-healthcheck-codermcpserver.yaml create mode 100644 config/samples/coder_v1alpha1_codermcpserver.yaml create mode 100644 docs/how-to/manage-mcp-servers.md create mode 100644 docs/reference/api/codermcpserver.md diff --git a/.cspell.json b/.cspell.json index df441da00..0d0ebbb3c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -51,6 +51,8 @@ "stakater", "coderaigatewaykey", "coderaigatewaykeys", + "codermcpserver", + "codermcpservers", "coderaiprovider", "coderaiproviders", "codertemplatetest", diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1607d5d32..f68573115 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -87,6 +87,9 @@ jobs: - 'internal/controller/codertemplatetest*' - 'internal/controller/coderaiprovider*' - 'internal/controller/coderaigatewaykey*' + - 'internal/controller/codermcpserver*' + # The credential Secret contract that CoderAIProvider and CoderMCPServer read through. + - 'internal/credentialsecret/**' - 'internal/controller/owned_secret.go' - 'internal/controller/reserved_secret_names.go' - 'internal/controller/credential_checks.go' @@ -172,22 +175,6 @@ jobs: exit 1 fi - - name: Verify the dormant CoderMCPServer CRD is not installed - # Remove this step when the CoderMCPServer controller ships (#155). - run: | - # grep exits 1 when nothing matches. Any other status (for example a missing file) fails the step. - status=0 - # The third pattern matches the CRD path entry in config/default/kustomization.yaml. - grep -n -E '^[[:space:]]+kind: CoderMCPServer$|codermcpservers\.coder\.com|coder\.com_codermcpservers\.yaml' \ - dist/install.yaml config/default/kustomization.yaml config/crd/bases/*.yaml || status=$? - if [[ "${status}" -eq 0 ]]; then - echo "assertion failed: the CoderMCPServer CRD must not ship before its controller" >&2 - exit 1 - elif [[ "${status}" -ne 1 ]]; then - echo "assertion failed: the CoderMCPServer CRD guard could not read the install manifests (grep exit ${status})" >&2 - exit 1 - fi - - name: Verify the dormant CoderWorkspacePool CRD is not installed # Remove this step when the CoderWorkspacePool controller ships (#153). run: | diff --git a/api/v1alpha1/codercontrolplane_types.go b/api/v1alpha1/codercontrolplane_types.go index 590be4142..3514d490a 100644 --- a/api/v1alpha1/codercontrolplane_types.go +++ b/api/v1alpha1/codercontrolplane_types.go @@ -127,8 +127,7 @@ type CoderControlPlaneSpec struct { // MCPServers configures CoderMCPServer objects that use this control // plane. Removing it disables in Coder each server config that a - // CoderMCPServer created through this control plane. It has no effect - // yet: the CoderMCPServer controller is not shipped. + // CoderMCPServer created through this control plane. // +optional MCPServers *MCPServersSpec `json:"mcpServers,omitempty"` diff --git a/api/v1alpha1/codermcpserver_types.go b/api/v1alpha1/codermcpserver_types.go index 5a4d34640..1a05fe4ee 100644 --- a/api/v1alpha1/codermcpserver_types.go +++ b/api/v1alpha1/codermcpserver_types.go @@ -34,9 +34,6 @@ type MCPServersSpec struct { // enabled: false, and then changes nothing else. After the organization // is listed again, a spec edit enables the server. Deleting a // CoderMCPServer still deletes the config it created. - // - // This field has no effect yet: the CoderMCPServer controller is not - // shipped. // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=64 // +kubebuilder:validation:items:MinLength=1 @@ -50,9 +47,6 @@ type MCPServersSpec struct { // without that user's choice. When false (the default), such objects // create and change nothing, and a server config that one of them // created is disabled in Coder, as for a removed organization. - // - // This field has no effect yet: the CoderMCPServer controller is not - // shipped. // +optional AllowForceOn bool `json:"allowForceOn,omitempty"` } @@ -76,7 +70,7 @@ type CoderMCPServerSpec struct { // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="organization is immutable" Organization string `json:"organization"` // Slug is the Coder slug of the config, unique in its organization. The - // model sees the server's tools as __. + // model sees the server's tools as `__`. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=32 // +kubebuilder:validation:Pattern=`^[a-z0-9]+(-[a-z0-9]+)*$` @@ -175,7 +169,7 @@ type CoderMCPServerAuth struct { } // CoderMCPServerAPIKeyAuth sends one Secret value in one header. Coder sends -// the value as it is, so a bearer token is stored as "Bearer ". +// the value as it is, so a bearer token is stored as `Bearer `. type CoderMCPServerAPIKeyAuth struct { // Header is the HTTP header name. // +kubebuilder:validation:MinLength=1 @@ -277,7 +271,7 @@ type CoderMCPServerAccess struct { // from it. type CoderMCPServerCredentialStatus struct { // Field names the credential: apiKey, oauth2ClientSecret, or - // header:. + // `header:`. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=160 Field string `json:"field"` diff --git a/config/crd/bases/coder.com_codercontrolplanes.yaml b/config/crd/bases/coder.com_codercontrolplanes.yaml index 3aa6f301e..dd6e9a4ab 100644 --- a/config/crd/bases/coder.com_codercontrolplanes.yaml +++ b/config/crd/bases/coder.com_codercontrolplanes.yaml @@ -1400,8 +1400,7 @@ spec: description: |- MCPServers configures CoderMCPServer objects that use this control plane. Removing it disables in Coder each server config that a - CoderMCPServer created through this control plane. It has no effect - yet: the CoderMCPServer controller is not shipped. + CoderMCPServer created through this control plane. properties: allowForceOn: description: |- @@ -1410,9 +1409,6 @@ spec: without that user's choice. When false (the default), such objects create and change nothing, and a server config that one of them created is disabled in Coder, as for a removed organization. - - This field has no effect yet: the CoderMCPServer controller is not - shipped. type: boolean organizations: description: |- @@ -1427,9 +1423,6 @@ spec: enabled: false, and then changes nothing else. After the organization is listed again, a spec edit enables the server. Deleting a CoderMCPServer still deletes the config it created. - - This field has no effect yet: the CoderMCPServer controller is not - shipped. items: maxLength: 32 minLength: 1 diff --git a/config/crd/dormant/coder.com_codermcpservers.yaml b/config/crd/bases/coder.com_codermcpservers.yaml similarity index 99% rename from config/crd/dormant/coder.com_codermcpservers.yaml rename to config/crd/bases/coder.com_codermcpservers.yaml index d653518bd..f2af937c6 100644 --- a/config/crd/dormant/coder.com_codermcpservers.yaml +++ b/config/crd/bases/coder.com_codermcpservers.yaml @@ -382,7 +382,7 @@ spec: slug: description: |- Slug is the Coder slug of the config, unique in its organization. The - model sees the server's tools as __. + model sees the server's tools as `__`. maxLength: 32 minLength: 1 pattern: ^[a-z0-9]+(-[a-z0-9]+)*$ @@ -513,7 +513,7 @@ spec: field: description: |- Field names the credential: apiKey, oauth2ClientSecret, or - header:. + `header:`. maxLength: 160 minLength: 1 type: string diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index d3f9b0e3a..26972f975 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -6,6 +6,7 @@ resources: - ../crd/bases/coder.com_coderaigatewaykeys.yaml - ../crd/bases/coder.com_coderaiproviders.yaml - ../crd/bases/coder.com_codercontrolplanes.yaml + - ../crd/bases/coder.com_codermcpservers.yaml - ../crd/bases/coder.com_coderprovisioners.yaml - ../crd/bases/coder.com_codertemplatetests.yaml - ../crd/bases/coder.com_coderworkspaceproxies.yaml diff --git a/config/gitops/argocd-health-codermcpserver.lua b/config/gitops/argocd-health-codermcpserver.lua new file mode 100644 index 000000000..7e0ac9e87 --- /dev/null +++ b/config/gitops/argocd-health-codermcpserver.lua @@ -0,0 +1,28 @@ +-- Argo CD health check for CoderMCPServer (coder.com/v1alpha1). +-- Install it as the argocd-cm key +-- resource.customizations.health.coder.com_CoderMCPServer. +-- docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. +local hs = { status = "Progressing", message = "Waiting for the controller" } +if obj.metadata.deletionTimestamp ~= nil then + -- Argo CD shows deletionMessage, not message, for a resource being deleted. + hs.deletionMessage = "Deleting the Coder MCP server config" + hs.message = hs.deletionMessage + return hs +end +local st = obj.status +if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then + return hs +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then + hs.message = c.reason .. ": " .. c.message + end +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.status == "True" then + hs.status = "Healthy" + elseif c.type == "Stalled" and c.status == "True" then + hs.status = "Degraded" + end +end +return hs diff --git a/config/gitops/flux-healthcheck-codermcpserver.yaml b/config/gitops/flux-healthcheck-codermcpserver.yaml new file mode 100644 index 000000000..ac7696642 --- /dev/null +++ b/config/gitops/flux-healthcheck-codermcpserver.yaml @@ -0,0 +1,9 @@ +# Flux health check expressions for CoderMCPServer (coder.com/v1alpha1). +# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization +# with spec.wait: true. +# docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. +- apiVersion: coder.com/v1alpha1 + kind: CoderMCPServer + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" + current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" diff --git a/config/gitops/health_test.go b/config/gitops/health_test.go index a2314c708..ca7cc4ba9 100644 --- a/config/gitops/health_test.go +++ b/config/gitops/health_test.go @@ -1,5 +1,6 @@ -// Package gitops_test checks the GitOps health rules for CoderTemplateTest. -// The rules are data files, so this package has no production code. +// Package gitops_test checks the GitOps health rules for CoderTemplateTest +// and CoderMCPServer. The rules are data files, so this package has no +// production code. package gitops_test import ( @@ -15,11 +16,20 @@ import ( "sigs.k8s.io/yaml" ) -const ( - luaPath = "argocd-health-codertemplatetest.lua" - fluxPath = "flux-healthcheck-codertemplatetest.yaml" - docPath = "../../docs/how-to/gitops.md" -) +// ruleSet is the Argo CD script and the Flux entry of one kind, the doc page +// that copies them, and the cases that both must pass. +type ruleSet struct { + kind string + luaPath string + fluxPath string + docPath string + cases func() []healthCase +} + +var ruleSets = []ruleSet{ + {"CoderTemplateTest", "argocd-health-codertemplatetest.lua", "flux-healthcheck-codertemplatetest.yaml", "../../docs/how-to/gitops.md", healthCases}, + {"CoderMCPServer", "argocd-health-codermcpserver.lua", "flux-healthcheck-codermcpserver.yaml", "../../docs/how-to/manage-mcp-servers.md", mcpServerCases}, +} // fluxHealthCheck is one entry of a Flux Kustomization spec.healthCheckExprs. type fluxHealthCheck struct { @@ -103,48 +113,118 @@ func healthCases() []healthCase { } } +// mcpServerObject returns a CoderMCPServer at generation 2 with the given +// status. +func mcpServerObject(status map[string]any) map[string]any { + obj := map[string]any{ + "apiVersion": "coder.com/v1alpha1", + "kind": "CoderMCPServer", + "metadata": map[string]any{"name": "github", "namespace": "coder", "generation": int64(2)}, + "spec": map[string]any{"organization": "coder", "slug": "github"}, + } + if status != nil { + obj["status"] = status + } + return obj +} + +// mcpServerStatus returns the three conditions that the controller always +// writes together, with one reason and message: ready, reconciling (neither +// ready nor stalled), or stalled. +func mcpServerStatus(state, reason string) map[string]any { + ready, reconciling, stalled := "False", "True", "False" + switch state { + case "ready": + ready, reconciling = "True", "False" + case "stalled": + reconciling, stalled = "False", "True" + case "reconciling": + default: + panic("assertion failed: unknown MCP server fixture state " + state) + } + condition := func(conditionType, status string) any { + return map[string]any{"type": conditionType, "status": status, "reason": reason, "message": "details", "observedGeneration": int64(2)} + } + return map[string]any{ + "observedGeneration": int64(2), + "conditions": []any{condition("Ready", ready), condition("Reconciling", reconciling), condition("Stalled", stalled)}, + } +} + +func mcpServerCases() []healthCase { + stale := mcpServerStatus("ready", "Applied") + stale["observedGeneration"] = int64(1) + noConditions := map[string]any{"observedGeneration": int64(2)} + deleting := mcpServerObject(mcpServerStatus("ready", "Applied")) + deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" + deletingWait := mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")) + deletingWait["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" + return []healthCase{ + {"no status", mcpServerObject(nil), "Progressing", "Waiting for the controller", "Unknown", ""}, + {"stale observedGeneration", mcpServerObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, + {"no conditions", mcpServerObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, + {"applied", mcpServerObject(mcpServerStatus("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, + {"control plane not ready", mcpServerObject(mcpServerStatus("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, + {"Coder unavailable", mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")), "Progressing", "CoderUnavailable: details", "InProgress", ""}, + {"group not found", mcpServerObject(mcpServerStatus("stalled", "GroupNotFound")), "Degraded", "GroupNotFound: details", "Failed", ""}, + {"not allowed", mcpServerObject(mcpServerStatus("stalled", "MCPServersNotAllowed")), "Degraded", "MCPServersNotAllowed: details", "Failed", ""}, + {"disabled externally", mcpServerObject(mcpServerStatus("stalled", "DisabledExternally")), "Degraded", "DisabledExternally: details", "Failed", ""}, + {"credential not opted in", mcpServerObject(mcpServerStatus("stalled", "CredentialNotOptedIn")), "Degraded", "CredentialNotOptedIn: details", "Failed", ""}, + {"slug conflict", mcpServerObject(mcpServerStatus("stalled", "SlugConflict")), "Degraded", "SlugConflict: details", "Failed", ""}, + // Flux has no deletion rule: it waits for pruned objects separately. + {"being deleted", deleting, "Progressing", "Deleting the Coder MCP server config", "Current", "Deleting the Coder MCP server config"}, + {"being deleted while Coder is down", deletingWait, "Progressing", "Deleting the Coder MCP server config", "InProgress", "Deleting the Coder MCP server config"}, + } +} + func TestArgoCDHealth(t *testing.T) { t.Parallel() - script := readFile(t, luaPath) - for _, tc := range healthCases() { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - gotStatus, gotMessage, gotDeletion := argoHealth(t, script, tc.obj) - if gotStatus != tc.argoStatus || gotMessage != tc.argoMessage || gotDeletion != tc.argoDeletion { - t.Fatalf("got %q %q %q, want %q %q %q", gotStatus, gotMessage, gotDeletion, tc.argoStatus, tc.argoMessage, tc.argoDeletion) - } - }) + for _, rules := range ruleSets { + script := readFile(t, rules.luaPath) + for _, tc := range rules.cases() { + t.Run(rules.kind+"/"+tc.name, func(t *testing.T) { + t.Parallel() + gotStatus, gotMessage, gotDeletion := argoHealth(t, script, tc.obj) + if gotStatus != tc.argoStatus || gotMessage != tc.argoMessage || gotDeletion != tc.argoDeletion { + t.Fatalf("got %q %q %q, want %q %q %q", gotStatus, gotMessage, gotDeletion, tc.argoStatus, tc.argoMessage, tc.argoDeletion) + } + }) + } } } func TestFluxHealth(t *testing.T) { t.Parallel() - var checks []fluxHealthCheck - if err := yaml.UnmarshalStrict([]byte(readFile(t, fluxPath)), &checks); err != nil { - t.Fatalf("parse %s: %v", fluxPath, err) - } - if len(checks) != 1 || checks[0].APIVersion != "coder.com/v1alpha1" || checks[0].Kind != "CoderTemplateTest" { - t.Fatalf("want one CoderTemplateTest entry, got %+v", checks) - } - for _, tc := range healthCases() { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if got := fluxHealth(t, checks[0], tc.obj); got != tc.fluxStatus { - t.Fatalf("got %q, want %q", got, tc.fluxStatus) - } - }) + for _, rules := range ruleSets { + var checks []fluxHealthCheck + if err := yaml.UnmarshalStrict([]byte(readFile(t, rules.fluxPath)), &checks); err != nil { + t.Fatalf("parse %s: %v", rules.fluxPath, err) + } + if len(checks) != 1 || checks[0].APIVersion != "coder.com/v1alpha1" || checks[0].Kind != rules.kind { + t.Fatalf("want one %s entry, got %+v", rules.kind, checks) + } + for _, tc := range rules.cases() { + t.Run(rules.kind+"/"+tc.name, func(t *testing.T) { + t.Parallel() + if got := fluxHealth(t, checks[0], tc.obj); got != tc.fluxStatus { + t.Fatalf("got %q, want %q", got, tc.fluxStatus) + } + }) + } } } -// TestDocCopiesRules keeps the copies in docs/how-to/gitops.md equal to the -// source files, because the docs build has no snippet include. +// TestDocCopiesRules keeps the copies in the docs equal to the source files, +// because the docs build has no snippet include. func TestDocCopiesRules(t *testing.T) { t.Parallel() - doc := readFile(t, docPath) - for lang, path := range map[string]string{"lua": luaPath, "yaml": fluxPath} { - block := "```" + lang + "\n" + readFile(t, path) + "```\n" - if !strings.Contains(doc, block) { - t.Errorf("%s has no ```%s block equal to %s", docPath, lang, path) + for _, rules := range ruleSets { + doc := readFile(t, rules.docPath) + for lang, path := range map[string]string{"lua": rules.luaPath, "yaml": rules.fluxPath} { + block := "```" + lang + "\n" + readFile(t, path) + "```\n" + if !strings.Contains(doc, block) { + t.Errorf("%s has no ```%s block equal to %s", rules.docPath, lang, path) + } } } } diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 561f059eb..c6aa1f0e9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -86,6 +86,7 @@ rules: resources: - coderaigatewaykeys - coderaiproviders + - codermcpservers verbs: - get - list @@ -97,6 +98,7 @@ rules: - coderaigatewaykeys/finalizers - coderaiproviders/finalizers - codercontrolplanes/finalizers + - codermcpservers/finalizers - coderprovisioners/finalizers - codertemplatetests/finalizers - coderworkspaceproxies/finalizers @@ -107,6 +109,7 @@ rules: resources: - coderaigatewaykeys/status - coderaiproviders/status + - codermcpservers/status verbs: - get - update @@ -135,12 +138,6 @@ rules: - get - patch - update -- apiGroups: - - coder.com - resources: - - codermcpservers - verbs: - - list - apiGroups: - coder.com resources: diff --git a/config/samples/coder_v1alpha1_codermcpserver.yaml b/config/samples/coder_v1alpha1_codermcpserver.yaml new file mode 100644 index 000000000..545bd1817 --- /dev/null +++ b/config/samples/coder_v1alpha1_codermcpserver.yaml @@ -0,0 +1,46 @@ +# Manages one MCP server config in a Coder organization. See +# docs/how-to/manage-mcp-servers.md. +# +# Prerequisites: +# - CoderControlPlane codercontrolplane-sample with status.operatorAccessReady +# true and spec.mcpServers.organizations listing coder. +# - Replace the placeholder key below. Never commit a real key. +apiVersion: v1 +kind: Secret +metadata: + name: example-mcp-key + namespace: coder + labels: + coder.com/credential: "true" + annotations: + # The origin of spec.url. The operator sends the key to no other origin. + coder.com/credential-destinations: https://mcp.example.com +type: Opaque +stringData: + api-key: replace-with-the-mcp-server-key +--- +apiVersion: coder.com/v1alpha1 +kind: CoderMCPServer +metadata: + name: example + namespace: coder +spec: + controlPlaneRef: + name: codercontrolplane-sample + # Immutable: Coder cannot move a config to another organization. + organization: coder + # Immutable: the model sees the tools as example__. + slug: example + displayName: Example + url: https://mcp.example.com/mcp + auth: + type: api_key + apiKey: + header: X-Api-Key + secretRef: + name: example-mcp-key + key: api-key + # Exactly these groups can read the server. Every sync removes other + # entries, also user grants added in the Coder dashboard. + access: + groups: [Everyone] diff --git a/dist/install.yaml b/dist/install.yaml index 23b1e3b72..6cfcce5fb 100644 --- a/dist/install.yaml +++ b/dist/install.yaml @@ -2062,8 +2062,7 @@ spec: description: |- MCPServers configures CoderMCPServer objects that use this control plane. Removing it disables in Coder each server config that a - CoderMCPServer created through this control plane. It has no effect - yet: the CoderMCPServer controller is not shipped. + CoderMCPServer created through this control plane. properties: allowForceOn: description: |- @@ -2072,9 +2071,6 @@ spec: without that user's choice. When false (the default), such objects create and change nothing, and a server config that one of them created is disabled in Coder, as for a removed organization. - - This field has no effect yet: the CoderMCPServer controller is not - shipped. type: boolean organizations: description: |- @@ -2089,9 +2085,6 @@ spec: enabled: false, and then changes nothing else. After the organization is listed again, a spec edit enables the server. Deleting a CoderMCPServer still deletes the config it created. - - This field has no effect yet: the CoderMCPServer controller is not - shipped. items: maxLength: 32 minLength: 1 @@ -5199,6 +5192,563 @@ spec: --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.0 + name: codermcpservers.coder.com +spec: + group: coder.com + names: + kind: CoderMCPServer + listKind: CoderMCPServerList + plural: codermcpservers + singular: codermcpserver + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.organization + name: Organization + type: string + - jsonPath: .spec.slug + name: Slug + type: string + - jsonPath: .spec.auth.type + name: Auth + type: string + - jsonPath: .spec.enabled + name: Enabled + type: boolean + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: CoderMCPServer manages one MCP server config in a Coder organization. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + CoderMCPServerSpec defines one MCP server config in a Coder organization. + Coder trims every text field with strings.TrimSpace, so the patterns reject + leading and trailing Unicode white space. The class [\s\v\x{85}\p{Z}] is + exactly the set of runes that unicode.IsSpace reports. + properties: + access: + description: |- + Access, when set, makes the controller own the server's access list in + Coder: exactly the listed groups can read the server. Every sync + therefore also removes each user entry, for example a user that an admin + added in the Coder dashboard. Coder does not show the entry of a deleted + group, so the sync cannot remove it, but it grants nothing. + When omitted, the controller leaves the access list alone, and Coder's + default lets every organization member read the server. + properties: + groups: + description: |- + Groups are Coder group names in the server's organization. Everyone is + the group of all organization members. An empty list leaves the server + to organization admins. Coder reserves the names new and create. While a + listed group does not exist, the controller removes the access of every + group that this list does not name, adds none, disables the server if + the spec disables it, and waits with GroupNotFound for the rest. + items: + maxLength: 255 + minLength: 1 + pattern: ^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$ + type: string + maxItems: 64 + type: array + x-kubernetes-list-type: set + x-kubernetes-validations: + - message: access.groups must not hold new or create + rule: self.all(g, g != 'new' && g != 'create') + type: object + allowInPlanMode: + description: AllowInPlanMode offers the server in plan-mode chats. + type: boolean + auth: + description: Auth is how Coder authenticates to the server. + properties: + apiKey: + description: APIKey sends one Secret value in one header. + properties: + header: + description: Header is the HTTP header name. + maxLength: 128 + minLength: 1 + pattern: ^[!#$%&'*+.^_\x60|~0-9A-Za-z-]+$ + type: string + x-kubernetes-validations: + - message: header is reserved for the HTTP transport, MCP, + or Coder + rule: '!(self.lowerAscii() in [''host'', ''content-length'', + ''content-type'', ''content-encoding'', ''transfer-encoding'', + ''connection'', ''keep-alive'', ''upgrade'', ''te'', ''trailer'', + ''accept'', ''accept-encoding'', ''proxy-authorization'', + ''proxy-connection'', ''mcp-session-id'', ''mcp-protocol-version'', + ''last-event-id'']) && !self.lowerAscii().startsWith(''x-coder-'')' + secretRef: + description: |- + SecretRef references the key in a Secret in the same namespace that + holds the header value. The Secret must opt in to the credential Secret + contract and list the origin of spec.url. + properties: + key: + description: Key is the key inside the Secret data map. + type: string + name: + description: Name is the Kubernetes Secret name. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: secretRef.name must not be empty + rule: size(self.name) > 0 + - message: secretRef.key must not be empty + rule: has(self.key) && size(self.key) > 0 + required: + - header + - secretRef + type: object + customHeaders: + description: CustomHeaders sends each Secret value in its own + header. + items: + description: CoderMCPServerHeader is one header whose value + comes from a Secret. + properties: + name: + description: Name is the HTTP header name. + maxLength: 128 + minLength: 1 + pattern: ^[!#$%&'*+.^_\x60|~0-9A-Za-z-]+$ + type: string + x-kubernetes-validations: + - message: header is reserved for the HTTP transport, MCP, + or Coder + rule: '!(self.lowerAscii() in [''host'', ''content-length'', + ''content-type'', ''content-encoding'', ''transfer-encoding'', + ''connection'', ''keep-alive'', ''upgrade'', ''te'', + ''trailer'', ''accept'', ''accept-encoding'', ''proxy-authorization'', + ''proxy-connection'', ''mcp-session-id'', ''mcp-protocol-version'', + ''last-event-id'']) && !self.lowerAscii().startsWith(''x-coder-'')' + secretRef: + description: |- + SecretRef references the key in a Secret in the same namespace that + holds the header value. The Secret must opt in to the credential Secret + contract and list the origin of spec.url. + properties: + key: + description: Key is the key inside the Secret data map. + type: string + name: + description: Name is the Kubernetes Secret name. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: secretRef.name must not be empty + rule: size(self.name) > 0 + - message: secretRef.key must not be empty + rule: has(self.key) && size(self.key) > 0 + required: + - name + - secretRef + type: object + maxItems: 16 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: auth.customHeaders names must be unique, ignoring case + rule: self.all(a, self.filter(b, b.name.lowerAscii() == a.name.lowerAscii()).size() + == 1) + oauth2: + description: |- + OAuth2 lets each user connect with an OAuth2 client that the admin + registered at the authorization server. + properties: + authURL: + description: AuthURL is the authorization endpoint that users' + browsers open. + maxLength: 2048 + minLength: 1 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + x-kubernetes-validations: + - message: authURL must be a valid URL with a host + rule: isURL(self) && url(self).getHostname() != '' + - message: authURL must be an http:// or https:// URL with + a host and no user info + rule: self.matches('^https?://[^/?#@\\s]+([/?#].*)?$') + clientID: + description: ClientID is the OAuth2 client ID. + maxLength: 256 + minLength: 1 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + clientSecretRef: + description: |- + ClientSecretRef references the key in a Secret in the same namespace + that holds the client secret. The Secret must opt in to the credential + Secret contract and list the origins of spec.url, authURL, tokenURL and + revocationURL. + properties: + key: + description: Key is the key inside the Secret data map. + type: string + name: + description: Name is the Kubernetes Secret name. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: clientSecretRef.name must not be empty + rule: size(self.name) > 0 + - message: clientSecretRef.key must not be empty + rule: has(self.key) && size(self.key) > 0 + revocationURL: + description: |- + RevocationURL is the https:// token revocation endpoint. Coder sends the + client secret there when a user disconnects. + maxLength: 2048 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + x-kubernetes-validations: + - message: revocationURL must be a valid URL with a host + rule: isURL(self) && url(self).getHostname() != '' + - message: revocationURL must be an https:// URL with a host + and no user info + rule: self.matches('^https://[^/?#@\\s]+([/?#].*)?$') + scopes: + description: Scopes are the OAuth2 scopes that Coder requests. + items: + maxLength: 256 + minLength: 1 + pattern: ^[!#-\[\]-~]+$ + type: string + maxItems: 32 + type: array + x-kubernetes-list-type: set + tokenURL: + description: TokenURL is the token endpoint that Coder sends + the client secret to. + maxLength: 2048 + minLength: 1 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + x-kubernetes-validations: + - message: tokenURL must be a valid URL with a host + rule: isURL(self) && url(self).getHostname() != '' + - message: tokenURL must be an http:// or https:// URL with + a host and no user info + rule: self.matches('^https?://[^/?#@\\s]+([/?#].*)?$') + required: + - authURL + - clientID + - clientSecretRef + - tokenURL + type: object + type: + description: |- + Type is the auth type. Coder's user_oidc type is not supported, because + it sends each user's login token to the server URL. + enum: + - none + - api_key + - custom_headers + - oauth2 + type: string + required: + - type + type: object + x-kubernetes-validations: + - message: auth.apiKey must be set exactly when auth.type is api_key + rule: (self.type == 'api_key') == has(self.apiKey) + - message: auth.customHeaders must be set exactly when auth.type is + custom_headers + rule: (self.type == 'custom_headers') == has(self.customHeaders) + - message: auth.oauth2 must be set exactly when auth.type is oauth2 + rule: (self.type == 'oauth2') == has(self.oauth2) + availability: + default: default_off + description: |- + Availability decides how chats offer the server. force_on adds it to + every new chat of a user who can read it. Coder's web UI preselects + default_on servers. The Coder server treats default_on and default_off + alike. + enum: + - force_on + - default_on + - default_off + type: string + controlPlaneRef: + description: |- + ControlPlaneRef names the CoderControlPlane in the same namespace. The + controller calls Coder with that control plane's operator token. + properties: + name: + description: Name is the CoderControlPlane name. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: controlPlaneRef is immutable + rule: self == oldSelf + description: + description: Description is a short summary of what the server provides. + maxLength: 512 + pattern: ^[^\s\v\x{85}\p{Z}]([\s\S]*[^\s\v\x{85}\p{Z}])?$ + type: string + displayName: + description: DisplayName is the name that Coder shows to users in + chat. + maxLength: 128 + minLength: 1 + pattern: ^[^\s\v\x{85}\p{Z}]([\s\S]*[^\s\v\x{85}\p{Z}])?$ + type: string + enabled: + default: true + description: |- + Enabled turns the server on or off in Coder. Chats never connect to a + disabled server. When omitted, the default is true. + type: boolean + forwardCoderHeaders: + description: |- + ForwardCoderHeaders sends the chat owner, chat and workspace IDs to the + server as X-Coder-* headers. Coder sends no token in them. + type: boolean + iconURL: + description: IconURL is the icon URL, path or emoji that Coder shows. + maxLength: 2048 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + modelIntent: + description: ModelIntent asks the model to state its intent with each + tool call. + type: boolean + organization: + description: |- + Organization is the name of the Coder organization that holds the + config. Coder cannot move a config to another organization. Coder + reserves the names new and create. + maxLength: 32 + minLength: 1 + pattern: ^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$ + type: string + x-kubernetes-validations: + - message: organization must not be new or create + rule: self != 'new' && self != 'create' + - message: organization is immutable + rule: self == oldSelf + slug: + description: |- + Slug is the Coder slug of the config, unique in its organization. The + model sees the server's tools as `__`. + maxLength: 32 + minLength: 1 + pattern: ^[a-z0-9]+(-[a-z0-9]+)*$ + type: string + x-kubernetes-validations: + - message: slug is immutable + rule: self == oldSelf + transport: + default: streamable_http + description: Transport is the MCP transport that Coder uses. + enum: + - streamable_http + - sse + type: string + url: + description: |- + URL is the http:// or https:// MCP endpoint that Coder connects to. It + must not hold user info. Every Secret in auth must list its origin as an + allowed destination. + maxLength: 2048 + minLength: 1 + pattern: ^[^\s\v\x{85}\p{Z}]+$ + type: string + x-kubernetes-validations: + - message: url must be a valid URL with a host + rule: isURL(self) && url(self).getHostname() != '' + - message: url must be an http:// or https:// URL with a host and + no user info + rule: self.matches('^https?://[^/?#@\\s]+([/?#].*)?$') + required: + - auth + - controlPlaneRef + - displayName + - organization + - slug + - url + type: object + status: + description: CoderMCPServerStatus defines the observed state of a CoderMCPServer. + properties: + appliedGeneration: + description: |- + AppliedGeneration is the last generation whose spec Coder fully holds, + or that a kill switch holds: when the controller disables the config + because a credential Secret or the control plane withdrew consent, it + sets this field before it sends the disable, so only a later spec edit + enables the config again. Ready tells whether Coder holds the spec. + format: int64 + type: integer + conditions: + description: Conditions are Ready, Reconciling, and Stalled. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + configID: + description: ConfigID is the Coder MCP server config ID. + type: string + createAttemptTime: + description: |- + CreateAttemptTime is set before every create request. While it is set + and ConfigID is empty, a create request can be in flight or already + committed in Coder. + format: date-time + type: string + credentials: + description: Credentials record the Secret values last sent to Coder. + items: + description: |- + CoderMCPServerCredentialStatus records which Secret value was last sent to + Coder for one credential field. It holds no secret data and nothing derived + from it. + properties: + field: + description: |- + Field names the credential: apiKey, oauth2ClientSecret, or + `header:`. + maxLength: 160 + minLength: 1 + type: string + secretKey: + description: SecretKey is the key in the Secret that was read. + type: string + secretName: + description: SecretName is the Secret name that was read. + type: string + secretResourceVersion: + description: SecretResourceVersion is the resourceVersion of + the Secret that was read. + type: string + secretUID: + description: SecretUID is the UID of the Secret that was read. + type: string + required: + - field + type: object + type: array + x-kubernetes-list-map-keys: + - field + x-kubernetes-list-type: map + observedGeneration: + description: ObservedGeneration is the generation the controller last + processed. + format: int64 + type: integer + organizationID: + description: |- + OrganizationID is the Coder organization ID that spec.organization + resolved to first. + type: string + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.20.0 @@ -6496,6 +7046,7 @@ rules: resources: - coderaigatewaykeys - coderaiproviders + - codermcpservers verbs: - get - list @@ -6507,6 +7058,7 @@ rules: - coderaigatewaykeys/finalizers - coderaiproviders/finalizers - codercontrolplanes/finalizers + - codermcpservers/finalizers - coderprovisioners/finalizers - codertemplatetests/finalizers - coderworkspaceproxies/finalizers @@ -6517,6 +7069,7 @@ rules: resources: - coderaigatewaykeys/status - coderaiproviders/status + - codermcpservers/status verbs: - get - update @@ -6545,12 +7098,6 @@ rules: - get - patch - update -- apiGroups: - - coder.com - resources: - - codermcpservers - verbs: - - list - apiGroups: - coder.com resources: diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index c6a02ff4f..52533e09e 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -31,7 +31,7 @@ graph TD | --- | --- | --- | --- | | Code | `internal/app/controllerapp/`, `internal/controller/` | `internal/app/apiserverapp/`, `internal/aggregated/storage/`, `internal/aggregated/coder/` | `internal/app/mcpapp/` | | Listens on | `:8081` (`/healthz`, `/readyz`) | `:6443` HTTPS (default) | `127.0.0.1:8090` only. `/mcp` requires the bearer token. `/healthz` and `/readyz` do not. | -| Resources | `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey` | `coderworkspaces`, `codertemplates`, `codertemplateversions` | Tools for control planes, templates, workspaces, events, pod logs, and run state | +| Resources | `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey`, `CoderMCPServer` | `coderworkspaces`, `codertemplates`, `codertemplateversions` | Tools for control planes, templates, workspaces, events, pod logs, and run state | ### Controller @@ -75,8 +75,9 @@ graph TD | Path | Contents | | --- | --- | -| `config/crd/bases/` | The generated CRDs for `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, and `CoderAIGatewayKey` | +| `config/crd/bases/` | The generated CRDs for `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey`, and `CoderMCPServer` | | `config/rbac/` | The `coder-k8s` ServiceAccount, the `manager-role` ClusterRole, and the bindings (including auth-delegator) | +| `config/admission/` | Opt-in admission policies that the install bundle does not apply, for example the operator-only status write policy for `CoderMCPServer` | | `config/apiserver-standalone/` | For standalone `--app=aggregated-apiserver`: the `coder-k8s-apiserver` ServiceAccount, its RBAC, and the placeholder Secret for the serving CA | | `deploy/deployment.yaml` | The `coder-k8s` Deployment (default `--app=all`) | | `deploy/apiserver-service.yaml`, `deploy/apiserver-apiservice.yaml` | The Service and APIService that make the aggregated API available | diff --git a/docs/how-to/deploy-controller.md b/docs/how-to/deploy-controller.md index ee0691edd..88702c766 100644 --- a/docs/how-to/deploy-controller.md +++ b/docs/how-to/deploy-controller.md @@ -1,6 +1,6 @@ # Deploy the controller -Run `coder-k8s` as an operator only (`--app=controller`). The operator reconciles `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, and `CoderAIGatewayKey` resources. +Run `coder-k8s` as an operator only (`--app=controller`). The operator reconciles `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey`, and `CoderMCPServer` resources. Run the commands from a clone of this repository. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md new file mode 100644 index 000000000..813754fdf --- /dev/null +++ b/docs/how-to/manage-mcp-servers.md @@ -0,0 +1,346 @@ +# Manage MCP servers with CoderMCPServer + +A `CoderMCPServer` manages one MCP server config in a Coder organization (Coder's **AI settings > MCP servers**). Coder chats then offer the server's tools to the model. The operator creates the config, keeps it equal to the spec, and deletes it when you delete the object. + +!!! note "Not the coder-k8s MCP server" + This page is about MCP servers that Coder chats use. To run the MCP server that coder-k8s itself ships (`--app=mcp-http`), see [Run the MCP server](mcp-server.md). + +!!! warning "Who may write these resources" + The operator calls Coder as a site owner. A subject who can create or update `CoderMCPServer` objects acts as an organization admin for MCP servers in every organization that the control plane allows. See [Grant access](#grant-access). + +## Before you start + +- A `CoderControlPlane` in the same namespace with `status.operatorAccessReady: true`. The operator user must be a site owner. Without that, a create cannot be confirmed, and the object waits with `ConfirmingCreate`. +- Coder v2.37.2 or later. The Kind end-to-end test runs Coder v2.37.2, and the operator is built against the v2.37.3 SDK. +- The control plane opt-in below. + +## 1. Allow organizations on the control plane + +The control plane admin decides where `CoderMCPServer` objects can reach. Without `spec.mcpServers`, every `CoderMCPServer` of the control plane stalls with `MCPServersNotAllowed`, and the operator sends no MCP request. + +```yaml +apiVersion: coder.com/v1alpha1 +kind: CoderControlPlane +metadata: + name: coder + namespace: coder +spec: + mcpServers: + organizations: [coder] + allowForceOn: false +``` + +- `organizations` lists 1 to 64 Coder organization names. The operator resolves each name to its Coder organization ID and compares IDs, so the letter case of a name does not matter. A name that does not resolve yet allows nothing. The operator checks again every 10 minutes. +- `allowForceOn` lets objects use `availability: force_on`. A `force_on` server joins every new chat of each user who can read it, without that user's choice. The default is `false`. Then a `force_on` object stalls with `ForceOnNotAllowed`. +- When you remove an organization, remove `spec.mcpServers`, or set `allowForceOn` back to `false`, the operator disables each affected server in Coder. It sends one update that holds only `enabled: false`, and it reads no credential. Then it changes nothing more. The object stalls with `MCPServersNotAllowed` or `ForceOnNotAllowed`. After you restore the opt-in, edit the object's spec (any change, for example `description`) to enable the server again. +- Deleting a `CoderMCPServer` still deletes its config when the opt-in is gone. + +## 2. Prepare the credential Secret + +An `auth.type` of `api_key`, `custom_headers` or `oauth2` takes its credential from a Secret. The operator reads a value only from a Secret that opted in to the credential contract: + +- Type `Opaque`. +- The label `coder.com/credential: "true"`. +- The annotation `coder.com/credential-destinations` with the origin of every URL that receives the value. An origin is `scheme://host`, plus `:port` for a port that is not the default. Separate more than one origin with commas. + +| `auth.type` | Value | Origins that the Secret must list | +| --- | --- | --- | +| `none` | no Secret | none | +| `api_key` | `apiKey.secretRef`, sent in the header `apiKey.header` | `spec.url` | +| `custom_headers` | one `secretRef` per header, up to 16 headers | `spec.url` | +| `oauth2` | `oauth2.clientSecretRef` | `spec.url`, `authURL`, `tokenURL`, and `revocationURL` when set | + +```bash +kubectl -n coder create secret generic example-mcp-key --from-literal=api-key="$MCP_KEY" +kubectl -n coder label secret example-mcp-key coder.com/credential=true +kubectl -n coder annotate secret example-mcp-key coder.com/credential-destinations=https://mcp.example.com +``` + +The operator refuses a value that is empty, larger than 8 KiB, starts or ends with whitespace (a trailing newline counts), or is not valid UTF-8. A header value must also hold no control byte such as CR, LF or NUL. Coder sends an API key exactly as stored, so store a bearer token as `Bearer `. [Configure credential Secrets](workspace-proxy-credentials.md#checks-the-operator-runs) lists every rule of the contract. + +List only origins that you trust with the credential on every path. Coder sends the API key and the header values again on every request, also after a redirect to another origin. So a listed origin means "this origin and every place that it sends Coder". A plain `http://` URL is allowed for servers in the cluster, but Coder then sends the credential in clear text. The Secret must list the `http://` origin. + +## 3. Create the server + +```yaml +apiVersion: coder.com/v1alpha1 +kind: CoderMCPServer +metadata: + name: example + namespace: coder +spec: + controlPlaneRef: + name: coder + organization: coder + slug: example + displayName: Example + url: https://mcp.example.com/mcp + auth: + type: api_key + apiKey: + header: X-Api-Key + secretRef: + name: example-mcp-key + key: api-key + access: + groups: [Everyone] +``` + +```bash +kubectl apply -f example-mcp-server.yaml +kubectl -n coder wait --for=condition=Ready codermcpserver/example --timeout=2m +``` + +The sample, with its Secret, is in `config/samples/coder_v1alpha1_codermcpserver.yaml`. + +- `controlPlaneRef`, `organization` and `slug` cannot change. The model sees the tools as `__`. To rename a slug, delete the object and create a new one. Chats then lose the old server. +- The operator never takes over a config that it did not create. When the organization already has a config with the slug, the object stalls with `SlugConflict`, and the operator sends no write. +- The operator creates the config disabled, with a temporary display name. It then applies the spec. Members do not see the config before that. +- The operator pins the organization ID at the first lookup. If the name later resolves to another ID, the object stalls with `OrganizationChanged`. Re-create the object to use the new organization. +- Coder's `user_oidc` auth type is not supported. If someone sets it in the dashboard, the operator sets the auth type back at the next sync. + +## 4. Read the result + +```bash +kubectl -n coder get codermcpservers +kubectl -n coder get codermcpserver example -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' +``` + +The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled object waits for a change of its spec or of an input. + +| Reason | Meaning | +| --- | --- | +| `Applied` | Coder matches the spec (`Ready=True`). | +| `ControlPlaneNotReady`, `OperatorAccessNotReady` | The control plane cannot serve Coder calls yet. | +| `CoderUnavailable` | A Coder call failed. The operator retries every 30 seconds. | +| `OrganizationNotFound`, `OrganizationChanged` | The organization does not exist, or its ID changed. | +| `MCPServersNotAllowed`, `ForceOnNotAllowed` | The control plane does not allow this object. See [step 1](#1-allow-organizations-on-the-control-plane). | +| `CredentialSecretNotFound`, `CredentialNotOptedIn`, `CredentialDestinationNotAllowed`, `CredentialRefused`, `CredentialKeyMissing`, `CredentialMalformed` | A credential Secret fails the contract. See [Credential kill switch](#credential-kill-switch). | +| `SlugConflict` | The organization has a config with this slug that the operator did not create. | +| `CreatingConfig`, `ConfirmingCreate`, `CreateRetrying` | A create is in progress, or the operator reads Coder to learn whether a create committed. | +| `GroupNotFound` | A name in `access.groups` is not a Coder group yet. See [Access](#access). | +| `CoderRejected` | Coder rejected the request. Fix the spec. | +| `CoderAnswerMismatch` | Coder's answer does not match the request, for example an answer for another config. The operator checks Coder again later. | +| `DisabledExternally`, `DeletedExternally`, `CredentialClearedExternally` | Someone disabled the server, deleted it, or cleared a credential. See [Kill switches](#kill-switches). | + +## Access + +`spec.access` decides which Coder groups can read the server. Coder checks it when a chat selects the server. + +- With `access` omitted, the operator never reads or writes the server's access list. Coder's default lets the organization's `Everyone` group read the server. So every member of the organization can use it. Removing `access` later leaves the last access list in place. +- With `access` set, the access list is exactly `access.groups`, each with read access. `spec.access` lists groups only. So every sync removes user grants that someone added in the Coder dashboard, and every group that the list does not name. +- `access: {}` or `groups: []` leaves the server to organization admins. +- `Everyone` is the group of all organization members. Without a Coder license, only `Everyone` exists. + +When a group in `access.groups` does not exist, the sync fails closed: + +1. It removes every entry that `access.groups` no longer names, also user entries. +2. It adds nothing. Access never widens while a group is missing. +3. If the same spec generation disables the server (`enabled: false`), it sends one update that holds only `enabled: false`. +4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound` and checks again every 10 minutes. +5. Once the missing groups exist, the sync adds them and applies the rest of the generation. + +A new server stays disabled in Coder while one of its groups is missing. + +Removing a group from `access` does not detach chats that already use the server. Those chats keep it until the server is disabled or deleted. In an emergency, disable the server (see [Kill switches](#kill-switches)). + +## Rotate a credential + +Change the Secret value. The operator sends the new value with one update. Coder stores all custom headers in one field, so a change of one header value sends every header value. A change of only the Secret's labels or annotations sends no value, as long as the Secret keeps its consent for the origins. The operator remembers a hash of each sent value in memory only. After an operator restart, the first change of the Secret sends its value once more, also a change of only its metadata. Coder records each credential write in its audit log, with the value marked as secret. + +A new value that breaks the contract, for example a value with a trailing newline, triggers the [credential kill switch](#credential-kill-switch). + +Coder shows only whether a credential is set, not its value. So the operator cannot detect a dashboard replacement of a value, or the removal of one custom header of several. The next change of the Secret value overwrites it. + +## Change the URL + +The Secret must list the origin of every URL that receives its value. Change a URL in this order: + +1. Add the new origin to `coder.com/credential-destinations`. +2. Change `spec.url` (or an OAuth2 URL). +3. Wait for `Ready=True`. Then remove the old origin, if nothing else uses it. + +If you change the URL before the Secret lists the new origin, the object stalls with `CredentialDestinationNotAllowed`. The operator sends nothing, and Coder keeps the old URL and credential, which the Secret still allows. When you add the origin, the operator applies the new URL. + +An update that changes a URL or the auth type also sends every credential of the auth type. Coder deletes every user's OAuth2 token for the server when `url`, the auth type, `tokenURL`, `revocationURL` or `clientID` changes. Users must connect again. + +## Kill switches + +### Dashboard changes + +After the operator applied a spec generation, three dashboard changes are kill switches: + +- A disable stalls the object with `DisabledExternally`. +- A delete stalls it with `DeletedExternally`. +- A cleared credential, or a changed auth type, stalls it with `CredentialClearedExternally`. + +The operator then changes nothing until the spec changes. A spec edit (any change of `spec`, for example `displayName`) enables the server again, sends the Secret's credential again, or creates the server again. + +Coder sends no events. The operator sees a dashboard change at its next read: at the resync, about every 10 minutes, or at once when the object, its Secret or its control plane changes. Until then the object can still show `Ready=True`. + +### Credential kill switch + +For a server that the operator created, every credential error disables the server in Coder: the Secret is deleted, loses its label, no longer lists an origin that Coder sends the credential to, is refused, has no such key, or holds a malformed value. The update holds only `enabled: false`, with no credential and no URL. The operator first checks what Coder holds. When every credential that Coder holds comes from a recorded Secret that still allows the URLs that Coder holds, the operator disables nothing and sends nothing. This is the case during a URL change to an origin that the Secret does not list yet. + +The object stalls with the credential reason. Repairing the Secret does not enable the server again. To enable it, fix the Secret, then edit the spec. When the disable fails, the operator retries every 30 seconds, and the condition message starts with `Coder can still send the credentials`. + +### The disable window + +The operator marks the disable in the object's status before it sends the disable to Coder. This holds the disable also when the update reaches Coder but its answer is lost. If the operator stops after the status write and before the update, the next reconcile still sends the disable while consent or the opt-in stays withdrawn. + +This window has one known limit. If consent or the opt-in comes back between the status write and the update, the sync applies the current spec without a later spec edit, and the server stays enabled. Coder never disabled the server in that window. The sync checks every credential first, so no credential goes to an origin without consent. + +### Emergency + +To stop a server at once: + +1. Disable the server in the Coder dashboard (**AI settings > MCP servers**). The object reports `DisabledExternally`, and the operator does not enable it again until a spec edit. +2. Or set `spec.enabled: false`. +3. If a credential leaked, revoke it at the MCP server too. + +Disabling stops every chat from using the server, also chats that selected it earlier. + +## Delete a server + +Deleting the object deletes the config in Coder. Coder removes the server from every chat and deletes the users' OAuth2 tokens for it. The finalizer `coder.com/mcp-server-cleanup` keeps the object until then. + +- The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. +- When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. +- When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard. +- While the control plane is being deleted, the operator sends one delete with a 10-second limit, then removes the finalizer, also when that delete fails. So a namespace deletion finishes, but it can leave the config in Coder. +- A withdrawn opt-in never blocks a delete. + +Delete `CoderMCPServer` objects before their control plane and before their namespace. + +To release a stuck object by hand, remove the finalizer with a JSON patch whose `test` operation checks the index first, as for [AI providers](manage-ai-providers.md#stuck-finalizer). Then check **AI settings > MCP servers** in the dashboard for a leftover config with the slug. + +## Coder-side limits + +These limits come from Coder, and the operator cannot change them: + +- Coder checks server access when a chat selects the server. Chats that already use it keep it until the server is disabled or deleted. +- Coder applies tool allow and deny lists by exact, case-sensitive tool name. With both lists set, Coder ignores the deny list. This kind does not manage tool lists yet. Lists that you set in the dashboard stay. +- A change of the URL deletes every user's OAuth2 token for the server. +- Coder encrypts the stored credentials at rest only when enterprise database encryption keys are set. +- Coder has no address guard for MCP traffic. A server URL can point at a cluster-internal or private address. `auth.type: none` needs no Secret, so only the control plane opt-in and RBAC limit it. +- Coder's MCP client sets the API key, the header values and users' bearer tokens on every request, also after a redirect to another origin. A Secret's destination list therefore means "this origin and every place that it sends Coder". + +## GitOps health checks + +Argo CD and Flux need health rules to wait for a `CoderMCPServer`. The rules live in `config/gitops/`. This page copies them, and a unit test keeps the copies equal to the files. Both rules wait until `status.observedGeneration` equals `metadata.generation`. Then they read the conditions: + +| Conditions | Argo CD | Flux | +| --- | --- | --- | +| no status, or a stale `observedGeneration` | `Progressing` | `InProgress` (`Unknown` before the first status) | +| `Reconciling=True` | `Progressing` | `InProgress` | +| `Ready=True` | `Healthy` | `Current` | +| `Stalled=True` | `Degraded` | `Failed` | + +The Argo CD message is `: ` of the `Ready` condition. While the object is being deleted, Argo CD shows `Deleting the Coder MCP server config`. + +Add the script as the `argocd-cm` key `resource.customizations.health.coder.com_CoderMCPServer` (`config/gitops/argocd-health-codermcpserver.lua`): + +```lua +-- Argo CD health check for CoderMCPServer (coder.com/v1alpha1). +-- Install it as the argocd-cm key +-- resource.customizations.health.coder.com_CoderMCPServer. +-- docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. +local hs = { status = "Progressing", message = "Waiting for the controller" } +if obj.metadata.deletionTimestamp ~= nil then + -- Argo CD shows deletionMessage, not message, for a resource being deleted. + hs.deletionMessage = "Deleting the Coder MCP server config" + hs.message = hs.deletionMessage + return hs +end +local st = obj.status +if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then + return hs +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then + hs.message = c.reason .. ": " .. c.message + end +end +for _, c in ipairs(st.conditions or {}) do + if c.type == "Ready" and c.status == "True" then + hs.status = "Healthy" + elseif c.type == "Stalled" and c.status == "True" then + hs.status = "Degraded" + end +end +return hs +``` + +Add this entry (`config/gitops/flux-healthcheck-codermcpserver.yaml`) to `spec.healthCheckExprs` of a Flux v2.5+ Kustomization with `spec.wait: true`: + +```yaml +# Flux health check expressions for CoderMCPServer (coder.com/v1alpha1). +# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization +# with spec.wait: true. +# docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. +- apiVersion: coder.com/v1alpha1 + kind: CoderMCPServer + inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" + failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" + current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" +``` + +## Grant access + +The operator acts as a Coder site owner. So each permission in the control plane's namespace maps to Coder power: + +| Permission | Coder power | +| --- | --- | +| `create`, `update`, `patch` on `codermcpservers` | Organization admin for MCP servers in the organizations that the control plane lists: add servers with any URL and choose the access groups. With `allowForceOn`, also add servers to every member's new chats. The operator sends a credential only to origins that its Secret lists. | +| `update`, `patch` on `codermcpservers/status` | Operator only. `status.configID` tells the operator which config an object owns. A holder can point an object at any config in the listed organizations, then change or delete that config. | +| `delete` on `codermcpservers` | Removes owned servers from every chat | +| `update`, `patch` on `codercontrolplanes` | Chooses the organizations and allows `force_on` | +| `patch` on credential Secrets | Decides whether a credential can be used (label) and where it can go (annotation) | +| `get`, `list` on `codermcpservers` | Sees slugs, URLs, Secret names, Coder IDs and group names. No secrets. | + +A viewer Role: + +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: codermcpserver-viewer + namespace: coder +rules: + - apiGroups: ["coder.com"] + resources: ["codermcpservers"] + verbs: ["get", "list", "watch"] +``` + +An editor Role. Give it only to subjects who may act as an organization admin for MCP servers and who can also read the credential Secrets that their objects use: + +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: codermcpserver-editor + namespace: coder +rules: + - apiGroups: ["coder.com"] + resources: ["codermcpservers"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +``` + +Neither Role grants `codermcpservers/status`. Only the operator ServiceAccount must write it. + +!!! warning "Wildcards grant subresources" + A rule with `resources: ["*"]` in the `coder.com` group also grants `codermcpservers/status`. Do not give such a rule to anyone but the operator. + +A `ResourceQuota` on `count/codermcpservers.coder.com` limits how many servers a namespace can create. + +The emergency path needs no Kubernetes access: a Coder admin disables the server in the dashboard. + +### Enforce operator-only status writes + +`config/admission/status-operator-only.yaml` holds an opt-in `ValidatingAdmissionPolicy` and its binding. The policy denies every update and patch of `codermcpservers/status` by any subject except the operator ServiceAccount `system:serviceaccount:coder-system:coder-k8s`. It denies cluster admins too. The install bundle does not apply it. Apply it yourself: + +```bash +kubectl apply -f config/admission/ +``` + +The policy needs Kubernetes v1.30 or later. If the operator runs as another ServiceAccount or in another namespace, change the user name in the policy's expression first, or the operator cannot write status. To fix a status by hand, delete the binding `coder-k8s-status-operator-only` first, then apply it again. diff --git a/docs/index.md b/docs/index.md index 9016006f0..be93c69ae 100644 --- a/docs/index.md +++ b/docs/index.md @@ -11,7 +11,7 @@ Use native Kubernetes APIs to run and manage [Coder](https://coder.com). | Component | Resources | | --- | --- | -| Operator | `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey` (`coder.com/v1alpha1`) | +| Operator | `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey`, `CoderMCPServer` (`coder.com/v1alpha1`) | | Aggregated API server | `CoderWorkspace`, `CoderTemplate`, `CoderTemplateVersion` (`aggregation.coder.com/v1alpha1`) | | MCP server | Operational tools over HTTP | diff --git a/docs/reference/api/codercontrolplane.md b/docs/reference/api/codercontrolplane.md index 38356227a..581dc1ac0 100644 --- a/docs/reference/api/codercontrolplane.md +++ b/docs/reference/api/codercontrolplane.md @@ -40,7 +40,7 @@ | `tolerations` | [Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#toleration-v1-core) array | Tolerations are applied to the control plane pod. | | `affinity` | [Affinity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#affinity-v1-core) | Affinity configures pod affinity/anti-affinity rules. | | `topologySpreadConstraints` | [TopologySpreadConstraint](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#topologyspreadconstraint-v1-core) array | TopologySpreadConstraints control pod topology spread. | -| `mcpServers` | [MCPServersSpec](#mcpserversspec) | MCPServers configures CoderMCPServer objects that use this control plane. Removing it disables in Coder each server config that a CoderMCPServer created through this control plane. It has no effect yet: the CoderMCPServer controller is not shipped. | +| `mcpServers` | [MCPServersSpec](#mcpserversspec) | MCPServers configures CoderMCPServer objects that use this control plane. Removing it disables in Coder each server config that a CoderMCPServer created through this control plane. | | `aiGateway` | [AIGatewaySpec](#aigatewayspec) | AIGateway configures CoderAIGatewayKey objects that use this control plane. | | `workspacePools` | [WorkspacePoolsSpec](#workspacepoolsspec) | WorkspacePools configures CoderWorkspacePool objects that use this control plane. It has no effect yet: the CoderWorkspacePool controller is not shipped. | | `templateTests` | [TemplateTestsSpec](#templatetestsspec) | TemplateTests configures CoderTemplateTest runs against this control plane. | @@ -157,8 +157,8 @@ CoderControlPlane (CoderControlPlane.spec.mcpServers). | Field | Type | Description | | --- | --- | --- | -| `organizations` | string array | Organizations are the names of the Coder organizations whose MCP server configs CoderMCPServer objects can create, change and delete with this control plane's operator token. This delegates Coder organization-admin power over MCP servers in these organizations to everyone who can create CoderMCPServer objects here. A CoderMCPServer for an organization outside this list creates and changes nothing. Removing an organization disables in Coder each server config that a CoderMCPServer created there, with one request that holds only enabled: false, and then changes nothing else. After the organization is listed again, a spec edit enables the server. Deleting a CoderMCPServer still deletes the config it created. This field has no effect yet: the CoderMCPServer controller is not shipped. | -| `allowForceOn` | boolean | AllowForceOn lets CoderMCPServer objects use availability: force_on, which adds a server to every new chat of each user who can read it, without that user's choice. When false (the default), such objects create and change nothing, and a server config that one of them created is disabled in Coder, as for a removed organization. This field has no effect yet: the CoderMCPServer controller is not shipped. | +| `organizations` | string array | Organizations are the names of the Coder organizations whose MCP server configs CoderMCPServer objects can create, change and delete with this control plane's operator token. This delegates Coder organization-admin power over MCP servers in these organizations to everyone who can create CoderMCPServer objects here. A CoderMCPServer for an organization outside this list creates and changes nothing. Removing an organization disables in Coder each server config that a CoderMCPServer created there, with one request that holds only enabled: false, and then changes nothing else. After the organization is listed again, a spec edit enables the server. Deleting a CoderMCPServer still deletes the config it created. | +| `allowForceOn` | boolean | AllowForceOn lets CoderMCPServer objects use availability: force_on, which adds a server to every new chat of each user who can read it, without that user's choice. When false (the default), such objects create and change nothing, and a server config that one of them created is disabled in Coder, as for a removed organization. | ### OperatorAccessSpec diff --git a/docs/reference/api/codermcpserver.md b/docs/reference/api/codermcpserver.md new file mode 100644 index 000000000..831f48566 --- /dev/null +++ b/docs/reference/api/codermcpserver.md @@ -0,0 +1,136 @@ + + +# `CoderMCPServer` + +## API identity + +- Group/version: `coder.com/v1alpha1` +- Kind: `CoderMCPServer` +- Resource: `codermcpservers` +- Scope: namespaced + +## Spec + +| Field | Type | Description | +| --- | --- | --- | +| `controlPlaneRef` | [CoderControlPlaneReference](#codercontrolplanereference) | ControlPlaneRef names the CoderControlPlane in the same namespace. The controller calls Coder with that control plane's operator token. | +| `organization` | string | Organization is the name of the Coder organization that holds the config. Coder cannot move a config to another organization. Coder reserves the names new and create. | +| `slug` | string | Slug is the Coder slug of the config, unique in its organization. The model sees the server's tools as `__`. | +| `displayName` | string | DisplayName is the name that Coder shows to users in chat. | +| `description` | string | Description is a short summary of what the server provides. | +| `iconURL` | string | IconURL is the icon URL, path or emoji that Coder shows. | +| `url` | string | URL is the http:// or https:// MCP endpoint that Coder connects to. It must not hold user info. Every Secret in auth must list its origin as an allowed destination. | +| `transport` | string | Transport is the MCP transport that Coder uses. | +| `availability` | string | Availability decides how chats offer the server. force_on adds it to every new chat of a user who can read it. Coder's web UI preselects default_on servers. The Coder server treats default_on and default_off alike. | +| `enabled` | boolean | Enabled turns the server on or off in Coder. Chats never connect to a disabled server. When omitted, the default is true. | +| `allowInPlanMode` | boolean | AllowInPlanMode offers the server in plan-mode chats. | +| `modelIntent` | boolean | ModelIntent asks the model to state its intent with each tool call. | +| `forwardCoderHeaders` | boolean | ForwardCoderHeaders sends the chat owner, chat and workspace IDs to the server as X-Coder-* headers. Coder sends no token in them. | +| `auth` | [CoderMCPServerAuth](#codermcpserverauth) | Auth is how Coder authenticates to the server. | +| `access` | [CoderMCPServerAccess](#codermcpserveraccess) | Access, when set, makes the controller own the server's access list in Coder: exactly the listed groups can read the server. Every sync therefore also removes each user entry, for example a user that an admin added in the Coder dashboard. Coder does not show the entry of a deleted group, so the sync cannot remove it, but it grants nothing. When omitted, the controller leaves the access list alone, and Coder's default lets every organization member read the server. | + +## Status + +| Field | Type | Description | +| --- | --- | --- | +| `observedGeneration` | integer | ObservedGeneration is the generation the controller last processed. | +| `appliedGeneration` | integer | AppliedGeneration is the last generation whose spec Coder fully holds, or that a kill switch holds: when the controller disables the config because a credential Secret or the control plane withdrew consent, it sets this field before it sends the disable, so only a later spec edit enables the config again. Ready tells whether Coder holds the spec. | +| `organizationID` | string | OrganizationID is the Coder organization ID that spec.organization resolved to first. | +| `configID` | string | ConfigID is the Coder MCP server config ID. | +| `createAttemptTime` | [Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#time-v1-meta) | CreateAttemptTime is set before every create request. While it is set and ConfigID is empty, a create request can be in flight or already committed in Coder. | +| `credentials` | [CoderMCPServerCredentialStatus](#codermcpservercredentialstatus) array | Credentials record the Secret values last sent to Coder. | +| `conditions` | [Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#condition-v1-meta) array | Conditions are Ready, Reconciling, and Stalled. | + +## Referenced types + +### CoderControlPlaneReference + +CoderControlPlaneReference names a CoderControlPlane in the same namespace. + +| Field | Type | Description | +| --- | --- | --- | +| `name` | string | Name is the CoderControlPlane name. | + +### CoderMCPServerAPIKeyAuth + +CoderMCPServerAPIKeyAuth sends one Secret value in one header. Coder sends +the value as it is, so a bearer token is stored as `Bearer `. + +| Field | Type | Description | +| --- | --- | --- | +| `header` | string | Header is the HTTP header name. | +| `secretRef` | [SecretKeySelector](#secretkeyselector) | SecretRef references the key in a Secret in the same namespace that holds the header value. The Secret must opt in to the credential Secret contract and list the origin of spec.url. | + +### CoderMCPServerAccess + +CoderMCPServerAccess lists the Coder groups that can read the server. + +| Field | Type | Description | +| --- | --- | --- | +| `groups` | string array | Groups are Coder group names in the server's organization. Everyone is the group of all organization members. An empty list leaves the server to organization admins. Coder reserves the names new and create. While a listed group does not exist, the controller removes the access of every group that this list does not name, adds none, disables the server if the spec disables it, and waits with GroupNotFound for the rest. | + +### CoderMCPServerAuth + +CoderMCPServerAuth is how Coder authenticates to the MCP server. Exactly +the block that matches type is set. ++kubebuilder:validation:XValidation:rule="(self.type == 'api_key') == has(self.apiKey)",message="auth.apiKey must be set exactly when auth.type is api_key" ++kubebuilder:validation:XValidation:rule="(self.type == 'custom_headers') == has(self.customHeaders)",message="auth.customHeaders must be set exactly when auth.type is custom_headers" ++kubebuilder:validation:XValidation:rule="(self.type == 'oauth2') == has(self.oauth2)",message="auth.oauth2 must be set exactly when auth.type is oauth2" + +| Field | Type | Description | +| --- | --- | --- | +| `type` | string | Type is the auth type. Coder's user_oidc type is not supported, because it sends each user's login token to the server URL. | +| `apiKey` | [CoderMCPServerAPIKeyAuth](#codermcpserverapikeyauth) | APIKey sends one Secret value in one header. | +| `customHeaders` | [CoderMCPServerHeader](#codermcpserverheader) array | CustomHeaders sends each Secret value in its own header. | +| `oauth2` | [CoderMCPServerOAuth2Auth](#codermcpserveroauth2auth) | OAuth2 lets each user connect with an OAuth2 client that the admin registered at the authorization server. | + +### CoderMCPServerCredentialStatus + +CoderMCPServerCredentialStatus records which Secret value was last sent to +Coder for one credential field. It holds no secret data and nothing derived +from it. + +| Field | Type | Description | +| --- | --- | --- | +| `field` | string | Field names the credential: apiKey, oauth2ClientSecret, or `header:`. | +| `secretName` | string | SecretName is the Secret name that was read. | +| `secretKey` | string | SecretKey is the key in the Secret that was read. | +| `secretUID` | string | SecretUID is the UID of the Secret that was read. | +| `secretResourceVersion` | string | SecretResourceVersion is the resourceVersion of the Secret that was read. | + +### CoderMCPServerHeader + +CoderMCPServerHeader is one header whose value comes from a Secret. + +| Field | Type | Description | +| --- | --- | --- | +| `name` | string | Name is the HTTP header name. | +| `secretRef` | [SecretKeySelector](#secretkeyselector) | SecretRef references the key in a Secret in the same namespace that holds the header value. The Secret must opt in to the credential Secret contract and list the origin of spec.url. | + +### CoderMCPServerOAuth2Auth + +CoderMCPServerOAuth2Auth is a manually registered OAuth2 client. Coder's +automatic discovery and dynamic client registration are not supported. + +| Field | Type | Description | +| --- | --- | --- | +| `clientID` | string | ClientID is the OAuth2 client ID. | +| `clientSecretRef` | [SecretKeySelector](#secretkeyselector) | ClientSecretRef references the key in a Secret in the same namespace that holds the client secret. The Secret must opt in to the credential Secret contract and list the origins of spec.url, authURL, tokenURL and revocationURL. | +| `authURL` | string | AuthURL is the authorization endpoint that users' browsers open. | +| `tokenURL` | string | TokenURL is the token endpoint that Coder sends the client secret to. | +| `revocationURL` | string | RevocationURL is the https:// token revocation endpoint. Coder sends the client secret there when a user disconnects. | +| `scopes` | string array | Scopes are the OAuth2 scopes that Coder requests. | + +### SecretKeySelector + +SecretKeySelector identifies a key in a Secret. + +| Field | Type | Description | +| --- | --- | --- | +| `name` | string | Name is the Kubernetes Secret name. | +| `key` | string | Key is the key inside the Secret data map. | + +## Source + +- Go type: `api/v1alpha1/codermcpserver_types.go` +- Generated CRD: `config/crd/bases/coder.com_codermcpservers.yaml` diff --git a/docs/tutorials/getting-started.md b/docs/tutorials/getting-started.md index b24c9c189..a2f46bd1e 100644 --- a/docs/tutorials/getting-started.md +++ b/docs/tutorials/getting-started.md @@ -63,16 +63,17 @@ Then open `http://127.0.0.1:3000` in a browser. ## 5. Clean up (optional) -Delete the `CoderAIProvider` and `CoderAIGatewayKey` objects first, if you created any, and then the control plane, while the operator still runs. The operator then deletes the providers and keys in Coder, cleans up, and removes its finalizers. A provider or key deleted after its control plane stays in Coder. A `CoderAIGatewayKey` can take up to 5 minutes after its last key create. After that, remove the bundle. The commands set `BASE` again, because you can be in a new shell. +Delete the `CoderAIProvider`, `CoderAIGatewayKey` and `CoderMCPServer` objects first, if you created any, and then the control plane, while the operator still runs. The operator then deletes the providers, keys and MCP servers in Coder, cleans up, and removes its finalizers. A provider, key or MCP server deleted after its control plane stays in Coder. A `CoderAIGatewayKey` can take up to 5 minutes after its last key create. After that, remove the bundle. The commands set `BASE` again, because you can be in a new shell. !!! warning - Delete all other `CoderAIGatewayKey`, `CoderAIProvider`, `CoderControlPlane`, `CoderProvisioner`, and `CoderWorkspaceProxy` resources before you delete the bundle. Delete `CoderAIProvider` and `CoderAIGatewayKey` resources before their control plane. When you delete the bundle, you also delete the `coder.com` CRDs, and Kubernetes then removes all remaining resources of these kinds in the cluster. When you delete the `coder` namespace, you also delete all other objects in it, for example Secrets and PersistentVolumeClaims. + Delete all other `CoderAIGatewayKey`, `CoderAIProvider`, `CoderControlPlane`, `CoderMCPServer`, `CoderProvisioner`, and `CoderWorkspaceProxy` resources before you delete the bundle. Delete `CoderAIProvider`, `CoderAIGatewayKey` and `CoderMCPServer` resources before their control plane. When you delete the bundle, you also delete the `coder.com` CRDs, and Kubernetes then removes all remaining resources of these kinds in the cluster. When you delete the `coder` namespace, you also delete all other objects in it, for example Secrets and PersistentVolumeClaims. ```bash BASE="https://raw.githubusercontent.com/coder/coder-k8s/main" kubectl delete coderaiproviders --all -n coder --wait --timeout=120s kubectl delete coderaigatewaykeys --all -n coder --wait --timeout=420s +kubectl delete codermcpservers --all -n coder --wait --timeout=120s kubectl delete -f "$BASE/config/samples/coder_v1alpha1_codercontrolplane.yaml" --ignore-not-found kubectl wait --for=delete codercontrolplane/codercontrolplane-sample -n coder --timeout=120s kubectl delete -f "$BASE/dist/install.yaml" --ignore-not-found diff --git a/hack/update-manifests.sh b/hack/update-manifests.sh index 841e7a4f5..c205c5e0a 100755 --- a/hack/update-manifests.sh +++ b/hack/update-manifests.sh @@ -31,7 +31,7 @@ list_manifests() { # CRDs of kinds whose controller is not registered yet. They go to config/crd/dormant/, which no kustomization, # example, doc or script applies, so the kind is never served without its controller. # Move a file back by deleting its entry when the controller ships. -DORMANT_CRDS=(coder.com_codermcpservers.yaml coder.com_coderworkspacepools.yaml coder.com_coderworkspacepoolmembers.yaml coder.com_coderworkspaceclaims.yaml) +DORMANT_CRDS=(coder.com_coderworkspacepools.yaml coder.com_coderworkspacepoolmembers.yaml coder.com_coderworkspaceclaims.yaml) # RBAC manifests that only the aggregated API server needs; the controller-only install bundle leaves them out. BUNDLE_EXCLUDED_RBAC=(apiservice-cabundle-role.yaml auth-delegator-binding.yaml authentication-reader-binding.yaml) diff --git a/internal/app/controllerapp/controllerapp.go b/internal/app/controllerapp/controllerapp.go index d0ca9b118..defca81c5 100644 --- a/internal/app/controllerapp/controllerapp.go +++ b/internal/app/controllerapp/controllerapp.go @@ -144,6 +144,14 @@ func SetupControllers(mgr manager.Manager) error { return fmt.Errorf("unable to create AI Gateway key controller: %w", err) } + mcpServerReconciler := &controller.CoderMCPServerReconciler{ + Client: client, + Scheme: managerScheme, + } + if err := mcpServerReconciler.SetupWithManager(mgr); err != nil { + return fmt.Errorf("unable to create MCP server controller: %w", err) + } + return nil } diff --git a/internal/controller/codermcpserver_controller.go b/internal/controller/codermcpserver_controller.go index 11723133b..cf3c838a6 100644 --- a/internal/controller/codermcpserver_controller.go +++ b/internal/controller/codermcpserver_controller.go @@ -46,10 +46,14 @@ const ( mcpServerNoncePrefix = "coder-k8s:" ) -// CoderMCPServerReconciler reconciles CoderMCPServer objects (#155). It is not -// registered yet. It checks the control plane, the organization and its -// opt-in, and the credential Secrets, reports a slug that a config of -// someone else holds, creates the config, and syncs it on every reconcile. +// +kubebuilder:rbac:groups=coder.com,resources=codermcpservers,verbs=get;list;watch;patch +// +kubebuilder:rbac:groups=coder.com,resources=codermcpservers/status,verbs=get;update +// +kubebuilder:rbac:groups=coder.com,resources=codermcpservers/finalizers,verbs=update + +// CoderMCPServerReconciler reconciles CoderMCPServer objects (#155). It checks +// the control plane, the organization and its opt-in, and the credential +// Secrets, reports a slug that a config of someone else holds, creates the +// config, and syncs it on every reconcile. type CoderMCPServerReconciler struct { client.Client Scheme *runtime.Scheme @@ -78,9 +82,9 @@ func mcpServerStalled(requeue time.Duration, reason, format string, args ...any) return &mcpServerStep{stalled: true, reason: reason, message: fmt.Sprintf(format, args...), requeue: requeue} } -// SetupWithManager wires the reconciler into controller-runtime. Nothing calls -// it until activation. The controller watches its own kind without a -// generation predicate, so an annotation change also triggers a reconcile. +// SetupWithManager wires the reconciler into controller-runtime. The +// controller watches its own kind without a generation predicate, so an +// annotation change also triggers a reconcile. // A change of the referenced control plane or of a credential Secret requeues // the object. func (r *CoderMCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/codermcpserver_controller_test.go b/internal/controller/codermcpserver_controller_test.go index 142f9c3aa..016a160d4 100644 --- a/internal/controller/codermcpserver_controller_test.go +++ b/internal/controller/codermcpserver_controller_test.go @@ -341,6 +341,19 @@ func TestCoderMCPServerControlPlaneAndOptIn(t *testing.T) { } } + // The gate compares resolved IDs, not names: a listed name that Coder + // resolves to the object's organization ID allows the object. + env.fake.addOrganization("alias") + env.setOptIn(ctx, t, &coderv1alpha1.MCPServersSpec{Organizations: []string{"alias"}}) + env.fake.failNext(routeOrganization, fakeFault{Rewrite: func(answer any) any { return answer }}) // The object's own organization. + env.fake.failNext(routeOrganization, fakeFault{Rewrite: func(answer any) any { + org := answer.(codersdk.Organization) + org.ID = env.orgID + return org + }}) + got, _ = env.reconcile(ctx, t, server) + requireMCPServerStep(t, got, false, "Applied") + // A failed lookup of a listed name is a wait, not a refusal. env.setOptIn(ctx, t, &coderv1alpha1.MCPServersSpec{Organizations: []string{"other", "coder"}}) env.fake.failNext(routeOrganization, fakeFault{Rewrite: func(answer any) any { return answer }}) // The object's own organization. diff --git a/internal/controller/controllerapp_setup_test.go b/internal/controller/controllerapp_setup_test.go index f9144ba7c..3e9361998 100644 --- a/internal/controller/controllerapp_setup_test.go +++ b/internal/controller/controllerapp_setup_test.go @@ -6,11 +6,16 @@ import ( "time" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/config" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" "github.com/coder/coder-k8s/internal/app/controllerapp" ) @@ -18,7 +23,8 @@ import ( // against the envtest API server: no field index is registered twice, every // watched kind is served, and the caches sync. The test runs serially, and // its cache sees only an empty namespace, so its reconcilers never touch -// the objects of other tests. +// the objects of other tests. A CoderMCPServer in that namespace gets its +// finalizer and a status from the registered controller. func TestSetupControllersStartsEveryController(t *testing.T) { ctx := context.Background() namespace := createTestNamespace(ctx, t, "setup") @@ -41,6 +47,25 @@ func TestSetupControllersStartsEveryController(t *testing.T) { syncCtx, syncCancel := context.WithTimeout(ctx, 30*time.Second) defer syncCancel() require.True(t, mgr.GetCache().WaitForCacheSync(syncCtx), "the caches sync") + + server := &coderv1alpha1.CoderMCPServer{ObjectMeta: metav1.ObjectMeta{Name: "github", Namespace: namespace}, Spec: validMCPServerSpec()} + require.NoError(t, k8sClient.Create(ctx, server)) + t.Cleanup(func() { + _ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(server), server) + server.Finalizers = nil + _ = k8sClient.Update(context.Background(), server) + _ = k8sClient.Delete(context.Background(), server) + }) + require.Eventually(t, func() bool { + got := &coderv1alpha1.CoderMCPServer{} + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(server), got); err != nil { + return false + } + ready := meta.FindStatusCondition(got.Status.Conditions, coderv1alpha1.CoderMCPServerConditionReady) + return controllerutil.ContainsFinalizer(got, coderv1alpha1.CoderMCPServerCleanupFinalizer) && + ready != nil && ready.Reason == "ControlPlaneNotReady" + }, 30*time.Second, 100*time.Millisecond, "the CoderMCPServer controller reconciles the object") + cancel() require.NoError(t, <-done) } diff --git a/mkdocs.yml b/mkdocs.yml index 62e3d4bd8..9103d0d9b 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -45,6 +45,7 @@ nav: - Test a template version: how-to/test-templates.md - Manage AI providers: how-to/manage-ai-providers.md - Manage AI Gateway keys: how-to/manage-ai-gateway-keys.md + - Manage MCP servers: how-to/manage-mcp-servers.md - Gate template promotion with GitOps: how-to/gitops.md - Run the MCP server: how-to/mcp-server.md - Troubleshooting: how-to/troubleshooting.md @@ -55,6 +56,7 @@ nav: - CoderAIGatewayKey: reference/api/coderaigatewaykey.md - CoderAIProvider: reference/api/coderaiprovider.md - CoderControlPlane: reference/api/codercontrolplane.md + - CoderMCPServer: reference/api/codermcpserver.md - CoderProvisioner: reference/api/coderprovisioner.md - CoderTemplateTest: reference/api/codertemplatetest.md - CoderWorkspaceProxy: reference/api/coderworkspaceproxy.md From beece33da9831096bbcdab551fc5d4caa460cf9b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:01:59 +0000 Subject: [PATCH 06/11] =?UTF-8?q?=F0=9F=A4=96=20docs:=20drop=20the=20how-t?= =?UTF-8?q?o=20reference=20from=20the=20status=20policy=20until=20the=20ho?= =?UTF-8?q?w-to=20ships?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ieaabbcdd0d9d86fee7c40ae1c75a1d3be5c7c39c Signed-off-by: Thomas Kosiewski --- config/admission/status-operator-only.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/admission/status-operator-only.yaml b/config/admission/status-operator-only.yaml index 6f75298ac..f1cc5c6ae 100644 --- a/config/admission/status-operator-only.yaml +++ b/config/admission/status-operator-only.yaml @@ -5,7 +5,6 @@ # config/default does not apply this file. Apply it with # `kubectl apply -f config/admission/`. If the operator runs as another # ServiceAccount, change the user name in the expression first. -# docs/how-to/manage-mcp-servers.md describes the policy. apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingAdmissionPolicy metadata: From edaad03074ff53b96225f9e325a19545e321f19f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:02:41 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=A4=96=20docs:=20point=20the=20stat?= =?UTF-8?q?us=20policy=20at=20the=20CoderMCPServer=20how-to?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ic5ecff3c40758ebbfa45e8ef225240737741e6f4 Signed-off-by: Thomas Kosiewski --- config/admission/status-operator-only.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/admission/status-operator-only.yaml b/config/admission/status-operator-only.yaml index f1cc5c6ae..6f75298ac 100644 --- a/config/admission/status-operator-only.yaml +++ b/config/admission/status-operator-only.yaml @@ -5,6 +5,7 @@ # config/default does not apply this file. Apply it with # `kubectl apply -f config/admission/`. If the operator runs as another # ServiceAccount, change the user name in the expression first. +# docs/how-to/manage-mcp-servers.md describes the policy. apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingAdmissionPolicy metadata: From 8d88f0de93fa52200246e8dde248db4b09cb962d Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:08:27 +0000 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=A4=96=20ci:=20drop=20the=20duplica?= =?UTF-8?q?te=20credentialsecret=20path=20filter=20entry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I5b4346439f1f02c38cda613a32c45b37781acd5f Signed-off-by: Thomas Kosiewski --- .github/workflows/ci.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f68573115..149267861 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -88,8 +88,6 @@ jobs: - 'internal/controller/coderaiprovider*' - 'internal/controller/coderaigatewaykey*' - 'internal/controller/codermcpserver*' - # The credential Secret contract that CoderAIProvider and CoderMCPServer read through. - - 'internal/credentialsecret/**' - 'internal/controller/owned_secret.go' - 'internal/controller/reserved_secret_names.go' - 'internal/controller/credential_checks.go' From 55202d4b49f0e978b4f8e14b80652318fdcab05c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:09:24 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=A4=96=20docs:=20move=20the=20Coder?= =?UTF-8?q?MCPServer=20GitOps=20health=20rules=20to=20their=20own=20PR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ib12d8baad8b551aa8ce2561db582b6f794d29673 Signed-off-by: Thomas Kosiewski --- .../gitops/argocd-health-codermcpserver.lua | 28 ---- .../flux-healthcheck-codermcpserver.yaml | 9 - config/gitops/health_test.go | 154 +++++------------- docs/how-to/manage-mcp-servers.md | 60 ------- 4 files changed, 37 insertions(+), 214 deletions(-) delete mode 100644 config/gitops/argocd-health-codermcpserver.lua delete mode 100644 config/gitops/flux-healthcheck-codermcpserver.yaml diff --git a/config/gitops/argocd-health-codermcpserver.lua b/config/gitops/argocd-health-codermcpserver.lua deleted file mode 100644 index 7e0ac9e87..000000000 --- a/config/gitops/argocd-health-codermcpserver.lua +++ /dev/null @@ -1,28 +0,0 @@ --- Argo CD health check for CoderMCPServer (coder.com/v1alpha1). --- Install it as the argocd-cm key --- resource.customizations.health.coder.com_CoderMCPServer. --- docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. -local hs = { status = "Progressing", message = "Waiting for the controller" } -if obj.metadata.deletionTimestamp ~= nil then - -- Argo CD shows deletionMessage, not message, for a resource being deleted. - hs.deletionMessage = "Deleting the Coder MCP server config" - hs.message = hs.deletionMessage - return hs -end -local st = obj.status -if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then - return hs -end -for _, c in ipairs(st.conditions or {}) do - if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then - hs.message = c.reason .. ": " .. c.message - end -end -for _, c in ipairs(st.conditions or {}) do - if c.type == "Ready" and c.status == "True" then - hs.status = "Healthy" - elseif c.type == "Stalled" and c.status == "True" then - hs.status = "Degraded" - end -end -return hs diff --git a/config/gitops/flux-healthcheck-codermcpserver.yaml b/config/gitops/flux-healthcheck-codermcpserver.yaml deleted file mode 100644 index ac7696642..000000000 --- a/config/gitops/flux-healthcheck-codermcpserver.yaml +++ /dev/null @@ -1,9 +0,0 @@ -# Flux health check expressions for CoderMCPServer (coder.com/v1alpha1). -# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization -# with spec.wait: true. -# docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. -- apiVersion: coder.com/v1alpha1 - kind: CoderMCPServer - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" - failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" - current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" diff --git a/config/gitops/health_test.go b/config/gitops/health_test.go index ca7cc4ba9..a2314c708 100644 --- a/config/gitops/health_test.go +++ b/config/gitops/health_test.go @@ -1,6 +1,5 @@ -// Package gitops_test checks the GitOps health rules for CoderTemplateTest -// and CoderMCPServer. The rules are data files, so this package has no -// production code. +// Package gitops_test checks the GitOps health rules for CoderTemplateTest. +// The rules are data files, so this package has no production code. package gitops_test import ( @@ -16,20 +15,11 @@ import ( "sigs.k8s.io/yaml" ) -// ruleSet is the Argo CD script and the Flux entry of one kind, the doc page -// that copies them, and the cases that both must pass. -type ruleSet struct { - kind string - luaPath string - fluxPath string - docPath string - cases func() []healthCase -} - -var ruleSets = []ruleSet{ - {"CoderTemplateTest", "argocd-health-codertemplatetest.lua", "flux-healthcheck-codertemplatetest.yaml", "../../docs/how-to/gitops.md", healthCases}, - {"CoderMCPServer", "argocd-health-codermcpserver.lua", "flux-healthcheck-codermcpserver.yaml", "../../docs/how-to/manage-mcp-servers.md", mcpServerCases}, -} +const ( + luaPath = "argocd-health-codertemplatetest.lua" + fluxPath = "flux-healthcheck-codertemplatetest.yaml" + docPath = "../../docs/how-to/gitops.md" +) // fluxHealthCheck is one entry of a Flux Kustomization spec.healthCheckExprs. type fluxHealthCheck struct { @@ -113,118 +103,48 @@ func healthCases() []healthCase { } } -// mcpServerObject returns a CoderMCPServer at generation 2 with the given -// status. -func mcpServerObject(status map[string]any) map[string]any { - obj := map[string]any{ - "apiVersion": "coder.com/v1alpha1", - "kind": "CoderMCPServer", - "metadata": map[string]any{"name": "github", "namespace": "coder", "generation": int64(2)}, - "spec": map[string]any{"organization": "coder", "slug": "github"}, - } - if status != nil { - obj["status"] = status - } - return obj -} - -// mcpServerStatus returns the three conditions that the controller always -// writes together, with one reason and message: ready, reconciling (neither -// ready nor stalled), or stalled. -func mcpServerStatus(state, reason string) map[string]any { - ready, reconciling, stalled := "False", "True", "False" - switch state { - case "ready": - ready, reconciling = "True", "False" - case "stalled": - reconciling, stalled = "False", "True" - case "reconciling": - default: - panic("assertion failed: unknown MCP server fixture state " + state) - } - condition := func(conditionType, status string) any { - return map[string]any{"type": conditionType, "status": status, "reason": reason, "message": "details", "observedGeneration": int64(2)} - } - return map[string]any{ - "observedGeneration": int64(2), - "conditions": []any{condition("Ready", ready), condition("Reconciling", reconciling), condition("Stalled", stalled)}, - } -} - -func mcpServerCases() []healthCase { - stale := mcpServerStatus("ready", "Applied") - stale["observedGeneration"] = int64(1) - noConditions := map[string]any{"observedGeneration": int64(2)} - deleting := mcpServerObject(mcpServerStatus("ready", "Applied")) - deleting["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" - deletingWait := mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")) - deletingWait["metadata"].(map[string]any)["deletionTimestamp"] = "2026-10-14T10:00:00Z" - return []healthCase{ - {"no status", mcpServerObject(nil), "Progressing", "Waiting for the controller", "Unknown", ""}, - {"stale observedGeneration", mcpServerObject(stale), "Progressing", "Waiting for the controller", "InProgress", ""}, - {"no conditions", mcpServerObject(noConditions), "Progressing", "Waiting for the controller", "InProgress", ""}, - {"applied", mcpServerObject(mcpServerStatus("ready", "Applied")), "Healthy", "Applied: details", "Current", ""}, - {"control plane not ready", mcpServerObject(mcpServerStatus("reconciling", "ControlPlaneNotReady")), "Progressing", "ControlPlaneNotReady: details", "InProgress", ""}, - {"Coder unavailable", mcpServerObject(mcpServerStatus("reconciling", "CoderUnavailable")), "Progressing", "CoderUnavailable: details", "InProgress", ""}, - {"group not found", mcpServerObject(mcpServerStatus("stalled", "GroupNotFound")), "Degraded", "GroupNotFound: details", "Failed", ""}, - {"not allowed", mcpServerObject(mcpServerStatus("stalled", "MCPServersNotAllowed")), "Degraded", "MCPServersNotAllowed: details", "Failed", ""}, - {"disabled externally", mcpServerObject(mcpServerStatus("stalled", "DisabledExternally")), "Degraded", "DisabledExternally: details", "Failed", ""}, - {"credential not opted in", mcpServerObject(mcpServerStatus("stalled", "CredentialNotOptedIn")), "Degraded", "CredentialNotOptedIn: details", "Failed", ""}, - {"slug conflict", mcpServerObject(mcpServerStatus("stalled", "SlugConflict")), "Degraded", "SlugConflict: details", "Failed", ""}, - // Flux has no deletion rule: it waits for pruned objects separately. - {"being deleted", deleting, "Progressing", "Deleting the Coder MCP server config", "Current", "Deleting the Coder MCP server config"}, - {"being deleted while Coder is down", deletingWait, "Progressing", "Deleting the Coder MCP server config", "InProgress", "Deleting the Coder MCP server config"}, - } -} - func TestArgoCDHealth(t *testing.T) { t.Parallel() - for _, rules := range ruleSets { - script := readFile(t, rules.luaPath) - for _, tc := range rules.cases() { - t.Run(rules.kind+"/"+tc.name, func(t *testing.T) { - t.Parallel() - gotStatus, gotMessage, gotDeletion := argoHealth(t, script, tc.obj) - if gotStatus != tc.argoStatus || gotMessage != tc.argoMessage || gotDeletion != tc.argoDeletion { - t.Fatalf("got %q %q %q, want %q %q %q", gotStatus, gotMessage, gotDeletion, tc.argoStatus, tc.argoMessage, tc.argoDeletion) - } - }) - } + script := readFile(t, luaPath) + for _, tc := range healthCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + gotStatus, gotMessage, gotDeletion := argoHealth(t, script, tc.obj) + if gotStatus != tc.argoStatus || gotMessage != tc.argoMessage || gotDeletion != tc.argoDeletion { + t.Fatalf("got %q %q %q, want %q %q %q", gotStatus, gotMessage, gotDeletion, tc.argoStatus, tc.argoMessage, tc.argoDeletion) + } + }) } } func TestFluxHealth(t *testing.T) { t.Parallel() - for _, rules := range ruleSets { - var checks []fluxHealthCheck - if err := yaml.UnmarshalStrict([]byte(readFile(t, rules.fluxPath)), &checks); err != nil { - t.Fatalf("parse %s: %v", rules.fluxPath, err) - } - if len(checks) != 1 || checks[0].APIVersion != "coder.com/v1alpha1" || checks[0].Kind != rules.kind { - t.Fatalf("want one %s entry, got %+v", rules.kind, checks) - } - for _, tc := range rules.cases() { - t.Run(rules.kind+"/"+tc.name, func(t *testing.T) { - t.Parallel() - if got := fluxHealth(t, checks[0], tc.obj); got != tc.fluxStatus { - t.Fatalf("got %q, want %q", got, tc.fluxStatus) - } - }) - } + var checks []fluxHealthCheck + if err := yaml.UnmarshalStrict([]byte(readFile(t, fluxPath)), &checks); err != nil { + t.Fatalf("parse %s: %v", fluxPath, err) + } + if len(checks) != 1 || checks[0].APIVersion != "coder.com/v1alpha1" || checks[0].Kind != "CoderTemplateTest" { + t.Fatalf("want one CoderTemplateTest entry, got %+v", checks) + } + for _, tc := range healthCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := fluxHealth(t, checks[0], tc.obj); got != tc.fluxStatus { + t.Fatalf("got %q, want %q", got, tc.fluxStatus) + } + }) } } -// TestDocCopiesRules keeps the copies in the docs equal to the source files, -// because the docs build has no snippet include. +// TestDocCopiesRules keeps the copies in docs/how-to/gitops.md equal to the +// source files, because the docs build has no snippet include. func TestDocCopiesRules(t *testing.T) { t.Parallel() - for _, rules := range ruleSets { - doc := readFile(t, rules.docPath) - for lang, path := range map[string]string{"lua": rules.luaPath, "yaml": rules.fluxPath} { - block := "```" + lang + "\n" + readFile(t, path) + "```\n" - if !strings.Contains(doc, block) { - t.Errorf("%s has no ```%s block equal to %s", rules.docPath, lang, path) - } + doc := readFile(t, docPath) + for lang, path := range map[string]string{"lua": luaPath, "yaml": fluxPath} { + block := "```" + lang + "\n" + readFile(t, path) + "```\n" + if !strings.Contains(doc, block) { + t.Errorf("%s has no ```%s block equal to %s", docPath, lang, path) } } } diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 813754fdf..245b55a38 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -225,66 +225,6 @@ These limits come from Coder, and the operator cannot change them: - Coder has no address guard for MCP traffic. A server URL can point at a cluster-internal or private address. `auth.type: none` needs no Secret, so only the control plane opt-in and RBAC limit it. - Coder's MCP client sets the API key, the header values and users' bearer tokens on every request, also after a redirect to another origin. A Secret's destination list therefore means "this origin and every place that it sends Coder". -## GitOps health checks - -Argo CD and Flux need health rules to wait for a `CoderMCPServer`. The rules live in `config/gitops/`. This page copies them, and a unit test keeps the copies equal to the files. Both rules wait until `status.observedGeneration` equals `metadata.generation`. Then they read the conditions: - -| Conditions | Argo CD | Flux | -| --- | --- | --- | -| no status, or a stale `observedGeneration` | `Progressing` | `InProgress` (`Unknown` before the first status) | -| `Reconciling=True` | `Progressing` | `InProgress` | -| `Ready=True` | `Healthy` | `Current` | -| `Stalled=True` | `Degraded` | `Failed` | - -The Argo CD message is `: ` of the `Ready` condition. While the object is being deleted, Argo CD shows `Deleting the Coder MCP server config`. - -Add the script as the `argocd-cm` key `resource.customizations.health.coder.com_CoderMCPServer` (`config/gitops/argocd-health-codermcpserver.lua`): - -```lua --- Argo CD health check for CoderMCPServer (coder.com/v1alpha1). --- Install it as the argocd-cm key --- resource.customizations.health.coder.com_CoderMCPServer. --- docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. -local hs = { status = "Progressing", message = "Waiting for the controller" } -if obj.metadata.deletionTimestamp ~= nil then - -- Argo CD shows deletionMessage, not message, for a resource being deleted. - hs.deletionMessage = "Deleting the Coder MCP server config" - hs.message = hs.deletionMessage - return hs -end -local st = obj.status -if st == nil or st.observedGeneration == nil or st.observedGeneration ~= obj.metadata.generation then - return hs -end -for _, c in ipairs(st.conditions or {}) do - if c.type == "Ready" and c.reason ~= nil and c.message ~= nil then - hs.message = c.reason .. ": " .. c.message - end -end -for _, c in ipairs(st.conditions or {}) do - if c.type == "Ready" and c.status == "True" then - hs.status = "Healthy" - elseif c.type == "Stalled" and c.status == "True" then - hs.status = "Degraded" - end -end -return hs -``` - -Add this entry (`config/gitops/flux-healthcheck-codermcpserver.yaml`) to `spec.healthCheckExprs` of a Flux v2.5+ Kustomization with `spec.wait: true`: - -```yaml -# Flux health check expressions for CoderMCPServer (coder.com/v1alpha1). -# Add this entry to spec.healthCheckExprs of a Flux v2.5+ Kustomization -# with spec.wait: true. -# docs/how-to/manage-mcp-servers.md copies this file. A test keeps the copy equal. -- apiVersion: coder.com/v1alpha1 - kind: CoderMCPServer - inProgress: "!has(status.observedGeneration) || status.observedGeneration != metadata.generation || !has(status.conditions) || status.conditions.exists(c, c.type == 'Reconciling' && c.status == 'True')" - failed: "status.conditions.exists(c, c.type == 'Stalled' && c.status == 'True')" - current: "status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')" -``` - ## Grant access The operator acts as a Coder site owner. So each permission in the control plane's namespace maps to Coder power: From 09f7ebf0204964315343aa40647fabd83457c2a3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:20:16 +0000 Subject: [PATCH 10/11] =?UTF-8?q?=F0=9F=A4=96=20docs:=20name=20the=20reaso?= =?UTF-8?q?n=20after=20a=20restored=20opt-in=20or=20repaired=20Secret?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I5f46e212918e0211f4d7b7ce91c60e4787f94485 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-mcp-servers.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 245b55a38..b0a0b5af5 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -32,7 +32,7 @@ spec: - `organizations` lists 1 to 64 Coder organization names. The operator resolves each name to its Coder organization ID and compares IDs, so the letter case of a name does not matter. A name that does not resolve yet allows nothing. The operator checks again every 10 minutes. - `allowForceOn` lets objects use `availability: force_on`. A `force_on` server joins every new chat of each user who can read it, without that user's choice. The default is `false`. Then a `force_on` object stalls with `ForceOnNotAllowed`. -- When you remove an organization, remove `spec.mcpServers`, or set `allowForceOn` back to `false`, the operator disables each affected server in Coder. It sends one update that holds only `enabled: false`, and it reads no credential. Then it changes nothing more. The object stalls with `MCPServersNotAllowed` or `ForceOnNotAllowed`. After you restore the opt-in, edit the object's spec (any change, for example `description`) to enable the server again. +- When you remove an organization, remove `spec.mcpServers`, or set `allowForceOn` back to `false`, the operator disables each affected server in Coder. It sends one update that holds only `enabled: false`, and it reads no credential. Then it changes nothing more. The object stalls with `MCPServersNotAllowed` or `ForceOnNotAllowed`. After you restore the opt-in, the object reports `DisabledExternally` until you edit its spec. Edit the spec (any change, for example `description`) to enable the server again. - Deleting a `CoderMCPServer` still deletes its config when the opt-in is gone. ## 2. Prepare the credential Secret @@ -121,7 +121,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob | `GroupNotFound` | A name in `access.groups` is not a Coder group yet. See [Access](#access). | | `CoderRejected` | Coder rejected the request. Fix the spec. | | `CoderAnswerMismatch` | Coder's answer does not match the request, for example an answer for another config. The operator checks Coder again later. | -| `DisabledExternally`, `DeletedExternally`, `CredentialClearedExternally` | Someone disabled the server, deleted it, or cleared a credential. See [Kill switches](#kill-switches). | +| `DisabledExternally`, `DeletedExternally`, `CredentialClearedExternally` | Someone disabled the server, deleted it, or cleared a credential. `DisabledExternally` also follows a disable by the operator itself, after the opt-in or the credential consent came back. See [Kill switches](#kill-switches). | ## Access @@ -182,7 +182,7 @@ Coder sends no events. The operator sees a dashboard change at its next read: at For a server that the operator created, every credential error disables the server in Coder: the Secret is deleted, loses its label, no longer lists an origin that Coder sends the credential to, is refused, has no such key, or holds a malformed value. The update holds only `enabled: false`, with no credential and no URL. The operator first checks what Coder holds. When every credential that Coder holds comes from a recorded Secret that still allows the URLs that Coder holds, the operator disables nothing and sends nothing. This is the case during a URL change to an origin that the Secret does not list yet. -The object stalls with the credential reason. Repairing the Secret does not enable the server again. To enable it, fix the Secret, then edit the spec. When the disable fails, the operator retries every 30 seconds, and the condition message starts with `Coder can still send the credentials`. +The object stalls with the credential reason. Repairing the Secret does not enable the server again: the object then reports `DisabledExternally`. To enable it, fix the Secret, then edit the spec. When the disable fails, the operator retries every 30 seconds, and the condition message starts with `Coder can still send the credentials`. ### The disable window From 2b6f82e73bc0d0753d6241ea39823123c85d8253 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 17:25:16 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=A4=96=20docs:=20say=20that=20Valid?= =?UTF-8?q?atingAdmissionPolicy=20is=20GA=20in=20Kubernetes=201.30?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I0aa9d5a3fbd27961db36dfb3c74f2353df242ef0 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-mcp-servers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index b0a0b5af5..b216fc8a0 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -283,4 +283,4 @@ The emergency path needs no Kubernetes access: a Coder admin disables the server kubectl apply -f config/admission/ ``` -The policy needs Kubernetes v1.30 or later. If the operator runs as another ServiceAccount or in another namespace, change the user name in the policy's expression first, or the operator cannot write status. To fix a status by hand, delete the binding `coder-k8s-status-operator-only` first, then apply it again. +`ValidatingAdmissionPolicy` is GA in Kubernetes v1.30, so the policy needs v1.30 or later. If the operator runs as another ServiceAccount or in another namespace, change the user name in the policy's expression first, or the operator cannot write status. To fix a status by hand, delete the binding `coder-k8s-status-operator-only` first, then apply it again.