Skip to content

Commit ff5d672

Browse files
committed
Sync primaryUpstreamProvider guidance in user-facing docs
Two user-facing docs and one in-code constant comment had drifted relative to the schema and runtime behaviour shipped earlier on this branch. The schema docstrings and CRD reference were already in sync with the code; this commit catches up the prose that users actually read first. * `docs/operator/virtualmcpserver-api.md`: the `authzConfig` section listed `primaryUpstreamProvider` under `inline` with no deprecation note, did not mention the new top-level claim-mapping fields on `AuthzConfigRef`, and did not document the failure modes on `AuthConfigured`. The new prose marks the inline location as deprecated, points to `spec.authServerConfig.primaryUpstreamProvider`, describes `groupClaimName` / `roleClaimName` / `groupEntityType` with their spec-over-ConfigMap precedence, and names the `AuthzConfigMapNotFound` / `AuthzConfigMapInvalid` reasons users will see on misconfiguration. Addresses F2. * `docs/operator/virtualmcpserver-kubernetes-guide.md`: the "Multiple upstream IDPs" troubleshooting block pinned `primaryUpstreamProvider` at the deprecated inline location, so anyone following the guide would emit `AuthzPrimaryUpstreamProviderDeprecated` events on every reconcile. The example now uses `spec.authServerConfig.primaryUpstreamProvider`. Adds a "Migration: `primaryUpstreamProvider` location" note that names the deprecation event reason and the planned removal cadence. Adds an "Authorization policy errors" subsection covering `AuthzConfigMapNotFound` / `AuthzConfigMapInvalid` and an "Enterprise Cedar policies that deny every request" subsection covering the claim-mapping knobs. Addresses F3, F7, and F13. * `cmd/thv-operator/api/v1beta1/mcpserver_types.go`: `ConditionTypeAuthzPrimaryUpstreamProviderIgnored` now carries the deprecation lineage in its doc comment. The condition fires only for the deprecated `InlineAuthzConfig.PrimaryUpstreamProvider` field; when that field is removed at end of the deprecation cycle, this condition and its reason constant should be removed in the same change. Addresses Jakub's PR-level nit. No code changes; comment and prose only.
1 parent 30b57ff commit ff5d672

3 files changed

Lines changed: 89 additions & 14 deletions

File tree

cmd/thv-operator/api/v1beta1/mcpserver_types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ const (
9696
// when spec.authzConfig.inline.primaryUpstreamProvider is non-empty on a CR type
9797
// that has no embedded auth server (MCPServer / MCPRemoteProxy). The field has
9898
// no effect on those resources and is documented as VirtualMCPServer-only.
99+
//
100+
// Tied to the deprecated InlineAuthzConfig.PrimaryUpstreamProvider field
101+
// (see mcpserver_types.go). When that field is removed at end of the
102+
// deprecation cycle, this condition and ConditionReasonAuthzPrimaryUpstreamProviderIgnored
103+
// below should be removed in the same change: there is no other path that
104+
// fires this advisory.
99105
ConditionTypeAuthzPrimaryUpstreamProviderIgnored = "AuthzPrimaryUpstreamProviderIgnored"
100106
)
101107

docs/operator/virtualmcpserver-api.md

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,32 @@ Configures authentication for clients connecting to the Virtual MCP server. Reus
8686
- `type` (string, required): `inline` or `configMap`
8787
- `inline` (InlineAuthzConfig, required when type=inline): Inline Cedar policies
8888
- `policies` ([]string, required): Cedar policy strings
89-
- `entitiesJson` (string, optional): Cedar entities (JSON)
90-
- `primaryUpstreamProvider` (string, optional): Names the upstream IDP whose
91-
access token claims Cedar should evaluate. Only meaningful when
92-
`spec.authServerConfig` is set with multiple upstreamProviders. When
93-
empty, the controller defaults to the first upstream. Must match a
94-
configured upstream name; the VirtualMCPServer is rejected with
95-
`AuthServerConfigValidated=False` otherwise.
96-
- `configMap` (ConfigMapAuthzRef, required when type=configMap): Reference to a ConfigMap holding policies
89+
- `entitiesJson` (string, optional): Cedar entities (JSON). Required when
90+
transitive policies (e.g. `ClaimGroup` → `PlatformRole`) need a static
91+
entity store. Defaults to `"[]"`.
92+
- `primaryUpstreamProvider` (string, optional): **Deprecated.** Use
93+
`.spec.authServerConfig.primaryUpstreamProvider` instead. Setting this
94+
field still resolves a primary upstream for backward compatibility and
95+
emits a Warning event with reason
96+
`AuthzPrimaryUpstreamProviderDeprecated`. Planned removal one release
97+
after the deprecation cycle.
98+
- `configMap` (ConfigMapAuthzRef, required when type=configMap): Reference to
99+
a ConfigMap holding Cedar policies. The operator resolves the ConfigMap at
100+
reconcile time and bakes the policies into the rendered vmcp `config.yaml`.
101+
Failures surface as `AuthConfigured=False` with reason
102+
`AuthzConfigMapNotFound` (missing reference) or `AuthzConfigMapInvalid`
103+
(parse, validation, or non-Cedar payload).
104+
- `groupClaimName` (string, optional): JWT claim key Cedar should treat as
105+
the group list (overrides the well-known defaults `groups`, `roles`,
106+
`cognito:groups`). When `type` is `configMap`, the spec value overrides
107+
any `group_claim_name` set in the ConfigMap payload.
108+
- `roleClaimName` (string, optional): JWT claim key Cedar should treat as
109+
the role list. Same spec-over-ConfigMap precedence as `groupClaimName`.
110+
- `groupEntityType` (string, optional): Cedar entity type used for principal
111+
parent UIDs synthesised from JWT group/role claims. Defaults to
112+
`THVGroup`. Must match the entity type used in the static entity store
113+
for transitive `in` checks to resolve. Same spec-over-ConfigMap
114+
precedence as `groupClaimName`.
97115

