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: 33253fc |
3651fe8 to
7a204f8
Compare
|
@coderabbitai review |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR adds optional MCP Gateway integration to the StackRox MCP Helm chart. New configuration values enable the feature and reference a Kubernetes Gateway resource. Two conditional Helm templates create HTTPRoute and MCPServerRegistration resources with CRD and deployment constraint validation. Documentation explains the feature, configuration parameters, and architectural flow. ChangesMCP Gateway Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
383c953 to
7b05e16
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@charts/stackrox-mcp/templates/mcp-server-registration.yaml`:
- Line 13: The template currently renders toolPrefix unquoted which can produce
invalid or coerced YAML for certain inputs; update the mcp-server-registration
template so the toolPrefix value is emitted as a quoted string by quoting the
.Values.mcpGateway.toolPrefix interpolation (i.e., ensure the rendered line uses
"toolPrefix: \"{{ .Values.mcpGateway.toolPrefix }}\"" style quoting) so
user-provided values are treated as strings and YAML parsing/coercion issues are
avoided.
🪄 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 YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c9c8e2e1-c92f-498e-ac05-cc9a276a1a5d
📒 Files selected for processing (6)
charts/stackrox-mcp/README.mdcharts/stackrox-mcp/templates/_helpers.tplcharts/stackrox-mcp/templates/http-route.yamlcharts/stackrox-mcp/templates/mcp-server-registration.yamlcharts/stackrox-mcp/values.yamldocs/mcp-gateway-integration.md
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
e8cd32f to
33253fc
Compare
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: