OLS-3185: spec changes for operator MCP auto-injection#271
Conversation
Add disableDefaultMCP field to ToolsSpec CRD spec. Add rules for auto-injecting the default OpenShift MCP server into sandbox pods when OLSConfig.introspectionEnabled is true, and cross-operator RBAC to read OLSConfig. Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@onmete: This pull request references OLS-3185 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature request to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughDocumentation-only changes describing a new optional ChangesdisableDefaultMCP Documentation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 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 @.ai/spec/what/sandbox-execution.md:
- Around line 47-48: The default OpenShift MCP injection behavior in the sandbox
execution spec needs an explicit collision rule for the reserved name openshift.
Update the MCP server handling described here so the logic in the ToolsSpec
serialization path clearly defines whether an injected default entry is
rejected, overrides a user entry, or is skipped when MCPServerConfig.Name
already equals openshift. Reference the default injection flow and
LIGHTSPEED_MCP_SERVERS assembly so the precedence is unambiguous.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0846a326-8411-4afe-a5fa-fbf75f8fac29
📒 Files selected for processing (2)
.ai/spec/what/crd-api.md.ai/spec/what/sandbox-execution.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
| 19a. **Default MCP server auto-injection**: When the classic `OLSConfig` CR has `spec.olsConfig.introspectionEnabled` set to `true` (or absent, since it defaults to `true`) AND the effective `ToolsSpec` for the step does NOT have `disableDefaultMCP: true`, the operator MUST prepend the default OpenShift MCP server entry to the MCP server list before serializing to `LIGHTSPEED_MCP_SERVERS`. The default entry MUST use: name `openshift`, URL pointing to the `Service` created by the classic `lightspeed-operator` for the ocp-mcp sidecar (e.g., `http://openshift-mcp-server.<namespace>.svc:8080/mcp`), and an `Authorization` header with source `ServiceAccountToken`. When `introspectionEnabled` is `false` or `disableDefaultMCP` is `true`, the default entry MUST NOT be injected. User-defined `mcpServers` from the `ToolsSpec` are always included regardless of this flag. | ||
| 19b. **OLSConfig cross-operator read**: The agentic operator MUST have RBAC to `get` and `watch` `OLSConfig` resources (API group `ols.openshift.io`, version `v1alpha1`) to read `introspectionEnabled`. When no `OLSConfig` CR exists, the operator MUST behave as if `introspectionEnabled` is `true` (matching the classic operator's default). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --glob '.ai/spec/**' --glob 'api/**' --glob '*.md' 'openshift'Repository: openshift/lightspeed-agentic-operator
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- matches for relevant terms ---'
rg -n --glob '.ai/spec/**' --glob 'api/**' --glob '*.md' \
'LIGHTSPEED_MCP_SERVERS|disableDefaultMCP|mcpServers|introspectionEnabled|openshift-mcp-server|ServiceAccountToken|OLSConfig' \
.
echo
echo '--- spec files mentioning openshift ---'
rg -n --glob '.ai/spec/**' 'openshift' .ai/spec || true
echo
echo '--- outline of the target spec file ---'
ast-grep outline .ai/spec/what/sandbox-execution.md --view expanded || trueRepository: openshift/lightspeed-agentic-operator
Length of output: 11546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- targeted lines around the injected MCP entry ---'
sed -n '35,70p' .ai/spec/what/sandbox-execution.mdRepository: openshift/lightspeed-agentic-operator
Length of output: 9701
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' api/v1alpha1/shared_types.go
echo
sed -n '110,170p' api/v1alpha1/tools_types.goRepository: openshift/lightspeed-agentic-operator
Length of output: 11094
Define the openshift MCP server collision rule. MCPServerConfig.Name allows openshift, and ToolsSpec.mcpServers only enforces uniqueness within the user list, so this needs an explicit precedence rule here: reject, override, or skip the injected entry. Otherwise LIGHTSPEED_MCP_SERVERS is ambiguous.
🤖 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 @.ai/spec/what/sandbox-execution.md around lines 47 - 48, The default
OpenShift MCP injection behavior in the sandbox execution spec needs an explicit
collision rule for the reserved name openshift. Update the MCP server handling
described here so the logic in the ToolsSpec serialization path clearly defines
whether an injected default entry is rejected, overrides a user entry, or is
skipped when MCPServerConfig.Name already equals openshift. Reference the
default injection flow and LIGHTSPEED_MCP_SERVERS assembly so the precedence is
unambiguous.
2 participants
Summary
disableDefaultMCPbool field toToolsSpecincrd-api.md(rule 36a)sandbox-execution.md(rule 19a): whenOLSConfig.introspectionEnabledis true anddisableDefaultMCPis not set, prepend ocp-mcp server entry toLIGHTSPEED_MCP_SERVERSget/watchonOLSConfig(ols.openshift.io/v1alpha1)Design decisions
From OLS-3185 design interview:
disableDefaultMCPon ToolsSpec provides per-Proposal opt-outTest plan
Made with Cursor