Skip to content

Commit 35fc2de

Browse files
jiaenrenclaude
andcommitted
Fold MCP chart validation into the standard render tests
MCP was the only chart area in the repository with a dedicated ci/validate-*.sh. It had grown to 360 lines against a house harness of 78 that covered MCP not at all, and it ran from its own CI step, so MCP chart rendering was checked by a mechanism nothing else in the repository used. Most of what it asserted was not behaviour. Numeric ranges, URL shapes and scope patterns are enforced by MCPAuthConfig at start-up, so re-proving them at render time stated the same contract twice in two languages, and each new bound needed editing in both. Move the behavioural assertions into tests/render-tests.sh: that MCP renders nothing when disabled, that the gateway publishes one /mcp prefix route and rewrites it off, that the health carve-out answers 404 itself rather than leaning on jwt_authn, that Redis is inherited from services.redis, that derived values do not reappear as deployer inputs, that the proxy scales out, and that extraEnv cannot redirect the relay. The roles Lua filter assertions come along because they are its only coverage anywhere in the repository. Each new assertion was negative-tested by reintroducing the defect it guards and confirming the harness fails. The route extractor scopes a route by its own indentation rather than ending at the first nested "- name:". Every route with a headers matcher has one at line five, so the naive version returned only the match block and the prefix_rewrite assertion for mcp-authorization-server-metadata could never see the value it checks. The proxy fixture moves to tests/ alongside the harness that uses it, and ci/ is now empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 22415f1 commit 35fc2de

6 files changed

Lines changed: 87 additions & 278 deletions

File tree

.github/workflows/helm-chart-lint.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,6 @@ jobs:
193193
>/dev/null
194194
fi
195195
196-
- name: Validate MCP-enabled service chart
197-
if: matrix.chart == 'service'
198-
run: bash deployments/charts/service/ci/validate-mcp-chart.sh
199-
200196
- name: Chart-specific render tests
201197
run: |
202198
TEST_SCRIPT="deployments/charts/${{ matrix.chart }}/tests/render-tests.sh"

deployments/charts/service/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ not by itself make the pod unhealthy.
175175
Run the enabled and disabled rendering checks locally with:
176176

177177
```bash
178-
bash deployments/charts/service/ci/validate-mcp-chart.sh
178+
bash deployments/charts/service/tests/render-tests.sh
179179
```
180180

181181
### Backend API Token Settings

deployments/charts/service/ci/validate-mcp-chart.sh

Lines changed: 0 additions & 272 deletions
This file was deleted.

deployments/charts/service/ci/mcp-oidc-proxy-values.yaml renamed to deployments/charts/service/tests/mcp-proxy-values.yaml

File renamed without changes.

