[BFA-130] Default MCP public origin from BASE_DOMAIN - #392
arnold-retool wants to merge 2 commits into
Conversation
 [High risk] Changes how the MCP service discovers its public origin in Kubernetes. The PR is not ready to merge because valid route setups can fail rendering and the backend can receive an origin without a scheme. FindingsReviews (1) · Last reviewed commit: "[BFA-130] Default MCP public origin from..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {{- if and $.Values.httpRoute.enabled $.Values.httpRoute.hostnames -}} | ||
| {{- $matched := false -}} | ||
| {{- range $routeHost := $.Values.httpRoute.hostnames -}} | ||
| {{- if or (eq (lower $routeHost) $host) (and (hasPrefix "*." $routeHost) (hasSuffix (trimPrefix "*" (lower $routeHost)) $host)) -}}{{- $matched = true -}}{{- end -}} | ||
| {{- end -}} | ||
| {{- if not $matched -}}{{- fail (printf "%s host %q does not match a chart-managed HTTPRoute hostname (%s)" $source $host (join ", " $.Values.httpRoute.hostnames)) -}}{{- end -}} |
There was a problem hiding this comment.
If both chart-managed Ingress and HTTPRoute are enabled with different hostnames, BASE_DOMAIN must match both. The chart fails to render even when one route serves the public origin. Accept a match on either enabled route.
| {{- if $mcpExternalEnv -}} | ||
| {{- toYaml (list $mcpExternalEnv) }} |
There was a problem hiding this comment.
If MCP_SERVICE_EXTERNAL_URL in mcp.environmentVariables has no scheme, the new check adds https:// to validate it, but this copy sends the unchanged value to the backend. The backend can then produce bad discovery or upload links. Reject the bare hostname or normalize it before copying it.
| {{- end }} | ||
| {{- end }} | ||
| {{- if not (or $mcpOAuthMainDomain $hasOAuthMainDomainEnv) }} | ||
| {{- if not (or $mcpOAuthMainDomain $hasOAuthMainDomainEnv $mcpBaseDomain $hasBaseDomainEnv) }} |
There was a problem hiding this comment.
A secret-backed env.BASE_DOMAIN, or a BASE_DOMAIN set only in mcp.environmentVariables, now passes this check. But the chart cannot derive OAUTH_MAIN_DOMAIN from either value, so it leaves that variable out of the MCP pod. Require an explicit OAuth domain in these cases, or derive it when the server starts.
1 participant
Summary
BFA-130: Use
env.BASE_DOMAINas the MCP public origin fallback in a single-host Retool installation. The chart passes it to the MCP process, derivesOAUTH_MAIN_DOMAINwhen it can resolve the value, and passes trusted-proxy CIDRs when configured. An explicitmcp.config.mcpServiceExternalUrl(ormcp.environmentVariablesentry) is copied to the backend unless the backend has its own explicit value, keeping OAuth discovery, Bearer challenges, resource metadata, and upload links aligned. Existing explicit values are preserved.Render-time validation rejects malformed public origin values and clear mismatches with chart-managed Ingress hosts and HTTPRoute hostnames. Multiple managed hosts, externally managed ingress, and secret-backed values that cannot be resolved during templating remain supported. Tests cover both
backendRelayanddirectrouting modes. The chart version moves to 6.12.4 and both values files remain synchronized.Companion server PR: https://github.com/tryretool/retool_development/pull/86710
This PR is stacked on routing PR #391 (itself stacked on #390) and targets
arnold/mcp-routing-modeso the diff contains only the public-origin change.Migration
For one public host, set
env.BASE_DOMAIN: https://retool.example.comand serve that host via the configured Ingress or HTTPRoute. Removemcp.config.mcpServiceExternalUrlwhen custom Space domains should be advertised per request; the existing explicit override continues to pin every MCP response to one origin. If a proxy rewrites Host or terminates TLS, setenv.MCP_TRUSTED_PROXY_CIDRSto the immediate proxy peer CIDRs. The server PR consumes this contract. With external ingress, disable chart-managed routing and provide the public path mappings documented in the README.Validation
python3 .github/test_mcp_routing.py: 16 render tests passed, including defaults, overrides, mismatches, secret-backed values, custom hosts, and both routing modes.python3 .github/test_mcp_helm_test.py: 14 render and localhost HTTP smoke tests passed.helm lint charts/retool --values charts/retool/ci/test-install-values.yaml --values charts/retool/ci/test-mcp-enabled-option.yaml: passed.helm template ... | kubeconform -strict -ignore-missing-schemas -summary: 18 valid resources, zero invalid, one skipped.git diff --check: passed.No deployment was performed.