github
/
consul
mirror of https://github.com/hashicorp/consul
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Default managed proxy TCP check address sanely when proxy is bound to 0.0.0.0. 

This also provides a mechanism to configure custom address or disable the check entirely from managed proxy config.
pull/4381/head
Paul Banks 2018-07-12 12:57:10 +01:00
parent c13e0dbe8c
commit bb9a5c703b
No known key found for this signature in database
GPG Key ID: C25A851A849B8221
2 changed files with 131 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
agent/agent.go
View File

@ -2132,13 +2132,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)
@ -2155,6 +2159,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

126
agent/agent_test.go
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,69 @@ func TestAgent_AddProxy(t *testing.T) {
wantTCPCheck: "127.10.10.10:1234",
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 +2757,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)
})
}
}