ACM-33066 Trust OCM CA bundle for PlacementDebugServer proxy#6090
ACM-33066 Trust OCM CA bundle for PlacementDebugServer proxy#6090Randy424 wants to merge 2 commits intostolostron:mainfrom
Conversation
Adds a dedicated HTTPS agent that trusts the OCM-managed CA bundle when proxying to the PlacementDebugServer. The CA bundle path is read from PLACEMENT_CA_BUNDLE_PATH with file-watch rotation support. Falls back to the existing service-ca agent when unset. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Randy424 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds reading/caching of a placement-debug CA bundle, a placement-debug HTTPS agent factory that uses that CA (with cache invalidation on file-change outside development), and switches the placement-debug route to use the new agent. Tests for CA loading, caching, watching, agent creation, caching, invalidation, and fallback were added. ChangesPlacement-Debug Agent Integration
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(200,200,255,0.5)
Participant Client
end
rect rgba(200,255,200,0.5)
Participant Server as PlacementDebugRoute
Participant AgentFactory as getPlacementDebugAgent
Participant FS as FileSystem
Participant Upstream
end
Client->>Server: HTTP request
Server->>AgentFactory: getPlacementDebugAgent()
AgentFactory->>FS: getPlacementDebugCACertificate() (cached or read file)
FS-->>AgentFactory: CA bundle or undefined
AgentFactory-->>Server: https.Agent (placement-debug CA or service agent)
Server->>Upstream: HTTPS request using Agent
Upstream-->>Server: response
Server-->>Client: proxied response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@backend/src/lib/serviceAccountToken.ts`:
- Around line 118-120: The catch block in serviceAccountToken.ts currently
swallows the thrown error; update the catch to capture the error (e.g., catch
(err)) and include the error details in the warning log call to logger.warn
(preserve the existing msg and path fields but add the error or
err.message/stack as an additional field), then keep the fallback return
undefined; this change targets the catch around reading PLACEMENT_CA_BUNDLE_PATH
in the serviceAccountToken logic so you can locate it by the existing
logger.warn call.
🪄 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: 0749bc05-637b-4632-b51b-4cb29b74de92
📒 Files selected for processing (3)
backend/src/lib/agent.tsbackend/src/lib/serviceAccountToken.tsbackend/src/routes/placementDebug.ts
|
/hold |
Also log the error details when the CA bundle file read fails, instead of silently swallowing the exception. Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
90d182c to
f32f82a
Compare
Summary
getPlacementDebugCACertificate()to read the OCM-managed CA bundle fromPLACEMENT_CA_BUNDLE_PATHwith file-watch rotation supportgetPlacementDebugAgent()HTTPS agent that trusts the OCM CA bundle, with graceful fallback togetServiceAgent()when the env var is unsetgetServiceAgent()togetPlacementDebugAgent()Context
The PlacementDebugServer's TLS serving certificate is being moved from the OpenShift-specific
serving-cert-secret-nameannotation to the OCM-native CertRotationController (ocm#1505). The CertRotationController uses its own self-signed CA, so the console backend needs to trust that CA instead of (or in addition to) the OpenShift service-ca.A separate backplane-operator PR will mount the OCM
ca-bundle-configmapConfigMap into the console pod and setPLACEMENT_CA_BUNDLE_PATH.Files changed
backend/src/lib/serviceAccountToken.tsgetPlacementDebugCACertificate()— reads CA from arbitrary path, watches for rotationbackend/src/lib/agent.tsgetPlacementDebugAgent()— HTTPS agent trusting OCM CA, falls back to service-ca agentbackend/src/routes/placementDebug.tsgetServiceAgent()togetPlacementDebugAgent()Test plan
npm run checkpasses (lint, prettier, tsc)PLACEMENT_CA_BUNDLE_PATHis unset,getPlacementDebugAgent()returnsgetServiceAgent()(no behavioral change)PLACEMENT_CA_BUNDLE_PATHpoints to a valid cert file, a dedicated agent is created trusting that CA🤖 Generated with Claude Code
Summary by CodeRabbit