Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reverseproxy: fix breakage in handling SRV lookup introduced by d55d50b #3756

Merged
merged 7 commits into from
Oct 1, 2020
103 changes: 103 additions & 0 deletions caddytest/integration/reverseproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,109 @@ import (
"github.com/caddyserver/caddy/v2/caddytest"
)

func TestSRVReverseProxy(t *testing.T) {
tester := caddytest.NewTester(t)
tester.InitServer(`
{
"apps": {
"http": {
"servers": {
"srv0": {
"listen": [
":8080"
],
"routes": [
{
"handle": [
{
"handler": "reverse_proxy",
"upstreams": [
{
"lookup_srv": "srv.host.service.consul"
}
]
}
]
}
]
}
}
}
}
}
`, "json")
}

func TestSRVWithDial(t *testing.T) {
caddytest.AssertLoadError(t, `
{
"apps": {
"http": {
"servers": {
"srv0": {
"listen": [
":8080"
],
"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\"}`)
}

func TestSRVWithActiveHealthcheck(t *testing.T) {
caddytest.AssertLoadError(t, `
{
"apps": {
"http": {
"servers": {
"srv0": {
"listen": [
":8080"
],
"routes": [
{
"handle": [
{
"handler": "reverse_proxy",
"health_checks": {
"active": {
"path": "/ok"
}
},
"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\"}`)
}

func TestReverseProxyHealthCheck(t *testing.T) {
tester := caddytest.NewTester(t)
tester.InitServer(`
Expand Down
33 changes: 24 additions & 9 deletions modules/caddyhttp/reverseproxy/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ func (h *Handler) Provision(ctx caddy.Context) error {
h.ctx = ctx
h.logger = ctx.Logger(h)

// get validation out of the way
for i, v := range h.Upstreams {
// the special cases arise with LookupSRV present, so skip the iteration if it's empty.
mholt marked this conversation as resolved.
Show resolved Hide resolved
if v.LookupSRV == "" {
continue
}
if h.HealthChecks != nil && h.HealthChecks.Active != nil {
return fmt.Errorf(`upstream: lookup_srv is incompatible with active health checks: %d: {"dial": %q, "lookup_srv": %q}`, i, v.Dial, v.LookupSRV)
}
if v.Dial != "" {
return fmt.Errorf(`upstream: specifying dial address is incompatible with lookup_srv: %d: {"dial": %q, "lookup_srv": %q}`, i, v.Dial, v.LookupSRV)
}
}

// start by loading modules
if h.TransportRaw != nil {
mod, err := ctx.LoadModule(h, "TransportRaw")
Expand Down Expand Up @@ -204,17 +218,18 @@ func (h *Handler) Provision(ctx caddy.Context) error {

// set up upstreams
for _, upstream := range h.Upstreams {
addr, err := caddy.ParseNetworkAddress(upstream.Dial)
if err != nil {
return err
}

if addr.PortRangeSize() != 1 {
return fmt.Errorf("multiple addresses (upstream must map to only one address): %v", addr)
}
if upstream.LookupSRV == "" {
addr, err := caddy.ParseNetworkAddress(upstream.Dial)
if err != nil {
return err
}

upstream.networkAddress = addr
if addr.PortRangeSize() != 1 {
return fmt.Errorf("multiple addresses (upstream must map to only one address): %v", addr)
}

upstream.networkAddress = addr
}
// create or get the host representation for this upstream
var host Host = new(upstreamHost)
existingHost, loaded := hosts.LoadOrStore(upstream.String(), host)
Expand Down