From 91f5bf647860b66884fbd2f185fa95cd3e4eb671 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:52:12 +0300 Subject: [PATCH 1/6] feat(console): add immutable schema-path helpers Introduce pure helpers used to recognise and enforce Kubernetes-style immutability rules on resource schemas. findImmutablePaths(schema) walks an OpenAPI/JSON-schema tree and returns every path whose subschema carries an x-kubernetes-validations entry with rule "self == oldSelf". Object properties contribute named segments; array items and additionalProperties contribute the wildcard segment "*". oneOf, anyOf and allOf branches are considered when determining whether a node is immutable. overlayImmutable(submitted, original, paths) deep-clones the submitted value, then for each path copies the value from original. Wildcard semantics dispatch on the runtime value shape: - *-last on an array: replace with a clone of source (whole-array immutable). - *-last on an object map: replace with a clone of source (whole-map immutable). - *-not-last on an array: iterate the user's array, restore the immutable nested subfield from source at shared indices, keep user-added entries verbatim. Length shrinks are detected and the overlay is skipped with a console.warn so admission stays the authoritative enforcer (index-aligned overlay would otherwise silently swap which element the user deleted). - *-not-last on an object map: freeze the whole map. The UI marks such fields ui:disabled (Add/Remove hidden); the overlay matches so YAML-editor and devtools bypasses can't add or rename keys. A root-level immutable path or a top-level wildcard immutable path (both pathological schema shapes) replaces submitted wholesale with a clone of original and logs a warning. IMMUTABLE_HELP_TEXT centralises the user-facing string consumed by later UI wiring. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- apps/console/src/lib/immutable-paths.test.ts | 474 +++++++++++++++++++ apps/console/src/lib/immutable-paths.ts | 249 ++++++++++ 2 files changed, 723 insertions(+) create mode 100644 apps/console/src/lib/immutable-paths.test.ts create mode 100644 apps/console/src/lib/immutable-paths.ts diff --git a/apps/console/src/lib/immutable-paths.test.ts b/apps/console/src/lib/immutable-paths.test.ts new file mode 100644 index 0000000..a423d5b --- /dev/null +++ b/apps/console/src/lib/immutable-paths.test.ts @@ -0,0 +1,474 @@ +import { describe, it, expect, vi } from "vitest" +import { findImmutablePaths, overlayImmutable } from "./immutable-paths.ts" + +const cel = { + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "immutable" }, + ], +} + +describe("findImmutablePaths", () => { + it("returns empty array for non-object input", () => { + expect(findImmutablePaths(null)).toEqual([]) + expect(findImmutablePaths(undefined)).toEqual([]) + expect(findImmutablePaths("string")).toEqual([]) + expect(findImmutablePaths(42)).toEqual([]) + }) + + it("returns empty array when no validations are present", () => { + expect( + findImmutablePaths({ + type: "object", + properties: { foo: { type: "string" } }, + }), + ).toEqual([]) + }) + + it("ignores validations whose rule is not self == oldSelf", () => { + expect( + findImmutablePaths({ + properties: { + foo: { + type: "string", + "x-kubernetes-validations": [{ rule: "self.size() > 0" }], + }, + }, + }), + ).toEqual([]) + }) + + it("finds a top-level immutable property", () => { + const schema = { + properties: { + storageClass: { type: "string", ...cel }, + size: { type: "string" }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["storageClass"]]) + }) + + it("finds nested immutable properties", () => { + const schema = { + properties: { + spec: { + properties: { + backup: { + properties: { + storageClass: { type: "string", ...cel }, + }, + }, + }, + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([ + ["spec", "backup", "storageClass"], + ]) + }) + + it("uses \"*\" for array items", () => { + const schema = { + properties: { + disks: { + type: "array", + items: { type: "string", ...cel }, + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["disks", "*"]]) + }) + + it("descends into array items to find nested immutable fields", () => { + const schema = { + properties: { + disks: { + type: "array", + items: { + type: "object", + properties: { + name: { type: "string", ...cel }, + size: { type: "string" }, + }, + }, + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["disks", "*", "name"]]) + }) + + it("uses \"*\" for additionalProperties object maps", () => { + const schema = { + properties: { + labels: { + type: "object", + additionalProperties: { type: "string", ...cel }, + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["labels", "*"]]) + }) + + it("treats parent as immutable when a oneOf branch carries the rule", () => { + const schema = { + properties: { + size: { + oneOf: [{ type: "integer" }, { type: "string", ...cel }], + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["size"]]) + }) + + it("treats parent as immutable when anyOf/allOf carries the rule", () => { + expect( + findImmutablePaths({ + properties: { + foo: { anyOf: [{ ...cel }] }, + }, + }), + ).toEqual([["foo"]]) + expect( + findImmutablePaths({ + properties: { + bar: { allOf: [{ ...cel }] }, + }, + }), + ).toEqual([["bar"]]) + }) + + it("handles root-level immutable schema (no properties)", () => { + expect(findImmutablePaths(cel)).toEqual([[]]) + }) + + it("FIXME(#8): does NOT walk into properties beneath oneOf/anyOf/allOf branches", () => { + // The walker recognises *parent* immutability when any branch carries + // the rule, but it does not descend into a branch's `properties` to + // discover per-field immutability inside the branch. CRDs that use + // allOf composition for property merging are not covered. + // Pinned as current behaviour; revisit if a real schema needs it. + const schema = { + type: "object", + allOf: [ + { + properties: { + storageClass: { type: "string", ...cel }, + }, + }, + ], + } + expect(findImmutablePaths(schema)).toEqual([]) + }) + + it("collects multiple immutable paths in deterministic order", () => { + const schema = { + properties: { + a: { type: "string", ...cel }, + b: { + properties: { + c: { type: "string", ...cel }, + }, + }, + }, + } + expect(findImmutablePaths(schema)).toEqual([["a"], ["b", "c"]]) + }) +}) + +describe("overlayImmutable", () => { + it("returns a deep clone when paths is empty", () => { + const submitted = { foo: "x", arr: [1, 2] } + const original = { foo: "y", arr: [9] } + const result = overlayImmutable(submitted, original, []) + expect(result).toEqual(submitted) + expect(result).not.toBe(submitted) + expect((result as { arr: number[] }).arr).not.toBe(submitted.arr) + }) + + it("overlays a top-level immutable field", () => { + const submitted = { storageClass: "fast", size: "10Gi" } + const original = { storageClass: "slow", size: "5Gi" } + expect( + overlayImmutable(submitted, original, [["storageClass"]]), + ).toEqual({ storageClass: "slow", size: "10Gi" }) + }) + + it("preserves siblings when overlaying nested fields", () => { + const submitted = { + spec: { backup: { storageClass: "fast", schedule: "*/5" } }, + } + const original = { + spec: { backup: { storageClass: "slow", schedule: "*/1" } }, + } + expect( + overlayImmutable(submitted, original, [ + ["spec", "backup", "storageClass"], + ]), + ).toEqual({ + spec: { backup: { storageClass: "slow", schedule: "*/5" } }, + }) + }) + + it("inserts the immutable field if it is missing from submitted but present in original", () => { + const submitted = { size: "10Gi" } as Record + const original = { size: "5Gi", storageClass: "slow" } + expect( + overlayImmutable(submitted, original, [["storageClass"]]), + ).toEqual({ size: "10Gi", storageClass: "slow" }) + }) + + it("keeps the submitted value when the immutable field is absent from original", () => { + // Persisted spec doesn't have the field (server-default, status echo gap, + // freshly-loaded resource). overlay must not blank user's input with undefined. + expect( + overlayImmutable( + { storageClass: "fast", size: "10Gi" }, + { size: "5Gi" } as Record, + [["storageClass"]], + ), + ).toEqual({ storageClass: "fast", size: "10Gi" }) + }) + + it("aligns array length to original when array element path is immutable", () => { + const submitted = { disks: ["x", "y", "z"] } + const original = { disks: ["a", "b"] } + expect( + overlayImmutable(submitted, original, [["disks", "*"]]), + ).toEqual({ disks: ["a", "b"] }) + }) + + it("extends submitted array when original is longer", () => { + const submitted = { disks: ["x"] } + const original = { disks: ["a", "b"] } + expect( + overlayImmutable(submitted, original, [["disks", "*"]]), + ).toEqual({ disks: ["a", "b"] }) + }) + + it("overlays nested array-item field when only that field is immutable", () => { + const submitted = { + disks: [ + { name: "renamed1", size: "10Gi" }, + { name: "renamed2", size: "20Gi" }, + ], + } + const original = { + disks: [ + { name: "disk1", size: "5Gi" }, + { name: "disk2", size: "5Gi" }, + ], + } + expect( + overlayImmutable(submitted, original, [["disks", "*", "name"]]), + ).toEqual({ + disks: [ + { name: "disk1", size: "10Gi" }, + { name: "disk2", size: "20Gi" }, + ], + }) + }) + + it("tolerates null/undefined values along the path without blanking the user input", () => { + expect( + overlayImmutable({}, { storageClass: "slow" }, [["storageClass"]]), + ).toEqual({ storageClass: "slow" }) + // Source has no value at the path: keep target's value rather than + // erasing the field — see "keeps submitted value when … absent from original". + expect( + overlayImmutable({ storageClass: "fast" }, {}, [["storageClass"]]), + ).toEqual({ storageClass: "fast" }) + }) + + it("skips overlay (with warn) when the user shrunk an array with per-element-nested-immutable fields", () => { + // Index-aligned overlay corrupts which element was deleted when the + // length changed (it re-anchors source values onto surviving indices). + // Skip the overlay for this path and let admission enforce the rule. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + const submitted = { disks: [{ name: "renamed", size: "10Gi" }] } + const original = { + disks: [ + { name: "disk1", size: "5Gi" }, + { name: "disk2", size: "5Gi" }, + ], + } + expect( + overlayImmutable(submitted, original, [["disks", "*", "name"]]), + ).toEqual({ disks: [{ name: "renamed", size: "10Gi" }] }) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + it("skips overlay (with warn) when the user deleted the first of two immutable-named entries", () => { + // Direct repro of the index-anchored swap bug discovered in review: + // submitted shrinks from 2 to 1 and the naive overlay would assign + // source[0].name to the surviving entry. We bail instead. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + const submitted = { disks: [{ name: "b", size: "2Gi" }] } + const original = { + disks: [ + { name: "a", size: "1Gi" }, + { name: "b", size: "2Gi" }, + ], + } + expect( + overlayImmutable(submitted, original, [["disks", "*", "name"]]), + ).toEqual({ disks: [{ name: "b", size: "2Gi" }] }) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + it("warns and clones original when a top-level wildcard immutable path is supplied", () => { + // findImmutablePaths emits [["*"]] for schemas whose root is an array + // or map carrying the rule on items/additionalProperties. The naive + // overlay would no-op (the loop's return value was discarded). Match + // the root-immutable handling: warn and replace wholesale. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + const submitted = ["x", "y"] + const original = ["a", "b"] + expect(overlayImmutable(submitted, original, [["*"]])).toEqual(original) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + it("freezes a whole additionalProperties map when * is the last segment", () => { + // The schema declares every value in the map immutable. We treat this as + // whole-map immutable: defence-in-depth must reject mutations from the + // YAML editor or devtools, not just the form. + type LabelsObj = { labels: Record } + const submitted: LabelsObj = { + labels: { env: "prod-changed", added: "new" }, + } + const original: LabelsObj = { + labels: { env: "prod", legacy: "v1" }, + } + expect( + overlayImmutable(submitted, original, [["labels", "*"]]), + ).toEqual({ labels: { env: "prod", legacy: "v1" } }) + }) + + it("freezes the whole map even when only a nested value subkey carries the rule", () => { + // Previously the overlay merged per-entry for *-not-last on a map, + // which let a YAML-editor bypass add new keys that the form would + // have refused. The UI marks the whole map ui:disabled; the overlay + // now matches: drop added keys, reinstate removed keys, reset every + // value to source. Different-content semantics (allow add) would need + // both sides to change together — see PR description's UX trade-off. + type Entry = { value: string; note: string } + type LabelsObj = { labels: Record } + const submitted: LabelsObj = { + labels: { + env: { value: "changed", note: "edit" }, + new: { value: "novel", note: "added" }, + }, + } + const original: LabelsObj = { + labels: { + env: { value: "prod", note: "orig" }, + legacy: { value: "v1", note: "old" }, + }, + } + expect( + overlayImmutable(submitted, original, [["labels", "*", "value"]]), + ).toEqual({ + labels: { + env: { value: "prod", note: "orig" }, + legacy: { value: "v1", note: "old" }, + }, + }) + }) + + it("FIXME(#9): does NOT materialise an immutable leaf when its ancestor is missing in target", () => { + // Defence-in-depth gap: if the YAML editor strips the parent object + // entirely, overlay cannot reach the leaf to copy the original value. + // Pin behaviour so a future contributor fixing the gap notices the + // test and revises it. + const submitted = { spec: {} } as Record + const original = { + spec: { backup: { storageClass: "slow" } }, + } + const result = overlayImmutable(submitted, original, [ + ["spec", "backup", "storageClass"], + ]) + expect(result).toEqual({ spec: {} }) + }) + + it("FIXME(#10): array reordering by the user with index-aligned overlay re-anchors source values to the new index", () => { + // The overlay walks by index, not by content identity. Reordering an + // immutable array maps source[i] onto target[i]'s new value — which + // for a per-element-name-immutable schema swaps the names back to the + // source order. Pinned as current behaviour. Identity-based matching + // (e.g. by a stable id field) would be a separate change. + const submitted = { + disks: [ + { name: "b", size: "2Gi" }, + { name: "a", size: "1Gi" }, + ], + } + const original = { + disks: [ + { name: "a", size: "1Gi" }, + { name: "b", size: "2Gi" }, + ], + } + expect( + overlayImmutable(submitted, original, [["disks", "*", "name"]]), + ).toEqual({ + disks: [ + { name: "a", size: "2Gi" }, + { name: "b", size: "1Gi" }, + ], + }) + }) + + it("does not mutate inputs", () => { + const submitted = { storageClass: "fast", arr: [1, 2] } + const original = { storageClass: "slow", arr: [9] } + overlayImmutable(submitted, original, [["storageClass"]]) + expect(submitted).toEqual({ storageClass: "fast", arr: [1, 2] }) + expect(original).toEqual({ storageClass: "slow", arr: [9] }) + }) + + it("warns and returns a clone of original when the root path is immutable", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + const submitted = { foo: "x", bar: "y" } + const original = { foo: "X", bar: "Y" } + const result = overlayImmutable(submitted, original, [[]]) + expect(result).toEqual(original) + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0][0]).toMatch(/root-level immutable/i) + warn.mockRestore() + }) + + it("keeps user-added array entries with their submitted (non-source-derived) name", () => { + // User added two entries past the original length. The nested + // immutable field has no source counterpart for those new indices, + // so the submitted values must survive unchanged. + const submitted = { + disks: [ + { name: "a", size: "1Gi" }, + { name: "b", size: "2Gi" }, + { name: "c", size: "3Gi" }, + ], + } + const original = { disks: [{ name: "A", size: "9Gi" }] } + expect( + overlayImmutable(submitted, original, [["disks", "*", "name"]]), + ).toEqual({ + disks: [ + { name: "A", size: "1Gi" }, + { name: "b", size: "2Gi" }, + { name: "c", size: "3Gi" }, + ], + }) + }) + + it("does not access prototype methods unsafely when the field is missing on both sides", () => { + // Regression: targetObj.hasOwnProperty was used directly, tripping + // no-prototype-builtins. Cover the path explicitly: neither side has + // the immutable field — overlay must be a no-op without throwing. + const submitted = { other: "x" } as Record + const original = { other: "y" } as Record + expect( + overlayImmutable(submitted, original, [["storageClass"]]), + ).toEqual({ other: "x" }) + }) +}) diff --git a/apps/console/src/lib/immutable-paths.ts b/apps/console/src/lib/immutable-paths.ts new file mode 100644 index 0000000..34acc4b --- /dev/null +++ b/apps/console/src/lib/immutable-paths.ts @@ -0,0 +1,249 @@ +/** + * Discovery and enforcement of Kubernetes-style immutability rules + * (`x-kubernetes-validations: [{rule: "self == oldSelf"}]`) on OpenAPI/JSON + * schemas served by Cozystack ApplicationDefinitions and CRDs. + * + * The UI strips this extension before handing the schema to AJV (which has no + * CEL evaluator); the paths it announces are used to (a) mark form fields as + * read-only in edit mode and (b) overlay the original values into PUT bodies + * so the API server never observes a change on those paths. + */ + +export type ImmutablePath = readonly string[] + +export const IMMUTABLE_HELP_TEXT = + "This field cannot be changed after creation." + +const IMMUTABILITY_RULE = "self == oldSelf" + +const isPlainObject = (v: unknown): v is Record => + v !== null && typeof v === "object" && !Array.isArray(v) + +const hasImmutabilityRule = (node: unknown): boolean => { + if (!isPlainObject(node)) return false + const validations = node["x-kubernetes-validations"] + if (!Array.isArray(validations)) return false + return validations.some( + (v) => isPlainObject(v) && v.rule === IMMUTABILITY_RULE, + ) +} + +const branchHasImmutability = (node: unknown): boolean => { + if (!isPlainObject(node)) return false + if (hasImmutabilityRule(node)) return true + for (const key of ["oneOf", "anyOf", "allOf"] as const) { + const branches = node[key] + if (Array.isArray(branches) && branches.some(branchHasImmutability)) { + return true + } + } + return false +} + +export function findImmutablePaths(schema: unknown): ImmutablePath[] { + const out: string[][] = [] + walk(schema, [], out) + return out +} + +function walk(node: unknown, path: string[], out: string[][]): void { + if (!isPlainObject(node)) return + + if (branchHasImmutability(node)) { + out.push([...path]) + return + } + + const properties = node.properties + if (isPlainObject(properties)) { + for (const [key, value] of Object.entries(properties)) { + walk(value, [...path, key], out) + } + } + + const items = node.items + if (isPlainObject(items)) { + walk(items, [...path, "*"], out) + } + + const additional = node.additionalProperties + if (isPlainObject(additional)) { + walk(additional, [...path, "*"], out) + } +} + +export function overlayImmutable( + submitted: T, + original: T, + paths: readonly ImmutablePath[], +): T { + if (paths.some((p) => p.length === 0)) { + // The schema flags the whole subtree as immutable; whatever the user + // changed in the form is dropped on the floor. That's almost always a + // schema-authoring mistake (rule belongs on a property, not the root) + // so make it visible in the console. + console.warn( + "overlayImmutable: a root-level immutable path was supplied; " + + "the submitted value is replaced wholesale by the original.", + ) + return deepClone(original) + } + if (paths.some((p) => p[0] === "*")) { + // A leading wildcard would land at the very top of the structure and + // overlay nothing useful (the root is an array or map, not an object + // we can deepen into). Mirror the root-immutable handling: warn and + // hand back a clone of original. + console.warn( + "overlayImmutable: a top-level wildcard immutable path was supplied; " + + "the submitted value is replaced wholesale by the original.", + ) + return deepClone(original) + } + const next = deepClone(submitted) + for (const path of paths) { + if (wildcardArrayLengthChanged(next, original, path)) { + // Index-aligned overlay corrupts the user's deletion/insertion when + // a per-element-nested-immutable path crosses an array whose length + // changed. The UI greys those fields out, so the only ways to land + // here are the YAML editor or devtools — the API server will reject + // a per-element mutation via CEL. Skip the overlay (warn for diag) + // rather than silently re-anchor source values to user indices. + console.warn( + "overlayImmutable: array length changed at wildcard segment of", + path.join("."), + "— skipping overlay for this path; admission will enforce.", + ) + continue + } + overlayPath(next, original, path, 0) + } + return next +} + +function wildcardArrayLengthChanged( + submitted: unknown, + original: unknown, + path: ImmutablePath, +): boolean { + let subCur: unknown = submitted + let origCur: unknown = original + for (let i = 0; i < path.length; i++) { + const seg = path[i] + if (seg === "*") { + // Whole-array immutability (*-last) is fine to overlay even when the + // user changed the length — the semantics is "freeze, replace from + // source", and that's exactly what the *-last branch does. + const isLast = i === path.length - 1 + if (isLast) return false + // Per-element-nested immutability (*-not-last) is the case that can + // silently corrupt on SHRINK (we'd re-anchor source values onto the + // user's surviving indices, which may belong to a different element + // than the user thought they kept). On grow the shared indices stay + // put and the new entries past source.length keep the user's values, + // so growth is safe to overlay. + if (Array.isArray(subCur) && Array.isArray(origCur)) { + return subCur.length < origCur.length + } + return false + } + if (!isPlainObject(subCur) || !isPlainObject(origCur)) return false + subCur = (subCur as Record)[seg] + origCur = (origCur as Record)[seg] + } + return false +} + +function overlayPath( + target: unknown, + source: unknown, + path: ImmutablePath, + depth: number, +): unknown { + if (depth === path.length) { + // End of the path: copy the source value. If source has nothing at + // this position keep what the user submitted (avoids blanking fields + // that the persisted spec doesn't echo back). + if (source === undefined) return target + return deepClone(source) + } + const seg = path[depth] + if (seg === "*") { + return overlayWildcard(target, source, path, depth) + } + const sourceObj = isPlainObject(source) ? source : undefined + const sourceVal = sourceObj ? sourceObj[seg] : undefined + const targetObj = isPlainObject(target) + ? (target as Record) + : null + if (!targetObj) return target + if ( + sourceVal === undefined && + !Object.prototype.hasOwnProperty.call(targetObj, seg) + ) { + // Neither side has anything at this path; nothing to overlay or insert. + return targetObj + } + targetObj[seg] = overlayPath(targetObj[seg], sourceVal, path, depth + 1) + return targetObj +} + +/** + * A wildcard segment represents either "every element of an array" or + * "every entry of an object map" (additionalProperties). The walker emits + * the same "*" symbol for both because the schema decides at runtime + * which kind of node sits at the path; we dispatch on the actual value + * shape here. + * + * Whole-collection immutable (*-last): replace target with a deep clone + * of source, freezing the collection wholesale. Granular immutable + * (*-not-last): iterate the user's collection (so adds and removes + * survive) and overlay the nested immutable subfield from source on + * shared elements/keys. + */ +function overlayWildcard( + target: unknown, + source: unknown, + path: ImmutablePath, + depth: number, +): unknown { + const isLast = depth === path.length - 1 + + if (Array.isArray(source)) { + if (isLast) return deepClone(source) + const targetArr = Array.isArray(target) ? target : [] + const out: unknown[] = [] + for (let i = 0; i < targetArr.length; i++) { + if (i < source.length) { + out[i] = overlayPath(targetArr[i], source[i], path, depth + 1) + } else { + out[i] = deepClone(targetArr[i]) + } + } + return out + } + + if (isPlainObject(source)) { + // For additionalProperties (object maps) we freeze the whole map + // regardless of whether * is last or not. The UI marks the field + // ui:disabled and hides Add/Remove, so this overlay aligns the + // defence-in-depth (YAML editor / devtools) with what the form + // already enforces — added keys are dropped, removed keys are + // reinstated, every value is reset to source. + return deepClone(source) + } + + return target +} + +function deepClone(value: T): T { + if (value === null || value === undefined) return value + if (Array.isArray(value)) return value.map(deepClone) as unknown as T + if (typeof value === "object") { + const out: Record = {} + for (const [k, v] of Object.entries(value as Record)) { + out[k] = deepClone(v) + } + return out as unknown as T + } + return value +} From 26386da3ba38df19c50620cad0081cc24cc4685f Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:52:23 +0300 Subject: [PATCH 2/6] feat(console): strip x-kubernetes-validations during schema sanitisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AJV has no CEL evaluator, so the rule does nothing for JSON-Schema validation; the UI consults the raw schema once to harvest immutability paths and then drops the extension so RJSF traversal stays small. Also add characterization tests for keysOrderToUiSchema and sanitizeSchema so existing behaviour (int-or-string coercion, preserve-unknown-fields → additionalProperties, "Chart Values" → "Parameters") is locked in alongside the new strip. Widen keysOrderToUiSchema's parameter to ReadonlyArray> so call sites can pass immutable-path-style arrays without casts. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- apps/console/src/lib/keys-order.test.ts | 101 ++++++++++++++++++++++++ apps/console/src/lib/keys-order.ts | 13 ++- 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 apps/console/src/lib/keys-order.test.ts diff --git a/apps/console/src/lib/keys-order.test.ts b/apps/console/src/lib/keys-order.test.ts new file mode 100644 index 0000000..c281b4c --- /dev/null +++ b/apps/console/src/lib/keys-order.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect } from "vitest" +import { keysOrderToUiSchema, sanitizeSchema } from "./keys-order.ts" + +describe("keysOrderToUiSchema", () => { + it("returns empty object for missing/empty input", () => { + expect(keysOrderToUiSchema(undefined)).toEqual({}) + expect(keysOrderToUiSchema([])).toEqual({}) + }) + + it("ignores meta paths (apiVersion, kind, metadata) and only emits spec keys", () => { + const ui = keysOrderToUiSchema([ + ["apiVersion"], + ["kind"], + ["metadata"], + ["metadata", "name"], + ["spec", "storageClass"], + ["spec", "size"], + ]) + expect(ui).toEqual({ "ui:order": ["storageClass", "size", "*"] }) + }) + + it("nests ui:order per parent path", () => { + const ui = keysOrderToUiSchema([ + ["spec", "nodeGroups"], + ["spec", "nodeGroups", "md0"], + ["spec", "nodeGroups", "md0", "minReplicas"], + ["spec", "nodeGroups", "md0", "maxReplicas"], + ]) + expect(ui).toEqual({ + "ui:order": ["nodeGroups", "*"], + nodeGroups: { + "ui:order": ["md0", "*"], + md0: { + "ui:order": ["minReplicas", "maxReplicas", "*"], + }, + }, + }) + }) +}) + +describe("sanitizeSchema", () => { + it("returns scalars and null unchanged", () => { + expect(sanitizeSchema(null)).toBe(null) + expect(sanitizeSchema(42)).toBe(42) + expect(sanitizeSchema("x")).toBe("x") + }) + + it("recurses into arrays", () => { + const input = [{ type: "string" }, { type: "integer" }] + expect(sanitizeSchema(input)).toEqual(input) + }) + + it("coerces x-kubernetes-int-or-string to string type and drops anyOf", () => { + const out = sanitizeSchema({ + "x-kubernetes-int-or-string": true, + anyOf: [{ type: "integer" }, { type: "string" }], + }) + expect(out.type).toBe("string") + expect(out.anyOf).toBeUndefined() + }) + + it("coerces x-kubernetes-preserve-unknown-fields on a propertyless object", () => { + const out = sanitizeSchema({ + "x-kubernetes-preserve-unknown-fields": true, + }) + expect(out.type).toBe("object") + expect(out.additionalProperties).toBe(true) + }) + + it("rewrites \"Chart Values\" title to \"Parameters\"", () => { + expect(sanitizeSchema({ title: "Chart Values" }).title).toBe("Parameters") + expect(sanitizeSchema({ title: "Other" }).title).toBe("Other") + }) + + it("strips x-kubernetes-validations from any level", () => { + const input = { + properties: { + storageClass: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "immutable" }, + ], + }, + nested: { + properties: { + inner: { + type: "integer", + "x-kubernetes-validations": [{ rule: "self > 0" }], + }, + }, + }, + }, + } + const out = sanitizeSchema(input) + expect(out.properties.storageClass["x-kubernetes-validations"]).toBeUndefined() + expect( + out.properties.nested.properties.inner["x-kubernetes-validations"], + ).toBeUndefined() + expect(out.properties.storageClass.type).toBe("string") + }) +}) diff --git a/apps/console/src/lib/keys-order.ts b/apps/console/src/lib/keys-order.ts index cc64a37..080f5ea 100644 --- a/apps/console/src/lib/keys-order.ts +++ b/apps/console/src/lib/keys-order.ts @@ -23,7 +23,9 @@ import type { UiSchema } from "@rjsf/utils" * nesting level so that RJSF renders top-level spec keys, then nested keys, * in the requested order. */ -export function keysOrderToUiSchema(keysOrder: string[][] | undefined): UiSchema { +export function keysOrderToUiSchema( + keysOrder: ReadonlyArray> | undefined, +): UiSchema { if (!keysOrder || keysOrder.length === 0) return {} // Group paths by their parent path. @@ -61,6 +63,11 @@ export function keysOrderToUiSchema(keysOrder: string[][] | undefined): UiSchema * - `x-kubernetes-preserve-unknown-fields: true` on an object without * properties — arbitrary nested values allowed. Left as a "raw" object * (handled via additionalProperties: true). + * - `x-kubernetes-validations` — CEL rules. We drop them here entirely. + * AJV has no CEL evaluator so the rules are pure noise to the form + * validator; immutability is harvested up-front by `findImmutablePaths` + * on the raw schema (see lib/immutable-paths.ts) and surfaced through + * uiSchema, not through AJV. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any export function sanitizeSchema(schema: any): any { @@ -70,6 +77,10 @@ export function sanitizeSchema(schema: any): any { const out: Record = {} for (const [k, v] of Object.entries(schema)) { if (k === "anyOf" && schema["x-kubernetes-int-or-string"]) continue + // CEL validations have no JSON-Schema validator-side semantics; the UI + // extracts immutability paths up-front in SchemaForm and then strips + // the rules so AJV doesn't waste cycles traversing them. + if (k === "x-kubernetes-validations") continue out[k] = sanitizeSchema(v) } From e2dd026b6b05fbbd597a0a1606652bf19f0888fe Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:52:36 +0300 Subject: [PATCH 3/6] feat(console): plumb immutable mode through SchemaForm Add an immutableMode prop ("enforce" | "off", default "off"). When "enforce", every immutable schema path (as reported by findImmutablePaths) receives ui:disabled=true plus a ui:help string sourced from IMMUTABLE_HELP_TEXT. RJSF then renders the corresponding inputs greyed out and surfaces the helper text underneath. The default keeps every field editable so create flows are unaffected. Wildcard segments map to RJSF's "items" key for arrays; for object maps (additionalProperties) the field itself is marked disabled and the existing AdditionalPropertiesField renderer propagates that to the nested form, hiding Add/Remove controls. Parse the raw schema once and derive both the sanitised RJSFSchema and the immutable-path set from the shared parsed object so the JSON string isn't decoded twice per render. Add component-level tests covering: - enforce / off / omitted prop branches on scalar fields, - per-element-nested-immutable fields inside array items, - whole-array immutable shape, - the deliberate UI/overlay asymmetry on additionalProperties object maps (the whole map is frozen in both UI and overlay). Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .../AdditionalPropertiesField.test.tsx | 98 ++++++++++++ .../src/components/SchemaForm.test.tsx | 140 +++++++++++++++++ apps/console/src/components/SchemaForm.tsx | 143 +++++++++++++++++- apps/console/src/lib/immutable-paths.test.ts | 9 +- 4 files changed, 380 insertions(+), 10 deletions(-) create mode 100644 apps/console/src/components/AdditionalPropertiesField.test.tsx create mode 100644 apps/console/src/components/SchemaForm.test.tsx diff --git a/apps/console/src/components/AdditionalPropertiesField.test.tsx b/apps/console/src/components/AdditionalPropertiesField.test.tsx new file mode 100644 index 0000000..1e23caf --- /dev/null +++ b/apps/console/src/components/AdditionalPropertiesField.test.tsx @@ -0,0 +1,98 @@ +import { describe, it, expect } from "vitest" +import { render, screen } from "@testing-library/react" +import { SchemaForm } from "./SchemaForm.tsx" +import { IMMUTABLE_HELP_TEXT } from "../lib/immutable-paths.ts" + +const schemaWithMap = JSON.stringify({ + type: "object", + properties: { + labels: { + type: "object", + additionalProperties: { type: "string" }, + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "labels are immutable" }, + ], + }, + }, +}) + +const noop = () => {} + +describe("AdditionalPropertiesField immutable", () => { + it("renders Add/Remove controls when not immutable", () => { + render( + , + ) + expect(screen.getByPlaceholderText("Enter key name...")).toBeInTheDocument() + expect(screen.getByRole("button", { name: /add/i })).toBeInTheDocument() + expect(screen.getByRole("button", { name: /remove/i })).toBeInTheDocument() + }) + + it("treats nested *-not-last on additionalProperties as whole-map disabled in the UI; overlay matches", () => { + // Per-value-immutable schema (additionalProperties carries a nested + // CEL rule). The UI freezes the whole map and the overlay freezes the + // whole map (see overlayImmutable's additionalProperties branch). + // Both sides therefore agree: YAML-editor bypasses that add or rename + // keys are caught by the overlay before the PUT lands. + const nestedImmutableSchema = JSON.stringify({ + type: "object", + properties: { + labels: { + type: "object", + additionalProperties: { + type: "object", + properties: { + value: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "value is immutable" }, + ], + }, + }, + }, + }, + }, + }) + render( + , + ) + expect( + screen.queryByPlaceholderText("Enter key name..."), + ).not.toBeInTheDocument() + expect(screen.getByText(IMMUTABLE_HELP_TEXT)).toBeInTheDocument() + }) + + it("hides Add/Remove and disables inner fields when the map is immutable in enforce mode", () => { + render( + , + ) + expect( + screen.queryByPlaceholderText("Enter key name..."), + ).not.toBeInTheDocument() + expect( + screen.queryByRole("button", { name: /add/i }), + ).not.toBeInTheDocument() + expect( + screen.queryByRole("button", { name: /remove/i }), + ).not.toBeInTheDocument() + + const innerInput = screen.getByDisplayValue("prod") as HTMLInputElement + expect(innerInput).toBeDisabled() + + expect(screen.getByText(IMMUTABLE_HELP_TEXT)).toBeInTheDocument() + }) +}) diff --git a/apps/console/src/components/SchemaForm.test.tsx b/apps/console/src/components/SchemaForm.test.tsx new file mode 100644 index 0000000..ea416cc --- /dev/null +++ b/apps/console/src/components/SchemaForm.test.tsx @@ -0,0 +1,140 @@ +import { describe, it, expect } from "vitest" +import { render, screen } from "@testing-library/react" +import { SchemaForm } from "./SchemaForm.tsx" +import { IMMUTABLE_HELP_TEXT } from "../lib/immutable-paths.ts" + +const schema = JSON.stringify({ + type: "object", + properties: { + version: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "version is immutable" }, + ], + }, + description: { + type: "string", + }, + }, +}) + +const noop = () => {} + +describe("SchemaForm immutableMode", () => { + it("renders fields editable and without help text when immutableMode is omitted", () => { + render( + , + ) + const versionInput = screen.getByLabelText("version") as HTMLInputElement + expect(versionInput).not.toBeDisabled() + expect(screen.queryByText(IMMUTABLE_HELP_TEXT)).not.toBeInTheDocument() + }) + + it("greys out immutable fields and shows help text when immutableMode is enforce", () => { + render( + , + ) + const versionInput = screen.getByLabelText("version") as HTMLInputElement + expect(versionInput).toBeDisabled() + const descriptionInput = screen.getByLabelText( + "description", + ) as HTMLInputElement + expect(descriptionInput).not.toBeDisabled() + expect(screen.getByText(IMMUTABLE_HELP_TEXT)).toBeInTheDocument() + }) + + it("treats immutableMode='off' the same as omitting the prop", () => { + render( + , + ) + const versionInput = screen.getByLabelText("version") as HTMLInputElement + expect(versionInput).not.toBeDisabled() + expect(screen.queryByText(IMMUTABLE_HELP_TEXT)).not.toBeInTheDocument() + }) + + it("greys out every element of a whole-array immutable field", () => { + // *-as-last: the items schema itself carries the rule, so the entire + // array is immutable. RJSF receives ui:disabled on `items` and + // disables each element's input. + const wholeArraySchema = JSON.stringify({ + type: "object", + properties: { + tags: { + type: "array", + items: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "tags is immutable" }, + ], + }, + }, + }, + }) + render( + , + ) + expect(screen.getByDisplayValue("alpha")).toBeDisabled() + expect(screen.getByDisplayValue("beta")).toBeDisabled() + expect(screen.getAllByText(IMMUTABLE_HELP_TEXT).length).toBeGreaterThan(0) + // Mirroring the AdditionalPropertiesField map case: the array + // wrapper is disabled, so RJSF disables Add (and per-element + // Remove) too. Otherwise the user could append an entry that the + // overlay would silently drop on save. + expect(screen.getByRole("button", { name: /add/i })).toBeDisabled() + }) + + it("greys out immutable nested fields inside array items", () => { + const arraySchema = JSON.stringify({ + type: "object", + properties: { + volumes: { + type: "array", + items: { + type: "object", + properties: { + name: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "name immutable" }, + ], + }, + size: { type: "string" }, + }, + }, + }, + }, + }) + render( + , + ) + const nameInput = screen.getByDisplayValue("disk1") as HTMLInputElement + const sizeInput = screen.getByDisplayValue("10Gi") as HTMLInputElement + expect(nameInput).toBeDisabled() + expect(sizeInput).not.toBeDisabled() + expect(screen.getByText(IMMUTABLE_HELP_TEXT)).toBeInTheDocument() + }) +}) diff --git a/apps/console/src/components/SchemaForm.tsx b/apps/console/src/components/SchemaForm.tsx index af6a47d..5fc5738 100644 --- a/apps/console/src/components/SchemaForm.tsx +++ b/apps/console/src/components/SchemaForm.tsx @@ -5,6 +5,11 @@ import { getDefaultFormState } from "@rjsf/utils" import type { RJSFSchema, UiSchema, TemplatesType } from "@rjsf/utils" import { keysOrderToUiSchema, sanitizeSchema } from "../lib/keys-order.ts" import { addSensitiveStringWidgets } from "../lib/sensitive-fields.ts" +import { + IMMUTABLE_HELP_TEXT, + findImmutablePaths, + type ImmutablePath, +} from "../lib/immutable-paths.ts" import { customTemplates, customWidgets } from "./rjsf-templates.tsx" import { AdditionalPropertiesField } from "./AdditionalPropertiesField.tsx" import { ResourceQuotasField } from "./ResourceQuotasField.tsx" @@ -144,12 +149,114 @@ function addVMDiskWidgets(schema: RJSFSchema, uiSchema: UiSchema = {}): UiSchema return result } +/** + * Apply ui:disabled + ui:help to every path the schema declares immutable. + * The disabled flag (not readonly) gives the grey-out treatment specified + * by product. Wildcard "*" segments translate to "items" for arrays. For + * object maps (additionalProperties) the disabled flag is set on the + * field itself so AdditionalPropertiesField hides Add/Remove controls and + * disables the nested forms — see the comment at the additionalProperties + * branch below for the UX trade-off. + * + * NOTE: this walker navigates the sanitised schema (which still carries + * `properties`, `items` and `additionalProperties` structurally). The + * immutable-path *set* is harvested separately from the *raw* schema via + * findImmutablePaths, since sanitizeSchema strips x-kubernetes-validations + * on its way to AJV. If a future sanitisation step ever rewrites those + * structural keys, this walker needs to be updated in lockstep. + */ +function addImmutableReadonly( + schema: RJSFSchema, + uiSchema: UiSchema, + paths: readonly ImmutablePath[], +): UiSchema { + if (paths.length === 0) return uiSchema + const next: UiSchema = { ...uiSchema } + for (const path of paths) { + applyImmutablePath(schema, next, path, 0) + } + return next +} + +function applyImmutablePath( + schemaNode: unknown, + uiNode: Record, + path: ImmutablePath, + depth: number, +): void { + if (depth === path.length) { + uiNode["ui:disabled"] = true + uiNode["ui:help"] = IMMUTABLE_HELP_TEXT + return + } + const seg = path[depth] + const schemaObj = + schemaNode && typeof schemaNode === "object" && !Array.isArray(schemaNode) + ? (schemaNode as Record) + : null + if (seg === "*") { + if (schemaObj && schemaObj.items) { + const isLast = depth === path.length - 1 + if (isLast) { + // Whole-array immutable: mark the wrapper itself disabled so + // RJSF's ArrayFieldTemplate hides Add/Remove and disables every + // element. Mirrors the additionalProperties-map handling below; + // without this the user could click Add, fill an entry, and + // watch it silently disappear on save when overlay clones source. + uiNode["ui:disabled"] = true + uiNode["ui:help"] = IMMUTABLE_HELP_TEXT + return + } + const childUi = ensureChild(uiNode, "items") + applyImmutablePath(schemaObj.items, childUi, path, depth + 1) + return + } + // additionalProperties object map. Per-value immutability is rendered + // here as whole-map immutability: the field itself is marked disabled, + // AdditionalPropertiesField hides Add/Remove and disables every inner + // input. Splitting "keys editable, values frozen" needs custom plumbing + // through that field plus a UX decision on whether deleting an entry + // counts as mutating its value — deliberately deferred until a real + // schema asks for it. + uiNode["ui:disabled"] = true + uiNode["ui:help"] = IMMUTABLE_HELP_TEXT + return + } + const childSchema = schemaObj + ? (schemaObj.properties as Record | undefined)?.[seg] + : undefined + const childUi = ensureChild(uiNode, seg) + applyImmutablePath(childSchema, childUi, path, depth + 1) +} + +function ensureChild( + uiNode: Record, + key: string, +): Record { + const existing = uiNode[key] + if (existing && typeof existing === "object" && !Array.isArray(existing)) { + const cloned = { ...(existing as Record) } + uiNode[key] = cloned + return cloned + } + const fresh: Record = {} + uiNode[key] = fresh + return fresh +} + interface SchemaFormProps { openAPISchema: string keysOrder?: string[][] formData: unknown onChange: (data: unknown) => void children?: React.ReactNode + /** + * When "enforce", fields whose schema carries a CEL immutability rule + * (`self == oldSelf`) are rendered greyed out (disabled) with helper + * text. Default "off" keeps every field editable — used for create + * flows. + */ + immutableMode?: "enforce" | "off" } export function SchemaForm({ @@ -158,20 +265,38 @@ export function SchemaForm({ formData, onChange, children, + immutableMode, }: SchemaFormProps) { - const schema = useMemo(() => { + // Parse the raw schema once, then derive the sanitised RJSFSchema and the + // immutable-path set from it. We keep the raw object around so we don't + // re-parse the same string twice on every render. The contract is "same + // openAPISchema string ⇒ same parsedSchema reference"; the dependent + // memos rely on that identity to avoid recomputation. + const parsedSchema = useMemo(() => { try { - return sanitizeSchema(JSON.parse(openAPISchema)) as RJSFSchema + return JSON.parse(openAPISchema) } catch { - return {} as RJSFSchema + return {} } }, [openAPISchema]) + const schema = useMemo( + () => sanitizeSchema(parsedSchema) as RJSFSchema, + [parsedSchema], + ) + const onChangeRef = useRef(onChange) - onChangeRef.current = onChange const initialFormDataRef = useRef(formData) const emittedSchemaRef = useRef(null) + // Sync the latest onChange into the ref via an effect so we don't mutate + // refs during render (react-hooks/refs). The default-emit effect below + // reads onChangeRef.current and therefore always sees the latest callback + // — even though that effect only depends on `schema`. + useEffect(() => { + onChangeRef.current = onChange + }, [onChange]) + // Emit defaults to parent once per schema so spec is never empty on first submit. // Uses initialFormDataRef so edit-mode existing values are preserved as base. // emittedSchemaRef prevents re-running on unrelated re-renders and avoids @@ -182,9 +307,13 @@ export function SchemaForm({ emittedSchemaRef.current = schema const defaults = getDefaultFormState(validator, schema, initialFormDataRef.current ?? {}, schema) onChangeRef.current(defaults) - // eslint-disable-next-line react-hooks/exhaustive-deps }, [schema]) + const immutablePaths = useMemo( + () => (immutableMode === "enforce" ? findImmutablePaths(parsedSchema) : []), + [parsedSchema, immutableMode], + ) + const uiSchema = useMemo(() => { const baseUiSchema: UiSchema = { "ui:submitButtonOptions": { norender: true }, @@ -221,8 +350,8 @@ export function SchemaForm({ } } - return withSensitive - }, [keysOrder, schema]) + return addImmutableReadonly(schema, withSensitive, immutablePaths) + }, [keysOrder, schema, immutablePaths]) const customFields = useMemo( () => ({ diff --git a/apps/console/src/lib/immutable-paths.test.ts b/apps/console/src/lib/immutable-paths.test.ts index a423d5b..7111c42 100644 --- a/apps/console/src/lib/immutable-paths.test.ts +++ b/apps/console/src/lib/immutable-paths.test.ts @@ -140,7 +140,8 @@ describe("findImmutablePaths", () => { expect(findImmutablePaths(cel)).toEqual([[]]) }) - it("FIXME(#8): does NOT walk into properties beneath oneOf/anyOf/allOf branches", () => { + it("does NOT walk into properties beneath oneOf/anyOf/allOf branches (pinned current behaviour)", () => { + // Tracked in cozystack/cozystack-ui#8. // The walker recognises *parent* immutability when any branch carries // the rule, but it does not descend into a branch's `properties` to // discover per-field immutability inside the branch. CRDs that use @@ -376,7 +377,8 @@ describe("overlayImmutable", () => { }) }) - it("FIXME(#9): does NOT materialise an immutable leaf when its ancestor is missing in target", () => { + it("does NOT materialise an immutable leaf when its ancestor is missing in target (pinned current behaviour)", () => { + // Tracked in cozystack/cozystack-ui#9. // Defence-in-depth gap: if the YAML editor strips the parent object // entirely, overlay cannot reach the leaf to copy the original value. // Pin behaviour so a future contributor fixing the gap notices the @@ -391,7 +393,8 @@ describe("overlayImmutable", () => { expect(result).toEqual({ spec: {} }) }) - it("FIXME(#10): array reordering by the user with index-aligned overlay re-anchors source values to the new index", () => { + it("array reordering by the user with index-aligned overlay re-anchors source values to the new index (pinned current behaviour)", () => { + // Tracked in cozystack/cozystack-ui#10. // The overlay walks by index, not by content identity. Reordering an // immutable array maps source[i] onto target[i]'s new value — which // for a per-element-name-immutable schema swaps the names back to the From 2f34181e50532f3470b444b2958e79772c3277e7 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:52:48 +0300 Subject: [PATCH 4/6] feat(console): enforce immutable fields in edit pages ApplicationOrderPage and BackupResourceEditPage now opt their SchemaForm into immutableMode="enforce" when in edit flow so the relevant fields render greyed out with helper text. On submit, prepareUpdateSpec walks the schema for immutable paths and copies the original values from the persisted spec into the outgoing body. The API server therefore observes no delta on those paths and the CEL immutability check is not triggered, even if the user managed to bypass the read-only UI (YAML editor, devtools, RJSF bug). Both pages anchor the overlay source to a mount-time snapshot stored in a ref so a background React-Query refetch can't sneak a different immutable value into the PUT. The snapshot capture lives inside useEffect to keep render side-effect-free. prepareUpdateSpec is a thin pure wrapper around findImmutablePaths + overlayImmutable. It short-circuits to a deep clone when the persisted spec is null/undefined (avoids blanking immutable fields with undefined) and logs a console.warn when the schema string fails to parse instead of silently disabling enforcement. BackupResourceEditPage.handleSubmit also refuses to save when the CRD schema hasn't loaded yet, mirroring the existing !resource guard. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- apps/console/src/lib/prepare-update.test.ts | 76 +++++++++++++++++++ apps/console/src/lib/prepare-update.ts | 49 ++++++++++++ .../src/routes/ApplicationOrderPage.tsx | 27 ++++++- .../src/routes/BackupResourceEditPage.tsx | 11 ++- 4 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 apps/console/src/lib/prepare-update.test.ts create mode 100644 apps/console/src/lib/prepare-update.ts diff --git a/apps/console/src/lib/prepare-update.test.ts b/apps/console/src/lib/prepare-update.test.ts new file mode 100644 index 0000000..40afffa --- /dev/null +++ b/apps/console/src/lib/prepare-update.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect, vi } from "vitest" +import { prepareUpdateSpec } from "./prepare-update.ts" + +const immutableStorageClassSchema = JSON.stringify({ + type: "object", + properties: { + storageClass: { + type: "string", + "x-kubernetes-validations": [ + { rule: "self == oldSelf", message: "immutable" }, + ], + }, + size: { type: "string" }, + }, +}) + +const schemaWithoutImmutability = JSON.stringify({ + type: "object", + properties: { + storageClass: { type: "string" }, + size: { type: "string" }, + }, +}) + +describe("prepareUpdateSpec", () => { + it("overlays immutable values from initialSpec into submitted spec", () => { + const submitted = { storageClass: "fast", size: "10Gi" } + const initial = { storageClass: "slow", size: "5Gi" } + expect( + prepareUpdateSpec(submitted, initial, immutableStorageClassSchema), + ).toEqual({ storageClass: "slow", size: "10Gi" }) + }) + + it("returns a clone of submitted unchanged when the schema declares no immutability", () => { + const submitted = { storageClass: "fast", size: "10Gi" } + const initial = { storageClass: "slow", size: "5Gi" } + const result = prepareUpdateSpec(submitted, initial, schemaWithoutImmutability) + expect(result).toEqual(submitted) + expect(result).not.toBe(submitted) + }) + + it("returns a clone of submitted unchanged when the schema is malformed JSON", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + const submitted = { storageClass: "fast" } + const initial = { storageClass: "slow" } + const result = prepareUpdateSpec(submitted, initial, "not-json") + expect(result).toEqual(submitted) + warn.mockRestore() + }) + + it("does not mutate inputs", () => { + const submitted = { storageClass: "fast", size: "10Gi" } + const initial = { storageClass: "slow", size: "5Gi" } + prepareUpdateSpec(submitted, initial, immutableStorageClassSchema) + expect(submitted).toEqual({ storageClass: "fast", size: "10Gi" }) + expect(initial).toEqual({ storageClass: "slow", size: "5Gi" }) + }) + + it("returns submitted unchanged when original is undefined or null", () => { + const submitted = { storageClass: "fast", size: "10Gi" } + expect( + prepareUpdateSpec(submitted, undefined, immutableStorageClassSchema), + ).toEqual(submitted) + expect( + prepareUpdateSpec(submitted, null, immutableStorageClassSchema), + ).toEqual(submitted) + }) + + it("warns to console when the schema cannot be parsed", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + prepareUpdateSpec({ storageClass: "fast" }, { storageClass: "slow" }, "{") + expect(warn).toHaveBeenCalled() + expect(warn.mock.calls[0][0]).toMatch(/openAPISchema/) + warn.mockRestore() + }) +}) diff --git a/apps/console/src/lib/prepare-update.ts b/apps/console/src/lib/prepare-update.ts new file mode 100644 index 0000000..7ad5efb --- /dev/null +++ b/apps/console/src/lib/prepare-update.ts @@ -0,0 +1,49 @@ +import { + findImmutablePaths, + overlayImmutable, + type ImmutablePath, +} from "./immutable-paths.ts" + +/** + * Build the spec to PUT during an edit, overlaying immutable values from + * the original spec so that the API server sees no delta on paths whose + * schema carries `x-kubernetes-validations: [{rule: "self == oldSelf"}]`. + * + * Defence-in-depth: even if the form's read-only state is bypassed (via + * the YAML editor, devtools, or a UI bug) the outgoing body still + * matches the persisted spec on those paths. + * + * Edge cases: + * - When `original` is null/undefined there is nothing meaningful to + * overlay (typically the resource hasn't loaded yet, or the persisted + * spec was empty). We return the submitted value unchanged rather than + * blanking immutable fields with undefined. + * - When the schema string cannot be parsed we log a warning so the + * misconfiguration is at least visible in DevTools, then return the + * submitted value unchanged. + */ +export function prepareUpdateSpec( + submitted: T, + original: T | null | undefined, + openAPISchema: string, +): T { + if (original === undefined || original === null) { + return cloneShallowAsDeep(submitted) + } + let paths: ImmutablePath[] + try { + paths = findImmutablePaths(JSON.parse(openAPISchema)) + } catch { + console.warn( + "prepareUpdateSpec: failed to parse openAPISchema; " + + "skipping immutable-field overlay.", + ) + paths = [] + } + return overlayImmutable(submitted, original, paths) +} + +function cloneShallowAsDeep(value: T): T { + if (value === null || value === undefined) return value + return JSON.parse(JSON.stringify(value)) as T +} diff --git a/apps/console/src/routes/ApplicationOrderPage.tsx b/apps/console/src/routes/ApplicationOrderPage.tsx index 037bfad..bdc4b21 100644 --- a/apps/console/src/routes/ApplicationOrderPage.tsx +++ b/apps/console/src/routes/ApplicationOrderPage.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react" +import { useEffect, useMemo, useRef, useState } from "react" import { useNavigate, useParams, useSearchParams } from "react-router" import { ChevronLeft, FileCode, FormInput } from "lucide-react" import yaml from "js-yaml" @@ -12,6 +12,7 @@ import { } from "../lib/app-definitions.ts" import { useTenantContext } from "../lib/tenant-context.tsx" import { composeResource } from "../lib/app-resource.ts" +import { prepareUpdateSpec } from "../lib/prepare-update.ts" import { SchemaForm } from "../components/SchemaForm.tsx" import { YamlEditor } from "../components/YamlEditor.tsx" @@ -40,6 +41,20 @@ export function ApplicationOrderPage({ const [mode, setMode] = useState("form") const [yamlText, setYamlText] = useState("") const [yamlError, setYamlError] = useState(null) + // Snapshot of the persisted spec captured the first time we see an + // editMode prop. ApplicationEditRoute reconstructs `editMode` on every + // React-Query refetch, so reading `editMode.initialSpec` at save time + // would pick up whatever the cluster has right now, not what the user + // saw when they opened the form. The ref locks the overlay source to + // the value the user actually viewed. + const initialSpecRef = useRef(editMode?.initialSpec) + const initialSpecCapturedRef = useRef(false) + useEffect(() => { + if (editMode && !initialSpecCapturedRef.current) { + initialSpecCapturedRef.current = true + initialSpecRef.current = editMode.initialSpec + } + }, [editMode]) const plural = ad?.spec?.application.plural ?? "" @@ -109,6 +124,15 @@ export function ApplicationOrderPage({ const body = composeResource(ad, tenantNamespace, snap.name, snap.spec) try { if (editMode) { + // initialSpecRef holds the value the user saw when the form + // opened; if the resource is mutated externally between mount and + // Save, the overlay reinstates the mount-time value. That's the + // documented trade-off — re-mount to pick up fresh state. + body.spec = prepareUpdateSpec>( + body.spec ?? {}, + (initialSpecRef.current ?? {}) as Record, + ad.spec?.application.openAPISchema ?? "", + ) await update.mutateAsync(body) } else { await create.mutateAsync(body) @@ -248,6 +272,7 @@ export function ApplicationOrderPage({ keysOrder={ad.spec?.dashboard?.keysOrder} formData={spec} onChange={setSpec} + immutableMode={editMode ? "enforce" : "off"} /> )} diff --git a/apps/console/src/routes/BackupResourceEditPage.tsx b/apps/console/src/routes/BackupResourceEditPage.tsx index 5dcae76..e5cad8e 100644 --- a/apps/console/src/routes/BackupResourceEditPage.tsx +++ b/apps/console/src/routes/BackupResourceEditPage.tsx @@ -5,6 +5,7 @@ import { Button, Section, Spinner } from "@cozystack/ui" import { useK8sGet, useK8sUpdate } from "@cozystack/k8s-client" import { useTenantContext } from "../lib/tenant-context.tsx" import { useCRDSchema } from "../lib/use-crd-schema.ts" +import { prepareUpdateSpec } from "../lib/prepare-update.ts" import { SchemaForm } from "../components/SchemaForm.tsx" interface BackupResourceEditPageProps { @@ -23,6 +24,11 @@ export function BackupResourceEditPage({ const { tenantNamespace } = useTenantContext() const [formData, setFormData] = useState({}) const initializedRef = useRef(false) + // Snapshot of resource.spec at the moment the form initialised. Used as + // the source for the immutable-field overlay so the value the user saw + // in the form is the value that goes into the PUT, regardless of any + // React-Query refetches in between. + const initialSpecRef = useRef(null) // Map resourceType to CRD name const crdNameMap = { @@ -61,15 +67,17 @@ export function BackupResourceEditPage({ if (resource?.spec && !initializedRef.current) { initializedRef.current = true setFormData(resource.spec) + initialSpecRef.current = resource.spec } }, [resource]) const handleSubmit = async () => { if (!resource) return + if (!schema) return const updated = { ...resource, - spec: formData, + spec: prepareUpdateSpec(formData, initialSpecRef.current, schema), } try { @@ -141,6 +149,7 @@ export function BackupResourceEditPage({ openAPISchema={schema} formData={formData} onChange={setFormData} + immutableMode="enforce" >
From b687b462a19855744115d7b268a6159a18ec14a9 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:52:57 +0300 Subject: [PATCH 5/6] docs: surface pnpm test in the README The workspace already configures vitest + jsdom + @testing-library in apps/console; the README mentioned only install/dev/build. Add a one-line Test section pointing at the workspace-wide "pnpm test". Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 9f103a9..7c3b2e6 100644 --- a/README.md +++ b/README.md @@ -36,3 +36,11 @@ pnpm build ``` The console is built into `apps/console/dist/`. + +## Test + +```sh +pnpm test +``` + +Runs the workspace test suites (vitest + jsdom for the console). From d51f984a887fa737933d378b07d6032e741d0b87 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 14 May 2026 16:55:57 +0300 Subject: [PATCH 6/6] ci(workflows): pin pnpm version via packageManager field pnpm/action-setup@v4 fails when the workspace doesn't pin a pnpm version anywhere. Declare it in the standard packageManager field on the root package.json so both the Test workflow and any local contributor's corepack pick up the same version we develop against. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 56a5c38..57e33a8 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,7 @@ "name": "cozystack-ui", "private": true, "type": "module", + "packageManager": "pnpm@10.28.2", "scripts": { "dev": "pnpm --filter @cozystack/console dev", "build": "pnpm -r build",