From 1fdda51839830ea84de48db94dab10dc8b4439e5 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 30 Jul 2019 09:56:56 -0400 Subject: [PATCH] Fix envoy canBind (#6238) * Fix envoy cli canBind function The string form of an Addr was including the CIDR causing the str equals to not match. * Remove debug prints --- command/connect/envoy/envoy.go | 30 ++++++--- command/connect/envoy/envoy_test.go | 99 ++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go index 89406ecd51..1075d05144 100644 --- a/command/connect/envoy/envoy.go +++ b/command/connect/envoy/envoy.go @@ -165,7 +165,8 @@ func parseAddress(addrStr string) (string, int, error) { return addr, port, nil } -func canBind(addr string) bool { +// canBindInternal is here mainly so we can unit test this with a constant net.Addr list +func canBindInternal(addr string, ifAddrs []net.Addr) bool { if addr == "" { return false } @@ -175,19 +176,32 @@ func canBind(addr string) bool { return false } + ipStr := ip.String() + + for _, addr := range ifAddrs { + switch v := addr.(type) { + case *net.IPNet: + if v.IP.String() == ipStr { + return true + } + default: + if addr.String() == ipStr { + return true + } + } + } + + return false +} + +func canBind(addr string) bool { ifAddrs, err := net.InterfaceAddrs() if err != nil { return false } - for _, addr := range ifAddrs { - if addr.String() == ip.String() { - return true - } - } - - return false + return canBindInternal(addr, ifAddrs) } func (c *cmd) Run(args []string) int { diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go index 73ec77aff7..2aa9a292e7 100644 --- a/command/connect/envoy/envoy_test.go +++ b/command/connect/envoy/envoy_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "flag" "io/ioutil" + "net" "net/http" "net/http/httptest" "os" @@ -20,7 +21,7 @@ import ( var update = flag.Bool("update", false, "update golden files") -func TestCatalogCommand_noTabs(t *testing.T) { +func TestEnvoyCommand_noTabs(t *testing.T) { t.Parallel() if strings.ContainsRune(New(nil).Help(), '\t') { t.Fatal("help has tabs") @@ -527,3 +528,99 @@ func testMockAgentProxyConfig(cfg map[string]interface{}) http.HandlerFunc { w.Write(cfgJSON) }) } + +func TestEnvoyCommand_canBindInternal(t *testing.T) { + t.Parallel() + type testCheck struct { + expected bool + addr string + } + + type testCase struct { + ifAddrs []net.Addr + checks map[string]testCheck + } + + parseIPNets := func(t *testing.T, in ...string) []net.Addr { + var out []net.Addr + for _, addr := range in { + ip := net.ParseIP(addr) + require.NotNil(t, ip) + out = append(out, &net.IPNet{IP: ip}) + } + return out + } + + parseIPs := func(t *testing.T, in ...string) []net.Addr { + var out []net.Addr + for _, addr := range in { + ip := net.ParseIP(addr) + require.NotNil(t, ip) + out = append(out, &net.IPAddr{IP: ip}) + } + return out + } + + cases := map[string]testCase{ + "IPNet": { + parseIPNets(t, "10.3.0.2", "198.18.0.1", "2001:db8:a0b:12f0::1"), + map[string]testCheck{ + "ipv4": { + true, + "10.3.0.2", + }, + "secondary ipv4": { + true, + "198.18.0.1", + }, + "ipv6": { + true, + "2001:db8:a0b:12f0::1", + }, + "ipv4 not found": { + false, + "1.2.3.4", + }, + "ipv6 not found": { + false, + "::ffff:192.168.0.1", + }, + }, + }, + "IPAddr": { + parseIPs(t, "10.3.0.2", "198.18.0.1", "2001:db8:a0b:12f0::1"), + map[string]testCheck{ + "ipv4": { + true, + "10.3.0.2", + }, + "secondary ipv4": { + true, + "198.18.0.1", + }, + "ipv6": { + true, + "2001:db8:a0b:12f0::1", + }, + "ipv4 not found": { + false, + "1.2.3.4", + }, + "ipv6 not found": { + false, + "::ffff:192.168.0.1", + }, + }, + }, + } + + for name, tcase := range cases { + t.Run(name, func(t *testing.T) { + for checkName, check := range tcase.checks { + t.Run(checkName, func(t *testing.T) { + require.Equal(t, check.expected, canBindInternal(check.addr, tcase.ifAddrs)) + }) + } + }) + } +}