Follow the chartname verification rule consistency when creating and all other operations

Signed-off-by: wawa0210 <xiaozhang0210@hotmail.com>

cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt

@@ -13,8 +13,14 @@
 [ERROR] : unable to load chart
 	validation: chart.metadata.name is required
 
+==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length
+[ERROR] Chart.yaml: chart name must be between 1 and 250 characters
+[ERROR] Chart.yaml: apiVersion is required. The value must be either "v1" or "v2"
+[INFO] Chart.yaml: icon is recommended
+[WARNING] templates/: directory not found
+
 ==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart
 [INFO] Chart.yaml: icon is recommended
 [WARNING] templates/: directory not found
 
-Error: 3 chart(s) linted, 2 chart(s) failed
+Error: 4 chart(s) linted, 3 chart(s) failed

cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/Chart.yaml

@@ -0,0 +1,3 @@
+name: EaXsmQUlBJAzyFiVRNvITYZEnANrNRBOZMvCpWgsZmlEjBfNVSXpQRlBCGyzfwPWQIaRVZLBqxXwJrIFMTtzKgeWwmdLurDeoyDEaXsmQUlBJAzyFiVRNvITYZEnANrNRBOZMvCpWgsZmlEjBfNVSXpQRlBCGyzfwPWQIaRVZLBqxXwJrIFMTtzKgeWwmdLurDeoyDEaXsmQUlBJAzyFiVRNvITYZEnANrNRBOZMvCpWgsZmlEjBfNVSXpQRlBCGyzfwPWQIaRVZLBqxXwJrIFMTtzKgeWwmdLurDeoyDEaXsmQUlBJAzyFiVRNvITYZEnANrNRBOZMvCpWgsZmlEjBfNVSXpQRlBCGyzfwPWQIaRVZLBqxXwJrIFMTtzKgeWwmdLurDeoyDEaXsmQUlBJAzyFiVRNvITYZEnANrNRBOZMvCpWgsZmlEjBfNVSXpQRlBCGyzfwPWQIaRVZLBqxXwJrIFMTtzKgeWwmdLurDeoyD
+description: Bad chart whose name length exceeds the maximum length
+version: 0.0.1

pkg/chartutil/create.go

@@ -22,7 +22,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -32,12 +31,6 @@ import (
 	"helm.sh/helm/v3/pkg/chart/loader"
 )
 
-// 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._-]+$")
-
 const (
 	// ChartfileName is the default Chart file name.
 	ChartfileName = "Chart.yaml"
@@ -71,10 +64,6 @@ const (
 	TestConnectionName = TemplatesTestsDir + sep + "test-connection.yaml"
 )
 
-// maxChartNameLength is lower than the limits we know of with certain file systems,
-// and with certain Kubernetes fields.
-const maxChartNameLength = 250
-
 const sep = string(filepath.Separator)
 
 const defaultChartfile = `apiVersion: v2
@@ -547,7 +536,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
 	}
 
@@ -656,13 +645,3 @@ func writeFile(name string, content []byte) error {
 	}
 	return ioutil.WriteFile(name, content, 0644)
 }
-
-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 nil
-}

pkg/chartutil/create_test.go

@@ -156,30 +156,3 @@ func TestCreate_Overwrite(t *testing.T) {
 		t.Errorf("Expected warnings about overwriting files.")
 	}
 }
-
-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)
-		}
-	}
-}

pkg/chartutil/validate_name.go

@@ -35,6 +35,12 @@ import (
 // https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
 var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
 
+// validchartName 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 validchartName = regexp.MustCompile("^[a-zA-Z0-9._-]+$")
+
 var (
 	// errMissingName indicates that a release (name) was not provided.
 	errMissingName = errors.New("no name provided")
@@ -56,6 +62,10 @@ const (
 	maxReleaseNameLen = 53
 	// maxMetadataNameLen is the maximum length Kubernetes allows for any name.
 	maxMetadataNameLen = 253
+
+	// maxChartNameLength is lower than the limits we know of with certain file systems,
+	// and with certain Kubernetes fields.
+	maxChartNameLength = 250
 )
 
 // ValidateReleaseName performs checks for an entry for a Helm release name
@@ -102,3 +112,26 @@ func ValidateMetadataName(name string) error {
 	}
 	return nil
 }
+
+// ValidateChartName validates the name of helm charts
+//
+// Empty strings, strings longer than 250 chars, or strings that don't match the regexp
+// will fail.
+//
+// According to the Kubernetes help text, the regular expression it uses is:
+//
+//	[a-zA-Z0-9._-]+$
+//
+// This follows the above regular expression (but requires a full string match, not partial).
+func ValidateChartName(name string) error {
+	if name == "" {
+		return fmt.Errorf("name is required")
+	}
+	if name == "" || len(name) > maxChartNameLength {
+		return fmt.Errorf("chart name must be between 1 and %d characters", maxChartNameLength)
+	}
+	if !validchartName.MatchString(name) {
+		return fmt.Errorf("chart name must match the regular expression %q", validchartName.String())
+	}
+	return nil
+}

pkg/chartutil/validate_name_test.go

@@ -89,3 +89,30 @@ func TestValidateMetadataName(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)
+		}
+	}
+}

pkg/lint/rules/chartfile.go

@@ -51,7 +51,7 @@ func Chartfile(linter *support.Linter) {
 	// errors would already be caught in the above load function
 	chartFileForTypeCheck, _ := loadChartFileForTypeCheck(chartPath)
 
-	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartName(chartFile))
+	linter.RunLinterRule(support.ErrorSev, chartFileName, chartutil.ValidateChartName(chartFile.Name))
 
 	// Chart metadata
 	linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartAPIVersion(chartFile))
@@ -103,13 +103,6 @@ func validateChartYamlFormat(chartFileError error) error {
 	return nil
 }
 
-func validateChartName(cf *chart.Metadata) error {
-	if cf.Name == "" {
-		return errors.New("name is required")
-	}
-	return nil
-}
-
 func validateChartAPIVersion(cf *chart.Metadata) error {
 	if cf.APIVersion == "" {
 		return errors.New("apiVersion is required. The value must be either \"v1\" or \"v2\"")

pkg/lint/rules/chartfile_test.go

@@ -65,7 +65,7 @@ func TestValidateChartYamlFormat(t *testing.T) {
 }
 
 func TestValidateChartName(t *testing.T) {
-	err := validateChartName(badChart)
+	err := chartutil.ValidateChartName(badChart.Name)
 	if err == nil {
 		t.Errorf("validateChartName to return a linter error, got no error")
 	}