Merge pull request #30754 from mattfarina/simplify-jsonschema

Simplify the JSON Schema checking

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
 

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

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=

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"
 }

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)
 	}

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
 

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
 

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
 

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'
 

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",