give an uniform check for release process (#2565)

* give an uniform check for release process

* fixed as the review of adamreese: update the err message when releasename is empty and update the test units.

* fixed as the review of bacongobbler: add more detail information to return message. the regex rule is added to the return message.

pkg/tiller/release_content.go

@@ -24,8 +24,9 @@ import (
 
 // GetReleaseContent gets all of the stored information for the given release.
 func (s *ReleaseServer) GetReleaseContent(c ctx.Context, req *services.GetReleaseContentRequest) (*services.GetReleaseContentResponse, error) {
-	if !ValidName.MatchString(req.Name) {
+	if err := validateReleaseName(req.Name); err != nil {
-		return nil, errMissingRelease
+		s.Log("releaseContent: Release name is invalid: %s", req.Name)
+		return nil, err
 	}
 
 	if req.Version <= 0 {

pkg/tiller/release_history.go

@@ -25,6 +25,11 @@ import (
 
 // GetHistory gets the history for a given release.
 func (s *ReleaseServer) GetHistory(ctx context.Context, req *tpb.GetHistoryRequest) (*tpb.GetHistoryResponse, error) {
+	if err := validateReleaseName(req.Name); err != nil {
+		s.Log("getHistory: Release name is invalid: %s", req.Name)
+		return nil, err
+	}
+
 	s.Log("getting history for release %s", req.Name)
 	h, err := s.env.Releases.History(req.Name)
 	if err != nil {

pkg/tiller/release_rollback.go

@@ -60,10 +60,12 @@ func (s *ReleaseServer) RollbackRelease(c ctx.Context, req *services.RollbackRel
 // prepareRollback finds the previous release and prepares a new release object with
 // the previous release's configuration
 func (s *ReleaseServer) prepareRollback(req *services.RollbackReleaseRequest) (*release.Release, *release.Release, error) {
-	switch {
+	if err := validateReleaseName(req.Name); err != nil {
-	case !ValidName.MatchString(req.Name):
+		s.Log("prepareRollback: Release name is invalid: %s", req.Name)
-		return nil, nil, errMissingRelease
+		return nil, nil, err
-	case req.Version < 0:
+	}
+
+	if req.Version < 0 {
 		return nil, nil, errInvalidRevision
 	}
 

pkg/tiller/release_server.go

@@ -59,6 +59,8 @@ 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")
 )
 
 // ListDefaultLimit is the default limit for number of items returned in a list.
@@ -359,3 +361,15 @@ func validateManifest(c environment.KubeClient, ns string, manifest []byte) erro
 	_, err := c.BuildUnstructured(ns, r)
 	return err
 }
+
+func validateReleaseName(releaseName string) error {
+	if releaseName == "" {
+		return errMissingRelease
+	}
+
+	if !ValidName.MatchString(releaseName) || (len(releaseName) > releaseNameMaxLen) {
+		return errInvalidName
+	}
+
+	return nil
+}

pkg/tiller/release_server_test.go

@@ -183,22 +183,23 @@ func upgradeReleaseVersion(rel *release.Release) *release.Release {
 }
 
 func TestValidName(t *testing.T) {
-	for name, valid := range map[string]bool{
+	for name, valid := range map[string]error{
-		"nina pinta santa-maria": false,
+		"nina pinta santa-maria": errInvalidName,
-		"nina-pinta-santa-maria": true,
+		"nina-pinta-santa-maria": nil,
-		"-nina":                  false,
+		"-nina":                  errInvalidName,
-		"pinta-":                 false,
+		"pinta-":                 errInvalidName,
-		"santa-maria":            true,
+		"santa-maria":            nil,
-		"niña":                   false,
+		"niña":                   errInvalidName,
-		"...":                    false,
+		"...":                    errInvalidName,
-		"pinta...":               false,
+		"pinta...":               errInvalidName,
-		"santa...maria":          true,
+		"santa...maria":          nil,
-		"":                       false,
+		"":                       errMissingRelease,
-		" ":                      false,
+		" ":                      errInvalidName,
-		".nina.":                 false,
+		".nina.":                 errInvalidName,
-		"nina.pinta":             true,
+		"nina.pinta":             nil,
+		"abcdefghi-abcdefghi-abcdefghi-abcdefghi-abcdefghi-abcd": errInvalidName,
 	} {
-		if valid != ValidName.MatchString(name) {
+		if valid != validateReleaseName(name) {
 			t.Errorf("Expected %q to be %t", name, valid)
 		}
 	}

pkg/tiller/release_status.go

@@ -28,8 +28,9 @@ import (
 
 // GetReleaseStatus gets the status information for a named release.
 func (s *ReleaseServer) GetReleaseStatus(c ctx.Context, req *services.GetReleaseStatusRequest) (*services.GetReleaseStatusResponse, error) {
-	if !ValidName.MatchString(req.Name) {
+	if err := validateReleaseName(req.Name); err != nil {
-		return nil, errMissingRelease
+		s.Log("getStatus: Release name is invalid: %s", req.Name)
+		return nil, err
 	}
 
 	var rel *release.Release

pkg/tiller/release_testing.go

@@ -25,8 +25,9 @@ import (
 // RunReleaseTest runs pre-defined tests stored as hooks on a given release
 func (s *ReleaseServer) RunReleaseTest(req *services.TestReleaseRequest, stream services.ReleaseService_RunReleaseTestServer) error {
 
-	if !ValidName.MatchString(req.Name) {
+	if err := validateReleaseName(req.Name); err != nil {
-		return errMissingRelease
+		s.Log("releaseTest: Release name is invalid: %s", req.Name)
+		return err
 	}
 
 	// finds the non-deleted release with the given name

pkg/tiller/release_uninstall.go

@@ -31,13 +31,9 @@ import (
 
 // UninstallRelease deletes all of the resources associated with this release, and marks the release DELETED.
 func (s *ReleaseServer) UninstallRelease(c ctx.Context, req *services.UninstallReleaseRequest) (*services.UninstallReleaseResponse, error) {
-	if !ValidName.MatchString(req.Name) {
+	if err := validateReleaseName(req.Name); err != nil {
-		s.Log("uninstall: Release not found: %s", req.Name)
+		s.Log("uninstallRelease: Release name is invalid: %s", req.Name)
-		return nil, errMissingRelease
+		return nil, err
-	}
-
-	if len(req.Name) > releaseNameMaxLen {
-		return nil, fmt.Errorf("release name %q exceeds max length of %d", req.Name, releaseNameMaxLen)
 	}
 
 	err := s.env.Releases.LockRelease(req.Name)

pkg/tiller/release_update.go

@@ -30,6 +30,11 @@ import (
 
 // UpdateRelease takes an existing release and new information, and upgrades the release.
 func (s *ReleaseServer) UpdateRelease(c ctx.Context, req *services.UpdateReleaseRequest) (*services.UpdateReleaseResponse, error) {
+	if err := validateReleaseName(req.Name); err != nil {
+		s.Log("updateRelease: Release name is invalid: %s", req.Name)
+		return nil, err
+	}
+
 	err := s.env.Releases.LockRelease(req.Name)
 	if err != nil {
 		return nil, err
@@ -60,10 +65,6 @@ func (s *ReleaseServer) UpdateRelease(c ctx.Context, req *services.UpdateRelease
 
 // prepareUpdate builds an updated release for an update operation.
 func (s *ReleaseServer) prepareUpdate(req *services.UpdateReleaseRequest) (*release.Release, *release.Release, error) {
-	if !ValidName.MatchString(req.Name) {
-		return nil, nil, errMissingRelease
-	}
-
 	if req.Chart == nil {
 		return nil, nil, errMissingChart
 	}