Skip to content

Commit

Permalink
address comments from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
tgross committed Feb 14, 2024
1 parent 0725001 commit 17688c5
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 37 deletions.
57 changes: 35 additions & 22 deletions client/fingerprint/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/hashicorp/go-hclog"
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-sockaddr"
"github.com/hashicorp/go-version"
agentconsul "github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/helper"
Expand Down Expand Up @@ -313,47 +314,59 @@ func (cfs *consulFingerprintState) dnsPort(info agentconsul.Self) (string, bool)
func (cfs *consulFingerprintState) dnsAddr(logger hclog.Logger) func(info agentconsul.Self) (string, bool) {
return func(info agentconsul.Self) (string, bool) {

// only addresses we can use for an iptables rule from a container to the
// host will be fingerprinted
isValidForTaskUse := func(addr netip.Addr) (string, bool) {
if !addr.IsLoopback() && !addr.IsUnspecified() && addr.IsValid() {
return addr.String(), true
}
return "", false
}
var listenOnEveryIP bool

// first try to find an explicitly configured address
dnsAddrs, ok := info["DebugConfig"]["DNSAddrs"].([]string)
if ok {
for _, dnsAddr := range dnsAddrs {
dnsAddr = strings.TrimPrefix(dnsAddr, "tcp://")
dnsAddr = strings.TrimPrefix(dnsAddr, "udp://")

parsed, err := netip.ParseAddrPort(dnsAddr)
if err != nil {
if err != nil || !parsed.IsValid() {
logger.Warn("could not parse Consul addresses.dns config", "value", dnsAddr)
return "", false
return "", false // response is somehow malformed
}

// only addresses we can use for an iptables rule from a
// container to the host will be fingerprinted
if parsed.Addr().IsUnspecified() {
listenOnEveryIP = true
break
}
if val, ok := isValidForTaskUse(parsed.Addr()); ok {
return val, true
if !parsed.Addr().IsLoopback() {
return parsed.Addr().String(), true
}
}
}

// fallback to the bind address
bindAddr, ok := info["DebugConfig"]["BindAddr"].(string)
if ok {
parsed, err := netip.ParseAddr(bindAddr)
// if Consul DNS is bound on 0.0.0.0, we want to fingerprint the private
// IP (or at worst, the public IP) of the host so that we have a valid
// IP address for the iptables rule
if listenOnEveryIP {

privateIP, err := sockaddr.GetPrivateIP()
if err != nil {
logger.Warn("could not query network interfaces", "error", err)
return "", false // something is very wrong, so bail out
}
if privateIP != "" {
return privateIP, true
}
publicIP, err := sockaddr.GetPublicIP()
if err != nil {
logger.Warn("could not parse Consul bind_addr config", "value", bindAddr)
return "", false
logger.Warn("could not query network interfaces", "error", err)
return "", false // something is very wrong, so bail out
}
if val, ok := isValidForTaskUse(parsed); ok {
return val, true
if publicIP != "" {
return publicIP, true
}
}

return "", true // we can't fingerprint a useful value
// if we've hit here, Consul is bound on localhost and we won't be able
// to configure container DNS to use it, but we also don't want to have
// the fingerprinter return an error
return "", true
}
}

Expand Down
17 changes: 2 additions & 15 deletions client/fingerprint/consul_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,37 +528,24 @@ func TestConsulFingerprint_dns(t *testing.T) {
must.Eq(t, "0", port)
})

t.Run("bind on 0.0.0.0", func(t *testing.T) {
addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{
"DebugConfig": {
"DNSAddrs": []string{"tcp://0.0.0.0:8601", "udp://0.0.0.0:8601"},
"BindAddr": "0.0.0.0",
},
})
must.True(t, ok)
must.Eq(t, "", addr)
})

t.Run("get first IP", func(t *testing.T) {
addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{
"DebugConfig": {
"DNSAddrs": []string{"tcp://192.168.1.170:8601", "udp://192.168.1.171:8601"},
"BindAddr": "192.168.1.172",
},
})
must.True(t, ok)
must.Eq(t, "192.168.1.170", addr)
})

t.Run("fallback to bind_addr", func(t *testing.T) {
t.Run("fallback to private or public IP", func(t *testing.T) {
addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{
"DebugConfig": {
"DNSAddrs": []string{"tcp://0.0.0.0:8601", "udp://0.0.0.0:8601"},
"BindAddr": "192.168.1.172",
},
})
must.True(t, ok)
must.Eq(t, "192.168.1.172", addr)
must.NotEq(t, "", addr)
})
}

Expand Down

0 comments on commit 17688c5

Please sign in to comment.