diff --git a/.gitignore b/.gitignore index b248db7..1c6f3cd 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,3 @@ dist/ .vscode -helm-schema diff --git a/cmd/helm-schema/main.go b/cmd/helm-schema/main.go index 112b062..d1c1b6f 100644 --- a/cmd/helm-schema/main.go +++ b/cmd/helm-schema/main.go @@ -222,7 +222,38 @@ func exec(cmd *cobra.Command, _ []string) error { defer os.RemoveAll(tempDir) } - go searching.SearchFiles(chartSearchRoot, chartSearchRoot, "Chart.yaml", dependenciesFilterMap, queue, errs) + // Search for Chart.yaml files in both the chart root and extracted temp directory + go func() { + queue1 := make(chan string) + queue2 := make(chan string) + + go searching.SearchFiles(chartSearchRoot, chartSearchRoot, "Chart.yaml", dependenciesFilterMap, queue1, errs) + + if tempDir != "" { + go searching.SearchFiles(chartSearchRoot, tempDir, "Chart.yaml", dependenciesFilterMap, queue2, errs) + } else { + close(queue2) + } + + // Merge results from both searches into the main queue + for queue1 != nil || queue2 != nil { + select { + case path, ok := <-queue1: + if !ok { + queue1 = nil + } else { + queue <- path + } + case path, ok := <-queue2: + if !ok { + queue2 = nil + } else { + queue <- path + } + } + } + close(queue) + }() wg := sync.WaitGroup{} @@ -321,6 +352,36 @@ drainErrors: } conditionsToPatch := make(map[string][][]string) + // Detect subchart schema overrides to skip validation errors + overrideInfo := schema.NewOverrideInfo() + if !noDeps { + for _, result := range results { + if result.Chart != nil && len(result.Chart.Dependencies) > 0 { + overrides, err := schema.DetectSubchartOverrides(result.ValuesPath, result.Chart.Dependencies) + if err != nil { + log.Warnf("Failed to detect subchart overrides for %s: %v", result.Chart.Name, err) + continue + } + + // Mark all overridden subcharts + for chartName := range overrides { + overrideInfo.MarkOverridden(chartName) + log.Debugf("Detected schema override for subchart: %s", chartName) + } + } + } + + // Clear validation errors for overridden subcharts + for _, result := range results { + if result.Chart != nil && overrideInfo.IsOverridden(result.Chart.Name) { + if len(result.Errors) > 0 { + log.Infof("Skipping validation errors for subchart %s (overridden by parent with $ref)", result.Chart.Name) + result.Errors = nil + } + } + } + } + if !noDeps { for _, result := range results { if len(result.Errors) > 0 { @@ -462,12 +523,46 @@ drainErrors: depSchema.Type = []string{"object", "boolean"} } depSchema.DisableRequiredProperties() + // Merge subchart definitions into parent root definitions + if dependencyResult.Schema.Definitions != nil { + if result.Schema.Definitions == nil { + result.Schema.Definitions = make(map[string]*schema.Schema) + } + for defName, defSchema := range dependencyResult.Schema.Definitions { + if _, exists := result.Schema.Definitions[defName]; exists { + log.Warnf("Definition '%s' from subchart %s conflicts with existing definition in parent %s", defName, dep.Name, result.Chart.Name) + } else { + result.Schema.Definitions[defName] = defSchema + log.Debugf("Merged definition '%s' from subchart %s into parent %s", defName, dep.Name, result.Chart.Name) + } + } + } + // Only add subchart schema if parent doesn't already have a property for this dependency + // (parent may have a $ref override) + depKey := dep.Name if dep.Alias != "" { - result.Schema.Properties[dep.Alias] = &depSchema + depKey = dep.Alias + } + + if existingProp, exists := result.Schema.Properties[depKey]; !exists { + result.Schema.Properties[depKey] = &depSchema } else { - result.Schema.Properties[dep.Name] = &depSchema + // Parent has properties for this subchart - merge and allow additional properties + log.Debugf("Parent chart %s has property '%s', merging with subchart schema", result.Chart.Name, depKey) + mergeSchemaProperties( + existingProp, + &depSchema, + nil, + fmt.Sprintf("subchart %s", dep.Name), + fmt.Sprintf("parent chart %s property %s", result.Chart.Name, depKey), + ) + // Set additionalProperties to true to allow all subchart properties + additionalPropsTrue := true + existingProp.AdditionalProperties = &additionalPropsTrue + log.Debugf("Set additionalProperties=true for %s in parent %s (has partial values)", depKey, result.Chart.Name) } + } } else { diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 8ded277..873a5bd 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -1927,6 +1927,7 @@ func decodeNodeValue(node *yaml.Node) (interface{}, error) { // - If it's a relative file path, it attempts to load and parse the referenced schema // - If it includes a JSON pointer (#/path/to/schema), it extracts the specific schema section // - The resolved schema replaces the original reference +// - If the pointer references a definition, it extracts all definitions from the file // // Parameters: // - schema: Pointer to the Schema object containing the references to resolve @@ -1958,11 +1959,38 @@ func handleSchemaRefs(schema *Schema, valuesPath string) error { } if len(refParts) > 1 { - // Found json-pointer + // Found json-pointer - extract definitions but keep the reference + + // First, unmarshal the full schema to extract definitions + var fullSchema Schema + if err := json.Unmarshal(byteValue, &fullSchema); err != nil { + return fmt.Errorf("failed to unmarshal full schema from %s: %w", relFilePath, err) + } + + // If the pointer references a definition, extract all definitions and convert to internal ref + if strings.HasPrefix(refParts[1], "/definitions/") && fullSchema.Definitions != nil { + // Copy all definitions from the source file + if schema.Definitions == nil { + schema.Definitions = make(map[string]*Schema) + } + for defName, defSchema := range fullSchema.Definitions { + if _, exists := schema.Definitions[defName]; !exists { + schema.Definitions[defName] = defSchema + } + } + + // Convert external reference to internal reference + schema.Ref = "#" + refParts[1] + schema.HasData = true + return nil + } + + // For non-definition references, resolve and inline as before var obj interface{} if err := json.Unmarshal(byteValue, &obj); err != nil { return fmt.Errorf("failed to unmarshal JSON from %s: %w", relFilePath, err) } + jsonPointerResultRaw, err := jsonpointer.Get(obj, refParts[1]) if err != nil { return fmt.Errorf("failed to resolve JSON pointer %s in %s: %w", refParts[1], relFilePath, err) diff --git a/pkg/schema/worker.go b/pkg/schema/worker.go index bc2714d..88da545 100644 --- a/pkg/schema/worker.go +++ b/pkg/schema/worker.go @@ -20,6 +20,25 @@ type Result struct { Errors []error } +// OverrideInfo tracks which subcharts have schema overrides in parent charts +type OverrideInfo struct { + overriddenCharts map[string]bool // map of chart names that have $ref overrides +} + +func NewOverrideInfo() *OverrideInfo { + return &OverrideInfo{ + overriddenCharts: make(map[string]bool), + } +} + +func (o *OverrideInfo) MarkOverridden(chartName string) { + o.overriddenCharts[chartName] = true +} + +func (o *OverrideInfo) IsOverridden(chartName string) bool { + return o.overriddenCharts[chartName] +} + func Worker( dryRun, uncomment, addSchemaReference, keepFullComment, helmDocsCompatibilityMode, dontRemoveHelmDocsPrefix, dontAddGlobal, annotate bool, valueFileNames []string, @@ -162,3 +181,77 @@ func Worker( results <- result } } + +// DetectSubchartOverrides scans a values file for subchart keys with $ref overrides +// Returns a map of subchart names that have schema overrides +func DetectSubchartOverrides(valuesPath string, dependencies []*chart.Dependency) (map[string]bool, error) { + overrides := make(map[string]bool) + + if len(dependencies) == 0 { + return overrides, nil + } + + valuesFile, err := os.Open(valuesPath) + if err != nil { + return overrides, err + } + defer valuesFile.Close() + + content, err := util.ReadFileAndFixNewline(valuesFile) + if err != nil { + return overrides, err + } + + var values yaml.Node + if err := yaml.Unmarshal(content, &values); err != nil { + return overrides, err + } + + // Navigate to the mapping node + if values.Kind != yaml.DocumentNode || len(values.Content) != 1 { + return overrides, nil + } + + mappingNode := values.Content[0] + if mappingNode.Kind != yaml.MappingNode { + return overrides, nil + } + + // Create a map of dependency names and aliases for quick lookup + depNames := make(map[string]bool) + for _, dep := range dependencies { + depNames[dep.Name] = true + if dep.Alias != "" { + depNames[dep.Alias] = true + } + } + + // Check each key in the values file + for i := 0; i < len(mappingNode.Content); i += 2 { + keyNode := mappingNode.Content[i] + + // Check if this key matches a dependency name or alias + if !depNames[keyNode.Value] { + continue + } + + // Check if this key has a @schema annotation with $ref + comment := keyNode.HeadComment + if comment == "" { + continue + } + + keyNodeSchema, _, err := GetSchemaFromComment(comment) + if err != nil { + continue // Ignore parse errors, just skip this key + } + + // If this dependency key has a $ref, mark it as overridden + if keyNodeSchema.Ref != "" { + overrides[keyNode.Value] = true + } + } + + return overrides, nil +} + diff --git a/pkg/schema/worker_test.go b/pkg/schema/worker_test.go index 1fd725d..9a75995 100644 --- a/pkg/schema/worker_test.go +++ b/pkg/schema/worker_test.go @@ -108,7 +108,7 @@ key1: value1 // Create test files for filename, content := range tt.setupFiles { path := filepath.Join(tmpDir, filename) - err := os.WriteFile(path, []byte(content), 0644) + err := os.WriteFile(path, []byte(content), 0o644) assert.NoError(t, err) } diff --git a/tests/config_schema.json b/tests/config_schema.json new file mode 100644 index 0000000..7ea132c --- /dev/null +++ b/tests/config_schema.json @@ -0,0 +1,28 @@ +{ + "$defs": { + "RestConfig": { + "additionalProperties": false, + "properties": { + "url": { + "default": "http://localhost:8000/", + "format": "uri", + "maxLength": 2083, + "minLength": 1, + "title": "Url", + "type": "string" + } + }, + "title": "RestConfig", + "type": "object" + } + }, + "additionalProperties": false, + "description": "Config for the worker application as a whole. Root of config tree.", + "properties": { + "api": { + "$ref": "#/$defs/RestConfig" + } + }, + "title": "ApplicationConfig", + "type": "object" +} diff --git a/tests/test_defs_propagation.yaml b/tests/test_defs_propagation.yaml new file mode 100644 index 0000000..843d717 --- /dev/null +++ b/tests/test_defs_propagation.yaml @@ -0,0 +1,7 @@ +# @schema +# $ref: config_schema.json +# @schema +# -- Config for the worker goes here +worker: + api: + url: http://0.0.0.0:8000/ diff --git a/tests/test_defs_propagation_expected.schema.json b/tests/test_defs_propagation_expected.schema.json new file mode 100644 index 0000000..86630c2 --- /dev/null +++ b/tests/test_defs_propagation_expected.schema.json @@ -0,0 +1,45 @@ +{ + "$defs": { + "RestConfig": { + "additionalProperties": false, + "properties": { + "url": { + "default": "http://localhost:8000/", + "format": "uri", + "maxLength": 2083, + "minLength": 1, + "title": "Url", + "type": "string" + } + }, + "required": [], + "title": "RestConfig", + "type": "object" + } + }, + "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, + "properties": { + "global": { + "description": "Global values are values that can be accessed from any chart or subchart by exactly the same name.", + "required": [], + "title": "global", + "type": "object" + }, + "worker": { + "additionalProperties": false, + "description": "Config for the worker application as a whole. Root of config tree.", + "properties": { + "api": { + "$ref": "#/$defs/RestConfig", + "required": [] + } + }, + "required": [], + "title": "ApplicationConfig", + "type": "object" + } + }, + "required": [], + "type": "object" +}