fix: resolve socket settings across listeners sharing an address and port

api/v1alpha1/clienttrafficpolicy_types.go

@@ -50,6 +50,10 @@ type ClientTrafficPolicySpec struct {
 	// If defined, sets SO_KEEPALIVE on the listener socket to enable TCP Keepalives.
 	// Disabled by default.
 	//
+	// This setting belongs to the listener socket, which is shared by every Gateway
+	// listener on the same address and port, so it applies to all of them. If more
+	// than one of those listeners configures it, the first one wins.
+	//
 	// +optional
 	TCPKeepalive *TCPKeepalive `json:"tcpKeepalive,omitempty"`
 	// EnableProxyProtocol interprets the ProxyProtocol header and adds the

api/v1alpha1/connection_types.go

@@ -22,6 +22,10 @@ type ClientConnection struct {
 	// Note that when the suffix is not provided, the value is interpreted as bytes.
 	// Default: 32768 bytes.
 	//
+	// This setting belongs to the listener socket, which is shared by every Gateway
+	// listener on the same address and port, so it applies to all of them. If more
+	// than one of those listeners configures it, the first one wins.
+	//
 	// +kubebuilder:validation:XIntOrString
 	// +kubebuilder:validation:Pattern="^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$"
 	// +optional
@@ -42,6 +46,10 @@ type ClientConnection struct {
 	// this threshold will be accepted in later event loop iterations.
 	// Defaults to 1 and can be disabled by setting to 0 for allowing unlimited accepted connections.
 	//
+	// This setting belongs to the listener socket, which is shared by every Gateway
+	// listener on the same address and port, so it applies to all of them. If more
+	// than one of those listeners configures it, the first one wins.
+	//
 	// +optional
 	// +kubebuilder:default=1
 	MaxAcceptPerSocketEvent *uint32 `json:"maxAcceptPerSocketEvent,omitempty"`

api/v1alpha1/timeout_types.go

@@ -94,6 +94,10 @@ type TCPClientTimeout struct {
 	// If exceeded, the connection is dropped.
 	// Default: 15 seconds.
 	//
+	// This setting belongs to the listener socket, which is shared by every Gateway
+	// listener on the same address and port, so it applies to all of them. If more
+	// than one of those listeners configures it, the first one wins.
+	//
 	// +optional
 	ConnectionInspectionTimeout *gwapiv1.Duration `json:"connectionInspectionTimeout,omitempty"`
 }

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml

@@ -163,6 +163,10 @@ spec:
                       For example, 20Mi, 1Gi, 256Ki etc.
                       Note that when the suffix is not provided, the value is interpreted as bytes.
                       Default: 32768 bytes.
+
+                      This setting belongs to the listener socket, which is shared by every Gateway
+                      listener on the same address and port, so it applies to all of them. If more
+                      than one of those listeners configures it, the first one wins.
                     x-kubernetes-int-or-string: true
                   connectionLimit:
                     description: ConnectionLimit defines limits related to connections
@@ -211,6 +215,10 @@ spec:
                       per socket event. If there are more than MaxAcceptPerSocketEvent connections pending accept, connections over
                       this threshold will be accepted in later event loop iterations.
                       Defaults to 1 and can be disabled by setting to 0 for allowing unlimited accepted connections.
+
+                      This setting belongs to the listener socket, which is shared by every Gateway
+                      listener on the same address and port, so it applies to all of them. If more
+                      than one of those listeners configures it, the first one wins.
                     format: int32
                     type: integer
                   socketBufferLimit:
@@ -1319,6 +1327,10 @@ spec:
                   TcpKeepalive settings associated with the downstream client connection.
                   If defined, sets SO_KEEPALIVE on the listener socket to enable TCP Keepalives.
                   Disabled by default.
+
+                  This setting belongs to the listener socket, which is shared by every Gateway
+                  listener on the same address and port, so it applies to all of them. If more
+                  than one of those listeners configures it, the first one wins.
                 properties:
                   idleTime:
                     description: |-
@@ -1384,6 +1396,10 @@ spec:
                           (TLS / SNI and protocol detection, or HTTP protocol parsing) of an incoming connection on the listener socket.
                           If exceeded, the connection is dropped.
                           Default: 15 seconds.
+
+                          This setting belongs to the listener socket, which is shared by every Gateway
+                          listener on the same address and port, so it applies to all of them. If more
+                          than one of those listeners configures it, the first one wins.
                         pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$
                         type: string
                       idleTimeout:

charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml

@@ -162,6 +162,10 @@ spec:
                       For example, 20Mi, 1Gi, 256Ki etc.
                       Note that when the suffix is not provided, the value is interpreted as bytes.
                       Default: 32768 bytes.
+
+                      This setting belongs to the listener socket, which is shared by every Gateway
+                      listener on the same address and port, so it applies to all of them. If more
+                      than one of those listeners configures it, the first one wins.
                     x-kubernetes-int-or-string: true
                   connectionLimit:
                     description: ConnectionLimit defines limits related to connections
@@ -210,6 +214,10 @@ spec:
                       per socket event. If there are more than MaxAcceptPerSocketEvent connections pending accept, connections over
                       this threshold will be accepted in later event loop iterations.
                       Defaults to 1 and can be disabled by setting to 0 for allowing unlimited accepted connections.
+
+                      This setting belongs to the listener socket, which is shared by every Gateway
+                      listener on the same address and port, so it applies to all of them. If more
+                      than one of those listeners configures it, the first one wins.
                     format: int32
                     type: integer
                   socketBufferLimit:
@@ -1318,6 +1326,10 @@ spec:
                   TcpKeepalive settings associated with the downstream client connection.
                   If defined, sets SO_KEEPALIVE on the listener socket to enable TCP Keepalives.
                   Disabled by default.
+
+                  This setting belongs to the listener socket, which is shared by every Gateway
+                  listener on the same address and port, so it applies to all of them. If more
+                  than one of those listeners configures it, the first one wins.
                 properties:
                   idleTime:
                     description: |-
@@ -1383,6 +1395,10 @@ spec:
                           (TLS / SNI and protocol detection, or HTTP protocol parsing) of an incoming connection on the listener socket.
                           If exceeded, the connection is dropped.
                           Default: 15 seconds.
+
+                          This setting belongs to the listener socket, which is shared by every Gateway
+                          listener on the same address and port, so it applies to all of them. If more
+                          than one of those listeners configures it, the first one wins.
                         pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$
                         type: string
                       idleTimeout:

internal/xds/translator/listener.go

@@ -36,6 +36,7 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -209,21 +210,83 @@ func originalIPDetectionExtensions(clientIPDetection *ir.ClientIPDetectionSettin
 	return extensionConfig
 }
 
+// socketSettings holds the settings that Envoy applies to the listener socket itself
+// rather than to an individual filter chain.
+//
+// Gateway listeners that share an address and port collapse into a single xDS
+// listener, so these settings belong to the whole socket and have to be resolved
+// across all the IR listeners that end up on it.
+type socketSettings struct {
+	keepalive                   *ir.TCPKeepalive
+	bufferLimitBytes            *uint32
+	maxAcceptPerSocketEvent     *uint32
+	connectionInspectionTimeout *metav1.Duration
+}
+
+// buildSocketSettings resolves the socket settings for every address and port
+// combination in the IR.
+//
+// The settings are resolved per field: a listener that leaves a field unset does not
+// shadow another listener on the same socket that sets it, which would otherwise
+// silently replace a configured value with the hardcoded default. When more than one
+// listener sets the same field, the first one still wins, so the resulting
+// configuration only changes for the sockets that were getting a default they never
+// asked for.
+func buildSocketSettings(xdsIR *ir.Xds) map[listenerKey]*socketSettings {
+	resolved := make(map[listenerKey]*socketSettings)
+
+	collect := func(details *ir.CoreListenerDetails, keepalive *ir.TCPKeepalive, connection *ir.ClientConnection, timeout *ir.ClientTimeout) {
+		key := listenerKey{Address: details.Address, Port: details.Port}
+		settings, ok := resolved[key]
+		if !ok {
+			settings = &socketSettings{}
+			resolved[key] = settings
+		}
+
+		if settings.keepalive == nil {
+			settings.keepalive = keepalive
+		}
+		if connection != nil {
+			if settings.bufferLimitBytes == nil {
+				settings.bufferLimitBytes = connection.BufferLimitBytes
+			}
+			if settings.maxAcceptPerSocketEvent == nil {
+				settings.maxAcceptPerSocketEvent = connection.MaxAcceptPerSocketEvent
+			}
+		}
+		if timeout != nil && timeout.TCP != nil && settings.connectionInspectionTimeout == nil {
+			settings.connectionInspectionTimeout = timeout.TCP.ConnectionInspectionTimeout
+		}
+	}
+
+	// The HTTP listeners are translated before the TCP ones, so they are visited in
+	// the same order here to keep the winning listener unchanged.
+	for _, httpListener := range xdsIR.HTTP {
+		collect(&httpListener.CoreListenerDetails, httpListener.TCPKeepalive, httpListener.Connection, httpListener.Timeout)
+	}
+	for _, tcpListener := range xdsIR.TCP {
+		collect(&tcpListener.CoreListenerDetails, tcpListener.TCPKeepalive, tcpListener.Connection, tcpListener.Timeout)
+	}
+
+	return resolved
+}
+
 // buildXdsTCPListener creates a xds Listener resource
 func (t *Translator) buildXdsTCPListener(
 	listenerDetails *ir.CoreListenerDetails,
-	keepalive *ir.TCPKeepalive,
-	connection *ir.ClientConnection,
-	timeout *ir.ClientTimeout,
+	settings *socketSettings,
 	accesslog *ir.AccessLog,
 ) (*listenerv3.Listener, error) {
-	socketOptions := buildTCPSocketOptions(keepalive)
+	if settings == nil {
+		settings = &socketSettings{}
+	}
+	socketOptions := buildTCPSocketOptions(settings.keepalive)
 	al, err := buildXdsAccessLog(accesslog, ir.ProxyAccessLogTypeListener)
 	if err != nil {
 		return nil, err
 	}
-	bufferLimitBytes := buildPerConnectionBufferLimitBytes(connection)
-	maxAcceptPerSocketEvent := buildMaxAcceptPerSocketEvent(connection)
+	bufferLimitBytes := buildPerConnectionBufferLimitBytes(settings.bufferLimitBytes)
+	maxAcceptPerSocketEvent := buildMaxAcceptPerSocketEvent(settings.maxAcceptPerSocketEvent)
 	listener := &listenerv3.Listener{
 		Name: xdsListenerName(
 			listenerDetails.Name, listenerDetails.ExternalPort,
@@ -250,8 +313,8 @@ func (t *Translator) buildXdsTCPListener(
 		socketAddress.Ipv4Compat = true
 	}
 
-	if timeout != nil && timeout.TCP != nil && timeout.TCP.ConnectionInspectionTimeout != nil {
-		listener.ListenerFiltersTimeout = durationpb.New(timeout.TCP.ConnectionInspectionTimeout.Duration)
+	if settings.connectionInspectionTimeout != nil {
+		listener.ListenerFiltersTimeout = durationpb.New(settings.connectionInspectionTimeout.Duration)
 	}
 
 	return listener, nil
@@ -274,21 +337,21 @@ func xdsListenerName(name string, externalPort uint32, protocol corev3.SocketAdd
 	return name
 }
 
-func buildPerConnectionBufferLimitBytes(connection *ir.ClientConnection) *wrapperspb.UInt32Value {
-	if connection != nil && connection.BufferLimitBytes != nil {
-		return wrapperspb.UInt32(*connection.BufferLimitBytes)
+func buildPerConnectionBufferLimitBytes(bufferLimitBytes *uint32) *wrapperspb.UInt32Value {
+	if bufferLimitBytes != nil {
+		return wrapperspb.UInt32(*bufferLimitBytes)
 	}
 	return wrapperspb.UInt32(tcpListenerPerConnectionBufferLimitBytes)
 }
 
-func buildMaxAcceptPerSocketEvent(connection *ir.ClientConnection) *wrapperspb.UInt32Value {
-	if connection == nil || connection.MaxAcceptPerSocketEvent == nil {
+func buildMaxAcceptPerSocketEvent(maxAcceptPerSocketEvent *uint32) *wrapperspb.UInt32Value {
+	if maxAcceptPerSocketEvent == nil {
 		return wrapperspb.UInt32(defaultMaxAcceptConnectionsPerSocketEvent)
 	}
-	if *connection.MaxAcceptPerSocketEvent == 0 {
+	if *maxAcceptPerSocketEvent == 0 {
 		return nil
 	}
-	return wrapperspb.UInt32(*connection.MaxAcceptPerSocketEvent)
+	return wrapperspb.UInt32(*maxAcceptPerSocketEvent)
 }
 
 // buildXdsQuicListener creates a xds Listener resource for quic

internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-socket-settings.yaml

@@ -0,0 +1,70 @@
+# The listeners below all share the same address and port, so they collapse into a
+# single xDS listener. The first one leaves every socket setting unset and must not
+# make the shared socket fall back to the defaults: the buffer limit and the keepalive
+# come from the second HTTP listener, and the max accept per socket event and the
+# connection inspection timeout come from the TCP listener.
+http:
+- name: "first-listener"
+  address: "::"
+  port: 10080
+  hostnames:
+  - "foo.com"
+  path:
+    mergeSlashes: true
+    escapedSlashesAction: UnescapeAndRedirect
+  routes:
+  - name: "first-route"
+    hostname: "*"
+    destination:
+      name: "first-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
+        name: "first-route-dest/backend/0"
+- name: "second-listener"
+  address: "::"
+  port: 10080
+  hostnames:
+  - "foo.net"
+  path:
+    mergeSlashes: true
+    escapedSlashesAction: UnescapeAndRedirect
+  tcpKeepalive:
+    probes: 7
+    interval: 200
+    idleTime: 50
+  connection:
+    bufferLimit: 1048576
+  routes:
+  - name: "second-route"
+    hostname: "*"
+    destination:
+      name: "second-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
+        name: "second-route-dest/backend/0"
+tcp:
+- name: "third-listener"
+  address: "::"
+  port: 10080
+  connection:
+    maxAcceptPerSocketEvent: 64
+  timeout:
+    tcp:
+      connectionInspectionTimeout: 5s
+  routes:
+  - name: "third-route"
+    tls:
+      inspector:
+        snis:
+        - bar.com
+    destination:
+      name: "tcp-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
+        name: "tcp-route-dest/backend/0"