Repository: envoyproxy/gateway

Issue:
A `DualStack` `Gateway` with `envoyService.type: NodePort` only contains one IP per node in its status
*Description*:
In spite of `spec.ipFamily: DualStack`, a `Gateway` with `envoyService.type: NodePort` only contains one IP per node.
It should contain both IPs.

*Repro steps*:
Set up an `EnvoyProxy` with `spec.provider.kubernetes.envoyService.type: NodePort` and `spec.ipFamily: DualStack`. Check the `Gateway`'s `status.addresses`.

*Environment*:
Envoy gateway v1.4.1

*Logs*:
None relevant


Discussion:

Solution patch:
diff --git a/internal/gatewayapi/status/gateway.go b/internal/gatewayapi/status/gateway.go
index af45012b6..fd4785b33 100644
--- a/internal/gatewayapi/status/gateway.go
+++ b/internal/gatewayapi/status/gateway.go
@@ -7,6 +7,7 @@ package status
 
 import (
 	"fmt"
+	"slices"
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -43,10 +44,15 @@ func GatewayAccepted(gw *gwapiv1.Gateway) bool {
 	return !GatewayNotAccepted(gw)
 }
 
+type NodeAddresses struct {
+	IPv4 []string
+	IPv6 []string
+}
+
 // UpdateGatewayStatusProgrammedCondition updates the status addresses for the provided gateway
 // based on the status IP/Hostname of svc and updates the Programmed condition based on the
 // service and deployment or daemonset state.
-func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Service, envoyObj client.Object, nodeAddresses ...string) {
+func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Service, envoyObj client.Object, nodeAddresses NodeAddresses) {
 	var addresses, hostnames []string
 	// Update the status addresses field.
 	if svc != nil {
@@ -92,7 +98,14 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Ser
 			}
 
 			if svc.Spec.Type == corev1.ServiceTypeNodePort {
-				addresses = nodeAddresses
+				var relevantAddresses []string
+				if slices.Contains(svc.Spec.IPFamilies, corev1.IPv4Protocol) {
+					relevantAddresses = append(relevantAddresses, nodeAddresses.IPv4...)
+				}
+				if slices.Contains(svc.Spec.IPFamilies, corev1.IPv6Protocol) {
+					relevantAddresses = append(relevantAddresses, nodeAddresses.IPv6...)
+				}
+				addresses = relevantAddresses
 			}
 		}
 
diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go
index 5712f9aa7..b88df467c 100644
--- a/internal/provider/kubernetes/status.go
+++ b/internal/provider/kubernetes/status.go
@@ -576,7 +576,7 @@ func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *
 		// to true in the Gateway API translator
 		status.UpdateGatewayStatusAccepted(gtw)
 		// update address field and programmed condition
-		status.UpdateGatewayStatusProgrammedCondition(gtw, svc, envoyObj, r.store.listNodeAddresses()...)
+		status.UpdateGatewayStatusProgrammedCondition(gtw, svc, envoyObj, r.store.listNodeAddresses())
 	}
 
 	key := utils.NamespacedName(gtw)
diff --git a/internal/provider/kubernetes/store.go b/internal/provider/kubernetes/store.go
index eeb63a1da..c9da30903 100644
--- a/internal/provider/kubernetes/store.go
+++ b/internal/provider/kubernetes/store.go
@@ -6,14 +6,17 @@
 package kubernetes
 
 import (
+	"net"
 	"sync"
 
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/envoyproxy/gateway/internal/gatewayapi/status"
 )
 
 type nodeDetails struct {
-	name    string
-	address string
+	name      string
+	addresses status.NodeAddresses
 }
 
 // kubernetesProviderStore holds cached information for the kubernetes provider.