deployments/charts/service/tests/render-tests.sh

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,3 +322,88 @@ if grep -q 'osmo.nvidia.com/mek-consumer' <<<"$mek_render"; then
322322
echo 'Product chart rendered the KIND-only MEK consumer label' >&2
323323
exit 1
324324
fi
325+
326+
# --- MCP -------------------------------------------------------------------
327+
# Behaviour only. Numeric ranges and URL shapes are enforced by MCPAuthConfig
328+
# at start-up; re-proving them here just duplicates the same contract in a
329+
# second language.
330+
331+
mcp_values="$CHART_DIR/tests/mcp-proxy-values.yaml"
332+
333+
disabled_render=$(helm template mcp-disabled "$CHART_DIR")
334+
for forbidden in 'name: osmo-mcp' 'cluster: osmo-mcp' 'path: /mcp'; do
335+
if grep -q "$forbidden" <<<"$disabled_render"; then
336+
echo "MCP is disabled but the render still contains: $forbidden" >&2
337+
exit 1
338+
fi
339+
done
340+
341+
mcp_render=$(helm template mcp-test "$CHART_DIR" --values "$mcp_values")
342+
mcp_workload=$(helm template mcp-test "$CHART_DIR" --values "$mcp_values" \
343+
--show-only templates/mcp-service.yaml)
344+
345+
# FastMCP advertises its OAuth endpoints under /mcp; the gateway publishes that
346+
# prefix and rewrites it off before forwarding to the root paths the MCP SDK
347+
# registers. One prefix route, not an entry per endpoint name.
348+
mcp_route() {
349+
# Stop at the next entry *at the route's own indentation*. Matching any
350+
# "- name:" ends the route at its first header matcher, which hides
351+
# everything below it -- including prefix_rewrite.
352+
awk -v route="- name: $1" '
353+
function indent(line) { match(line, /^ */); return RLENGTH }
354+
index($0, route) { found = 1; depth = indent($0); next }
355+
found && indent($0) == depth && /- name: / { exit }
356+
found { print }
357+
' <<<"$mcp_render"
358+
}
359+
360+
grep -q 'prefix: /mcp/' <<<"$(mcp_route mcp-oauth)"
361+
grep -q 'prefix_rewrite: /$' <<<"$(mcp_route mcp-oauth)"
362+
grep -q 'prefix_rewrite: /.well-known/oauth-authorization-server' \
363+
<<<"$(mcp_route mcp-authorization-server-metadata)"
364+
365+
# The /mcp prefix publishes the container's whole root namespace, so the health
366+
# endpoints are carved out ahead of it -- and the carve-out has to answer 404
367+
# itself rather than let jwt_authn answer 401 first.
368+
health_route=$(mcp_route mcp-health-not-public)
369+
grep -q 'status: 404' <<<"$health_route"
370+
grep -q 'envoy.filters.http.jwt_authn:' <<<"$health_route"
371+
grep -q 'envoy.filters.http.ext_authz:' <<<"$health_route"
372+
373+
# The roles Lua filter has no other coverage in the repo.
374+
grep -q 'local safe_roles = {}' <<<"$mcp_render"
375+
grep -q "not string.find(role, '\[,%c\]')" <<<"$mcp_render"
376+
grep -q "table.concat(safe_roles, ',')" <<<"$mcp_render"
377+
378+
# Redis connection details come from services.redis; only the database is local.
379+
grep -q 'value: "rediss://redis:6379/14"' <<<"$mcp_workload"
380+
381+
# Values the deployment derives must not reappear as deployer inputs.
382+
for derived in OSMO_MCP_AUTH_ISSUER_URL OSMO_MCP_AUTH_SCOPE \
383+
OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_AUDIENCE \
384+
OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_JWKS_URL; do
385+
if grep -q "name: $derived" <<<"$mcp_workload"; then
386+
echo "MCP still asks for a derived value: $derived" >&2
387+
exit 1
388+
fi
389+
done
390+
391+
# The proxy keeps its state in Redis, so scaling out must render.
392+
helm template mcp-scale "$CHART_DIR" --values "$mcp_values" \
393+
--set 'services.mcp.replicas=2' >/dev/null
394+
395+
# A deployer must not be able to redirect the relay through extraEnv.
396+
if helm template mcp-override "$CHART_DIR" --values "$mcp_values" \
397+
--set-json 'services.mcp.extraEnv=[{"name":"OSMO_GATEWAY_URL","value":"https://evil.example.com"}]' \
398+
>/dev/null 2>&1; then
399+
echo 'MCP accepted an extraEnv override of a managed variable' >&2
400+
exit 1
401+
fi
402+
403+
# resourceUrl is the value everything else is derived from.
404+
if helm template mcp-bad-url "$CHART_DIR" --values "$mcp_values" \
405+
--set 'services.mcp.resourceUrl=https://osmo.example.com%40evil.example.com/mcp' \
406+
>/dev/null 2>&1; then
407+
echo 'MCP accepted a malformed resourceUrl' >&2
408+
exit 1
409+
fi

src/service/mcp/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ bazel build \
400400
--platforms=//bzl/platforms:linux_x86_64 \
401401
//src/service/mcp:mcp_image_x86_64
402402
bazel test //test/smoke:mcp-checks-pylint
403-
bash deployments/charts/service/ci/validate-mcp-chart.sh
403+
bash deployments/charts/service/tests/render-tests.sh
404404
```
405405

406406
The chart validation covers MCP-disabled, direct-provider, and in-process OIDC

0 commit comments

Comments
 (0)