From 0f953403a2dbcc58375278e4743cead269e1bc53 Mon Sep 17 00:00:00 2001
From: "Y.W" <devinyan@tencent.com>
Date: Fri, 23 Jun 2017 00:05:29 +0800
Subject: [PATCH] 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 | 5 +++--
pkg/tiller/release_history.go | 5 +++++
pkg/tiller/release_rollback.go | 10 ++++++----
pkg/tiller/release_server.go | 14 ++++++++++++++
pkg/tiller/release_server_test.go | 31 ++++++++++++++++---------------
pkg/tiller/release_status.go | 5 +++--
pkg/tiller/release_testing.go | 5 +++--
pkg/tiller/release_uninstall.go | 10 +++-------
pkg/tiller/release_update.go | 9 +++++----
9 files changed, 58 insertions(+), 36 deletions(-)
diff --git a/pkg/tiller/release_content.go b/pkg/tiller/release_content.go
index e9f6f05c9..fd783d6b6 100644
--- a/pkg/tiller/release_content.go
+++ b/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) {
- return nil, errMissingRelease
+ if err := validateReleaseName(req.Name); err != nil {
+ s.Log("releaseContent: Release name is invalid: %s", req.Name)
+ return nil, err
}
if req.Version <= 0 {
diff --git a/pkg/tiller/release_history.go b/pkg/tiller/release_history.go
index 09131fad8..0dd525978 100644
--- a/pkg/tiller/release_history.go
+++ b/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 {
diff --git a/pkg/tiller/release_rollback.go b/pkg/tiller/release_rollback.go
index 9a313ead5..02c76f9b0 100644
--- a/pkg/tiller/release_rollback.go
+++ b/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 {
- case !ValidName.MatchString(req.Name):
- return nil, nil, errMissingRelease
- case req.Version < 0:
+ if err := validateReleaseName(req.Name); err != nil {
+ s.Log("prepareRollback: Release name is invalid: %s", req.Name)
+ return nil, nil, err
+ }
+
+ if req.Version < 0 {
return nil, nil, errInvalidRevision
}
diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go
index 07ea872df..db7c0b568 100644
--- a/pkg/tiller/release_server.go
+++ b/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
+}
diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go
index 8425a58f5..b7b14a4f1 100644
--- a/pkg/tiller/release_server_test.go
+++ b/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{
- "nina pinta santa-maria": false,
- "nina-pinta-santa-maria": true,
- "-nina": false,
- "pinta-": false,
- "santa-maria": true,
- "niГ±a": false,
- "...": false,
- "pinta...": false,
- "santa...maria": true,
- "": false,
- " ": false,
- ".nina.": false,
- "nina.pinta": true,
+ for name, valid := range map[string]error{
+ "nina pinta santa-maria": errInvalidName,
+ "nina-pinta-santa-maria": nil,
+ "-nina": errInvalidName,
+ "pinta-": errInvalidName,
+ "santa-maria": nil,
+ "niГ±a": errInvalidName,
+ "...": errInvalidName,
+ "pinta...": errInvalidName,
+ "santa...maria": nil,
+ "": errMissingRelease,
+ " ": errInvalidName,
+ ".nina.": errInvalidName,
+ "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)
}
}
diff --git a/pkg/tiller/release_status.go b/pkg/tiller/release_status.go
index b39bdab98..e0d75877d 100644
--- a/pkg/tiller/release_status.go
+++ b/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) {
- return nil, errMissingRelease
+ if err := validateReleaseName(req.Name); err != nil {
+ s.Log("getStatus: Release name is invalid: %s", req.Name)
+ return nil, err
}
var rel *release.Release
diff --git a/pkg/tiller/release_testing.go b/pkg/tiller/release_testing.go
index 4f9a38a96..a44b67e6f 100644
--- a/pkg/tiller/release_testing.go
+++ b/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) {
- return errMissingRelease
+ if err := validateReleaseName(req.Name); err != nil {
+ s.Log("releaseTest: Release name is invalid: %s", req.Name)
+ return err
}
// finds the non-deleted release with the given name
diff --git a/pkg/tiller/release_uninstall.go b/pkg/tiller/release_uninstall.go
index e98488425..5467c7a50 100644
--- a/pkg/tiller/release_uninstall.go
+++ b/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) {
- s.Log("uninstall: Release not found: %s", req.Name)
- return nil, errMissingRelease
- }
-
- if len(req.Name) > releaseNameMaxLen {
- return nil, fmt.Errorf("release name %q exceeds max length of %d", req.Name, releaseNameMaxLen)
+ if err := validateReleaseName(req.Name); err != nil {
+ s.Log("uninstallRelease: Release name is invalid: %s", req.Name)
+ return nil, err
}
err := s.env.Releases.LockRelease(req.Name)
diff --git a/pkg/tiller/release_update.go b/pkg/tiller/release_update.go
index b9aaa17c5..2e68905f6 100644
--- a/pkg/tiller/release_update.go
+++ b/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
}
--
GitLab