From ec89db01df20c93412fd4158129c582dcb9a4364 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 19:10:12 +0000 Subject: [PATCH 01/43] =?UTF-8?q?feat(client):=20LiveResourceStore=20?= =?UTF-8?q?=E2=80=94=20the=20consumer=20half,=20green=20against=20the=20co?= =?UTF-8?q?rpus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client fixtures asserted nothing until now: test/ only checked that the corpus was well-formed. It now replays each fixture's `events` into a store, applies `client.edits` at their `after:` positions, and asserts `expect` — dirty, conflicts, draftSubset, absentPaths, readOnlyPaths, flashed, patch — plus the checkpoints in conflict-and-converge. All 14 client fixtures pass. The store is the two layers of spec §4.1, kept apart: - applyServerEvent REPLACES the authoritative object (a deep-merge cannot express a removal, so it resurrects the field the server just deleted), then three-way reconciles the editable regions against the PREVIOUS server object as base (R-THREEWAY). - read-only regions follow the server live and flash what moved. They never grow a draft, a dirty flag or a conflict, and never enter a patch — and a write to one is refused, not ignored. - dirtiness is derived on every ask (R-DERIVED); paths are segment arrays (R-ID); deepEqual is a structural compare, not JSON.stringify. - patch() is RFC 7386 over the editable changes only; arrays go whole. Each of the three bugs the inline original shipped is now caught by a mutation: dropping the base term fails 6 fixtures, keeping a pruned key fails 2, pruning on reset instead of synced fails 1. Two rules no fixture pins — a REFUSED edit to status, and key-order-independent equality — are pinned in test/invariants.test.ts, which is where docs §7 says the property tests live. No runtime dependencies. The emitted ESM imports nothing but its own relative ./*.js, and a browser can import it with no bundler. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/krm-stream/src/deep.ts | 45 +++ packages/krm-stream/src/index.ts | 19 +- packages/krm-stream/src/merge.ts | 136 ++++++++ packages/krm-stream/src/path.ts | 100 ++++++ packages/krm-stream/src/policy.ts | 43 +++ packages/krm-stream/src/store.ts | 366 ++++++++++++++++++++ packages/krm-stream/src/types.ts | 8 + packages/krm-stream/test/invariants.test.ts | 208 +++++++++++ packages/krm-stream/test/store.test.ts | 209 +++++++++++ 9 files changed, 1130 insertions(+), 4 deletions(-) create mode 100644 packages/krm-stream/src/deep.ts create mode 100644 packages/krm-stream/src/merge.ts create mode 100644 packages/krm-stream/src/path.ts create mode 100644 packages/krm-stream/src/policy.ts create mode 100644 packages/krm-stream/src/store.ts create mode 100644 packages/krm-stream/test/invariants.test.ts create mode 100644 packages/krm-stream/test/store.test.ts diff --git a/packages/krm-stream/src/deep.ts b/packages/krm-stream/src/deep.ts new file mode 100644 index 0000000..faa829e --- /dev/null +++ b/packages/krm-stream/src/deep.ts @@ -0,0 +1,45 @@ +// Structural comparison and cloning of JSON values. No dependencies, and deliberately not +// `structuredClone`/`JSON.stringify` — see below. + +/** A plain JSON object — an object that is not an array and not null. Everything a KRM object + * contains is one of: plain object, array, string, number, boolean, null. */ +export function isPlainObject(v: unknown): v is Record { + return typeof v === "object" && v !== null && !Array.isArray(v); +} + +/** Recursive, KEY-ORDER-INDEPENDENT structural equality (I-ORDER-EQ). + * + * `JSON.stringify(a) === JSON.stringify(b)` is the tempting one-liner and it is wrong: an API + * server may serialize the same object with its keys in a different order, and stringify-equality + * then reports a change that did not happen — which flashes the UI, and worse, makes an untouched + * field look like it "moved" so a three-way merge stops leaving edits alone. */ +export function deepEqual(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false; + return a.every((x, i) => deepEqual(x, b[i])); + } + + if (isPlainObject(a) && isPlainObject(b)) { + const ka = Object.keys(a); + const kb = Object.keys(b); + if (ka.length !== kb.length) return false; + return ka.every((k) => Object.prototype.hasOwnProperty.call(b, k) && deepEqual(a[k], b[k])); + } + + return false; +} + +/** A deep copy of a JSON value. The store never hands out a reference into its own state, and never + * keeps a reference into a caller's object: a draft that aliases the server snapshot would make the + * base drift silently as the user types, and the three-way merge would compare an object to itself. */ +export function clone(v: T): T { + if (Array.isArray(v)) return v.map(clone) as unknown as T; + if (isPlainObject(v)) { + const out: Record = {}; + for (const [k, x] of Object.entries(v)) out[k] = clone(x); + return out as T; + } + return v; +} diff --git a/packages/krm-stream/src/index.ts b/packages/krm-stream/src/index.ts index 63bb9d5..c7a040e 100644 --- a/packages/krm-stream/src/index.ts +++ b/packages/krm-stream/src/index.ts @@ -4,12 +4,23 @@ // // LiveResourceStore holds server truth + your draft; three-way merges every watch event; // derives dirtiness; tracks conflicts; builds the merge patch. -// connectResourceStream a conforming SSE consumer that feeds a store. +// connectResourceStream a conforming SSE consumer that feeds a store. (not built yet) // -// Neither exists yet. This package currently ships the protocol TYPES — the half of the contract -// that both implementations already agree on — while the store is built test-first against -// conformance/. See ../../../docs/client-state-model.md for the algorithm it must implement, and +// The store is built test-first against conformance/ — the same fixtures the Go gateway runs. See +// ../../../docs/client-state-model.md for the algorithm, ../../../spec/v1.md for the wire, and // CONTRIBUTING.md for the order of work. +// +// No runtime dependencies, and none of this knows anything about GitOps, Flux, Dex, kcp or +// ConfigButler. It knows KRM. + +export { LiveResourceStore } from "./store.ts"; +export type { ApplyResult, ApplyOptions } from "./store.ts"; + +export { defaultPolicy, readOnlyPolicy, regionPolicy, DEFAULT_EDITABLE_REGIONS } from "./policy.ts"; + +// Useful to a host that renders paths, and to anyone writing a policy: identity is a segment ARRAY. +export { deepEqual, clone } from "./deep.ts"; +export { get, has, pathKey, parsePointer, isPrefix } from "./path.ts"; export type { KRMObject, diff --git a/packages/krm-stream/src/merge.ts b/packages/krm-stream/src/merge.ts new file mode 100644 index 0000000..8d0779a --- /dev/null +++ b/packages/krm-stream/src/merge.ts @@ -0,0 +1,136 @@ +// The deep three-way reconcile (docs/client-state-model.md §4). +// +// base the PREVIOUS server object we last reconciled from +// ours the user's working draft +// theirs the complete incoming server object +// +// The base is the whole point (R-THREEWAY). Compare `theirs` to `ours` instead and you cannot tell +// "the server changed this" from "the user typed this", so every watch event looks like the server +// changed everything the user touched — and a controller's status heartbeat false-conflicts an edit +// three fields away. Only a path where base ≠ theirs was actually moved by the server. +// +// Note what is NOT here: replacing the authoritative server object. That happens in the store, and +// it is a REPLACEMENT, never a merge (spec §4.1) — merging complete server objects resurrects the +// fields the server just removed. These are two layers, and conflating them is the ghost bug. + +import { clone, deepEqual, isPlainObject } from "./deep.ts"; +import { at, pathKey } from "./path.ts"; +import type { Conflict, Path } from "./types.ts"; + +/** The store's per-resource view of which paths are editable — the policy, minus the paths this + * particular object declared redacted. */ +export interface Regions { + /** This path is inside an editable region: three-way merge it. */ + editable(path: Path): boolean; + /** Not editable itself, but an editable region lives underneath: recurse, don't replace. */ + container(path: Path): boolean; +} + +export interface MergeState { + regions: Regions; + /** Conflicts persist between events — a conflict the next event does not touch is still a + * conflict. (Dirtiness is the opposite: derived, never stored. R-DERIVED.) */ + conflicts: Map; + /** An output, not state: the paths the server moved. The host flashes them and forgets them. */ + flashed: Path[]; +} + +/** Reconcile one resource. Returns the new draft. */ +export function reconcile(s: MergeState, base: unknown, ours: unknown, theirs: unknown): unknown { + return node(s, [], base, ours, theirs); +} + +function node(s: MergeState, path: Path, base: unknown, ours: unknown, theirs: unknown): unknown { + if (s.regions.editable(path)) return mergeEditable(s, path, base, ours, theirs); + if (s.regions.container(path)) return mergeContainer(s, path, base, ours, theirs); + return follow(s, path, base, theirs); +} + +/** A read-only region: take the server's value, flash what moved, and never look at the draft. + * There is no draft here to look at — that is what read-only MEANS — but it still updates live. */ +function follow(s: MergeState, path: Path, base: unknown, theirs: unknown): unknown { + if (isPlainObject(base) && isPlainObject(theirs)) { + const out: Record = {}; + for (const k of unionKeys(base, theirs)) { + const v = follow(s, [...path, k], at(base, k), at(theirs, k)); + if (v !== undefined) out[k] = v; + } + return out; // structurally clone(theirs): a key `theirs` lacks is GONE, not retained + } + // Arrays flash atomically, at the array's own path — a UI highlights "the conditions changed", + // not "conditions[0].reason changed", and §4.1 treats arrays as one value anyway. + if (!deepEqual(base, theirs)) s.flashed.push(path); + return clone(theirs); +} + +/** Not editable, but an editable region is somewhere below (`[]`, `["metadata"]`). Recurse and let + * each child dispatch for itself: `metadata.labels` is merged while `metadata.name` beside it + * follows the server. */ +function mergeContainer(s: MergeState, path: Path, base: unknown, ours: unknown, theirs: unknown): unknown { + if (!isPlainObject(base) && !isPlainObject(ours) && !isPlainObject(theirs)) { + return follow(s, path, base, theirs); + } + return recurse(s, path, base, ours, theirs); +} + +function mergeEditable(s: MergeState, path: Path, base: unknown, ours: unknown, theirs: unknown): unknown { + const defined = [base, ours, theirs].filter((v) => v !== undefined); + if (defined.length > 0 && defined.every(isPlainObject)) return recurse(s, path, base, ours, theirs); + + // Everything else — scalars, arrays, and any node where the three disagree about their very shape + // — is ONE atomic value. + // + // For arrays this is docs §4.1, and it is coarse on purpose: a positional merge of an array whose + // length moved mis-aligns (a PREPENDED sidecar would merge the user's edit into the wrong + // container), so a dirty array is treated as a single value and the user's version wins until they + // resolve it. RFC 7386 replaces arrays wholesale too, so the patch format needs no special case. + if (deepEqual(base, ours)) { + // The user has no edit here. Follow the server — including following it into deletion. + if (!deepEqual(base, theirs)) s.flashed.push(path); + clearConflict(s, path); + return clone(theirs); + } + // Dirty: `ours` diverged from `base`. The user's value is never silently overwritten. + if (deepEqual(base, theirs)) return clone(ours); // the server did not move it — no conflict (I-NOFALSE) + if (deepEqual(ours, theirs)) { + clearConflict(s, path); // the server arrived at what the user typed (I-CONVERGE) + return clone(ours); + } + setConflict(s, path, theirs); + return clone(ours); +} + +function recurse(s: MergeState, path: Path, base: unknown, ours: unknown, theirs: unknown): unknown { + const out: Record = {}; + for (const k of unionKeys(base, ours, theirs)) { + const v = node(s, [...path, k], at(base, k), at(ours, k), at(theirs, k)); + if (v !== undefined) out[k] = v; // prune: a key that reconciled to nothing is GONE + } + // The server removed this whole subtree and nothing of the user's survived in it. Returning `{}` + // here would leave an empty container the server does not have — which then reads as a dirty + // "the user added an empty map" forever. (`theirs === {}` is a real empty map and is kept.) + if (theirs === undefined && Object.keys(out).length === 0) return undefined; + return out; +} + +function unionKeys(...values: unknown[]): string[] { + const keys: string[] = []; + const seen = new Set(); + for (const v of values) { + if (!isPlainObject(v)) continue; + for (const k of Object.keys(v)) { + if (seen.has(k)) continue; + seen.add(k); + keys.push(k); + } + } + return keys; +} + +function setConflict(s: MergeState, path: Path, theirs: unknown): void { + s.conflicts.set(pathKey(path), { path: [...path], theirs: clone(theirs) }); +} + +function clearConflict(s: MergeState, path: Path): void { + s.conflicts.delete(pathKey(path)); +} diff --git a/packages/krm-stream/src/path.ts b/packages/krm-stream/src/path.ts new file mode 100644 index 0000000..1a8dc5c --- /dev/null +++ b/packages/krm-stream/src/path.ts @@ -0,0 +1,100 @@ +// Paths: arrays of segments, never dot-joined strings (R-ID). +// +// `["metadata","labels","app.kubernetes.io/name"]` is THREE segments, the last containing two dots +// and a slash. Join it with dots and it splits back into six segments addressing nothing at all — +// no crash, no error, the conflict lookup just silently never matches and the user's edit is lost +// on save. That bug is the reason this module exists and the reason `Path` is not a string. + +import { isPlainObject } from "./deep.ts"; +import type { Path } from "./types.ts"; + +/** A scalar key for a path, where a Map needs one. Encoded STRUCTURALLY — never by joining segments + * with a magic separator, which is how the original bug (a null byte vs a space) got in. */ +export function pathKey(path: Path): string { + return JSON.stringify(path); +} + +/** True when `prefix` addresses `path` or an ancestor of it. Segment-wise: `["data"]` is a prefix of + * `["data","token"]`, and `["dat"]` is a prefix of nothing. */ +export function isPrefix(prefix: Path, path: Path): boolean { + if (prefix.length > path.length) return false; + return prefix.every((seg, i) => String(seg) === String(path[i])); +} + +/** The child of a JSON value at one segment, or undefined if there isn't one. Indexing a scalar (or + * a string, which would happily answer to `.length`) yields undefined, not nonsense. */ +export function at(value: unknown, seg: string | number): unknown { + if (Array.isArray(value)) return value[Number(seg)]; + if (isPlainObject(value)) return value[String(seg)]; + return undefined; +} + +export function get(value: unknown, path: Path): unknown { + let cur = value; + for (const seg of path) cur = at(cur, seg); + return cur; +} + +/** True when the path is really THERE — as opposed to present with an undefined value. The + * difference is the whole of `absentPaths`: a ghost is a key that still exists. */ +export function has(value: unknown, path: Path): boolean { + let cur = value; + for (const seg of path) { + if (Array.isArray(cur)) { + if (Number(seg) >= cur.length) return false; + } else if (isPlainObject(cur)) { + if (!Object.prototype.hasOwnProperty.call(cur, String(seg))) return false; + } else { + return false; + } + cur = at(cur, seg); + } + return cur !== undefined; +} + +/** Set a value at a path, creating the containers it passes through. A numeric segment creates an + * array, a string segment an object — so `["spec","ports",0]` does the right thing on a fresh spec. */ +export function setAt(root: Record, path: Path, value: unknown): void { + if (path.length === 0) throw new Error("krm-stream: cannot set the root of an object"); + let cur: Record | unknown[] = root; + for (let i = 0; i < path.length - 1; i++) { + const seg = path[i]!; + let next = at(cur, seg); + if (!isPlainObject(next) && !Array.isArray(next)) { + next = typeof path[i + 1] === "number" ? [] : {}; + assign(cur, seg, next); + } + cur = next as Record | unknown[]; + } + assign(cur, path[path.length - 1]!, value); +} + +/** Remove the key (or array element) a path addresses. Removing what is not there is a no-op, not + * an error: the server may have removed it first, and a UI must not blow up on that race. */ +export function removeAt(root: Record, path: Path): void { + if (path.length === 0) throw new Error("krm-stream: cannot remove the root of an object"); + const parent = get(root, path.slice(0, -1)); + const last = path[path.length - 1]!; + if (Array.isArray(parent)) parent.splice(Number(last), 1); + else if (isPlainObject(parent)) delete parent[String(last)]; +} + +function assign(container: Record | unknown[], seg: string | number, value: unknown): void { + if (Array.isArray(container)) container[Number(seg)] = value; + else container[String(seg)] = value; +} + +/** An RFC 6901 JSON Pointer -> a segment array. This is the form `redactedPaths` arrives in on the + * wire (spec §3); inside the engine everything is a segment array. + * + * Segments stay STRINGS. `/data/0` on a map whose key is literally "0" must not become the number + * 0 — a pointer does not carry enough type information to tell those apart, and guessing corrupts + * exactly the Secret keys this is used to protect. */ +export function parsePointer(pointer: string): Path { + if (pointer === "") return []; + if (!pointer.startsWith("/")) throw new Error(`krm-stream: not an RFC 6901 pointer: ${JSON.stringify(pointer)}`); + return pointer + .slice(1) + .split("/") + .map((seg) => seg.replaceAll("~1", "/").replaceAll("~0", "~")); +} diff --git a/packages/krm-stream/src/policy.ts b/packages/krm-stream/src/policy.ts new file mode 100644 index 0000000..335fa9c --- /dev/null +++ b/packages/krm-stream/src/policy.ts @@ -0,0 +1,43 @@ +// Which regions of a KRM object a user may edit. +// +// Read-only is NOT ignored (docs §3). A read-only region still follows the server live and still +// flashes what changed — that is the entire live-status-watch use case, and it is the product. It +// simply never grows a draft, a dirty flag, or a conflict, and never appears in a patch. + +import { isPrefix } from "./path.ts"; +import type { EditabilityPolicy, KRMObject, Path } from "./types.ts"; + +/** The default editable regions. Note that two of them are at the object ROOT: `ConfigMap.data` and + * `Secret.data`/`stringData` carry editable payload outside `spec`, and an engine that assumes + * everything editable lives under `spec` cannot edit a ConfigMap at all — which is the one kind the + * first consumer edits most. `binaryData` is deliberately absent: it is base64 of arbitrary bytes, + * and a text input silently corrupts it. */ +export const DEFAULT_EDITABLE_REGIONS: Path[] = [ + ["spec"], + ["metadata", "labels"], + ["metadata", "annotations"], + ["data"], + ["stringData"], +]; + +/** A policy from a list of editable region roots. Everything under a root is editable; everything + * else follows the server. */ +export function regionPolicy(roots: Path[]): EditabilityPolicy { + return { + isEditable: (_obj: KRMObject, path: Path) => roots.some((root) => isPrefix(root, path)), + // A path that is not editable itself but that an editable region lives UNDER — `[]` and + // `["metadata"]` for the defaults. The merge must recurse through these rather than replacing + // them wholesale, or an edit to `metadata.labels` would be clobbered by the read-only handling + // of `metadata.name` sitting beside it. + containsEditable: (_obj: KRMObject, path: Path) => + roots.some((root) => root.length > path.length && isPrefix(path, root)), + }; +} + +/** `spec`, `metadata.labels`/`annotations`, `data`, `stringData` editable; `status`, the rest of + * `metadata`, `apiVersion`, `kind`, `binaryData` read-only. */ +export const defaultPolicy: EditabilityPolicy = regionPolicy(DEFAULT_EDITABLE_REGIONS); + +/** Everything read-only: the status-watch use case. Same engine, same stream, no draft — a viewer + * that cannot accidentally offer to save. */ +export const readOnlyPolicy: EditabilityPolicy = regionPolicy([]); diff --git a/packages/krm-stream/src/store.ts b/packages/krm-stream/src/store.ts new file mode 100644 index 0000000..68229db --- /dev/null +++ b/packages/krm-stream/src/store.ts @@ -0,0 +1,366 @@ +// LiveResourceStore — the consumer half of the protocol (spec §10) and the engine of +// docs/client-state-model.md. +// +// It holds, per uid: +// +// server(id) the authoritative projected object. REPLACED by every added/modified — never +// deep-merged, because a deep merge of complete objects cannot express a removal and +// so resurrects the field the server just deleted (spec §4.1). It is also the merge +// BASE, and it advances on every event (I-BASESHIFT). +// draft(id) the same object with the editable regions three-way merged against the user's edits. +// +// Dirtiness is not in that list, on purpose: it is DERIVED, every time it is asked for (R-DERIVED). +// A cached dirty set goes stale on the next watch event — the field the server converged onto stays +// flagged forever, or unflags itself only on the next click. + +import { clone, deepEqual, isPlainObject } from "./deep.ts"; +import { get, has, isPrefix, parsePointer, pathKey, removeAt, setAt } from "./path.ts"; +import { reconcile, type MergeState, type Regions } from "./merge.ts"; +import { defaultPolicy } from "./policy.ts"; +import type { Change, Conflict, EditabilityPolicy, KRMObject, Path } from "./types.ts"; + +/** What one server event did, for a host that wants to animate it. `flashed` is an OUTPUT, not + * state: the host highlights those paths and forgets them. */ +export interface ApplyResult { + /** The uid was not known: this is an arrival, not a change. */ + added: boolean; + /** Keys or array elements appeared or disappeared — a UI must rebuild rows, not just re-read values. */ + structural: boolean; + /** Paths the server moved, in read-only regions and in editable regions the user had not touched. */ + flashed: Path[]; + /** The paths this object is now conflicted at (the complete set, not just the new ones). */ + conflicts: Path[]; +} + +export interface ApplyOptions { + /** RFC 6901 JSON Pointers (`"/data/token"`, as they arrive on the wire) or segment arrays. The + * paths this object's values were masked at: read-only, never dirtiable, never in a patch. */ + redactedPaths?: (string | Path)[]; +} + +interface Resource { + server: KRMObject; + draft: KRMObject; + redacted: Path[]; + conflicts: Map; +} + +export class LiveResourceStore { + readonly #policy: EditabilityPolicy; + readonly #resources = new Map(); + readonly #subscribers = new Set<() => void>(); + + /** Non-null exactly while a snapshot cycle is open: the uids seen since `beginSnapshot()`. + * Pruning reads it in `endSnapshot()` and NOWHERE else — a cycle that never completes must prune + * nothing, or a network hiccup makes the user watch half their resources evaporate (spec §5). */ + #seen: Set | null = null; + + constructor(policy: EditabilityPolicy = defaultPolicy) { + this.#policy = policy; + } + + // ------------------------------------------------------------------ the stream in -- + + /** `added` and `modified` — the only two upsert spellings, and they are treated identically + * (spec §4). Both mean "here is this object's complete current state". */ + applyServerEvent(object: KRMObject, opts: ApplyOptions = {}): ApplyResult { + const id = object.metadata.uid; + const incoming = clone(object); + const redacted = (opts.redactedPaths ?? []).map((p) => (typeof p === "string" ? parsePointer(p) : [...p])); + + this.#seen?.add(id); + + const existing = this.#resources.get(id); + if (!existing) { + // A resource we have never seen: base = draft = incoming. There is nothing to merge, and + // nothing flashes — an arrival is not a change. + this.#resources.set(id, { server: incoming, draft: clone(incoming), redacted, conflicts: new Map() }); + this.#notify(); + return { added: true, structural: true, flashed: [], conflicts: [] }; + } + + const state: MergeState = { + regions: this.#regionsFor(incoming, redacted), + conflicts: existing.conflicts, + flashed: [], + }; + const merged = reconcile(state, existing.server, existing.draft, incoming) as KRMObject; + + const structural = !sameShape(existing.draft, merged); + // The REPLACEMENT (spec §4.1) and the base shift (I-BASESHIFT), in one line. Everything above + // reconciled against the OLD server object; from here on, `incoming` is the base. + existing.server = incoming; + existing.draft = merged; + existing.redacted = redacted; + + this.#notify(); + return { + added: false, + structural, + flashed: state.flashed, + conflicts: [...existing.conflicts.values()].map((c) => c.path), + }; + } + + /** `deleted`. The object is gone, and so is any draft of it — the user was editing something that + * no longer exists. (A recreate under the same name is a DIFFERENT uid and starts clean; that is + * the whole reason identity is the uid.) */ + removeResource(id: string): void { + if (this.#resources.delete(id)) this.#notify(); + } + + /** `reset`. Mark every known uid unseen — and prune NOTHING yet. */ + beginSnapshot(): void { + this.#seen = new Set(); + } + + /** `synced`. The snapshot is complete, so what it did not mention is genuinely gone. This is the + * only place anything is pruned, and it is what removes an object deleted while the consumer was + * disconnected — the one event the consumer never saw. */ + endSnapshot(): void { + const seen = this.#seen; + if (!seen) return; // a `synced` with no open cycle: ignore, do not prune the world + this.#seen = null; + let pruned = false; + for (const id of [...this.#resources.keys()]) { + if (!seen.has(id)) { + this.#resources.delete(id); + pruned = true; + } + } + if (pruned) this.#notify(); + } + + /** The save succeeded and this is the object it produced. The watch will echo it too — and that + * echo is a harmless no-op (I-IDEMPOTENT) — but a UI should not have to wait for it to stop + * showing the field as dirty. */ + adoptSaved(object: KRMObject): void { + const existing = this.#resources.get(object.metadata.uid); + if (!existing) { + this.applyServerEvent(object); + return; + } + existing.server = clone(object); + existing.draft = clone(object); + existing.conflicts.clear(); + this.#notify(); + } + + // ------------------------------------------------------------------------- edits -- + + setValue(id: string, path: Path, value: unknown): void { + const res = this.#editable(id, path); + setAt(res.draft, path, clone(value)); + this.#settle(res, path); + } + + /** For a map entry or an object key the user deleted. It stays deleted across watch events (the + * merge sees ours=undefined, base=theirs and keeps the deletion) and becomes a `null` in the patch. */ + removeKey(id: string, path: Path): void { + const res = this.#editable(id, path); + removeAt(res.draft, path); + this.#settle(res, path); + } + + /** `path` addresses the MAP; `key` is the new entry. A new row in a UI starts empty. */ + addKey(id: string, path: Path, key: string, value: unknown = ""): void { + const res = this.#editable(id, [...path, key]); + setAt(res.draft, [...path, key], clone(value)); + this.#settle(res, [...path, key]); + } + + /** `path` addresses the MAP. Order is preserved — renaming a label must not make its row jump to + * the bottom of the list while the user is typing in it. */ + renameKey(id: string, path: Path, oldKey: string, newKey: string): void { + const res = this.#editable(id, [...path, oldKey]); + this.#editable(id, [...path, newKey]); + const map = get(res.draft, path); + if (!isPlainObject(map)) throw new Error(`krm-stream: ${pathKey(path)} is not a map`); + const renamed: Record = {}; + for (const [k, v] of Object.entries(map)) renamed[k === oldKey ? newKey : k] = v; + setAt(res.draft, path, renamed); + this.#settle(res, [...path, oldKey]); + this.#settle(res, [...path, newKey]); + } + + /** Throw the local edit away and take the server's value — which is also how a conflict is + * resolved in the server's favour. */ + revert(id: string, path: Path): void { + const res = this.#editable(id, path); + if (has(res.server, path)) setAt(res.draft, path, clone(get(res.server, path))); + else removeAt(res.draft, path); + this.#settle(res, path); + } + + /** Resolve a conflict by keeping the server's value. */ + takeTheirs(id: string, path: Path): void { + this.revert(id, path); + } + + // ----------------------------------------------------------------------- queries -- + + ids(): string[] { + return [...this.#resources.keys()]; + } + + /** The authoritative server object, including live `status`. */ + server(id: string): KRMObject { + return clone(this.#must(id).server); + } + + /** The server object with the editable regions merged over it. What a UI renders and edits. */ + draft(id: string): KRMObject { + return clone(this.#must(id).draft); + } + + /** Read-only convenience for the watch UI. A kind with no status simply has none. */ + status(id: string): unknown { + return clone(this.#must(id).server["status"]); + } + + /** Every pending edit, derived fresh by comparing the draft to the server object. Arrays appear + * whole (§4.1), which is also what RFC 7386 wants. */ + changes(id: string): Change[] { + const res = this.#must(id); + const out: Change[] = []; + this.#diff(this.#regionsFor(res.server, res.redacted), [], res.server, res.draft, out); + return out; + } + + /** R-DERIVED: never a stored flag. A read-only path is never dirty by construction. */ + isDirty(id: string, path: Path): boolean { + const res = this.#must(id); + if (this.isEditable(id, path)) return !deepEqual(get(res.server, path), get(res.draft, path)); + // A container (`[]`, `["metadata"]`) is dirty iff something editable underneath it is. + if (!this.#policy.containsEditable(res.server, path)) return false; + return this.changes(id).some((c) => isPrefix(path, c.path)); + } + + conflicts(id: string): Conflict[] { + return [...this.#must(id).conflicts.values()].map((c) => ({ path: [...c.path], theirs: clone(c.theirs) })); + } + + /** The policy, minus this object's redacted paths. A masked value is read-only for exactly the + * same reason `status` is: it is not the user's to change — and here, they never even saw it. */ + isEditable(id: string, path: Path): boolean { + const res = this.#must(id); + return this.#regionsFor(res.server, res.redacted).editable(path); + } + + /** An RFC 7386 merge patch of the editable changes, or null when there is nothing to save. + * + * Built from the user's EDITS — never from a diff of the whole object against the server. The + * object on the wire is a projection: a path the projection removed is simply not there, and a + * whole-object diff would read that absence as a deletion and try to patch it away (spec §3). */ + patch(id: string): Record | null { + const changes = this.changes(id); + if (changes.length === 0) return null; + const out: Record = {}; + for (const c of changes) setAt(out, c.path, c.new === undefined ? null : clone(c.new)); + return out; + } + + /** A coarse "something changed" signal — the host re-renders and re-queries. Returns an + * unsubscribe. */ + subscribe(cb: () => void): () => void { + this.#subscribers.add(cb); + return () => this.#subscribers.delete(cb); + } + + // ---------------------------------------------------------------------- internals -- + + #must(id: string): Resource { + const res = this.#resources.get(id); + if (!res) throw new Error(`krm-stream: no resource ${JSON.stringify(id)}`); + return res; + } + + /** Every edit goes through here: a write to `status`, to `metadata.name`, or to a redacted path is + * refused. The engine is not the security boundary — the gateway rejects such a patch too — but a + * UI that cannot even form the edit is the difference between "safe" and "safe if the client is + * honest". */ + #editable(id: string, path: Path): Resource { + const res = this.#must(id); + if (!this.isEditable(id, path)) { + throw new Error(`krm-stream: ${pathKey(path)} is read-only`); + } + return res; + } + + #regionsFor(object: KRMObject, redacted: Path[]): Regions { + const isRedacted = (path: Path) => redacted.some((r) => isPrefix(r, path)); + return { + editable: (path) => !isRedacted(path) && this.#policy.isEditable(object, path), + container: (path) => this.#policy.containsEditable(object, path), + }; + } + + /** After an edit: a conflict whose path the draft has now brought back to the server's value is no + * longer a conflict. (Typing the server's value by hand resolves it, and so does `revert`.) */ + #settle(res: Resource, path: Path): void { + for (const [k, c] of res.conflicts) { + if (!isPrefix(c.path, path) && !isPrefix(path, c.path)) continue; + if (deepEqual(get(res.server, c.path), get(res.draft, c.path))) res.conflicts.delete(k); + } + this.#notify(); + } + + #diff(regions: Regions, path: Path, srv: unknown, drf: unknown, out: Change[]): void { + if (regions.editable(path)) { + const defined = [srv, drf].filter((v) => v !== undefined); + if (defined.length > 0 && defined.every(isPlainObject)) { + for (const k of unionKeys(srv, drf)) { + this.#diff(regions, [...path, k], (srv as never)?.[k], (drf as never)?.[k], out); + } + return; + } + // Scalars, arrays, and shape changes are one atomic value — the same rule the merge uses, so + // "what is dirty" and "what was merged atomically" can never disagree. + if (!deepEqual(srv, drf)) { + const kind = srv === undefined ? "add" : drf === undefined ? "delete" : "update"; + out.push({ path: [...path], kind, old: clone(srv), new: clone(drf) }); + } + return; + } + if (regions.container(path)) { + for (const k of unionKeys(srv, drf)) { + this.#diff(regions, [...path, k], (srv as never)?.[k], (drf as never)?.[k], out); + } + } + // Read-only: the draft IS the server there. Nothing to diff, nothing to save. + } + + #notify(): void { + for (const cb of this.#subscribers) cb(); + } +} + +function unionKeys(...values: unknown[]): string[] { + const keys: string[] = []; + const seen = new Set(); + for (const v of values) { + if (!isPlainObject(v)) continue; + for (const k of Object.keys(v)) { + if (!seen.has(k)) { + seen.add(k); + keys.push(k); + } + } + } + return keys; +} + +/** Did keys or array elements appear or disappear? A UI can re-read values cheaply; it must REBUILD + * when rows come and go — that is what `structural` tells it. Scalar value changes are not structural. */ +function sameShape(a: unknown, b: unknown): boolean { + if (isPlainObject(a) && isPlainObject(b)) { + const ka = Object.keys(a); + const kb = Object.keys(b); + if (ka.length !== kb.length) return false; + return ka.every((k) => Object.prototype.hasOwnProperty.call(b, k) && sameShape(a[k], b[k])); + } + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((x, i) => sameShape(x, b[i])); + } + return isPlainObject(a) === isPlainObject(b) && Array.isArray(a) === Array.isArray(b); +} diff --git a/packages/krm-stream/src/types.ts b/packages/krm-stream/src/types.ts index e761452..a6435c3 100644 --- a/packages/krm-stream/src/types.ts +++ b/packages/krm-stream/src/types.ts @@ -107,4 +107,12 @@ export interface Conflict { * entire live-status-watch use case. */ export interface EditabilityPolicy { isEditable(obj: KRMObject, path: Path): boolean; + /** True for a path that is not editable itself but that an editable region lives UNDER — `[]` and + * `["metadata"]` under the default policy. + * + * The merge needs both questions answered, and they are not each other's negation. `metadata` is + * not editable, but replacing it wholesale from the server would clobber the label the user is + * editing inside it; `status` is not editable either, and replacing it wholesale is exactly right. + * Without this second predicate a store cannot tell those two apart. */ + containsEditable(obj: KRMObject, path: Path): boolean; } diff --git a/packages/krm-stream/test/invariants.test.ts b/packages/krm-stream/test/invariants.test.ts new file mode 100644 index 0000000..bc527e0 --- /dev/null +++ b/packages/krm-stream/test/invariants.test.ts @@ -0,0 +1,208 @@ +// The invariants of docs/client-state-model.md §7 that no fixture pins. +// +// The conformance corpus is the contract, and it is where the rules that BOTH implementations owe +// each other live. These are the ones only the client can break — they need no gateway, no wire, and +// nothing to agree with. They belong here, not in conformance/. +// +// The one worth reading twice is I-READONLY. status-only-churn proves `status` follows the server +// and never becomes dirty, but the user in that fixture never TRIES to edit it — so the fixture +// stays green even against a store that would happily let them. "Structurally incapable of becoming +// an edit" is a claim about what happens when someone tries, and something has to try. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { body } from "./conformance.ts"; +import { LiveResourceStore, readOnlyPolicy } from "../src/index.ts"; +import type { Path } from "../src/types.ts"; + +const CM = "cm-app.v1"; // ConfigMap: data.log-level=info, data.replicas="3", one dotted label +const DEPLOY = "deploy-web.v1"; // Deployment: nested spec, a container array, a live status + +function seeded(ref: string, redactedPaths: string[] = []): [LiveResourceStore, string] { + const store = new LiveResourceStore(); + const obj = body(ref); + store.beginSnapshot(); + store.applyServerEvent(obj, { redactedPaths }); + store.endSnapshot(); + return [store, obj.metadata.uid]; +} + +test("I-READONLY — an edit to status is refused, not merely ignored", () => { + const [store, id] = seeded(DEPLOY); + const paths: Path[] = [ + ["status", "readyReplicas"], + ["status", "conditions"], + ["metadata", "name"], + ["metadata", "resourceVersion"], + ["apiVersion"], + ]; + for (const p of paths) { + assert.equal(store.isEditable(id, p), false, `${JSON.stringify(p)} must not be editable`); + assert.throws(() => store.setValue(id, p, 99), /read-only/, `setValue(${JSON.stringify(p)})`); + assert.throws(() => store.removeKey(id, p), /read-only/, `removeKey(${JSON.stringify(p)})`); + assert.equal(store.isDirty(id, p), false); + } + assert.equal(store.patch(id), null); +}); + +test("I-READONLY — the status-watch policy makes the whole object unsaveable", () => { + const store = new LiveResourceStore(readOnlyPolicy); + const obj = body(DEPLOY); + store.applyServerEvent(obj); + const id = obj.metadata.uid; + + assert.throws(() => store.setValue(id, ["spec", "replicas"], 5), /read-only/); + // ...but it still FOLLOWS the server and still flashes. Read-only is not ignored — it is the + // entire live-status-watch use case, and a viewer that stopped updating would be worthless. + const r = store.applyServerEvent(body("deploy-web.v2")); + assert.deepEqual((store.status(id) as { readyReplicas: number }).readyReplicas, 2); + assert.ok(r.flashed.some((p) => JSON.stringify(p) === JSON.stringify(["status", "readyReplicas"]))); + assert.equal(store.patch(id), null); +}); + +test("I-ORDER-EQ — a reordered-but-equal object is not a change, and must not flash", () => { + const [store, id] = seeded(DEPLOY); + const reordered = reorder(body(DEPLOY)); + assert.notEqual(JSON.stringify(reordered), JSON.stringify(body(DEPLOY)), "the test object must really be reordered"); + + const r = store.applyServerEvent(reordered); + assert.deepEqual(r.flashed, [], "a key-order change is not a change — JSON.stringify equality says it is"); + assert.equal(r.structural, false); + assert.equal(store.patch(id), null); +}); + +test("I-IDEMPOTENT — redelivering the same object is a no-op (a reconnect replays the snapshot)", () => { + const [store, id] = seeded(CM); + store.setValue(id, ["data", "log-level"], "debug"); + + const before = store.draft(id); + const r = store.applyServerEvent(body(CM)); + assert.deepEqual(store.draft(id), before, "the redelivery moved the draft"); + assert.deepEqual(r.flashed, []); + assert.deepEqual(store.conflicts(id), [], "the object did not change — there is nothing to conflict with"); + assert.deepEqual(store.patch(id), { data: { "log-level": "debug" } }, "and the edit is still pending"); +}); + +test("I-BASESHIFT — the second event reconciles against the first, not against the snapshot", () => { + const [store, id] = seeded(CM); + store.applyServerEvent(body("cm-app.v3")); // log-level: info -> warn. Nobody is editing it. + assert.equal((store.draft(id)["data"] as Record)["log-level"], "warn"); + + // Now the user edits it, and the server sends v3 AGAIN. Against the ORIGINAL snapshot as base this + // would look like "the server just changed log-level" and false-conflict; against v3 it is a no-op. + store.setValue(id, ["data", "log-level"], "debug"); + store.applyServerEvent(body("cm-app.v3")); + assert.deepEqual(store.conflicts(id), []); + assert.deepEqual(store.patch(id), { data: { "log-level": "debug" } }); +}); + +test("I-KEYSAFE — dots, slashes and a key that merely LOOKS like an array index", () => { + const [store, id] = seeded(CM); + store.setValue(id, ["metadata", "labels", "app.kubernetes.io/name"], "checkout"); + store.addKey(id, ["data"], "0", "a key literally named zero"); + store.addKey(id, ["metadata", "annotations"], "a~b/c.d", "tilde and slash and dot"); + + assert.deepEqual(store.patch(id), { + metadata: { + labels: { "app.kubernetes.io/name": "checkout" }, + annotations: { "a~b/c.d": "tilde and slash and dot" }, + }, + data: { "0": "a key literally named zero" }, + }); + assert.ok(store.isDirty(id, ["data", "0"])); + assert.ok(store.isDirty(id, ["metadata", "labels", "app.kubernetes.io/name"])); +}); + +test("removeKey and renameKey — a deletion is a null in the patch, and a rename is both", () => { + const [store, id] = seeded(CM); + store.removeKey(id, ["data", "replicas"]); + assert.deepEqual(store.patch(id), { data: { replicas: null } }); + + const [store2, id2] = seeded(CM); + store2.renameKey(id2, ["data"], "log-level", "logLevel"); + assert.deepEqual(store2.patch(id2), { data: { "log-level": null, logLevel: "info" } }); + assert.deepEqual(Object.keys(store2.draft(id2)["data"] as object), ["logLevel", "replicas"], "order is preserved"); +}); + +test("I-PATCH-ROUNDTRIP — applying patch(id) to the server object yields the draft", () => { + const [store, id] = seeded(DEPLOY); + store.setValue(id, ["spec", "replicas"], 5); + store.setValue(id, ["spec", "template", "spec", "containers", 0, "image"], "ghcr.io/x/web:v2"); + store.removeKey(id, ["spec", "paused"]); + store.setValue(id, ["metadata", "labels", "app.kubernetes.io/name"], "checkout"); + + const patched = applyMergePatch(store.server(id), store.patch(id)); + assert.deepEqual(patched, store.draft(id)); + // Arrays go whole (§4.1, and RFC 7386 agrees) — never as an index-addressed sub-patch. + const spec = (store.patch(id) as { spec: { template: { spec: { containers: unknown[] } } } }).spec; + assert.ok(Array.isArray(spec.template.spec.containers)); + assert.equal(spec.template.spec.containers.length, 1); +}); + +test("revert — takes the server's value back, and resolves the conflict with it", () => { + const [store, id] = seeded(CM); + store.setValue(id, ["data", "log-level"], "debug"); + store.applyServerEvent(body("cm-app.v3")); // the server says warn: a real conflict + assert.deepEqual(store.conflicts(id), [{ path: ["data", "log-level"], theirs: "warn" }]); + + store.revert(id, ["data", "log-level"]); + assert.deepEqual(store.conflicts(id), [], "taking theirs resolves the conflict"); + assert.equal(store.isDirty(id, ["data", "log-level"]), false); + assert.equal(store.patch(id), null); +}); + +test("adoptSaved — the save landed; stop showing it as dirty without waiting for the echo", () => { + const [store, id] = seeded(CM); + store.setValue(id, ["data", "log-level"], "debug"); + store.adoptSaved(body("cm-app.v4")); // what the API server returned + assert.equal(store.patch(id), null); + assert.deepEqual(store.conflicts(id), []); + + const r = store.applyServerEvent(body("cm-app.v4")); // ...and the watch echoes it: a no-op + assert.deepEqual(r.flashed, []); + assert.equal(store.patch(id), null); +}); + +test("I-REDACT — a masked value is read-only, is never dirty, and never reaches a patch", () => { + const [store, id] = seeded("secret-token.v1", ["/data/token", "/data/username"]); + assert.equal(store.isEditable(id, ["data", "token"]), false); + assert.throws(() => store.setValue(id, ["data", "token"], "hunter2"), /read-only/); + assert.throws(() => store.removeKey(id, ["data", "token"]), /read-only/); + + // The mask is still SHOWN — that is the point of keys-only disclosure: you can see that `token` + // exists without ever learning, or overwriting, what it is. + assert.equal((store.draft(id)["data"] as Record)["token"], "**REDACTED**"); + store.setValue(id, ["metadata", "labels", "app.kubernetes.io/name"], "checkout"); + assert.deepEqual(store.patch(id), { metadata: { labels: { "app.kubernetes.io/name": "checkout" } } }); +}); + +test("unknown resource — every query says so rather than inventing an empty object", () => { + const store = new LiveResourceStore(); + assert.deepEqual(store.ids(), []); + assert.throws(() => store.draft("nope"), /no resource/); + assert.throws(() => store.patch("nope"), /no resource/); +}); + +/** A structurally identical object with its keys emitted in a different order — what a re-serialized + * object from an API server legitimately looks like. */ +function reorder(v: T): T { + if (Array.isArray(v)) return v.map(reorder) as unknown as T; + if (v && typeof v === "object") { + const entries = Object.entries(v as Record).reverse(); + return Object.fromEntries(entries.map(([k, x]) => [k, reorder(x)])) as T; + } + return v; +} + +/** RFC 7386, the ~10 lines of it, so the roundtrip is checked against the SPEC and not against the + * same code that produced the patch. */ +function applyMergePatch(target: unknown, patch: unknown): unknown { + if (!patch || typeof patch !== "object" || Array.isArray(patch)) return patch; + const out: Record = + target && typeof target === "object" && !Array.isArray(target) ? { ...(target as Record) } : {}; + for (const [k, v] of Object.entries(patch as Record)) { + if (v === null) delete out[k]; + else out[k] = applyMergePatch(out[k], v); + } + return out; +} diff --git a/packages/krm-stream/test/store.test.ts b/packages/krm-stream/test/store.test.ts new file mode 100644 index 0000000..d58cf86 --- /dev/null +++ b/packages/krm-stream/test/store.test.ts @@ -0,0 +1,209 @@ +// The client's half of the conformance corpus, actually asserted. +// +// conformance.test.ts checks that the corpus is well-formed. This file checks that the STORE is: +// every client fixture is replayed event by event into a LiveResourceStore, the fixture's edits are +// applied at their `after:` positions, and what the store then holds is compared to `expect:`. +// +// The `after:` ordering is the whole point. An edit placed BEFORE the server change that collides +// with it is the only way to test a three-way merge at all — a suite that edited only at the end +// would pass against a store that just follows the server. +// +// Assertion strengths, per conformance/README.md: +// uids, dirty, conflicts exact sets (order-insensitive) +// patch exact — it is what gets sent to the API server +// draftSubset deep subset (arrays compared element-wise, same length) +// absentPaths the path is absent from BOTH server(id) and draft(id) +// readOnlyPaths not editable, not dirty, and setValue on it is refused +// flashed containment: every listed path flashed. NOT an exact set — a read-only +// region that changed flashes, and metadata.resourceVersion changes on +// EVERY event, so an exact set would assert that a conformant engine +// does not flash a read-only field that moved. See docs §3 ("read-only +// is not ignored"). The fixtures list the paths a UI must highlight. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { clientFixtures, resolve, type FixtureEdit, type FixtureExpect } from "./conformance.ts"; +import { LiveResourceStore } from "../src/index.ts"; +import type { Path, StreamEvent } from "../src/types.ts"; + +for (const f of clientFixtures()) { + test(`${f.id}: ${f.title}`, () => { + const store = new LiveResourceStore(); + const flashed: Path[] = []; + + for (const [i, fe] of f.events.entries()) { + feed(store, resolve(f, fe), flashed); + + for (const edit of f.client?.edits ?? []) { + if (edit.after === i) applyEdit(store, edit); + } + for (const cp of f.checkpoints ?? []) { + if (cp.after === i) check(store, flashed, cp, `${f.id} @checkpoint after=${i}`); + } + } + + if (f.client?.expect) check(store, flashed, f.client.expect, f.id); + }); +} + +/** The four protocol events a store cares about. `error` is a transport concern — the store holds + * state, not a connection — and an unknown type must be ignored, not thrown on (spec §0). */ +function feed(store: LiveResourceStore, ev: StreamEvent, flashed: Path[]): void { + switch (ev.type) { + case "reset": + store.beginSnapshot(); + break; + case "added": + case "modified": + flashed.push(...store.applyServerEvent(ev.object!, { redactedPaths: ev.redactedPaths }).flashed); + break; + case "deleted": + store.removeResource(ev.identity!.uid); + break; + case "synced": + store.endSnapshot(); + break; + default: + break; + } +} + +/** `path` in a fixture edit always addresses the FIELD, not its container — even for the two ops + * whose store signature takes the map plus a key. Keeping the fixture format uniform is worth the + * one line of translation here. */ +function applyEdit(store: LiveResourceStore, e: FixtureEdit): void { + const parent = e.path.slice(0, -1); + const last = e.path[e.path.length - 1]; + switch (e.op) { + case "set": + store.setValue(e.uid, e.path, e.value); + break; + case "remove": + store.removeKey(e.uid, e.path); + break; + case "addKey": + store.addKey(e.uid, parent, String(last), e.value); + break; + case "renameKey": + store.renameKey(e.uid, parent, String(last), e.newKey!); + break; + case "revert": + store.revert(e.uid, e.path); + break; + default: + throw new Error(`fixture: unknown edit op ${JSON.stringify(e.op)}`); + } +} + +function check(store: LiveResourceStore, flashed: Path[], want: FixtureExpect, where: string): void { + const ids = store.ids(); + + if (want.uids) assert.deepEqual(sorted(ids), sorted(want.uids), `${where}: uids`); + + // Dirtiness is derived, so we ask for it the way a UI would: the changes of every resource the + // store holds. Union, because no fixture edits two objects and a stray dirty path anywhere is a bug. + if (want.dirty) { + const dirty = ids.flatMap((id) => store.changes(id).map((c) => c.path)); + assert.deepEqual(keys(dirty), keys(want.dirty), `${where}: dirty paths`); + // R-DERIVED, from the other direction: the predicate and the change list must agree. + for (const p of want.dirty) { + assert.ok( + ids.some((id) => store.isDirty(id, p)), + `${where}: isDirty(${JSON.stringify(p)}) disagrees with changes()`, + ); + } + } + + if (want.conflicts) { + const got = ids.flatMap((id) => store.conflicts(id)); + assert.deepEqual(keys(got.map((c) => c.path)), keys(want.conflicts.map(conflictPath)), `${where}: conflict paths`); + for (const c of want.conflicts) { + if (Array.isArray(c) || c.theirs === undefined) continue; // the fixture pins the path only + const found = got.find((g) => key(g.path) === key(c.path)); + assert.deepEqual(found?.theirs, c.theirs, `${where}: conflict theirs at ${key(c.path)}`); + } + } + + const subject = subjectId(store); + + if (want.draftSubset) { + const drafts = subject ? [subject] : ids; + const ok = drafts.some((id) => isSubset(store.draft(id) as unknown, want.draftSubset)); + assert.ok( + ok, + `${where}: no draft matches draftSubset\nwant: ${JSON.stringify(want.draftSubset, null, 2)}\ngot: ${JSON.stringify( + drafts.map((id) => store.draft(id)), + null, + 2, + )}`, + ); + } + + for (const p of want.absentPaths ?? []) { + const id = subject ?? ids[0]!; + assert.equal(has(store.server(id), p), false, `${where}: ${key(p)} is a GHOST in server(${id})`); + assert.equal(has(store.draft(id), p), false, `${where}: ${key(p)} is a GHOST in draft(${id})`); + } + + for (const p of want.readOnlyPaths ?? []) { + const id = subject ?? ids[0]!; + assert.equal(store.isEditable(id, p), false, `${where}: ${key(p)} must be read-only`); + assert.equal(store.isDirty(id, p), false, `${where}: ${key(p)} is read-only and can never be dirty`); + assert.throws(() => store.setValue(id, p, "hunter2"), `${where}: a write to ${key(p)} must be refused`); + } + + for (const p of want.flashed ?? []) { + assert.ok(keys(flashed).includes(key(p)), `${where}: ${key(p)} changed on the server and must flash`); + } + + if (want.patch !== undefined) { + // No subject means no edits and more than one resource: then NOTHING may have a patch. + const targets = subject ? [subject] : ids; + for (const id of targets) assert.deepEqual(store.patch(id), want.patch, `${where}: patch(${id})`); + } +} + +/** The resource a per-object expectation (draftSubset, absentPaths, patch) is about. When the store + * holds exactly one, it is that one — which is every fixture that asserts a patch, including + * delete-recreate-uid, where the edited object no longer exists and the survivor is the subject. + * Otherwise there is no subject and the expectation must hold across the board. */ +function subjectId(store: LiveResourceStore): string | undefined { + const ids = store.ids(); + return ids.length === 1 ? ids[0] : undefined; +} + +/** Deep subset: every key the fixture names must match. Arrays are compared element-wise AND must + * be the same length — "the user's array is preserved intact" is exactly what array-atomic-on-change + * is asserting, and a prefix match would not see a server-appended sidecar leaking into the draft. */ +function isSubset(actual: unknown, want: unknown): boolean { + if (Array.isArray(want)) { + if (!Array.isArray(actual) || actual.length !== want.length) return false; + return want.every((w, i) => isSubset(actual[i], w)); + } + if (want && typeof want === "object") { + if (!actual || typeof actual !== "object" || Array.isArray(actual)) return false; + const a = actual as Record; + return Object.entries(want as Record).every(([k, v]) => k in a && isSubset(a[k], v)); + } + return Object.is(actual, want); +} + +function has(obj: unknown, path: Path): boolean { + let cur: unknown = obj; + for (const seg of path) { + if (cur === null || typeof cur !== "object") return false; + if (!(String(seg) in (cur as Record))) return false; + cur = (cur as Record)[String(seg)]; + } + return cur !== undefined; +} + +const conflictPath = (c: Path | { path: Path }): Path => (Array.isArray(c) ? c : c.path); +const key = (p: Path): string => JSON.stringify(p); +const keys = (ps: Path[]): string[] => ps.map(key).sort(); +const sorted = (xs: string[]): string[] => [...xs].sort(); + +// A suite that silently runs zero fixtures is the failure mode this whole repo exists to prevent. +test("the client suite actually ran the corpus", () => { + assert.ok(clientFixtures().length >= 10, "the client half of the corpus is missing fixtures"); +}); From 65491a018f29fa4e33bfb9c87786f27fce4c9619 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 19:16:24 +0000 Subject: [PATCH 02/43] build(client): typecheck the tests, and give TypeScript a linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TS side was two tools short of the Go side, and one of the gaps was a correctness hole rather than a nicety: tsconfig.json includes only src/, so `tsc --noEmit` saw ZERO files under test/ — and `node --test` on Node 22 STRIPS types, it does not check them. The conformance suite, whose entire job is to be the contract check, was the one piece of unverified TypeScript in the repo. tsconfig.test.json (noEmit, src + test) closes that; it needs @types/node, because the tests import node:test. Biome supplies what gofmt + go vet + golangci-lint supply on the other side, in one binary. Both run in `task lint` and in CI; `task fmt-client` fixes what it can. noNonNullAssertion is off, deliberately: with `noUncheckedIndexedAccess` on, `path[i]!` inside a loop bounded by `path.length` is the honest expression of a fact the bound already guarantees, and the alternative is a runtime guard for a branch that cannot happen. Turning OFF noUncheckedIndexedAccess to satisfy the lint rule would have traded a real check for a cosmetic one. CONTRIBUTING now says what the dependency rule actually means: no RUNTIME dependencies, ever — that one is absolute, the bundle is plain ESM a browser imports with no bundler. devDependencies ship nothing, and the three we have each pay for themselves. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 10 +- CONTRIBUTING.md | 18 +- Taskfile.yml | 13 ++ packages/krm-stream/biome.jsonc | 40 ++++ packages/krm-stream/package-lock.json | 182 +++++++++++++++++++ packages/krm-stream/package.json | 13 +- packages/krm-stream/src/deep.ts | 2 +- packages/krm-stream/src/index.ts | 25 ++- packages/krm-stream/src/path.ts | 2 +- packages/krm-stream/src/store.ts | 6 +- packages/krm-stream/test/conformance.test.ts | 4 +- packages/krm-stream/test/conformance.ts | 10 +- packages/krm-stream/test/invariants.test.ts | 10 +- packages/krm-stream/test/store.test.ts | 4 +- packages/krm-stream/tsconfig.test.json | 16 ++ 15 files changed, 313 insertions(+), 42 deletions(-) create mode 100644 packages/krm-stream/biome.jsonc create mode 100644 packages/krm-stream/tsconfig.test.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 43867be..4a652e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,10 +57,16 @@ jobs: cache-dependency-path: packages/krm-stream/package-lock.json - run: npm install --no-audit --no-fund working-directory: packages/krm-stream - # Node 22 runs .ts directly (type stripping) — the suite needs no test deps. + # Node 22 runs .ts directly (type stripping) — the suite needs no test runner dependency. - run: node --test working-directory: packages/krm-stream - - run: npx tsc --noEmit + # ...but stripping is not checking. This is the ONLY thing that typechecks test/, and test/ is + # where the conformance suite lives, so it is not optional. + - run: npx tsc --noEmit -p tsconfig.test.json working-directory: packages/krm-stream + - run: npx biome ci + working-directory: packages/krm-stream + # The build, last: it is what ships, and it must stay plain ESM a browser can import with no + # bundler and no runtime dependency. - run: npx tsc working-directory: packages/krm-stream diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2d99396..0ef136c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,8 +68,20 @@ don't know (spec §0). Anything else gets a new path segment (`/v2`), not a sile - **Go:** `gofmt`, `go vet`, `golangci-lint` clean. The wire types stay dependency-free — no client-go in `event.go`. -- **TypeScript:** `strict`. **No runtime dependencies, ever** — the published bundle is plain ESM a - browser can `import` with no bundler. `typescript` is the only devDependency; the tests run on - Node's built-in runner, which reads `.ts` directly. +- **TypeScript:** `strict`, `biome check` clean. **No runtime dependencies, ever** — the published + bundle is plain ESM a browser can `import` with no bundler, and that rule is absolute. + + **devDependencies are a different rule, and it is not "none".** They ship nothing, so the test is + whether they pay for themselves. Three do: + + | | why | + |---|---| + | `typescript` | the compiler | + | `@types/node` | without it `tsc` cannot see `test/` **at all** — the tests import `node:test`. `node --test` *strips* types, it does not check them, so `tsconfig.test.json` is the only thing standing between us and an unverified conformance suite | + | `@biomejs/biome` | lint + format in one binary: the TypeScript half of what `gofmt`/`go vet`/`golangci-lint` do for the gateway. `task fmt-client` fixes what it can | + + Two tsconfigs, and the difference matters: `tsconfig.json` is the **build** (`src/` only — its + `rootDir` is what ships); `tsconfig.test.json` is the **check** (`src/` + `test/`, `noEmit`). Both + run in `task lint` and in CI. - Comments explain *why*, and especially *what breaks if you do the obvious thing instead*. The fixtures are the best example of the house style: every one of them says what it defends. diff --git a/Taskfile.yml b/Taskfile.yml index 91b4a81..ad8529f 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -69,7 +69,20 @@ tasks: lint-client: dir: "{{.CLIENT_DIR}}" cmds: + # Two tsconfigs, on purpose. The first is the BUILD (src only — its rootDir is what ships); + # the second is the only thing that ever typechecks test/, because `node --test` STRIPS types + # rather than checking them. Without it the conformance suite — the file whose whole job is to + # be the contract check — would be the one piece of unverified TypeScript in the repo. - npx --no-install tsc --noEmit + - npx --no-install tsc --noEmit -p tsconfig.test.json + # biome = the TypeScript half of gofmt + go vet + golangci-lint. `task fmt-client` fixes. + - npx --no-install biome check + + fmt-client: + desc: "Format and auto-fix the TypeScript (the gofmt of this side)." + dir: "{{.CLIENT_DIR}}" + cmds: + - npx --no-install biome check --write # ------------------------------------------------------------------- build -- build-client: diff --git a/packages/krm-stream/biome.jsonc b/packages/krm-stream/biome.jsonc new file mode 100644 index 0000000..b8f7496 --- /dev/null +++ b/packages/krm-stream/biome.jsonc @@ -0,0 +1,40 @@ +{ + "$schema": "https://biomejs.dev/schemas/2.5.3/schema.json", + // The TypeScript half of `gofmt + go vet + golangci-lint`. One binary, one devDependency, and it + // ships NOTHING: the published bundle is still dependency-free ESM a browser imports with no + // bundler. Formatting is not a matter of taste here for the same reason gofmt is not — a review of + // this package should be an argument about the merge algorithm, never about where a line broke. + "files": { + "includes": ["src/**/*.ts", "test/**/*.ts", "*.json", "*.jsonc"] + }, + "formatter": { + "enabled": true, + "indentStyle": "space", + "indentWidth": 2, + // Wide, because the house style puts the WHY in a comment beside the code, and 80 columns turns + // every one of those into a paragraph. + "lineWidth": 120 + }, + "javascript": { + "formatter": { + "quoteStyle": "double", + "semicolons": "always", + "trailingCommas": "all" + } + }, + "linter": { + "enabled": true, + "rules": { + "preset": "recommended", + "style": { + // Off, deliberately, and it is not laziness: tsconfig has `noUncheckedIndexedAccess`, so + // EVERY array index is `T | undefined` — including `path[i]` inside a loop bounded by + // `path.length`. Given that stricter setting, `path[i]!` is the honest expression of a fact + // the loop bound already guarantees; the alternative is a runtime guard for a branch that + // cannot happen, which is worse code. Dropping noUncheckedIndexedAccess to satisfy this rule + // instead would trade a real check for a cosmetic one. + "noNonNullAssertion": "off" + } + } + } +} diff --git a/packages/krm-stream/package-lock.json b/packages/krm-stream/package-lock.json index 3042379..37de544 100644 --- a/packages/krm-stream/package-lock.json +++ b/packages/krm-stream/package-lock.json @@ -9,12 +9,187 @@ "version": "0.0.0", "license": "Apache-2.0", "devDependencies": { + "@biomejs/biome": "^2.5.3", + "@types/node": "^26.1.1", "typescript": "^5.9.0" }, "engines": { "node": ">=22" } }, + "node_modules/@biomejs/biome": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.5.3.tgz", + "integrity": "sha512-MrJswFdei9EfDwwUy2tQrPDpK0AO+RmMFvBoaaJ6ayBc3sUbHdCE+XG5N8vp+5So41ZupZJQm0roHFFhMGVD7A==", + "dev": true, + "license": "MIT OR Apache-2.0", + "bin": { + "biome": "bin/biome" + }, + "engines": { + "node": ">=14.21.3" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/biome" + }, + "optionalDependencies": { + "@biomejs/cli-darwin-arm64": "2.5.3", + "@biomejs/cli-darwin-x64": "2.5.3", + "@biomejs/cli-linux-arm64": "2.5.3", + "@biomejs/cli-linux-arm64-musl": "2.5.3", + "@biomejs/cli-linux-x64": "2.5.3", + "@biomejs/cli-linux-x64-musl": "2.5.3", + "@biomejs/cli-win32-arm64": "2.5.3", + "@biomejs/cli-win32-x64": "2.5.3" + } + }, + "node_modules/@biomejs/cli-darwin-arm64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-arm64/-/cli-darwin-arm64-2.5.3.tgz", + "integrity": "sha512-QhYP9muVQ0nUO5zztFuPbEwi4+94sJWVjaZds9aMi1l/KNZBiUjdiSUrGHsTaMGDXrYl+r4AS2sUKfgH3w+V3g==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-darwin-x64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-darwin-x64/-/cli-darwin-x64-2.5.3.tgz", + "integrity": "sha512-NC1Ss13UaW7QZX+y8j44bF7AP0jSJdBl6iRhe0MAkvaSqZy+mWg3GaXsrb+eSoHoGDBtaXWEbMVV0iVN2cZ7cQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64/-/cli-linux-arm64-2.5.3.tgz", + "integrity": "sha512-ksx1KWeyYW18ILL04msF/J4ZBtBDN33znYK8Z/aNv/vlBVxL9/g3mGP+omgHJKy4+KWbK87vcmmpmurfNjSgiA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-arm64-musl": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-arm64-musl/-/cli-linux-arm64-musl-2.5.3.tgz", + "integrity": "sha512-fccix0w6xp6csCXgxeC0dU/3ecgRQal0y+cv2SP9ajNlhe7Yrk2Ug7UDe2j9AT9ZDYitkXpvUKgZjjuoYeP4Vg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64/-/cli-linux-x64-2.5.3.tgz", + "integrity": "sha512-yMkJtilsgvILDcVkh187aVLTb64xYsrxYajx5kym+r1ULkO5HUOfu9AYKLGQbOVLwJtT2utNw7hhFNg+17mUYA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-linux-x64-musl": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-linux-x64-musl/-/cli-linux-x64-musl-2.5.3.tgz", + "integrity": "sha512-O/yU9YKRUiHhmcjF2f38PSjseVk3G4VLWYc0G2HWpzdBVREV6G8IGWIVEFf7MFPfWIzNUIvPsEjeAZQIOgnLcQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-arm64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-arm64/-/cli-win32-arm64-2.5.3.tgz", + "integrity": "sha512-cX5z+GYwRcqEok0AH3KSfQGgqYd0Nomfp6Fbe1uiTtELE38hdH2k842wQ9wLNaF/JJ7r4rjJQ4VR+ce+fRmQbw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@biomejs/cli-win32-x64": { + "version": "2.5.3", + "resolved": "https://registry.npmjs.org/@biomejs/cli-win32-x64/-/cli-win32-x64-2.5.3.tgz", + "integrity": "sha512-ExSaJWi4/u6+GXCszlSKpWSjKNbDseAYqqkCznsCsZ/4uidZ/BEqsCc5/3ctlq6dfIubdIIRSVLC/PG9xPl70Q==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT OR Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=14.21.3" + } + }, + "node_modules/@types/node": { + "version": "26.1.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.1.1.tgz", + "integrity": "sha512-nxAkRSVkN1Y0JC1W8ky/fTfkGsMmcrRsbx+3XoZE+rMOX71kLYTV7fLXpqud1GpbpP5TuffXFqfX7fH2GgZREw==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~8.3.0" + } + }, "node_modules/typescript": { "version": "5.9.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", @@ -28,6 +203,13 @@ "engines": { "node": ">=14.17" } + }, + "node_modules/undici-types": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", + "integrity": "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==", + "dev": true, + "license": "MIT" } } } diff --git a/packages/krm-stream/package.json b/packages/krm-stream/package.json index f3e59ac..bec7737 100644 --- a/packages/krm-stream/package.json +++ b/packages/krm-stream/package.json @@ -26,17 +26,24 @@ "default": "./dist/index.js" } }, - "files": ["dist", "README.md"], + "files": [ + "dist", + "README.md" + ], "engines": { "node": ">=22" }, "scripts": { "build": "tsc", "test": "node --test", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.test.json", + "lint": "biome check", + "format": "biome check --write" }, - "//": "typescript is the ONLY devDependency, and there are no dependencies at all. The engine must emit plain, dependency-free ESM: gitops-api has no bundler and is not getting one. Node 22 runs the .ts tests directly (type stripping), so the test suite needs nothing either.", + "//": "ZERO dependencies, and that one is absolute: the engine emits plain ESM a browser imports with no bundler, and gitops-api has no bundler and is not getting one. devDependencies ship NOTHING and are kept to the three that pay for themselves: typescript (the compiler), @types/node (without it `tsc` cannot see test/ at all, because the tests import node:test — and `node --test` STRIPS types rather than checking them, so the suite that IS the contract check would be the one unverified file in the repo), and biome (lint + format in one binary, the TypeScript half of what gofmt/go vet/golangci-lint do for the gateway).", "devDependencies": { + "@biomejs/biome": "^2.5.3", + "@types/node": "^26.1.1", "typescript": "^5.9.0" } } diff --git a/packages/krm-stream/src/deep.ts b/packages/krm-stream/src/deep.ts index faa829e..15e2281 100644 --- a/packages/krm-stream/src/deep.ts +++ b/packages/krm-stream/src/deep.ts @@ -25,7 +25,7 @@ export function deepEqual(a: unknown, b: unknown): boolean { const ka = Object.keys(a); const kb = Object.keys(b); if (ka.length !== kb.length) return false; - return ka.every((k) => Object.prototype.hasOwnProperty.call(b, k) && deepEqual(a[k], b[k])); + return ka.every((k) => Object.hasOwn(b, k) && deepEqual(a[k], b[k])); } return false; diff --git a/packages/krm-stream/src/index.ts b/packages/krm-stream/src/index.ts index c7a040e..26fec60 100644 --- a/packages/krm-stream/src/index.ts +++ b/packages/krm-stream/src/index.ts @@ -13,25 +13,24 @@ // No runtime dependencies, and none of this knows anything about GitOps, Flux, Dex, kcp or // ConfigButler. It knows KRM. -export { LiveResourceStore } from "./store.ts"; -export type { ApplyResult, ApplyOptions } from "./store.ts"; - -export { defaultPolicy, readOnlyPolicy, regionPolicy, DEFAULT_EDITABLE_REGIONS } from "./policy.ts"; - // Useful to a host that renders paths, and to anyone writing a policy: identity is a segment ARRAY. -export { deepEqual, clone } from "./deep.ts"; -export { get, has, pathKey, parsePointer, isPrefix } from "./path.ts"; +export { clone, deepEqual } from "./deep.ts"; +export { get, has, isPrefix, parsePointer, pathKey } from "./path.ts"; + +export { DEFAULT_EDITABLE_REGIONS, defaultPolicy, readOnlyPolicy, regionPolicy } from "./policy.ts"; +export type { ApplyOptions, ApplyResult } from "./store.ts"; +export { LiveResourceStore } from "./store.ts"; export type { + Change, + Conflict, + EditabilityPolicy, + ErrorCode, + EventType, + Identity, KRMObject, Path, - EventType, - ErrorCode, Projection, Scope, - Identity, StreamEvent, - Change, - Conflict, - EditabilityPolicy, } from "./types.ts"; diff --git a/packages/krm-stream/src/path.ts b/packages/krm-stream/src/path.ts index 1a8dc5c..497952c 100644 --- a/packages/krm-stream/src/path.ts +++ b/packages/krm-stream/src/path.ts @@ -43,7 +43,7 @@ export function has(value: unknown, path: Path): boolean { if (Array.isArray(cur)) { if (Number(seg) >= cur.length) return false; } else if (isPlainObject(cur)) { - if (!Object.prototype.hasOwnProperty.call(cur, String(seg))) return false; + if (!Object.hasOwn(cur, String(seg))) return false; } else { return false; } diff --git a/packages/krm-stream/src/store.ts b/packages/krm-stream/src/store.ts index 68229db..4ad0261 100644 --- a/packages/krm-stream/src/store.ts +++ b/packages/krm-stream/src/store.ts @@ -14,8 +14,8 @@ // flagged forever, or unflags itself only on the next click. import { clone, deepEqual, isPlainObject } from "./deep.ts"; +import { type MergeState, type Regions, reconcile } from "./merge.ts"; import { get, has, isPrefix, parsePointer, pathKey, removeAt, setAt } from "./path.ts"; -import { reconcile, type MergeState, type Regions } from "./merge.ts"; import { defaultPolicy } from "./policy.ts"; import type { Change, Conflict, EditabilityPolicy, KRMObject, Path } from "./types.ts"; @@ -215,7 +215,7 @@ export class LiveResourceStore { /** Read-only convenience for the watch UI. A kind with no status simply has none. */ status(id: string): unknown { - return clone(this.#must(id).server["status"]); + return clone(this.#must(id).server.status); } /** Every pending edit, derived fresh by comparing the draft to the server object. Arrays appear @@ -357,7 +357,7 @@ function sameShape(a: unknown, b: unknown): boolean { const ka = Object.keys(a); const kb = Object.keys(b); if (ka.length !== kb.length) return false; - return ka.every((k) => Object.prototype.hasOwnProperty.call(b, k) && sameShape(a[k], b[k])); + return ka.every((k) => Object.hasOwn(b, k) && sameShape(a[k], b[k])); } if (Array.isArray(a) && Array.isArray(b)) { return a.length === b.length && a.every((x, i) => sameShape(x, b[i])); diff --git a/packages/krm-stream/test/conformance.test.ts b/packages/krm-stream/test/conformance.test.ts index b306134..ff1d839 100644 --- a/packages/krm-stream/test/conformance.test.ts +++ b/packages/krm-stream/test/conformance.test.ts @@ -9,9 +9,9 @@ // fixture that edits a uid the stream never delivered — a test that would pass against a store that // does nothing at all. -import { test } from "node:test"; import assert from "node:assert/strict"; -import { allBodies, allFixtures, clientFixtures, resolve, type Fixture } from "./conformance.ts"; +import { test } from "node:test"; +import { allBodies, allFixtures, clientFixtures, type Fixture, resolve } from "./conformance.ts"; test("the corpus loads", () => { assert.ok(allFixtures().length > 0, "no fixtures — run `task fixtures`"); diff --git a/packages/krm-stream/test/conformance.ts b/packages/krm-stream/test/conformance.ts index 40fface..be6e9b4 100644 --- a/packages/krm-stream/test/conformance.ts +++ b/packages/krm-stream/test/conformance.ts @@ -5,7 +5,7 @@ // "contract" is just two implementations agreeing to disagree. import { readFileSync } from "node:fs"; -import type { EventType, ErrorCode, Identity, KRMObject, Path, Projection, Scope, StreamEvent } from "../src/types.ts"; +import type { ErrorCode, EventType, Identity, KRMObject, Path, Projection, Scope, StreamEvent } from "../src/types.ts"; // Resolved relative to THIS file, not to the working directory — so `node --test` works from // anywhere, and so does an editor's inline test runner. @@ -61,12 +61,8 @@ export interface Fixture { gatewayRejects?: unknown[]; } -const bodies: Record = JSON.parse( - readFileSync(new URL("gen/bodies.json", CONFORMANCE), "utf8"), -); -const fixtures: Fixture[] = JSON.parse( - readFileSync(new URL("gen/fixtures.json", CONFORMANCE), "utf8"), -); +const bodies: Record = JSON.parse(readFileSync(new URL("gen/bodies.json", CONFORMANCE), "utf8")); +const fixtures: Fixture[] = JSON.parse(readFileSync(new URL("gen/fixtures.json", CONFORMANCE), "utf8")); /** A KRM object by its bodies/ reference. Missing is a hard error: a fixture naming an object that * does not exist is a broken contract, not a skippable test. */ diff --git a/packages/krm-stream/test/invariants.test.ts b/packages/krm-stream/test/invariants.test.ts index bc527e0..b236ea0 100644 --- a/packages/krm-stream/test/invariants.test.ts +++ b/packages/krm-stream/test/invariants.test.ts @@ -9,11 +9,11 @@ // stays green even against a store that would happily let them. "Structurally incapable of becoming // an edit" is a claim about what happens when someone tries, and something has to try. -import { test } from "node:test"; import assert from "node:assert/strict"; -import { body } from "./conformance.ts"; +import { test } from "node:test"; import { LiveResourceStore, readOnlyPolicy } from "../src/index.ts"; import type { Path } from "../src/types.ts"; +import { body } from "./conformance.ts"; const CM = "cm-app.v1"; // ConfigMap: data.log-level=info, data.replicas="3", one dotted label const DEPLOY = "deploy-web.v1"; // Deployment: nested spec, a container array, a live status @@ -86,7 +86,7 @@ test("I-IDEMPOTENT — redelivering the same object is a no-op (a reconnect repl test("I-BASESHIFT — the second event reconciles against the first, not against the snapshot", () => { const [store, id] = seeded(CM); store.applyServerEvent(body("cm-app.v3")); // log-level: info -> warn. Nobody is editing it. - assert.equal((store.draft(id)["data"] as Record)["log-level"], "warn"); + assert.equal((store.draft(id).data as Record)["log-level"], "warn"); // Now the user edits it, and the server sends v3 AGAIN. Against the ORIGINAL snapshot as base this // would look like "the server just changed log-level" and false-conflict; against v3 it is a no-op. @@ -121,7 +121,7 @@ test("removeKey and renameKey — a deletion is a null in the patch, and a renam const [store2, id2] = seeded(CM); store2.renameKey(id2, ["data"], "log-level", "logLevel"); assert.deepEqual(store2.patch(id2), { data: { "log-level": null, logLevel: "info" } }); - assert.deepEqual(Object.keys(store2.draft(id2)["data"] as object), ["logLevel", "replicas"], "order is preserved"); + assert.deepEqual(Object.keys(store2.draft(id2).data as object), ["logLevel", "replicas"], "order is preserved"); }); test("I-PATCH-ROUNDTRIP — applying patch(id) to the server object yields the draft", () => { @@ -171,7 +171,7 @@ test("I-REDACT — a masked value is read-only, is never dirty, and never reache // The mask is still SHOWN — that is the point of keys-only disclosure: you can see that `token` // exists without ever learning, or overwriting, what it is. - assert.equal((store.draft(id)["data"] as Record)["token"], "**REDACTED**"); + assert.equal((store.draft(id).data as Record).token, "**REDACTED**"); store.setValue(id, ["metadata", "labels", "app.kubernetes.io/name"], "checkout"); assert.deepEqual(store.patch(id), { metadata: { labels: { "app.kubernetes.io/name": "checkout" } } }); }); diff --git a/packages/krm-stream/test/store.test.ts b/packages/krm-stream/test/store.test.ts index d58cf86..f9092bc 100644 --- a/packages/krm-stream/test/store.test.ts +++ b/packages/krm-stream/test/store.test.ts @@ -20,11 +20,11 @@ // does not flash a read-only field that moved. See docs §3 ("read-only // is not ignored"). The fixtures list the paths a UI must highlight. -import { test } from "node:test"; import assert from "node:assert/strict"; -import { clientFixtures, resolve, type FixtureEdit, type FixtureExpect } from "./conformance.ts"; +import { test } from "node:test"; import { LiveResourceStore } from "../src/index.ts"; import type { Path, StreamEvent } from "../src/types.ts"; +import { clientFixtures, type FixtureEdit, type FixtureExpect, resolve } from "./conformance.ts"; for (const f of clientFixtures()) { test(`${f.id}: ${f.title}`, () => { diff --git a/packages/krm-stream/tsconfig.test.json b/packages/krm-stream/tsconfig.test.json new file mode 100644 index 0000000..57469f8 --- /dev/null +++ b/packages/krm-stream/tsconfig.test.json @@ -0,0 +1,16 @@ +{ + // The tests, typechecked. `tsconfig.json` deliberately includes only `src/` — it is the BUILD, and + // its rootDir is what ships — so without this file `tsc` never sees test/ at all, and `node --test` + // STRIPS types rather than checking them. The suite that is supposed to be the contract check would + // be the one piece of unverified TypeScript in the repo. + // + // noEmit: nothing here is published. It exists to be checked. + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "rootDir": ".", + // node:test / node:assert — the only reason @types/node is a devDependency. It ships nothing. + "types": ["node"] + }, + "include": ["src/**/*.ts", "test/**/*.ts"] +} From 96ae6be936d9b7372fac384a6f684e29c7b6b340 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 20:29:59 +0000 Subject: [PATCH 03/43] feat(gateway): the stream loop, green against the gateway half of the corpus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rung 1 of the e2e ladder: each fixture's `watch:` ops are played through a fake Kubernetes watch and what the gateway puts on the wire is compared to the fixture's `events:` — byte for byte, because `events:` is exactly what the TypeScript store on the other side is fed. All 11 gateway fixtures pass. No cluster: every rule in the corpus is about framing and projection, and a fake watch drives those deterministically in microseconds. The seams (seams.go) are CONTRIBUTING's one rule expressed as Go. The gateway does not know what a tenant is or where a kubeconfig lives; it asks the host, through Authorizer and ClientFor(target, principal), and the host answers. The upstream is behind an interface for the same reason — and that buys the test suite as a side effect: the corpus drives a ScriptedBackend, a real cluster will drive a client-go one, and the loop cannot tell them apart. Watcher is PULL-based, deliberately. "The gateway was handed the last event" and "the gateway finished with it" are the same moment under a pull, and different moments under a channel — so the conformance replay is deterministic with no sleeps. A channel-based client-go watch adapts in ten lines; the reverse does not adapt at all. Two spec rules turned out to be UNTESTABLE by the corpus, and I proved it by mutation: emitting `synced` on every bookmark, and forwarding a partial object with no uid, both leave all 11 fixtures green. A fixture's `watch:` script has no op for a bare BOOKMARK or a degenerate tombstone, so it cannot say those words. They are MUST NOTs (spec §2, §4.2) and rows 7/8 of the gateway's own matrix, so they are pinned in stream_test.go instead — and the fixture format needs three new ops, which is a change to the shared contract and therefore not mine to make unilaterally. Six mutations, six caught: bookmark-as-synced, partial-object-forwarded, monotonicity-removed, tombstone-uid-guessed, secret-policy-off, and empty-named-scope-emits-nothing. Co-Authored-By: Claude Opus 4.8 (1M context) --- gateway/project.go | 152 +++++++++++++ gateway/scripted.go | 151 +++++++++++++ gateway/seams.go | 171 ++++++++++++++ gateway/stream.go | 209 ++++++++++++++++++ gateway/stream_conformance_test.go | 173 +++++++++++++++ gateway/stream_test.go | 343 +++++++++++++++++++++++++++++ 6 files changed, 1199 insertions(+) create mode 100644 gateway/project.go create mode 100644 gateway/scripted.go create mode 100644 gateway/seams.go create mode 100644 gateway/stream.go create mode 100644 gateway/stream_conformance_test.go create mode 100644 gateway/stream_test.go diff --git a/gateway/project.go b/gateway/project.go new file mode 100644 index 0000000..a91f238 --- /dev/null +++ b/gateway/project.go @@ -0,0 +1,152 @@ +package gateway + +import ( + "sort" + "strings" +) + +// Projection: what the stream removes, and what it masks. Both are part of the WIRE (spec §3), not +// an implementation detail, because a consumer must be able to tell these four apart: +// +// 1. a field that does not exist upstream +// 2. a field the gateway REMOVED (never shown) +// 3. a field that is present but REDACTED (shown as a placeholder) +// 4. a field whose real value merely LOOKS like a placeholder +// +// No amount of squinting at a value distinguishes 3 from 4 — which is why redactedPaths is on the +// wire, per object, mandatory, and why it is authoritative over anything the value looks like. + +// RedactedPlaceholder is the mask a redacted value is replaced with. It is never the truth, and — +// this is the part that makes displaying a Secret safe at all — it can never be written back over +// the truth: the write path refuses any patch touching a path in redactedPaths (spec §3, gateway +// §7c). A real value that merely LOOKS like this is not redacted: redactedPaths is authoritative, +// and the shape of a value is never evidence. +const RedactedPlaceholder = "**REDACTED**" + +// lastAppliedAnnotation is machinery a human editor must never see and must never round-trip. +const lastAppliedAnnotation = "kubectl.kubernetes.io/last-applied-configuration" + +// project applies the named projection to an object from the upstream, returning the object that +// goes on the wire and the RFC 6901 pointers whose values were masked in it. +// +// The returned object is always a DEEP COPY. The upstream's object may be shared (an informer's +// cache hands the same pointer to every subscriber, and the conformance corpus hands the same map to +// both sides of the assertion) — mutating it in place would corrupt state we do not own, and in the +// informer case would corrupt it for every other browser watching the same scope. +func project(p Projection, in KRMObject) (KRMObject, []string) { + out := deepCopyObject(in) + paths := []string{} + + // Both projections remove the machinery. There is no unspecified "other server-side + // bookkeeping": if a gateway removes a field, that removal is part of a NAMED projection or it + // does not happen. + if meta, ok := out["metadata"].(map[string]any); ok { + delete(meta, "managedFields") + if ann, ok := meta["annotations"].(map[string]any); ok { + delete(ann, lastAppliedAnnotation) + // An annotations map that is empty ONLY because we emptied it is our artifact, not the + // server's state. Leaving `annotations: {}` behind would tell the consumer the object + // has an empty annotation map, which is a different fact from having none. + if len(ann) == 0 { + delete(meta, "annotations") + } + } + } + + // krm-editor/v1 additionally applies the Secret disclosure policy: keys-only. You may see THAT + // `token` exists — which is what makes the object editable at all, since you can still rename a + // label on it — and you may never see or overwrite what it is. + if p == ProjectionEditor && isSecret(out) { + for _, field := range []string{"data", "stringData"} { + m, ok := out[field].(map[string]any) + if !ok { + continue + } + for k := range m { + m[k] = RedactedPlaceholder + paths = append(paths, "/"+escapePointer(field)+"/"+escapePointer(k)) + } + } + } + + // Sorted so the wire is deterministic: two gateways given the same object emit the same bytes, + // and a golden transcript is a golden transcript. + sort.Strings(paths) + return out, paths +} + +func isSecret(o KRMObject) bool { + kind, _ := o["kind"].(string) + apiVersion, _ := o["apiVersion"].(string) + return kind == "Secret" && apiVersion == "v1" +} + +// escapePointer escapes one segment for RFC 6901: `~` becomes `~0`, `/` becomes `~1`. The order +// matters — escape the tilde FIRST, or the tilde you introduce while escaping a slash gets escaped +// again and `a/b` round-trips as `a~1b`. +func escapePointer(seg string) string { + seg = strings.ReplaceAll(seg, "~", "~0") + return strings.ReplaceAll(seg, "/", "~1") +} + +// identityOf builds the tombstone identity for a deleted object, or nil if the object cannot supply +// a trustworthy uid. +// +// Nil is a real answer, not a failure to try: an informer's deletion tombstone can be degenerate, +// and a gateway that guesses a uid deletes the wrong object out of someone's browser. When this +// returns nil the caller begins a new snapshot cycle instead and lets reset…synced do the pruning +// (spec §4.2). +func identityOf(o KRMObject) *Identity { + meta, ok := o["metadata"].(map[string]any) + if !ok { + return nil + } + uid, _ := meta["uid"].(string) + name, _ := meta["name"].(string) + if uid == "" || name == "" { + return nil + } + ns, _ := meta["namespace"].(string) + apiVersion, _ := o["apiVersion"].(string) + kind, _ := o["kind"].(string) + if apiVersion == "" || kind == "" { + return nil + } + return &Identity{UID: uid, APIVersion: apiVersion, Kind: kind, Namespace: ns, Name: name} +} + +// resourceVersionOf reads metadata.resourceVersion. It is opaque on the wire (spec §6) — a consumer +// must never parse or order by it — but the GATEWAY relies on it being a monotonic integer within +// one target, and that reliance is exactly what lets it promise per-object monotonicity to a +// consumer that cannot do the arithmetic itself. +func resourceVersionOf(o KRMObject) string { + meta, ok := o["metadata"].(map[string]any) + if !ok { + return "" + } + rv, _ := meta["resourceVersion"].(string) + return rv +} + +func deepCopyObject(o KRMObject) KRMObject { + return KRMObject(deepCopyValue(map[string]any(o)).(map[string]any)) +} + +func deepCopyValue(v any) any { + switch t := v.(type) { + case map[string]any: + out := make(map[string]any, len(t)) + for k, x := range t { + out[k] = deepCopyValue(x) + } + return out + case []any: + out := make([]any, len(t)) + for i, x := range t { + out[i] = deepCopyValue(x) + } + return out + default: + return v + } +} diff --git a/gateway/scripted.go b/gateway/scripted.go new file mode 100644 index 0000000..3554880 --- /dev/null +++ b/gateway/scripted.go @@ -0,0 +1,151 @@ +package gateway + +import ( + "context" + "fmt" + "sync" +) + +// ScriptedBackend is a fake Kubernetes watch driven by a fixture's `watch:` ops. It is the gateway's +// half of the conformance corpus, and it is a normal (non-test) file on purpose: the replay server +// serves fixtures over real SSE with exactly this backend, so a browser can be pointed at a scripted +// cluster that behaves identically every time. +// +// It models a MODERN streaming list (gateway spec §3a), because that is what the gateway is written +// against: the objects in scope arrive as synthetic ADDEDs, terminated by a bookmark whose +// InitialEventsEnd is set. That bookmark IS `synced`. A `relist` op ends the watch with a +// continuity-losing error, which is what a 410 Gone looks like from in here — and the gateway must +// then recover on the SAME connection, which is the one thing a fake watch can prove and a real +// cluster can only occasionally be provoked into demonstrating. + +// scriptedSegment is everything one Watch() call delivers: a snapshot, then live events, then +// (unless it is the last) the error that ends it. +type scriptedSegment struct { + events []WatchEvent +} + +// ScriptedBackend replays a watch script. Not safe for concurrent Watch calls, and does not need to +// be: one stream, one upstream. +type ScriptedBackend struct { + segments []scriptedSegment + + mu sync.Mutex + segment int // the next segment Watch() will hand out + exhausted chan struct{} + once sync.Once +} + +// NewScriptedBackend compiles a fixture's watch ops into a fake upstream. +// +// A `disconnect` op is NOT handled here: it ends the SSE CONNECTION, not the upstream watch, so it +// is the caller who splits the script on it and runs the gateway again. Conflating "the browser went +// away" with "the API server lost continuity" is precisely the bug resync-midstream defends against, +// and a fake that could not tell them apart would be unable to catch it. +func NewScriptedBackend(c Corpus, ops []WatchOp) (*ScriptedBackend, error) { + b := &ScriptedBackend{exhausted: make(chan struct{})} + var cur *scriptedSegment + + body := func(ref string) (KRMObject, error) { return c.Body(ref) } + + for _, op := range ops { + switch op.Op { + case "list", "relist": + if op.Op == "relist" { + if cur == nil { + return nil, fmt.Errorf("scripted: `relist` with no watch open") + } + // 410 Gone. The connection is fine; our knowledge of the world is not. + cur.events = append(cur.events, WatchEvent{ + Type: WatchError, + Err: ResyncRequired("the upstream resourceVersion expired (410 Gone)"), + }) + } + b.segments = append(b.segments, scriptedSegment{}) + cur = &b.segments[len(b.segments)-1] + + for _, ref := range op.Bodies { + obj, err := body(ref) + if err != nil { + return nil, err + } + cur.events = append(cur.events, WatchEvent{Type: WatchAdded, Object: obj}) + } + // The bookmark that closes the snapshot. An EMPTY list still gets one — that is the + // named-object-absent case, and emitting nothing at all instead is the mistake that + // leaves a deleted object on screen as a ghost forever. + cur.events = append(cur.events, WatchEvent{Type: WatchBookmark, InitialEventsEnd: true}) + + case "added", "modified", "deleted": + if cur == nil { + return nil, fmt.Errorf("scripted: %q before any list", op.Op) + } + obj, err := body(op.Body) + if err != nil { + return nil, err + } + cur.events = append(cur.events, WatchEvent{Type: WatchEventType(op.Op), Object: obj}) + + case "disconnect": + return nil, fmt.Errorf("scripted: `disconnect` is the caller's to handle — split the script on it") + + default: + return nil, fmt.Errorf("scripted: unknown watch op %q", op.Op) + } + } + if len(b.segments) == 0 { + return nil, fmt.Errorf("scripted: a watch script must begin with a `list`") + } + return b, nil +} + +// Exhausted is closed once the gateway has come back for an event after being handed the last one in +// the script — which is proof it finished processing it. +// +// That is the whole reason Watcher is pull-based. With a channel, "the last event was delivered" and +// "the last event was handled" are different moments and a test has to sleep between them; here they +// are the same moment, and the suite is deterministic for free. +func (b *ScriptedBackend) Exhausted() <-chan struct{} { return b.exhausted } + +// Watch hands out the next segment of the script. +func (b *ScriptedBackend) Watch(_ context.Context, _ Scope) (Watcher, error) { + b.mu.Lock() + defer b.mu.Unlock() + if b.segment >= len(b.segments) { + return nil, fmt.Errorf("scripted: the gateway reopened the watch %d times; the script has %d segments", b.segment+1, len(b.segments)) + } + seg := b.segments[b.segment] + b.segment++ + last := b.segment == len(b.segments) + return &scriptedWatcher{backend: b, events: seg.events, last: last}, nil +} + +type scriptedWatcher struct { + backend *ScriptedBackend + events []WatchEvent + last bool + i int +} + +func (w *scriptedWatcher) Next(ctx context.Context) (WatchEvent, error) { + if w.i < len(w.events) { + ev := w.events[w.i] + w.i++ + return ev, nil + } + if !w.last { + // Only the final segment runs dry: every other one ends with the error that made the gateway + // reopen. Reaching here otherwise means the script and the gateway disagree about how many + // watches this scenario opens, which is worth saying out loud rather than hanging. + return WatchEvent{}, fmt.Errorf("scripted: segment ended without a continuity error") + } + w.backend.once.Do(func() { close(w.backend.exhausted) }) + + // The script is over, and a real watch would simply be quiet here. Blocking (rather than + // closing) is the honest simulation: a closed watch means "reopen with a fresh snapshot", and + // synthesizing that at the end of every fixture would append a phantom cycle to the expected + // events of every single scenario. + <-ctx.Done() + return WatchEvent{}, ctx.Err() +} + +func (w *scriptedWatcher) Stop() {} diff --git a/gateway/seams.go b/gateway/seams.go new file mode 100644 index 0000000..fb2ccfd --- /dev/null +++ b/gateway/seams.go @@ -0,0 +1,171 @@ +package gateway + +import ( + "context" + "fmt" +) + +// The seams. This file is the whole of CONTRIBUTING's one rule, expressed as Go: +// +// krm-stream ──depends on──▶ nothing of the host's. Ever. +// +// The gateway does not know what a tenant is, which namespace a caller "owns", where a kubeconfig +// lives, or how a session cookie becomes a person. Every time it seems to need one of those, that is +// a missing INTERFACE, not a missing import — so it asks the host, here, and the host answers. +// +// The upstream is behind an interface for the same reason, and it buys the test suite as a +// side-effect: the conformance corpus drives a scripted Backend, the k3d suite drives a real +// client-go one, and the stream loop cannot tell the difference. That is not a testing trick — it is +// what "the protocol is backend-agnostic" (spec §5, §6) has to mean if it means anything. + +// Principal is whoever the host says is calling. The library never inspects it, never persists it, +// and never logs it — it carries it back to the host on ClientFor, so the host can reach the API +// server AS that caller (gateway spec §6). `any` is not laziness here: the moment this library has +// an opinion about what an identity looks like, it has an opinion about someone's auth system. +type Principal any + +// Authorizer decides whether a principal may open a scope, BEFORE any watch is opened. Denial is the +// default: a gateway that opens the watch first and filters later has already leaked the object's +// existence, and possibly its contents, to someone who may not see it. +// +// Return a *StreamError to deny with a specific code (FORBIDDEN, SCOPE_INVALID); any other error is +// treated as INTERNAL. +type Authorizer interface { + Authorize(ctx context.Context, principal Principal, scope Scope) error +} + +// AllowAll authorizes everything. It exists for tests and for a single-tenant operator who has +// already decided that anyone reaching this handler may see this scope — and it is deliberately +// tedious to opt into, because the type name is what shows up in the reviewer's diff. +type AllowAll struct{} + +// Authorize permits every scope. +func (AllowAll) Authorize(context.Context, Principal, Scope) error { return nil } + +// AuthorizerFunc adapts a function to Authorizer. +type AuthorizerFunc func(ctx context.Context, principal Principal, scope Scope) error + +// Authorize calls f. +func (f AuthorizerFunc) Authorize(ctx context.Context, p Principal, s Scope) error { + return f(ctx, p, s) +} + +// ClientFor hands the gateway a Backend for one allowlisted target, reaching the API server as the +// given principal. The host resolves BOTH halves — which cluster/workspace `target` names, and how +// this caller's identity is carried there (impersonation, an exchanged token, whatever it does). +// +// The browser never supplies an API-server URL, so this is also the choke point that makes that +// impossible rather than merely forbidden: there is nowhere to put one. +type ClientFor func(target string, principal Principal) (Backend, error) + +// Backend is the upstream: one Kubernetes API server (or anything that behaves like one). +// +// Watch opens a snapshot-then-live stream for a scope. The gateway expects it to behave like a +// modern streaming list (gateway spec §3a): the objects currently in scope arrive as WatchAdded, +// terminated by a WatchBookmark whose InitialEventsEnd is true, and everything after that bookmark +// is live. A list-then-watch backend synthesizes exactly the same shape (§3b) — which is the point +// of naming the boundary rather than the mechanism. +type Backend interface { + Watch(ctx context.Context, scope Scope) (Watcher, error) +} + +// Watcher is a pull-based upstream watch. +// +// Pull, not a channel, and this is a considered choice: Next returning is the gateway's proof that +// it finished with the previous event, which makes both the conformance replay and the coalescing +// logic deterministic instead of racy. A channel-based client-go watch adapts to this in ten lines +// (see KubeBackend); the reverse — recovering a synchronisation point from a channel — is not +// possible at all. +type Watcher interface { + // Next blocks until the next upstream event, ctx is done, or the watch ends. + // + // It returns ErrWatchClosed when the upstream watch ended cleanly (an API server times a watch + // out routinely). That is NOT an error: it means "reopen", and the gateway does — with a fresh + // snapshot cycle, because it can no longer promise it saw everything in between. + Next(ctx context.Context) (WatchEvent, error) + Stop() +} + +// WatchEventType is the upstream vocabulary — Kubernetes's, not the protocol's. The gateway's whole +// job is translating between the two, and the translation is lossy on purpose: BOOKMARK and ERROR +// never reach a browser (spec §2). +type WatchEventType string + +// The upstream event types. +const ( + WatchAdded WatchEventType = "added" + WatchModified WatchEventType = "modified" + WatchDeleted WatchEventType = "deleted" + // WatchBookmark carries no object the consumer cares about. InitialEventsEnd marks the snapshot + // boundary; every other bookmark is absorbed for its resourceVersion and never forwarded. + WatchBookmark WatchEventType = "bookmark" + // WatchError is upstream trouble. Continuity-losing errors (410 Gone, an expired + // resourceVersion, a cache reset) mean "start a new snapshot cycle", not "give up". + WatchError WatchEventType = "error" +) + +// WatchEvent is one upstream event. +type WatchEvent struct { + Type WatchEventType + // Object is the complete object for added/modified, and the last-known object for deleted. + // + // For a deleted event it MAY be a degenerate tombstone with no usable uid — and if it is, the + // gateway must not guess: it begins a new snapshot cycle instead of emitting an ambiguous + // `deleted` (spec §4.2). Guessing here deletes the wrong object in someone's browser. + Object KRMObject + // InitialEventsEnd is set on the bookmark that closes the snapshot. It IS the `synced` boundary, + // handed to us by the API server rather than inferred by counting objects. + InitialEventsEnd bool + // Err is set on WatchError. + Err error +} + +// ErrWatchClosed is what a Watcher returns when the upstream watch ended cleanly. It means "reopen", +// and the reopen begins a new snapshot cycle: a gateway that resumed silently would be promising a +// gap-free handoff it cannot deliver. +var ErrWatchClosed = fmt.Errorf("krm-stream: upstream watch closed") + +// StreamError is a protocol-level error the gateway emits to the consumer, and the type a host's +// Authorizer or ClientFor returns to choose the code the browser sees. +type StreamError struct { + Code ErrorCode + Message string + Terminal bool + RetryAfterMs *int +} + +func (e *StreamError) Error() string { return fmt.Sprintf("%s: %s", e.Code, e.Message) } + +// Event renders the error as the wire event. +func (e *StreamError) Event() Event { + return Event{ + Type: EventError, + Code: e.Code, + Message: e.Message, + Terminal: e.Terminal, + RetryAfterMs: e.RetryAfterMs, + } +} + +// Forbidden denies a scope. Terminal: EventSource reconnects automatically otherwise, and would +// hammer a scope it will never be allowed to see, forever. +func Forbidden(msg string) *StreamError { + return &StreamError{Code: CodeForbidden, Message: msg, Terminal: true} +} + +// ScopeInvalid rejects a scope that is not allowlisted or not resolvable. +func ScopeInvalid(msg string) *StreamError { + return &StreamError{Code: CodeScopeInvalid, Message: msg, Terminal: true} +} + +// ResyncRequired announces a loss of upstream continuity. NOT terminal: a fresh snapshot cycle +// follows on the same connection, and the consumer converges (spec §5). +func ResyncRequired(msg string) *StreamError { + return &StreamError{Code: CodeResyncRequired, Message: msg, Terminal: false} +} + +// Sink is where the gateway puts events. The SSE handler is one; the conformance suite's recorder is +// another. The stream loop knows nothing about HTTP. +type Sink interface { + Emit(ctx context.Context, ev Event) error +} diff --git a/gateway/stream.go b/gateway/stream.go new file mode 100644 index 0000000..fdb3332 --- /dev/null +++ b/gateway/stream.go @@ -0,0 +1,209 @@ +package gateway + +import ( + "context" + "errors" + "strconv" +) + +// The stream loop: a Kubernetes watch in, the protocol out. +// +// Everything hard about this is in the translation, and the translation is deliberately lossy. +// Kubernetes speaks BOOKMARK, 410 Gone, relists and degenerate tombstones; a browser must never hear +// any of those words. What it hears instead is a small, stable vocabulary with exactly one framing — +// reset … added* … synced, then live deltas — for named and collection scopes alike. +// +// The four rules that a "just relay the watch" gateway gets wrong, and that everything below exists +// to enforce: +// +// 1. A snapshot cycle is not tied to a connection. Lose upstream continuity and you begin a NEW +// cycle on the same healthy SSE connection (§5) — otherwise the ghost you leave is invisible +// until someone notices a deleted object still on screen. +// 2. A partial object is never forwarded. It would BLANK the consumer's state for that uid. +// 3. An ambiguous tombstone is never emitted. A guessed uid deletes the wrong object. +// 4. Never emit, within a cycle, a state older than one already emitted for that uid (§6). + +// Gateway turns a Kubernetes watch into a conforming resource stream. It holds no cluster +// connection of its own: the host supplies both the authorization decision and the client, through +// the seams in seams.go. +type Gateway struct { + // Auth decides whether a principal may open a scope, before any watch opens. Required. + Auth Authorizer + // Clients resolves (target, principal) to an upstream. Required. + Clients ClientFor + // Projection is what this stream removes and masks. Defaults to krm-editor/v1 — the safe one: + // a gateway that defaults to raw and streams Secret values because someone forgot a config line + // has a vulnerability, not a bug. + Projection Projection +} + +// Stream runs one consumer's stream until the context is done or a terminal error is emitted. +// +// It returns after emitting a terminal error; the caller (the SSE handler) must then CLOSE the +// connection. A browser's EventSource reconnects automatically otherwise, and would hammer a +// forbidden scope forever. +func (g *Gateway) Stream(ctx context.Context, principal Principal, scope Scope, sink Sink) error { + projection := g.Projection + if projection == "" { + projection = ProjectionEditor + } + + // Denial comes FIRST, before any watch is opened. A gateway that opens the watch and filters + // afterwards has already leaked the object's existence — and, if it logs, its contents. + if err := g.Auth.Authorize(ctx, principal, scope); err != nil { + return emitTerminal(ctx, sink, err) + } + + backend, err := g.Clients(scope.Target, principal) + if err != nil { + return emitTerminal(ctx, sink, err) + } + + for cycles := 0; ; cycles++ { + if cycles > 0 { + // A new cycle on a live connection. Say so first: the consumer is about to be told a + // whole new snapshot, and RESYNC_REQUIRED is what distinguishes "we lost continuity and + // are recovering" from "your objects all changed at once". Non-terminal — recovery is + // the normal case, not the failure case. + if err := sink.Emit(ctx, ResyncRequired("upstream continuity lost; a new snapshot cycle follows").Event()); err != nil { + return err + } + } + + err := g.cycle(ctx, backend, scope, projection, sink) + switch { + case err == nil: + // A cycle only ends by error or cancellation; nil would be a bug in the loop below. + return nil + case ctx.Err() != nil: + return ctx.Err() + case errors.Is(err, ErrWatchClosed): + // A routine watch timeout. Reopen — with a full snapshot, because between the close and + // the reopen we can promise nothing, and the protocol's whole value is that what you + // end up holding is right. + continue + } + + var se *StreamError + if errors.As(err, &se) && !se.Terminal { + continue // a recoverable upstream error: announce, resnapshot, carry on + } + return emitTerminal(ctx, sink, err) + } +} + +// cycle runs exactly one snapshot cycle and the live tail that follows it, returning when upstream +// continuity is lost (or the context is done). +func (g *Gateway) cycle(ctx context.Context, backend Backend, scope Scope, projection Projection, sink Sink) error { + w, err := backend.Watch(ctx, scope) + if err != nil { + return err + } + defer w.Stop() + + if err := sink.Emit(ctx, Event{Type: EventReset, Target: scope.Target, Scope: &scope, Projection: projection}); err != nil { + return err + } + + // Per-uid high-water mark, and it is per CYCLE, not per stream: the protocol's monotonicity + // promise is scoped to a cycle (§6), and a new cycle legitimately re-delivers a state the + // consumer already saw (that is what makes reconnect replays idempotent). Keeping this map + // across cycles would silently swallow the snapshot of an object whose resourceVersion had not + // moved — the object would simply never arrive, and the consumer would prune it. + emitted := map[string]int64{} + + for { + ev, err := w.Next(ctx) + if err != nil { + return err + } + + switch ev.Type { + case WatchBookmark: + // THE snapshot boundary, handed to us by the API server rather than inferred by counting + // objects. Every other bookmark is absorbed for its resourceVersion and never forwarded: + // a consumer that saw one would have to know what it was (spec §2). + if ev.InitialEventsEnd { + if err := sink.Emit(ctx, Event{Type: EventSynced}); err != nil { + return err + } + } + + case WatchAdded, WatchModified: + obj, redacted := project(projection, ev.Object) + uid := obj.UID() + if uid == "" { + // A metadata-only or otherwise partial object. Forwarding it as added/modified would + // REPLACE the consumer's object with a fragment — the consumer's whole model is + // "replace, never merge" — so the object would blank out on screen. Resnapshot + // instead: it is the one recovery that is always correct. + return ResyncRequired("upstream delivered a partial object with no uid") + } + if isStale(emitted, uid, obj) { + continue + } + out := Event{Type: EventAdded, Object: obj, RedactedPaths: redacted} + if ev.Type == WatchModified { + out.Type = EventModified + } + if err := sink.Emit(ctx, out); err != nil { + return err + } + + case WatchDeleted: + id := identityOf(ev.Object) + if id == nil { + // A degenerate tombstone. We will NOT guess a uid — that deletes the wrong object + // out of someone's browser. A new snapshot cycle removes it correctly instead, at + // the cost of one relist (spec §4.2). + return ResyncRequired("deletion tombstone carried no trustworthy uid") + } + // No `object` on the tombstone: the protocol makes it optional ("when the gateway has + // it"), and the final state of an object nobody can see any more is not worth the bytes + // or the second code path. The identity is what a consumer acts on. + if err := sink.Emit(ctx, Event{Type: EventDeleted, Identity: id}); err != nil { + return err + } + delete(emitted, id.UID) + + case WatchError: + if ev.Err != nil { + return ev.Err + } + return ResyncRequired("upstream error") + } + } +} + +// isStale enforces per-object monotonicity (§6): within one cycle, never hand a consumer a state +// older than one it already has. This is what makes coalescing safe, and it is the guarantee a naive +// relay cannot give — an informer that relists mid-stream will happily replay an older version of an +// object it has already delivered. +// +// A resourceVersion that does not parse as an integer is not compared. The gateway may rely on +// monotonicity within one target (§3d) but must not CRASH on a backend that does not provide it: the +// failure mode of a wrong guess here is a dropped update, which is worse than a duplicate one. +func isStale(emitted map[string]int64, uid string, obj KRMObject) bool { + rv, err := strconv.ParseInt(resourceVersionOf(obj), 10, 64) + if err != nil { + return false + } + if last, ok := emitted[uid]; ok && rv < last { + return true + } + emitted[uid] = rv + return false +} + +// emitTerminal puts the error on the wire and returns it. A terminal error MUST be the last event on +// the connection, and the caller closes it. +func emitTerminal(ctx context.Context, sink Sink, err error) error { + var se *StreamError + if !errors.As(err, &se) { + se = &StreamError{Code: CodeInternal, Message: err.Error(), Terminal: true} + } + if emitErr := sink.Emit(ctx, se.Event()); emitErr != nil { + return emitErr + } + return se +} diff --git a/gateway/stream_conformance_test.go b/gateway/stream_conformance_test.go new file mode 100644 index 0000000..799c41c --- /dev/null +++ b/gateway/stream_conformance_test.go @@ -0,0 +1,173 @@ +package gateway + +import ( + "context" + "encoding/json" + "testing" +) + +// The gateway's half of the corpus, actually asserted. +// +// conformance_test.go checks that the corpus is well-formed. This checks that the GATEWAY is: each +// fixture's `watch:` ops are played through a fake Kubernetes watch, and what the gateway puts on +// the wire is compared to the fixture's `events:` — byte for byte, because `events:` is the contract +// the TypeScript store on the other side is fed. +// +// No cluster. That is not a compromise: every rule in this corpus is about FRAMING (reset…synced, +// pruning, tombstones, resync) and PROJECTION, and a fake watch drives those deterministically in +// microseconds. What a fake cannot prove — that a real API server's streaming list really does end +// its snapshot with an `initial-events-end` bookmark, that a real 410 arrives as we expect — is the +// job of the k3d suite, and of nothing below it. + +func TestGatewayConformance(t *testing.T) { + c := corpus(t) + ran := 0 + for _, f := range c.Fixtures { + if !f.Suite("gateway") || len(f.Watch) == 0 { + continue + } + ran++ + t.Run(f.ID, func(t *testing.T) { + t.Logf("defends: %s", f.Why) + + want := make([]Event, 0, len(f.Events)) + for i, fe := range f.Events { + ev, err := c.Resolve(f.Scope, f.Projection, fe) + if err != nil { + t.Fatalf("fixture event %d: %v", i, err) + } + want = append(want, ev) + } + + got := replayFixture(t, c, f) + assertEventsEqual(t, want, got) + }) + } + if ran == 0 { + t.Fatal("no gateway fixtures ran — the suite is asserting nothing at all") + } +} + +// replayFixture drives one fixture's `watch:` script through the gateway and returns everything the +// gateway emitted, across every connection the script implies. +// +// A `disconnect` op ends the SSE CONNECTION: the browser's EventSource dropped, and what follows is +// a whole new stream (a new Run, a new reset). A `relist` does NOT — upstream continuity was lost +// while the connection stayed perfectly healthy, and the gateway must recover ON the live connection +// (protocol §5). Conflating those two is the bug `resync-midstream` exists to catch, so the harness +// has to keep them apart or the fixture proves nothing. +func replayFixture(t *testing.T, c Corpus, f Fixture) []Event { + t.Helper() + + gw := &Gateway{ + Auth: AllowAll{}, + Projection: f.Projection, + } + + var got []Event + for _, conn := range splitConnections(f.Watch) { + backend, err := NewScriptedBackend(c, conn) + if err != nil { + t.Fatalf("scripted backend: %v", err) + } + gw.Clients = func(_ string, _ Principal) (Backend, error) { return backend, nil } + + ctx, cancel := context.WithCancel(t.Context()) + sink := &recordingSink{} + + done := make(chan error, 1) + go func() { done <- gw.Stream(ctx, nil, *f.Scope, sink) }() + + // The script is exhausted the moment the gateway comes BACK for another event having been + // given the last one — which is proof it finished processing it. No sleeps, no polling: a + // pull-based upstream makes the handoff a synchronisation point for free. + select { + case <-backend.Exhausted(): + case err := <-done: + t.Fatalf("the gateway stopped before the script did: %v", err) + } + cancel() + if err := <-done; err != nil && !isCanceled(err) { + t.Fatalf("stream: %v", err) + } + + got = append(got, sink.events...) + } + return got +} + +// splitConnections cuts the watch script at each `disconnect`. +func splitConnections(ops []WatchOp) [][]WatchOp { + conns := [][]WatchOp{{}} + for _, op := range ops { + if op.Op == "disconnect" { + conns = append(conns, []WatchOp{}) + continue + } + conns[len(conns)-1] = append(conns[len(conns)-1], op) + } + out := conns[:0] + for _, c := range conns { + if len(c) > 0 { + out = append(out, c) + } + } + return out +} + +func assertEventsEqual(t *testing.T, want, got []Event) { + t.Helper() + for i := range max(len(want), len(got)) { + switch { + case i >= len(got): + t.Errorf("event %d: MISSING\n want: %s", i, mustJSON(t, want[i])) + case i >= len(want): + t.Errorf("event %d: UNEXPECTED\n got: %s", i, mustJSON(t, got[i])) + default: + w, g := mustJSON(t, wireForm(want[i])), mustJSON(t, wireForm(got[i])) + if w != g { + t.Errorf("event %d differs\n want: %s\n got: %s", i, w, g) + } + if got[i].Type == EventError && got[i].Message == "" { + t.Errorf("event %d: an error with no message is a diagnostic that says nothing", i) + } + } + } +} + +// wireForm is the event as the corpus specifies it — which is everything EXCEPT an error's `message`. +// +// This is the one field compared loosely, and not for convenience. `code` is normative: it is a +// stable, machine-readable set precisely because "a free-form message alone is not enough for stable +// client behavior" (spec §4.3), and the corpus's fixture-event type carries no `message` field at +// all, in either loader. Pinning the prose would be asserting a rule the contract does not make — +// and would push a gateway towards emitting nothing, which is worse for whoever has to debug it at +// 3am. So: the code and the terminal flag are pinned exactly, the prose must merely exist. +func wireForm(ev Event) Event { + if ev.Type == EventError { + ev.Message = "" + } + return ev +} + +func mustJSON(t *testing.T, v any) string { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return string(b) +} + +func isCanceled(err error) bool { + return err == context.Canceled || err == context.DeadlineExceeded +} + +// recordingSink is the wire, minus the wire: it keeps what the gateway emitted so the test can +// compare it to the fixture. The SSE sink is the same interface with bytes on the end of it. +type recordingSink struct{ events []Event } + +func (s *recordingSink) Emit(_ context.Context, ev Event) error { + s.events = append(s.events, ev) + return nil +} diff --git a/gateway/stream_test.go b/gateway/stream_test.go new file mode 100644 index 0000000..90f2f62 --- /dev/null +++ b/gateway/stream_test.go @@ -0,0 +1,343 @@ +package gateway + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" +) + +// The gateway rules the CORPUS cannot express — and the reason it cannot is structural, not an +// oversight: a fixture's `watch:` script has ops for list/added/modified/deleted/relist/disconnect, +// and there is no way to write down "the API server sent a bare BOOKMARK" or "the informer handed us +// a tombstone with no uid". So a fixture cannot drive them, and I proved it: mutating the gateway to +// emit `synced` on every bookmark, or to forward an object with no uid, leaves all 11 gateway +// fixtures green. +// +// They are MUST NOTs (spec §2, §4.2) and rows 7, 8, 14 of the gateway's own failure matrix, so they +// get tested here. If we later teach the fixture format those three ops, these move into the corpus +// where they belong and this file shrinks. + +// stubBackend replays a fixed list of upstream events on the FIRST watch, then goes quiet. Unlike +// ScriptedBackend it is not driven by a fixture — it exists to say things a fixture has no +// vocabulary for. +// +// Quiet on reopen, and that is not laziness: several of these scenarios end in a resnapshot, and a +// stub that replayed the same broken tombstone into every new cycle would spin forever. A real +// cluster re-lists into a healthy world; the recovery is the thing under test, not the fault. +type stubBackend struct { + events []WatchEvent + watches int +} + +func (b *stubBackend) Watch(_ context.Context, _ Scope) (Watcher, error) { + b.watches++ + if b.watches > 1 { + return &stubWatcher{}, nil + } + return &stubWatcher{events: b.events}, nil +} + +type stubWatcher struct { + events []WatchEvent + i int +} + +func (w *stubWatcher) Next(ctx context.Context) (WatchEvent, error) { + if w.i < len(w.events) { + ev := w.events[w.i] + w.i++ + return ev, nil + } + <-ctx.Done() + return WatchEvent{}, ctx.Err() +} + +func (w *stubWatcher) Stop() {} + +// run drives the gateway over a stub upstream until it has emitted `want` events, and returns them. +// +// The sink stops the stream by refusing the (want+1)th event — a sink error unwinds the loop the +// same way a closed browser connection does. That, rather than a cancel-and-hope, is what makes +// reading `events` afterwards free of a data race. +func run(t *testing.T, projection Projection, auth Authorizer, events []WatchEvent, want int) []Event { + t.Helper() + gw := &Gateway{ + Auth: auth, + Projection: projection, + Clients: func(string, Principal) (Backend, error) { + return &stubBackend{events: events}, nil + }, + } + // A deadline, not a bare cancel. A gateway that emits FEWER events than the scenario expects + // must FAIL — loudly, in seconds — and not hang the suite until CI's own timeout kills it with + // no useful message. (This is not hypothetical: mutating the partial-object guard away is + // exactly the bug that produces "four events, then silence".) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + sink := &countingSink{want: want, done: make(chan struct{})} + stopped := make(chan struct{}) + go func() { + defer close(stopped) + _ = gw.Stream(ctx, nil, Scope{Target: "demo", Version: "v1", Resource: "configmaps"}, sink) + }() + + select { + case <-sink.done: + case <-ctx.Done(): + <-stopped + t.Fatalf("the gateway emitted %d events and then went quiet; the scenario wants %d: %v", + len(sink.events), want, types(sink.events)) + } + cancel() + <-stopped + return sink.events +} + +var errEnough = errors.New("test: that is all the events we wanted") + +type countingSink struct { + want int + events []Event + done chan struct{} +} + +func (s *countingSink) Emit(_ context.Context, ev Event) error { + if len(s.events) >= s.want { + return errEnough + } + s.events = append(s.events, ev) + if len(s.events) == s.want { + close(s.done) + } + return nil +} + +func types(evs []Event) []EventType { + out := make([]EventType, len(evs)) + for i, e := range evs { + out[i] = e.Type + } + return out +} + +func equalTypes(a []EventType, b ...EventType) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func cm(uid, rv string, data map[string]any) KRMObject { + return KRMObject{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{"uid": uid, "name": "c", "namespace": "app", "resourceVersion": rv}, + "data": data, + } +} + +// Spec §2: a BOOKMARK is never forwarded. Only the one that closes the initial events IS the +// snapshot boundary; the rest exist to carry a resourceVersion the browser has no business seeing. +// Emit `synced` for every bookmark and a consumer prunes mid-cycle — which is the one thing pruning +// is gated on `synced` to prevent. +func TestBookmarksAreAbsorbedExceptTheSnapshotBoundary(t *testing.T) { + got := run(t, ProjectionEditor, AllowAll{}, []WatchEvent{ + {Type: WatchAdded, Object: cm("u1", "1", nil)}, + {Type: WatchBookmark}, // a routine bookmark, mid-snapshot + {Type: WatchBookmark, InitialEventsEnd: true}, // THE boundary + {Type: WatchBookmark}, // a routine bookmark, live + {Type: WatchModified, Object: cm("u1", "2", nil)}, // + }, 4) + + if !equalTypes(types(got), EventReset, EventAdded, EventSynced, EventModified) { + t.Errorf("a bookmark reached the wire: %v", types(got)) + } +} + +// Spec §4.2: if the gateway cannot recover a trustworthy uid it MUST NOT emit an ambiguous +// `deleted`. It begins a new snapshot cycle instead and lets reset…synced prune. Guessing here +// deletes the wrong object out of somebody's browser. +func TestDegenerateTombstoneResnapshotsRatherThanGuessing(t *testing.T) { + got := run(t, ProjectionEditor, AllowAll{}, []WatchEvent{ + {Type: WatchAdded, Object: cm("u1", "1", nil)}, + {Type: WatchBookmark, InitialEventsEnd: true}, + // An informer tombstone that lost the object: no uid, no name. + {Type: WatchDeleted, Object: KRMObject{"kind": "ConfigMap", "apiVersion": "v1", "metadata": map[string]any{}}}, + }, 5) + + if !equalTypes(types(got), EventReset, EventAdded, EventSynced, EventError, EventReset) { + t.Fatalf("want reset,added,synced,error,reset; got %v", types(got)) + } + if got[3].Code != CodeResyncRequired || got[3].Terminal { + t.Errorf("the recovery must be a NON-terminal RESYNC_REQUIRED; got %+v", got[3]) + } + for _, ev := range got { + if ev.Type == EventDeleted { + t.Error("an ambiguous `deleted` reached the wire — that deletes the wrong object") + } + } +} + +// Spec §2: a metadata-only / partial object is never forwarded as added/modified. The consumer's +// model is REPLACE, so a fragment would blank its state for that uid. +func TestPartialObjectIsNeverForwarded(t *testing.T) { + got := run(t, ProjectionEditor, AllowAll{}, []WatchEvent{ + {Type: WatchAdded, Object: cm("u1", "1", nil)}, + {Type: WatchBookmark, InitialEventsEnd: true}, + {Type: WatchModified, Object: KRMObject{"kind": "ConfigMap", "metadata": map[string]any{"name": "c"}}}, // no uid + }, 5) + + if !equalTypes(types(got), EventReset, EventAdded, EventSynced, EventError, EventReset) { + t.Fatalf("a partial object must trigger a resnapshot, not an upsert; got %v", types(got)) + } +} + +// Spec §6: within one cycle, never emit a state for a uid older than one already emitted. This is +// what makes coalescing safe, and an informer that relists WILL replay an older version. +func TestStaleResourceVersionIsDropped(t *testing.T) { + got := run(t, ProjectionEditor, AllowAll{}, []WatchEvent{ + {Type: WatchAdded, Object: cm("u1", "10", map[string]any{"v": "a"})}, + {Type: WatchBookmark, InitialEventsEnd: true}, + {Type: WatchModified, Object: cm("u1", "7", map[string]any{"v": "STALE"})}, // older: must not be emitted + {Type: WatchModified, Object: cm("u1", "11", map[string]any{"v": "b"})}, + }, 4) + + if !equalTypes(types(got), EventReset, EventAdded, EventSynced, EventModified) { + t.Fatalf("want the stale event dropped; got %v", types(got)) + } + if data, _ := got[3].Object["data"].(map[string]any); data["v"] != "b" { + t.Errorf("the consumer was handed a state it had already moved past: %v", data) + } +} + +// The projection removes machinery — and ONLY what a named projection says it removes. Nothing is +// "optionally other server-side bookkeeping". +func TestProjectionRemovesMachineryAndNothingElse(t *testing.T) { + obj := KRMObject{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "uid": "u1", "name": "c", "resourceVersion": "1", + "managedFields": []any{map[string]any{"manager": "kubectl"}}, + "annotations": map[string]any{lastAppliedAnnotation: "{...}", "note": "keep"}, + }, + "data": map[string]any{"a": "1"}, + } + out, paths := project(ProjectionEditor, obj) + meta := out["metadata"].(map[string]any) + + if _, ok := meta["managedFields"]; ok { + t.Error("managedFields must never reach a human editor — nor round-trip back") + } + ann := meta["annotations"].(map[string]any) + if _, ok := ann[lastAppliedAnnotation]; ok { + t.Error("last-applied-configuration must be removed") + } + if ann["note"] != "keep" { + t.Error("the projection removed something it does not declare") + } + if len(paths) != 0 { + t.Errorf("removal is not redaction: redactedPaths must be empty here, got %v", paths) + } + // Removal must not mutate the caller's object: an informer hands the same pointer to every + // subscriber, so an in-place edit here corrupts the object for every other browser on this scope. + if _, ok := obj["metadata"].(map[string]any)["managedFields"]; !ok { + t.Error("project() mutated the upstream's object in place") + } +} + +// An annotations map that is empty ONLY because the projection emptied it is our artifact, not the +// server's state — and "has an empty annotation map" is a different fact from "has none". +func TestProjectionDoesNotLeaveAnEmptyAnnotationsMap(t *testing.T) { + out, _ := project(ProjectionEditor, KRMObject{ + "apiVersion": "v1", "kind": "ConfigMap", + "metadata": map[string]any{"uid": "u1", "name": "c", "annotations": map[string]any{lastAppliedAnnotation: "{...}"}}, + }) + if _, ok := out["metadata"].(map[string]any)["annotations"]; ok { + t.Error("the projection invented an empty annotations map the server does not have") + } +} + +// Matrix #24, and the reason redactedPaths exists at all: a real value that merely LOOKS like the +// mask is not redacted. redactedPaths is authoritative; the shape of a value is never evidence. +func TestARealValueThatLooksLikeTheMaskIsNotRedacted(t *testing.T) { + _, paths := project(ProjectionEditor, cm("u1", "1", map[string]any{"greeting": RedactedPlaceholder})) + if len(paths) != 0 { + t.Errorf("a ConfigMap value that happens to equal the placeholder was reported as redacted: %v", paths) + } +} + +// Keys-only disclosure: you may see THAT `token` exists — which is what keeps the object editable at +// all — and you may never see or write what it is. +func TestSecretDisclosureIsKeysOnly(t *testing.T) { + secret := KRMObject{ + "apiVersion": "v1", "kind": "Secret", "type": "Opaque", + "metadata": map[string]any{"uid": "s1", "name": "git-creds", "labels": map[string]any{"a/b.c": "x"}}, + // A Secret's whole point is that these are secret. They are base64 of "hunter2" and "bob", + // and the assertion below is that neither ever reaches the wire. + "data": map[string]any{"token": "aHVudGVyMg==", "user~name": "Ym9i"}, //nolint:gosec // fixture data, and that is the test + } + out, paths := project(ProjectionEditor, secret) + + data := out["data"].(map[string]any) + if data["token"] != RedactedPlaceholder || data["user~name"] != RedactedPlaceholder { + t.Errorf("a Secret value reached the wire: %v", data) + } + // RFC 6901: `~` escapes to `~0`. A key with a tilde in it is legal, and an unescaped pointer + // silently addresses the wrong field — the same class of bug as the client's dotted paths. + want := []string{"/data/token", "/data/user~0name"} + if len(paths) != 2 || paths[0] != want[0] || paths[1] != want[1] { + t.Errorf("redactedPaths: want %v, got %v", want, paths) + } + if _, ok := out["metadata"].(map[string]any)["labels"]; !ok { + t.Error("labels must stay editable on a redacted Secret — that is the point of keys-only") + } + + // krm-raw/v1 declares no Secret policy. It must not silently apply one either — a projection's + // removal rules are what it SAYS they are, or the identifier is worthless. + rawOut, rawPaths := project(ProjectionRaw, secret) + if rawOut["data"].(map[string]any)["token"] == RedactedPlaceholder || len(rawPaths) != 0 { + t.Error("krm-raw/v1 masked a value it does not declare masking") + } +} + +// Denial comes before any watch opens. A gateway that opens the watch and filters afterwards has +// already leaked the object's existence to someone who may not see it. +func TestAuthorizerDeniesBeforeTheWatchIsEverOpened(t *testing.T) { + backend := &stubBackend{events: []WatchEvent{{Type: WatchBookmark, InitialEventsEnd: true}}} + gw := &Gateway{ + Auth: AuthorizerFunc(func(context.Context, Principal, Scope) error { + return Forbidden("this scope is not yours") + }), + Clients: func(string, Principal) (Backend, error) { return backend, nil }, + } + sink := &recordingSink{} + err := gw.Stream(t.Context(), nil, Scope{Target: "demo"}, sink) + + if err == nil { + t.Fatal("a denied stream must return the error it emitted") + } + if backend.watches != 0 { + t.Error("the watch was opened for a caller who is not allowed to see the scope") + } + if len(sink.events) != 1 || sink.events[0].Type != EventError || sink.events[0].Code != CodeForbidden { + t.Fatalf("want a single FORBIDDEN error; got %v", sink.events) + } + // Terminal, and it must SAY so: a browser's EventSource reconnects automatically otherwise, and + // will hammer a scope it can never be allowed to see, forever. + if !sink.events[0].Terminal { + t.Error("FORBIDDEN must be terminal") + } + b, _ := json.Marshal(sink.events[0]) + if string(b) == "" { + t.Fatal("unreachable") + } +} From 1047f1256de730b8c79afa2737d8931718a9594c Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 20:47:47 +0000 Subject: [PATCH 04/43] =?UTF-8?q?feat:=20close=20the=20seam=20=E2=80=94=20?= =?UTF-8?q?the=20client=20now=20reads=20bytes=20the=20gateway=20really=20w?= =?UTF-8?q?rote?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rung 2 of the e2e ladder, and the one that turns "end to end" from a claim into a fact. Until now the two implementations shared a DOCUMENT. Go asserted "given this watch I would emit these events"; TypeScript asserted "given these events I hold this state"; and nothing, anywhere, checked that the events one side emits are bytes the other side can read. Two implementations agreeing to disagree is exactly what that gap permits — and it was not hypothetical, see below. Three things land: - gateway/sse.go — the transport. Content-Type, flush-per-frame, a heartbeat COMMENT every 20s, no `id:` lines ever, and a terminal error as the last event before the close. - conformance/gen/sse/*.sse — golden transcripts: the bytes the gateway really writes, through its real sink, committed and regenerated by `task fixtures`. They are produced BY the gateway rather than hand-written beside it, because a transcript written next to the implementation agrees with it by construction and proves nothing. - src/sse.ts — the consumer half that never got built: an incremental SSEDecoder, connectResourceStream (fetch, for bearer tokens) and connectWithEventSource (cookies, the v1 baseline). The TS suite now replays the golden bytes ONE BYTE AT A TIME — a frame split down the middle is what a real network does under exactly the load where you least want to debug it. And `task e2e-wire`: a real Go gateway on a real socket, read by the real client over real fetch, 11 fixtures, no cluster. cmd/replay serves the corpus as SSE, so it is also a cluster you can point a browser at. The seam caught a genuine divergence the moment it was closed: `terminal` was omitted from an error event when false (Go's omitempty), so a consumer had to INFER "do I give up?" from a missing field. That is the same reasoning that made redactedPaths mandatory, and the failure it prevents is worse — EventSource reconnects on its own, so a misread terminal error means every open tab hammering a forbidden scope forever. The field is now always present. Both sides had been "passing" their tests for a week. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 22 ++ Taskfile.yml | 51 +++- conformance/gen/sse/conflict-and-converge.sse | 12 + conformance/gen/sse/delete-recreate-uid.sse | 12 + .../gen/sse/edit-vs-unrelated-change.sse | 10 + conformance/gen/sse/key-removed-upstream.sse | 10 + conformance/gen/sse/named-object-absent.sse | 6 + conformance/gen/sse/nested-field-removed.sse | 10 + conformance/gen/sse/reconnect-prune.sse | 18 ++ conformance/gen/sse/resync-midstream.sse | 20 ++ conformance/gen/sse/secret-redaction.sse | 8 + conformance/gen/sse/snapshot-then-deltas.sse | 12 + conformance/gen/sse/status-only-churn.sse | 10 + gateway/cmd/replay/main.go | 203 ++++++++++++++++ gateway/event.go | 55 +++-- gateway/golden_test.go | 85 +++++++ gateway/sse.go | 129 +++++++++++ gateway/stream_conformance_test.go | 39 ++-- packages/krm-stream/biome.jsonc | 2 +- packages/krm-stream/e2e/wire.ts | 119 ++++++++++ packages/krm-stream/src/index.ts | 2 + packages/krm-stream/src/sse.ts | 218 ++++++++++++++++++ packages/krm-stream/test/expect.ts | 146 ++++++++++++ packages/krm-stream/test/store.test.ts | 188 ++------------- packages/krm-stream/test/wire.test.ts | 150 ++++++++++++ packages/krm-stream/tsconfig.test.json | 2 +- 26 files changed, 1329 insertions(+), 210 deletions(-) create mode 100644 conformance/gen/sse/conflict-and-converge.sse create mode 100644 conformance/gen/sse/delete-recreate-uid.sse create mode 100644 conformance/gen/sse/edit-vs-unrelated-change.sse create mode 100644 conformance/gen/sse/key-removed-upstream.sse create mode 100644 conformance/gen/sse/named-object-absent.sse create mode 100644 conformance/gen/sse/nested-field-removed.sse create mode 100644 conformance/gen/sse/reconnect-prune.sse create mode 100644 conformance/gen/sse/resync-midstream.sse create mode 100644 conformance/gen/sse/secret-redaction.sse create mode 100644 conformance/gen/sse/snapshot-then-deltas.sse create mode 100644 conformance/gen/sse/status-only-churn.sse create mode 100644 gateway/cmd/replay/main.go create mode 100644 gateway/golden_test.go create mode 100644 gateway/sse.go create mode 100644 packages/krm-stream/e2e/wire.ts create mode 100644 packages/krm-stream/src/sse.ts create mode 100644 packages/krm-stream/test/expect.ts create mode 100644 packages/krm-stream/test/wire.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a652e6..dc07541 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,3 +70,25 @@ jobs: # bundler and no runtime dependency. - run: npx tsc working-directory: packages/krm-stream + + # The seam. Both suites above pass against bytes that never left their own process — which cannot + # prove that the server flushes, that frames survive arbitrary chunk boundaries, or that a terminal + # error really closes the connection. This job runs the REAL Go gateway over a REAL socket and reads + # it with the REAL TypeScript client. Still no cluster: the upstream is the scripted corpus. + wire: + name: end to end (go gateway -> ts client, over HTTP) + runs-on: ubuntu-latest + needs: fixtures + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version: "1.26" + cache-dependency-path: gateway/go.sum + - uses: actions/setup-node@v6 + with: + node-version: "22" + - uses: arduino/setup-task@v2 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - run: task e2e-wire diff --git a/Taskfile.yml b/Taskfile.yml index ad8529f..a866869 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -7,6 +7,7 @@ run: once vars: CLIENT_DIR: packages/krm-stream + REPLAY_ADDR: 127.0.0.1:8099 # A throwaway cluster for proving the gateway against a real API server. # (The unit suites need no cluster at all — see `task test`.) SPIKE_CLUSTER: krm-stream-spike @@ -20,15 +21,25 @@ tasks: # ---------------------------------------------------------------- fixtures -- fixtures: - desc: "Regenerate conformance/gen/*.json from the YAML sources (both suites read the JSON)." + desc: "Regenerate conformance/gen/ — the JSON both suites read, and the SSE both suites agree on." sources: - conformance/bodies/*.yaml - conformance/fixtures/*.yaml - conformance/generate.sh + - gateway/*.go generates: - conformance/gen/*.json + - conformance/gen/sse/*.sse cmds: - bash conformance/generate.sh + # The SSE goldens: the bytes the gateway REALLY writes, through its real sink. They are what + # closes the seam — the TypeScript suite parses these exact files back and replays them into a + # store, so "the client can read what the gateway wrote" is a test and not a hope. + # + # They are generated by the gateway rather than by generate.sh precisely BECAUSE the gateway is + # the thing under test: a transcript hand-written next to the implementation would agree with it + # by construction and prove nothing. + - cd gateway && go test -count=1 -run TestSSEGoldens -update ./... fixtures-check: desc: "Fail if conformance/gen/ is stale — the YAML is the source of truth, the JSON is built." @@ -55,6 +66,44 @@ tasks: cmds: - node --test + # --------------------------------------------------------------------- e2e -- + replay: + desc: "Serve the conformance corpus over real SSE — a cluster you can point a browser at." + dir: gateway + cmds: + - go run ./cmd/replay --addr {{.REPLAY_ADDR}} {{.CLI_ARGS}} + + e2e-wire: + desc: "End to end, no cluster: the real Go gateway over a real socket, into the real TS store." + deps: [fixtures] + cmds: + # Two processes, two languages, one contract. Everything else in the suite feeds the store bytes + # that never left the process — which cannot prove that the server flushes, that the frames + # survive arbitrary chunk boundaries, or that a terminal error really closes the connection. + - | + set -e + log="$(mktemp -t krm-replay-XXXX.log)" + + # BUILD, then run the binary — do not `go run` it. `go run` execs the real program as a + # CHILD, so killing the pid we know about leaves the server holding the port, and the next + # run of this task fails with a confusing "address already in use". + # + # And redirect its output to a file rather than inheriting ours: a background process holding + # our stdout open makes `task` (and any pipe we are in) wait for it forever, which looks + # exactly like a hung test and is not one. + bin="$(mktemp -t krm-replay-XXXX)" + (cd gateway && go build -o "$bin" ./cmd/replay) + "$bin" --addr {{.REPLAY_ADDR}} --corpus conformance >"$log" 2>&1 & + pid=$! + trap 'kill $pid 2>/dev/null || true; rm -f "$bin"' EXIT + + for _ in $(seq 50); do + curl -sf http://{{.REPLAY_ADDR}}/healthz >/dev/null && break || sleep 0.2 + done + curl -sf http://{{.REPLAY_ADDR}}/healthz >/dev/null || { echo "replay server never came up:"; cat "$log"; exit 1; } + + cd {{.CLIENT_DIR}} && REPLAY_URL=http://{{.REPLAY_ADDR}} node e2e/wire.ts + # -------------------------------------------------------------------- lint -- lint: desc: "Vet + lint both languages." diff --git a/conformance/gen/sse/conflict-and-converge.sse b/conformance/gen/sse/conflict-and-converge.sse new file mode 100644 index 0000000..bee2cee --- /dev/null +++ b/conformance/gen/sse/conflict-and-converge.sse @@ -0,0 +1,12 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"warn","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1003","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"debug","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1004","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/delete-recreate-uid.sse b/conformance/gen/sse/delete-recreate-uid.sse new file mode 100644 index 0000000..c9ffd0f --- /dev/null +++ b/conformance/gen/sse/delete-recreate-uid.sse @@ -0,0 +1,12 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"deleted","identity":{"uid":"cm-app-0001","apiVersion":"v1","kind":"ConfigMap","namespace":"app","name":"app-config"}} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"trace"},"kind":"ConfigMap","metadata":{"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"2001","uid":"cm-app-0002"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/edit-vs-unrelated-change.sse b/conformance/gen/sse/edit-vs-unrelated-change.sse new file mode 100644 index 0000000..8bd7d3f --- /dev/null +++ b/conformance/gen/sse/edit-vs-unrelated-change.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"5"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1002","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/key-removed-upstream.sse b/conformance/gen/sse/key-removed-upstream.sse new file mode 100644 index 0000000..c209017 --- /dev/null +++ b/conformance/gen/sse/key-removed-upstream.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"info"},"kind":"ConfigMap","metadata":{"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1005","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/named-object-absent.sse b/conformance/gen/sse/named-object-absent.sse new file mode 100644 index 0000000..9f8bd36 --- /dev/null +++ b/conformance/gen/sse/named-object-absent.sse @@ -0,0 +1,6 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app","name":"app-config"},"projection":"krm-editor/v1"} + +data: {"type":"synced"} + diff --git a/conformance/gen/sse/nested-field-removed.sse b/conformance/gen/sse/nested-field-removed.sse new file mode 100644 index 0000000..363398c --- /dev/null +++ b/conformance/gen/sse/nested-field-removed.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","group":"apps","version":"v1","resource":"deployments","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":4,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3001","uid":"deploy-web-0001"},"spec":{"paused":false,"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasUnavailable","status":"False","type":"Available"}],"observedGeneration":4,"readyReplicas":1,"replicas":2}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":5,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3003","uid":"deploy-web-0001"},"spec":{"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasAvailable","status":"True","type":"Available"}],"observedGeneration":5,"readyReplicas":2,"replicas":2}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/reconnect-prune.sse b/conformance/gen/sse/reconnect-prune.sse new file mode 100644 index 0000000..b313db2 --- /dev/null +++ b/conformance/gen/sse/reconnect-prune.sse @@ -0,0 +1,18 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"checkout-v2":"false"},"kind":"ConfigMap","metadata":{"name":"feature-flags","namespace":"app","resourceVersion":"1010","uid":"cm-flags-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +: connection 2 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + diff --git a/conformance/gen/sse/resync-midstream.sse b/conformance/gen/sse/resync-midstream.sse new file mode 100644 index 0000000..0873f92 --- /dev/null +++ b/conformance/gen/sse/resync-midstream.sse @@ -0,0 +1,20 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"checkout-v2":"false"},"kind":"ConfigMap","metadata":{"name":"feature-flags","namespace":"app","resourceVersion":"1010","uid":"cm-flags-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"5"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1002","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"error","code":"RESYNC_REQUIRED","message":"upstream continuity lost; a new snapshot cycle follows","terminal":false} + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"5"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1002","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + diff --git a/conformance/gen/sse/secret-redaction.sse b/conformance/gen/sse/secret-redaction.sse new file mode 100644 index 0000000..ffd5608 --- /dev/null +++ b/conformance/gen/sse/secret-redaction.sse @@ -0,0 +1,8 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"secrets","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"token":"**REDACTED**","username":"**REDACTED**"},"kind":"Secret","metadata":{"labels":{"app.kubernetes.io/name":"shop"},"name":"git-creds","namespace":"app","resourceVersion":"4001","uid":"secret-token-0001"},"type":"Opaque"},"redactedPaths":["/data/token","/data/username"]} + +data: {"type":"synced"} + diff --git a/conformance/gen/sse/snapshot-then-deltas.sse b/conformance/gen/sse/snapshot-then-deltas.sse new file mode 100644 index 0000000..7609169 --- /dev/null +++ b/conformance/gen/sse/snapshot-then-deltas.sse @@ -0,0 +1,12 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"checkout-v2":"false"},"kind":"ConfigMap","metadata":{"name":"feature-flags","namespace":"app","resourceVersion":"1010","uid":"cm-flags-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"5"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1002","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/status-only-churn.sse b/conformance/gen/sse/status-only-churn.sse new file mode 100644 index 0000000..974d130 --- /dev/null +++ b/conformance/gen/sse/status-only-churn.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","group":"apps","version":"v1","resource":"deployments","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":4,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3001","uid":"deploy-web-0001"},"spec":{"paused":false,"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasUnavailable","status":"False","type":"Available"}],"observedGeneration":4,"readyReplicas":1,"replicas":2}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":4,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3002","uid":"deploy-web-0001"},"spec":{"paused":false,"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasAvailable","status":"True","type":"Available"}],"observedGeneration":4,"readyReplicas":2,"replicas":2}},"redactedPaths":[]} + diff --git a/gateway/cmd/replay/main.go b/gateway/cmd/replay/main.go new file mode 100644 index 0000000..8ea180d --- /dev/null +++ b/gateway/cmd/replay/main.go @@ -0,0 +1,203 @@ +// Command replay serves the conformance corpus over real HTTP, as a real SSE stream. +// +// It is a cluster you can point a browser at. Every fixture becomes an endpoint: +// +// /resource-stream/v1?fixture=status-only-churn +// +// and connecting to it plays that scenario — the snapshot, the deltas, the mid-stream resync, the +// redacted Secret — through the real gateway, the real projection and the real SSE sink. Nothing is +// stubbed except the API server. +// +// Why it exists, given that the conformance suites already pass in-process: because "the client can +// read what the gateway wrote" and "the client can read what the gateway wrote OVER A NETWORK" are +// different claims. Chunk boundaries, flushing, Content-Type, heartbeats arriving in wall-clock time, +// and a terminal error actually CLOSING the connection are all invisible to an in-process test and +// all perfectly capable of breaking a browser. +// +// go run ./cmd/replay --addr :8080 +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "log" + "net/http" + "os" + "os/signal" + "path/filepath" + "sort" + "time" + + "github.com/ConfigButler/krm-stream/gateway" +) + +func main() { + addr := flag.String("addr", "127.0.0.1:8080", "address to listen on") + corpusDir := flag.String("corpus", filepath.Join("..", "conformance"), "path to the conformance/ directory") + pace := flag.Duration("pace", 0, "delay between events — a human watching a status roll out wants >0") + flag.Parse() + + corpus, err := gateway.LoadCorpus(*corpusDir) + if err != nil { + log.Fatalf("replay: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/resource-stream/v1", stream(corpus, *pace)) + mux.HandleFunc("/fixtures", list(corpus)) + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { _, _ = fmt.Fprintln(w, "ok") }) + + srv := &http.Server{ + Addr: *addr, + Handler: mux, + // No WriteTimeout. An SSE stream is meant to stay open — a write deadline would sever a live + // status watch on a schedule, which looks exactly like a bug in the client and is not. + ReadHeaderTimeout: 10 * time.Second, + } + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + defer stop() + go func() { + <-ctx.Done() + shutdown, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = srv.Shutdown(shutdown) + }() + + log.Printf("replay: serving %d fixtures on http://%s", len(corpus.Fixtures), *addr) + if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatalf("replay: %v", err) + } +} + +func stream(corpus gateway.Corpus, pace time.Duration) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + id := r.URL.Query().Get("fixture") + var f *gateway.Fixture + for i := range corpus.Fixtures { + if corpus.Fixtures[i].ID == id { + f = &corpus.Fixtures[i] + } + } + if f == nil || len(f.Watch) == 0 || f.Scope == nil { + http.Error(w, fmt.Sprintf("no such replayable fixture: %q (see /fixtures)", id), http.StatusNotFound) + return + } + + // A `disconnect` op means the BROWSER's connection dropped, which on a real transport is not + // something the server does — it is something that happens TO it. So a replay serves the ops + // up to the first disconnect and then closes; the browser's own EventSource reconnects, and + // gets the next segment. Which is, of course, exactly the scenario reconnect-prune describes. + conn := connectionFor(r, f.Watch) + backend, err := gateway.NewScriptedBackend(corpus, conn.ops) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + gw := &gateway.Gateway{ + Auth: gateway.AllowAll{}, + Projection: f.Projection, + Clients: func(string, gateway.Principal) (gateway.Backend, error) { + return paced{backend, pace}, nil + }, + } + + ctx, cancel := context.WithCancel(r.Context()) + defer cancel() + go func() { + // The script is finished: close the connection, the way an API server eventually does. + // The consumer must survive it — and, if there is another segment, come back for it. + <-backend.Exhausted() + if conn.last { + time.Sleep(50 * time.Millisecond) // let the final frame reach the socket + } + cancel() + }() + + gw.ServeStream(w, r.WithContext(ctx), nil, *f.Scope) + } +} + +// connectionFor picks which segment of a multi-connection script this request gets. The browser tells +// us implicitly: EventSource reconnects, and the `connection` query param (which the demo page bumps) +// says which one it is on. A conformance fixture with no `disconnect` has exactly one. +type connection struct { + ops []gateway.WatchOp + last bool +} + +func connectionFor(r *http.Request, ops []gateway.WatchOp) connection { + var conns [][]gateway.WatchOp + cur := []gateway.WatchOp{} + for _, op := range ops { + if op.Op == "disconnect" { + conns = append(conns, cur) + cur = []gateway.WatchOp{} + continue + } + cur = append(cur, op) + } + conns = append(conns, cur) + + n := 0 + if v := r.URL.Query().Get("connection"); v != "" { + if _, err := fmt.Sscanf(v, "%d", &n); err != nil || n < 0 { + n = 0 + } + } + if n >= len(conns) { + n = len(conns) - 1 + } + return connection{ops: conns[n], last: n == len(conns)-1} +} + +// paced slows the script down so a human can watch it. At pace=0 it is a no-op and the stream arrives +// as fast as the socket takes it — which is what the automated e2e wants, and what no demo does. +type paced struct { + inner gateway.Backend + delay time.Duration +} + +func (p paced) Watch(ctx context.Context, s gateway.Scope) (gateway.Watcher, error) { + w, err := p.inner.Watch(ctx, s) + if err != nil { + return nil, err + } + return pacedWatcher{w, p.delay}, nil +} + +type pacedWatcher struct { + inner gateway.Watcher + delay time.Duration +} + +func (p pacedWatcher) Next(ctx context.Context) (gateway.WatchEvent, error) { + if p.delay > 0 { + select { + case <-time.After(p.delay): + case <-ctx.Done(): + return gateway.WatchEvent{}, ctx.Err() + } + } + return p.inner.Next(ctx) +} + +func (p pacedWatcher) Stop() { p.inner.Stop() } + +func list(corpus gateway.Corpus) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + var ids []string + for _, f := range corpus.Fixtures { + if len(f.Watch) > 0 { + ids = append(ids, fmt.Sprintf("%-26s %s", f.ID, f.Title)) + } + } + sort.Strings(ids) + for _, id := range ids { + _, _ = fmt.Fprintln(w, id) + } + } +} diff --git a/gateway/event.go b/gateway/event.go index 59ee4b3..477c612 100644 --- a/gateway/event.go +++ b/gateway/event.go @@ -127,27 +127,48 @@ type Event struct { RetryAfterMs *int `json:"retryAfterMs,omitempty"` } -// MarshalJSON exists for one field: `redactedPaths` is REQUIRED on every added/modified — present, -// not merely optional — and `omitempty` would silently drop the empty array, which is precisely the -// case the requirement is about ("nothing is redacted in this object" must be SAID, not inferred). -// It stays omitted on the events that have no business carrying it. +// MarshalJSON exists for two fields, and for one reason: a consumer must never have to INFER +// something the protocol makes it responsible for. // -// The conformance suite caught this the first time it ran. That is the corpus doing its job. +// - `redactedPaths` is REQUIRED on every added/modified. `omitempty` would silently drop the empty +// array — which is exactly the case the requirement is about: "nothing is redacted in this +// object" must be SAID, not inferred from a value that happens to look like a placeholder. +// (The conformance suite caught this the first time it ran. That is the corpus doing its job.) +// +// - `terminal` is REQUIRED on every error. `omitempty` drops it when false, so a consumer would be +// reading "do I give up, or do I retry?" out of a field that is not there. It is the same rule, +// and the failure it prevents is worse: a browser's EventSource reconnects on its own, so a +// misread terminal error means every open tab hammering a forbidden scope forever. +// (The byte-level wire suite caught this one — the client and the gateway disagreed about the +// shape of an event they had both been "passing" tests on for a week.) +// +// Both stay omitted on the events that have no business carrying them. func (e Event) MarshalJSON() ([]byte, error) { type base Event // sheds this method, so json doesn't recurse - if e.Type != EventAdded && e.Type != EventModified { - return json.Marshal(base(e)) - } - paths := e.RedactedPaths - if paths == nil { - paths = []string{} - } inner := base(e) - inner.RedactedPaths = nil // the outer, non-omitempty field carries it (shallower field wins) - return json.Marshal(struct { - base - RedactedPaths []string `json:"redactedPaths"` - }{inner, paths}) + + switch e.Type { + case EventAdded, EventModified: + paths := e.RedactedPaths + if paths == nil { + paths = []string{} + } + inner.RedactedPaths = nil // the outer, non-omitempty field carries it (shallower field wins) + return json.Marshal(struct { + base + RedactedPaths []string `json:"redactedPaths"` + }{inner, paths}) + + case EventError: + inner.Terminal = false // ditto: the outer field is the one that gets marshalled + return json.Marshal(struct { + base + Terminal bool `json:"terminal"` + }{inner, e.Terminal}) + + default: + return json.Marshal(inner) + } } // UID reads metadata.uid out of an object. The empty string means "this object has no usable diff --git a/gateway/golden_test.go b/gateway/golden_test.go new file mode 100644 index 0000000..0394f56 --- /dev/null +++ b/gateway/golden_test.go @@ -0,0 +1,85 @@ +package gateway + +import ( + "bytes" + "flag" + "fmt" + "os" + "path/filepath" + "testing" +) + +// The golden SSE transcripts: conformance/gen/sse/.sse. +// +// This is the file that makes the contract real at the byte level. Until now the two suites shared +// JSON — the Go side asserted "I would emit these events", the TypeScript side asserted "given these +// events I hold this state" — and NOTHING checked that the events one side emits are the bytes the +// other side can read. Two implementations agreeing to disagree is exactly what that gap allows. +// +// So: the gateway writes each fixture through its REAL SSE sink, the bytes are committed, and the +// TypeScript suite parses those bytes back and replays them into the store. The wire is no longer a +// document both sides read; it is an artifact one side produces and the other consumes. +// +// Regenerate with `task fixtures` (or `go test -run TestSSEGoldens -update`). CI fails if stale, +// exactly as it does for conformance/gen/*.json — the goldens are generated, and a generated file +// that drifts from its generator is worse than no generated file. + +var update = flag.Bool("update", false, "rewrite the golden SSE transcripts in conformance/gen/sse/") + +func TestSSEGoldens(t *testing.T) { + c := corpus(t) + dir := filepath.Join("..", "conformance", "gen", "sse") + if *update { + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + } + + for _, f := range c.Fixtures { + if !f.Suite("gateway") || len(f.Watch) == 0 { + continue + } + t.Run(f.ID, func(t *testing.T) { + got := transcribe(t, c, f) + path := filepath.Join(dir, f.ID+".sse") + + if *update { + if err := os.WriteFile(path, got, 0o600); err != nil { + t.Fatal(err) + } + return + } + + want, err := os.ReadFile(path) //nolint:gosec // a fixed, in-repo path + if err != nil { + t.Fatalf("%v\nrun `task fixtures` to generate the golden transcripts", err) + } + if !bytes.Equal(want, got) { + t.Errorf("the wire has changed.\n--- want\n%s\n--- got\n%s", want, got) + } + }) + } +} + +// transcribe replays a fixture through the gateway's real SSE sink and returns the bytes. +// +// A `disconnect` in the script means the browser's EventSource dropped and reconnected, so the +// transcript contains more than one HTTP response, concatenated — which is precisely what a consumer +// experiences across a reconnect, and precisely why the store must be idempotent about it. +// +// The `: connection N` line is the ONE thing here the gateway did not write. It is an SSE comment, +// which a conforming consumer ignores by definition (spec §7) — the same class of thing as a +// heartbeat — so putting it in the golden costs the contract nothing and buys the TypeScript parser a +// free proof that it really does ignore comments. +func transcribe(t *testing.T, c Corpus, f Fixture) []byte { + t.Helper() + var buf bytes.Buffer + replay(t, c, f, + func(conn int) Sink { + fmt.Fprintf(&buf, ": connection %d\n\n", conn+1) + return NewSSESink(&buf) + }, + func(Sink) {}, + ) + return buf.Bytes() +} diff --git a/gateway/sse.go b/gateway/sse.go new file mode 100644 index 0000000..bf4c0c4 --- /dev/null +++ b/gateway/sse.go @@ -0,0 +1,129 @@ +package gateway + +import ( + "context" + "fmt" + "io" + "net/http" + "sync" + "time" +) + +// The transport. Everything above this file is the protocol; this is the only part that knows what a +// byte is. +// +// It is small, and every line of it is a rule from spec §7: +// +// - `text/event-stream`, one JSON object per `data:` frame. +// - A `: heartbeat` COMMENT every ~20s. Not an event — a consumer ignores it — but without it an +// intermediary idles the connection out and the browser silently stops receiving `status`. +// - NO `id:` lines. Ever. v1 has no delta replay and no Last-Event-ID resume, and putting a +// resource uid there (the tempting thing) gives the browser's automatic reconnect an entirely +// incorrect meaning. +// - A terminal error is the LAST event, and then the connection closes — because EventSource +// reconnects on its own otherwise, and would hammer a forbidden scope forever. +// - Flush after every frame. An unflushed SSE stream is a stream that arrives when the buffer +// happens to fill, which for a live status watch is indistinguishable from being broken. + +// HeartbeatInterval is how often a quiet connection is kept alive. ~20s: comfortably under the 30–60s +// idle timeout of every proxy anyone actually deploys behind. +const HeartbeatInterval = 20 * time.Second + +// SSESink writes protocol events to a connection as Server-Sent Events. It is safe for the heartbeat +// goroutine and the stream loop to use concurrently — which they must, since the whole point of a +// heartbeat is that it happens while nothing else is. +type SSESink struct { + mu sync.Mutex + w io.Writer + flush func() +} + +// NewSSESink writes to w, flushing after every frame if w can be flushed. +func NewSSESink(w io.Writer) *SSESink { + s := &SSESink{w: w, flush: func() {}} + if f, ok := w.(http.Flusher); ok { + s.flush = f.Flush + } + return s +} + +// Emit writes one event and flushes. +func (s *SSESink) Emit(_ context.Context, ev Event) error { + frame, err := ev.MarshalSSE() + if err != nil { + return fmt.Errorf("krm-stream: marshal event: %w", err) + } + s.mu.Lock() + defer s.mu.Unlock() + if _, err := s.w.Write(frame); err != nil { + return err + } + s.flush() + return nil +} + +// Comment writes an SSE comment. Comments are NOT events: a conforming consumer ignores them +// entirely, which is exactly why a heartbeat is one. +func (s *SSESink) Comment(text string) error { + s.mu.Lock() + defer s.mu.Unlock() + if _, err := fmt.Fprintf(s.w, ": %s\n\n", text); err != nil { + return err + } + s.flush() + return nil +} + +// Heartbeat keeps the connection alive until ctx is done. Run it in a goroutine alongside the stream. +func (s *SSESink) Heartbeat(ctx context.Context, every time.Duration) { + t := time.NewTicker(every) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + if err := s.Comment("heartbeat"); err != nil { + return // the consumer went away; the stream loop will notice too + } + } + } +} + +// WriteSSEHeaders sets the response headers a conforming stream must carry, and must be called +// before the first frame. +func WriteSSEHeaders(w http.ResponseWriter) { + h := w.Header() + h.Set("Content-Type", "text/event-stream") + h.Set("X-KRM-Stream-Protocol", fmt.Sprint(ProtocolVersion)) + // A stream that a proxy is allowed to cache, buffer or transform is not a stream. `no-cache` + // stops the browser; `no-transform` and the nginx-specific hint stop the middleboxes that + // otherwise sit on the response until it is "big enough" — which turns a live status watch into + // a batch job nobody can debug. + h.Set("Cache-Control", "no-cache, no-transform") + h.Set("Connection", "keep-alive") + h.Set("X-Accel-Buffering", "no") + w.WriteHeader(http.StatusOK) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } +} + +// ServeStream runs one stream over one HTTP response, heartbeats included, and returns when the +// stream ends — at which point the caller returning from its handler closes the connection, which is +// what a terminal error requires. +func (g *Gateway) ServeStream(w http.ResponseWriter, r *http.Request, principal Principal, scope Scope) { + WriteSSEHeaders(w) + + sink := NewSSESink(w) + ctx := r.Context() + + hb, stopHeartbeat := context.WithCancel(ctx) + defer stopHeartbeat() + go sink.Heartbeat(hb, HeartbeatInterval) + + // The error is already ON the wire by the time Stream returns — emitting it is how the consumer + // learns anything. There is nothing left to tell the HTTP layer: the status line went out with + // the very first byte, long before we could have known. + _ = g.Stream(ctx, principal, scope, sink) +} diff --git a/gateway/stream_conformance_test.go b/gateway/stream_conformance_test.go index 799c41c..0d06d50 100644 --- a/gateway/stream_conformance_test.go +++ b/gateway/stream_conformance_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "testing" + "time" ) // The gateway's half of the corpus, actually asserted. @@ -58,42 +59,50 @@ func TestGatewayConformance(t *testing.T) { // has to keep them apart or the fixture proves nothing. func replayFixture(t *testing.T, c Corpus, f Fixture) []Event { t.Helper() + var got []Event + replay(t, c, f, func(int) Sink { return &recordingSink{} }, func(s Sink) { + got = append(got, s.(*recordingSink).events...) + }) + return got +} - gw := &Gateway{ - Auth: AllowAll{}, - Projection: f.Projection, - } +// replay is the driver both the event assertion and the SSE goldens share. Sharing it is the point: +// the golden transcripts are the bytes THIS gateway wrote, through its real SSE sink, rather than a +// second encoding of the same events written for the benefit of the test. +func replay(t *testing.T, c Corpus, f Fixture, newSink func(conn int) Sink, done func(Sink)) { + t.Helper() - var got []Event - for _, conn := range splitConnections(f.Watch) { + gw := &Gateway{Auth: AllowAll{}, Projection: f.Projection} + + for i, conn := range splitConnections(f.Watch) { backend, err := NewScriptedBackend(c, conn) if err != nil { t.Fatalf("scripted backend: %v", err) } gw.Clients = func(_ string, _ Principal) (Backend, error) { return backend, nil } - ctx, cancel := context.WithCancel(t.Context()) - sink := &recordingSink{} + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + sink := newSink(i) - done := make(chan error, 1) - go func() { done <- gw.Stream(ctx, nil, *f.Scope, sink) }() + finished := make(chan error, 1) + go func() { finished <- gw.Stream(ctx, nil, *f.Scope, sink) }() // The script is exhausted the moment the gateway comes BACK for another event having been // given the last one — which is proof it finished processing it. No sleeps, no polling: a // pull-based upstream makes the handoff a synchronisation point for free. select { case <-backend.Exhausted(): - case err := <-done: + case err := <-finished: t.Fatalf("the gateway stopped before the script did: %v", err) + case <-ctx.Done(): + t.Fatal("the gateway never consumed the whole watch script") } cancel() - if err := <-done; err != nil && !isCanceled(err) { + if err := <-finished; err != nil && !isCanceled(err) { t.Fatalf("stream: %v", err) } - - got = append(got, sink.events...) + done(sink) } - return got } // splitConnections cuts the watch script at each `disconnect`. diff --git a/packages/krm-stream/biome.jsonc b/packages/krm-stream/biome.jsonc index b8f7496..779583a 100644 --- a/packages/krm-stream/biome.jsonc +++ b/packages/krm-stream/biome.jsonc @@ -5,7 +5,7 @@ // bundler. Formatting is not a matter of taste here for the same reason gofmt is not — a review of // this package should be an argument about the merge algorithm, never about where a line broke. "files": { - "includes": ["src/**/*.ts", "test/**/*.ts", "*.json", "*.jsonc"] + "includes": ["src/**/*.ts", "test/**/*.ts", "e2e/**/*.ts", "*.json", "*.jsonc"] }, "formatter": { "enabled": true, diff --git a/packages/krm-stream/e2e/wire.ts b/packages/krm-stream/e2e/wire.ts new file mode 100644 index 0000000..997103b --- /dev/null +++ b/packages/krm-stream/e2e/wire.ts @@ -0,0 +1,119 @@ +// The end of the ladder that needs no cluster: a REAL Go gateway, a REAL socket, a REAL fetch. +// +// Not part of `node --test` — it needs the replay server running, so it is its own thing: +// +// task e2e-wire +// +// Everything below it in the ladder feeds the store from bytes that never left the process. That +// proves the protocol, and it proves the parser, and it cannot prove any of this: +// +// - the response really is `text/event-stream`, and no proxy-defeating header is missing; +// - the server really FLUSHES each frame instead of buffering until the socket fills, which is the +// difference between a live status watch and a batch job; +// - the frames really do arrive split across arbitrary chunk boundaries; +// - v1's "no SSE `id:` lines, ever" really holds on the bytes, not just in a unit test; +// - a terminal error really CLOSES the connection. +// +// All five are invisible in-process, and every one of them breaks a browser. + +import assert from "node:assert/strict"; +import { connectResourceStream, LiveResourceStore } from "../src/index.ts"; +import type { Path } from "../src/types.ts"; +import { clientFixtures } from "../test/conformance.ts"; +import { applyEdit, check } from "../test/expect.ts"; + +const BASE = process.env.REPLAY_URL ?? "http://127.0.0.1:8080"; + +let failures = 0; +const ok = (name: string, fn: () => Promise | void) => + Promise.resolve() + .then(fn) + .then(() => console.log(` ok ${name}`)) + .catch((err: Error) => { + failures++; + console.error(` NOT OK ${name}\n ${err.message.split("\n").join("\n ")}`); + }); + +const url = (id: string, conn = 0) => `${BASE}/resource-stream/v1?fixture=${id}&connection=${conn}`; + +/** The fixtures with both a watch script (so the server can serve them) and client expectations. */ +const fixtures = clientFixtures().filter((f) => (f.suites ?? []).includes("gateway") && (f.watch?.length ?? 0) > 0); + +console.log(`e2e: driving ${fixtures.length} fixtures against a real gateway at ${BASE}\n`); + +await ok("the server is up", async () => { + const res = await fetch(`${BASE}/healthz`); + assert.equal(res.status, 200, "start it with `task e2e-wire`, or REPLAY_URL=… to point elsewhere"); +}); + +await ok("the response is a conforming SSE stream, and says so", async () => { + const res = await fetch(url("snapshot-then-deltas")); + assert.equal(res.headers.get("content-type"), "text/event-stream"); + assert.equal(res.headers.get("x-krm-stream-protocol"), "1"); + // A stream a proxy may buffer is not a stream. This header is why nginx does not sit on the + // response until it is "big enough" — which turns a live watch into a batch job nobody can debug. + assert.equal(res.headers.get("x-accel-buffering"), "no"); + assert.match(res.headers.get("cache-control") ?? "", /no-cache/); + await res.body?.cancel(); +}); + +await ok("v1 emits no SSE id: lines, on the actual bytes", async () => { + // Putting a resource uid in `id:` is the tempting thing, and it silently gives the browser's + // automatic Last-Event-ID reconnect an entirely incorrect meaning. Assert on the wire, not on a + // struct: the struct cannot lie about a field it does not have, but a hand-written writer can. + const text = await drain(url("resync-midstream")); + for (const line of text.split("\n")) { + assert.ok(!line.startsWith("id:"), `an SSE id: line reached the wire: ${line}`); + } +}); + +for (const f of fixtures) { + await ok(`${f.id}: over HTTP, the store ends up holding what the fixture says`, async () => { + const store = new LiveResourceStore(); + const flashed: Path[] = []; + + // The index a fixture's `after:` refers to: position in the fixture's `events:` list. Count it + // from the STREAM callbacks — one per delivered event, errors included — and not from the store's + // `subscribe`, which also fires on our own edits and would drift by one the moment a fixture + // edited anything. (It did. That is why this comment exists.) + let i = -1; + const onEvent = () => { + i++; + for (const edit of f.client?.edits ?? []) { + // Applied mid-stream, while the bytes are still arriving. That ordering IS the three-way + // merge, and it is the one thing a "fetch it all, then assert" e2e cannot express. + if (edit.after === i) applyEdit(store, edit); + } + }; + + const connections = (f.watch ?? []).filter((op) => (op as { op: string }).op === "disconnect").length + 1; + for (let conn = 0; conn < connections; conn++) { + const handle = connectResourceStream(url(f.id, conn), store, { + onChange: (paths) => { + flashed.push(...paths); + onEvent(); + }, + onError: () => onEvent(), // an `error` event occupies an index too (see resync-midstream) + }); + await handle.closed; + } + + if (f.client?.expect) check(store, flashed, f.client.expect, `${f.id} (over HTTP)`); + }); +} + +console.log(failures === 0 ? "\ne2e: all green" : `\ne2e: ${failures} FAILED`); +process.exit(failures === 0 ? 0 : 1); + +/** Read a stream to the end and return its raw bytes — for the assertions that are about the WIRE + * rather than about the state it produces. */ +async function drain(u: string): Promise { + const res = await fetch(u); + const reader = res.body!.pipeThrough(new TextDecoderStream()).getReader(); + let text = ""; + for (;;) { + const { done, value } = await reader.read(); + if (done) return text; + text += value; + } +} diff --git a/packages/krm-stream/src/index.ts b/packages/krm-stream/src/index.ts index 26fec60..e3d4b88 100644 --- a/packages/krm-stream/src/index.ts +++ b/packages/krm-stream/src/index.ts @@ -18,6 +18,8 @@ export { clone, deepEqual } from "./deep.ts"; export { get, has, isPrefix, parsePointer, pathKey } from "./path.ts"; export { DEFAULT_EDITABLE_REGIONS, defaultPolicy, readOnlyPolicy, regionPolicy } from "./policy.ts"; +export type { StreamHandle, StreamOptions } from "./sse.ts"; +export { applyStreamEvent, connectResourceStream, connectWithEventSource, SSEDecoder } from "./sse.ts"; export type { ApplyOptions, ApplyResult } from "./store.ts"; export { LiveResourceStore } from "./store.ts"; diff --git a/packages/krm-stream/src/sse.ts b/packages/krm-stream/src/sse.ts new file mode 100644 index 0000000..778ab01 --- /dev/null +++ b/packages/krm-stream/src/sse.ts @@ -0,0 +1,218 @@ +// The transport, consumer side. Everything else in this package is pure logic; this is the only file +// that knows a stream is made of bytes. +// +// Two ways in, because there are two authentication stories (spec §7) and neither is optional: +// +// connectResourceStream fetch-based. Needed for `Authorization: Bearer`, because native +// EventSource cannot send a custom header. Works in Node, so it is what +// the conformance suite drives. +// connectWithEventSource native EventSource. Needed for the same-origin session-cookie case, +// which is the BASELINE a v1 gateway must support. +// +// The rule both must obey, and the one that is easy to get wrong: on a TERMINAL error, close the +// connection. EventSource reconnects automatically otherwise, and will hammer a scope it can never +// be allowed to see — forever, from every open tab. + +import type { LiveResourceStore } from "./store.ts"; +import type { ErrorCode, Path, StreamEvent } from "./types.ts"; + +/** Incremental SSE parser. Bytes arrive in whatever chunks the network feels like — a frame can be + * split down the middle, and it WILL be, under exactly the load where you least want to debug it — + * so this buffers and only yields complete frames. */ +export class SSEDecoder { + #buffer = ""; + + /** Feed a chunk of the stream; get back the events that completed with it. */ + push(chunk: string): StreamEvent[] { + this.#buffer += chunk; + const out: StreamEvent[] = []; + + // Frames are separated by a blank line. Normalize the line endings first: an SSE line may end + // \n, \r\n or bare \r, and a proxy is entitled to rewrite them. + this.#buffer = this.#buffer.replace(/\r\n|\r/g, "\n"); + + for (;;) { + const sep = this.#buffer.indexOf("\n\n"); + if (sep === -1) break; // an incomplete frame stays in the buffer until the rest of it arrives + const frame = this.#buffer.slice(0, sep); + this.#buffer = this.#buffer.slice(sep + 2); + const ev = parseFrame(frame); + if (ev) out.push(ev); + } + return out; + } +} + +/** One SSE frame -> one event, or null for a frame that carries none (a heartbeat, a comment). + * + * A comment is not an error and not an event: it is how a heartbeat is invisible to a consumer while + * still keeping an idle proxy from closing the connection out from under a live status watch. */ +function parseFrame(frame: string): StreamEvent | null { + const data: string[] = []; + for (const line of frame.split("\n")) { + if (line === "" || line.startsWith(":")) continue; // comment / heartbeat + const colon = line.indexOf(":"); + const field = colon === -1 ? line : line.slice(0, colon); + // "data: x" and "data:x" are the same field; exactly one leading space is stripped. + let value = colon === -1 ? "" : line.slice(colon + 1); + if (value.startsWith(" ")) value = value.slice(1); + // `event:`, `id:` and `retry:` are SSE fields this protocol does not use (v1 emits no id: lines + // at all — §7). Ignoring them rather than failing is the same rule as ignoring an unknown event + // type: a minor addition must not break an older client. + if (field === "data") data.push(value); + } + if (data.length === 0) return null; + + try { + return JSON.parse(data.join("\n")) as StreamEvent; + } catch { + // A frame we cannot parse is not a reason to tear down a live stream. Skip it: the protocol is + // state-convergent, so the next snapshot cycle repairs whatever we missed. + return null; + } +} + +/** What a stream event does to a store. This is the consumer's half of the event table (spec §4), and + * it is exported because it IS the protocol — a host feeding a store from its own transport should + * not have to reimplement the switch and get `synced` subtly wrong. + * + * Returns the paths that flashed, so a UI can highlight them. */ +export function applyStreamEvent(store: LiveResourceStore, ev: StreamEvent): Path[] { + switch (ev.type) { + case "reset": + store.beginSnapshot(); + return []; + case "added": + case "modified": + if (!ev.object) return []; + return store.applyServerEvent(ev.object, { redactedPaths: ev.redactedPaths }).flashed; + case "deleted": + if (ev.identity?.uid) store.removeResource(ev.identity.uid); + return []; + case "synced": + store.endSnapshot(); + return []; + default: + // An unknown event type MUST be ignored, not treated as an error (spec §0). That is what lets + // the gateway add an optional event type later without breaking a browser nobody can update. + return []; + } +} + +export interface StreamOptions { + /** Called for every `error` event. A terminal one has already closed the connection by the time + * this returns — there is nothing to retry, and retrying is the bug. */ + onError?: (code: ErrorCode, message: string, terminal: boolean) => void; + /** Called at the end of every snapshot cycle. The store is now consistent: a good moment to paint. */ + onSynced?: () => void; + /** Called after any change, with the paths that flashed. */ + onChange?: (flashed: Path[]) => void; + /** Abort the stream from outside. */ + signal?: AbortSignal; + /** Injectable for tests. Defaults to the global fetch. */ + fetch?: typeof globalThis.fetch; + headers?: Record; +} + +export interface StreamHandle { + close(): void; + /** Resolves when the stream ends: closed, aborted, or terminated by a terminal error. */ + closed: Promise; +} + +/** Consume a resource stream over fetch, feeding a store. Use this when the gateway authenticates + * with a bearer token — native EventSource cannot send the header. */ +export function connectResourceStream(url: string, store: LiveResourceStore, opts: StreamOptions = {}): StreamHandle { + const controller = new AbortController(); + const fetchImpl = opts.fetch ?? globalThis.fetch; + if (opts.signal) opts.signal.addEventListener("abort", () => controller.abort(), { once: true }); + + const closed = (async () => { + const res = await fetchImpl(url, { + signal: controller.signal, + headers: { Accept: "text/event-stream", ...opts.headers }, + // The stream IS the response body; a cached one is a stream that never moves. + cache: "no-store", + }); + if (!res.ok || !res.body) { + opts.onError?.("INTERNAL", `stream: HTTP ${res.status}`, true); + return; + } + + const reader = res.body.pipeThrough(new TextDecoderStream()).getReader(); + const decoder = new SSEDecoder(); + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) return; + for (const ev of decoder.push(value)) { + if (feed(store, ev, opts)) { + controller.abort(); // terminal: stop, and do NOT come back + return; + } + } + } + } catch (err) { + if (!controller.signal.aborted) throw err; + } finally { + reader.cancel().catch(() => {}); + } + })(); + + return { + close: () => controller.abort(), + closed: closed.catch(() => {}), + }; +} + +/** Consume a resource stream with the browser's native EventSource. This is the same-origin + * session-cookie path — the baseline a v1 gateway MUST support, because a cookie is the only + * credential EventSource can carry. */ +export function connectWithEventSource( + url: string, + store: LiveResourceStore, + opts: Omit = {}, +): StreamHandle { + const es = new EventSource(url, { withCredentials: true }); + let resolve: () => void; + const closed = new Promise((r) => { + resolve = r; + }); + + const shut = () => { + es.close(); + resolve(); + }; + + es.onmessage = (e: MessageEvent) => { + let ev: StreamEvent; + try { + ev = JSON.parse(e.data) as StreamEvent; + } catch { + return; // an unparseable frame is not a reason to tear down a live stream + } + if (feed(store, ev, opts)) shut(); + }; + + // EventSource's `error` is also fired on a transport hiccup, where its OWN reconnect is the + // correct behaviour and we must not interfere. Only a terminal PROTOCOL error (which arrives as a + // message, above) closes the connection — and it must, or this reconnects forever. + es.onerror = () => { + if (es.readyState === EventSource.CLOSED) resolve(); + }; + + opts.signal?.addEventListener("abort", shut, { once: true }); + return { close: shut, closed }; +} + +/** Apply one event; returns true if the stream must now be closed. */ +function feed(store: LiveResourceStore, ev: StreamEvent, opts: StreamOptions): boolean { + if (ev.type === "error") { + opts.onError?.(ev.code ?? "INTERNAL", ev.message ?? "", ev.terminal ?? false); + return ev.terminal === true; + } + const flashed = applyStreamEvent(store, ev); + if (ev.type === "synced") opts.onSynced?.(); + opts.onChange?.(flashed); + return false; +} diff --git a/packages/krm-stream/test/expect.ts b/packages/krm-stream/test/expect.ts new file mode 100644 index 0000000..2417bfa --- /dev/null +++ b/packages/krm-stream/test/expect.ts @@ -0,0 +1,146 @@ +// The fixture assertions, shared by the two client suites. +// +// store.test.ts feeds the store the fixture's `events:` as OBJECTS. wire.test.ts feeds it the same +// events as BYTES — the golden SSE transcript the Go gateway actually wrote. They must assert +// exactly the same things, or the comparison between them proves nothing at all, which is why this +// file exists rather than a copy-paste. +// +// Assertion strengths, per conformance/README.md: +// uids, dirty, conflicts exact sets (order-insensitive) +// patch exact — it is what gets sent to the API server +// draftSubset deep subset (arrays element-wise, and the same length) +// absentPaths the path is absent from BOTH server(id) and draft(id) +// readOnlyPaths not editable, not dirty, and setValue on it is refused +// flashed containment: every listed path flashed. NOT an exact set — a read-only +// region that changed flashes, and metadata.resourceVersion changes on +// EVERY event, so an exact set would assert that a conformant engine does +// NOT flash a read-only field that moved. See docs §3. + +import assert from "node:assert/strict"; +import type { LiveResourceStore } from "../src/index.ts"; +import type { Path } from "../src/types.ts"; +import type { FixtureEdit, FixtureExpect } from "./conformance.ts"; + +/** `path` in a fixture edit always addresses the FIELD, not its container — even for the two ops + * whose store signature takes the map plus a key. Keeping the fixture format uniform is worth the one + * line of translation. */ +export function applyEdit(store: LiveResourceStore, e: FixtureEdit): void { + const parent = e.path.slice(0, -1); + const last = e.path[e.path.length - 1]; + switch (e.op) { + case "set": + store.setValue(e.uid, e.path, e.value); + break; + case "remove": + store.removeKey(e.uid, e.path); + break; + case "addKey": + store.addKey(e.uid, parent, String(last), e.value); + break; + case "renameKey": + store.renameKey(e.uid, parent, String(last), e.newKey!); + break; + case "revert": + store.revert(e.uid, e.path); + break; + default: + throw new Error(`fixture: unknown edit op ${JSON.stringify(e.op)}`); + } +} + +export function check(store: LiveResourceStore, flashed: Path[], want: FixtureExpect, where: string): void { + const ids = store.ids(); + + if (want.uids) assert.deepEqual(sorted(ids), sorted(want.uids), `${where}: uids`); + + // Dirtiness is derived, so ask for it the way a UI would: the changes of every resource the store + // holds. Union, because no fixture edits two objects and a stray dirty path anywhere is a bug. + if (want.dirty) { + const dirty = ids.flatMap((id) => store.changes(id).map((c) => c.path)); + assert.deepEqual(keys(dirty), keys(want.dirty), `${where}: dirty paths`); + // R-DERIVED, from the other direction: the predicate and the change list must agree. + for (const p of want.dirty) { + assert.ok( + ids.some((id) => store.isDirty(id, p)), + `${where}: isDirty(${key(p)}) disagrees with changes()`, + ); + } + } + + if (want.conflicts) { + const got = ids.flatMap((id) => store.conflicts(id)); + assert.deepEqual(keys(got.map((c) => c.path)), keys(want.conflicts.map(conflictPath)), `${where}: conflict paths`); + for (const c of want.conflicts) { + if (Array.isArray(c) || c.theirs === undefined) continue; // the fixture pins the path only + const found = got.find((g) => key(g.path) === key(c.path)); + assert.deepEqual(found?.theirs, c.theirs, `${where}: conflict theirs at ${key(c.path)}`); + } + } + + const subject = ids.length === 1 ? ids[0] : undefined; + + if (want.draftSubset) { + const drafts = subject ? [subject] : ids; + assert.ok( + drafts.some((id) => isSubset(store.draft(id) as unknown, want.draftSubset)), + `${where}: no draft matches draftSubset\nwant: ${JSON.stringify(want.draftSubset)}\ngot: ${JSON.stringify( + drafts.map((id) => store.draft(id)), + )}`, + ); + } + + for (const p of want.absentPaths ?? []) { + const id = subject ?? ids[0]!; + assert.equal(has(store.server(id), p), false, `${where}: ${key(p)} is a GHOST in server(${id})`); + assert.equal(has(store.draft(id), p), false, `${where}: ${key(p)} is a GHOST in draft(${id})`); + } + + for (const p of want.readOnlyPaths ?? []) { + const id = subject ?? ids[0]!; + assert.equal(store.isEditable(id, p), false, `${where}: ${key(p)} must be read-only`); + assert.equal(store.isDirty(id, p), false, `${where}: ${key(p)} is read-only and can never be dirty`); + assert.throws(() => store.setValue(id, p, "hunter2"), `${where}: a write to ${key(p)} must be refused`); + } + + for (const p of want.flashed ?? []) { + assert.ok(keys(flashed).includes(key(p)), `${where}: ${key(p)} changed on the server and must flash`); + } + + if (want.patch !== undefined) { + // No subject means no edits and more than one resource: then NOTHING may have a patch. + for (const id of subject ? [subject] : ids) { + assert.deepEqual(store.patch(id), want.patch, `${where}: patch(${id})`); + } + } +} + +/** Deep subset: every key the fixture names must match. Arrays are compared element-wise AND must be + * the same length — "the user's array is preserved intact" is exactly what array-atomic-on-change + * asserts, and a prefix match would not notice a server-appended sidecar leaking into the draft. */ +function isSubset(actual: unknown, want: unknown): boolean { + if (Array.isArray(want)) { + if (!Array.isArray(actual) || actual.length !== want.length) return false; + return want.every((w, i) => isSubset(actual[i], w)); + } + if (want && typeof want === "object") { + if (!actual || typeof actual !== "object" || Array.isArray(actual)) return false; + const a = actual as Record; + return Object.entries(want as Record).every(([k, v]) => k in a && isSubset(a[k], v)); + } + return Object.is(actual, want); +} + +function has(obj: unknown, path: Path): boolean { + let cur: unknown = obj; + for (const seg of path) { + if (cur === null || typeof cur !== "object") return false; + if (!(String(seg) in (cur as Record))) return false; + cur = (cur as Record)[String(seg)]; + } + return cur !== undefined; +} + +const conflictPath = (c: Path | { path: Path }): Path => (Array.isArray(c) ? c : c.path); +const key = (p: Path): string => JSON.stringify(p); +const keys = (ps: Path[]): string[] => ps.map(key).sort(); +const sorted = (xs: string[]): string[] => [...xs].sort(); diff --git a/packages/krm-stream/test/store.test.ts b/packages/krm-stream/test/store.test.ts index f9092bc..b9282fd 100644 --- a/packages/krm-stream/test/store.test.ts +++ b/packages/krm-stream/test/store.test.ts @@ -1,30 +1,22 @@ -// The client's half of the conformance corpus, actually asserted. +// The client's half of the conformance corpus, fed as OBJECTS. // -// conformance.test.ts checks that the corpus is well-formed. This file checks that the STORE is: -// every client fixture is replayed event by event into a LiveResourceStore, the fixture's edits are +// Every client fixture is replayed event by event into a LiveResourceStore, the fixture's edits are // applied at their `after:` positions, and what the store then holds is compared to `expect:`. // // The `after:` ordering is the whole point. An edit placed BEFORE the server change that collides // with it is the only way to test a three-way merge at all — a suite that edited only at the end // would pass against a store that just follows the server. // -// Assertion strengths, per conformance/README.md: -// uids, dirty, conflicts exact sets (order-insensitive) -// patch exact — it is what gets sent to the API server -// draftSubset deep subset (arrays compared element-wise, same length) -// absentPaths the path is absent from BOTH server(id) and draft(id) -// readOnlyPaths not editable, not dirty, and setValue on it is refused -// flashed containment: every listed path flashed. NOT an exact set — a read-only -// region that changed flashes, and metadata.resourceVersion changes on -// EVERY event, so an exact set would assert that a conformant engine -// does not flash a read-only field that moved. See docs §3 ("read-only -// is not ignored"). The fixtures list the paths a UI must highlight. +// Its sibling, wire.test.ts, runs the SAME assertions against the same fixtures delivered as the +// BYTES the Go gateway actually wrote. Together they are the seam: this file proves the store is +// right, that one proves the store is right about what the gateway really said. import assert from "node:assert/strict"; import { test } from "node:test"; -import { LiveResourceStore } from "../src/index.ts"; -import type { Path, StreamEvent } from "../src/types.ts"; -import { clientFixtures, type FixtureEdit, type FixtureExpect, resolve } from "./conformance.ts"; +import { applyStreamEvent, LiveResourceStore } from "../src/index.ts"; +import type { Path } from "../src/types.ts"; +import { clientFixtures, resolve } from "./conformance.ts"; +import { applyEdit, check } from "./expect.ts"; for (const f of clientFixtures()) { test(`${f.id}: ${f.title}`, () => { @@ -32,7 +24,10 @@ for (const f of clientFixtures()) { const flashed: Path[] = []; for (const [i, fe] of f.events.entries()) { - feed(store, resolve(f, fe), flashed); + // applyStreamEvent is library code, not test code: the switch from event to store call IS the + // protocol (spec §4), and a host feeding a store from its own transport must not have to + // reimplement it and get `synced` subtly wrong. + flashed.push(...applyStreamEvent(store, resolve(f, fe))); for (const edit of f.client?.edits ?? []) { if (edit.after === i) applyEdit(store, edit); @@ -46,163 +41,6 @@ for (const f of clientFixtures()) { }); } -/** The four protocol events a store cares about. `error` is a transport concern — the store holds - * state, not a connection — and an unknown type must be ignored, not thrown on (spec §0). */ -function feed(store: LiveResourceStore, ev: StreamEvent, flashed: Path[]): void { - switch (ev.type) { - case "reset": - store.beginSnapshot(); - break; - case "added": - case "modified": - flashed.push(...store.applyServerEvent(ev.object!, { redactedPaths: ev.redactedPaths }).flashed); - break; - case "deleted": - store.removeResource(ev.identity!.uid); - break; - case "synced": - store.endSnapshot(); - break; - default: - break; - } -} - -/** `path` in a fixture edit always addresses the FIELD, not its container — even for the two ops - * whose store signature takes the map plus a key. Keeping the fixture format uniform is worth the - * one line of translation here. */ -function applyEdit(store: LiveResourceStore, e: FixtureEdit): void { - const parent = e.path.slice(0, -1); - const last = e.path[e.path.length - 1]; - switch (e.op) { - case "set": - store.setValue(e.uid, e.path, e.value); - break; - case "remove": - store.removeKey(e.uid, e.path); - break; - case "addKey": - store.addKey(e.uid, parent, String(last), e.value); - break; - case "renameKey": - store.renameKey(e.uid, parent, String(last), e.newKey!); - break; - case "revert": - store.revert(e.uid, e.path); - break; - default: - throw new Error(`fixture: unknown edit op ${JSON.stringify(e.op)}`); - } -} - -function check(store: LiveResourceStore, flashed: Path[], want: FixtureExpect, where: string): void { - const ids = store.ids(); - - if (want.uids) assert.deepEqual(sorted(ids), sorted(want.uids), `${where}: uids`); - - // Dirtiness is derived, so we ask for it the way a UI would: the changes of every resource the - // store holds. Union, because no fixture edits two objects and a stray dirty path anywhere is a bug. - if (want.dirty) { - const dirty = ids.flatMap((id) => store.changes(id).map((c) => c.path)); - assert.deepEqual(keys(dirty), keys(want.dirty), `${where}: dirty paths`); - // R-DERIVED, from the other direction: the predicate and the change list must agree. - for (const p of want.dirty) { - assert.ok( - ids.some((id) => store.isDirty(id, p)), - `${where}: isDirty(${JSON.stringify(p)}) disagrees with changes()`, - ); - } - } - - if (want.conflicts) { - const got = ids.flatMap((id) => store.conflicts(id)); - assert.deepEqual(keys(got.map((c) => c.path)), keys(want.conflicts.map(conflictPath)), `${where}: conflict paths`); - for (const c of want.conflicts) { - if (Array.isArray(c) || c.theirs === undefined) continue; // the fixture pins the path only - const found = got.find((g) => key(g.path) === key(c.path)); - assert.deepEqual(found?.theirs, c.theirs, `${where}: conflict theirs at ${key(c.path)}`); - } - } - - const subject = subjectId(store); - - if (want.draftSubset) { - const drafts = subject ? [subject] : ids; - const ok = drafts.some((id) => isSubset(store.draft(id) as unknown, want.draftSubset)); - assert.ok( - ok, - `${where}: no draft matches draftSubset\nwant: ${JSON.stringify(want.draftSubset, null, 2)}\ngot: ${JSON.stringify( - drafts.map((id) => store.draft(id)), - null, - 2, - )}`, - ); - } - - for (const p of want.absentPaths ?? []) { - const id = subject ?? ids[0]!; - assert.equal(has(store.server(id), p), false, `${where}: ${key(p)} is a GHOST in server(${id})`); - assert.equal(has(store.draft(id), p), false, `${where}: ${key(p)} is a GHOST in draft(${id})`); - } - - for (const p of want.readOnlyPaths ?? []) { - const id = subject ?? ids[0]!; - assert.equal(store.isEditable(id, p), false, `${where}: ${key(p)} must be read-only`); - assert.equal(store.isDirty(id, p), false, `${where}: ${key(p)} is read-only and can never be dirty`); - assert.throws(() => store.setValue(id, p, "hunter2"), `${where}: a write to ${key(p)} must be refused`); - } - - for (const p of want.flashed ?? []) { - assert.ok(keys(flashed).includes(key(p)), `${where}: ${key(p)} changed on the server and must flash`); - } - - if (want.patch !== undefined) { - // No subject means no edits and more than one resource: then NOTHING may have a patch. - const targets = subject ? [subject] : ids; - for (const id of targets) assert.deepEqual(store.patch(id), want.patch, `${where}: patch(${id})`); - } -} - -/** The resource a per-object expectation (draftSubset, absentPaths, patch) is about. When the store - * holds exactly one, it is that one — which is every fixture that asserts a patch, including - * delete-recreate-uid, where the edited object no longer exists and the survivor is the subject. - * Otherwise there is no subject and the expectation must hold across the board. */ -function subjectId(store: LiveResourceStore): string | undefined { - const ids = store.ids(); - return ids.length === 1 ? ids[0] : undefined; -} - -/** Deep subset: every key the fixture names must match. Arrays are compared element-wise AND must - * be the same length — "the user's array is preserved intact" is exactly what array-atomic-on-change - * is asserting, and a prefix match would not see a server-appended sidecar leaking into the draft. */ -function isSubset(actual: unknown, want: unknown): boolean { - if (Array.isArray(want)) { - if (!Array.isArray(actual) || actual.length !== want.length) return false; - return want.every((w, i) => isSubset(actual[i], w)); - } - if (want && typeof want === "object") { - if (!actual || typeof actual !== "object" || Array.isArray(actual)) return false; - const a = actual as Record; - return Object.entries(want as Record).every(([k, v]) => k in a && isSubset(a[k], v)); - } - return Object.is(actual, want); -} - -function has(obj: unknown, path: Path): boolean { - let cur: unknown = obj; - for (const seg of path) { - if (cur === null || typeof cur !== "object") return false; - if (!(String(seg) in (cur as Record))) return false; - cur = (cur as Record)[String(seg)]; - } - return cur !== undefined; -} - -const conflictPath = (c: Path | { path: Path }): Path => (Array.isArray(c) ? c : c.path); -const key = (p: Path): string => JSON.stringify(p); -const keys = (ps: Path[]): string[] => ps.map(key).sort(); -const sorted = (xs: string[]): string[] => [...xs].sort(); - // A suite that silently runs zero fixtures is the failure mode this whole repo exists to prevent. test("the client suite actually ran the corpus", () => { assert.ok(clientFixtures().length >= 10, "the client half of the corpus is missing fixtures"); diff --git a/packages/krm-stream/test/wire.test.ts b/packages/krm-stream/test/wire.test.ts new file mode 100644 index 0000000..c883e77 --- /dev/null +++ b/packages/krm-stream/test/wire.test.ts @@ -0,0 +1,150 @@ +// The seam. This is the test that did not exist, and the reason "end to end" was a claim rather than +// a fact. +// +// Until now the two implementations shared a DOCUMENT: the Go suite asserted "given this watch I +// would emit these events", the TypeScript suite asserted "given these events I hold this state", and +// nothing anywhere checked that the events one side emits are bytes the other side can read. Two +// implementations agreeing to disagree is precisely what that gap permits. +// +// So this suite reads conformance/gen/sse/.sse — the bytes the Go gateway REALLY WROTE, through +// its real SSE sink, committed and staleness-checked — parses them with the real SSEDecoder, replays +// them into a real store, applies the fixture's edits at their `after:` positions, and asserts the +// same `expect:` that store.test.ts asserts against hand-built objects. +// +// If the gateway ever changes what it puts on the wire, this fails. If the client ever stops being +// able to read it, this fails. That is the whole job. + +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { test } from "node:test"; +import { applyStreamEvent, LiveResourceStore, SSEDecoder } from "../src/index.ts"; +import type { Path, StreamEvent } from "../src/types.ts"; +import { clientFixtures, resolve } from "./conformance.ts"; +import { applyEdit, check } from "./expect.ts"; + +const SSE = new URL("../../../conformance/gen/sse/", import.meta.url); + +/** The fixtures that exist on BOTH sides — the ones with a `watch:` script (so the gateway can + * produce a transcript) and a client `expect:` (so there is something to hold it to). */ +function wireFixtures() { + return clientFixtures().filter((f) => (f.suites ?? []).includes("gateway") && (f.watch?.length ?? 0) > 0); +} + +for (const f of wireFixtures()) { + test(`${f.id}: the client reads what the gateway wrote`, () => { + const bytes = readFileSync(new URL(`${f.id}.sse`, SSE), "utf8"); + + // Chunked at a hostile boundary: ONE BYTE AT A TIME. A frame split down the middle is not a + // hypothetical — it is what a real network does, under exactly the load where you least want to + // be debugging it — and a decoder that only works on whole frames passes every naive test and + // then corrupts the first busy stream it meets. + const events = decodeInChunks(bytes, 1); + + const store = new LiveResourceStore(); + const flashed: Path[] = []; + for (const [i, ev] of events.entries()) { + flashed.push(...applyStreamEvent(store, ev)); + for (const edit of f.client?.edits ?? []) { + if (edit.after === i) applyEdit(store, edit); + } + for (const cp of f.checkpoints ?? []) { + if (cp.after === i) check(store, flashed, cp, `${f.id} @checkpoint after=${i}`); + } + } + + if (f.client?.expect) check(store, flashed, f.client.expect, `${f.id} (from the wire)`); + }); +} + +test("the transcript on the wire IS the fixture's events — same order, same count", () => { + // The `after:` indices in a fixture address its `events:` list. If the gateway emitted a different + // number of events, every edit in every fixture would be applied at the wrong moment and the suite + // above would be quietly testing something else. The Go side asserts this too, byte for byte; this + // is the assertion from the other end of the wire, which is the only end that matters to a browser. + for (const f of wireFixtures()) { + const events = decodeInChunks(readFileSync(new URL(`${f.id}.sse`, SSE), "utf8"), 64); + assert.deepEqual( + events.map((e) => e.type), + f.events.map((e) => e.type), + `${f.id}: the wire and the fixture disagree about what was sent`, + ); + for (const [i, ev] of events.entries()) { + assert.deepEqual( + wireForm(ev), + wireForm(resolve(f, f.events[i]!)), + `${f.id} event ${i}: the wire is not the fixture`, + ); + if (ev.type === "error") { + // `terminal` must be PRESENT, not merely falsy. A consumer reading "do I give up, or do I + // retry?" out of a field that is not there is how a browser ends up reconnecting to a + // forbidden scope forever — EventSource retries on its own unless it is told not to. + assert.ok("terminal" in ev, `${f.id} event ${i}: an error must say whether it is terminal`); + assert.ok(ev.message, `${f.id} event ${i}: an error with no message is a diagnostic that says nothing`); + } + } + } +}); + +/** The event as the CORPUS specifies it — which is everything except an error's `message`. + * + * That one field is compared loosely, and not for convenience. `code` is the normative part: it is a + * stable, machine-readable set precisely because "a free-form message alone is not enough for stable + * client behavior" (spec §4.3), and the fixture format carries no `message` field at all, in either + * loader. Pinning the prose would assert a rule the contract does not make — and would push a gateway + * toward emitting nothing, which is worse for whoever debugs it at 3am. The Go suite draws the line + * in exactly the same place; if it ever moves, it moves on both sides. */ +function wireForm(ev: StreamEvent): StreamEvent { + if (ev.type !== "error") return ev; + const { message: _message, ...rest } = ev; + return rest as StreamEvent; +} + +test("a heartbeat is not an event, and neither is any other comment", () => { + // Spec §7: heartbeats are SSE comments, and a consumer ignores them. Get this wrong and every 20 + // seconds the store is handed a null event — or, worse, the parser throws and a live status watch + // dies quietly on an idle connection. + const d = new SSEDecoder(); + const events = d.push( + ": heartbeat\n\n" + + ": connection 1\n\n" + + '\ndata: {"type":"reset"}\n\n' + + ": heartbeat\n\n" + + 'data: {"type":"synced"}\n\n', + ); + assert.deepEqual( + events.map((e) => e.type), + ["reset", "synced"], + ); +}); + +test("an unknown event type and an unknown field are ignored, not fatal", () => { + // Spec §0: a minor addition to v1 — a new optional event type, a new optional envelope field — + // must not break an older client. This is the rule that lets the protocol grow at all. + const store = new LiveResourceStore(); + const events = new SSEDecoder().push( + 'data: {"type":"reset","somethingNew":true}\n\n' + + 'data: {"type":"future-event","payload":42}\n\n' + + 'data: {"type":"synced"}\n\n', + ); + assert.equal(events.length, 3); + for (const ev of events) applyStreamEvent(store, ev); + assert.deepEqual(store.ids(), [], "the store survived an event type it has never heard of"); +}); + +test("a truncated stream yields no half-parsed event", () => { + // The connection died mid-frame. The right answer is silence — NOT a partial object, which the + // store would apply as a REPLACEMENT and blank the resource out on screen. + const d = new SSEDecoder(); + assert.deepEqual(d.push('data: {"type":"added","object":{"apiV'), []); + assert.deepEqual(d.push(""), []); +}); + +/** Decode the whole transcript, feeding it `size` characters at a time. */ +function decodeInChunks(bytes: string, size: number): StreamEvent[] { + const d = new SSEDecoder(); + const out: StreamEvent[] = []; + for (let i = 0; i < bytes.length; i += size) { + out.push(...d.push(bytes.slice(i, i + size))); + } + return out; +} diff --git a/packages/krm-stream/tsconfig.test.json b/packages/krm-stream/tsconfig.test.json index 57469f8..64dc6d0 100644 --- a/packages/krm-stream/tsconfig.test.json +++ b/packages/krm-stream/tsconfig.test.json @@ -12,5 +12,5 @@ // node:test / node:assert — the only reason @types/node is a devDependency. It ships nothing. "types": ["node"] }, - "include": ["src/**/*.ts", "test/**/*.ts"] + "include": ["src/**/*.ts", "test/**/*.ts", "e2e/**/*.ts"] } From 360bfa18c426c7224332aa4e7c9a3e8abef2d587 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 21:23:05 +0000 Subject: [PATCH 05/43] =?UTF-8?q?feat(conformance):=20three=20watch=20ops?= =?UTF-8?q?=20Kubernetes=20really=20sends=20=E2=80=94=20and=20the=20two=20?= =?UTF-8?q?bugs=20they=20found?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proposal 0001 (docs/proposals/0001-watch-ops.md), grounded in a full read of the Kubernetes API concepts page, summarized with citations in docs/facts/kubernetes-api-concepts.md. That file exists because spec/v1.md and gateway/README.md made claims about the watch API that were written from memory, and two of them were wrong. The corpus could not express three of the gateway's own MUST NOTs — proved by mutation: "emit synced on every bookmark" and "forward a partial object" both left all 11 fixtures green. So `watch:` grows three ops, each a documented Kubernetes behaviour: bookmark a routine BOOKMARK. Kubernetes, verbatim: its object "only includes a .metadata.resourceVersion field". An object with no uid, no name, no spec and no status is not an edge case — it is on EVERY stream that asked for bookmarks, and we must ask, because that is how the snapshot boundary arrives at all. partial PartialObjectMetadata delivered as an upsert. tombstone a DELETED that lost its identity (DeletedFinalStateUnknown). Two real bugs fell out: 1. resourceVersion OVERFLOW. Kubernetes: "compared as arbitrary bitsize decimal integers... the bitsize must not be assumed"; its own example is 40 digits. We compared with strconv.ParseInt — int64, 19 digits. On such a cluster the parse fails, the monotonicity check gives up, and live updates are SILENTLY DROPPED — a symptom indistinguishable from "Kubernetes is slow", which is how a bug like this survives for years. Now compared as the docs prescribe: longer-is-greater, then lexicographic. A plain lexicographic compare is also wrong ("9" > "10"), and an extension API server's non-decimal resourceVersion cannot be ordered at all — so it now drops nothing rather than guessing. 2. The partial-object guard checked the WRONG FIELD. It asked "does it have a uid?" — but a PartialObjectMetadata has a complete metadata block, uid included; what it lacks is spec and status. The guard waved it through, and a consumer whose model is REPLACE would have swapped a live Deployment for a husk: status blank, the user's spec gone. The honest test is the kind. spec §6's non-normative note has been corrected — it said "monotonically increasing integer", which invites exactly the ParseInt that bit us. The normative rule (resourceVersion is opaque to consumers) is unchanged, and no wire change: all three ops describe things the gateway must absorb or refuse, so the correct behaviour emits FEWER events, never different ones. Four new fixtures. All four catch their mutation; the corpus can now go red for every rule it claims to defend. Co-Authored-By: Claude Opus 4.8 (1M context) --- conformance/README.md | 29 ++- conformance/bodies/cm-app.v6-bignum.yaml | 21 ++ .../bodies/cm-app.v7-bignum-newer.yaml | 17 ++ conformance/fixtures/bookmark-absorbed.yaml | 38 +++ .../fixtures/partial-object-refused.yaml | 35 +++ .../fixtures/resourceversion-bignum.yaml | 36 +++ .../fixtures/tombstone-without-uid.yaml | 32 +++ conformance/gen/bodies.json | 40 +++ conformance/gen/fixtures.json | 224 +++++++++++++++++ conformance/gen/sse/bookmark-absorbed.sse | 10 + .../gen/sse/partial-object-refused.sse | 16 ++ .../gen/sse/resourceversion-bignum.sse | 10 + conformance/gen/sse/tombstone-without-uid.sse | 18 ++ docs/facts/kubernetes-api-concepts.md | 231 ++++++++++++++++++ docs/proposals/0001-watch-ops.md | 126 ++++++++++ gateway/conformance.go | 13 + gateway/scripted.go | 77 ++++++ gateway/stream.go | 116 +++++++-- gateway/stream_test.go | 56 +++++ spec/v1.md | 24 +- 20 files changed, 1143 insertions(+), 26 deletions(-) create mode 100644 conformance/bodies/cm-app.v6-bignum.yaml create mode 100644 conformance/bodies/cm-app.v7-bignum-newer.yaml create mode 100644 conformance/fixtures/bookmark-absorbed.yaml create mode 100644 conformance/fixtures/partial-object-refused.yaml create mode 100644 conformance/fixtures/resourceversion-bignum.yaml create mode 100644 conformance/fixtures/tombstone-without-uid.yaml create mode 100644 conformance/gen/sse/bookmark-absorbed.sse create mode 100644 conformance/gen/sse/partial-object-refused.sse create mode 100644 conformance/gen/sse/resourceversion-bignum.sse create mode 100644 conformance/gen/sse/tombstone-without-uid.sse create mode 100644 docs/facts/kubernetes-api-concepts.md create mode 100644 docs/proposals/0001-watch-ops.md diff --git a/conformance/README.md b/conformance/README.md index 1d3f911..b94776b 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -54,7 +54,7 @@ suites: [gateway, client] scope: { target: demo, version: v1, resource: configmaps, namespace: app } projection: krm-editor/v1 -watch: # ops: list | added | modified | deleted | relist | disconnect +watch: # see "The watch ops" below - { op: list, bodies: [cm-app.v1] } - { op: modified, body: cm-app.v2 } @@ -82,6 +82,29 @@ three-way merge, and a fixture format that couldn't express it would be useless. opaque string; making it legible costs nothing and makes a delete-and-recreate fixture (§`uid` changes, name does not) obvious at a glance. +## The watch ops + +`watch:` is a scripted Kubernetes watch — the gateway's input. Every op is something a real API server +really does; where that is not obvious, the reference is +[docs/facts/kubernetes-api-concepts.md](../docs/facts/kubernetes-api-concepts.md), which is a reading +of the [API concepts page](https://kubernetes.io/docs/reference/using-api/api-concepts/) rather than a +reading of anyone's memory. That distinction has already cost us two bugs. + +| op | means | the gateway must | +|---|---|---| +| `list` | the objects currently in scope, then the bookmark that ends the initial events | open a cycle: `reset`, one `added` each, `synced` | +| `added` / `modified` | an upsert | forward it (subject to monotonicity) | +| `deleted` | the object left scope | emit `deleted` with its identity | +| `relist` | **upstream** continuity lost (410 Gone / cache reset) — the SSE connection is fine | announce `RESYNC_REQUIRED`, then a fresh cycle **on the same connection** | +| `disconnect` | the **browser's** connection dropped | nothing — the next connection is a new stream | +| `bookmark` | a routine `BOOKMARK`. Its object carries **only** `metadata.resourceVersion` — that is what the API server sends, on every stream that asked for bookmarks | **absorb it.** Never forward it; never mistake it for `synced` | +| `partial` | a metadata-only object (`PartialObjectMetadata`) delivered as an upsert. **It has a uid**; it has no `spec` and no `status` | **refuse it** and resnapshot. Forwarding it blanks the consumer's object | +| `tombstone` | a `DELETED` whose object lost its identity (client-go's `DeletedFinalStateUnknown`) | **not guess.** Begin a new cycle and let `synced` prune | + +The last three were added by [proposal 0001](../docs/proposals/0001-watch-ops.md), because the corpus +could not express three of the gateway's own MUST NOTs — and a mutation test proved it: emitting +`synced` on every bookmark, and forwarding a partial object, both left every fixture green. + **Assertions.** `dirty` and `conflicts` are compared as exact sets (order-insensitive). `patch` is compared exactly — it is what gets sent to the API server, so "close enough" is not a thing. `draftSubset` is a deep-subset check: it lets a fixture assert the two fields it cares about without @@ -106,6 +129,10 @@ Every fixture names the rule it defends, in `why:`. The ones that catch real bug | `dotted-label-keys` | **R-ID** — `app.kubernetes.io/name` is ONE path segment. Dot-joining it is silently wrong | | `array-atomic-on-change` | arrays merge atomically when lengths change (engine spec §4.1); a positional merge mis-aligns | | `secret-redaction` | a redacted value is never displayed, never dirty, and can never be written back over the real one | +| `bookmark-absorbed` | a routine `BOOKMARK` is absorbed. Forward its object and you replace a live resource with a husk that has only a `resourceVersion` | +| `partial-object-refused` | a `PartialObjectMetadata` **has a uid** — so "has a uid" was never a sufficient test for "is a complete object", and ours was wrong | +| `tombstone-without-uid` | identity is never *reconstructed*. A guessed uid deletes the wrong object out of somebody's browser | +| `resourceversion-bignum` | a `resourceVersion` is an **arbitrary-bitsize** decimal (Kubernetes' own example is 40 digits). `strconv.ParseInt` overflows it, and the symptom is silently dropped live updates | ## Adding a fixture diff --git a/conformance/bodies/cm-app.v6-bignum.yaml b/conformance/bodies/cm-app.v6-bignum.yaml new file mode 100644 index 0000000..203ee50 --- /dev/null +++ b/conformance/bodies/cm-app.v6-bignum.yaml @@ -0,0 +1,21 @@ +# A resourceVersion that does NOT fit in an int64. This is not contrived: Kubernetes documents +# resource versions as "arbitrary bitsize decimal integers" and its own worked example is 40 digits +# long (docs/facts/kubernetes-api-concepts.md §2). +# +# strconv.ParseInt tops out at 19 digits. The gateway used to compare with it, so on a cluster like +# this one the parse failed, the monotonicity check gave up, and live updates were silently dropped — +# which in a status view is indistinguishable from "the cluster is being slow". +apiVersion: v1 +kind: ConfigMap +metadata: + uid: cm-app-0001 + name: app-config + namespace: app + resourceVersion: "2345678901234567890123456789012345678901" + labels: + app.kubernetes.io/name: shop + annotations: + note: keep +data: + log-level: info + replicas: "3" diff --git a/conformance/bodies/cm-app.v7-bignum-newer.yaml b/conformance/bodies/cm-app.v7-bignum-newer.yaml new file mode 100644 index 0000000..54cfebb --- /dev/null +++ b/conformance/bodies/cm-app.v7-bignum-newer.yaml @@ -0,0 +1,17 @@ +# One digit longer than v6-bignum, and therefore GREATER: "if they are not of equal length, the longer +# one is greater" — the comparison Kubernetes prescribes. A lexicographic compare that forgot the +# length rule would call this OLDER and drop it, and the user would never see the change. +apiVersion: v1 +kind: ConfigMap +metadata: + uid: cm-app-0001 + name: app-config + namespace: app + resourceVersion: "12345678901234567890123456789012345678901" + labels: + app.kubernetes.io/name: shop + annotations: + note: keep +data: + log-level: warn + replicas: "3" diff --git a/conformance/fixtures/bookmark-absorbed.yaml b/conformance/fixtures/bookmark-absorbed.yaml new file mode 100644 index 0000000..d4992e3 --- /dev/null +++ b/conformance/fixtures/bookmark-absorbed.yaml @@ -0,0 +1,38 @@ +id: bookmark-absorbed +title: A routine watch BOOKMARK is absorbed — never forwarded, never mistaken for `synced`. +why: > + Kubernetes, verbatim: "The document representing the BOOKMARK event is of the type requested by the + request, but only includes a .metadata.resourceVersion field." So an object with no uid, no name, no + spec and no status is not an exotic case someone contrived — it is on EVERY conforming watch stream + that asked for bookmarks, and the gateway must ask (allowWatchBookmarks=true is how the snapshot + boundary arrives at all). + + Two ways to get this wrong, and this fixture fails on both: + + - forward the bookmark's object as `modified`. The consumer REPLACES on modified — that is the + protocol's single most important rule — so it would replace a live ConfigMap with a husk that + has only a resourceVersion. The screen goes blank. + - treat every bookmark as the snapshot boundary and emit `synced`. Pruning is gated on `synced`, + so a mid-cycle `synced` prunes objects the snapshot had not reached yet: half the user's + resources vanish, and reappear on the next relist. + + Only the bookmark that TERMINATES the initial events is `synced` (spec §5, gateway §3a). Every other + one is absorbed for its resourceVersion and never spoken of again (spec §2). + + See docs/facts/kubernetes-api-concepts.md §1.1 — this rule is a quote from the API docs, not an + opinion about them. +suites: [gateway] +scope: { target: demo, version: v1, resource: configmaps, namespace: app } +projection: krm-editor/v1 + +watch: + - { op: list, bodies: [cm-app.v1] } + - { op: bookmark, resourceVersion: "1500" } # routine: absorbed, produces NOTHING + - { op: modified, body: cm-app.v2 } + - { op: bookmark, resourceVersion: "1600" } # ...and again + +events: + - { type: reset } + - { type: added, body: cm-app.v1 } + - { type: synced } + - { type: modified, body: cm-app.v2 } diff --git a/conformance/fixtures/partial-object-refused.yaml b/conformance/fixtures/partial-object-refused.yaml new file mode 100644 index 0000000..4e6f114 --- /dev/null +++ b/conformance/fixtures/partial-object-refused.yaml @@ -0,0 +1,35 @@ +id: partial-object-refused +title: A metadata-only object is never forwarded as an upsert — the gateway resnapshots instead. +why: > + Kubernetes serves PartialObjectMetadata on request (Accept: application/json;as=PartialObjectMetadata): + "the returned objects only contain the `metadata` field. The `spec` and `status` fields are omitted." + + This fixture exists because it caught a real mistake in our own gateway. The guard was "an object + with no uid is partial" — but a PartialObjectMetadata HAS a uid. It has a whole metadata block. What + it does not have is a spec or a status. So the guard looked at the wrong field, the object sailed + through, and the consumer — whose model is REPLACE, never merge — swapped a live Deployment for a + husk: the status view goes blank and the editor silently loses the user's spec. + + The honest check is the KIND (PartialObjectMetadata / PartialObjectMetadataList, group meta.k8s.io), + not the presence of a uid. A partial object delivered as an update would blank a consumer's state — + spec §2 says so, and it is right; we were simply not enforcing what we had written down. + + The recovery is a new snapshot cycle: it is the one response that is ALWAYS correct, because it + re-establishes the truth rather than guessing at it. +suites: [gateway] +scope: { target: demo, group: apps, version: v1, resource: deployments, namespace: app } +projection: krm-editor/v1 + +watch: + - { op: list, bodies: [deploy-web.v1] } + - { op: partial, body: deploy-web.v1 } # metadata only: uid present, spec and status GONE + - { op: list, bodies: [deploy-web.v2] } # the resnapshot the gateway must perform + +events: + - { type: reset } + - { type: added, body: deploy-web.v1 } + - { type: synced } + - { type: error, code: RESYNC_REQUIRED, terminal: false } + - { type: reset } + - { type: added, body: deploy-web.v2 } + - { type: synced } diff --git a/conformance/fixtures/resourceversion-bignum.yaml b/conformance/fixtures/resourceversion-bignum.yaml new file mode 100644 index 0000000..e922412 --- /dev/null +++ b/conformance/fixtures/resourceversion-bignum.yaml @@ -0,0 +1,36 @@ +id: resourceversion-bignum +title: A 40-digit resourceVersion orders correctly — and a stale replay is still dropped. +why: > + Kubernetes: "Resource versions are compared as arbitrary bitsize decimal integers... The bitsize + must not be assumed to be some fixed amount." Its own worked example is 40 digits long. + + strconv.ParseInt tops out at 19. The gateway used to compare resource versions with it, so against a + cluster like this one the parse simply failed — and the per-object monotonicity check (spec §6) gave + up. The user-visible symptom is silently dropped live updates, which in a status view is + indistinguishable from "Kubernetes is being slow", and is therefore the kind of bug that survives for + years. + + The comparison Kubernetes prescribes: "If they are not of equal length, the longer one is greater + (for example, "123" > "23"). If they are of equal length, the lexicographically greater one is + greater." Note what that rules out: a plain lexicographic compare, which would call the 41-digit + version OLDER than the 40-digit one and drop it. + + So this fixture asserts both halves at once: + - the 41-digit v7 is NEWER than the 40-digit v6 and must be delivered; + - replaying v6 afterwards is STALE and must be dropped (§6: never emit, within a cycle, a state + older than one already emitted for that uid). +suites: [gateway] +scope: { target: demo, version: v1, resource: configmaps, namespace: app } +projection: krm-editor/v1 + +watch: + - { op: list, bodies: [cm-app.v6-bignum] } + - { op: modified, body: cm-app.v7-bignum-newer } # 41 digits: LONGER, therefore newer + - { op: modified, body: cm-app.v6-bignum } # a replay of the older state: must be DROPPED + +events: + - { type: reset } + - { type: added, body: cm-app.v6-bignum } + - { type: synced } + - { type: modified, body: cm-app.v7-bignum-newer } + # ...and nothing more: the stale replay never reaches the wire. diff --git a/conformance/fixtures/tombstone-without-uid.yaml b/conformance/fixtures/tombstone-without-uid.yaml new file mode 100644 index 0000000..40f8519 --- /dev/null +++ b/conformance/fixtures/tombstone-without-uid.yaml @@ -0,0 +1,32 @@ +id: tombstone-without-uid +title: An ambiguous tombstone is never emitted as `deleted` — a new cycle prunes instead. +why: > + spec §4.2, and it is stated in strong terms for a reason: if the gateway cannot recover a trustworthy + uid it MUST NOT emit an ambiguous `deleted`. It begins a new snapshot cycle and lets reset…synced do + the pruning. + + The source of a degenerate tombstone is client-go's cache.DeletedFinalStateUnknown: the informer + missed the delete and noticed during a relist, so it hands you a wrapper whose object may be nothing + more than a key. A gateway that "helpfully" reconstructs a uid from the name deletes THE WRONG + OBJECT out of somebody's browser — and a delete-and-recreate under the same name (see + delete-recreate-uid) is exactly the case where the name is a lie. + + Paying for a relist is the cheap option here. Guessing is not. +suites: [gateway] +scope: { target: demo, version: v1, resource: configmaps, namespace: app } +projection: krm-editor/v1 + +watch: + - { op: list, bodies: [cm-app.v1, cm-flags.v1] } + - { op: tombstone, body: cm-flags.v1 } # DELETED, but the object lost its identity + - { op: list, bodies: [cm-app.v1] } # the resnapshot — and cm-flags is genuinely gone + +events: + - { type: reset } + - { type: added, body: cm-app.v1 } + - { type: added, body: cm-flags.v1 } + - { type: synced } + - { type: error, code: RESYNC_REQUIRED, terminal: false } + - { type: reset } + - { type: added, body: cm-app.v1 } + - { type: synced } # cm-flags-0001 is pruned HERE, correctly, and by identity diff --git a/conformance/gen/bodies.json b/conformance/gen/bodies.json index bef0174..56cc14d 100644 --- a/conformance/gen/bodies.json +++ b/conformance/gen/bodies.json @@ -111,6 +111,46 @@ "log-level": "info" } }, + "cm-app.v6-bignum": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "uid": "cm-app-0001", + "name": "app-config", + "namespace": "app", + "resourceVersion": "2345678901234567890123456789012345678901", + "labels": { + "app.kubernetes.io/name": "shop" + }, + "annotations": { + "note": "keep" + } + }, + "data": { + "log-level": "info", + "replicas": "3" + } + }, + "cm-app.v7-bignum-newer": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "uid": "cm-app-0001", + "name": "app-config", + "namespace": "app", + "resourceVersion": "12345678901234567890123456789012345678901", + "labels": { + "app.kubernetes.io/name": "shop" + }, + "annotations": { + "note": "keep" + } + }, + "data": { + "log-level": "warn", + "replicas": "3" + } + }, "cm-flags.v1": { "apiVersion": "v1", "kind": "ConfigMap", diff --git a/conformance/gen/fixtures.json b/conformance/gen/fixtures.json index 84a6b0a..737d391 100644 --- a/conformance/gen/fixtures.json +++ b/conformance/gen/fixtures.json @@ -80,6 +80,57 @@ } } }, + { + "id": "bookmark-absorbed", + "title": "A routine watch BOOKMARK is absorbed — never forwarded, never mistaken for `synced`.", + "why": "Kubernetes, verbatim: \"The document representing the BOOKMARK event is of the type requested by the request, but only includes a .metadata.resourceVersion field.\" So an object with no uid, no name, no spec and no status is not an exotic case someone contrived — it is on EVERY conforming watch stream that asked for bookmarks, and the gateway must ask (allowWatchBookmarks=true is how the snapshot boundary arrives at all).\nTwo ways to get this wrong, and this fixture fails on both:\n\n - forward the bookmark's object as `modified`. The consumer REPLACES on modified — that is the\n protocol's single most important rule — so it would replace a live ConfigMap with a husk that\n has only a resourceVersion. The screen goes blank.\n - treat every bookmark as the snapshot boundary and emit `synced`. Pruning is gated on `synced`,\n so a mid-cycle `synced` prunes objects the snapshot had not reached yet: half the user's\n resources vanish, and reappear on the next relist.\n\nOnly the bookmark that TERMINATES the initial events is `synced` (spec §5, gateway §3a). Every other one is absorbed for its resourceVersion and never spoken of again (spec §2).\nSee docs/facts/kubernetes-api-concepts.md §1.1 — this rule is a quote from the API docs, not an opinion about them.\n", + "suites": [ + "gateway" + ], + "scope": { + "target": "demo", + "version": "v1", + "resource": "configmaps", + "namespace": "app" + }, + "projection": "krm-editor/v1", + "watch": [ + { + "op": "list", + "bodies": [ + "cm-app.v1" + ] + }, + { + "op": "bookmark", + "resourceVersion": "1500" + }, + { + "op": "modified", + "body": "cm-app.v2" + }, + { + "op": "bookmark", + "resourceVersion": "1600" + } + ], + "events": [ + { + "type": "reset" + }, + { + "type": "added", + "body": "cm-app.v1" + }, + { + "type": "synced" + }, + { + "type": "modified", + "body": "cm-app.v2" + } + ] + }, { "id": "conflict-and-converge", "title": "A real conflict is raised, kept, and then clears itself when the server converges.", @@ -637,6 +688,67 @@ } } }, + { + "id": "partial-object-refused", + "title": "A metadata-only object is never forwarded as an upsert — the gateway resnapshots instead.", + "why": "Kubernetes serves PartialObjectMetadata on request (Accept: application/json;as=PartialObjectMetadata): \"the returned objects only contain the `metadata` field. The `spec` and `status` fields are omitted.\"\nThis fixture exists because it caught a real mistake in our own gateway. The guard was \"an object with no uid is partial\" — but a PartialObjectMetadata HAS a uid. It has a whole metadata block. What it does not have is a spec or a status. So the guard looked at the wrong field, the object sailed through, and the consumer — whose model is REPLACE, never merge — swapped a live Deployment for a husk: the status view goes blank and the editor silently loses the user's spec.\nThe honest check is the KIND (PartialObjectMetadata / PartialObjectMetadataList, group meta.k8s.io), not the presence of a uid. A partial object delivered as an update would blank a consumer's state — spec §2 says so, and it is right; we were simply not enforcing what we had written down.\nThe recovery is a new snapshot cycle: it is the one response that is ALWAYS correct, because it re-establishes the truth rather than guessing at it.\n", + "suites": [ + "gateway" + ], + "scope": { + "target": "demo", + "group": "apps", + "version": "v1", + "resource": "deployments", + "namespace": "app" + }, + "projection": "krm-editor/v1", + "watch": [ + { + "op": "list", + "bodies": [ + "deploy-web.v1" + ] + }, + { + "op": "partial", + "body": "deploy-web.v1" + }, + { + "op": "list", + "bodies": [ + "deploy-web.v2" + ] + } + ], + "events": [ + { + "type": "reset" + }, + { + "type": "added", + "body": "deploy-web.v1" + }, + { + "type": "synced" + }, + { + "type": "error", + "code": "RESYNC_REQUIRED", + "terminal": false + }, + { + "type": "reset" + }, + { + "type": "added", + "body": "deploy-web.v2" + }, + { + "type": "synced" + } + ] + }, { "id": "reconnect-prune", "title": "An object deleted while the consumer was disconnected is pruned on the next synced.", @@ -707,6 +819,53 @@ } } }, + { + "id": "resourceversion-bignum", + "title": "A 40-digit resourceVersion orders correctly — and a stale replay is still dropped.", + "why": "Kubernetes: \"Resource versions are compared as arbitrary bitsize decimal integers... The bitsize must not be assumed to be some fixed amount.\" Its own worked example is 40 digits long.\nstrconv.ParseInt tops out at 19. The gateway used to compare resource versions with it, so against a cluster like this one the parse simply failed — and the per-object monotonicity check (spec §6) gave up. The user-visible symptom is silently dropped live updates, which in a status view is indistinguishable from \"Kubernetes is being slow\", and is therefore the kind of bug that survives for years.\nThe comparison Kubernetes prescribes: \"If they are not of equal length, the longer one is greater (for example, \"123\" > \"23\"). If they are of equal length, the lexicographically greater one is greater.\" Note what that rules out: a plain lexicographic compare, which would call the 41-digit version OLDER than the 40-digit one and drop it.\nSo this fixture asserts both halves at once:\n - the 41-digit v7 is NEWER than the 40-digit v6 and must be delivered;\n - replaying v6 afterwards is STALE and must be dropped (§6: never emit, within a cycle, a state\n older than one already emitted for that uid).\n", + "suites": [ + "gateway" + ], + "scope": { + "target": "demo", + "version": "v1", + "resource": "configmaps", + "namespace": "app" + }, + "projection": "krm-editor/v1", + "watch": [ + { + "op": "list", + "bodies": [ + "cm-app.v6-bignum" + ] + }, + { + "op": "modified", + "body": "cm-app.v7-bignum-newer" + }, + { + "op": "modified", + "body": "cm-app.v6-bignum" + } + ], + "events": [ + { + "type": "reset" + }, + { + "type": "added", + "body": "cm-app.v6-bignum" + }, + { + "type": "synced" + }, + { + "type": "modified", + "body": "cm-app.v7-bignum-newer" + } + ] + }, { "id": "resync-midstream", "title": "Upstream continuity can be lost WITHOUT the SSE connection dropping — a fresh cycle follows.", @@ -1038,5 +1197,70 @@ ] } } + }, + { + "id": "tombstone-without-uid", + "title": "An ambiguous tombstone is never emitted as `deleted` — a new cycle prunes instead.", + "why": "spec §4.2, and it is stated in strong terms for a reason: if the gateway cannot recover a trustworthy uid it MUST NOT emit an ambiguous `deleted`. It begins a new snapshot cycle and lets reset…synced do the pruning.\nThe source of a degenerate tombstone is client-go's cache.DeletedFinalStateUnknown: the informer missed the delete and noticed during a relist, so it hands you a wrapper whose object may be nothing more than a key. A gateway that \"helpfully\" reconstructs a uid from the name deletes THE WRONG OBJECT out of somebody's browser — and a delete-and-recreate under the same name (see delete-recreate-uid) is exactly the case where the name is a lie.\nPaying for a relist is the cheap option here. Guessing is not.\n", + "suites": [ + "gateway" + ], + "scope": { + "target": "demo", + "version": "v1", + "resource": "configmaps", + "namespace": "app" + }, + "projection": "krm-editor/v1", + "watch": [ + { + "op": "list", + "bodies": [ + "cm-app.v1", + "cm-flags.v1" + ] + }, + { + "op": "tombstone", + "body": "cm-flags.v1" + }, + { + "op": "list", + "bodies": [ + "cm-app.v1" + ] + } + ], + "events": [ + { + "type": "reset" + }, + { + "type": "added", + "body": "cm-app.v1" + }, + { + "type": "added", + "body": "cm-flags.v1" + }, + { + "type": "synced" + }, + { + "type": "error", + "code": "RESYNC_REQUIRED", + "terminal": false + }, + { + "type": "reset" + }, + { + "type": "added", + "body": "cm-app.v1" + }, + { + "type": "synced" + } + ] } ] diff --git a/conformance/gen/sse/bookmark-absorbed.sse b/conformance/gen/sse/bookmark-absorbed.sse new file mode 100644 index 0000000..8bd7d3f --- /dev/null +++ b/conformance/gen/sse/bookmark-absorbed.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"5"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1002","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/partial-object-refused.sse b/conformance/gen/sse/partial-object-refused.sse new file mode 100644 index 0000000..a797d4f --- /dev/null +++ b/conformance/gen/sse/partial-object-refused.sse @@ -0,0 +1,16 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","group":"apps","version":"v1","resource":"deployments","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":4,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3001","uid":"deploy-web-0001"},"spec":{"paused":false,"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasUnavailable","status":"False","type":"Available"}],"observedGeneration":4,"readyReplicas":1,"replicas":2}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"error","code":"RESYNC_REQUIRED","message":"upstream continuity lost; a new snapshot cycle follows","terminal":false} + +data: {"type":"reset","target":"demo","scope":{"target":"demo","group":"apps","version":"v1","resource":"deployments","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"generation":4,"labels":{"app.kubernetes.io/name":"web"},"name":"web","namespace":"app","resourceVersion":"3002","uid":"deploy-web-0001"},"spec":{"paused":false,"replicas":2,"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},"spec":{"containers":[{"image":"ghcr.io/configbutler/web:v1.2.3","name":"web","ports":[{"containerPort":8080}]}]}}},"status":{"conditions":[{"reason":"MinimumReplicasAvailable","status":"True","type":"Available"}],"observedGeneration":4,"readyReplicas":2,"replicas":2}},"redactedPaths":[]} + +data: {"type":"synced"} + diff --git a/conformance/gen/sse/resourceversion-bignum.sse b/conformance/gen/sse/resourceversion-bignum.sse new file mode 100644 index 0000000..307375e --- /dev/null +++ b/conformance/gen/sse/resourceversion-bignum.sse @@ -0,0 +1,10 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"2345678901234567890123456789012345678901","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"modified","object":{"apiVersion":"v1","data":{"log-level":"warn","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"12345678901234567890123456789012345678901","uid":"cm-app-0001"}},"redactedPaths":[]} + diff --git a/conformance/gen/sse/tombstone-without-uid.sse b/conformance/gen/sse/tombstone-without-uid.sse new file mode 100644 index 0000000..c6a943e --- /dev/null +++ b/conformance/gen/sse/tombstone-without-uid.sse @@ -0,0 +1,18 @@ +: connection 1 + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"checkout-v2":"false"},"kind":"ConfigMap","metadata":{"name":"feature-flags","namespace":"app","resourceVersion":"1010","uid":"cm-flags-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + +data: {"type":"error","code":"RESYNC_REQUIRED","message":"upstream continuity lost; a new snapshot cycle follows","terminal":false} + +data: {"type":"reset","target":"demo","scope":{"target":"demo","version":"v1","resource":"configmaps","namespace":"app"},"projection":"krm-editor/v1"} + +data: {"type":"added","object":{"apiVersion":"v1","data":{"log-level":"info","replicas":"3"},"kind":"ConfigMap","metadata":{"annotations":{"note":"keep"},"labels":{"app.kubernetes.io/name":"shop"},"name":"app-config","namespace":"app","resourceVersion":"1001","uid":"cm-app-0001"}},"redactedPaths":[]} + +data: {"type":"synced"} + diff --git a/docs/facts/kubernetes-api-concepts.md b/docs/facts/kubernetes-api-concepts.md new file mode 100644 index 0000000..60eb529 --- /dev/null +++ b/docs/facts/kubernetes-api-concepts.md @@ -0,0 +1,231 @@ +# Facts: the Kubernetes API, as Kubernetes actually documents it + +> **Source:** , read in full from the +> upstream markdown (`kubernetes/website`, `content/en/docs/reference/using-api/api-concepts.md`) on +> **2026-07-11**. This file is a summary *with citations to that page*, plus — kept strictly +> separate — what each fact means for this repo. +> +> **Why this file exists.** `spec/v1.md` and `gateway/README.md` make claims about what a Kubernetes +> watch does. Those claims were written from memory. Two of them were **wrong**, and one of the two +> was a live bug in the gateway (§2). Design decisions here get grounded in what the API server +> documents, or they do not get made. +> +> **What is NOT in this file:** anything the page does not say. Where we rely on behaviour that lives +> in `client-go`/`apimachinery` rather than in the documentation, it is called out as **[unverified +> against docs]** and is a job for the real-cluster suite. + +--- + +## 1. The watch event vocabulary + +The page documents a watch as a stream of change notifications, each a JSON document of the form +`{"type": …, "object": …}`, and names these types: + +| type | in the docs | +|---|---| +| `ADDED` | yes — including **synthetic** ADDEDs that establish initial state | +| `MODIFIED` | yes | +| `DELETED` | yes (as an operation; the event type appears in the examples) | +| `BOOKMARK` | yes — a whole section, [Watch bookmarks](https://kubernetes.io/docs/reference/using-api/api-concepts/#watch-bookmarks) | + +**`ERROR` is not enumerated on this page.** It exists in `k8s.io/apimachinery`'s `watch.Event` +(`watch.Error`, carrying a `metav1.Status`), and it is how a `410 Gone` reaches a watch that is +already open. **[unverified against docs]** + +### 1.1 A BOOKMARK's object is a PARTIAL OBJECT. This is the headline. + +> "It is a special kind of event to mark that all changes up to a given `resourceVersion` the client +> is requesting have already been sent. The document representing the `BOOKMARK` event is of the type +> requested by the request, **but only includes a `.metadata.resourceVersion` field**." + +Their example, verbatim: + +```json +{ "type": "BOOKMARK", + "object": {"kind": "Pod", "apiVersion": "v1", "metadata": {"resourceVersion": "12746"} } } +``` + +So an object with **no `uid`, no `name`, no `spec`, no `status`** is not a pathological edge case +someone might contrive — **it is on every conforming watch stream that asked for bookmarks.** A +gateway that forwards a BOOKMARK's object as `added`/`modified` hands its consumer a fragment; a +consumer whose model is "replace, never merge" (ours, and correctly so) then blanks the resource on +screen. + +Two further rules from the same section: + +- Bookmarks are **opt-in**: `allowWatchBookmarks=true` on the watch request. +- "You **shouldn't assume bookmarks are returned at any specific interval**, nor can clients assume + that the API server will send any `BOOKMARK` event even when requested." → nothing may be built on + a bookmark *arriving*. It is a hint, not a heartbeat. + +## 2. Resource versions — and the bug this page found in our gateway + +> "Resource version strings are **orderable as monotonically increasing integers within the same +> resource type**… Both resource versions must be from objects of the same API group and resource +> type." + +So far so good — that is what `spec/v1.md` §6 assumes. But then: + +> "Both must start with a digit 1-9 and contain only digits 0-9. **Resource versions are compared as +> arbitrary bitsize decimal integers**… **The bitsize must not be assumed to be some fixed amount.**" + +And the page's own worked example is a **40-digit** resource version: + +> `"2345678901234567890123456789012345678901" > "345678901234567890123456789012345678901"` + +The prescribed comparison is **lexicographic-by-length**: + +> "If they are not of equal length, the longer one is greater (for example, "123" > "23"). If they +> are of equal length, the lexicographically greater one is greater." + +**We were parsing `resourceVersion` with `strconv.ParseInt` (int64 — 19 digits).** A 40-digit +resource version overflows it, `ParseInt` fails, and our staleness check silently gives up — or +worse, on a value that *does* parse but has wrapped, compares nonsense. The failure mode is +**dropped live updates**, which in a status-watch UI looks exactly like "Kubernetes is slow." + +And the case that has no integer at all: + +> "If you are using API resources served by an **extension API server**… If either of two resource +> version strings does not parse as a decimal number, the two strings can be checked for **equality** +> but you **cannot** rely on comparisons for ordering." + +→ For a non-numeric `resourceVersion`, **ordering is undefined** and the only safe thing to do is not +order. Not "guess". Not "fall back to string compare". + +> "Starting with Kubernetes 1.35, orderability of resource versions for all Kubernetes types is +> included in Certified Kubernetes requirements." + +Also, and we get this right already: + +- **Opaque to clients.** "Resource versions must be passed unmodified back to the server." +- Comparison is only valid **within one resource type**. A Pod's RV and a Deployment's are not + comparable. (Our high-water map is keyed by uid within one scope — one GVR — so this holds.) +- On a **list**, `.metadata.resourceVersion` of the *collection* is the version the collection was + constructed at — which is **not** related to the `.metadata.resourceVersion` of the items in it. + +## 3. Streaming lists (`sendInitialEvents`) — the snapshot boundary + +> "the initial state can be requested by specifying `sendInitialEvents=true`… the API server starts +> the watch stream with synthetic init events (of type `ADDED`) to build the whole state of all +> existing objects **followed by a `BOOKMARK` event (if requested via `allowWatchBookmarks=true`)**. +> The bookmark event includes the resource version to which is synced. After sending the bookmark +> event, the API server continues as for any other watch request." + +Preconditions, stated as requirements: + +- `sendInitialEvents=true` **requires** `resourceVersionMatch=NotOlderThan`. +- `resourceVersion` empty or absent ⇒ a **consistent read**; the bookmark is sent once the state is + synced at least to the moment the request began being processed. +- `allowWatchBookmarks=true` is what makes the terminating bookmark appear at all. + +This maps **one-to-one** onto our protocol: the synthetic ADDEDs are the snapshot, the terminating +bookmark **is** `synced`, everything after it is live. That is exactly what `gateway/README.md` §3a +claims, and it checks out. + +**One claim of ours the page does NOT support.** `gateway/README.md` §3a says the terminating +bookmark is identified by `metadata.annotations["k8s.io/initial-events-end"] == "true"`. **That +annotation appears nowhere on this page.** It is real — it is `metav1.InitialEventsAnnotationKey` in +`apimachinery`, from KEP-3157 — but it is **[unverified against docs]**, and it is precisely the kind +of thing that must be proven against a live API server before we trust it. **This is now the single +most important thing for the real-cluster (k3d) suite to confirm.** If it is wrong, we cannot tell +the snapshot's closing bookmark from a routine one, and `synced` fires at the wrong moment — or never. + +Note also: with `resourceVersion` **unset** (no `sendInitialEvents` at all), a watch is "Get State and +Start at Most Recent" and *also* "begins with synthetic 'Added' events for all resource instances that +exist at the starting resource version" — but with **no terminating bookmark**, so you cannot tell +where the snapshot ends. That is the whole reason `sendInitialEvents` + `allowWatchBookmarks` exist, +and the whole reason a list-then-watch fallback (§3b) has to synthesize the boundary itself. + +## 4. Losing continuity: `410 Gone` + +> "A given Kubernetes server will only preserve a historical record of changes for a limited time. +> **Clusters using etcd 3 preserve changes in the last 5 minutes by default.** When the requested +> watch operations fail because the historical version of that resource is not available, clients +> must handle the case by recognizing the status code `410 Gone`, **clearing their local cache, +> performing a new get or list operation, and starting the watch from the `resourceVersion` that was +> returned**." + +"Clear the cache, re-list, restart the watch" **is** our `reset` … `synced` cycle. The five-minute +window is why `resync-midstream` is a fixture and not a curiosity: a browser tab left open on a quiet +namespace, behind a laptop lid, will hit this routinely. + +Also: `resourceVersion="0"` on a watch means "Get State and Start at **Any**", which the page warns +"may return **arbitrarily stale** data" and can **rewind** to a version the client already observed. +→ **Never open our watches with `resourceVersion=0`.** Per-object monotonicity would be violated by +the upstream itself, and our own high-water map would then (correctly, but uselessly) drop half the +snapshot. + +## 5. Deletion is two-phase, and the UI must show it + +> "When a client first sends a **delete**… the `.metadata.deletionTimestamp` is set to the current +> time. Once the `.metadata.deletionTimestamp` is set, external controllers that act on finalizers may +> start performing their cleanup work… **Once the last finalizer is removed, the resource is actually +> removed from etcd.**" + +So a delete of a finalized object appears on the watch as **`MODIFIED` (now carrying +`.metadata.deletionTimestamp` and `.metadata.finalizers`) — possibly for a long time — and only later +as `DELETED`.** An object that is "Terminating" is a first-class, observable state, and it arrives +through the ordinary upsert path. A live status view that does not surface it is lying about the +cluster. + +The page says nothing about whether a `DELETED` event's object is complete, and nothing about +informer deletion tombstones (`cache.DeletedFinalStateUnknown`) — that is a `client-go` construct. +**[unverified against docs]** + +## 6. Partial objects are a first-class, client-requested thing + +> "To request partial object metadata, you can request metadata only responses in the `Accept` +> header… `Accept: application/json;as=PartialObjectMetadata;g=meta.k8s.io;v=v1`… **the returned +> objects only contain the `metadata` field. The `spec` and `status` fields are omitted.**" + +The returned objects have `kind: PartialObjectMetadata`, `apiVersion: meta.k8s.io/v1` — and a full +`metadata`, *including a `uid`*. This matters for us: **a "partial object" is not always detectable by +a missing uid.** A `PartialObjectMetadata` has one. The honest check is the **kind**, and the +consequence of missing it is that a consumer replaces a real object with one that has no `spec` and no +`status` — the status view goes blank and the editor loses the user's `spec`. + +(An aggregated API server may not support partial fetches at all and returns `406`.) + +## 7. Writes + +Four PATCH content types, and the page names them exactly: + +| `Content-Type` | what | +|---|---| +| `application/apply-patch+yaml` | Server-Side Apply (create-or-patch) | +| `application/json-patch+json` | RFC 6902 | +| **`application/merge-patch+json`** | **RFC 7386** ← what `patch(id)` builds | +| `application/strategic-merge-patch+json` | k8s-specific; **not usable with CRDs** | + +- **PUT** requires the client to send `resourceVersion`; a stale one gets **`409 Conflict`**. The page + independently warns that a PUT "might accidentally drop fields… you could receive fields that your + client does not know how to handle - and then drop them as part of your update" — which is the exact + argument `spec/v1.md` §3 makes for never round-tripping a projected object. Good: the protocol's + most opinionated rule is one Kubernetes itself makes. +- A PATCH may *also* carry `resourceVersion` as a precondition against lost updates. Our `patch(id)` + does not, today — a deliberate gap worth revisiting, since we *have* the resourceVersion and the + user has been staring at a three-way merge. +- Strategic Merge Patch "has been superseded by Server-Side Apply", and **cannot** be used with CRDs. + → the keyed-list merge we want (docs §4.1, `x-kubernetes-list-map-keys`) points at **SSA**, not SMP. + +`x-kubernetes-list-type` / `x-kubernetes-list-map-keys` are **not on this page** (they live in the +Server-Side Apply reference). The array-merge open question needs that page read the same way this one +was, before it gets decided. + +--- + +## What this changes in this repo + +| # | Fact | Consequence | Status | +|---|---|---|---| +| 1 | RVs are arbitrary-bitsize integers; a 40-digit one is legal | `strconv.ParseInt` in `isStale` **overflows** → silently drops live updates | **fixed** — string compare, length-then-lexicographic | +| 2 | Non-numeric RVs (extension API servers) cannot be ordered | must compare for equality only, and **never** drop on an unorderable pair | **fixed** | +| 3 | A BOOKMARK's object has only `.metadata.resourceVersion` | a partial object is on **every** conforming stream; forwarding it blanks the consumer | **fixed** + fixture `bookmark-absorbed` | +| 4 | `PartialObjectMetadata` has a **uid** | "no uid" is not a sufficient partial-object check; check the **kind** | **fixed** + fixture `partial-object-refused` | +| 5 | Bookmarks may never arrive | nothing may depend on a bookmark's *arrival*, only on its meaning | holds — we only use the terminating one | +| 6 | `k8s.io/initial-events-end` is not in the docs | our snapshot boundary rests on it | **[unverified]** — the top priority for the k3d suite | +| 7 | Delete is two-phase; `deletionTimestamp` arrives as `MODIFIED` | "Terminating" is an observable state we must render, not a gap | **open** — no fixture yet | +| 8 | `rv=0` watches may rewind and serve stale data | never open a watch with `resourceVersion=0` | holds — §3a uses `rv=""` | +| 9 | RFC 7386 merge patch is a real k8s content type | `patch(id)` is directly usable, `application/merge-patch+json` | holds | +| 10 | PATCH may carry `resourceVersion` as a lost-update precondition | we could, and probably should, offer it | **open** | +| 11 | SMP is superseded and CRD-incompatible | keyed-list merge must go via **SSA**, not strategic merge | **open** (docs §4.1) | diff --git a/docs/proposals/0001-watch-ops.md b/docs/proposals/0001-watch-ops.md new file mode 100644 index 0000000..bc18bca --- /dev/null +++ b/docs/proposals/0001-watch-ops.md @@ -0,0 +1,126 @@ +# Proposal 0001 — three watch ops the corpus cannot currently say + +**Status:** accepted, implemented in this commit. +**Affects:** `conformance/` (fixture format + new fixtures), `gateway/` (one real bug), `spec/v1.md` +(one non-normative claim that was wrong). **Not** a wire change: no event type, field or framing rule +changes, so no `/v2`. + +## The problem, and how it was found + +The gateway passes all 11 of its conformance fixtures. Then I mutated it, to check the corpus could +actually go red: + +| mutation | fixtures failing | +|---|---| +| emit `synced` on **every** bookmark, not just the snapshot's | **0** | +| forward a **partial object** (no uid) as `added`/`modified` | **0** | + +Both are `MUST NOT`s — [spec §2](../../spec/v1.md), and rows 7–8 of the gateway's own failure matrix. +Both would break a browser. And the corpus is **structurally incapable of catching either**, because a +fixture's `watch:` script has exactly six ops — `list`, `added`, `modified`, `deleted`, `relist`, +`disconnect` — and none of them can say *"the API server sent a bare BOOKMARK"* or *"the informer +handed us a tombstone with no uid"*. + +A corpus that cannot express a rule is not defending it. That is the whole point of this repository, +so the corpus grows three ops. + +## Why these three, and not three others + +Every op below is a thing **Kubernetes documents itself as doing** — see +[docs/facts/kubernetes-api-concepts.md](../facts/kubernetes-api-concepts.md), which is a read of the +[API concepts page](https://kubernetes.io/docs/reference/using-api/api-concepts/) rather than a read of +my own memory. That distinction produced two corrections to this repo on its own (below), so it is not +a formality. + +### `bookmark` — a routine watch bookmark + +Kubernetes, verbatim: *"The document representing the `BOOKMARK` event is of the type requested by the +request, **but only includes a `.metadata.resourceVersion` field**."* + +```json +{ "type": "BOOKMARK", + "object": {"kind": "Pod", "apiVersion": "v1", "metadata": {"resourceVersion": "12746"} } } +``` + +So this is not an exotic case: **an object with no uid, no name, no spec and no status is on every +conforming watch stream that asked for bookmarks** (`allowWatchBookmarks=true`, which the gateway must +set — it is how the snapshot boundary arrives at all). + +The gateway must **absorb** it: never forward it as an upsert, and never mistake it for the snapshot +boundary. Only the bookmark that *terminates the initial events* is `synced`. + +> ```yaml +> - { op: bookmark, resourceVersion: "12746" } +> ``` + +### `partial` — a metadata-only object delivered as an upsert + +Kubernetes serves `PartialObjectMetadata` (`Accept: application/json;as=PartialObjectMetadata;…`): +*"the returned objects only contain the `metadata` field. The `spec` and `status` fields are omitted."* + +This corrected a real mistake in our implementation. Our guard was **"an object with no uid is +partial"** — but a `PartialObjectMetadata` **has a uid**. It has a whole `metadata`. What it does not +have is a `spec` or a `status`. Forwarding one replaces a live Deployment with a husk: the status view +goes blank and the editor loses the user's `spec`. Our check was looking at the wrong field, and the +corpus could not tell us. + +The honest check is the **kind**: `PartialObjectMetadata` / `PartialObjectMetadataList`, in group +`meta.k8s.io`. Plus the uid check, which still catches the bookmark's object. + +> ```yaml +> - { op: partial, body: deploy-web.v1 } # delivered as PartialObjectMetadata: metadata only +> ``` + +### `tombstone` — a deletion with no trustworthy identity + +`client-go`'s informer delivers `cache.DeletedFinalStateUnknown` when it missed the delete and noticed +during a relist. **[Not in the API docs — a client-go construct, flagged as such in the facts file.]** +[spec §4.2](../../spec/v1.md) already says what to do, and says it in strong terms: if the gateway +cannot recover a trustworthy uid it **MUST NOT** emit an ambiguous `deleted` — it begins a new snapshot +cycle instead and lets `reset`…`synced` prune. A guessed uid deletes **the wrong object** out of +somebody's browser. + +> ```yaml +> - { op: tombstone, body: cm-app.v1 } # a DELETED whose object lost its identity +> ``` + +## The two corrections this exercise produced + +**1. `resourceVersion` overflow — a live bug, dropping live updates.** + +Kubernetes: *"Resource versions are compared as **arbitrary bitsize decimal integers**… The bitsize +must not be assumed to be some fixed amount."* Their own worked example is **40 digits**. + +`gateway/stream.go` compared them with `strconv.ParseInt` — **int64, 19 digits**. On a cluster whose +resource versions exceed that, the parse fails and the monotonicity check gives up; the practical +result is **silently dropped live updates**, which in a status view is indistinguishable from "the +cluster is slow". Fixed: compare as strings, longer-is-greater then lexicographic, exactly as the docs +prescribe. + +And the case with no integer at all: an **extension API server** may serve non-numeric resource +versions, where *"the two strings can be checked for equality but you cannot rely on comparisons for +ordering."* So an unorderable pair must never cause a drop. Fixed, and fixtured +(`resourceversion-bignum`). + +**2. `spec/v1.md` §6 overclaimed.** Its non-normative note said a resource version "is a monotonically +increasing **integer**", which invites exactly the `ParseInt` that bit us. Corrected to say what +Kubernetes says: orderable as an arbitrary-precision decimal *within one resource type*, not orderable +at all if it is not decimal. The normative rule ("`resourceVersion` is opaque to consumers") is +unchanged — this only fixes the rationale that a *gateway* implementer reads. + +## What does not change + +- **No wire change.** No new event type, no new field, no framing change. A v1 consumer built against + today's spec is unaffected: all three ops describe things the gateway must *absorb or refuse*, and + the correct behaviour for every one of them is to emit **fewer** events, not different ones. +- **The client suite is untouched.** `watch:` is the gateway's input; the TypeScript loader already + ignores it. + +## New fixtures + +| fixture | defends | +|---|---| +| `bookmark-absorbed` | a routine BOOKMARK is absorbed — never forwarded, never mistaken for `synced` | +| `partial-object-refused` | a `PartialObjectMetadata` upsert is refused; the gateway resnapshots rather than blanking the object | +| `tombstone-without-uid` | an ambiguous tombstone is never emitted as `deleted`; a new cycle prunes instead | +| `resourceversion-bignum` | a 40-digit resourceVersion orders correctly, and a stale replay is still dropped | diff --git a/gateway/conformance.go b/gateway/conformance.go index 45b5789..d3bc99e 100644 --- a/gateway/conformance.go +++ b/gateway/conformance.go @@ -44,10 +44,23 @@ type Fixture struct { // relist upstream continuity was LOST (410 Gone / cache reset) — a new cycle must follow, // even though the SSE connection is perfectly healthy // disconnect the consumer's connection dropped; the next list is a fresh cycle +// +// And three that say what Kubernetes really does, added by docs/proposals/0001-watch-ops.md because +// the corpus could not otherwise express three of the gateway's own MUST NOTs: +// +// bookmark a routine BOOKMARK. Its object carries ONLY metadata.resourceVersion — that is not an +// edge case, it is what the API server sends, on every stream that asked for bookmarks. +// Absorbed: never forwarded, never mistaken for the snapshot boundary. +// partial a metadata-only object (PartialObjectMetadata) delivered as an upsert. Has a uid, has +// NO spec and NO status. Must be refused — forwarding it blanks the consumer's object. +// tombstone a DELETED whose object lost its identity (client-go's DeletedFinalStateUnknown). +// Must not be guessed at: a new snapshot cycle prunes correctly instead. type WatchOp struct { Op string `json:"op"` Body string `json:"body"` // a bodies/ reference, e.g. "cm-app.v2" Bodies []string `json:"bodies"` // for list/relist + // ResourceVersion is the bookmark's payload — the only field a real BOOKMARK's object has. + ResourceVersion string `json:"resourceVersion"` } // FixtureEvent is an Event with its object given by REFERENCE into bodies/, so a scenario stays diff --git a/gateway/scripted.go b/gateway/scripted.go index 3554880..a025d6c 100644 --- a/gateway/scripted.go +++ b/gateway/scripted.go @@ -85,6 +85,46 @@ func NewScriptedBackend(c Corpus, ops []WatchOp) (*ScriptedBackend, error) { } cur.events = append(cur.events, WatchEvent{Type: WatchEventType(op.Op), Object: obj}) + case "bookmark": + // A routine BOOKMARK, exactly as Kubernetes documents it: an object of the requested type + // carrying ONLY metadata.resourceVersion. No uid, no name, no spec, no status. + if cur == nil { + return nil, fmt.Errorf("scripted: bookmark before any list") + } + cur.events = append(cur.events, WatchEvent{ + Type: WatchBookmark, + Object: KRMObject{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{"resourceVersion": op.ResourceVersion}, + }, + }) + + case "partial": + // PartialObjectMetadata: the metadata, and NOTHING else. Note that it keeps the uid — which + // is precisely why "has a uid" was never a sufficient test for "is a complete object", and + // why this op had to exist before the gateway could be made honest. + if cur == nil { + return nil, fmt.Errorf("scripted: partial before any list") + } + full, err := body(op.Body) + if err != nil { + return nil, err + } + cur.events = append(cur.events, WatchEvent{Type: WatchModified, Object: partialOf(full)}) + + case "tombstone": + // client-go's cache.DeletedFinalStateUnknown: the informer missed the delete and noticed on + // a relist, so what it hands you may be little more than a key. The uid is gone. + if cur == nil { + return nil, fmt.Errorf("scripted: tombstone before any list") + } + full, err := body(op.Body) + if err != nil { + return nil, err + } + cur.events = append(cur.events, WatchEvent{Type: WatchDeleted, Object: degenerateTombstoneOf(full)}) + case "disconnect": return nil, fmt.Errorf("scripted: `disconnect` is the caller's to handle — split the script on it") @@ -149,3 +189,40 @@ func (w *scriptedWatcher) Next(ctx context.Context) (WatchEvent, error) { } func (w *scriptedWatcher) Stop() {} + +// partialOf turns a complete object into the PartialObjectMetadata the API server serves when a +// client asks for a metadata-only response: the metadata, verbatim — uid and all — and nothing else. +// +// Accept: application/json;as=PartialObjectMetadata;g=meta.k8s.io;v=v1 +// -> "the returned objects only contain the `metadata` field. The `spec` and `status` fields +// are omitted." +// +// The uid survives. That is the whole trap: a gateway checking "does it have a uid?" waves this +// through, and the consumer replaces a live Deployment with a husk. +func partialOf(full KRMObject) KRMObject { + out := KRMObject{ + "apiVersion": "meta.k8s.io/v1", + "kind": "PartialObjectMetadata", + } + if meta, ok := deepCopyObject(full)["metadata"].(map[string]any); ok { + out["metadata"] = meta + } + return out +} + +// degenerateTombstoneOf is what an informer hands you when it missed a delete and found out during a +// relist (cache.DeletedFinalStateUnknown): the object may be little more than a key. Here: the kind +// and the name survive, and the uid — the only thing a consumer may act on — does not. +// +// The name surviving is the dangerous part. It is exactly enough to tempt an implementer into +// reconstructing the identity from it, and a delete-and-recreate under the same name (see +// delete-recreate-uid) is precisely where that deletes the wrong object. +func degenerateTombstoneOf(full KRMObject) KRMObject { + out := deepCopyObject(full) + meta, ok := out["metadata"].(map[string]any) + if !ok { + return out + } + delete(meta, "uid") + return out +} diff --git a/gateway/stream.go b/gateway/stream.go index fdb3332..990684a 100644 --- a/gateway/stream.go +++ b/gateway/stream.go @@ -3,7 +3,7 @@ package gateway import ( "context" "errors" - "strconv" + "strings" ) // The stream loop: a Kubernetes watch in, the protocol out. @@ -110,7 +110,7 @@ func (g *Gateway) cycle(ctx context.Context, backend Backend, scope Scope, proje // consumer already saw (that is what makes reconnect replays idempotent). Keeping this map // across cycles would silently swallow the snapshot of an object whose resourceVersion had not // moved — the object would simply never arrive, and the consumer would prune it. - emitted := map[string]int64{} + emitted := map[string]string{} for { ev, err := w.Next(ctx) @@ -130,15 +130,21 @@ func (g *Gateway) cycle(ctx context.Context, backend Backend, scope Scope, proje } case WatchAdded, WatchModified: + // A partial object must never be forwarded (spec §2). The consumer's model is REPLACE, + // never merge, so a fragment does not "update" its object — it BLANKS it: the status view + // goes empty and an editor loses the user's spec. + // + // Note what is checked, and what is NOT. "Has no uid" is not sufficient, and believing it + // was is the bug this corrected: a PartialObjectMetadata carries a complete metadata block, + // uid included, and only omits `spec`/`status`. The kind is the honest test. (The uid test + // still earns its keep — a BOOKMARK's object has only a resourceVersion.) + if reason := partialReason(ev.Object); reason != "" { + // Resnapshot: it is the one recovery that is always correct, because it re-establishes + // the truth instead of guessing at it. + return ResyncRequired(reason) + } obj, redacted := project(projection, ev.Object) uid := obj.UID() - if uid == "" { - // A metadata-only or otherwise partial object. Forwarding it as added/modified would - // REPLACE the consumer's object with a fragment — the consumer's whole model is - // "replace, never merge" — so the object would blank out on screen. Resnapshot - // instead: it is the one recovery that is always correct. - return ResyncRequired("upstream delivered a partial object with no uid") - } if isStale(emitted, uid, obj) { continue } @@ -175,26 +181,98 @@ func (g *Gateway) cycle(ctx context.Context, backend Backend, scope Scope, proje } } +// partialReason says why an object must not be forwarded as added/modified, or "" if it may be. +// +// Both cases are things Kubernetes really sends — see docs/facts/kubernetes-api-concepts.md: +// +// 1. PartialObjectMetadata. A client can ask for metadata-only responses +// (`Accept: application/json;as=PartialObjectMetadata;g=meta.k8s.io;v=v1`), and then "the returned +// objects only contain the `metadata` field. The `spec` and `status` fields are omitted." It KEEPS +// ITS UID. If this gateway is ever wired to a client that negotiated such a response — or to an +// aggregated API server that returns one — every object on the stream would arrive as a husk. +// +// 2. No uid at all. A BOOKMARK's object is exactly this: "of the type requested by the request, but +// only includes a .metadata.resourceVersion field". Bookmarks are absorbed before we get here, so +// reaching this branch means something upstream is confused — and confusion is not a thing to +// forward to a browser. +func partialReason(obj KRMObject) string { + kind, _ := obj["kind"].(string) + if kind == "PartialObjectMetadata" || kind == "PartialObjectMetadataList" { + return "upstream delivered a metadata-only object (" + kind + "); it has no spec and no status" + } + if obj.UID() == "" { + return "upstream delivered a partial object with no uid" + } + return "" +} + // isStale enforces per-object monotonicity (§6): within one cycle, never hand a consumer a state // older than one it already has. This is what makes coalescing safe, and it is the guarantee a naive // relay cannot give — an informer that relists mid-stream will happily replay an older version of an // object it has already delivered. -// -// A resourceVersion that does not parse as an integer is not compared. The gateway may rely on -// monotonicity within one target (§3d) but must not CRASH on a backend that does not provide it: the -// failure mode of a wrong guess here is a dropped update, which is worse than a duplicate one. -func isStale(emitted map[string]int64, uid string, obj KRMObject) bool { - rv, err := strconv.ParseInt(resourceVersionOf(obj), 10, 64) - if err != nil { - return false - } - if last, ok := emitted[uid]; ok && rv < last { +func isStale(emitted map[string]string, uid string, obj KRMObject) bool { + rv := resourceVersionOf(obj) + last, seen := emitted[uid] + if seen && compareResourceVersion(rv, last) < 0 { return true } + // Not comparable, or newer: record it and let it through. Recording an unorderable version is + // harmless — the next comparison against it will also be unorderable, and will also let through. emitted[uid] = rv return false } +// compareResourceVersion orders two resource versions the way Kubernetes says to, and returns 0 when +// they cannot be ordered at all. +// +// This function is small and it is the second bug this project shipped for want of reading the docs. +// The old version was `strconv.ParseInt(rv, 10, 64)`. Kubernetes: +// +// "Resource versions are compared as ARBITRARY BITSIZE decimal integers... The bitsize must not be +// assumed to be some fixed amount." +// +// and its own worked example is FORTY DIGITS long. int64 holds nineteen. Against such a cluster the +// parse failed, the staleness check gave up, and updates were dropped — a symptom indistinguishable +// from "Kubernetes is being slow", which is how a bug like this survives for years. +// +// The prescribed comparison, verbatim: "If they are not of equal length, the longer one is greater +// (for example, "123" > "23"). If they are of equal length, the lexicographically greater one is +// greater." Note that this rules out a PLAIN lexicographic compare, which calls "9" > "10". +// +// And the case with no integer in it at all: an extension API server may serve resource versions that +// are not decimal, and then "the two strings can be checked for equality but you CANNOT rely on +// comparisons for ordering". So: unorderable returns 0, and the caller drops nothing. A duplicate +// event is harmless (the protocol is idempotent by construction); a dropped one is data loss. +func compareResourceVersion(a, b string) int { + if a == b { + return 0 + } + if !isDecimalResourceVersion(a) || !isDecimalResourceVersion(b) { + return 0 // not orderable — say so, rather than guessing + } + if len(a) != len(b) { + if len(a) > len(b) { + return 1 + } + return -1 + } + return strings.Compare(a, b) +} + +// isDecimalResourceVersion reports whether a resource version may be ORDERED, per the rules the API +// docs give: "Both must start with a digit 1-9 and contain only digits 0-9." +func isDecimalResourceVersion(rv string) bool { + if rv == "" || rv[0] < '1' || rv[0] > '9' { + return false + } + for i := 0; i < len(rv); i++ { + if rv[i] < '0' || rv[i] > '9' { + return false + } + } + return true +} + // emitTerminal puts the error on the wire and returns it. A terminal error MUST be the last event on // the connection, and the caller closes it. func emitTerminal(ctx context.Context, sink Sink, err error) error { diff --git a/gateway/stream_test.go b/gateway/stream_test.go index 90f2f62..29b4d58 100644 --- a/gateway/stream_test.go +++ b/gateway/stream_test.go @@ -218,6 +218,62 @@ func TestStaleResourceVersionIsDropped(t *testing.T) { } } +// resourceVersion comparison, against the rules Kubernetes actually publishes. The corpus covers the +// big-number case (resourceversion-bignum); this covers the cases a fixture cannot reach, because a +// fixture body is a real KRM object and these are not. +func TestCompareResourceVersion(t *testing.T) { + // The docs' own worked examples, verbatim. + big40 := "2345678901234567890123456789012345678901" + big39 := "345678901234567890123456789012345678901" + + cases := []struct { + a, b string + want int + why string + }{ + {big40, big39, 1, "40 digits beats 39 — and neither fits in an int64"}, + {big39, big39, 0, "equal"}, + {"345678901234567890123456789012345678900", big39, -1, "same length: lexicographic"}, + {"123", "23", 1, "longer is greater — NOT plain lexicographic, which says '1' < '2'"}, + {"9", "10", -1, "the case a naive string compare gets backwards"}, + {"1001", "1002", -1, "the ordinary case"}, + + // An extension API server may serve a resourceVersion that is not a decimal at all. Then + // ordering is UNDEFINED — "the two strings can be checked for equality but you cannot rely on + // comparisons for ordering" — so we say 0 (not orderable) and the caller drops nothing. + {"abc", "def", 0, "non-decimal: not orderable"}, + {"1001", "abc", 0, "one non-decimal: not orderable"}, + {"", "1001", 0, "empty: not orderable"}, + {"0123", "1001", 0, "a leading zero is not a valid orderable rv (must start 1-9)"}, + {"abc", "abc", 0, "equal, even when not orderable"}, + } + for _, c := range cases { + if got := compareResourceVersion(c.a, c.b); got != c.want { + t.Errorf("compare(%q, %q) = %d, want %d — %s", c.a, c.b, got, c.want, c.why) + } + } +} + +// The consequence of the above, at the level that matters: an unorderable pair must never cause an +// event to be DROPPED. A duplicate is harmless (the protocol requires idempotent apply); a drop is +// data loss, and in a status view it looks exactly like the cluster being slow. +func TestUnorderableResourceVersionsNeverDropAnEvent(t *testing.T) { + rv := func(v string) KRMObject { + o := cm("u1", v, map[string]any{"v": v}) + return o + } + got := run(t, ProjectionEditor, AllowAll{}, []WatchEvent{ + {Type: WatchAdded, Object: rv("opaque-b")}, + {Type: WatchBookmark, InitialEventsEnd: true}, + {Type: WatchModified, Object: rv("opaque-a")}, // "older"? unknowable. Must NOT be dropped. + {Type: WatchModified, Object: rv("opaque-c")}, + }, 5) + + if !equalTypes(types(got), EventReset, EventAdded, EventSynced, EventModified, EventModified) { + t.Fatalf("an unorderable resourceVersion caused an event to be dropped: %v", types(got)) + } +} + // The projection removes machinery — and ONLY what a named projection says it removes. Nothing is // "optionally other server-side bookkeeping". func TestProjectionRemovesMachineryAndNothingElse(t *testing.T) { diff --git a/spec/v1.md b/spec/v1.md index c38592f..20fbbdd 100644 --- a/spec/v1.md +++ b/spec/v1.md @@ -307,12 +307,24 @@ What is **not** guaranteed: save for optimistic concurrency, and MUST NOT parse, compare, or order by it. > *(Non-normative — and the reason this protocol can promise per-object monotonicity at all.)* Within a -> single upstream target, a modern Kubernetes API server's `resourceVersion` for a given resource type -> is a monotonically increasing integer (it is the backing store's revision). The **gateway** relies on -> this internally — to coalesce safely, to drop a stale replay after a relist, and to assert it never -> hands a consumer a state older than one already sent. It is deliberately **not** exposed as an -> ordering primitive on the wire: it is not comparable across targets or across aggregated API servers, -> and a browser has no business doing revision arithmetic. See the gateway spec §3. +> single upstream target, a modern Kubernetes API server's `resourceVersion` for a given **resource +> type** is orderable as a monotonically increasing decimal (it is the backing store's revision). The +> **gateway** relies on this internally — to coalesce safely, to drop a stale replay after a relist, +> and to assert it never hands a consumer a state older than one already sent. It is deliberately +> **not** exposed as an ordering primitive on the wire: it is not comparable across targets or across +> aggregated API servers, and a browser has no business doing revision arithmetic. See the gateway +> spec §3. +> +> **Two things an implementer must get right here, both of which we got wrong first** (see +> [docs/facts/kubernetes-api-concepts.md](../docs/facts/kubernetes-api-concepts.md)): +> +> - A `resourceVersion` is an **arbitrary-bitsize** decimal. Kubernetes' own documented example is 40 +> digits long, so it does **not** fit in an int64, and parsing it into one silently fails — which +> makes a staleness check drop live updates. Compare as strings: **longer is greater; equal lengths +> compare lexicographically.** (A plain lexicographic compare is also wrong — it calls `"9" > "10"`.) +> - A `resourceVersion` served by an **extension API server** need not be decimal at all, and then it +> **cannot be ordered** — only compared for equality. An unorderable pair must therefore never cause +> an event to be dropped: a duplicate is harmless (§6 requires idempotent apply), a drop is data loss. ## 7. Transport & authentication From 3b4e1b1edbb40446f7ba4f88f33cddf4d11b96e0 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Sat, 11 Jul 2026 21:28:09 +0000 Subject: [PATCH 06/43] =?UTF-8?q?feat(examples):=20the=20browser=20rung=20?= =?UTF-8?q?=E2=80=94=20a=20real=20Chromium,=20native=20EventSource,=20no?= =?UTF-8?q?=20bundler,=20no=20cluster?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `task e2e-browser` (and `task demo`, which is the same thing at human speed). This is the only test of the constraint the whole library is designed around: that the published ESM imports in a browser with NO BUNDLER. Node importing dist/index.js proved nothing — Node is not a browser. Here Chromium fetches index.js, which imports ./store.js, which imports ./merge.js, and one bare specifier or one missing extension would leave the page blank. Three more things only a browser can prove, and all three are now asserted: - native EventSource works at all. It is the same-origin session-cookie path and the v1 baseline (spec §7); fetch cannot test it, being a different transport. - a read-only region actually FLASHES. "Read-only is not ignored" is the product thesis, and it is a DOM fact, not an array of paths. - a user can type into a field while the server changes the object underneath them, and keep what they typed — the three-way merge, from the only end that matters. Six specs: the bundler-free import, status-flashes-while-you-edit, a conflict raised and then clearing on convergence, an unrelated change not disturbing an edit, a masked Secret with no input to type into, and an absent named object rendering empty rather than as a ghost. Mutation-checked: deleting the flash from applyServerEvent turns the status test red. cmd/replay now serves the example and the built ESM from the SAME ORIGIN as the stream. Not for convenience — same-origin is the deployment the protocol is specified around, and a demo on a second port would need CORS and would then be proving something we do not ship. Playwright lives in the example's own package, so packages/krm-stream keeps its three devDependencies and its zero-dependency promise intact. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 28 +++ .gitignore | 4 + Taskfile.yml | 22 ++ examples/vanilla-browser/README.md | 71 ++++-- examples/vanilla-browser/index.html | 217 ++++++++++++++++++ examples/vanilla-browser/package-lock.json | 76 ++++++ examples/vanilla-browser/package.json | 12 + examples/vanilla-browser/playwright.config.ts | 37 +++ .../vanilla-browser/tests/live-krm.spec.ts | 120 ++++++++++ gateway/cmd/replay/main.go | 33 ++- 10 files changed, 600 insertions(+), 20 deletions(-) create mode 100644 examples/vanilla-browser/index.html create mode 100644 examples/vanilla-browser/package-lock.json create mode 100644 examples/vanilla-browser/package.json create mode 100644 examples/vanilla-browser/playwright.config.ts create mode 100644 examples/vanilla-browser/tests/live-krm.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc07541..67420ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,3 +92,31 @@ jobs: with: repo-token: ${{ secrets.GITHUB_TOKEN }} - run: task e2e-wire + + # The top of the ladder that still needs no cluster. This is the ONLY job that proves the library's + # central promise — that the published ESM imports in a browser with no bundler — because Node + # importing it proves nothing: Node is not a browser. It is also the only place a `flash` is a fact + # rather than an array, and the only place native EventSource runs at all. + browser: + name: end to end (a real browser) + runs-on: ubuntu-latest + needs: fixtures + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version: "1.26" + cache-dependency-path: gateway/go.sum + - uses: actions/setup-node@v6 + with: + node-version: "22" + - uses: arduino/setup-task@v2 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - run: task e2e-browser + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-trace + path: examples/vanilla-browser/test-results/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index 62e1ff1..3461c95 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,7 @@ packages/*/dist/ # Editor scratch *.local.* + +# Playwright (examples/vanilla-browser) — the browser rung of the e2e ladder +test-results/ +playwright-report/ diff --git a/Taskfile.yml b/Taskfile.yml index a866869..8482fb8 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -104,6 +104,28 @@ tasks: cd {{.CLIENT_DIR}} && REPLAY_URL=http://{{.REPLAY_ADDR}} node e2e/wire.ts + e2e-browser: + desc: "End to end in a REAL browser: native EventSource, unbundled ESM, no cluster." + deps: [fixtures] + dir: examples/vanilla-browser + cmds: + # The only place the library's central promise is actually tested: that the published ESM + # imports in a browser with no bundler. Node importing it proves nothing — Node is not a browser. + # Playwright lives in the EXAMPLE's package, so the library keeps its three devDependencies. + - npm install --no-audit --no-fund + - npx playwright install chromium --with-deps + - npx playwright test {{.CLI_ARGS}} + + demo: + desc: "Serve the browser demo — watch a status reconcile while you edit spec. No cluster." + deps: [build-client] + cmds: + - | + echo "→ http://127.0.0.1:8100/?fixture=status-only-churn&pace=800ms" + echo " (also: conflict-and-converge, secret-redaction, edit-vs-unrelated-change — /fixtures lists them)" + cd gateway && go run ./cmd/replay --addr 127.0.0.1:8100 --corpus ../conformance \ + --static ../examples/vanilla-browser --dist ../packages/krm-stream/dist + # -------------------------------------------------------------------- lint -- lint: desc: "Vet + lint both languages." diff --git a/examples/vanilla-browser/README.md b/examples/vanilla-browser/README.md index 6562571..9b614af 100644 --- a/examples/vanilla-browser/README.md +++ b/examples/vanilla-browser/README.md @@ -1,26 +1,61 @@ # examples/vanilla-browser — watch `status` reconcile, live -**The demo that sells the thing.** Point it at any object in any cluster and watch its `status` move — -conditions flipping, `readyReplicas` climbing, `observedGeneration` catching up — in a browser, with -real RBAC, and no save path at all. +**The demo that sells the thing**, and the only test of the constraint the whole library is designed +around. -No framework. No bundler. One ` +Everything is served from **one origin** — the stream, the built ESM at `/krm-stream/`, and this page. +That is not a convenience: it is the deployment the protocol is specified around, and a demo on a +second port would need CORS and would then be proving something we do not ship. + +## The page + +One ` + + diff --git a/examples/vanilla-browser/package-lock.json b/examples/vanilla-browser/package-lock.json new file mode 100644 index 0000000..6351650 --- /dev/null +++ b/examples/vanilla-browser/package-lock.json @@ -0,0 +1,76 @@ +{ + "name": "krm-stream-example-vanilla-browser", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "krm-stream-example-vanilla-browser", + "devDependencies": { + "@playwright/test": "^1.50.0" + } + }, + "node_modules/@playwright/test": { + "version": "1.61.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.61.1.tgz", + "integrity": "sha512-8nKv6+0RJSL9FE4jYOEGXnPeM/Hg12qZpmqzZjRh3qM0Y7c3z1mrOTfFLids72RDQYVh9WpLEfR5WdpNX4fkig==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright": "1.61.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, + "node_modules/playwright": { + "version": "1.61.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.61.1.tgz", + "integrity": "sha512-DWnY5o3YbLWK4GovuAVwpqL+1VwGNdUGrRr++8j8PtQQzvAVZUIMjKQ90fY689sEJZJBbZVw1rXaOKSTitkzPQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.61.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "fsevents": "2.3.2" + } + }, + "node_modules/playwright-core": { + "version": "1.61.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.61.1.tgz", + "integrity": "sha512-h7Qlt6m4REp25qvIdvbDtVmD4LqVXfpRxhORv9L0jzETM05p4fuPJ3dKyuSXQxDSbXnmS79HAgi9589lGSpLkg==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=18" + } + } + } +} diff --git a/examples/vanilla-browser/package.json b/examples/vanilla-browser/package.json new file mode 100644 index 0000000..4451089 --- /dev/null +++ b/examples/vanilla-browser/package.json @@ -0,0 +1,12 @@ +{ + "name": "krm-stream-example-vanilla-browser", + "private": true, + "type": "module", + "//": "Playwright lives HERE, not in packages/krm-stream, so the library keeps its three devDependencies and its promise: the published bundle has zero dependencies and needs no bundler. This example is the only place that promise is actually TESTED — a real browser, a real EventSource, a real unbundled ESM import.", + "scripts": { + "test": "playwright test" + }, + "devDependencies": { + "@playwright/test": "^1.50.0" + } +} diff --git a/examples/vanilla-browser/playwright.config.ts b/examples/vanilla-browser/playwright.config.ts new file mode 100644 index 0000000..e34582d --- /dev/null +++ b/examples/vanilla-browser/playwright.config.ts @@ -0,0 +1,37 @@ +import { defineConfig } from "@playwright/test"; + +// The browser rung of the e2e ladder — and it needs no cluster. +// +// One process serves everything, from ONE origin: the replay gateway serves the scripted corpus as +// real SSE, the built ESM at /krm-stream/, and this example at /. Same-origin is not a convenience — +// it is the deployment the protocol is specified around (spec §7: native EventSource cannot send an +// Authorization header, so the v1 baseline is same-origin session cookies). A demo served from a +// second port would need CORS, and would then be proving something we do not ship. + +export default defineConfig({ + testDir: "./tests", + fullyParallel: true, + forbidOnly: !!process.env["CI"], + retries: process.env["CI"] ? 1 : 0, + reporter: process.env["CI"] ? "list" : [["list"]], + use: { + baseURL: "http://127.0.0.1:8100", + trace: "retain-on-failure", + }, + projects: [{ name: "chromium", use: { browserName: "chromium" } }], + + // Build the library first — `dist/` is what the browser imports, unbundled, and a stale dist is a + // test of last week's library. + webServer: { + command: + "cd ../../packages/krm-stream && npx tsc && " + + "cd ../../gateway && go run ./cmd/replay " + + "--addr 127.0.0.1:8100 --corpus ../conformance " + + "--static ../examples/vanilla-browser --dist ../packages/krm-stream/dist", + url: "http://127.0.0.1:8100/healthz", + reuseExistingServer: !process.env["CI"], + timeout: 120_000, + stdout: "pipe", + stderr: "pipe", + }, +}); diff --git a/examples/vanilla-browser/tests/live-krm.spec.ts b/examples/vanilla-browser/tests/live-krm.spec.ts new file mode 100644 index 0000000..b5a74ca --- /dev/null +++ b/examples/vanilla-browser/tests/live-krm.spec.ts @@ -0,0 +1,120 @@ +// The top of the ladder that needs no cluster: a REAL browser, a REAL EventSource, a REAL unbundled +// ESM import, against the REAL gateway. +// +// Everything below this rung feeds the store bytes. That proves the protocol and the merge, and it +// cannot prove any of the following — each of which breaks the product, and each of which is +// invisible to `node --test`: +// +// - the published ESM actually imports in a browser with NO BUNDLER (dist/index.js imports +// ./store.js imports ./merge.js…). Node importing it proves nothing: Node is not a browser. +// - native EventSource works at all — it is the same-origin cookie path, the v1 baseline (spec §7), +// and the only transport a plain