[feat][MCP] Make public routing mode explicit

.github/test_mcp_routing.py

@@ -0,0 +1,179 @@
+"""Check MCP public routes and migration errors in rendered Helm manifests."""
+
+import re
+import subprocess
+import unittest
+from pathlib import Path
+
+
+ROOT = Path(__file__).resolve().parents[1]
+BASE = [
+    "helm", "template", "routing", "charts/retool",
+    "--values", "charts/retool/ci/test-install-values.yaml",
+    "--values", "charts/retool/ci/test-mcp-enabled-option.yaml",
+    "--set", "ingress.hosts[0].host=retool.example.com",
+    "--set", "ingress.hosts[0].paths[0].path=/",
+]
+MAIN = "routing-retool"
+MCP = "routing-retool-mcp"
+BACKEND_API = "routing-retool-backend-internal"
+
+
+def render(*settings):
+    command = BASE.copy()
+    for setting in settings:
+        command.extend(("--set", setting))
+    return subprocess.run(command, cwd=ROOT, text=True, capture_output=True)
+
+
+def manifest(output, filename):
+    marker = f"# Source: retool/templates/{filename}\n"
+    return output.split(marker, 1)[1].split("\n---\n", 1)[0]
+
+
+def ingress_routes(output):
+    document = manifest(output, "ingress.yaml")
+    return re.findall(
+        r"(?m)^\s+- path: (\S+)\n\s+pathType: (\S+)\n\s+backend:\n\s+service:\n\s+name: (\S+)\n\s+port:\n\s+number: (\d+)",
+        document,
+    )
+
+
+def http_routes(output):
+    document = manifest(output, "httproute.yaml")
+    return re.findall(
+        r"(?m)^\s+- matches:\n\s+- path:\n\s+type: (\S+)\n\s+value: (\S+)\n\s+backendRefs:\n\s+- name: (\S+)\n\s+port: (\d+)",
+        document,
+    )
+
+
+INGRESS_DIRECT = [
+    ("/.well-known/oauth-authorization-server", "ImplementationSpecific", BACKEND_API, "3001"),
+    ("/.well-known/oauth-protected-resource", "ImplementationSpecific", MCP, "4010"),
+    ("/mcp", "Prefix", MCP, "4010"),
+    ("/", "ImplementationSpecific", MAIN, "3000"),
+]
+HTTP_DIRECT = [
+    ("Exact", "/.well-known/oauth-authorization-server", BACKEND_API, "3001"),
+    ("Exact", "/.well-known/oauth-protected-resource", MCP, "4010"),
+    ("PathPrefix", "/mcp", MCP, "4010"),
+    ("PathPrefix", "/", MAIN, "3000"),
+]
+
+
+class RoutingTests(unittest.TestCase):
+    def assert_rendered(self, *settings):
+        result = render(*settings)
+        self.assertEqual(result.returncode, 0, result.stderr)
+        return result.stdout
+
+    def assert_rejected(self, message, *settings):
+        result = render(*settings)
+        self.assertNotEqual(result.returncode, 0, result.stdout)
+        self.assertIn(message, result.stderr)
+
+    def test_default_direct_preserves_legacy_routes_for_both_route_types(self):
+        output = self.assert_rendered()
+        self.assertEqual(ingress_routes(output), INGRESS_DIRECT)
+        self.assertEqual(http_routes(output), HTTP_DIRECT)
+        self.assertIn('value: "http://routing-retool-mcp:4010"', manifest(output, "deployment_backend.yaml"))
+        self.assertIn("- name: MCP_SERVICE_INGRESS_DOMAIN", manifest(output, "deployment_backend.yaml"))
+
+    def test_backend_relay_uses_main_service_for_both_route_types(self):
+        output = self.assert_rendered("mcp.routing.mode=backendRelay")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+        self.assertEqual(http_routes(output), [HTTP_DIRECT[-1]])
+        self.assertIn("- name: MCP_SERVICE_INGRESS_DOMAIN", manifest(output, "deployment_backend.yaml"))
+
+    def test_hostname_ingress_branch_follows_mode(self):
+        output = self.assert_rendered("ingress.hostName=retool.example.com")
+        self.assertEqual(ingress_routes(output), INGRESS_DIRECT)
+        output = self.assert_rendered("ingress.hostName=retool.example.com", "mcp.routing.mode=backendRelay")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+
+    def test_explicit_true_legacy_flags_work_in_direct_mode(self):
+        output = self.assert_rendered("mcp.routing.mode=direct", "mcp.ingress.enabled=true", "mcp.httpRoute.enabled=true")
+        self.assertEqual(ingress_routes(output), INGRESS_DIRECT)
+        self.assertEqual(http_routes(output), HTTP_DIRECT)
+
+    def test_disabled_mcp_has_only_main_routes(self):
+        output = self.assert_rendered("mcp.enabled=false", "mcp.routing.mode=direct", "mcp.ingress.enabled=true", "mcp.httpRoute.enabled=true")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+        self.assertEqual(http_routes(output), [HTTP_DIRECT[-1]])
+        self.assertNotIn("- name: MCP_SERVICE_INGRESS_DOMAIN", manifest(output, "deployment_backend.yaml"))
+
+    def test_explicit_false_skips_direct_rules_on_that_surface(self):
+        output = self.assert_rendered("mcp.routing.mode=direct", "mcp.ingress.enabled=false")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+        self.assertEqual(http_routes(output), HTTP_DIRECT)
+        output = self.assert_rendered("mcp.routing.mode=direct", "mcp.httpRoute.enabled=false")
+        self.assertEqual(ingress_routes(output), INGRESS_DIRECT)
+        self.assertEqual(http_routes(output), [HTTP_DIRECT[-1]])
+
+    def test_custom_legacy_ports_survive_in_direct_mode(self):
+        output = self.assert_rendered(
+            "mcp.routing.mode=direct",
+            "mcp.ingress.paths[0].path=/.well-known/oauth-authorization-server",
+            "mcp.ingress.paths[0].pathType=Exact",
+            "mcp.ingress.paths[0].target=backendInternal",
+            "mcp.ingress.paths[0].port=3002",
+            "mcp.httpRoute.rules[0].path=/mcp",
+            "mcp.httpRoute.rules[0].pathType=PathPrefix",
+            "mcp.httpRoute.rules[0].port=4020",
+        )
+        self.assertEqual(ingress_routes(output)[0], ("/.well-known/oauth-authorization-server", "Exact", BACKEND_API, "3002"))
+        self.assertEqual(http_routes(output)[0], ("PathPrefix", "/mcp", MCP, "4020"))
+
+    def test_backend_internal_path_without_type_uses_nginx_compatible_default(self):
+        output = self.assert_rendered(
+            "mcp.ingress.paths[0].path=/.well-known/oauth-authorization-server",
+            "mcp.ingress.paths[0].target=backendInternal",
+        )
+        self.assertEqual(ingress_routes(output)[0], INGRESS_DIRECT[0])
+
+    def test_explicit_false_is_accepted_in_backend_relay(self):
+        output = self.assert_rendered("mcp.routing.mode=backendRelay", "mcp.ingress.enabled=false", "mcp.httpRoute.enabled=false")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+        self.assertEqual(http_routes(output), [HTTP_DIRECT[-1]])
+
+    def test_old_explicit_true_flags_fail_with_migration_guidance(self):
+        for surface in ("ingress", "httpRoute"):
+            with self.subTest(surface=surface):
+                self.assert_rejected(
+                    f"mcp.{surface}.enabled=true conflicts with mcp.routing.mode=backendRelay",
+                    "mcp.routing.mode=backendRelay",
+                    f"mcp.{surface}.enabled=true",
+                )
+
+    def test_legacy_flags_reject_non_boolean_values(self):
+        for surface in ("ingress", "httpRoute"):
+            for value in ("0", ""):
+                with self.subTest(surface=surface, value=value):
+                    self.assert_rejected(
+                        f"mcp.{surface}.enabled must be true, false, or null",
+                        "mcp.routing.mode=direct",
+                        f"mcp.{surface}.enabled={value}",
+                    )
+
+    def test_unknown_mode_fails_when_public_routes_are_enabled(self):
+        self.assert_rejected('mcp.routing.mode must be "backendRelay" or "direct"', "mcp.routing.mode=other")
+
+    def test_mode_is_not_validated_without_chart_managed_public_routes(self):
+        for routing in ("mcp.routing.mode=other", "mcp.routing=null"):
+            with self.subTest(routing=routing):
+                self.assert_rendered(routing, "ingress.enabled=false", "httpRoute.enabled=false")
+
+    def test_missing_mode_gives_upgrade_guidance(self):
+        self.assert_rejected(
+            "mcp.routing.mode is missing: when upgrading from an older chart, use --reset-then-reuse-values",
+            "mcp.routing=null",
+        )
+
+    def test_missing_mode_is_ignored_when_mcp_is_disabled(self):
+        output = self.assert_rendered("mcp.routing=null", "mcp.enabled=false")
+        self.assertEqual(ingress_routes(output), [INGRESS_DIRECT[-1]])
+        self.assertEqual(http_routes(output), [HTTP_DIRECT[-1]])
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/workflows/ci.yaml

