From 0dffc580b0aec957b686fc08be681d3f74707749 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Mon, 14 Apr 2025 19:28:47 -0400 Subject: [PATCH] Simpligy the JSON Schema checking A new library was introduced that provides JSON Schema checking for newer versions of the schema. In Helm v4, there is no need to have two packages doing the JSON schema validation. The message output can have breaking changes. This change moves everything to the newer library. It also uses a wrapper error to enable a clean Helm only interface for the public Go API validation functions. This would enable the replacement of the Schema validation library, if needed, without breaking the Go API contract. Signed-off-by: Matt Farina --- Makefile | 2 +- go.mod | 3 - go.sum | 7 -- pkg/chart/v2/util/jsonschema.go | 74 +++++++------------ pkg/chart/v2/util/jsonschema_test.go | 12 +-- .../testdata/output/schema-negative-cli.txt | 2 +- pkg/cmd/testdata/output/schema-negative.txt | 4 +- .../output/subchart-schema-cli-negative.txt | 2 +- .../output/subchart-schema-negative.txt | 4 +- pkg/lint/rules/values_test.go | 4 +- 10 files changed, 41 insertions(+), 73 deletions(-) diff --git a/Makefile b/Makefile index 21144cf5a..0785fdb2e 100644 --- a/Makefile +++ b/Makefile @@ -156,7 +156,7 @@ format: $(GOIMPORTS) # Generate golden files used in unit tests .PHONY: gen-test-golden gen-test-golden: -gen-test-golden: PKG = ./cmd/helm ./pkg/action +gen-test-golden: PKG = ./pkg/cmd ./pkg/action gen-test-golden: TESTFLAGS = -update gen-test-golden: test-unit diff --git a/go.mod b/go.mod index ad119b6b8..36455fdba 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,6 @@ require ( github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 - github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.37.0 golang.org/x/term v0.31.0 golang.org/x/text v0.24.0 @@ -136,8 +135,6 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cast v1.7.0 // indirect github.com/x448/float16 v0.8.4 // indirect - github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect - github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xlab/treeprint v1.2.0 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/contrib/bridges/prometheus v0.57.0 // indirect diff --git a/go.sum b/go.sum index 4fcd483a4..2cf584745 100644 --- a/go.sum +++ b/go.sum @@ -326,13 +326,6 @@ github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOf github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= -github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo= -github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= -github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= -github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= -github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74= -github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ= github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/pkg/chart/v2/util/jsonschema.go b/pkg/chart/v2/util/jsonschema.go index 66ab42542..a8baef0f6 100644 --- a/pkg/chart/v2/util/jsonschema.go +++ b/pkg/chart/v2/util/jsonschema.go @@ -18,14 +18,11 @@ package util import ( "bytes" - "encoding/json" "fmt" "strings" "github.com/pkg/errors" "github.com/santhosh-tekuri/jsonschema/v6" - "github.com/xeipuuv/gojsonschema" - "sigs.k8s.io/yaml" chart "helm.sh/helm/v4/pkg/chart/v2" ) @@ -64,69 +61,50 @@ func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) (reterr error } }() - valuesData, err := yaml.Marshal(values) + // This unmarshal function leverages UseNumber() for number precision. The parser + // used for values does this as well. + schema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaJSON)) if err != nil { return err } - valuesJSON, err := yaml.YAMLToJSON(valuesData) + + compiler := jsonschema.NewCompiler() + err = compiler.AddResource("file:///values.schema.json", schema) if err != nil { return err } - if bytes.Equal(valuesJSON, []byte("null")) { - valuesJSON = []byte("{}") - } - - if schemaIs2020(schemaJSON) { - return validateUsingNewValidator(valuesJSON, schemaJSON) - } - - schemaLoader := gojsonschema.NewBytesLoader(schemaJSON) - valuesLoader := gojsonschema.NewBytesLoader(valuesJSON) - result, err := gojsonschema.Validate(schemaLoader, valuesLoader) + validator, err := compiler.Compile("file:///values.schema.json") if err != nil { return err } - if !result.Valid() { - var sb strings.Builder - for _, desc := range result.Errors() { - sb.WriteString(fmt.Sprintf("- %s\n", desc)) - } - return errors.New(sb.String()) + err = validator.Validate(values.AsMap()) + if err != nil { + return JSONSchemaValidationError{err} } return nil } -func validateUsingNewValidator(valuesJSON, schemaJSON []byte) error { - schema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaJSON)) - if err != nil { - return err - } - values, err := jsonschema.UnmarshalJSON(bytes.NewReader(valuesJSON)) - if err != nil { - return err - } +// Note, JSONSchemaValidationError is used to wrap the error from the underlying +// validation package so that Helm has a clean interface and the validation package +// could be replaced without changing the Helm SDK API. - compiler := jsonschema.NewCompiler() - err = compiler.AddResource("file:///values.schema.json", schema) - if err != nil { - return err - } +// JSONSchemaValidationError is the error returned when there is a schema validation +// error. +type JSONSchemaValidationError struct { + embeddedErr error +} - validator, err := compiler.Compile("file:///values.schema.json") - if err != nil { - return err - } +// Error prints the error message +func (e JSONSchemaValidationError) Error() string { + errStr := e.embeddedErr.Error() - return validator.Validate(values) -} + // This string prefixes all of our error details. Further up the stack of helm error message + // building more detail is provided to users. This is removed. + errStr = strings.TrimPrefix(errStr, "jsonschema validation failed with 'file:///values.schema.json#'\n") -func schemaIs2020(schemaJSON []byte) bool { - var partialSchema struct { - Schema string `json:"$schema"` - } - _ = json.Unmarshal(schemaJSON, &partialSchema) - return partialSchema.Schema == "https://json-schema.org/draft/2020-12/schema" + // The extra new line is needed for when there are sub-charts. + return errStr + "\n" } diff --git a/pkg/chart/v2/util/jsonschema_test.go b/pkg/chart/v2/util/jsonschema_test.go index 6337ab259..d781aa4be 100644 --- a/pkg/chart/v2/util/jsonschema_test.go +++ b/pkg/chart/v2/util/jsonschema_test.go @@ -69,7 +69,7 @@ func TestValidateAgainstSingleSchemaNegative(t *testing.T) { } schema, err := os.ReadFile("./testdata/test-values.schema.json") if err != nil { - t.Fatalf("Error reading YAML file: %s", err) + t.Fatalf("Error reading JSON file: %s", err) } var errString string @@ -79,8 +79,8 @@ func TestValidateAgainstSingleSchemaNegative(t *testing.T) { errString = err.Error() } - expectedErrString := `- (root): employmentInfo is required -- age: Must be greater than or equal to 0 + expectedErrString := `- at '': missing property 'employmentInfo' +- at '/age': minimum: got -5, want 0 ` if errString != expectedErrString { t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) @@ -174,7 +174,7 @@ func TestValidateAgainstSchemaNegative(t *testing.T) { } expectedErrString := `subchart: -- (root): age is required +- at '': missing property 'age' ` if errString != expectedErrString { t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) @@ -238,9 +238,9 @@ func TestValidateAgainstSchema2020Negative(t *testing.T) { } expectedErrString := `subchart: -jsonschema validation failed with 'file:///values.schema.json#' - at '/data': no items match contains schema - - at '/data/0': got number, want string` + - at '/data/0': got number, want string +` if errString != expectedErrString { t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) } diff --git a/pkg/cmd/testdata/output/schema-negative-cli.txt b/pkg/cmd/testdata/output/schema-negative-cli.txt index c4a5cc516..12bcc5103 100644 --- a/pkg/cmd/testdata/output/schema-negative-cli.txt +++ b/pkg/cmd/testdata/output/schema-negative-cli.txt @@ -1,4 +1,4 @@ Error: INSTALLATION FAILED: values don't meet the specifications of the schema(s) in the following chart(s): empty: -- age: Must be greater than or equal to 0 +- at '/age': minimum: got -5, want 0 diff --git a/pkg/cmd/testdata/output/schema-negative.txt b/pkg/cmd/testdata/output/schema-negative.txt index 929af5518..daf132635 100644 --- a/pkg/cmd/testdata/output/schema-negative.txt +++ b/pkg/cmd/testdata/output/schema-negative.txt @@ -1,5 +1,5 @@ Error: INSTALLATION FAILED: values don't meet the specifications of the schema(s) in the following chart(s): empty: -- (root): employmentInfo is required -- age: Must be greater than or equal to 0 +- at '': missing property 'employmentInfo' +- at '/age': minimum: got -5, want 0 diff --git a/pkg/cmd/testdata/output/subchart-schema-cli-negative.txt b/pkg/cmd/testdata/output/subchart-schema-cli-negative.txt index 7396b4bfe..179550f69 100644 --- a/pkg/cmd/testdata/output/subchart-schema-cli-negative.txt +++ b/pkg/cmd/testdata/output/subchart-schema-cli-negative.txt @@ -1,4 +1,4 @@ Error: INSTALLATION FAILED: values don't meet the specifications of the schema(s) in the following chart(s): subchart-with-schema: -- age: Must be greater than or equal to 0 +- at '/age': minimum: got -25, want 0 diff --git a/pkg/cmd/testdata/output/subchart-schema-negative.txt b/pkg/cmd/testdata/output/subchart-schema-negative.txt index 7b1f654a2..7522ef3e4 100644 --- a/pkg/cmd/testdata/output/subchart-schema-negative.txt +++ b/pkg/cmd/testdata/output/subchart-schema-negative.txt @@ -1,6 +1,6 @@ Error: INSTALLATION FAILED: values don't meet the specifications of the schema(s) in the following chart(s): chart-without-schema: -- (root): lastname is required +- at '': missing property 'lastname' subchart-with-schema: -- (root): age is required +- at '': missing property 'age' diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go index 8a2556a60..348695785 100644 --- a/pkg/lint/rules/values_test.go +++ b/pkg/lint/rules/values_test.go @@ -96,7 +96,7 @@ func TestValidateValuesFileSchemaFailure(t *testing.T) { t.Fatal("expected values file to fail parsing") } - assert.Contains(t, err.Error(), "Expected: string, given: integer", "integer should be caught by schema") + assert.Contains(t, err.Error(), "- at '/username': got number, want string") } func TestValidateValuesFileSchemaOverrides(t *testing.T) { @@ -129,7 +129,7 @@ func TestValidateValuesFile(t *testing.T) { name: "value not overridden", yaml: "username: admin\npassword:", overrides: map[string]interface{}{"username": "anotherUser"}, - errorMessage: "Expected: string, given: null", + errorMessage: "- at '/password': got null, want string", }, { name: "value overridden",