Skip to content

Commit e5ca945

Browse files
committed
Add back validation
1 parent b214d78 commit e5ca945

File tree

2 files changed

+167
-27
lines changed

2 files changed

+167
-27
lines changed

internal/controller/state/graph/common_filter.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,40 @@ func validateFilterMirror(mirror *v1.HTTPRequestMirrorFilter, filterPath *field.
230230
return field.ErrorList{field.Required(mirrorPath, "cannot be nil")}
231231
}
232232

233+
if mirror.Percent != nil && mirror.Fraction != nil {
234+
return field.ErrorList{field.Invalid(mirrorPath, mirror, "cannot set both percent and fraction")}
235+
}
236+
237+
if mirror.Fraction != nil && mirror.Fraction.Denominator != nil {
238+
if *mirror.Fraction.Denominator <= 0 {
239+
return field.ErrorList{
240+
field.Invalid(
241+
mirrorPath.Child("fraction", "denominator"),
242+
mirror.Fraction.Denominator,
243+
"must be greater than 0",
244+
),
245+
}
246+
}
247+
if mirror.Fraction.Numerator < 0 {
248+
return field.ErrorList{
249+
field.Invalid(
250+
mirrorPath.Child("fraction", "numerator"),
251+
mirror.Fraction.Numerator,
252+
"must be greater than or equal to 0",
253+
),
254+
}
255+
}
256+
if mirror.Fraction.Numerator > *mirror.Fraction.Denominator {
257+
return field.ErrorList{
258+
field.Invalid(
259+
mirrorPath.Child("fraction", "numerator"),
260+
mirror.Fraction.Numerator,
261+
"must be less than or equal to denominator",
262+
),
263+
}
264+
}
265+
}
266+
233267
return nil
234268
}
235269

internal/controller/state/graph/common_filter_test.go

Lines changed: 133 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1"
1212
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/validation/validationfakes"
13+
"github.com/nginx/nginx-gateway-fabric/internal/framework/helpers"
1314
"github.com/nginx/nginx-gateway-fabric/internal/framework/kinds"
1415
)
1516

