From 403d2f6b251ed742c993a6840bb1c71cde5866e8 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 14 Feb 2024 17:15:58 +0000 Subject: [PATCH] backport of commit a74775814cd315a0c8a63844f0d822df072483dd --- .changelog/19969.txt | 3 + client/fingerprint/consul.go | 86 +++++++++++++- client/fingerprint/consul_test.go | 105 ++++++++++++++++++ .../test_fixtures/consul/agent_self_ent.json | 4 +- 4 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 .changelog/19969.txt diff --git a/.changelog/19969.txt b/.changelog/19969.txt new file mode 100644 index 000000000000..bc4fe6540601 --- /dev/null +++ b/.changelog/19969.txt @@ -0,0 +1,3 @@ +```release-note:improvement +fingerprint: Added a fingerprint for Consul DNS address and port +``` diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go index 987b44c66ec7..dff911ca5fe9 100644 --- a/client/fingerprint/consul.go +++ b/client/fingerprint/consul.go @@ -5,6 +5,7 @@ package fingerprint import ( "fmt" + "net/netip" "strconv" "strings" "time" @@ -13,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" @@ -165,6 +167,8 @@ func (cfs *consulFingerprintState) initialize(cfg *config.ConsulConfig, logger h "consul.grpc": cfs.grpc(consulConfig.Scheme, logger), "consul.ft.namespaces": cfs.namespaces, "consul.partition": cfs.partition, + "consul.dns.port": cfs.dnsPort, + "consul.dns.addr": cfs.dnsAddr(logger), } } else { cfs.extractors = map[string]consulExtractor{ @@ -178,6 +182,8 @@ func (cfs *consulFingerprintState) initialize(cfg *config.ConsulConfig, logger h fmt.Sprintf("consul.%s.grpc", cfg.Name): cfs.grpc(consulConfig.Scheme, logger), fmt.Sprintf("consul.%s.ft.namespaces", cfg.Name): cfs.namespaces, fmt.Sprintf("consul.%s.partition", cfg.Name): cfs.partition, + fmt.Sprintf("consul.%s.dns.port", cfg.Name): cfs.dnsPort, + fmt.Sprintf("consul.%s.dns.addr", cfg.Name): cfs.dnsAddr(logger), } } @@ -191,7 +197,7 @@ func (cfs *consulFingerprintState) query(logger hclog.Logger) agentconsul.Self { if err != nil { // indicate consul no longer available if cfs.isAvailable { - logger.Info("consul agent is unavailable: %v", err) + logger.Info("consul agent is unavailable", "error", err) } cfs.isAvailable = false cfs.nextCheck = time.Time{} // force check on next interval @@ -298,6 +304,84 @@ func (cfs *consulFingerprintState) grpcTLSPort(info agentconsul.Self) (string, b return fmt.Sprintf("%d", int(p)), ok } +func (cfs *consulFingerprintState) dnsPort(info agentconsul.Self) (string, bool) { + p, ok := info["DebugConfig"]["DNSPort"].(float64) + return fmt.Sprintf("%d", int(p)), ok +} + +// dnsAddr fingerprints the Consul DNS address, but only if Nomad can use it +// usefully to provide an iptables rule to a task +func (cfs *consulFingerprintState) dnsAddr(logger hclog.Logger) func(info agentconsul.Self) (string, bool) { + return func(info agentconsul.Self) (string, bool) { + + var listenOnEveryIP bool + + dnsAddrs, ok := info["DebugConfig"]["DNSAddrs"].([]any) + if !ok { + logger.Warn("Consul returned invalid addresses.dns config", + "value", info["DebugConfig"]["DNSAddrs"]) + return "", false + } + + for _, d := range dnsAddrs { + dnsAddr, ok := d.(string) + if !ok { + logger.Warn("Consul returned invalid addresses.dns config", + "value", info["DebugConfig"]["DNSAddrs"]) + return "", false + + } + dnsAddr = strings.TrimPrefix(dnsAddr, "tcp://") + dnsAddr = strings.TrimPrefix(dnsAddr, "udp://") + + parsed, err := netip.ParseAddrPort(dnsAddr) + if err != nil { + logger.Warn("could not parse Consul addresses.dns config", + "value", dnsAddr, "error", err) + 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 !parsed.Addr().IsLoopback() { + return parsed.Addr().String(), true + } + } + + // 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 query network interfaces", "error", err) + return "", false // something is very wrong, so bail out + } + if publicIP != "" { + return publicIP, true + } + } + + // 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 + } +} + func (cfs *consulFingerprintState) namespaces(info agentconsul.Self) (string, bool) { return strconv.FormatBool(agentconsul.Namespaces(info)), true } diff --git a/client/fingerprint/consul_test.go b/client/fingerprint/consul_test.go index a190d65b3c61..9cd110a00ea9 100644 --- a/client/fingerprint/consul_test.go +++ b/client/fingerprint/consul_test.go @@ -491,6 +491,105 @@ func TestConsulFingerprint_partition(t *testing.T) { }) } +func TestConsulFingerprint_dns(t *testing.T) { + ci.Parallel(t) + + cfs := consulFingerprintState{} + + t.Run("dns port not enabled", func(t *testing.T) { + port, ok := cfs.dnsPort(agentconsul.Self{ + "DebugConfig": {"DNSPort": -1.0}, // JSON numbers are floats + }) + must.True(t, ok) + must.Eq(t, "-1", port) + }) + + t.Run("non-default port value", func(t *testing.T) { + port, ok := cfs.dnsPort(agentconsul.Self{ + "DebugConfig": {"DNSPort": 8601.0}, // JSON numbers are floats + }) + must.True(t, ok) + must.Eq(t, "8601", port) + }) + + t.Run("missing port", func(t *testing.T) { + port, ok := cfs.dnsPort(agentconsul.Self{ + "DebugConfig": {}, + }) + must.False(t, ok) + must.Eq(t, "0", port) + }) + + t.Run("malformed port", func(t *testing.T) { + port, ok := cfs.dnsPort(agentconsul.Self{ + "DebugConfig": {"DNSPort": "A"}, + }) + must.False(t, ok) + must.Eq(t, "0", port) + }) + + t.Run("get first IP", func(t *testing.T) { + addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": { + "DNSAddrs": []any{"tcp://192.168.1.170:8601", "udp://192.168.1.171:8601"}, + }, + }) + must.True(t, ok) + must.Eq(t, "192.168.1.170", addr) + + addr, ok = cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": {"DNSAddrs": []any{"tcp://[2001:0db8:85a3::8a2e:0370:7334]:8601"}}, + }) + must.True(t, ok) + must.Eq(t, "2001:db8:85a3::8a2e:370:7334", addr) + }) + + t.Run("loopback address", func(t *testing.T) { + addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": { + "DNSAddrs": []any{"tcp://127.0.0.1:8601", "udp://127.0.0.1:8601"}, + }, + }) + must.True(t, ok) + must.Eq(t, "", addr) + + addr, ok = cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": {"DNSAddrs": []any{"tcp://[::1]:8601"}}, + }) + must.True(t, ok) + must.Eq(t, "", addr) + + }) + + t.Run("fallback to private or public IP", func(t *testing.T) { + addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": { + "DNSAddrs": []any{"tcp://0.0.0.0:8601", "udp://0.0.0.0:8601"}, + }, + }) + must.True(t, ok) + must.NotEq(t, "", addr) + }) + + t.Run("malformed DNSAddrs", func(t *testing.T) { + addr, ok := cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": {"DNSAddrs": []int{0}}}) + must.False(t, ok) + must.Eq(t, "", addr) + + addr, ok = cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": {"DNSAddrs": []any{0}}}) + must.False(t, ok) + must.Eq(t, "", addr) + + addr, ok = cfs.dnsAddr(testlog.HCLogger(t))(agentconsul.Self{ + "DebugConfig": {"DNSAddrs": []any{"tcp://XXXXX"}}}) + must.False(t, ok) + must.Eq(t, "", addr) + }) + +} + func TestConsulFingerprint_Fingerprint_oss(t *testing.T) { ci.Parallel(t) @@ -510,6 +609,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) { must.NoError(t, err) must.Eq(t, map[string]string{ "consul.datacenter": "dc1", + "consul.dns.port": "8600", "consul.revision": "3c1c22679", "consul.segment": "seg1", "consul.server": "true", @@ -564,6 +664,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) { "consul.version": "1.9.5", "consul.connect": "true", "consul.grpc": "8502", + "consul.dns.port": "8600", "consul.ft.namespaces": "false", "unique.consul.name": "HAL9000", }, resp3.Attributes) @@ -600,6 +701,8 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) { "consul.ft.namespaces": "true", "consul.connect": "true", "consul.grpc": "8502", + "consul.dns.addr": "192.168.1.117", + "consul.dns.port": "8600", "consul.partition": "default", "unique.consul.name": "HAL9000", }, resp.Attributes) @@ -649,6 +752,8 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) { "consul.ft.namespaces": "true", "consul.connect": "true", "consul.grpc": "8502", + "consul.dns.addr": "192.168.1.117", + "consul.dns.port": "8600", "consul.partition": "default", "unique.consul.name": "HAL9000", }, resp3.Attributes) diff --git a/client/fingerprint/test_fixtures/consul/agent_self_ent.json b/client/fingerprint/test_fixtures/consul/agent_self_ent.json index c045688cc239..833d7c88673a 100644 --- a/client/fingerprint/test_fixtures/consul/agent_self_ent.json +++ b/client/fingerprint/test_fixtures/consul/agent_self_ent.json @@ -129,8 +129,8 @@ "ConsulServerHealthInterval": "10ms", "DNSARecordLimit": 0, "DNSAddrs": [ - "tcp://127.0.0.1:8600", - "udp://127.0.0.1:8600" + "tcp://192.168.1.117:8600", + "udp://192.168.1.117:8600" ], "DNSAllowStale": true, "DNSAltDomain": "",