@@ -34,23 +37,31 @@ func newProviderStore() *kubernetesProviderStore {
 func (p *kubernetesProviderStore) addNode(n *corev1.Node) {
 	details := nodeDetails{name: n.Name}
 
-	var internalIP, externalIP string
+	var internalIPs, externalIPs status.NodeAddresses
 	for _, addr := range n.Status.Addresses {
-		if addr.Type == corev1.NodeExternalIP {
-			externalIP = addr.Address
+		var addrs *status.NodeAddresses
+		switch addr.Type {
+		case corev1.NodeExternalIP:
+			addrs = &externalIPs
+		case corev1.NodeInternalIP:
+			addrs = &internalIPs
+		default:
+			continue
 		}
-		if addr.Type == corev1.NodeInternalIP {
-			internalIP = addr.Address
+		if net.ParseIP(addr.Address).To4() != nil {
+			addrs.IPv4 = append(addrs.IPv4, addr.Address)
+		} else {
+			addrs.IPv6 = append(addrs.IPv6, addr.Address)
 		}
 	}
 
 	// In certain scenarios (like in local KinD clusters), the Node
 	// externalIP is not provided, in that case we default back
 	// to the internalIP of the Node.
-	if externalIP != "" {
-		details.address = externalIP
-	} else if internalIP != "" {
-		details.address = internalIP
+	if len(externalIPs.IPv4) > 0 || len(externalIPs.IPv6) > 0 {
+		details.addresses = externalIPs
+	} else if len(internalIPs.IPv4) > 0 || len(internalIPs.IPv6) > 0 {
+		details.addresses = internalIPs
 	}
 	p.mu.Lock()
 	defer p.mu.Unlock()
@@ -63,14 +74,13 @@ func (p *kubernetesProviderStore) removeNode(n *corev1.Node) {
 	delete(p.nodes, n.Name)
 }
 
-func (p *kubernetesProviderStore) listNodeAddresses() []string {
-	addrs := []string{}
+func (p *kubernetesProviderStore) listNodeAddresses() status.NodeAddresses {
 	p.mu.Lock()
 	defer p.mu.Unlock()
+	addrs := status.NodeAddresses{}
 	for _, n := range p.nodes {
-		if n.address != "" {
-			addrs = append(addrs, n.address)
-		}
+		addrs.IPv4 = append(addrs.IPv4, n.addresses.IPv4...)
+		addrs.IPv6 = append(addrs.IPv6, n.addresses.IPv6...)
 	}
 	return addrs
 }
diff --git a/release-notes/current.yaml b/release-notes/current.yaml
index 8a12601b1..67f627997 100644
--- a/release-notes/current.yaml
+++ b/release-notes/current.yaml
@@ -47,6 +47,7 @@ bug fixes: |
   Add ConfigMap indexers for EnvoyExtensionPolicies to reconcile Lua changes
   Fixed issue that default accesslog format not working.
   Fixed validation errors when the rateLimit url for Redis in the EnvoyGateway API includes multiple comma separated hosts.
+  Fixes addresses in status of DualStack NodePort Gateways.
 
 # Enhancements that improve performance.
 performance improvements: |



Test patch:
diff --git a/internal/gatewayapi/status/gateway_test.go b/internal/gatewayapi/status/gateway_test.go
index 0dcb550a1..fb6fa0faa 100644
--- a/internal/gatewayapi/status/gateway_test.go
+++ b/internal/gatewayapi/status/gateway_test.go
@@ -27,7 +27,7 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 		gw            *gwapiv1.Gateway
 		svc           *corev1.Service
 		deployment    *appsv1.Deployment
-		nodeAddresses []string
+		nodeAddresses NodeAddresses
 	}
 	tests := []struct {
 		name          string
@@ -52,6 +52,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 					Spec: corev1.ServiceSpec{
 						ClusterIPs: []string{"127.0.0.1"},
 						Type:       corev1.ServiceTypeLoadBalancer,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv4Protocol,
+						},
 					},
 					Status: corev1.ServiceStatus{
 						LoadBalancer: corev1.LoadBalancerStatus{
@@ -81,6 +84,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 					Spec: corev1.ServiceSpec{
 						ClusterIPs: []string{"127.0.0.1"},
 						Type:       corev1.ServiceTypeLoadBalancer,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv4Protocol,
+						},
 					},
 					Status: corev1.ServiceStatus{
 						LoadBalancer: corev1.LoadBalancerStatus{
@@ -114,6 +120,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 					Spec: corev1.ServiceSpec{
 						ClusterIPs: []string{"127.0.0.1"},
 						Type:       corev1.ServiceTypeClusterIP,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv4Protocol,
+						},
 					},
 				},
 			},
@@ -127,13 +136,18 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 		{
 			name: "Nodeport svc",
 			args: args{
-				gw:            &gwapiv1.Gateway{},
-				nodeAddresses: []string{"1", "2"},
+				gw: &gwapiv1.Gateway{},
+				nodeAddresses: NodeAddresses{
+					IPv4: []string{"1", "2"},
+				},
 				svc: &corev1.Service{
 					TypeMeta:   metav1.TypeMeta{},
 					ObjectMeta: metav1.ObjectMeta{},
 					Spec: corev1.ServiceSpec{
 						Type: corev1.ServiceTypeNodePort,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv4Protocol,
+						},
 					},
 				},
 			},
@@ -153,9 +167,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 			args: args{
 				gw: &gwapiv1.Gateway{},
 				// 20 node addresses
-				nodeAddresses: func() (addr []string) {
+				nodeAddresses: func() (addr NodeAddresses) {
 					for i := 0; i < 20; i++ {
-						addr = append(addr, strconv.Itoa(i))
+						addr.IPv4 = append(addr.IPv4, strconv.Itoa(i))
 					}
 					return
 				}(),
@@ -164,6 +178,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{},
 					Spec: corev1.ServiceSpec{
 						Type: corev1.ServiceTypeNodePort,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv4Protocol,
+						},
 					},
 				},
 			},
@@ -185,6 +202,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 				svc: &corev1.Service{
 					Spec: corev1.ServiceSpec{
 						Type: corev1.ServiceTypeLoadBalancer,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv6Protocol,
+						},
 					},
 					Status: corev1.ServiceStatus{
 						LoadBalancer: corev1.LoadBalancerStatus{
@@ -210,6 +230,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 					Spec: corev1.ServiceSpec{
 						ClusterIPs: []string{"2001:db8::2"},
 						Type:       corev1.ServiceTypeClusterIP,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv6Protocol,
+						},
 					},
 				},
 			},
@@ -223,11 +246,16 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 		{
 			name: "Nodeport svc with IPv6 node addresses",
 			args: args{
-				gw:            &gwapiv1.Gateway{},
-				nodeAddresses: []string{"2001:db8::3", "2001:db8::4"},
+				gw: &gwapiv1.Gateway{},
+				nodeAddresses: NodeAddresses{
+					IPv6: []string{"2001:db8::3", "2001:db8::4"},
+				},
 				svc: &corev1.Service{
 					Spec: corev1.ServiceSpec{
 						Type: corev1.ServiceTypeNodePort,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv6Protocol,
+						},
 					},
 				},
 			},
@@ -281,10 +309,34 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
 			},
 			wantAddresses: []gwapiv1.GatewayStatusAddress{},
 		},
+		{
+			name: "Nodeport svc Ipv6 with dual stack node addresses",
+			args: args{
+				gw: &gwapiv1.Gateway{},
+				nodeAddresses: NodeAddresses{
+					IPv4: []string{"10.0.0.1"},
+					IPv6: []string{"2001:db8::4"},
+				},
+				svc: &corev1.Service{
+					Spec: corev1.ServiceSpec{
+						Type: corev1.ServiceTypeNodePort,
+						IPFamilies: []corev1.IPFamily{
+							corev1.IPv6Protocol,
+						},
+					},
+				},
+			},
+			wantAddresses: []gwapiv1.GatewayStatusAddress{
+				{
+					Type:  ptr.To(gwapiv1.IPAddressType),
+					Value: "2001:db8::4",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			UpdateGatewayStatusProgrammedCondition(tt.args.gw, tt.args.svc, tt.args.deployment, tt.args.nodeAddresses...)
+			UpdateGatewayStatusProgrammedCondition(tt.args.gw, tt.args.svc, tt.args.deployment, tt.args.nodeAddresses)
 			assert.True(t, reflect.DeepEqual(tt.wantAddresses, tt.args.gw.Status.Addresses))
 		})
 	}
diff --git a/internal/provider/kubernetes/store_test.go b/internal/provider/kubernetes/store_test.go
index 07289d8b1..62563c17b 100644
--- a/internal/provider/kubernetes/store_test.go
+++ b/internal/provider/kubernetes/store_test.go
@@ -11,6 +11,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/envoyproxy/gateway/internal/gatewayapi/status"
 )
 
 func TestNodeDetailsAddressStore(t *testing.T) {
@@ -18,7 +20,7 @@ func TestNodeDetailsAddressStore(t *testing.T) {
 	testCases := []struct {
 		name              string
 		nodeObject        *corev1.Node
-		expectedAddresses []string
+		expectedAddresses status.NodeAddresses
 	}{
 		{
 			name: "No node addresses",
@@ -26,7 +28,7 @@ func TestNodeDetailsAddressStore(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{Name: "node1"},
 				Status:     corev1.NodeStatus{Addresses: []corev1.NodeAddress{{}}},
 			},
-			expectedAddresses: []string{},
+			expectedAddresses: status.NodeAddresses{},
 		},
 		{
 			name: "only external address",
@@ -37,7 +39,9 @@ func TestNodeDetailsAddressStore(t *testing.T) {
 					Type:    corev1.NodeExternalIP,
 				}}},
 			},
-			expectedAddresses: []string{"1.1.1.1"},
+			expectedAddresses: status.NodeAddresses{
+				IPv4: []string{"1.1.1.1"},
+			},
 		},
 		{
 			name: "only internal address",
@@ -48,7 +52,9 @@ func TestNodeDetailsAddressStore(t *testing.T) {
 					Type:    corev1.NodeInternalIP,
 				}}},
 			},
