From 2ce368eac7a6388e5f49d5a1c953b87bd65cb3bf Mon Sep 17 00:00:00 2001 From: Janit Sriganeshaelankovan Date: Mon, 14 Jul 2025 22:14:35 -0400 Subject: [PATCH 1/3] updated chart name check for create and lint Signed-off-by: Janit Sriganeshaelankovan --- pkg/chart/v2/util/create.go | 5 +--- pkg/chart/v2/util/create_test.go | 6 +++-- pkg/lint/rules/chartfile.go | 17 ++++++++++---- pkg/lint/rules/chartfile_test.go | 40 ++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/pkg/chart/v2/util/create.go b/pkg/chart/v2/util/create.go index 93699e0fd..2fea0942c 100644 --- a/pkg/chart/v2/util/create.go +++ b/pkg/chart/v2/util/create.go @@ -31,10 +31,7 @@ import ( ) // chartName is a regular expression for testing the supplied name of a chart. -// This regular expression is probably stricter than it needs to be. We can relax it -// somewhat. Newline characters, as well as $, quotes, +, parens, and % are known to be -// problematic. -var chartName = regexp.MustCompile("^[a-zA-Z0-9._-]+$") +var chartName = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) const ( // ChartfileName is the default Chart file name. diff --git a/pkg/chart/v2/util/create_test.go b/pkg/chart/v2/util/create_test.go index 086c4e5c8..973e65831 100644 --- a/pkg/chart/v2/util/create_test.go +++ b/pkg/chart/v2/util/create_test.go @@ -147,8 +147,10 @@ func TestCreate_Overwrite(t *testing.T) { func TestValidateChartName(t *testing.T) { for name, shouldPass := range map[string]bool{ "": false, - "abcdefghijklmnopqrstuvwxyz-_.": true, - "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": true, + "abcdefghijklmnopqrstuvwxyz-_.": false, + "abcdefghijklmnopqrstuvwxyz": true, + "abcdefghijklm-nopqrstuvwxyz": true, + "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": false, "$hello": false, "Hellô": false, "he%%o": false, diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 724c3f2ea..e6d9a1769 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "github.com/Masterminds/semver/v3" "github.com/asaskevich/govalidator" @@ -31,6 +32,13 @@ import ( "helm.sh/helm/v4/pkg/lint/support" ) +// chartName is a regular expression for testing the supplied name of a chart. +var chartName = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) + +// maxChartNameLength is lower than the limits we know of with certain file systems, +// and with certain Kubernetes fields. +const maxChartNameLength = 250 + // Chartfile runs a set of linter rules related to Chart.yaml file func Chartfile(linter *support.Linter) { chartFileName := "Chart.yaml" @@ -113,12 +121,11 @@ func validateChartYamlStrictFormat(chartFileError error) error { } func validateChartName(cf *chart.Metadata) error { - if cf.Name == "" { - return errors.New("name is required") + if cf.Name == "" || len(cf.Name) > maxChartNameLength { + return fmt.Errorf("chart name must be between 1 and %d characters", maxChartNameLength) } - name := filepath.Base(cf.Name) - if name != cf.Name { - return fmt.Errorf("chart name %q is invalid", cf.Name) + if !chartName.MatchString(cf.Name) { + return fmt.Errorf("chart name must use only lowercase letters, digits, and dashes (no underscores, dots, or uppercase letters)") } return nil } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index bbb14a5e8..54664c60c 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -76,6 +76,46 @@ func TestValidateChartName(t *testing.T) { if err == nil { t.Error("expected validateChartName to return a linter error for an invalid name, got no error") } + + failTests := []*chart.Metadata{ + + {Name: ""}, // empty + {Name: "ChartName"}, // uppercase + {Name: "chart_name"}, // underscore + {Name: "chart.name"}, // dot + {Name: "chart--name"}, // double dash (optional) + {Name: "-chartname"}, // starts with dash + {Name: "chartname-"}, // ends with dash + {Name: "chart$name"}, // special character + {Name: "chart/name"}, // forward slash + {Name: "chart\\name"}, // backslash + {Name: "chart name"}, // space + {Name: strings.Repeat("a", 251)}, // 241 chars — too long + + } + + successTests := []*chart.Metadata{ + {Name: "nginx"}, + {Name: "nginx-lego"}, + {Name: "aws-cluster-autoscaler"}, + {Name: "abc123"}, + {Name: "chartname123"}, + {Name: strings.Repeat("a", 250)}, + } + + for _, chart := range failTests { + err := validateChartName(chart) + if err == nil { + t.Errorf("validateChartName(%q) expected error, got nil", chart.Name) + } + } + + for _, chart := range successTests { + err := validateChartName(chart) + if err != nil { + t.Errorf("validateChartName(%q) expected no error, got: %s", chart.Name, err) + } + } } func TestValidateChartVersion(t *testing.T) { From dc1694107527685bed063d20e396ffac7d59a88b Mon Sep 17 00:00:00 2001 From: Janit Sriganeshaelankovan Date: Mon, 21 Jul 2025 16:46:31 -0400 Subject: [PATCH 2/3] updated error logging to include list of allowed Signed-off-by: Janit Sriganeshaelankovan --- pkg/chart/v2/util/create.go | 2 +- pkg/lint/rules/chartfile.go | 2 +- pkg/lint/rules/chartfile_test.go | 11 ++++------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/chart/v2/util/create.go b/pkg/chart/v2/util/create.go index 2fea0942c..1822ed3e3 100644 --- a/pkg/chart/v2/util/create.go +++ b/pkg/chart/v2/util/create.go @@ -823,7 +823,7 @@ func validateChartName(name string) error { return fmt.Errorf("chart name must be between 1 and %d characters", maxChartNameLength) } if !chartName.MatchString(name) { - return fmt.Errorf("chart name must match the regular expression %q", chartName.String()) + return fmt.Errorf("chart name must use only lowercase letters or digits optionally with dash separators") } return nil } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index e6d9a1769..9c071155a 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -125,7 +125,7 @@ func validateChartName(cf *chart.Metadata) error { return fmt.Errorf("chart name must be between 1 and %d characters", maxChartNameLength) } if !chartName.MatchString(cf.Name) { - return fmt.Errorf("chart name must use only lowercase letters, digits, and dashes (no underscores, dots, or uppercase letters)") + return fmt.Errorf("chart name must use only lowercase letters or digits optionally with dash separators") } return nil } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 54664c60c..03503468d 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -78,7 +78,6 @@ func TestValidateChartName(t *testing.T) { } failTests := []*chart.Metadata{ - {Name: ""}, // empty {Name: "ChartName"}, // uppercase {Name: "chart_name"}, // underscore @@ -90,15 +89,13 @@ func TestValidateChartName(t *testing.T) { {Name: "chart/name"}, // forward slash {Name: "chart\\name"}, // backslash {Name: "chart name"}, // space - {Name: strings.Repeat("a", 251)}, // 241 chars — too long - + {Name: strings.Repeat("a", 251)}, // 251 chars — too long } successTests := []*chart.Metadata{ - {Name: "nginx"}, - {Name: "nginx-lego"}, - {Name: "aws-cluster-autoscaler"}, - {Name: "abc123"}, + {Name: "chartname"}, + {Name: "chart-name"}, + {Name: "chart-name-success"}, {Name: "chartname123"}, {Name: strings.Repeat("a", 250)}, } From b0584157d859036ce677cca78bc768f17a5b2008 Mon Sep 17 00:00:00 2001 From: Janit Sriganeshaelankovan Date: Mon, 21 Jul 2025 16:49:55 -0400 Subject: [PATCH 3/3] updated test comment Signed-off-by: Janit Sriganeshaelankovan --- pkg/lint/rules/chartfile_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 03503468d..a75b5fe2a 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -82,7 +82,7 @@ func TestValidateChartName(t *testing.T) { {Name: "ChartName"}, // uppercase {Name: "chart_name"}, // underscore {Name: "chart.name"}, // dot - {Name: "chart--name"}, // double dash (optional) + {Name: "chart--name"}, // double dash {Name: "-chartname"}, // starts with dash {Name: "chartname-"}, // ends with dash {Name: "chart$name"}, // special character