fix(control-plane): inject AMBIENT_TOKEN into MCP sidecar at pod creation#1274
fix(control-plane): inject AMBIENT_TOKEN into MCP sidecar at pod creation#1274markturansky merged 1 commit intoalphafrom
Conversation
…tion The ambient-mcp sidecar uses the Go SDK which requires AMBIENT_TOKEN. Fetch the CP's current API token at pod-creation time via the factory's Token() method and inject it as AMBIENT_TOKEN into the sidecar env. Fixes: MCP sidecar crash "AMBIENT_TOKEN is required" 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified the MCP sidecar provisioning logic to fetch ambient tokens from a factory and inject them as environment variables into sidecar containers, with graceful fallback on token fetch failures. Changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ambient-control-plane/internal/reconciler/kube_reconciler.go`:
- Around line 444-449: The current code silently swallows failures from
r.factory.Token and appends an MCP sidecar with an empty ambientToken which
produces a broken pod; change the logic so token resolution is fatal: if
r.factory.Token(ctx) returns an error, propagate/return that error (or otherwise
abort pod creation) instead of setting ambientToken = "" and calling
r.buildMCPSidecar; ensure ensurePod/ensure flow does not create or mark a pod as
Running when the token is missing. Update the analogous token-fetch site (the
other occurrence around r.buildMCPSidecar) to follow the same fatal behavior so
no pod with a missing AMBIENT_TOKEN is ever created.
- Around line 444-449: The code currently calls r.factory.Token(ctx) and injects
that control-plane OIDC access token into the MCP sidecar env (ambientToken) via
buildMCPSidecar, which exposes a long-lived control-plane credential to the pod;
instead change buildMCPSidecar so it does NOT accept or embed
r.factory.Token(ctx) directly — remove the ambientToken env injection and
instead configure the sidecar to use a renewable, least-privilege credential
flow (e.g., reference a projected ServiceAccount token file or a
TokenRequest-based short-lived token, or have the sidecar call a local
control-plane token proxy endpoint); update places that call
r.factory.Token(ctx) (the TokenProvider) to stop passing the raw token into pod
spec and instead ensure tokens are obtained at runtime via the chosen
projection/proxy mechanism.
🪄 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: Pro
Run ID: 01bda881-8d36-4578-9892-d66b7517ba2f
📒 Files selected for processing (1)
components/ambient-control-plane/internal/reconciler/kube_reconciler.go
| ambientToken, err := r.factory.Token(ctx) | ||
| if err != nil { | ||
| r.logger.Warn().Err(err).Str("session_id", session.ID).Msg("failed to fetch token for MCP sidecar; sidecar will start without AMBIENT_TOKEN") | ||
| ambientToken = "" | ||
| } | ||
| containers = append(containers, r.buildMCPSidecar(ambientToken)) |
There was a problem hiding this comment.
Make MCP token resolution fatal here
ambient-mcp does not have a fallback mode: it exits immediately when AMBIENT_TOKEN is missing (components/ambient-mcp/main.go:19-23). So this path still creates a broken pod, and because ensurePod returns early for existing pods on Line 394, a transient token-fetch failure can leave the session permanently stuck with a dead sidecar while the reconciler still advances it to Running on Line 165.
Possible fix
if useMCPSidecar {
ambientToken, err := r.factory.Token(ctx)
if err != nil {
- r.logger.Warn().Err(err).Str("session_id", session.ID).Msg("failed to fetch token for MCP sidecar; sidecar will start without AMBIENT_TOKEN")
- ambientToken = ""
+ return fmt.Errorf("fetching token for MCP sidecar: %w", err)
}
containers = append(containers, r.buildMCPSidecar(ambientToken))
r.logger.Info().Str("session_id", session.ID).Msg("MCP sidecar enabled for session")
}As per coding guidelines, "Flag only errors, security risks, or functionality-breaking problems."
Also applies to: 832-834
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-control-plane/internal/reconciler/kube_reconciler.go`
around lines 444 - 449, The current code silently swallows failures from
r.factory.Token and appends an MCP sidecar with an empty ambientToken which
produces a broken pod; change the logic so token resolution is fatal: if
r.factory.Token(ctx) returns an error, propagate/return that error (or otherwise
abort pod creation) instead of setting ambientToken = "" and calling
r.buildMCPSidecar; ensure ensurePod/ensure flow does not create or mark a pod as
Running when the token is missing. Update the analogous token-fetch site (the
other occurrence around r.buildMCPSidecar) to follow the same fatal behavior so
no pod with a missing AMBIENT_TOKEN is ever created.
Do not inject the control-plane bearer token directly into the pod
r.factory.Token(ctx) is returning the control plane's current OIDC access token (components/ambient-control-plane/internal/auth/token_provider.go:51-69). Putting that raw token into the sidecar env means: 1) long-lived sessions will lose MCP API access when the token expires, and 2) the control-plane credential is now exposed anywhere this pod spec or container env is readable. This needs a renewable, least-privilege credential flow instead of copying the control plane's current token into workload pods.
As per coding guidelines, "Flag only errors, security risks, or functionality-breaking problems."
Also applies to: 825-846
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-control-plane/internal/reconciler/kube_reconciler.go`
around lines 444 - 449, The code currently calls r.factory.Token(ctx) and
injects that control-plane OIDC access token into the MCP sidecar env
(ambientToken) via buildMCPSidecar, which exposes a long-lived control-plane
credential to the pod; instead change buildMCPSidecar so it does NOT accept or
embed r.factory.Token(ctx) directly — remove the ambientToken env injection and
instead configure the sidecar to use a renewable, least-privilege credential
flow (e.g., reference a projected ServiceAccount token file or a
TokenRequest-based short-lived token, or have the sidecar call a local
control-plane token proxy endpoint); update places that call
r.factory.Token(ctx) (the TokenProvider) to stop passing the raw token into pod
spec and instead ensure tokens are obtained at runtime via the chosen
projection/proxy mechanism.
Summary
ambient-mcpsidecar uses the Go SDK which requiresAMBIENT_TOKENto startfactory.Token(ctx)at pod-creation time and injects it asAMBIENT_TOKENinto the sidecar envFixes: MCP sidecar crash
AMBIENT_TOKEN is required(seen in mpp-openshift runner pods)Test plan
ambient-mcpsidecar starts successfully (noAMBIENT_TOKEN is requiredcrash)http://localhost:8090🤖 Generated with Claude Code
Summary by CodeRabbit