From 01f6a215e34b15c38539c999f61d1ace4d4de374 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Mon, 1 Nov 2021 14:07:53 +0000 Subject: [PATCH 1/7] Clarifying GetInterfaceIP behaviour The interfaces are filtered by the `forwardable` flag as described in the equivalent sockaddr command. However, the user might rely on the name/description assuming that all network interfaces are considered without exclusions. Signed-off-by: Alessandro De Blasis --- ifaddr.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ifaddr.go b/ifaddr.go index 0811b2759..ad312ddcd 100644 --- a/ifaddr.go +++ b/ifaddr.go @@ -141,9 +141,9 @@ func GetPublicIPs() (string, error) { return strings.Join(ips, " "), nil } -// GetInterfaceIP returns a string with a single IP address sorted by the size -// of the network (i.e. IP addresses with a smaller netmask, larger network -// size, are sorted first). This function is the `eval` equivalent of: +// GetInterfaceIP returns a string with a single forwardable IP address sorted +// by the size of the network (i.e. IP addresses with a smaller netmask, larger +// network size, are sorted first). This function is the `eval` equivalent of: // // ``` // $ sockaddr eval -r '{{GetAllInterfaces | include "name" <> | sort "type,size" | include "flag" "forwardable" | attr "address" }}' From fb44e603d1ebccb3c03a7400541b0c820b3a9660 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Mon, 1 Nov 2021 18:35:54 +0000 Subject: [PATCH 2/7] Clarified GetInterfaceIP behaviour, Added GetInterfaceIPRegardlessOfInterfaceFlags GetInterfaceIP currently implicitly filters the interfaces excluding the ones with a non-forwardable address. This commit clarifies the behaviour and also adds a new command that can be used to retrieve the IP of an interface even if it's not forwardable such as: 169.254.0.0/16 Signed-off-by: Alessandro De Blasis --- ifaddr.go | 41 ++++++++++++++++- ifaddr_private_test.go | 100 +++++++++++++++++++++++++++++++++++++++++ ifaddr_test.go | 11 +++++ ifaddrs.go | 3 +- template/template.go | 3 ++ 5 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 ifaddr_private_test.go diff --git a/ifaddr.go b/ifaddr.go index ad312ddcd..ad22319d1 100644 --- a/ifaddr.go +++ b/ifaddr.go @@ -153,17 +153,54 @@ func GetInterfaceIP(namedIfRE string) (string, error) { if err != nil { return "", err } + flags := []string{ + "forwardable", + } + return getInterfaceIPByFlags(namedIfRE, flags, ifAddrs) +} - ifAddrs, _, err = IfByName(namedIfRE, ifAddrs) +// GetInterfaceIPRegardlessOfInterfaceFlags returns a string with a single IP address sorted +// by the size of the network (i.e. IP addresses with a smaller netmask, larger +// network size, are sorted first). This function is the `eval` equivalent of: +// +// ``` +// $ sockaddr eval -r '{{GetAllInterfaces | include "name" <> | sort "type,size" | attr "address" }}' +/// ``` +func GetInterfaceIPRegardlessOfInterfaceFlags(namedIfRE string) (string, error) { + ifAddrs, err := GetAllInterfaces() if err != nil { return "", err } + return getInterfaceIPByFlags(namedIfRE, nil, ifAddrs) +} - ifAddrs, _, err = IfByFlag("forwardable", ifAddrs) +// getInterfaceIPByFlags returns a string with a single IP address sorted +// by the size of the network (i.e. IP addresses with a smaller netmask, larger +// network size, are sorted first). This function is the `eval` equivalent of: +// +// ``` +// $ sockaddr eval -r '{{GetAllInterfaces | include "name" <> | sort "type,size" | <> | attr "address" }}' +/// ``` +// +// where <> represents a logical AND between all the supplied flags. +// +// For example: +// +// with flags:=[]string{"forwardable", "broadcast"} => `include "flag" "forwardable" | include "flag" "broadcast"` +/// +func getInterfaceIPByFlags(namedIfRE string, flags []string, ifAddrs IfAddrs) (string, error) { + ifAddrs, _, err := IfByName(namedIfRE, ifAddrs) if err != nil { return "", err } + for _, flag := range flags { + ifAddrs, _, err = IfByFlag(flag, ifAddrs) + if err != nil { + return "", err + } + } + ifAddrs, err = SortIfBy("+type,+size", ifAddrs) if err != nil { return "", err diff --git a/ifaddr_private_test.go b/ifaddr_private_test.go new file mode 100644 index 000000000..f40add4df --- /dev/null +++ b/ifaddr_private_test.go @@ -0,0 +1,100 @@ +package sockaddr + +import ( + "net" + "testing" +) + +//Test_getInterfaceIPByFlags is here to facilitate testing of the private function getInterfaceIPByFlags +func Test_getInterfaceIPByFlags(t *testing.T) { + + ifAddrs := IfAddrs{ + { + SockAddr: MustIPv4Addr("127.0.0.0/8"), + Interface: net.Interface{ + Index: 1, + MTU: 65536, + Name: "lo", + Flags: net.FlagUp | net.FlagLoopback, + }, + }, + { + SockAddr: MustIPv4Addr("172.16.0.0/12"), + Interface: net.Interface{ + Index: 2, + MTU: 1500, + Name: "eth0", + Flags: net.FlagUp, + }, + }, + { + SockAddr: MustIPv4Addr("169.254.0.0/16"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummy", + Flags: net.FlagBroadcast, + }, + }, + } + + type args struct { + namedIfRE string + flags []string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "loopback: no flags provided => 127.0.0.0", + args: args{namedIfRE: "lo", flags: []string{}}, + want: "127.0.0.0", + wantErr: false, + }, + { + name: "loopback: `forwardable` flag provided => empty string", + args: args{namedIfRE: "lo", flags: []string{"forwardable"}}, + want: "", + wantErr: false, + }, + { + name: "private (RFC1918): no flags provided => 172.16.0.0", + args: args{namedIfRE: "eth0", flags: []string{}}, + want: "172.16.0.0", + wantErr: false, + }, + { + name: "private (RFC1918): `broadcast` flag provided but iface is not broadcast => empty string", + args: args{namedIfRE: "eth0", flags: []string{"broadcast"}}, + want: "", + wantErr: false, + }, + { + name: "dummy (RFC3927): no flags provided => 169.254.0.0", + args: args{namedIfRE: "dummy", flags: []string{}}, + want: "169.254.0.0", + wantErr: false, + }, + { + name: "dummy (RFC3927): `forwardable` flag provided => empty string", + args: args{namedIfRE: "dummy", flags: []string{"forwardable"}}, + want: "", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getInterfaceIPByFlags(tt.args.namedIfRE, tt.args.flags, ifAddrs) + if (err != nil) != tt.wantErr { + t.Errorf("getInterfaceIPByFlags() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("getInterfaceIPByFlags() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/ifaddr_test.go b/ifaddr_test.go index 45a0cc788..6f1bcb0f0 100644 --- a/ifaddr_test.go +++ b/ifaddr_test.go @@ -128,6 +128,17 @@ func TestGetInterfaceIP(t *testing.T) { } } +func TestGetInterfaceIPRegardlessOfInterfaceFlags(t *testing.T) { + ip, err := sockaddr.GetInterfaceIPRegardlessOfInterfaceFlags(`^.*[\d]$`) + if err != nil { + t.Fatalf("regexp failed: %v", err) + } + + if ip == "" { + t.Skip("it's hard to test this reliably") + } +} + func TestIfAddrAttr(t *testing.T) { tests := []struct { name string diff --git a/ifaddrs.go b/ifaddrs.go index 80f61bef6..d58cb50af 100644 --- a/ifaddrs.go +++ b/ifaddrs.go @@ -1214,7 +1214,7 @@ func parseDefaultIfNameFromIPCmd(routeOut string) (string, error) { // Android. func parseDefaultIfNameFromIPCmdAndroid(routeOut string) (string, error) { parsedLines := parseIfNameFromIPCmd(routeOut) - if (len(parsedLines) > 0) { + if len(parsedLines) > 0 { ifName := strings.TrimSpace(parsedLines[0][4]) return ifName, nil } @@ -1222,7 +1222,6 @@ func parseDefaultIfNameFromIPCmdAndroid(routeOut string) (string, error) { return "", errors.New("No default interface found") } - // parseIfNameFromIPCmd parses interfaces from ip(8) for // Linux. func parseIfNameFromIPCmd(routeOut string) [][]string { diff --git a/template/template.go b/template/template.go index bbed51361..84f055c87 100644 --- a/template/template.go +++ b/template/template.go @@ -95,6 +95,9 @@ func init() { // the largest network size. "GetInterfaceIP": sockaddr.GetInterfaceIP, + // Returns the first IP address of the named interfaces, regardless of any interface flag + "GetInterfaceIPRegardlessOfInterfaceFlags": sockaddr.GetInterfaceIPRegardlessOfInterfaceFlags, + // Return all IP addresses on the named interface, sorted by the largest // network size. "GetInterfaceIPs": sockaddr.GetInterfaceIPs, From b1c7f61cadcb673d326c8ae57cf175a24e583b49 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Tue, 16 Nov 2021 09:02:22 +0000 Subject: [PATCH 3/7] code-review: fixed odd namings, added ipv6 tests Signed-off-by: Alessandro De Blasis --- ifaddr.go | 12 ++++++------ ifaddr_private_test.go | 23 ++++++++++++++++++++++- ifaddr_test.go | 2 +- template/template.go | 2 +- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/ifaddr.go b/ifaddr.go index ad22319d1..c57429bf5 100644 --- a/ifaddr.go +++ b/ifaddr.go @@ -156,25 +156,25 @@ func GetInterfaceIP(namedIfRE string) (string, error) { flags := []string{ "forwardable", } - return getInterfaceIPByFlags(namedIfRE, flags, ifAddrs) + return getInterfaceIP(namedIfRE, flags, ifAddrs) } -// GetInterfaceIPRegardlessOfInterfaceFlags returns a string with a single IP address sorted +// GetInterfaceIPWithoutInterfaceFlags returns a string with a single IP address sorted // by the size of the network (i.e. IP addresses with a smaller netmask, larger // network size, are sorted first). This function is the `eval` equivalent of: // // ``` // $ sockaddr eval -r '{{GetAllInterfaces | include "name" <> | sort "type,size" | attr "address" }}' /// ``` -func GetInterfaceIPRegardlessOfInterfaceFlags(namedIfRE string) (string, error) { +func GetInterfaceIPWithoutInterfaceFlags(namedIfRE string) (string, error) { ifAddrs, err := GetAllInterfaces() if err != nil { return "", err } - return getInterfaceIPByFlags(namedIfRE, nil, ifAddrs) + return getInterfaceIP(namedIfRE, nil, ifAddrs) } -// getInterfaceIPByFlags returns a string with a single IP address sorted +// getInterfaceIP returns a string with a single IP address sorted // by the size of the network (i.e. IP addresses with a smaller netmask, larger // network size, are sorted first). This function is the `eval` equivalent of: // @@ -188,7 +188,7 @@ func GetInterfaceIPRegardlessOfInterfaceFlags(namedIfRE string) (string, error) // // with flags:=[]string{"forwardable", "broadcast"} => `include "flag" "forwardable" | include "flag" "broadcast"` /// -func getInterfaceIPByFlags(namedIfRE string, flags []string, ifAddrs IfAddrs) (string, error) { +func getInterfaceIP(namedIfRE string, flags []string, ifAddrs IfAddrs) (string, error) { ifAddrs, _, err := IfByName(namedIfRE, ifAddrs) if err != nil { return "", err diff --git a/ifaddr_private_test.go b/ifaddr_private_test.go index f40add4df..6120bafae 100644 --- a/ifaddr_private_test.go +++ b/ifaddr_private_test.go @@ -36,6 +36,15 @@ func Test_getInterfaceIPByFlags(t *testing.T) { Flags: net.FlagBroadcast, }, }, + { + SockAddr: MustIPv6Addr("fe80::/10"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummyv6", + Flags: net.FlagBroadcast, + }, + }, } type args struct { @@ -84,10 +93,22 @@ func Test_getInterfaceIPByFlags(t *testing.T) { want: "", wantErr: false, }, + { + name: "dummyv6 (RFC4291) IPv6: no flags provided => fe80::", + args: args{namedIfRE: "dummyv6", flags: []string{}}, + want: "fe80::", + wantErr: false, + }, + { + name: "dummyv6 (RFC4291) IPv6: `forwardable` flag provided => empty string", + args: args{namedIfRE: "dummyv6", flags: []string{"forwardable"}}, + want: "", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := getInterfaceIPByFlags(tt.args.namedIfRE, tt.args.flags, ifAddrs) + got, err := getInterfaceIP(tt.args.namedIfRE, tt.args.flags, ifAddrs) if (err != nil) != tt.wantErr { t.Errorf("getInterfaceIPByFlags() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/ifaddr_test.go b/ifaddr_test.go index 6f1bcb0f0..85d807f7f 100644 --- a/ifaddr_test.go +++ b/ifaddr_test.go @@ -129,7 +129,7 @@ func TestGetInterfaceIP(t *testing.T) { } func TestGetInterfaceIPRegardlessOfInterfaceFlags(t *testing.T) { - ip, err := sockaddr.GetInterfaceIPRegardlessOfInterfaceFlags(`^.*[\d]$`) + ip, err := sockaddr.GetInterfaceIPWithoutInterfaceFlags(`^.*[\d]$`) if err != nil { t.Fatalf("regexp failed: %v", err) } diff --git a/template/template.go b/template/template.go index 84f055c87..549733e18 100644 --- a/template/template.go +++ b/template/template.go @@ -96,7 +96,7 @@ func init() { "GetInterfaceIP": sockaddr.GetInterfaceIP, // Returns the first IP address of the named interfaces, regardless of any interface flag - "GetInterfaceIPRegardlessOfInterfaceFlags": sockaddr.GetInterfaceIPRegardlessOfInterfaceFlags, + "GetInterfaceIPWithoutInterfaceFlags": sockaddr.GetInterfaceIPWithoutInterfaceFlags, // Return all IP addresses on the named interface, sorted by the largest // network size. From a29c7f28296d3bd15dccfbaa7dc61dca5943638c Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Mon, 29 Nov 2021 10:05:24 +0000 Subject: [PATCH 4/7] Testing the public funcs this commit introduces a way of controlling what's returned by `sockaddr.GetAllInterfaces()` in tests. Useful in tests that can now stub real world setups. Signed-off-by: Alessandro De Blasis --- ifaddr_test.go | 198 ++++++++++++++++++++++++++++++++++++++++++++++--- ifaddrs.go | 8 +- 2 files changed, 193 insertions(+), 13 deletions(-) diff --git a/ifaddr_test.go b/ifaddr_test.go index 85d807f7f..6b4a8497b 100644 --- a/ifaddr_test.go +++ b/ifaddr_test.go @@ -118,24 +118,200 @@ func TestGetPublicIPs(t *testing.T) { } func TestGetInterfaceIP(t *testing.T) { - ip, err := sockaddr.GetInterfaceIP(`^.*[\d]$`) - if err != nil { - t.Fatalf("regexp failed: %v", err) + + ifAddrs := sockaddr.IfAddrs{ + { + SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), + Interface: net.Interface{ + Index: 1, + MTU: 65536, + Name: "lo", + Flags: net.FlagUp | net.FlagLoopback, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), + Interface: net.Interface{ + Index: 2, + MTU: 1500, + Name: "eth0", + Flags: net.FlagUp, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummy", + Flags: net.FlagBroadcast, + }, + }, + { + SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummyv6", + Flags: net.FlagBroadcast, + }, + }, } - if ip == "" { - t.Skip("it's hard to test this reliably") + type args struct { + namedIfRE string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "loopback => empty string", + args: args{namedIfRE: "lo"}, + want: "", + wantErr: false, + }, + { + name: "private (RFC1918) => 172.16.0.0", + args: args{namedIfRE: "eth0"}, + want: "172.16.0.0", + wantErr: false, + }, + { + name: "dummy (RFC3927) => empty string", + args: args{namedIfRE: "dummy"}, + want: "", + wantErr: false, + }, + { + name: "dummyv6 (RFC4291) IPv6 => empty string", + args: args{namedIfRE: "dummyv6"}, + want: "", + wantErr: false, + }, + } + + // setting up a "fake environment" by temporarily controlling what's returned by + // `sockaddr.GetAllInterfaces` + realGetAllInterfaces := sockaddr.GetAllInterfaces + defer func() { sockaddr.GetAllInterfaces = realGetAllInterfaces }() + + sockaddr.GetAllInterfaces = func() (sockaddr.IfAddrs, error) { + return ifAddrs, nil + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := sockaddr.GetInterfaceIP(tt.args.namedIfRE) + if (err != nil) != tt.wantErr { + t.Errorf("GetInterfaceIP() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetInterfaceIP() = %v, want %v", got, tt.want) + } + }) } } -func TestGetInterfaceIPRegardlessOfInterfaceFlags(t *testing.T) { - ip, err := sockaddr.GetInterfaceIPWithoutInterfaceFlags(`^.*[\d]$`) - if err != nil { - t.Fatalf("regexp failed: %v", err) +func TestGetInterfaceIPWithoutInterfaceFlags(t *testing.T) { + + ifAddrs := sockaddr.IfAddrs{ + { + SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), + Interface: net.Interface{ + Index: 1, + MTU: 65536, + Name: "lo", + Flags: net.FlagUp | net.FlagLoopback, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), + Interface: net.Interface{ + Index: 2, + MTU: 1500, + Name: "eth0", + Flags: net.FlagUp, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummy", + Flags: net.FlagBroadcast, + }, + }, + { + SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummyv6", + Flags: net.FlagBroadcast, + }, + }, } - if ip == "" { - t.Skip("it's hard to test this reliably") + type args struct { + namedIfRE string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "loopback => 127.0.0.0", + args: args{namedIfRE: "lo"}, + want: "127.0.0.0", + wantErr: false, + }, + { + name: "private (RFC1918) => 172.16.0.0", + args: args{namedIfRE: "eth0"}, + want: "172.16.0.0", + wantErr: false, + }, + { + name: "dummy (RFC3927) => 169.254.0.0", + args: args{namedIfRE: "dummy"}, + want: "169.254.0.0", + wantErr: false, + }, + { + name: "dummyv6 (RFC4291) IPv6 => fe80::", + args: args{namedIfRE: "dummyv6"}, + want: "fe80::", + wantErr: false, + }, + } + + // setting up a "fake environment" by temporarily controlling what's returned by + // `sockaddr.GetAllInterfaces` + realGetAllInterfaces := sockaddr.GetAllInterfaces + defer func() { sockaddr.GetAllInterfaces = realGetAllInterfaces }() + + sockaddr.GetAllInterfaces = func() (sockaddr.IfAddrs, error) { + return ifAddrs, nil + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := sockaddr.GetInterfaceIPWithoutInterfaceFlags(tt.args.namedIfRE) + if (err != nil) != tt.wantErr { + t.Errorf("GetInterfaceIPWithoutInterfaceFlags() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetInterfaceIPWithoutInterfaceFlags() = %v, want %v", got, tt.want) + } + }) } } diff --git a/ifaddrs.go b/ifaddrs.go index d58cb50af..bd23feea8 100644 --- a/ifaddrs.go +++ b/ifaddrs.go @@ -236,10 +236,14 @@ func IfAttrs(selectorName string, ifAddrs IfAddrs) (string, error) { return attrVal, err } -// GetAllInterfaces iterates over all available network interfaces and finds all +// GetAllInterfaces proxies the reference to getAllInterfaces in order to +// allow stubbing in tests by temporarily swapping implementations. +var GetAllInterfaces = getAllInterfaces + +// getAllInterfaces iterates over all available network interfaces and finds all // available IP addresses on each interface and converts them to // sockaddr.IPAddrs, and returning the result as an array of IfAddr. -func GetAllInterfaces() (IfAddrs, error) { +func getAllInterfaces() (IfAddrs, error) { ifs, err := net.Interfaces() if err != nil { return nil, err From cb0bec0f557aadc805c3ec01917b23c0775a5e91 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Mon, 29 Nov 2021 10:05:40 +0000 Subject: [PATCH 5/7] Removing private test Signed-off-by: Alessandro De Blasis --- ifaddr_private_test.go | 121 ----------------------------------------- 1 file changed, 121 deletions(-) delete mode 100644 ifaddr_private_test.go diff --git a/ifaddr_private_test.go b/ifaddr_private_test.go deleted file mode 100644 index 6120bafae..000000000 --- a/ifaddr_private_test.go +++ /dev/null @@ -1,121 +0,0 @@ -package sockaddr - -import ( - "net" - "testing" -) - -//Test_getInterfaceIPByFlags is here to facilitate testing of the private function getInterfaceIPByFlags -func Test_getInterfaceIPByFlags(t *testing.T) { - - ifAddrs := IfAddrs{ - { - SockAddr: MustIPv4Addr("127.0.0.0/8"), - Interface: net.Interface{ - Index: 1, - MTU: 65536, - Name: "lo", - Flags: net.FlagUp | net.FlagLoopback, - }, - }, - { - SockAddr: MustIPv4Addr("172.16.0.0/12"), - Interface: net.Interface{ - Index: 2, - MTU: 1500, - Name: "eth0", - Flags: net.FlagUp, - }, - }, - { - SockAddr: MustIPv4Addr("169.254.0.0/16"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummy", - Flags: net.FlagBroadcast, - }, - }, - { - SockAddr: MustIPv6Addr("fe80::/10"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummyv6", - Flags: net.FlagBroadcast, - }, - }, - } - - type args struct { - namedIfRE string - flags []string - } - tests := []struct { - name string - args args - want string - wantErr bool - }{ - { - name: "loopback: no flags provided => 127.0.0.0", - args: args{namedIfRE: "lo", flags: []string{}}, - want: "127.0.0.0", - wantErr: false, - }, - { - name: "loopback: `forwardable` flag provided => empty string", - args: args{namedIfRE: "lo", flags: []string{"forwardable"}}, - want: "", - wantErr: false, - }, - { - name: "private (RFC1918): no flags provided => 172.16.0.0", - args: args{namedIfRE: "eth0", flags: []string{}}, - want: "172.16.0.0", - wantErr: false, - }, - { - name: "private (RFC1918): `broadcast` flag provided but iface is not broadcast => empty string", - args: args{namedIfRE: "eth0", flags: []string{"broadcast"}}, - want: "", - wantErr: false, - }, - { - name: "dummy (RFC3927): no flags provided => 169.254.0.0", - args: args{namedIfRE: "dummy", flags: []string{}}, - want: "169.254.0.0", - wantErr: false, - }, - { - name: "dummy (RFC3927): `forwardable` flag provided => empty string", - args: args{namedIfRE: "dummy", flags: []string{"forwardable"}}, - want: "", - wantErr: false, - }, - { - name: "dummyv6 (RFC4291) IPv6: no flags provided => fe80::", - args: args{namedIfRE: "dummyv6", flags: []string{}}, - want: "fe80::", - wantErr: false, - }, - { - name: "dummyv6 (RFC4291) IPv6: `forwardable` flag provided => empty string", - args: args{namedIfRE: "dummyv6", flags: []string{"forwardable"}}, - want: "", - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := getInterfaceIP(tt.args.namedIfRE, tt.args.flags, ifAddrs) - if (err != nil) != tt.wantErr { - t.Errorf("getInterfaceIPByFlags() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("getInterfaceIPByFlags() = %v, want %v", got, tt.want) - } - }) - } -} From ae049f80d1f59cb8d122dbe1c31e0f4ea7f9657f Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Sun, 26 Dec 2021 16:28:03 +0000 Subject: [PATCH 6/7] Added NetworkInterfacesProvider default implementation (OSNetProvider) and overridability in tests Signed-off-by: Alessandro De Blasis --- ifaddr_test.go | 160 +++++++++++++++++++-------------------------- ifaddrs.go | 44 +++---------- os_net_provider.go | 42 ++++++++++++ 3 files changed, 118 insertions(+), 128 deletions(-) create mode 100644 os_net_provider.go diff --git a/ifaddr_test.go b/ifaddr_test.go index 6b4a8497b..bcd38fe7f 100644 --- a/ifaddr_test.go +++ b/ifaddr_test.go @@ -119,44 +119,10 @@ func TestGetPublicIPs(t *testing.T) { func TestGetInterfaceIP(t *testing.T) { - ifAddrs := sockaddr.IfAddrs{ - { - SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), - Interface: net.Interface{ - Index: 1, - MTU: 65536, - Name: "lo", - Flags: net.FlagUp | net.FlagLoopback, - }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), - Interface: net.Interface{ - Index: 2, - MTU: 1500, - Name: "eth0", - Flags: net.FlagUp, - }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummy", - Flags: net.FlagBroadcast, - }, - }, - { - SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummyv6", - Flags: net.FlagBroadcast, - }, - }, - } + // setting up a "fake environment" by temporarily controlling what's returned by + // `sockaddr.GetAllInterfaces` + teardown := overrideOsNetProvider(&fakeNetProvider{}) + defer teardown() type args struct { namedIfRE string @@ -193,15 +159,6 @@ func TestGetInterfaceIP(t *testing.T) { }, } - // setting up a "fake environment" by temporarily controlling what's returned by - // `sockaddr.GetAllInterfaces` - realGetAllInterfaces := sockaddr.GetAllInterfaces - defer func() { sockaddr.GetAllInterfaces = realGetAllInterfaces }() - - sockaddr.GetAllInterfaces = func() (sockaddr.IfAddrs, error) { - return ifAddrs, nil - } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := sockaddr.GetInterfaceIP(tt.args.namedIfRE) @@ -218,44 +175,10 @@ func TestGetInterfaceIP(t *testing.T) { func TestGetInterfaceIPWithoutInterfaceFlags(t *testing.T) { - ifAddrs := sockaddr.IfAddrs{ - { - SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), - Interface: net.Interface{ - Index: 1, - MTU: 65536, - Name: "lo", - Flags: net.FlagUp | net.FlagLoopback, - }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), - Interface: net.Interface{ - Index: 2, - MTU: 1500, - Name: "eth0", - Flags: net.FlagUp, - }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummy", - Flags: net.FlagBroadcast, - }, - }, - { - SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummyv6", - Flags: net.FlagBroadcast, - }, - }, - } + // setting up a "fake environment" by temporarily controlling what's returned by + // `sockaddr.GetAllInterfaces` + teardown := overrideOsNetProvider(&fakeNetProvider{}) + defer teardown() type args struct { namedIfRE string @@ -292,15 +215,6 @@ func TestGetInterfaceIPWithoutInterfaceFlags(t *testing.T) { }, } - // setting up a "fake environment" by temporarily controlling what's returned by - // `sockaddr.GetAllInterfaces` - realGetAllInterfaces := sockaddr.GetAllInterfaces - defer func() { sockaddr.GetAllInterfaces = realGetAllInterfaces }() - - sockaddr.GetAllInterfaces = func() (sockaddr.IfAddrs, error) { - return ifAddrs, nil - } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := sockaddr.GetInterfaceIPWithoutInterfaceFlags(tt.args.namedIfRE) @@ -779,3 +693,61 @@ func TestIfAddrMath(t *testing.T) { } } } + +// overrideOsNetProvider allows overriding the sockaddr.NetProvider +// +// It returns a cleanup/teardown func that restores the default OSNetProvider after use +// +// Usage: +// teardown := overrideOsNetProvider(fakeNetProvider{}) +// defer teardown() +func overrideOsNetProvider(override sockaddr.NetworkInterfacesProvider) func() { + sockaddr.NetProvider = override + return func() { + sockaddr.NetProvider = &sockaddr.OSNetProvider{} + } +} + +type fakeNetProvider struct{} + +func (n *fakeNetProvider) GetAllInterfaces() (sockaddr.IfAddrs, error) { + ifAddrs := sockaddr.IfAddrs{ + { + SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), + Interface: net.Interface{ + Index: 1, + MTU: 65536, + Name: "lo", + Flags: net.FlagUp | net.FlagLoopback, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), + Interface: net.Interface{ + Index: 2, + MTU: 1500, + Name: "eth0", + Flags: net.FlagUp, + }, + }, + { + SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummy", + Flags: net.FlagBroadcast, + }, + }, + { + SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummyv6", + Flags: net.FlagBroadcast, + }, + }, + } + return ifAddrs, nil +} diff --git a/ifaddrs.go b/ifaddrs.go index bd23feea8..3b40fa3a6 100644 --- a/ifaddrs.go +++ b/ifaddrs.go @@ -236,42 +236,18 @@ func IfAttrs(selectorName string, ifAddrs IfAddrs) (string, error) { return attrVal, err } -// GetAllInterfaces proxies the reference to getAllInterfaces in order to -// allow stubbing in tests by temporarily swapping implementations. -var GetAllInterfaces = getAllInterfaces - -// getAllInterfaces iterates over all available network interfaces and finds all -// available IP addresses on each interface and converts them to -// sockaddr.IPAddrs, and returning the result as an array of IfAddr. -func getAllInterfaces() (IfAddrs, error) { - ifs, err := net.Interfaces() - if err != nil { - return nil, err - } - - ifAddrs := make(IfAddrs, 0, len(ifs)) - for _, intf := range ifs { - addrs, err := intf.Addrs() - if err != nil { - return nil, err - } - - for _, addr := range addrs { - var ipAddr IPAddr - ipAddr, err = NewIPAddr(addr.String()) - if err != nil { - return IfAddrs{}, fmt.Errorf("unable to create an IP address from %q", addr.String()) - } +// NetworkInterfacesProvider abstracts the way we gather network interfaces in order +// to improve testability +type NetworkInterfacesProvider interface { + GetAllInterfaces() (IfAddrs, error) +} - ifAddr := IfAddr{ - SockAddr: ipAddr, - Interface: intf, - } - ifAddrs = append(ifAddrs, ifAddr) - } - } +// NetProvider implements NetworkInterfacesProvider and is defined here with its default +var NetProvider NetworkInterfacesProvider = &OSNetProvider{} - return ifAddrs, nil +// GetAllInterfaces proxies the call to the NetProvider +func GetAllInterfaces() (IfAddrs, error) { + return NetProvider.GetAllInterfaces() } // GetDefaultInterfaces returns IfAddrs of the addresses attached to the default diff --git a/os_net_provider.go b/os_net_provider.go new file mode 100644 index 000000000..9f5ec88c8 --- /dev/null +++ b/os_net_provider.go @@ -0,0 +1,42 @@ +package sockaddr + +import ( + "fmt" + "net" +) + +type OSNetProvider struct{} + +// GetAllInterfaces iterates over all available network interfaces and finds all +// available IP addresses on each interface and converts them to +// sockaddr.IPAddrs, and returning the result as an array of IfAddr. +func (n *OSNetProvider) GetAllInterfaces() (IfAddrs, error) { + ifs, err := net.Interfaces() + if err != nil { + return nil, err + } + + ifAddrs := make(IfAddrs, 0, len(ifs)) + for _, intf := range ifs { + addrs, err := intf.Addrs() + if err != nil { + return nil, err + } + + for _, addr := range addrs { + var ipAddr IPAddr + ipAddr, err = NewIPAddr(addr.String()) + if err != nil { + return IfAddrs{}, fmt.Errorf("unable to create an IP address from %q", addr.String()) + } + + ifAddr := IfAddr{ + SockAddr: ipAddr, + Interface: intf, + } + ifAddrs = append(ifAddrs, ifAddr) + } + } + + return ifAddrs, nil +} From aba260ca3b8cd125e61e6f330d8922c2effd2faf Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Sun, 13 Mar 2022 16:40:00 +0000 Subject: [PATCH 7/7] simplified GetAllInterfaces mocking Signed-off-by: Alessandro De Blasis --- ifaddr_test.go | 106 ++++++++++++++++++++------------------------- ifaddrs.go | 44 ++++++++++++++----- os_net_provider.go | 42 ------------------ 3 files changed, 81 insertions(+), 111 deletions(-) delete mode 100644 os_net_provider.go diff --git a/ifaddr_test.go b/ifaddr_test.go index bcd38fe7f..a708a8e8d 100644 --- a/ifaddr_test.go +++ b/ifaddr_test.go @@ -119,10 +119,9 @@ func TestGetPublicIPs(t *testing.T) { func TestGetInterfaceIP(t *testing.T) { - // setting up a "fake environment" by temporarily controlling what's returned by - // `sockaddr.GetAllInterfaces` - teardown := overrideOsNetProvider(&fakeNetProvider{}) - defer teardown() + orig := sockaddr.GetAllInterfaces + sockaddr.GetAllInterfaces = mockGetAllInterfaces() + t.Cleanup(func() { sockaddr.GetAllInterfaces = orig }) type args struct { namedIfRE string @@ -175,10 +174,9 @@ func TestGetInterfaceIP(t *testing.T) { func TestGetInterfaceIPWithoutInterfaceFlags(t *testing.T) { - // setting up a "fake environment" by temporarily controlling what's returned by - // `sockaddr.GetAllInterfaces` - teardown := overrideOsNetProvider(&fakeNetProvider{}) - defer teardown() + orig := sockaddr.GetAllInterfaces + sockaddr.GetAllInterfaces = mockGetAllInterfaces() + t.Cleanup(func() { sockaddr.GetAllInterfaces = orig }) type args struct { namedIfRE string @@ -694,60 +692,50 @@ func TestIfAddrMath(t *testing.T) { } } -// overrideOsNetProvider allows overriding the sockaddr.NetProvider -// -// It returns a cleanup/teardown func that restores the default OSNetProvider after use -// -// Usage: -// teardown := overrideOsNetProvider(fakeNetProvider{}) -// defer teardown() -func overrideOsNetProvider(override sockaddr.NetworkInterfacesProvider) func() { - sockaddr.NetProvider = override - return func() { - sockaddr.NetProvider = &sockaddr.OSNetProvider{} - } -} - -type fakeNetProvider struct{} - -func (n *fakeNetProvider) GetAllInterfaces() (sockaddr.IfAddrs, error) { - ifAddrs := sockaddr.IfAddrs{ - { - SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), - Interface: net.Interface{ - Index: 1, - MTU: 65536, - Name: "lo", - Flags: net.FlagUp | net.FlagLoopback, +// mockGetAllInterfaces returns a mocked implementation of an otherwise OS provided +// sockaddr.GetAllInterfaces() +// Currently it returns a func so that it could be easily parametrized in order to support +// additional scenarios selectable by adding an argument and the relevant switch logic below +func mockGetAllInterfaces() func() (sockaddr.IfAddrs, error) { + return func() (sockaddr.IfAddrs, error) { + ifAddrs := sockaddr.IfAddrs{ + { + SockAddr: sockaddr.MustIPv4Addr("127.0.0.0/8"), + Interface: net.Interface{ + Index: 1, + MTU: 65536, + Name: "lo", + Flags: net.FlagUp | net.FlagLoopback, + }, }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), - Interface: net.Interface{ - Index: 2, - MTU: 1500, - Name: "eth0", - Flags: net.FlagUp, + { + SockAddr: sockaddr.MustIPv4Addr("172.16.0.0/12"), + Interface: net.Interface{ + Index: 2, + MTU: 1500, + Name: "eth0", + Flags: net.FlagUp, + }, }, - }, - { - SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummy", - Flags: net.FlagBroadcast, + { + SockAddr: sockaddr.MustIPv4Addr("169.254.0.0/16"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummy", + Flags: net.FlagBroadcast, + }, }, - }, - { - SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), - Interface: net.Interface{ - Index: 3, - MTU: 1500, - Name: "dummyv6", - Flags: net.FlagBroadcast, + { + SockAddr: sockaddr.MustIPv6Addr("fe80::/10"), + Interface: net.Interface{ + Index: 3, + MTU: 1500, + Name: "dummyv6", + Flags: net.FlagBroadcast, + }, }, - }, + } + return ifAddrs, nil } - return ifAddrs, nil } diff --git a/ifaddrs.go b/ifaddrs.go index 3b40fa3a6..03d9009d5 100644 --- a/ifaddrs.go +++ b/ifaddrs.go @@ -236,18 +236,42 @@ func IfAttrs(selectorName string, ifAddrs IfAddrs) (string, error) { return attrVal, err } -// NetworkInterfacesProvider abstracts the way we gather network interfaces in order -// to improve testability -type NetworkInterfacesProvider interface { - GetAllInterfaces() (IfAddrs, error) -} +// GetAllInterfaces proxies the calls to the real implementation +// to provide mocking capabilities while testing +var GetAllInterfaces func() (IfAddrs, error) = getAllInterfaces + +// getAllInterfaces iterates over all available network interfaces and finds all +// available IP addresses on each interface and converts them to +// sockaddr.IPAddrs, and returning the result as an array of IfAddr. +func getAllInterfaces() (IfAddrs, error) { + ifs, err := net.Interfaces() + if err != nil { + return nil, err + } + + ifAddrs := make(IfAddrs, 0, len(ifs)) + for _, intf := range ifs { + addrs, err := intf.Addrs() + if err != nil { + return nil, err + } -// NetProvider implements NetworkInterfacesProvider and is defined here with its default -var NetProvider NetworkInterfacesProvider = &OSNetProvider{} + for _, addr := range addrs { + var ipAddr IPAddr + ipAddr, err = NewIPAddr(addr.String()) + if err != nil { + return IfAddrs{}, fmt.Errorf("unable to create an IP address from %q", addr.String()) + } + + ifAddr := IfAddr{ + SockAddr: ipAddr, + Interface: intf, + } + ifAddrs = append(ifAddrs, ifAddr) + } + } -// GetAllInterfaces proxies the call to the NetProvider -func GetAllInterfaces() (IfAddrs, error) { - return NetProvider.GetAllInterfaces() + return ifAddrs, nil } // GetDefaultInterfaces returns IfAddrs of the addresses attached to the default diff --git a/os_net_provider.go b/os_net_provider.go deleted file mode 100644 index 9f5ec88c8..000000000 --- a/os_net_provider.go +++ /dev/null @@ -1,42 +0,0 @@ -package sockaddr - -import ( - "fmt" - "net" -) - -type OSNetProvider struct{} - -// GetAllInterfaces iterates over all available network interfaces and finds all -// available IP addresses on each interface and converts them to -// sockaddr.IPAddrs, and returning the result as an array of IfAddr. -func (n *OSNetProvider) GetAllInterfaces() (IfAddrs, error) { - ifs, err := net.Interfaces() - if err != nil { - return nil, err - } - - ifAddrs := make(IfAddrs, 0, len(ifs)) - for _, intf := range ifs { - addrs, err := intf.Addrs() - if err != nil { - return nil, err - } - - for _, addr := range addrs { - var ipAddr IPAddr - ipAddr, err = NewIPAddr(addr.String()) - if err != nil { - return IfAddrs{}, fmt.Errorf("unable to create an IP address from %q", addr.String()) - } - - ifAddr := IfAddr{ - SockAddr: ipAddr, - Interface: intf, - } - ifAddrs = append(ifAddrs, ifAddr) - } - } - - return ifAddrs, nil -}