proposal for must-gather agentic debugging via MCP - #2064
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe proposal defines opt-in agentic must-gather analysis through Lightspeed and a shared MCP server. It covers API validation, serialized PVC processing, deployment constraints, adapter behavior, lifecycle rules, testing, and operations. ChangesAgentic debugging proposal
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant MustGather
participant MCPServer
participant Proposal
participant Lightspeed
participant AnalysisResult
MustGather->>MCPServer: Serve gathered data
MustGather->>Proposal: Create analysis proposal
Proposal->>Lightspeed: Request High-priority analysis
Lightspeed->>MCPServer: Query must-gather toolset
Lightspeed->>AnalysisResult: Produce analysis result
Suggested labels: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md (1)
125-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to fenced blocks.
The CEL block and the argument block at Lines 188-191 omit fence languages, triggering markdownlint MD040. Use identifiers such as
celandyaml(or the repository-approved equivalents).🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 125 - 127, Add language identifiers to the fenced CEL and argument blocks near the referenced rule, using the repository-approved identifiers such as cel and yaml, while leaving the block contents unchanged.Source: Linters/SAST tools
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Line 92: Update the MCP connection instructions to construct the service
hostname using the actual operator namespace from r.OperatorNamespace instead of
hardcoding must-gather-operator. Revise the surrounding operational examples so
the default namespace is presented only as an example, not a requirement.
- Around line 98-103: Update the MCP setup and Proposal creation flow described
in the Error Cases section so a Proposal is not created when MCP server creation
fails. Create the Proposal only after the MCP Deployment/Service is confirmed
ready, or preserve the setup failure for retry while skipping Proposal creation.
- Around line 207-211: Enforce the TechPreview feature gate in the reconcile
path before any MCP server or Proposal processing when agenticDebuggingEnabled
is true, using the existing gate mechanism and ignoring the field when the gate
is disabled; otherwise remove the TechPreview gating claim from the graduation
documentation.
- Around line 175-184: The MCP_SERVER_IMAGE configuration must not use an
unverified or mutable :latest default. Update the documented default in the MCP
server image configuration and the corresponding deployment guidance to use a
verified immutable published tag or digest; if none is available, require
MCP_SERVER_IMAGE to be explicitly provided.
- Around line 197-205: The shared MCP server design must enforce its single-PVC
limitation instead of treating Deployment AlreadyExists as sufficient. Update
ensureMCPServer() and the MustGather reconciliation flow to reject any CR
referencing a PVC different from the one already mounted, or partition server
instances so each PVC gets its own MCP Deployment; ensure Proposal path
resolution always uses the correct PVC.
- Around line 233-235: Align the primary flow’s stated IntelliAide pipeline
scope with the Proposal produced by createIntelliAideProposal(): either update
the generated instructions to run analyze_data.py and perform_rca.py for high,
medium, and low priorities, or explicitly limit the proposal’s goals, tests, and
graduation criteria to High-priority analysis only. Ensure all scope claims
consistently match the selected behavior.
- Around line 88-94: Update the documented MCP flow around ensureMCPServer(),
resolveMustGatherDataPath(), and createIntelliAideProposal() to enforce
collection isolation at the server boundary rather than relying on the prompt or
mustgather_use() path. Specify a per-request collection root, per-collection
credentials, or an isolated MCP instance so agents cannot access other PVC
collections and concurrent analyses remain separated; apply the same correction
to the corresponding section referenced by the additional lines.
---
Nitpick comments:
In `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 125-127: Add language identifiers to the fenced CEL and argument
blocks near the referenced rule, using the repository-approved identifiers such
as cel and yaml, while leaving the block contents unchanged.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f3e8a32-63d7-4f8b-bfbf-ac8d0aa44f8d
📒 Files selected for processing (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md
|
[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 |
d61f8c7 to
628e9e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 218-221: The stale Proposal handling around ensureMCPDeployment()
must not allow a PVC swap based solely on analysis.timeoutMinutes and
grace-period age. Before swapping, cancel or terminate the existing analysis and
confirm it has completed; alternatively, isolate each analysis with its own MCP
instance, while preserving the errMCPServerBusy retry and cleanup deferral until
the shared server is safe to reuse.
- Around line 215-221: Make the PVC ownership transition in ensureMCPDeployment
atomic across concurrent reconcilers by using a Kubernetes Lease or
resource-version compare-and-swap on the Deployment annotation. Ensure only the
reconciler that successfully claims the ownership update rewrites the volume and
creates its Proposal, while losing reconcilers return errMCPServerBusy and
preserve their deferred cleanup/requeue behavior. Add tests covering concurrent
reconciles and verifying that only the successful owner creates a Proposal.
- Around line 203-204: Update the shared MCP server deployment documentation to
require a dedicated ServiceAccount rather than reusing the must-gather-operator
ServiceAccount. Configure that account with automountServiceAccountToken: false,
while retaining PVC-only access and the disabled cluster-provider behavior;
mention narrowly scoped permissions only if future API access is introduced.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 82c12159-ee07-4fe2-966f-e6e99de3bc41
📒 Files selected for processing (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md
628e9e1 to
fa9cc72
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md (1)
197-203: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRestrict access to the MCP HTTP service.
--cluster-provider disabledprevents live API access, not unauthorized HTTP callers. The design specifies neither authentication nor a NetworkPolicy, so any pod able to reach the Service could query sensitive MustGather data. Require an authenticated, allowlisted connection from the Lightspeed sandbox.🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 197 - 203, Update the Shared MCP Server Design to require authenticated access to the MCP HTTP Service, restricted through an allowlist or NetworkPolicy to only the Lightspeed sandbox. Specify the authentication mechanism and required credentials/configuration, and ensure unauthenticated or non-allowlisted callers cannot query mounted MustGather data.
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 205-209: Enforce the operator-namespace requirement before
creating the MCP resources in the agenticDebuggingEnabled flow. Reject or
conditionally disable agentic processing when the MustGather CR namespace
differs from the operator namespace, while preserving normal gathering behavior
as appropriate. Add a negative test covering the namespace mismatch and
confirming no MCP Deployment or Proposal flow is created.
- Around line 309-320: Expand the Unit and Integration tests sections to
explicitly cover gate-disabled behavior, namespace mismatch, MCP readiness
before Proposal creation, atomic concurrent ownership, an active stale agent
during PVC swap, and unauthorized MCP access. Associate each case with the
relevant symbols or flows such as MustGather reconciliation, MCP readiness
checks, proposal ownership, PVC-swap guards, and MCP authorization, while
preserving the existing test coverage.
---
Outside diff comments:
In `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 197-203: Update the Shared MCP Server Design to require
authenticated access to the MCP HTTP Service, restricted through an allowlist or
NetworkPolicy to only the Lightspeed sandbox. Specify the authentication
mechanism and required credentials/configuration, and ensure unauthenticated or
non-allowlisted callers cannot query mounted MustGather data.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b5281a2f-ce22-4e38-826f-da636b382e07
📒 Files selected for processing (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md
fa9cc72 to
a724236
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (7)
enhancements/support-log-gather/must-gather-agentic-debugging.md (7)
92-92: 🎯 Functional Correctness | 🟠 MajorBuild the MCP URL from the actual operator namespace.
Line 92 and the support commands use
must-gather-operatoras a fixed namespace. Build the Service DNS name fromr.OperatorNamespaceand use the same value in operational commands. Otherwise, installations in another namespace create an unreachable Proposal.Also applies to: 272-272, 392-393
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` at line 92, Replace the hard-coded must-gather-operator namespace in the MCP URL and the referenced support commands with the actual operator namespace from r.OperatorNamespace, preserving the existing Service DNS and command structure so installations in any namespace produce reachable proposals.
211-212: 🔒 Security & Privacy | 🟠 MajorSecurity Misconfiguration (CWE-250)
Reachability: External
Use a dedicated ServiceAccount for the MCP server.
The HTTP-facing third-party image runs as the operator ServiceAccount. Unless token automount is disabled, the pod receives operator credentials.
--cluster-provider disableddoes not remove or scope that token. Use a dedicated ServiceAccount withautomountServiceAccountToken: falseand no RBAC, or document the exact equivalent.🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 211 - 212, Update the shared MCP server deployment documentation to use a dedicated ServiceAccount with automountServiceAccountToken disabled and no RBAC permissions, rather than the must-gather-operator ServiceAccount. Ensure the documented configuration applies this account to the MCP server while keeping spec.serviceAccountName scoped to the gather Job.
215-217: 🎯 Functional Correctness | 🟠 MajorEnforce the operator-namespace precondition before MCP creation.
The document says that a CR in another namespace can gather successfully and then fail when the MCP Deployment mounts the PVC. Check the namespace before creating MCP or Proposal resources. Preserve normal gathering behavior as appropriate, and add the negative test.
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 215 - 217, Update the agentic-debugging reconciliation flow to validate that the MustGather CR namespace matches the operator namespace before creating MCP or Proposal resources. Return or skip those agentic resources on mismatch while preserving normal gathering behavior, and add a negative test covering a CR created outside the operator namespace.
207-209: 🔒 Security & Privacy | 🟠 MajorAuthorization Bypass (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: External
Enforce collection isolation at the MCP boundary.
The server mounts the entire PVC, and the agent selects
/data/{subPath}/{podName}through the prompt. The prompt is not an authorization boundary. An agent or reachable client can select another collection on the same PVC. Add server-side root scoping, per-collection credentials, or an isolated MCP instance before claiming independent collection isolation.Also applies to: 40-40, 47-47, 93-93
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 207 - 209, Update the shared must-gather MCP serving design to enforce collection isolation at the server boundary rather than relying on the Proposal prompt path. Implement server-side root scoping, per-collection credentials, or isolated MCP instances so clients cannot access other collections on the PVC, and revise the affected isolation claims to reflect the enforced mechanism.
221-226: 🗄️ Data Integrity & Integration | 🟠 MajorMake shared-server ownership transitions atomic.
AlreadyExistshandling makes Deployment creation idempotent only. Two reconcilers can both observe a free server, update the owner and volume, and create Proposals. Use a Lease or resource-version compare-and-swap. Only the successful owner may rewrite the volume and create its Proposal.Also applies to: 267-268
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 221 - 226, Make the shared MCP server ownership transition atomic in ensureMCPDeployment and the related Proposal-creation flow: use a Lease or resource-version compare-and-swap so concurrent reconcilers cannot both claim the Deployment, rewrite its PVC, and create Proposals. Ensure only the reconciler that successfully acquires ownership performs the volume/owner annotation update and Proposal creation, while preserving errMCPServerBusy for an active unAnalyzed owner.
88-101: 🩺 Stability & Availability | 🟠 MajorCreate the Proposal only after MCP readiness.
The error case permits Proposal creation after MCP setup fails. The primary flow also confirms resource creation, not Deployment readiness or MCP endpoint readiness. Create the Proposal only after the MCP server is ready and reachable. Otherwise, retain the MustGather for retry without creating a doomed Proposal.
Also applies to: 382-384
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 88 - 101, The documented flow must gate createIntelliAideProposal() on confirmed MCP Deployment readiness and reachable MCP endpoint availability, not merely ensureMCPServer() resource creation. If readiness or reachability fails, retain the MustGather for retry and skip Proposal creation; update the related error-case and corresponding flow text consistently.
209-209: 🗄️ Data Integrity & Integration | 🟠 MajorEnforce the single-PVC scope.
The proposal declares one shared Deployment per operator namespace, but it does not define a check that rejects a CR referencing a different PVC. A second CR can then use the wrong mounted claim or wait on an unrelated analysis. Reject mismatched PVC names or create one MCP instance per PVC.
Also applies to: 215-217, 308-308
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` at line 209, Update the shared Deployment and “PVC-swap guard” design to enforce the single-PVC scope: validate each MustGather CR’s referenced PVC against the configured/shared claim and reject mismatches before analysis or serving begins. Ensure the same constraint is stated in the related deployment and concurrency sections, rather than allowing a CR to use an unrelated mounted claim.
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Line 16: Update the document metadata and corresponding “Update” entry from
the future date 2026-08-06 to the actual update date, ensuring no future-dated
document updates remain.
- Around line 231-234: Update the agentic debugging workflow and graduation
criteria to require checking the TechPreview feature gate before processing
agenticDebuggingEnabled: true; when disabled, skip all MCP and Proposal
processing. Add a negative test covering the disabled-gate path and ensure the
documentation uses mandatory behavior rather than future-tense wording.
- Around line 223-229: The PVC-swap guard must not treat Proposal age as proof
that analysis has stopped. Update ensureMCPDeployment and the errMCPServerBusy
flow so a non-terminal prior Proposal cannot be swapped solely because its
timeout plus grace period elapsed; cancel or terminate the existing analysis and
confirm its terminal Analyzed condition before swapping, or isolate analyses
with separate PVC/server resources.
- Line 184: The documentation must define MCP_SERVER_IMAGE as an immutable
contract: specify one verified, pullable Red Hat catalog tag or digest as the
operator default, or explicitly require the variable and fail configuration when
it is unset. Update the related sections describing the operator Deployment and
startup behavior consistently, and remove any ambiguity around rolling :latest
or unverified openshift/mustgather images.
- Line 25: Align the documented pipeline scope with the generated Lightspeed
Proposal around the Proposal definition and referenced summary, goals, workflow,
and E2E test sections. Either configure the Proposal to run the High-, Medium-,
and Low-priority IntelliAide passes, or revise all claims and graduation
criteria to describe High-only analysis; keep the documentation and test
expectations consistent with the selected scope.
- Line 50: Extend Goal 6 to require idempotent createIntelliAideProposal()
behavior: use a deterministic Proposal name or ownership marker, detect an
existing Proposal, and handle AlreadyExists by reusing it rather than creating a
duplicate or starting duplicate analysis.
---
Duplicate comments:
In `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Line 92: Replace the hard-coded must-gather-operator namespace in the MCP URL
and the referenced support commands with the actual operator namespace from
r.OperatorNamespace, preserving the existing Service DNS and command structure
so installations in any namespace produce reachable proposals.
- Around line 211-212: Update the shared MCP server deployment documentation to
use a dedicated ServiceAccount with automountServiceAccountToken disabled and no
RBAC permissions, rather than the must-gather-operator ServiceAccount. Ensure
the documented configuration applies this account to the MCP server while
keeping spec.serviceAccountName scoped to the gather Job.
- Around line 215-217: Update the agentic-debugging reconciliation flow to
validate that the MustGather CR namespace matches the operator namespace before
creating MCP or Proposal resources. Return or skip those agentic resources on
mismatch while preserving normal gathering behavior, and add a negative test
covering a CR created outside the operator namespace.
- Around line 207-209: Update the shared must-gather MCP serving design to
enforce collection isolation at the server boundary rather than relying on the
Proposal prompt path. Implement server-side root scoping, per-collection
credentials, or isolated MCP instances so clients cannot access other
collections on the PVC, and revise the affected isolation claims to reflect the
enforced mechanism.
- Around line 221-226: Make the shared MCP server ownership transition atomic in
ensureMCPDeployment and the related Proposal-creation flow: use a Lease or
resource-version compare-and-swap so concurrent reconcilers cannot both claim
the Deployment, rewrite its PVC, and create Proposals. Ensure only the
reconciler that successfully acquires ownership performs the volume/owner
annotation update and Proposal creation, while preserving errMCPServerBusy for
an active unAnalyzed owner.
- Around line 88-101: The documented flow must gate createIntelliAideProposal()
on confirmed MCP Deployment readiness and reachable MCP endpoint availability,
not merely ensureMCPServer() resource creation. If readiness or reachability
fails, retain the MustGather for retry and skip Proposal creation; update the
related error-case and corresponding flow text consistently.
- Line 209: Update the shared Deployment and “PVC-swap guard” design to enforce
the single-PVC scope: validate each MustGather CR’s referenced PVC against the
configured/shared claim and reject mismatches before analysis or serving begins.
Ensure the same constraint is stated in the related deployment and concurrency
sections, rather than allowing a CR to use an unrelated mounted claim.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 71c9d7ae-2f82-4552-b652-2a26f7155080
📒 Files selected for processing (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md
a724236 to
63a6302
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md (1)
92-93: 🎯 Functional Correctness | 🟠 MajorBuild the MCP URL from the actual operator namespace.
The workflow and support commands hardcode
must-gather-operator, while the risk table requiresr.OperatorNamespace. ConstructmcpServerURLfrom the configured operator namespace and use a placeholder in operational examples. The current URL fails when the operator is installed elsewhere.Also applies to: 273-273, 395-396
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md` around lines 92 - 93, Replace hardcoded must-gather-operator namespace values in the agent workflow, support commands, and operational examples with the configured r.OperatorNamespace when constructing mcpServerURL. Use a namespace placeholder in documentation examples, while preserving the existing MCP endpoint and path structure.
🤖 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 `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 88-90: Update the flow around ensureMCPServer() and
createIntelliAideProposal() to wait until the MCP Deployment is ready and the
Service has an endpoint before creating the Proposal. If MCP setup or readiness
fails, do not create the Proposal; instead requeue the MustGather and propagate
the failure through the existing error path.
- Around line 93-94: Update the must-gather MCP design around mustgather_use and
the IntelliAide pass to enforce collection isolation server-side rather than
relying on the agent-selected path. Scope each MCP instance or credential to its
assigned collection root, and add coverage verifying requests for another
collection are denied.
- Line 211: Update the MCP server design to use a dedicated ServiceAccount
instead of the must-gather-operator ServiceAccount. Configure the dedicated
account with automountServiceAccountToken: false, and state that no RBAC
permissions are added unless future MCP requirements need them; remove the open
design question about reusing the operator account.
- Around line 88-93: Update the must-gather MCP deployment documentation around
ensureMCPServer() and createIntelliAideProposal() to restrict Service ingress to
the Lightspeed sandbox through an explicit NetworkPolicy allow-path, and define
MCP authentication for cases network policy cannot enforce client identity.
Document the expected rejection behavior and add coverage for unauthorized MCP
clients and invalid authentication tokens when describing the flow.
---
Duplicate comments:
In `@enhancements/support-log-gather/must-gather-agentic-debugging.md`:
- Around line 92-93: Replace hardcoded must-gather-operator namespace values in
the agent workflow, support commands, and operational examples with the
configured r.OperatorNamespace when constructing mcpServerURL. Use a namespace
placeholder in documentation examples, while preserving the existing MCP
endpoint and path structure.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bae62517-ef4e-468d-9f20-dc5bbd17626b
📒 Files selected for processing (1)
enhancements/support-log-gather/must-gather-agentic-debugging.md
63a6302 to
4f188d2
Compare
| CRD validation rule (CEL): | ||
|
|
||
| ``` | ||
| rule: "!(has(self.agenticDebuggingEnabled) && self.agenticDebuggingEnabled && !has(self.storage))" |
There was a problem hiding this comment.
hmm..adding CEL rules will be a problem later (when we want to remove or support multiple storage interfaces) if we make the API GA.
|
let's also add what the future design will be - what the first phase is and what the second phase will be. |
| ## Open Questions | ||
|
|
||
| 1. **Multi-PVC concurrent serving**: v1 uses a single shared MCP server with serialized PVC access and a PVC-swap guard (see "Idempotency and Concurrency"). Should a future release invest in true concurrent multi-PVC serving (e.g. one MCP Deployment per PVC, per Alternative 1), or is serialization sufficient given expected overlap of `agenticDebuggingEnabled: true` completions? | ||
| 2. **Feature gate mechanism**: Should the TechPreview gate be an environment variable on the operator Deployment, or should it integrate with the cluster-level `FeatureGate` resource? |
There was a problem hiding this comment.
yes we should feature gate it for sure. especially since this involves api changes which might be volatile. just like regular featuregates i guess we can put the new apis behind a gate?
4f188d2 to
2a58e48
Compare
2a58e48 to
a675689
Compare
2 participants
Summary
Enhancement proposal for must-gather agentic debugging adapter with MCP server and IntelliAide integration.
What this proposes
When a user sets
agenticDebuggingEnabled: trueon a MustGather CR, the operator - after a successful gather Job - will:openshift-mcp-serverimage with only theopenshift/mustgathertoolset enabled) that mounts the same PVCThe agent never requires cluster-admin privileges - it accesses must-gather data exclusively through MCP tool calls over HTTP.
Key design decisions
registry.redhat.io/openshift-mcp-beta/openshift-mcp-server-rhel9agenticDebuggingEnableddefaults tofalse; feature gated for initial releasesComponents involved
openshift/must-gather-operator- new fields, MCP server management, Proposal creationopenshift/openshift-mcp-server- consumed as container image (no code changes)openshift/lightspeed-agentic-sandbox- consumed via existing Proposal/AnalysisResult APIs (no code changes needed from us)Summary by CodeRabbit