Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: fix ipFamily always nil #4782

Merged
merged 13 commits into from
Dec 5, 2024
48 changes: 42 additions & 6 deletions internal/gatewayapi/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,20 +610,56 @@
}
}

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 {
zirain marked this conversation as resolved.
Show resolved Hide resolved
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)

Check warning on line 657 in internal/gatewayapi/helpers.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/helpers.go#L657

Added line #L657 was not covered by tests
case egv1a1.IPv6:
result = ir.IPv6
return ptr.To(egv1a1.IPv6)

Check warning on line 659 in internal/gatewayapi/helpers.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/helpers.go#L659

Added line #L659 was not covered by tests
case egv1a1.DualStack:
result = ir.DualStack
return ptr.To(egv1a1.DualStack)

Check warning on line 661 in internal/gatewayapi/helpers.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/helpers.go#L661

Added line #L661 was not covered by tests
default:
return nil
}
return &result
}
65 changes: 65 additions & 0 deletions internal/gatewayapi/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}
11 changes: 4 additions & 7 deletions internal/gatewayapi/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion internal/gatewayapi/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand Down
20 changes: 8 additions & 12 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 6 additions & 1 deletion internal/ir/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions internal/xds/translator/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ type ExtraArgs struct {
metrics *ir.Metrics
http1Settings *ir.HTTP1Settings
http2Settings *ir.HTTP2Settings
ipFamily *egv1a1.IPFamily
}

type clusterArgs interface {
Expand All @@ -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,
}
}

Expand All @@ -737,6 +739,7 @@ func (route *TCPRouteTranslator) asClusterArgs(extra *ExtraArgs) *xdsClusterArgs
metrics: extra.metrics,
backendConnection: route.BackendConnection,
dns: route.DNS,
ipFamily: extra.ipFamily,
}
}

Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions internal/xds/translator/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions internal/xds/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 40 additions & 0 deletions internal/xds/translator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Loading
Loading