xds: fix ipFamily always nil (envoyproxy#4782)

* chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> --------- Signed-off-by: Juwon Hwang (Kevin) <juwon8891@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com>
guydc · Dec 5, 2024 · 226082b · 226082b
1 parent c5f7fc1
commit 226082b
Show file tree

Hide file tree

Showing 13 changed files with 323 additions and 30 deletions.
diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go
@@ -610,20 +610,56 @@ func setIfNil[T any](target **T, value *T) {
 	}
 }
 
-func getIPFamily(envoyProxy *egv1a1.EnvoyProxy) *ir.IPFamily {
+// getServiceIPFamily returns the IP family configuration from a Kubernetes Service
+// following the dual-stack service configuration scenarios:
+// https://kubernetes.io/docs/concepts/services-networking/dual-stack/#dual-stack-service-configuration-scenarios
+//
+// The IP family is determined in the following order:
+// 1. Service.Spec.IPFamilyPolicy == RequireDualStack -> DualStack
+// 2. Service.Spec.IPFamilies length > 1 -> DualStack
+// 3. Service.Spec.IPFamilies[0] -> IPv4 or IPv6
+// 4. nil if not specified
+func getServiceIPFamily(service *corev1.Service) *egv1a1.IPFamily {
+	if service == nil {
+		return nil
+	}
+
+	// If ipFamilyPolicy is RequireDualStack, return DualStack
+	if service.Spec.IPFamilyPolicy != nil &&
+		*service.Spec.IPFamilyPolicy == corev1.IPFamilyPolicyRequireDualStack {
+		return ptr.To(egv1a1.DualStack)
+	}
+
+	// Check ipFamilies array
+	if len(service.Spec.IPFamilies) > 0 {
+		if len(service.Spec.IPFamilies) > 1 {
+			return ptr.To(egv1a1.DualStack)
+		}
+		switch service.Spec.IPFamilies[0] {
+		case corev1.IPv4Protocol:
+			return ptr.To(egv1a1.IPv4)
+		case corev1.IPv6Protocol:
+			return ptr.To(egv1a1.IPv6)
+		}
+	}
+
+	return nil
+}
+
+// getEnvoyIPFamily returns the IPFamily configuration from EnvoyProxy
+func getEnvoyIPFamily(envoyProxy *egv1a1.EnvoyProxy) *egv1a1.IPFamily {
 	if envoyProxy == nil || envoyProxy.Spec.IPFamily == nil {
 		return nil
 	}
-	var result ir.IPFamily
+
 	switch *envoyProxy.Spec.IPFamily {
 	case egv1a1.IPv4:
-		result = ir.IPv4
+		return ptr.To(egv1a1.IPv4)
 	case egv1a1.IPv6:
-		result = ir.IPv6
+		return ptr.To(egv1a1.IPv6)
 	case egv1a1.DualStack:
-		result = ir.DualStack
+		return ptr.To(egv1a1.DualStack)
 	default:
 		return nil
 	}
-	return &result
 }
diff --git a/internal/gatewayapi/helpers_test.go b/internal/gatewayapi/helpers_test.go
@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -551,3 +552,67 @@ func TestIsRefToGateway(t *testing.T) {
 		})
 	}
 }
+
+func TestGetServiceIPFamily(t *testing.T) {
+	testCases := []struct {
+		name     string
+		service  *corev1.Service
+		expected *egv1a1.IPFamily
+	}{
+		{
+			name:     "nil service",
+			service:  nil,
+			expected: nil,
+		},
+		{
+			name: "require dual stack",
+			service: &corev1.Service{
+				Spec: corev1.ServiceSpec{
+					IPFamilyPolicy: ptr.To(corev1.IPFamilyPolicyRequireDualStack),
+				},
+			},
+			expected: ptr.To(egv1a1.DualStack),
+		},
+		{
+			name: "multiple ip families",
+			service: &corev1.Service{
+				Spec: corev1.ServiceSpec{
+					IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol, corev1.IPv6Protocol},
+				},
+			},
+			expected: ptr.To(egv1a1.DualStack),
+		},
+		{
+			name: "ipv4 only",
+			service: &corev1.Service{
+				Spec: corev1.ServiceSpec{
+					IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol},
+				},
+			},
+			expected: ptr.To(egv1a1.IPv4),
+		},
+		{
+			name: "ipv6 only",
+			service: &corev1.Service{
+				Spec: corev1.ServiceSpec{
+					IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol},
+				},
+			},
+			expected: ptr.To(egv1a1.IPv6),
+		},
+		{
+			name: "no ip family specified",
+			service: &corev1.Service{
+				Spec: corev1.ServiceSpec{},
+			},
+			expected: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := getServiceIPFamily(tc.service)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}
diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go
@@ -102,8 +102,8 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource
 			}
 
 			address := net.IPv4ListenerAddress
