refactor the release name validation to be consistent across Helm

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

pkg/action/action.go

@@ -58,14 +58,15 @@ var (
 	errMissingRelease = errors.New("no release provided")
 	// errInvalidRevision indicates that an invalid release revision number was provided.
 	errInvalidRevision = errors.New("invalid release revision")
-	// errInvalidName indicates that an invalid release name was provided
-	errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 53")
 	// errPending indicates that another instance of Helm is already applying an operation on a release.
 	errPending = errors.New("another operation (install/upgrade/rollback) is in progress")
 )
 
 // ValidName is a regular expression for resource names.
 //
+// DEPRECATED: This will be removed in Helm 4, and is no longer used here. See
+// pkg/chartutil.ValidateName for the replacement.
+//
 // According to the Kubernetes help text, the regular expression it uses is:
 //
 //	[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
@@ -294,7 +295,7 @@ func (c *Configuration) Now() time.Time {
 }
 
 func (c *Configuration) releaseContent(name string, version int) (*release.Release, error) {
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, errors.Errorf("releaseContent: Release name is invalid: %s", name)
 	}
 

pkg/action/action_test.go

@@ -319,40 +319,3 @@ func TestGetVersionSet(t *testing.T) {
 		t.Error("Non-existent version is reported found.")
 	}
 }
-
-// TestValidName is a regression test for ValidName
-//
-// Kubernetes has strict naming conventions for resource names. This test represents
-// those conventions.
-//
-// See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
-//
-// NOTE: At the time of this writing, the docs above say that names cannot begin with
-// digits. However, `kubectl`'s regular expression explicit allows this, and
-// Kubernetes (at least as of 1.18) also accepts resources whose names begin with digits.
-func TestValidName(t *testing.T) {
-	names := map[string]bool{
-		"":                          false,
-		"foo":                       true,
-		"foo.bar1234baz.seventyone": true,
-		"FOO":                       false,
-		"123baz":                    true,
-		"foo.BAR.baz":               false,
-		"one-two":                   true,
-		"-two":                      false,
-		"one_two":                   false,
-		"a..b":                      false,
-		"%^&#$%*@^*@&#^":            false,
-		"example:com":               false,
-		"example%%com":              false,
-	}
-	for input, expectPass := range names {
-		if ValidName.MatchString(input) != expectPass {
-			st := "fail"
-			if expectPass {
-				st = "succeed"
-			}
-			t.Errorf("Expected %q to %s", input, st)
-		}
-	}
-}

pkg/action/history.go

@@ -19,6 +19,7 @@ package action
 import (
 	"github.com/pkg/errors"
 
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
 )
 
@@ -45,7 +46,7 @@ func (h *History) Run(name string) ([]*release.Release, error) {
 		return nil, err
 	}
 
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, errors.Errorf("release name is invalid: %s", name)
 	}
 

pkg/action/release_testing.go

@@ -25,6 +25,7 @@ import (
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
 )
 
@@ -51,7 +52,7 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) {
 		return nil, err
 	}
 
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, errors.Errorf("releaseTest: Release name is invalid: %s", name)
 	}
 

pkg/action/rollback.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/pkg/errors"
 
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
 	helmtime "helm.sh/helm/v3/pkg/time"
 )
@@ -90,7 +91,7 @@ func (r *Rollback) Run(name string) error {
 // prepareRollback finds the previous release and prepares a new release object with
 // the previous release's configuration
 func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Release, error) {
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, nil, errors.Errorf("prepareRollback: Release name is invalid: %s", name)
 	}
 

pkg/action/uninstall.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/pkg/errors"
 
+	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/releaseutil"
 	helmtime "helm.sh/helm/v3/pkg/time"
@@ -62,7 +63,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error)
 		return &release.UninstallReleaseResponse{Release: r}, nil
 	}
 
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, errors.Errorf("uninstall: Release name is invalid: %s", name)
 	}
 

pkg/action/upgrade.go

@@ -115,7 +115,7 @@ func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface
 	// the user doesn't have to specify both
 	u.Wait = u.Wait || u.Atomic
 
-	if err := validateReleaseName(name); err != nil {
+	if err := chartutil.ValidateReleaseName(name); err != nil {
 		return nil, errors.Errorf("release name is invalid: %s", name)
 	}
 	u.cfg.Log("preparing upgrade for %s", name)
@@ -142,19 +142,6 @@ func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface
 	return res, nil
 }
 
