From 87afc64de58bd69853c7a0436fea1f53a1868c78 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 11 Apr 2018 15:55:35 -0400 Subject: [PATCH] fix(helm) support prefix for chart dir name Signed-off-by: Arash Deshmeh --- cmd/helm/lint.go | 8 +++--- cmd/helm/lint_test.go | 15 +++++++---- .../testcharts/chart-dir-prefix/Chart.yaml | 3 +++ .../testcharts/chart-dir-prefix/values.yaml | 4 +++ docs/helm/helm_lint.md | 3 ++- pkg/lint/lint.go | 4 +-- pkg/lint/lint_test.go | 26 ++++++++++++++++--- pkg/lint/rules/chartfile.go | 15 +++++++---- pkg/lint/rules/chartfile_test.go | 19 +++++++++++--- 9 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/chart-dir-prefix/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-dir-prefix/values.yaml diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index 63f11c062..991cb69df 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -51,6 +51,7 @@ type lintCmd struct { strict bool paths []string out io.Writer + dirPrefix string } func newLintCmd(out io.Writer) *cobra.Command { @@ -75,6 +76,7 @@ func newLintCmd(out io.Writer) *cobra.Command { cmd.Flags().StringArrayVar(&l.sValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") cmd.Flags().StringVar(&l.namespace, "namespace", "default", "namespace to install the release into (only used if --install is set)") cmd.Flags().BoolVar(&l.strict, "strict", false, "fail on lint warnings") + cmd.Flags().StringVar(&l.dirPrefix, "dir-prefix", "", "require the specified prefix on chart dir name") return cmd } @@ -98,7 +100,7 @@ func (l *lintCmd) run() error { var total int var failures int for _, path := range l.paths { - if linter, err := lintChart(path, rvals, l.namespace, l.strict); err != nil { + if linter, err := lintChart(path, rvals, l.namespace, l.strict, l.dirPrefix); err != nil { fmt.Println("==> Skipping", path) fmt.Println(err) if err == errLintNoChart { @@ -133,7 +135,7 @@ func (l *lintCmd) run() error { return nil } -func lintChart(path string, vals []byte, namespace string, strict bool) (support.Linter, error) { +func lintChart(path string, vals []byte, namespace string, strict bool, dirPrefix string) (support.Linter, error) { var chartPath string linter := support.Linter{} @@ -169,7 +171,7 @@ func lintChart(path string, vals []byte, namespace string, strict bool) (support return linter, errLintNoChart } - return lint.All(chartPath, vals, namespace, strict), nil + return lint.All(chartPath, vals, namespace, strict, dirPrefix), nil } func (l *lintCmd) vals() ([]byte, error) { diff --git a/cmd/helm/lint_test.go b/cmd/helm/lint_test.go index 973af9b63..2418b5e0f 100644 --- a/cmd/helm/lint_test.go +++ b/cmd/helm/lint_test.go @@ -29,26 +29,31 @@ var ( invalidArchivedChartPath = "testdata/testcharts/invalidcompressedchart0.1.0.tgz" chartDirPath = "testdata/testcharts/decompressedchart/" chartMissingManifest = "testdata/testcharts/chart-missing-manifest" + chartDirWithPrefix = "testdata/testcharts/chart-dir-prefix" ) func TestLintChart(t *testing.T) { - if _, err := lintChart(chartDirPath, values, namespace, strict); err != nil { + if _, err := lintChart(chartDirPath, values, namespace, strict, ""); err != nil { t.Errorf("%s", err) } - if _, err := lintChart(archivedChartPath, values, namespace, strict); err != nil { + if _, err := lintChart(archivedChartPath, values, namespace, strict, ""); err != nil { t.Errorf("%s", err) } - if _, err := lintChart(archivedChartPathWithHyphens, values, namespace, strict); err != nil { + if _, err := lintChart(archivedChartPathWithHyphens, values, namespace, strict, ""); err != nil { t.Errorf("%s", err) } - if _, err := lintChart(invalidArchivedChartPath, values, namespace, strict); err == nil { + if _, err := lintChart(chartDirWithPrefix, values, namespace, strict, "chart-"); err != nil { + t.Errorf("%s", err) + } + + if _, err := lintChart(invalidArchivedChartPath, values, namespace, strict, ""); err == nil { t.Errorf("Expected a chart parsing error") } - if _, err := lintChart(chartMissingManifest, values, namespace, strict); err == nil { + if _, err := lintChart(chartMissingManifest, values, namespace, strict, ""); err == nil { t.Errorf("Expected a chart parsing error") } } diff --git a/cmd/helm/testdata/testcharts/chart-dir-prefix/Chart.yaml b/cmd/helm/testdata/testcharts/chart-dir-prefix/Chart.yaml new file mode 100644 index 000000000..b1bd3d187 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-dir-prefix/Chart.yaml @@ -0,0 +1,3 @@ +description: A Helm chart for Kubernetes +name: dir-prefix +version: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/chart-dir-prefix/values.yaml b/cmd/helm/testdata/testcharts/chart-dir-prefix/values.yaml new file mode 100644 index 000000000..c307cfc00 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-dir-prefix/values.yaml @@ -0,0 +1,4 @@ +# Default values for dir-prefix chart +# This is a YAML-formatted file. +# Declare name/value pairs to be passed into your templates. + name: value diff --git a/docs/helm/helm_lint.md b/docs/helm/helm_lint.md index 596edf2bb..8bce148c4 100644 --- a/docs/helm/helm_lint.md +++ b/docs/helm/helm_lint.md @@ -21,6 +21,7 @@ helm lint [flags] PATH ### Options ``` + --dir-prefix string require the specified prefix on chart dir name --namespace string namespace to install the release into (only used if --install is set) (default "default") --set stringArray set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2) --set-string stringArray set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2) @@ -42,4 +43,4 @@ helm lint [flags] PATH ### SEE ALSO * [helm](helm.md) - The Helm package manager for Kubernetes. -###### Auto generated by spf13/cobra on 9-Mar-2018 +###### Auto generated by spf13/cobra on 11-Apr-2018 diff --git a/pkg/lint/lint.go b/pkg/lint/lint.go index 256eab906..4da9bfc3c 100644 --- a/pkg/lint/lint.go +++ b/pkg/lint/lint.go @@ -24,12 +24,12 @@ import ( ) // All runs all of the available linters on the given base directory. -func All(basedir string, values []byte, namespace string, strict bool) support.Linter { +func All(basedir string, values []byte, namespace string, strict bool, dirPrefix string) support.Linter { // Using abs path to get directory context chartDir, _ := filepath.Abs(basedir) linter := support.Linter{ChartDir: chartDir} - rules.Chartfile(&linter) + rules.Chartfile(&linter, dirPrefix) rules.Values(&linter) rules.Templates(&linter, values, namespace, strict) return linter diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index d84faa10b..47fae49d6 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -35,7 +35,7 @@ const badYamlFileDir = "rules/testdata/albatross" const goodChartDir = "rules/testdata/goodone" func TestBadChart(t *testing.T) { - m := All(badChartDir, values, namespace, strict).Messages + m := All(badChartDir, values, namespace, strict, "").Messages if len(m) != 5 { t.Errorf("Number of errors %v", len(m)) t.Errorf("All didn't fail with expected errors, got %#v", m) @@ -70,8 +70,26 @@ func TestBadChart(t *testing.T) { } } +func TestBadChartDirnameDueToPrefix(t *testing.T) { + prefix := "foo" + expectedMsg := "directory name (badchartfile) must be the same as chart name () appended to required prefix (foo)" + m := All(badChartDir, values, namespace, strict, prefix).Messages + var e bool + for _, msg := range m { + if msg.Severity == support.ErrorSev { + if strings.Contains(msg.Err.Error(), expectedMsg) { + e = true + } + } + } + + if !e { + t.Errorf("Did not find the expected error: %s, got %#v", expectedMsg, m) + } +} + func TestInvalidYaml(t *testing.T) { - m := All(badYamlFileDir, values, namespace, strict).Messages + m := All(badYamlFileDir, values, namespace, strict, "").Messages if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } @@ -81,7 +99,7 @@ func TestInvalidYaml(t *testing.T) { } func TestBadValues(t *testing.T) { - m := All(badValuesFileDir, values, namespace, strict).Messages + m := All(badValuesFileDir, values, namespace, strict, "").Messages if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } @@ -91,7 +109,7 @@ func TestBadValues(t *testing.T) { } func TestGoodChart(t *testing.T) { - m := All(goodChartDir, values, namespace, strict).Messages + m := All(goodChartDir, values, namespace, strict, "").Messages if len(m) != 0 { t.Errorf("All failed but shouldn't have: %#v", m) } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 0dab0d250..5abe74ea3 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -32,7 +32,7 @@ import ( ) // Chartfile runs a set of linter rules related to Chart.yaml file -func Chartfile(linter *support.Linter) { +func Chartfile(linter *support.Linter, dirPrefix string) { chartFileName := "Chart.yaml" chartPath := filepath.Join(linter.ChartDir, chartFileName) @@ -47,7 +47,7 @@ func Chartfile(linter *support.Linter) { } linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartName(chartFile)) - linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartNameDirMatch(linter.ChartDir, chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartNameDirMatch(linter.ChartDir, chartFile, dirPrefix)) // Chart metadata linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartVersion(chartFile)) @@ -81,9 +81,14 @@ func validateChartName(cf *chart.Metadata) error { return nil } -func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) error { - if cf.Name != filepath.Base(chartDir) { - return fmt.Errorf("directory name (%s) and chart name (%s) must be the same", filepath.Base(chartDir), cf.Name) +func validateChartNameDirMatch(chartDir string, cf *chart.Metadata, dirPrefix string) error { + base := filepath.Base(chartDir) + if dirPrefix != "" { + if !strings.HasPrefix(base, dirPrefix) || cf.Name != strings.Replace(base, dirPrefix, "", 1) { + return fmt.Errorf("directory name (%s) must be the same as chart name (%s) appended to required prefix (%s)", base, cf.Name, dirPrefix) + } + } else if cf.Name != base { + return fmt.Errorf("directory name (%s) and chart name (%s) must be the same", base, cf.Name) } return nil } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 99dc4de0f..4ec3db1d2 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -73,18 +73,18 @@ func TestValidateChartName(t *testing.T) { } func TestValidateChartNameDirMatch(t *testing.T) { - err := validateChartNameDirMatch(goodChartDir, goodChart) + err := validateChartNameDirMatch(goodChartDir, goodChart, "") if err != nil { t.Errorf("validateChartNameDirMatch to return no error, gor a linter error") } // It has not name - err = validateChartNameDirMatch(badChartDir, badChart) + err = validateChartNameDirMatch(badChartDir, badChart, "") if err == nil { t.Errorf("validatechartnamedirmatch to return a linter error, got no error") } // Wrong path - err = validateChartNameDirMatch(badChartDir, goodChart) + err = validateChartNameDirMatch(badChartDir, goodChart, "") if err == nil { t.Errorf("validatechartnamedirmatch to return a linter error, got no error") } @@ -223,7 +223,7 @@ func TestValidateChartIconURL(t *testing.T) { func TestChartfile(t *testing.T) { linter := support.Linter{ChartDir: badChartDir} - Chartfile(&linter) + Chartfile(&linter, "") msgs := linter.Messages if len(msgs) != 4 { @@ -247,3 +247,14 @@ func TestChartfile(t *testing.T) { } } + +func TestChartfileWithDirPrefix(t *testing.T) { + prefix := "foo" + linter := support.Linter{ChartDir: badChartDir} + Chartfile(&linter, prefix) + msgs := linter.Messages + + if !strings.Contains(msgs[1].Err.Error(), "directory name (badchartfile) must be the same as chart name () appended to required prefix (foo)") { + t.Errorf("Unexpected message 1: %s", msgs[1].Err) + } +}