From bb208a02b3ce15305d6b5ad005e418a9dce623fd Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Thu, 28 Jan 2016 23:22:57 -0800
Subject: [PATCH] Make IsValidPercent return error strings

---
 pkg/apis/extensions/validation/validation.go  |  9 +++--
 .../extensions/validation/validation_test.go  |  2 +-
 pkg/util/validation/validation.go             |  7 ++--
 pkg/util/validation/validation_test.go        | 36 +++++++++++++++++++
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go
index d5e25665394..c13d747c8d0 100644
--- a/pkg/apis/extensions/validation/validation.go
+++ b/pkg/apis/extensions/validation/validation.go
@@ -149,8 +149,8 @@ func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *fiel
 	allErrs := field.ErrorList{}
 	switch intOrPercent.Type {
 	case intstr.String:
-		if !validation.IsValidPercent(intOrPercent.StrVal) {
-			allErrs = append(allErrs, field.Invalid(fldPath, intOrPercent, "must be an integer or percentage (e.g '5%%')"))
+		for _, msg := range validation.IsValidPercent(intOrPercent.StrVal) {
+			allErrs = append(allErrs, field.Invalid(fldPath, intOrPercent, msg))
 		}
 	case intstr.Int:
 		allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(intOrPercent.IntValue()), fldPath)...)
@@ -161,7 +161,10 @@ func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *fiel
 }
 
 func getPercentValue(intOrStringValue intstr.IntOrString) (int, bool) {
-	if intOrStringValue.Type != intstr.String || !validation.IsValidPercent(intOrStringValue.StrVal) {
+	if intOrStringValue.Type != intstr.String {
+		return 0, false
+	}
+	if len(validation.IsValidPercent(intOrStringValue.StrVal)) != 0 {
 		return 0, false
 	}
 	value, _ := strconv.Atoi(intOrStringValue.StrVal[:len(intOrStringValue.StrVal)-1])
diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go
index 8c0cc83865c..6dc72c8e355 100644
--- a/pkg/apis/extensions/validation/validation_test.go
+++ b/pkg/apis/extensions/validation/validation_test.go
@@ -632,7 +632,7 @@ func TestValidateDeployment(t *testing.T) {
 			MaxSurge: intstr.FromString("20Percent"),
 		},
 	}
-	errorCases["must be an integer or percentage"] = invalidMaxSurgeDeployment
+	errorCases["must match the regex"] = invalidMaxSurgeDeployment
 
 	// MaxSurge and MaxUnavailable cannot both be zero.
 	invalidRollingUpdateDeployment := validDeployment()
diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go
index 72a132bc50a..ad7b1ff2998 100644
--- a/pkg/util/validation/validation.go
+++ b/pkg/util/validation/validation.go
@@ -221,8 +221,11 @@ const percentFmt string = "[0-9]+%"
 
 var percentRegexp = regexp.MustCompile("^" + percentFmt + "$")
 
-func IsValidPercent(percent string) bool {
-	return percentRegexp.MatchString(percent)
+func IsValidPercent(percent string) []string {
+	if !percentRegexp.MatchString(percent) {
+		return []string{RegexError(percentFmt, "1%", "93%")}
+	}
+	return nil
 }
 
 const HTTPHeaderNameFmt string = "[-A-Za-z0-9]+"
diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go
index b9bcd1b7684..741df458775 100644
--- a/pkg/util/validation/validation_test.go
+++ b/pkg/util/validation/validation_test.go
@@ -338,3 +338,39 @@ func TestIsHTTPHeaderName(t *testing.T) {
 		}
 	}
 }
+
+func TestIsValidPercent(t *testing.T) {
+	goodValues := []string{
+		"0%",
+		"00000%",
+		"1%",
+		"01%",
+		"99%",
+		"100%",
+		"101%",
+	}
+	for _, val := range goodValues {
+		if msgs := IsValidPercent(val); len(msgs) != 0 {
+			t.Errorf("expected true for %q: %v", val, msgs)
+		}
+	}
+
+	badValues := []string{
+		"",
+		"0",
+		"100",
+		"0.0%",
+		"99.9%",
+		"hundred",
+		" 1%",
+		"1% ",
+		"-0%",
+		"-1%",
+		"+1%",
+	}
+	for _, val := range badValues {
+		if msgs := IsValidPercent(val); len(msgs) == 0 {
+			t.Errorf("expected false for %q", val)
+		}
+	}
+}