Skip to content

Commit 03fcc72

Browse files
seropersonseroperson
committed
reverseproxy: add lb_retry_match condition on response status
Closes #6267
1 parent acf8d6a commit 03fcc72

File tree

6 files changed

+968
-28
lines changed

6 files changed

+968
-28
lines changed

‎caddytest/integration/caddyfile_adapt/reverse_proxy_retry_match_response.caddyfiletest‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
:8884
2+
3+
reverse_proxy 127.0.0.1:65535 {
4+
lb_retries 5
5+
6+
# request matchers (backward-compatible, non-expression)
7+
lb_retry_match {
8+
method POST PUT
9+
}
10+
lb_retry_match {
11+
path /foo*
12+
}
13+
lb_retry_match {
14+
header X-Idempotency-Key *
15+
}
16+
17+
# response status code via expression
18+
lb_retry_match {
19+
expression `{rp.status_code} in [502, 503, 504]`
20+
}
21+
22+
# response header via expression
23+
lb_retry_match {
24+
expression `{rp.header.X-Retry} == "true"`
25+
}
26+
27+
# CEL request functions combined with response placeholders
28+
lb_retry_match {
29+
expression `method('POST') && {rp.status_code} >= 500`
30+
}
31+
lb_retry_match {
32+
expression `path('/api*') && {rp.status_code} in [502, 503]`
33+
}
34+
lb_retry_match {
35+
expression `host('example.com') && {rp.status_code} == 503`
36+
}
37+
lb_retry_match {
38+
expression `query({'retry': 'true'}) && {rp.status_code} >= 500`
39+
}
40+
lb_retry_match {
41+
expression `header({'X-Idempotency-Key': '*'}) && {rp.status_code} in [502, 503]`
42+
}
43+
lb_retry_match {
44+
expression `protocol('https') && {rp.status_code} == 502`
45+
}
46+
lb_retry_match {
47+
expression `path_regexp('^/api/v[0-9]+/') && {rp.status_code} >= 500`
48+
}
49+
lb_retry_match {
50+
expression `header_regexp('Content-Type', '^application/json') && {rp.status_code} == 502`
51+
}
52+
53+
# transport error retry methods
54+
lb_retry_transport_errors GET POST PUT
55+
}
56+
----------
57+
{
58+
"apps": {
59+
"http": {
60+
"servers": {
61+
"srv0": {
62+
"listen": [
63+
":8884"
64+
],
65+
"routes": [
66+
{
67+
"handle": [
68+
{
69+
"handler": "reverse_proxy",
70+
"load_balancing": {
71+
"retries": 5,
72+
"retry_match": [
73+
{
74+
"method": [
75+
"POST",
76+
"PUT"
77+
]
78+
},
79+
{
80+
"path": [
81+
"/foo*"
82+
]
83+
},
84+
{
85+
"header": {
86+
"X-Idempotency-Key": [
87+
"*"
88+
]
89+
}
90+
},
91+
{
92+
"expression": "{http.reverse_proxy.status_code} in [502, 503, 504]"
93+
},
94+
{
95+
"expression": "{http.reverse_proxy.header.X-Retry} == \"true\""
96+
},
97+
{
98+
"expression": "method('POST') \u0026\u0026 {http.reverse_proxy.status_code} \u003e= 500"
99+
},
100+
{
101+
"expression": "path('/api*') \u0026\u0026 {http.reverse_proxy.status_code} in [502, 503]"
102+
},
103+
{
104+
"expression": "host('example.com') \u0026\u0026 {http.reverse_proxy.status_code} == 503"
105+
},
106+
{
107+
"expression": "query({'retry': 'true'}) \u0026\u0026 {http.reverse_proxy.status_code} \u003e= 500"
108+
},
109+
{
110+
"expression": "header({'X-Idempotency-Key': '*'}) \u0026\u0026 {http.reverse_proxy.status_code} in [502, 503]"
111+
},
112+
{
113+
"expression": "protocol('https') \u0026\u0026 {http.reverse_proxy.status_code} == 502"
114+
},
115+
{
116+
"expression": "path_regexp('^/api/v[0-9]+/') \u0026\u0026 {http.reverse_proxy.status_code} \u003e= 500"
117+
},
118+
{
119+
"expression": "header_regexp('Content-Type', '^application/json') \u0026\u0026 {http.reverse_proxy.status_code} == 502"
120+
}
121+
],
122+
"retry_transport_error_methods": [
123+
"GET",
124+
"POST",
125+
"PUT"
126+
]
127+
},
128+
"upstreams": [
129+
{
130+
"dial": "127.0.0.1:65535"
131+
}
132+
]
133+
}
134+
]
135+
}
136+
]
137+
}
138+
}
139+
}
140+
}
141+
}

