From a2fafd3c5fdd70d54e7d5f05192fc3af74c04778 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 0af9765dd..5bec878dd 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]interface{}, ski 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 acd2ca100..b94ed8840 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" "time" @@ -79,8 +80,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro } 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 { sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name())) sb.WriteString(err.Error()) @@ -107,7 +107,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro } // 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) @@ -131,12 +131,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 b34f9d514..3660aace2 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 994a6a463..175effe2c 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]interface{}, ski if err != nil { return err } - + baseDir := filepath.Dir(schemaPath) if !skipSchemaValidation { - return util.ValidateAgainstSingleSchema(coalescedValues, schema) + return util.ValidateAgainstSingleSchema(coalescedValues, schema, baseDir) } return nil