diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6aff3bd..37bd5b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,11 +106,11 @@ jobs: # for the whole job. - name: Load CI image run: docker load -i "$RUNNER_TEMP/ci-image.tar" && rm -f "$RUNNER_TEMP/ci-image.tar" - # go mod tidy, go vet, golangci-lint, actionlint and hadolint. - - name: task tidy-check lint + # Module and browser-bundle integrity, then the linters, as in task verify. + - name: task tidy-check vendor-check lint run: | docker run --rm -v "$GITHUB_WORKSPACE:$CI_WORKDIR" -w "$CI_WORKDIR" \ - "$CI_IMAGE" bash -lc 'git config --global --add safe.directory "$PWD" && task tidy-check lint' + "$CI_IMAGE" bash -lc 'git config --global --add safe.directory "$PWD" && task tidy-check vendor-check lint' test: name: Unit tests diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e65ce8..1471b0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,14 +3,14 @@ ## Prerequisites Open the repository in its devcontainer. It provides Go, Task, golangci-lint, actionlint, -hadolint, `kubectl` and `k3d`, at the versions CI uses. +hadolint, Node, Helm, `kubectl` and `k3d`, at the versions CI uses. ```bash task # list every task -task test # go test -race ./... +task test # Go race tests, short fuzz runs, and Node tests for the browser helper task lint # go vet, golangci-lint, actionlint, hadolint task test-e2e # a real API server trusting a real Dex; see docs/testing.md -task verify # the whole gate, in CI's order; if this passes, CI passes +task verify # the local gate: the same checks CI runs in separate jobs ``` ## Design rules diff --git a/README.md b/README.md index 6415c83..a8f69c9 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,8 @@ krm-foyer is a backend for frontend (BFF) for browser applications built on Kube APIs. It owns OIDC login and server-side sessions, proxies Kubernetes API requests with the user's own credential, and hosts [krm-stream](https://github.com/ConfigButler/krm-stream) resource streams on the same -origin as your frontend. The browser never holds a cluster credential. +origin as your frontend. krm-foyer keeps its login and upstream credentials on the +server; the browser receives an opaque session cookie. KRM is the [Kubernetes Resource Model](https://github.com/kubernetes/design-proposals-archive/blob/main/architecture/resource-management.md): the idea that everything is a declarative resource with a spec and a status. krm-foyer is @@ -15,10 +16,12 @@ for applications whose domain is modelled that way. > **Status: a working prototype, not yet for a cluster that matters.** Sign-in through > OIDC, server-side sessions and the API proxy work, and e2e specs against a real API > server and Dex try to get past each security boundary. `task demo` runs an example -> application against them in your browser. Still missing: the bounds on request rate and -> response size that make it safe in front of a real cluster, token refresh, more than -> one replica, and streams. The [roadmap](docs/roadmap.md) tracks which of the principles -> below have tests. +> application against them in your browser. Request and response bounds, live streams +> and opt-in shared watches are implemented and tested. Sessions still live in memory: +> run one replica, expect a new login after a restart or ID-token expiry, and do not +> treat the e2e manifests as a production deployment. Certificate reload, token refresh, +> and shared session storage remain to be built; a Helm chart is being developed separately. +> The [roadmap](docs/roadmap.md) gives the next priorities and their required evidence. ## Try it @@ -45,13 +48,13 @@ change in one appear in the other, and a conflict when both edit the same note. ## Principles -- **The issuer decides who you are; Kubernetes decides what you may do.** Every request - reaches the API server with the user's own OIDC token, and RBAC and admission answer it. +- **The issuer decides who you are; Kubernetes decides what you may do.** Proxied API + requests and per-user watches use the user's own OIDC token; RBAC and admission answer them. No impersonation, and no fallback to krm-foyer's service account. A [shared watch](docs/watches.md), where configured, is opened once with an identity of its own, and the API server is asked about every user who reads it. -- **Tokens stay on the server.** The browser holds only an opaque session ID in a Secure, - HttpOnly cookie. Frontend code never sees a token. The session ID is itself a bearer +- **Login tokens stay on the server.** The browser holds an opaque session ID in a Secure, + HttpOnly cookie. Frontend code never receives those tokens. The session ID is itself a bearer credential, and is guarded like one. - **One domain is one trust boundary.** The application, krm-foyer and any domain backend share an origin, routed by path. Everything on that domain can act as the signed-in @@ -62,7 +65,7 @@ change in one appear in the other, and a conflict when both edit the same note. user's full Kubernetes access; give users grants that match the application. [Why, and what a scope would add](docs/application-scope.md) - **Kubernetes semantics, exactly.** Status codes, errors, patch types and conflicts pass - through unchanged, and nothing is retried on the user's behalf. + through unchanged. A mutation that reached Kubernetes is never automatically replayed. [Why](docs/heritage.md#what-broke-on-stage) ## Documentation @@ -73,6 +76,8 @@ change in one appear in the other, and a conflict when both edit the same note. | [docs/design.md](docs/design.md) | The contract: routes, access, sessions, upstream responses, streams and release criteria | | [docs/application-scope.md](docs/application-scope.md) | Why a browser application might be limited beyond RBAC, what that would block, and why krm-foyer starts without it | | [docs/roadmap.md](docs/roadmap.md) | What exists, what is next, and in what order | +| [docs/bounds.md](docs/bounds.md) | Limits, defaults, metrics and the scope of the capacity measurements | +| [docs/watches.md](docs/watches.md) | Choosing per-user streams, shared streams or native watches, and configuring sharing | | [docs/bff-choice.md](docs/bff-choice.md) | Whether your application should use a universal BFF like this one, a domain backend, or both | | [docs/ingress.md](docs/ingress.md) | Terminating TLS itself or behind an ingress, sharing one domain with other services, and the login gate for an ingress | | [docs/frontend.md](docs/frontend.md) | Which pages krm-foyer serves itself, and why it ships no single-page application | @@ -90,7 +95,7 @@ task verify # everything CI checks, including e2e against k3d and Dex ``` Without flags, krm-foyer serves its start page and probes only. Sign-in and the API -proxy come together, and need: +proxy and streams come together, and need: | Flag | | | --- | --- | @@ -104,7 +109,12 @@ Optional: `-oidc-ca-file` and `-kubernetes-ca-file` (CAs to trust), `-oidc-scope [bounds](docs/bounds.md) each have a flag and a documented default: the request rate and concurrent requests per session, concurrent requests per replica, how long a response may stay open, its size, and how often an open response checks its session. Sessions -live in memory: run one replica. +live in memory: run one replica. Shared watches are opt-in; see their +[configuration and identity requirements](docs/watches.md#setting-up-shared-watches). +The OIDC redirect URI to register is `/auth/callback`. The public URL, +issuer and Kubernetes server must use HTTPS. Mounted TLS certificates, CA bundles and +the OIDC client secret are read at startup; restart after changing them. The explicitly +configured shared-watch token file is reread by client-go for rotation. Or as a container: diff --git a/SECURITY.md b/SECURITY.md index d33d4b4..d25e634 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -20,13 +20,15 @@ receives security fixes. krm-foyer holds users' sessions and forwards their requests to a Kubernetes API server, so we treat the following as security bugs: -- **A token reaching the browser.** An ID, access or refresh token in a response body, +- **A login or shared-watch token reaching the browser.** A token krm-foyer holds in a response body, header, readable cookie, log line or error message. - **A session ID leaking.** The session ID is a bearer credential: one in a log line, a URL, an error page, or anywhere outside the cookie is a vulnerability. - **krm-foyer deciding access.** A request answered differently through krm-foyer than the - API server answers the same token directly, or a non-canonical path forwarded instead of - rejected. + API server answers the same token directly, except the documented + [interruptions](docs/design.md#interruptions), or a non-canonical path forwarded + instead of rejected. Shared streams must follow their documented review and + reauthorization contract. - **A request made with the wrong credential.** Another user's, or the service's own service account standing in for a user. - **A shared watch read without the API server's say.** A stream served from a shared @@ -48,7 +50,9 @@ we treat the following as security bugs: ## What does not - Anything a user can do with permissions Kubernetes RBAC already grants them, including - through the browser. krm-foyer has no [application scope](docs/application-scope.md) + through the browser, including reading Secrets or creating tokens where those + permissions are granted. The server-held-token guarantee is not a filter on + authorized Kubernetes resource data. krm-foyer has no [application scope](docs/application-scope.md) yet, so a session carries the user's full access by design. Enumerating a namespace you are allowed to list is not a leak. - Denial of service by an already authenticated user within the configured limits. diff --git a/Taskfile.yml b/Taskfile.yml index a61bc39..a63da6e 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -7,8 +7,8 @@ vars: IMAGE: '{{.IMAGE | default "ghcr.io/configbutler/krm-foyer"}}' # The krm-stream browser bundle the hello example vendors, and npm's integrity for # its package. Bump both together, then `task vendor-krm-stream`. - KRM_STREAM_VERSION: 0.6.0 - KRM_STREAM_INTEGRITY: sha512-8vGYVE3np6Ec0bs+Bd0/Y7+PqPp4sQ2YRwbR3Y/VrPPeiIxcHyKt8GHtHa9uUqCp59YXgPALMIJwRh/RXIxhFQ== + KRM_STREAM_VERSION: 0.7.0 + KRM_STREAM_INTEGRITY: sha512-LFXyJIv4UmlBBEJEdx68wzwaqUIVai5HbDe70ACTWIpFZexlJbd/wyhzryoW5uBg55y/f+dZ4bapajybPJ7c0w== VERSION: sh: git describe --tags --always --dirty 2>/dev/null || echo dev diff --git a/docs/alternatives.md b/docs/alternatives.md index 4c5313b..88b44d3 100644 --- a/docs/alternatives.md +++ b/docs/alternatives.md @@ -24,10 +24,14 @@ Use it when all of these hold: - **The user is in a browser**, in an application you build, not in `kubectl` or a dashboard someone else built. -- **Kubernetes should decide with the user's identity.** No service account acts for - the user, and nothing impersonates them. -- **No token reaches the browser.** A script injected into the page can act as the user - while the page is open. It cannot carry a credential away. +- **Kubernetes should decide with the user's identity.** API requests use the user's + token. Opt-in shared watches use a separate identity and API-server reviews of every + subscriber; no service account substitutes for a user's API request, and nothing + impersonates them. +- **Login tokens stay on the server.** A script injected into the page can still act as + the user and read anything their grants allow, including Secrets or token-issuing + APIs if granted. HttpOnly protects the session cookie; it does not narrow Kubernetes + access. See [application scope](application-scope.md). - **Your API server trusts an OIDC issuer**, or you can make it trust one. It is weaker when you want a ready-made UI, a CLI, or a cluster whose API server you @@ -114,8 +118,10 @@ header per change, in a browser with no Node polyfills. paths (no token, a proof on every change, a resend only of krm-foyer's own [refusal](design.md#interruptions)) stay short enough to audit, and they are tested under `node --test` ([foyer.test.js](../internal/pages/foyer.test.js)). -- **Watches need no library.** A `fetch` whose body is read line by line, resumed from - the last `resourceVersion` and re-listed on 410 Gone, is about 40 lines. +- **Live views use krm-stream.** The helper handles sessions and API calls; the + [hello example](../examples/hello) uses krm-stream for snapshots, recovery and drafts. + Native watches remain available for clients that already implement that protocol; + see [which watch to use](watches.md). - **For types, use `kubernetes-models` with `import type`.** It costs nothing at run time, is current, and covers CRDs. client-node's model classes do not fit, because they turn timestamps into `Date` objects and no longer match the JSON. diff --git a/docs/bounds.md b/docs/bounds.md index 14356be..4513d06 100644 --- a/docs/bounds.md +++ b/docs/bounds.md @@ -12,7 +12,7 @@ A *bound* is something krm-foyer limits, such as how long a response may stay op - **Native watches stay open, and are not a special case.** Every bound applies to every request through `/k8s`, so krm-foyer never has to tell a watch from any other request. - Live views in applications belong to krm-stream ([roadmap step 5](roadmap.md#order-of-work)); + Live views in applications belong to krm-stream ([watches](watches.md)); the native watch stays available underneath it. See [native watches](#native-watches). - **Five bounds**, each configurable with a documented default: the request rate per session, concurrent requests per session and per replica, how long a response may @@ -55,7 +55,7 @@ They are not three other things: **Every bound is per replica.** krm-foyer keeps no shared counters. With one replica, as today, that is the whole deployment. Once there are several -([step 6](roadmap.md#order-of-work)), a session whose requests reach N replicas gets up +([shared storage on the roadmap](roadmap.md#order-of-work)), a session whose requests reach N replicas gets up to N times its per-session limits, and the per-replica limits are what hold. The per-replica bounds on concurrent requests are the ones that protect krm-foyer itself, @@ -72,8 +72,8 @@ stream into events, reconnect when it ends, and resume from the last `resourceVe without losing or repeating a change. krm-stream does that work once, for every application. It is the path krm-foyer -recommends for live views ([step 5](roadmap.md#order-of-work)), and the hello example -moves to it. So the question was whether native watches through `/k8s` should stay at +recommends for live views ([watches](watches.md)), and the hello example +uses it. So the question was whether native watches through `/k8s` should stay at all. **They stay**, for three reasons: - **Refusing them would be an access rule.** RBAC already says who may watch what. @@ -82,7 +82,7 @@ all. **They stay**, for three reasons: - **They work today**, against the real API server, and a spec proves it. - **They cost almost nothing extra.** What they need, an end when the session ends and cancellation at the API server, every long response needs too: a followed log, or a - slow list open at logout. krm-stream's user-authenticated watches will need the same. + slow list open at logout. krm-stream's per-user streams use the same gate. What is not offered: a watch client in the [browser helper](frontend.md), or an example that reads native watches. That would be a second, lesser krm-stream. @@ -230,8 +230,9 @@ within one interval of its session ending, plus at most one more for a check tha 5 seconds normally, 10 at worst, with the defaults. Each open response reads the store once per interval: nothing for the memory store, and -some 400 reads a second at the per-replica limit of 2000 once the store is shared -([step 6](roadmap.md#order-of-work)). +up to about 800 reads a second when both the 2000-request and 2000-stream limits +are occupied, once the store is shared +([shared storage on the roadmap](roadmap.md#order-of-work)). ### Response bytes @@ -311,8 +312,19 @@ apart again: blocked write would also hold off the recheck that applies a revoked grant. A test stops reading, fills the buffers, revokes the grant and sees the stream end. -What is not bounded yet: a native watch through `/k8s` whose tab stops reading holds -its slot until the response duration ends it. +What is not bounded yet: + +- A native watch through `/k8s` whose tab stops reading has no separate write timeout; + its session ending or response duration still cuts it short. +- Snapshot size, per-stream object bookkeeping and shared-cache bytes have no foyer + limit. `-max-response-bytes` applies only to `/k8s`; it does not protect `/stream`. + A few large scopes can therefore exhaust memory below the stream-count limit. +- Active sessions have no count limit. The 10,000 pending-login cap bounds login + transaction storage, but is not a rate limit on anonymous login/callback traffic. + The API gate's per-session rate applies to `/k8s` and `/stream`, not `/auth`. + +These are capacity/admission questions for the [next work](roadmap.md#order-of-work), +not guarantees established by the small-snapshot rehearsal. ## Measured: the rehearsal @@ -432,8 +444,8 @@ What is left out on purpose: - **No automatic HTTP instrumentation.** Middleware of that kind labels requests by route, and under `/k8s` the path names namespaces and objects: unbounded label values, and a record of what each user looks at, kept outside the audit log. krm-foyer's - metrics are labelled only by bound, cause and interruption reason, fixed sets known in - advance. Details of a single request belong in its log line. + metrics use fixed label sets: bound, cause, interruption reason, watch identity + class, access-check source and result. They never label a series with a user or scope. Details of a single request belong in its log line. - **No tracing yet.** Tracing is where OpenTelemetry is the right choice, and it would come with its tracing SDK only, not its metrics. But the API server accepts a W3C `traceparent`, and forwarding the browser's would let a page write into the cluster's @@ -534,6 +546,7 @@ Rules of thumb for tuning: - **Lower a limit when the 99th percentile stays far below it** for weeks: the room above it serves only runaway pages. - **Expect `session_ended` at logout and at expiry**, one for each response the tab had - open. Idle expiry cannot cause it, since reconnects renew the session; the absolute - timeout and the token's expiry can. Far more of them than sessions ending means - something else: a session store that does not answer the check in time. + open. Ordinary reconnects renew idle time, but failed or refused reconnects need not; + idle, absolute and token expiry can all end a session. One session can own many + responses, so compare against open-response counts rather than expecting one cut + per logout. Store checks that fail or time out are another cause. diff --git a/docs/design.md b/docs/design.md index 33d3fb4..1805b13 100644 --- a/docs/design.md +++ b/docs/design.md @@ -5,10 +5,11 @@ APIs. It owns OIDC login and server-side sessions, proxies Kubernetes API reques the user's own credential, and hosts krm-stream resource streams on the frontend's origin. -This document is the contract: what krm-foyer must do. **Most of it is not implemented -yet.** Every guarantee below is a requirement until the test that tries to break it -exists; the [roadmap](roadmap.md) tracks which do. Why the service exists, and what it expects from -the domains behind it, is in the [vision](vision.md). +This document is the contract: what krm-foyer must do. Login, single-replica sessions, +the API proxy, bounds, streams and opt-in shared watches have implementation and tests. +Planned routes and lifecycle requirements are marked below; the [roadmap](roadmap.md) +tracks the evidence still missing. A requirement is not a delivered guarantee until +its test exists and passes. The [vision](vision.md) explains why the service exists. ## Architecture and ownership @@ -32,18 +33,20 @@ boundary, since it can act as the signed-in user; see The initial design uses one configured cluster per deployment and a configurable OIDC provider. No particular identity provider or frontend framework is required. -krm-foyer terminates TLS itself from a mounted certificate, reloaded when it changes, or -sits behind an ingress that terminates TLS. Both are supported, but they are not equally -safe: behind an ingress, the hop to krm-foyer carries session cookies, and in plain HTTP -anything that can observe that hop can take a session. Plain HTTP is acceptable only +krm-foyer terminates TLS itself from a mounted certificate, or +sits behind an ingress that terminates TLS. Certificates are currently read at startup; +reload on rotation is a planned requirement. Both network models are supported, but +they are not equally safe: behind an ingress, the hop to krm-foyer carries session +cookies, and in plain HTTP anything that can observe that hop can take a session. Plain HTTP is acceptable only where nothing but the ingress can reach krm-foyer; otherwise the ingress re-encrypts and verifies krm-foyer's certificate. See [both TLS models](ingress.md#decision-2026-10-01-both-tls-models). Its public URL is configuration: the OIDC redirect URI, same-origin and CSRF checks and return paths use it, never `Host` or `X-Forwarded-*`. An ingress in front must be transparent: no buffering of streams, and no authentication or header rewriting of its own on krm-foyer's -routes. An ingress's external-authentication feature (`auth_request`, ForwardAuth) is used only as a -login gate for the application's pages, through `/auth/check`. It never decides on `/k8s` or -`/stream` traffic. The [ingress decision](ingress.md) explains why, and what would make +routes. The planned `/auth/check` lets an ingress's external-authentication feature +(`auth_request`, ForwardAuth) act only as a login gate for the application's pages. +Until it exists, the helper's `requireSession()` handles that navigation. A login gate +never decides on `/k8s` or `/stream` traffic. The [ingress decision](ingress.md) explains why, and what would make more worth revisiting. krm-foyer has two halves. The **login half** (`/auth/...`) obtains the user's OIDC token @@ -87,13 +90,18 @@ come from krm-stream, not from a reimplementation here. | `/auth/callback` | Validate the callback and establish a session, then `303` to the return path | | `/auth/session` | Return minimal identity/session state and CSRF information, never bearer tokens: `200` with `authenticated`, `issuer`, `subject`, `email`, `expiresAt`, `csrfToken` and `csrfHeader`, or `401` with `{"authenticated":false}` | | `/auth/logout` | CSRF-protected POST that destroys the server session and answers `204`; the caller then goes where it likes, `/auth/logged-out` by default | -| `/auth/check` | 204 or 401 (or 302 to login on request) for an ingress gating the application's pages; never a token or identity. See the [login gate](ingress.md#decision-2026-10-01-a-login-gate-for-the-applications-pages) | +| `/auth/check` **(planned)** | 204 or 401 (or 302 to login on request) for an ingress gating the application's pages; never a token or identity. See the [login gate](ingress.md#decision-2026-10-01-a-login-gate-for-the-applications-pages) | | `/k8s/api/...` | Proxy core Kubernetes APIs after stripping `/k8s` | | `/k8s/apis/...` | Proxy grouped APIs, including CRDs and aggregated APIs | | `/k8s/api`, `/k8s/apis`, `/k8s/version`, `/k8s/openapi/...` | Proxy discovery and schema endpoints | -| `/stream/v1` | A [krm-stream](https://github.com/ConfigButler/krm-stream/blob/main/spec/v1.md) resource stream (`GET`, the scope in the query), with the watch opened as the user: whatever RBAC lets the user watch, and nothing krm-foyer lists. See [streams and editing](#streams-and-editing) | -| `/auth/whoami` | Who Kubernetes takes the user to be, from a SelfSubjectReview, plus the session's issuer and expiry. Never tokens | -| `/_foyer/access` | A page showing what the user may do, from Kubernetes' own reviews. See [what may I do](#what-may-i-do) | +| `/stream/v1` | A [krm-stream](https://github.com/ConfigButler/krm-stream/blob/main/spec/v1.md) resource stream (`GET`, the scope in the query): a watch opened as the user, or an opt-in shared watch guarded by API-server reviews. See [streams and editing](#streams-and-editing) | +| `/auth/whoami` **(planned)** | Who Kubernetes takes the user to be, from a SelfSubjectReview, plus the session's issuer and expiry. Never tokens | +| `/_foyer/access` **(planned)** | A page showing what the user may do, from Kubernetes' own reviews. See [what may I do](#what-may-i-do) | + +Planned routes currently return 404. The start page (`/`), logout confirmation +(`/auth/logged-out`), probes (`/healthz`, `/readyz`) and static assets under `/_foyer/` +also exist. `/readyz` records successful initial issuer discovery; it is not a live +check of the issuer, API server or shared-watch permissions. Metrics are served on a listener of their own (`-metrics-listen`), never on the origin; see [metrics](bounds.md#metrics). @@ -169,8 +177,9 @@ subresources, which return whatever their backend sends: A held-back response is answered as an [interruption](#interruptions): code gets a `Status` with status 502 and the reason, and a person browsing gets a page that -explains it. For a redirect, both carry the target; for HTML, the page shows its source -as escaped text. Every interruption is logged. +explains it. For a redirect, both carry the target. For held-back HTML, the page names +the content type; a bounded, escaped source preview is planned. Every interruption is +logged ([how](#interruptions)). ### Interruptions @@ -193,8 +202,9 @@ answer, it says so in a form the requester can read: | A [bound](bounds.md) reached | 429 for the request rate, for concurrent requests and for streams open; 502 for a response known to exceed its size bound before it starts | `Status` naming the bound, with `Retry-After` for the request rate | A page saying so | No for a 429; yes for a 502 | | API server unreachable | 502 | `Status` | A page saying so | Perhaps: a connection can fail after the request was sent | -This table is the complete list of answers krm-foyer gives instead of the API server's. -Anything not in it is the API server's answer. +This table lists interruptions from the API gate and proxy. Once `/stream/v1` starts, +its protocol uses the [stream errors](#streams-and-editing) below. Login routes have +their own [failure responses](#login). Rules: @@ -219,7 +229,14 @@ Rules: - **Answers from Kubernetes are never replaced.** A 403 from RBAC, a 404 or a 409 is the API server's answer and reaches the tab as its JSON. Interruptions are only what krm-foyer itself decided. For a 403, [`/_foyer/access`](#what-may-i-do) is where a - person finds out why. + person will be able to investigate the refusal once that page exists. +- **Every refusal is one log line**, whoever refused: `"msg":"refused"`, with `by` + (`krm-foyer` or `kubernetes`), the `user` as the issuer named them once the request + has a session, the `route`, method and path (never the query), and the reason. That is + an interruption below 500; the API server's 401, 403, 409 or 422 on `/k8s`; and a + stream's `FORBIDDEN`, `UNAUTHENTICATED` or `SCOPE_INVALID`, with the scope asked for. + An interruption from 500 is a failure, not a refusal, and is logged as + `"msg":"interruption"`. What the API server wrote is never logged. - **Permission is a link, never an action.** Following a redirect is a navigation the user starts; nothing is re-sent, and no request with a body is ever continued. An upstream HTML page is never rendered on the origin, with or without consent: consent @@ -276,7 +293,7 @@ SelfSubjectAccessReview through `/k8s` says whether the user may do one thing, a SelfSubjectRulesReview lists what they may do in a namespace. Without a scope layer, those native answers are exactly what requests through krm-foyer will get. -For a person with a browser tab, krm-foyer adds one page, `/_foyer/access`: +For a person with a browser tab, the planned `/_foyer/access` page will provide: - **The rules for a namespace,** from a SelfSubjectRulesReview sent with the user's own token, as a table of resources and verbs. When Kubernetes marks the answer incomplete, @@ -306,8 +323,8 @@ what may be stored and what may be processed before exposing creates. Tokens and session contents stay server-side. The browser holds only an opaque session ID in a Secure, HttpOnly, host-scoped cookie with appropriate SameSite settings. Server-side -sessions support per-session revocation and refresh-token custody. They require shared -storage across replicas, bounded logout propagation and a defined failure policy. +sessions support per-session revocation. Refresh-token custody and shared storage +across replicas are planned, with bounded logout propagation and a defined failure policy. **The session ID is a bearer credential.** Whoever holds it can call krm-foyer as the user, from anywhere, until the session ends. `HttpOnly` keeps it from page scripts and @@ -412,7 +429,10 @@ from making requests as the user. ### Session lifecycle -Revocation is as fast as its slowest path. Each row is a requirement with a test: +Revocation is as fast as its slowest path. The table includes future requirements: +refresh and shared storage do not exist yet. Logout and open-response cancellation +are tested with the current in-memory store on one replica; store failures are tested +with unit-test doubles. The [roadmap](roadmap.md#login-and-sessions) tracks the rest. | Event | Required behavior | | --- | --- | @@ -421,11 +441,17 @@ Revocation is as fast as its slowest path. Each row is a requirement with a test | Session store unavailable | Fail closed: API and stream requests get 503. Never a stale local copy, never anonymous, never the service account | | Responses and streams open at logout | Cut short on every replica within the [session-check interval](bounds.md#the-session-check) (`-session-check-interval`, 5 seconds by default, twice that at worst when the check itself hangs), and cancelled at the API server | | Token expiry or failed refresh | API requests get 401; open responses and streams are cut short within the session-check interval of the token's expiry or the session's, whichever comes first | -| RBAC change | Kubernetes applies it on the next request. A shared watch applies it within its SubjectAccessReview recheck interval; that is reauthorization, not revocation | +| RBAC change | Kubernetes applies it on the next request. A shared stream applies it within the [full reauthorization budget](watches.md#revocation), 60 seconds at defaults, including decision reuse, a check and a write; a per-user watch is checked again when it reopens | | Issuer refuses a refresh | The session ends at once: API requests get 401 and its streams close. krm-foyer never retries a refusal into a success or keeps using the old token past its expiry | | User disabled at the issuer | Provider-dependent; see below. The only bound krm-foyer itself guarantees is the session's absolute expiry | -Disabling a user reaches krm-foyer only as a refused refresh. Until then the API server +Today, without refresh, disablement at the issuer does not actively end a session: +the earlier of session expiry and ID-token expiry ends its use here. A shared stream's +reviews recheck permissions for the subject captured when it opened; they do not +re-resolve issuer group membership. + +With refresh implemented, disabling a user can reach krm-foyer as a refused refresh. +Until then the API server accepts the ID token krm-foyer already holds, so the user keeps access for the rest of that token's lifetime **plus** however long the issuer goes on granting refreshes after the account is disabled. That second part belongs to the issuer, and OAuth leaves it to @@ -540,7 +566,7 @@ of partial success. | Reuse | Two frontends using different API groups without application-specific backend handlers | | Authentication | Callback failure, expiry, refresh, restart and logout tests across replicas | | Session lifecycle | Every bound in [session lifecycle](#session-lifecycle) measured by a test, including logout racing refresh, store outage, streams open at logout and a refused refresh; a disablement bound is claimed only for issuer configurations where a test measured it | -| Credential custody | No token krm-foyer holds, including refreshed ones, and no session ID in any response, log line or error page the suite collects | +| Credential custody | No token krm-foyer holds, including refreshed ones, in any collected response or log; session IDs appear only in the session cookie's issuing `Set-Cookie` header | | Access | Every answer through krm-foyer equals the API server's answer for the same token, except the [interruptions](#interruptions), each with a test that it happens exactly when the table says; non-canonical paths are rejected; nothing falls back to the service account | | Proxy semantics | Kubernetes errors and patch types preserved; conflicting writes and ambiguous create outcomes handled without automatic replay | | Upstream responses | An upstream that sends HTML, `Set-Cookie`, CORS headers, cache headers or a redirect has none of them reach the browser unasked; a gzip response arrives decoded, and the size bound holds for its decoded bytes | diff --git a/docs/frontend.md b/docs/frontend.md index 96aacfc..73536b5 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -24,14 +24,17 @@ browser is on a krm-foyer URL and no application page exists yet: ## What ships in the binary +The table distinguishes implemented pages from planned ones. Planned routes currently +return 404; they are requirements, not configuration switches. + | Path | Page | Why it is ours | | --- | --- | --- | | `/` | Start page, for requests that reach krm-foyer itself: a port-forward, or before routing is set up | Confirms the deployment works and shows what to route next. It links to the docs and to the session page. Behind an ingress, `/` belongs to the application and this page is never seen. | | `/auth/login?return_to=/path` | No page: a redirect to the OIDC provider | The return path is validated as a local path before it is stored in the login transaction. | | `/auth/callback` | A redirect to the stored return path on success. On failure, an error page with a stable reason and a "try again" link | An application cannot render this: its code is not loaded yet. | | `/auth/logged-out` | Plain confirmation with a "sign in again" link, and a note that the issuer may still have the user signed in | Where logout lands when the application does not supply its own destination. `POST /auth/logout` answers `204`, and the caller navigates here or to its own page. | -| `/auth/whoami` | Who Kubernetes takes you to be (username, groups and extra, from a SelfSubjectReview), with the session's issuer and expiry. Never tokens | The first thing to check when Kubernetes answers 403. It shows the API server's view, not the token's claims, because that is what RBAC matches. `/auth/session` stays the JSON form for code. | -| `/_foyer/access` | What you may do in a namespace, from a SelfSubjectRulesReview, and a "can I?" form answered by a SelfSubjectAccessReview | `kubectl auth can-i` for someone with only a browser. Code asks the same reviews natively. See [what may I do](design.md#what-may-i-do). | +| `/auth/whoami` **(planned)** | Who Kubernetes takes you to be (username, groups and extra, from a SelfSubjectReview), with the session's issuer and expiry. Never tokens | A way to investigate a 403. `/auth/session` already returns login/session information, but does not resolve the Kubernetes identity. | +| `/_foyer/access` **(planned)** | What you may do in a namespace, from a SelfSubjectRulesReview, and a "can I?" form answered by a SelfSubjectAccessReview | `kubectl auth can-i` for someone with only a browser. Code asks the same reviews natively. See [what may I do](design.md#what-may-i-do). | | `/k8s/...`, `/stream` when krm-foyer interrupts | For a browser navigation only: sign in, redirect notice, held-back content, store unavailable. Same status code as the JSON form | The proxy is explorable in a tab. These say what krm-foyer decided, as opposed to what Kubernetes answered. See [interruptions](design.md#interruptions). | | `/healthz`, `/readyz` | Plain text | For probes, not people. | | `/_foyer/...` | The pages' stylesheet and images | The one prefix krm-foyer reserves for its own pages and files that are not part of login, so they never collide with an application on the same origin. | @@ -43,8 +46,9 @@ Rules for these pages: add an npm dependency tree to a security-sensitive binary. - **No inline script and a strict Content-Security-Policy.** The service that holds session cookies should be the easiest one to audit. -- **Every page is replaceable.** Operators can point the logged-out and error destinations - at their own application URLs. The built-in pages are defaults, not branding. +- **The application chooses where to go after logout.** The helper's `logout(next)` + accepts a destination. Callback failures currently use the built-in error page; + configurable error destinations and page replacement are not implemented. - **Nothing reveals configuration to anonymous visitors.** The start page says which features are switched on, never issuer secrets or cluster addresses. diff --git a/docs/heritage.md b/docs/heritage.md index 1287d88..1dd629f 100644 --- a/docs/heritage.md +++ b/docs/heritage.md @@ -18,7 +18,7 @@ Voter settled several questions that krm-foyer's design now takes as given: | --- | --- | | The first design (March 2026) used an auth service with `Impersonate-User`. It was removed: impersonation lets the application write its own audit provenance | Requests go to Kubernetes with the user's own credential. There is no impersonation and no service-account fallback | | Tokens stay server-side. JavaScript receives identity and a CSRF token from `/auth/session`, never a bearer token | [Login and sessions](design.md#login-and-sessions) | -| `/auth/whoami` (SelfSubjectReview) is the first thing to check when Kubernetes answers 403 | [/auth/whoami](frontend.md#what-ships-in-the-binary) | +| `/auth/whoami` (SelfSubjectReview) is the first thing to check when Kubernetes answers 403 | Planned [/auth/whoami](frontend.md#what-ships-in-the-binary) | | One shared watch can serve two hundred browsers, if each subscriber is re-checked with a SubjectAccessReview | [Streams and editing](design.md#streams-and-editing) | | A 30-second SubjectAccessReview recheck covers RBAC for the captured subject. It does not cover identity-provider changes, and must not be described as revocation | [Session lifecycle](design.md#session-lifecycle) | @@ -61,7 +61,8 @@ means inheriting its semantics exactly, not approximately.* [krm-stream](https://github.com/ConfigButler/krm-stream) owns the watch-to-browser protocol, shared watches, projections, drafts and reconciliation. Voter was its first real consumer, at 0.4.0, and krm-foyer its second: krm-stream 0.5.0 and 0.6.0 took its -[feedback](investigations/krm-stream-feedback.md). krm-foyer hosts krm-stream; it does not +[initial feedback](investigations/krm-stream-feedback.md), and 0.7.0 took the +[shared-watch fixes](investigations/krm-stream-shared-watches.md). krm-foyer hosts krm-stream; it does not reimplement any of it. Problems found while building on it are reported to krm-stream, the way Voter did in its consumer feedback notes. diff --git a/docs/ingress.md b/docs/ingress.md index aaa1249..eb7711d 100644 --- a/docs/ingress.md +++ b/docs/ingress.md @@ -12,7 +12,7 @@ krm-foyer supports two ways of being deployed. They are **not** equally safe: | Model | How | When | What it assumes | | --- | --- | --- | --- | -| **krm-foyer terminates TLS** | Certificate and key are mounted files, for example from cert-manager. krm-foyer reloads them when they change, so rotation needs no restart | The default | Nothing beyond the usual: no hop carries a session cookie in plain text | +| **krm-foyer terminates TLS** | Certificate and key are mounted files, for example from cert-manager. Currently a restart loads a changed certificate; automatic reload is planned | Used by the e2e fixture | Nothing beyond the usual: no hop carries a session cookie in plain text | | **Behind an ingress** | The ingress or gateway terminates the browser's TLS and forwards to krm-foyer | An organization that routes everything through one ingress, or a team that already manages certificates there | The ingress is trusted, and the hop behind it is protected (see below) | Behind an ingress, every request on the hop to krm-foyer carries the session cookie, and @@ -27,7 +27,8 @@ hop is one of two things: NetworkPolicy admits the ingress's pods to krm-foyer's port and nothing else, and the cluster network is not observable by other tenants. Anything that can read that hop can take a session. Operators choosing this accept that assumption explicitly; the - chart's default is the policy, not an open port. + chart being developed separately must default to that policy. Until its deployment + contract is tested, supply and verify the policy in the deployment. What both models do share is that **krm-foyer never works out its own public address from the request.** It is configured with its public URL. That URL is the @@ -35,18 +36,20 @@ OIDC redirect URI, the origin that CSRF and same-origin checks compare against, base for return paths. `Host` and `X-Forwarded-*` headers do not change any of those, so a spoofed header cannot poison a redirect or the CSRF origin. That protects against header tricks; it does nothing for a cookie read off an unencrypted hop. Forwarded client -addresses are used only for logging and rate limiting, and only when they come from proxy -addresses the operator configured. +addresses are not used for logging or rate limiting. Current rate limits are keyed by +session, and there is no trusted-proxy-address configuration. Session cookies are always `Secure`, in both models. Behind an ingress the browser still -sees HTTPS, and browsers accept `Secure` cookies from `http://localhost`, so local -development needs no exception. +sees HTTPS. Login configuration requires an HTTPS public URL, including locally; +`task demo` supplies TLS for the browser. Plain `task run` serves only the start page, +assets and probes. An ingress in front of krm-foyer has to stay transparent: - **No response buffering** on `/k8s` watches and `/stream`, and read timeouts longer - than a watch. krm-foyer sends `X-Accel-Buffering: no` on streaming responses, which - nginx honors. Other proxies need their own setting. + than a watch. krm-stream sends `X-Accel-Buffering: no` on `/stream/v1`; the `/k8s` + proxy flushes every chunk but does not emit that header. Configure buffering off + for both routes in the ingress rather than depending on a response header. - **No authentication of its own** on krm-foyer's routes. The ingress may limit request sizes and rates, but it must not add or rewrite `Authorization` or `Impersonate-*` headers. @@ -56,8 +59,10 @@ An ingress in front of krm-foyer has to stay transparent: The e2e fixture runs krm-foyer with its own TLS, because that is the default, and its browser specs already reach krm-foyer through a gateway (Traefik, re-encrypting). The -ingress model gets one more e2e spec through that gateway. It checks two things: a spoofed `Host` or `X-Forwarded-Host` changes neither the redirect -URI nor the CSRF origin, and watch events arrive without delay. +dedicated ingress specs remain pending: spoofed `Host` or `X-Forwarded-Host` must +change neither the redirect URI nor the CSRF origin, and watch events must arrive +without buffering. The existing browser journeys exercise routing and live updates, +but do not replace those targeted checks. ## Decision (2026-10-01): sharing one domain with other services @@ -203,6 +208,9 @@ credential to code that krm-foyer does not test. ## Decision (2026-10-01): a login gate for the application's pages +**Planned, not implemented.** `/auth/check` currently returns 404. The behavior below +is the proposed contract; use the implemented `requireSession()` helper today. + The ingress can ask krm-foyer one question before serving the application's pages: *is this browser signed in?* If it is not, the browser goes to the login, and comes back to the page it asked for. Every frontend needs this, and it is wasteful to write it in each @@ -226,7 +234,10 @@ Ingresses differ in what they do with a refusal, which is why there are two refu - **Traefik ForwardAuth and Envoy's external authorization** pass the check's response to the browser, so they use `?redirect=true`. -Gateway API has no standard external-auth filter yet. Without one, the browser helper's +Gateway API introduced an experimental `externalAuth` filter in +[v1.4](https://kubernetes.io/blog/2025/11/06/gateway-api-v1-4/); support depends on the +installed CRDs and gateway implementation. This fixture does not configure it. +Without a usable page gate, the browser helper's `requireSession()` does the same at page load, in a few lines of JavaScript. The gate is an improvement for whoever has it, not a requirement. @@ -280,8 +291,9 @@ The ingress then forwards the request to the API server with that token. - **Streams do not fit.** `/stream` needs the principal inside krm-stream, not a yes or no in front of it. - **Every ingress is different.** nginx, Traefik, Envoy and Gateway API each have their - own external-auth configuration, and each would need its own tests. Gateway API has no - standard for it yet, and the Kubernetes project has retired ingress-nginx, the most + own external-auth configuration, and each would need its own tests. Gateway API's + experimental filter also needs implementation-specific validation. The Kubernetes + project has retired ingress-nginx, the most common place `auth_request` was configured. - **We have already tried this.** Voter's first design (March 2026) was Traefik ForwardAuth deciding from a cookie Voter issued, followed by impersonation. It was @@ -361,5 +373,5 @@ auth proxy would make krm-foyer less opinionated, much larger, and impossible to across all of those combinations. The flexibility people need is in the network, not in who decides. Both TLS models are -supported, a transparent ingress is fine, the ingress can gate pages on the login, and +supported, a transparent ingress is fine, the planned login gate can cover page loads, and the seam for external login exists if it is ever needed. diff --git a/docs/investigations/choosing-an-issuer.md b/docs/investigations/choosing-an-issuer.md index 4ef1174..1b1c9e4 100644 --- a/docs/investigations/choosing-an-issuer.md +++ b/docs/investigations/choosing-an-issuer.md @@ -22,7 +22,7 @@ its source. work.** krm-foyer has no Dex-specific code. - **Several issuers need token refresh, which krm-foyer does not do yet.** Pinniped and OpenUnison issue tokens that last minutes. Today a krm-foyer session ends when its ID - token expires. Refresh is [roadmap step 6](../roadmap.md#order-of-work), and krm-foyer + token expires. Refresh is [roadmap](../roadmap.md#order-of-work), and krm-foyer must support it before those issuers are a comfortable choice. ## What krm-foyer needs from an issuer @@ -49,11 +49,11 @@ its source. | Issuer | What it is | State it keeps | Fit for krm-foyer today | | --- | --- | --- | --- | | Dex | A small OIDC issuer, with its own users or in front of LDAP, GitHub, SAML or another OIDC provider | Kubernetes custom resources with `storage: type: kubernetes`; no volume | Tested. Used by the tests and the demo | -| Pinniped | Kubernetes login: a Supervisor that federates an upstream provider, and a Concierge for clusters you cannot configure | Kubernetes Secrets; no volume. Needs an upstream provider | Works, with sessions of 30 minutes at most until refresh lands | +| Pinniped | Kubernetes login: a Supervisor that federates an upstream provider, and a Concierge for clusters you cannot configure | Kubernetes Secrets; no volume. Needs an upstream provider | Untested; direct API-server trust should fit, with sessions of 30 minutes at most until refresh lands | | Keycloak | Full identity management: users, roles, brokering, OIDC and SAML | A relational database. Its built-in `dev-file` database is not for production | Should work. Raise the token lifespan for krm-foyer's client until refresh lands | | authentik | Authentication flows and application integrations; documents Kubernetes CLI login | PostgreSQL (Redis is no longer needed since 2025.10). Uploaded files need a volume or S3 | Should work, once `email_verified` is fixed (see below) | | Authelia | Reverse-proxy authentication with an OIDC provider; documents kubelogin | SQLite, PostgreSQL or MySQL. Several replicas also need Redis for sessions | Should work | -| OpenUnison | Kubernetes login portal for kubectl and dashboards, with optional access provisioning | The login portal keeps sessions as Kubernetes custom resources; some provisioning features add a database | Works with short tokens only until refresh lands | +| OpenUnison | Kubernetes login portal for kubectl and dashboards, with optional access provisioning | The login portal keeps sessions as Kubernetes custom resources; some provisioning features add a database | Untested; short tokens would limit sessions until refresh lands | If you want no database at all, Dex, Pinniped and OpenUnison's login portal keep their state in the cluster. Keycloak, authentik and Authelia each bring a database to run, back @@ -66,7 +66,7 @@ and Pinniped leave to an upstream provider. | --- | --- | --- | | Users without an outside service | `staticPasswords`, in the same container | Pinniped and OpenUnison need an upstream provider. Keycloak, authentik and Authelia have users, but also a database | | A user's token without a browser | The password grant. The suite asks the API server directly what each user may do and compares that with krm-foyer's answer | A registered Pinniped web client may use only `authorization_code`, `refresh_token` and token exchange | -| A session longer than the ID token, without refresh | ID token lifetime is set in `expiry` | Pinniped: 2 minutes by default, 30 at most. OpenUnison: about a minute in its examples | +| A useful session lifetime without refresh | ID token lifetime is configurable in `expiry`; the session never outlives it | Pinniped: 2 minutes by default, 30 at most. OpenUnison: about a minute in its examples | | Parts to install | One Deployment | Pinniped and OpenUnison: the issuer plus an upstream provider. The others: the issuer plus its database | The security boundaries krm-foyer tests are the same with any issuer: the token stays on @@ -81,10 +81,14 @@ whatever the issuer is. Today it is in memory, so krm-foyer runs one replica. Pinniped and OpenUnison show an approach worth copying: keep sessions in the cluster. Pinniped's Supervisor keeps them as Secrets and removes expired ones with a garbage -collector (`internal/fositestorage` in its source). A store like that would let krm-foyer +collector (`internal/fositestorage` in its source). A store like that could let krm-foyer run several replicas with no Redis or database. Two things need care: how often it writes to etcd (not on every request), and who can read Secrets in its namespace. Keying sessions -by a hash of the ID, as krm-foyer already does, helps with the second. +by a hash of the ID, as krm-foyer already does, prevents recovery of usable session +IDs. It does not protect the raw ID tokens, or future refresh tokens, held in each +record. Store access and encryption still need their own design. The current manager +touches the store per request and reads it per open response every check interval; a +Kubernetes-backed store is an option to measure, not a storage decision already made. ## Notes per issuer @@ -101,9 +105,12 @@ These notes were checked against Pinniped's `main` branch of August 2026. **The API server must trust the Supervisor directly.** Add the FederationDomain's issuer to the API server's `AuthenticationConfiguration` with krm-foyer's `OIDCClient` name as the -audience. A Concierge `JWTAuthenticator` with the same issuer and audience also works. +audience. A Concierge `JWTAuthenticator` alone does not configure the API server to +accept that token directly. Its credential-exchange and impersonation-proxy paths are +different integrations, not drop-in replacements for this configuration. -Pinniped's guide for web applications describes a different path: +Pinniped's [web-application guide](https://pinniped.dev/docs/howto/configure-auth-for-webapps/) +describes a different path: 1. Exchange the token for one scoped to a single cluster (RFC 8693). 2. Trade that token for a short-lived mTLS client certificate through the Concierge's @@ -130,8 +137,9 @@ documented 401 and must sign in again. [session lifecycle](../design.md#session-lifecycle) table leaves to the issuer. - **Group changes that arrive on their own.** A refresh picks up new memberships. - **Several upstream providers and identity transformations** in one FederationDomain. -- **Managed clusters** whose API server flags you cannot set. The Concierge's impersonation - proxy can accept Supervisor tokens there. krm-foyer has not been tested against it. +- **Managed clusters** whose API server flags you cannot set. The Concierge offers credential exchange and an impersonation + proxy for that use. This is outside krm-foyer's tested direct-token integration and + its no-impersonation contract; refresh alone does not make it supported. ### Keycloak @@ -166,7 +174,7 @@ Its tokens are short-lived, so it needs refresh, as Pinniped does. None of these is scheduled. They would follow refresh: -- [ ] Refresh and shared session storage ([roadmap step 6](../roadmap.md#order-of-work)) +- [ ] Refresh and shared session storage ([roadmap](../roadmap.md#order-of-work)) - [ ] An e2e fixture for the issuer, running the login, refresh and logout journeys with its default token lifetimes. For Pinniped or OpenUnison, Dex can be the upstream - [ ] The disablement bound measured for that issuer: disable the user, then time how long diff --git a/docs/investigations/krm-stream-feedback.md b/docs/investigations/krm-stream-feedback.md index 8801284..3ad47a9 100644 --- a/docs/investigations/krm-stream-feedback.md +++ b/docs/investigations/krm-stream-feedback.md @@ -1,11 +1,14 @@ # krm-stream feedback from krm-foyer -krm-foyer is about to host the krm-stream gateway with user-authenticated watches -([roadmap step 5](../roadmap.md#order-of-work)). This note asks for five changes, most -important first. None of them blocks us: for each we say what krm-foyer does meanwhile. -Everything was checked against krm-stream v0.4.0 (`gateway`, `gateway/kube` and -`@configbutler/krm-stream`) on 2026-10-02, by reading the source and, for the error -output below, by running the gateway. +**Historical investigation, 2026-10-02.** All asks 1–8 below were addressed in +krm-stream 0.5.0 and 0.6.0. The current integration uses 0.7.0 (see `go.mod` and +`Taskfile.yml`); its behavior is documented in [design](../design.md#streams-and-editing) +and [watches](../watches.md). The problem descriptions and “meanwhile” workarounds +below describe the versions investigated, not defects or code still present today. + +The original review covered krm-stream 0.4.0 (`gateway`, `gateway/kube` and the browser +package), by reading its source and running the gateway. Asks 6–8 came from the +subsequent integration review. ## Status (2026-10-02): asks 1 to 5 are in krm-stream 0.5.0 @@ -45,20 +48,9 @@ the watches open at the API server. and a reconnect would cost every browser a fresh snapshot. That reasoning holds for krm-foyer too, and we leave it there. -One thing we noticed while moving: the 0.6.0 release job failed at "a stranger can `go -get` this". `sum.golang.org` and `proxy.golang.org` answered `unknown revision -gateway/v0.6.0` for some time after the tag was pushed, so `go get` of 0.6.0 failed for -everyone, and the npm publish after that job did not run. 0.5.0 released through the -same flow without it. We do not know which lookup reached the proxy before the tag -existed; the release-PR builds of #47 are where we would look first, since they name the -unreleased version. krm-foyer fetched 0.6.0 with `GOPROXY=direct` meanwhile; within about -a quarter of an hour the proxy and `sum.golang.org` served it, with the same hashes. The -npm package followed the next day (krm-stream #49), and krm-foyer's bundle moved to it: it -differs from 0.5.0's only in the version string. - ## The asks at a glance -| # | Ask | Priority | What krm-foyer does meanwhile | +| # | Original ask | Original priority | Workaround at the time (superseded) | | --- | --- | --- | --- | | 1 | Map Kubernetes errors to the spec's codes, and recover from the retryable ones with backoff | High | Wraps `kube.Backend` and maps errors itself | | 2 | Send a generic message with an unexpected `INTERNAL`, and give the detail to a diagnostic hook | High | The same wrapper keeps raw errors from reaching the gateway | @@ -66,7 +58,7 @@ differs from 0.5.0's only in the version string. | 4 | Let `Principal` say why it failed, so a missing credential is `UNAUTHENTICATED` | Low | Answers before the gateway runs, so it never sees this | | 5 | Honour retry hints at the layer that retries | Low | Nothing needed for now | -## How krm-foyer uses krm-stream +## How krm-foyer used krm-stream 0.4.0 krm-foyer is a backend for frontend: it signs users in with OIDC, keeps their ID token server-side behind a session cookie, and proxies `/k8s` to the API server with that @@ -86,7 +78,7 @@ So krm-foyer uses the gateway in per-user mode: - **When the session ends,** krm-foyer cancels the request context. That aborts the stream and cancels the upstream watch; the managed connector then gets a 401 and stops. -Shared watches come later. Everything below is about per-user mode unless it says +At the time of this investigation, shared watches were still planned. Everything below is about per-user mode unless it says otherwise. ## Ask 1: map Kubernetes errors, and recover from them with backoff diff --git a/docs/investigations/krm-stream-shared-watches.md b/docs/investigations/krm-stream-shared-watches.md index 036f021..34bf55d 100644 --- a/docs/investigations/krm-stream-shared-watches.md +++ b/docs/investigations/krm-stream-shared-watches.md @@ -1,23 +1,41 @@ # krm-stream feedback from krm-foyer: shared watches -krm-foyer now hosts `SharedBackend` with `kube.SubjectAccessReviewAuthorizer` -([roadmap step 5b](../roadmap.md#order-of-work), [watches](../watches.md)). This note asks -for five changes, numbered on from the [first feedback](krm-stream-feedback.md), whose -asks 1 to 8 krm-stream 0.5.0 and 0.6.0 took. Two (9 and 10) are about guarantees that -do not hold today in a failure case; three (11 to 13) would cut the authorization load -that sharing costs. For each we say what krm-foyer does meanwhile. Everything was -checked against krm-stream main at `5ca19ed` (gateway 0.6.0) on 2026-10-04, by reading -the source and by tests in krm-foyer that reproduce each problem. +**Historical findings with two deferred requests.** Asks 9, 10 and the documentation +part of 13 were addressed in krm-stream 0.7.0, which krm-foyer now uses. Asks 11 and 12 +remain deferred. Current setup and guarantees are in [watches](../watches.md). + +The original investigation checked krm-stream `5ca19ed` (gateway 0.6.0) on 2026-10-04, +by reading the source and reproducing the failures in krm-foyer tests. The problem +and workaround descriptions below are that historical record; in particular, the +lazy-open workaround under ask 9 has been removed. + +## Status (2026-10-04): asks 9, 10 and 13 are in krm-stream 0.7.0 + +krm-stream took ask 9, the "require a write timeout" form of ask 10, and the documentation +part of ask 13 ([its proposal 0008](https://github.com/ConfigButler/krm-stream/blob/main/docs/proposals/0008-shared-watch-hardening.md)), +and krm-foyer moved to 0.7.0: + +- **Ask 9:** krm-foyer's upstream backend opens the shared watch in `Watch` again; the + workaround that opened it on the first `Next` is gone. The test that reproduced the + problem stays, and fails against 0.6.0. +- **Ask 10:** nothing changed in krm-foyer, which always set a write timeout. A test now + checks that every route in front of a stream lets it flush and set write deadlines. +- **Ask 13:** the decision cache's key cites krm-stream's documented review attributes; the + test that fails if a review starts carrying a selector stays, since the key is + krm-foyer's to get right. + +Asks 11 and 12 are deferred, with what would reopen them recorded in the proposal. +krm-foyer keeps rechecking on the timer and reusing decisions in its own authorizer. ## The asks at a glance -| # | Ask | Priority | What krm-foyer does meanwhile | +| # | Ask | Status | Current integration | | --- | --- | --- | --- | -| 9 | Open a shared watch without holding the backend-wide lock, and cancel an opening nobody waits for | High | Its upstream backend returns at once and opens the watch on the first `Next` | -| 10 | Make timed rechecks independent of a blocked write, or require a write timeout with them | High | Sets `WriteTimeout` on every stream, and counts it in its revocation bound | -| 11 | Let a host trigger a recheck | Medium | Rechecks on the timer only | -| 12 | Recheck once per principal and scope, not once per subscriber | Medium | Reuses decisions for a short time in its own authorizer | -| 13 | Say which attributes the SubjectAccessReview asks about | Low | Keys its decisions on them, with a test that fails if they change | +| 9 | Open a shared watch without holding the backend-wide lock, and cancel an opening nobody waits for | Fixed in 0.7.0 | Opens normally in `Watch`; the regression test remains | +| 10 | Make timed rechecks independent of a blocked write, or require a write timeout with them | Fixed in 0.7.0 | Sets `WriteTimeout` on every stream, and counts it in its revocation bound | +| 11 | Let a host trigger a recheck | Deferred | Rechecks on the timer only | +| 12 | Recheck once per principal and scope, not once per subscriber | Deferred | Reuses decisions for a short time in its own authorizer | +| 13 | Say which attributes the SubjectAccessReview asks about | Documented in 0.7.0 | Keys its decisions on them, with a test that fails if they change | ## Ask 9: open a shared watch without holding the backend-wide lock @@ -43,7 +61,7 @@ without the lock. Give the opening a context that its waiting subscribers can ca together: when the last one leaves before the watch opened, cancel the opening and drop the entry. The backoff bookkeeping can stay under the lock. -**What krm-foyer does meanwhile.** Its upstream backend's `Watch` returns a watcher at +**Historical workaround (removed in 0.7.0).** Its upstream backend's `Watch` returns a watcher at once, and opens the real watch on the first `Next`, with the context `Watch` was given. `pump` calls `Next` without the lock, and `leave` cancels that context when the last subscriber goes, so the opening is the scope's alone and ends with it. An opening diff --git a/docs/roadmap.md b/docs/roadmap.md index 78d274c..9f8cde6 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -8,89 +8,78 @@ it: `task demo` starts an example application in a browser.** Server-held tokens decided by Kubernetes alone and no service-account fallback each have an e2e spec that tries to get past them, against a real API server and Dex, with krm-foyer's service account as cluster-admin bait. Each other requirement becomes a property of krm-foyer -when its test exists and passes, and not before. Since step 4, every -[bound](bounds.md) is reached by a test, and an open response ends with its session. -Since step 5, the hello example follows its notes live through krm-stream, and a -rehearsal holds 1800 streams of 200 identities on one replica. Since step 5b, those -streams share one watch per scope, with the API server asked about every user. +when its test exists and passes, and not before. Request/response +[bounds](bounds.md) and session cancellation have tests. The hello example follows +notes live through krm-stream. A rehearsal holds 1800 streams of 200 identities on one +replica, both per-user and shared; sharing asks the API server about every user. ## Order of work -Each step makes pending specs real and ends with `task verify` green. - -1. **The proxy, unit tests only** (done). Path checking first, as a pure function. Then the - proxy, which takes the user's credential from the session and nothing else, so it can - be tested against an `httptest` API server before login exists. There is deliberately no test-only way to hand krm-foyer a token: a back door - behind a build tag is still a back door. So the binary does not serve `/k8s` until - step 2 gives the proxy a credential source. -2. **OIDC login and sessions, deployed into the e2e fixture** (done). krm-foyer runs in the - cluster with a cluster-admin bait service account, and the suite logs in by walking - Dex's login form with a cookie jar. The differential, audit, session, CSRF, logout and - token-scan specs go green together. The [interruption](design.md#interruptions) pages - come with it: the 401 page's sign-in link needs login, and this is the first step at - which a person can browse `/k8s`. - - Sessions live in memory, one replica, and there is no refresh: krm-foyer does not ask - for `offline_access`, holds no refresh token, and a session ends with its ID token. - That keeps the step small and gives the token scan a complete list of what to look - for. Five changes, in order: sessions and CSRF (unit tests); OIDC login and the - binary serving `/k8s`; interruption pages; deployment into the fixture with the login - and identity specs; then the differential, CSRF, logout and token-scan specs. - -3. **A working demo** (done). Moved ahead of refresh, shared storage and bounds - (2026-10-01), because a first useful experience shows what the rest must serve, and - the backend already worked against a real Dex and API server. One command, `task - demo`, starts the [hello example](../examples/hello) for a browser on this machine: - everything runs in one k3d cluster, a Gateway API front door (Traefik, the chart - gitops-reverser uses) puts the example and krm-foyer on one origin, and - `kubectl port-forward` brings the front door and Dex to - `*.localhost`, which browsers resolve to loopback with no setup. The example - signs in, lists, creates and edits a custom resource, shows Kubernetes' 403 and 409, - and signs out, with no application-specific code in krm-foyer; the generic browser - helper (`/_foyer/foyer.js`) came with it. Browser specs drive that journey in - Chromium. It reloads on request rather than streaming, and keeps one replica, - in-memory sessions and a new login when the ID token expires. -4. **Bounds** (done), before anyone runs krm-foyer for real: the request rate per session, - concurrent requests per session and per replica, response duration and response - bytes (counted decoded), each configurable with a documented default and a test that - reaches it. Every open response ends when its session ends, at logout or expiry, and - is cancelled at the API server, proved against the real cluster. Metrics show how - close real traffic comes to each limit. Native watches stay open and are not a - special case: krm-foyer does not tell them apart, and no bound needs it to. A bound on - page size was left out (2026-10-02): it would not reliably bound what a list costs. - See [bounds](bounds.md). -5. **Live notes with krm-stream** (done; was step 6). Moved ahead of refresh and shared - storage (2026-10-02), because live state is the experience krm-foyer is for: `/k8s` - for reads and changes, krm-stream for what changes while a page is open. krm-foyer - hosts krm-stream with user-authenticated upstream watches, and the hello example - follows its notes live instead of reloading. Its bounds come with it: browser - subscriptions and upstream watches counted separately (with sharing, several - subscriptions use one upstream watch, and the `/k8s` bounds do not cover watches - krm-stream opens itself), subscriptions closed when their session ends, and - recovery after a disconnect tested. The rehearsal with 200 identities measures what - one replica holds, which the per-replica defaults in [bounds](bounds.md) only - assume. -5b. **Shared watches** (done, 2026-10-03), moved ahead of refresh, which is not needed - yet. For the resources configured, every stream of one scope reads one watch, opened - with a narrow identity of krm-foyer's own; who each user is comes from a - SelfSubjectReview with their token, and whether they may from SubjectAccessReviews at - opening and every recheck, reused briefly for exactly the same question. Metrics - count watches by identity, the streams on shared watches and every access decision. - Every stream's writes are bounded, so a browser that stops reading cannot hold off - a recheck, and a shared watch slow to open holds up no other scope; both are asked - of krm-stream as well ([requests](investigations/krm-stream-shared-watches.md)). - The hello example's notes are shared, and the rehearsal runs both ways. A - [decision guide](watches.md) says when to share, stream per-user, or use a native - watch, which stays available but is not where the work goes. -6. **Refresh and shared session storage, when the demo shows the need** (was step 5, and - before that 2b): refresh serialized per session and bounded, a refused refresh ending - the session, the disablement bound measured for Dex, and a shared store so more than - one replica works. The token scan then also reads the store, for tokens krm-foyer - obtained by refresh. -7. **An example domain** with pending, accepted, rejected and failed outcomes, and a - second frontend on a different API group, with no application-specific code in - krm-foyer. Then measure the operational cost against keeping auth and transport in - each application. +The next work should make the existing service usable and operable by someone outside +this repository. Each change ends with `task verify` green; a checked box requires +an implemented test, not just a design or a pending spec. + +1. **Finish the deployment path.** The Helm chart is being developed separately. + Review it against the contract and install it through e2e. Give the pod no API + grants in per-user mode; for sharing, use an explicitly configured, narrowly granted + projected token. Include resource + requests/limits, network policy, verified backend TLS, the OIDC client and cluster + authentication setup, and protected metrics. The current fixture deliberately gives + the pod cluster-admin bait and is not that deployment. Document certificate, CA and + client-secret rotation, restart/sign-out behavior, and supported Kubernetes/issuer + versions. Add certificate reload and tests for draining traffic during termination. + Finish the targeted ingress tests using the existing Traefik fixture. +2. **Capacity and failure behavior beyond one tiny scope.** Measure large snapshots, + many distinct scopes, staggered subscriptions, sustained writes, slow readers and + reconnect bursts; record API-server load as well as foyer memory. Stream count does + not bound snapshot/cache bytes. Decide and test memory/admission budgets, including + active sessions and anonymous login traffic, before claiming production capacity. + Exercise API-server and issuer outages, rotating shared credentials, and recovery. + Preserve the current rehearsal as a regression test for watch consolidation. +3. **A second application and an integration guide.** Use another API group without new + backend handlers, with a domain operator showing pending, accepted, rejected and + failed outcomes. Document helper outcomes, conditional saves, unknown write results, + stream errors and draft preservation at sign-out. State browser support: only + Chromium has browser e2e today. Add `/auth/whoami` and + `/_foyer/access` to make identity and RBAC problems diagnosable. This establishes + reuse and identifies which lifecycle features the application actually needs. +4. **Refresh, when sessions must outlive short ID tokens.** Serialize refresh per + session, bound it, make logout win every race, never replay a mutation, and scan + refreshed credentials for leaks. Measure disablement for the chosen issuer + configuration. This can be delivered and tested on one replica before adding a + distributed store; it need not wait for high availability. +5. **Shared storage, before multiple replicas.** Share login transactions as well as + sessions, so a callback may land on a different replica. Test cross-replica logout, + refresh coordination, store outages, restart and rolling updates. Measure store + read/write load before choosing its implementation: the current code touches sessions + per request and checks each open response periodically. Hashing session IDs does not + protect the raw cluster tokens a store holds; its access and encryption need a design. +6. **Release and maintenance evidence.** Add vulnerability scanning to the CI image, + exercise the published artifact as an adopter, and document upgrades, rollback and + image/provenance verification. The release workflow already requests an SBOM and + build provenance; multi-arch builds and cosign image signing remain separate work. + Add docs/link checks early enough to prevent status drift; keep broader coverage + targets secondary to tests of specific failure modes. + +The page login gate (`/auth/check`) is a convenience after the deployment and integration +work: `requireSession()` already handles arrival while signed out. Application scopes, +external-login modes and upgrade protocols stay driven by actual adopter requirements. + +## Completed milestones + +These numbers identify earlier investigation notes, not the priority of future work. + +| Milestone | What exists now | +| --- | --- | +| 1–2: proxy and login | OIDC with PKCE/state/nonce, in-memory sessions, CSRF, interruption pages and differential/audit tests against Dex and Kubernetes | +| 3: browser demo | `task demo`, the hello CRD editor, browser helper and Chromium journeys through Traefik | +| 4: bounds | Request rate/concurrency, response duration/bytes, session cancellation and metrics | +| 5: streams | krm-stream, live notes, conflict/draft recovery and a 200-identity rehearsal | +| 5b: shared watches | Opt-in shared resources, API-server subject/access reviews, decision reuse, bounded rechecks and watch/access metrics | + +The [krm-stream investigations](investigations/krm-stream-feedback.md) retain the +version-specific findings behind milestones 5 and 5b. Their superseded workarounds +are historical, not instructions for the current integration. ## Versions @@ -119,14 +108,14 @@ Security items need tests that try to get past the boundary. - [x] PR title check for conventional commits (squash merges take the PR title) - [x] krm-foyer deployed into the e2e fixture (image imported with `k3d image import`), with its service account as cluster-admin bait -- [x] The `foyer` specs for step 2 made real: login, identity, differential answers, +- [x] The implemented `foyer` authentication/access specs: login, identity, differential answers, RBAC changes, watches, path and subresource refusals, CSRF, logout and the token scan, each checked by deploying a krm-foyer broken on purpose -- [ ] The remaining `foyer` specs: refusal of a refresh (step 6), the ingress (later) +- [ ] The remaining `foyer` specs: refusal of a refresh, and dedicated ingress/login-gate behavior - [x] Browser e2e: log in, read, create, edit, get refused with 403, hit a 409, log out, and no credential within the page's reach. Chromium driven from the Go suite - with chromedp, rather than Playwright: no Node or npm in CI, one language for the - suite + with chromedp, keeping browser e2e in Go. Node separately runs the helper unit + tests; there is no application JavaScript build - [x] `task demo`: the e2e fixture with the hello example behind a front door, for a browser on this machine - [ ] Coverage baseline that ratchets upward @@ -134,7 +123,9 @@ Security items need tests that try to get past the boundary. return paths, with a short fuzz run of each in `task verify` - [ ] Helm chart with `values.schema.json`, `helm lint`, `helm template` tests, and e2e that installs through the chart -- [ ] Signed multi-arch image (cosign keyless) with an SBOM +- [x] Release workflow configured to generate an SBOM and attest build provenance +- [ ] Signed multi-arch image (cosign keyless), with documented artifact verification +- [ ] Dependency/image vulnerability scanning inside the CI image - [ ] Docs lint: markdownlint and link checking - [ ] Parse the whole squash message the way release-please does, once a dropped changelog entry makes it worth it @@ -154,8 +145,9 @@ Security items need tests that try to get past the boundary. - [x] krm-foyer serves TLS from a mounted certificate (the e2e deployment does) - [ ] It reloads the certificate on rotation - [ ] Behind an ingress: the public URL from configuration and no trust in `Host` or - `X-Forwarded-*`, with one e2e spec running nginx in front. See [ingress](ingress.md) -- [ ] Behind an ingress with re-encryption: the ingress verifies krm-foyer's certificate + `X-Forwarded-*`, with targeted e2e specs through the Traefik fixture. See [ingress](ingress.md) +- [x] Browser fixture behind Traefik with re-encryption and a BackendTLSPolicy for each + backend; targeted spoofing and buffering checks remain above - [ ] A NetworkPolicy in the chart that admits only the ingress to krm-foyer's port, and only the monitoring system to the metrics port - [ ] Helm chart values for both models @@ -165,9 +157,9 @@ Security items need tests that try to get past the boundary. wait before shutdown, or readiness turned off first, with a test - [ ] Routing recipes for one shared domain: a Gateway API `HTTPRoute`, an nginx server block and a Vite dev-server proxy +- [x] `requireSession()` in the browser helper for navigation to login - [ ] Login gate: `GET /auth/check` for an ingress gating the application's pages, with - nginx and Traefik recipes, and `requireSession()` in the helper where there is no - ingress support + nginx and Traefik recipes - [ ] Later, when a hybrid application asks: identity headers from the check for a domain backend, never the token @@ -193,9 +185,9 @@ Security items need tests that try to get past the boundary. - [x] A test proves that no request falls back to the service account: with the bait in place, a request without a session gets 401 and never reaches the API server, and the audit log names the user for every request with one -- [x] A test proves that no response, on any route, and no log line contains a token - krm-foyer holds, a session ID or a client secret -- [ ] The same for tokens obtained by refresh, read from the session store (step 6) +- [x] A test scans responses and logs for tokens krm-foyer holds and client secrets; + session IDs are allowed only in the session cookie's issuing `Set-Cookie` header +- [ ] The same for tokens obtained by refresh, read from the session store - [x] Upstream bodies reach the browser decoded: the browser's `Accept-Encoding` is dropped, and a gzip answer from the API server arrives uncompressed without `Content-Encoding` @@ -209,25 +201,24 @@ Security items need tests that try to get past the boundary. ### Login and sessions - [x] OIDC authorization code with PKCE, state and nonce, through a maintained library - (unit tests against an issuer that misbehaves on request; e2e in step 2) + (unit tests against an issuer that misbehaves on request; e2e against Dex) - [x] Opaque server-side sessions: rotated at login, with idle and absolute expiry, and - ended with the ID token while there is no refresh (unit tests; e2e in step 2) -- [ ] Refresh is serialized per session and bounded (step 6) -- [ ] A refused refresh ends the session at once: 401s, and its streams close (step 6) + ended with the ID token while there is no refresh (unit tests; e2e against Dex) +- [ ] Refresh is serialized per session and bounded +- [ ] A refused refresh ends the session at once: 401s, and its streams close - [ ] The disablement bound measured for Dex in the fixture (remove a user, time the refused refresh), and documented per issuer configuration; elsewhere the documented bound is the absolute session expiry - [x] CSRF proof and same-origin checks on every mutation through `/k8s`, with repeated - header fields refused (unit tests and a fuzz property; e2e in step 2) + header fields refused (unit tests and a fuzz property; e2e against Dex) - [x] The same checks on logout -- [x] An unauthenticated API request gets a JSON 401, not a redirect (unit tests; e2e in - step 2) +- [x] An unauthenticated API request gets a JSON 401, not a redirect (unit tests; e2e against Dex) - [x] `/auth/session` - [ ] `/auth/whoami` from a SelfSubjectReview - [ ] `/_foyer/access`: the rules for a namespace from a SelfSubjectRulesReview, and a "can I?" form answered by a SelfSubjectAccessReview. See [what may I do](design.md#what-may-i-do) -- [ ] Shared session storage, so more than one replica works (step 6) +- [ ] Shared session storage, so more than one replica works - [ ] The [session lifecycle](design.md#session-lifecycle) bounds, each with a test: logout seen by every replica at once, logout racing a refresh, and the session store unavailable @@ -240,7 +231,8 @@ Security items need tests that try to get past the boundary. - [x] `Status` errors, content types, patch types, dry-run and Server-Side Apply pass through unchanged (the differential specs) -- [ ] Mutations are never replayed, including after the session is refreshed +- [x] A dropped create response is not replayed (`TestMutationsAreNotReplayed`) +- [ ] The same guarantee across refresh and logout races - [x] Native watches and logs stream without buffering, and cancellation reaches the upstream - [x] Every open response ends when its session ends: at logout and at expiry it is @@ -249,7 +241,7 @@ Security items need tests that try to get past the boundary. witness, each checked by deploying a krm-foyer broken on purpose) - [x] [Bounds](bounds.md) on the request rate per session, concurrent requests per session and per replica, response duration and response bytes, each reached by a - test (step 4) + test - [x] A response cut short by a bound is aborted, never ended cleanly, and cancelled at the API server (unit tests over HTTP/1.1 and HTTP/2, and e2e with the audit log as witness) @@ -259,17 +251,17 @@ Security items need tests that try to get past the boundary. - [x] Host krm-stream, with RBAC deciding what a user may watch, and no list of resources in krm-foyer (unit tests; e2e with the audit log as witness, checked by deploying a krm-foyer that opens watches as its service account). Which resources - use a shared watch will be configuration for efficiency, not access + use a shared watch is configuration for efficiency, not access - [x] User-authenticated watches first, with the hello example following its notes live: a change made elsewhere appears without a reload, a change to a note being edited is a conflict the page shows, and a 409 from a change the stream does not show saves nothing until asked again (browser specs, each checked against a broken example) -- [x] Bounds on browser subscriptions and on upstream watches, counted separately: +- [x] Bounds on browser subscriptions, with upstream watches counted separately: streams per session and per replica, apart from requests, and the watches they hold at the API server counted on their own (unit tests and e2e, each checked - against a build broken on purpose). A bound of their own on upstream watches - comes with shared watches + against a build broken on purpose). Upstream watches have no separate bound: + there are no more watches than streams ([why](bounds.md#streams)) - [x] Recovery after a disconnect: a watch the API server ends, or ends with 410 Gone, is opened again on the same stream with a fresh snapshot (unit tests); a dropped connection between browser and krm-foyer is recovered by krm-stream's client, with @@ -295,10 +287,11 @@ Security items need tests that try to get past the boundary. ### Seeing what happened -- [ ] One log line per refusal (policy denial, upstream 401, 403, 409 or 422) with - subject, route and reason +- [x] One log line per refusal (krm-foyer's own, the API server's 401, 403, 409 or 422, + and a stream's refusals) with user, route and reason, and who refused (unit + tests, each checked against a build broken on purpose) - [x] Metrics on every bound, requests in flight, why responses are cut short, and - interruptions by reason, on a listener of their own (step 4; see + interruptions by reason, on a listener of their own (see [metrics](bounds.md#metrics)) - [ ] Metrics for requests and active sessions diff --git a/docs/testing.md b/docs/testing.md index 616cd08..dfdad73 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -3,7 +3,8 @@ krm-foyer sits between browsers and a cluster, and its main promise is a negative one: **it does not invent authentication or authorization.** Dex, or whichever issuer is configured, says who the user is. The API server says what that user may do. krm-foyer -carries the user's credential and decides nothing about access. This document +carries the user's credential for API calls and per-user watches. Shared watches +use an explicit service identity, with API-server reviews for every subscriber. This document describes how the tests prove that. ## What has to be proved @@ -11,10 +12,10 @@ describes how the tests prove that. | Claim | What would break it | How the suite catches it | | --- | --- | --- | | Identity comes from the issuer | krm-foyer asserting a user name, impersonating, or accepting a token issued to another client | The audit log names the user and shows no impersonation; tokens for other clients are rejected | -| Permission comes from RBAC | krm-foyer deciding something itself, or caching a decision | The same request gets the same answer through krm-foyer as it does directly; a RoleBinding change shows on the next request | +| Permission comes from Kubernetes | krm-foyer granting access itself or reusing a decision beyond its subject, scope or lifetime | Proxied requests match direct answers and see RoleBinding changes on the next request; shared streams test exact decision keys and bounded rechecks | | No service-account fallback | A request without a usable user credential being sent with krm-foyer's own identity | The e2e deployment gives krm-foyer's service account cluster-admin, so a fallback turns a 403 into a 200 | | One parse of each path | A path krm-foyer reads one way and Kubernetes another | Non-canonical paths are rejected; fuzzing shows the path forwarded is byte-for-byte the path received | -| The credential stays on the server | A token or session ID in a response body, header, page or log line | Every response and log line is scanned for every token involved, including those krm-foyer obtained by refresh | +| The credential stays on the server | A token or session ID in a response body, header, page or log line | Every response and log line is scanned for every token involved, with refreshed credentials to be added when refresh exists | ## Four techniques @@ -45,8 +46,9 @@ again after the whole suite. It looks for every token the suite obtained, exactl anything shaped like a JWT. The second covers the tokens krm-foyer holds and never showed the suite: the ID tokens it got by redeeming codes, and its own service-account token. krm-foyer holds no refresh token yet, since it asks for no `offline_access`. When refresh -arrives (roadmap step 6), its session store moves out of process, and the scan also reads -it as admin for the opaque refresh tokens no pattern can find. +arrives, the scan must also inspect stored credentials for opaque refresh tokens no +pattern can find. Shared storage is a separate roadmap item, not a prerequisite for +testing refresh on one replica. A session ID has exactly one place it belongs: the `Set-Cookie` header that issues it, on the login callback and wherever the ID is rotated. The scan allows the ID there, and @@ -84,8 +86,10 @@ bugs are here, where they are cheap to find: single content type that a browser cannot read as anything but an allowed one. The property is written independently of the check, so it can catch the check's own blind spots, such as a repeated `Content-Type` field. -- **Interruption pages**: every row of the interruptions table is triggered twice, as - code and as a browser navigation, and must give the same status both times. A +- **Interruption pages**: table-driven tests exercise proxy interruptions as + code and as browser navigations, checking that the status is the same. Rate, + concurrency, size and stream-method refusals also have tests in their own suites; + the roadmap retains a full contract-to-test audit as unfinished work. A `fetch`, an iframe, another method, a capitalized or repeated `Sec-Fetch-Dest` and `Accept: text/html` alone all get JSON, and the API server's own 401, 403, 404, 409, 422, 429, 500 and 503 reach a navigation unchanged. @@ -136,7 +140,9 @@ bugs are here, where they are cheap to find: else. A 401 or 403 on an open watch ends the stream as it does at opening. A failure that may pass (a 503 or 429 at opening, a 500 or 429 on an open watch) ends the stream with a non-terminal `UPSTREAM_UNAVAILABLE` and the API server's hint, after one - attempt: the browser's client retries, through the gate. A watch that ends before its + attempt in these fixtures: they put the hint in the `Status`, not a `Retry-After` + header that client-go would retry internally. The browser's client retries through + the gate. A watch that ends before its snapshot or hardly after it, or with a 410 right after it, is opened once more and then ends the stream the same way, after two attempts and never more. What the API server wrote when it failed, the token it echoed among it, reaches neither the browser nor the log, and an ended @@ -173,7 +179,9 @@ bugs are here, where they are cheap to find: the scope, against an oracle that knows nothing of the cache. Keys missing the UID, the extras, the name, the version or the group each failed it within a second; the label selector is left out on purpose, as the reviews do not ask about it, and a test - fails if they start to. + fails if they start to. A rate-budget regression spends the shared identity's burst + on two reviews and checks that opening its watch waits for the same budget to refill; + client-go's built-in limiter alone does not throttle watch openings. - **Upstream text in the log** ([internal/proxy](../internal/proxy), [internal/upstream](../internal/upstream)): an API server that echoes the token in a malformed response, a `Content-Type` or a `Location` never gets it into krm-foyer's @@ -319,7 +327,7 @@ flowchart LR T -->|bearer token, directly| K T -->|session cookie, NodePort| F FD -->|/| W - FD -->|/auth, /k8s, /_foyer, TLS verified| F + FD -->|/auth, /k8s, /stream, /_foyer, TLS verified| F F -->|user's token| K F -->|discovery and code exchange| D K -->|discovery and keys| D @@ -388,9 +396,9 @@ The test issuer is nginx serving a discovery document and a JWKS. The suite hold signing key (`.e2e/issuer-signing.key`), so it can mint tokens with claims Dex never issues. Use it for claims; use Dex for anything a real login would do. -Dex has two static users, `alice@example.com` and `bob@example.com` (password +Dex has two demo users, `alice@example.com` and `bob@example.com` (password `password`), which Kubernetes sees as `oidc:alice@example.com` and -`oidc:bob@example.com`. There are two clients: `krm-foyer`, whose tokens the cluster +`oidc:bob@example.com`, plus 200 generated rehearsal users. There are two clients: `krm-foyer`, whose tokens the cluster accepts, and `other-app`, whose tokens it must reject. ```bash @@ -407,12 +415,15 @@ When something fails, the API server's view is usually the answer: thought was asking. `KUBECONFIG=.e2e/kubeconfig kubectl ...` gives admin access for looking around. -## Order of work +## What remains to prove -The [roadmap](roadmap.md#order-of-work) sets the order: the proxy, then login and -sessions deployed into the fixture, then streams. Each step makes -pending specs real. +The [roadmap](roadmap.md#order-of-work) sets the next priorities. Pending e2e specs +currently cover refused refresh and targeted ingress/login-gate behavior. They are +listed by Ginkgo but do not fail the suite. Existing browser specs already exercise +Traefik routing, verified backend TLS and live notes; a passing run does not mean the +pending behaviors are implemented. -To keep krm-foyer small, the OIDC work uses maintained libraries (`coreos/go-oidc` and -`golang.org/x/oauth2`), and the proxy uses `net/http/httputil`. Neither the service nor -its tests need `client-go`. +OIDC uses `coreos/go-oidc` and `golang.org/x/oauth2`; `/k8s` uses `net/http/httputil`. +Streams and Kubernetes subject/access reviews use `client-go`, directly and through +krm-stream. The Go modules and the vendored browser bundle are checked separately by +`task tidy-check` and `task vendor-check`, both included in `task verify` and CI. diff --git a/docs/vision.md b/docs/vision.md index c296b40..4feb396 100644 --- a/docs/vision.md +++ b/docs/vision.md @@ -40,8 +40,8 @@ see what Kubernetes answered for you. Where krm-foyer itself steps in, because y signed in, or the upstream answered with a redirect or a page it will not render, it tells you, in a page instead of a bare error code. For a redirect, you see where it leads and choose whether to go. -Two pages answer the questions everyone asks first. `/auth/whoami` shows who Kubernetes -thinks you are. `/_foyer/access` shows what you may do, from Kubernetes' own reviews, +Two planned pages will answer the questions everyone asks first. `/auth/whoami` will show who Kubernetes +thinks you are. `/_foyer/access` will show what you may do, from Kubernetes' own reviews, with a "can I?" form. Code asks Kubernetes the same questions natively, to hide buttons that would only produce a 403. diff --git a/docs/watches.md b/docs/watches.md index 8e54b05..5e1cced 100644 --- a/docs/watches.md +++ b/docs/watches.md @@ -2,7 +2,7 @@ A page that shows live Kubernetes state can get it three ways through krm-foyer. This page helps you choose one, then explains how to set up the one that needs setting up: -shared watches. In short: **use a stream for anything live in a page, share it when +shared watches. **Use a stream for anything live in a page, share it when many users watch the same thing, and treat native watches as a fallback.** ## The three kinds @@ -76,10 +76,13 @@ The token file holds a service account's token. That account needs: - `list` and `watch` on each shared resource, wherever users will watch it: usually a ClusterRole bound with a ClusterRoleBinding; -- `create` on `subjectaccessreviews`, which the built-in `system:auth-delegator` - ClusterRole grants; +- `create` on `subjectaccessreviews` in `authorization.k8s.io`; - nothing else. +The fixture binds `system:auth-delegator` for the reviews. That built-in role also +grants `create` on `tokenreviews`, which krm-foyer does not need. A deployment can use +a narrower ClusterRole containing only the SubjectAccessReview grant. + krm-foyer reads the file again as it changes, so a rotating token works. It never uses the pod's own service account by itself: if you want that account to be the shared-watch identity, point the flag at its projected token @@ -126,10 +129,15 @@ A grant taken away ends a shared stream within: The last line is there because krm-stream delivers events and rechecks one at a time: a recheck waits for the write in progress. Without a bound on writes, a browser that stopped reading would hold its stream open, unchecked, until the 30-minute response -duration. With it, the write fails and the stream ends. +duration. With it, the write fails and the stream ends. Since 0.7.0 krm-stream refuses +timed rechecks without a write timeout, and its +[revocation budget](https://github.com/ConfigButler/krm-stream/blob/main/docs/auth.md#revocation-budget) +breaks down the same parts. Logout and session expiry are not affected: they end every stream within the -session-check interval, as before. +session-check interval, as before. These access reviews use the subject captured by +the SelfSubjectReview when the stream opened. They do not refresh the OIDC token or +learn new issuer group membership; the RBAC bound is not an issuer-disablement bound. ### What the browser gets when something fails @@ -167,7 +175,9 @@ in-memory evaluation, with no storage. With a webhook authorizer in the chain, i a blocking HTTP call to that webhook, so the load lands there too, at the webhook's latency; measure it with `krm_foyer_access_check_duration_seconds`. The shared-watch identity's client sends at most `-shared-watch-qps` (100) requests a second, bursts -twice that, and the API server's priority and fairness applies on top. Reviews that +twice that (at least one request). Reviews, watch openings, fallback lists and retries +share one transport budget; SelfSubjectReviews use each user's separate client. The +API server's priority and fairness applies on top. Reviews that cannot be sent in time show as `error` results and `UPSTREAM_UNAVAILABLE` streams, never as streams served without a check. @@ -192,9 +202,10 @@ Kubernetes sends no notice that what a user may do has changed, so there is noth wait for. One could watch RBAC's Roles and Bindings and recheck when they change, but: - **RBAC is not all of authorization.** A webhook authorizer, the Node authorizer, or - group membership, which comes from the issuer and changes with no Kubernetes object, - can change an answer with nothing to watch. A periodic recheck stays as the safety - net either way. + an external policy service can change an answer with no RBAC object to watch. + Periodic reviews cover those changes for the captured subject. Issuer group changes + also have no RBAC event, but require fresh identity resolution and a new token; + repeating a SubjectAccessReview alone does not pick them up. - **The shared-watch identity would read every Role and Binding** in the cluster: the whole access policy, a broader grant than it has now. - **krm-stream rechecks on its own timer,** per stream; a host cannot ask it to diff --git a/examples/hello/web/krm-stream.js b/examples/hello/web/krm-stream.js index 725b6c7..6088230 100644 --- a/examples/hello/web/krm-stream.js +++ b/examples/hello/web/krm-stream.js @@ -1108,7 +1108,7 @@ function resourceStreamURL(base, scope) { } // dist/version.js -var VERSION = "0.6.0"; +var VERSION = "0.7.0"; var PROTOCOL_VERSION = 1; export { DEFAULT_EDITABLE_REGIONS, diff --git a/go.mod b/go.mod index 6b30f86..b105546 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/ConfigButler/krm-foyer go 1.27.1 require ( - github.com/ConfigButler/krm-stream/gateway v0.6.0 - github.com/ConfigButler/krm-stream/gateway/kube v0.6.0 + github.com/ConfigButler/krm-stream/gateway v0.7.0 + github.com/ConfigButler/krm-stream/gateway/kube v0.7.0 github.com/chromedp/cdproto v0.0.0-20260714215040-dc233986426f github.com/chromedp/chromedp v0.16.0 github.com/coreos/go-oidc/v3 v3.21.0 diff --git a/go.sum b/go.sum index 333085f..12eabf0 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -github.com/ConfigButler/krm-stream/gateway v0.6.0 h1:PJk7Q2Ec3DPG3qloAPxhbJq39jlObirYGTREzOK7U2U= -github.com/ConfigButler/krm-stream/gateway v0.6.0/go.mod h1:sVRhasWxXotQQHsyY4mKvvT0npkupDjrvg/i3Elq5+E= -github.com/ConfigButler/krm-stream/gateway/kube v0.6.0 h1:SNEmImC9oJiPxneFA8M9dIvtk1hC2AYw1qsTlCfAAqQ= -github.com/ConfigButler/krm-stream/gateway/kube v0.6.0/go.mod h1:h/WjlrcDwfmyF1Vv4YtzqhOXDvIdjGaOyHkbotU128c= +github.com/ConfigButler/krm-stream/gateway v0.7.0 h1:Pb1fWshxt44D+LMcD7sIrzo3lJQyKtYivIfRuXpBLxI= +github.com/ConfigButler/krm-stream/gateway v0.7.0/go.mod h1:sVRhasWxXotQQHsyY4mKvvT0npkupDjrvg/i3Elq5+E= +github.com/ConfigButler/krm-stream/gateway/kube v0.7.0 h1:8Zm8+il5xgFlRWLWehm1P7e4qSentcMzkRa5AcbqyS4= +github.com/ConfigButler/krm-stream/gateway/kube v0.7.0/go.mod h1:4NPwh1gb4ewfGunozln3G8W4y0//VPirwx+AdeNsmuo= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 6e4379b..9b54b1d 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -471,7 +471,11 @@ func (a *Auth) Token(r *http.Request) (gate.Credential, *interruption.Interrupti if err != nil { return gate.Credential{}, a.refusal(err) } - return gate.Credential{Token: s.IDToken, Session: session.Handle(r), Live: func(ctx context.Context) bool { + user := s.Email + if user == "" { + user = s.Subject + } + return gate.Credential{Token: s.IDToken, User: user, Session: session.Handle(r), Live: func(ctx context.Context) bool { err := a.cfg.Sessions.Check(r.WithContext(ctx)) if err != nil && !errors.Is(err, session.ErrNoSession) { a.logger.Warn("session store failed during a session check", "err", err) diff --git a/internal/auth/credentials_test.go b/internal/auth/credentials_test.go index bca0335..4b6f7aa 100644 --- a/internal/auth/credentials_test.go +++ b/internal/auth/credentials_test.go @@ -169,6 +169,10 @@ func TestLiveFollowsTheSession(t *testing.T) { if !cred.Live(t.Context()) { t.Fatal("a live session's credential is not live") } + // Log lines name the user as the issuer did. + if cred.User != alice { + t.Errorf("User = %q, want %q", cred.User, alice) + } // The session's name for bounds is stable, its own, and not its ID. again, _ := h.auth.Token(cookieRequest(b)) other := h.browser() diff --git a/internal/gate/gate.go b/internal/gate/gate.go index 46922b4..fba473c 100644 --- a/internal/gate/gate.go +++ b/internal/gate/gate.go @@ -33,6 +33,10 @@ type Credentials interface { type Credential struct { // Token is the user's bearer token. Token string + // User is who the issuer says the user is, for log lines: their email, or their + // subject when the issuer gave none. The API server decides who the token + // belongs to. + User string // Session names the session the token came from, for the bounds kept per // session. It is opaque, and never logged or sent anywhere. Session string @@ -186,13 +190,59 @@ func (g *Gate) Metrics() *metrics.Metrics { return g.metrics } // Interrupt answers r with i, instead of the API server, and logs and counts it. func (g *Gate) Interrupt(w http.ResponseWriter, r *http.Request, i *interruption.Interruption) { - path, _, _ := strings.Cut(r.RequestURI, "?") - g.logger.Info("interruption", "status", i.Status, "reason", i.Reason, "message", i.Message, - "method", r.Method, "path", path) + g.interrupt(w, r, "", i) +} + +// Interrupt answers a's request with i, as Gate.Interrupt does, and logs its user. +func (a *Admission) Interrupt(w http.ResponseWriter, i *interruption.Interruption) { + a.gate.interrupt(w, a.Request, a.User, i) +} + +// An interruption below 500 is krm-foyer refusing the request, and is logged as a +// refusal; one from 500 is a failure, which the browser may try again. +func (g *Gate) interrupt(w http.ResponseWriter, r *http.Request, user string, i *interruption.Interruption) { + attrs := []any{"status", i.Status, "reason", i.Reason, "message", i.Message} + if i.Status < http.StatusInternalServerError { + g.refused(r, ByFoyer, user, attrs...) + } else { + g.logger.Info("interruption", append(attrs, request(r, user)...)...) + } g.metrics.Interruption(i.Reason) i.Serve(w, r) } +// Who refused a request. +const ( + ByFoyer = "krm-foyer" + ByKubernetes = "kubernetes" +) + +// Refused logs that a's request was refused by, for the reason attrs give. Every +// refusal is one line, whoever refused: an operator finds who was refused what, +// and by whom, in one place. +func (a *Admission) Refused(by string, attrs ...any) { + a.gate.refused(a.Request, by, a.User, attrs...) +} + +func (g *Gate) refused(r *http.Request, by, user string, attrs ...any) { + g.logger.Info("refused", append(append([]any{"by", by}, attrs...), request(r, user)...)...) +} + +// request is what a log line says about r: its method, path and route, and its +// user once known. The query is left out, as it may name anything. +func request(r *http.Request, user string) []any { + path, _, _ := strings.Cut(r.RequestURI, "?") + route := "/" + if first, _, _ := strings.Cut(strings.TrimPrefix(path, "/"), "/"); first != "" { + route += first + } + attrs := []any{"method", r.Method, "path", path, "route", route} + if user != "" { + attrs = append(attrs, "user", user) + } + return attrs +} + // Admission is a request the gate let through. Close it when the response has // ended, exactly once. type Admission struct { @@ -233,12 +283,12 @@ func (g *Gate) admit(w http.ResponseWriter, r *http.Request, slots *concurrency) return nil } if ok, wait := g.rate.allow(cred.Session); !ok { - g.Interrupt(w, r, g.rate.refuse(wait)) + g.interrupt(w, r, cred.User, g.rate.refuse(wait)) return nil } release, refused := slots.acquire(cred.Session) if refused != nil { - g.Interrupt(w, r, refused) + g.interrupt(w, r, cred.User, refused) return nil } diff --git a/internal/pages/templates/start.html b/internal/pages/templates/start.html index 3e4ee9f..3ff3fcc 100644 --- a/internal/pages/templates/start.html +++ b/internal/pages/templates/start.html @@ -23,15 +23,16 @@

