From eb8fb41837edcfd7a87b60a8d6311588c76066d6 Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Sat, 25 Apr 2026 03:38:12 +0530 Subject: [PATCH] Align chart name validation with create Signed-off-by: Akash Kumar --- internal/chart/v3/lint/rules/chartfile.go | 9 +- .../chart/v3/lint/rules/chartfile_test.go | 4 +- internal/chart/v3/util/create.go | 19 +-- internal/chart/v3/util/create_test.go | 114 ++++++++++++++---- pkg/chart/v2/lint/rules/chartfile.go | 9 +- pkg/chart/v2/lint/rules/chartfile_test.go | 4 +- pkg/chart/v2/util/create.go | 19 +-- pkg/chart/v2/util/create_test.go | 114 ++++++++++++++---- 8 files changed, 216 insertions(+), 76 deletions(-) diff --git a/internal/chart/v3/lint/rules/chartfile.go b/internal/chart/v3/lint/rules/chartfile.go index d115cb219..963222599 100644 --- a/internal/chart/v3/lint/rules/chartfile.go +++ b/internal/chart/v3/lint/rules/chartfile.go @@ -21,7 +21,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "github.com/Masterminds/semver/v3" "github.com/asaskevich/govalidator" @@ -32,10 +31,6 @@ import ( chartutil "helm.sh/helm/v4/internal/chart/v3/util" ) -const validChartNameMessage = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" - -var validChartName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) - // Chartfile runs a set of linter rules related to Chart.yaml file func Chartfile(linter *support.Linter) { chartFileName := "Chart.yaml" @@ -125,8 +120,8 @@ func validateChartName(cf *chart.Metadata) error { if name != cf.Name { return fmt.Errorf("chart name %q is invalid", cf.Name) } - if !validChartName.MatchString(cf.Name) { - return fmt.Errorf("chart name %q is invalid; %s", cf.Name, validChartNameMessage) + if err := chartutil.ValidateChartName(cf.Name); err != nil { + return fmt.Errorf("chart name %q is invalid; %w", cf.Name, err) } return nil } diff --git a/internal/chart/v3/lint/rules/chartfile_test.go b/internal/chart/v3/lint/rules/chartfile_test.go index 0e6781c56..bdbc19bbd 100644 --- a/internal/chart/v3/lint/rules/chartfile_test.go +++ b/internal/chart/v3/lint/rules/chartfile_test.go @@ -64,6 +64,8 @@ func TestValidateChartYamlFormat(t *testing.T) { } func TestValidateChartName(t *testing.T) { + const chartNameRule = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" + tests := []struct { name string wantErr string @@ -78,7 +80,7 @@ func TestValidateChartName(t *testing.T) { }, { name: "BadChart", - wantErr: `chart name "BadChart" is invalid`, + wantErr: `chart name "BadChart" is invalid; ` + chartNameRule, }, { name: "bad.chart", diff --git a/internal/chart/v3/util/create.go b/internal/chart/v3/util/create.go index 48d2120e5..64143f8fa 100644 --- a/internal/chart/v3/util/create.go +++ b/internal/chart/v3/util/create.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "errors" "fmt" "io" "os" @@ -32,10 +33,9 @@ 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]*[a-z0-9])?$`) + +const validChartNameMessage = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" const ( // ChartfileName is the default Chart file name. @@ -650,6 +650,10 @@ var Stderr io.Writer = os.Stderr // CreateFrom creates a new chart, but scaffolds it from the src chart. func CreateFrom(chartfile *chart.Metadata, dest, src string) error { + if err := ValidateChartName(chartfile.Name); err != nil { + return err + } + schart, err := loader.Load(src) if err != nil { return fmt.Errorf("could not load %s: %w", src, err) @@ -704,7 +708,7 @@ func CreateFrom(chartfile *chart.Metadata, dest, src string) error { func Create(name, dir string) (string, error) { // Sanity-check the name of a chart so user doesn't create one that causes problems. - if err := validateChartName(name); err != nil { + if err := ValidateChartName(name); err != nil { return "", err } @@ -823,12 +827,13 @@ func writeFile(name string, content []byte) error { return os.WriteFile(name, content, 0644) } -func validateChartName(name string) error { +// ValidateChartName validates a chart name used by chart creation and linting. +func ValidateChartName(name string) error { if name == "" || len(name) > maxChartNameLength { 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 errors.New(validChartNameMessage) } return nil } diff --git a/internal/chart/v3/util/create_test.go b/internal/chart/v3/util/create_test.go index abdd52a82..eb75b8261 100644 --- a/internal/chart/v3/util/create_test.go +++ b/internal/chart/v3/util/create_test.go @@ -107,6 +107,13 @@ func TestCreateFrom(t *testing.T) { t.Errorf("File %s contains ", f) } } + + cf.Name = "Bad.Chart" + if err := CreateFrom(cf, t.TempDir(), srcdir); err == nil { + t.Fatal("CreateFrom with invalid chart name returned no error") + } else if err.Error() != validChartNameMessage { + t.Fatalf("CreateFrom returned %q, want %q", err, validChartNameMessage) + } } // TestCreate_Overwrite is a regression test for making sure that files are overwritten. @@ -145,28 +152,89 @@ func TestCreate_Overwrite(t *testing.T) { } func TestValidateChartName(t *testing.T) { - for name, shouldPass := range map[string]bool{ - "": false, - "abcdefghijklmnopqrstuvwxyz-_.": true, - "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": true, - "$hello": false, - "Hellô": false, - "he%%o": false, - "he\nllo": false, - - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": false, - } { - if err := validateChartName(name); (err != nil) == shouldPass { - t.Errorf("test for %q failed", name) - } + tests := []struct { + name string + wantErr string + }{ + { + name: "", + wantErr: "chart name must be between 1 and 250 characters", + }, + { + name: "abcdefghijklmnopqrstuvwxyz", + }, + { + name: "bad-chart-1", + }, + { + name: "bad--chart", + }, + { + name: "BadChart", + wantErr: validChartNameMessage, + }, + { + name: "bad.chart", + wantErr: validChartNameMessage, + }, + { + name: "bad_chart", + wantErr: validChartNameMessage, + }, + { + name: "-badchart", + wantErr: validChartNameMessage, + }, + { + name: "badchart-", + wantErr: validChartNameMessage, + }, + { + name: "$hello", + wantErr: validChartNameMessage, + }, + { + name: "Hellô", + wantErr: validChartNameMessage, + }, + { + name: "he%%o", + wantErr: validChartNameMessage, + }, + { + name: "he\nllo", + wantErr: validChartNameMessage, + }, + { + name: "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.", + wantErr: "chart name must be between 1 and 250 characters", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateChartName(tt.name) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("ValidateChartName(%q) returned unexpected error: %s", tt.name, err) + } + return + } + if err == nil { + t.Fatalf("ValidateChartName(%q) returned no error, want %q", tt.name, tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("ValidateChartName(%q) returned %q, want %q", tt.name, err, tt.wantErr) + } + }) } } diff --git a/pkg/chart/v2/lint/rules/chartfile.go b/pkg/chart/v2/lint/rules/chartfile.go index 9b9fb5377..919da95f9 100644 --- a/pkg/chart/v2/lint/rules/chartfile.go +++ b/pkg/chart/v2/lint/rules/chartfile.go @@ -21,7 +21,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "github.com/Masterminds/semver/v3" "github.com/asaskevich/govalidator" @@ -32,10 +31,6 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) -const validChartNameMessage = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" - -var validChartName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) - // Chartfile runs a set of linter rules related to Chart.yaml file func Chartfile(linter *support.Linter) { chartFileName := "Chart.yaml" @@ -126,8 +121,8 @@ func validateChartName(cf *chart.Metadata) error { if name != cf.Name { return fmt.Errorf("chart name %q is invalid", cf.Name) } - if !validChartName.MatchString(cf.Name) { - return fmt.Errorf("chart name %q is invalid; %s", cf.Name, validChartNameMessage) + if err := chartutil.ValidateChartName(cf.Name); err != nil { + return fmt.Errorf("chart name %q is invalid; %w", cf.Name, err) } return nil } diff --git a/pkg/chart/v2/lint/rules/chartfile_test.go b/pkg/chart/v2/lint/rules/chartfile_test.go index 51101cf70..533c8fa94 100644 --- a/pkg/chart/v2/lint/rules/chartfile_test.go +++ b/pkg/chart/v2/lint/rules/chartfile_test.go @@ -64,6 +64,8 @@ func TestValidateChartYamlFormat(t *testing.T) { } func TestValidateChartName(t *testing.T) { + const chartNameRule = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" + tests := []struct { name string wantErr string @@ -78,7 +80,7 @@ func TestValidateChartName(t *testing.T) { }, { name: "BadChart", - wantErr: `chart name "BadChart" is invalid`, + wantErr: `chart name "BadChart" is invalid; ` + chartNameRule, }, { name: "bad.chart", diff --git a/pkg/chart/v2/util/create.go b/pkg/chart/v2/util/create.go index 0d7ae8d5c..d561a04d6 100644 --- a/pkg/chart/v2/util/create.go +++ b/pkg/chart/v2/util/create.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "errors" "fmt" "io" "os" @@ -32,10 +33,9 @@ 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]*[a-z0-9])?$`) + +const validChartNameMessage = "chart names must contain lowercase letters, numbers, and dashes, and must start and end with a lowercase letter or number" const ( // ChartfileName is the default Chart file name. @@ -649,6 +649,10 @@ var Stderr io.Writer = os.Stderr // CreateFrom creates a new chart, but scaffolds it from the src chart. func CreateFrom(chartfile *chart.Metadata, dest, src string) error { + if err := ValidateChartName(chartfile.Name); err != nil { + return err + } + schart, err := loader.Load(src) if err != nil { return fmt.Errorf("could not load %s: %w", src, err) @@ -703,7 +707,7 @@ func CreateFrom(chartfile *chart.Metadata, dest, src string) error { func Create(name, dir string) (string, error) { // Sanity-check the name of a chart so user doesn't create one that causes problems. - if err := validateChartName(name); err != nil { + if err := ValidateChartName(name); err != nil { return "", err } @@ -822,12 +826,13 @@ func writeFile(name string, content []byte) error { return os.WriteFile(name, content, 0644) } -func validateChartName(name string) error { +// ValidateChartName validates a chart name used by chart creation and linting. +func ValidateChartName(name string) error { if name == "" || len(name) > maxChartNameLength { 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 errors.New(validChartNameMessage) } return nil } diff --git a/pkg/chart/v2/util/create_test.go b/pkg/chart/v2/util/create_test.go index 967972fc8..7204f0fdb 100644 --- a/pkg/chart/v2/util/create_test.go +++ b/pkg/chart/v2/util/create_test.go @@ -107,6 +107,13 @@ func TestCreateFrom(t *testing.T) { t.Errorf("File %s contains ", f) } } + + cf.Name = "Bad.Chart" + if err := CreateFrom(cf, t.TempDir(), srcdir); err == nil { + t.Fatal("CreateFrom with invalid chart name returned no error") + } else if err.Error() != validChartNameMessage { + t.Fatalf("CreateFrom returned %q, want %q", err, validChartNameMessage) + } } // TestCreate_Overwrite is a regression test for making sure that files are overwritten. @@ -145,28 +152,89 @@ func TestCreate_Overwrite(t *testing.T) { } func TestValidateChartName(t *testing.T) { - for name, shouldPass := range map[string]bool{ - "": false, - "abcdefghijklmnopqrstuvwxyz-_.": true, - "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": true, - "$hello": false, - "Hellô": false, - "he%%o": false, - "he\nllo": false, - - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "abcdefghijklmnopqrstuvwxyz-_." + - "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": false, - } { - if err := validateChartName(name); (err != nil) == shouldPass { - t.Errorf("test for %q failed", name) - } + tests := []struct { + name string + wantErr string + }{ + { + name: "", + wantErr: "chart name must be between 1 and 250 characters", + }, + { + name: "abcdefghijklmnopqrstuvwxyz", + }, + { + name: "bad-chart-1", + }, + { + name: "bad--chart", + }, + { + name: "BadChart", + wantErr: validChartNameMessage, + }, + { + name: "bad.chart", + wantErr: validChartNameMessage, + }, + { + name: "bad_chart", + wantErr: validChartNameMessage, + }, + { + name: "-badchart", + wantErr: validChartNameMessage, + }, + { + name: "badchart-", + wantErr: validChartNameMessage, + }, + { + name: "$hello", + wantErr: validChartNameMessage, + }, + { + name: "Hellô", + wantErr: validChartNameMessage, + }, + { + name: "he%%o", + wantErr: validChartNameMessage, + }, + { + name: "he\nllo", + wantErr: validChartNameMessage, + }, + { + name: "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "abcdefghijklmnopqrstuvwxyz-_." + + "ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.", + wantErr: "chart name must be between 1 and 250 characters", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateChartName(tt.name) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("ValidateChartName(%q) returned unexpected error: %s", tt.name, err) + } + return + } + if err == nil { + t.Fatalf("ValidateChartName(%q) returned no error, want %q", tt.name, tt.wantErr) + } + if err.Error() != tt.wantErr { + t.Fatalf("ValidateChartName(%q) returned %q, want %q", tt.name, err, tt.wantErr) + } + }) } }