From 00f8561ad4502286d49868055777537c92a869de Mon Sep 17 00:00:00 2001 From: Edward Miller Date: Tue, 12 Sep 2023 09:52:45 +0100 Subject: [PATCH 1/5] fix(pkg/lint): unmarshals Chart.yaml strictly When "helm lint" is run, it now errors on invalid chartfiles, e.g. those with duplicate keys Closes #12381 Signed-off-by: Edward Miller --- pkg/chart/v2/util/chartfile.go | 2 +- pkg/lint/lint_test.go | 11 +++++++++++ pkg/lint/rules/testdata/invalidchartfile/Chart.yaml | 5 +++++ pkg/lint/rules/testdata/invalidchartfile/values.yaml | 0 4 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 pkg/lint/rules/testdata/invalidchartfile/Chart.yaml create mode 100644 pkg/lint/rules/testdata/invalidchartfile/values.yaml diff --git a/pkg/chart/v2/util/chartfile.go b/pkg/chart/v2/util/chartfile.go index 87323c201..0a6ca0e20 100644 --- a/pkg/chart/v2/util/chartfile.go +++ b/pkg/chart/v2/util/chartfile.go @@ -33,7 +33,7 @@ func LoadChartfile(filename string) (*chart.Metadata, error) { return nil, err } y := new(chart.Metadata) - err = yaml.Unmarshal(b, y) + err = yaml.UnmarshalStrict(b, y) return y, err } diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 067d140f6..ecf544686 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -35,6 +35,7 @@ const badYamlFileDir = "rules/testdata/albatross" const goodChartDir = "rules/testdata/goodone" const subChartValuesDir = "rules/testdata/withsubchart" const malformedTemplate = "rules/testdata/malformed-template" +const invalidChartFileDir = "rules/testdata/invalidchartfile" func TestBadChart(t *testing.T) { m := RunAll(badChartDir, values, namespace).Messages @@ -90,6 +91,16 @@ func TestInvalidYaml(t *testing.T) { } } +func TestInvalidChartYaml(t *testing.T) { + m := All(invalidChartFileDir, values, namespace, strict).Messages + if len(m) != 1 { + t.Fatalf("All didn't fail with expected errors, got %#v", m) + } + if !strings.Contains(m[0].Err.Error(), "unable to parse YAML") { + t.Errorf("All didn't have the error for duplicate YAML keys") + } +} + func TestBadValues(t *testing.T) { m := RunAll(badValuesFileDir, values, namespace).Messages if len(m) < 1 { diff --git a/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml b/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml new file mode 100644 index 000000000..01dcf1864 --- /dev/null +++ b/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml @@ -0,0 +1,5 @@ +name: some-chart +apiVersion: v2 +apiVersion: v1 +description: A Helm chart for Kubernetes +version: 1.3.0 diff --git a/pkg/lint/rules/testdata/invalidchartfile/values.yaml b/pkg/lint/rules/testdata/invalidchartfile/values.yaml new file mode 100644 index 000000000..e69de29bb From 14a468f24de9e88467322e25e9fedc19490d2dfc Mon Sep 17 00:00:00 2001 From: Edward Miller Date: Thu, 28 Dec 2023 14:38:05 +0000 Subject: [PATCH 2/5] Add chartutil.StrictLoadChartfile for strict (WARNING-level) lint Signed-off-by: Edward Miller --- pkg/chart/v2/util/chartfile.go | 11 +++++++++++ pkg/lint/lint_test.go | 2 +- pkg/lint/rules/chartfile.go | 10 ++++++++++ pkg/lint/rules/testdata/invalidchartfile/Chart.yaml | 1 + 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/chart/v2/util/chartfile.go b/pkg/chart/v2/util/chartfile.go index 0a6ca0e20..b48687d55 100644 --- a/pkg/chart/v2/util/chartfile.go +++ b/pkg/chart/v2/util/chartfile.go @@ -28,6 +28,17 @@ import ( // LoadChartfile loads a Chart.yaml file into a *chart.Metadata. func LoadChartfile(filename string) (*chart.Metadata, error) { + b, err := os.ReadFile(filename) + if err != nil { + return nil, err + } + y := new(chart.Metadata) + err = yaml.Unmarshal(b, y) + return y, err +} + +// StrictLoadChartFile loads a Chart.yaml into a *chart.Metadata using a strict unmarshaling +func StrictLoadChartfile(filename string) (*chart.Metadata, error) { b, err := os.ReadFile(filename) if err != nil { return nil, err diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index ecf544686..11d53d0e0 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -96,7 +96,7 @@ func TestInvalidChartYaml(t *testing.T) { if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } - if !strings.Contains(m[0].Err.Error(), "unable to parse YAML") { + if !strings.Contains(m[0].Err.Error(), "failed to strictly parse chartfile") { t.Errorf("All didn't have the error for duplicate YAML keys") } } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 598557a97..7c71c9df6 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -46,6 +46,9 @@ func Chartfile(linter *support.Linter) { return } + _, err = chartutil.StrictLoadChartfile(chartPath) + linter.RunLinterRule(support.WarningSev, chartFileName, validateChartYamlStrictFormat(err)) + // type check for Chart.yaml . ignoring error as any parse // errors would already be caught in the above load function chartFileForTypeCheck, _ := loadChartFileForTypeCheck(chartPath) @@ -102,6 +105,13 @@ func validateChartYamlFormat(chartFileError error) error { return nil } +func validateChartYamlStrictFormat(chartFileError error) error { + if chartFileError != nil { + return errors.Errorf("failed to strictly parse chartfile\n\t%s", chartFileError.Error()) + } + return nil +} + func validateChartName(cf *chart.Metadata) error { if cf.Name == "" { return errors.New("name is required") diff --git a/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml b/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml index 01dcf1864..0fd58d1d4 100644 --- a/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml +++ b/pkg/lint/rules/testdata/invalidchartfile/Chart.yaml @@ -3,3 +3,4 @@ apiVersion: v2 apiVersion: v1 description: A Helm chart for Kubernetes version: 1.3.0 +icon: http://example.com From 9d43f706436bd921c9396f95a80bcbdf54b8b449 Mon Sep 17 00:00:00 2001 From: Edward Miller <55854159+edbmiller@users.noreply.github.com> Date: Tue, 11 Feb 2025 10:52:18 +0000 Subject: [PATCH 3/5] Update error message Co-authored-by: Andrew Block Signed-off-by: Edward Miller <55854159+edbmiller@users.noreply.github.com> --- pkg/lint/lint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 11d53d0e0..a776aecff 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -96,7 +96,7 @@ func TestInvalidChartYaml(t *testing.T) { if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } - if !strings.Contains(m[0].Err.Error(), "failed to strictly parse chartfile") { + if !strings.Contains(m[0].Err.Error(), "failed to strictly parse chart metadata file") { t.Errorf("All didn't have the error for duplicate YAML keys") } } From 629780a34a834f0bbe1123a42033e48648add8ae Mon Sep 17 00:00:00 2001 From: Edward Miller Date: Fri, 18 Apr 2025 19:12:34 +0100 Subject: [PATCH 4/5] fix: reapply error message fix Signed-off-by: Edward Miller --- pkg/lint/rules/chartfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 7c71c9df6..13ae77222 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -107,7 +107,7 @@ func validateChartYamlFormat(chartFileError error) error { func validateChartYamlStrictFormat(chartFileError error) error { if chartFileError != nil { - return errors.Errorf("failed to strictly parse chartfile\n\t%s", chartFileError.Error()) + return errors.Errorf("failed to strictly parse chart metadata file\n\t%s", chartFileError.Error()) } return nil } From e414695a5b8ba0717c7897785ac742899554f474 Mon Sep 17 00:00:00 2001 From: Edward Miller Date: Sat, 19 Apr 2025 13:40:32 +0100 Subject: [PATCH 5/5] fix Signed-off-by: Edward Miller --- pkg/lint/lint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index a776aecff..888d3dfe6 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -92,7 +92,7 @@ func TestInvalidYaml(t *testing.T) { } func TestInvalidChartYaml(t *testing.T) { - m := All(invalidChartFileDir, values, namespace, strict).Messages + m := RunAll(invalidChartFileDir, values, namespace).Messages if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) }