What works in this deployment

  • Sign-in through the configured OIDC issuer: sign in, and /auth/session to see the session
  • The Kubernetes API under /k8s/, with your own credential
    • +
  • Live Kubernetes resources through krm-stream at /stream/v1
    • {{- end}}

    Not here

    Read the service design diff --git a/internal/proxy/interruption_test.go b/internal/proxy/interruption_test.go index 8a0e447..8bc3755 100644 --- a/internal/proxy/interruption_test.go +++ b/internal/proxy/interruption_test.go @@ -268,17 +268,70 @@ func TestSignInReturnsToTheURL(t *testing.T) { } } -// Every interruption is logged with the path that was asked for, including the -// ones decided after the API server answered. +// Every interruption is logged once, with the path that was asked for, including +// the ones decided after the API server answered. One below 500 is krm-foyer +// refusing the request, logged as a refusal like the API server's; one from 500 is +// a failure. The user is named once the request has one. func TestInterruptionsAreLoggedWithThePath(t *testing.T) { for _, tc := range interruptionCases() { t.Run(tc.name, func(t *testing.T) { f := tc.foyer(t) f.request(t, http.MethodGet, tc.target, nil, tc.header) path, _, _ := strings.Cut(tc.target, "?") - if logs := f.logs.String(); !strings.Contains(logs, `"msg":"interruption"`) || !strings.Contains(logs, `"path":"`+path+`"`) { - t.Errorf("logged:\n%s", logs) + want := map[string]any{"msg": "interruption", "path": path, "route": "/k8s", "status": float64(tc.status)} + if tc.status < http.StatusInternalServerError { + want["msg"], want["by"] = "refused", gate.ByFoyer + } + lines := logLines(t, f.logs.String(), want) + if len(lines) != 1 { + t.Fatalf("%d lines like %v; logged:\n%s", len(lines), want, f.logs.String()) + } + // Past the credential: the bound counts per session, and the API + // server's answers come after the gate. + admitted := tc.occupy || tc.upstream != nil + if user, ok := lines[0]["user"]; admitted != ok || (ok && user != testUser) { + t.Errorf("user = %v; want it named only past the credential", user) } }) } } + +// The API server refusing the user is logged once, as krm-foyer's own refusals +// are: who, what, and why. Any other answer is not a refusal, and is not logged. +func TestKubernetesRefusalsAreLogged(t *testing.T) { + api := newAPIServer(t, func(w http.ResponseWriter, r *http.Request) { + status, _ := strconv.Atoi(r.URL.Query().Get("answer")) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = io.WriteString(w, `{"kind":"Status","message":"`+userToken+`"}`) + }) + f := newFoyer(t, api, credentials{token: userToken}) + for status, reason := range map[int]string{ + http.StatusUnauthorized: "Unauthorized", http.StatusForbidden: "Forbidden", + http.StatusConflict: "Conflict", http.StatusUnprocessableEntity: "Unprocessable Entity", + http.StatusOK: "", http.StatusNotFound: "", http.StatusTooManyRequests: "", http.StatusInternalServerError: "", + } { + before := len(logLines(t, f.logs.String(), nil)) + target := "/k8s/api/v1/namespaces/a/configmaps?answer=" + strconv.Itoa(status) + if resp := f.get(t, target); resp.StatusCode != status { + t.Fatalf("GET %s = %d", target, resp.StatusCode) + } + lines := logLines(t, f.logs.String(), nil)[before:] + if reason == "" { + if len(lines) != 0 { + t.Errorf("%d is not a refusal, but logged %v", status, lines) + } + continue + } + want := map[string]any{ + "msg": "refused", "by": gate.ByKubernetes, "user": testUser, "status": float64(status), "reason": reason, + "method": http.MethodGet, "path": "/k8s/api/v1/namespaces/a/configmaps", "route": "/k8s", + } + if len(lines) != 1 || len(logLines(t, f.logs.String(), want)) != 1 { + t.Errorf("%d: logged %v; want one line like %v", status, lines, want) + } + } + if strings.Contains(f.logs.String(), userToken) { + t.Errorf("what the API server wrote reached the log:\n%s", f.logs.String()) + } +} diff --git a/internal/proxy/lifecycle_test.go b/internal/proxy/lifecycle_test.go index 874a79d..d80c571 100644 --- a/internal/proxy/lifecycle_test.go +++ b/internal/proxy/lifecycle_test.go @@ -280,5 +280,5 @@ func TestUpstreamErrorAfterACutAborts(t *testing.T) { t.Errorf("something was written before the abort: %d %q", w.Code, w.Body) } }() - p.upstreamError(w, r, context.Canceled) + _ = p.upstreamError(r, context.Canceled) } diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 51b5f9f..4a22e4c 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -165,15 +165,30 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err := checkResponse(resp); err != nil { return err } + if refusals[resp.StatusCode] { + a.Refused(gate.ByKubernetes, "status", resp.StatusCode, "reason", http.StatusText(resp.StatusCode)) + } return p.limitBytes(a, resp) }, // ReverseProxy hands its error handler the outgoing request, whose headers // are the allowlisted ones. How to answer depends on the browser's request. - ErrorHandler: func(w http.ResponseWriter, _ *http.Request, err error) { p.upstreamError(w, r, err) }, + ErrorHandler: func(w http.ResponseWriter, _ *http.Request, err error) { + if refused := p.upstreamError(r, err); refused != nil { + a.Interrupt(w, refused) + } + }, } rp.ServeHTTP(&headWriter{ResponseWriter: w}, r) } +// refusals are the API server's answers that refuse the user, rather than fail: +// not signed in, not allowed, in conflict, or invalid. Each is logged, as +// krm-foyer's own refusals are. +var refusals = map[int]bool{ + http.StatusUnauthorized: true, http.StatusForbidden: true, + http.StatusConflict: true, http.StatusUnprocessableEntity: true, +} + // checkResponse applies the upstream response rules (docs/design.md). An error it // returns is an *interruption.Interruption, answered by upstreamError. func checkResponse(resp *http.Response) error { @@ -253,24 +268,26 @@ func heldBack(field, value string) *interruption.Interruption { } } -func (p *Proxy) upstreamError(w http.ResponseWriter, r *http.Request, err error) { +// upstreamError is the interruption to answer r with for err, or nil when nobody +// is left to answer. It panics with http.ErrAbortHandler when the gate cut r short. +func (p *Proxy) upstreamError(r *http.Request, err error) *interruption.Interruption { var refused *interruption.Interruption - if !errors.As(err, &refused) { - if gate.CutShort(r.Context()) { - // The request may have reached Kubernetes, so no interruption may say it - // did not, and an empty answer would pass for a complete one. - panic(http.ErrAbortHandler) - } - if r.Context().Err() != nil { - return // the browser went away; nobody is left to answer - } - p.logger.Warn("API server unreachable", "cause", upstream.Class(err)) - refused = &interruption.Interruption{ - Status: http.StatusBadGateway, Reason: "BadGateway", - Message: "the API server could not be reached", - } + if errors.As(err, &refused) { + return refused + } + if gate.CutShort(r.Context()) { + // The request may have reached Kubernetes, so no interruption may say it + // did not, and an empty answer would pass for a complete one. + panic(http.ErrAbortHandler) + } + if r.Context().Err() != nil { + return nil // the browser went away; nobody is left to answer + } + p.logger.Warn("API server unreachable", "cause", upstream.Class(err)) + return &interruption.Interruption{ + Status: http.StatusBadGateway, Reason: "BadGateway", + Message: "the API server could not be reached", } - p.interrupt(w, r, refused) } func (p *Proxy) interrupt(w http.ResponseWriter, r *http.Request, i *interruption.Interruption) { diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index ddbbcf1..12483b0 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -24,6 +24,9 @@ import ( const userToken = "user-token-4f1c9a" //nolint:gosec // a marker to search for, not a credential +// testUser is who the issuer says the test's user is. +const testUser = "alice@example.test" + // credentials is the test's session: it hands the proxy a fixed answer. It exists // only in this test file; the binary has no way to be given a token but a session. // Its session stays live unless live says otherwise. @@ -44,7 +47,30 @@ func (c credentials) Token(*http.Request) (gate.Credential, *interruption.Interr if session == "" { session = "s1" } - return gate.Credential{Token: c.token, Live: live, Session: session}, c.refused + return gate.Credential{Token: c.token, User: testUser, Live: live, Session: session}, c.refused +} + +// logLines is what krm-foyer logged, a line at a time, for those that match all of +// want. +func logLines(t *testing.T, logs string, want map[string]any) []map[string]any { + t.Helper() + var found []map[string]any + for line := range strings.Lines(logs) { + var l map[string]any + if err := json.Unmarshal([]byte(line), &l); err != nil { + continue // not krm-foyer's: Go's own, in plain text + } + matches := true + for k, v := range want { + if got, ok := l[k]; !ok || got != v { + matches = false + } + } + if matches { + found = append(found, l) + } + } + return found } // apiServer stands in for the API server and records what reached it. @@ -865,8 +891,8 @@ func TestTokenNeverLeaves(t *testing.T) { if strings.Contains(f.logs.String(), userToken) { t.Error("the token is in krm-foyer's logs") } - if !strings.Contains(f.logs.String(), `"msg":"interruption"`) { - t.Error("interruptions are not logged") + if !strings.Contains(f.logs.String(), `"msg":"refused"`) || !strings.Contains(f.logs.String(), `"msg":"interruption"`) { + t.Error("refusals and interruptions are not logged") } } diff --git a/internal/server/server.go b/internal/server/server.go index 35a7061..4bb812d 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -109,8 +109,8 @@ func plain(body string) http.HandlerFunc { } // securityHeaders applies to every response. krm-foyer's own pages load nothing -// but same-origin stylesheets, so the policy can be this strict; application -// pages will need their own policy once static hosting exists. +// but same-origin stylesheets, so the policy can be this strict. Application +// pages are served separately and need their own policy. func securityHeaders(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { h := w.Header() diff --git a/internal/server/server_test.go b/internal/server/server_test.go index ba4c959..d205c48 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -5,6 +5,8 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/ConfigButler/krm-stream/gateway" ) func get(t *testing.T, path string) *httptest.ResponseRecorder { @@ -182,3 +184,38 @@ func TestStreamRoute(t *testing.T) { t.Errorf("without streams, /stream/v1 = %d", rec.Code) } } + +// A stream gets a response writer it can flush and give write deadlines, over +// HTTP/1.1 and HTTP/2, through every route krm-foyer puts in front of it. The write +// timeout that bounds a stalled browser, and with it a shared stream's recheck, +// depends on both; a wrapper that hid them would end every stream at once. +func TestStreamsCanFlushAndSetWriteDeadlines(t *testing.T) { + got := make(chan error, 1) + h := New(Config{Version: "test", Kubernetes: http.NotFoundHandler(), Stream: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + got <- gateway.CheckHTTPStreaming(w) + })}) + h1 := httptest.NewServer(h) + t.Cleanup(h1.Close) + h2 := httptest.NewUnstartedServer(h) + h2.EnableHTTP2 = true + h2.StartTLS() + t.Cleanup(h2.Close) + + for name, srv := range map[string]*httptest.Server{"HTTP/1.1": h1, "HTTP/2": h2} { + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL+"/stream/v1?resource=notes&version=v1", nil) + if err != nil { + t.Fatal(err) + } + resp, err := srv.Client().Do(req) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if want := name[len("HTTP/")] - '0'; resp.ProtoMajor != int(want) { + t.Errorf("%s: served over %s", name, resp.Proto) + } + if err := <-got; err != nil { + t.Errorf("%s: %v", name, err) + } + } +} diff --git a/internal/stream/shared.go b/internal/stream/shared.go index 01f4cce..cdc84f4 100644 --- a/internal/stream/shared.go +++ b/internal/stream/shared.go @@ -21,6 +21,7 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/util/flowcontrol" "github.com/ConfigButler/krm-foyer/internal/gate" "github.com/ConfigButler/krm-foyer/internal/metrics" @@ -142,13 +143,19 @@ func newShared(cfg SharedConfig, s *Streams) (*shared, error) { BearerTokenFile: cfg.TokenFile, Transport: s.transport, UserAgent: "krm-foyer shared watches", - QPS: qps, - Burst: int(2 * qps), + // Limit at the transport below: client-go deliberately skips its own + // limiter for watches. Disable its per-client limiters to avoid charging + // reviews and fallback lists twice. + QPS: -1, } httpClient, err := kube.HTTPClientFor(rc) if err != nil { return nil, err } + httpClient.Transport = sharedTransport{ + next: httpClient.Transport, + rate: flowcontrol.NewTokenBucketRateLimiter(qps, max(1, int(2*qps))), + } data, err := dynamic.NewForConfigAndClient(rc, httpClient) if err != nil { return nil, err @@ -172,6 +179,24 @@ func newShared(cfg SharedConfig, s *Streams) (*shared, error) { }, nil } +// sharedTransport charges every request by the shared identity to one budget: +// access reviews, watch openings, fallback lists and retries. Waiting ends with +// the request, so a queued watch can still be abandoned and a review times out. +type sharedTransport struct { + next http.RoundTripper + rate flowcontrol.RateLimiter +} + +func (t sharedTransport) RoundTrip(r *http.Request) (*http.Response, error) { + if err := t.rate.Wait(r.Context()); err != nil { + if r.Body != nil { + _ = r.Body.Close() + } + return nil, err + } + return t.next.RoundTrip(r) +} + // serves reports whether streams of scope share a watch. func (sh *shared) serves(scope gateway.Scope) bool { if sh == nil { @@ -292,11 +317,13 @@ func (d *decisions) Authorize(ctx context.Context, p gateway.Principal, scope ga if err != nil { return gateway.Unauthenticated("not authenticated") } - // The question is what the reviews ask: the subject, and the group, version, - // resource, namespace and name of the scope. A label selector is not part of it - // (RBAC cannot grant by label, and the reviews do not send one), so streams that - // differ only in their selector share a decision. TestASharedStreamAsksAboutEachUser - // fails if the reviews start sending one. + // The question is what the reviews ask, as krm-stream documents it + // (docs/auth.md, "What the SubjectAccessReview asks"): the subject, and the + // group, version, resource, namespace and name of the scope, for list and then + // watch. One decision answers both verbs, as Authorize asks both. Selectors and + // subresource are not sent, so streams that differ only in their selector share + // a decision; TestTheReviewsAskAboutNoSelector fails if that changes. There is + // one target, so decisions need no keeping apart by API server. key, err := json.Marshal(struct { Subject kube.Subject Group, Version, Resource, Namespace, Name string @@ -393,47 +420,28 @@ type sharedUpstream struct { metrics *metrics.Metrics } -// Watch returns at once, and the watch is opened on the first Next. krm-stream calls -// Watch holding a lock every shared scope takes, and with a context nobody cancels -// but the scope's own: opened here, one API server slow to answer would hold up the -// streams of every other scope, and a stream that gave up waiting could not end the -// request. Opened in Next, it is the scope's alone, and its last stream leaving -// cancels it (docs/investigations/krm-stream-shared-watches.md, ask 9). +// Watch opens the scope's watch. krm-stream opens it outside the lock its scopes +// share, with a context its waiting streams share, so a slow opening holds up only +// its own scope, and its last stream leaving cancels it (krm-stream 0.7.0, from +// docs/investigations/krm-stream-shared-watches.md, ask 9). func (b *sharedUpstream) Watch(ctx context.Context, scope gateway.Scope) (gateway.Watcher, error) { - return &sharedWatcher{upstream: b, ctx: ctx, scope: scope}, nil + open, err := b.kube.Watch(ctx, scope) + if err != nil { + return nil, ownRefusal(err) + } + return &sharedWatcher{Watcher: open, done: b.metrics.UpstreamWatch(metrics.IdentityShared)}, nil } +// sharedWatcher counts its watch until it is stopped, and reports the shared +// identity being refused as krm-foyer's own fault. type sharedWatcher struct { - upstream *sharedUpstream - ctx context.Context // the scope's: cancelled when its last stream leaves - scope gateway.Scope - - mu sync.Mutex - open gateway.Watcher // nil until opened - done func() - stopped bool + gateway.Watcher + stop sync.Once + done func() } func (w *sharedWatcher) Next(ctx context.Context) (gateway.WatchEvent, error) { - w.mu.Lock() - open := w.open - w.mu.Unlock() - if open == nil { - opened, err := w.upstream.kube.Watch(w.ctx, w.scope) - if err != nil { - return gateway.WatchEvent{}, ownRefusal(err) - } - w.mu.Lock() - if w.stopped { - w.mu.Unlock() - opened.Stop() - return gateway.WatchEvent{}, context.Canceled - } - w.open, w.done = opened, w.upstream.metrics.UpstreamWatch(metrics.IdentityShared) - open = opened - w.mu.Unlock() - } - ev, err := open.Next(ctx) + ev, err := w.Watcher.Next(ctx) if ev.Type == gateway.WatchError { ev.Err = ownRefusal(ev.Err) } @@ -441,16 +449,10 @@ func (w *sharedWatcher) Next(ctx context.Context) (gateway.WatchEvent, error) { } func (w *sharedWatcher) Stop() { - w.mu.Lock() - defer w.mu.Unlock() - if w.stopped { - return - } - w.stopped = true - if w.open != nil { - w.open.Stop() + w.stop.Do(func() { + w.Watcher.Stop() w.done() - } + }) } // errSharedRefused is the cause of an INTERNAL sent for the shared identity being diff --git a/internal/stream/shared_test.go b/internal/stream/shared_test.go index 1aed0ff..1182320 100644 --- a/internal/stream/shared_test.go +++ b/internal/stream/shared_test.go @@ -21,6 +21,7 @@ import ( "github.com/ConfigButler/krm-stream/gateway/kube" authorizationv1 "k8s.io/api/authorization/v1" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/flowcontrol" "github.com/ConfigButler/krm-foyer/internal/gate" "github.com/ConfigButler/krm-foyer/internal/interruption" @@ -254,6 +255,69 @@ func eventuallyEqual(t *testing.T, what string, got func() string, want string) } } +// Reviews and watch openings must share the configured budget, even though they +// use different client-go clients. The user's SelfSubjectReview is separate. +func TestSharedReviewsAndWatchesUseOneRateBudget(t *testing.T) { + api := newSharedAPI(t) + opened := make(chan time.Time, 1) + api.watch = func(w http.ResponseWriter, r *http.Request) { + opened <- time.Now() + api.serveWatch(w, r) + } + f := sharedFoyer(t, api, byHeader{}, metrics.New(), time.Minute, func(c *Config) { + c.Shared.QPS = 1 // A burst of two: the list and watch reviews spend it. + }) + ctx, cancel := context.WithTimeout(t.Context(), within) + defer cancel() + start := time.Now() + s := f.open(ctx, t, notes, as(userToken)) + s.until(t, "synced") + select { + case at := <-opened: + if elapsed := at.Sub(start); elapsed < 800*time.Millisecond { + t.Fatalf("two reviews and a watch reached the API server in %v; the watch bypassed the shared rate budget", elapsed) + } + case <-ctx.Done(): + t.Fatal("the shared watch never opened after the rate budget refilled") + } +} + +// Cancelling a request waiting for the shared budget must release its body and +// reach no API server, as any failed RoundTrip must. +func TestSharedRateWaitHonorsCancellation(t *testing.T) { + api := newSharedAPI(t) + limiter := flowcontrol.NewTokenBucketRateLimiter(1, 1) + limiter.TryAccept() // Spend the burst, so the next request must wait. + transport := sharedTransport{next: api.Client().Transport, rate: limiter} + ctx, cancel := context.WithCancel(t.Context()) + cancel() + body := &closeTrackedBody{Reader: strings.NewReader("{}")} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, api.URL, body) + if err != nil { + t.Fatal(err) + } + resp, err := transport.RoundTrip(req) + if resp != nil { + _ = resp.Body.Close() + } + if resp != nil || !errors.Is(err, context.Canceled) { + t.Fatalf("cancelled request: response %v, error %v", resp, err) + } + if !body.closed || len(api.received()) != 0 { + t.Fatalf("body closed: %v; upstream requests: %d", body.closed, len(api.received())) + } +} + +type closeTrackedBody struct { + io.Reader + closed bool +} + +func (b *closeTrackedBody) Close() error { + b.closed = true + return nil +} + // Streams of one scope share one watch at the API server, whoever opens them: three // for alice and three for bob are one watch, opened with the shared identity's // token. Each user is asked about once, not once a stream, and one change reaches @@ -334,6 +398,9 @@ func TestASharedStreamAsksAboutEachUser(t *testing.T) { if strings.Contains(carol.raw.String(), "note first") { t.Errorf("carol was sent the shared watch's notes: %s", carol.raw.String()) } + if refusals := logLines(f.logs.String(), map[string]any{"msg": "refused", "by": gate.ByKubernetes, "code": "FORBIDDEN"}); len(refusals) != 1 { + t.Errorf("carol's refusal logged %d times as Kubernetes':\n%s", len(refusals), f.logs.String()) + } var carols []authorizationv1.SubjectAccessReviewSpec for _, spec := range api.subjectReviews() { @@ -360,8 +427,9 @@ func TestASharedStreamAsksAboutEachUser(t *testing.T) { } // Decisions are reused across label selectors because the reviews ask about none -// (see decisions.Authorize). This fails if they start to, for a stream that asks for -// a selector: the key must then take it. +// (see decisions.Authorize). krm-stream documents that since 0.7.0 and announces a +// change to it; this fails if one comes anyway, for a stream that asks for a +// selector: the key must then take it. func TestTheReviewsAskAboutNoSelector(t *testing.T) { api := newSharedAPI(t) f := sharedFoyer(t, api, byHeader{}, nil, time.Minute) @@ -373,9 +441,9 @@ func TestTheReviewsAskAboutNoSelector(t *testing.T) { t.Fatalf("a review without resource attributes: %+v", spec) } verbs[attrs.Verb] = true - if attrs.LabelSelector != nil || attrs.FieldSelector != nil { - t.Errorf("the %s review asks about selectors (%+v, %+v): key decisions on them", - attrs.Verb, attrs.LabelSelector, attrs.FieldSelector) + if attrs.LabelSelector != nil || attrs.FieldSelector != nil || attrs.Subresource != "" { + t.Errorf("the %s review asks about selectors or a subresource (%+v, %+v, %q): key decisions on them", + attrs.Verb, attrs.LabelSelector, attrs.FieldSelector, attrs.Subresource) } } if !verbs["list"] || !verbs["watch"] { @@ -774,8 +842,13 @@ func TestAStuckOpeningHoldsUpNoOtherScope(t *testing.T) { return } stuck <- struct{}{} - <-r.Context().Done() // no answer, not even headers, until krm-foyer gives up - released <- struct{}{} + // No answer, not even headers, until krm-foyer gives up; or until the test + // ends, so a krm-foyer that never gives up fails it rather than hangs it. + select { + case <-r.Context().Done(): + released <- struct{}{} + case <-t.Context().Done(): + } } m := metrics.New() f := sharedFoyer(t, api, byHeader{}, m, time.Minute) diff --git a/internal/stream/stream.go b/internal/stream/stream.go index 82c0a99..08d9a42 100644 --- a/internal/stream/stream.go +++ b/internal/stream/stream.go @@ -21,6 +21,7 @@ import ( "github.com/ConfigButler/krm-stream/gateway" "github.com/ConfigButler/krm-stream/gateway/kube" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/rest" "github.com/ConfigButler/krm-foyer/internal/gate" @@ -136,7 +137,7 @@ func (s *Streams) ServeHTTP(w http.ResponseWriter, r *http.Request) { // unnamed target, the one cluster, exists. Scopes: gateway.ScopePolicy{Targets: []string{""}, AnyResource: true, AllowLabelSelector: true}, Projections: gateway.ProjectionPolicyFunc(project), - Diagnostics: s.diagnose, + Diagnostics: func(d gateway.Diagnostic) { s.diagnose(a, d) }, // Every write to the browser is bounded, so one that stopped reading ends its // stream rather than holding it, and a shared stream's rechecks, which wait // for the write in progress. @@ -181,11 +182,27 @@ func project(_ context.Context, _ gateway.Principal, _ gateway.Scope, requested // diagnose logs what went wrong behind an error the browser was sent: its code and // the kind of failure, never its text, which the API server or whatever answered -// instead wrote (see package upstream). A recovered resync is routine. -func (s *Streams) diagnose(d gateway.Diagnostic) { - if d.Code == gateway.CodeResyncRequired { +// instead wrote (see package upstream). A refusal is logged as every refusal is: by +// Kubernetes when it did not let the user list and watch, or did not accept their +// token; by krm-foyer when the scope is not one it serves. A recovered resync is +// routine. +func (s *Streams) diagnose(a *gate.Admission, d gateway.Diagnostic) { + switch d.Code { + case gateway.CodeResyncRequired: s.logger.Debug("stream resynchronised", "cause", class(d.Err)) return + case gateway.CodeForbidden, gateway.CodeUnauthenticated, gateway.CodeScopeInvalid: + // FORBIDDEN and UNAUTHENTICATED are Kubernetes' answers: a 403 or 401, or a + // SubjectAccessReview's denial. SCOPE_INVALID is the API server's only when it + // does not serve the resource; otherwise the gateway refused the scope. + var status apierrors.APIStatus + by := gate.ByFoyer + if d.Code != gateway.CodeScopeInvalid || errors.As(d.Err, &status) { + by = gate.ByKubernetes + } + a.Refused(by, "code", string(d.Code), "group", d.Scope.Group, "resource", d.Scope.Resource, + "namespace", d.Scope.Namespace, "name", d.Scope.Name) + return } s.logger.Warn("API server could not serve a stream", "code", string(d.Code), "terminal", d.Terminal, "cause", class(d.Err)) diff --git a/internal/stream/stream_test.go b/internal/stream/stream_test.go index b3674a9..ad53506 100644 --- a/internal/stream/stream_test.go +++ b/internal/stream/stream_test.go @@ -51,9 +51,12 @@ func (c credentials) Token(*http.Request) (gate.Credential, *interruption.Interr if session == "" { session = "s1" } - return gate.Credential{Token: c.token, Session: session, Live: live}, c.refused + return gate.Credential{Token: c.token, User: testUser, Session: session, Live: live}, c.refused } +// testUser is who the issuer says every test user is. +const testUser = "alice@example.test" + // byHeader hands each request the token named by its Test-Token header, as if each // came from a session of its own. type byHeader struct{} @@ -468,6 +471,33 @@ func TestScopesTheGatewayRefuses(t *testing.T) { if n := len(api.received()); n != 0 { t.Errorf("%d requests reached the API server", n) } + refusals := logLines(f.logs.String(), map[string]any{"msg": "refused", "by": gate.ByFoyer, + "code": "SCOPE_INVALID", "user": testUser, "route": "/stream", "path": "/stream/v1"}) + if len(refusals) != 7 { + t.Errorf("%d refusals logged by krm-foyer, want 7:\n%s", len(refusals), f.logs.String()) + } +} + +// logLines is what krm-foyer logged, a line at a time, for those that match all of +// want. +func logLines(logs string, want map[string]any) []map[string]any { + var found []map[string]any + for line := range strings.Lines(logs) { + var l map[string]any + if json.Unmarshal([]byte(line), &l) != nil { + continue + } + matches := true + for k, v := range want { + if l[k] != v { + matches = false + } + } + if matches { + found = append(found, l) + } + } + return found } // Each built-in projection may be asked for: a projection hides nothing from a @@ -519,6 +549,12 @@ func TestKubernetesRefusalsKeepTheirMeaning(t *testing.T) { if host := strings.TrimPrefix(api.URL, "https://"); strings.Contains(s.raw.String(), host) { t.Errorf("the API server's address reached the browser: %s", s.raw.String()) } + // Logged once, as every refusal is: Kubernetes' own, and what was asked for. + want := map[string]any{"msg": "refused", "by": gate.ByKubernetes, "code": tc.code, "user": testUser, + "route": "/stream", "group": "hello.krm-foyer.example", "resource": "notes", "namespace": "hello"} + if n := len(logLines(f.logs.String(), want)); n != 1 || len(logLines(f.logs.String(), map[string]any{"msg": "refused"})) != 1 { + t.Errorf("want one line like %v; logged:\n%s", want, f.logs.String()) + } }) } }