From af709e67efab60d15eb07b06cc57185e0cb88e8c Mon Sep 17 00:00:00 2001 From: Alexey Kazakov Date: Sat, 9 May 2026 08:17:47 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20add=20MCP=20server=20support=20(Pha?= =?UTF-8?q?se=201=20=E2=80=94=20HTTP/SSE=20and=20basic=20stdio)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add McpServerSpec type with CEL validation (command xor url) - Add spec.mcpServers map field to ClawSpec - Inject mcp.servers config into operator.json at reconciliation - Auto-extract HTTP MCP URL domains as proxy passthrough routes - Add McpServersConfigured status condition (set when non-empty) - Mark builtin domains in coveredDomains to prevent MCP duplicates Signed-off-by: Alexey Kazakov Co-authored-by: Cursor --- api/v1alpha1/claw_types.go | 34 ++ api/v1alpha1/zz_generated.deepcopy.go | 34 ++ .../bases/claw.sandbox.redhat.com_claws.yaml | 39 ++ docs/proposals/mcp-support-design.md | 2 +- internal/controller/claw_channels_test.go | 6 +- internal/controller/claw_mcp.go | 96 ++++ internal/controller/claw_mcp_test.go | 466 ++++++++++++++++++ internal/controller/claw_proxy.go | 36 +- internal/controller/claw_proxy_test.go | 186 ++++++- .../controller/claw_resource_controller.go | 10 +- 10 files changed, 877 insertions(+), 32 deletions(-) create mode 100644 internal/controller/claw_mcp.go create mode 100644 internal/controller/claw_mcp_test.go diff --git a/api/v1alpha1/claw_types.go b/api/v1alpha1/claw_types.go index a9e2e02..ec23212 100644 --- a/api/v1alpha1/claw_types.go +++ b/api/v1alpha1/claw_types.go @@ -51,6 +51,7 @@ const ( ConditionTypeCredentialsResolved = "CredentialsResolved" ConditionTypeProxyConfigured = "ProxyConfigured" ConditionTypeDevicePairingConfigured = "DevicePairingConfigured" + ConditionTypeMcpServersConfigured = "McpServersConfigured" ) // Annotation keys used on pod templates to trigger rollouts on config changes. @@ -214,6 +215,34 @@ type CredentialSpec struct { AllowedPaths []string `json:"allowedPaths,omitempty"` } +// McpServerSpec defines an MCP server the operator injects into OpenClaw's config. +// +kubebuilder:validation:XValidation:rule="has(self.command) || has(self.url)",message="either command (stdio) or url (HTTP) must be set" +// +kubebuilder:validation:XValidation:rule="!has(self.command) || !has(self.url)",message="command and url are mutually exclusive" +type McpServerSpec struct { + // Command is the executable for a stdio MCP server. + // +optional + Command string `json:"command,omitempty"` + + // Args are command-line arguments for the stdio server. + // +optional + Args []string `json:"args,omitempty"` + + // URL is the endpoint for an HTTP MCP server. + // +optional + URL string `json:"url,omitempty"` + + // Transport selects the HTTP transport type ("streamable-http" or "sse"). + // Only valid when url is set. + // +optional + Transport string `json:"transport,omitempty"` + + // Env are plain environment variables passed to the stdio server process + // and written into the MCP server config in operator.json. + // Use for non-secret values and tier-2 placeholder tokens. + // +optional + Env map[string]string `json:"env,omitempty"` +} + // ClawSpec defines the desired state of Claw type ClawSpec struct { // ConfigMode controls how operator config is applied on pod start. @@ -227,6 +256,11 @@ type ClawSpec struct { // Credentials configures proxy credential injection per domain. // +optional Credentials []CredentialSpec `json:"credentials,omitempty"` + + // McpServers declares MCP servers injected into OpenClaw's config. + // Map keys are server names as they appear in the mcp.servers config. + // +optional + McpServers map[string]McpServerSpec `json:"mcpServers,omitempty"` } // ClawStatus defines the observed state of Claw diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a92c68e..0051094 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -206,6 +206,13 @@ func (in *ClawSpec) DeepCopyInto(out *ClawSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.McpServers != nil { + in, out := &in.McpServers, &out.McpServers + *out = make(map[string]McpServerSpec, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClawSpec. @@ -312,6 +319,33 @@ func (in *GCPConfig) DeepCopy() *GCPConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *McpServerSpec) DeepCopyInto(out *McpServerSpec) { + *out = *in + if in.Args != nil { + in, out := &in.Args, &out.Args + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Env != nil { + in, out := &in.Env, &out.Env + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new McpServerSpec. +func (in *McpServerSpec) DeepCopy() *McpServerSpec { + if in == nil { + return nil + } + out := new(McpServerSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OAuth2Config) DeepCopyInto(out *OAuth2Config) { *out = *in diff --git a/config/crd/bases/claw.sandbox.redhat.com_claws.yaml b/config/crd/bases/claw.sandbox.redhat.com_claws.yaml index e8b633a..0e1cd13 100644 --- a/config/crd/bases/claw.sandbox.redhat.com_claws.yaml +++ b/config/crd/bases/claw.sandbox.redhat.com_claws.yaml @@ -243,6 +243,45 @@ spec: inferred defaults rule: self.type != 'oauth2' || has(self.oauth2) || has(self.channel) type: array + mcpServers: + additionalProperties: + description: McpServerSpec defines an MCP server the operator injects + into OpenClaw's config. + properties: + args: + description: Args are command-line arguments for the stdio server. + items: + type: string + type: array + command: + description: Command is the executable for a stdio MCP server. + type: string + env: + additionalProperties: + type: string + description: |- + Env are plain environment variables passed to the stdio server process + and written into the MCP server config in operator.json. + Use for non-secret values and tier-2 placeholder tokens. + type: object + transport: + description: |- + Transport selects the HTTP transport type ("streamable-http" or "sse"). + Only valid when url is set. + type: string + url: + description: URL is the endpoint for an HTTP MCP server. + type: string + type: object + x-kubernetes-validations: + - message: either command (stdio) or url (HTTP) must be set + rule: has(self.command) || has(self.url) + - message: command and url are mutually exclusive + rule: '!has(self.command) || !has(self.url)' + description: |- + McpServers declares MCP servers injected into OpenClaw's config. + Map keys are server names as they appear in the mcp.servers config. + type: object type: object status: description: ClawStatus defines the observed state of Claw diff --git a/docs/proposals/mcp-support-design.md b/docs/proposals/mcp-support-design.md index 98f1537..7d1f03e 100644 --- a/docs/proposals/mcp-support-design.md +++ b/docs/proposals/mcp-support-design.md @@ -236,7 +236,7 @@ Failures also set `Ready=False`. Each phase corresponds to a single PR. Phases must be merged in order. -### Phase 1: HTTP/SSE and basic stdio MCP support (tiers 1 & 2) +### Phase 1: HTTP/SSE and basic stdio MCP support (tiers 1 & 2) - DONE Delivers working MCP for HTTP servers (auto-allowlisted) and stdio servers with static/placeholder env vars. No `envFrom` field yet — the CRD only advertises capabilities the reconciler supports. diff --git a/internal/controller/claw_channels_test.go b/internal/controller/claw_channels_test.go index 4f970fd..03108f7 100644 --- a/internal/controller/claw_channels_test.go +++ b/internal/controller/claw_channels_test.go @@ -461,7 +461,7 @@ func TestGenerateProxyConfigWithChannelCompanions(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred})) + data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred}), nil) require.NoError(t, err) var cfg proxyConfig @@ -484,7 +484,7 @@ func TestGenerateProxyConfigWithChannelCompanions(t *testing.T) { Type: clawv1alpha1.CredentialTypeNone, } - data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred})) + data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred}), nil) require.NoError(t, err) var cfg proxyConfig @@ -513,7 +513,7 @@ func TestGenerateProxyConfigWithChannelCompanions(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred})) + data, err := generateProxyConfig(toResolved([]clawv1alpha1.CredentialSpec{cred}), nil) require.NoError(t, err) var cfg proxyConfig diff --git a/internal/controller/claw_mcp.go b/internal/controller/claw_mcp.go new file mode 100644 index 0000000..f93a067 --- /dev/null +++ b/internal/controller/claw_mcp.go @@ -0,0 +1,96 @@ +/* +Copyright 2026 Red Hat. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "encoding/json" + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + clawv1alpha1 "github.com/codeready-toolchain/claw-operator/api/v1alpha1" +) + +// injectMcpServersIntoConfigMap injects MCP server configuration into operator.json +// for all entries in spec.mcpServers. Stdio servers get command/args/env; HTTP servers +// get url/transport. +func injectMcpServersIntoConfigMap(objects []*unstructured.Unstructured, instance *clawv1alpha1.Claw) error { + if len(instance.Spec.McpServers) == 0 { + return nil + } + + servers := make(map[string]any, len(instance.Spec.McpServers)) + for name, spec := range instance.Spec.McpServers { + servers[name] = buildMcpServerConfig(spec) + } + + configMapName := getConfigMapName(instance.Name) + for _, obj := range objects { + if obj.GetKind() != ConfigMapKind || obj.GetName() != configMapName { + continue + } + + operatorJSON, found, err := unstructured.NestedString(obj.Object, "data", "operator.json") + if err != nil { + return fmt.Errorf("failed to extract operator.json from ConfigMap: %w", err) + } + if !found { + return fmt.Errorf("operator.json not found in ConfigMap data") + } + + var config map[string]any + if err := json.Unmarshal([]byte(operatorJSON), &config); err != nil { + return fmt.Errorf("failed to parse operator.json: %w", err) + } + + config["mcp"] = map[string]any{"servers": servers} + + updatedJSON, err := json.MarshalIndent(config, " ", " ") + if err != nil { + return fmt.Errorf("failed to marshal operator.json: %w", err) + } + + if err := unstructured.SetNestedField(obj.Object, string(updatedJSON), "data", "operator.json"); err != nil { + return fmt.Errorf("failed to set updated operator.json in ConfigMap: %w", err) + } + return nil + } + + return fmt.Errorf("ConfigMap %q not found in manifests", configMapName) +} + +// buildMcpServerConfig builds the JSON-ready config for a single MCP server entry. +func buildMcpServerConfig(spec clawv1alpha1.McpServerSpec) map[string]any { + entry := map[string]any{} + + if spec.Command != "" { + entry["command"] = spec.Command + if len(spec.Args) > 0 { + entry["args"] = spec.Args + } + if len(spec.Env) > 0 { + entry["env"] = spec.Env + } + } else { + entry["url"] = spec.URL + if spec.Transport != "" { + entry["transport"] = spec.Transport + } + } + + return entry +} diff --git a/internal/controller/claw_mcp_test.go b/internal/controller/claw_mcp_test.go new file mode 100644 index 0000000..4bde446 --- /dev/null +++ b/internal/controller/claw_mcp_test.go @@ -0,0 +1,466 @@ +/* +Copyright 2026 Red Hat. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + clawv1alpha1 "github.com/codeready-toolchain/claw-operator/api/v1alpha1" +) + +func makeMcpConfigMap(jsonContent string) []*unstructured.Unstructured { + cm := &unstructured.Unstructured{} + cm.SetKind(ConfigMapKind) + cm.SetName(getConfigMapName(testInstanceName)) + cm.Object["data"] = map[string]any{ + "operator.json": jsonContent, + } + return []*unstructured.Unstructured{cm} +} + +func getMcpConfig(t *testing.T, objects []*unstructured.Unstructured) map[string]any { + t.Helper() + raw, _, err := unstructured.NestedString(objects[0].Object, "data", "operator.json") + require.NoError(t, err) + var config map[string]any + require.NoError(t, json.Unmarshal([]byte(raw), &config)) + return config +} + +func testClawWithMcpServers(servers map[string]clawv1alpha1.McpServerSpec) *clawv1alpha1.Claw { + return &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{McpServers: servers}, + } +} + +func TestInjectMcpServersIntoConfigMap(t *testing.T) { + t.Run("should inject HTTP MCP server with url and transport", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "context7": { + URL: "https://mcp.context7.com/mcp", + Transport: "streamable-http", + }, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + mcp := config["mcp"].(map[string]any) + servers := mcp["servers"].(map[string]any) + server := servers["context7"].(map[string]any) + + assert.Equal(t, "https://mcp.context7.com/mcp", server["url"]) + assert.Equal(t, "streamable-http", server["transport"]) + assert.NotContains(t, server, "command") + assert.NotContains(t, server, "args") + assert.NotContains(t, server, "env") + }) + + t.Run("should inject stdio MCP server with command, args, and env", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "github": { + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-github"}, + Env: map[string]string{"GITHUB_PERSONAL_ACCESS_TOKEN": "placeholder"}, + }, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + mcp := config["mcp"].(map[string]any) + servers := mcp["servers"].(map[string]any) + server := servers["github"].(map[string]any) + + assert.Equal(t, "npx", server["command"]) + args := server["args"].([]any) + assert.Equal(t, []any{"-y", "@modelcontextprotocol/server-github"}, args) + env := server["env"].(map[string]any) + assert.Equal(t, "placeholder", env["GITHUB_PERSONAL_ACCESS_TOKEN"]) + assert.NotContains(t, server, "url") + assert.NotContains(t, server, "transport") + }) + + t.Run("should inject mixed HTTP and stdio servers", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "context7": { + URL: "https://mcp.context7.com/mcp", + Transport: "streamable-http", + }, + "github": { + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-github"}, + Env: map[string]string{"GITHUB_PERSONAL_ACCESS_TOKEN": "placeholder"}, + }, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + mcp := config["mcp"].(map[string]any) + servers := mcp["servers"].(map[string]any) + + require.Contains(t, servers, "context7") + require.Contains(t, servers, "github") + + ctx7 := servers["context7"].(map[string]any) + assert.Equal(t, "https://mcp.context7.com/mcp", ctx7["url"]) + + gh := servers["github"].(map[string]any) + assert.Equal(t, "npx", gh["command"]) + }) + + t.Run("should skip injection when mcpServers is empty", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(nil) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + assert.NotContains(t, config, "mcp") + }) + + t.Run("should omit args when empty", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "simple": {Command: "my-server"}, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + server := config["mcp"].(map[string]any)["servers"].(map[string]any)["simple"].(map[string]any) + assert.Equal(t, "my-server", server["command"]) + assert.NotContains(t, server, "args") + assert.NotContains(t, server, "env") + }) + + t.Run("should omit env when empty", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "tool": { + Command: "tool-server", + Args: []string{"--verbose"}, + }, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + server := config["mcp"].(map[string]any)["servers"].(map[string]any)["tool"].(map[string]any) + assert.Contains(t, server, "args") + assert.NotContains(t, server, "env") + }) + + t.Run("should omit transport when empty for HTTP server", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "remote": {URL: "https://api.example.com/mcp"}, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + server := config["mcp"].(map[string]any)["servers"].(map[string]any)["remote"].(map[string]any) + assert.Equal(t, "https://api.example.com/mcp", server["url"]) + assert.NotContains(t, server, "transport") + }) + + t.Run("should include env but omit args for stdio server with env only", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "db": { + Command: "node", + Env: map[string]string{"DB_HOST": "postgres.internal"}, + }, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + server := config["mcp"].(map[string]any)["servers"].(map[string]any)["db"].(map[string]any) + assert.Equal(t, "node", server["command"]) + assert.NotContains(t, server, "args") + assert.Contains(t, server, "env") + assert.Equal(t, "postgres.internal", server["env"].(map[string]any)["DB_HOST"]) + }) + + t.Run("should return error when ConfigMap not found", func(t *testing.T) { + objects := []*unstructured.Unstructured{} + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "test": {URL: "https://example.com/mcp"}, + }) + + err := injectMcpServersIntoConfigMap(objects, instance) + require.Error(t, err) + assert.Contains(t, err.Error(), "not found in manifests") + }) + + t.Run("should preserve existing config keys", func(t *testing.T) { + objects := makeMcpConfigMap(`{"gateway":{"port":18789},"models":{"providers":{}}}`) + instance := testClawWithMcpServers(map[string]clawv1alpha1.McpServerSpec{ + "test": {URL: "https://example.com/mcp"}, + }) + + require.NoError(t, injectMcpServersIntoConfigMap(objects, instance)) + + config := getMcpConfig(t, objects) + assert.Contains(t, config, "gateway") + assert.Contains(t, config, "models") + assert.Contains(t, config, "mcp") + }) +} + +func TestMcpServersIntegration(t *testing.T) { + ctx := context.Background() + + t.Run("should inject MCP servers into ConfigMap after reconciliation", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + secret := createTestAPIKeySecret(aiModelSecret, namespace, aiModelSecretKey, aiModelSecretValue) + require.NoError(t, k8sClient.Create(ctx, secret)) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + Credentials: testCredentials(), + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "context7": { + URL: "https://mcp.context7.com/mcp", + Transport: "streamable-http", + }, + }, + }, + } + require.NoError(t, k8sClient.Create(ctx, instance)) + + reconciler := createClawReconciler() + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + cm := &corev1.ConfigMap{} + waitFor(t, timeout, interval, func() bool { + return k8sClient.Get(ctx, client.ObjectKey{ + Name: getConfigMapName(testInstanceName), + Namespace: namespace, + }, cm) == nil + }, "ConfigMap should be created") + + operatorJSON, ok := cm.Data["operator.json"] + require.True(t, ok, "operator.json should exist") + + var config map[string]any + require.NoError(t, json.Unmarshal([]byte(operatorJSON), &config)) + + mcp, ok := config["mcp"].(map[string]any) + require.True(t, ok, "mcp section should exist") + servers, ok := mcp["servers"].(map[string]any) + require.True(t, ok, "mcp.servers section should exist") + require.Contains(t, servers, "context7") + + ctx7 := servers["context7"].(map[string]any) + assert.Equal(t, "https://mcp.context7.com/mcp", ctx7["url"]) + assert.Equal(t, "streamable-http", ctx7["transport"]) + }) + + t.Run("should set McpServersConfigured condition when mcpServers present", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + secret := createTestAPIKeySecret(aiModelSecret, namespace, aiModelSecretKey, aiModelSecretValue) + require.NoError(t, k8sClient.Create(ctx, secret)) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + Credentials: testCredentials(), + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "test": {URL: "https://example.com/mcp"}, + }, + }, + } + require.NoError(t, k8sClient.Create(ctx, instance)) + + reconciler := createClawReconciler() + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + updated := &clawv1alpha1.Claw{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{ + Name: testInstanceName, Namespace: namespace, + }, updated)) + + condition := meta.FindStatusCondition(updated.Status.Conditions, clawv1alpha1.ConditionTypeMcpServersConfigured) + require.NotNil(t, condition, "McpServersConfigured condition should be set") + assert.Equal(t, metav1.ConditionTrue, condition.Status) + assert.Equal(t, clawv1alpha1.ConditionReasonConfigured, condition.Reason) + }) + + t.Run("should not set McpServersConfigured condition when mcpServers empty", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + createClawInstance(t, ctx, testInstanceName, namespace) + reconciler := createClawReconciler() + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + updated := &clawv1alpha1.Claw{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{ + Name: testInstanceName, Namespace: namespace, + }, updated)) + + condition := meta.FindStatusCondition(updated.Status.Conditions, clawv1alpha1.ConditionTypeMcpServersConfigured) + assert.Nil(t, condition, "McpServersConfigured condition should not be set when no MCP servers") + }) + + t.Run("should inject stdio MCP server into ConfigMap after reconciliation", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + secret := createTestAPIKeySecret(aiModelSecret, namespace, aiModelSecretKey, aiModelSecretValue) + require.NoError(t, k8sClient.Create(ctx, secret)) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + Credentials: testCredentials(), + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "github": { + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-github"}, + Env: map[string]string{"GITHUB_PERSONAL_ACCESS_TOKEN": "placeholder"}, + }, + }, + }, + } + require.NoError(t, k8sClient.Create(ctx, instance)) + + reconciler := createClawReconciler() + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + cm := &corev1.ConfigMap{} + waitFor(t, timeout, interval, func() bool { + return k8sClient.Get(ctx, client.ObjectKey{ + Name: getConfigMapName(testInstanceName), + Namespace: namespace, + }, cm) == nil + }, "ConfigMap should be created") + + operatorJSON, ok := cm.Data["operator.json"] + require.True(t, ok, "operator.json should exist") + + var config map[string]any + require.NoError(t, json.Unmarshal([]byte(operatorJSON), &config)) + + mcp, ok := config["mcp"].(map[string]any) + require.True(t, ok, "mcp section should exist") + servers, ok := mcp["servers"].(map[string]any) + require.True(t, ok, "mcp.servers section should exist") + require.Contains(t, servers, "github") + + gh := servers["github"].(map[string]any) + assert.Equal(t, "npx", gh["command"]) + args := gh["args"].([]any) + assert.Equal(t, []any{"-y", "@modelcontextprotocol/server-github"}, args) + env := gh["env"].(map[string]any) + assert.Equal(t, "placeholder", env["GITHUB_PERSONAL_ACCESS_TOKEN"]) + }) +} + +func TestMcpServerCELValidation(t *testing.T) { + ctx := context.Background() + + t.Run("should reject MCP server with neither command nor url", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "empty": {}, + }, + }, + } + err := k8sClient.Create(ctx, instance) + require.Error(t, err, "CEL should reject MCP server with neither command nor url") + assert.Contains(t, err.Error(), "either command (stdio) or url (HTTP) must be set") + }) + + t.Run("should reject MCP server with both command and url", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "both": { + Command: "npx", + URL: "https://example.com/mcp", + }, + }, + }, + } + err := k8sClient.Create(ctx, instance) + require.Error(t, err, "CEL should reject MCP server with both command and url") + assert.Contains(t, err.Error(), "command and url are mutually exclusive") + }) + + t.Run("should accept valid stdio MCP server", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "github": { + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-github"}, + }, + }, + }, + } + err := k8sClient.Create(ctx, instance) + require.NoError(t, err, "valid stdio MCP server should be accepted") + }) + + t.Run("should accept valid HTTP MCP server", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "context7": { + URL: "https://mcp.context7.com/mcp", + Transport: "streamable-http", + }, + }, + }, + } + err := k8sClient.Create(ctx, instance) + require.NoError(t, err, "valid HTTP MCP server should be accepted") + }) +} diff --git a/internal/controller/claw_proxy.go b/internal/controller/claw_proxy.go index 16ca383..ec25d9d 100644 --- a/internal/controller/claw_proxy.go +++ b/internal/controller/claw_proxy.go @@ -29,6 +29,7 @@ import ( "encoding/pem" "fmt" "math/big" + "net/url" "sort" "strings" "time" @@ -100,8 +101,13 @@ var builtinPassthroughDomains = []builtinPassthrough{ } // generateProxyConfig builds the proxy config JSON from resolved credentials. +// HTTP MCP server URLs are auto-extracted as passthrough routes when not already +// covered by a credential or builtin domain. // Exact-match domains are emitted before suffix-match domains for predictable matching. -func generateProxyConfig(credentials []resolvedCredential) ([]byte, error) { +func generateProxyConfig( + credentials []resolvedCredential, + mcpServers map[string]clawv1alpha1.McpServerSpec, +) ([]byte, error) { var exact, suffix []proxyRoute coveredDomains := make(map[string]bool) @@ -111,10 +117,13 @@ func generateProxyConfig(credentials []resolvedCredential) ([]byte, error) { for _, bp := range builtinPassthroughDomains { if !coveredDomains[bp.Domain] { + coveredDomains[bp.Domain] = true exact = append(exact, proxyRoute{Domain: bp.Domain, Injector: "none", AllowedPaths: bp.AllowedPaths}) } } + exact = append(exact, mcpPassthroughRoutes(mcpServers, coveredDomains)...) + for _, rc := range credentials { cred := rc.CredentialSpec @@ -223,6 +232,31 @@ func generateProxyConfig(credentials []resolvedCredential) ([]byte, error) { return json.Marshal(cfg) } +// mcpPassthroughRoutes extracts domains from HTTP MCP server URLs and returns +// passthrough routes for domains not already covered by credentials or builtins. +func mcpPassthroughRoutes( + mcpServers map[string]clawv1alpha1.McpServerSpec, + coveredDomains map[string]bool, +) []proxyRoute { + var routes []proxyRoute + for _, mcp := range mcpServers { + if mcp.URL == "" { + continue + } + parsed, err := url.Parse(mcp.URL) + if err != nil || parsed.Hostname() == "" { + continue + } + domain := strings.ToLower(parsed.Hostname()) + if coveredDomains[domain] { + continue + } + coveredDomains[domain] = true + routes = append(routes, proxyRoute{Domain: domain, Injector: "none"}) + } + return routes +} + // applyProxyConfigMap creates or updates the proxy config ConfigMap with the precomputed JSON. func (r *ClawResourceReconciler) applyProxyConfigMap(ctx context.Context, instance *clawv1alpha1.Claw, configJSON []byte) error { logger := log.FromContext(ctx) diff --git a/internal/controller/claw_proxy_test.go b/internal/controller/claw_proxy_test.go index 26d78d9..0ff7603 100644 --- a/internal/controller/claw_proxy_test.go +++ b/internal/controller/claw_proxy_test.go @@ -176,7 +176,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -205,7 +205,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -231,7 +231,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -260,7 +260,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -293,7 +293,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -323,7 +323,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -349,7 +349,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -377,7 +377,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -407,7 +407,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -437,7 +437,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -463,7 +463,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -490,7 +490,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -520,7 +520,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -559,7 +559,7 @@ func TestGenerateProxyConfig(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -601,7 +601,7 @@ func TestGenerateProxyConfig(t *testing.T) { func TestBuiltinPassthroughDomains(t *testing.T) { t.Run("should include clawhub.ai as none route with no credentials", func(t *testing.T) { - data, err := generateProxyConfig(nil) //nolint:staticcheck + data, err := generateProxyConfig(nil, nil) require.NoError(t, err) var cfg proxyConfig @@ -612,7 +612,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }) t.Run("should include openrouter.ai as none route with no credentials", func(t *testing.T) { - data, err := generateProxyConfig(nil) //nolint:staticcheck + data, err := generateProxyConfig(nil, nil) require.NoError(t, err) var cfg proxyConfig @@ -623,7 +623,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }) t.Run("should include raw.githubusercontent.com with path restriction", func(t *testing.T) { - data, err := generateProxyConfig(nil) //nolint:staticcheck + data, err := generateProxyConfig(nil, nil) require.NoError(t, err) var cfg proxyConfig @@ -634,7 +634,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }) t.Run("should include registry.npmjs.org as none route with no credentials", func(t *testing.T) { - data, err := generateProxyConfig(nil) //nolint:staticcheck + data, err := generateProxyConfig(nil, nil) require.NoError(t, err) var cfg proxyConfig @@ -645,7 +645,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }) t.Run("should have no path restriction on unrestricted builtins", func(t *testing.T) { - data, err := generateProxyConfig(nil) //nolint:staticcheck + data, err := generateProxyConfig(nil, nil) require.NoError(t, err) var cfg proxyConfig @@ -666,7 +666,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -694,7 +694,7 @@ func TestBuiltinPassthroughDomains(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -866,7 +866,7 @@ func TestGenerateProxyConfigVertexSDK(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -909,7 +909,7 @@ func TestGenerateProxyConfigVertexSDK(t *testing.T) { }, } - data, err := generateProxyConfig(toResolved(credentials)) + data, err := generateProxyConfig(toResolved(credentials), nil) require.NoError(t, err) var cfg proxyConfig @@ -1216,7 +1216,7 @@ func TestGenerateProxyConfigKubernetes(t *testing.T) { }, } - data, err := generateProxyConfig(creds) + data, err := generateProxyConfig(creds, nil) require.NoError(t, err) var cfg proxyConfig @@ -1241,7 +1241,7 @@ func TestGenerateProxyConfigKubernetes(t *testing.T) { }, } - data, err := generateProxyConfig(creds) + data, err := generateProxyConfig(creds, nil) require.NoError(t, err) var cfg proxyConfig @@ -1267,7 +1267,7 @@ func TestGenerateProxyConfigKubernetes(t *testing.T) { }, } - data, err := generateProxyConfig(creds) + data, err := generateProxyConfig(creds, nil) require.NoError(t, err) var cfg proxyConfig @@ -1284,3 +1284,137 @@ func TestGenerateProxyConfigKubernetes(t *testing.T) { assert.Empty(t, stagingRoute.CACert, "route without CAData should have empty caCert") }) } + +func TestMcpServerDomainExtraction(t *testing.T) { + t.Run("should auto-extract HTTP MCP URL domain as passthrough route", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "context7": {URL: "https://mcp.context7.com/mcp", Transport: "streamable-http"}, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + route := findRouteByDomain(t, cfg.Routes, "mcp.context7.com") + assert.Equal(t, "none", route.Injector) + }) + + t.Run("should not duplicate domain already covered by credential", func(t *testing.T) { + credentials := []clawv1alpha1.CredentialSpec{ + { + Name: "mcp-auth", + Type: clawv1alpha1.CredentialTypeBearer, + Domain: "mcp.example.com", + SecretRef: []clawv1alpha1.SecretRefEntry{ + {Name: "mcp-secret", Key: "token"}, + }, + }, + } + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "example": {URL: "https://mcp.example.com/mcp"}, + } + + data, err := generateProxyConfig(toResolved(credentials), mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + count := 0 + for _, r := range cfg.Routes { + if r.Domain == "mcp.example.com" { + count++ + } + } + assert.Equal(t, 1, count, "domain should appear exactly once") + }) + + t.Run("should not duplicate domain already covered by builtin", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "npm-mcp": {URL: "https://registry.npmjs.org/mcp"}, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + count := 0 + for _, r := range cfg.Routes { + if r.Domain == "registry.npmjs.org" { + count++ + } + } + assert.Equal(t, 1, count, "builtin domain should not be duplicated") + }) + + t.Run("should not extract domain from stdio MCP server", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "github": { + Command: "npx", + Args: []string{"-y", "@modelcontextprotocol/server-github"}, + }, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + assert.Len(t, cfg.Routes, len(builtinPassthroughDomains), "only builtin routes should be present") + }) + + t.Run("should skip MCP server with invalid URL", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "bad": {URL: "://not-a-url"}, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + assert.Len(t, cfg.Routes, len(builtinPassthroughDomains), "invalid URL should be skipped") + }) + + t.Run("should deduplicate multiple MCP servers with same domain", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "svc1": {URL: "https://api.example.com/mcp1"}, + "svc2": {URL: "https://api.example.com/mcp2"}, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + count := 0 + for _, r := range cfg.Routes { + if r.Domain == "api.example.com" { + count++ + } + } + assert.Equal(t, 1, count, "same domain from multiple MCP servers should appear once") + }) + + t.Run("should handle case-insensitive domain matching", func(t *testing.T) { + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "upper": {URL: "https://MCP.Example.COM/api"}, + } + + data, err := generateProxyConfig(nil, mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + route := findRouteByDomain(t, cfg.Routes, "mcp.example.com") + assert.Equal(t, "none", route.Injector, "domain should be lowercased") + }) +} diff --git a/internal/controller/claw_resource_controller.go b/internal/controller/claw_resource_controller.go index 047172a..d5e5429 100644 --- a/internal/controller/claw_resource_controller.go +++ b/internal/controller/claw_resource_controller.go @@ -483,6 +483,11 @@ func (r *ClawResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + if len(instance.Spec.McpServers) > 0 { + setCondition(instance, clawv1alpha1.ConditionTypeMcpServersConfigured, metav1.ConditionTrue, + clawv1alpha1.ConditionReasonConfigured, "MCP server configuration injected") + } + // Filter out Route (applied in phase above) and proxy ConfigMap (controller-managed) remainingObjects := []*unstructured.Unstructured{} for _, obj := range objects { @@ -597,6 +602,9 @@ func (r *ClawResourceReconciler) enrichConfigAndNetworkPolicy( if err := injectChannelsIntoConfigMap(objects, instance); err != nil { return fmt.Errorf("failed to inject channels into ConfigMap: %w", err) } + if err := injectMcpServersIntoConfigMap(objects, instance); err != nil { + return fmt.Errorf("failed to inject MCP servers into ConfigMap: %w", err) + } if err := injectKubernetesSkill(objects, resolvedCreds, instance.Name); err != nil { return fmt.Errorf("failed to inject Kubernetes skill: %w", err) } @@ -646,7 +654,7 @@ func (r *ClawResourceReconciler) configureDeployments( func (r *ClawResourceReconciler) applyProxyResources(ctx context.Context, instance *clawv1alpha1.Claw, resolvedCreds []resolvedCredential) ([]byte, error) { logger := log.FromContext(ctx) - proxyConfigJSON, err := generateProxyConfig(resolvedCreds) + proxyConfigJSON, err := generateProxyConfig(resolvedCreds, instance.Spec.McpServers) if err != nil { logger.Error(err, "Failed to generate proxy config") setCondition(instance, clawv1alpha1.ConditionTypeProxyConfigured, metav1.ConditionFalse, clawv1alpha1.ConditionReasonConfigFailed, err.Error()) From 676d1810970539a7fb4a4ba8c8bd285f508c5c72 Mon Sep 17 00:00:00 2001 From: Alexey Kazakov Date: Sat, 9 May 2026 12:36:39 -0700 Subject: [PATCH 2/3] fix: respect suffix credentials in MCP domain extraction - Add domainCovered() to check suffix-match credential domains - Prevents MCP passthrough routes from shadowing auth injection when a suffix credential (e.g. ".googleapis.com") covers the domain Signed-off-by: Alexey Kazakov Co-authored-by: Cursor --- internal/controller/claw_proxy.go | 20 +++++++++++++++++- internal/controller/claw_proxy_test.go | 29 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/controller/claw_proxy.go b/internal/controller/claw_proxy.go index ec25d9d..5fa8d19 100644 --- a/internal/controller/claw_proxy.go +++ b/internal/controller/claw_proxy.go @@ -234,6 +234,9 @@ func generateProxyConfig( // mcpPassthroughRoutes extracts domains from HTTP MCP server URLs and returns // passthrough routes for domains not already covered by credentials or builtins. +// Suffix-match credentials (e.g. ".googleapis.com") cover subdomains, so an MCP +// URL like "https://us-central1-aiplatform.googleapis.com/..." won't emit a +// redundant exact route that would shadow the credential's auth injection. func mcpPassthroughRoutes( mcpServers map[string]clawv1alpha1.McpServerSpec, coveredDomains map[string]bool, @@ -248,7 +251,7 @@ func mcpPassthroughRoutes( continue } domain := strings.ToLower(parsed.Hostname()) - if coveredDomains[domain] { + if domainCovered(domain, coveredDomains) { continue } coveredDomains[domain] = true @@ -257,6 +260,21 @@ func mcpPassthroughRoutes( return routes } +// domainCovered returns true if domain is already covered by an exact entry or +// a suffix entry (leading dot) in coveredDomains. This prevents MCP passthrough +// routes from shadowing credential injection on suffix-matched domains. +func domainCovered(domain string, coveredDomains map[string]bool) bool { + if coveredDomains[domain] { + return true + } + for covered := range coveredDomains { + if strings.HasPrefix(covered, ".") && strings.HasSuffix(domain, covered) { + return true + } + } + return false +} + // applyProxyConfigMap creates or updates the proxy config ConfigMap with the precomputed JSON. func (r *ClawResourceReconciler) applyProxyConfigMap(ctx context.Context, instance *clawv1alpha1.Claw, configJSON []byte) error { logger := log.FromContext(ctx) diff --git a/internal/controller/claw_proxy_test.go b/internal/controller/claw_proxy_test.go index 0ff7603..e73d545 100644 --- a/internal/controller/claw_proxy_test.go +++ b/internal/controller/claw_proxy_test.go @@ -1417,4 +1417,33 @@ func TestMcpServerDomainExtraction(t *testing.T) { route := findRouteByDomain(t, cfg.Routes, "mcp.example.com") assert.Equal(t, "none", route.Injector, "domain should be lowercased") }) + + t.Run("should not add passthrough when suffix credential covers the domain", func(t *testing.T) { + credentials := []clawv1alpha1.CredentialSpec{ + { + Name: "gcp", + Type: clawv1alpha1.CredentialTypeGCP, + Domain: ".googleapis.com", + SecretRef: []clawv1alpha1.SecretRefEntry{ + {Name: "gcp-sa", Key: "key.json"}, + }, + GCP: &clawv1alpha1.GCPConfig{Project: "my-project", Location: "us-central1"}, + }, + } + mcpServers := map[string]clawv1alpha1.McpServerSpec{ + "vertex": {URL: "https://us-central1-aiplatform.googleapis.com/mcp"}, + } + + data, err := generateProxyConfig(toResolved(credentials), mcpServers) + require.NoError(t, err) + + var cfg proxyConfig + require.NoError(t, json.Unmarshal(data, &cfg)) + + for _, r := range cfg.Routes { + if r.Domain == "us-central1-aiplatform.googleapis.com" { + t.Fatal("MCP passthrough should not shadow suffix credential for .googleapis.com") + } + } + }) } From dafd024a09b7facaba3cf6813efb5f813272fc58 Mon Sep 17 00:00:00 2001 From: Alexey Kazakov Date: Sat, 9 May 2026 12:42:33 -0700 Subject: [PATCH 3/3] fix: clear stale McpServersConfigured condition on removal - Remove McpServersConfigured condition when spec.mcpServers is empty/removed instead of leaving a stale True - Add integration test verifying condition lifecycle across add-then-remove reconciliation cycles Signed-off-by: Alexey Kazakov Co-authored-by: Cursor --- internal/controller/claw_mcp_test.go | 40 +++++++++++++++++++ .../controller/claw_resource_controller.go | 2 + 2 files changed, 42 insertions(+) diff --git a/internal/controller/claw_mcp_test.go b/internal/controller/claw_mcp_test.go index 4bde446..36af36b 100644 --- a/internal/controller/claw_mcp_test.go +++ b/internal/controller/claw_mcp_test.go @@ -337,6 +337,46 @@ func TestMcpServersIntegration(t *testing.T) { assert.Nil(t, condition, "McpServersConfigured condition should not be set when no MCP servers") }) + t.Run("should remove McpServersConfigured condition when mcpServers removed", func(t *testing.T) { + t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) + + secret := createTestAPIKeySecret(aiModelSecret, namespace, aiModelSecretKey, aiModelSecretValue) + require.NoError(t, k8sClient.Create(ctx, secret)) + + instance := &clawv1alpha1.Claw{ + ObjectMeta: metav1.ObjectMeta{Name: testInstanceName, Namespace: namespace}, + Spec: clawv1alpha1.ClawSpec{ + Credentials: testCredentials(), + McpServers: map[string]clawv1alpha1.McpServerSpec{ + "test": {URL: "https://example.com/mcp"}, + }, + }, + } + require.NoError(t, k8sClient.Create(ctx, instance)) + + reconciler := createClawReconciler() + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + updated := &clawv1alpha1.Claw{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{ + Name: testInstanceName, Namespace: namespace, + }, updated)) + condition := meta.FindStatusCondition(updated.Status.Conditions, clawv1alpha1.ConditionTypeMcpServersConfigured) + require.NotNil(t, condition, "condition should be set after first reconcile") + + updated.Spec.McpServers = nil + require.NoError(t, k8sClient.Update(ctx, updated)) + + reconcileClaw(t, ctx, reconciler, testInstanceName, namespace) + + after := &clawv1alpha1.Claw{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{ + Name: testInstanceName, Namespace: namespace, + }, after)) + condition = meta.FindStatusCondition(after.Status.Conditions, clawv1alpha1.ConditionTypeMcpServersConfigured) + assert.Nil(t, condition, "McpServersConfigured condition should be removed after mcpServers cleared") + }) + t.Run("should inject stdio MCP server into ConfigMap after reconciliation", func(t *testing.T) { t.Cleanup(func() { deleteAndWaitAllResources(t, namespace) }) diff --git a/internal/controller/claw_resource_controller.go b/internal/controller/claw_resource_controller.go index d5e5429..2f9788a 100644 --- a/internal/controller/claw_resource_controller.go +++ b/internal/controller/claw_resource_controller.go @@ -486,6 +486,8 @@ func (r *ClawResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request if len(instance.Spec.McpServers) > 0 { setCondition(instance, clawv1alpha1.ConditionTypeMcpServersConfigured, metav1.ConditionTrue, clawv1alpha1.ConditionReasonConfigured, "MCP server configuration injected") + } else { + meta.RemoveStatusCondition(&instance.Status.Conditions, clawv1alpha1.ConditionTypeMcpServersConfigured) } // Filter out Route (applied in phase above) and proxy ConfigMap (controller-managed)