OCPBUGS-10695: Use subnet mask to detect IP stack

pkg/utils/addresses.go

@@ -140,7 +140,7 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
 	}
 
 	var routeMap map[int][]netlink.Route
-	matches := make([]net.IP, 0)
+	matches := make([]net.IPNet, 0)
 	for link, addresses := range addrMap {
 	addrLoop:
 		for _, address := range addresses {
@@ -168,9 +168,12 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
 						containmentNet := net.IPNet{IP: address.IP, Mask: route.Dst.Mask}
 						for _, vip := range vips {
 							log.Debugf("Checking whether address %s with route %s contains VIP %s", address, route, vip)
+							if IsNetIPv6(containmentNet) != IsIPv6(vip) {
+								log.Debugf("Stack of address '%s' and VIP '%s' does not match. Skipping.", address, vip)
+							}
 							if containmentNet.Contains(vip) {
 								log.Debugf("Address %s with route %s contains VIP %s", address, route, vip)
-								matches = append(matches, address.IP)
+								matches = append(matches, *address.IPNet)
 								break addrLoop
 							}
 						}
@@ -179,9 +182,12 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
 			} else {
 				for _, vip := range vips {
 					log.Debugf("Checking whether address %s contains VIP %s", address, vip)
+					if IsNetIPv6(*address.IPNet) != IsIPv6(vip) {
+						log.Debugf("Stack of address '%s' and VIP '%s' does not match. Skipping.", address, vip)
+					}
 					if address.Contains(vip) {
 						log.Debugf("Address %s contains VIP %s", address, vip)
-						matches = append(matches, address.IP)
+						matches = append(matches, *address.IPNet)
 						break addrLoop
 					}
 				}
@@ -191,8 +197,8 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
 		if len(matches) > 0 {
 			// Find an address of the opposite IP family on the same interface
 			for _, address := range addresses {
-				if IsIPv6(address.IP) != IsIPv6(matches[0]) {
-					matches = append(matches, address.IP)
+				if IsNetIPv6(*address.IPNet) != IsNetIPv6(matches[0]) {
+					matches = append(matches, *address.IPNet)
 					break
 				}
 			}
@@ -202,9 +208,9 @@ func addressesRoutingInternal(vips []net.IP, af AddressFilter, getAddrs addressM
 	// Ensure we return addresses in the desired order
 	sort.SliceStable(matches, func(i, j int) bool {
 		// Very simplified since we only ever have two items in this list
-		return IsIPv6(matches[i]) == preferIPv6
+		return IsNetIPv6(matches[i]) == preferIPv6
 	})
-	return matches, nil
+	return Mapper(matches, func(ipnet net.IPNet) net.IP { return ipnet.IP }), nil
 }
 
 // defaultRoute returns true if the passed route is a default route
@@ -218,7 +224,7 @@ func AddressesDefault(preferIPv6 bool, af AddressFilter) ([]net.IP, error) {
 }
 
 type FoundAddress struct {
-	Address         net.IP
+	Network         net.IPNet
 	Priority        int
 	LinkIndex       int
 	GatewayOnSubnet bool
@@ -245,7 +251,7 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres
 			log.Debugf("Address %s is on interface %s with default route", address, link.Attrs().Name)
 			// We should only have one default route per interface
 			addrs = append(addrs, FoundAddress{
-				Address:         address.IP,
+				Network:         *address.IPNet,
 				Priority:        routeMap[linkIndex][0].Priority,
 				LinkIndex:       linkIndex,
 				GatewayOnSubnet: address.IPNet.Contains(routeMap[linkIndex][0].Gw),
@@ -262,13 +268,13 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres
 	sort.SliceStable(addrs, func(i, j int) bool {
 		if addrs[i].Priority == addrs[j].Priority {
 			if addrs[i].LinkIndex == addrs[j].LinkIndex {
-				if IsIPv6(addrs[i].Address) == IsIPv6(addrs[j].Address) {
+				if IsNetIPv6(addrs[i].Network) == IsNetIPv6(addrs[j].Network) {
 					if addrs[i].GatewayOnSubnet == addrs[j].GatewayOnSubnet {
-						return !addrs[i].Address.IsPrivate()
+						return !addrs[i].Network.IP.IsPrivate()
 					}
 					return addrs[i].GatewayOnSubnet
 				}
-				return IsIPv6(addrs[i].Address) == preferIPv6
+				return IsNetIPv6(addrs[i].Network) == preferIPv6
 			}
 			return addrs[i].LinkIndex < addrs[j].LinkIndex
 		}
@@ -278,11 +284,11 @@ func addressesDefaultInternal(preferIPv6 bool, af AddressFilter, getAddrs addres
 	foundv4 := false
 	foundv6 := false
 	for _, addr := range addrs {
-		if (IsIPv6(addr.Address) && foundv6) || (!IsIPv6(addr.Address) && foundv4) {
+		if (IsNetIPv6(addr.Network) && foundv6) || (!IsNetIPv6(addr.Network) && foundv4) {
 			continue
 		}
-		matches = append(matches, addr.Address)
-		if IsIPv6(addr.Address) {
+		matches = append(matches, addr.Network.IP)
+		if IsNetIPv6(addr.Network) {
 			foundv6 = true
 		} else {
 			foundv4 = true

pkg/utils/addresses_test.go

@@ -98,6 +98,7 @@ func addIPv6Addrs(addrs map[netlink.Link][]netlink.Addr, af AddressFilter) {
 	maybeAddAddress(addrs, af, lo, "::1/128", false)
 	maybeAddAddress(addrs, af, eth0, "fd00::5/64", false)
 	maybeAddAddress(addrs, af, eth0, "fe80::1234/64", false)
+	maybeAddAddress(addrs, af, eth0, "::ffff:192.168.1.160/64", false)
 	maybeAddAddress(addrs, af, eth1, "fd69::2/125", false)
 	maybeAddAddress(addrs, af, eth1, "fd01::3/64", true)
 	maybeAddAddress(addrs, af, eth1, "fd01::4/64", true)

pkg/utils/network.go

@@ -17,14 +17,40 @@ func SplitCIDR(s string) (string, string, error) {
 }
 
 func IsIPv4(ip net.IP) bool {
-	// func IsIPv4(ip string) bool {
 	return strings.Contains(ip.String(), ".")
 }
 
 func IsIPv6(ip net.IP) bool {
 	return strings.Contains(ip.String(), ":")
 }
 
+func IsNetIPv6(network net.IPNet) bool {
+	if IsIPv6(network.IP) {
+		return true
+	}
+
+	if IsIPv4(network.IP) {
+		// Sadly the fact that address has "." doesn't mean it's an IPv4 address. We need to check
+		// subnet masks to make sure this is really the case. If subnet mask is longer than 32 we
+		// are for sure dealing with IPv6 address. If it is shorter, then we need to look at the
+		// total capacity of the mask as it can be simply an IPv4 address but could be as well an
+		// IPv6 in really enormous subnet.
+		//
+		// To simplify this logic because subnet longer than 32 can happen if the capacity of mask
+		// matches IPv6, we can simply look at the capacity of the mask and determine IP stack.
+		_, n := network.Mask.Size()
+		if n == 32 {
+			return false
+		}
+		if n == 128 {
+			return true
+		}
+	}
+
+	log.Debugf("Failed to find IP stack of '%+v'", &network)
+	return false
+}
+
 func IpInCidr(ipAddr, cidr string) (bool, error) {
 	_, ipNet, err := net.ParseCIDR(cidr)
 	if err != nil {

pkg/utils/network_test.go

@@ -43,6 +43,93 @@ var _ = Describe("IP stack detection", func() {
 			res := IsIPv6(net.ParseIP("this-is-not-an-ip"))
 			Expect(res).To(Equal(false))
 		})
+
+	})
+
+	Context("for IPv4-mapped-to-IPv6 addresses", func() {
+		// (mko) This test suite is intentionally returning wrong results. This is because the
+		// mapped form of those addresses is making them impossible to distinguish without
+		// knowing more properties of the link.
+		//
+		// Due to the fact how net.IPv4() initialization is implemented and the fact that it
+		// always adds the v4InV6Prefix header in a binary form, those addresses look like IPv4
+		// and in most cases behave like such.
+		//
+		// The tests here are saying that "::ffff:192.168.0.14" is IPv4 and not an IPv6 address
+		// whenever fed into our own IsIPv4 and IsIPv6 functions. For a proper answer returning
+		// IPv6 for those, we have IsNetIPv6 function which looks at IP address as well as subnet
+		// mask.
+
+		It("IsIPv4 returns true for IPv4-mapped-to-IPv6 address", func() {
+			res := IsIPv4(net.ParseIP("::ffff:192.168.0.14"))
+			Expect(res).To(Equal(true))
+		})
+		It("IsIPv4 returns true for binary IPv4-mapped-to-IPv6 address", func() {
+			ip := net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160})
+			res := IsIPv4(ip)
+			Expect(res).To(Equal(true))
+		})
+
+		It("IsIPv6 returns false for IPv4-mapped-to-IPv6 address", func() {
+			res := IsIPv6(net.ParseIP("::ffff:192.168.0.14"))
+			Expect(res).To(Equal(false))
+		})
+		It("IsIPv6 returns true for binary IPv4-mapped-to-IPv6 address", func() {
+			ip := net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160})
+			res := IsIPv6(ip)
+			Expect(res).To(Equal(false))
+		})
+	})
+
+	Context("using IsNetIPv6", func() {
+		It("returns false for IPv4 addresses", func() {
+			addrs := []string{
+				"192.168.0.1/24",
+				"192.168.0.1/32",
+				"192.168.0.1/0",
+				"0.0.0.0/0",
+			}
+			for _, addr := range addrs {
+				_, addr, _ := net.ParseCIDR(addr)
+				res := IsNetIPv6(*addr)
+				Expect(res).To(Equal(false))
+			}
+		})
+		It("returns true for IPv6 address", func() {
+			addrs := []string{
+				"fe80::84ca:4a24:ffbf:1778/64",
+				"::/0",
+				"::1/128",
+				"::ffff:192.168.0.14/64",
+				"::ffff:192.168.0.14/16",
+			}
+			for _, addr := range addrs {
+				_, addr, _ := net.ParseCIDR(addr)
+				res := IsNetIPv6(*addr)
+				Expect(res).To(Equal(true))
+			}
+		})
+		It("returns true for IPv4-mapped-to-IPv6 address", func() {
+			// Because of non-obvious behaviour of net library when it comes to
+			// IPv4-mapped-to-IPv6 addresses (more in https://github.com/golang/go/issues/51906)
+			// we are adding tests where we explicitly create net.IPNet struct from a binary
+			// input.
+			addrs := []net.IPNet{
+				{
+					IP:   net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160}),
+					Mask: net.IPMask([]byte{255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0}),
+				},
+				{
+					IP:   net.IP([]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 192, 168, 1, 160}),
+					Mask: net.IPMask([]byte{255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}),
+				},
+			}
+
+			for _, addr := range addrs {
+				res := IsNetIPv6(addr)
+				Expect(res).To(Equal(true))
+			}
+		})
 	})
 })
 

pkg/utils/utils.go

@@ -7,13 +7,14 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 	"net/http"
 	"os"
 	"strings"
 	"time"
 
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
+
 	"github.com/sirupsen/logrus"
 )
 
@@ -135,3 +136,11 @@ func GetClientConfig(kubeApiServerUrl, kubeconfigPath string) (*rest.Config, err
 	config.Timeout = kubeClientTimeout
 	return config, err
 }
+
+func Mapper[T, U any](data []T, f func(T) U) []U {
+	res := make([]U, 0, len(data))
+	for _, e := range data {
+		res = append(res, f(e))
+	}
+	return res
+}