feat: publish tool schemas to dataplane - #6348
Conversation
|
No longer needed. The dataplane consumer now treats Mcp-Param-* as transparent gateway metadata: it forwards the client-supplied headers unchanged and leaves validation to the upstream MCP server. It no longer consumes published tool schemas or calls tools/list on the call path. contextforge-org/contextforge-data-plane#109 passes the complete composed conformance run against the stock published control-plane image. |
Signed-off-by: lucarlig <luca.carlig@ibm.com>
Signed-off-by: lucarlig <luca.carlig@ibm.com>
Signed-off-by: lucarlig <luca.carlig@ibm.com>
95c6e3e to
c5d88d8
Compare
|
Reopened: the dataplane design now validates header/body consistency from control-plane-published schemas before forwarding, so this publisher contract is required. This supersedes the earlier closure rationale; the dataplane still does not call tools/list or reconstruct parameter headers. |
msureshkumar88
left a comment
There was a problem hiding this comment.
Thanks for this — the wiring for tenant-safe schema publishing (tool_schemas keyed by user → virtual host → gateway → tool name) looks right, and the diff stays tightly scoped to that goal. Most of the extra lines are just typing scaffolding forced by the new TypedDict fields, no scope creep. No Alembic migration needed here, correctly.
One thing I'd like addressed before merge though:
Blocking: single bad tool schema takes down dataplane publishing for every tenant, not just its owner
In _build_user_data (mcpgateway/services/dataplane_publisher.py):
for tool in visible_tools:
if not isinstance(tool.input_schema, dict):
raise ValueError(f"Tool {tool.id} has a non-object input schema")This runs inside a dict comprehension in get_data_from_db that builds every active user's config in one expression. If this raises for any tool visible to any user, the whole comprehension aborts, get_data_from_db returns None, and the publish cycle is skipped entirely — leaving all tenants on a stale (eventually expired) snapshot until someone finds and fixes the one offending row.
Given input_schema is NOT NULL and both the Pydantic layer and the ORM before_insert/before_update event make it hard (not impossible — legacy rows, bulk imports, bootstrap_db.py can bypass both) to get a non-dict value in there, this is a low-probability but high-blast-radius failure mode: one tenant's bad data silently breaks routing for everyone else.
Suggest scoping the fail-closed behavior to the single tool instead — exclude it from tool_by_id and log a warning with the tool id, rather than raising out of the comprehension. Would also want test_build_user_data_rejects_non_object_tool_schema updated to assert exclusion (and ideally a test proving other users/tools in the same cycle are unaffected) rather than pytest.raises.
Everything else (no docs needed, no unrelated changes, tests passing, CI green) looks good — happy to re-review once that's addressed.
Summary
This supplies the request-scoped schema source required by contextforge-org/contextforge-data-plane#109. The dataplane validates client-supplied
Mcp-Param-*headers before forwarding them unchanged; it does not reconstruct headers or calltools/liston the request path. The existing periodic control-plane discovery/publisher flow remains the schema producer.Related: #6147 and #6256.
Validation
400/ JSON-RPC-32020