Remove filter validation already covered by CEL validation

Author: bjee19

internal/controller/state/graph/common_filter.go

@@ -230,40 +230,6 @@ func validateFilterMirror(mirror *v1.HTTPRequestMirrorFilter, filterPath *field.
 		return field.ErrorList{field.Required(mirrorPath, "cannot be nil")}
 	}
 
-	if mirror.Percent != nil && mirror.Fraction != nil {
-		return field.ErrorList{field.Invalid(mirrorPath, mirror, "cannot set both percent and fraction")}
-	}
-
-	if mirror.Fraction != nil && mirror.Fraction.Denominator != nil {
-		if *mirror.Fraction.Denominator <= 0 {
-			return field.ErrorList{
-				field.Invalid(
-					mirrorPath.Child("fraction", "denominator"),
-					mirror.Fraction.Denominator,
-					"must be greater than 0",
-				),
-			}
-		}
-		if mirror.Fraction.Numerator < 0 {
-			return field.ErrorList{
-				field.Invalid(
-					mirrorPath.Child("fraction", "numerator"),
-					mirror.Fraction.Numerator,
-					"must be greater than or equal to 0",
-				),
-			}
-		}
-		if mirror.Fraction.Numerator > *mirror.Fraction.Denominator {
-			return field.ErrorList{
-				field.Invalid(
-					mirrorPath.Child("fraction", "numerator"),
-					mirror.Fraction.Numerator,
-					"must be less than or equal to denominator",
-				),
-			}
-		}
-	}
-
 	return nil
 }
 

internal/controller/state/graph/common_filter_test.go

@@ -10,7 +10,6 @@ import (
 
 	ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1"
 	"github.com/nginx/nginx-gateway-fabric/internal/controller/state/validation/validationfakes"
-	"github.com/nginx/nginx-gateway-fabric/internal/framework/helpers"
 	"github.com/nginx/nginx-gateway-fabric/internal/framework/kinds"
 )
 
@@ -49,6 +48,24 @@ func TestValidateFilter(t *testing.T) {
 			expectErrCount: 0,
 			name:           "valid HTTP request header modifiers filter",
 		},
+		{
+			filter: Filter{
+				RouteType:     RouteTypeHTTP,
+				FilterType:    FilterRequestMirror,
+				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
+			},
+			expectErrCount: 0,
+			name:           "valid HTTP mirror filter",
+		},
+		{
+			filter: Filter{
+				RouteType:     RouteTypeHTTP,
+				FilterType:    FilterRequestMirror,
+				RequestMirror: nil,
+			},
+			expectErrCount: 1,
+			name:           "invalid HTTP mirror filter",
+		},
 		{
 			filter: Filter{
 				RouteType:              RouteTypeHTTP,
@@ -79,6 +96,15 @@ func TestValidateFilter(t *testing.T) {
 			expectErrCount: 1,
 			name:           "unsupported HTTP filter type",
 		},
+		{
+			filter: Filter{
+				RouteType:     RouteTypeGRPC,
+				FilterType:    FilterRequestMirror,
+				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
+			},
+			expectErrCount: 0,
+			name:           "valid GRPC mirror filter",
+		},
 		{
 			filter: Filter{
 				RouteType:             RouteTypeGRPC,
@@ -133,138 +159,6 @@ func TestValidateFilter(t *testing.T) {
 	}
 }
 
-func TestValidateFilterMirror(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		filter         Filter
-		name           string
-		expectErrCount int
-	}{
-		{
-			filter: Filter{
-				RouteType:     RouteTypeHTTP,
-				FilterType:    FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
-			},
-			expectErrCount: 0,
-			name:           "valid HTTP mirror filter",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Percent: helpers.GetPointer(int32(50)),
-				},
-			},
-			expectErrCount: 0,
-			name:           "valid HTTP mirror filter with percentage set",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Fraction: &gatewayv1.Fraction{
-						Numerator:   1,
-						Denominator: helpers.GetPointer(int32(2)),
-					},
-				},
-			},
-			expectErrCount: 0,
-			name:           "valid HTTP mirror filter with fraction set",
-		},
-		{
-			filter: Filter{
-				RouteType:     RouteTypeHTTP,
-				FilterType:    FilterRequestMirror,
-				RequestMirror: nil,
-			},
-			expectErrCount: 1,
-			name:           "invalid nil HTTP mirror filter",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Percent: helpers.GetPointer(int32(5)),
-					Fraction: &gatewayv1.Fraction{
-						Numerator:   1,
-						Denominator: helpers.GetPointer(int32(3)),
-					},
-				},
-			},
-			expectErrCount: 1,
-			name:           "invalid HTTP mirror filter both percent and fraction set",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Fraction: &gatewayv1.Fraction{
-						Numerator:   1,
-						Denominator: helpers.GetPointer(int32(0)),
-					},
-				},
-			},
-			expectErrCount: 1,
-			name:           "invalid HTTP mirror filter, fraction denominator value must be greater than 0",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Fraction: &gatewayv1.Fraction{
-						Numerator:   -1,
-						Denominator: helpers.GetPointer(int32(2)),
-					},
-				},
-			},
-			expectErrCount: 1,
-			name:           "invalid HTTP mirror filter, fraction numerator value must be greater than or equal to 0",
-		},
-		{
-			filter: Filter{
-				RouteType:  RouteTypeHTTP,
-				FilterType: FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
-					Fraction: &gatewayv1.Fraction{
-						Numerator:   5,
-						Denominator: helpers.GetPointer(int32(2)),
-					},
-				},
-			},
-			expectErrCount: 1,
-			name:           "invalid HTTP mirror filter, fraction numerator value must be less than denominator",
-		},
-		{
-			filter: Filter{
-				RouteType:     RouteTypeGRPC,
-				FilterType:    FilterRequestMirror,
-				RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
-			},
-			expectErrCount: 0,
-			name:           "valid GRPC mirror filter",
-		},
-	}
-
-	filterPath := field.NewPath("test")
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			g := NewWithT(t)
-			allErrs := validateFilter(&validationfakes.FakeHTTPFieldsValidator{}, test.filter, filterPath)
-			g.Expect(allErrs).To(HaveLen(test.expectErrCount))
-		})
-	}
-}
-
 func TestValidateFilterResponseHeaderModifier(t *testing.T) {
 	t.Parallel()