fix(control-plane,manifests): resolve MCP sidecar TLS and api-server proxy failures#1546
Conversation
Initial draft of the security specification covering: - OpenShift namespace-scoped build agent ServiceAccount - Control Plane SA as the single SRE-owned cluster identity - Per-project Vertex AI credential scoping - User SSO token propagation into runners - Integration credential lifecycle (Credential provider=*) - Dynamic MCP credential watching (sidecar vs pod mode) - Per-session ServiceAccount isolation (closing the shared-SA gap) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Move security.md to specs/security/security.spec.md. Extract HOW content (credential authorization model, RBAC runtime grant semantics, proxy authentication, design decisions) from ambient-model.spec.md into the security spec. Model spec retains WHAT (schemas, endpoints, provider enum, permission matrix, CLI mappings). Cross-references link both documents. Also moves ambient-model.spec.md from specs/sessions/ to specs/api/ and updates all references (BOOKMARKS.md, design README, devflow skill, workflows). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Rewrite to Requirement:/Scenario: format with RFC 2119 keywords (SHALL/MUST/SHOULD) - Fix broken GFM table (double pipe in Design Decisions header separator) - Remove implementation details (file paths, function names) from spec - Use "Project" consistently instead of "namespace" for Ambient boundary; add terminology note - Register api/ and security/ domains in specs/index.spec.md - Fix BOOKMARKS.md domain label (sessions -> api) - Remove Draft/Authors/Last Updated metadata header to match other specs - Replace fragile §N anchors with descriptive anchor links in model spec cross-refs 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Credentials are now global resources bound to Projects via RoleBindings instead of project-scoped with a project_id FK. This eliminates duplication when the same PAT is used across multiple Projects. Adds vertex and kubeconfig to the provider enum. Splits the security spec Accounts and Tokens table into scoped subsections for independent implementation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add credential:owner and credential:viewer roles for self-service CRUD - Update security spec: credentials are global, bound via RoleBindings - Fix all stale references: endpoint paths, K8s analogy, named patterns, design decisions (5-scope RBAC), key invariants, accounts table scopes - Update cross-reference anchor from project-scoped to credential-access Addresses review feedback from jsell-rh on PR #1514. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…proxy failures The MCP sidecar in runner pods failed with "x509: certificate signed by unknown authority" because it lacked the OpenShift service-ca volume mount and SSL_CERT_FILE env var. Additionally, the ambient-api-server returned 502 on proxied requests (credentials, session listing) because BACKEND_URL was never set, defaulting to unreachable localhost:8080. - Add service-ca volume mount and SSL_CERT_FILE to MCP sidecar container - Add BACKEND_URL to production ambient-api-server env patch 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
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 (2)
📝 WalkthroughWalkthroughThe PR configures TLS certificate access for the MCP sidecar container in the reconciler and sets the backend service URL for the production API server deployment through independent manifest and code updates. ChangesMCP Sidecar TLS Configuration
API Server Backend Configuration
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Merge Queue Status
This pull request spent 42 seconds in the queue, including 9 seconds running CI. Required conditions to merge |
❌ Deploy Preview for cheerful-kitten-f556a0 failed.
|
…nt-code namespace (#1553) ## Summary - Adds a `NetworkPolicy` (`allow-from-runner-namespaces`) to the base kustomize manifests that permits ingress from runner pods (`app: ambient-code-runner`) in any namespace to the `ambient-code` namespace - Fixes `INITIAL_PROMPT TimeoutError` caused by default-deny NetworkPolicies blocking cross-namespace traffic from runner pods to `backend-service` - Already applied on the live `hcmais01ue1` cluster; this PR codifies the fix in manifests ## Context This is a follow-up to PR #1546 (MCP sidecar TLS + BACKEND_URL fixes). The NetworkPolicy was applied on-cluster during incident response but was not included in the merged PR. ## Test plan - [x] Verified on live cluster: runner pods can reach `backend-service` and `INITIAL_PROMPT` succeeds - [ ] `kustomize build components/manifests/overlays/production` renders without errors - [ ] Next production deploy includes the NetworkPolicy 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added network security policy to restrict ingress traffic, allowing connections only from designated runner pods in their respective namespaces. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: user <u@example.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
ambient-mcpsidecar container in runner pods was missing the OpenShiftservice-cavolume mount andSSL_CERT_FILEenv var, causingx509: certificate signed by unknown authorityerrors when callingambient-api-serverover HTTPS (service-serving certs). Added both, matching the existing runner container configuration.ambient-api-serverreverse proxy plugin defaultsBACKEND_URLtohttp://localhost:8080when unset. Since the backend runs in a separate pod (backend-service:8080), all proxied requests (credentials, session listing) returned 502. AddedBACKEND_URLto the production env patch.Root Cause
These issues were exposed by the
ambient-control-planedeployment fix (env[15]value/valueFromconflict onCP_RUNTIME_NAMESPACE). Once the control-plane was redeployed with the correct manifest, the MCP sidecar started being injected into runner pods, surfacing the missing TLS config. TheBACKEND_URLgap has existed since the api-server proxy plugin was introduced but was previously masked.Test plan
ambient-control-planeimage to clusterlist_sessions,list_projects) work without TLS errorsacp_list_sessionsworks without 502🤖 Generated with Claude Code
Summary by CodeRabbit