@@ -48,24 +49,6 @@ func TestValidateFilter(t *testing.T) {
4849
expectErrCount: 0,
4950
name: "valid HTTP request header modifiers filter",
5051
},
51-
{
52-
filter: Filter{
53-
RouteType: RouteTypeHTTP,
54-
FilterType: FilterRequestMirror,
55-
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
56-
},
57-
expectErrCount: 0,
58-
name: "valid HTTP mirror filter",
59-
},
60-
{
61-
filter: Filter{
62-
RouteType: RouteTypeHTTP,
63-
FilterType: FilterRequestMirror,
64-
RequestMirror: nil,
65-
},
66-
expectErrCount: 1,
67-
name: "invalid HTTP mirror filter",
68-
},
6952
{
7053
filter: Filter{
7154
RouteType: RouteTypeHTTP,
@@ -96,15 +79,6 @@ func TestValidateFilter(t *testing.T) {
9679
expectErrCount: 1,
9780
name: "unsupported HTTP filter type",
9881
},
99-
{
100-
filter: Filter{
101-
RouteType: RouteTypeGRPC,
102-
FilterType: FilterRequestMirror,
103-
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
104-
},
105-
expectErrCount: 0,
106-
name: "valid GRPC mirror filter",
107-
},
10882
{
10983
filter: Filter{
11084
RouteType: RouteTypeGRPC,
@@ -159,6 +133,138 @@ func TestValidateFilter(t *testing.T) {
159133
}
160134
}
161135

136+
func TestValidateFilterMirror(t *testing.T) {
137+
t.Parallel()
138+
139+
tests := []struct {
140+
filter Filter
141+
name string
142+
expectErrCount int
143+
}{
144+
{
145+
filter: Filter{
146+
RouteType: RouteTypeHTTP,
147+
FilterType: FilterRequestMirror,
148+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
149+
},
150+
expectErrCount: 0,
151+
name: "valid HTTP mirror filter",
152+
},
153+
{
154+
filter: Filter{
155+
RouteType: RouteTypeHTTP,
156+
FilterType: FilterRequestMirror,
157+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
158+
Percent: helpers.GetPointer(int32(50)),
159+
},
160+
},
161+
expectErrCount: 0,
162+
name: "valid HTTP mirror filter with percentage set",
163+
},
164+
{
165+
filter: Filter{
166+
RouteType: RouteTypeHTTP,
167+
FilterType: FilterRequestMirror,
168+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
169+
Fraction: &gatewayv1.Fraction{
170+
Numerator: 1,
171+
Denominator: helpers.GetPointer(int32(2)),
172+
},
173+
},
174+
},
175+
expectErrCount: 0,
176+
name: "valid HTTP mirror filter with fraction set",
177+
},
178+
{
179+
filter: Filter{
180+
RouteType: RouteTypeHTTP,
181+
FilterType: FilterRequestMirror,
182+
RequestMirror: nil,
183+
},
184+
expectErrCount: 1,
185+
name: "invalid nil HTTP mirror filter",
186+
},
187+
{
188+
filter: Filter{
189+
RouteType: RouteTypeHTTP,
190+
FilterType: FilterRequestMirror,
191+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
192+
Percent: helpers.GetPointer(int32(5)),
193+
Fraction: &gatewayv1.Fraction{
194+
Numerator: 1,
195+
Denominator: helpers.GetPointer(int32(3)),
196+
},
197+
},
198+
},
199+
expectErrCount: 1,
200+
name: "invalid HTTP mirror filter both percent and fraction set",
201+
},
202+
{
203+
filter: Filter{
204+
RouteType: RouteTypeHTTP,
205+
FilterType: FilterRequestMirror,
206+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
207+
Fraction: &gatewayv1.Fraction{
208+
Numerator: 1,
209+
Denominator: helpers.GetPointer(int32(0)),
210+
},
211+
},
212+
},
213+
expectErrCount: 1,
214+
name: "invalid HTTP mirror filter, fraction denominator value must be greater than 0",
215+
},
216+
{
217+
filter: Filter{
218+
RouteType: RouteTypeHTTP,
219+
FilterType: FilterRequestMirror,
220+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
221+
Fraction: &gatewayv1.Fraction{
222+
Numerator: -1,
223+
Denominator: helpers.GetPointer(int32(2)),
224+
},
225+
},
226+
},
227+
expectErrCount: 1,
228+
name: "invalid HTTP mirror filter, fraction numerator value must be greater than or equal to 0",
229+
},
230+
{
231+
filter: Filter{
232+
RouteType: RouteTypeHTTP,
233+
FilterType: FilterRequestMirror,
234+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{
235+
Fraction: &gatewayv1.Fraction{
236+
Numerator: 5,
237+
Denominator: helpers.GetPointer(int32(2)),
238+
},
239+
},
240+
},
241+
expectErrCount: 1,
242+
name: "invalid HTTP mirror filter, fraction numerator value must be less than denominator",
243+
},
244+
{
245+
filter: Filter{
246+
RouteType: RouteTypeGRPC,
247+
FilterType: FilterRequestMirror,
248+
RequestMirror: &gatewayv1.HTTPRequestMirrorFilter{},
249+
},
250+
expectErrCount: 0,
251+
name: "valid GRPC mirror filter",
252+
},
253+
}
254+
255+
filterPath := field.NewPath("test")
256+
257+
for _, test := range tests {
258+
t.Run(test.name, func(t *testing.T) {
259+
t.Parallel()
260+
261+
g := NewWithT(t)
262+
allErrs := validateFilter(&validationfakes.FakeHTTPFieldsValidator{}, test.filter, filterPath)
263+
g.Expect(allErrs).To(HaveLen(test.expectErrCount))
264+
})
265+
}
266+
}
267+
162268
func TestValidateFilterResponseHeaderModifier(t *testing.T) {
163269
t.Parallel()
164270

0 commit comments

Comments
 (0)