diff --git a/cmd/helm/create.go b/cmd/helm/create.go index 21a7e026c..294f153d7 100644 --- a/cmd/helm/create.go +++ b/cmd/helm/create.go @@ -41,8 +41,9 @@ something like this: ├── Chart.yaml # Information about your chart ├── values.yaml # The default values for your templates ├── charts/ # Charts that this chart depends on - └── templates/ # The template files - └── tests/ # The test files + ├── templates/ # The template files + │ └── tests/ # The test files + └── values/ # The values template files 'helm create' takes a path for an argument. If directories in the given path do not exist, Helm will attempt to create them as it goes. If the given diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt index e77aa387f..29a30fbc2 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt +++ b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt @@ -1,8 +1,7 @@ ==> Linting testdata/testcharts/chart-with-bad-subcharts [INFO] Chart.yaml: icon is recommended [WARNING] templates/: directory not found -[ERROR] : unable to load chart - error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required +[ERROR] : error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required ==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart [ERROR] Chart.yaml: name is required @@ -10,8 +9,7 @@ [ERROR] Chart.yaml: version is required [INFO] Chart.yaml: icon is recommended [WARNING] templates/: directory not found -[ERROR] : unable to load chart - validation: chart.metadata.name is required +[ERROR] : validation: chart.metadata.name is required ==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart [INFO] Chart.yaml: icon is recommended diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt index 265e555f7..89cbbda20 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt +++ b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts.txt @@ -1,7 +1,6 @@ ==> Linting testdata/testcharts/chart-with-bad-subcharts [INFO] Chart.yaml: icon is recommended [WARNING] templates/: directory not found -[ERROR] : unable to load chart - error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required +[ERROR] : error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required Error: 1 chart(s) linted, 1 chart(s) failed diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index bd75375a4..9f56e2db4 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -40,6 +40,8 @@ type Chart struct { Lock *Lock `json:"lock"` // Templates for this chart. Templates []*File `json:"templates"` + // Values templates for this chart. + ValuesTemplates []*File `json:"valuesTemplates"` // Values are default config for this chart. Values map[string]interface{} `json:"values"` // Schema is an optional JSON schema for imposing structure on Values diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go index dd4fd2dff..a9444cb0b 100644 --- a/pkg/chart/loader/load.go +++ b/pkg/chart/loader/load.go @@ -129,6 +129,8 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { case strings.HasPrefix(f.Name, "templates/"): c.Templates = append(c.Templates, &chart.File{Name: f.Name, Data: f.Data}) + case strings.HasPrefix(f.Name, "values/"): + c.ValuesTemplates = append(c.ValuesTemplates, &chart.File{Name: f.Name, Data: f.Data}) case strings.HasPrefix(f.Name, "charts/"): if filepath.Ext(f.Name) == ".prov" { c.Files = append(c.Files, &chart.File{Name: f.Name, Data: f.Data}) diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 16a94d4eb..15e94d96e 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -245,6 +245,10 @@ icon: https://example.com/64x64.png Name: "templates/service.yaml", Data: []byte("some service"), }, + { + Name: "values/test.yaml", + Data: []byte("some other values"), + }, } c, err := LoadFiles(goodFiles) @@ -260,7 +264,7 @@ icon: https://example.com/64x64.png t.Error("Expected chart values to be populated with default values") } - if len(c.Raw) != 5 { + if len(c.Raw) != 6 { t.Errorf("Expected %d files, got %d", 5, len(c.Raw)) } @@ -272,6 +276,10 @@ icon: https://example.com/64x64.png t.Errorf("Expected number of templates == 2, got %d", len(c.Templates)) } + if len(c.ValuesTemplates) != 1 { + t.Errorf("Expected number of values templates == 1, got %d", len(c.ValuesTemplates)) + } + if _, err = LoadFiles([]*BufferedFile{}); err == nil { t.Fatal("Expected err to be non-nil") } @@ -405,6 +413,9 @@ func verifyChart(t *testing.T, c *chart.Chart) { if len(c.Templates) != 1 { t.Errorf("Expected 1 template, got %d", len(c.Templates)) } + if len(c.ValuesTemplates) != 1 { + t.Errorf("Expected 1 values template, got %d", len(c.ValuesTemplates)) + } numfiles := 6 if len(c.Files) != numfiles { @@ -509,6 +520,15 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) { if len(c.Templates[0].Data) == 0 { t.Error("No template data.") } + if len(c.ValuesTemplates) != 1 { + t.Fatalf("Expected 1 values template, got %d", len(c.ValuesTemplates)) + } + if c.ValuesTemplates[0].Name != "values/value.yaml" { + t.Errorf("Unexpected values template: %s", c.ValuesTemplates[0].Name) + } + if len(c.ValuesTemplates[0].Data) == 0 { + t.Error("No values template data.") + } if len(c.Files) != 6 { t.Fatalf("Expected 6 Files, got %d", len(c.Files)) } diff --git a/pkg/chart/loader/testdata/frobnitz-1.2.3.tgz b/pkg/chart/loader/testdata/frobnitz-1.2.3.tgz index b2b76a83c..1830f2d95 100644 Binary files a/pkg/chart/loader/testdata/frobnitz-1.2.3.tgz and b/pkg/chart/loader/testdata/frobnitz-1.2.3.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz.v1.tgz b/pkg/chart/loader/testdata/frobnitz.v1.tgz index 6282f9b73..dde875395 100644 Binary files a/pkg/chart/loader/testdata/frobnitz.v1.tgz and b/pkg/chart/loader/testdata/frobnitz.v1.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz.v1/values/value.yaml b/pkg/chart/loader/testdata/frobnitz.v1/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz.v1/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz.v2.reqs/values/value.yaml b/pkg/chart/loader/testdata/frobnitz.v2.reqs/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz.v2.reqs/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz/values/value.yaml b/pkg/chart/loader/testdata/frobnitz/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz_backslash-1.2.3.tgz b/pkg/chart/loader/testdata/frobnitz_backslash-1.2.3.tgz index a9d4c11d8..f14eb8c0c 100644 Binary files a/pkg/chart/loader/testdata/frobnitz_backslash-1.2.3.tgz and b/pkg/chart/loader/testdata/frobnitz_backslash-1.2.3.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz_backslash/values/value.yaml b/pkg/chart/loader/testdata/frobnitz_backslash/values/value.yaml new file mode 100755 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_backslash/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom.tgz index be0cd027d..d24775b1b 100644 Binary files a/pkg/chart/loader/testdata/frobnitz_with_bom.tgz and b/pkg/chart/loader/testdata/frobnitz_with_bom.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/values/value.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz_with_dev_null/values/value.yaml b/pkg/chart/loader/testdata/frobnitz_with_dev_null/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_dev_null/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chart/loader/testdata/frobnitz_with_symlink/values/value.yaml b/pkg/chart/loader/testdata/frobnitz_with_symlink/values/value.yaml new file mode 100644 index 000000000..6c32007ff --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_symlink/values/value.yaml @@ -0,0 +1 @@ +name: "{{ .Values.name }} Too" diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go index 8d8f48176..3edd05152 100644 --- a/pkg/chartutil/create.go +++ b/pkg/chartutil/create.go @@ -39,6 +39,8 @@ const ( SchemafileName = "values.schema.json" // TemplatesDir is the relative directory name for templates. TemplatesDir = "templates" + // ValuesTemplatesDir is the relative directory name for values templates. + ValuesTemplatesDir = "values" // ChartsDir is the relative directory name for charts dependencies. ChartsDir = "charts" // TemplatesTestsDir is the relative directory name for tests. @@ -482,7 +484,15 @@ func CreateFrom(chartfile *chart.Metadata, dest, src string) error { updatedTemplates = append(updatedTemplates, &chart.File{Name: template.Name, Data: newData}) } + var updatedValuesTemplates []*chart.File + + for _, template := range schart.ValuesTemplates { + newData := transform(string(template.Data), schart.Name()) + updatedValuesTemplates = append(updatedValuesTemplates, &chart.File{Name: template.Name, Data: newData}) + } + schart.Templates = updatedTemplates + schart.ValuesTemplates = updatedValuesTemplates b, err := yaml.Marshal(schart.Values) if err != nil { return errors.Wrap(err, "reading values file") @@ -606,10 +616,21 @@ func Create(name, dir string) (string, error) { return cdir, err } } - // Need to add the ChartsDir explicitly as it does not contain any file OOTB - if err := os.MkdirAll(filepath.Join(cdir, ChartsDir), 0755); err != nil { - return cdir, err + + // Need to add empty directories explicitly + edirs := []string{ + // values/ + filepath.Join(cdir, ValuesTemplatesDir), + // charts/ + filepath.Join(cdir, ChartsDir), + } + + for _, edir := range edirs { + if err := os.MkdirAll(edir, 0755); err != nil { + return cdir, err + } } + return cdir, nil } diff --git a/pkg/chartutil/create_test.go b/pkg/chartutil/create_test.go index a11c45140..83f8a0aa2 100644 --- a/pkg/chartutil/create_test.go +++ b/pkg/chartutil/create_test.go @@ -62,6 +62,7 @@ func TestCreate(t *testing.T) { TemplatesTestsDir, TestConnectionName, ValuesfileName, + ValuesTemplatesDir, } { if _, err := os.Stat(filepath.Join(dir, f)); err != nil { t.Errorf("Expected %s file: %s", f, err) @@ -102,6 +103,7 @@ func TestCreateFrom(t *testing.T) { ChartfileName, ValuesfileName, filepath.Join(TemplatesDir, "placeholder.tpl"), + filepath.Join(ValuesTemplatesDir, "placeholder.yaml"), } { if _, err := os.Stat(filepath.Join(dir, f)); err != nil { t.Errorf("Expected %s file: %s", f, err) diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go index 2ce4eddaf..0db42fc73 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -70,8 +70,8 @@ func SaveDir(c *chart.Chart, dest string) error { } } - // Save templates and files - for _, o := range [][]*chart.File{c.Templates, c.Files} { + // Save values templates, templates and files + for _, o := range [][]*chart.File{c.ValuesTemplates, c.Templates, c.Files} { for _, f := range o { n := filepath.Join(outdir, f.Name) if err := writeFile(n, f.Data); err != nil { @@ -202,6 +202,14 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error { } } + // Save values templates + for _, f := range c.ValuesTemplates { + n := filepath.Join(base, f.Name) + if err := writeToTar(out, n, f.Data); err != nil { + return err + } + } + // Save templates for _, f := range c.Templates { n := filepath.Join(base, f.Name) diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go index 3a45b2992..c569a9417 100644 --- a/pkg/chartutil/save_test.go +++ b/pkg/chartutil/save_test.go @@ -221,6 +221,9 @@ func TestSaveDir(t *testing.T) { Templates: []*chart.File{ {Name: filepath.Join(TemplatesDir, "nested", "dir", "thing.yaml"), Data: []byte("abc: {{ .Values.abc }}")}, }, + ValuesTemplates: []*chart.File{ + {Name: filepath.Join(ValuesTemplatesDir, "other", "nested", "stuff.yaml"), Data: []byte("def: {{ .Values.def }}")}, + }, } if err := SaveDir(c, tmp); err != nil { @@ -240,6 +243,10 @@ func TestSaveDir(t *testing.T) { t.Fatal("Templates data did not match") } + if len(c2.ValuesTemplates) != 1 || c2.ValuesTemplates[0].Name != filepath.Join(ValuesTemplatesDir, "other", "nested", "stuff.yaml") { + t.Fatal("ValuesTemplates data did not match") + } + if len(c2.Files) != 1 || c2.Files[0].Name != "scheherazade/shahryar.txt" { t.Fatal("Files data did not match") } diff --git a/pkg/chartutil/testdata/frobnitz/charts/mariner/values/placeholder.yaml b/pkg/chartutil/testdata/frobnitz/charts/mariner/values/placeholder.yaml new file mode 100644 index 000000000..29c11843a --- /dev/null +++ b/pkg/chartutil/testdata/frobnitz/charts/mariner/values/placeholder.yaml @@ -0,0 +1 @@ +# This is a placeholder. diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index cacd4b136..a3181ef2d 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -28,6 +28,7 @@ import ( "github.com/pkg/errors" "k8s.io/client-go/rest" + "sigs.k8s.io/yaml" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" @@ -64,7 +65,7 @@ type Engine struct { // section contains a value named "bar", that value will be passed on to the // bar chart during render time. func (e Engine) Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, error) { - // update values and dependencies + // parse values templates and update values and dependencies if err := e.updateRenderValues(chrt, values); err != nil { return nil, err } @@ -302,10 +303,10 @@ func cleanupExecError(filename string, err error) error { return err } -// updateRenderValues update render values with chart values. +// updateRenderValues update render values with chart values and values templates. func (e Engine) updateRenderValues(c *chart.Chart, vals chartutil.Values) error { var sb strings.Builder - // update values and dependencies + // parse values templates and update values and dependencies if err := e.recUpdateRenderValues(c, vals, nil, &sb); err != nil { return err } @@ -344,7 +345,7 @@ func (e Engine) recUpdateRenderValues(c *chart.Chart, vals chartutil.Values, tag return err } next["Values"] = nvals - // Get validations errors of chart values + // Get validations errors of chart values, before applying values template if c.Schema != nil { err = chartutil.ValidateAgainstSingleSchema(nvals, c.Schema) if err != nil { @@ -352,6 +353,34 @@ func (e Engine) recUpdateRenderValues(c *chart.Chart, vals chartutil.Values, tag sb.WriteString(err.Error()) } } + // Get all values templates of the chart + templates := map[string]renderable{} + newParentID := c.ChartFullPath() + for _, t := range c.ValuesTemplates { + if !isTemplateValid(c, t.Name) { + continue + } + templates[path.Join(newParentID, t.Name)] = renderable{ + tpl: string(t.Data), + vals: next, + basePath: path.Join(newParentID, "values"), + } + } + // Render all values templates + rendered, err := e.render(templates) + if err != nil { + return err + } + // Parse and apply all values templates + if len(rendered) > 0 { + for _, filename := range sortValuesTemplates(rendered) { + src := map[string]interface{}{} + if err := yaml.Unmarshal([]byte(rendered[filename]), &src); err != nil { + return errors.Wrap(err, fmt.Sprintf("cannot load %s", filename)) + } + chartutil.CoalesceTablesUpdate(nvals, src) + } + } // Get tags of the root if c.IsRoot() { tags = chartutil.GetTags(nvals) @@ -361,7 +390,7 @@ func (e Engine) recUpdateRenderValues(c *chart.Chart, vals chartutil.Values, tag if err != nil { return err } - // Recursive upudate on enabled dependencies + // Recursive update on enabled dependencies for _, child := range c.Dependencies() { err = e.recUpdateRenderValues(child, next, tags, sb) if err != nil { @@ -371,6 +400,18 @@ func (e Engine) recUpdateRenderValues(c *chart.Chart, vals chartutil.Values, tag return nil } +// sortValuesTemplates sorts the rendered yaml values files from lowest to highest priority +func sortValuesTemplates(tpls map[string]string) []string { + keys := make(sort.StringSlice, len(tpls)) + i := 0 + for key := range tpls { + keys[i] = key + i++ + } + sort.Sort(keys) + return keys +} + func sortTemplates(tpls map[string]renderable) []string { keys := make([]string, len(tpls)) i := 0 diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 745cf256f..361d23018 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -742,7 +742,197 @@ func TestRenderRecursionLimit(t *testing.T) { if got := out["overlook/templates/quote"]; got != expect { t.Errorf("Expected %q, got %q (%v)", expect, got, out) } +} + +func TestUpdateRenderValues_values_templates(t *testing.T) { + values := map[string]interface{}{} + rv := map[string]interface{}{ + "Release": map[string]interface{}{ + "Name": "Test Name", + }, + "Values": values, + } + c := loadChart(t, "testdata/values_templates") + if err := new(Engine).updateRenderValues(c, rv); err != nil { + t.Fatal(err) + } + if v, ok := values["releaseName"]; !ok { + t.Errorf("field 'releaseName' missing") + } else if vs, ok := v.(string); !ok || vs != "Test Name" { + t.Errorf("wrong value on field 'releaseName': %v", v) + } + // Check root remplacements + if v, ok := values["replaced"]; !ok { + t.Errorf("field 'replaced' missing") + } else if vs := v.(string); vs != "values/replaced2.yaml" { + t.Errorf("wrong priority on field 'replaced', value from %s", vs) + } + if v, ok := values["currentReplaced1"]; !ok { + t.Errorf("field 'currentReplaced1' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong evaluation order on field 'currentReplaced1', value from %s", vs) + } + if v, ok := values["currentReplaced2"]; !ok { + t.Errorf("field 'currentReplaced2' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong evaluation order on field 'currentReplaced2', value from %s", vs) + } + // check root coalesce + if vm, ok := values["coalesce"]; !ok { + t.Errorf("field 'coalesce' missing") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["old"]; !ok { + t.Errorf("field 'coalesce.old' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong priority on field 'coalesce.old', value from %s", vs) + } + if v, ok := m["common"]; !ok { + t.Errorf("field 'coalesce.common' missing") + } else if vs := v.(string); vs != "values/coalesce.yaml" { + t.Errorf("wrong priority on field 'coalesce.common', value from %s", vs) + } + if v, ok := m["new"]; !ok { + t.Errorf("field 'coalesce.new' missing") + } else if vs := v.(string); vs != "values/coalesce.yaml" { + t.Errorf("wrong priority on field 'coalesce.new', value from %s", vs) + } + } + // check root global + if vm, ok := values["global"]; !ok { + t.Errorf("field 'global' missing") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["parentValues"]; !ok || !v.(bool) { + t.Errorf("field 'global.parentValues' missing") + } + if v, ok := m["parentTemplate"]; !ok || !v.(bool) { + t.Errorf("field 'global.parentTemplate' missing") + } + if _, ok := m["subValues"]; ok { + t.Errorf("field 'global.subValues' unexpected") + } + if _, ok := m["subTeamplate"]; ok { + t.Errorf("field 'global.subTeamplate' unexpected") + } + } + // check subchart + if vm, ok := values["subchart"]; !ok { + t.Errorf("field 'subchart' missing") + } else { + // check subchart evaluated + m := vm.(map[string]interface{}) + if v, ok := m["evaluated"]; !ok || !v.(bool) { + t.Errorf("chart 'subchart' not evaluated") + } + // check subchart replaced + if v, ok := m["replaced1"]; !ok { + t.Errorf("field 'subchart.replaced1' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong priority on field 'subchart.replaced1', value from %s", vs) + } + if v, ok := m["replaced2"]; !ok { + t.Errorf("field 'subchart.replaced2' missing") + } else if vs := v.(string); vs != "subchart/values/replaced.yaml" { + t.Errorf("wrong priority on field 'subchart.replaced2', value from %s", vs) + } + if v, ok := m["replaced3"]; !ok { + t.Errorf("field 'subchart.replaced3' missing") + } else if vs := v.(string); vs != "values/sub_replaced.yaml" { + t.Errorf("wrong priority on field 'subchart.replaced3', value from %s", vs) + } + if v, ok := m["replaced4"]; !ok { + t.Errorf("field 'subchart.replaced4' missing") + } else if vs := v.(string); vs != "subchart/values/replaced.yaml" { + t.Errorf("wrong priority on field 'subchart.replaced4', value from %s", vs) + } + if v, ok := m["currentReplaced2"]; !ok { + t.Errorf("field 'subchart.currentReplaced2' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong evaluation order on field 'subchart.currentReplaced2', value from %s", vs) + } + // check subchart coalesce + if vm, ok := m["coalesce"]; !ok { + t.Errorf("field 'subchart.coalesce' missing") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["value1"]; !ok { + t.Errorf("field 'subchart.coalesce.value1' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value1', value from %s", vs) + } + if v, ok := m["value2"]; !ok { + t.Errorf("field 'subchart.coalesce.value2' missing") + } else if vs := v.(string); vs != "values.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value2', value from %s", vs) + } + if v, ok := m["value3"]; !ok { + t.Errorf("field 'subchart.coalesce.value3' missing") + } else if vs := v.(string); vs != "subchart/values/coalesce.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value3', value from %s", vs) + } + if v, ok := m["value4"]; !ok { + t.Errorf("field 'subchart.coalesce.value4' missing") + } else if vs := v.(string); vs != "subchart/values.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value4', value from %s", vs) + } + if v, ok := m["value5"]; !ok { + t.Errorf("field 'subchart.coalesce.value5' missing") + } else if vs := v.(string); vs != "subchart/values/coalesce.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value5', value from %s", vs) + } + if v, ok := m["value6"]; !ok { + t.Errorf("field 'subchart.coalesce.value6' missing") + } else if vs := v.(string); vs != "subchart/values/coalesce.yaml" { + t.Errorf("wrong priority on field 'subchart.coalesce.value6', value from %s", vs) + } + } + // check subchart global + if vm, ok := m["global"]; !ok { + t.Errorf("field 'subchart.global' missing") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["parentValues"]; !ok || !v.(bool) { + t.Errorf("field 'subchart.global.parentValues' missing") + } + if v, ok := m["parentTemplate"]; !ok || !v.(bool) { + t.Errorf("field 'subchart.global.parentTemplate' missing") + } + if v, ok := m["subTeamplate"]; !ok || !v.(bool) { + t.Errorf("field 'subchart.global.subTeamplate' missing") + } + if v, ok := m["parentValues"]; !ok || !v.(bool) { + t.Errorf("field 'subchart.global.parentValues' missing") + } + } + // check subchart globalEvaluated + if vm, ok := m["globalEvaluated"]; !ok { + t.Errorf("field 'subchart.globalEvaluated' missing") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["parentValues"]; !ok { + t.Errorf("field 'subchart.globalEvaluated.parentValues' missing") + } else if vb, ok := v.(bool); !ok || !vb { + t.Errorf("field 'subchart.globalEvaluated.parentValues' has wrong value: %v", vb) + } + if v, ok := m["parentTemplate"]; !ok { + t.Errorf("field 'subchart.globalEvaluated.parentTemplate' missing") + } else if vb, ok := v.(bool); !ok || !vb { + t.Errorf("field 'subchart.globalEvaluated.parentTemplate' has wrong value: %v", vb) + } + if v, ok := m["subValues"]; !ok { + t.Errorf("field 'subchart.globalEvaluated.subValues' missing") + } else if vb, ok := v.(bool); !ok || !vb { + t.Errorf("field 'subchart.globalEvaluated.subValues' has wrong value: %v", vb) + } + if v, ok := m["subTeamplate"]; !ok { + t.Errorf("field 'subchart.globalEvaluated.subTeamplate' missing") + } else if v != nil { + t.Errorf("field 'subchart.globalEvaluated.subTeamplate' has wrong value: %v", v) + } + } + } } func TestUpdateRenderValues_dependencies(t *testing.T) { @@ -827,6 +1017,14 @@ func TestUpdateRenderValues_dependencies(t *testing.T) { t.Errorf("value 'importValues.imported' not imported") } } + if vm, ok := values["importTemplate"]; !ok { + t.Errorf("value 'importTemplate' not imported") + } else { + m := vm.(map[string]interface{}) + if v, ok := m["imported"]; !ok || !v.(bool) { + t.Errorf("value 'importTemplate.imported' not imported") + } + } if vm, ok := values["subImport"]; !ok { t.Errorf("value 'subImport' not imported") } else { @@ -836,6 +1034,16 @@ func TestUpdateRenderValues_dependencies(t *testing.T) { } else if vs, ok := v.(string); !ok || vs != "values.yaml" { t.Errorf("wrong 'subImport.old' imported: %v", v) } + if v, ok := m["common"]; !ok { + t.Errorf("value 'subImport.common' not imported") + } else if vs, ok := v.(string); !ok || vs != "values/import.yaml" { + t.Errorf("wrong 'subImport.common' imported: %v", v) + } + if v, ok := m["new"]; !ok { + t.Errorf("value 'subImport.new' not imported") + } else if vs, ok := v.(string); !ok || vs != "values/import.yaml" { + t.Errorf("wrong 'subImport.old' imported: %v", v) + } } names := extractChartNames(c) diff --git a/pkg/engine/testdata/dependencies/charts/import_values/values/import.yaml b/pkg/engine/testdata/dependencies/charts/import_values/values/import.yaml new file mode 100644 index 000000000..a79dddd45 --- /dev/null +++ b/pkg/engine/testdata/dependencies/charts/import_values/values/import.yaml @@ -0,0 +1,5 @@ +import: + common: "values/import.yaml" + new: "values/import.yaml" +importTemplate: + imported: true diff --git a/pkg/engine/testdata/dependencies/values.yaml b/pkg/engine/testdata/dependencies/values.yaml index 9148fc233..ae2ed6ccb 100644 --- a/pkg/engine/testdata/dependencies/values.yaml +++ b/pkg/engine/testdata/dependencies/values.yaml @@ -1,7 +1,7 @@ condition: - "true": true - "false": false - "null": null + "true": false + "false": true + "null": false tags: - true_tag: true - false_tag: false + true_tag: false + false_tag: true \ No newline at end of file diff --git a/pkg/engine/testdata/dependencies/values/condition.yaml b/pkg/engine/testdata/dependencies/values/condition.yaml new file mode 100644 index 000000000..cdf5d0186 --- /dev/null +++ b/pkg/engine/testdata/dependencies/values/condition.yaml @@ -0,0 +1,4 @@ +condition: + "true": true + "false": false + "null": null diff --git a/pkg/engine/testdata/dependencies/values/tags.yaml b/pkg/engine/testdata/dependencies/values/tags.yaml new file mode 100644 index 000000000..3e06d0d66 --- /dev/null +++ b/pkg/engine/testdata/dependencies/values/tags.yaml @@ -0,0 +1,3 @@ +tags: + true_tag: true + false_tag: false diff --git a/pkg/engine/testdata/values_templates/Chart.yaml b/pkg/engine/testdata/values_templates/Chart.yaml new file mode 100644 index 000000000..ef6bac8b8 --- /dev/null +++ b/pkg/engine/testdata/values_templates/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +description: A Helm chart for Kubernetes +name: parentchart +version: 0.1.0 diff --git a/pkg/engine/testdata/values_templates/charts/subchart/Chart.yaml b/pkg/engine/testdata/values_templates/charts/subchart/Chart.yaml new file mode 100644 index 000000000..e8831783e --- /dev/null +++ b/pkg/engine/testdata/values_templates/charts/subchart/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v2 +description: A Helm chart for Kubernetes +name: subchart +version: 0.1.0 +dependencies: diff --git a/pkg/engine/testdata/values_templates/charts/subchart/values.yaml b/pkg/engine/testdata/values_templates/charts/subchart/values.yaml new file mode 100644 index 000000000..08ad6a7ca --- /dev/null +++ b/pkg/engine/testdata/values_templates/charts/subchart/values.yaml @@ -0,0 +1,13 @@ +# should coalesce with parent global +global: + subValues: true +# should be evaluated +evaluated: true +# original value should be kept +replaced1: "subchart/values.yaml" +replaced3: "subchart/values.yaml" +# should cloalesce with parent values.yaml +coalesce: + value2: "subchart/values.yaml" + value4: "subchart/values.yaml" + value5: "subchart/values.yaml" diff --git a/pkg/engine/testdata/values_templates/charts/subchart/values/coalesce.yaml b/pkg/engine/testdata/values_templates/charts/subchart/values/coalesce.yaml new file mode 100644 index 000000000..e0ed61adf --- /dev/null +++ b/pkg/engine/testdata/values_templates/charts/subchart/values/coalesce.yaml @@ -0,0 +1,5 @@ +# should cloalesce with both values.yaml +coalesce: + value3: "subchart/values/coalesce.yaml" + value5: "subchart/values/coalesce.yaml" + value6: "subchart/values/coalesce.yaml" diff --git a/pkg/engine/testdata/values_templates/charts/subchart/values/global.yaml b/pkg/engine/testdata/values_templates/charts/subchart/values/global.yaml new file mode 100644 index 000000000..c328ad356 --- /dev/null +++ b/pkg/engine/testdata/values_templates/charts/subchart/values/global.yaml @@ -0,0 +1,9 @@ +# should coalesce with parent global +global: + subTeamplate: true +# should capture everything except subTeamplate +globalEvaluated: + parentValues: {{ .Values.global.parentValues }} + parentTemplate: {{ .Values.global.parentTemplate }} + subValues: {{ .Values.global.subValues }} + subTeamplate: {{ .Values.global.subTeamplate }} diff --git a/pkg/engine/testdata/values_templates/charts/subchart/values/replaced.yaml b/pkg/engine/testdata/values_templates/charts/subchart/values/replaced.yaml new file mode 100644 index 000000000..e3df8c1d1 --- /dev/null +++ b/pkg/engine/testdata/values_templates/charts/subchart/values/replaced.yaml @@ -0,0 +1,4 @@ +# replacement should work +replaced2: "subchart/values/replaced.yaml" +replaced4: "subchart/values/replaced.yaml" +currentReplaced2: "{{ .Values.replaced2 }}" diff --git a/pkg/engine/testdata/values_templates/values.yaml b/pkg/engine/testdata/values_templates/values.yaml new file mode 100644 index 000000000..8714ced77 --- /dev/null +++ b/pkg/engine/testdata/values_templates/values.yaml @@ -0,0 +1,20 @@ +# should appear in subcharts too +global: + parentValues: true +# should be replaced by values templates values/replaced[12].yaml +replaced: "values.yaml" +# should coalesce with values/coalesce.yaml +coalesce: + old: "values.yaml" + common: "values.yaml" + +subchart: + # should replaced by subchart/values/replaced.yaml + replaced1: "values.yaml" + replaced2: "values.yaml" + # should coalesce with values/sub_coalesce.yaml, subchart/values.yaml + # and subchart/values/coalesce.yaml + coalesce: + value1: "values.yaml" + value2: "values.yaml" + value3: "values.yaml" diff --git a/pkg/engine/testdata/values_templates/values/coalesce.yaml b/pkg/engine/testdata/values_templates/values/coalesce.yaml new file mode 100644 index 000000000..9a1673df9 --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/coalesce.yaml @@ -0,0 +1,4 @@ +# should cloalesce with values.yaml +coalesce: + common: "values/coalesce.yaml" + new: "values/coalesce.yaml" diff --git a/pkg/engine/testdata/values_templates/values/global.yaml b/pkg/engine/testdata/values_templates/values/global.yaml new file mode 100644 index 000000000..5de1bbe31 --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/global.yaml @@ -0,0 +1,3 @@ +# should appear in subcharts too +global: + parentTemplate: true diff --git a/pkg/engine/testdata/values_templates/values/release.yaml b/pkg/engine/testdata/values_templates/values/release.yaml new file mode 100644 index 000000000..ef86f3999 --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/release.yaml @@ -0,0 +1 @@ +releaseName: {{ .Release.Name }} diff --git a/pkg/engine/testdata/values_templates/values/replaced1.yaml b/pkg/engine/testdata/values_templates/values/replaced1.yaml new file mode 100644 index 000000000..e30830c6c --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/replaced1.yaml @@ -0,0 +1,4 @@ +# shoud be replaced again by replaced2.yaml +replaced: "values/replaced1.yaml" +# should still be "values.yaml" +currentReplaced1: "{{ .Values.replaced }}" diff --git a/pkg/engine/testdata/values_templates/values/replaced2.yaml b/pkg/engine/testdata/values_templates/values/replaced2.yaml new file mode 100644 index 000000000..2e14ce47e --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/replaced2.yaml @@ -0,0 +1,4 @@ +# shoud be the final value +replaced: "values/replaced2.yaml" +# should still be "values.yaml" +currentReplaced2: "{{ .Values.replaced }}" diff --git a/pkg/engine/testdata/values_templates/values/sub_replaced.yaml b/pkg/engine/testdata/values_templates/values/sub_replaced.yaml new file mode 100644 index 000000000..b1b34fda7 --- /dev/null +++ b/pkg/engine/testdata/values_templates/values/sub_replaced.yaml @@ -0,0 +1,5 @@ + +subchart: + # should replaced by subchart/values/replaced.yaml + replaced3: "values/sub_replaced.yaml" + replaced4: "values/sub_replaced.yaml" diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 29ed67026..9224b7498 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -74,13 +74,13 @@ func TestBadChart(t *testing.T) { if strings.Contains(msg.Err.Error(), "dependencies are not valid in the Chart file with apiVersion") { e5 = true } - // This comes from the dependency check, which loads dependency info from the Chart.yaml - if strings.Contains(msg.Err.Error(), "unable to load chart") { + + if strings.Contains(msg.Err.Error(), "chart.metadata.name is required") { e6 = true } } } - if !e || !e2 || !e3 || !e4 || !e5 || !w || !i || !e6 { + if !e || !e2 || !e3 || !e4 || !e5 || !e6 || !w || !i { t.Errorf("Didn't find all the expected errors, got %#v", m) } } diff --git a/pkg/lint/rules/dependencies.go b/pkg/lint/rules/dependencies.go index abecd1feb..0df62875f 100644 --- a/pkg/lint/rules/dependencies.go +++ b/pkg/lint/rules/dependencies.go @@ -20,8 +20,6 @@ import ( "fmt" "strings" - "github.com/pkg/errors" - "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/lint/support" @@ -32,21 +30,14 @@ import ( // See https://github.com/helm/helm/issues/7910 func Dependencies(linter *support.Linter) { c, err := loader.LoadDir(linter.ChartDir) - if !linter.RunLinterRule(support.ErrorSev, "", validateChartFormat(err)) { - return + if err != nil { + return // None of our business, Templates should deal with that } linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateDependencyInMetadata(c)) linter.RunLinterRule(support.WarningSev, linter.ChartDir, validateDependencyInChartsDir(c)) } -func validateChartFormat(chartError error) error { - if chartError != nil { - return errors.Errorf("unable to load chart\n\t%s", chartError) - } - return nil -} - func validateDependencyInChartsDir(c *chart.Chart) (err error) { dependencies := map[string]struct{}{} missing := []string{} diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 9a3a2a1ba..0500b68a7 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -19,7 +19,6 @@ package rules import ( "fmt" "os" - "path" "path/filepath" "regexp" "strings" @@ -48,20 +47,18 @@ var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([- // Templates lints the templates in the Linter. func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) { - fpath := "templates/" - templatesPath := filepath.Join(linter.ChartDir, fpath) + path := "" - templatesDirExist := linter.RunLinterRule(support.WarningSev, fpath, validateTemplatesDir(templatesPath)) + // No warning for values templates directory yet + // linter.RunLinterRule(support.WarningSev, "values/", validateTemplatesDir(filepath.Join(linter.ChartDir, "values/"))) // Templates directory is optional for now - if !templatesDirExist { - return - } + linter.RunLinterRule(support.WarningSev, "templates/", validateTemplatesDir(filepath.Join(linter.ChartDir, "templates/"))) // Load chart and parse templates chart, err := loader.Load(linter.ChartDir) - chartLoaded := linter.RunLinterRule(support.ErrorSev, fpath, err) + chartLoaded := linter.RunLinterRule(support.ErrorSev, path, err) if !chartLoaded { return @@ -72,25 +69,46 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace Namespace: namespace, } - cvals, err := chartutil.CoalesceValues(chart, values) - if err != nil { - return - } - valuesToRender, err := chartutil.ToRenderValues(chart, cvals, options, nil) + valuesToRender, err := chartutil.ToRenderValues(chart, values, options, nil) if err != nil { - linter.RunLinterRule(support.ErrorSev, fpath, err) + linter.RunLinterRule(support.ErrorSev, path, err) return } var e engine.Engine e.LintMode = true renderedContentMap, err := e.Render(chart, valuesToRender) - renderOk := linter.RunLinterRule(support.ErrorSev, fpath, err) + renderOk := linter.RunLinterRule(support.ErrorSev, path, err) if !renderOk { return } + /* Iterate over all the values templates to check: + - It is a .yaml file + - All the values in the template file is defined + - {{}} include | quote + */ + for _, template := range chart.ValuesTemplates { + fileName, data := template.Name, template.Data + path = fileName + + linter.RunLinterRule(support.ErrorSev, path, validateAllowedExtension(fileName)) + // These are v3 specific checks to make sure and warn people if their + // chart is not compatible with v3 + linter.RunLinterRule(support.WarningSev, path, validateNoCRDHooks(data)) + linter.RunLinterRule(support.ErrorSev, path, validateNoReleaseTime(data)) + + // NOTE: disabled for now, Refs https://github.com/helm/helm/issues/1463 + // Check that all the templates have a matching value + //linter.RunLinterRule(support.WarningSev, path, validateNoMissingValues(templatesPath, valuesToRender, preExecutedTemplate)) + + // NOTE: disabled for now, Refs https://github.com/helm/helm/issues/1037 + // linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate))) + + // e.Render already have already checked if the content is a valid Yaml file + } + /* Iterate over all the templates to check: - It is a .yaml file - All the values in the template file is defined @@ -100,13 +118,13 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace */ for _, template := range chart.Templates { fileName, data := template.Name, template.Data - fpath = fileName + path = fileName - linter.RunLinterRule(support.ErrorSev, fpath, validateAllowedExtension(fileName)) + linter.RunLinterRule(support.ErrorSev, path, validateAllowedExtension(fileName)) // These are v3 specific checks to make sure and warn people if their // chart is not compatible with v3 - linter.RunLinterRule(support.WarningSev, fpath, validateNoCRDHooks(data)) - linter.RunLinterRule(support.ErrorSev, fpath, validateNoReleaseTime(data)) + linter.RunLinterRule(support.WarningSev, path, validateNoCRDHooks(data)) + linter.RunLinterRule(support.ErrorSev, path, validateNoReleaseTime(data)) // We only apply the following lint rules to yaml files if filepath.Ext(fileName) != ".yaml" || filepath.Ext(fileName) == ".yml" { @@ -115,12 +133,12 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace // NOTE: disabled for now, Refs https://github.com/helm/helm/issues/1463 // Check that all the templates have a matching value - //linter.RunLinterRule(support.WarningSev, fpath, validateNoMissingValues(templatesPath, valuesToRender, preExecutedTemplate)) + //linter.RunLinterRule(support.WarningSev, path, validateNoMissingValues(templatesPath, valuesToRender, preExecutedTemplate)) // NOTE: disabled for now, Refs https://github.com/helm/helm/issues/1037 - // linter.RunLinterRule(support.WarningSev, fpath, validateQuotes(string(preExecutedTemplate))) + // linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate))) - renderedContent := renderedContentMap[path.Join(chart.Name(), fileName)] + renderedContent := renderedContentMap[filepath.Join(chart.Name(), fileName)] if strings.TrimSpace(renderedContent) != "" { var yamlStruct K8sYamlStruct // Even though K8sYamlStruct only defines a few fields, an error in any other @@ -129,10 +147,10 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace // If YAML linting fails, we sill progress. So we don't capture the returned state // on this linter run. - linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) - linter.RunLinterRule(support.ErrorSev, fpath, validateMetadataName(&yamlStruct)) - linter.RunLinterRule(support.ErrorSev, fpath, validateNoDeprecations(&yamlStruct)) - linter.RunLinterRule(support.ErrorSev, fpath, validateMatchSelector(&yamlStruct, renderedContent)) + linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) + linter.RunLinterRule(support.ErrorSev, path, validateMetadataName(&yamlStruct)) + linter.RunLinterRule(support.ErrorSev, path, validateNoDeprecations(&yamlStruct)) + linter.RunLinterRule(support.ErrorSev, path, validateMatchSelector(&yamlStruct, renderedContent)) } } }