Refactor IPv4-only functions to also work for IPv6 (#12303)

The main change here is the refactoring of the address functions in `addr.go` that support the Destination controller and Viz's Tap controller. Some of those functions only worked for IPv4, so this change refactored them to make them IP family agnostic.
This enabled adding (and fixing) IPv6 unit tests as detailed in the following sections.

Other changes:

- The `ProxyAddressesToString()` function was no longer used, so it got removed.
- The `ProxyIPToString()` function was only used by the destination-client script, so that got stripped out.

## `addr_test.go`

We added IPv6 cases to each test, that would have failed previously.

## `endpoint_translator_test.go`

One of the test pods (pod3) was changed to have an IPv6. Without the other changes in this PR those tests would still have passed, but just because when comparing actual IPs with expected ones we weren't checking if they were both zero. So here we added checks against that.

## `server_test.go`

As above, we added checks against empty IPs. And in the mocked resources in `test_util.go` we added an IPv6 EndpointSlice.

controller/api/destination/endpoint_translator.go

@@ -478,7 +478,7 @@ func (et *endpointTranslator) sendClientRemove(set watcher.AddressSet) {
 }
 
 func toAddr(address watcher.Address) (*net.TcpAddress, error) {
-	ip, err := addr.ParseProxyIPV4(address.IP)
+	ip, err := addr.ParseProxyIP(address.IP)
 	if err != nil {
 		return nil, err
 	}

controller/api/destination/endpoint_translator_test.go

@@ -14,6 +14,7 @@ import (
 	ewv1beta1 "github.com/linkerd/linkerd2/controller/gen/apis/externalworkload/v1beta1"
 	"github.com/linkerd/linkerd2/pkg/addr"
 	"github.com/linkerd/linkerd2/pkg/k8s"
+	"google.golang.org/protobuf/proto"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -56,7 +57,7 @@ var (
 	}
 
 	pod3 = watcher.Address{
-		IP:   "1.1.1.3",
+		IP:   "2001:0db8:85a3:0000:0000:8a2e:0370:7334",
 		Port: 3,
 		Pod: &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
@@ -819,7 +820,7 @@ func checkAddressAndWeight(t *testing.T, actual *pb.WeightedAddr, expected watch
 func checkAddress(t *testing.T, actual *net.TcpAddress, expected watcher.Address) {
 	t.Helper()
 
-	expectedAddr, err := addr.ParseProxyIPV4(expected.IP)
+	expectedAddr, err := addr.ParseProxyIP(expected.IP)
 	expectedTCP := net.TcpAddress{
 		Ip:   expectedAddr,
 		Port: expected.Port,
@@ -827,11 +828,14 @@ func checkAddress(t *testing.T, actual *net.TcpAddress, expected watcher.Address
 	if err != nil {
 		t.Fatalf("Failed to parse expected IP [%s]: %s", expected.IP, err)
 	}
-	if actual.Ip.GetIpv4() != expectedTCP.Ip.GetIpv4() {
+	if actual.Ip.GetIpv4() == 0 && actual.Ip.GetIpv6() == nil {
-		t.Fatalf("Expected IP [%+v] but got [%+v]", expectedTCP.Ip, actual.Ip)
+		t.Fatal("Actual IP is empty")
 	}
-	if actual.Ip.GetIpv6() != expectedTCP.Ip.GetIpv6() {
+	if actual.Ip.GetIpv4() != expectedTCP.Ip.GetIpv4() {
-		t.Fatalf("Expected IP [%+v] but got [%+v]", expectedTCP.Ip, actual.Ip)
+		t.Fatalf("Expected IPv4 [%+v] but got [%+v]", expectedTCP.Ip, actual.Ip)
+	}
+	if !proto.Equal(actual.Ip.GetIpv6(), expectedTCP.Ip.GetIpv6()) {
+		t.Fatalf("Expected IPv6 [%+v] but got [%+v]", expectedTCP.Ip, actual.Ip)
 	}
 	if actual.Port != expectedTCP.Port {
 		t.Fatalf("Expected port [%+v] but got [%+v]", expectedTCP.Port, actual.Port)

controller/api/destination/server_test.go

@@ -449,6 +449,9 @@ func TestGetProfiles(t *testing.T) {
 		if first.GetEndpoint().GetProtocolHint().GetOpaqueTransport() != nil {
 			t.Fatalf("Expected pod to not support opaque traffic on port %d", port)
 		}
+		if first.Endpoint.Addr.Ip.GetIpv4() == 0 && first.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if first.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP to be %s, but it was %s", epAddr.Ip, first.Endpoint.Addr.Ip)
 		}
@@ -490,6 +493,9 @@ func TestGetProfiles(t *testing.T) {
 		if first.GetEndpoint().GetProtocolHint().GetOpaqueTransport() != nil {
 			t.Fatalf("Expected pod to not support opaque traffic on port %d", port)
 		}
+		if first.Endpoint.Addr.Ip.GetIpv4() == 0 && first.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if first.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP to be %s, but it was %s", epAddr.Ip, first.Endpoint.Addr.Ip)
 		}
@@ -531,6 +537,9 @@ func TestGetProfiles(t *testing.T) {
 		if first.GetEndpoint().GetProtocolHint().GetOpaqueTransport() != nil {
 			t.Fatalf("Expected pod to not support opaque traffic on port %d", port)
 		}
+		if first.Endpoint.Addr.Ip.GetIpv4() == 0 && first.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if first.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP to be %s, but it was %s", epAddr.Ip, first.Endpoint.Addr.Ip)
 		}
@@ -572,6 +581,9 @@ func TestGetProfiles(t *testing.T) {
 		if first.GetEndpoint().GetProtocolHint().GetOpaqueTransport() != nil {
 			t.Fatalf("Expected externalworkload to not support opaque traffic on port %d", port)
 		}
+		if first.Endpoint.Addr.Ip.GetIpv4() == 0 && first.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if first.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP to be %s, but it was %s", epAddr.Ip, first.Endpoint.Addr.Ip)
 		}
@@ -691,6 +703,9 @@ func TestGetProfiles(t *testing.T) {
 		if profile.Endpoint.GetProtocolHint().GetOpaqueTransport().GetInboundPort() != 4143 {
 			t.Fatalf("Expected pod to support opaque traffic on port 4143")
 		}
+		if profile.Endpoint.Addr.Ip.GetIpv4() == 0 && profile.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if profile.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP port to be %d, but it was %d", epAddr.Port, profile.Endpoint.Addr.Port)
 		}
@@ -766,6 +781,9 @@ func TestGetProfiles(t *testing.T) {
 		if profile.Endpoint.GetProtocolHint().GetOpaqueTransport().GetInboundPort() != 4143 {
 			t.Fatalf("Expected pod to support opaque traffic on port 4143")
 		}
+		if profile.Endpoint.Addr.Ip.GetIpv4() == 0 && profile.Endpoint.Addr.Ip.GetIpv6() == nil {
+			t.Fatal("IP is empty")
+		}
 		if profile.Endpoint.Addr.String() != epAddr.String() {
 			t.Fatalf("Expected endpoint IP port to be %d, but it was %d", epAddr.Port, profile.Endpoint.Addr.Port)
 		}
@@ -854,7 +872,7 @@ func TestGetProfiles(t *testing.T) {
 			t.Fatalf("Expected dst_pod to be %s got %s", "hostport-mapping", dstPod)
 		}
 
-		ip, err := addr.ParseProxyIPV4(externalIP)
+		ip, err := addr.ParseProxyIP(externalIP)
 		if err != nil {
 			t.Fatalf("Error parsing IP: %s", err)
 		}
@@ -1047,7 +1065,7 @@ func updateAddAddress(t *testing.T, update *pb.Update) []string {
 }
 
 func toAddress(path string, port uint32) (*net.TcpAddress, error) {
-	ip, err := addr.ParseProxyIPV4(path)
+	ip, err := addr.ParseProxyIP(path)
 	if err != nil {
 		return nil, err
 	}

controller/api/destination/test_util.go

@@ -41,7 +41,7 @@ spec:
 apiVersion: discovery.k8s.io/v1
 kind: EndpointSlice
 metadata:
-  name: name1
+  name: name1-ipv4
   namespace: ns
   labels:
     kubernetes.io/service-name: name1
@@ -54,6 +54,25 @@ endpoints:
     name: name1-1
     namespace: ns
 ports:
+- port: 8989
+  protocol: TCP`,
+		`
+apiVersion: discovery.k8s.io/v1
+kind: EndpointSlice
+metadata:
+  name: name1-ipv6
+  namespace: ns
+  labels:
+    kubernetes.io/service-name: name1
+addressType: IPv6
+endpoints:
+- addresses:
+  - 2001:db8::90
+  targetRef:
+    kind: Pod
+    name: name1-1
+    namespace: ns
+ports:
 - port: 8989
   protocol: TCP`,
 		`

controller/script/destination-client/main.go

@@ -64,7 +64,7 @@ func get(client pb.DestinationClient, req *pb.GetDestination) {
 			log.Println("Add:")
 			log.Printf("labels: %v", updateType.Add.MetricLabels)
 			for _, addr := range updateType.Add.Addrs {
-				log.Printf("- %s:%d", addrUtil.ProxyIPToString(addr.Addr.GetIp()), addr.Addr.Port)
+				log.Printf("- %s:%d", addrUtil.ProxyAddressToString(addr.Addr), addr.Addr.Port)
 				log.Printf("  - labels: %v", addr.MetricLabels)
 				switch addr.GetProtocolHint().GetProtocol().(type) {
 				case *pb.ProtocolHint_H2_:
@@ -82,7 +82,7 @@ func get(client pb.DestinationClient, req *pb.GetDestination) {
 		case *pb.Update_Remove:
 			log.Println("Remove:")
 			for _, addr := range updateType.Remove.Addrs {
-				log.Printf("- %s:%d", addrUtil.ProxyIPToString(addr.GetIp()), addr.Port)
+				log.Printf("- %s:%d", addrUtil.ProxyAddressToString(addr), addr.Port)
 			}
 			log.Println()
 		case *pb.Update_NoEndpoints:

pkg/addr/addr.go

@@ -5,8 +5,8 @@ import (
 	"fmt"
 	"math/big"
 	"net"
+	"net/netip"
 	"strconv"
-	"strings"
 
 	pb "github.com/linkerd/linkerd2-proxy-api/go/net"
 	l5dNetPb "github.com/linkerd/linkerd2/controller/gen/common/net"
@@ -46,78 +46,57 @@ func PublicIPToString(ip *l5dNetPb.IPAddress) string {
 
 // ProxyAddressToString formats a Proxy API TCPAddress as a string.
 func ProxyAddressToString(addr *pb.TcpAddress) string {
-	netIP := decodeIPv4ToNetIP(addr.GetIp().GetIpv4())
+	vizIP := proxyToVizIPAddr(addr.GetIp())
+	if vizIP == nil {
+		return ""
+	}
+	strIP := PublicIPToString(vizIP)
 	strPort := strconv.Itoa(int(addr.GetPort()))
-	return net.JoinHostPort(netIP.String(), strPort)
+	return net.JoinHostPort(strIP, strPort)
 }
 
-// ProxyAddressesToString formats a list of Proxy API TCPAddresses as a string.
+// ParseProxyIP parses an IP Address string into a Proxy API IPAddress.
-func ProxyAddressesToString(addrs []pb.TcpAddress) string {
+func ParseProxyIP(ip string) (*pb.IPAddress, error) {
-	addrStrs := make([]string, len(addrs))
+	addr, err := netip.ParseAddr(ip)
-	for i := range addrs {
+	if err != nil {
-		addrStrs[i] = ProxyAddressToString(&addrs[i])
+		return nil, fmt.Errorf("invalid IP address: %s", ip)
-	}
-	return "[" + strings.Join(addrStrs, ",") + "]"
-}
-
-// ProxyIPToString formats a Proxy API IPAddress as a string.
-func ProxyIPToString(ip *pb.IPAddress) string {
-	netIP := decodeIPv4ToNetIP(ip.GetIpv4())
-	return netIP.String()
-}
-
-// ParseProxyIPV4 parses an IP Address string into a Proxy API IPAddress.
-func ParseProxyIPV4(ip string) (*pb.IPAddress, error) {
-	netIP := net.ParseIP(ip)
-	if netIP == nil {
-		return nil, fmt.Errorf("Invalid IP address: %s", ip)
 	}
 
-	oBigInt := IPToInt(netIP.To4())
+	if addr.Is4() {
-	return &pb.IPAddress{
+		ipBytes := addr.As4()
-		Ip: &pb.IPAddress_Ipv4{
+		return &pb.IPAddress{
-			Ipv4: uint32(oBigInt.Uint64()),
+			Ip: &pb.IPAddress_Ipv4{
-		},
+				Ipv4: binary.BigEndian.Uint32(ipBytes[:]),
-	}, nil
-}
-
-// ParsePublicIPV4 parses an IP Address string into a Viz API IPAddress.
-func ParsePublicIPV4(ip string) (*l5dNetPb.IPAddress, error) {
-	netIP := net.ParseIP(ip)
-	if netIP != nil {
-		oBigInt := IPToInt(netIP.To4())
-		netIPAddress := &l5dNetPb.IPAddress{
-			Ip: &l5dNetPb.IPAddress_Ipv4{
-				Ipv4: uint32(oBigInt.Uint64()),
 			},
-		}
+		}, nil
-		return netIPAddress, nil
+	} else if addr.Is6() {
+		ipBytes := addr.As16()
+		return &pb.IPAddress{
+			Ip: &pb.IPAddress_Ipv6{
+				Ipv6: &pb.IPv6{
+					First: binary.BigEndian.Uint64(ipBytes[:8]),
+					Last:  binary.BigEndian.Uint64(ipBytes[8:]),
+				},
+			},
+		}, nil
 	}
-	return nil, fmt.Errorf("Invalid IP address: %s", ip)
+
+	return nil, fmt.Errorf("invalid IP address: %s", ip)
+}
+
+// ParsePublicIP parses an IP Address string into a Viz API IPAddress.
+func ParsePublicIP(ip string) (*l5dNetPb.IPAddress, error) {
+	addr, err := ParseProxyIP(ip)
+	if err != nil {
+		return nil, err
+	}
+	return proxyToVizIPAddr(addr), nil
 }
 
 // NetToPublic converts a Proxy API TCPAddress to a Viz API
 // TCPAddress.
 func NetToPublic(net *pb.TcpAddress) *l5dNetPb.TcpAddress {
-	var ip *l5dNetPb.IPAddress
+	ip := proxyToVizIPAddr(net.GetIp())
-
-	switch i := net.GetIp().GetIp().(type) {
-	case *pb.IPAddress_Ipv6:
-		ip = &l5dNetPb.IPAddress{
-			Ip: &l5dNetPb.IPAddress_Ipv6{
-				Ipv6: &l5dNetPb.IPv6{
-					First: i.Ipv6.First,
-					Last:  i.Ipv6.Last,
-				},
-			},
-		}
-	case *pb.IPAddress_Ipv4:
-		ip = &l5dNetPb.IPAddress{
-			Ip: &l5dNetPb.IPAddress_Ipv4{
-				Ipv4: i.Ipv4,
-			},
-		}
-	}
 
 	return &l5dNetPb.TcpAddress{
 		Ip:   ip,
@@ -125,6 +104,28 @@ func NetToPublic(net *pb.TcpAddress) *l5dNetPb.TcpAddress {
 	}
 }
 
+func proxyToVizIPAddr(net *pb.IPAddress) *l5dNetPb.IPAddress {
+	switch ip := net.GetIp().(type) {
+	case *pb.IPAddress_Ipv6:
+		return &l5dNetPb.IPAddress{
+			Ip: &l5dNetPb.IPAddress_Ipv6{
+				Ipv6: &l5dNetPb.IPv6{
+					First: ip.Ipv6.First,
+					Last:  ip.Ipv6.Last,
+				},
+			},
+		}
+	case *pb.IPAddress_Ipv4:
+		return &l5dNetPb.IPAddress{
+			Ip: &l5dNetPb.IPAddress_Ipv4{
+				Ipv4: ip.Ipv4,
+			},
+		}
+	}
+
+	return nil
+}
+
 // decodeIPv4ToNetIP converts IPv4 uint32 to an IPv4 net IP.
 func decodeIPv4ToNetIP(ip uint32) net.IP {
 	oBigInt := big.NewInt(0)

pkg/addr/addr_test.go

@@ -76,7 +76,7 @@ func TestPublicIPToString(t *testing.T) {
 			expected: "192.168.0.1",
 		},
 		{
-			name: "narmal ipv6",
+			name: "normal ipv6",
 			addr: &l5dNetPb.IPAddress{
 				Ip: &l5dNetPb.IPAddress_Ipv6{
 					Ipv6: &l5dNetPb.IPv6{
@@ -117,85 +117,6 @@ func TestPublicIPToString(t *testing.T) {
 	}
 }
 
-func TestProxyAddressesToString(t *testing.T) {
-	cases := []struct {
-		name     string
-		addrs    []pb.TcpAddress
-		expected string
-	}{
-		{
-			name: "ipv4",
-			addrs: []pb.TcpAddress{
-				{
-					Ip: &pb.IPAddress{
-						Ip: &pb.IPAddress_Ipv4{
-							Ipv4: 3232235521,
-						},
-					},
-					Port: 1234,
-				},
-				{
-					Ip: &pb.IPAddress{
-						Ip: &pb.IPAddress_Ipv4{
-							Ipv4: 3232235522,
-						},
-					},
-					Port: 1234,
-				},
-			},
-			expected: "[192.168.0.1:1234,192.168.0.2:1234]",
-		},
-		{
-			name:     "nil",
-			addrs:    nil,
-			expected: "[]",
-		},
-	}
-
-	for _, c := range cases {
-		c := c
-		t.Run(c.name, func(t *testing.T) {
-			got := ProxyAddressesToString(c.addrs)
-			if c.expected != got {
-				t.Errorf("expected: %v, got: %v", c.expected, got)
-			}
-		})
-	}
-}
-
-func TestProxyIPToString(t *testing.T) {
-	cases := []struct {
-		name     string
-		ip       *pb.IPAddress
-		expected string
-	}{
-		{
-			name: "ipv4",
-			ip: &pb.IPAddress{
-				Ip: &pb.IPAddress_Ipv4{
-					Ipv4: 3232235521,
-				},
-			},
-			expected: "192.168.0.1",
-		},
-		{
-			name:     "nil",
-			ip:       nil,
-			expected: "0.0.0.0",
-		},
-	}
-
-	for _, c := range cases {
-		c := c
-		t.Run(c.name, func(t *testing.T) {
-			got := ProxyIPToString(c.ip)
-			if c.expected != got {
-				t.Errorf("expected: %v, got: %v", c.expected, got)
-			}
-		})
-	}
-}
-
 func TestNetToPublic(t *testing.T) {
 
 	type addrExp struct {
@@ -255,7 +176,7 @@ func TestNetToPublic(t *testing.T) {
 	}
 }
 
-func TestParseProxyIPV4(t *testing.T) {
+func TestParseProxyIP(t *testing.T) {
 	var testCases = []struct {
 		ip      string
 		expAddr *pb.IPAddress
@@ -278,10 +199,22 @@ func TestParseProxyIPV4(t *testing.T) {
 			},
 			expErr: false,
 		},
+		{
+			ip: "2001:db8:85a3::8a2e:370:7334",
+			expAddr: &pb.IPAddress{
+				Ip: &pb.IPAddress_Ipv6{
+					Ipv6: &pb.IPv6{
+						First: 2306139570357600256,
+						Last:  151930230829876,
+					},
+				},
+			},
+			expErr: false,
+		},
 	}
 
 	for _, testCase := range testCases {
-		res, err := ParseProxyIPV4(testCase.ip)
+		res, err := ParseProxyIP(testCase.ip)
 		if testCase.expErr && err == nil {
 			t.Fatalf("expected get err, but get nil")
 		}
@@ -296,7 +229,7 @@ func TestParseProxyIPV4(t *testing.T) {
 	}
 }
 
-func TestParsePublicIPV4(t *testing.T) {
+func TestParsePublicIP(t *testing.T) {
 	var testCases = []struct {
 		ip      string
 		expAddr *l5dNetPb.IPAddress
@@ -319,10 +252,22 @@ func TestParsePublicIPV4(t *testing.T) {
 			},
 			expErr: false,
 		},
+		{
+			ip: "2001:db8:85a3::8a2e:370:7334",
+			expAddr: &l5dNetPb.IPAddress{
+				Ip: &l5dNetPb.IPAddress_Ipv6{
+					Ipv6: &l5dNetPb.IPv6{
+						First: 2306139570357600256,
+						Last:  151930230829876,
+					},
+				},
+			},
+			expErr: false,
+		},
 	}
 
 	for _, testCase := range testCases {
-		res, err := ParsePublicIPV4(testCase.ip)
+		res, err := ParsePublicIP(testCase.ip)
 		if testCase.expErr && err == nil {
 			t.Fatalf("expected get err, but get nil")
 		}
@@ -356,6 +301,20 @@ func TestProxyAddressToString(t *testing.T) {
 			},
 			expStr: "0.0.255.255:5678",
 		},
+		{
+			addr: &pb.TcpAddress{
+				Ip: &pb.IPAddress{
+					Ip: &pb.IPAddress_Ipv6{
+						Ipv6: &pb.IPv6{
+							First: 2306139570357600256,
+							Last:  151930230829876,
+						},
+					},
+				},
+				Port: 5678,
+			},
+			expStr: "[2001:db8:85a3::8a2e:370:7334]:5678",
+		},
 	}
 
 	for _, testCase := range testCases {

viz/cmd/tap_test.go

@@ -271,8 +271,8 @@ func TestEventToString(t *testing.T) {
 			httpEvent.GetResponseEnd().Id = streamID
 		}
 
-		srcIP, _ := addr.ParsePublicIPV4("1.2.3.4")
+		srcIP, _ := addr.ParsePublicIP("1.2.3.4")
-		destIP, _ := addr.ParsePublicIPV4("2.3.4.5")
+		destIP, _ := addr.ParsePublicIP("2.3.4.5")
 		return &tapPb.TapEvent{
 			ProxyDirection: tapPb.TapEvent_OUTBOUND,
 			Source: &netPb.TcpAddress{

viz/tap/api/grpc_server_test.go

@@ -667,7 +667,7 @@ status:
 			k8sAPI.Sync(nil)
 
 			labels := make(map[string]string)
-			ip, err := addr.ParsePublicIPV4(exp.requestedIP)
+			ip, err := addr.ParsePublicIP(exp.requestedIP)
 			if err != nil {
 				t.Fatalf("Error parsing IP %s: %s", exp.requestedIP, err)
 			}