diff --git a/api/v1alpha1/clienttrafficpolicy_types.go b/api/v1alpha1/clienttrafficpolicy_types.go index cae9efbbf0a..94b56dc7c84 100644 --- a/api/v1alpha1/clienttrafficpolicy_types.go +++ b/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 diff --git a/api/v1alpha1/connection_types.go b/api/v1alpha1/connection_types.go index 8f96f6df4bd..d83c107ef83 100644 --- a/api/v1alpha1/connection_types.go +++ b/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"` diff --git a/api/v1alpha1/timeout_types.go b/api/v1alpha1/timeout_types.go index f1c664e38d3..49022ee9dad 100644 --- a/api/v1alpha1/timeout_types.go +++ b/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"` } diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 0d70b99aa41..3e8511cb2c6 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/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: diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 79dda3f2794..e9b987f711f 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/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: diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index 0bbf0eb7ba6..79b07248f98 100644 --- a/internal/xds/translator/listener.go +++ b/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 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 00000000000..7fdd34aff22 --- /dev/null +++ b/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" 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 00000000000..1e713534160 --- /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 00000000000..2a15ac80cbe --- /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 00000000000..d3533a1d4c7 --- /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: {} + 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 + listenerFiltersTimeout: 5s + 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 00000000000..1dcebeacae1 --- /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 1c763409923..8e2f056ac03 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -199,12 +199,18 @@ func (t *Translator) Translate(ctx context.Context, xdsIR *ir.Xds) (*types.Resou 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) + phases.Start("XdsTranslator.processHTTPListenerXdsTranslation", attribute.Int("http-listeners.count", len(xdsIR.HTTP)), attribute.Int("ir-http-routes.count", irHTTPRoutes), ) if err := t.processHTTPListenerXdsTranslation( - tCtx, xdsIR.HTTP, xdsIR.AccessLog, xdsIR.Tracing, xdsIR.Metrics, xdsIR.HealthCheckLog); err != nil { + tCtx, xdsIR.HTTP, xdsIR.AccessLog, xdsIR.Tracing, xdsIR.Metrics, xdsIR.HealthCheckLog, + socketSettingsByListener); err != nil { errs = errors.Join(errs, err) } phases.End() @@ -212,7 +218,9 @@ func (t *Translator) Translate(ctx context.Context, xdsIR *ir.Xds) (*types.Resou // The TCP and UDP listeners get no phase span: they are cheap next to the HTTP // listeners on a large cluster. Their input sizes are on the enclosing span, so a // cluster where that assumption does not hold is still visible. - if err := t.processTCPListenerXdsTranslation(tCtx, xdsIR.TCP, xdsIR.AccessLog, xdsIR.Metrics, xdsIR.HealthCheckLog); err != nil { + if err := t.processTCPListenerXdsTranslation( + tCtx, xdsIR.TCP, xdsIR.AccessLog, xdsIR.Metrics, xdsIR.HealthCheckLog, + socketSettingsByListener); err != nil { errs = errors.Join(errs, err) } @@ -433,6 +441,7 @@ func (t *Translator) processHTTPListenerXdsTranslation( tracing *ir.Tracing, metrics *ir.Metrics, healthCheckLog *ir.ProxyHealthCheckLog, + 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. @@ -497,9 +506,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, - httpListener.Timeout, + socketSettingsByListener[listenerKey{Address: httpListener.Address, Port: httpListener.Port}], accessLog, ); err != nil { errs = errors.Join(errs, err) @@ -944,6 +951,7 @@ func (t *Translator) processTCPListenerXdsTranslation( accesslog *ir.AccessLog, metrics *ir.Metrics, healthCheckLog *ir.ProxyHealthCheckLog, + 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. @@ -957,9 +965,7 @@ func (t *Translator) processTCPListenerXdsTranslation( if xdsListener == nil { if xdsListener, err = t.buildXdsTCPListener( &tcpListener.CoreListenerDetails, - tcpListener.TCPKeepalive, - tcpListener.Connection, - tcpListener.Timeout, + 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 00000000000..3d1c6124093 --- /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, max accept per socket event and connection inspection timeout 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`, `max_connections_to_accept_per_socket_event` and `listener_filters_timeout` from the defaults to what was actually configured; the field names and layout are unchanged, so EnvoyPatchPolicies and extension servers targeting them keep matching. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 2d2c7343a86..63c59fb634c 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -905,8 +905,8 @@ _Appears in:_ | Field | Type | Required | Default | Description | | --- | --- | --- | --- | --- | | `connectionLimit` | _[ConnectionLimit](#connectionlimit)_ | false | | ConnectionLimit defines limits related to connections | -| `bufferLimit` | _[Quantity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#quantity-resource-api)_ | false | | BufferLimit provides configuration for the maximum buffer size in bytes for each incoming connection.
BufferLimit applies to connection streaming (maybe non-streaming) channel between processes, it's in user space.
For example, 20Mi, 1Gi, 256Ki etc.
Note that when the suffix is not provided, the value is interpreted as bytes.
Default: 32768 bytes. | -| `maxAcceptPerSocketEvent` | _integer_ | false | 1 | MaxAcceptPerSocketEvent provides configuration for the maximum number of connections to accept from the kernel
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. | +| `bufferLimit` | _[Quantity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#quantity-resource-api)_ | false | | BufferLimit provides configuration for the maximum buffer size in bytes for each incoming connection.
BufferLimit applies to connection streaming (maybe non-streaming) channel between processes, it's in user space.
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. | +| `maxAcceptPerSocketEvent` | _integer_ | false | 1 | MaxAcceptPerSocketEvent provides configuration for the maximum number of connections to accept from the kernel
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. | #### ClientIPDetectionSettings @@ -1015,7 +1015,7 @@ _Appears in:_ | `targetRef` | _[LocalPolicyTargetReferenceWithSectionName](#localpolicytargetreferencewithsectionname)_ | true | | TargetRef is the name of the resource this policy is being attached to.
This policy and the TargetRef MUST be in the same namespace for this
Policy to have effect
Deprecated: use targetRefs/targetSelectors instead | | `targetRefs` | _LocalPolicyTargetReferenceWithSectionName array_ | true | | TargetRefs are the names of the Gateway resources this policy
is being attached to. | | `targetSelectors` | _[TargetSelector](#targetselector) array_ | true | | TargetSelectors allow targeting resources for this policy based on labels | -| `tcpKeepalive` | _[TCPKeepalive](#tcpkeepalive)_ | false | | TcpKeepalive settings associated with the downstream client connection.
If defined, sets SO_KEEPALIVE on the listener socket to enable TCP Keepalives.
Disabled by default. | +| `tcpKeepalive` | _[TCPKeepalive](#tcpkeepalive)_ | false | | 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. | | `enableProxyProtocol` | _boolean_ | false | | EnableProxyProtocol interprets the ProxyProtocol header and adds the
Client Address into the X-Forwarded-For header.
Note Proxy Protocol must be present when this field is set, else the connection
is closed.
Deprecated: Use ProxyProtocol instead. | | `proxyProtocol` | _[ProxyProtocolSettings](#proxyprotocolsettings)_ | false | | ProxyProtocol configures the Proxy Protocol settings. When configured,
the Proxy Protocol header will be interpreted and the Client Address
will be added into the X-Forwarded-For header.
If both EnableProxyProtocol and ProxyProtocol are set, ProxyProtocol takes precedence. | | `clientIPDetection` | _[ClientIPDetectionSettings](#clientipdetectionsettings)_ | false | | ClientIPDetectionSettings provides configuration for determining the original client IP address for requests. | @@ -6520,7 +6520,7 @@ _Appears in:_ | --- | --- | --- | --- | --- | | `idleTimeout` | _[Duration](https://gateway-api.sigs.k8s.io/reference/api-spec/1.5/spec/#duration)_ | false | | IdleTimeout for a TCP connection. Idle time is defined as a period in which there are no
bytes sent or received on either the upstream or downstream connection.
Default: 1 hour. | | `tlsHandshakeTimeout` | _[Duration](https://gateway-api.sigs.k8s.io/reference/api-spec/1.5/spec/#duration)_ | false | | TLSHandshakeTimeout for a TCP connection. The maximum time to complete transport level connection negotiation
(e.g. the TLS handshake) after a connection is accepted.
If this expires before the transport reports connection establishment, the connection is summarily closed. | -| `connectionInspectionTimeout` | _[Duration](https://gateway-api.sigs.k8s.io/reference/api-spec/1.5/spec/#duration)_ | false | | ConnectionInspectionTimeout is the maximum time to wait for initial inspection
(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. | +| `connectionInspectionTimeout` | _[Duration](https://gateway-api.sigs.k8s.io/reference/api-spec/1.5/spec/#duration)_ | false | | ConnectionInspectionTimeout is the maximum time to wait for initial inspection
(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. | #### TCPKeepalive diff --git a/site/content/en/latest/tasks/traffic/client-traffic-policy.md b/site/content/en/latest/tasks/traffic/client-traffic-policy.md index 2d57fc0850a..e85fa35979a 100644 --- a/site/content/en/latest/tasks/traffic/client-traffic-policy.md +++ b/site/content/en/latest/tasks/traffic/client-traffic-policy.md @@ -13,6 +13,27 @@ the behavior for how the Envoy Proxy server behaves with downstream clients. This API was added as a new policy attachment resource that can be applied to Gateway resources and it is meant to hold settings for configuring behavior of the connection between the downstream client and Envoy Proxy listener. It is the counterpart to the [BackendTrafficPolicy][] API resource. +## Settings shared by listeners on the same port + +Gateway listeners that share an address and port are translated into a single Envoy +listener with one socket. Four ClientTrafficPolicy fields configure that socket +rather than an individual Gateway listener: + +- `tcpKeepalive` +- `connection.bufferLimit` +- `connection.maxAcceptPerSocketEvent` +- `timeout.tcp.connectionInspectionTimeout` + +Setting one of them on a single listener therefore applies it to every listener on +that address and port, including listeners that no policy targets. When two +listeners on the same address and port set the same field to different values, the +first one wins. That order comes from translation and is not something to rely on, +so keep these fields consistent across the listeners on a port — or set them once +with a policy that targets the whole Gateway. + +Listeners that leave a field unset do not affect it, so one listener configuring a +buffer limit no longer has a second listener reset it to the default. + ## Quickstart ### Prerequisites diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 197aec2547f..e9a1a605390 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -28686,6 +28686,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 @@ -28734,6 +28738,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: @@ -29842,6 +29850,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: |- @@ -29907,6 +29919,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: diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 32f54c37669..2c6f2a578e8 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -4624,6 +4624,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 @@ -4672,6 +4676,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: @@ -5780,6 +5788,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: |- @@ -5845,6 +5857,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: diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index db1da4ef9c1..170bed2a65d 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -4624,6 +4624,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 @@ -4672,6 +4676,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: @@ -5780,6 +5788,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: |- @@ -5845,6 +5857,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: