feat(mcp): Simplify deployment of Replication MCP server over HTTPS#1069
feat(mcp): Simplify deployment of Replication MCP server over HTTPS#1069Ella6882 wants to merge 1 commit into
Conversation
|
Note 📝 PR Converted to Draft More info...Thank you for creating this PR. As a policy to protect our engineers' time, Airbyte requires all PRs to be created first in draft status. Your PR has been automatically converted to draft status in respect for this policy. As soon as your PR is ready for formal review, you can proceed to convert the PR to "ready for review" status by clicking the "Ready for review" button at the bottom of the PR page. To skip draft status in future PRs, please include |
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@feat/mcp-https-dust-deployment' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@feat/mcp-https-dust-deployment'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
📝 WalkthroughWalkthroughAdds hosted HTTPS deployment support for the Airbyte Replication MCP server: a new CORS middleware config module wired into the HTTP entry point, Docker Compose/Caddy deployment assets, a new poe task, and documentation covering remote client integration, authentication, and reverse proxy setup. ChangesMCP HTTPS Deployment Support
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Caddy
participant MCPServer as MCP Server (http_main.py)
Client->>Caddy: HTTPS request to MCP_PUBLIC_HOST
Caddy->>MCPServer: Proxy /mcp* or /health to mcp:8080
MCPServer->>MCPServer: build_http_middleware() applies CORS rules
MCPServer-->>Caddy: Response (with CORS headers if configured)
Caddy-->>Client: HTTPS response
Possibly related issues
Possibly related PRs
Nice tidy set of deployment docs and a middleware helper here — wdyt about eventually adding a small integration test that spins up the compose stack to verify the CORS headers actually reach a client through Caddy? 🐰✨ 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@airbyte/mcp/_http_config.py`:
- Line 45: The CORS origin selection in build_http_middleware currently treats
an explicit empty string the same as no value, so cors_origins="" falls back to
MCP_CORS_ORIGINS unexpectedly. Update the logic around parse_cors_origins in
_http_config so only None triggers the environment lookup, while an empty string
is passed through as an intentional opt-out; this will keep
test_build_http_middleware_without_origins and similar callers independent of
env state.
In `@deploy/Caddyfile`:
- Around line 19-22: The fallback in Caddyfile still proxies the original
request URI, so the path-prefix deployment flow won’t reach the root app. Update
the fallback `handle` block to strip or rewrite the configured prefix before
`reverse_proxy mcp:8080`, using the same path-prefix logic expected by
`MCP_SERVER_URL`; if that can’t be done here, remove the path-prefixed
deployment claim from the docs/config comments. Refer to the fallback `handle`
and `reverse_proxy` block when making the change.
In `@deploy/docker-compose.mcp.yml`:
- Around line 21-23: The MCP public origin is currently falling back to
localhost via MCP_SERVER_URL and related public-host settings, which makes the
compose stack unusable for remote access unless overridden manually. Update the
docker-compose MCP configuration to require an explicit MCP_PUBLIC_HOST instead
of defaulting it to localhost, and ensure the MCP_SERVER_URL and any other
public-host dependent values are derived from that required hostname in the
compose file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c4744d8b-7e0e-4509-9691-c0ea5400114a
📒 Files selected for processing (9)
airbyte/mcp/__init__.pyairbyte/mcp/_http_config.pyairbyte/mcp/http_main.pydeploy/Caddyfiledeploy/docker-compose.mcp.ymldocs/CONTRIBUTING.mddocs/MCP_HTTP_DEPLOYMENT.mdpyproject.tomltests/unit_tests/test_mcp_http_config.py
| remote MCP clients that connect from a browser context (for example, | ||
| browser-based OAuth flows). | ||
| """ | ||
| origins = parse_cors_origins(cors_origins or os.getenv(MCP_CORS_ORIGINS_ENV, "")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Falsy "" falls through to env var — intentional?
cors_origins or os.getenv(...) treats an explicitly passed empty string the same as "unset", so it silently falls back to MCP_CORS_ORIGINS from the environment. That also makes test_build_http_middleware_without_origins (passing cors_origins="") implicitly depend on the env var being unset in CI. Want to switch to a None sentinel so callers can explicitly opt out of CORS regardless of env state?
💡 Proposed fix
def build_http_middleware(
*,
cors_origins: str | None = None,
) -> list[Middleware]:
...
- origins = parse_cors_origins(cors_origins or os.getenv(MCP_CORS_ORIGINS_ENV, ""))
+ if cors_origins is None:
+ cors_origins = os.getenv(MCP_CORS_ORIGINS_ENV, "")
+ origins = parse_cors_origins(cors_origins)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| origins = parse_cors_origins(cors_origins or os.getenv(MCP_CORS_ORIGINS_ENV, "")) | |
| if cors_origins is None: | |
| cors_origins = os.getenv(MCP_CORS_ORIGINS_ENV, "") | |
| origins = parse_cors_origins(cors_origins) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@airbyte/mcp/_http_config.py` at line 45, The CORS origin selection in
build_http_middleware currently treats an explicit empty string the same as no
value, so cors_origins="" falls back to MCP_CORS_ORIGINS unexpectedly. Update
the logic around parse_cors_origins in _http_config so only None triggers the
environment lookup, while an empty string is passed through as an intentional
opt-out; this will keep test_build_http_middleware_without_origins and similar
callers independent of env state.
| # Fallback for path-stripped deployments (MCP_SERVER_URL with a path prefix). | ||
| handle { | ||
| reverse_proxy mcp:8080 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Strip the prefix before proxying this fallback.
handle { reverse_proxy mcp:8080 } still forwards the original URI, so the documented MCP_SERVER_URL=https://host/airbyte flow will hit mcp:8080/airbyte instead of the root-mounted app. Could we either rewrite the prefix here or drop the path-prefixed deployment claim until that rewrite exists?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/Caddyfile` around lines 19 - 22, The fallback in Caddyfile still
proxies the original request URI, so the path-prefix deployment flow won’t reach
the root app. Update the fallback `handle` block to strip or rewrite the
configured prefix before `reverse_proxy mcp:8080`, using the same path-prefix
logic expected by `MCP_SERVER_URL`; if that can’t be done here, remove the
path-prefixed deployment claim from the docs/config comments. Refer to the
fallback `handle` and `reverse_proxy` block when making the change.
| MCP_SERVER_URL: https://${MCP_PUBLIC_HOST:-localhost} | ||
| MCP_CORS_ORIGINS: ${MCP_CORS_ORIGINS:-} | ||
| OIDC_CONFIG_URL: ${OIDC_CONFIG_URL:-} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don’t default the public hostname to localhost.
With automatic HTTPS, {$MCP_PUBLIC_HOST} becomes the certificate name and MCP_SERVER_URL becomes the public origin. Defaulting both to localhost leaves the stack in a state that won’t produce a usable remote endpoint unless every user overrides it manually. Could we require an explicit public hostname instead?
Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/docker-compose.mcp.yml` around lines 21 - 23, The MCP public origin is
currently falling back to localhost via MCP_SERVER_URL and related public-host
settings, which makes the compose stack unusable for remote access unless
overridden manually. Update the docker-compose MCP configuration to require an
explicit MCP_PUBLIC_HOST instead of defaulting it to localhost, and ensure the
MCP_SERVER_URL and any other public-host dependent values are derived from that
required hostname in the compose file.
1 participant
Description
Create docs/MCP_HTTP_DEPLOYMENT.md covering:
Add deploy/docker-compose.mcp.yml and a reverse-proxy config (e.g. Caddy) for a reference stack:
Optional CORS support
Add MCP_CORS_ORIGINS (comma-separated origins) and wire it into the HTTP transport for clients that need CORS (e.g. OAuth in the browser). Include MCP and Airbyte credential headers.
Improve discoverability
Unit tests for CORS origin parsing and middleware configuration.
Relates
Testing
and
confirm successful Streamable HTTP MCP handshake:
and
can List tools in MCP Inspector
That shows both the HTTP transport and MCP protocol working.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation