You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add an experimental OpenZiti extension on a Netty client transport SPI - #1668
This adds an experimental OpenZiti extension. It lets an HTTP client reach the origins an OpenZiti identity has a service for, such as crm.internal:443, over the Ziti overlay network. Their host names never go to the system DNS, and every other origin is reached over TCP. Only the client side is covered.
Core: a NettyClientTransport SPI (org.restlet.engine.netty)
It is set with the org.restlet.engine.netty.clientTransport client context attribute.
For each request, NettyClientHelper asks it for a Route to the origin's host and port: a Netty ChannelFactory and AddressResolverGroup.
With a route, it connects through them to the unresolved host and port, bypassing the HTTP and SOCKS proxies and HTTP/3. The request is sent in origin form, without Proxy-Authorization, and routed connections are pooled apart from TCP ones.
Without one, nothing changes: NIO over TCP, with TCP_NODELAY and the bind address.
TLS, HTTP/1.1 and HTTP/2 run over the route's channel as over TCP. The channel initializer now takes a Channel instead of a SocketChannel.
ZitiClientTransport routes an origin over Ziti when a loaded service of the context intercepts it. It waits up to 10 s (configurable) for the context to load its services. It never calls getService(InetSocketAddress), which waits for a service event, forever for an identity without services. Lookups are cached until the next service event.
ZitiSocketChannel is a Netty channel over the SDK's asynchronous socket (ZitiContext.open()). It behaves like Netty's own socket channels:
one read at a time;
writes that don't block the event loop: one in flight, the next started when it completes;
every SDK completion run as an event loop task, never nested in Channel.read().
It hands the unresolved host:port to the SDK, which dials the intercepting service with its dial data and intercept dial options, without the system DNS. So the route uses Netty's no-op resolver.
Why not ziti-netty's ZitiChannel
The spike started with ziti-netty's ZitiChannel, and it doesn't hold up under this connector:
It fires read events from the thread completing the read. The SDK releases its pending read before calling the completion handler, so a read requested during the delivery starts at once. It then completes nested in the call (IllegalReferenceCountException in the HTTP decoder) or on another thread at the same time (bytes out of order, broken chunks).
Its config() returns a new configuration on each call, so AUTO_READ=false and the connect timeout are lost.
It blocks the event loop on every write.
About one run in three failed under 32 concurrent calls. We'll report these upstream.
Tests
NettyClientTransportTestCase (core), with a route over plain TCP and a resolver that never asks the DNS:
a routed origin keeps its host name and goes through the route's resolver;
its connection is pooled and reused;
the proxy is bypassed;
other origins go over TCP.
ZitiSocketChannelTestCase, over a scripted SDK socket:
a read requested while one is pending is ignored;
a read completed inside the call isn't fired nested in it;
partial writes stay in order;
connect, read and write failures, the connect timeout and the end of stream.
ZitiClientTransportTestCase, with the overlay replaced by a fake ZitiContext: routing, keep-alive, a 2 MiB echo, the TCP fallback and HTTPS.
ZitiNetworkTestCase, against a real OpenZiti network.
It only runs when given enrolled identities (-Dziti.client.identity=… -Dziti.server.identity=…, plus -Dziti.router.port for the router-hosted service), so it is skipped in CI.
It covers DNS-free routing, keep-alive with 4 MiB both ways, HTTPS with host name verification, and 32 concurrent calls with the service hosted by a Java identity or by the edge router.
On a local OpenZiti 2.0.8 quickstart it passed 12 runs out of 12, and 4 out of 4 with the router-hosted service.
Build
./mvnw -B verify -P default,test -pl org.restlet.ext.ziti passes, with 26 tests (6 skipped: the network ones), coverage and SpotBugs.
org.restlet passes in full except ShutdownHookTestCase.whenServerIsHandlingBlockingRequestThenItRefusesNewRequest. Its timing assertion (1,000 ms expected, 1,200 to 1,560 ms measured) fails the same way on a clean 3.0 checkout of this machine.
For review
The SPI exposes Netty types (ChannelFactory, Channel, AddressResolverGroup) from org.restlet, under @SuppressWarnings("exports"). Applications that don't use it still see no Netty type, and the changes.md line saying none is exposed now names the exception. The alternative is a Netty-free route type that the connector adapts, but an overlay network needs to provide a channel anyway.
The OpenZiti jars declare no Automatic-Module-Name, so the module requires the filename-based ziti and edge.api. Maven warns not to publish such a module. We'll ask OpenZiti for module names; until then, releasing this artifact is a judgement call.
The SDK's runtime-scope slf4j-simple and clikt are excluded, so the extension doesn't force a logging binding on applications.
Context: Ikanos plans to replace its Jetty-based Ziti tunnel with this once it moves to 3.0.0-m2 (naftiko/ikanos#961).
The Netty client connector gains the NettyClientTransport SPI, set with
the "org.restlet.engine.netty.clientTransport" client context
attribute: for each new connection it can return a route (a Netty
channel factory and resolver) to the origin's host and port, used
instead of TCP and the HTTP and SOCKS proxies.
The org.restlet.ext.ziti extension (OpenZiti Java SDK 0.35.0) uses it to
route the calls to the origins an OpenZiti identity has a service for
over the Ziti overlay network, without resolving their host name with
the system DNS. ZitiSocketChannel is a Netty channel over the SDK's
asynchronous socket with Netty's semantics: one read at a time,
non-blocking writes, every SDK completion run on the event loop.
ziti-netty's ZitiChannel isn't used: it fires its read events off the
event loop, loses its channel options and blocks on writes, and failed
about one run in three under concurrent calls. Client side only.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved transport defects can expose proxy credentials, bypass overlay routing, hang requests indefinitely, and send queued data before the caller flushes it.
Adds experimental, client-side OpenZiti support through a new Netty transport SPI, allowing intercepted HTTP origins to use the overlay while other origins retain the existing TCP path.
Changes:
Adds per-connection channel and resolver selection, with proxy bypass for routed connections.
Introduces an OpenZiti extension with an asynchronous Netty channel adapter.
Adds routing, channel lifecycle, TLS, and optional real-network tests, plus build and changelog updates.
… extension
- ZitiSocketChannelTestCase: wait for channelInactive, which Netty fires after
completing the close future, instead of reading the flag right away (failed
in CI). The recorder now waits for its text with wait/notifyAll.
- ZitiNetworkTestCase: poll the network with a scheduled executor instead of
Thread.sleep loops.
- ZitiSocketChannel: move the read, write and dial helpers into the inner
classes that call them, and use unnamed catch variables.
- NettyClientHelper: extract the transport and remote address selection out
of connect() to reduce its cognitive complexity.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…port and channel
Core (Netty client connector):
- Choose the route once per request, in create(): a routed request is sent in
origin form, its Proxy-Authorization header is removed, it never uses HTTP/3
(which resolved the host with the system DNS and bypassed the route), and its
connections are pooled apart from the TCP ones.
OpenZiti extension:
- ZitiClientTransport no longer calls ZitiContext.getService(InetSocketAddress),
which waits for a service event, forever for an identity without services. It
waits, up to a timeout (10 s by default), for the context to load its services,
then looks the origin up without waiting, caching the lookups until a service
event.
- install() sets a context on a client without one.
- ZitiSocketChannel hands the unresolved address to the SDK, which dials the
intercepting service with its dial data and intercept dial options (such as
the terminator identity) instead of a service-only Dial.
- A completed write resumes the flushed writes only (flush0()), instead of
flushing the messages written since.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
- Build the warning on missing Ziti services lazily.
- Catch the TimeoutException of the service lookup through a lookup interface
declaring it, instead of an instanceof test.
- Use assertNotEquals in the HTTP/3 routing test.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Identity-hash pool-key collisions can cause same-origin requests to reuse another route’s pool and resolver, sending traffic through the wrong overlay identity.
The one-argument service lookup can block indefinitely when the client identity receives no service events. After the 60-second timeout, poller.close() waits for that blocked task, so the test hangs instead of failing. Use a bounded service lookup and cancel the polling task before closing the executor on timeout.
…fix a cache race
- NettyClientHelper: key the connection pools with a PoolKey record holding the
route, instead of the route's identity hash code, which two routes can share.
Routed pools are transient like the tunnel ones: dropped once they have no
connection left, so retired routes don't accumulate.
- ZitiClientTransport: uncache a lookup when a service event was counted while it
was published, so that a pre-event result isn't left in the cache.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
When the client identity has no services, this SDK overload can block the polling task indefinitely. The 60-second timeout does not end the readiness check because closing the scheduled executor waits for that blocked task. Use a non-waiting service lookup, treat its timeout as "not ready," and reuse the result for the terminator check.
Reclaim idle HTTP/2 connections across route pools
With HTTP/2 and maxConnectionsPerDestination=1, switching an origin from route A to route B leaves A's parent holding the only connection permit after its streams finish. Because acquirePermit() only reclaims HTTP/1.1 idle connections, B can time out with "Too many connections" before A's idle timeout expires. Reclaim HTTP/2 parents across route pools only when they have no active streams, and cover this route-switch case.
Retain and cancel transport subscriptions on disposal
These subscriptions capture the transport, but their cancellation handles are discarded and there is no cleanup path. Stopping/recreating clients, or replacing a transport with install(), while retaining the application-owned ZitiContext therefore retains every old transport and its lookup cache and keeps invoking its callbacks. Please retain the subscription handles and provide a disposal/uninstallation mechanism that cancels them without destroying the shared context.
…ts, and close Ziti transports
Address the third Copilot review:
- Count a new connection in a routed pool and drop an empty one under the
pool map's lock of their key, so a connection is never opened in a pool
already dropped, where it couldn't be reused nor its permit reclaimed.
A call whose pool was dropped takes the current one.
- Count the streams of each HTTP/2 connection. Waiting for a permit, a
call closes an idle connection of another pool of the origin: an HTTP/1.1
one, or now an HTTP/2 one without streams, retired first so that no
stream is opened on it. It looks again while it waits, until one closes,
since a connection becomes idle only once its last stream has closed.
- ZitiClientTransport keeps its subscriptions to the context's events and
cancels them on close(), without destroying the context; install()
closes the Ziti transport it replaces.
- The network test looks the services up without waiting, and interrupts
a check still running when it gives up.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The three issues Copilot's last review listed under "previously missed issues" are fixed in 113c4b2:
Reclaim idle HTTP/2 connections across route pools (NettyClientHelper): each HTTP/2 connection now counts its streams, open or reserved. When a call is waiting for a permit, it closes an idle connection of another pool of the same origin. That can be an HTTP/1.1 connection, or now an HTTP/2 connection without streams, which is first marked retired so no new stream opens on it. The call keeps looking while it waits, until one is closed, because a connection only becomes idle once its last stream has closed. NettyClientTransportTestCase.reclaimsTheIdleHttp2ConnectionOfAnotherRoute covers this: HTTP/2, maxConnectionsPerDestination=1, with the route switching from A to B. Without the fix, it fails on the second call with "Connection Error (1000)".
Retain and cancel transport subscriptions (ZitiClientTransport): the transport keeps the Futures returned by onStatus and onServiceEvent, and is now AutoCloseable. close() cancels them without destroying the context, which it doesn't own. A closed transport still routes, but no longer caches lookups, since it no longer receives service events. install() closes the Ziti transport it replaces. Covered by closingStopsListeningToTheContext and installingClosesTheReplacedTransport. The changelog says to close it once the client is stopped.
Readiness check that never returns (ZitiNetworkTestCase): services are now looked up with the zero-timeout overloads (getService(name, 0) and getService(address, 0)), which don't wait. Their TimeoutException counts as "not ready", and the terminator check reuses that lookup. When it gives up, awaitUntil now calls shutdownNow() on the poller, which interrupts a check still running instead of waiting for it.
Checks: 238 core connector tests (org.restlet.engine.netty.* and org.restlet.engine.connector.*) pass, and SpotBugs reports no findings in NettyClientHelper. org.restlet.ext.ziti verify passes: 32 tests, 6 skipped because they need a real network, and no SpotBugs findings.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Concurrency races in connection-pool reclamation and asynchronous connect completion warrant human assessment because they can cause spurious connection failures or report closed channels as active.
A server may answer an HTTP/2 request as soon as it has its headers, for
example with an immediate 401, then close the stream. The client's last,
empty write of the request then failed at once, and the call reported a
communication error instead of the response it had received. When the
stream ended that way before the request was marked sent, the request was
also reported as disconnected.
Once the response is complete, a failed last write is no longer an error,
and a later failure of the exchange no longer reports a disconnect.
This made HttpDisconnectTestCase's HTTP/2 case fail now and then, 2 runs
in 10 before, none in 15 after.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds an experimental OpenZiti extension. It lets an HTTP client reach the origins an OpenZiti identity has a service for, such as
crm.internal:443, over the Ziti overlay network. Their host names never go to the system DNS, and every other origin is reached over TCP. Only the client side is covered.Core: a
NettyClientTransportSPI (org.restlet.engine.netty)org.restlet.engine.netty.clientTransportclient context attribute.NettyClientHelperasks it for aRouteto the origin's host and port: a NettyChannelFactoryandAddressResolverGroup.Proxy-Authorization, and routed connections are pooled apart from TCP ones.TCP_NODELAYand the bind address.Channelinstead of aSocketChannel.Extension:
org.restlet.ext.ziti(OpenZiti Java SDK 0.35.0)ZitiClientTransportroutes an origin over Ziti when a loaded service of the context intercepts it. It waits up to 10 s (configurable) for the context to load its services. It never callsgetService(InetSocketAddress), which waits for a service event, forever for an identity without services. Lookups are cached until the next service event.ZitiSocketChannelis a Netty channel over the SDK's asynchronous socket (ZitiContext.open()). It behaves like Netty's own socket channels:Channel.read().host:portto the SDK, which dials the intercepting service with its dial data and intercept dial options, without the system DNS. So the route uses Netty's no-op resolver.Why not ziti-netty's
ZitiChannelThe spike started with ziti-netty's
ZitiChannel, and it doesn't hold up under this connector:IllegalReferenceCountExceptionin the HTTP decoder) or on another thread at the same time (bytes out of order, broken chunks).config()returns a new configuration on each call, soAUTO_READ=falseand the connect timeout are lost.About one run in three failed under 32 concurrent calls. We'll report these upstream.
Tests
NettyClientTransportTestCase(core), with a route over plain TCP and a resolver that never asks the DNS:ZitiSocketChannelTestCase, over a scripted SDK socket:ZitiClientTransportTestCase, with the overlay replaced by a fakeZitiContext: routing, keep-alive, a 2 MiB echo, the TCP fallback and HTTPS.ZitiNetworkTestCase, against a real OpenZiti network.-Dziti.client.identity=… -Dziti.server.identity=…, plus-Dziti.router.portfor the router-hosted service), so it is skipped in CI.Build
./mvnw -B verify -P default,test -pl org.restlet.ext.zitipasses, with 26 tests (6 skipped: the network ones), coverage and SpotBugs.org.restletpasses in full exceptShutdownHookTestCase.whenServerIsHandlingBlockingRequestThenItRefusesNewRequest. Its timing assertion (1,000 ms expected, 1,200 to 1,560 ms measured) fails the same way on a clean3.0checkout of this machine.For review
ChannelFactory,Channel,AddressResolverGroup) fromorg.restlet, under@SuppressWarnings("exports"). Applications that don't use it still see no Netty type, and thechanges.mdline saying none is exposed now names the exception. The alternative is a Netty-free route type that the connector adapts, but an overlay network needs to provide a channel anyway.Automatic-Module-Name, so the module requires the filename-basedzitiandedge.api. Maven warns not to publish such a module. We'll ask OpenZiti for module names; until then, releasing this artifact is a judgement call.slf4j-simpleandcliktare excluded, so the extension doesn't force a logging binding on applications.Context: Ikanos plans to replace its Jetty-based Ziti tunnel with this once it moves to 3.0.0-m2 (naftiko/ikanos#961).
🤖 Generated with Claude Code