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

internal/xds/translator/listener.go

@@ -209,20 +209,80 @@ 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
+}
+
+// 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) {
+		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 {
+			return
+		}
+		if settings.bufferLimitBytes == nil {
+			settings.bufferLimitBytes = connection.BufferLimitBytes
+		}
+		if settings.maxAcceptPerSocketEvent == nil {
+			settings.maxAcceptPerSocketEvent = connection.MaxAcceptPerSocketEvent
+		}
+	}
+
+	// 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)
+	}
+	for _, tcpListener := range xdsIR.TCP {
+		collect(&tcpListener.CoreListenerDetails, tcpListener.TCPKeepalive, tcpListener.Connection)
+	}
+
+	return resolved
+}
+
 // buildXdsTCPListener creates a xds Listener resource
 func (t *Translator) buildXdsTCPListener(
 	listenerDetails *ir.CoreListenerDetails,
-	keepalive *ir.TCPKeepalive,
-	connection *ir.ClientConnection,
+	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,
@@ -269,21 +329,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,67 @@
+# 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 comes 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
+  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"

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

@@ -0,0 +1,69 @@
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_PREFERRED
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: first-route-dest
+  ignoreHealthOnHostRemoval: true
+  loadBalancingPolicy:
+    policies:
+    - typedExtensionConfig:
+        name: envoy.load_balancing_policies.least_request
+        typedConfig:
+          '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest
+          localityLbConfig:
+            localityWeightedLbConfig: {}
+  name: first-route-dest
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_PREFERRED
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: second-route-dest
+  ignoreHealthOnHostRemoval: true
+  loadBalancingPolicy:
+    policies:
+    - typedExtensionConfig:
+        name: envoy.load_balancing_policies.least_request
+        typedConfig:
+          '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest
+          localityLbConfig:
+            localityWeightedLbConfig: {}
+  name: second-route-dest
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_PREFERRED
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: tcp-route-dest
+  ignoreHealthOnHostRemoval: true
+  loadBalancingPolicy:
+    policies:
+    - typedExtensionConfig:
+        name: envoy.load_balancing_policies.least_request
+        typedConfig:
+          '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest
+          localityLbConfig:
+            localityWeightedLbConfig: {}
+  name: tcp-route-dest
+  perConnectionBufferLimitBytes: 32768
+  type: EDS

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

@@ -0,0 +1,36 @@
+- clusterName: first-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.2.3.4
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: first-route-dest/backend/0
+- clusterName: second-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.2.3.4
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: second-route-dest/backend/0
+- clusterName: tcp-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.2.3.4
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: tcp-route-dest/backend/0

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

@@ -0,0 +1,68 @@
+- address:
+    socketAddress:
+      address: '::'
+      portValue: 10080
+  defaultFilterChain:
+    filters:
+    - name: envoy.filters.network.http_connection_manager
+      typedConfig:
+        '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+        commonHttpProtocolOptions:
+          headersWithUnderscoresAction: REJECT_REQUEST
+        http2ProtocolOptions:
+          initialConnectionWindowSize: 1048576
+          initialStreamWindowSize: 65536
+          maxConcurrentStreams: 100
+        httpFilters:
+        - name: envoy.filters.http.router
+          typedConfig:
+            '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+            suppressEnvoyHeaders: true
+        mergeSlashes: true
+        normalizePath: true
+        pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT
+        rds:
+          configSource:
+            ads: {}
+            initialFetchTimeout: 0s
+            resourceApiVersion: V3
+          routeConfigName: first-listener
+        serverHeaderTransformation: PASS_THROUGH
+        statPrefix: http-10080
+        useRemoteAddress: true
+    name: first-listener
+  filterChains:
+  - filterChainMatch:
+      serverNames:
+      - bar.com
+    filters:
+    - name: envoy.filters.network.tcp_proxy
+      typedConfig:
+        '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
+        cluster: tcp-route-dest
+        statPrefix: tls-passthrough-10080
+    name: third-route
+  listenerFilters:
+  - name: envoy.filters.listener.tls_inspector
+    typedConfig:
+      '@type': type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
+  maxConnectionsToAcceptPerSocketEvent: 64
+  name: first-listener
+  perConnectionBufferLimitBytes: 1048576
+  socketOptions:
+  - description: socket option to enable tcp keep alive
+    intValue: "1"
+    level: "1"
+    name: "9"
+  - description: socket option for keep alive probes
+    intValue: "7"
+    level: "6"
+    name: "6"
+  - description: socket option for keep alive idle time
+    intValue: "50"
+    level: "6"
+    name: "4"
+  - description: socket option for keep alive interval
+    intValue: "200"
+    level: "6"
+    name: "5"

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

@@ -0,0 +1,25 @@
+- ignorePortInHostMatching: true
+  name: first-listener
+  virtualHosts:
+  - domains:
+    - '*'
+    name: first-listener/*
+    routes:
+    - match:
+        prefix: /
+      name: first-route
+      route:
+        cluster: first-route-dest
+        upgradeConfigs:
+        - upgradeType: websocket
+  - domains:
+    - '*'
+    name: second-listener/*
+    routes:
+    - match:
+        prefix: /
+      name: second-route
+      route:
+        cluster: second-route-dest
+        upgradeConfigs:
+        - upgradeType: websocket