Listener-scoped ClientTrafficPolicy settings are silently dropped when multiple listeners share an address:port

[HusseinKabbout]

Follow up issue from: #9315 (comment)

Description:

When multiple Gateway listeners share an address:port, they collapse into a
single xDS Listener with one filter chain per section. ClientTrafficPolicy
fields that map to Listener-scoped Envoy config have only one value available
for the whole socket:

• ConnectionInspectionTimeout -> listener_filters_timeout
• Connection.BufferLimit -> per_connection_buffer_limit_bytes
• TCPKeepalive -> tcp_keepalive
• EnableProxyProtocol -> proxy protocol listener filter

Since CTP is tracked one-per-section, both policies are Accepted, the
first-processed value wins for the entire socket, and the other is dropped with
no status condition, warning, or log line.

The existing port-overlap check skips TLS listeners, assuming each gets its own
filter chain, which is true for filter-chain-scoped settings, but not listener-scoped ones.
See the existing TODO in internal/xds/translator/translator.go.

Expected: a cross-listener consistency check that rejects conflicting
listener-scoped values across sections sharing an address:port, surfacing the
conflict via status rather than resolving it silently.

Repro steps:

1. Gateway with two HTTPS listeners on :443 (different hostnames, own certs).
2. One ClientTrafficPolicy per section with differing
   Connection.BufferLimit values (e.g. 29Mi and 300Mi).
3. Both policies report Accepted=True.
4. envoy config_dump shows a single per_connection_buffer_limit_bytes on :443 --> one
   value was dropped.

[kadircanyildirm-crypto]

I'd like to pick this up. I spent a while tracing what actually happens on main today, and it turns out to be a bit worse than the description — sharing it here since it might change how you'd want it fixed.

Everything that lands on the socket is written in exactly one place: buildXdsTCPListener. It only runs for the first IR listener on a given address+port, and it's the sole writer of socket_options (from tcpKeepalive), per_connection_buffer_limit_bytes (from connection.bufferLimit) and max_connections_to_accept_per_socket_event (from connection.maxAcceptPerSocketEvent). Nothing patches them afterwards, so whatever the first listener happened to have is simply what the socket gets.

Three things fall out of that.

A listener with no policy on it still wins. buildPerConnectionBufferLimitBytes and buildMaxAcceptPerSocketEvent fall back to defaults (32768 and 1) when there's no Connection. So this isn't only one CTP beating another — a plain listener with no CTP at all can quietly overwrite a configured one with a default value.

connection.maxAcceptPerSocketEvent belongs on the list, connection.limit doesn't. The limit is a network filter, so it stays per filter chain and is genuinely unaffected. connectionInspectionTimeout isn't on main yet, so I'd fold it in once #9315 lands.

Proxy protocol breaks the other way round. patchProxyProtocolFilter bails out early if the filter is already present, so it's "whoever enables it wins" and nobody else on that socket can turn it off — the TODO from #3337, still standing since 2024. Same root cause, opposite symptom, so it probably needs its own wording in the status message.

And one case I don't think is on the radar yet: HTTPS and TLS passthrough sharing a port. getProtocolForListener maps both to https/tls on purpose so they can coexist, but HTTPS goes into xdsIR.HTTP while passthrough goes into xdsIR.TCP, and HTTP is translated first — so the TLS listener's entire CTP is dropped, not just one field. The current check can't see it either: listenersWithSameHTTPPort walks only xdsIR.HTTP and compares Port alone, while the real collapse key is address + port + protocol (findXdsListenerByHostPort).

What I'd write, then, is a pass at the end of ProcessClientTrafficPolicies that groups the IR listeners — HTTP and TCP together — by that same key and compares those four fields. Pointing the status at the right policy looks straightforward, since sections are already tracked in claimedSections.

Two things I'd rather hear from you than guess at:

1. Reject, or just warn? Accepted=False with PolicyReasonConflicted would be consistent with how a duplicate section claim is handled today. But it also means setups that look healthy right now start failing on upgrade, so a Warning condition — the way UnsupportedHTTP3ClientValidation does it — might be the kinder call.
2. Who should win? Today it's whatever order the IR was built in: Gateway creationTimestamp, then namespace/name, then listener order within the spec. Stable, but not something a user would ever guess. Happy to leave it as is and only report the conflict, or to write down an explicit rule.

@zhaohuabing

[zhaohuabing]

Hi @kadircanyildirm-crypto

Thanks for digging in.

Reject, or just warn?

I'd prefer warn. Reject is a breaking change and would affect existing users.

Who should win?

I'd split this into two changes.

A listener with no CTP overwriting a configured one with a hardcoded default: this should be fixed unconditionally, no status involved.

Two listeners each explicitly setting different values: I'd keep today's ordering — changing the winner is its own silent behavior change on upgrade — but say so explicitly in the status message.

cc @envoyproxy/gateway-maintainers

[kadircanyildirm-crypto]

Opened #9673 for the first half — the unconditional fix, no status involved.

It resolves the keepalive, buffer limit and max accept per socket event across every HTTP and TCP listener on the same address and port, per field, before the listener is built. A listener that leaves one unset no longer replaces a configured value with the default; when two listeners both set the same field the first still wins, so nothing changes for setups that were already explicit. All 273 existing xds-ir golden files are untouched.

The status message for the genuine conflict case will follow in a second PR.

@zhaohuabing

[HusseinKabbout]

There is the same issue regarding filter chains and not just listener specific configuration, see #9315 (comment)

[kadircanyildirm-crypto]

Right, and it is one layer down: non-TLS listeners on the same port share the default filter chain, so there is a single HCM and the first listener's timeouts stick. Worth tracking here so it does not get lost.

[guydc]

IMHO, even for the non-conflicted case, we may want to consider leaving things as they are. A policy applied to a specific TLS listener essentially applying to the entire xds listener is also incorrect, so in a sense, I prefer to keep the current incorrect behavior until we figure out something that is more explicit.

The main concern for me is the fact that we don't really have a way for users to cleanly express intent here. Translation/Creation order determines things.

Maybe we can think of a more explicit way to control these xds listener level settings.

• New Policy - rejected in the past in this context
• Using Existing resource hierarchy:
  • For GWs with conflicting listener scenario: maybe we can consider only translating these settings for GW-level attached policies and warning when they are specified for section targets? This will change override semantics for listener-scoped policies and may force users to break up gateways to achieve granular configurability.
  • For Merge Gateways - no great solution besides moving away to listenerSets or allowing attachment + translation for GWC.

[kadircanyildirm-crypto]

@zhaohuabing @guydc I need a decision before going further.

#9673 does the unconditional fix: a listener with no policy stops replacing a configured value with the hardcoded default. That is the case @zhaohuabing asked for, and it is also the exact case @guydc suggests leaving alone with only a warning.

Which way do you want it? I am happy to turn the PR into warning-only, I just do not want to build on the wrong side of that.

[guydc]

Hi @kadircanyildirm-crypto . I remove my previous objection. I really don't see a better option at this time. Let's also do our best to highlight these constraints in doc strings, and create a dedicated section in the CTP user guide calling out these special fields.
Please proceed with @zhaohuabing's suggestion.