internal_upstream: provision mutable filter state placeholders, readable on pool failure

[MyUmmaGumma]

Commit Message

internal_upstream: provision mutable filter state placeholders, readable on pool failure

Additional Description

In the internal-listener tunneling topology — outer L7 listener → cluster with
transport_socket: envoy.transport_sockets.internal_upstream → internal listener → tcp_proxy
issuing CONNECT — the response captured by propagate_response_headers lands on the inner
connection's filter state. The outer L7 listener, which received the original request, cannot read
it. A non-2xx CONNECT (an authorization denial returning 403, say) therefore reaches the client as a
canonical 503.

This is the alternative @kyessenov suggested in
#45237: rather than adding
a reverse propagation direction, provision an empty, mutable filter state object at the boundary,
forward-share it by reference over the existing PassthroughState path, and let tcp_proxy fill it
in place. One shared_ptr, visible on both sides. No new sharing direction and no close-ordering
dependency.

Relationship to #45237

The two are alternatives for the same goal, not dependencies. Either can land alone.

	#45237 — reverse primitive	this PR — forward placeholder
mechanism	new SharedWithDownstreamConnectionOnClose direction; objects copied out at close	existing forward share; one object, mutated in place
close ordering	effective only when the inner side closes first	not applicable — the object exists before the CONNECT is sent
new interfaces	IoHandle::addOnPreCloseCallback, PassthroughState::captureReverse/mergeReverse, new enum variant	one repeated proto field
generality	inner side can send back anything, unilaterally	outer side must name the object up front
cost	on the tunneled path only, and only when there is something to carry	one empty object per boundary connection

The reverse primitive is the more general channel and remains the better long-term contribution. This
one is smaller, order-independent, and adds no new sharing semantics. Part 3 below is required by
both.

Why the object is provisioned at the boundary, and not seeded from a downstream filter

The same object can be seeded by an ordinary downstream HTTP filter on the outer listener, with no core
change at all. That variant works — it is one of the test cases below — and its smaller footprint makes
it the obvious first suggestion. It is not viable for the deployment this is written for because
the outer listener here is a sidecar's shared outbound listener. It carries every outbound request
from the local application, whether that request is tunnelled or not. A downstream filter on it runs for
all of them, and the object cannot be narrowed to the cases that need it:

• it has to exist before the upstream connection, so it cannot wait until a failure is known; and
• tunnelled destinations are resolved on demand, after routing, so there is no per-route hook to attach
  it to.

The payload only matters in the error case, and only on the tunnelled path. Seeding downstream therefore
means paying an allocation on every outbound request to serve a rare error on a subset of them.

Provisioning at the transport socket avoids that without giving up the idea: the internal_upstream
transport socket exists only on clusters that tunnel, so the object is created only on that path — and
once per connection, not once per request. That is the whole reason this PR provisions the object one
hop in rather than at the listener.

The topology and the cost model are set out in more detail in
this comment on #45237.

The change, in three parts

Part 1 — internal_upstream can be told to provision empty filter state objects. A new
provisioned_placeholder_factories field names registered FilterState::ObjectFactory instances.
Each name is resolved when the config loads, so a name that does not resolve is a config rejection
rather than a silent no-op at request time. For each one, an empty object is placed on the upstream
connection and forward-shared into the internal connection over the existing PassthroughState path.
It is one object, not a copy, so either side can see what the other writes.

Where the object lives, since @kyessenov asked about multiplexing on #45237: on the upstream
connection this transport socket creates. There is one of those per cluster, so the object is never
shared between different upstreams, and it is created per connection rather than per request.

Part 2 — tcp_proxy fills such an object instead of replacing it. When it captures a CONNECT
response and an object for that key already exists, it now writes the headers into that object rather
than storing a new one over the top. Anything else holding the original therefore sees the headers.
With no object already present, the behaviour is unchanged. An object factory is registered for the
existing envoy.tcp_proxy.propagate_response_headers key so that it can be named from config.

Part 3 — the router records upstream filter state when a connection fails, not only when it
succeeds. Today onPoolReady records it and onPoolFailure does not, because the failure callback
is given no StreamInfo. A rejected CONNECT is a connection failure, so %UPSTREAM_FILTER_STATE%
reads nothing on exactly the path where the response status matters.

The HTTP connection pool now passes the failing connection's filter state along with the failure, and
the router records it the same way it already does on success. The new callback forms default to the
existing ones, so no current implementation has to change, and the existing failure handling still
runs unaltered.

Why Part 3 is in this PR rather than its own

Part 3 is the failure-path counterpart of an existing success-path capture, and every approach that reads the outer connection's filter state needs it —
including #45237, which defers it as an intended follow-up. On that symmetry argument alone it would stand as a PR of its own.
It is included in this PR because because standing alone it is an API widening with no observable behaviour change and no
test that demonstrates value. Parts 1–2 are that use case: together they make the failure-path
capture observable end to end. If reviewers would rather see Part 3 separately, happy to split it —
Parts 1–2 are then blocked until it lands.

What is not in this PR

• Rewriting the client-visible status. This makes the CONNECT response readable on the outer
  side. Acting on it is a filter, and out of scope here.
• Clearing the object between requests on a reused tunnel. Not required: propagateResponseHeaders
  is invoked on every CONNECT response, before the validity check, so the object always describes
  the CONNECT that established the tunnel it is attached to.
