reverseproxy: allow no port for SRV; fix regression in d55d50b (#3756)

* reverseproxy: fix breakage in handling SRV lookup introduced by 3695

* reverseproxy: validate against incompatible config options with lookup_srv

* reverseproxy: add integration test cases for validations involving lookup_srv

* reverseproxy: clarify the reason for skipping an iteration

* grammar.. Oxford comma

Co-authored-by: Francis Lavoie <lavofr@gmail.com>

Co-authored-by: Francis Lavoie <lavofr@gmail.com>

Fixes #3753
This commit is contained in:
Mohammed Al Sahaf 2020-10-01 23:05:39 +03:00 committed by GitHub
parent 3b9eae70c9
commit 6722426f1a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 129 additions and 10 deletions

View file

@ -13,6 +13,109 @@ import (
"github.com/caddyserver/caddy/v2/caddytest" "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) { func TestReverseProxyHealthCheck(t *testing.T) {
tester := caddytest.NewTester(t) tester := caddytest.NewTester(t)
tester.InitServer(` tester.InitServer(`

View file

@ -129,6 +129,21 @@ func (h *Handler) Provision(ctx caddy.Context) error {
h.ctx = ctx h.ctx = ctx
h.logger = ctx.Logger(h) h.logger = ctx.Logger(h)
// get validation out of the way
for i, v := range h.Upstreams {
// Having LookupSRV non-empty conflicts with 2 other config parameters: active health checks, and upstream dial address.
// Therefore if LookupSRV is empty, then there are no immediately apparent config conflicts, and the iteration can be skipped.
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 // start by loading modules
if h.TransportRaw != nil { if h.TransportRaw != nil {
mod, err := ctx.LoadModule(h, "TransportRaw") mod, err := ctx.LoadModule(h, "TransportRaw")
@ -204,17 +219,18 @@ func (h *Handler) Provision(ctx caddy.Context) error {
// set up upstreams // set up upstreams
for _, upstream := range h.Upstreams { for _, upstream := range h.Upstreams {
addr, err := caddy.ParseNetworkAddress(upstream.Dial) if upstream.LookupSRV == "" {
if err != nil { addr, err := caddy.ParseNetworkAddress(upstream.Dial)
return err if err != nil {
return err
}
if addr.PortRangeSize() != 1 {
return fmt.Errorf("multiple addresses (upstream must map to only one address): %v", addr)
}
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 // create or get the host representation for this upstream
var host Host = new(upstreamHost) var host Host = new(upstreamHost)
existingHost, loaded := hosts.LoadOrStore(upstream.String(), host) existingHost, loaded := hosts.LoadOrStore(upstream.String(), host)