From 29842e040fd90b5d19d4393a5345a4709020252d Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 14 May 2019 14:12:47 -0400 Subject: [PATCH 1/3] Adding lint check for apiVersion which is a required field Fixes #5727 Signed-off-by: Matt Farina --- pkg/lint/lint_test.go | 10 +++++++--- pkg/lint/rules/chartfile.go | 13 +++++++++++++ pkg/lint/rules/chartfile_test.go | 14 +++++++++----- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index e4fc67152..326a02e69 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -35,12 +35,12 @@ const goodChartDir = "rules/testdata/goodone" func TestBadChart(t *testing.T) { m := All(badChartDir, values, namespace, strict).Messages - if len(m) != 5 { + if len(m) != 6 { t.Errorf("Number of errors %v", len(m)) t.Errorf("All didn't fail with expected errors, got %#v", m) } // There should be one INFO, 2 WARNINGs and one ERROR messages, check for them - var i, w, e, e2, e3 bool + var i, w, e, e2, e3, e4 bool for _, msg := range m { if msg.Severity == support.InfoSev { if strings.Contains(msg.Err.Error(), "icon is recommended") { @@ -62,9 +62,13 @@ func TestBadChart(t *testing.T) { if strings.Contains(msg.Err.Error(), "directory name (badchartfile) and chart name () must be the same") { e3 = true } + + if strings.Contains(msg.Err.Error(), "apiVersion is required") { + e4 = true + } } } - if !e || !e2 || !e3 || !w || !i { + if !e || !e2 || !e3 || !e4 || !w || !i { t.Errorf("Didn't find all the expected errors, got %#v", m) } } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index f1abf55d4..2f9707884 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -48,6 +48,7 @@ func Chartfile(linter *support.Linter) { linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartNameDirMatch(linter.ChartDir, chartFile)) // Chart metadata + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartApiVersion(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartVersion(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartMaintainer(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartSources(chartFile)) @@ -85,6 +86,18 @@ func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) error { return nil } +func validateChartApiVersion(cf *chart.Metadata) error { + if cf.ApiVersion == "" { + return errors.New("apiVersion is required") + } + + if cf.ApiVersion != "v1" { + return fmt.Errorf("apiVersion '%s' is not valid. The value must be \"v1\"", cf.ApiVersion) + } + + return nil +} + func validateChartVersion(cf *chart.Metadata) error { if cf.Version == "" { return errors.New("version is required") diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index e9ef3a29d..461616034 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -209,8 +209,8 @@ func TestChartfile(t *testing.T) { Chartfile(&linter) msgs := linter.Messages - if len(msgs) != 4 { - t.Errorf("Expected 3 errors, got %d", len(msgs)) + if len(msgs) != 5 { + t.Errorf("Expected 4 errors, got %d", len(msgs)) } if !strings.Contains(msgs[0].Err.Error(), "name is required") { @@ -221,12 +221,16 @@ func TestChartfile(t *testing.T) { t.Errorf("Unexpected message 1: %s", msgs[1].Err) } - if !strings.Contains(msgs[2].Err.Error(), "version 0.0.0 is less than or equal to 0") { + if !strings.Contains(msgs[2].Err.Error(), "apiVersion is required") { t.Errorf("Unexpected message 2: %s", msgs[2].Err) } - if !strings.Contains(msgs[3].Err.Error(), "icon is recommended") { - t.Errorf("Unexpected message 3: %s", msgs[3].Err) + if !strings.Contains(msgs[3].Err.Error(), "version 0.0.0 is less than or equal to 0") { + t.Errorf("Unexpected message 3: %s", msgs[2].Err) + } + + if !strings.Contains(msgs[4].Err.Error(), "icon is recommended") { + t.Errorf("Unexpected message 4: %s", msgs[3].Err) } } From 069eec9e425c727323d7eb866662c1cff67757e6 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 14 May 2019 14:46:40 -0400 Subject: [PATCH 2/3] Updatin gthe apiVersion linting for Helm v3 Signed-off-by: Matt Farina --- pkg/lint/rules/chartfile.go | 11 ++++++----- pkg/lint/rules/testdata/badchartfile/Chart.yaml | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 2f9707884..24c8a6f44 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -17,6 +17,7 @@ limitations under the License. package rules // import "helm.sh/helm/pkg/lint/rules" import ( + "fmt" "os" "path/filepath" @@ -48,7 +49,7 @@ func Chartfile(linter *support.Linter) { linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartNameDirMatch(linter.ChartDir, chartFile)) // Chart metadata - linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartApiVersion(chartFile)) + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartAPIVersion(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartVersion(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartMaintainer(chartFile)) linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartSources(chartFile)) @@ -86,13 +87,13 @@ func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) error { return nil } -func validateChartApiVersion(cf *chart.Metadata) error { - if cf.ApiVersion == "" { +func validateChartAPIVersion(cf *chart.Metadata) error { + if cf.APIVersion == "" { return errors.New("apiVersion is required") } - if cf.ApiVersion != "v1" { - return fmt.Errorf("apiVersion '%s' is not valid. The value must be \"v1\"", cf.ApiVersion) + if cf.APIVersion != "v1" && cf.APIVersion != "v2" { + return fmt.Errorf("apiVersion '%s' is not valid. The value must be either \"v1\" or \"v2\"", cf.APIVersion) } return nil diff --git a/pkg/lint/rules/testdata/badchartfile/Chart.yaml b/pkg/lint/rules/testdata/badchartfile/Chart.yaml index b64052eb9..dbb4a1501 100644 --- a/pkg/lint/rules/testdata/badchartfile/Chart.yaml +++ b/pkg/lint/rules/testdata/badchartfile/Chart.yaml @@ -1,4 +1,3 @@ -apiVersion: v1 description: A Helm chart for Kubernetes version: 0.0.0 home: "" From 50e06b14474b1acc204b7c05e85696cf69415f49 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 14 May 2019 16:14:52 -0400 Subject: [PATCH 3/3] Adding apiVersion guidance to the linting Signed-off-by: Matt Farina --- pkg/lint/lint_test.go | 2 +- pkg/lint/rules/chartfile.go | 2 +- pkg/lint/rules/chartfile_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 326a02e69..962a9ca41 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -63,7 +63,7 @@ func TestBadChart(t *testing.T) { e3 = true } - if strings.Contains(msg.Err.Error(), "apiVersion is required") { + if strings.Contains(msg.Err.Error(), "apiVersion is required. The value must be either \"v1\" or \"v2\"") { e4 = true } } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 24c8a6f44..da0675868 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -89,7 +89,7 @@ func validateChartNameDirMatch(chartDir string, cf *chart.Metadata) error { func validateChartAPIVersion(cf *chart.Metadata) error { if cf.APIVersion == "" { - return errors.New("apiVersion is required") + return errors.New("apiVersion is required. The value must be either \"v1\" or \"v2\"") } if cf.APIVersion != "v1" && cf.APIVersion != "v2" { diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 461616034..4e71b860a 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -221,7 +221,7 @@ func TestChartfile(t *testing.T) { t.Errorf("Unexpected message 1: %s", msgs[1].Err) } - if !strings.Contains(msgs[2].Err.Error(), "apiVersion is required") { + if !strings.Contains(msgs[2].Err.Error(), "apiVersion is required. The value must be either \"v1\" or \"v2\"") { t.Errorf("Unexpected message 2: %s", msgs[2].Err) }