@@ -80,6 +80,8 @@ jobs:
         run: ct lint --config .github/ct.yaml
       - name: Verify MCP Helm test render and HTTP behavior
         run: python3 .github/test_mcp_helm_test.py
+      - name: Verify MCP routing modes and migration errors
+        run: python3 .github/test_mcp_routing.py
 
   # We don't use helm-docs yet, can set this up later
   #

charts/retool/Chart.yaml

@@ -2,7 +2,7 @@ apiVersion: v2
 name: retool
 description: A Helm chart for Kubernetes
 type: application
-version: 6.13.3
+version: 6.13.4
 maintainers:
   - name: Retool Engineering
     email: engineering+helm@retool.com

charts/retool/templates/_helpers.tpl

@@ -51,6 +51,34 @@ env.BASE_DOMAIN. Secret-backed values cannot be resolved at template time.
 {{- trimSuffix "/" (trimPrefix "http://" (trimPrefix "https://" (toString $domain))) -}}
 {{- end }}
 
+{{/* Resolve MCP routing mode and reject incompatible explicit route flags. */}}
+{{- define "retool.mcp.routingMode" -}}
+{{- $mcp := .Values.mcp | default dict -}}
+{{- $routing := $mcp.routing | default dict -}}
+{{- $mode := $routing.mode -}}
+{{- if not (hasKey $routing "mode") -}}
+{{- if $mcp.enabled -}}
+{{- fail "mcp.routing.mode is missing: when upgrading from an older chart, use --reset-then-reuse-values so Helm loads the new chart defaults, or explicitly set mcp.routing.mode=direct (compatible default) or backendRelay (Retool 4.0.7+)" -}}
+{{- else -}}
+{{- $mode = "direct" -}}
+{{- end -}}
+{{- end -}}
+{{- if not (has $mode (list "backendRelay" "direct")) -}}
+{{- fail (printf "mcp.routing.mode must be \"backendRelay\" or \"direct\" (got %q)" (toString $mode)) -}}
+{{- end -}}
+{{- range $surface := list "ingress" "httpRoute" -}}
+  {{- $legacy := get $mcp $surface | default dict -}}
+  {{- $enabled := get $legacy "enabled" -}}
+  {{- if and (hasKey $legacy "enabled") (not (kindIs "bool" $enabled)) (not (kindIs "invalid" $enabled)) -}}
+    {{- fail (printf "mcp.%s.enabled must be true, false, or null" $surface) -}}
+  {{- end -}}
+  {{- if and $mcp.enabled (eq $mode "backendRelay") $enabled -}}
+    {{- fail (printf "mcp.%s.enabled=true conflicts with mcp.routing.mode=backendRelay: remove mcp.%s.enabled or set it to false to route through the main Retool Service; use mcp.routing.mode=direct to keep dedicated routes" $surface $surface) -}}
+  {{- end -}}
+{{- end -}}
+{{- $mode -}}
+{{- end }}
+
 {{/*
 MCP Service env var for the main Retool backend. Explicit backend env settings
 take precedence over the chart-generated in-cluster Service URL.
@@ -89,7 +117,6 @@ target: backendInternal routes to the backend API Service.
 {{- if eq $target "backendInternal" -}}
 {{- $serviceName = include "retool.backendInternal.name" $root -}}
 {{- $servicePort = $path.port | default (.backendInternalPort | default 3001) -}}
-{{- $pathType = $path.pathType | default "Exact" -}}
 {{- end -}}
 - path: {{ $path.path }}
   {{- if (semverCompare ">=1.18-0" $root.Capabilities.KubeVersion.Version) }}

charts/retool/templates/httproute.yaml

@@ -4,6 +4,8 @@
 {{- $mcp := .Values.mcp | default dict -}}
 {{- $mcpBackendMetadata := $mcp.backendMetadata | default dict -}}
 {{- $mcpHttpRoute := $mcp.httpRoute | default dict -}}
+{{- $mcpRoutingMode := include "retool.mcp.routingMode" . -}}
+{{- $mcpDirectHttpRoute := and $mcp.enabled (eq $mcpRoutingMode "direct") (or (not (kindIs "bool" $mcpHttpRoute.enabled)) $mcpHttpRoute.enabled) -}}
 {{- $backendInternalService := $mcpBackendMetadata.service | default dict -}}
 {{- $backendInternalPort := $backendInternalService.externalPort | default 3001 -}}
 apiVersion: gateway.networking.k8s.io/v1
@@ -40,7 +42,7 @@ spec:
           port: {{ .port }}
     {{- end }}
     {{- end }}
-    {{- if ( and ((.Values.mcp).enabled) $mcpHttpRoute.enabled ) }}
+    {{- if $mcpDirectHttpRoute }}
     {{- range $mcpHttpRoute.rules }}
     {{- include "retool.httpRoute.mcpRule" (dict "root" $ "rule" . "backendInternalPort" $backendInternalPort) | nindent 4 }}
     {{- end }}

charts/retool/templates/ingress.yaml

@@ -4,6 +4,8 @@
 {{- $mcp := .Values.mcp | default dict -}}
 {{- $mcpBackendMetadata := $mcp.backendMetadata | default dict -}}
 {{- $mcpIngress := $mcp.ingress | default dict -}}
+{{- $mcpRoutingMode := include "retool.mcp.routingMode" . -}}
+{{- $mcpDirectIngress := and $mcp.enabled (eq $mcpRoutingMode "direct") (or (not (kindIs "bool" $mcpIngress.enabled)) $mcpIngress.enabled) -}}
 {{- $backendInternalService := $mcpBackendMetadata.service | default dict -}}
 {{- $backendInternalPort := $backendInternalService.externalPort | default 3001 -}}
 {{- $pathType := .Values.ingress.pathType -}}
@@ -49,12 +51,12 @@ spec:
                   number: {{ .port }}
           {{- end }}
           {{- end }}
-          {{- if ( and ((.Values.mcp).enabled) $mcpIngress.enabled ) }}
+          {{- if $mcpDirectIngress }}
           {{- range $mcpIngress.paths }}
           {{- include "retool.ingress.mcpPath" (dict "root" $ "path" . "backendInternalPort" $backendInternalPort) | nindent 10 }}
           {{- end }}
           {{- end }}
-          - path:
+          - path: /
             {{- if and $pathType (semverCompare ">=1.18-0" $.Capabilities.KubeVersion.Version) }}
             pathType: {{ $pathType }}
             {{- end }}
@@ -90,7 +92,7 @@ spec:
                   number: {{ .port }}
           {{- end }}
           {{- end }}
-          {{- if ( and (($.Values.mcp).enabled) $mcpIngress.enabled ) }}
+          {{- if $mcpDirectIngress }}
           {{- range $mcpIngress.paths }}
           {{- include "retool.ingress.mcpPath" (dict "root" $ "path" . "backendInternalPort" $backendInternalPort) | nindent 10 }}
           {{- end }}

charts/retool/values.yaml

@@ -699,9 +699,8 @@ multiplayer:
 #     oauthIntrospectionAuthTokenSecretName: retool-mcp-oauth
 #     oauthIntrospectionAuthTokenSecretKey: token
 #
-# The chart injects the token into both MCP and the backend. The MCP ingress
-# routes below are enabled by default; reproduce their mapping when ingress is
-# managed outside this chart.
+# The chart injects the token into both MCP and the backend. Choose how public
+# MCP requests are routed with mcp.routing.mode below.
 mcp:
   # Set to true to deploy the MCP server.
   enabled: false
@@ -819,7 +818,7 @@ mcp:
     limits:
       memory: "4096Mi"
 
-  # Backend API Service for the OAuth authorization-server metadata route.
+  # Backend API Service for OAuth authorization-server discovery in direct mode.
   backendMetadata:
     service:
       enabled: true
@@ -829,41 +828,56 @@ mcp:
       annotations: {}
       labels: {}
 
-  # Retool 4.0.7 and later support a simplified ingress setup: route all public
-  # paths, including /mcp and /.well-known, to the main Retool Service on port
-  # 3000. The main backend must also be able to relay /mcp to the in-cluster MCP
-  # Service. When mcp.enabled is true, the chart configures the backend with
-  # MCP_SERVICE_INGRESS_DOMAIN=http://<fullname>-mcp:<service.externalPort>.
-  # You can override it through the top-level env, environmentSecrets, or
-  # environmentVariables settings.
-  #
-  # With that setting, omit or disable the MCP-specific ingress or HTTPRoute
-  # rules below and keep only the normal "/" route to <fullname>:3000. Replace
-  # <fullname> with this chart release's full name.
-  #
-  # Retool versions before 4.0.7 require the explicit MCP routes below,
-  # rendered before the main Retool route. External ingress must preserve this
-  # order and target mapping:
-  #   Exact  /.well-known/oauth-authorization-server -> <fullname>-backend-internal:3001
-  #   Exact  /.well-known/oauth-protected-resource    -> <fullname>-mcp:4010
-  #   Prefix /mcp                                     -> <fullname>-mcp:4010
-  #   Prefix /                                         -> <fullname>:3000
+  routing:
+    # direct (default): required before Retool 4.0.7; also works with newer
+    # versions. Public /mcp and OAuth discovery requests use dedicated routes.
+    # backendRelay (Retool 4.0.7+): send those requests through the main Retool
+    # Service on port 3000. Its backend relays /mcp to the MCP Service. When
+    # MCP is enabled, the chart sets MCP_SERVICE_INGRESS_DOMAIN to the in-cluster
+    # MCP Service URL unless you override that backend environment variable.
+    # The chart does not infer the mode from image.tag; custom and PR tags may
+    # not identify a Retool version.
+    #
+    # If you manage public routes outside this chart, configure them yourself:
+    # backendRelay needs a catchall / route to <fullname>:3000 that matches
+    # /mcp and both /.well-known OAuth discovery URLs. For direct, route:
+    #   /.well-known/oauth-authorization-server -> <fullname>-backend-internal:3001
+    #   /.well-known/oauth-protected-resource    -> <fullname>-mcp:4010
+    #   /mcp                                     -> <fullname>-mcp:4010
+    #   / (remaining requests)                   -> <fullname>:3000
+    # With ingress-nginx strict path validation, use ImplementationSpecific on
+    # the /.well-known paths and Prefix on /mcp. Check path matching with your
+    # Ingress controller. For HTTPRoute, use Exact on the /.well-known paths
+    # and PathPrefix on /mcp. Replace <fullname> with this release's full name.
+    #
+    # When upgrading from an older chart, use helm upgrade
+    # --reset-then-reuse-values to load new defaults while keeping saved
+    # overrides. Plain --reuse-values can retain old chart defaults. If your
+    # Helm lacks that flag, use -f with your values file instead.
+    mode: direct
+
   ingress:
-    # Also requires mcp.enabled.
-    enabled: true
+    # null adds these paths to the chart's Ingress only in direct mode when MCP
+    # is enabled. Set false if an external Ingress supplies them. true with
+    # backendRelay is invalid; remove an older true override when switching.
+    # ingress-nginx with strict path validation rejects dots in Exact and
+    # Prefix paths, so the /.well-known paths use ImplementationSpecific.
+    enabled: null
     paths:
       - path: /.well-known/oauth-authorization-server
-        pathType: Exact
+        pathType: ImplementationSpecific
         target: backendInternal
       - path: /.well-known/oauth-protected-resource
-        pathType: Exact
+        pathType: ImplementationSpecific
       - path: /mcp
         pathType: Prefix
 
   # Equivalent Gateway API routes.
   httpRoute:
-    # Also requires mcp.enabled.
-    enabled: true
+    # null adds these rules to the chart's HTTPRoute only in direct mode when
+    # MCP is enabled. Set false if an external HTTPRoute supplies them. true
+    # with backendRelay is invalid; remove an older true override when switching.
+    enabled: null
     rules:
       - path: /.well-known/oauth-authorization-server
         pathType: Exact