diff --git a/site/src/content/docs/commands/zarf_dev.md b/site/src/content/docs/commands/zarf_dev.md index 195ddca4e9..15d24fcda7 100644 --- a/site/src/content/docs/commands/zarf_dev.md +++ b/site/src/content/docs/commands/zarf_dev.md @@ -37,7 +37,7 @@ Commands useful for developing packages * [zarf dev find-images](/commands/zarf_dev_find-images/) - Evaluates components in a Zarf file to identify images specified in their helm charts and manifests. * [zarf dev generate](/commands/zarf_dev_generate/) - Creates a zarf.yaml automatically from a given remote (git) Helm chart * [zarf dev generate-config](/commands/zarf_dev_generate-config/) - Generates a config file for Zarf -* [zarf dev generate-schema](/commands/zarf_dev_generate-schema/) - Generates a JSON schema for Zarf values based on the package definition and chart defaults +* [zarf dev generate-schema](/commands/zarf_dev_generate-schema/) - Generates a JSON schema for Zarf values based on the package definition, chart defaults, and chart schemas * [zarf dev inspect](/commands/zarf_dev_inspect/) - Commands to gather information about a Zarf package using its package definition * [zarf dev lint](/commands/zarf_dev_lint/) - Lints the given package for valid schema and recommended practices * [zarf dev patch-git](/commands/zarf_dev_patch-git/) - Converts all .git URLs to the specified Zarf HOST and with the Zarf URL pattern in a given FILE. NOTE: diff --git a/site/src/content/docs/commands/zarf_dev_generate-schema.md b/site/src/content/docs/commands/zarf_dev_generate-schema.md index ef6b3e4753..0db8851982 100644 --- a/site/src/content/docs/commands/zarf_dev_generate-schema.md +++ b/site/src/content/docs/commands/zarf_dev_generate-schema.md @@ -8,7 +8,7 @@ tableOfContents: false ## zarf dev generate-schema -Generates a JSON schema for Zarf values based on the package definition and chart defaults +Generates a JSON schema for Zarf values based on the package definition, chart defaults, and chart schemas ``` zarf dev generate-schema [ DIRECTORY ] [flags] diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 4c64ce1b8b..c0230a7dfc 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -91,13 +91,19 @@ type devGenerateSchemaOptions struct { deleteNotFound bool } +type mappedChartSchema struct { + sourcePath value.Path + schema map[string]any + excludePath []value.Path +} + func newDevGenerateSchemaCommand(v *viper.Viper) *cobra.Command { o := &devGenerateSchemaOptions{} cmd := &cobra.Command{ Use: "generate-schema [ DIRECTORY ]", Args: cobra.MaximumNArgs(1), - Short: "Generates a JSON schema for Zarf values based on the package definition and chart defaults", + Short: "Generates a JSON schema for Zarf values based on the package definition, chart defaults, and chart schemas", RunE: func(cmd *cobra.Command, args []string) error { return o.run(cmd.Context(), args) }, @@ -149,6 +155,8 @@ func (o *devGenerateSchemaOptions) run(ctx context.Context, args []string) error return fmt.Errorf("unable to parse package values files: %w", err) } + var mappedSchemas []mappedChartSchema + // Step 2: Discover source target mappings and load defaults from chart values where Zarf Value defaults aren't specified tmpDir, err := utils.MakeTempDir(config.CommonOptions.TempDirectory) if err != nil { @@ -178,6 +186,21 @@ func (o *devGenerateSchemaOptions) run(ctx context.Context, args []string) error } appliedValues := helpers.MergeMapRecursive(helmChart.Values, valuesFilesValues) + var chartSchema map[string]any + if len(helmChart.Schema) > 0 { + if err := json.Unmarshal(helmChart.Schema, &chartSchema); err != nil { + l.Warn("unable to parse Helm chart values schema; falling back to inferred types", "chart", chart.Name, "error", err) + chartSchema = nil + } else { + chartSchema = value.FilterChartSchema(chartSchema) + if chartSchema != nil { + if err := value.ValidateSchemaDocument(chartSchema); err != nil { + l.Warn("unable to validate Helm chart values schema; falling back to inferred types", "chart", chart.Name, "error", err) + chartSchema = nil + } + } + } + } // Map ChartValues' Source to Target and merge into zarfValues if not already present for _, cv := range chart.Values { @@ -192,6 +215,26 @@ func (o *devGenerateSchemaOptions) run(ctx context.Context, args []string) error if err := zarfValues.Set(value.Path(cv.SourcePath), val); err != nil { return fmt.Errorf("unable to set chart %q value at sourcePath %q: %w", chart.Name, cv.SourcePath, err) } + + if chartSchema != nil { + targetSchema, found, err := value.ExtractJSONSchema(chartSchema, value.Path(cv.TargetPath)) + if err != nil { + return fmt.Errorf("unable to inspect chart %q schema at targetPath %q: %w", chart.Name, cv.TargetPath, err) + } + if found { + excludes := make([]value.Path, len(cv.ExcludePaths)) + for i, excludePath := range cv.ExcludePaths { + excludes[i] = value.Path(excludePath) + } + mappedSchemas = append(mappedSchemas, mappedChartSchema{ + sourcePath: value.Path(cv.SourcePath), + schema: targetSchema, + excludePath: excludes, + }) + } else { + l.Warn("chart values schema does not define mapped target; falling back to inferred types", "chart", chart.Name, "targetPath", cv.TargetPath) + } + } for _, excludePath := range cv.ExcludePaths { if err := zarfValues.Delete(value.Path(excludePath)); err != nil { return fmt.Errorf("unable to exclude path %q from schema for chart %q: %w", excludePath, chart.Name, err) @@ -203,6 +246,16 @@ func (o *devGenerateSchemaOptions) run(ctx context.Context, args []string) error // Step 3: Generate JSON generatedSchema from the final map generatedSchema := value.GenerateJSONSchema(zarfValues) + for _, mapped := range mappedSchemas { + if err := value.MergeJSONSchemaAtPath(generatedSchema, mapped.sourcePath, mapped.schema); err != nil { + return fmt.Errorf("unable to apply Helm schema at sourcePath %q: %w", mapped.sourcePath, err) + } + for _, excludePath := range mapped.excludePath { + if err := value.DeleteJSONSchemaAtPath(generatedSchema, excludePath); err != nil { + return fmt.Errorf("unable to exclude schema path %q: %w", excludePath, err) + } + } + } // Step 4: Merge and reconcile any existing schema existingSchema, mergeErr := value.MergeSchemaFiles(defined.Pkg.Values.Schema, defined.ImportedSchemas, basePath) diff --git a/src/pkg/value/generate.go b/src/pkg/value/generate.go index 999c6c1fba..20d331d3ba 100644 --- a/src/pkg/value/generate.go +++ b/src/pkg/value/generate.go @@ -3,7 +3,9 @@ package value -import "maps" +import ( + "fmt" +) // GenerateJSONSchema infers a JSON schema from the structure and scalar types in values. func GenerateJSONSchema(vals Values) map[string]any { @@ -21,24 +23,21 @@ func GenerateJSONSchema(vals Values) map[string]any { return schema } -// ReconcileJSONSchema updates structural fields in an existing schema from inferred values. -// Non-structural fields (description/enum/required/etc.) are preserved by default. +// ReconcileJSONSchema updates inferred fields in an existing schema. Existing +// fields take precedence for non-structural validation fields while inferred +// structure and missing validation fields are carried forward. func ReconcileJSONSchema(existing, inferred map[string]any, deleteNotFound bool) map[string]any { - existing = maps.Clone(existing) + existing = copyMap(existing) typeVal, hasType := inferred["type"] if hasType { existing["type"] = typeVal } - typeStr, ok := typeVal.(string) - if !ok { - typeStr = "" - } - if typeStr == "object" { + if schemaTypeIncludes(typeVal, "object") { reconcileSchemaProperties(existing, inferred, deleteNotFound) } - if typeStr == "array" { + if schemaTypeIncludes(typeVal, "array") { reconcileSchemaItems(existing, inferred, deleteNotFound) } @@ -46,9 +45,301 @@ func ReconcileJSONSchema(existing, inferred map[string]any, deleteNotFound bool) existing["$schema"] = schemaURI } + // Preserve explicitly-authored fields while carrying inferred validation + // fields into schemas that do not define them. This lets chart schemas add + // constraints without overriding package-owned constraints. + for key, inferredValue := range inferred { + switch key { + case "$schema", "type", "properties", "items": + continue + } + if _, exists := existing[key]; !exists { + existing[key] = copyValue(inferredValue) + } + } + return existing } +func schemaTypeIncludes(typeVal any, wanted string) bool { + switch val := typeVal.(type) { + case string: + return val == wanted + case []any: + for _, item := range val { + if item == wanted { + return true + } + } + case []string: + for _, item := range val { + if item == wanted { + return true + } + } + } + return false +} + +// ExtractJSONSchema returns the schema object at a JSON value path. The root +// path (".") returns the supplied schema itself. +func ExtractJSONSchema(schema map[string]any, path Path) (map[string]any, bool, error) { + if err := path.Validate(); err != nil { + return nil, false, err + } + if path == "." { + return schema, true, nil + } + + current := schema + for _, part := range path.Segments() { + child, ok := schemaChild(current, part) + if !ok { + return nil, false, nil + } + current = child + } + return current, true, nil +} + +// MergeJSONSchemaAtPath overlays a chart schema at a JSON value path. Chart +// fields are copied into the inferred schema; authored package schemas are +// reconciled later and take precedence over conflicting fields. +func MergeJSONSchemaAtPath(schema map[string]any, path Path, overlay map[string]any) error { + if err := path.Validate(); err != nil { + return err + } + if path == "." { + mergeChartSchema(schema, overlay) + return nil + } + + current := schema + parts := path.Segments() + for i, part := range parts { + child, ok := schemaChild(current, part) + if !ok { + return fmt.Errorf("schema path %s: key %q is not an object schema", path, part) + } + if i == len(parts)-1 { + mergeChartSchema(child, overlay) + return nil + } + current = child + } + return nil +} + +// DeleteJSONSchemaAtPath removes a property schema at a JSON value path. A +// missing path is treated as a no-op, matching Values.Delete behavior. +func DeleteJSONSchemaAtPath(schema map[string]any, path Path) error { + if err := path.Validate(); err != nil { + return err + } + if path == "." { + return fmt.Errorf("cannot delete root schema") + } + + current := schema + parts := path.Segments() + for i, part := range parts { + properties, ok := current["properties"].(map[string]any) + if !ok { + return nil + } + if i == len(parts)-1 { + delete(properties, part) + return nil + } + child, ok := properties[part].(map[string]any) + if !ok { + return nil + } + current = child + } + return nil +} + +func schemaChild(schema map[string]any, part string) (map[string]any, bool) { + if properties, ok := schema["properties"].(map[string]any); ok { + if child, ok := properties[part].(map[string]any); ok { + return child, true + } + } + + // A chart schema commonly describes arbitrary map keys through + // additionalProperties rather than enumerating them in properties. + if additionalProperties, ok := schema["additionalProperties"].(map[string]any); ok { + return additionalProperties, true + } + + return nil, false +} + +func mergeChartSchema(destination, source map[string]any) { + for key, sourceValue := range FilterChartSchema(source) { + switch key { + case "properties": + sourceProperties, ok := sourceValue.(map[string]any) + if !ok { + destination[key] = copyValue(sourceValue) + continue + } + destinationProperties, ok := destination[key].(map[string]any) + if !ok { + destination[key] = copyValue(sourceValue) + continue + } + for propertyName, sourceProperty := range sourceProperties { + sourcePropertyMap, sourceIsMap := sourceProperty.(map[string]any) + destinationPropertyMap, destinationIsMap := destinationProperties[propertyName].(map[string]any) + if sourceIsMap && destinationIsMap { + mergeChartSchema(destinationPropertyMap, sourcePropertyMap) + } else { + destinationProperties[propertyName] = copyValue(sourceProperty) + } + } + case "items", "additionalProperties": + sourceMap, sourceIsMap := sourceValue.(map[string]any) + destinationMap, destinationIsMap := destination[key].(map[string]any) + if sourceIsMap && destinationIsMap { + mergeChartSchema(destinationMap, sourceMap) + } else { + destination[key] = copyValue(sourceValue) + } + default: + // Validation keywords are chart-owned at this stage and should be + // retained when they describe values supplied by the package. + destination[key] = copyValue(sourceValue) + } + } +} + +// FilterChartSchema retains schema keywords that describe value types or +// validation rules, while dropping annotations such as description, title, +// default, and examples from the chart schema. Reference-dependent schema +// nodes are dropped while independent child properties are retained. Presence +// and lower-bound map constraints are intentionally excluded because Helm +// applies chart defaults before validating the final values object. +func FilterChartSchema(schema map[string]any) map[string]any { + if hasUnsupportedChartSchemaReference(schema) { + return nil + } + + filtered := make(map[string]any) + for key, value := range schema { + if !isChartSchemaKeyword(key) { + continue + } + if key == "const" || key == "enum" { + filtered[key] = copyValue(value) + continue + } + // Strip default `additionalProperties=true` to prevent schema bloat + if key == "additionalProperties" { + if allowed, ok := value.(bool); ok && allowed { + continue + } + } + + filteredValue, keep := filterChartSchemaValue(key, value) + if !keep || isEmptyChartSchemaFragment(key, filteredValue) { + continue + } + filtered[key] = filteredValue + } + return filtered +} + +// isEmptyChartSchemaFragment identifies empty schemas that add no validation +// beyond JSON Schema's defaults. +func isEmptyChartSchemaFragment(key string, value any) bool { + if key != "items" && key != "properties" && key != "additionalProperties" { + return false + } + schema, ok := value.(map[string]any) + return ok && len(schema) == 0 +} + +func filterChartSchemaValue(key string, value any) (any, bool) { + if key == "properties" { + if schemas, ok := value.(map[string]any); ok { + filtered := make(map[string]any, len(schemas)) + for name, schema := range schemas { + if schemaMap, ok := schema.(map[string]any); ok { + filteredSchema := FilterChartSchema(schemaMap) + if len(filteredSchema) == 0 { + continue + } + filtered[name] = filteredSchema + } else { + filtered[name] = copyValue(schema) + } + } + return filtered, true + } + } + + switch value := value.(type) { + case map[string]any: + filtered := FilterChartSchema(value) + return filtered, len(filtered) > 0 + case []any: + filtered := make([]any, len(value)) + for i, item := range value { + filteredItem, keep := filterChartSchemaValue("", item) + if !keep { + return nil, false + } + filtered[i] = filteredItem + } + return filtered, true + default: + return copyValue(value), true + } +} + +// hasUnsupportedChartSchemaReference reports whether a schema node contains a +// reference that cannot be retained while filtering that node. Child schemas +// under properties, items, and additionalProperties are handled independently. +// Definitions are intentionally ignored because they are not copied into the +// generated schema; a field that uses one still has its own reference. +func hasUnsupportedChartSchemaReference(schema map[string]any) bool { + for key, value := range schema { + if isReferenceKeyword(key) { + return true + } + + switch key { + case "properties", "items", "additionalProperties", "definitions", "$defs": + continue + case "additionalItems", "allOf", "anyOf", "oneOf", "not", + "if", "then", "else", "contains", "propertyNames", + "patternProperties", "dependencies", "dependentSchemas", + "prefixItems", "unevaluatedItems", "unevaluatedProperties", "contentSchema": + if hasJSONSchemaReference(value) { + return true + } + } + } + return false +} + +// isChartSchemaKeyword keeps the imported Helm schema surface deliberately +// small: basic shape and scalar/array constraints only. +func isChartSchemaKeyword(key string) bool { + switch key { + case "additionalProperties", "const", "enum", + "exclusiveMaximum", "exclusiveMinimum", + "items", "maxItems", "maxLength", "maximum", + "minItems", "minLength", "minimum", "pattern", + "properties", "type": + return true + default: + return false + } +} + func reconcileSchemaProperties(existing, inferred map[string]any, deleteNotFound bool) { inferredProps, ok := inferred["properties"].(map[string]any) if !ok { @@ -72,13 +363,13 @@ func reconcileSchemaProperties(existing, inferred map[string]any, deleteNotFound for key, inferredProp := range inferredProps { inferredPropMap, ok := inferredProp.(map[string]any) if !ok { - existingProps[key] = inferredProp + existingProps[key] = copyValue(inferredProp) continue } existingPropMap, ok := existingProps[key].(map[string]any) if !ok { - existingProps[key] = inferredPropMap + existingProps[key] = copyMap(inferredPropMap) continue } @@ -94,7 +385,7 @@ func reconcileSchemaItems(existing, inferred map[string]any, deleteNotFound bool existingItems, hasExistingItems := existing["items"].(map[string]any) if !hasExistingItems { - existing["items"] = inferredItems + existing["items"] = copyMap(inferredItems) return } diff --git a/src/pkg/value/generate_test.go b/src/pkg/value/generate_test.go index 76a5195acb..f6cdbb268c 100644 --- a/src/pkg/value/generate_test.go +++ b/src/pkg/value/generate_test.go @@ -60,6 +60,251 @@ func TestGenerateJSONSchema(t *testing.T) { }) } +func TestMergeJSONSchemaAtPathPreservesNullableObjects(t *testing.T) { + schema := GenerateJSONSchema(Values{ + "serviceAccount": map[string]any{ + "server": map[string]any{ + "annotations": nil, + }, + }, + }) + + err := MergeJSONSchemaAtPath(schema, Path(".serviceAccount.server.annotations"), map[string]any{ + "type": []any{"object", "null"}, + "properties": map[string]any{}, + "required": []any{"not-imported"}, + }) + require.NoError(t, err) + + annotations, found, err := ExtractJSONSchema(schema, Path(".serviceAccount.server.annotations")) + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, []any{"object", "null"}, annotations["type"]) + assert.NotContains(t, annotations, "properties") + assert.NotContains(t, annotations, "required") + + existing := map[string]any{ + "type": "object", + "properties": map[string]any{ + "serviceAccount": map[string]any{ + "type": "object", + "properties": map[string]any{ + "server": map[string]any{ + "type": "object", + "properties": map[string]any{ + "annotations": map[string]any{ + "type": "string", + "description": "preserve this", + "required": []any{"package-owned"}, + }, + }, + }, + }, + }, + }, + } + result := ReconcileJSONSchema(existing, schema, false) + annotations, found, err = ExtractJSONSchema(result, Path(".serviceAccount.server.annotations")) + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, []any{"object", "null"}, annotations["type"]) + assert.Equal(t, "preserve this", annotations["description"]) + assert.Equal(t, []any{"package-owned"}, annotations["required"]) +} + +func TestMergeJSONSchemaAtPathAtMappedObject(t *testing.T) { + schema := GenerateJSONSchema(Values{ + "backend": map[string]any{ + "configMap": map[string]any{ + "annotations": nil, + }, + }, + }) + + err := MergeJSONSchemaAtPath(schema, Path(".backend"), map[string]any{ + "type": "object", + "properties": map[string]any{ + "configMap": map[string]any{ + "type": "object", + "properties": map[string]any{ + "annotations": map[string]any{ + "type": []any{"object", "null"}, + }, + }, + }, + }, + }) + require.NoError(t, err) + + annotations, found, err := ExtractJSONSchema(schema, Path(".backend.configMap.annotations")) + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, []any{"object", "null"}, annotations["type"]) +} + +func TestExtractJSONSchemaUsesAdditionalProperties(t *testing.T) { + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "config": map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "string", + }, + }, + }, + } + + result, found, err := ExtractJSONSchema(schema, Path(".config.database")) + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, "string", result["type"]) +} + +func TestMergeJSONSchemaAtPathCopiesValidationFields(t *testing.T) { + schema := GenerateJSONSchema(Values{ + "config": map[string]any{ + "database": "postgres", + "ports": []any{"http"}, + }, + }) + + err := MergeJSONSchemaAtPath(schema, Path(".config.database"), map[string]any{ + "type": "string", + "minLength": float64(3), + "enum": []any{"postgres", "mysql"}, + }) + require.NoError(t, err) + + database, found, err := ExtractJSONSchema(schema, Path(".config.database")) + require.NoError(t, err) + require.True(t, found) + assert.InDelta(t, float64(3), database["minLength"], 0) + assert.Equal(t, []any{"postgres", "mysql"}, database["enum"]) + + err = MergeJSONSchemaAtPath(schema, Path(".config.ports"), map[string]any{ + "type": "array", + "minItems": float64(1), + "maxItems": float64(3), + }) + require.NoError(t, err) + ports, found, err := ExtractJSONSchema(schema, Path(".config.ports")) + require.NoError(t, err) + require.True(t, found) + assert.InDelta(t, float64(1), ports["minItems"], 0) + assert.InDelta(t, float64(3), ports["maxItems"], 0) + + err = MergeJSONSchemaAtPath(schema, Path(".config"), map[string]any{ + "minProperties": float64(1), + "required": []any{"database"}, + "allOf": []any{map[string]any{"const": "postgres"}}, + }) + require.NoError(t, err) + config, found, err := ExtractJSONSchema(schema, Path(".config")) + require.NoError(t, err) + require.True(t, found) + assert.NotContains(t, config, "minProperties") + assert.NotContains(t, config, "required") + assert.NotContains(t, config, "allOf") +} + +func TestMergeJSONSchemaAtPathKeepsInferredFieldWhenChartRefIsDropped(t *testing.T) { + schema := GenerateJSONSchema(Values{ + "bad": map[string]any{ + "value": "inferred", + }, + }) + + err := MergeJSONSchemaAtPath(schema, Path("."), map[string]any{ + "type": "object", + "additionalProperties": false, + "properties": map[string]any{ + "good": map[string]any{ + "type": "string", + "minLength": float64(3), + }, + "bad": map[string]any{ + "type": "object", + "$ref": "schemas/external.json", + }, + }, + }) + require.NoError(t, err) + + properties, ok := schema["properties"].(map[string]any) + require.True(t, ok) + assert.Contains(t, properties, "good") + assert.Contains(t, properties, "bad", "the inferred field must remain available") + bad, ok := properties["bad"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "object", bad["type"]) + assert.Equal(t, false, schema["additionalProperties"]) +} + +func TestFilterChartSchemaDropsUnsupportedChildSchemas(t *testing.T) { + filtered := FilterChartSchema(map[string]any{ + "properties": map[string]any{ + "descriptionOnly": map[string]any{ + "description": "unsupported chart metadata", + }, + "name": map[string]any{ + "type": "string", + "description": "application name", + }, + }, + }) + + properties, ok := filtered["properties"].(map[string]any) + require.True(t, ok) + assert.NotContains(t, properties, "descriptionOnly") + assert.Equal(t, map[string]any{"type": "string"}, properties["name"]) +} + +func TestFilterChartSchemaDropsReferenceDependentChildSchemas(t *testing.T) { + filtered := FilterChartSchema(map[string]any{ + "properties": map[string]any{ + "good": map[string]any{ + "type": "string", + "minLength": float64(3), + }, + "external": map[string]any{ + "type": "object", + "$ref": "schemas/external.json", + }, + "local": map[string]any{ + "$ref": "#/$defs/Shared", + }, + "composed": map[string]any{ + "type": "object", + "allOf": []any{ + map[string]any{"$ref": "https://example.com/schema.json"}, + }, + }, + "nested": map[string]any{ + "type": "object", + "properties": map[string]any{ + "goodChild": map[string]any{"type": "boolean"}, + "badChild": map[string]any{"$ref": "#/$defs/Child"}, + }, + }, + }, + }) + + properties, ok := filtered["properties"].(map[string]any) + require.True(t, ok) + assert.Equal(t, map[string]any{"type": "string", "minLength": float64(3)}, properties["good"]) + assert.NotContains(t, properties, "external") + assert.NotContains(t, properties, "local") + assert.NotContains(t, properties, "composed") + + nested, ok := properties["nested"].(map[string]any) + require.True(t, ok) + nestedProperties, ok := nested["properties"].(map[string]any) + require.True(t, ok) + assert.Contains(t, nestedProperties, "goodChild") + assert.NotContains(t, nestedProperties, "badChild") +} + func TestReconcileJSONSchema(t *testing.T) { tests := []struct { name string diff --git a/src/pkg/value/schema.go b/src/pkg/value/schema.go index 6823dd35af..6e87a4466a 100644 --- a/src/pkg/value/schema.go +++ b/src/pkg/value/schema.go @@ -92,18 +92,45 @@ func CheckNoExternalRefs(schema map[string]any) error { return checkNoExternalRefsInObject(schema) } -var externalRefKeywords = []string{"$ref", "$dynamicRef", "$recursiveRef"} +func isReferenceKeyword(key string) bool { + switch key { + case "$ref", "$dynamicRef", "$recursiveRef": + return true + default: + return false + } +} -func checkNoExternalRefsInObject(node map[string]any) error { - for _, kw := range externalRefKeywords { - if val, has := node[kw]; has { - if ref, ok := val.(string); ok && !strings.HasPrefix(ref, "#") { - return fmt.Errorf("schema contains an external %q pointer %q; only internal references (\"#/...\") are supported — external files are not bundled into the assembled package", kw, ref) +func hasJSONSchemaReference(value any) bool { + switch value := value.(type) { + case map[string]any: + for key, child := range value { + if isReferenceKeyword(key) { + return true + } + if hasJSONSchemaReference(child) { + return true + } + } + case []any: + for _, child := range value { + if hasJSONSchemaReference(child) { + return true } } } + return false +} + +func checkNoExternalRefsInObject(node map[string]any) error { for _, key := range slices.Sorted(maps.Keys(node)) { - if err := checkNoExternalRefsInValue(key, node[key]); err != nil { + value := node[key] + if isReferenceKeyword(key) { + if ref, ok := value.(string); ok && !strings.HasPrefix(ref, "#") { + return fmt.Errorf("schema contains an external %q pointer %q; only internal references (\"#/...\") are supported — external files are not bundled into the assembled package", key, ref) + } + } + if err := checkNoExternalRefsInValue(key, value); err != nil { return err } } diff --git a/src/test/e2e/14_zarf_package_generate_test.go b/src/test/e2e/14_zarf_package_generate_test.go index b9e1b4d328..d81ac96ae2 100644 --- a/src/test/e2e/14_zarf_package_generate_test.go +++ b/src/test/e2e/14_zarf_package_generate_test.go @@ -5,6 +5,8 @@ package test import ( + "encoding/json" + "os" "path/filepath" "testing" @@ -111,8 +113,70 @@ func TestZarfDevGenerate(t *testing.T) { require.True(t, ok) require.Equal(t, "number", port["type"]) + configMap, ok := props["configMap"].(map[string]any) + require.True(t, ok) + configMapProps, ok := configMap["properties"].(map[string]any) + require.True(t, ok) + for _, field := range []string{"annotations", "labels"} { + fieldSchema, ok := configMapProps[field].(map[string]any) + require.True(t, ok) + require.Equal(t, []any{"object", "null"}, fieldSchema["type"]) + require.NotContains(t, fieldSchema, "properties") + require.NotContains(t, fieldSchema, "minProperties") + additionalProperties, ok := fieldSchema["additionalProperties"].(map[string]any) + require.True(t, ok) + require.Equal(t, "string", additionalProperties["type"]) + } + + // .backend.image should be dropped because it is excluded from the chart mapping. + _, hasExcludedImage := backendProps["image"] + require.False(t, hasExcludedImage) + // .oldField should be dropped from the values.schema.json _, hasOldField := props["oldField"] require.False(t, hasOldField) }) + + t.Run("Test generate-schema keeps usable chart fields when another field has an external ref", func(t *testing.T) { + packagePath := t.TempDir() + err := helpers.CreatePathAndCopy("src/test/packages/14-generate-schema", packagePath) + require.NoError(t, err) + + chartSchemaPath := filepath.Join(packagePath, "chart", "values.schema.json") + chartSchemaBytes, err := os.ReadFile(chartSchemaPath) + require.NoError(t, err) + + var chartSchema map[string]any + require.NoError(t, json.Unmarshal(chartSchemaBytes, &chartSchema)) + chartProperties, ok := chartSchema["properties"].(map[string]any) + require.True(t, ok) + // This field is not mapped and should be ignored without affecting the + // usable configMap schema below it. + chartProperties["unmapped"] = map[string]any{ + "$ref": "schemas/external.json#/definitions/Unmapped", + } + chartSchemaBytes, err = json.MarshalIndent(chartSchema, "", " ") + require.NoError(t, err) + require.NoError(t, os.WriteFile(chartSchemaPath, chartSchemaBytes, 0o644)) + + stdOut, stdErr, err := e2e.ZarfInDir(t, packagePath, "dev", "generate-schema", ".", "-u", "--delete-not-found", "--features=values=true") + require.NoError(t, err, stdOut, stdErr) + + schemaPath := filepath.Join(packagePath, "values.schema.json") + schema, _, err := value.LoadValidatedSchema(packagePath, schemaPath) + require.NoError(t, err) + + props, ok := schema["properties"].(map[string]any) + require.True(t, ok) + configMap, ok := props["configMap"].(map[string]any) + require.True(t, ok) + configMapProps, ok := configMap["properties"].(map[string]any) + require.True(t, ok) + annotations, ok := configMapProps["annotations"].(map[string]any) + require.True(t, ok) + additionalProperties, ok := annotations["additionalProperties"].(map[string]any) + require.True(t, ok) + require.Equal(t, "string", additionalProperties["type"]) + require.NotContains(t, props, "unmapped") + }) } diff --git a/src/test/packages/14-generate-schema/chart/templates/configmap.yaml b/src/test/packages/14-generate-schema/chart/templates/configmap.yaml index 4c4d62fbba..0cfadac7c9 100644 --- a/src/test/packages/14-generate-schema/chart/templates/configmap.yaml +++ b/src/test/packages/14-generate-schema/chart/templates/configmap.yaml @@ -2,6 +2,14 @@ apiVersion: v1 kind: ConfigMap metadata: name: schema-chart-values + {{ with .Values.configMap.annotations }} + annotations: + {{ toYaml . | nindent 4 }} + {{ end }} + {{ with .Values.configMap.labels }} + labels: + {{ toYaml . | nindent 4 }} + {{ end }} data: name: {{ .Values.name | default "unknown" | quote }} replicas: {{ .Values.replicaCount | quote }} diff --git a/src/test/packages/14-generate-schema/chart/values.schema.json b/src/test/packages/14-generate-schema/chart/values.schema.json new file mode 100644 index 0000000000..35ed68b2ae --- /dev/null +++ b/src/test/packages/14-generate-schema/chart/values.schema.json @@ -0,0 +1,27 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "configMap": { + "type": "object", + "properties": { + "annotations": { + "type": ["object", "null"], + "properties": {}, + "additionalProperties": { + "type": "string" + }, + "minProperties": 0 + }, + "labels": { + "type": ["object", "null"], + "properties": {}, + "additionalProperties": { + "type": "string" + }, + "minProperties": 0 + } + } + } + } +} diff --git a/src/test/packages/14-generate-schema/chart/values.yaml b/src/test/packages/14-generate-schema/chart/values.yaml index 6d161d76c2..04d4e79894 100644 --- a/src/test/packages/14-generate-schema/chart/values.yaml +++ b/src/test/packages/14-generate-schema/chart/values.yaml @@ -4,3 +4,7 @@ service: image: ref: nginx:latest + +configMap: + annotations: + labels: diff --git a/src/test/packages/14-generate-schema/zarf.yaml b/src/test/packages/14-generate-schema/zarf.yaml index ed0876ef9c..f59d50c9f7 100644 --- a/src/test/packages/14-generate-schema/zarf.yaml +++ b/src/test/packages/14-generate-schema/zarf.yaml @@ -22,6 +22,10 @@ components: targetPath: ".replicaCount" - sourcePath: ".network.port" targetPath: ".service.port" + - sourcePath: ".configMap.annotations" + targetPath: ".configMap.annotations" + - sourcePath: ".configMap.labels" + targetPath: ".configMap.labels" - name: backend-chart required: true