fix: DualStack NodePort Gateway's status.addresses should contain addresses from both IP families

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
 			}
 		}
 

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))
 		})
 	}

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)

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
 }

internal/provider/kubernetes/store_test.go

@@ -11,22 +11,24 @@ 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) {
 	store := newProviderStore()
 	testCases := []struct {
 		name              string
 		nodeObject        *corev1.Node
-		expectedAddresses []string
+		expectedAddresses status.NodeAddresses
 	}{
 		{
 			name: "No node addresses",
 			nodeObject: &corev1.Node{
 				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"},
+			},
 		},
 	}
 

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: |