Skip to content

Commit 4844d9a

Browse files
arkodgarkodg
authored
fail validation if baseInterval is 0s (envoyproxy#5176)
* fail validation if baseInterval is 0s Fixes: envoyproxy#5147 Signed-off-by: Arko Dasgupta <[email protected]> * more validations Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]>
1 parent 4fba2bf commit 4844d9a

File tree

4 files changed

+280
-14
lines changed

4 files changed

+280
-14
lines changed

‎internal/gatewayapi/backendtrafficpolicy.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,12 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(
333333
err = perr.WithMessage(err, "TCPKeepalive")
334334
errs = errors.Join(errs, err)
335335
}
336-
if policy.Spec.Retry != nil {
337-
rt = buildRetry(policy.Spec.Retry)
336+
337+
if rt, err = buildRetry(policy.Spec.Retry); err != nil {
338+
err = perr.WithMessage(err, "Retry")
339+
errs = errors.Join(errs, err)
338340
}
341+
339342
if to, err = buildClusterSettingsTimeout(policy.Spec.ClusterSettings); err != nil {
340343
err = perr.WithMessage(err, "Timeout")
341344
errs = errors.Join(errs, err)
@@ -484,9 +487,12 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(
484487
err = perr.WithMessage(err, "TCPKeepalive")
485488
errs = errors.Join(errs, err)
486489
}
487-
if policy.Spec.Retry != nil {
488-
rt = buildRetry(policy.Spec.Retry)
490+
491+
if rt, err = buildRetry(policy.Spec.Retry); err != nil {
492+
err = perr.WithMessage(err, "Retry")
493+
errs = errors.Join(errs, err)
489494
}
495+
490496
if ct, err = buildClusterSettingsTimeout(policy.Spec.ClusterSettings); err != nil {
491497
err = perr.WithMessage(err, "Timeout")
492498
errs = errors.Join(errs, err)

‎internal/gatewayapi/clustersettings.go‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ func translateTrafficFeatures(policy *egv1a1.ClusterSettings) (*ir.TrafficFeatur
7272
ret.HTTP2 = h2
7373
}
7474

75-
ret.Retry = buildRetry(policy.Retry)
75+
var err error
76+
if ret.Retry, err = buildRetry(policy.Retry); err != nil {
77+
return nil, err
78+
}
7679

7780
// If nothing was set in any of the above calls, return nil instead of an empty
7881
// container
@@ -477,9 +480,9 @@ func translateDNS(policy egv1a1.ClusterSettings) *ir.DNS {
477480
}
478481
}
479482

480-
func buildRetry(r *egv1a1.Retry) *ir.Retry {
483+
func buildRetry(r *egv1a1.Retry) (*ir.Retry, error) {
481484
if r == nil {
482-
return nil
485+
return nil, nil
483486
}
484487

485488
rt := &ir.Retry{}
@@ -518,13 +521,22 @@ func buildRetry(r *egv1a1.Retry) *ir.Retry {
518521
if r.PerRetry.BackOff != nil {
519522
if r.PerRetry.BackOff.MaxInterval != nil || r.PerRetry.BackOff.BaseInterval != nil {
520523
bop := &ir.BackOffPolicy{}
524+
if r.PerRetry.BackOff.BaseInterval != nil {
525+
bop.BaseInterval = r.PerRetry.BackOff.BaseInterval
526+
if bop.BaseInterval.Duration == 0 {
527+
return nil, fmt.Errorf("baseInterval cannot be set to 0s")
528+
}
529+
}
521530
if r.PerRetry.BackOff.MaxInterval != nil {
522531
bop.MaxInterval = r.PerRetry.BackOff.MaxInterval
532+
if bop.MaxInterval.Duration == 0 {
533+
return nil, fmt.Errorf("maxInterval cannot be set to 0s")
534+
}
535+
if bop.BaseInterval != nil && bop.BaseInterval.Duration > bop.MaxInterval.Duration {
536+
return nil, fmt.Errorf("maxInterval cannot be less than baseInterval")
537+
}
523538
}
524539

525-
if r.PerRetry.BackOff.BaseInterval != nil {
526-
bop.BaseInterval = r.PerRetry.BackOff.BaseInterval
527-
}
528540
pr.BackOff = bop
529541
bpr = true
530542
}
@@ -535,5 +547,5 @@ func buildRetry(r *egv1a1.Retry) *ir.Retry {
535547
}
536548
}
537549

538-
return rt
550+
return rt, nil
539551
}

‎internal/gatewayapi/testdata/backendtrafficpolicy-with-retries.in.yaml‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,44 @@ httpRoutes:
6262
backendRefs:
6363
- name: service-1
6464
port: 8080
65+
- apiVersion: gateway.networking.k8s.io/v1
66+
kind: HTTPRoute
67+
metadata:
68+
namespace: default
69+
name: httproute-2
70+
spec:
71+
hostnames:
72+
- gateway.envoyproxy.io
73+
parentRefs:
74+
- namespace: envoy-gateway
75+
name: gateway-2
76+
sectionName: http
77+
rules:
78+
- matches:
79+
- path:
80+
value: "/route2"
81+
backendRefs:
82+
- name: service-1
83+
port: 8080
84+
- apiVersion: gateway.networking.k8s.io/v1
85+
kind: HTTPRoute
86+
metadata:
87+
namespace: default
88+
name: httproute-3
89+
spec:
90+
hostnames:
91+
- gateway.envoyproxy.io
92+
parentRefs:
93+
- namespace: envoy-gateway
94+
name: gateway-2
95+
sectionName: http
96+
rules:
97+
- matches:
98+
- path:
99+
value: "/route3"
100+
backendRefs:
101+
- name: service-1
102+
port: 8080
65103
backendTrafficPolicies:
66104
- apiVersion: gateway.envoyproxy.io/v1alpha1
67105
kind: BackendTrafficPolicy
@@ -86,7 +124,7 @@ backendTrafficPolicies:
86124
kind: BackendTrafficPolicy
87125
metadata:
88126
namespace: default
89-
name: policy-for-route
127+
name: policy-for-route-1
90128
spec:
91129
targetRef:
92130
group: gateway.networking.k8s.io
@@ -106,3 +144,32 @@ backendTrafficPolicies:
106144
backoff:
107145
baseInterval: 100ms
108146
maxInterval: 10s
147+
- apiVersion: gateway.envoyproxy.io/v1alpha1
148+
kind: BackendTrafficPolicy
149+
metadata:
150+
namespace: default
151+
name: policy-for-route-2
152+
spec:
153+
targetRef:
154+
group: gateway.networking.k8s.io
155+
kind: HTTPRoute
156+
name: httproute-2
157+
retry:
158+
perRetry:
159+
backoff:
160+
baseInterval: 0s
161+
- apiVersion: gateway.envoyproxy.io/v1alpha1
162+
kind: BackendTrafficPolicy
163+
metadata:
164+
namespace: default
165+
name: policy-for-route-3
166+
spec:
167+
targetRef:
168+
group: gateway.networking.k8s.io
169+
kind: HTTPRoute
170+
name: httproute-3
171+
retry:
172+
perRetry:
173+
backoff:
174+
baseInterval: 2s
175+
maxInterval: 1s

‎internal/gatewayapi/testdata/backendtrafficpolicy-with-retries.out.yaml‎

Lines changed: 183 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ backendTrafficPolicies:
33
kind: BackendTrafficPolicy
44
metadata:
55
creationTimestamp: null
6-
name: policy-for-route
6+
name: policy-for-route-1
77
namespace: default
88
spec:
99
retry:
@@ -39,6 +39,67 @@ backendTrafficPolicies:
3939
status: "True"
4040
type: Accepted
4141
controllerName: gateway.envoyproxy.io/gatewayclass-controller
42+
- apiVersion: gateway.envoyproxy.io/v1alpha1
43+
kind: BackendTrafficPolicy
44+
metadata:
45+
creationTimestamp: null
46+
name: policy-for-route-2
47+
namespace: default
48+
spec:
49+
retry:
50+
perRetry:
51+
backOff:
52+
baseInterval: 0s
53+
targetRef:
54+
group: gateway.networking.k8s.io
55+
kind: HTTPRoute
56+
name: httproute-2
57+
status:
58+
ancestors:
59+
- ancestorRef:
60+
group: gateway.networking.k8s.io
61+
kind: Gateway
62+
name: gateway-2
63+
namespace: envoy-gateway
64+
sectionName: http
65+
conditions:
66+
- lastTransitionTime: null
67+
message: 'Retry: baseInterval cannot be set to 0s.'
68+
reason: Invalid
69+
status: "False"
70+
type: Accepted
71+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
72+
- apiVersion: gateway.envoyproxy.io/v1alpha1
73+
kind: BackendTrafficPolicy
74+
metadata:
75+
creationTimestamp: null
76+
name: policy-for-route-3
77+
namespace: default
78+
spec:
79+
retry:
80+
perRetry:
81+
backOff:
82+
baseInterval: 2s
83+
maxInterval: 1s
84+
targetRef:
85+
group: gateway.networking.k8s.io
86+
kind: HTTPRoute
87+
name: httproute-3
88+
status:
89+
ancestors:
90+
- ancestorRef:
91+
group: gateway.networking.k8s.io
92+
kind: Gateway
93+
name: gateway-2
94+
namespace: envoy-gateway
95+
sectionName: http
96+
conditions:
97+
- lastTransitionTime: null
98+
message: 'Retry: maxInterval cannot be less than baseInterval.'
99+
reason: Invalid
100+
status: "False"
101+
type: Accepted
102+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
42103
- apiVersion: gateway.envoyproxy.io/v1alpha1
43104
kind: BackendTrafficPolicy
44105
metadata:
@@ -131,7 +192,7 @@ gateways:
131192
protocol: HTTP
132193
status:
133194
listeners:
134-
- attachedRoutes: 1
195+
- attachedRoutes: 3
135196
conditions:
136197
- lastTransitionTime: null
137198
message: Sending translated listener configuration to the data plane
@@ -227,6 +288,82 @@ httpRoutes:
227288
name: gateway-2
228289
namespace: envoy-gateway
229290
sectionName: http
291+
- apiVersion: gateway.networking.k8s.io/v1
292+
kind: HTTPRoute
293+
metadata:
294+
creationTimestamp: null
295+
name: httproute-2
296+
namespace: default
297+
spec:
298+
hostnames:
299+
- gateway.envoyproxy.io
300+
parentRefs:
301+
- name: gateway-2
302+
namespace: envoy-gateway
303+
sectionName: http
304+
rules:
305+
- backendRefs:
306+
- name: service-1
307+
port: 8080
308+
matches:
309+
- path:
310+
value: /route2
311+
status:
312+
parents:
313+
- conditions:
314+
- lastTransitionTime: null
315+
message: Route is accepted
316+
reason: Accepted
317+
status: "True"
318+
type: Accepted
319+
- lastTransitionTime: null
320+
message: Resolved all the Object references for the Route
321+
reason: ResolvedRefs
322+
status: "True"
323+
type: ResolvedRefs
324+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
325+
parentRef:
326+
name: gateway-2
327+
namespace: envoy-gateway
328+
sectionName: http
329+
- apiVersion: gateway.networking.k8s.io/v1
330+
kind: HTTPRoute
331+
metadata:
332+
creationTimestamp: null
333+
name: httproute-3
334+
namespace: default
335+
spec:
336+
hostnames:
337+
- gateway.envoyproxy.io
338+
parentRefs:
339+
- name: gateway-2
340+
namespace: envoy-gateway
341+
sectionName: http
342+
rules:
343+
- backendRefs:
344+
- name: service-1
345+
port: 8080
346+
matches:
347+
- path:
348+
value: /route3
349+
status:
350+
parents:
351+
- conditions:
352+
- lastTransitionTime: null
353+
message: Route is accepted
354+
reason: Accepted
355+
status: "True"
356+
type: Accepted
357+
- lastTransitionTime: null
358+
message: Resolved all the Object references for the Route
359+
reason: ResolvedRefs
360+
status: "True"
361+
type: ResolvedRefs
362+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
363+
parentRef:
364+
name: gateway-2
365+
namespace: envoy-gateway
366+
sectionName: http
230367
infraIR:
231368
envoy-gateway/gateway-1:
232369
proxy:
@@ -325,6 +462,50 @@ xdsIR:
325462
mergeSlashes: true
326463
port: 10080
327464
routes:
465+
- destination:
466+
name: httproute/default/httproute-2/rule/0
467+
settings:
468+
- addressType: IP
469+
endpoints:
470+
- host: 7.7.7.7
471+
port: 8080
472+
protocol: HTTP
473+
weight: 1
474+
directResponse:
475+
statusCode: 500
476+
hostname: gateway.envoyproxy.io
477+
isHTTP2: false
478+
metadata:
479+
kind: HTTPRoute
480+
name: httproute-2
481+
namespace: default
482+
name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io
483+
pathMatch:
484+
distinct: false
485+
name: ""
486+
prefix: /route2
487+
- destination:
488+
name: httproute/default/httproute-3/rule/0
489+
settings:
490+
- addressType: IP
491+
endpoints:
492+
- host: 7.7.7.7
493+
port: 8080
494+
protocol: HTTP
495+
weight: 1
496+
directResponse:
497+
statusCode: 500
498+
hostname: gateway.envoyproxy.io
499+
isHTTP2: false
500+
metadata:
501+
kind: HTTPRoute
502+
name: httproute-3
503+
namespace: default
504+
name: httproute/default/httproute-3/rule/0/match/0/gateway_envoyproxy_io
505+
pathMatch:
506+
distinct: false
507+
name: ""
508+
prefix: /route3
328509
- destination:
329510
name: httproute/default/httproute-1/rule/0
330511
settings:

0 commit comments

Comments
 (0)