Skip to content

OLS-3185: spec changes for operator MCP auto-injection #271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
onmete wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −2
Open

OLS-3185: spec changes for operator MCP auto-injection #271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .ai/spec/what/crd-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ Kubernetes API surface for the agentic operator. **Lifecycle and gates** are in
33. **EscalationResult**: `status.summary`, `status.content`, optional `failureReason`, `sandbox`; no `retryIndex`.
34. **RemediationOption**: Cohesion rules require `diagnosis` and `proposal` to be paired when present; `components` holds schemaless JSON for adapter data shaped by `spec.analysisOutput.schema`.
35. **RBACResult / RBACRule**: Analysis MAY request namespace-scoped and cluster-scoped rules with verb/apigroup/resource metadata and mandatory `justification`; `namespace` on rules MUST align with proposal targeting rules (validated at runtime by policy engine per field comments).
36. **ToolsSpec**: MAY include `skills` (unique images), `mcpServers` (unique names), `requiredSecrets` (unique names). `SkillsSource.image` MUST be a valid pullspec; optional `paths` restrict mounted subtrees.
36. **ToolsSpec**: MAY include `skills` (unique images), `mcpServers` (unique names), `requiredSecrets` (unique names), and `disableDefaultMCP` (bool). `SkillsSource.image` MUST be a valid pullspec; optional `paths` restrict mounted subtrees.
36a. **ToolsSpec — `disableDefaultMCP`**: Optional bool, default `false`. When `true`, the operator MUST NOT auto-inject the default OpenShift MCP server into `LIGHTSPEED_MCP_SERVERS` for sandbox pods using this `ToolsSpec`, even when introspection is enabled on the classic `OLSConfig`. User-defined `mcpServers` entries are unaffected. When `false` or absent and introspection is enabled, the operator prepends the default OpenShift MCP server entry to the MCP server list (see `sandbox-execution.md`).
37. **SecretRequirement**: Names a namespace-local `Secret`; `mountAs` discriminates `EnvVar` vs `FilePath` with required nested config per type.
38. **MCPHeaderValueSource**: Discriminated by `type`; `Secret` requires nested `secret` name reference.
39. **Result CR ownership**: Result CRs MUST declare controller `ownerReferences` to their `Proposal` for GC; naming follows operator conventions (see `sandbox-execution.md` for when they are created).
Expand Down Expand Up @@ -79,7 +80,7 @@ Kubernetes API surface for the agentic operator. **Lifecycle and gates** are in
- `metadata.name`, `metadata.namespace`, `spec.*`, `status.*`

### Shared / embedded types
- `ToolsSpec`: `skills[]`, `mcpServers[]`, `requiredSecrets[]`
- `ToolsSpec`: `skills[]`, `mcpServers[]`, `requiredSecrets[]`, `disableDefaultMCP`
- `SkillsSource`: `image`, `paths[]`
- `SecretRequirement`: `name`, `description`, `mountAs.*`
- `StepResultRef`: `name`, `outcome`
Expand Down
2 changes: 2 additions & 0 deletions .ai/spec/what/sandbox-execution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes
17. **Secrets — MCP headers**: When an MCP header sources a Secret, the template MUST mount that secret on a dedicated read-only path suitable for header injection configuration.
18. **Skills volumes**: Skills MUST be conveyed as OCI image volume(s) on the sandbox pod template; when `SkillsSource.paths` is set, the controller MUST mount each path as a `subPath` under the configured skills mount root using stable mount naming derived from the path’s final segment. When multiple `skills` entries exist in `ToolsSpec`, template derivation MUST apply image/path patching based on the **first** non-empty skills source (current behavior).
19. **MCP servers**: MCP configuration MUST be serialized to an environment variable payload listing servers, URLs, timeouts, and header sources so the agent runtime can open MCP connections without CR-specific code in the agent.
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).
Comment on lines +47 to +48

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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 || true

Repository: 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.md

Repository: 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.go

Repository: 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.

20. **Sandbox observability patch**: Immediately after creating a claim (or Pod in `bare-pod` mode), the controller MUST patch `Proposal.status.steps.<step>.sandbox` with the resource name and operator namespace so consoles/CLIs can tail logs before the sandbox is ready. In `bare-pod` mode, `status.steps.<step>.sandbox.claimName` MUST be set to the Pod name (same field used for SandboxClaim names in `sandbox-claim` mode).
21. **Execution RBAC materialization**: When the approved remediation option includes RBAC requests, before execution the controller MUST create a **per-proposal ServiceAccount** named `ls-exec-{proposal-namespace}-{proposal-name}` in the operator namespace (truncated to 63 chars), then create namespace-scoped `Role`+`RoleBinding` pairs in each target namespace and `ClusterRole`+`ClusterRoleBinding` for cluster-scoped rules, binding subjects to **this per-proposal SA** (not the shared `lightspeed-agent` SA). The per-proposal SA MUST NOT carry an owner reference — cross-namespace owner refs are unsupported by Kubernetes GC (the SA lives in the operator namespace while the Proposal may be in a different namespace). Cleanup is handled explicitly by `cleanupExecutionRBAC` via the Proposal's finalizer. Idempotent create MUST tolerate existing objects (`AlreadyExists` is a no-op). Only execution sandbox pods use this SA — analysis and verification pods continue using `lightspeed-agent`. The operator's `cluster-admin` privilege (external prerequisite) ensures no RBAC escalation issues when creating arbitrary Roles/SAs. See `agentic-security.md` rules 7-15 for full specification.
22. **RBAC subjects namespace**: RoleBindings MUST reference the **per-proposal service account** in the **operator namespace** (where sandbox pods run), even when roles live in target namespaces.
Expand Down