Ingress annotation validation earlier in k8s lifecycle

internal/configs/annotations.go

@@ -93,29 +93,21 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
 		if err != nil {
 			glog.Error(err)
 		}
-		if isPlus {
-			cfgParams.HealthCheckEnabled = healthCheckEnabled
-		} else {
-			glog.Warning("Annotation 'nginx.com/health-checks' requires NGINX Plus")
-		}
+		cfgParams.HealthCheckEnabled = healthCheckEnabled
 	}
 
-	if cfgParams.HealthCheckEnabled {
-		if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists {
-			if err != nil {
-				glog.Error(err)
-			}
-			cfgParams.HealthCheckMandatory = healthCheckMandatory
+	if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists {
+		if err != nil {
+			glog.Error(err)
 		}
+		cfgParams.HealthCheckMandatory = healthCheckMandatory
 	}
 
-	if cfgParams.HealthCheckMandatory {
-		if healthCheckQueue, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
-			if err != nil {
-				glog.Error(err)
-			}
-			cfgParams.HealthCheckMandatoryQueue = healthCheckQueue
+	if healthCheckQueue, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
+		if err != nil {
+			glog.Error(err)
 		}
+		cfgParams.HealthCheckMandatoryQueue = healthCheckQueue
 	}
 
 	if slowStart, exists := ingEx.Ingress.Annotations["nginx.com/slow-start"]; exists {

internal/configs/parsing_helpers.go

@@ -21,7 +21,7 @@ type apiObject interface {
 // GetMapKeyAsBool searches the map for the given key and parses the key as bool.
 func GetMapKeyAsBool(m map[string]string, key string, context apiObject) (bool, bool, error) {
 	if str, exists := m[key]; exists {
-		b, err := strconv.ParseBool(str)
+		b, err := ParseBool(str)
 		if err != nil {
 			return false, exists, fmt.Errorf("%s %v/%v '%s' contains invalid bool: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
 		}
@@ -35,7 +35,7 @@ func GetMapKeyAsBool(m map[string]string, key string, context apiObject) (bool,
 // GetMapKeyAsInt tries to find and parse a key in a map as int.
 func GetMapKeyAsInt(m map[string]string, key string, context apiObject) (int, bool, error) {
 	if str, exists := m[key]; exists {
-		i, err := strconv.Atoi(str)
+		i, err := ParseInt(str)
 		if err != nil {
 			return 0, exists, fmt.Errorf("%s %v/%v '%s' contains invalid integer: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
 		}
@@ -49,7 +49,7 @@ func GetMapKeyAsInt(m map[string]string, key string, context apiObject) (int, bo
 // GetMapKeyAsInt64 tries to find and parse a key in a map as int64.
 func GetMapKeyAsInt64(m map[string]string, key string, context apiObject) (int64, bool, error) {
 	if str, exists := m[key]; exists {
-		i, err := strconv.ParseInt(str, 10, 64)
+		i, err := ParseInt64(str)
 		if err != nil {
 			return 0, exists, fmt.Errorf("%s %v/%v '%s' contains invalid integer: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
 		}
@@ -63,7 +63,7 @@ func GetMapKeyAsInt64(m map[string]string, key string, context apiObject) (int64
 // GetMapKeyAsUint64 tries to find and parse a key in a map as uint64.
 func GetMapKeyAsUint64(m map[string]string, key string, context apiObject, nonZero bool) (uint64, bool, error) {
 	if str, exists := m[key]; exists {
-		i, err := strconv.ParseUint(str, 10, 64)
+		i, err := ParseUint64(str)
 		if err != nil {
 			return 0, exists, fmt.Errorf("%s %v/%v '%s' contains invalid uint64: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
 		}
@@ -162,6 +162,23 @@ func validateHashLBMethod(method string) (string, error) {
 	return "", fmt.Errorf("Invalid load balancing method: %q", method)
 }
 
+// ParseBool ensures that the string value in the annotation is a valid bool
+func ParseBool(s string) (bool, error) {
+	return strconv.ParseBool(s)
+}
+
+func ParseInt(s string) (int, error) {
+	return strconv.Atoi(s)
+}
+
+func ParseInt64(s string) (int64, error) {
+	return strconv.ParseInt(s, 10, 64)
+}
+
+func ParseUint64(s string) (uint64, error) {
+	return strconv.ParseUint(s, 10, 64)
+}
+
 // http://nginx.org/en/docs/syntax.html
 var validTimeSuffixes = []string{
 	"ms",
@@ -187,6 +204,45 @@ func ParseTime(s string) (string, error) {
 	return "", errors.New("Invalid time string")
 }
 
+// http://nginx.org/en/docs/syntax.html
+const OffsetFmt = `\d+[kKmMgG]?`
+
+var offsetRegexp = regexp.MustCompile("^" + OffsetFmt + "$")
+
+func ParseOffset(s string) (string, error) {
+	s = strings.TrimSpace(s)
+
+	if offsetRegexp.MatchString(s) {
+		return s, nil
+	}
+	return "", errors.New("Invalid offset string")
+}
+
+// http://nginx.org/en/docs/syntax.html
+const SizeFmt = `\d+[kKmM]?`
+
+var sizeRegexp = regexp.MustCompile("^" + SizeFmt + "$")
+
+func ParseSize(s string) (string, error) {
+	s = strings.TrimSpace(s)
+
+	if sizeRegexp.MatchString(s) {
+		return s, nil
+	}
+	return "", errors.New("Invalid size string")
+}
+
+var proxyBuffersRegexp = regexp.MustCompile(`^\d+ \d+[kKmM]?$`)
+
+func ParseProxyBuffers(s string) (string, error) {
+	s = strings.TrimSpace(s)
+
+	if proxyBuffersRegexp.MatchString(s) {
+		return s, nil
+	}
+	return "", errors.New("must be a valid proxy buffers spec as specified here: https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers, e.g. \"8 4k\"")
+}
+
 var threshEx = regexp.MustCompile(`high=([1-9]|[1-9][0-9]|100) low=([1-9]|[1-9][0-9]|100)\b`)
 var threshExR = regexp.MustCompile(`low=([1-9]|[1-9][0-9]|100) high=([1-9]|[1-9][0-9]|100)\b`)
 

internal/configs/parsing_helpers_test.go

@@ -383,6 +383,46 @@ func TestParseTime(t *testing.T) {
 	}
 }
 
+func TestParseOffset(t *testing.T) {
+	var testsWithValidInput = []string{"1", "2k", "2K", "3m", "3M", "4g", "4G"}
+	var invalidInput = []string{"-1", "", "blah"}
+	for _, test := range testsWithValidInput {
+		result, err := ParseOffset(test)
+		if err != nil {
+			t.Errorf("TestParseOffset(%q) returned an error for valid input", test)
+		}
+		if test != result {
+			t.Errorf("TestParseOffset(%q) returned %q expected %q", test, result, test)
+		}
+	}
+	for _, test := range invalidInput {
+		result, err := ParseOffset(test)
+		if err == nil {
+			t.Errorf("TestParseOffset(%q) didn't return error. Returned: %q", test, result)
+		}
+	}
+}
+
+func TestParseSize(t *testing.T) {
+	var testsWithValidInput = []string{"1", "2k", "2K", "3m", "3M"}
+	var invalidInput = []string{"-1", "", "blah", "4g", "4G"}
+	for _, test := range testsWithValidInput {
+		result, err := ParseSize(test)
+		if err != nil {
+			t.Errorf("TestParseSize(%q) returned an error for valid input", test)
+		}
+		if test != result {
+			t.Errorf("TestParseSize(%q) returned %q expected %q", test, result, test)
+		}
+	}
+	for _, test := range invalidInput {
+		result, err := ParseSize(test)
+		if err == nil {
+			t.Errorf("TestParseSize(%q) didn't return error. Returned: %q", test, result)
+		}
+	}
+}
+
 func TestVerifyThresholds(t *testing.T) {
 	validInput := []string{
 		"high=3 low=1",
@@ -416,3 +456,152 @@ func TestVerifyThresholds(t *testing.T) {
 		}
 	}
 }
+
+func TestParseBool(t *testing.T) {
+	var testsWithValidInput = []struct {
+		input    string
+		expected bool
+	}{
+		{"0", false},
+		{"1", true},
+		{"true", true},
+		{"false", false},
+	}
+
+	var invalidInput = []string{
+		"",
+		"blablah",
+		"-100",
+		"-1",
+	}
+
+	for _, test := range testsWithValidInput {
+		result, err := ParseBool(test.input)
+		if err != nil {
+			t.Errorf("TestParseBool(%q) returned an error for valid input", test.input)
+		}
+
+		if result != test.expected {
+			t.Errorf("TestParseBool(%q) returned %t expected %t", test.input, result, test.expected)
+		}
+	}
+
+	for _, input := range invalidInput {
+		_, err := ParseBool(input)
+		if err == nil {
+			t.Errorf("TestParseBool(%q) does not return an error for invalid input", input)
+		}
+	}
+}
+
+func TestParseInt(t *testing.T) {
+	var testsWithValidInput = []struct {
+		input    string
+		expected int
+	}{
+		{"0", 0},
+		{"1", 1},
+		{"-100", -100},
+		{"123456789", 123456789},
+	}
+
+	var invalidInput = []string{
+		"",
+		"blablah",
+		"10000000000000000000000000000000000000000000000000000000000000000",
+		"1,000",
+	}
+
+	for _, test := range testsWithValidInput {
+		result, err := ParseInt(test.input)
+		if err != nil {
+			t.Errorf("TestParseInt(%q) returned an error for valid input", test.input)
+		}
+
+		if result != test.expected {
+			t.Errorf("TestParseInt(%q) returned %d expected %d", test.input, result, test.expected)
+		}
+	}
+
+	for _, input := range invalidInput {
+		_, err := ParseInt(input)
+		if err == nil {
+			t.Errorf("TestParseInt(%q) does not return an error for invalid input", input)
+		}
+	}
+}
+
+func TestParseInt64(t *testing.T) {
+	var testsWithValidInput = []struct {
+		input    string
+		expected int64
+	}{
+		{"0", 0},
+		{"1", 1},
+		{"-100", -100},
+		{"123456789", 123456789},
+	}
+
+	var invalidInput = []string{
+		"",
+		"blablah",
+		"10000000000000000000000000000000000000000000000000000000000000000",
+		"1,000",
+	}
+
+	for _, test := range testsWithValidInput {
+		result, err := ParseInt64(test.input)
+		if err != nil {
+			t.Errorf("TestParseInt64(%q) returned an error for valid input", test.input)
+		}
+
+		if result != test.expected {
+			t.Errorf("TestParseInt64(%q) returned %d expected %d", test.input, result, test.expected)
+		}
+	}
+
+	for _, input := range invalidInput {
+		_, err := ParseInt64(input)
+		if err == nil {
+			t.Errorf("TestParseInt64(%q) does not return an error for invalid input", input)
+		}
+	}
+}
+
+func TestParseUint64(t *testing.T) {
+	var testsWithValidInput = []struct {
+		input    string
+		expected uint64
+	}{
+		{"0", 0},
+		{"1", 1},
+		{"100", 100},
+		{"123456789", 123456789},
+	}
+
+	var invalidInput = []string{
+		"",
+		"blablah",
+		"10000000000000000000000000000000000000000000000000000000000000000",
+		"1,000",
+		"-1023",
+	}
+
+	for _, test := range testsWithValidInput {
+		result, err := ParseUint64(test.input)
+		if err != nil {
+			t.Errorf("TestParseUint64(%q) returned an error for valid input", test.input)
+		}
+
+		if result != test.expected {
+			t.Errorf("TestParseUint64(%q) returned %d expected %d", test.input, result, test.expected)
+		}
+	}
+
+	for _, input := range invalidInput {
+		_, err := ParseUint64(input)
+		if err == nil {
+			t.Errorf("TestParseUint64(%q) does not return an error for invalid input", input)
+		}
+	}
+}

internal/k8s/configuration.go

@@ -307,6 +307,8 @@ type Configuration struct {
 	appPolicyReferenceChecker  *appProtectResourceReferenceChecker
 	appLogConfReferenceChecker *appProtectResourceReferenceChecker
 
+	isPlus bool
+
 	lock sync.RWMutex
 }
 
@@ -325,6 +327,7 @@ func NewConfiguration(hasCorrectIngressClass func(interface{}) bool, isPlus bool
 		policyReferenceChecker:     newPolicyReferenceChecker(),
 		appPolicyReferenceChecker:  newAppProtectResourceReferenceChecker(configs.AppProtectPolicyAnnotation),
 		appLogConfReferenceChecker: newAppProtectResourceReferenceChecker(configs.AppProtectLogConfAnnotation),
+		isPlus:                     isPlus,
 	}
 }
 
@@ -339,7 +342,7 @@ func (c *Configuration) AddOrUpdateIngress(ing *networking.Ingress) ([]ResourceC
 	if !c.hasCorrectIngressClass(ing) {
 		delete(c.ingresses, key)
 	} else {
-		validationError = validateIngress(ing).ToAggregate()
+		validationError = validateIngress(ing, c.isPlus).ToAggregate()
 		if validationError != nil {
 			delete(c.ingresses, key)
 		} else {