diff --git a/internal/chart/v3/lint/rules/chartfile.go b/internal/chart/v3/lint/rules/chartfile.go index 29991a8d5..d115cb219 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,10 @@ 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" @@ -120,6 +125,9 @@ 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) + } return nil } diff --git a/internal/chart/v3/lint/rules/chartfile_test.go b/internal/chart/v3/lint/rules/chartfile_test.go index a7669a0aa..0e6781c56 100644 --- a/internal/chart/v3/lint/rules/chartfile_test.go +++ b/internal/chart/v3/lint/rules/chartfile_test.go @@ -29,19 +29,16 @@ import ( ) const ( - badChartNameDir = "testdata/badchartname" badChartDir = "testdata/badchartfile" anotherBadChartDir = "testdata/anotherbadchartfile" ) var ( - badChartNamePath = filepath.Join(badChartNameDir, "Chart.yaml") badChartFilePath = filepath.Join(badChartDir, "Chart.yaml") nonExistingChartFilePath = filepath.Join(os.TempDir(), "Chart.yaml") ) var badChart, _ = chartutil.LoadChartfile(badChartFilePath) -var badChartName, _ = chartutil.LoadChartfile(badChartNamePath) // Validation functions Test func TestValidateChartYamlNotDirectory(t *testing.T) { @@ -67,14 +64,65 @@ func TestValidateChartYamlFormat(t *testing.T) { } func TestValidateChartName(t *testing.T) { - err := validateChartName(badChart) - if err == nil { - t.Error("validateChartName to return a linter error, got no error") + tests := []struct { + name string + wantErr string + }{ + { + name: "", + wantErr: "name is required", + }, + { + name: "../badchartname", + wantErr: `chart name "../badchartname" is invalid`, + }, + { + name: "BadChart", + wantErr: `chart name "BadChart" is invalid`, + }, + { + name: "bad.chart", + wantErr: `chart name "bad.chart" is invalid`, + }, + { + name: "bad_chart", + wantErr: `chart name "bad_chart" is invalid`, + }, + { + name: "-badchart", + wantErr: `chart name "-badchart" is invalid`, + }, + { + name: "badchart-", + wantErr: `chart name "badchart-" is invalid`, + }, + { + name: "badchart", + }, + { + name: "bad-chart-1", + }, + { + name: "bad--chart", + }, } - err = validateChartName(badChartName) - if err == nil { - t.Error("expected validateChartName to return a linter error for an invalid name, got no error") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateChartName(&chart.Metadata{Name: 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 !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("validateChartName(%q) returned %q, want to contain %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 f8f609280..9b9fb5377 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,10 @@ 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" @@ -121,6 +126,9 @@ 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) + } return nil } diff --git a/pkg/chart/v2/lint/rules/chartfile_test.go b/pkg/chart/v2/lint/rules/chartfile_test.go index c9e202770..51101cf70 100644 --- a/pkg/chart/v2/lint/rules/chartfile_test.go +++ b/pkg/chart/v2/lint/rules/chartfile_test.go @@ -29,19 +29,16 @@ import ( ) const ( - badChartNameDir = "testdata/badchartname" badChartDir = "testdata/badchartfile" anotherBadChartDir = "testdata/anotherbadchartfile" ) var ( - badChartNamePath = filepath.Join(badChartNameDir, "Chart.yaml") badChartFilePath = filepath.Join(badChartDir, "Chart.yaml") nonExistingChartFilePath = filepath.Join(os.TempDir(), "Chart.yaml") ) var badChart, _ = chartutil.LoadChartfile(badChartFilePath) -var badChartName, _ = chartutil.LoadChartfile(badChartNamePath) // Validation functions Test func TestValidateChartYamlNotDirectory(t *testing.T) { @@ -67,14 +64,65 @@ func TestValidateChartYamlFormat(t *testing.T) { } func TestValidateChartName(t *testing.T) { - err := validateChartName(badChart) - if err == nil { - t.Error("validateChartName to return a linter error, got no error") + tests := []struct { + name string + wantErr string + }{ + { + name: "", + wantErr: "name is required", + }, + { + name: "../badchartname", + wantErr: `chart name "../badchartname" is invalid`, + }, + { + name: "BadChart", + wantErr: `chart name "BadChart" is invalid`, + }, + { + name: "bad.chart", + wantErr: `chart name "bad.chart" is invalid`, + }, + { + name: "bad_chart", + wantErr: `chart name "bad_chart" is invalid`, + }, + { + name: "-badchart", + wantErr: `chart name "-badchart" is invalid`, + }, + { + name: "badchart-", + wantErr: `chart name "badchart-" is invalid`, + }, + { + name: "badchart", + }, + { + name: "bad-chart-1", + }, + { + name: "bad--chart", + }, } - err = validateChartName(badChartName) - if err == nil { - t.Error("expected validateChartName to return a linter error for an invalid name, got no error") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateChartName(&chart.Metadata{Name: 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 !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("validateChartName(%q) returned %q, want to contain %q", tt.name, err, tt.wantErr) + } + }) } }