Merge pull request #3492 from aledbf/fix-units

Fix data size validations
kubernetes · Dec 2, 2018 · ccd7b89 · ccd7b89
2 parents 3112e12 + 6098f6c
commit ccd7b89
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 57 deletions.
diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
@@ -26,7 +26,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
-	"strconv"
+	"regexp"
 	"strings"
 	text_template "text/template"
 	"time"
@@ -727,39 +727,24 @@ func buildNextUpstream(i, r interface{}) string {
 	return strings.Join(nextUpstreamCodes, " ")
 }
 
+var sizeUnitRegex = regexp.MustCompile("^[0-9]+[kKmMgG]{0,1}$")
+
+// isValidByteSize validates size units valid in nginx
+// http://nginx.org/en/docs/syntax.html
 func isValidByteSize(input interface{}) bool {
 	s, ok := input.(string)
 	if !ok {
 		glog.Errorf("expected an 'string' type but %T was returned", input)
 		return false
 	}
 
+	s = strings.TrimSpace(s)
 	if s == "" {
 		glog.V(2).Info("empty byte size, hence it will not be set")
 		return false
 	}
 
-	_, err := strconv.Atoi(s)
-	if err != nil {
-		sLowercase := strings.ToLower(s)
-
-		check := strings.TrimSuffix(sLowercase, "k")
-		_, err := strconv.Atoi(check)
-		if err == nil {
-			return true
-		}
-
-		mCheck := strings.TrimSuffix(sLowercase, "m")
-		_, err = strconv.Atoi(mCheck)
-		if err == nil {
-			return true
-		}
-
-		glog.Errorf("incorrect byte size format '%v', hence it will not be set.", s)
-		return false
-	}
-
-	return true
+	return sizeUnitRegex.MatchString(s)
 }
 
 type ingressInformation struct {

diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
@@ -558,41 +558,28 @@ func TestBuildDenyVariable(t *testing.T) {
 }
 
 func TestBuildByteSize(t *testing.T) {
-	a := isValidByteSize("1000")
-	if !a {
-		t.Errorf("Expected '%v' but returned '%v'", true, a)
-	}
-	b := isValidByteSize("1000k")
-	if !b {
-		t.Errorf("Expected '%v' but returned '%v'", true, b)
-	}
-	c := isValidByteSize("1000m")
-	if !c {
-		t.Errorf("Expected '%v' but returned '%v'", true, c)
-	}
-	d := isValidByteSize("1000km")
-	if d {
-		t.Errorf("Expected '%v' but returned '%v'", false, d)
-	}
-	e := isValidByteSize("1000mk")
-	if e {
-		t.Errorf("Expected '%v' but returned '%v'", false, e)
-	}
-	f := isValidByteSize("1000kk")
-	if f {
-		t.Errorf("Expected '%v' but returned '%v'", false, f)
-	}
-	g := isValidByteSize("1000mm")
-	if g {
-		t.Errorf("Expected '%v' but returned '%v'", false, g)
-	}
-	h := isValidByteSize(nil)
-	if h {
-		t.Errorf("Expected '%v' but returned '%v'", false, h)
-	}
-	i := isValidByteSize("")
-	if i {
-		t.Errorf("Expected '%v' but returned '%v'", false, i)
+	cases := []struct {
+		input    interface{}
+		expected bool
+	}{
+		{"1000", true},
+		{"1000k", true},
+		{"1m", true},
+		{"10g", true},
+		{" 1m ", true},
+		{"1000kk", false},
+		{"1000km", false},
+		{"1mm", false},
+		{nil, false},
+		{"", false},
+		{"    ", false},
+	}
+
+	for _, tc := range cases {
+		val := isValidByteSize(tc.input)
+		if tc.expected != val {
+			t.Errorf("Expected '%v' but returned '%v'", tc.expected, val)
+		}
 	}
 }