-			expectedAddresses: []string{"1.1.1.1"},
+			expectedAddresses: status.NodeAddresses{
+				IPv4: []string{"1.1.1.1"},
+			},
 		},
 		{
 			name: "prefer external address",
@@ -65,7 +71,43 @@ func TestNodeDetailsAddressStore(t *testing.T) {
 					},
 				}},
 			},
-			expectedAddresses: []string{"1.1.1.1"},
+			expectedAddresses: status.NodeAddresses{
+				IPv4: []string{"1.1.1.1"},
+			},
+		},
+		{
+			name: "all external addresses",
+			nodeObject: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node1"},
+				Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{
+					Address: "1.1.1.1",
+					Type:    corev1.NodeExternalIP,
+				}, {
+					Address: "2606:4700:4700::1111",
+					Type:    corev1.NodeExternalIP,
+				}}},
+			},
+			expectedAddresses: status.NodeAddresses{
+				IPv4: []string{"1.1.1.1"},
+				IPv6: []string{"2606:4700:4700::1111"},
+			},
+		},
+		{
+			name: "all internal addresses",
+			nodeObject: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node1"},
+				Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{
+					Address: "1.1.1.1",
+					Type:    corev1.NodeInternalIP,
+				}, {
+					Address: "2606:4700:4700::1111",
+					Type:    corev1.NodeInternalIP,
+				}}},
+			},
+			expectedAddresses: status.NodeAddresses{
+				IPv4: []string{"1.1.1.1"},
+				IPv6: []string{"2606:4700:4700::1111"},
+			},
 		},
 	}