From b49db9e6e6fb1913a125b8cf9b31f81199c6753c Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Wed, 22 May 2019 19:38:11 +0200 Subject: [PATCH] ref(pkg/chartutil): break up chartutil into logical files Signed-off-by: Adam Reese --- cmd/helm/install.go | 19 +- cmd/helm/package.go | 3 +- cmd/helm/status_test.go | 1 - .../output/install-chart-bad-type.txt | 2 +- .../testdata/output/template-lib-chart.txt | 2 +- pkg/action/package.go | 5 - pkg/chart/errors.go | 23 ++ pkg/chart/loader/load_test.go | 5 +- pkg/chart/metadata.go | 21 +- pkg/chartutil/chartfile.go | 40 --- pkg/chartutil/coalesce.go | 195 +++++++++++++ pkg/chartutil/coalesce_test.go | 168 +++++++++++ pkg/chartutil/errors.go | 31 ++ pkg/chartutil/jsonschema.go | 87 ++++++ pkg/chartutil/jsonschema_test.go | 143 ++++++++++ pkg/chartutil/values.go | 242 ---------------- pkg/chartutil/values_test.go | 267 ------------------ pkg/engine/engine.go | 17 +- 18 files changed, 697 insertions(+), 574 deletions(-) create mode 100644 pkg/chart/errors.go create mode 100644 pkg/chartutil/coalesce.go create mode 100644 pkg/chartutil/coalesce_test.go create mode 100644 pkg/chartutil/errors.go create mode 100644 pkg/chartutil/jsonschema.go create mode 100644 pkg/chartutil/jsonschema_test.go diff --git a/cmd/helm/install.go b/cmd/helm/install.go index fefa18c05..40e810c4a 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -20,17 +20,17 @@ import ( "io" "time" - "helm.sh/helm/pkg/release" - + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chart/loader" - "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/getter" + "helm.sh/helm/pkg/release" ) const installDesc = ` @@ -180,7 +180,7 @@ func runInstall(args []string, client *action.Install, out io.Writer) (*release. return nil, err } - validInstallableChart, err := chartutil.IsChartInstallable(chartRequested) + validInstallableChart, err := isChartInstallable(chartRequested) if !validInstallableChart { return nil, err } @@ -211,3 +211,14 @@ func runInstall(args []string, client *action.Install, out io.Writer) (*release. client.Namespace = getNamespace() return client.Run(chartRequested) } + +// isChartInstallable validates if a chart can be installed +// +// Application chart type is only installable +func isChartInstallable(ch *chart.Chart) (bool, error) { + switch ch.Metadata.Type { + case "", "application": + return true, nil + } + return false, errors.Errorf("%s charts are not installable", ch.Metadata.Type) +} diff --git a/cmd/helm/package.go b/cmd/helm/package.go index 09241fea7..c33164a0a 100644 --- a/cmd/helm/package.go +++ b/cmd/helm/package.go @@ -22,11 +22,10 @@ import ( "io/ioutil" "path/filepath" - "helm.sh/helm/pkg/action" - "github.com/pkg/errors" "github.com/spf13/cobra" + "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/getter" ) diff --git a/cmd/helm/status_test.go b/cmd/helm/status_test.go index 169e6c621..54117bdc7 100644 --- a/cmd/helm/status_test.go +++ b/cmd/helm/status_test.go @@ -21,7 +21,6 @@ import ( "time" "helm.sh/helm/pkg/chart" - "helm.sh/helm/pkg/release" ) diff --git a/cmd/helm/testdata/output/install-chart-bad-type.txt b/cmd/helm/testdata/output/install-chart-bad-type.txt index 22308ac51..d8a3bf275 100644 --- a/cmd/helm/testdata/output/install-chart-bad-type.txt +++ b/cmd/helm/testdata/output/install-chart-bad-type.txt @@ -1 +1 @@ -Error: Invalid chart types are not installable +Error: validation: chart.metadata.type must be application or library diff --git a/cmd/helm/testdata/output/template-lib-chart.txt b/cmd/helm/testdata/output/template-lib-chart.txt index 5f119cb51..d8a3bf275 100644 --- a/cmd/helm/testdata/output/template-lib-chart.txt +++ b/cmd/helm/testdata/output/template-lib-chart.txt @@ -1 +1 @@ -Error: Library charts are not installable +Error: validation: chart.metadata.type must be application or library diff --git a/pkg/action/package.go b/pkg/action/package.go index af7edea44..7306959c9 100644 --- a/pkg/action/package.go +++ b/pkg/action/package.go @@ -59,11 +59,6 @@ func (p *Package) Run(path string) (string, error) { return "", err } - validChartType, err := chartutil.IsValidChartType(ch) - if !validChartType { - return "", err - } - combinedVals, err := chartutil.CoalesceValues(ch, p.ValueOptions.rawValues) if err != nil { return "", err diff --git a/pkg/chart/errors.go b/pkg/chart/errors.go new file mode 100644 index 000000000..4cb4189e6 --- /dev/null +++ b/pkg/chart/errors.go @@ -0,0 +1,23 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chart + +// ValidationError represents a data validation error. +type ValidationError string + +func (v ValidationError) Error() string { + return "validation: " + string(v) +} diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 45a3cff41..9e6697c40 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -127,11 +127,10 @@ icon: https://example.com/64x64.png t.Errorf("Expected number of templates == 2, got %d", len(c.Templates)) } - _, err = LoadFiles([]*BufferedFile{}) - if err == nil { + if _, err = LoadFiles([]*BufferedFile{}); err == nil { t.Fatal("Expected err to be non-nil") } - if err.Error() != "metadata is required" { + if err.Error() != "validation: chart.metadata is required" { t.Errorf("Expected chart metadata missing error, got '%s'", err.Error()) } } diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index 4a4139d1b..8c2c586ab 100644 --- a/pkg/chart/metadata.go +++ b/pkg/chart/metadata.go @@ -15,8 +15,6 @@ limitations under the License. package chart -import "errors" - // Maintainer describes a Chart maintainer. type Maintainer struct { // Name is a user name or organization name @@ -70,17 +68,28 @@ type Metadata struct { func (md *Metadata) Validate() error { if md == nil { - return errors.New("metadata is required") + return ValidationError("chart.metadata is required") } if md.APIVersion == "" { - return errors.New("metadata apiVersion is required") + return ValidationError("chart.metadata.apiVersion is required") } if md.Name == "" { - return errors.New("metadata name is required") + return ValidationError("chart.metadata.name is required") } if md.Version == "" { - return errors.New("metadata version is required") + return ValidationError("chart.metadata.version is required") + } + if !isValidChartType(md.Type) { + return ValidationError("chart.metadata.type must be application or library") } // TODO validate valid semver here? return nil } + +func isValidChartType(in string) bool { + switch in { + case "", "application", "library": + return true + } + return false +} diff --git a/pkg/chartutil/chartfile.go b/pkg/chartutil/chartfile.go index 9caa3f87e..7deebaa98 100644 --- a/pkg/chartutil/chartfile.go +++ b/pkg/chartutil/chartfile.go @@ -20,7 +20,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "github.com/ghodss/yaml" "github.com/pkg/errors" @@ -83,42 +82,3 @@ func IsChartDir(dirName string) (bool, error) { return true, nil } - -// IsChartInstallable validates if a chart can be installed -// -// Application chart type is only installable -func IsChartInstallable(chart *chart.Chart) (bool, error) { - if IsLibraryChart(chart) { - return false, errors.New("Library charts are not installable") - } - validChartType, _ := IsValidChartType(chart) - if !validChartType { - return false, errors.New("Invalid chart types are not installable") - } - return true, nil -} - -// IsValidChartType validates the chart type -// -// Valid types are: application or library -func IsValidChartType(chart *chart.Chart) (bool, error) { - chartType := chart.Metadata.Type - if chartType != "" && !strings.EqualFold(chartType, "library") && - !strings.EqualFold(chartType, "application") { - return false, errors.New("Invalid chart type. Valid types are: application or library") - } - return true, nil -} - -// IsLibraryChart returns true if the chart is a library chart -func IsLibraryChart(c *chart.Chart) bool { - return strings.EqualFold(c.Metadata.Type, "library") -} - -// IsTemplateValid returns true if the template is valid for the chart type -func IsTemplateValid(templateName string, isLibChart bool) bool { - if isLibChart { - return strings.HasPrefix(filepath.Base(templateName), "_") - } - return true -} diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go new file mode 100644 index 000000000..466e0d119 --- /dev/null +++ b/pkg/chartutil/coalesce.go @@ -0,0 +1,195 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "log" + + "github.com/pkg/errors" + + "helm.sh/helm/pkg/chart" +) + +// CoalesceValues coalesces all of the values in a chart (and its subcharts). +// +// Values are coalesced together using the following rules: +// +// - Values in a higher level chart always override values in a lower-level +// dependency chart +// - Scalar values and arrays are replaced, maps are merged +// - A chart has access to all of the variables for it, as well as all of +// the values destined for its dependencies. +func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { + if vals == nil { + vals = make(map[string]interface{}) + } + if _, err := coalesce(chrt, vals); err != nil { + return vals, err + } + return coalesceDeps(chrt, vals) +} + +// coalesce coalesces the dest values and the chart values, giving priority to the dest values. +// +// This is a helper function for CoalesceValues. +func coalesce(ch *chart.Chart, dest map[string]interface{}) (map[string]interface{}, error) { + coalesceValues(ch, dest) + return coalesceDeps(ch, dest) +} + +// coalesceDeps coalesces the dependencies of the given chart. +func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}) (map[string]interface{}, error) { + for _, subchart := range chrt.Dependencies() { + if c, ok := dest[subchart.Name()]; !ok { + // If dest doesn't already have the key, create it. + dest[subchart.Name()] = make(map[string]interface{}) + } else if !istable(c) { + return dest, errors.Errorf("type mismatch on %s: %t", subchart.Name(), c) + } + if dv, ok := dest[subchart.Name()]; ok { + dvmap := dv.(map[string]interface{}) + + // Get globals out of dest and merge them into dvmap. + coalesceGlobals(dvmap, dest) + + // Now coalesce the rest of the values. + var err error + dest[subchart.Name()], err = coalesce(subchart, dvmap) + if err != nil { + return dest, err + } + } + } + return dest, nil +} + +// coalesceGlobals copies the globals out of src and merges them into dest. +// +// For convenience, returns dest. +func coalesceGlobals(dest, src map[string]interface{}) { + var dg, sg map[string]interface{} + + if destglob, ok := dest[GlobalKey]; !ok { + dg = make(map[string]interface{}) + } else if dg, ok = destglob.(map[string]interface{}); !ok { + log.Printf("warning: skipping globals because destination %s is not a table.", GlobalKey) + return + } + + if srcglob, ok := src[GlobalKey]; !ok { + sg = make(map[string]interface{}) + } else if sg, ok = srcglob.(map[string]interface{}); !ok { + log.Printf("warning: skipping globals because source %s is not a table.", GlobalKey) + return + } + + // EXPERIMENTAL: In the past, we have disallowed globals to test tables. This + // reverses that decision. It may somehow be possible to introduce a loop + // here, but I haven't found a way. So for the time being, let's allow + // tables in globals. + for key, val := range sg { + if istable(val) { + vv := copyMap(val.(map[string]interface{})) + if destv, ok := dg[key]; !ok { + // Here there is no merge. We're just adding. + dg[key] = vv + } else { + if destvmap, ok := destv.(map[string]interface{}); !ok { + log.Printf("Conflict: cannot merge map onto non-map for %q. Skipping.", key) + } else { + // Basically, we reverse order of coalesce here to merge + // top-down. + CoalesceTables(vv, destvmap) + dg[key] = vv + continue + } + } + } else if dv, ok := dg[key]; ok && istable(dv) { + // It's not clear if this condition can actually ever trigger. + log.Printf("key %s is table. Skipping", key) + continue + } + // TODO: Do we need to do any additional checking on the value? + dg[key] = val + } + dest[GlobalKey] = dg +} + +func copyMap(src map[string]interface{}) map[string]interface{} { + m := make(map[string]interface{}, len(src)) + for k, v := range src { + m[k] = v + } + return m +} + +// coalesceValues builds up a values map for a particular chart. +// +// Values in v will override the values in the chart. +func coalesceValues(c *chart.Chart, v map[string]interface{}) { + for key, val := range c.Values { + if value, ok := v[key]; ok { + if value == nil { + // When the YAML value is null, we remove the value's key. + // This allows Helm's various sources of values (value files or --set) to + // remove incompatible keys from any previous chart, file, or set values. + delete(v, key) + } else if dest, ok := value.(map[string]interface{}); ok { + // if v[key] is a table, merge nv's val table into v[key]. + src, ok := val.(map[string]interface{}) + if !ok { + log.Printf("warning: skipped value for %s: Not a table.", key) + continue + } + // Because v has higher precedence than nv, dest values override src + // values. + CoalesceTables(dest, src) + } + } else { + // If the key is not in v, copy it from nv. + v[key] = val + } + } +} + +// CoalesceTables merges a source map into a destination map. +// +// dest is considered authoritative. +func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} { + if dst == nil || src == nil { + return src + } + // Because dest has higher precedence than src, dest values override src + // values. + for key, val := range src { + if istable(val) { + switch innerdst, ok := dst[key]; { + case !ok: + dst[key] = val + case istable(innerdst): + CoalesceTables(innerdst.(map[string]interface{}), val.(map[string]interface{})) + default: + log.Printf("warning: cannot overwrite table with non table for %s (%v)", key, val) + } + } else if dv, ok := dst[key]; ok && istable(dv) { + log.Printf("warning: destination for %s is a table. Ignoring non-table value %v", key, val) + } else if !ok { // <- ok is still in scope from preceding conditional. + dst[key] = val + } + } + return dst +} diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go new file mode 100644 index 000000000..62d9e4064 --- /dev/null +++ b/pkg/chartutil/coalesce_test.go @@ -0,0 +1,168 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "encoding/json" + "testing" +) + +// ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 +var testCoalesceValuesYaml = []byte(` +top: yup +bottom: null +right: Null +left: NULL +front: ~ +back: "" + +global: + name: Ishmael + subject: Queequeg + nested: + boat: true + +pequod: + global: + name: Stinky + harpooner: Tashtego + nested: + boat: false + sail: true + ahab: + scope: whale +`) + +func TestCoalesceValues(t *testing.T) { + c := loadChart(t, "testdata/moby") + + vals, err := ReadValues(testCoalesceValuesYaml) + if err != nil { + t.Fatal(err) + } + + v, err := CoalesceValues(c, vals) + if err != nil { + t.Fatal(err) + } + j, _ := json.MarshalIndent(v, "", " ") + t.Logf("Coalesced Values: %s", string(j)) + + tests := []struct { + tpl string + expect string + }{ + {"{{.top}}", "yup"}, + {"{{.back}}", ""}, + {"{{.name}}", "moby"}, + {"{{.global.name}}", "Ishmael"}, + {"{{.global.subject}}", "Queequeg"}, + {"{{.global.harpooner}}", ""}, + {"{{.pequod.name}}", "pequod"}, + {"{{.pequod.ahab.name}}", "ahab"}, + {"{{.pequod.ahab.scope}}", "whale"}, + {"{{.pequod.ahab.global.name}}", "Ishmael"}, + {"{{.pequod.ahab.global.subject}}", "Queequeg"}, + {"{{.pequod.ahab.global.harpooner}}", "Tashtego"}, + {"{{.pequod.global.name}}", "Ishmael"}, + {"{{.pequod.global.subject}}", "Queequeg"}, + {"{{.spouter.global.name}}", "Ishmael"}, + {"{{.spouter.global.harpooner}}", ""}, + + {"{{.global.nested.boat}}", "true"}, + {"{{.pequod.global.nested.boat}}", "true"}, + {"{{.spouter.global.nested.boat}}", "true"}, + {"{{.pequod.global.nested.sail}}", "true"}, + {"{{.spouter.global.nested.sail}}", ""}, + } + + for _, tt := range tests { + if o, err := ttpl(tt.tpl, v); err != nil || o != tt.expect { + t.Errorf("Expected %q to expand to %q, got %q", tt.tpl, tt.expect, o) + } + } + + nullKeys := []string{"bottom", "right", "left", "front"} + for _, nullKey := range nullKeys { + if _, ok := v[nullKey]; ok { + t.Errorf("Expected key %q to be removed, still present", nullKey) + } + } +} + +func TestCoalesceTables(t *testing.T) { + dst := map[string]interface{}{ + "name": "Ishmael", + "address": map[string]interface{}{ + "street": "123 Spouter Inn Ct.", + "city": "Nantucket", + }, + "details": map[string]interface{}{ + "friends": []string{"Tashtego"}, + }, + "boat": "pequod", + } + src := map[string]interface{}{ + "occupation": "whaler", + "address": map[string]interface{}{ + "state": "MA", + "street": "234 Spouter Inn Ct.", + }, + "details": "empty", + "boat": map[string]interface{}{ + "mast": true, + }, + } + + // What we expect is that anything in dst overrides anything in src, but that + // otherwise the values are coalesced. + CoalesceTables(dst, src) + + if dst["name"] != "Ishmael" { + t.Errorf("Unexpected name: %s", dst["name"]) + } + if dst["occupation"] != "whaler" { + t.Errorf("Unexpected occupation: %s", dst["occupation"]) + } + + addr, ok := dst["address"].(map[string]interface{}) + if !ok { + t.Fatal("Address went away.") + } + + if addr["street"].(string) != "123 Spouter Inn Ct." { + t.Errorf("Unexpected address: %v", addr["street"]) + } + + if addr["city"].(string) != "Nantucket" { + t.Errorf("Unexpected city: %v", addr["city"]) + } + + if addr["state"].(string) != "MA" { + t.Errorf("Unexpected state: %v", addr["state"]) + } + + if det, ok := dst["details"].(map[string]interface{}); !ok { + t.Fatalf("Details is the wrong type: %v", dst["details"]) + } else if _, ok := det["friends"]; !ok { + t.Error("Could not find your friends. Maybe you don't have any. :-(") + } + + if dst["boat"].(string) != "pequod" { + t.Errorf("Expected boat string, got %v", dst["boat"]) + } +} diff --git a/pkg/chartutil/errors.go b/pkg/chartutil/errors.go new file mode 100644 index 000000000..061610d41 --- /dev/null +++ b/pkg/chartutil/errors.go @@ -0,0 +1,31 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "fmt" +) + +// ErrNoTable indicates that a chart does not have a matching table. +type ErrNoTable string + +func (e ErrNoTable) Error() string { return fmt.Sprintf("%q is not a table", e) } + +// ErrNoValue indicates that Values does not contain a key with a value +type ErrNoValue string + +func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e) } diff --git a/pkg/chartutil/jsonschema.go b/pkg/chartutil/jsonschema.go new file mode 100644 index 000000000..b2d704422 --- /dev/null +++ b/pkg/chartutil/jsonschema.go @@ -0,0 +1,87 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "bytes" + "fmt" + "strings" + + "github.com/ghodss/yaml" + "github.com/pkg/errors" + "github.com/xeipuuv/gojsonschema" + + "helm.sh/helm/pkg/chart" +) + +// ValidateAgainstSchema checks that values does not violate the structure laid out in schema +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 { + sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name())) + sb.WriteString(err.Error()) + } + } + + // For each dependency, recurively call this function with the coalesced values + for _, subchrt := range chrt.Dependencies() { + subchrtValues := values[subchrt.Name()].(map[string]interface{}) + if err := ValidateAgainstSchema(subchrt, subchrtValues); err != nil { + sb.WriteString(err.Error()) + } + } + + if sb.Len() > 0 { + return errors.New(sb.String()) + } + + return nil +} + +// ValidateAgainstSingleSchema checks that values does not violate the structure laid out in this schema +func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) error { + valuesData, err := yaml.Marshal(values) + if err != nil { + return err + } + valuesJSON, err := yaml.YAMLToJSON(valuesData) + if err != nil { + return err + } + if bytes.Equal(valuesJSON, []byte("null")) { + valuesJSON = []byte("{}") + } + schemaLoader := gojsonschema.NewBytesLoader(schemaJSON) + valuesLoader := gojsonschema.NewBytesLoader(valuesJSON) + + result, err := gojsonschema.Validate(schemaLoader, valuesLoader) + 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()) + } + + return nil +} diff --git a/pkg/chartutil/jsonschema_test.go b/pkg/chartutil/jsonschema_test.go new file mode 100644 index 000000000..2a8f48102 --- /dev/null +++ b/pkg/chartutil/jsonschema_test.go @@ -0,0 +1,143 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package chartutil + +import ( + "io/ioutil" + "testing" + + "helm.sh/helm/pkg/chart" +) + +func TestValidateAgainstSingleSchema(t *testing.T) { + values, err := ReadValuesFile("./testdata/test-values.yaml") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + schema, err := ioutil.ReadFile("./testdata/test-values.schema.json") + if err != nil { + t.Fatalf("Error reading YAML file: %s", err) + } + + if err := ValidateAgainstSingleSchema(values, schema); err != nil { + t.Errorf("Error validating Values against Schema: %s", err) + } +} + +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 := ioutil.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() + } + + expectedErrString := `- (root): employmentInfo is required +- age: Must be greater than or equal to 0/1 +` + if errString != expectedErrString { + t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) + } +} + +const subchrtSchema = `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Values", + "type": "object", + "properties": { + "age": { + "description": "Age", + "minimum": 0, + "type": "integer" + } + }, + "required": [ + "age" + ] +} +` + +func TestValidateAgainstSchema(t *testing.T) { + subchrtJSON := []byte(subchrtSchema) + subchrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "subchrt", + }, + Schema: subchrtJSON, + } + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "chrt", + }, + } + chrt.AddDependency(subchrt) + + vals := map[string]interface{}{ + "name": "John", + "subchrt": map[string]interface{}{ + "age": 25, + }, + } + + if err := ValidateAgainstSchema(chrt, vals); err != nil { + t.Errorf("Error validating Values against Schema: %s", err) + } +} + +func TestValidateAgainstSchemaNegative(t *testing.T) { + subchrtJSON := []byte(subchrtSchema) + subchrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "subchrt", + }, + Schema: subchrtJSON, + } + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "chrt", + }, + } + chrt.AddDependency(subchrt) + + vals := map[string]interface{}{ + "name": "John", + "subchrt": 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 := `subchrt: +- (root): age is required +` + if errString != expectedErrString { + t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) + } +} diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index c8a02382e..bdf8a3ff4 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -17,30 +17,17 @@ limitations under the License. package chartutil import ( - "bytes" "fmt" "io" "io/ioutil" - "log" "strings" "github.com/ghodss/yaml" "github.com/pkg/errors" - "github.com/xeipuuv/gojsonschema" "helm.sh/helm/pkg/chart" ) -// ErrNoTable indicates that a chart does not have a matching table. -type ErrNoTable string - -func (e ErrNoTable) Error() string { return fmt.Sprintf("%q is not a table", e) } - -// ErrNoValue indicates that Values does not contain a key with a value -type ErrNoValue string - -func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e) } - // GlobalKey is the name of the Values key that is used for storing global vars. const GlobalKey = "global" @@ -89,7 +76,6 @@ func (v Values) AsMap() map[string]interface{} { // Encode writes serialized Values information to the given io.Writer. func (v Values) Encode(w io.Writer) error { - //return yaml.NewEncoder(w).Encode(v) out, err := yaml.Marshal(v) if err != nil { return err @@ -135,234 +121,6 @@ func ReadValuesFile(filename string) (Values, error) { return ReadValues(data) } -// ValidateAgainstSchema checks that values does not violate the structure laid out in schema -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 { - sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name())) - sb.WriteString(err.Error()) - } - } - - // For each dependency, recurively call this function with the coalesced values - for _, subchrt := range chrt.Dependencies() { - subchrtValues := values[subchrt.Name()].(map[string]interface{}) - if err := ValidateAgainstSchema(subchrt, subchrtValues); err != nil { - sb.WriteString(err.Error()) - } - } - - if sb.Len() > 0 { - return errors.New(sb.String()) - } - - return nil -} - -// ValidateAgainstSingleSchema checks that values does not violate the structure laid out in this schema -func ValidateAgainstSingleSchema(values Values, schemaJSON []byte) error { - valuesData, err := yaml.Marshal(values) - if err != nil { - return err - } - valuesJSON, err := yaml.YAMLToJSON(valuesData) - if err != nil { - return err - } - if bytes.Equal(valuesJSON, []byte("null")) { - valuesJSON = []byte("{}") - } - schemaLoader := gojsonschema.NewBytesLoader(schemaJSON) - valuesLoader := gojsonschema.NewBytesLoader(valuesJSON) - - result, err := gojsonschema.Validate(schemaLoader, valuesLoader) - 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()) - } - - return nil -} - -// CoalesceValues coalesces all of the values in a chart (and its subcharts). -// -// Values are coalesced together using the following rules: -// -// - Values in a higher level chart always override values in a lower-level -// dependency chart -// - Scalar values and arrays are replaced, maps are merged -// - A chart has access to all of the variables for it, as well as all of -// the values destined for its dependencies. -func CoalesceValues(chrt *chart.Chart, vals map[string]interface{}) (Values, error) { - if vals == nil { - vals = make(map[string]interface{}) - } - if _, err := coalesce(chrt, vals); err != nil { - return vals, err - } - return coalesceDeps(chrt, vals) -} - -// coalesce coalesces the dest values and the chart values, giving priority to the dest values. -// -// This is a helper function for CoalesceValues. -func coalesce(ch *chart.Chart, dest map[string]interface{}) (map[string]interface{}, error) { - coalesceValues(ch, dest) - return coalesceDeps(ch, dest) -} - -// coalesceDeps coalesces the dependencies of the given chart. -func coalesceDeps(chrt *chart.Chart, dest map[string]interface{}) (map[string]interface{}, error) { - for _, subchart := range chrt.Dependencies() { - if c, ok := dest[subchart.Name()]; !ok { - // If dest doesn't already have the key, create it. - dest[subchart.Name()] = make(map[string]interface{}) - } else if !istable(c) { - return dest, errors.Errorf("type mismatch on %s: %t", subchart.Name(), c) - } - if dv, ok := dest[subchart.Name()]; ok { - dvmap := dv.(map[string]interface{}) - - // Get globals out of dest and merge them into dvmap. - coalesceGlobals(dvmap, dest) - - // Now coalesce the rest of the values. - var err error - dest[subchart.Name()], err = coalesce(subchart, dvmap) - if err != nil { - return dest, err - } - } - } - return dest, nil -} - -// coalesceGlobals copies the globals out of src and merges them into dest. -// -// For convenience, returns dest. -func coalesceGlobals(dest, src map[string]interface{}) { - var dg, sg map[string]interface{} - - if destglob, ok := dest[GlobalKey]; !ok { - dg = make(map[string]interface{}) - } else if dg, ok = destglob.(map[string]interface{}); !ok { - log.Printf("warning: skipping globals because destination %s is not a table.", GlobalKey) - return - } - - if srcglob, ok := src[GlobalKey]; !ok { - sg = make(map[string]interface{}) - } else if sg, ok = srcglob.(map[string]interface{}); !ok { - log.Printf("warning: skipping globals because source %s is not a table.", GlobalKey) - return - } - - // EXPERIMENTAL: In the past, we have disallowed globals to test tables. This - // reverses that decision. It may somehow be possible to introduce a loop - // here, but I haven't found a way. So for the time being, let's allow - // tables in globals. - for key, val := range sg { - if istable(val) { - vv := copyMap(val.(map[string]interface{})) - if destv, ok := dg[key]; !ok { - // Here there is no merge. We're just adding. - dg[key] = vv - } else { - if destvmap, ok := destv.(map[string]interface{}); !ok { - log.Printf("Conflict: cannot merge map onto non-map for %q. Skipping.", key) - } else { - // Basically, we reverse order of coalesce here to merge - // top-down. - CoalesceTables(vv, destvmap) - dg[key] = vv - continue - } - } - } else if dv, ok := dg[key]; ok && istable(dv) { - // It's not clear if this condition can actually ever trigger. - log.Printf("key %s is table. Skipping", key) - continue - } - // TODO: Do we need to do any additional checking on the value? - dg[key] = val - } - dest[GlobalKey] = dg -} - -func copyMap(src map[string]interface{}) map[string]interface{} { - m := make(map[string]interface{}, len(src)) - for k, v := range src { - m[k] = v - } - return m -} - -// coalesceValues builds up a values map for a particular chart. -// -// Values in v will override the values in the chart. -func coalesceValues(c *chart.Chart, v map[string]interface{}) { - for key, val := range c.Values { - if value, ok := v[key]; ok { - if value == nil { - // When the YAML value is null, we remove the value's key. - // This allows Helm's various sources of values (value files or --set) to - // remove incompatible keys from any previous chart, file, or set values. - delete(v, key) - } else if dest, ok := value.(map[string]interface{}); ok { - // if v[key] is a table, merge nv's val table into v[key]. - src, ok := val.(map[string]interface{}) - if !ok { - log.Printf("warning: skipped value for %s: Not a table.", key) - continue - } - // Because v has higher precedence than nv, dest values override src - // values. - CoalesceTables(dest, src) - } - } else { - // If the key is not in v, copy it from nv. - v[key] = val - } - } -} - -// CoalesceTables merges a source map into a destination map. -// -// dest is considered authoritative. -func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} { - if dst == nil || src == nil { - return src - } - // Because dest has higher precedence than src, dest values override src - // values. - for key, val := range src { - if istable(val) { - switch innerdst, ok := dst[key]; { - case !ok: - dst[key] = val - case istable(innerdst): - CoalesceTables(innerdst.(map[string]interface{}), val.(map[string]interface{})) - default: - log.Printf("warning: cannot overwrite table with non table for %s (%v)", key, val) - } - } else if dv, ok := dst[key]; ok && istable(dv) { - log.Printf("warning: destination for %s is a table. Ignoring non-table value %v", key, val) - } else if !ok { // <- ok is still in scope from preceding conditional. - dst[key] = val - } - } - return dst -} - // ReleaseOptions represents the additional release options needed // for the composition of the final values struct type ReleaseOptions struct { diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index b5bb76801..0fadd897f 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -18,9 +18,7 @@ package chartutil import ( "bytes" - "encoding/json" "fmt" - "io/ioutil" "testing" "text/template" @@ -154,125 +152,6 @@ func TestReadValuesFile(t *testing.T) { matchValues(t, data) } -func TestValidateAgainstSingleSchema(t *testing.T) { - values, err := ReadValuesFile("./testdata/test-values.yaml") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } - schema, err := ioutil.ReadFile("./testdata/test-values.schema.json") - if err != nil { - t.Fatalf("Error reading YAML file: %s", err) - } - - if err := ValidateAgainstSingleSchema(values, schema); err != nil { - t.Errorf("Error validating Values against Schema: %s", err) - } -} - -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 := ioutil.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() - } - - expectedErrString := `- (root): employmentInfo is required -- age: Must be greater than or equal to 0/1 -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) - } -} - -const subchrtSchema = `{ - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "Values", - "type": "object", - "properties": { - "age": { - "description": "Age", - "minimum": 0, - "type": "integer" - } - }, - "required": [ - "age" - ] -} -` - -func TestValidateAgainstSchema(t *testing.T) { - subchrtJSON := []byte(subchrtSchema) - subchrt := &chart.Chart{ - Metadata: &chart.Metadata{ - Name: "subchrt", - }, - Schema: subchrtJSON, - } - chrt := &chart.Chart{ - Metadata: &chart.Metadata{ - Name: "chrt", - }, - } - chrt.AddDependency(subchrt) - - vals := map[string]interface{}{ - "name": "John", - "subchrt": map[string]interface{}{ - "age": 25, - }, - } - - if err := ValidateAgainstSchema(chrt, vals); err != nil { - t.Errorf("Error validating Values against Schema: %s", err) - } -} - -func TestValidateAgainstSchemaNegative(t *testing.T) { - subchrtJSON := []byte(subchrtSchema) - subchrt := &chart.Chart{ - Metadata: &chart.Metadata{ - Name: "subchrt", - }, - Schema: subchrtJSON, - } - chrt := &chart.Chart{ - Metadata: &chart.Metadata{ - Name: "chrt", - }, - } - chrt.AddDependency(subchrt) - - vals := map[string]interface{}{ - "name": "John", - "subchrt": 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 := `subchrt: -- (root): age is required -` - if errString != expectedErrString { - t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) - } -} - func ExampleValues() { doc := ` title: "Moby Dick" @@ -367,152 +246,6 @@ func ttpl(tpl string, v map[string]interface{}) (string, error) { return b.String(), err } -// ref: http://www.yaml.org/spec/1.2/spec.html#id2803362 -var testCoalesceValuesYaml = []byte(` -top: yup -bottom: null -right: Null -left: NULL -front: ~ -back: "" - -global: - name: Ishmael - subject: Queequeg - nested: - boat: true - -pequod: - global: - name: Stinky - harpooner: Tashtego - nested: - boat: false - sail: true - ahab: - scope: whale -`) - -func TestCoalesceValues(t *testing.T) { - c := loadChart(t, "testdata/moby") - - vals, err := ReadValues(testCoalesceValuesYaml) - if err != nil { - t.Fatal(err) - } - - v, err := CoalesceValues(c, vals) - if err != nil { - t.Fatal(err) - } - j, _ := json.MarshalIndent(v, "", " ") - t.Logf("Coalesced Values: %s", string(j)) - - tests := []struct { - tpl string - expect string - }{ - {"{{.top}}", "yup"}, - {"{{.back}}", ""}, - {"{{.name}}", "moby"}, - {"{{.global.name}}", "Ishmael"}, - {"{{.global.subject}}", "Queequeg"}, - {"{{.global.harpooner}}", ""}, - {"{{.pequod.name}}", "pequod"}, - {"{{.pequod.ahab.name}}", "ahab"}, - {"{{.pequod.ahab.scope}}", "whale"}, - {"{{.pequod.ahab.global.name}}", "Ishmael"}, - {"{{.pequod.ahab.global.subject}}", "Queequeg"}, - {"{{.pequod.ahab.global.harpooner}}", "Tashtego"}, - {"{{.pequod.global.name}}", "Ishmael"}, - {"{{.pequod.global.subject}}", "Queequeg"}, - {"{{.spouter.global.name}}", "Ishmael"}, - {"{{.spouter.global.harpooner}}", ""}, - - {"{{.global.nested.boat}}", "true"}, - {"{{.pequod.global.nested.boat}}", "true"}, - {"{{.spouter.global.nested.boat}}", "true"}, - {"{{.pequod.global.nested.sail}}", "true"}, - {"{{.spouter.global.nested.sail}}", ""}, - } - - for _, tt := range tests { - if o, err := ttpl(tt.tpl, v); err != nil || o != tt.expect { - t.Errorf("Expected %q to expand to %q, got %q", tt.tpl, tt.expect, o) - } - } - - nullKeys := []string{"bottom", "right", "left", "front"} - for _, nullKey := range nullKeys { - if _, ok := v[nullKey]; ok { - t.Errorf("Expected key %q to be removed, still present", nullKey) - } - } -} - -func TestCoalesceTables(t *testing.T) { - dst := map[string]interface{}{ - "name": "Ishmael", - "address": map[string]interface{}{ - "street": "123 Spouter Inn Ct.", - "city": "Nantucket", - }, - "details": map[string]interface{}{ - "friends": []string{"Tashtego"}, - }, - "boat": "pequod", - } - src := map[string]interface{}{ - "occupation": "whaler", - "address": map[string]interface{}{ - "state": "MA", - "street": "234 Spouter Inn Ct.", - }, - "details": "empty", - "boat": map[string]interface{}{ - "mast": true, - }, - } - - // What we expect is that anything in dst overrides anything in src, but that - // otherwise the values are coalesced. - CoalesceTables(dst, src) - - if dst["name"] != "Ishmael" { - t.Errorf("Unexpected name: %s", dst["name"]) - } - if dst["occupation"] != "whaler" { - t.Errorf("Unexpected occupation: %s", dst["occupation"]) - } - - addr, ok := dst["address"].(map[string]interface{}) - if !ok { - t.Fatal("Address went away.") - } - - if addr["street"].(string) != "123 Spouter Inn Ct." { - t.Errorf("Unexpected address: %v", addr["street"]) - } - - if addr["city"].(string) != "Nantucket" { - t.Errorf("Unexpected city: %v", addr["city"]) - } - - if addr["state"].(string) != "MA" { - t.Errorf("Unexpected state: %v", addr["state"]) - } - - if det, ok := dst["details"].(map[string]interface{}); !ok { - t.Fatalf("Details is the wrong type: %v", dst["details"]) - } else if _, ok := det["friends"]; !ok { - t.Error("Could not find your friends. Maybe you don't have any. :-(") - } - - if dst["boat"].(string) != "pequod" { - t.Errorf("Expected boat string, got %v", dst["boat"]) - } -} - func TestPathValue(t *testing.T) { doc := ` title: "Moby Dick" diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 4d3e418bc..4e8328266 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -19,6 +19,7 @@ package engine import ( "fmt" "path" + "path/filepath" "sort" "strings" "text/template" @@ -268,10 +269,9 @@ func recAllTpls(c *chart.Chart, templates map[string]renderable, vals chartutil. recAllTpls(child, templates, next) } - isLibChart := chartutil.IsLibraryChart(c) newParentID := c.ChartFullPath() for _, t := range c.Templates { - if !chartutil.IsTemplateValid(t.Name, isLibChart) { + if !isTemplateValid(c, t.Name) { continue } templates[path.Join(newParentID, t.Name)] = renderable{ @@ -281,3 +281,16 @@ func recAllTpls(c *chart.Chart, templates map[string]renderable, vals chartutil. } } } + +// isTemplateValid returns true if the template is valid for the chart type +func isTemplateValid(ch *chart.Chart, templateName string) bool { + if isLibraryChart(ch) { + return strings.HasPrefix(filepath.Base(templateName), "_") + } + return true +} + +// isLibraryChart returns true if the chart is a library chart +func isLibraryChart(c *chart.Chart) bool { + return strings.EqualFold(c.Metadata.Type, "library") +}