-			ipFamily := getIPFamily(gateway.envoyProxy)
-			if ipFamily != nil && (*ipFamily == ir.IPv6 || *ipFamily == ir.DualStack) {
+			ipFamily := getEnvoyIPFamily(gateway.envoyProxy)
+			if ipFamily != nil && (*ipFamily == egv1a1.IPv6 || *ipFamily == egv1a1.DualStack) {
 				address = net.IPv6ListenerAddress
 			}
 
@@ -118,17 +118,14 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource
 						Address:  address,
 						Port:     uint32(containerPort),
 						Metadata: buildListenerMetadata(listener, gateway),
-						IPFamily: getIPFamily(gateway.envoyProxy),
+						IPFamily: ipFamily,
 					},
 					TLS: irTLSConfigs(listener.tlsSecrets...),
 					Path: ir.PathSettings{
 						MergeSlashes:         true,
 						EscapedSlashesAction: ir.UnescapeAndRedirect,
 					},
 				}
-				if ipFamily := getIPFamily(gateway.envoyProxy); ipFamily != nil {
-					irListener.CoreListenerDetails.IPFamily = ipFamily
-				}
 				if listener.Hostname != nil {
 					irListener.Hostnames = append(irListener.Hostnames, string(*listener.Hostname))
 				} else {
@@ -144,7 +141,7 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource
 						Name:     irListenerName(listener),
 						Address:  address,
 						Port:     uint32(containerPort),
-						IPFamily: getIPFamily(gateway.envoyProxy),
+						IPFamily: ipFamily,
 					},
 
 					// Gateway is processed firstly, then ClientTrafficPolicy, then xRoute.

diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -1232,6 +1232,7 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext,
 		addrType  *ir.DestinationAddressType
 	)
 	protocol := inspectAppProtocolByRouteKind(routeType)
+
 	switch KindDerefOr(backendRef.Kind, resource.KindService) {
 	case resource.KindServiceImport:
 		serviceImport := resources.GetServiceImport(backendNamespace, string(backendRef.Name))
@@ -1262,9 +1263,9 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext,
 			Endpoints:   endpoints,
 			AddressType: addrType,
 		}
+
 	case resource.KindService:
 		ds = t.processServiceDestinationSetting(backendRef.BackendObjectReference, backendNamespace, protocol, resources, envoyProxy)
-
 		ds.TLS = t.applyBackendTLSSetting(
 			backendRef.BackendObjectReference,
 			backendNamespace,
@@ -1280,6 +1281,7 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext,
 			envoyProxy,
 		)
 		ds.Filters = t.processDestinationFilters(routeType, backendRefContext, parentRef, route, resources)
+		ds.IPFamily = getServiceIPFamily(resources.GetService(backendNamespace, string(backendRef.Name)))
 	case egv1a1.KindBackend:
 		ds = t.processBackendDestinationSetting(backendRef.BackendObjectReference, backendNamespace, resources)
 

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -250,19 +250,12 @@ type CoreListenerDetails struct {
 	ExtensionRefs []*UnstructuredRef `json:"extensionRefs,omitempty" yaml:"extensionRefs,omitempty"`
 	// Metadata is used to enrich envoy resource metadata with user and provider-specific information
 	Metadata *ResourceMetadata `json:"metadata,omitempty" yaml:"metadata,omitempty"`
-	// IPFamily specifies the IP address family for the gateway.
-	// It can be IPv4, IPv6, or DualStack.
+	// IPFamily specifies the IP address family used by the Gateway for its listening ports.
 	IPFamily *IPFamily `json:"ipFamily,omitempty" yaml:"ipFamily,omitempty"`
 }
 
 // IPFamily specifies the IP address family used by the Gateway for its listening ports.
-type IPFamily string
-
-const (
-	IPv4      IPFamily = "IPv4"
-	IPv6      IPFamily = "IPv6"
-	DualStack IPFamily = "DualStack"
-)
+type IPFamily = egv1a1.IPFamily
 
 func (l CoreListenerDetails) GetName() string {
 	return l.Name
@@ -1308,9 +1301,11 @@ type DestinationSetting struct {
 	Endpoints []*DestinationEndpoint `json:"endpoints,omitempty" yaml:"endpoints,omitempty"`
 	// AddressTypeState specifies the state of DestinationEndpoint address type.
 	AddressType *DestinationAddressType `json:"addressType,omitempty" yaml:"addressType,omitempty"`
-
-	TLS     *TLSUpstreamConfig  `json:"tls,omitempty" yaml:"tls,omitempty"`
-	Filters *DestinationFilters `json:"filters,omitempty" yaml:"filters,omitempty"`
+	// IPFamily specifies the IP family (IPv4 or IPv6) to use for this destination's endpoints.
+	// This is derived from the backend service and endpoint slice information.
+	IPFamily *IPFamily           `json:"ipFamily,omitempty" yaml:"ipFamily,omitempty"`
+	TLS      *TLSUpstreamConfig  `json:"tls,omitempty" yaml:"tls,omitempty"`
+	Filters  *DestinationFilters `json:"filters,omitempty" yaml:"filters,omitempty"`
 }
 
 // Validate the fields within the RouteDestination structure
@@ -1686,6 +1681,7 @@ func (t TCPListener) Validate() error {
 
 func (t TCPRoute) Validate() error {
 	var errs error
+
 	if t.Name == "" {
 		errs = errors.Join(errs, ErrRouteNameEmpty)
 	}

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go
@@ -698,6 +698,7 @@ type ExtraArgs struct {
 	metrics       *ir.Metrics
 	http1Settings *ir.HTTP1Settings
 	http2Settings *ir.HTTP2Settings
+	ipFamily      *egv1a1.IPFamily
 }
 
 type clusterArgs interface {
@@ -716,6 +717,7 @@ func (route *UDPRouteTranslator) asClusterArgs(extra *ExtraArgs) *xdsClusterArgs
 		endpointType: buildEndpointType(route.Destination.Settings),
 		metrics:      extra.metrics,
 		dns:          route.DNS,
+		ipFamily:     extra.ipFamily,
 	}
 }
 
@@ -737,6 +739,7 @@ func (route *TCPRouteTranslator) asClusterArgs(extra *ExtraArgs) *xdsClusterArgs
 		metrics:           extra.metrics,
 		backendConnection: route.BackendConnection,
 		dns:               route.DNS,
+		ipFamily:          extra.ipFamily,
 	}
 }
 
