feat: add @x-<keyword> directive for vendor schema extensions - #27
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces support for OpenAPI/JSON-Schema vendor extensions (keys starting with x-) via @x-<keyword> directives, which are emitted into the generated JSON schema but excluded from Go types. The feedback highlights a limitation in the current implementation of injectExtensions, which only supports one level of array or map nesting. It is recommended to introduce a recursive unwrapSchema helper to handle arbitrary nesting levels and to expand the test suite to cover these nested structures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // injectExtensions walks a marshaled JSON schema node alongside its | ||
| // corresponding Node, writing any vendor extensions (x-* keys) declared on | ||
| // the node and recursively descending into nested object properties, array | ||
| // items and map values. Extensions are emitted into the JSON schema only. | ||
| func injectExtensions(schema map[string]interface{}, n *Node, aliases map[string]*Node) { | ||
| if schema == nil || n == nil { | ||
| return | ||
| } | ||
|
|
||
| for k, v := range n.Extensions { | ||
| schema[k] = v | ||
| } | ||
|
|
||
| children := structuralChildren(n, aliases) | ||
| if children == nil { | ||
| return | ||
| } | ||
|
|
||
| // Object properties. | ||
| if props, ok := schema["properties"].(map[string]interface{}); ok { | ||
| for name, child := range children { | ||
| if sub, ok := props[name].(map[string]interface{}); ok { | ||
| injectExtensions(sub, child, aliases) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Array element type ([]Type) — children describe the element struct. | ||
| if items, ok := schema["items"].(map[string]interface{}); ok { | ||
| injectExtensionsForElement(items, n, aliases) | ||
| } | ||
|
|
||
| // Map value type (map[string]Type) — children describe the value struct. | ||
| if ap, ok := schema["additionalProperties"].(map[string]interface{}); ok { | ||
| injectExtensionsForElement(ap, n, aliases) | ||
| } | ||
| } | ||
|
|
||
| // injectExtensionsForElement injects extensions into the element schema of an | ||
| // array or map, using the children that describe that element's struct. | ||
| func injectExtensionsForElement(elem map[string]interface{}, n *Node, aliases map[string]*Node) { | ||
| children := structuralChildren(n, aliases) | ||
| if children == nil { | ||
| return | ||
| } | ||
| if props, ok := elem["properties"].(map[string]interface{}); ok { | ||
| for name, child := range children { | ||
| if sub, ok := props[name].(map[string]interface{}); ok { | ||
| injectExtensions(sub, child, aliases) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of injectExtensions and injectExtensionsForElement only supports one level of array or map nesting (e.g., []Type or map[string]Type). If a type is nested deeper (e.g., [][]Type or map[string][]Type), the properties of the underlying struct will not have their extensions injected because injectExtensionsForElement expects the direct element schema to contain properties.
We can simplify the implementation and support arbitrary nesting levels of arrays and maps by introducing a helper function unwrapSchema that recursively traverses down items and additionalProperties until it finds the underlying object schema, and then applying the extensions to its properties. This also eliminates the duplicated property-traversal logic in injectExtensionsForElement.
// unwrapSchema traverses down array items and map additionalProperties to find the underlying object schema.
func unwrapSchema(schema map[string]interface{}) map[string]interface{} {
if items, ok := schema["items"].(map[string]interface{}); ok {
return unwrapSchema(items)
}
if ap, ok := schema["additionalProperties"].(map[string]interface{}); ok {
return unwrapSchema(ap)
}
return schema
}
// injectExtensions walks a marshaled JSON schema node alongside its
// corresponding Node, writing any vendor extensions (x-* keys) declared on
// the node and recursively descending into nested object properties, array
// items and map values. Extensions are emitted into the JSON schema only.
func injectExtensions(schema map[string]interface{}, n *Node, aliases map[string]*Node) {
if schema == nil || n == nil {
return
}
for k, v := range n.Extensions {
schema[k] = v
}
children := structuralChildren(n, aliases)
if children == nil {
return
}
target := unwrapSchema(schema)
if props, ok := target["properties"].(map[string]interface{}); ok {
for name, child := range children {
if sub, ok := props[name].(map[string]interface{}); ok {
injectExtensions(sub, child, aliases)
}
}
}
}| func TestVendorExtensionOnTypedefFieldPropagates(t *testing.T) { | ||
| const yaml = ` | ||
| ## @typedef {struct} GPU - GPU device configuration. | ||
| ## @field {string} name - The name of the GPU resource to attach. | ||
| ## @x-cozystack-options {source: gpu} | ||
|
|
||
| ## @param {GPU} gpu - Single GPU. | ||
| gpu: {} | ||
|
|
||
| ## @param {[]GPU} gpus - GPU list. | ||
| gpus: [] | ||
|
|
||
| ## @param {map[string]GPU} gpuMap - GPU map. | ||
| gpuMap: {} | ||
| ` | ||
| props := schemaProps(t, yaml) | ||
|
|
||
| want := map[string]any{"source": "gpu"} | ||
|
|
||
| // Direct use: gpu.properties.name | ||
| gpu := props["gpu"].(map[string]any) | ||
| gpuName := gpu["properties"].(map[string]any)["name"].(map[string]any) | ||
| require.Equal(t, want, gpuName["x-cozystack-options"]) | ||
|
|
||
| // Array use: gpus.items.properties.name | ||
| gpus := props["gpus"].(map[string]any) | ||
| items := gpus["items"].(map[string]any) | ||
| itemName := items["properties"].(map[string]any)["name"].(map[string]any) | ||
| require.Equal(t, want, itemName["x-cozystack-options"]) | ||
|
|
||
| // Map use: gpuMap.additionalProperties.properties.name | ||
| gpuMap := props["gpuMap"].(map[string]any) | ||
| ap := gpuMap["additionalProperties"].(map[string]any) | ||
| apName := ap["properties"].(map[string]any)["name"].(map[string]any) | ||
| require.Equal(t, want, apName["x-cozystack-options"]) | ||
| } |
There was a problem hiding this comment.
To ensure that vendor extensions propagate correctly through arbitrarily nested arrays and maps (e.g., [][]GPU), we should expand the test suite to cover nested structures. This will prevent future regressions.
func TestVendorExtensionOnTypedefFieldPropagates(t *testing.T) {
const yaml = `
## @typedef {struct} GPU - GPU device configuration.
## @field {string} name - The name of the GPU resource to attach.
## @x-cozystack-options {source: gpu}
## @param {GPU} gpu - Single GPU.
gpu: {}
## @param {[]GPU} gpus - GPU list.
gpus: []
## @param {map[string]GPU} gpuMap - GPU map.
gpuMap: {}
## @param {[][]GPU} nestedGpus - Nested GPU list.
nestedGpus: []
`
props := schemaProps(t, yaml)
want := map[string]any{"source": "gpu"}
// Direct use: gpu.properties.name
gpu := props["gpu"].(map[string]any)
gpuName := gpu["properties"].(map[string]any)["name"].(map[string]any)
require.Equal(t, want, gpuName["x-cozystack-options"])
// Array use: gpus.items.properties.name
gpus := props["gpus"].(map[string]any)
items := gpus["items"].(map[string]any)
itemName := items["properties"].(map[string]any)["name"].(map[string]any)
require.Equal(t, want, itemName["x-cozystack-options"])
// Map use: gpuMap.additionalProperties.properties.name
gpuMap := props["gpuMap"].(map[string]any)
ap := gpuMap["additionalProperties"].(map[string]any)
apName := ap["properties"].(map[string]any)["name"].(map[string]any)
require.Equal(t, want, apName["x-cozystack-options"])
// Nested array use: nestedGpus.items.items.properties.name
nestedGpus := props["nestedGpus"].(map[string]any)
innerItems := nestedGpus["items"].(map[string]any)["items"].(map[string]any)
nestedName := innerItems["properties"].(map[string]any)["name"].(map[string]any)
require.Equal(t, want, nestedName["x-cozystack-options"])
}Add VendorExtensionPattern matching '## @x-<keyword> <value>'. The key always starts with x- so it never collides with the built-in directives (@param/@field/@enum/@minimum/...), none of which begin with x-. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Parse @x-<keyword> directives onto the preceding @param/@field, storing the YAML-flow-parsed value on an Extensions map of the Raw/Node. Inject those x-* keys into values.schema.json only — never into the generated Go types or as controller-gen markers. origin/main writes each property via json.MarshalIndent of the JSONSchemaProps struct, which preserves struct-field order. To keep that output byte-identical when no extension is present, properties whose subtree carries no extension keep the plain MarshalIndent fast path. Only properties that actually carry an x-* directive round-trip through an order-preserving orderedMap, appending the x-* keys last so no existing key is reordered. Extensions on a @typedef @field propagate wherever the type is referenced: directly, via []Type (items.properties) and via map[string]Type (additionalProperties.properties). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Cover parsing, schema emission for object/array/scalar values, multiple extensions on one field, typedef-field propagation through direct / []Type / map[string]Type, and absence from generated Go types. Add order-preservation tests: an un-annotated multi-constraint fixture must reproduce JSONSchemaProps struct-field order (not alphabetical), and adding one @x- directive must differ only by the appended x-* key. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
dd704c4 to
1aad9ba
Compare
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — correct, well-tested, and byte-compatible with the existing generator for its documented scope. The two findings below are non-blocking and tracked as follow-up issues.
Business context: adds a vendor-neutral @x-<keyword> directive so a chart author can attach arbitrary OpenAPI/JSON-Schema x-* vendor extensions to a @param/@field, emitted into values.schema.json only.
The design is sound. The hard part — injecting non-struct x-* keys without disturbing the struct-field key order that JSONSchemaProps marshaling produces — is handled by a fast-path / injection-path split plus an order-preserving orderedMap, and the byte-identity guarantee is locked down by golden tests (TestSchemaKeyOrderPreservedWithoutExtensions, TestSchemaKeyOrderWithExtensionInsertedAtEnd). The directive cannot collide with built-ins (none start with x-), invalid YAML values fail loudly, and an @x-* line before any @param is safely ignored.
Non-blocking follow-ups
- Vendor extensions are silently dropped when a typedef type is referenced through more than one container layer (
[][]Type,map[string][]Type) —injectExtensionsForElementdescends only one level. Single-level cases ([]Type,map[string]Type) work and are tested. Cheap to fix and it simplifies the walker, so worth doing before merge. Tracked in #28. - The exported
WriteValuesSchemahelper (nil root) routes to the fallback branch, which neither preserves key order nor injects extensions. Not a regression and the CLI path is unaffected; worth a doc comment. Tracked in #29.
| if children == nil { | ||
| return | ||
| } | ||
| if props, ok := elem.get("properties").(*orderedMap); ok { |
There was a problem hiding this comment.
Only elem["properties"] is inspected here, so a type referenced through more than one container layer ([][]Type, map[string][]Type) never gets its extension injected — the walker stops at the first items/additionalProperties. See #28.
…rm dropdowns (#2778) ## What this PR does Adds a generic mechanism for **runtime-populated dropdowns** in dashboard create/edit forms. Many fields are references to live cluster resources (GPU device names, KubeVirt instancetypes/preferences, Multus networks, VM images, bucket storage pools, storage classes, backup classes/plans) but were free-text or static enums that drift from reality. - **`Option` resource** — `core.cozystack.io/v1alpha1`, namespaced, read-only (list/get/watch). Items are computed on read by an in-process provider registry using a privileged client, so tenants get curated option lists without broad cluster reads. Providers: `gpu`, `instancetype`, `instanceprofile`, `network`, `image`, `storagepool`, `storageclass`, and backup-related `backupclass`, `plan`, `backup`, `appkind`. - **`x-cozystack-options` schema keyword** — app `values.yaml` declare it via the new cozyvalues-gen `@x-<keyword>` directive (e.g. `## @x-cozystack-options {source: instancetype}`). Schemas, Go types, and the `-rd` ApplicationDefinitions are regenerated from source so the extension is carried into the served OpenAPI. The dashboard renders annotated fields as dropdowns sourced from the live cluster (UI widget ships in a separate cozystack-ui PR). - **RBAC** — the apiserver ServiceAccount gets the privileged reads; tenants get read-only access to `options` in their namespace via the aggregated tenant ClusterRoles (granted at `cozy:tenant:view:base`, so every access level from view upward can populate dropdowns). - Drops the redundant manual `instanceProfile` enum injection from vm-instance's Makefile (now driven dynamically). ## Dependency on cozyvalues-gen The `x-cozystack-options` annotations require the new vendor-extension directive from cozystack/cozyvalues-gen#27. This PR bumps the pinned cozyvalues-gen to **v1.6.0**. **A cozyvalues-gen v1.6.0 release must be cut before CI can pass** — until then the pre-commit schema-generation job cannot download the binary and will fail. ### Screenshots N/A — no UI changes in this repository. The consuming dashboard widget ships in a separate cozystack-ui PR. ### Release note ```release-note feat(api): add a namespaced read-only Option resource (core.cozystack.io) and the x-cozystack-options schema keyword, enabling dashboard form fields (GPU, instancetype/profile, network, image, storage pool/class, backup class/plan) to render as dropdowns populated from live cluster resources ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded dynamic dropdowns across many apps (storageClass, storagePool, instanceType/instanceProfile, GPU, image, vmdisk, network, backup/plan/backupclass, appkind) so chart fields can be populated from live cluster resources. * **Chores** * Updated API/RBAC manifests to grant read access needed for option providers. * **Tests** * Added provider and REST tests to validate option sources, scoping, ordering, and failure handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What
Adds a vendor-neutral
@x-<keyword>directive to the JSDoc-like annotation grammar parsed from Helmvalues.yamlcomments. It lets a chart author attach any OpenAPI/JSON-Schema vendor extension keyword (any key starting withx-) to a@paramor@field. The directive name carries the keyword, and the value is parsed as YAML flow:emits into the generated JSON schema:
Declared on a typedef field, the extension propagates everywhere the type is used — including
[]Type(items.properties) andmap[string]Type(additionalProperties.properties).Why
This is a generic
x-*passthrough mechanism: anyx-foo-barkey works and nothing is interpreted or hardcoded. The motivating consumer is cozystack's dynamic-dropdown UI, which readsx-cozystack-options.sourcefrom the schema to populate selectable values — but the directive is useful for any tool that consumes vendor extensions.Behavior
@x-*lines are allowed on one field (distinct keys).values.schema.jsononly; they never leak into the generated Go types (-g), which remain pure data shapes. README generation is unaffected.Tests
Covers top-level
@paramextensions, typedef@fieldpropagation through direct/array/map usage, object/array/scalar value types, multiple extensions per field, non-collision with built-in directives, and an assertion that nox-key appears in generated Go types.go build,go test, andgofmtare clean.