• The same change for the TCP connection pool. Tcp::ConnPoolImpl has the identical asymmetry. Its
  onPoolFailure override does take the new parameter, because the base virtual requires it, but
  ignores the value; carrying it further needs a matching overload on Tcp::ConnectionPool::Callbacks
  and a hop in the upstreams/http/tcp adapter. Left out deliberately — nothing here exercises that
  path — and it is a small self-contained follow-up.
• Any change to tcp_proxy wire behaviour (CONNECT semantics, reset behaviour, retries).

Risk Level

Low.

• Opt-in throughout. With provisioned_placeholder_factories empty, internal_upstream behaves
  exactly as today and tcp_proxy takes its existing setData path.
• Both new onPoolFailure overloads default to the existing three-argument form, and
  ActiveClient::connectionFilterState() defaults to nullptr. No existing implementor changes.
• The capture in UpstreamRequest::onPoolFailure is additive; the existing three-argument body still
  runs, so failure handling is otherwise untouched.
• The provisioned object is created empty with a valid header map, so a read before the fill yields
  an empty object rather than a null dereference.

Testing

New integration tests in
test/extensions/transport_sockets/internal_upstream/fwd_mutable_placeholder_integration_test.cc.
The topology in each is: outer HCM → internal-listener cluster carrying the internal_upstream
transport socket → internal listener running tcp_proxy with propagate_response_headers: true →
upstream that rejects the CONNECT with a non-2xx.

1. Placeholder seeded by a downstream HTTP filter, read via %FILTER_STATE%. Covers the
   by-reference in-place mutation independently of Part 1.
2. Placeholder provisioned by internal_upstream via provisioned_placeholder_factories, read
   via %UPSTREAM_FILTER_STATE%. Covers Parts 1 and 2.
3. Same, but the socket internal_upstream wraps is TLS rather than raw buffer. With no handshake
   the pool reports readiness as soon as the user-space connect returns, so the failure path is never
   taken. A TLS handshake has to travel through the CONNECT tunnel, so a rejected CONNECT means the
   pool reports failure instead. This is the case that covers Part 3, and it asserts
   cluster.internal_listener.upstream_cx_connect_fail > 0 so the premise cannot silently rot back
   into a readiness test.

Verified end to end in custom deployment mirroring the architecture above

Docs Changes

internal_upstream.proto: documents provisioned_placeholder_factories, including that a name
already present on the connection from the downstream side is left untouched (no double
provisioning), and that the objects must be mutable for a downstream consumer to fill them.

Release Notes

changelogs/current/new_features/sockets__internal-upstream-provisioned-placeholder-factories.rst:

Added
:ref:provisioned_placeholder_factories <envoy_v3_api_field_extensions.transport_sockets.internal_upstream.v3.InternalUpstreamTransport.provisioned_placeholder_factories>
to the internal_upstream transport socket, naming registered filter state object factories to
provision as empty objects on the upstream connection and share by reference with the downstream
internal connection. A filter on the internal connection can populate such an object in place, and
the value is then readable on the upstream connection.

changelogs/current/new_features/router__upstream-filter-state-on-pool-failure.rst:

The upstream connection filter state is now recorded on the connection pool failure path as well as
on the pool ready path, so filter state written by an upstream transport socket is readable through
%UPSTREAM_FILTER_STATE% when the upstream connection fails. Previously it was only readable when
the connection succeeded.

changelogs/current/new_features/tcp_proxy__fill-existing-tunnel-response-headers-object.rst:

When propagate_response_headers is enabled and a filter state object is already present under
envoy.tcp_proxy.propagate_response_headers, the captured CONNECT response headers are now written
into that object instead of replacing it, so that anything else holding the object observes the value.
Behavior is unchanged when no such object is present.

Platform Specific Features

N/A.

Related

• tcp_proxy: Propagate non-2xx response status on CONNECT to client when using internal listeners #43977 — the report this addresses. It describes this exact topology, asks for the non-2xx
  CONNECT status to reach the client, and notes that
  UPSTREAM_FILTER_STATE(envoy.tcp_proxy.propagate_response_headers) at the egress HCM shows nothing
  while the inner tcp_proxy access log does show the 403. Part 3 is the fix for that specific
  observation. The issue was closed by tcp_proxy: record non-2xx CONNECT response status in transport failure reason #44157, which added visibility on the inner side — the side
  the reporter already had — so what the issue actually asks for is still outstanding.
• tcp_proxy: record non-2xx CONNECT response status in transport failure reason #44157 — records the non-2xx CONNECT status in the transport failure reason, on the inner side.
• internal_upstream: reverse-propagate filter state across the internal listener boundary at close #45237 — the reverse-propagation alternative; see the table above.

No Fixes: line, because this is not the whole of #43977: it makes the status readable at the
outer listener, and the issue asks for it to reach the client's response, which needs the
response-rewriting filter that is out of scope here.

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #46797 was opened by MyUmmaGumma.

see: more, trace.

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #46797 was ready_for_review by MyUmmaGumma.

see: more, trace.

[MyUmmaGumma]

Closing this PR. We found an extension-only solution that requires no Envoy core changes: a shared mutable filter-state holder is passed through internal_upstream, populated by an inner network filter from tcp_proxy’s CONNECT response headers, and read by the outer HTTP filter. We verified it on the actual CONNECT rejection/pool- failure path. Thanks for the guidance and reviews.