diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index fbe7bda278..ff8df1be0a 100644 --- a/internal/xds/translator/listener.go +++ b/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 diff --git a/internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-socket-settings.yaml b/internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-socket-settings.yaml new file mode 100644 index 0000000000..3e6d5f1431 --- /dev/null +++ b/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" diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.clusters.yaml new file mode 100644 index 0000000000..1e71353416 --- /dev/null +++ b/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 diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.endpoints.yaml new file mode 100644 index 0000000000..2a15ac80cb --- /dev/null +++ b/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 diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.listeners.yaml new file mode 100644 index 0000000000..f8d27a7e78 --- /dev/null +++ b/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" diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-socket-settings.routes.yaml new file mode 100644 index 0000000000..1dcebeacae --- /dev/null +++ b/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 diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index 87f7ba4f9b..7f3091ee75 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -121,12 +121,18 @@ func (t *Translator) Translate(xdsIR *ir.Xds) (*types.ResourceVersionTable, erro errs = errors.Join(errs, err) } + // The HTTP and TCP listeners that share an address and port are translated into a + // single xDS listener, so the settings that apply to the socket are resolved + // across all of them up front. + socketSettingsByListener := buildSocketSettings(xdsIR) + if err := t.processHTTPListenerXdsTranslation( - tCtx, xdsIR.HTTP, xdsIR.AccessLog, xdsIR.Tracing, xdsIR.Metrics); err != nil { + tCtx, xdsIR.HTTP, xdsIR.AccessLog, xdsIR.Tracing, xdsIR.Metrics, socketSettingsByListener); err != nil { errs = errors.Join(errs, err) } - if err := t.processTCPListenerXdsTranslation(tCtx, xdsIR.TCP, xdsIR.AccessLog, xdsIR.Metrics); err != nil { + if err := t.processTCPListenerXdsTranslation( + tCtx, xdsIR.TCP, xdsIR.AccessLog, xdsIR.Metrics, socketSettingsByListener); err != nil { errs = errors.Join(errs, err) } @@ -325,6 +331,7 @@ func (t *Translator) processHTTPListenerXdsTranslation( accessLog *ir.AccessLog, tracing *ir.Tracing, metrics *ir.Metrics, + socketSettingsByListener map[listenerKey]*socketSettings, ) error { // The XDS translation is done in a best-effort manner, so we collect all // errors and return them at the end. @@ -389,8 +396,7 @@ func (t *Translator) processHTTPListenerXdsTranslation( // Create a new TCP listener for HTTP1/HTTP2 traffic. if tcpXDSListener, err = t.buildXdsTCPListener( &httpListener.CoreListenerDetails, - httpListener.TCPKeepalive, - httpListener.Connection, + socketSettingsByListener[listenerKey{Address: httpListener.Address, Port: httpListener.Port}], accessLog, ); err != nil { errs = errors.Join(errs, err) @@ -831,6 +837,7 @@ func (t *Translator) processTCPListenerXdsTranslation( tcpListeners []*ir.TCPListener, accesslog *ir.AccessLog, metrics *ir.Metrics, + socketSettingsByListener map[listenerKey]*socketSettings, ) error { // The XDS translation is done in a best-effort manner, so we collect all // errors and return them at the end. @@ -844,8 +851,7 @@ func (t *Translator) processTCPListenerXdsTranslation( if xdsListener == nil { if xdsListener, err = t.buildXdsTCPListener( &tcpListener.CoreListenerDetails, - tcpListener.TCPKeepalive, - tcpListener.Connection, + socketSettingsByListener[listenerKey{Address: tcpListener.Address, Port: tcpListener.Port}], accesslog, ); err != nil { // skip this listener if failed to build xds listener diff --git a/release-notes/current/bug_fixes/9673-shared-socket-listener-settings.md b/release-notes/current/bug_fixes/9673-shared-socket-listener-settings.md new file mode 100644 index 0000000000..162352c3d9 --- /dev/null +++ b/release-notes/current/bug_fixes/9673-shared-socket-listener-settings.md @@ -0,0 +1 @@ +Fixed Gateway listeners that share an address and port silently losing their client connection settings. Those listeners collapse into a single xDS listener, and the TCP keepalive, connection buffer limit and max accept per socket event were taken from whichever listener happened to be translated first, so a listener without a ClientTrafficPolicy would replace the values configured on another listener on the same socket with the hardcoded defaults. These settings are now resolved across every listener on the socket, and a listener that leaves one unset no longer overrides a listener that sets it. On an affected socket this changes the emitted `socket_options`, `per_connection_buffer_limit_bytes` and `max_connections_to_accept_per_socket_event` from the defaults to what was actually configured; the field names and layout are unchanged, so EnvoyPatchPolicies and extension servers targeting them keep matching.