diff --git a/internal/chart/v3/lint/rules/chartfile.go b/internal/chart/v3/lint/rules/chartfile.go index fc246ba80..b9a560d92 100644 --- a/internal/chart/v3/lint/rules/chartfile.go +++ b/internal/chart/v3/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 ( chartutil "helm.sh/helm/v4/internal/chart/v3/util" ) +// 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 or digits optionally with dash separators") } return nil } diff --git a/internal/chart/v3/lint/rules/chartfile_test.go b/internal/chart/v3/lint/rules/chartfile_test.go index 57893e151..f652b9ed2 100644 --- a/internal/chart/v3/lint/rules/chartfile_test.go +++ b/internal/chart/v3/lint/rules/chartfile_test.go @@ -76,6 +76,43 @@ 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 + {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)}, // 251 chars — too long + } + + successTests := []*chart.Metadata{ + {Name: "chartname"}, + {Name: "chart-name"}, + {Name: "chart-name-success"}, + {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) { @@ -235,7 +272,7 @@ func TestV3Chartfile(t *testing.T) { return } - if !strings.Contains(msgs[0].Err.Error(), "name is required") { + if !strings.Contains(msgs[0].Err.Error(), "chart name must be between 1 and 250 characters") { t.Errorf("Unexpected message 0: %s", msgs[0].Err) } diff --git a/internal/chart/v3/util/create.go b/internal/chart/v3/util/create.go index 0dfa30995..fa613aca1 100644 --- a/internal/chart/v3/util/create.go +++ b/internal/chart/v3/util/create.go @@ -35,7 +35,7 @@ import ( // 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. @@ -828,7 +828,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/internal/chart/v3/util/create_test.go b/internal/chart/v3/util/create_test.go index b3b58cc5a..36b2fbd5e 100644 --- a/internal/chart/v3/util/create_test.go +++ b/internal/chart/v3/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/chart/v2/lint/rules/chartfile.go b/pkg/chart/v2/lint/rules/chartfile.go index 806363477..b44c13364 100644 --- a/pkg/chart/v2/lint/rules/chartfile.go +++ b/pkg/chart/v2/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 ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) +// 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" @@ -114,12 +122,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 or digits optionally with dash separators") } return nil } diff --git a/pkg/chart/v2/lint/rules/chartfile_test.go b/pkg/chart/v2/lint/rules/chartfile_test.go index 692358426..0e99be16d 100644 --- a/pkg/chart/v2/lint/rules/chartfile_test.go +++ b/pkg/chart/v2/lint/rules/chartfile_test.go @@ -76,6 +76,43 @@ 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 + {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)}, // 251 chars — too long + } + + successTests := []*chart.Metadata{ + {Name: "chartname"}, + {Name: "chart-name"}, + {Name: "chart-name-success"}, + {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) { @@ -262,7 +299,7 @@ func TestChartfile(t *testing.T) { return } - if !strings.Contains(msgs[0].Err.Error(), "name is required") { + if !strings.Contains(msgs[0].Err.Error(), "chart name must be between 1 and 250 characters") { t.Errorf("Unexpected message 0: %s", msgs[0].Err) } diff --git a/pkg/chart/v2/util/create.go b/pkg/chart/v2/util/create.go index bf572c707..98e362c8d 100644 --- a/pkg/chart/v2/util/create.go +++ b/pkg/chart/v2/util/create.go @@ -32,10 +32,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. @@ -827,7 +824,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/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,