From 226082b52c9dcbf85b22bd3c01f35509dc0cce2f Mon Sep 17 00:00:00 2001 From: Kevin Date: Thu, 5 Dec 2024 09:17:29 +0900 Subject: [PATCH] xds: fix ipFamily always nil (#4782) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) * chore: fix ipFamily always nil Signed-off-by: Juwon Hwang (Kevin) --------- Signed-off-by: Juwon Hwang (Kevin) Co-authored-by: zirain --- internal/gatewayapi/helpers.go | 48 +++++++-- internal/gatewayapi/helpers_test.go | 65 ++++++++++++ internal/gatewayapi/listener.go | 11 +- internal/gatewayapi/route.go | 4 +- internal/ir/xds.go | 20 ++-- internal/ir/zz_generated.deepcopy.go | 7 +- internal/xds/translator/cluster.go | 4 + internal/xds/translator/listener.go | 6 +- internal/xds/translator/translator.go | 1 + internal/xds/translator/utils.go | 40 ++++++++ internal/xds/translator/utils_test.go | 112 +++++++++++++++++++++ test/e2e/testdata/httproute-dualstack.yaml | 32 ++++++ test/e2e/tests/httproute_dualstack.go | 3 + 13 files changed, 323 insertions(+), 30 deletions(-) create mode 100644 internal/xds/translator/utils_test.go diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go index 6ed1d7699a6..2626e1b4be3 100644 --- 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 index 5698867c3ca..6403279a5a9 100644 --- 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 index bf369e7b827..75739609609 100644 --- 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,7 +118,7 @@ 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{ @@ -126,9 +126,6 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource 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 index 26627a07285..ddada5f17b6 100644 --- 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 index 7114afc1f22..486b7514cd4 100644 --- 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 index f7384181dff..6db14262456 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -592,7 +592,7 @@ func (in *CoreListenerDetails) DeepCopyInto(out *CoreListenerDetails) { } if in.IPFamily != nil { in, out := &in.IPFamily, &out.IPFamily - *out = new(IPFamily) + *out = new(v1alpha1.IPFamily) **out = **in } } @@ -772,6 +772,11 @@ func (in *DestinationSetting) DeepCopyInto(out *DestinationSetting) { *out = new(DestinationAddressType) **out = **in } + if in.IPFamily != nil { + in, out := &in.IPFamily, &out.IPFamily + *out = new(v1alpha1.IPFamily) + **out = **in + } if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSUpstreamConfig) diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go index c5064c29eef..10792bae24b 100644 --- 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 index 1568ed3e570..36cf9a8953b 100644 --- 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 index 1e0ae77e915..79f16d5d1b5 100644 --- 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 index 882d9b1e926..30c2f771ef0 100644 --- 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 + } +} diff --git a/internal/xds/translator/utils_test.go b/internal/xds/translator/utils_test.go new file mode 100644 index 00000000000..588c68690b6 --- /dev/null +++ b/internal/xds/translator/utils_test.go @@ -0,0 +1,112 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package translator + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/utils/ptr" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/ir" +) + +func TestDetermineIPFamily(t *testing.T) { + tests := []struct { + name string + settings []*ir.DestinationSetting + want *egv1a1.IPFamily + }{ + { + name: "nil settings should return nil", + settings: nil, + want: nil, + }, + { + name: "empty settings should return nil", + settings: []*ir.DestinationSetting{}, + want: nil, + }, + { + name: "single IPv4 setting", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv4)}, + }, + want: ptr.To(egv1a1.IPv4), + }, + { + name: "single IPv6 setting", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv6)}, + }, + want: ptr.To(egv1a1.IPv6), + }, + { + name: "single DualStack setting", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.DualStack)}, + }, + want: ptr.To(egv1a1.DualStack), + }, + { + name: "mixed IPv4 and IPv6 should return DualStack", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv4)}, + {IPFamily: ptr.To(egv1a1.IPv6)}, + }, + want: ptr.To(egv1a1.DualStack), + }, + { + name: "DualStack with IPv4 should return DualStack", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.DualStack)}, + {IPFamily: ptr.To(egv1a1.IPv4)}, + }, + want: ptr.To(egv1a1.DualStack), + }, + { + name: "DualStack with IPv6 should return DualStack", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.DualStack)}, + {IPFamily: ptr.To(egv1a1.IPv6)}, + }, + want: ptr.To(egv1a1.DualStack), + }, + { + name: "mixed with nil IPFamily should be ignored", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv4)}, + {IPFamily: nil}, + {IPFamily: ptr.To(egv1a1.IPv6)}, + }, + want: ptr.To(egv1a1.DualStack), + }, + { + name: "multiple IPv4 settings should return IPv4", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv4)}, + {IPFamily: ptr.To(egv1a1.IPv4)}, + }, + want: ptr.To(egv1a1.IPv4), + }, + { + name: "multiple IPv6 settings should return IPv6", + settings: []*ir.DestinationSetting{ + {IPFamily: ptr.To(egv1a1.IPv6)}, + {IPFamily: ptr.To(egv1a1.IPv6)}, + }, + want: ptr.To(egv1a1.IPv6), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := determineIPFamily(tt.settings) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/test/e2e/testdata/httproute-dualstack.yaml b/test/e2e/testdata/httproute-dualstack.yaml index e1289dac50e..97a79c78ac3 100644 --- a/test/e2e/testdata/httproute-dualstack.yaml +++ b/test/e2e/testdata/httproute-dualstack.yaml @@ -95,6 +95,30 @@ spec: selector: app: infra-backend-v1 --- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: infra-backend-v1-httproute-all-stacks + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: dualstack-gateway + rules: + - backendRefs: + - name: infra-backend-v1-service-ipv4 + port: 8080 + weight: 30 + - name: infra-backend-v1-service-ipv6 + port: 8080 + weight: 30 + - name: infra-backend-v1-service-dualstack + port: 8080 + weight: 40 + matches: + - path: + type: PathPrefix + value: /all-stacks +--- apiVersion: gateway.envoyproxy.io/v1alpha1 kind: EnvoyProxy metadata: @@ -119,3 +143,11 @@ spec: - name: http port: 80 protocol: HTTP +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + name: envoy-gateway + namespace: gateway-conformance-infra +spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller diff --git a/test/e2e/tests/httproute_dualstack.go b/test/e2e/tests/httproute_dualstack.go index b01fc392a12..f765b08cd1a 100644 --- a/test/e2e/tests/httproute_dualstack.go +++ b/test/e2e/tests/httproute_dualstack.go @@ -38,6 +38,9 @@ var HTTPRouteDualStackTest = suite.ConformanceTest{ t.Run("HTTPRoute to IPv4 only service", func(t *testing.T) { runHTTPRouteTest(t, suite, ns, gwNN, "infra-backend-v1-httproute-ipv4", "/ipv4-only") }) + t.Run("HTTPRoute to All-stacks services", func(t *testing.T) { + runHTTPRouteTest(t, suite, ns, gwNN, "infra-backend-v1-httproute-all-stacks", "/all-stacks") + }) }, }