diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 17d0c4f..8bb5192 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -59,3 +59,14 @@ updates: patterns: ["*"] commit-message: prefix: "chore(deps)" + + # Vue and its test tools remain confined to the private adapter example. + - package-ecosystem: npm + directory: /examples/vue + schedule: + interval: weekly + groups: + npm: + patterns: ["*"] + commit-message: + prefix: "chore(deps)" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5be7294..48ad196 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -286,6 +286,24 @@ jobs: if-no-files-found: error retention-days: 7 + vue: + name: Vue adapter example + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "22" + cache: npm + cache-dependency-path: examples/vue/package-lock.json + - run: npm ci --no-audit --no-fund + working-directory: examples/vue + - name: typecheck and test reactivity and subscription cleanup + run: npm test + working-directory: examples/vue + # 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 diff --git a/README.md b/README.md index 89bd752..98ece90 100644 --- a/README.md +++ b/README.md @@ -102,14 +102,14 @@ There are two halves, and they are usually two different people. ### The browser half -No bundler, no framework, no Kubernetes client. `EventSource` is native, and the store is plain ESM: +No bundler, no framework, no Kubernetes client. The managed connector uses fetch, and the store is plain ESM: ```ts -import { LiveResourceStore, connectWithEventSource, resourceStreamURL } from "@configbutler/krm-stream"; +import { LiveResourceStore, connectManagedResourceStream, resourceStreamURL } from "@configbutler/krm-stream"; const store = new LiveResourceStore(); -connectWithEventSource( +const connection = connectManagedResourceStream( resourceStreamURL("/resource-stream/v1", { target: "production", version: "v1", @@ -126,6 +126,9 @@ connectWithEventSource( store.setValue(uid, ["spec", "replicas"], 3); store.conflicts(uid); // paths where the server disagreed with an edit the user actually made store.patch(uid); // an RFC 7386 merge patch of just their changes, or null + +// In your host/view teardown callback: +connection.close(); // stop the stream and pending retries ``` If you have no bundler at all and vendor the library by copying it, import @@ -164,7 +167,7 @@ patch through its own save endpoint, which is the one place a write can happen. | Package | Purpose | |---|---| | `github.com/ConfigButler/krm-stream/gateway` | Dependency-free Go stream gateway and SSE handler. | -| `github.com/ConfigButler/krm-stream/gateway/kube` | Optional `client-go` backend and SSAR authorizer. | +| `github.com/ConfigButler/krm-stream/gateway/kube` | Optional `client-go` backend and SubjectAccessReview authorizer. | | `@configbutler/krm-stream` | Official dependency-free ESM client store and transports. | | `krm-stream@0.1.0` | Deprecated, frozen compatibility name claim. Use the scoped package instead. | | [`spec/v1.md`](spec/v1.md) | Normative protocol contract. | diff --git a/Taskfile.yml b/Taskfile.yml index 6bb3ac8..6e2d89b 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -91,7 +91,7 @@ tasks: # -------------------------------------------------------------------- test -- test: desc: "Run both suites against the shared conformance fixtures." - deps: [test-gateway, test-kube, test-client] + deps: [test-gateway, test-kube, test-client, test-vue] test-gateway: desc: "Go: the gateway + its half of the conformance suite." @@ -119,6 +119,24 @@ tasks: cmds: - node --test + test-vue: + desc: "Typecheck and test the optional Vue example." + deps: [_vue-deps] + dir: examples/vue + cmds: + - npm test + + _vue-deps: + internal: true + dir: examples/vue + sources: ["package.json", "package-lock.json"] + generates: ["node_modules/.package-lock.json"] + status: + - test -x node_modules/.bin/tsc + - test -f node_modules/vue/package.json + cmds: + - npm ci --no-audit --no-fund + # --------------------------------------------------------------------- e2e -- replay: desc: "Serve the conformance corpus over real SSE — a cluster you can point a browser at." diff --git a/docs/adopting.md b/docs/adopting.md index f2fb1ac..5ffa4e7 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -91,8 +91,8 @@ func mount(mux *http.ServeMux, dynamicClientFor func(*User) dynamic.Interface) { } ``` -The browser uses `connectWithEventSource` for this route. Its same-origin session cookie is the only -credential EventSource can carry. +The browser uses `connectManagedResourceStream` for this route. Fetch sends its same-origin session +cookie and the managed connection provides bounded recovery after network failures and sequence gaps. ## 3. Browser client @@ -100,7 +100,7 @@ credential EventSource can carry. import { defaultPolicy, LiveResourceStore, - connectWithEventSource, + connectManagedResourceStream, resourceStreamURL, withOpenAPIKeyedLists, } from "@configbutler/krm-stream"; @@ -114,15 +114,18 @@ const url = resourceStreamURL("/resource-stream/v1", { projection: "krm-full/v1", }); -connectWithEventSource(url, store, { - onGap: () => location.reload(), // reconnect for a new snapshot in an SPA-specific way +const connection = connectManagedResourceStream(url, store, { + onStateChange: state => renderConnection(state.status), // gaps recover with a fresh snapshot }); store.subscribe(() => render(store)); ``` -For a bearer-token client, use `connectResourceStream(url, store, { headers: { Authorization: ... } })`. +For a bearer-token client, use `connectManagedResourceStream(url, store, { headers: { Authorization: ... } })`. That is useful for a non-browser client or an intentionally token-bearing browser application; the -same-origin cookie route is the safer browser default. +same-origin cookie route is the safer browser default. The host must enforce HTTPS for bearer-token +requests, including the resolved destination of relative URLs and any redirects. Validate the +trusted endpoint before supplying credentials and enforce the same policy in a custom fetch wrapper. +The stream connectors delegate transport to fetch; they do not enforce a credential transport policy. For Deployment or CRD editing, a host may opt into OpenAPI-declared associative-list merging without exposing schemas to the browser: @@ -138,7 +141,7 @@ stays safely atomic. ## 4. Share watches only with Kubernetes-backed authorization `SharedBackend` saves upstream watches but runs as one service identity. Pair it with -`kube.SSARAuthorizer` so Kubernetes still decides whether each caller may list and watch the scope. +`kube.SubjectAccessReviewAuthorizer` so Kubernetes still decides whether each caller may list and watch the scope. ```go shared := gateway.NewSharedBackendWithOptions(serviceAccountBackend, gateway.SharedOptions{ @@ -146,7 +149,7 @@ shared := gateway.NewSharedBackendWithOptions(serviceAccountBackend, gateway.Sha Observer: metrics, }) -options.Authorizer = kube.SSARAuthorizer(clientset, subjectFromUser) +options.Authorizer = kube.SubjectAccessReviewAuthorizer(clientset, subjectFromUser) options.Clients = func(context.Context, string, gateway.Principal) (gateway.Backend, error) { return shared, nil } ``` diff --git a/docs/auth.md b/docs/auth.md index 2a706e5..da2bd00 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -103,10 +103,25 @@ too: - **`ClientFor` is your refresh point.** It is called again each cycle, so you can hand back a client bearing a *fresh* token. -**The gap:** a perfectly quiet stream may not cycle for a long time, so revocation is -noticed at the *next cycle*, not instantly. The credential half of that is solved properly on your -side of the seam — give the client a **refreshing token source** (Dex issues a refresh token; the -standard `oauth2.TokenSource` wraps it), and it never hands us a dead token in the first place. +Set `ReauthorizationInterval` to bound how long a quiet stream runs without checking entitlement: + +```go +options.ReauthorizationInterval = 30 * time.Second +options.ReauthorizationTimeout = 5 * time.Second +``` + +Timed checks are per subscriber and recheck both `Authorizer` and the projection policy. During a +check that subscriber's object delivery pauses. Denial, timeout or policy failure terminates only +that stream; other subscribers and the shared upstream continue. A changed projection terminates +the old stream so it cannot keep disclosing its previous view. Zero interval preserves cycle-only +checks; zero timeout uses 10 seconds. The bound assumes host callbacks honor context cancellation +and sinks do not block indefinitely. The check uses the principal captured at stream open: resolve +current session/account validity inside the host authorizer if those can change independently of RBAC. + +For 200 subscribers, a 30-second interval adds roughly 13 SubjectAccessReviews/second (list and watch +per subscriber), plus opening/cycle checks. Choose an interval and timeout for your revocation budget +and API-server capacity; checks are not cached across identities. `ClientFor` still runs per snapshot +cycle so the host can return a client backed by refreshing credentials. ## Two things that are easy to confuse @@ -120,11 +135,11 @@ scope, so it opens it **once**, so it opens it as **one identity** — your serv moment your `Authorizer` stops being defence in depth and becomes *the only thing* between a caller and the objects. That is why it is opt-in, and why it is not the default. -If you turn it on, use **`kube.SSARAuthorizer`**, and Kubernetes is the boundary again: +If you turn it on, use **`kube.SubjectAccessReviewAuthorizer`**, and Kubernetes is the boundary again: ```go shared := gateway.NewSharedBackend(serviceAccountBackend) // one watch, one identity… -opts.Authorizer = kube.SSARAuthorizer(clientset, subjectOf) // …but RBAC still decides +opts.Authorizer = kube.SubjectAccessReviewAuthorizer(clientset, subjectOf) // …but RBAC still decides opts.Clients = func(context.Context, string, gateway.Principal) (gateway.Backend, error) { return shared, nil } ``` @@ -145,7 +160,8 @@ Three things it does that are easy to get accidentally permissive, all tested: It needs your server's service account to hold `create` on `subjectaccessreviews` (the standard `system:auth-delegator` role). It does **not** need impersonate rights: it asks a question *about* a user, it does not act *as* one. And because the gateway re-authorizes every snapshot cycle, this is -also how a revocation reaches a stream that is already open. +also how a revocation reaches a stream that is already open. Timed checks bound quiet-stream revocation. +The old `SSARAuthorizer` name remains a deprecated alias; it never created SelfSubjectAccessReview. ## What this library never does diff --git a/docs/proposals/0004-views-and-bytes.md b/docs/proposals/0004-views-and-bytes.md index c6a5f10..97ca93c 100644 --- a/docs/proposals/0004-views-and-bytes.md +++ b/docs/proposals/0004-views-and-bytes.md @@ -104,16 +104,17 @@ actually emitted, including `reset`, `synced`, errors, and deletes. It does not cycle. A new HTTP connection begins at one. The TypeScript transport checks every sequence number. A missing, repeated, malformed, or out-of-order -number closes the transport and reports a gap; the host reconnects for a fresh snapshot. `seq` is not +number closes the transport and reports a gap; the managed connector retries for a fresh snapshot. `seq` is not an SSE `id` and does not provide replay or resume semantics. ## Consequences - `krm-spec/v1` makes a status-blind editor cheap under controller churn without weakening the complete-object invariant for the fields it receives. -- A suppressed update may leave the consumer's `metadata.resourceVersion` stale. Consumers must not - use that opaque value as a save precondition. Saves remain narrow merge patches built from local - edits, while the client-side three-way merge reports visible conflicts live. +- A suppressed update may leave the consumer's `metadata.resourceVersion` stale. It remains a safe + save precondition: the API server rejects a stale write with 409. Capture it with the patch, + reconcile before retrying, and never substitute a new version onto an old patch. Narrow patches + limit write scope; they do not prevent lost updates. See [saving](../saving.md). - Shared upstream watches remain safe: projection, redaction revision, suppression digest, and sequence state are per consumer stream, after upstream fan-out. diff --git a/docs/proposals/0005-kubernetes-stream-and-save-semantics.md b/docs/proposals/0005-kubernetes-stream-and-save-semantics.md new file mode 100644 index 0000000..1b393bc --- /dev/null +++ b/docs/proposals/0005-kubernetes-stream-and-save-semantics.md @@ -0,0 +1,498 @@ +# Proposal 0005: Kubernetes stream semantics and conditional editing + +**Status: design discussion; no protocol or runtime change approved by this document.** + +The follow-up [implementation plan](0006-stream-and-save-implementation-plan.md) incorporates the +latest review and supersedes the phase list below for sequencing and merge gates. + +This follows the second review of PR #25. It distinguishes Kubernetes behavior, the contract in +[spec/v1.md](../../spec/v1.md), the implementation, and proposed changes. The immediate recommendation +is to clarify the contract and save guidance, retain Kubernetes concurrency checks, and add focused +integration tests. Resume support, new wire events, and a general write abstraction should not be +bundled into that correction. + +The original priorities remain managed recovery, bounded authorization rechecks for shared streams, +and a complete conditional-save integration. Existing watch sharing and draft reconciliation remain +in place. Credentials, writes, application policy, and field ownership stay with the host. + +## 1. The boundary we should preserve + +Kubernetes owns object identity, resource versions, persistence, authorization decisions and write +conflict enforcement. The gateway provides a projected read stream for a UI; the store maintains a +local draft. These are related responsibilities, not interchangeable authorities. + +```mermaid +flowchart LR + K["Kubernetes API server
Objects, versions, RBAC, writes"] + G["Gateway
Watch translation, sharing, projection"] + S["Client store
Last delivered view and local draft"] + U["Editor
User intent and conflict presentation"] + H["Host save endpoint
Identity, validation, write policy"] + K -->|"Kubernetes watch"| G + G -->|"Projected SSE snapshots and changes"| S + S --> U + U -->|"Captured save intent"| H + H -->|"Conditional PATCH or deliberate apply policy"| K + K -->|"Write result"| H +``` + +**Recommended boundary:** keep the projected read stream and draft helpers small. Do not make the +SSE sequence, a projection digest, or a redaction revision into a substitute for Kubernetes write +concurrency. An optional managed connection is lifecycle convenience, not a new consistency model. + +Kubernetes documents version-based watch continuation, recovery when history expires, and conditional +writes. A default GET requests most-recent semantics; supported API servers may serve consistent +reads from their watch cache. “Most recent” should not be documented as a guarantee of a physical +etcd quorum read for every request. [Kubernetes API concepts](https://kubernetes.io/docs/reference/using-api/api-concepts/) + +## 2. What the feedback gets right, and where I disagree + +| Feedback | Assessment | Planned response | +|---|---|---| +| Healthy connections now replenish retries and reset backoff | Agree; preserve this behavior | Simplify its tests without expanding the public API | +| Cycle teardown now recovers while genuine denial remains terminal | Agree | Add one subscriber-visible test through `Gateway.Stream` | +| Suppression plus conditional saves can repeatedly produce 409s | Agree; this is a usability and progress problem | Explain it by projection and distinguish write conflicts from draft conflicts | +| resourceVersion preconditions are wrong for `krm-spec/v1` | Disagree as a safety claim | They remain safe, but can be expensive or make progress difficult under churn | +| The reversal is correct only for `krm-full/v1` | Too absolute | Full includes status but still suppresses changes to ignored metadata; any delivered version can also become stale in transit | +| Only a snapshot can refresh the held version | Incorrect for the current store | An accepted `captureReconciliation` GET updates the server object, including its version; a delivered upsert also updates it | +| Name Server-Side Apply as an alternative | Agree, with substantial qualification | Describe ownership semantics and its separate validation requirements | +| SSA is the server analogue of our three-way draft conflicts | Incomplete | Same-manager concurrent writers do not gain stale-read detection merely by using SSA | +| Unknown redaction paths require a fresh connection | Too strong | A later delivered upsert for that UID can supply authoritative revisions; a fresh snapshot is an explicit recovery option | +| Every reconnect sends a full snapshot | Agree, but distinguish two connections | Document upstream and downstream recovery costs separately | +| UID immutability may yield 422 rather than 409 | Plausible, path-dependent | Reproduce the actual endpoint race; never translate every 422 into a conflict | +| More test coverage is useful | Agree | Prioritize projection/save interaction and public stream recovery over broad new infrastructure | + +The suppression mechanism predates PR #25: it is specified in +[proposal 0004](0004-views-and-bytes.md) and implemented by +[`visibleDigest`](../../gateway/stream.go). What this PR adds is the conditional-save composition and +stronger guidance, exposing the tension more directly. + +### Follow-up review: scope and qualifications + +The reviewer now agrees that accepted GETs advance the base, all projections can suppress RV +changes, and SSA ownership is not per-user stale-read protection. Split the normative convergence +amendment from PR #25, but keep focused save/recovery regression tests with the example correction. +The [implementation plan](0006-stream-and-save-implementation-plan.md) defines that merge gate. + +Routine upstream closure fans out resnapshot work to all shared subscribers, making upstream +continuation a named follow-up. Calling it the dominant source of resets still requires measurement. +The five-second timeout in auth.md is an explicit override; its prose already states the ten-second +default. Label the example rather than change the default. + +## 3. Divergence map + +Some differences are intentional protocol choices. Others are ambiguities in our own prose. They +should not all be described as departures from Kubernetes correctness. + +| Area | Kubernetes behavior or authority | Current krm-stream behavior | Classification and consequence | +|---|---|---|---| +| Initial state | Streaming initial events or list followed by watch at the list version | Backend uses those mechanisms; gateway emits `reset`, `added`, `synced` | Translation; preserves the snapshot/watch boundary | +| Live changes | Versioned resource events | Complete projected objects; intermediate updates may be coalesced | Intentional view service, not an audit log | +| Suppression | Resource version identifies an object revision | Digest excludes resourceVersion for **all** projections | Intentional optimization; held version is from the last delivered object | +| Status omission | Status writes can advance object version | `krm-spec/v1` omits status and suppresses status-only changes | Intentional projection; systematically stale save bases under controller churn | +| Upstream reconnect | A client can resume from an observed watch checkpoint while history is retained | An ended backend watch returns `ErrWatchClosed`; gateway resnapshots | Conservative implementation choice; potentially avoidable work | +| Browser reconnect | Kubernetes does not define our SSE protocol | Every new SSE connection snapshots; no `Last-Event-ID` | Explicit v1 contract; warm cache reduces upstream cost, not browser bytes | +| Gap detection | Kubernetes watch version semantics | A per-connection `seq` checks emitted-frame continuity | Additional transport check; not a resource version or resume token | +| Redaction revisions | Kubernetes has no equivalent UI counter | Per-connection counters describe observed hidden-value changes | Deliberate disclosure feature; stateless GET cannot invent them | +| Shared authorization | Upstream watch operates as its authenticated identity | Shared watch uses host identity; subscribers get separate SAR checks | Necessary host/gateway boundary when sharing; not transparent impersonation | +| Draft reconciliation | API server enforces write semantics | Store merges user edits against delivered server views | UI assistance; cannot authorize or guarantee a write | +| Retry exhaustion | Client retry policy is not a Kubernetes wire guarantee | Managed connector bounds failures and replenishes after sustained health | Optional operational policy; preserve low-level transport access | + +Code anchors: [Kubernetes backend](../../gateway/kube/backend.go), +[stream loop](../../gateway/stream.go), [shared backend](../../gateway/shared.go), +[authorization gate](../../gateway/reauthorize.go), [managed connector](../../packages/krm-stream/src/connection.ts). + +## 4. The main tension: quiet views versus current write versions + +Consider a Deployment editor using `krm-spec/v1`. The numbers below are illustrative labels. The +browser echoes resourceVersion strings; it must not parse them or infer ordering from them. + +```mermaid +sequenceDiagram + participant C as Controller + participant K as Kubernetes + participant G as Gateway + participant E as Editor and store + participant H as Host endpoint + K->>G: Object A, resourceVersion 100 + G->>E: Projected object A, resourceVersion 100 + E->>E: User edits spec + C->>K: Update status + K->>G: Same spec, new status, resourceVersion 101 + G->>G: Visible digest unchanged, suppress event + E->>H: Captured patch with resourceVersion 100 + H->>K: Conditional PATCH at 100 + K-->>H: 409: version precondition failed + H-->>E: HTTP 409 + E->>H: Most-recent projected GET + H->>K: GET + K-->>H: Object A, resourceVersion 101 + H-->>E: Same visible spec, resourceVersion 101 + E->>E: Guard accepts GET, base advances, no draft conflict + Note over C,E: Another status write before the next PATCH can repeat the 409 +``` + +The accepted GET already refreshes the base. A snapshot is not required for this example to recover. +The problem is that another write can invalidate that base before the next PATCH. This is possible +with any watch-fed editor; suppression makes it especially common and less visible. + +There are three different facts a UI must not collapse into one “conflict” label: + +- **A version precondition failed:** the object changed since the captured base. This does not identify + which fields changed. +- **A local draft conflict exists:** the store found a disagreement between base, draft and a newer + delivered view at an editable path. +- **An ownership conflict exists:** an apply operation disputes another field manager's ownership. + +The current example returns `"conflict"` for a 409 even when reconciliation leaves no draft conflicts. +That is a misleading result for a human editor. It should describe refresh/review state separately +from an actual field conflict, without promising automatic save success. + +### Projection-specific expectations + +| Projection | Updates intentionally invisible | Conditional merge-patch implications | +|---|---|---| +| `krm-full/v1` | Removed bookkeeping metadata; Secret values are withheld but their revisions are reported | Good general editor default; no guarantee that the delivered RV remains current | +| `krm-spec/v1` | The above, plus status changes | Safe optimistic concurrency, but repeated refresh/409 cycles may dominate controller-heavy resources | +| `krm-raw/v1` | Still removes managedFields and last-applied configuration | Not a raw Kubernetes watch and not a concurrency solution; it can disclose Secret values | + +No projection makes a streamed resourceVersion into a lease. Calling a stale precondition “unsafe” +would reverse the safety argument: the rejection is exactly what prevents an old array replacement +or field value from silently overwriting a newer write. + +### Should status updates be the client's choice? + +Yes, within the host's authorization policy. We should not make a new library-wide decision that +every client must receive status, or that every client must ignore it. A dashboard may need status +continuously; a configuration viewer may prefer silence when only status changes. An editor may want +to omit status content while still learning about version changes that affect conditional saves. + +The current API already accepts a requested projection. The host's `ProjectionPolicy` selects the +authorized projection for the principal and scope; a host can also pin a static projection. Thus +client choice means requesting a supported view, not overriding host policy. The stream's `reset` +declares the effective projection. Keep `krm-full/v1` as the general default. + +There are two distinct questions, currently bundled into named projection contracts: + +1. **Content:** which fields should the client receive? +2. **Notifications:** which changes should cause the gateway to deliver an update? + +“I do not need to display status” does not necessarily mean “I do not want to hear that the object +version changed.” Conversely, sending version notifications to every spec-only viewer would remove +a useful reason to choose that projection. + +| Client need | Content and delivery behavior | Availability | +|---|---|---| +| Observe configuration and status | Include status and emit visible changes; bookkeeping-only RV changes can still be suppressed | Current `krm-full/v1` | +| Observe configuration with less notification traffic | Omit status and suppress status-only changes | Current `krm-spec/v1` | +| Edit configuration while learning about status-driven version changes | Omit status but deliver updates when RV changes, subject to ordinary stream coalescing | Possible future opt-in contract, not implemented | + +```mermaid +flowchart TD + C["Client requests a supported projection"] --> H["Host policy selects an authorized projection"] + H --> E["Reset declares the effective projection"] + E --> P["Projection contract bundles two decisions today"] + P --> F["Content: include or omit status"] + P --> N["Delivery: which changes trigger updates?"] + F --> Q["Future question: omit status but report version changes?"] + N --> Q + Q --> M["Measure benefit and cost before adding an explicit opt-in contract"] +``` + +**Proposed decision:** preserve client requests and host control, document the existing coupling, +and do not add independent configuration switches yet. If the third use case proves valuable, +compare one additional named contract with a separate delivery option. Either design must make the +effective behavior explicit and remain simple for clients and hosts to implement. It must not +silently change what existing `krm-spec/v1` subscribers receive. + +Omitting status saves payload bytes; suppressing status-only updates also saves notifications and +browser work. A version-only message could save bytes compared with a complete projected object, +but still causes notifications and requires additional protocol handling. More frequent version +delivery can reduce avoidable stale saves, but cannot prevent a write racing the next save: 409 +handling remains necessary. This is a choice about stream delivery, not a different write guarantee. + +### The constraint we cannot abstract away + +```mermaid +flowchart TD + A["Status-only write advances Kubernetes resourceVersion"] + A --> B["Send nothing to the browser"] + B --> C["Browser cannot learn the new version from this stream"] + C --> D["Old-version conditional save may return 409"] + A --> E["Send a complete object or version-bearing message"] + E --> F["Browser learns a later version"] + F --> G["Traffic and wakeups return, a later write can still race"] +``` + +We cannot promise zero notifications for ignored writes, always-current browser write versions, and +unconditional progress for object-version CAS writes at the same time. That is an information and +concurrency constraint, not a missing merge algorithm. + +## 5. Our own spec needs a precise statement of equality + +[Spec §3](../../spec/v1.md#3-projection-and-redaction--part-of-the-wire-not-an-implementation-detail) +explicitly permits status suppression. Proposal 0004 explicitly excludes resourceVersion from the +digest. But §6 says the consumer map equals the projected view and promises delivery of the final +state. Literal whole-object equality includes resourceVersion. A final suppressed metadata/status +write can leave that field behind indefinitely. + +This is at least an ambiguity between the declared suppression rule and the broad invariant. We +should resolve it explicitly, not claim that current code gives exact Kubernetes-object equality at +all times. The “corresponding logical stream position” wording limits the invariant, but does not +clearly tell adopters which version remains meaningful after suppression. + +**Recommended clarification, for review:** + +> Convergence is over the declared visible comparison: projected object content excluding +> metadata.resourceVersion, together with the stream's redaction records. The resourceVersion in a +> delivered object is genuine and belongs to that delivered revision; suppressed updates do not +> refresh it. It can be echoed as a conditional-write precondition, but is not a current-version +> guarantee or a downstream resume checkpoint. + +Keep completeness within the projection, safe snapshot pruning, and within-cycle ordering as separate +invariants. Scope redaction counters to one connection; do not equate counters from separate +connections. Do not change opaque-version handling in the browser. + +This documents the implemented semantics, but it narrows how a reader might interpret the existing +invariant. Call it out in release notes. If the project instead wants exact projected-object equality +including version metadata, it needs a different emission policy; that cannot be presented as a +wording-only fix. + +## 6. Write strategies: choices the host must make + +The following is a design comparison, not a proposal to implement four save engines. + +| Strategy | Protects | Tradeoff | Recommendation | +|---|---|---|---| +| Merge patch with captured UID and resourceVersion | Identity and object-version concurrency | Unrelated writes can reject it; arrays are replaced as units | Keep the supported baseline and demonstrate its limitations | +| Refresh, reconcile, then submit a newly reviewed conditional intent | Retains object CAS while allowing a newer base | Still races; review flow must distinguish actual field disagreements | Improve the example first; no blind version substitution | +| JSON Patch with deliberate `test` operations | Can guard specific values atomically | Paths, missing values, list indices and whole-array dependencies require care | Document as host-owned advanced work, not a new core patch compiler | +| Server-Side Apply with deliberate field ownership | Declarative ownership conflicts | Different semantics from stale-read detection; needs an apply-specific validator and intent model | Name it and scope a separate example, not a drop-in replacement | +| Merge patch with no concurrency condition | Only the patch's write scope | Deliberate last-writer-wins behavior can lose edits | Only when explicitly chosen by the host; do not silently select it by projection | + +Merge-patch array replacement and JSON Patch value tests are defined by +[RFC 7386](https://www.rfc-editor.org/rfc/rfc7386) and +[RFC 6902](https://www.rfc-editor.org/rfc/rfc6902), respectively. + +An eventual automatic retry must preserve the **submitted intent**, compare its relevant base values +against a fresh object, and bound retries. For RFC 7386 arrays, that comparison must protect the +replacement array, not just an item the user changed. Do not silently incorporate edits typed after +Save was clicked. `store.conflicts().length === 0` alone is not a complete host write policy. + +### Why SSA is an option, not a replacement guarantee + +Server-Side Apply tracks field managers. Different managers claiming changed values can conflict; +`force` can override ownership. Omission from a manager's subsequent applied configuration can remove +fields it previously managed. This means repeated “only this click's changes” payloads are not a safe +mechanical translation of merge patches. List behavior also depends on schema topology. +[Kubernetes Server-Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) + +```mermaid +sequenceDiagram + participant A as Attendee A + participant B as Attendee B + participant H as Host using fieldManager editor + participant K as Kubernetes + A->>H: Apply value A + H->>K: Apply as editor + K-->>H: Accepted, editor owns field + B->>H: Apply value B from an older view + H->>K: Apply as editor + Note over H,K: Same manager is not a separate-owner conflict + K-->>H: May accept value B +``` + +This example is an inference from ownership semantics: using one manager for 200 attendees does not +provide user-to-user optimistic locking. Conversely, one manager per tab changes ownership and +managedFields growth; it is not a free concurrency fix. A manager name is not authentication or RBAC. + +Spec §3 permits SSA but describes saves as constrained writes over edited paths. Clarify that SSA +needs the host's intended managed field set and omission/deletion policy. `ValidateMergePatch` and +`captureSave().patch` remain merge-patch-specific. The store's local keyed-list merge does not turn +that output into strategic merge patch or apply configuration. + +The [SSA design proposal](https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md) +also makes field management and schema topology central to this API. + +A future SSA example must establish manager lifetime, permitted fields, deletion semantics, +create-on-apply behavior, list topology, and conflict handling. Default to no forced ownership +transfer. Preserve the existing projection boundary on both requests and responses. + +## 7. Recovery has two layers, with different costs + +```mermaid +flowchart TB + subgraph Native["Kubernetes client recovery"] + N1["Watch closes"] --> N2["Resume using last valid observed watch checkpoint"] + N2 --> N3{"History retained?"} + N3 -->|Yes| N4["Continue changes"] + N3 -->|"Expired, for example 410"| N5["Relist or streaming initial events"] + end + subgraph Current["Current krm-stream recovery"] + U1["Upstream watcher ends"] --> U2["Gateway announces RESYNC_REQUIRED"] + U2 --> U3["New snapshot cycle on existing SSE connection"] + D1["Browser connection ends or detects a seq gap"] --> D2["Managed retry opens a new SSE connection"] + D2 --> D3["Full projected snapshot, seq starts at one"] + end +``` + +“Relist only on 410” is shorthand, not a universal rule: initial startup, scope changes, and other +losses of trustworthy state can also require initialization. Bookmarks are useful checkpoints, not +promises of a fixed heartbeat cadence. [Watch recovery semantics](https://kubernetes.io/docs/reference/using-api/api-concepts/#efficient-detection-of-changes) + +**Upstream:** today, routine watch closure can cause a new initial-state cycle even when retained +Kubernetes history could permit continuation. Prefer investigating a resumable Kubernetes backend +first. It could keep the existing `Watcher` seam and SSE protocol, returning a resync only when it +cannot establish continuity. The upstream [client-go reflector](https://github.com/kubernetes/client-go/blob/master/tools/cache/reflector.go) +is the reference to study before implementing more watch machinery. It must track the upstream checkpoint, including appropriate bookmarks, +rather than infer one from a browser's last visible resource. + +**Downstream:** §7 deliberately prohibits SSE resume in v1. SharedBackend can serve a warm snapshot +without opening one upstream watch per subscriber, but serialization, network bytes and browser +reconciliation still occur for each subscriber. A downstream replay protocol would need its own +cursor, scope/identity/projection binding, retained history, expiry, authorization rechecks and +snapshot fallback. A resource UID, an object RV, and a per-connection `seq` are not such a cursor. + +Illustrative scale estimate, not a benchmark: if a projected snapshot is 1 MB, reconnecting 200 +subscribers transfers roughly 200 MB before compression, regardless of how few upstream watches +exist. The same fan-out occurs when a routine shared upstream watch closure triggers fresh cycles +for all subscribers, even with healthy browser connections: `sharedScope.die` ends each subscriber +with a recoverable resync. Measure reset counts by cause before calling either source dominant. +Upstream continuation is a named follow-up, ahead of any downstream replay design. + +## 8. Version delivery options and the abstraction cost + +| Option | Wire impact | Benefit | Cost / conflict with current contract | +|---|---|---|---| +| Clarify current semantics and improve host save outcomes | None | Smallest change; honest safety/progress story | Does not eliminate churn-induced 409s | +| Emit complete projected objects when only RV changes | No new event shape | More Kubernetes-like version delivery | Defeats status-only silence for spec projection; increases bytes | +| Add a version-only event | New consumer behavior | Smaller version notifications | More wakeups; a new metadata-update path; still races writes | +| Add a write-base/read-ticket abstraction | New host/server protocol | Could coordinate reads and intended writes | State, expiration, identity binding and replay concerns; too much core machinery now | +| Move host to SSA or targeted JSON Patch | No SSE change | Different write tradeoffs | Host policy/validation work; not interchangeable concurrency semantics | + +**Recommendation:** take the first option now. Prefer complete existing event shapes over a new +version-only event if measurements later justify version delivery. If changing a named projection's +promised suppression behavior, use an explicit new contract/projection identity or a coordinated +pre-1.0 change; do not silently repurpose `krm-spec/v1`. Pre-release naming flexibility is useful, +but it does not excuse ambiguous guarantees. + +Describe `krm-spec/v1` explicitly as “status is omitted and status-only changes are suppressed,” +rather than imply that it is the universally best editing mode. The root adoption path should keep +`krm-full/v1` as the default, with client requests subject to host policy. Any future version-delivery +mode should be an explicit choice, not a global switch that restores status notifications for everyone. + +## 9. UID recreation and error classification + +The endpoint checks UID before patching and includes the captured UID in the patch. Deletion and +recreation between those operations must never make the old draft modify the new object. Kubernetes +validates UID immutability, but validation/storage ordering can affect the returned error. +[Metadata update validation](https://github.com/kubernetes/apimachinery/blob/master/pkg/api/validation/objectmeta.go) + +The review's precise 422 claim is not established by our existing 409 test. Add a real API-server test +that forces this race, records the structured Kubernetes Status and verifies the replacement is +unchanged. If the host normalizes an identity-mismatch error to a conflict response, classify that +specific case. Keep schema/admission validation failures as validation errors; do not map every 422 +to 409. Preserve useful structured error information without exposing a raw protected object. + +## 10. Implementation plan and acceptance criteria + +The phases below explain the design work. The [implementation plan](0006-stream-and-save-implementation-plan.md) +separates the normative amendment, defines required tests, and promotes upstream continuation to its +own follow-up. Use that document for implementation order. + +### Phase A — clarify the existing contract before adding mechanisms + +1. Update saving.md with projection-specific costs and the distinction between safety and progress. + Explain client-requested versus host-selected projections, and distinguish content selection from + notification behavior. State that these are bundled today, not independently configurable. +2. Amend spec §6 and proposal 0004 together to define the RV exception precisely, or explicitly choose + to change emission behavior instead. Clarify §3's SSA wording and ownership/omission semantics. +3. Explain GET/upsert recovery for redaction metadata. A new stream is an option, not a requirement + after every rejected read; a quiet stream may still need explicit recovery. +4. Name SSA and JSON Patch as host-owned alternatives. State which helpers do not support them. +5. Keep public naming and runtime behavior unchanged in this phase; cross-link this decision. + +**Done when:** README, spec, proposal and save example make compatible claims about object versions, +projection completeness, redactions and writes. A reader can identify who chooses the projection, +whether status-only writes produce notifications, and cannot mistake “live” for “my save will pass.” + +### Phase B — test the composition at the public boundary + +1. Simplify healthy-retry tests using the existing test runner's fake timers: one budget-reset test, + one backoff-reset test. Avoid adding a public injectable-clock API just to accommodate tests. +2. Add a `Gateway.Stream` test: recheck in flight, routine upstream close, `RESYNC_REQUIRED`, fresh + `reset`/`synced`, no terminal error. Keep explicit-denial and timeout tests. +3. Add `krm-spec/v1` plus conditional-save coverage: status update suppressed, stale save returns a + real 409, projected GET updates RV without a draft conflict, new reviewed intent can succeed when + the controller is quiet. A continuing status writer must not cause an unbounded write retry loop. + Also overlap the post-409 GET with a resnapshot: pending snapshots and changed snapshot epochs + reject reconciliation. Preserve that guard and drafts, show recovery, and verify later progress. + A refused GET does not always require another read: a newer watch may already have won. +4. Add a contrast case for full projection and a bookkeeping-only change showing that full is not an + always-current-version promise either. +5. Force the UID replacement race and classify its actual response before changing error mapping. + +**Done when:** tests assert observable behavior and winner preservation, not merely helper return +values. They explain normal 409s without requiring imaginary user-visible conflicts. + +### Phase C — improve the host example without making the core a writer + +```mermaid +flowchart TD + S["Capture and submit conditional intent"] --> R{"Response"} + R -->|204| W["Saved, watch echo settles the store"] + R -->|409| G["Guarded most-recent projected read"] + R -->|"Other error"| E["Show classified host error"] + G --> A{"Read accepted?"} + A -->|No| N["Newer stream state or metadata recovery takes precedence"] + A -->|Yes| C{"Draft conflicts?"} + C -->|Yes| V["Review specific field conflicts"] + C -->|No| F["Base refreshed, explain changed version and offer a new save"] + F --> S + N --> Q{"Same UID and authoritative metadata ready?"} + Q -->|Yes| F + Q -->|No| P["Wait for recovery or open replacement as a new editor"] +``` + +The arrow back to Save is a new user action, not an automatic infinite retry. For unknown redaction +paths, wait for authoritative upsert metadata or deliberately request a fresh snapshot before retrying. +An identity replacement requires a new editor for the new UID, not another attempt with the old draft. + +Change the example's outcomes to distinguish stale version, actual draft conflicts, missing/replaced +resource and transport/validation error. This is host presentation, not a new wire event taxonomy. +Only add automatic conflict-free retry later with frozen-intent semantics, a finite budget and tests +for arrays and edits made while saving. Avoid casually broadening `captureSave` into a save controller. + +**Done when:** no user is asked to resolve nonexistent field conflicts, no replacement UID is written, +and no old patch is assigned a fresh RV without checking its intended changes. + +### Phase D — measure and decide separately + +Measure projected bytes, resnapshot counts, watch recycling, 409 rate by projection, and the fraction +of rejected writes that reconcile without field conflicts. Keep telemetry low-cardinality; avoid UID +or username labels. + +Evaluate upstream continuation before downstream replay. Design an SSA example only for a concrete +host ownership model. Consider a version-delivery mode only if measured write usability justifies +its notification cost. Compare status-aware dashboards, quiet spec viewers and spec editors before +choosing a default or adding a mode. Resolve whether a named projection or a separate delivery option +communicates the choice most simply, how the host constrains it, and how clients learn the effective +contract. Each can be a separate proposal and PR, with its own compatibility decision. + +## 11. Verification and scope of this document + +For implementation phases, run fixture checks, Go race tests, TypeScript tests, Vue tests, browser +and wire tests, lint and package validation. Run the projection/save and UID-race cases against a +real API server; fake clients do not establish Kubernetes conflict behavior. Push the implementation +changes according to the separate implementation plan and verify CI on their actual final heads. +PR #25 gates on corrected guidance, save outcomes and focused regression evidence; the normative +spec amendment and upstream continuation have separate review scopes. + +This document itself adds no runtime behavior, changes no named projection, and does not expand the +write API. The reviewer reports independent probes of retry and teardown recovery; those reports +are useful supporting feedback, not a substitute for the planned reproducible tests in this repo. +The UID-race status and any throughput estimates remain unverified here. + +The decision proposed for the current PR is deliberately small: make our guarantees precise, explain +the projection/write interaction, and strengthen the boundary tests. Do not remove Kubernetes write +preconditions or add an ownership/replay abstraction to conceal the tradeoff. diff --git a/docs/proposals/0006-stream-and-save-implementation-plan.md b/docs/proposals/0006-stream-and-save-implementation-plan.md new file mode 100644 index 0000000..7cda49a --- /dev/null +++ b/docs/proposals/0006-stream-and-save-implementation-plan.md @@ -0,0 +1,266 @@ +# Proposal 0006: Implementation plan for stream and save semantics + +**Status: proposed work breakdown. This document does not implement or approve runtime changes.** + +[Proposal 0005](0005-kubernetes-stream-and-save-semantics.md) explains the tradeoffs. This document +supersedes its phase list for sequencing, merge gates and acceptance criteria. Keep Kubernetes in +charge of identity and conditional writes, keep credentials and writes host-owned, and compose the +existing stream, shared backend and draft store. The original priorities remain managed recovery, +bounded subscriber reauthorization and a complete conditional-save example. + +## 1. Review decisions + +| Feedback | Decision | +|---|---| +| Separate the normative spec amendment from the feature PR | Agree. Review the convergence invariant in a dedicated PR, with release-note visibility. | +| Gate PR #25 on guidance and save outcomes | Agree, with focused regression tests included in the gate. Tests proving the corrected composition should not be deferred wholesale. | +| Copy the existing fake-timer pattern | Agree. `connection.test.ts` already demonstrates it; no public clock injection is needed. | +| Routine upstream closure resnapshots every shared subscriber | Confirmed in `sharedScope.pump` and `sharedScope.die`. Make continuation a named follow-up. | +| Upstream closure is the dominant source of resnapshots | Not established. It is a fan-out multiplier; dominance depends on closure frequency, browser reconnects and slow-consumer overflow. Measure before claiming it. | +| A snapshot can reject the post-409 GET | Confirmed. The guard also rejects responses spanning a snapshot epoch, even if that snapshot has finished. Preserve the guard and explain recovery. | +| A refused GET means the user must click again | Qualify. A newer watch may already have supplied the base. Re-read current state; wait when recovery is incomplete. Never infer the rejection reason from the boolean alone. | +| The five-second auth example disagrees with the default | No behavioral mismatch: the example explicitly overrides the timeout and the adjacent prose says zero uses ten seconds. Label the override more clearly. | + +The review's example of an identical SSA reapply advancing resourceVersion is not a guaranteed +fixture: a server may treat an operation as a no-op. Tests must demonstrate an actual persisted +change and a changed RV before asserting suppression. Neither SSA nor more frequent version +notifications replaces the conditional-save behavior we need to demonstrate. + +## 2. Work boundaries and merge gate + +**PR #25 is ready to merge when save outcomes and guidance are corrected, the focused recovery/save +regressions below pass, and CI passes on its final commit; normative spec changes and upstream +continuation belong to separately reviewed PRs.** + +This is the proposed gate, not a report that these checks have already run. If #25 has merged by +implementation time, put this same bounded correction in a follow-up PR. + +```mermaid +flowchart TD + A["PR 25 correction: guidance, save outcomes, regression tests"] --> G["Focused validation and final-commit CI"] + G --> M["Feature merge"] + S["Separate spec PR: convergence invariant and release note"] --> R["Explicit contract review"] + M --> U["Named follow-up: upstream watch continuation"] + U --> T["Continuity, cancellation, auth and fan-out tests"] + T --> V["Measure resnapshot and save behavior"] + V --> O["Optional later proposals only with a concrete need"] +``` + +The spec PR can be prepared alongside the correction. PR #25 should link the known invariant issue +and describe actual delivery behavior without silently rewriting the normative text. Resolve the +spec PR before the next release that presents this contract as settled. Separating review is not a +reason to leave contradictory guarantees indefinitely. + +## 3. PR #25 correction: guidance and a small host example + +### Documentation changes + +| Files | Concrete change | +|---|---| +| [Saving guide](../saving.md), [conditional-save README](../../examples/conditional-save/README.md) | Describe held RV as the last delivered revision for every projection. Explain safe rejection versus progress under churn, accepted GET base advancement, and arrays as whole-value merge-patch replacements. | +| [Store API comments](../../packages/krm-stream/src/store.ts) | Explain that an authoritative same-UID upsert can restore redaction metadata; a new snapshot is one recovery route. Keep response guards explicit. | +| [Auth guide](../auth.md) | Label five seconds as an example override of the ten-second default. Do not change the default to make an example match. | +| [Examples index](../../examples/README.md) and root README | Link one complete adoption path and distinguish optional managed recovery from low-level transport access. Avoid repeating the whole save guide. | + +Name SSA and deliberate JSON Patch as host-owned alternatives, with links to proposal 0005's +tradeoffs. Do not add another save engine. Explain that client-requested projections remain subject +to host policy and currently bundle content and notification behavior. Keep current projection +names and the full default in this correction. + +### Save outcomes + +Change [editor.ts](../../examples/conditional-save/editor.ts), with tests that exercise this exported +example directly. Keep a local result type in the example, not a new core API or wire event family. +The exact spelling may change during implementation; the following distinctions must survive: + +| Outcome | Meaning and host presentation | +|---|---| +| `unchanged` / `busy` | No patch, or a save is already in flight. | +| `saved` | The write succeeded. Keep drafts and let the watch settle the store. | +| `draft-conflict` | Current store state contains conflicting editable paths. Show those paths. | +| `version-stale` | The write returned 409 and current state is ready for a newly captured intent, without draft conflicts. Explain the changed base and offer Save again. | +| `recovering` | A usable base is not established. Preserve the draft and wait for stream/metadata recovery. | +| `unavailable` | The old UID is missing or the read identifies its replacement. Open a replacement as a separate editor. | + +Transport and validation failures should remain explicit host errors with useful status information; +they must not become `draft-conflict`. A successful PATCH response must never overwrite newer watch +state or clear edits typed while the request was in flight. + +The example already requires a live connection. Make that requirement executable through a small +host-supplied readiness callback using the existing managed connection state. Recheck it after awaits; +being live at the first click does not imply being live when the GET returns. Do not create a second +connection controller in the example. + +On 409, capture the reconciliation guard before GET, then inspect its result and current store state. +A rejected GET can mean newer data won, the resource disappeared, a snapshot intervened, or redaction +metadata is insufficient. The boolean is not an error taxonomy. If readiness cannot be established +from existing state, conservatively return `recovering`; do not add a public diagnostic API merely +to label every rejection. The example conservatively requires a later guarded GET accepted while +live before permitting a new write. A recovery click performs only that read; it does not clear the wait by itself. This +avoids subscriptions and diagnostic APIs at the cost of an extra read when a watch already won. + +```mermaid +flowchart TD + P["Submit captured UID, RV and patch"] --> H{"HTTP result"} + H -->|Success| S["Saved, preserve edits awaiting watch"] + H -->|409| G["Guarded projected GET"] + H -->|Other failure| E["Host error presentation"] + G --> I{"Old UID still available?"} + I -->|No| U["Unavailable, do not write replacement"] + I -->|Yes| R{"Current base ready after reconciliation?"} + R -->|No or uncertain| W["Recovering, preserve draft"] + R -->|Yes| C{"Current draft conflicts?"} + C -->|Yes| D["Review conflicting paths"] + C -->|No| V["Explain stale version, offer a new Save"] + V --> N["User captures a fresh intent"] + N --> P +``` + +No automatic write retry. No replacement of an old patch's RV with the GET's RV. No GET admitted as +snapshot membership. These constraints keep the example useful without turning it into a writer +framework. + +## 4. Regression coverage and follow-ups + +The final review narrows the immediate gate to adopter guidance, example outcomes with direct tests, +and the `Gateway.Stream` teardown/recovery regression. Connection-test simplification, real-API +status/save composition, and UID-race classification are follow-up hardening, not merge blockers. +The table below retains that broader backlog; it does not assert that every row shipped in PR #25. + +| Test location | Scenario and assertion | +|---|---| +| [Connection tests](../../packages/krm-stream/test/connection.test.ts) | Reuse `t.mock.timers` and the existing asynchronous flush pattern. Verify sustained health resets both budget and backoff, brief live periods still exhaust, and close/reset cancels the health timer. Preserve coverage while simplifying setup. | +| [Reauthorization tests](../../gateway/reauthorize_test.go) | Drive `Gateway.Stream`: recheck pending, upstream closes, subscriber sees recoverable error followed by reset/synced. Use synchronization barriers, not sleeps. Explicit denial and timeout remain terminal for only the affected subscriber. | +| Example tests, wired into the existing TypeScript test task | Exercise the actual `conditionalEditor` through a fake host request and real store: 409 with no field conflict, real draft conflict, newer watch/GET winning, edits during save, missing/replaced UID and request failure. No new test framework. | +| [Saving tests](../../packages/krm-stream/test/saving.test.ts) plus example tests | GET arriving during a snapshot and GET spanning a completed snapshot are refused. Drafts survive and snapshot pruning remains correct. A later authoritative update can restore readiness; no blind retry or endless click loop. | +| [Gateway stream tests](../../gateway/stream_test.go) | Spec suppresses a status-only RV change; full delivers that visible change. Both suppress an RV change whose only other changes are stripped bookkeeping metadata. | +| [Real API tests](../../gateway/kube/e2e_test.go) and [host handler tests](../../gateway/kube/examples/conditionalsave/handler_test.go) | Prove actual stale-RV 409 and preservation of the winning object. Combine wire/store tests with this API evidence rather than treating a fake 409 as proof of Kubernetes behavior. | + +For the status/save composition, use a resource with a real status subresource and an existing host +validation pattern. Do not pretend a ConfigMap's arbitrary `status` field establishes that behavior, +and do not generalize the ConfigMap example into an unrestricted write endpoint just for testing. +Require: status update advances RV, spec update is suppressed, stale PATCH returns 409, guarded GET +advances the base without a draft conflict, and a fresh intent succeeds once churn stops. + +Add one schedule combining churn and resnapshot: save starts while live, controller advances RV, +upstream closure starts a snapshot, and the post-409 GET arrives before synced. Assert recovery +presentation and later progress rather than asking the user to resolve an empty conflict list. + +Force deletion/recreation between the host's preflight read and PATCH using a test-only client +wrapper/barrier. Record the structured API Status and verify the replacement UID's contents remain +unchanged. Keep exact 409/422 classification evidence-based. Only normalize a specifically proven +identity-mismatch case; ordinary validation errors retain their meaning. This race test and any +resulting classification belong in follow-up hardening; do not change normalization before the +evidence exists. + +**Acceptance:** the original winner and drafts are preserved, outcomes describe current state, and +normal version rejection never requires imaginary field conflicts. The added tests run from existing +tasks and CI rather than relying on an unrecorded manual probe. + +## 5. Separate PR: define convergence precisely + +Change [spec/v1.md](../../spec/v1.md), [proposal 0004](0004-views-and-bytes.md), affected conformance +wording/tests and release notes together. Adopt proposal 0005's recommended convergence over projected +content excluding `metadata.resourceVersion`, plus redaction records, unless explicit review chooses +a different emission contract. + +State that delivered RV belongs to the delivered revision, suppressed updates do not refresh it, +and it is neither a freshness guarantee nor a downstream cursor. Preserve snapshot completeness, +pruning, ordering and per-connection redaction semantics. Distinguish stream quiescence from immediate +equality while updates are still in flight. + +Add a contract case where the final write changes only ignored metadata: no event is emitted, visible +content converges, and held RV remains older. Add the spec-only status counterpart. Existing event +fixtures need no changes unless the reviewed decision actually changes emissions. + +**Acceptance:** invariant, suppression rules, conformance assertions and save guidance agree. The +release note explicitly identifies the narrowed invariant. This is a normative correction with +unchanged intended runtime behavior, not a feature hidden in editorial cleanup. + +## 6. Named follow-up: continue upstream watches without resnapshotting subscribers + +Implement behind [the Kubernetes backend](../../gateway/kube/backend.go) and its existing `Watcher` +seam. Kubernetes supports continuing from a retained watch version and rebuilding state after history +expires. Use that upstream mechanism; downstream v1 remains snapshot-based on a new connection. +[Kubernetes watch semantics](https://kubernetes.io/docs/reference/using-api/api-concepts/#efficient-detection-of-changes) + +First assess the client-go watch utilities available in the repository's pinned version. Prefer a +compatible upstream utility when it preserves our initial-snapshot boundary, cancellation and error +semantics. Do not introduce an informer/cache hierarchy merely to restart a watch. Document why the +chosen utility fits, or why a small internal loop is needed. + +```mermaid +flowchart TD + I["Initial streaming list or list then watch"] --> B["Snapshot boundary established"] + B --> L["Consume live events and retain upstream checkpoint"] + L --> X{"Watch outcome"} + X -->|Routine close or retryable failure| R["Cancellable bounded backoff, reopen at checkpoint"] + R -->|Continuity retained| L + R -->|History expired or continuity lost| S["Existing RESYNC_REQUIRED and snapshot recovery"] + X -->|Terminal auth failure| T["Propagate terminal outcome"] + X -->|Stop or cancellation| C["Close watcher and timers"] + S --> I +``` + +Implementation constraints: + +- Retain checkpoints from upstream events and bookmarks before projection/suppression. Never derive + them from a browser object, SSE seq or shared projected view. Do not require bookmark cadence. +- Continue only with a trustworthy checkpoint and established initial boundary. A failed partial + snapshot must not be relabeled as live or completed by guessing. +- Preserve both streaming-list and list-then-watch paths, including aggregated APIs that reject + streaming lists. The [recorded cluster observations](../facts/observed-v1.36.2+k3s1.md) are evidence + for that environment, not universal claims about every Kubernetes API server. +- Reuse existing error and cancellation seams. Bound backoff and repeated failure handling so a dead + upstream cannot leave subscribers reporting live forever. Specify the exhaustion transition before + implementation; avoid adding public retry knobs without a demonstrated host need. +- Recheck the auth implications: fewer cycles mean fewer cycle-only authorization checks and fewer + `ClientFor` refresh calls. Timed subscriber checks must keep running during upstream recovery. + Document refreshing credentials and revocation expectations; do not accidentally weaken them. +- Keep single-subscriber overflow recovery and the shared cache's ownership intact. Last subscriber + departure must cancel an in-progress reconnect. No second shared-watch implementation. + +Test retained-history continuation with at least two subscribers: a routine close causes one upstream +reopen, no new downstream reset, and subsequent changes reach both. Test checkpoint progress on a +suppressed update/bookmark, deletion during the interruption, 410 fallback, partial initial snapshot, +forbidden response, repeated transient failure, cancellation and subscriber departure. Exercise both +initialization paths against the real API server where supported. + +Measure baseline and changed runs with the same workload: upstream reopen count, downstream reset +count by cause, serialized bytes, recovery duration and save 409 rate. Use existing observations where +possible. A 1 MB snapshot delivered to 200 subscribers costs roughly 200 MB before compression whether +triggered by browser reconnects or a shared upstream recycle; this is an illustration, not a measured +production rate. Successful continuation avoids that particular reset, not every possible resnapshot. + +**Acceptance:** retained-history recycling preserves downstream continuity, lost history still +recovers safely, retries stop correctly, and authorization/credential lifecycle expectations remain +explicit. No new SSE events or downstream replay protocol. + +## 7. Verification and completion + +For the correction, use the existing task definitions for client/example tests, Go tests with race +detection, wire/browser integration, lint, fixture checks and package validation. Verify any added +example test is actually discovered, not merely typechecked. Run the real API cases via +`task test-cluster`; attach the server version and results to the implementation PR. + +Current CI does not execute the real-cluster suite on every PR. In the follow-up hardening PR, add +a focused real-API job for the new save/identity cases using the existing cluster tooling, or wire them into an explicitly invoked +workflow whose successful run is part of the merge evidence. Keep the broader aggregated-API suite +available through `task test-cluster`; don't imply a fake-client job covers API behavior. Update Task +and workflow comments to match whichever coverage is implemented. + +After pushing each implementation PR, inspect checks on that exact commit and report required checks, +failures and any explicitly separate integration run. A previous green commit is not evidence for the +new code. Documentation-only edits need link and Mermaid validation, not a cluster rebuild. + +## 8. Deliberately deferred + +Version-only wire events, independent content/delivery switches, downstream replay, write tickets, +automatic conflict-free retry and a general SSA save abstraction need separate concrete use cases. +A future “omit status, report versions” contract must be client-requestable within host policy and +must not silently change spec-only subscribers' current quiet behavior. + +Keep the optional Vue adapter thin and outside core dependencies. Keep the clearer authorizer naming +already introduced; no further naming sweep is needed to solve these problems. The next useful work +is a precise contract and an example people can copy safely, followed by upstream continuation through +Kubernetes' existing watch mechanisms. diff --git a/docs/saving.md b/docs/saving.md index 00b3185..c265ce3 100644 --- a/docs/saving.md +++ b/docs/saving.md @@ -4,37 +4,40 @@ krm-stream is a read library. Your application owns its HTTP save endpoint, audi Kubernetes client. The client store produces a narrow RFC 7386 JSON merge patch; use `gateway.ValidateMergePatch` immediately before sending it to Kubernetes. -```go -func (s *server) saveConfigMap(w http.ResponseWriter, r *http.Request) { - user := userFromSession(r) - scope := authorizedScope(user, r) - patch := readRequestBody(r) - - // Read the current object using the same identity and effective projection decision as the stream. - object := s.currentObject(r.Context(), user, scope) - if err := gateway.ValidateMergePatch(gateway.ProjectionFull, object, patch); err != nil { - var violation *gateway.PatchViolation - if errors.As(err, &violation) { - http.Error(w, violation.Error(), http.StatusBadRequest) - return - } - http.Error(w, "invalid merge patch", http.StatusBadRequest) - return - } +Use the complete [conditional-save example](../examples/conditional-save/README.md): it includes a +compilable host endpoint, client reconciliation, race tests, and a real-cluster 409 test. - // Your app performs this ordinary Kubernetes PATCH as the signed-in user. - _, err := s.dynamicFor(user).Resource(configMaps).Namespace(scope.Namespace). - Patch(r.Context(), scope.Name, types.MergePatchType, patch, metav1.PatchOptions{}) - if err != nil { - http.Error(w, "save failed", http.StatusBadGateway) - return - } +```ts +const intent = store.captureSave(uid); +// intent = { uid, resourceVersion, patch }, detached and captured together before any await. +if (intent) await hostSave(intent); +``` - // 204, and NOT the object Kubernetes just handed back. See below. - w.WriteHeader(http.StatusNoContent) -} +The host validates the patch and adds `metadata.uid` and `metadata.resourceVersion` from that intent +before sending a Kubernetes merge PATCH. It must not substitute the version from a newer GET. +Kubernetes performs the version check atomically with the write. Propagate a real Kubernetes 409 +as HTTP 409; do not turn all upstream errors into 502. + +A narrow patch limits which fields are written; it does **not** protect against concurrent edits. +JSON merge patch replaces arrays in full, including arrays the client reconciles by keyed items. +See Kubernetes' [conditional update guidance](https://kubernetes.io/docs/reference/using-api/api-concepts/#updates-to-existing-resources). + +On 409, keep the draft and reconcile a fresh, complete **projected** GET. Capture the response guard +before starting that GET: + +```ts +const reconcile = store.captureReconciliation(uid); +const { object, redactedPaths } = await hostRead(); +reconcile(object, { redactedPaths }); +// false can also mean snapshot recovery or unknown redaction paths. Do not force it. ``` +Local edits made during the request survive reconciliation. Render the current draft and conflicts; +let the user review before capturing another save intent. Serialize saves per editor. A host GET must +be a most-recent Kubernetes read with no intermediary cache and the same projection as the stream. +Do not apply a response for a replacement UID to the old editor. Deleted-object drafts are removed by +the store; archive them outside the store if the application needs recovery after deletion. + ## What the guard rejects - a value declared in `redacted`, including deletion of a parent map such as `data: null`; @@ -47,9 +50,14 @@ It does not grant write permission, choose a projection, fetch the object, issue optimistic concurrency. Those stay with the host. Do not use whole-object `PUT`: projected objects are intentionally incomplete, and a `PUT` can delete fields the browser never saw. -`metadata.resourceVersion` may be stale when `krm-spec/v1` suppresses invisible status churn. Do not -use the streamed value as a write precondition. The client-side three-way merge surfaces conflicts in -the fields the user can see; send only the user's explicit merge-patch changes. +Every projection can hold an older `metadata.resourceVersion`: all suppress changes to stripped +bookkeeping metadata, and `krm-spec/v1` additionally suppresses status-only changes. A version +precondition remains safe: rejection prevents a lost update. Sustained invisible churn can prevent +save progress, but a 409 is a failed version precondition, not necessarily a disagreement at an +editable field. An accepted projected GET advances the base without requiring a snapshot. Render +actual draft conflicts separately; when none exist, explain the refreshed base and offer a newly +captured save. If reconciliation is refused, preserve the draft and recover before writing again. +Do not remove concurrency protection or blindly retry the old patch with a newer version. ## Answer 204 and let the watch echo it @@ -65,17 +73,18 @@ clear and nothing to adopt. The echo settles it. ## If you must answer with the object -`store.adoptSaved(object)` is for a host that already holds a **projected** object, such as one doing -its own optimistic update. Project it first: - -```go -projected, redacted := gateway.Project(gateway.ProjectionFull, result) -_ = redacted // the paths withheld; the client keeps the redactions it already has -writeJSON(w, projected) -``` - -`gateway.Project` applies the same projection the stream applies. Never hand `adoptSaved` an object -straight from the Kubernetes client. +Prefer 204. If a host returns an object, capture `store.captureReconciliation(uid)` **before** the +save request and apply its projected response through that guard. This prevents a delayed response +from overwriting a newer watch event, and preserves local edits made while saving. + +`store.adoptSaved(object)` remains available for synchronous adoption and newly created objects. It +is unguarded and must not receive delayed responses that can race the watch. Never adopt a raw +Kubernetes response: project it first and provide the correct redaction metadata. Hosts exposing +redacted resources can return `redactedPaths` directly from `gateway.Project`. The guard retains +known stream revisions for paths still present and removes paths absent from that list. Omitted +redaction metadata preserves existing protections. Unknown paths reject the entire response: open a +later authoritative upsert for that UID or a fresh stream snapshot before retrying reconciliation. Never invent revision counters for a GET. +An explicit `redacted` array is still supported when the host has authoritative stream revisions. ## Creating and deleting whole objects diff --git a/docs/vue.md b/docs/vue.md new file mode 100644 index 0000000..53478b7 --- /dev/null +++ b/docs/vue.md @@ -0,0 +1,26 @@ +# Vue integration + +A copyable Vue 3 composable covers the adapter: reactive snapshots for drafts, changes, conflicts, +redactions and connection state, plus automatic subscription cleanup. Vue remains a host dependency; +the core package and its browser bundle have no framework dependencies. + +The [composable source](../examples/vue/useLiveResource.ts) and +[tests](../examples/vue/useLiveResource.test.ts) are typechecked and executed by `task test-vue` and CI. +Copy the source into your host and change its library import to `@configbutler/krm-stream`. + +```ts +const { resource, state } = useLiveResource(store, uid, connection); +``` + +Use `resource.value?.draft` in script and `resource?.draft` in a template. A missing/deleted UID gives +`null`. For a changing selection, remount a keyed editor or create a new effect scope. Use store +methods such as `setValue`, `removeKey` and conflict resolution methods for edits: do not bind +`v-model` directly to the draft snapshot. Store reads are detached copies, and editing them bypasses +policy checks and change notifications. + +The connection belongs to its creator. A parent sharing one connection across many editors closes +it when the parent unmounts; an editor that creates its own connection can register +`onScopeDispose(() => connection.close())`. Removing one editor's subscriptions must not close a +connection used by other editors. Capture and submit saves with the +[conditional-save example](../examples/conditional-save/README.md); expose request errors and saving +state from that host-owned controller. Enable Save while `state.status === "live"`. diff --git a/docs/why-a-gateway.md b/docs/why-a-gateway.md index 5d598f2..fdc8eeb 100644 --- a/docs/why-a-gateway.md +++ b/docs/why-a-gateway.md @@ -57,7 +57,7 @@ one identity: your service account. Without sharing, the client acts as the call enforces the boundary, so no bug in this library can hand someone an object they may not see. With sharing, your `Authorizer` is the only thing between a caller and the cache. -You can have both. Pair `SharedBackend` with [`kube.SSARAuthorizer`](../gateway/kube/authz.go), which +You can have both. Pair `SharedBackend` with [`kube.SubjectAccessReviewAuthorizer`](../gateway/kube/authz.go), which asks the API server through a SubjectAccessReview whether this user may list and watch this resource here, before serving them from the shared cache. Kubernetes decides again, per user, per snapshot cycle, at the cost of one round-trip. Read [auth.md](auth.md) before wiring it. diff --git a/examples/README.md b/examples/README.md index 13c05e6..7c1725e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -9,3 +9,8 @@ For host integration patterns, use these small recipes: explicit `Authorization` header for a deliberate token-bearing client. - [Shared backend with SSAR](../docs/adopting.md#4-share-watches-only-with-kubernetes-backed-authorization): one service-account watch plus Kubernetes SubjectAccessReviews for each subscriber. + +[Conditional save](conditional-save/README.md) composes a managed connection with atomic save capture, +projected reconciliation, and a host-owned Kubernetes endpoint that preserves real 409 conflicts. + +[Vue adapter](vue/README.md) is a copyable, typechecked composable with reactivity and cleanup tests. diff --git a/examples/conditional-save/README.md b/examples/conditional-save/README.md new file mode 100644 index 0000000..1875c46 --- /dev/null +++ b/examples/conditional-save/README.md @@ -0,0 +1,66 @@ +# Conditional save with a live draft + +This example composes the existing store, managed stream and host-owned writes. It adds no merge +algorithm or shared watch implementation. + +- [editor.ts](editor.ts) captures a save intent synchronously, handles HTTP 409 by reconciling a + projected GET, retains in-flight edits and rejects late responses superseded by the watch. +- [handler.go](../../gateway/kube/examples/conditionalsave/handler.go) is a compilable ConfigMap + GET/PATCH endpoint. Mount it on a fixed namespace/name with a host-authenticated Kubernetes client + acting as the caller. The host owns credentials, CSRF protection, route authorization and audit. +- [saving tests](../../packages/krm-stream/test/saving.test.ts) exercise the response races. +- `TestConditionalSaveConflict` in the Kubernetes e2e suite exercises this handler against a real API + server: a competing update makes a captured save return 409 without overwriting the winner. + +```ts +const store = new LiveResourceStore(); +const connection = connectManagedResourceStream(streamURL, store, { + onStateChange: state => renderConnection(state), +}); +const editor = conditionalEditor( + store, uid, "/editor/configmap", hostFetch, + () => connection.state.status === "live", +); +const unsubscribe = store.subscribe(renderEditor); +// Enable Save only while connection.state.status === "live" and !editor.saving. +// On Save: await editor.save(), then render errors/conflicts and the current draft. +// On unmount: +unsubscribe(); +connection.close(); +``` + +Use the same scope and `krm-full/v1` projection on the stream and this example endpoint. The GET +returns a complete projected object with redacted paths; it must use a most-recent Kubernetes read, with +no HTTP or application cache. A recreated name has a different UID and must be opened as a new editor. +Keep an external draft archive if users need to recover edits after deletion: the store intentionally +removes drafts of deleted objects. + +A narrow merge patch is **not concurrency protection**. JSON merge patch replaces arrays as a whole, +even when the client merges keyed list items intelligently. This endpoint puts the captured UID and +resourceVersion into the Kubernetes patch. A stale version produces a safe 409, including when +bookkeeping-only changes were suppressed by the full projection. Spec projection also suppresses +status-only updates. This is safe rejection, but sustained churn can hinder save progress. An accepted +GET advances the base without requiring a snapshot. Reconcile first; never transplant an old +patch onto the latest version. No automatic write retry is performed. + +Successful writes return 204. The stream echo settles the saved values while retaining later edits. +If the echo is delayed, dirty state remains visible; prevent repeated saves until your host's chosen +acknowledgment UX allows them. Save results never feed raw Kubernetes objects back into the store. + +The GET returns `redactedPaths` from `gateway.Project`, without stream revision counters. The client +preserves revisions for known paths. Unknown redaction paths reject reconciliation; a later +authoritative upsert for that UID or a fresh stream snapshot supplies the missing metadata. +A host with authoritative stream revisions may instead return `redacted`; the editor forwards +that array when `redactedPaths` is absent. If both are supplied, `redactedPaths` takes precedence. +Omitted redaction metadata never clears protection. + +The example distinguishes `draft-conflict` (show conflicting paths), `version-stale` (base refreshed, +o field disagreement), `recovering` (a usable base is not established), and `unavailable` (missing or +replacement UID), alongside `unchanged`, `busy` and `saved`. Transport and validation failures throw +host errors. The live-state callback is checked before saving and after reconciliation. + +After a refused read, `recovering` preserves the draft. Wait for stream recovery or authoritative +metadata, then the next Save performs only a guarded GET. An accepted read returns `version-stale` +(or actual draft conflicts); a subsequent user action captures a fresh write intent. This conservative +example may perform an extra read when a newer watch already won. It never guesses why the guard +returned false, installs a background retry loop, or retries a PATCH automatically. diff --git a/examples/conditional-save/editor.ts b/examples/conditional-save/editor.ts new file mode 100644 index 0000000..37e3d18 --- /dev/null +++ b/examples/conditional-save/editor.ts @@ -0,0 +1,80 @@ +import type { KRMObject, LiveResourceStore, Redaction } from "../../packages/krm-stream/src/index.ts"; + +export type SaveOutcome = + | "unchanged" + | "busy" + | "saved" + | "draft-conflict" + | "version-stale" + | "recovering" + | "unavailable"; + +/** Copy into the host. request owns session/CSRF/error policy; isLive reads managed connection state. */ +export function conditionalEditor( + store: LiveResourceStore, + uid: string, + url: string, + request: typeof fetch, + isLive: () => boolean, +) { + let saving = false; + let needsRead = false; + async function refresh(): Promise { + needsRead = true; + const reconcile = store.captureReconciliation(uid); + const latest = await request(url, { cache: "no-store" }); + if (latest.status === 404) return "unavailable"; + if (!latest.ok) throw new Error(`Reconciliation failed: HTTP ${latest.status}`); + const body = (await latest.json()) as { + object: KRMObject; + redactedPaths?: string[]; + redacted?: Redaction[]; + }; + if (!store.ids().includes(uid) || body.object.metadata.uid !== uid) return "unavailable"; + const accepted = reconcile( + body.object, + body.redactedPaths !== undefined + ? { redactedPaths: body.redactedPaths } + : body.redacted !== undefined + ? { redacted: body.redacted } + : undefined, + ); + // false has several causes. Do not infer readiness from it or force a refused response. + // A later Save attempts only another guarded read until one is accepted while live. + if (!isLive()) return "recovering"; + needsRead = !accepted; + if (store.conflicts(uid).length) return "draft-conflict"; + return accepted ? "version-stale" : "recovering"; + } + return { + get saving() { + return saving; + }, + async save(): Promise { + if (saving) return "busy"; + if (!store.ids().includes(uid)) return "unavailable"; + if (!isLive()) return "recovering"; + if (store.conflicts(uid).length) return "draft-conflict"; + saving = true; + try { + if (needsRead) return await refresh(); + const intent = store.captureSave(uid); // Patch, UID and base captured before any await. + if (!intent) return "unchanged"; + const result = await request(url, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(intent), + }); + if (result.status === 409) { + if (!store.ids().includes(uid)) return "unavailable"; + return await refresh(); + } + if (!result.ok) throw new Error(`Save failed: HTTP ${result.status}`); + // Never adopt a save response or clear drafts: edits made during the write must survive. + return "saved"; + } finally { + saving = false; + } + }, + }; +} diff --git a/examples/vanilla-browser/index.html b/examples/vanilla-browser/index.html index 987ab8d..7ef553c 100644 --- a/examples/vanilla-browser/index.html +++ b/examples/vanilla-browser/index.html @@ -5,7 +5,7 @@ No framework. No bundler. No build step for THIS page. One