From 5f728a556b948650f133f476292cbacd70c55567 Mon Sep 17 00:00:00 2001 From: Niklas Ye Date: Tue, 29 Sep 2026 16:19:06 +0200 Subject: [PATCH] Switched from referencegrant the ligther parentRef --- DESIGN.md | 194 +++++++++++++++++++++++++++++++----------------------- README.md | 11 ++-- 2 files changed, 115 insertions(+), 90 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 2fcabf3..c245ef3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -28,9 +28,9 @@ controller-runtime / Kubebuilder conventions. install, matching how terdut-server itself ships. - Cross-namespace references are limited to exactly one edge: `TerdutTeam.spec.serverRef` may name a `TerdutServer` in a different - namespace, gated by an explicit consent object in the target namespace - (§4.2, §4.6) — this is the multi-tenant shape the operator exists for - (one platform team owns a `TerdutServer`; other teams self-service a + namespace, gated by that `TerdutServer`'s own `spec.allowedTeams` consent + field (§4.1, §4.2) — this is the multi-tenant shape the operator exists + for (one platform team owns a `TerdutServer`; other teams self-service a `TerdutTeam` against it without needing write access to the server's namespace). Every other reference (`teamRef` on the escalation rule/dead-man-switch/alert-source CRDs) stays same-namespace-as-its-`TerdutTeam` @@ -59,13 +59,27 @@ This cross-namespace edge needs the target namespace's explicit consent — otherwise any namespace in the cluster could point a `TerdutTeam` at someone else's `TerdutServer` and have the operator provision a team + credentials against it, which is a namespace-boundary violation, not a -gitops convenience. §4.6 covers the consent object -(`TerdutServerReferenceGrant`), modeled directly on Gateway API's -`ReferenceGrant` — the established Kubernetes pattern for exactly this -"object A in namespace X wants to reference object B in namespace Y" -problem. No other reference in this design (`teamRef` on the child CRDs) -crosses a namespace boundary, so this is the only place `ReferenceGrant`-style -consent is needed (§1, §4.2, §5, §6, §9). +gitops convenience. Kubernetes has two established patterns for this kind +of consent, and Gateway API itself uses both, for two different +relationships: + +- **`ReferenceGrant`** (used for a Route reaching into an arbitrary + Service/Secret): a separate object, living in the *target* namespace, + enumerating exact `{fromNamespace, fromKind} → {toKind, toName}` pairs. + No wildcard, no selector — every permitted namespace is spelled out. +- **An inline field on the parent** (used for a `ListenerSet` attaching to + a shared `Gateway`, GA since Gateway API v1.5): the parent carries + `spec.allowedListeners.namespaces: {from: None|Same|All|Selector, + selector}` directly, no separate CRD. + +`TerdutTeam` attaching to a shared `TerdutServer` is structurally the +second case, not the first — a bounded set of expected children attaching +to a parent they were deliberately made shareable, not an arbitrary +backend reference — so this design follows the `ListenerSet` precedent: +`TerdutServer.spec.allowedTeams` (§4.1), no extra CRD. §4.2 covers how +`TerdutTeam` resolves against it. No other reference in this design +(`teamRef` on the child CRDs) crosses a namespace boundary, so this is the +only place cross-namespace consent is needed at all (§1, §5, §6, §9). > How does escalationrules, switches and alertsources connect to a team? @@ -99,9 +113,8 @@ models as a plain reference. | Kind | Scope | Purpose | |---|---|---| -| `TerdutServer` | Namespaced | One terdut-server install: Deployment, Service, database wiring, bootstrap, operator credentials. | +| `TerdutServer` | Namespaced | One terdut-server install: Deployment, Service, database wiring, bootstrap, operator credentials, cross-namespace team consent. | | `TerdutTeam` | Namespaced | One team on a `TerdutServer`, possibly in another namespace: name, OIDC group mapping. | -| `TerdutServerReferenceGrant` | Namespaced (lives with the `TerdutServer`) | Consent for a `TerdutTeam` in another namespace to reference this namespace's `TerdutServer`(s). | | `TerdutEscalationRule` | Namespaced | A team's escalation policy (levels, targets, repeat). | | `TerdutDeadmanSwitch` | Namespaced | One dead man's switch on a team. | | `TerdutAlertSource` | Namespaced | One alert-ingest integration on a team (currently: Alertmanager webhook). | @@ -153,6 +166,16 @@ spec: adminGroup: "" sessionMaxAge: 12h passwordLogin: true + # Consent for TerdutTeams in OTHER namespaces to set serverRef at this + # TerdutServer. Same-namespace TerdutTeams never need this. Modeled on + # Gateway API's Gateway.spec.allowedListeners.namespaces (the ListenerSet + # attachment pattern, not ReferenceGrant — see §2 for why). + allowedTeams: + namespaces: + from: None # None (default) | Same | All | Selector + # selector: # required, and only meaningful, when from: Selector + # matchLabels: + # terdut.ryuvia.com/allowed: "true" status: conditions: [...] # Ready, DatabaseReady, Bootstrapped observedGeneration: 3 @@ -169,6 +192,14 @@ be mutually exclusive (`dsn` xor `postgresClusterRef`); mirrors the chart's "chart provisions no database" stance — this operator provisions no database either, only wires up one that exists. +`spec.allowedTeams.namespaces.from` defaults to `None`, matching +`allowedListeners`'s own default — a fresh `TerdutServer` accepts no +cross-namespace `TerdutTeam` until its owner opts in, same-namespace +`TerdutTeam`s are unaffected either way. `Selector` deliberately has no +per-name allowlist (no "and only these teams") — namespace-level consent is +the right granularity here, same reasoning as `ListenerSet`: the namespace +is the tenancy boundary, not the object. + ### 4.2 `TerdutTeam` ```yaml @@ -176,8 +207,8 @@ spec: serverRef: name: terdut namespace: platform-oncall # optional; defaults to this TerdutTeam's own namespace. - # Cross-namespace requires a matching TerdutServerReferenceGrant - # in that namespace (§4.6) — otherwise Ready: False, reason: RefNotPermitted. + # Cross-namespace requires that TerdutServer's spec.allowedTeams + # (§4.1) to admit this namespace — otherwise Ready: False, reason: RefNotPermitted. displayName: "Platform" # -> POST /api/teams {"name": ...}; server assigns the ID oidc: memberGroup: "terdut-platform-members" @@ -257,52 +288,49 @@ or stores it anywhere else; the CR's `status` carries only the Secret reference, matching how e.g. cert-manager's `Certificate` exposes `spec.secretName` rather than the key material itself. -### 4.6 `TerdutServerReferenceGrant` +### 4.6 Cross-namespace consent: `TerdutServer.spec.allowedTeams` -Consent object living in the *`TerdutServer`'s* namespace, modeled directly -on Gateway API's `ReferenceGrant` (`gateway.networking.k8s.io/v1beta1`): -without one, no `TerdutTeam` outside this namespace may resolve a -`serverRef` into it, no matter what it names. +No separate CRD — the consent lives on `TerdutServer` itself (§4.1), following +Gateway API's `Gateway.spec.allowedListeners` (`ListenerSet` attachment) +rather than its `ReferenceGrant`, since a `TerdutTeam` attaching to a shared +`TerdutServer` is the same shape of relationship: a bounded set of expected +children attaching to a parent explicitly designed to be shared, not an +arbitrary cross-namespace backend reference (see §2 for the full comparison +of both patterns). -```yaml -apiVersion: terdut.ryuvia.com/v1alpha1 -kind: TerdutServerReferenceGrant -metadata: - name: allow-app-teams - namespace: platform-oncall # the TerdutServer's namespace -spec: - from: - - group: terdut.ryuvia.com - kind: TerdutTeam - namespace: team-checkout # one entry per namespace permitted to reference in - - group: terdut.ryuvia.com - kind: TerdutTeam - namespace: team-payments - to: - - group: terdut.ryuvia.com - kind: TerdutServer - name: terdut # optional: omit to permit any TerdutServer in this namespace -``` - -- **No selector/wildcard-across-namespaces field** (e.g. no `namespaceSelector`) - in v1, matching upstream `ReferenceGrant`'s own choice: the namespace owner - enumerates exactly which namespaces may reference in, which is the whole - point of requiring affirmative, auditable consent rather than an implicit - or pattern-matched grant. -- A `TerdutTeam`'s controller checks for a matching grant (namespace + - optionally name) on every reconcile before it will resolve `serverRef` - cross-namespace, exactly mirroring how `HTTPRoute`/`Certificate` - controllers check `ReferenceGrant` before honouring a cross-namespace - `backendRef`/`secretRef`. Same-namespace `serverRef` never needs a grant. -- Deleting the grant is a live revocation: the next reconcile of any - `TerdutTeam` it used to authorize finds no matching grant, flips - `Ready: False, reason: RefNotPermitted`, and — deliberately — does **not** - delete the team server-side on revocation alone; it stops reconciling - further changes until access is restored or the `TerdutTeam` CR itself is - deleted (whose finalizer still needs the credentials Secret described in - §6 to clean up, so a revoked grant blocking *new* changes rather than - forcing an immediate, possibly credential-less deletion is the safer - failure mode). +- `from: None` (default) — no cross-namespace `TerdutTeam` may resolve a + `serverRef` into this `TerdutServer`. Same-namespace `TerdutTeam`s are + always allowed regardless of this field. +- `from: Same` — equivalent to `None` in effect (same-namespace is already + unrestricted) but kept for parity with the upstream enum and to make the + policy self-documenting in a diff. +- `from: All` — any namespace in the cluster may reference in. Appropriate + for a genuinely shared, cluster-wide `TerdutServer`; the audit trail is + "check `allowedTeams` plus who has RBAC to create a `TerdutTeam` + anywhere," which is materially weaker than `Selector`. +- `from: Selector` — only namespaces matching `spec.allowedTeams.namespaces.selector` + (a standard `metav1.LabelSelector` over `Namespace` objects, exactly like + `allowedListeners`'s own `selector`) may reference in. This is the + recommended mode for the platform-team-owns-a-shared-server scenario this + design targets: label the consuming namespaces once + (e.g. `terdut.ryuvia.com/allowed-server: platform-oncall/terdut`) and new + namespaces opt in by carrying the label, without editing the + `TerdutServer` again. +- A `TerdutTeam`'s controller re-evaluates `allowedTeams` on every + reconcile before it will resolve a cross-namespace `serverRef` — for + `Selector`, this means a `Get` on its own `Namespace` object plus + reading the target `TerdutServer`'s spec, not a List across the cluster. + Same-namespace `serverRef` never consults this field at all. +- Narrowing or clearing `allowedTeams` (or unlabeling a namespace, under + `Selector`) is a live revocation: the next reconcile of any `TerdutTeam` + it used to authorize finds itself no longer permitted, flips + `Ready: False, reason: RefNotPermitted`, and — deliberately — does + **not** delete the team server-side on revocation alone; it stops + reconciling further changes until access is restored or the `TerdutTeam` + CR itself is deleted (whose finalizer still needs the credentials + Secret described in §6 to clean up, so blocking *new* changes rather + than forcing an immediate, possibly credential-less deletion is the + safer failure mode). ## 5. Reconciliation semantics @@ -340,11 +368,11 @@ General rules for every controller: referenced parent isn't `Ready` yet, the child requeues with backoff and reports `Ready: False, reason: WaitingForTeam` — no cross-controller RPC. - **Cross-namespace `serverRef` is re-checked every reconcile, not just at - creation**: `TerdutTeam`'s controller lists `TerdutServerReferenceGrant` - objects in the target namespace on every pass (cheap: a namespaced List - with a field/label index, not a full-cluster scan) before touching the - cross-namespace `TerdutServer` — revocation (§4.6) takes effect on the - team's very next reconcile, not just when the CR is first applied. + creation**: `TerdutTeam`'s controller reads the target `TerdutServer`'s + `spec.allowedTeams` (and, under `Selector`, a `Get` on its own `Namespace` + object for labels) on every pass before touching a cross-namespace + `TerdutServer` — revocation (§4.6) takes effect on the team's very next + reconcile, not just when the CR is first applied. ## 6. Bootstrap & authentication to terdut-server's API @@ -375,17 +403,21 @@ this rather than shelling out to curl: 4. **Cross-namespace `TerdutTeam` never reads the source Secret directly.** Granting arbitrary consuming namespaces `get`/`list`/`watch` on a Secret in the server's namespace would mean *any* future workload in that - namespace with Secret-read RBAC could be pointed at it too — the - `TerdutServerReferenceGrant` (§4.6) only authorizes the `TerdutTeam` - *kind*, not "read this Secret". Instead, once a `TerdutServer` observes at - least one valid grant for a namespace, its own controller (which already - holds the real credentials, and whose ServiceAccount is the only thing - with legitimate cross-namespace write access — see §9) mirrors a copy of - the Secret into that consenting namespace, named - `.-terdut-credentials`, owned not by - an `OwnerReference` (those can't cross namespaces) but tracked in the - `TerdutServer`'s status and cleaned up explicitly when the grant - authorizing that namespace is removed. The remote `TerdutTeam`'s + namespace with Secret-read RBAC could be pointed at it too — `allowedTeams` + (§4.1, §4.6) only authorizes the `TerdutTeam` *kind* to resolve a + reference, not "read this Secret". Instead, the `TerdutServer` controller + (which already holds the real credentials, and whose ServiceAccount is + the only thing with legitimate cross-namespace write access — see §9) + watches `TerdutTeam` objects across the cluster for ones that both name + it and pass its `allowedTeams` check, and mirrors a copy of the + credentials Secret into each such namespace on demand — not proactively + into every namespace a `Selector`/`All` policy *could* admit, only into + ones an actual permitted `TerdutTeam` currently references. The mirror is + named `.-terdut-credentials`, owned + not by an `OwnerReference` (those can't cross namespaces) but tracked in + the `TerdutServer`'s status and cleaned up once no permitted `TerdutTeam` + in that namespace references it any more (revoked `allowedTeams`, or the + last referencing `TerdutTeam` deleted). The remote `TerdutTeam`'s controller reads only this local mirror, never the original. - This is one shared mirrored Secret per (server, consuming namespace) pair, not one per `TerdutTeam` — terdut-server's own API key isn't @@ -450,8 +482,9 @@ documented and tested operationally: - The operator's own ServiceAccount needs, per namespace it's granted: `get/list/watch/create/update/patch/delete` on `Deployments`, `Services`, `Secrets` it owns, and `get/list/watch` on `postgresql.acid.zalan.do` - (optional, degrade gracefully if absent per §8), plus `get/list/watch` on - `TerdutServerReferenceGrant` (§4.6). + (optional, degrade gracefully if absent per §8), plus cluster-wide + `get/list` on `Namespace` (labels only, for `allowedTeams: {from: Selector}` + evaluation — §4.1, §4.6). - No cluster-scoped resources are created by this operator (namespaced CRDs only, per §1) — a `Role` + `RoleBinding` per watched namespace is sufficient; a `ClusterRole` is only needed for watching CRDs across all @@ -467,8 +500,8 @@ documented and tested operationally: namespace), and should be called out explicitly in the operator's ClusterRole/RBAC review, not left implicit. No human or team's own RBAC is ever granted cross-namespace Secret access by this design — - `TerdutServerReferenceGrant` only ever authorizes the operator to act, - never a person. + `TerdutServer.spec.allowedTeams` only ever authorizes the operator to act + on a `TerdutTeam`'s behalf, never a person or a workload directly. - terdut-server's own RBAC is unaffected — the operator talks to it purely over HTTP with the bot user's API key, never via the Kubernetes API for app-level state. @@ -531,11 +564,6 @@ conflict Kubernetes operators exist to avoid. Proposed path: `TerdutDeadmanSwitch`, `TerdutAlertSource`) — only `TerdutTeam.serverRef` crosses namespaces in v1 (§2, §4.2, §4.6); these stay same-namespace as their `TerdutTeam` until a real need for splitting them out shows up. -- A `TerdutServerReferenceGrant`-style namespace selector (rather than an - enumerated list of namespaces) — deliberately omitted to match upstream - `ReferenceGrant`'s own choice of explicit, auditable consent over - pattern-matching (§4.6); revisit only if enumerating namespaces becomes - genuinely unworkable at scale. - Gitops-managed team *membership* (see §4.2). - Automatic Deployment restart on upstream Postgres credential rotation. - Admission webhooks / CEL-only validation limits (e.g. verifying a diff --git a/README.md b/README.md index 125da87..802981f 100644 --- a/README.md +++ b/README.md @@ -11,19 +11,16 @@ open questions it used to carry are now resolved decisions there (§2). ### terdutServers Creates a server — Deployment, Service, database wiring, bootstrap, operator -credentials. See DESIGN.md §4.1. +credentials, and `allowedTeams` consent for cross-namespace teams. See +DESIGN.md §4.1, §4.6. ### terdutTeams - team name - oidc groups - `serverRef` — explicit reference to its `TerdutServer`, may be in a different namespace (one team owns the server, others self-service a - team against it), gated by a `TerdutServerReferenceGrant` in the - server's namespace (DESIGN.md §2, §4.2, §4.6) - -### terdutServerReferenceGrants - - lives in the `TerdutServer`'s namespace; lists which other namespaces' - `TerdutTeam` objects may reference it (DESIGN.md §4.6) + team against it), gated by that `TerdutServer`'s own `allowedTeams` + field (DESIGN.md §2, §4.1, §4.2, §4.6) ### terdutEscalationrules - rule