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). diff --git a/apps/console/src/components/AdditionalPropertiesField.test.tsx b/apps/console/src/components/AdditionalPropertiesField.test.tsx new file mode 100644 index 0000000..52034a3 --- /dev/null +++ b/apps/console/src/components/AdditionalPropertiesField.test.tsx @@ -0,0 +1,99 @@ +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 *-not-last on additionalProperties as whole-map disabled in the UI even though overlay is per-entry", () => { + // Per-value-immutable schema (additionalProperties carries a nested + // CEL rule). The UI deliberately freezes the whole map for simplicity; + // the overlay enforces per-entry semantics. This asymmetry is a known + // trade-off: a YAML-editor bypass that adds a new key will be accepted + // by the overlay, while the form denies the addition. Documented here + // so it isn't mistaken for a UI bug. + 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..327649d --- /dev/null +++ b/apps/console/src/components/SchemaForm.test.tsx @@ -0,0 +1,135 @@ +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) + }) + + 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..e632b48 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,103 @@ 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 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 +254,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 +296,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 +339,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 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 +} 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) } 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..2e45620 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 { 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,16 @@ 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) + if (editMode && initialSpecRef.current === undefined) { + initialSpecRef.current = editMode.initialSpec + } const plural = ad?.spec?.application.plural ?? "" @@ -109,6 +120,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 +268,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..a68f6df 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,6 +67,7 @@ export function BackupResourceEditPage({ if (resource?.spec && !initializedRef.current) { initializedRef.current = true setFormData(resource.spec) + initialSpecRef.current = resource.spec } }, [resource]) @@ -69,7 +76,7 @@ export function BackupResourceEditPage({ const updated = { ...resource, - spec: formData, + spec: prepareUpdateSpec(formData, initialSpecRef.current, schema ?? ""), } try { @@ -141,6 +148,7 @@ export function BackupResourceEditPage({ openAPISchema={schema} formData={formData} onChange={setFormData} + immutableMode="enforce" >