98116
**Important**: The `type` field must always be explicitly specified. When no authentication is required, use `type: anonymous`.
99117

docs/operator/virtualmcpserver-kubernetes-guide.md

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,17 +628,68 @@ Then gradually add restrictions. Common Cedar policy issues:
628628
**Multiple upstream IDPs**: when `spec.authServerConfig` declares more than
629629
one `upstreamProviders` entry, Cedar evaluates claims from the first one by
630630
default. Pin a specific provider explicitly via
631-
`authzConfig.inline.primaryUpstreamProvider`:
631+
`spec.authServerConfig.primaryUpstreamProvider`:
632632

633633
```yaml
634-
authzConfig:
635-
type: inline
636-
inline:
634+
spec:
635+
authServerConfig:
636+
issuer: https://vmcp.example.com
637637
primaryUpstreamProvider: okta # must match one of the configured upstreams
638-
policies:
639-
- 'permit(principal, action, resource);'
638+
upstreamProviders:
639+
- name: okta
640+
type: oidc
641+
# ...
642+
- name: github
643+
type: oauth2
644+
# ...
645+
incomingAuth:
646+
authzConfig:
647+
type: inline
648+
inline:
649+
policies:
650+
- 'permit(principal, action, resource);'
640651
```
641652

653+
> **Migration: `primaryUpstreamProvider` location**
654+
>
655+
> The field used to live under
656+
> `spec.incomingAuth.authzConfig.inline.primaryUpstreamProvider`. It has
657+
> moved to `spec.authServerConfig.primaryUpstreamProvider` to sit next to
658+
> the `upstreamProviders` list it selects from. The old location is read
659+
> for one release for backward compatibility; the controller emits a
660+
> Warning event with reason `AuthzPrimaryUpstreamProviderDeprecated`
661+
> whenever it consumes the deprecated location. Move the value to the new
662+
> location to clear the warning. The deprecated field is planned for
663+
> removal one release after the deprecation cycle.
664+
665+
**Authorization policy errors**: misconfigured authz surfaces on the
666+
`AuthConfigured` condition with one of:
667+
668+
* `AuthzConfigMapNotFound`: the ConfigMap referenced by
669+
`spec.incomingAuth.authzConfig.configMap` does not exist in the
670+
namespace. Create it before reconciling, or fix the name.
671+
* `AuthzConfigMapInvalid`: the ConfigMap exists but the payload is
672+
missing the configured key, empty, malformed YAML/JSON, fails Cedar
673+
validation, or is a registered non-Cedar authorizer (vMCP supports
674+
Cedar only). Check the payload shape (see the Cedar v1 schema in the
675+
example above).
676+
677+
**Enterprise Cedar policies that deny every request**: when a policy
678+
walks a transitive hierarchy like `Client → ClaimGroup → PlatformRole`,
679+
both Cedar JWT-claim mapping settings and the static entity store must
680+
agree on the entity type. The configuration fields live on
681+
`spec.incomingAuth.authzConfig`:
682+
683+
* `groupClaimName` / `roleClaimName`: JWT claim keys to extract.
684+
* `groupEntityType`: Cedar entity type used for principal parent UIDs.
685+
Must match the entity type used in `entitiesJson` (e.g. `ClaimGroup`
686+
rather than the default `THVGroup`).
687+
688+
For configMap-sourced authz, the same fields can be set in the
689+
ConfigMap payload (`cedar.group_claim_name`, `cedar.role_claim_name`,
690+
`cedar.group_entity_type`); spec-level values on `authzConfig` override
691+
the ConfigMap when set.
692+
642693
### Backend Discovery Issues
643694

644695
#### Backends Not Discovered

0 commit comments

Comments
 (0)