Merge pull request #4381 from hashicorp/proxy-check-default

Proxy check default
This commit is contained in:
Paul Banks 2018-07-12 17:08:35 +01:00 committed by GitHub
commit 6fe7faa554
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 165 additions and 51 deletions

View File

@ -2139,13 +2139,17 @@ func (a *Agent) AddProxy(proxy *structs.ConnectManagedProxy, persist bool,
if err != nil {
return err
}
chkTypes := []*structs.CheckType{
&structs.CheckType{
Name: "Connect Proxy Listening",
TCP: fmt.Sprintf("%s:%d", proxyCfg["bind_address"],
proxyCfg["bind_port"]),
Interval: 10 * time.Second,
},
chkAddr := a.resolveProxyCheckAddress(proxyCfg)
chkTypes := []*structs.CheckType{}
if chkAddr != "" {
chkTypes = []*structs.CheckType{
&structs.CheckType{
Name: "Connect Proxy Listening",
TCP: fmt.Sprintf("%s:%d", chkAddr,
proxyCfg["bind_port"]),
Interval: 10 * time.Second,
},
}
}
err = a.AddService(proxyService, chkTypes, persist, token)
@ -2162,6 +2166,44 @@ func (a *Agent) AddProxy(proxy *structs.ConnectManagedProxy, persist bool,
return nil
}
// resolveProxyCheckAddress returns the best address to use for a TCP check of
// the proxy's public listener. It expects the input to already have default
// values populated by applyProxyConfigDefaults. It may return an empty string
// indicating that the TCP check should not be created at all.
//
// By default this uses the proxy's bind address which in turn defaults to the
// agent's bind address. If the proxy bind address ends up being 0.0.0.0 we have
// to assume the agent can dial it over loopback which is usually true.
//
// In some topologies such as proxy being in a different container, the IP the
// agent used to dial proxy over a local bridge might not be the same as the
// container's public routable IP address so we allow a manual override of the
// check address in config "tcp_check_address" too.
//
// Finally the TCP check can be disabled by another manual override
// "disable_tcp_check" in cases where the agent will never be able to dial the
// proxy directly for some reason.
func (a *Agent) resolveProxyCheckAddress(proxyCfg map[string]interface{}) string {
// If user disabled the check return empty string
if disable, ok := proxyCfg["disable_tcp_check"].(bool); ok && disable {
return ""
}
// If user specified a custom one, use that
if chkAddr, ok := proxyCfg["tcp_check_address"].(string); ok && chkAddr != "" {
return chkAddr
}
// If we have a bind address and its diallable, use that
if bindAddr, ok := proxyCfg["bind_address"].(string); ok &&
bindAddr != "" && bindAddr != "0.0.0.0" && bindAddr != "[::]" {
return bindAddr
}
// Default to localhost
return "127.0.0.1"
}
// applyProxyConfigDefaults takes a *structs.ConnectManagedProxy and returns
// it's Config map merged with any defaults from the Agent's config. It would be
// nicer if this were defined as a method on structs.ConnectManagedProxy but we

View File

@ -2567,30 +2567,6 @@ func TestAgent_reloadWatchesHTTPS(t *testing.T) {
func TestAgent_AddProxy(t *testing.T) {
t.Parallel()
a := NewTestAgent(t.Name(), `
node_name = "node1"
connect {
proxy_defaults {
exec_mode = "script"
daemon_command = ["foo", "bar"]
script_command = ["bar", "foo"]
}
}
ports {
proxy_min_port = 20000
proxy_max_port = 20000
}
`)
defer a.Shutdown()
// Register a target service we can use
reg := &structs.NodeService{
Service: "web",
Port: 8080,
}
require.NoError(t, a.AddService(reg, nil, false, ""))
tests := []struct {
desc string
@ -2621,7 +2597,10 @@ func TestAgent_AddProxy(t *testing.T) {
},
TargetServiceID: "web",
},
wantErr: false,
// Proxy will inherit agent's 0.0.0.0 bind address but we can't check that
// so we should default to localhost in that case.
wantTCPCheck: "127.0.0.1:20000",
wantErr: false,
},
{
desc: "default global exec mode",
@ -2634,7 +2613,8 @@ func TestAgent_AddProxy(t *testing.T) {
Command: []string{"consul", "connect", "proxy"},
TargetServiceID: "web",
},
wantErr: false,
wantTCPCheck: "127.0.0.1:20000",
wantErr: false,
},
{
desc: "default daemon command",
@ -2647,7 +2627,8 @@ func TestAgent_AddProxy(t *testing.T) {
Command: []string{"foo", "bar"},
TargetServiceID: "web",
},
wantErr: false,
wantTCPCheck: "127.0.0.1:20000",
wantErr: false,
},
{
desc: "default script command",
@ -2660,7 +2641,8 @@ func TestAgent_AddProxy(t *testing.T) {
Command: []string{"bar", "foo"},
TargetServiceID: "web",
},
wantErr: false,
wantTCPCheck: "127.0.0.1:20000",
wantErr: false,
},
{
desc: "managed proxy with custom bind port",
@ -2677,7 +2659,6 @@ func TestAgent_AddProxy(t *testing.T) {
wantTCPCheck: "127.10.10.10:1234",
wantErr: false,
},
{
// This test is necessary since JSON and HCL both will parse
// numbers as a float64.
@ -2695,12 +2676,83 @@ func TestAgent_AddProxy(t *testing.T) {
wantTCPCheck: "127.10.10.10:1234",
wantErr: false,
},
{
desc: "managed proxy with overridden but unspecified ipv6 bind address",
proxy: &structs.ConnectManagedProxy{
ExecMode: structs.ProxyExecModeDaemon,
Command: []string{"consul", "connect", "proxy"},
Config: map[string]interface{}{
"foo": "bar",
"bind_address": "[::]",
},
TargetServiceID: "web",
},
wantTCPCheck: "127.0.0.1:20000",
wantErr: false,
},
{
desc: "managed proxy with overridden check address",
proxy: &structs.ConnectManagedProxy{
ExecMode: structs.ProxyExecModeDaemon,
Command: []string{"consul", "connect", "proxy"},
Config: map[string]interface{}{
"foo": "bar",
"tcp_check_address": "127.20.20.20",
},
TargetServiceID: "web",
},
wantTCPCheck: "127.20.20.20:20000",
wantErr: false,
},
{
desc: "managed proxy with disabled check",
proxy: &structs.ConnectManagedProxy{
ExecMode: structs.ProxyExecModeDaemon,
Command: []string{"consul", "connect", "proxy"},
Config: map[string]interface{}{
"foo": "bar",
"disable_tcp_check": true,
},
TargetServiceID: "web",
},
wantTCPCheck: "",
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
require := require.New(t)
a := NewTestAgent(t.Name(), `
node_name = "node1"
# Explicit test because proxies inheriting this value must have a health
# check on a different IP.
bind_addr = "0.0.0.0"
connect {
proxy_defaults {
exec_mode = "script"
daemon_command = ["foo", "bar"]
script_command = ["bar", "foo"]
}
}
ports {
proxy_min_port = 20000
proxy_max_port = 20000
}
`)
defer a.Shutdown()
// Register a target service we can use
reg := &structs.NodeService{
Service: "web",
Port: 8080,
}
require.NoError(a.AddService(reg, nil, false, ""))
err := a.AddProxy(tt.proxy, false, "")
if tt.wantErr {
require.Error(err)
@ -2719,23 +2771,23 @@ func TestAgent_AddProxy(t *testing.T) {
// Ensure a TCP check was created for the service.
gotCheck := a.State.Check("service:web-proxy")
require.NotNil(gotCheck)
require.Equal("Connect Proxy Listening", gotCheck.Name)
if tt.wantTCPCheck == "" {
require.Nil(gotCheck)
} else {
require.NotNil(gotCheck)
require.Equal("Connect Proxy Listening", gotCheck.Name)
// Confusingly, a.State.Check("service:web-proxy") will return the state
// but it's Definition field will be empty. This appears to be expected
// when adding Checks as part of `AddService`. Notice how `AddService`
// tests in this file don't assert on that state but instead look at the
// agent's check state directly to ensure the right thing was registered.
// We'll do the same for now.
gotTCP, ok := a.checkTCPs["service:web-proxy"]
require.True(ok)
wantTCPCheck := tt.wantTCPCheck
if wantTCPCheck == "" {
wantTCPCheck = "127.0.0.1:20000"
// Confusingly, a.State.Check("service:web-proxy") will return the state
// but it's Definition field will be empty. This appears to be expected
// when adding Checks as part of `AddService`. Notice how `AddService`
// tests in this file don't assert on that state but instead look at the
// agent's check state directly to ensure the right thing was registered.
// We'll do the same for now.
gotTCP, ok := a.checkTCPs["service:web-proxy"]
require.True(ok)
require.Equal(tt.wantTCPCheck, gotTCP.TCP)
require.Equal(10*time.Second, gotTCP.Interval)
}
require.Equal(wantTCPCheck, gotTCP.TCP)
require.Equal(10*time.Second, gotTCP.Interval)
})
}
}

View File

@ -62,6 +62,8 @@ described here, the rest of the service definition is shown for context and is
"config": {
"bind_address": "0.0.0.0",
"bind_port": 20000,
"tcp_check_address": "192.168.0.1",
"disable_tcp_check": false,
"local_service_address": "127.0.0.1:1234",
"local_connect_timeout_ms": 1000,
"handshake_timeout_ms": 10000,
@ -84,6 +86,8 @@ described here, the rest of the service definition is shown for context and is
#### Configuration Key Reference
All fields are optional with a sane default.
* <a name="bind_address"></a><a href="#bind_address">`bind_address`</a> -
The address the proxy will bind it's _public_ mTLS listener to. It
defaults to the same address the agent binds to.
@ -94,6 +98,22 @@ described here, the rest of the service definition is shown for context and is
range](/docs/agent/options.html#proxy_min_port) if available. By default the
range is [20000, 20255] and the port is selected at random from that range.
* <a name="tcp_check_address"></a><a
href="#tcp_check_address">`tcp_check_address`</a> - The address the agent will
run a [TCP health check](/docs/agent/checks.html) against. By default this is
the same as the proxy's [bind address](#bind_address) except if the
bind_address is `0.0.0.0` or `[::]` in which case this defaults to `127.0.0.1`
and assumes the agent can dial the proxy over loopback. For more complex
configurations where agent and proxy communicate over a bridge for example,
this configuration can be used to specify a different _address_ (but not port)
for the agent to use for health checks if it can't talk to the proxy over
localhost or it's publicly advertised port. The check always uses the same
port that the proxy is bound to.
* <a name="disable_tcp_check"></a><a
href="#disable_tcp_check">`disable_tcp_check`</a> - If true, this disables a
TCP check being setup for the proxy. Default is false.
* <a name="local_service_address"></a><a href="#local_service_address">`local_service_address`</a> - The
`[address]:port` that the proxy should use to connect to the local application
instance. By default it assumes `127.0.0.1` as the address and takes the port