@@ -754,6 +757,7 @@ func (httpRoute *HTTPRouteTranslator) asClusterArgs(extra *ExtraArgs) *xdsCluste
 		http1Settings:     extra.http1Settings,
 		http2Settings:     extra.http2Settings,
 		useClientProtocol: ptr.Deref(httpRoute.UseClientProtocol, false),
+		ipFamily:          extra.ipFamily,
 	}
 
 	// Populate traffic features.

diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go
@@ -180,7 +180,7 @@ func buildXdsTCPListener(
 		},
 	}
 
-	if ipFamily != nil && *ipFamily == ir.DualStack {
+	if ipFamily != nil && *ipFamily == egv1a1.DualStack {
 		socketAddress := listener.Address.GetSocketAddress()
 		socketAddress.Ipv4Compat = true
 	}
@@ -224,7 +224,7 @@ func buildXdsQuicListener(name, address string, port uint32, ipFamily *ir.IPFami
 		DrainType: listenerv3.Listener_MODIFY_ONLY,
 	}
 
-	if ipFamily != nil && *ipFamily == ir.DualStack {
+	if ipFamily != nil && *ipFamily == egv1a1.DualStack {
 		socketAddress := xdsListener.Address.GetSocketAddress()
 		socketAddress.Ipv4Compat = true
 	}
@@ -869,7 +869,7 @@ func buildXdsUDPListener(clusterName string, udpListener *ir.UDPListener, access
 		}},
 	}
 
-	if udpListener.IPFamily != nil && *udpListener.IPFamily == ir.DualStack {
+	if udpListener.IPFamily != nil && *udpListener.IPFamily == egv1a1.DualStack {
 		socketAddress := xdsListener.Address.GetSocketAddress()
 		socketAddress.Ipv4Compat = true
 	}

diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go
@@ -464,6 +464,7 @@ func (t *Translator) addRouteToRouteConfig(
 			ea := &ExtraArgs{
 				metrics:       metrics,
 				http1Settings: httpListener.HTTP1,
+				ipFamily:      determineIPFamily(httpRoute.Destination.Settings),
 			}
 
 			if httpRoute.Traffic != nil && httpRoute.Traffic.HTTP2 != nil {

diff --git a/internal/xds/translator/utils.go b/internal/xds/translator/utils.go
@@ -196,3 +196,43 @@ func addClusterFromURL(url string, tCtx *types.ResourceVersionTable) error {
 
 	return addXdsCluster(tCtx, clusterArgs)
 }
+
+// determineIPFamily determines the IP family based on multiple destination settings
+func determineIPFamily(settings []*ir.DestinationSetting) *egv1a1.IPFamily {
+	// If there's only one setting, return its IPFamily directly
+	if len(settings) == 1 {
+		return settings[0].IPFamily
+	}
+
+	hasIPv4 := false
+	hasIPv6 := false
+	hasDualStack := false
+
+	for _, setting := range settings {
+		if setting.IPFamily == nil {
+			continue
+		}
+
+		switch *setting.IPFamily {
+		case egv1a1.IPv4:
+			hasIPv4 = true
+		case egv1a1.IPv6:
+			hasIPv6 = true
+		case egv1a1.DualStack:
+			hasDualStack = true
+		}
+	}
+
+	switch {
+	case hasDualStack:
+		return ptr.To(egv1a1.DualStack)
+	case hasIPv4 && hasIPv6:
+		return ptr.To(egv1a1.DualStack)
+	case hasIPv4:
+		return ptr.To(egv1a1.IPv4)
+	case hasIPv6:
+		return ptr.To(egv1a1.IPv6)
+	default:
+		return nil
+	}
+}