From 7e89d0dcd32882d4a86e0bcdf051a2a02c148d7a Mon Sep 17 00:00:00 2001 From: Ferran Vidal Date: Tue, 13 Sep 2022 10:04:47 +0200 Subject: [PATCH] Replace xeipuuv/gojsonschema with santhosh-tekuri/jsonschema Signed-off-by: Ferran Vidal --- go.mod | 3 - go.sum | 7 -- pkg/chart/v2/util/jsonschema.go | 81 +++++++-------- pkg/chart/v2/util/jsonschema_test.go | 98 +++++++------------ .../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 | 6 +- 9 files changed, 76 insertions(+), 131 deletions(-) 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..1924f3579 100644 --- a/pkg/chart/v2/util/jsonschema.go +++ b/pkg/chart/v2/util/jsonschema.go @@ -24,8 +24,6 @@ import ( "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" ) @@ -34,10 +32,9 @@ import ( func ValidateAgainstSchema(chrt *chart.Chart, values map[string]interface{}) error { var sb strings.Builder if chrt.Schema != nil { - err := ValidateAgainstSingleSchema(values, chrt.Schema) - if err != nil { + if err := ValidateAgainstSingleSchema(values, chrt.Schema); err != nil { sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name())) - sb.WriteString(err.Error()) + sb.WriteString(fmt.Sprintf("%s\n", err.Error())) } } @@ -64,69 +61,57 @@ func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) (reterr error } }() - valuesData, err := yaml.Marshal(values) - if err != nil { - return err - } - valuesJSON, err := yaml.YAMLToJSON(valuesData) + schema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaJSON)) 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) - if err != nil { + compiler := jsonschema.NewCompiler() + if err := compiler.AddResource("file:///values.schema.json", schema); 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()) - } - - return nil -} - -func validateUsingNewValidator(valuesJSON, schemaJSON []byte) error { - schema, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaJSON)) + validator, err := compiler.Compile("file:///values.schema.json") if err != nil { return err } - values, err := jsonschema.UnmarshalJSON(bytes.NewReader(valuesJSON)) + + valuesJSON, err := json.Marshal(values) if err != nil { return err } - compiler := jsonschema.NewCompiler() - err = compiler.AddResource("file:///values.schema.json", schema) - if err != nil { - return err + if bytes.Equal(valuesJSON, []byte("null")) { + valuesJSON = []byte("{}") } - validator, err := compiler.Compile("file:///values.schema.json") + valuesObj, err := jsonschema.UnmarshalJSON(bytes.NewReader(valuesJSON)) if err != nil { return err } - return validator.Validate(values) -} + if err = validator.Validate(valuesObj); err != nil { + var jsonschemaError *jsonschema.ValidationError + if errors.As(err, &jsonschemaError) { + // We remove the initial error line as it points to a fake schema file location, and might lead to confusion. + // We replace the empty location string with `/` to make it more readable. + cleanErrMessage := strings.Replace( + strings.Replace( + jsonschemaError.Error(), + "jsonschema validation failed with 'file:///values.schema.json#'\n", + "", + -1, + ), + "- at '':", + "- at '/':", + -1, + ) + + return errors.New(cleanErrMessage) + } -func schemaIs2020(schemaJSON []byte) bool { - var partialSchema struct { - Schema string `json:"$schema"` + return err } - _ = json.Unmarshal(schemaJSON, &partialSchema) - return partialSchema.Schema == "https://json-schema.org/draft/2020-12/schema" + + return nil } diff --git a/pkg/chart/v2/util/jsonschema_test.go b/pkg/chart/v2/util/jsonschema_test.go index 6337ab259..1d30ebc87 100644 --- a/pkg/chart/v2/util/jsonschema_test.go +++ b/pkg/chart/v2/util/jsonschema_test.go @@ -20,6 +20,8 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" + chart "helm.sh/helm/v4/pkg/chart/v2" ) @@ -40,51 +42,31 @@ func TestValidateAgainstSingleSchema(t *testing.T) { func TestValidateAgainstInvalidSingleSchema(t *testing.T) { values, err := ReadValuesFile("./testdata/test-values.yaml") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } - schema, err := os.ReadFile("./testdata/test-values-invalid.schema.json") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } + assert.NoError(t, err) - var errString string - if err := ValidateAgainstSingleSchema(values, schema); err == nil { - t.Fatalf("Expected an error, but got nil") - } else { - errString = err.Error() - } + schema, err := os.ReadFile("./testdata/test-values-invalid.schema.json") + assert.NoError(t, err) - expectedErrString := "unable to validate schema: runtime error: invalid " + - "memory address or nil pointer dereference" - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) - } + assert.EqualError( + t, + ValidateAgainstSingleSchema(values, schema), + "unable to validate schema: runtime error: invalid memory address or nil pointer dereference", + ) } func TestValidateAgainstSingleSchemaNegative(t *testing.T) { values, err := ReadValuesFile("./testdata/test-values-negative.yaml") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } - schema, err := os.ReadFile("./testdata/test-values.schema.json") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } - - var errString string - if err := ValidateAgainstSingleSchema(values, schema); err == nil { - t.Fatalf("Expected an error, but got nil") - } else { - errString = err.Error() - } + assert.NoError(t, err) - expectedErrString := `- (root): employmentInfo is required -- age: Must be greater than or equal to 0 -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) - } + schema, err := os.ReadFile("./testdata/test-values.schema.json") + assert.NoError(t, err) + + assert.EqualError( + t, + ValidateAgainstSingleSchema(values, schema), + `- at '/': missing property 'employmentInfo' +- at '/age': minimum: got -5, want 0`, + ) } const subchartSchema = `{ @@ -166,19 +148,13 @@ func TestValidateAgainstSchemaNegative(t *testing.T) { "subchart": map[string]interface{}{}, } - var errString string - if err := ValidateAgainstSchema(chrt, vals); err == nil { - t.Fatalf("Expected an error, but got nil") - } else { - errString = err.Error() - } - - expectedErrString := `subchart: -- (root): age is required -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) - } + assert.EqualError( + t, + ValidateAgainstSchema(chrt, vals), + `subchart: +- at '/': missing property 'age' +`, + ) } func TestValidateAgainstSchema2020(t *testing.T) { @@ -230,18 +206,12 @@ func TestValidateAgainstSchema2020Negative(t *testing.T) { }, } - var errString string - if err := ValidateAgainstSchema(chrt, vals); err == nil { - t.Fatalf("Expected an error, but got nil") - } else { - errString = err.Error() - } - - expectedErrString := `subchart: -jsonschema validation failed with 'file:///values.schema.json#' + assert.EqualError( + t, + ValidateAgainstSchema(chrt, vals), + `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) - } + - at '/data/0': got number, want string +`, + ) } 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..c11fe3b0d 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..d52814c9c 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..43b19a743 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.Equal(t, err.Error(), "- at '/username': got number, want string", "integer should be caught by schema") } 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", @@ -153,7 +153,7 @@ func TestValidateValuesFile(t *testing.T) { case err == nil && tt.errorMessage != "": t.Error("expected values file to fail parsing") case err != nil && tt.errorMessage != "": - assert.Contains(t, err.Error(), tt.errorMessage, "Failed with unexpected error") + assert.Equal(t, err.Error(), tt.errorMessage, "Failed with unexpected error") } }) }