ROX-33010: Add MCP Gateway integraton to Helm chart#118
Conversation
❌ 2 Tests Failed:
View the full list of 2 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The CRD capability checks use only the group/version string (e.g.
gateway.networking.k8s.io/v1,mcp.kagenti.com/v1alpha1), but.Capabilities.APIVersions.Hastypically expects full GVK-like identifiers (e.g.gateway.networking.k8s.io/v1/HTTPRouteandmcp.kagenti.com/v1alpha1/MCPServerRegistration), so you may want to update these keys to avoid false negatives on clusters where the CRDs are installed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CRD capability checks use only the group/version string (e.g. `gateway.networking.k8s.io/v1`, `mcp.kagenti.com/v1alpha1`), but `.Capabilities.APIVersions.Has` typically expects full GVK-like identifiers (e.g. `gateway.networking.k8s.io/v1/HTTPRoute` and `mcp.kagenti.com/v1alpha1/MCPServerRegistration`), so you may want to update these keys to avoid false negatives on clusters where the CRDs are installed.
## Individual Comments
### Comment 1
<location path="charts/stackrox-mcp/templates/mcp-server-registration.yaml" line_range="2-5" />
<code_context>
+{{- if .Values.mcpGateway.enabled }}
+{{- if not (.Capabilities.APIVersions.Has "mcp.kagenti.com/v1alpha1") }}
+{{- fail "MCP Gateway integration requires the MCPServerRegistration CRD (mcp.kagenti.com/v1alpha1). Install MCP Gateway (https://github.com/Kuadrant/mcp-gateway) before enabling mcpGateway." }}
+{{- end }}
+apiVersion: mcp.kagenti.com/v1alpha1
+kind: MCPServerRegistration
+metadata:
</code_context>
<issue_to_address>
**issue (bug_risk):** Double-check the MCPServerRegistration API group and version string
Both the capability check and `apiVersion` use `mcp.kagenti.com/v1alpha1`, but the upstream `github.com/Kuadrant/mcp-gateway` CRDs may use a different group (e.g. `mcp.kuadrant.io/v1alpha1`). If this doesn’t exactly match the installed CRD, the Helm capability check will fail or the API server will reject the resources. Please confirm the precise group/version from the installed CRD and update both the capabilities check and `apiVersion` accordingly.
</issue_to_address>
### Comment 2
<location path="charts/stackrox-mcp/README.md" line_range="155" />
<code_context>
+| `mcpGateway.hostname` | Hostname for the HTTPRoute | `""` |
+| `mcpGateway.toolPrefix` | Prefix for tools exposed via the gateway | `stackrox_` |
+
+**Note:** Requires [MCP Gateway](https://github.com/Kuadrant/mcp-gateway) to be installed on the cluster. The chart validates that the required CRDs (`gateway.networking.k8s.io/v1` and `mcp.kagenti.com/v1alpha1`) are available and fails with a descriptive error if they are not. See [MCP Gateway Integration](../../docs/mcp-gateway-integration.md) for details.
+
### Scheduling
</code_context>
<issue_to_address>
**issue (bug_risk):** The CRD group `mcp.kagenti.com/v1alpha1` appears to be misspelled or incorrect.
Since this integrates with `Kuadrant/mcp-gateway`, please confirm the correct API group for `MCPServerRegistration` (the `kagenti.com` domain looks wrong) and update this reference so the chart validates against the actual CRD group users should install.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
E2E Test ResultsCommit: 8ec36c9 |
| rules: | ||
| - matches: | ||
| - path: | ||
| type: PathPrefix |
There was a problem hiding this comment.
I'm not sure if I fully understand your question.
Matches are required for HTTPRoute. Here is an example: https://docs.kuadrant.io/1.4.x/kuadrant-backstage-plugin/docs/getting-started/#step-2-create-the-gateway-and-httproute
Maybe we can change to /. But having only /mcp is also fine.
Description
Add opt-in MCP Gateway integration to the Helm chart, enabling StackRox MCP server registration with MCP Gateway for tool aggregation behind a centralized gateway endpoint.
When enabled (
mcpGateway.enabled: true), the chart creates:/mcptraffic from the gateway to the StackRox MCP servicestackrox_by default)The chart validates that the required CRDs (
gateway.networking.k8s.io/v1andmcp.kagenti.com/v1alpha1) are available on the cluster and fails with a descriptive error if they are not.Files added:
charts/stackrox-mcp/templates/http-route.yamlcharts/stackrox-mcp/templates/mcp-server-registration.yamldocs/mcp-gateway-integration.mdFiles modified:
charts/stackrox-mcp/values.yaml— newmcpGatewaysectioncharts/stackrox-mcp/README.md— MCP Gateway configuration tableValidation
mcpGateway.enabled: false(default)helm lintpassesAI-assisted development prompts
This PR was developed with AI assistance. Below are the prompts used: