From 686b620a1f87852e1b991f0114d72e600595220a Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Fri, 22 Feb 2019 16:25:28 -0600 Subject: [PATCH] Check that values.yaml matches schema This commit uses the gojsonschema package to validate a values.yaml file against a corresponding values.schema.yaml file. Signed-off-by: Ian Howell --- Gopkg.lock | 25 ++ Gopkg.toml | 3 + .../testdata/test-values-negative.schema.yaml | 45 +++ .../testdata/test-values-negative.yaml | 14 + .../testdata/test-values.schema.yaml | 32 +- pkg/chartutil/testdata/test-values.yaml | 17 ++ pkg/chartutil/values.go | 81 ++++-- pkg/chartutil/values_test.go | 275 +++++++++--------- 8 files changed, 316 insertions(+), 176 deletions(-) create mode 100644 pkg/chartutil/testdata/test-values-negative.schema.yaml create mode 100644 pkg/chartutil/testdata/test-values-negative.yaml create mode 100644 pkg/chartutil/testdata/test-values.yaml diff --git a/Gopkg.lock b/Gopkg.lock index c150a63cf..7e4514968 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -835,6 +835,30 @@ revision = "ffdc059bfe9ce6a4e144ba849dbedead332c6053" version = "v1.3.0" +[[projects]] + branch = "master" + digest = "1:f4e5276a3b356f4692107047fd2890f2fe534f4feeb6b1fd2f6dfbd87f1ccf54" + name = "github.com/xeipuuv/gojsonpointer" + packages = ["."] + pruneopts = "UT" + revision = "4e3ac2762d5f479393488629ee9370b50873b3a6" + +[[projects]] + branch = "master" + digest = "1:dc6a6c28ca45d38cfce9f7cb61681ee38c5b99ec1425339bfc1e1a7ba769c807" + name = "github.com/xeipuuv/gojsonreference" + packages = ["."] + pruneopts = "UT" + revision = "bd5ef7bd5415a7ac448318e64f11a24cd21e594b" + +[[projects]] + digest = "1:1c898ea6c30c16e8d55fdb6fe44c4bee5f9b7d68aa260cfdfc3024491dcc7bea" + name = "github.com/xeipuuv/gojsonschema" + packages = ["."] + pruneopts = "UT" + revision = "f971f3cd73b2899de6923801c147f075263e0c50" + version = "v1.1.0" + [[projects]] digest = "1:340553b2fdaab7d53e63fd40f8ed82203bdd3274253055bdb80a46828482ef81" name = "github.com/xenolf/lego" @@ -1676,6 +1700,7 @@ "github.com/spf13/pflag", "github.com/stretchr/testify/assert", "github.com/stretchr/testify/suite", + "github.com/xeipuuv/gojsonschema", "golang.org/x/crypto/openpgp", "golang.org/x/crypto/openpgp/clearsign", "golang.org/x/crypto/openpgp/errors", diff --git a/Gopkg.toml b/Gopkg.toml index ec0fdeae3..fe26c0010 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -107,3 +107,6 @@ go-tests = true unused-packages = true +[[constraint]] + name = "github.com/xeipuuv/gojsonschema" + version = "1.1.0" diff --git a/pkg/chartutil/testdata/test-values-negative.schema.yaml b/pkg/chartutil/testdata/test-values-negative.schema.yaml new file mode 100644 index 000000000..2c8345c35 --- /dev/null +++ b/pkg/chartutil/testdata/test-values-negative.schema.yaml @@ -0,0 +1,45 @@ +title: Values +type: object +properties: + firstname: + description: First name + type: string + lastname: + type: string + likesCoffee: + type: boolean + age: + description: Age + type: integer + minimum: 0 + employmentInfo: + type: object + properties: + salary: + type: number + minimum: 0 + title: + type: string + required: + - salary + addresses: + description: List of addresses + type: array + items: + type: object + properties: + city: + type: string + street: + type: string + number: + type: number + phoneNumbers: + type: array + items: + type: string +required: + - firstname + - lastname + - addresses + - employmentInfo diff --git a/pkg/chartutil/testdata/test-values-negative.yaml b/pkg/chartutil/testdata/test-values-negative.yaml new file mode 100644 index 000000000..5a1250bff --- /dev/null +++ b/pkg/chartutil/testdata/test-values-negative.yaml @@ -0,0 +1,14 @@ +firstname: John +lastname: Doe +age: -5 +likesCoffee: true +addresses: + - city: Springfield + street: Main + number: 12345 + - city: New York + street: Broadway + number: 67890 +phoneNumbers: + - "(888) 888-8888" + - "(555) 555-5555" diff --git a/pkg/chartutil/testdata/test-values.schema.yaml b/pkg/chartutil/testdata/test-values.schema.yaml index e35f4c84a..2c8345c35 100644 --- a/pkg/chartutil/testdata/test-values.schema.yaml +++ b/pkg/chartutil/testdata/test-values.schema.yaml @@ -6,17 +6,17 @@ properties: type: string lastname: type: string - likes-coffee: + likesCoffee: type: boolean age: description: Age type: integer minimum: 0 - employment-info: + employmentInfo: type: object properties: salary: - type: float + type: number minimum: 0 title: type: string @@ -24,18 +24,22 @@ properties: - salary addresses: description: List of addresses - type: list[object] - properties: - city: - type: string - street: - type: string - number: - type: number - phone-numbers: - type: list[string] + type: array + items: + type: object + properties: + city: + type: string + street: + type: string + number: + type: number + phoneNumbers: + type: array + items: + type: string required: - firstname - lastname - addresses - - employment-info + - employmentInfo diff --git a/pkg/chartutil/testdata/test-values.yaml b/pkg/chartutil/testdata/test-values.yaml new file mode 100644 index 000000000..042dea664 --- /dev/null +++ b/pkg/chartutil/testdata/test-values.yaml @@ -0,0 +1,17 @@ +firstname: John +lastname: Doe +age: 25 +likesCoffee: true +employmentInfo: + title: Software Developer + salary: 100000 +addresses: + - city: Springfield + street: Main + number: 12345 + - city: New York + street: Broadway + number: 67890 +phoneNumbers: + - "(888) 888-8888" + - "(555) 555-5555" diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index 8e5774fbe..50897c036 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -17,14 +17,17 @@ limitations under the License. package chartutil import ( + "encoding/json" "fmt" "io" "io/ioutil" "log" + "os" "strings" "github.com/ghodss/yaml" "github.com/pkg/errors" + "github.com/xeipuuv/gojsonschema" "helm.sh/helm/pkg/chart" ) @@ -45,24 +48,8 @@ const GlobalKey = "global" // Values represents a collection of chart values. type Values map[string]interface{} -// SchemaProperties represents the nested objects of a schema -type SchemaProperties map[string]*Schema - -// Schema is a JSON schema which can be applied to a values file to validate it -type Schema struct { - Title string `json:"title,omitempty"` - Description string `json:"description,omitempty"` - Type string `json:"type,omitempty"` - Properties SchemaProperties `json:"properties,omitempty"` - Required []string `json:"required,omitempty"` - Minimum int `json:"minimum,omitempty"` -} - -// YAML encodes the Values into a YAML string. -func (s Schema) YAML() (string, error) { - b, err := yaml.Marshal(s) - return string(b), err -} +// Schema represents the document structure to validate the values.yaml file against +type Schema map[string]interface{} // YAML encodes the Values into a YAML string. func (v Values) YAML() (string, error) { @@ -146,6 +133,9 @@ func ReadValues(data []byte) (vals Values, err error) { // ReadSchema will parse YAML byte data into a Schema. func ReadSchema(data []byte) (schema Schema, err error) { err = yaml.Unmarshal(data, &schema) + if len(schema) == 0 { + schema = Schema{} + } return schema, err } @@ -155,6 +145,18 @@ func ReadValuesFile(filename string) (Values, error) { if err != nil { return map[string]interface{}{}, err } + + schemaPath := strings.Replace(filename, ".yaml", ".schema.yaml", 1) + _, err = os.Stat(schemaPath) + if err == nil { + schemaData, err := ioutil.ReadFile(schemaPath) + if err != nil { + return map[string]interface{}{}, err + } + + return ReadSchematizedValues(data, schemaData) + } + return ReadValues(data) } @@ -167,6 +169,40 @@ func ReadSchemaFile(filename string) (Schema, error) { return ReadSchema(data) } +// ReadSchematizedValues parses a YAML file and asserts that it matches the schema +func ReadSchematizedValues(data, schemaData []byte) (Values, error) { + values, err := ReadValues(data) + if err != nil { + return Values{}, err + } + valuesJSON := convertToJSON(values) + + schema, err := ReadSchema(schemaData) + if err != nil { + return Values{}, err + } + schemaJSON := convertToJSON(schema) + + schemaLoader := gojsonschema.NewStringLoader(string(schemaJSON)) + valuesLoader := gojsonschema.NewStringLoader(string(valuesJSON)) + + result, err := gojsonschema.Validate(schemaLoader, valuesLoader) + if err != nil { + return Values{}, err + } + + if !result.Valid() { + var sb strings.Builder + sb.WriteString("The values.yaml is not valid. see errors :\n") + for _, desc := range result.Errors() { + sb.WriteString(fmt.Sprintf("- %s\n", desc)) + } + return Values{}, errors.New(sb.String()) + } + + return values, nil +} + // CoalesceValues coalesces all of the values in a chart (and its subcharts). // // Values are coalesced together using the following rules: @@ -375,6 +411,15 @@ func istable(v interface{}) bool { return ok } +// convertToJSON takes YAML and returns a []byte representation of the same object as JSON +func convertToJSON(data interface{}) []byte { + js, err := json.Marshal(data) + if err != nil { + panic(err.Error()) + } + return js +} + // PathValue takes a path that traverses a YAML structure and returns the value at the end of that path. // The path starts at the root of the YAML structure and is comprised of YAML keys separated by periods. // Given the following YAML data the value at path "chapter.one.title" is "Loomings". diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index d1cee8cd1..fbec5ae9d 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -20,6 +20,7 @@ import ( "bytes" "encoding/json" "fmt" + "strings" "testing" "text/template" @@ -168,6 +169,33 @@ func TestReadSchemaFile(t *testing.T) { matchSchema(t, data) } +func TestReadSchematizedValues(t *testing.T) { + _, err := ReadValuesFile("./testdata/test-values.yaml") + if err != nil { + t.Errorf("Got the following unexpected error while reading schematized values:\n%v", err) + } +} + +func TestReadSchematizedValuesNegative(t *testing.T) { + _, err := ReadValuesFile("./testdata/test-values-negative.yaml") + + if err == nil { + t.Errorf("Expected an error, but got none") + } + + errString := err.Error() + if !strings.Contains(errString, "The values.yaml is not valid. see errors :") { + t.Errorf("Error string does not contain expected string: \"The values.yaml is not valid. see errors :\"") + } + if !strings.Contains(errString, "- (root): employmentInfo is required") { + t.Errorf("Error string does not contain expected string: \"- (root): employmentInfo is required\"") + } + + if !strings.Contains(errString, "- age: Must be greater than or equal to 0/1") { + t.Errorf("Error string does not contain expected string: \"- age: Must be greater than or equal to 0/1\"") + } +} + func ExampleValues() { doc := ` title: "Moby Dick" @@ -454,17 +482,17 @@ properties: type: string lastname: type: string - likes-coffee: + likesCoffee: type: boolean age: description: Age type: integer minimum: 0 - employment-info: + employmentInfo: type: object properties: salary: - type: float + type: number minimum: 0 title: type: string @@ -472,21 +500,25 @@ properties: - salary addresses: description: List of addresses - type: list[object] - properties: - city: - type: string - street: - type: string - number: - type: number - phone-numbers: - type: list[string] + type: array + items: + type: object + properties: + city: + type: string + street: + type: string + number: + type: number + phoneNumbers: + type: array + items: + type: string required: - firstname - lastname - addresses - - employment-info + - employmentInfo ` data, err := ReadSchema([]byte(schemaTest)) if err != nil { @@ -496,154 +528,109 @@ required: } func matchSchema(t *testing.T, data Schema) { - if data.Title != "Values" { - t.Errorf("Expected .title to be 'Values', got '%s'", data.Title) + if data["title"] != "Values" { + t.Errorf("Expected .title to be 'Values', got '%s'", data["title"]) } - if data.Type != "object" { - t.Errorf("Expected .type to be 'object', got '%s'", data.Type) - } - expectedRequired := []string{ - "firstname", - "lastname", - "addresses", - "employment-info", - } - if len(data.Required) != 4 { - t.Errorf("Expected length of .required to be 4, got %d", len(data.Required)) + if data["type"] != "object" { + t.Errorf("Expected .type to be 'object', got '%s'", data["type"]) } - if !assertEqualSlices(data.Required, expectedRequired) { - t.Errorf("Expected .required to be %v, got %v", expectedRequired, data.Required) + if o, err := ttpl("{{len .required}}", data); err != nil { + t.Errorf("len required: %s", err) + } else if o != "4" { + t.Errorf("Expected length of .required to be 4, got %s", o) } - var ok bool - var firstname *Schema - if firstname, ok = data.Properties["firstname"]; !ok { - t.Errorf("Expected property '.properties.firstname' is missing") - } - if firstname.Description != "First name" { - t.Errorf("Expected .properties.firstname.description to be 'First name', got '%s'", firstname.Description) - } - if firstname.Type != "string" { - t.Errorf("Expected .properties.firstname.type to be 'string', got '%s'", firstname.Type) - } + property := ".required" + expected := "[firstname lastname addresses employmentInfo]" + assertEqualProperty(t, property, expected, data) - var lastname *Schema - if lastname, ok = data.Properties["lastname"]; !ok { - t.Errorf("Expected property '.properties.lastname' is missing") - } - if lastname.Type != "string" { - t.Errorf("Expected .properties.lastname.type to be 'string', got '%s'", lastname.Type) - } + property = ".properties.firstname.description" + expected = "First name" + assertEqualProperty(t, property, expected, data) - var likesCoffee *Schema - if likesCoffee, ok = data.Properties["likes-coffee"]; !ok { - t.Errorf("Expected property '.properties.likes-coffee' is missing") - } - if likesCoffee.Type != "boolean" { - t.Errorf("Expected .properties.likes-coffee.type to be 'boolean', got '%s'", likesCoffee.Type) - } + property = ".properties.firstname.type" + expected = "string" + assertEqualProperty(t, property, expected, data) - var age *Schema - if age, ok = data.Properties["age"]; !ok { - t.Errorf("Expected property '.properties.age' is missing") - } - if age.Description != "Age" { - t.Errorf("Expected .properties.age.description to be 'Age', got '%s'", age.Description) - } - if age.Type != "integer" { - t.Errorf("Expected .properties.age.type to be 'string', got '%s'", age.Type) - } - if age.Minimum != 0 { - t.Errorf("Expected .properties.age.minimum to be 0, got %d", age.Minimum) - } + property = ".properties.lastname.type" + expected = "string" + assertEqualProperty(t, property, expected, data) - var employmentInfo *Schema - if employmentInfo, ok = data.Properties["employment-info"]; !ok { - t.Errorf("Expected property '.properties.employment-info' is missing") - } - if employmentInfo.Type != "object" { - t.Errorf("Expected .properties.employment-info.type to be 'object', got '%s'", employmentInfo.Type) - } - if len(employmentInfo.Required) != 1 { - t.Errorf("Expected length of .properties.employment-info.required to be 1, got %d", len(employmentInfo.Required)) - } - if !assertEqualSlices(employmentInfo.Required, []string{"salary"}) { - t.Errorf("Expected .properties.employment-info.required to be %v, got %v", []string{"salary"}, data.Required) - } + property = ".properties.likesCoffee.type" + expected = "boolean" + assertEqualProperty(t, property, expected, data) - var salary *Schema - if salary, ok = employmentInfo.Properties["salary"]; !ok { - t.Errorf("Expected property '.properties.employment-info.properties.salary' is missing") - } - if salary.Type != "float" { - t.Errorf("Expected .properties.employment-info.properties.salary.type to be 'float', got '%s'", salary.Type) - } - if salary.Minimum != 0 { - t.Errorf("Expected .properties.employment-info.properties.salary.minimum to be 0, got %d", salary.Minimum) - } + property = ".properties.age.description" + expected = "Age" + assertEqualProperty(t, property, expected, data) - var title *Schema - if title, ok = employmentInfo.Properties["title"]; !ok { - t.Errorf("Expected property '.properties.employment-info.properties.title' is missing") - } - if title.Type != "string" { - t.Errorf("Expected .properties.employment-info.properties.title.type to be 'string', got '%s'", title.Type) - } + property = ".properties.age.type" + expected = "integer" + assertEqualProperty(t, property, expected, data) - var addresses *Schema - if addresses, ok = data.Properties["addresses"]; !ok { - t.Errorf("Expected property '.properties.addresses' is missing") - } - if addresses.Type != "list[object]" { - t.Errorf("Expected .properties.addresses.type to be 'list[object]', got '%s'", addresses.Type) - } - if addresses.Description != "List of addresses" { - t.Errorf("Expected .properties.addresses.description to be 'List of addresses', got '%s'", addresses.Description) - } + property = ".properties.age.minimum" + expected = "0" + assertEqualProperty(t, property, expected, data) - var city *Schema - if city, ok = addresses.Properties["city"]; !ok { - t.Errorf("Expected property '.properties.addresses.properties.city' is missing") - } - if city.Type != "string" { - t.Errorf("Expected .properties.addresses.properties.city.type to be 'string', got '%s'", city.Type) - } + property = ".properties.employmentInfo.type" + expected = "object" + assertEqualProperty(t, property, expected, data) - var street *Schema - if street, ok = addresses.Properties["street"]; !ok { - t.Errorf("Expected property '.properties.addresses.properties.street' is missing") - } - if street.Type != "string" { - t.Errorf("Expected .properties.addresses.properties.street.type to be 'string', got '%s'", street.Type) - } + property = ".properties.employmentInfo.required" + expected = "[salary]" + assertEqualProperty(t, property, expected, data) - var number *Schema - if number, ok = addresses.Properties["number"]; !ok { - t.Errorf("Expected property '.properties.addresses.properties.number' is missing") - } - if number.Type != "number" { - t.Errorf("Expected .properties.addresses.properties.number.type to be 'number', got '%s'", number.Type) - } + property = ".properties.employmentInfo.properties.salary.type" + expected = "number" + assertEqualProperty(t, property, expected, data) - var phoneNumbers *Schema - if phoneNumbers, ok = data.Properties["phone-numbers"]; !ok { - t.Errorf("Expected property '.properties.phone-numbers' is missing") - } - if phoneNumbers.Type != "list[string]" { - t.Errorf("Expected .properties.phone-numbers.type to be 'list[object]', got '%s'", addresses.Type) - } + property = ".properties.employmentInfo.properties.salary.minimum" + expected = "0" + assertEqualProperty(t, property, expected, data) + + property = ".properties.employmentInfo.properties.title.type" + expected = "string" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.description" + expected = "List of addresses" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.type" + expected = "array" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.items.type" + expected = "object" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.items.properties.city.type" + expected = "string" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.items.properties.street.type" + expected = "string" + assertEqualProperty(t, property, expected, data) + + property = ".properties.addresses.items.properties.number.type" + expected = "number" + assertEqualProperty(t, property, expected, data) + + property = ".properties.phoneNumbers.type" + expected = "array" + assertEqualProperty(t, property, expected, data) + + property = ".properties.phoneNumbers.items.type" + expected = "string" + assertEqualProperty(t, property, expected, data) } -func assertEqualSlices(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i := 0; i < len(a); i++ { - if a[i] != b[i] { - return false - } +func assertEqualProperty(t *testing.T, property, expected string, data map[string]interface{}) { + if o, err := ttpl("{{"+property+"}}", data); err != nil { + t.Errorf("%s: %s", property, err) + } else if o != expected { + t.Errorf("Expected %s to be %s, got %s", property, expected, o) } - return true }