From 19a6551de2b02c9b56483a34fdcc0fcc36dcc979 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 20 Jun 2017 08:49:38 +0100 Subject: [PATCH 1/4] ignore local IPv6 addresses/networks There is no point in paying attention to them since scope connection tracking only deals in IPv4. --- probe/host/reporter.go | 5 ++--- report/networks.go | 18 ++++-------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/probe/host/reporter.go b/probe/host/reporter.go index 3aff7a602..927b57ce4 100644 --- a/probe/host/reporter.go +++ b/probe/host/reporter.go @@ -93,9 +93,8 @@ var GetLocalNetworks = func() ([]*net.IPNet, error) { } localNets := report.Networks{} for _, addr := range addrs { - // Not all addrs are IPNets. - if ipNet, ok := addr.(*net.IPNet); ok { - localNets = append(localNets, ipNet) + if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { + localNets = append(localNets, ipnet) } } return localNets, nil diff --git a/report/networks.go b/report/networks.go index 42efd5e4b..f7e2fde39 100644 --- a/report/networks.go +++ b/report/networks.go @@ -52,12 +52,9 @@ func LocalAddresses() ([]net.IP, error) { } for _, addr := range addrs { - ipnet, ok := addr.(*net.IPNet) - if !ok { - continue + if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { + result = append(result, ipnet.IP) } - - result = append(result, ipnet.IP) } } @@ -78,16 +75,9 @@ func AddLocalBridge(name string) error { return err } for _, addr := range addrs { - _, network, err := net.ParseCIDR(addr.String()) - if err != nil { - return err + if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { + LocalNetworks = append(LocalNetworks, ipnet) } - - if network == nil { - continue - } - - LocalNetworks = append(LocalNetworks, network) } return nil From 4e0065a57d686ca35672b916ad0489ad6b1e46aa Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 20 Jun 2017 09:23:52 +0100 Subject: [PATCH 2/4] refactor: put all network detection code in one place --- probe/host/reporter.go | 15 +-------------- report/networks.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/probe/host/reporter.go b/probe/host/reporter.go index 927b57ce4..1f0ab5ce2 100644 --- a/probe/host/reporter.go +++ b/probe/host/reporter.go @@ -2,7 +2,6 @@ package host import ( "fmt" - "net" "runtime" "sync" "time" @@ -86,19 +85,7 @@ func NewReporter(hostID, hostName, probeID, version string, pipes controls.PipeC func (*Reporter) Name() string { return "Host" } // GetLocalNetworks is exported for mocking -var GetLocalNetworks = func() ([]*net.IPNet, error) { - addrs, err := net.InterfaceAddrs() - if err != nil { - return nil, err - } - localNets := report.Networks{} - for _, addr := range addrs { - if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { - localNets = append(localNets, ipnet) - } - } - return localNets, nil -} +var GetLocalNetworks = report.GetLocalNetworks // Report implements Reporter. func (r *Reporter) Report() (report.Report, error) { diff --git a/report/networks.go b/report/networks.go index f7e2fde39..334a3da9d 100644 --- a/report/networks.go +++ b/report/networks.go @@ -82,3 +82,18 @@ func AddLocalBridge(name string) error { return nil } + +// GetLocalNetworks returns all the local networks. +func GetLocalNetworks() ([]*net.IPNet, error) { + addrs, err := net.InterfaceAddrs() + if err != nil { + return nil, err + } + localNets := Networks{} + for _, addr := range addrs { + if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { + localNets = append(localNets, ipnet) + } + } + return localNets, nil +} From 7e5704b53b13c6fe234ed6776ed5c674bec0a52e Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 20 Jun 2017 10:04:05 +0100 Subject: [PATCH 3/4] fix tests ...by removing them. It was a ridiculous amount of contorted code to test some utterly trivial functionality that is largely provided by the golang stdlib. --- report/networks.go | 16 +++++----------- report/networks_test.go | 41 ----------------------------------------- 2 files changed, 5 insertions(+), 52 deletions(-) diff --git a/report/networks.go b/report/networks.go index 334a3da9d..c177743bf 100644 --- a/report/networks.go +++ b/report/networks.go @@ -8,17 +8,11 @@ import ( // Networks represent a set of subnets type Networks []*net.IPNet -// Interface is exported for testing. -type Interface interface { - Addrs() ([]net.Addr, error) -} - -// Variables exposed for testing. +// LocalNetworks helps in determining which addresses a probe reports +// as being host-scoped. +// // TODO this design is broken, make it consistent with probe networks. -var ( - LocalNetworks = Networks{} - InterfaceByNameStub = func(name string) (Interface, error) { return net.InterfaceByName(name) } -) +var LocalNetworks = Networks{} // Contains returns true if IP is in Networks. func (n Networks) Contains(ip net.IP) bool { @@ -65,7 +59,7 @@ func LocalAddresses() ([]net.IP, error) { // supplied, such that MakeAddressNodeID will scope addresses in this subnet // as local. func AddLocalBridge(name string) error { - inf, err := InterfaceByNameStub(name) + inf, err := net.InterfaceByName(name) if err != nil { return err } diff --git a/report/networks_test.go b/report/networks_test.go index a199fcf82..47ca616ac 100644 --- a/report/networks_test.go +++ b/report/networks_test.go @@ -4,9 +4,7 @@ import ( "net" "testing" - "github.com/weaveworks/common/test" "github.com/weaveworks/scope/report" - "github.com/weaveworks/scope/test/reflect" ) func TestContains(t *testing.T) { @@ -31,42 +29,3 @@ func mustParseCIDR(s string) *net.IPNet { } return ipNet } - -type mockInterface struct { - addrs []net.Addr -} - -type mockAddr string - -func (m mockInterface) Addrs() ([]net.Addr, error) { - return m.addrs, nil -} - -func (m mockAddr) Network() string { - return "ip+net" -} - -func (m mockAddr) String() string { - return string(m) -} - -func TestAddLocal(t *testing.T) { - oldInterfaceByNameStub := report.InterfaceByNameStub - defer func() { report.InterfaceByNameStub = oldInterfaceByNameStub }() - - report.InterfaceByNameStub = func(name string) (report.Interface, error) { - return mockInterface{[]net.Addr{mockAddr("52.53.54.55/16")}}, nil - } - - err := report.AddLocalBridge("foo") - if err != nil { - t.Errorf("%v", err) - } - - want := report.Networks([]*net.IPNet{mustParseCIDR("52.53.54.55/16")}) - have := report.LocalNetworks - - if !reflect.DeepEqual(want, have) { - t.Errorf("%s", test.Diff(want, have)) - } -} From 1ef56f6af6b25068b4265610801e806913b9db7c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 20 Jun 2017 11:03:06 +0100 Subject: [PATCH 4/4] refactor: extract common code --- report/networks.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/report/networks.go b/report/networks.go index c177743bf..c2797760d 100644 --- a/report/networks.go +++ b/report/networks.go @@ -45,10 +45,8 @@ func LocalAddresses() ([]net.IP, error) { return []net.IP{}, err } - for _, addr := range addrs { - if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { - result = append(result, ipnet.IP) - } + for _, ipnet := range ipv4Nets(addrs) { + result = append(result, ipnet.IP) } } @@ -68,11 +66,8 @@ func AddLocalBridge(name string) error { if err != nil { return err } - for _, addr := range addrs { - if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { - LocalNetworks = append(LocalNetworks, ipnet) - } - } + + LocalNetworks = ipv4Nets(addrs) return nil } @@ -83,11 +78,15 @@ func GetLocalNetworks() ([]*net.IPNet, error) { if err != nil { return nil, err } - localNets := Networks{} + return ipv4Nets(addrs), nil +} + +func ipv4Nets(addrs []net.Addr) []*net.IPNet { + nets := Networks{} for _, addr := range addrs { if ipnet, ok := addr.(*net.IPNet); ok && ipnet.IP.To4() != nil { - localNets = append(localNets, ipnet) + nets = append(nets, ipnet) } } - return localNets, nil + return nets }