From 899726d5a403bf0a1caaee76f515e3441a7f8522 Mon Sep 17 00:00:00 2001 From: wawa0210 Date: Wed, 6 Jan 2021 10:27:27 +0800 Subject: [PATCH] Follow the chartname verification rule consistency when creating and all other operations Signed-off-by: wawa0210 --- ...hart-with-bad-subcharts-with-subcharts.txt | 8 ++++- .../bad-subchart-name-length/Chart.yaml | 3 ++ .../bad-subchart-name-length/values.yaml | 0 pkg/chartutil/create.go | 23 +------------ pkg/chartutil/create_test.go | 27 --------------- pkg/chartutil/validate_name.go | 33 +++++++++++++++++++ pkg/chartutil/validate_name_test.go | 27 +++++++++++++++ pkg/lint/rules/chartfile.go | 9 +---- pkg/lint/rules/chartfile_test.go | 2 +- 9 files changed, 73 insertions(+), 59 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/values.yaml diff --git a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt b/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt index e77aa387f..3b550abdb 100644 --- a/cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt +++ b/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 diff --git a/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/Chart.yaml new file mode 100644 index 000000000..74c151b7f --- /dev/null +++ b/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 diff --git a/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/values.yaml b/cmd/helm/testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart-name-length/values.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go index ee6bf7721..14d926773 100644 --- a/pkg/chartutil/create.go +++ b/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 -} diff --git a/pkg/chartutil/create_test.go b/pkg/chartutil/create_test.go index 9a473fc59..49dcde633 100644 --- a/pkg/chartutil/create_test.go +++ b/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) - } - } -} diff --git a/pkg/chartutil/validate_name.go b/pkg/chartutil/validate_name.go index 913a477cf..fae3c3412 100644 --- a/pkg/chartutil/validate_name.go +++ b/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 +} diff --git a/pkg/chartutil/validate_name_test.go b/pkg/chartutil/validate_name_test.go index 5f0792f94..dcd1a24bc 100644 --- a/pkg/chartutil/validate_name_test.go +++ b/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) + } + } +} diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index b49f2cec0..8c859b93a 100644 --- a/pkg/lint/rules/chartfile.go +++ b/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\"") diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 087cda047..86bc621f8 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/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") }