Commit 9afbe4e
authored
Add CEL validation rule for RemoteMCPServer with CA Cert and AllowedNamespaces (#1988)
# Description
Adds a CEL admission rule on `RemoteMCPServer` that rejects setting both
`spec.allowedNamespaces` and `spec.tls.caCertSecretRef`. A
`RemoteMCPServer` that pins a CA Secret must be same-namespace-only; one
that allows cross-namespace references must not pin a CA Secret.
This closes a silent footgun in the cross-namespace + TLS combination
without changing any working behavior: the `AllowedNamespaces` field
stays, the reconciler is unchanged, and non-CA cross-namespace sharing
keeps working exactly as before.
# Why
When an agent consumes a `RemoteMCPServer` that pins a CA bundle, the
translator mounts that Secret onto the **agent's** pod by bare name
(`addTLSConfiguration`). A `SecretVolumeSource` has no namespace field —
Kubernetes always resolves it in the pod's own namespace, not the
`RemoteMCPServer`'s. So if the RMS is referenced cross-namespace, the CA
mount either:
- **dangles** — the Secret doesn't exist in the agent's namespace and
the pod fails to start; or
- **silently mounts the wrong CA** — an unrelated Secret of the same
name that happens to exist in the agent's namespace is trusted for the
upstream, with no error anywhere.
Meanwhile the controller validates and hashes the Secret in the
`RemoteMCPServer`'s namespace, so it reports `Accepted=true` while the
pod mount looks elsewhere. The cert reference itself is structurally
same-namespace (`caCertSecretRef` is a bare name with no namespace
field, resolved in the owner's namespace by design); the only thing that
broke co-location was opening the RMS object up cross-namespace via
`allowedNamespaces`. Making the two mutually exclusive removes the
unsound combination at admission time rather than letting it surface as
a runtime mount failure on the consuming agent.
# Validation rule
Spec-level `XValidation` on `RemoteMCPServerSpec`:
```
rule: !(has(self.allowedNamespaces) && has(self.allowedNamespaces.from)
&& (self.allowedNamespaces.from == 'All' || self.allowedNamespaces.from == 'Selector')
&& has(self.tls) && has(self.tls.caCertSecretRef) && size(self.tls.caCertSecretRef) > 0)
message: spec.allowedNamespaces must not permit cross-namespace access (from: All or from: Selector)
when spec.tls.caCertSecretRef is set: a pinned CA Secret is mounted onto the consuming agent's
pod and Kubernetes resolves it in the agent's namespace, not this RemoteMCPServer's. Use
from: Same (the default), or remove the CA Secret reference.
```
The rule only rejects the genuinely-hazardous case — a CA Secret
combined with an `allowedNamespaces` that actually permits other
namespaces (`from: All` or `from: Selector`). `from: Same` (or
omitted/empty, which defaults to same-namespace) is always allowed
alongside a CA Secret, since it carries no cross-namespace mount hazard.
Regenerated `kagent.dev_remotemcpservers.yaml` and synced the helm CRD
copy under `helm/kagent-crds/templates/`.
# Tests
Added cases to the existing envtest CEL suite (`tlsconfig_cel_test.go`),
exercised against a real kube-apiserver:
- `allowedNamespaces` `from: All` + `caCertSecretRef` → rejected.
- `allowedNamespaces` `from: Selector` + `caCertSecretRef` → rejected.
- `allowedNamespaces` `from: Same` + `caCertSecretRef` → accepted
(same-namespace, no hazard).
- `allowedNamespaces` `from: All`, no TLS → accepted (cross-namespace
sharing still works).
- `caCertSecretRef`, no `allowedNamespaces` → accepted (same-namespace
CA pinning still works).
# Compatibility
Not a breaking API change — no field is removed and no working
configuration changes behavior. The only new effect is at admission: a
manifest that combines a CA Secret with a *cross-namespace-permitting*
`allowedNamespaces` (`from: All` / `from: Selector`) is now rejected on
create/update. That combination was already non-functional (dangling or
wrong-CA mount), so this turns a silent runtime failure into an
explicit, actionable admission error. Same-namespace configurations
(`from: Same` or omitted) are unaffected, and objects already stored are
not re-validated until their next write.
---------
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>1 parent feb8cf9 commit 9afbe4e
4 files changed
Lines changed: 116 additions & 0 deletions
File tree
- go/api
- config/crd/bases
- v1alpha2
- helm/kagent-crds/templates
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
62 | 69 | | |
63 | 70 | | |
64 | 71 | | |
| |||
255 | 262 | | |
256 | 263 | | |
257 | 264 | | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
258 | 274 | | |
259 | 275 | | |
260 | 276 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
| 42 | + | |
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
| |||
63 | 64 | | |
64 | 65 | | |
65 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
66 | 74 | | |
67 | 75 | | |
68 | 76 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
220 | 296 | | |
221 | 297 | | |
222 | 298 | | |
| |||
Lines changed: 16 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
62 | 69 | | |
63 | 70 | | |
64 | 71 | | |
| |||
255 | 262 | | |
256 | 263 | | |
257 | 264 | | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
258 | 274 | | |
259 | 275 | | |
260 | 276 | | |
| |||
0 commit comments