From e8c72c70f9774cb3c47406e2dcb1b6dbefd87f4b Mon Sep 17 00:00:00 2001
From: joey <zchengjoey@gmail.com>
Date: Fri, 13 Sep 2024 16:28:00 +0800
Subject: [PATCH] fix `ipallowlist` parser not handling `validation` type
 errors

Signed-off-by: joey <zchengjoey@gmail.com>
---
 .../ingress/annotations/annotations_test.go   | 55 +++++++++++++++++++
 .../ingress/annotations/ipallowlist/main.go   |  3 +
 2 files changed, 58 insertions(+)

diff --git a/internal/ingress/annotations/annotations_test.go b/internal/ingress/annotations/annotations_test.go
index 5df3cdc0ee..9bbde2cdf6 100644
--- a/internal/ingress/annotations/annotations_test.go
+++ b/internal/ingress/annotations/annotations_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package annotations
 
 import (
+	"fmt"
 	"testing"
 
 	apiv1 "k8s.io/api/core/v1"
@@ -366,3 +367,57 @@ func TestCustomResponseHeaders(t *testing.T) {
 		}
 	}
 }
+
+func TestIPAllowList(t *testing.T) {
+	mockObj := mockCfg{}
+	mockObj.MockConfigMaps = map[string]*apiv1.ConfigMap{}
+
+	ec := NewAnnotationExtractor(mockObj)
+	ing := buildIngress()
+	annotationKeys := []string{"allowlist-source-range", "whitelist-source-range"}
+	for _, tc := range []struct {
+		name      string
+		net       string
+		expectErr bool
+		errOut    string
+	}{
+		{
+			name: "test parse a valid net",
+			net:  "10.0.0.0/24",
+		},
+		{
+			name:      "test parse a invalid net",
+			net:       "ww",
+			errOut:    "annotation nginx.ingress.kubernetes.io/%s contains invalid value",
+			expectErr: true,
+		},
+		{
+			name:      "test parse multiple valid cidr",
+			net:       "2.2.2.2/32,1.1.1.1/32,3.3.3.0/24",
+			expectErr: false,
+		},
+		{
+			name:      "test parse multiple invalid cidr(missing comma)",
+			net:       "1.1.1.1 2.2.2.2",
+			expectErr: true,
+			errOut:    "annotation nginx.ingress.kubernetes.io/%s contains invalid value",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, annotationKey := range annotationKeys {
+				ing.SetAnnotations(map[string]string{
+					parser.GetAnnotationWithPrefix(annotationKey): tc.net,
+				})
+				i, err := ec.Extract(ing)
+				if (err != nil) != tc.expectErr {
+					t.Errorf("expected error: %t got error: %t err value: %s. %+v", tc.expectErr, err != nil, err, i)
+				}
+				if tc.expectErr && err != nil {
+					if err.Error() != fmt.Sprintf(tc.errOut, annotationKey) {
+						t.Errorf("expected error %s but got %s", tc.errOut, err)
+					}
+				}
+			}
+		})
+	}
+}
diff --git a/internal/ingress/annotations/ipallowlist/main.go b/internal/ingress/annotations/ipallowlist/main.go
index 23a3bd9e78..760f917f4b 100644
--- a/internal/ingress/annotations/ipallowlist/main.go
+++ b/internal/ingress/annotations/ipallowlist/main.go
@@ -95,6 +95,9 @@ func (a ipallowlist) Parse(ing *networking.Ingress) (interface{}, error) {
 		if err == ing_errors.ErrMissingAnnotations {
 			return &SourceRange{CIDR: defaultAllowlistSourceRange}, nil
 		}
+		if ing_errors.IsValidationError(err) {
+			return &SourceRange{CIDR: defaultAllowlistSourceRange}, err
+		}
 
 		return &SourceRange{CIDR: defaultAllowlistSourceRange}, ing_errors.LocationDeniedError{
 			Reason: err,