This repository was archived by the owner on Jun 26, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1
fix(console): reattach backup dropdown options from CRD annotations #40
Merged
Aleksei Sviridkin (lexfrei)
merged 1 commit into
main
from
fix/backup-dropdowns-from-crd-annotations
Jun 4, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { describe, it, expect } from "vitest" | ||
| import { graftOptionSources } from "./crd-option-sources.ts" | ||
|
|
||
| interface SchemaNode { | ||
| type?: string | ||
| properties?: Record<string, SchemaNode> | ||
| "x-cozystack-options"?: { source: string } | ||
| } | ||
|
|
||
| describe("graftOptionSources", () => { | ||
| it("grafts a source onto a nested field (applicationRef.kind)", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { | ||
| applicationRef: { | ||
| type: "object", | ||
| properties: { | ||
| kind: { type: "string" }, | ||
| name: { type: "string" }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| const out = graftOptionSources(spec, { | ||
| "options.cozystack.io/source.applicationRef.kind": "appkind", | ||
| }) as SchemaNode | ||
|
|
||
| expect(out.properties?.applicationRef?.properties?.kind?.["x-cozystack-options"]).toEqual({ | ||
| source: "appkind", | ||
| }) | ||
| // Sibling left untouched. | ||
| expect( | ||
| out.properties?.applicationRef?.properties?.name?.["x-cozystack-options"], | ||
| ).toBeUndefined() | ||
| }) | ||
|
|
||
| it("grafts a source onto a top-level spec field (backupClassName)", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { backupClassName: { type: "string" } }, | ||
| } | ||
|
|
||
| const out = graftOptionSources(spec, { | ||
| "options.cozystack.io/source.backupClassName": "backupclass", | ||
| }) as SchemaNode | ||
|
|
||
| expect(out.properties?.backupClassName?.["x-cozystack-options"]).toEqual({ | ||
| source: "backupclass", | ||
| }) | ||
| }) | ||
|
|
||
| it("applies every source annotation on a CRD (BackupJob shape)", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { | ||
| applicationRef: { type: "object", properties: { kind: { type: "string" } } }, | ||
| planRef: { type: "object", properties: { name: { type: "string" } } }, | ||
| backupClassName: { type: "string" }, | ||
| }, | ||
| } | ||
|
|
||
| const out = graftOptionSources(spec, { | ||
| "controller-gen.kubebuilder.io/version": "v0.16.4", | ||
| "options.cozystack.io/source.applicationRef.kind": "appkind", | ||
| "options.cozystack.io/source.planRef.name": "plan", | ||
| "options.cozystack.io/source.backupClassName": "backupclass", | ||
| }) as SchemaNode | ||
|
|
||
| expect(out.properties?.applicationRef?.properties?.kind?.["x-cozystack-options"]).toEqual({ | ||
| source: "appkind", | ||
| }) | ||
| expect(out.properties?.planRef?.properties?.name?.["x-cozystack-options"]).toEqual({ | ||
| source: "plan", | ||
| }) | ||
| expect(out.properties?.backupClassName?.["x-cozystack-options"]).toEqual({ | ||
| source: "backupclass", | ||
| }) | ||
| }) | ||
|
|
||
| it("ignores annotations without the option-source prefix", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { backupClassName: { type: "string" } }, | ||
| } | ||
|
|
||
| const out = graftOptionSources(spec, { | ||
| "controller-gen.kubebuilder.io/version": "v0.16.4", | ||
| "options.cozystack.io/other": "noise", | ||
| }) as SchemaNode | ||
|
|
||
| expect(out.properties?.backupClassName?.["x-cozystack-options"]).toBeUndefined() | ||
| }) | ||
|
|
||
| it("is a no-op for a path that does not exist in the schema", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { backupClassName: { type: "string" } }, | ||
| } | ||
|
|
||
| expect(() => | ||
| graftOptionSources(spec, { | ||
| "options.cozystack.io/source.missing.field": "appkind", | ||
| }), | ||
| ).not.toThrow() | ||
| }) | ||
|
|
||
| it("does not mutate the input schema, including nested nodes", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { | ||
| backupClassName: { type: "string" }, | ||
| applicationRef: { type: "object", properties: { kind: { type: "string" } } }, | ||
| }, | ||
| } | ||
|
|
||
| graftOptionSources(spec, { | ||
| "options.cozystack.io/source.backupClassName": "backupclass", | ||
| "options.cozystack.io/source.applicationRef.kind": "appkind", | ||
| }) | ||
|
|
||
| // Both a top-level field and a nested one must be left untouched, so a | ||
| // future shallow/partial clone that corrupts deep nodes can't slip through. | ||
| expect(spec.properties?.backupClassName?.["x-cozystack-options"]).toBeUndefined() | ||
| expect(spec.properties?.applicationRef?.properties?.kind?.["x-cozystack-options"]).toBeUndefined() | ||
| }) | ||
|
|
||
| it("returns the schema unchanged when there are no annotations", () => { | ||
| const spec: SchemaNode = { | ||
| type: "object", | ||
| properties: { backupClassName: { type: "string" } }, | ||
| } | ||
|
|
||
| expect(graftOptionSources(spec, undefined)).toBe(spec) | ||
| expect(graftOptionSources(spec, {})).toEqual(spec) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /** | ||
| * Graft the `x-cozystack-options` vendor keyword back onto a CRD's spec schema | ||
| * from its metadata annotations. | ||
| * | ||
| * The backups.cozystack.io CRDs cannot carry `x-cozystack-options` in their | ||
| * OpenAPI schema: apiextensions `JSONSchemaProps` is a closed struct that only | ||
| * preserves `x-kubernetes-*` extensions, and server-side apply rejects any | ||
| * other `x-` key outright. The field→source mapping the dropdowns need is | ||
| * therefore stored in CRD metadata annotations (which the apiserver does | ||
| * preserve) and reattached client-side here, so DynamicOptionsWidget — which | ||
| * reads `x-cozystack-options.source` off the schema node — keeps working. | ||
| * | ||
| * Annotation contract (emitted by kubebuilder markers on the Go types): | ||
| * options.cozystack.io/source.<dotted spec-relative path> = <option source> | ||
| * e.g. `options.cozystack.io/source.applicationRef.kind: appkind`. | ||
| */ | ||
|
|
||
| const SOURCE_ANNOTATION_PREFIX = "options.cozystack.io/source." | ||
|
|
||
| /** | ||
| * Return a copy of `specSchema` with `x-cozystack-options: { source }` set on | ||
| * every field named by a matching annotation. The input is not mutated. Paths | ||
| * that do not resolve in the schema are skipped silently. | ||
| */ | ||
| export function graftOptionSources( | ||
| specSchema: unknown, | ||
| annotations: Record<string, string> | undefined | null, | ||
| ): unknown { | ||
| if (!specSchema || typeof specSchema !== "object" || !annotations) { | ||
| return specSchema | ||
| } | ||
|
|
||
| const cloned = structuredClone(specSchema) | ||
|
|
||
| for (const [key, source] of Object.entries(annotations)) { | ||
| if (!source || !key.startsWith(SOURCE_ANNOTATION_PREFIX)) continue | ||
| const path = key.slice(SOURCE_ANNOTATION_PREFIX.length).split(".") | ||
| applySource(cloned, path, source) | ||
| } | ||
|
|
||
| return cloned | ||
| } | ||
|
|
||
| function applySource(specSchema: unknown, path: string[], source: string): void { | ||
| let node = specSchema | ||
| for (const segment of path) { | ||
| const properties = (node as { properties?: Record<string, unknown> })?.properties | ||
| if (!properties || typeof properties !== "object" || !(segment in properties)) { | ||
| return | ||
| } | ||
| node = properties[segment] | ||
| } | ||
|
Comment on lines
+46
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the To prevent this, we should explicitly block sensitive keys like for (const segment of path) {
if (segment === "__proto__" || segment === "constructor") {
return
}
const properties = (node as { properties?: Record<string, unknown> })?.properties
if (
!properties ||
typeof properties !== "object" ||
!Object.prototype.hasOwnProperty.call(properties, segment)
) {
return
}
node = properties[segment]
} |
||
| if (node && typeof node === "object") { | ||
| ;(node as Record<string, unknown>)["x-cozystack-options"] = { source } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In JavaScript/TypeScript, doeskey in objinclude prototype-chain properties likeprotoandconstructor, and isObject.hasOwn(obj, key)the recommended safe check to prevent prototype-pollution-style key traversal?💡 Result:
Yes, the
inoperator in JavaScript and TypeScript checks for the existence of a property both on the object itself and throughout its entire prototype chain [1][2]. Consequently,key in objwill returntruefor inherited properties such asproto,constructor, or any other properties defined on the object's prototypes [3][4][2].Object.hasOwn(obj, key)is the recommended, modern, and safe way to check if a property is defined directly on an object (as an "own" property) without traversing its prototype chain [3][4][5]. UsingObject.hasOwn()helps mitigate risks related to prototype pollution and avoids common pitfalls associated with older methods [4][6][5]: - It works correctly on objects created withObject.create(null)(which do not inherit fromObject.prototypeand therefore lack thehasOwnPropertymethod) [4][7]. - It is immune to issues where an object might have a shadowed or overriddenhasOwnPropertyproperty [4][7]. For codebases that need to support older environments, the traditional workaround isObject.prototype.hasOwnProperty.call(obj, key), butObject.hasOwn()is preferred wherever ES2022+ is supported [3][5][8]. When iterating over object keys, you should also prefer methods that only access own properties, such asObject.keys(),Object.values(),Object.entries(), orfor...ofloops, over thefor...inloop, which also traverses the prototype chain [9][6].Citations:
Avoid prototype-chain key traversal in annotation-path resolution
Line 48’s
segment in propertiesmatches inherited keys via the prototype chain (e.g.__proto__,constructor). That can cause the traversal to setnodeto a prototype object and then mutate it at lines 54-55. Switching to an own-property check prevents this.Proposed fix
function applySource(specSchema: unknown, path: string[], source: string): void { let node = specSchema for (const segment of path) { const properties = (node as { properties?: Record<string, unknown> })?.properties - if (!properties || typeof properties !== "object" || !(segment in properties)) { + if ( + !properties || + typeof properties !== "object" || + segment === "__proto__" || + segment === "prototype" || + segment === "constructor" || + !Object.hasOwn(properties, segment) + ) { return } node = properties[segment] } if (node && typeof node === "object") { ;(node as Record<string, unknown>)["x-cozystack-options"] = { source } } }Add a regression test to ensure
options.cozystack.io/source.__proto__is ignored.📝 Committable suggestion
🤖 Prompt for AI Agents