feat: Support env.valueFrom in MCP server config

Closed
KE
5mo ago
+1269-53

What type of PR is this?

/kind feature

What this PR does / why we need it:

Changes AgentConfig.spec.mcpServers[].env from a map[string]string to []corev1.EnvVar so each entry can pull its value from a single key of a Secret or ConfigMap via the standard valueFrom shape (secretKeyRef, configMapKeyRef). The existing envFrom (whole-Secret) field is preserved and still wins over env on name collision.

Only SecretKeyRef and ConfigMapKeyRef variants of valueFrom are honored — fieldRef, resourceFieldRef and fileKeyRef are pod-scoped (or file-gated) and meaningless for an MCP server process, so they are rejected with an error. Unlike a pod env, MCP env is resolved by the controller (not the kubelet), so those variants have nothing to resolve against.

Because the env field is now a list rather than a map, this is a breaking change for any AgentConfig that sets mcpServers[].env. The CLI --mcp flag still accepts the {"KEY":"VAL"} shorthand alongside the new [{name,value/valueFrom}] form.

Includes RBAC permission to read ConfigMaps.

Which issue(s) this PR is related to:

Fixes #1152

Special notes for your reviewer:

The shape choice mirrors PodOverrides.Env in api/v1alpha1/task_types.go:78 so MCP env now uses the same idiom users already see elsewhere in the API. Migration for existing YAML:

# Before
env:
  DSN: postgres://localhost/db

# After
env:
- name: DSN
  value: postgres://localhost/db

Validation is enforced at reconcile time (the Task transitions to Failed with an MCPSecretFailed event and a clear message) rather than at admission. A CRD generated from corev1.EnvVar exposes the full upstream valueFrom schema, and CEL XValidation over that list is rejected by the API server's cost budget — the unbounded upstream string fields can't be given a maxLength, so even a trivial rule blows the per-element budget. Keeping []corev1.EnvVar (consistent with PodOverrides.Env) plus a clear reconcile-time error was preferred over introducing a divergent custom type.

valueFrom entries marked optional now match kubelet semantics: when the referenced Secret/ConfigMap or key is missing, the variable is omitted from the MCP server instead of being set to an empty string.

Does this PR introduce a user-facing change?

[ACTION REQUIRED] AgentConfig: `mcpServers[].env` is now a list of `EnvVar` (matching `PodOverrides.env`) and supports `valueFrom.secretKeyRef` and `valueFrom.configMapKeyRef` for pulling a single key out of a Secret or ConfigMap; other `valueFrom` variants (`fieldRef`, `resourceFieldRef`, `fileKeyRef`) are rejected. A `valueFrom` marked optional whose Secret/ConfigMap or key is missing omits the variable (matching kubelet semantics) instead of setting it empty. The previous `map[string]string` shape is no longer accepted — existing in-cluster AgentConfig objects that set `mcpServers[].env` must be re-applied in the new list form after upgrade.

Summary by cubic

Adds valueFrom support for MCP server env by changing AgentConfig.spec.mcpServers[].env to a list of corev1.EnvVar, enabling per-key Secret/ConfigMap references. envFrom still wins on name collision, the CLI keeps the {"KEY":"VAL"} shorthand, and RBAC/CRDs were updated.

  • New Features

    • mcpServers[].env is now []corev1.EnvVar with valueFrom.secretKeyRef and valueFrom.configMapKeyRef.
    • Controller resolves valueFrom to literals; fieldRef/resourceFieldRef are rejected.
    • envFrom continues to override env on key collision.
    • Added permission to read ConfigMaps; CRD/manifests updated.

  • Migration

    • Update YAML from map to list. Example: env: {DSN: "postgres://..."} → env: [{name: "DSN", value: "postgres://..."}]. The CLI --mcp flag still accepts the old map shorthand.