-func validateReleaseName(releaseName string) error {
-	if releaseName == "" {
-		return errMissingRelease
-	}
-
-	// Check length first, since that is a less expensive operation.
-	if len(releaseName) > releaseNameMaxLen || !ValidName.MatchString(releaseName) {
-		return errInvalidName
-	}
-
-	return nil
-}
-
 // prepareUpgrade builds an upgraded release for an upgrade operation.
 func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, *release.Release, error) {
 	if chart == nil {

pkg/chartutil/validate_name.go

@@ -0,0 +1,99 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package chartutil
+
+import (
+	"regexp"
+
+	"github.com/pkg/errors"
+)
+
+// validName is a regular expression for resource names.
+//
+// According to the Kubernetes help text, the regular expression it uses is:
+//
+//	[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
+//
+// This follows the above regular expression (but requires a full string match, not partial).
+//
+// The Kubernetes documentation is here, though it is not entirely correct:
+// 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])?)*$`)
+
+var (
+	// errMissingName indicates that a release (name) was not provided.
+	errMissingName = errors.New("no name provided")
+
+	// errInvalidName indicates that an invalid release name was provided
+	errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 53")
+
+	// errInvalidKubernetesName indicates that the name does not meet the Kubernetes
+	// restrictions on metadata names.
+	errInvalidKubernetesName = errors.New("invalid metadata name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 253")
+)
+
+const (
+	// maxNameLen is the maximum length Helm allows for a release name
+	maxReleaseNameLen = 53
+	// maxMetadataNameLen is the maximum length Kubernetes allows for any name.
+	maxMetadataNameLen = 253
+)
+
+// ValidateReleaseName performs checks for an entry for a Helm release name
+//
+// For Helm to allow a name, it must be below a certain character count (53) and also match
+// a reguar expression.
+//
+// According to the Kubernetes help text, the regular expression it uses is:
+//
+//	[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
+//
+// This follows the above regular expression (but requires a full string match, not partial).
+//
+// The Kubernetes documentation is here, though it is not entirely correct:
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+func ValidateReleaseName(name string) error {
+	// This case is preserved for backwards compatibility
+	if name == "" {
+		return errMissingName
+
+	}
+	if len(name) > maxReleaseNameLen || !validName.MatchString(name) {
+		return errInvalidName
+	}
+	return nil
+}
+
+// ValidateMetadataName validates the name field of a Kubernetes metadata object.
+//
+// Empty strings, strings longer than 253 chars, or strings that don't match the regexp
+// will fail.
+//
+// According to the Kubernetes help text, the regular expression it uses is:
+//
+//	[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
+//
+// This follows the above regular expression (but requires a full string match, not partial).
+//
+// The Kubernetes documentation is here, though it is not entirely correct:
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+func ValidateMetadataName(name string) error {
+	if name == "" || len(name) > maxMetadataNameLen || !validName.MatchString(name) {
+		return errInvalidKubernetesName
+	}
+	return nil
+}

pkg/chartutil/validate_name_test.go

@@ -0,0 +1,91 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package chartutil
+
+import "testing"
+
+// TestValidateName is a regression test for ValidateName
+//
+// Kubernetes has strict naming conventions for resource names. This test represents
+// those conventions.
+//
+// See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+//
+// NOTE: At the time of this writing, the docs above say that names cannot begin with
+// digits. However, `kubectl`'s regular expression explicit allows this, and
+// Kubernetes (at least as of 1.18) also accepts resources whose names begin with digits.
+func TestValidateReleaseName(t *testing.T) {
+	names := map[string]bool{
+		"":                          false,
+		"foo":                       true,
+		"foo.bar1234baz.seventyone": true,
+		"FOO":                       false,
+		"123baz":                    true,
+		"foo.BAR.baz":               false,
+		"one-two":                   true,
+		"-two":                      false,
+		"one_two":                   false,
+		"a..b":                      false,
+		"%^&#$%*@^*@&#^":            false,
+		"example:com":               false,
+		"example%%com":              false,
+		"a1111111111111111111111111111111111111111111111111111111111z": false,
+	}
+	for input, expectPass := range names {
+		if err := ValidateReleaseName(input); (err == nil) != expectPass {
+			st := "fail"
+			if expectPass {
+				st = "succeed"
+			}
+			t.Errorf("Expected %q to %s", input, st)
+		}
+	}
+}
+
+func TestValidateMetadataName(t *testing.T) {
+	names := map[string]bool{
+		"":                          false,
+		"foo":                       true,
+		"foo.bar1234baz.seventyone": true,
+		"FOO":                       false,
+		"123baz":                    true,
+		"foo.BAR.baz":               false,
+		"one-two":                   true,
+		"-two":                      false,
+		"one_two":                   false,
+		"a..b":                      false,
+		"%^&#$%*@^*@&#^":            false,
+		"example:com":               false,
+		"example%%com":              false,
+		"a1111111111111111111111111111111111111111111111111111111111z": true,
+		"a1111111111111111111111111111111111111111111111111111111111z" +
+			"a1111111111111111111111111111111111111111111111111111111111z" +
+			"a1111111111111111111111111111111111111111111111111111111111z" +
+			"a1111111111111111111111111111111111111111111111111111111111z" +
+			"a1111111111111111111111111111111111111111111111111111111111z" +
+			"a1111111111111111111111111111111111111111111111111111111111z": false,
+	}
+	for input, expectPass := range names {
+		if err := ValidateMetadataName(input); (err == nil) != expectPass {
+			st := "fail"
+			if expectPass {
+				st = "succeed"
+			}
+			t.Errorf("Expected %q to %s", input, st)
+		}
+	}
+}

pkg/lint/rules/template.go

@@ -40,14 +40,6 @@ var (
 	releaseTimeSearch = regexp.MustCompile(`\.Release\.Time`)
 )
 
-// validName is a regular expression for names.
-//
-// This is different than action.ValidName. It conforms to the regular expression
-// `kubectl` says it uses, plus it disallows empty names.
-//
-// For details, see 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])?)*$`)
-
 // Templates lints the templates in the Linter.
 func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) {
 	fpath := "templates/"
@@ -199,10 +191,10 @@ func validateMetadataName(obj *K8sYamlStruct) error {
 	}
 	// This will return an error if the characters do not abide by the standard OR if the
 	// name is left empty.
-	if validName.MatchString(obj.Metadata.Name) {
+	if err := chartutil.ValidateMetadataName(obj.Metadata.Name); err != nil {
-		return nil
+		return errors.Wrapf(err, "object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
 	}
-	return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name)
+	return nil
 }
 
 func validateNoCRDHooks(manifest []byte) error {