Skip to content

Commit 2da6594

Browse files
myasnikovdaniillexfrei
authored andcommitted
fix(dashboard): materialise an immutable leaf whose ancestor is gone
overlayImmutable stopped walking as soon as the submitted body had nothing at a path segment, so a YAML edit that dropped a whole parent object also dropped the immutable leaf under it -- reachable through foundationdb storage.storageClass and kafka kafka.storageClass. Create the missing ancestor when the persisted spec has one, and turn the pinned FIXME test into a test of the fixed behaviour. Fixes #3102 Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 4a5e18b commit 2da6594

2 files changed

Lines changed: 25 additions & 10 deletions

File tree

‎packages/system/dashboard/images/console/apps/console/src/lib/immutable-paths.test.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -377,20 +377,31 @@ describe("overlayImmutable", () => {
377377
})
378378
})
379379

380-
it("does NOT materialise an immutable leaf when its ancestor is missing in target (pinned current behaviour)", () => {
381-
// Tracked in cozystack/cozystack-ui#9.
382-
// Defence-in-depth gap: if the YAML editor strips the parent object
383-
// entirely, overlay cannot reach the leaf to copy the original value.
384-
// Pin behaviour so a future contributor fixing the gap notices the
385-
// test and revises it.
380+
it("materialises an immutable leaf when its ancestor is missing in target", () => {
381+
// A YAML edit that strips the parent object must not strip the immutable
382+
// leaf with it. foundationdb's storage.storageClass is a shipped path of
383+
// this shape.
386384
const submitted = { spec: {} } as Record<string, unknown>
387385
const original = {
388-
spec: { backup: { storageClass: "slow" } },
386+
spec: { storage: { storageClass: "replicated", size: "10Gi" } },
389387
}
390388
const result = overlayImmutable(submitted, original, [
391-
["spec", "backup", "storageClass"],
389+
["spec", "storage", "storageClass"],
390+
])
391+
expect(result).toEqual({ spec: { storage: { storageClass: "replicated" } } })
392+
})
393+
394+
it("materialises an immutable leaf when its ancestor is null in target", () => {
395+
// `storage:` with nothing under it parses to null, not to a missing key,
396+
// so the walker has to treat it the same as absent.
397+
const submitted = { spec: { storage: null } } as Record<string, unknown>
398+
const original = {
399+
spec: { storage: { storageClass: "replicated", size: "10Gi" } },
400+
}
401+
const result = overlayImmutable(submitted, original, [
402+
["spec", "storage", "storageClass"],
392403
])
393-
expect(result).toEqual({ spec: {} })
404+
expect(result).toEqual({ spec: { storage: { storageClass: "replicated" } } })
394405
})
395406

396407
it("array reordering by the user with index-aligned overlay re-anchors source values to the new index (pinned current behaviour)", () => {

‎packages/system/dashboard/images/console/apps/console/src/lib/immutable-paths.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,11 @@ function overlayPath(
174174
const sourceVal = sourceObj ? sourceObj[seg] : undefined
175175
const targetObj = isPlainObject(target)
176176
? (target as Record<string, unknown>)
177-
: null
177+
: // A YAML edit can drop the whole intermediate object; materialise it so
178+
// the immutable leaf underneath is still restored from source.
179+
sourceVal !== undefined && (target === undefined || target === null)
180+
? {}
181+
: null
178182
if (!targetObj) return target
179183
if (
180184
sourceVal === undefined &&

0 commit comments

Comments
 (0)