From 7aba8bbfe616b1c866655a7df4e443c9b10ee5ca Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 10 Sep 2025 11:36:07 +0200 Subject: [PATCH] Resolve JSON schema reference paths for relative imports When validating values against JSON schemas, relative file references in the schema (like "$ref": "./other-schema.json") were failing because in some cases. The JSON schema validator couldn't resolve the correct base directory. This change passes the absolute base directory to the schema validator, allowing it to properly resolve relative JSON schema references by constructing the correct file:// URLs with the proper path context. Tested with: - `helm lint .` - `helm lint subfolder` Fixes #31260 Signed-off-by: Benoit Tigeot Add test --- internal/chart/v3/lint/rules/values.go | 4 +- pkg/chart/common/util/jsonschema.go | 11 +-- pkg/chart/common/util/jsonschema_test.go | 82 +++++++++++++++---- .../current-dir-test/base.schema.json | 10 +++ .../current-dir-test/test-values.yaml | 3 + .../current-dir-test/values.schema.json | 14 ++++ .../testdata/subdir-test/shared.schema.json | 10 +++ .../subdir-test/subfolder/test-values.yaml | 3 + .../subdir-test/subfolder/values.schema.json | 14 ++++ pkg/chart/v2/lint/rules/values.go | 4 +- 10 files changed, 132 insertions(+), 23 deletions(-) create mode 100644 pkg/chart/common/util/testdata/current-dir-test/base.schema.json create mode 100644 pkg/chart/common/util/testdata/current-dir-test/test-values.yaml create mode 100644 pkg/chart/common/util/testdata/current-dir-test/values.schema.json create mode 100644 pkg/chart/common/util/testdata/subdir-test/shared.schema.json create mode 100644 pkg/chart/common/util/testdata/subdir-test/subfolder/test-values.yaml create mode 100644 pkg/chart/common/util/testdata/subdir-test/subfolder/values.schema.json diff --git a/internal/chart/v3/lint/rules/values.go b/internal/chart/v3/lint/rules/values.go index ba371cbe2..43eec40c2 100644 --- a/internal/chart/v3/lint/rules/values.go +++ b/internal/chart/v3/lint/rules/values.go @@ -75,9 +75,9 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV if err != nil { return err } - + baseDir := filepath.Dir(schemaPath) if !skipSchemaValidation { - return util.ValidateAgainstSingleSchema(coalescedValues, schema) + return util.ValidateAgainstSingleSchema(coalescedValues, schema, baseDir) } return nil diff --git a/pkg/chart/common/util/jsonschema.go b/pkg/chart/common/util/jsonschema.go index 63ca0c274..a24ee3a21 100644 --- a/pkg/chart/common/util/jsonschema.go +++ b/pkg/chart/common/util/jsonschema.go @@ -23,6 +23,7 @@ import ( "fmt" "log/slog" "net/http" + "path/filepath" "strings" "sync" "time" @@ -80,8 +81,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]any) error { } var sb strings.Builder if chrt.Schema() != nil { - slog.Debug("chart name", "chart-name", chrt.Name()) - err := ValidateAgainstSingleSchema(values, chrt.Schema()) + err := ValidateAgainstSingleSchema(values, chrt.Schema(), chrt.ChartFullPath()) if err != nil { fmt.Fprintf(&sb, "%s:\n", chrt.Name()) sb.WriteString(err.Error()) @@ -121,7 +121,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]any) error { } // ValidateAgainstSingleSchema checks that values does not violate the structure laid out in this schema -func ValidateAgainstSingleSchema(values common.Values, schemaJSON []byte) (reterr error) { +func ValidateAgainstSingleSchema(values common.Values, schemaJSON []byte, absBaseDir string) (reterr error) { defer func() { if r := recover(); r != nil { reterr = fmt.Errorf("unable to validate schema: %s", r) @@ -146,12 +146,13 @@ func ValidateAgainstSingleSchema(values common.Values, schemaJSON []byte) (reter compiler := jsonschema.NewCompiler() compiler.UseLoader(loader) - err = compiler.AddResource("file:///values.schema.json", schema) + base := "file://" + filepath.ToSlash(absBaseDir) + "/values.schema.json" + err = compiler.AddResource(base, schema) if err != nil { return err } - validator, err := compiler.Compile("file:///values.schema.json") + validator, err := compiler.Compile(base) if err != nil { return err } diff --git a/pkg/chart/common/util/jsonschema_test.go b/pkg/chart/common/util/jsonschema_test.go index 24073175c..06c518d73 100644 --- a/pkg/chart/common/util/jsonschema_test.go +++ b/pkg/chart/common/util/jsonschema_test.go @@ -20,6 +20,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "strings" "testing" @@ -37,7 +38,7 @@ func TestValidateAgainstSingleSchema(t *testing.T) { t.Fatalf("Error reading YAML file: %s", err) } - if err := ValidateAgainstSingleSchema(values, schema); err != nil { + if err := ValidateAgainstSingleSchema(values, schema, ""); err != nil { t.Errorf("Error validating Values against Schema: %s", err) } } @@ -53,7 +54,7 @@ func TestValidateAgainstInvalidSingleSchema(t *testing.T) { } var errString string - if err := ValidateAgainstSingleSchema(values, schema); err == nil { + if err := ValidateAgainstSingleSchema(values, schema, ""); err == nil { t.Fatalf("Expected an error, but got nil") } else { errString = err.Error() @@ -77,7 +78,7 @@ func TestValidateAgainstSingleSchemaNegative(t *testing.T) { } var errString string - if err := ValidateAgainstSingleSchema(values, schema); err == nil { + if err := ValidateAgainstSingleSchema(values, schema, ""); err == nil { t.Fatalf("Expected an error, but got nil") } else { errString = err.Error() @@ -177,11 +178,12 @@ func TestValidateAgainstSchemaNegative(t *testing.T) { errString = err.Error() } - expectedErrString := `subchart: -- at '': missing property 'age' -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) + expectedValidationError := "missing property 'age'" + if !strings.Contains(errString, "subchart:") { + t.Errorf("Error string should contain 'subchart:', got: %s", errString) + } + if !strings.Contains(errString, expectedValidationError) { + t.Errorf("Error string should contain '%s', got: %s", expectedValidationError, errString) } } @@ -241,12 +243,64 @@ func TestValidateAgainstSchema2020Negative(t *testing.T) { errString = err.Error() } - expectedErrString := `subchart: -- at '/data': no items match contains schema - - at '/data/0': got number, want string -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) + expectedValidationErrors := []string{ + "no items match contains schema", + "got number, want string", + } + if !strings.Contains(errString, "subchart:") { + t.Errorf("Error string should contain 'subchart:', got: %s", errString) + } + for _, expectedErr := range expectedValidationErrors { + if !strings.Contains(errString, expectedErr) { + t.Errorf("Error string should contain '%s', got: %s", expectedErr, errString) + } + } +} + +// TestValidateWithRelativeSchemaReferences tests schema validation with relative $ref paths +// This mimics the behavior of "helm lint ." where the schema is in the current directory +func TestValidateWithRelativeSchemaReferencesCurrentDir(t *testing.T) { + values, err := common.ReadValuesFile("./testdata/current-dir-test/test-values.yaml") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + schema, err := os.ReadFile("./testdata/current-dir-test/values.schema.json") + if err != nil { + t.Fatalf("Error reading JSON schema file: %s", err) + } + + // Test with absolute base directory - this should work with your fix + baseDir := "./testdata/current-dir-test" + absBaseDir, err := filepath.Abs(baseDir) + if err != nil { + t.Fatalf("Error getting absolute path: %s", err) + } + + if err := ValidateAgainstSingleSchema(values, schema, absBaseDir); err != nil { + t.Errorf("Error validating Values against Schema with relative references: %s", err) + } +} + +// TestValidateWithRelativeSchemaReferencesSubfolder tests schema validation with relative $ref paths +// This mimics the behavior of "helm lint subfolder" where the schema is in a subdirectory +func TestValidateWithRelativeSchemaReferencesSubfolder(t *testing.T) { + values, err := common.ReadValuesFile("./testdata/subdir-test/subfolder/test-values.yaml") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + schema, err := os.ReadFile("./testdata/subdir-test/subfolder/values.schema.json") + if err != nil { + t.Fatalf("Error reading JSON schema file: %s", err) + } + + baseDir := "./testdata/subdir-test/subfolder" + absBaseDir, err := filepath.Abs(baseDir) + if err != nil { + t.Fatalf("Error getting absolute path: %s", err) + } + + if err := ValidateAgainstSingleSchema(values, schema, absBaseDir); err != nil { + t.Errorf("Error validating Values against Schema with relative references from subfolder: %s", err) } } diff --git a/pkg/chart/common/util/testdata/current-dir-test/base.schema.json b/pkg/chart/common/util/testdata/current-dir-test/base.schema.json new file mode 100644 index 000000000..fddf95114 --- /dev/null +++ b/pkg/chart/common/util/testdata/current-dir-test/base.schema.json @@ -0,0 +1,10 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": ["name"] +} diff --git a/pkg/chart/common/util/testdata/current-dir-test/test-values.yaml b/pkg/chart/common/util/testdata/current-dir-test/test-values.yaml new file mode 100644 index 000000000..817fa4bd1 --- /dev/null +++ b/pkg/chart/common/util/testdata/current-dir-test/test-values.yaml @@ -0,0 +1,3 @@ +user: + name: "John Doe" +age: 30 diff --git a/pkg/chart/common/util/testdata/current-dir-test/values.schema.json b/pkg/chart/common/util/testdata/current-dir-test/values.schema.json new file mode 100644 index 000000000..a4cf4a53d --- /dev/null +++ b/pkg/chart/common/util/testdata/current-dir-test/values.schema.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "user": { + "$ref": "./base.schema.json" + }, + "age": { + "type": "integer", + "minimum": 0 + } + }, + "required": ["user", "age"] +} diff --git a/pkg/chart/common/util/testdata/subdir-test/shared.schema.json b/pkg/chart/common/util/testdata/subdir-test/shared.schema.json new file mode 100644 index 000000000..2bb750058 --- /dev/null +++ b/pkg/chart/common/util/testdata/subdir-test/shared.schema.json @@ -0,0 +1,10 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "config": { + "type": "string" + } + }, + "required": ["config"] +} diff --git a/pkg/chart/common/util/testdata/subdir-test/subfolder/test-values.yaml b/pkg/chart/common/util/testdata/subdir-test/subfolder/test-values.yaml new file mode 100644 index 000000000..b888e30f0 --- /dev/null +++ b/pkg/chart/common/util/testdata/subdir-test/subfolder/test-values.yaml @@ -0,0 +1,3 @@ +appConfig: + config: "production" +replicas: 3 diff --git a/pkg/chart/common/util/testdata/subdir-test/subfolder/values.schema.json b/pkg/chart/common/util/testdata/subdir-test/subfolder/values.schema.json new file mode 100644 index 000000000..9a50779b7 --- /dev/null +++ b/pkg/chart/common/util/testdata/subdir-test/subfolder/values.schema.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "appConfig": { + "$ref": "../shared.schema.json" + }, + "replicas": { + "type": "integer", + "minimum": 1 + } + }, + "required": ["appConfig", "replicas"] +} diff --git a/pkg/chart/v2/lint/rules/values.go b/pkg/chart/v2/lint/rules/values.go index 8fe849c7a..d06875320 100644 --- a/pkg/chart/v2/lint/rules/values.go +++ b/pkg/chart/v2/lint/rules/values.go @@ -75,9 +75,9 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV if err != nil { return err } - + baseDir := filepath.Dir(schemaPath) if !skipSchemaValidation { - return util.ValidateAgainstSingleSchema(coalescedValues, schema) + return util.ValidateAgainstSingleSchema(coalescedValues, schema, baseDir) } return nil