‎caddytest/integration/caddyfile_adapt/reverse_proxy_retry_transport_errors.caddyfiletest‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
:8884
2+
3+
reverse_proxy 127.0.0.1:65535 {
4+
lb_retries 3
5+
lb_retry_transport_errors GET PUT POST
6+
}
7+
----------
8+
{
9+
"apps": {
10+
"http": {
11+
"servers": {
12+
"srv0": {
13+
"listen": [
14+
":8884"
15+
],
16+
"routes": [
17+
{
18+
"handle": [
19+
{
20+
"handler": "reverse_proxy",
21+
"load_balancing": {
22+
"retries": 3,
23+
"retry_transport_error_methods": [
24+
"GET",
25+
"PUT",
26+
"POST"
27+
]
28+
},
29+
"upstreams": [
30+
{
31+
"dial": "127.0.0.1:65535"
32+
}
33+
]
34+
}
35+
]
36+
}
37+
]
38+
}
39+
}
40+
}
41+
}
42+
}

‎caddytest/integration/reverseproxy_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"runtime"
99
"strings"
10+
"sync/atomic"
1011
"testing"
1112

1213
"github.com/caddyserver/caddy/v2/caddytest"
@@ -562,3 +563,176 @@ func TestReverseProxyHealthCheckUnixSocketWithoutPort(t *testing.T) {
562563

563564
tester.AssertGetResponse("http://localhost:9080/", 200, "Hello, World!")
564565
}
566+
567+
// TestReverseProxyRetryMatchStatusCode verifies that lb_retry_match with a
568+
// CEL expression matching on {rp.status_code} causes the request to be
569+
// retried on the next upstream when the first upstream returns a matching
570+
// status code
571+
func TestReverseProxyRetryMatchStatusCode(t *testing.T) {
572+
// Bad upstream: returns 502
573+
badSrv := &http.Server{
574+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
575+
w.WriteHeader(http.StatusBadGateway)
576+
}),
577+
}
578+
badLn, err := net.Listen("tcp", "127.0.0.1:0")
579+
if err != nil {
580+
t.Fatalf("failed to listen: %v", err)
581+
}
582+
go badSrv.Serve(badLn)
583+
t.Cleanup(func() { badSrv.Close(); badLn.Close() })
584+
585+
// Good upstream: returns 200
586+
goodSrv := &http.Server{
587+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
588+
w.Write([]byte("ok"))
589+
}),
590+
}
591+
goodLn, err := net.Listen("tcp", "127.0.0.1:0")
592+
if err != nil {
593+
t.Fatalf("failed to listen: %v", err)
594+
}
595+
go goodSrv.Serve(goodLn)
596+
t.Cleanup(func() { goodSrv.Close(); goodLn.Close() })
597+
598+
tester := caddytest.NewTester(t)
599+
tester.InitServer(fmt.Sprintf(`
600+
{
601+
skip_install_trust
602+
admin localhost:2999
603+
http_port 9080
604+
https_port 9443
605+
grace_period 1ns
606+
}
607+
http://localhost:9080 {
608+
reverse_proxy %s %s {
609+
lb_policy round_robin
610+
lb_retries 1
611+
lb_retry_match {
612+
expression `+"`{rp.status_code} in [502, 503]`"+`
613+
}
614+
}
615+
}
616+
`, goodLn.Addr().String(), badLn.Addr().String()), "caddyfile")
617+
618+
tester.AssertGetResponse("http://localhost:9080/", 200, "ok")
619+
}
620+
621+
// TestReverseProxyRetryMatchHeader verifies that lb_retry_match with a CEL
622+
// expression matching on {rp.header.*} causes the request to be retried when
623+
// the upstream sets a matching response header
624+
func TestReverseProxyRetryMatchHeader(t *testing.T) {
625+
var badHits atomic.Int32
626+
627+
// Bad upstream: returns 200 but signals retry via header
628+
badSrv := &http.Server{
629+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
630+
badHits.Add(1)
631+
w.Header().Set("X-Upstream-Retry", "true")
632+
w.Write([]byte("bad"))
633+
}),
634+
}
635+
badLn, err := net.Listen("tcp", "127.0.0.1:0")
636+
if err != nil {
637+
t.Fatalf("failed to listen: %v", err)
638+
}
639+
go badSrv.Serve(badLn)
640+
t.Cleanup(func() { badSrv.Close(); badLn.Close() })
641+
642+
// Good upstream: returns 200 without retry header
643+
goodSrv := &http.Server{
644+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
645+
w.Write([]byte("good"))
646+
}),
647+
}
648+
goodLn, err := net.Listen("tcp", "127.0.0.1:0")
649+
if err != nil {
650+
t.Fatalf("failed to listen: %v", err)
651+
}
652+
go goodSrv.Serve(goodLn)
653+
t.Cleanup(func() { goodSrv.Close(); goodLn.Close() })
654+
655+
tester := caddytest.NewTester(t)
656+
tester.InitServer(fmt.Sprintf(`
657+
{
658+
skip_install_trust
659+
admin localhost:2999
660+
http_port 9080
661+
https_port 9443
662+
grace_period 1ns
663+
}
664+
http://localhost:9080 {
665+
reverse_proxy %s %s {
666+
lb_policy round_robin
667+
lb_retries 1
668+
lb_retry_match {
669+
expression `+"`{rp.header.X-Upstream-Retry} == \"true\"`"+`
670+
}
671+
}
672+
}
673+
`, goodLn.Addr().String(), badLn.Addr().String()), "caddyfile")
674+
675+
tester.AssertGetResponse("http://localhost:9080/", 200, "good")
676+
677+
if badHits.Load() != 1 {
678+
t.Errorf("bad upstream hits: got %d, want 1", badHits.Load())
679+
}
680+
}
681+
682+
// TestReverseProxyRetryMatchCombined verifies that a CEL expression combining
683+
// request path matching with response status code matching works correctly -
684+
// only retrying when both conditions are met
685+
func TestReverseProxyRetryMatchCombined(t *testing.T) {
686+
// Upstream: returns 502 for all requests
687+
srv := &http.Server{
688+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
689+
w.WriteHeader(http.StatusBadGateway)
690+
}),
691+
}
692+
ln, err := net.Listen("tcp", "127.0.0.1:0")
693+
if err != nil {
694+
t.Fatalf("failed to listen: %v", err)
695+
}
696+
go srv.Serve(ln)
697+
t.Cleanup(func() { srv.Close(); ln.Close() })
698+
699+
// Good upstream
700+
goodSrv := &http.Server{
701+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
702+
w.Write([]byte("ok"))
703+
}),
704+
}
705+
goodLn, err := net.Listen("tcp", "127.0.0.1:0")
706+
if err != nil {
707+
t.Fatalf("failed to listen: %v", err)
708+
}
709+
go goodSrv.Serve(goodLn)
710+
t.Cleanup(func() { goodSrv.Close(); goodLn.Close() })
711+
712+
tester := caddytest.NewTester(t)
713+
tester.InitServer(fmt.Sprintf(`
714+
{
715+
skip_install_trust
716+
admin localhost:2999
717+
http_port 9080
718+
https_port 9443
719+
grace_period 1ns
720+
}
721+
http://localhost:9080 {
722+
reverse_proxy %s %s {
723+
lb_policy round_robin
724+
lb_retries 1
725+
lb_retry_match {
726+
expression `+"`path('/retry*') && {rp.status_code} in [502, 503]`"+`
727+
}
728+
}
729+
}
730+
`, goodLn.Addr().String(), ln.Addr().String()), "caddyfile")
731+
732+
// /retry path matches the expression - should retry to good upstream
733+
tester.AssertGetResponse("http://localhost:9080/retry", 200, "ok")
734+
735+
// /other path does NOT match - should return the 502
736+
req, _ := http.NewRequest(http.MethodGet, "http://localhost:9080/other", nil)
737+
tester.AssertResponse(req, 502, "")
738+
}

‎modules/caddyhttp/reverseproxy/caddyfile.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
6767
// lb_retries <retries>
6868
// lb_try_duration <duration>
6969
// lb_try_interval <interval>
70-
// lb_retry_match <request-matcher>
70+
// lb_retry_match <matcher>
71+
// lb_retry_transport_errors <methods...>
7172
//
7273
// # active health checking
7374
// health_uri <uri>
@@ -332,6 +333,16 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
332333
}
333334
h.LoadBalancing.RetryMatchRaw = append(h.LoadBalancing.RetryMatchRaw, matcherSet)
334335

336+
case "lb_retry_transport_errors":
337+
args := d.RemainingArgs()
338+
if len(args) == 0 {
339+
return d.ArgErr()
340+
}
341+
if h.LoadBalancing == nil {
342+
h.LoadBalancing = new(LoadBalancing)
343+
}
344+
h.LoadBalancing.RetryTransportErrorMethods = append(h.LoadBalancing.RetryTransportErrorMethods, args...)
345+
335346
case "health_uri":
336347
if !d.NextArg() {
337348
return d.ArgErr()

0 commit comments

Comments
 (0)