From e3909cc385f68a636cc8675dc12f6cd5185d722d Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Fri, 24 Feb 2023 22:54:04 +0300 Subject: [PATCH] reverseproxy: refactor HTTP transport layer (#5369) Co-authored-by: Francis Lavoie Co-authored-by: Weidi Deng --- caddytest/integration/reverseproxy_test.go | 268 +++++++++--------- .../caddyhttp/reverseproxy/httptransport.go | 20 +- 2 files changed, 148 insertions(+), 140 deletions(-) diff --git a/caddytest/integration/reverseproxy_test.go b/caddytest/integration/reverseproxy_test.go index f7b19672..4333d129 100644 --- a/caddytest/integration/reverseproxy_test.go +++ b/caddytest/integration/reverseproxy_test.go @@ -22,39 +22,39 @@ func TestSRVReverseProxy(t *testing.T) { }, "apps": { "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false + "certificate_authorities": { + "local": { + "install_trust": false } } }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":8080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "upstreams": [ - { - "lookup_srv": "srv.host.service.consul" - } + "http": { + "grace_period": 1, + "servers": { + "srv0": { + "listen": [ + ":18080" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "upstreams": [ + { + "lookup_srv": "srv.host.service.consul" + } + ] + } + ] + } ] - } - ] - } - ] - } + } + } } - } } - } - `, "json") + } + `, "json") } func TestSRVWithDial(t *testing.T) { @@ -62,39 +62,39 @@ func TestSRVWithDial(t *testing.T) { { "apps": { "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false - } + "certificate_authorities": { + "local": { + "install_trust": false + } } - }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":8080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "upstreams": [ - { - "dial": "tcp/address.to.upstream:80", - "lookup_srv": "srv.host.service.consul" - } + }, + "http": { + "grace_period": 1, + "servers": { + "srv0": { + "listen": [ + ":18080" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "tcp/address.to.upstream:80", + "lookup_srv": "srv.host.service.consul" + } + ] + } + ] + } ] - } - ] - } - ] - } + } + } } - } } - } + } `, "json", `upstream: specifying dial address is incompatible with lookup_srv: 0: {\"dial\": \"tcp/address.to.upstream:80\", \"lookup_srv\": \"srv.host.service.consul\"}`) } @@ -138,41 +138,41 @@ func TestDialWithPlaceholderUnix(t *testing.T) { }, "apps": { "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false - } + "certificate_authorities": { + "local": { + "install_trust": false + } } - }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":8080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "upstreams": [ - { - "dial": "unix/{http.request.header.X-Caddy-Upstream-Dial}" - } + }, + "http": { + "grace_period": 1, + "servers": { + "srv0": { + "listen": [ + ":18080" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "unix/{http.request.header.X-Caddy-Upstream-Dial}" + } + ] + } + ] + } ] - } - ] - } - ] - } - } - } + } + } + } } - } + } `, "json") - req, err := http.NewRequest(http.MethodGet, "http://localhost:8080", nil) + req, err := http.NewRequest(http.MethodGet, "http://localhost:18080", nil) if err != nil { t.Fail() return @@ -190,18 +190,18 @@ func TestReverseProxyWithPlaceholderDialAddress(t *testing.T) { }, "apps": { "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false - } + "certificate_authorities": { + "local": { + "install_trust": false + } } - }, + }, "http": { "grace_period": 1, "servers": { "srv0": { "listen": [ - ":8080" + ":18080" ], "routes": [ { @@ -264,14 +264,14 @@ func TestReverseProxyWithPlaceholderDialAddress(t *testing.T) { } } } - `, "json") + `, "json") req, err := http.NewRequest(http.MethodGet, "http://localhost:9080", nil) if err != nil { t.Fail() return } - req.Header.Set("X-Caddy-Upstream-Dial", "localhost:8080") + req.Header.Set("X-Caddy-Upstream-Dial", "localhost:18080") tester.AssertResponse(req, 200, "Hello, World!") } @@ -284,18 +284,18 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) { }, "apps": { "pki": { - "certificate_authorities" : { - "local" : { - "install_trust": false - } + "certificate_authorities": { + "local": { + "install_trust": false + } } - }, + }, "http": { "grace_period": 1, "servers": { "srv0": { "listen": [ - ":8080" + ":18080" ], "routes": [ { @@ -340,7 +340,7 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) { "handler": "reverse_proxy", "upstreams": [ { - "dial": "tcp/{http.request.header.X-Caddy-Upstream-Dial}:8080" + "dial": "tcp/{http.request.header.X-Caddy-Upstream-Dial}:18080" } ] } @@ -358,7 +358,7 @@ func TestReverseProxyWithPlaceholderTCPDialAddress(t *testing.T) { } } } - `, "json") + `, "json") req, err := http.NewRequest(http.MethodGet, "http://localhost:9080", nil) if err != nil { @@ -375,42 +375,42 @@ func TestSRVWithActiveHealthcheck(t *testing.T) { "apps": { "pki": { "certificate_authorities" : { - "local" : { - "install_trust": false - } + "local" : { + "install_trust": false + } } - }, - "http": { - "grace_period": 1, - "servers": { - "srv0": { - "listen": [ - ":8080" - ], - "routes": [ - { - "handle": [ - { - "handler": "reverse_proxy", - "health_checks": { - "active": { - "path": "/ok" + }, + "http": { + "grace_period": 1, + "servers": { + "srv0": { + "listen": [ + ":18080" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "health_checks": { + "active": { + "path": "/ok" + } + }, + "upstreams": [ + { + "lookup_srv": "srv.host.service.consul" + } + ] + } + ] } - }, - "upstreams": [ - { - "lookup_srv": "srv.host.service.consul" - } ] - } - ] - } - ] - } + } + } } - } } - } + } `, "json", `upstream: lookup_srv is incompatible with active health checks: 0: {\"dial\": \"\", \"lookup_srv\": \"srv.host.service.consul\"}`) } @@ -440,7 +440,7 @@ func TestReverseProxyHealthCheck(t *testing.T) { health_timeout 100ms } } - `, "caddyfile") + `, "caddyfile") time.Sleep(100 * time.Millisecond) // TODO: for some reason this test seems particularly flaky, getting 503 when it should be 200, unless we wait tester.AssertGetResponse("http://localhost:9080/", 200, "Hello, World!") diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index ec5d2f23..71d06d5a 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -172,12 +172,19 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e } } - // Set up the dialer to pull the correct information from the context dialContext := func(ctx context.Context, network, address string) (net.Conn, error) { - // the proper dialing information should be embedded into the request's context + // For unix socket upstreams, we need to recover the dial info from + // the request's context, because the Host on the request's URL + // will have been modified by directing the request, overwriting + // the unix socket filename. + // Also, we need to avoid overwriting the address at this point + // when not necessary, because http.ProxyFromEnvironment may have + // modified the address according to the user's env proxy config. if dialInfo, ok := GetDialInfo(ctx); ok { - network = dialInfo.Network - address = dialInfo.Address + if strings.HasPrefix(dialInfo.Network, "unix") { + network = dialInfo.Network + address = dialInfo.Address + } } conn, err := dialer.DialContext(ctx, network, address) @@ -188,8 +195,8 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e return nil, DialError{err} } - // if read/write timeouts are configured and this is a TCP connection, enforce the timeouts - // by wrapping the connection with our own type + // if read/write timeouts are configured and this is a TCP connection, + // enforce the timeouts by wrapping the connection with our own type if tcpConn, ok := conn.(*net.TCPConn); ok && (h.ReadTimeout > 0 || h.WriteTimeout > 0) { conn = &tcpRWTimeoutConn{ TCPConn: tcpConn, @@ -203,6 +210,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e } rt := &http.Transport{ + Proxy: http.ProxyFromEnvironment, DialContext: dialContext, MaxConnsPerHost: h.MaxConnsPerHost, ResponseHeaderTimeout: time.Duration(h.ResponseHeaderTimeout),