You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "Kalvin2077 (via GitHub)" <gi...@apache.org> on 2026/10/22 16:33:02 UTC
[PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Kalvin2077 opened a new pull request, #3894:
URL: https://github.com/apache/celeborn/pull/3894
### What changes were proposed in this pull request?
Extract the configurable transport bootstrap support from #3857:
- Add module-specific client/server bootstrap settings in `CelebornConf`.
- Load, order, and close configured bootstraps in `TransportContext`.
- Integrate lifecycle management into RPC, Worker, and shuffle clients.
### Why are the changes needed?
Allow deployments to plug in custom transport authentication through configuration without modifying Celeborn's core transport code.
### Does this PR resolve a correctness bug?
- [ ] Yes
### Does this PR introduce _any_ user-facing change?
- [x] Yes
Adds `celeborn.<module>.client.bootstrap.classes` and `celeborn.<module>.server.bootstrap.classes`, both empty by default. Configured bootstraps run before native registration/SASL bootstraps when enabled.
### How was this patch tested?
Focused validation covered bootstrap loading, ordering, cleanup, and client/Worker lifecycle. 85 tests passed on JDK 8 and 9 passed with the JDK 17 runtime, using the Flink 1.18 profile. Spotless passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Posted by "Kalvin2077 (via GitHub)" <gi...@apache.org>.
Kalvin2077 commented on code in PR #3894:
URL: https://github.com/apache/celeborn/pull/3894#discussion_r4375970487
##########
common/src/main/java/org/apache/celeborn/common/network/server/TransportServerBootstrap.java:
##########
@@ -24,13 +24,24 @@
* server. This allows customizing the client channel to allow for things such as SASL
* authentication.
*/
-public interface TransportServerBootstrap {
+public interface TransportServerBootstrap extends AutoCloseable {
/**
- * Customizes the channel to include new features, if needed.
+ * Customizes the channel to include new features, if needed. Configured instances are shared
+ * across connections; create a separate handler for each connection. Authentication handlers must
+ * establish identity before forwarding business requests and preserve the delegate's registration
+ * checks and channel lifecycle callbacks.
*
* @param channel The connected channel opened by the client.
* @param baseMessageHandler The RPC handler for the server.
* @return The base message handler to use for the channel.
*/
BaseMessageHandler doBootstrap(Channel channel, BaseMessageHandler baseMessageHandler);
+
+ /**
+ * Releases resources owned by a configured bootstrap. The transport context owns the shared
+ * bootstrap and closes it after its factories and servers. Caller-supplied instances remain the
+ * caller's responsibility.
+ */
+ @Override
+ default void close() throws Exception {}
Review Comment:
Using the same class for both client and server bootstraps is unusual, but it is a valid implementation.
I've moved the default close() into a shared TransportBootstrap parent, so both interfaces inherit one implementation. This resolves the compatibility issue Copilot identified for plugins implementing both interfaces without overriding close().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3894:
URL: https://github.com/apache/celeborn/pull/3894#discussion_r4374581913
##########
common/src/main/java/org/apache/celeborn/common/network/server/TransportServerBootstrap.java:
##########
@@ -24,13 +24,24 @@
* server. This allows customizing the client channel to allow for things such as SASL
* authentication.
*/
-public interface TransportServerBootstrap {
+public interface TransportServerBootstrap extends AutoCloseable {
/**
- * Customizes the channel to include new features, if needed.
+ * Customizes the channel to include new features, if needed. Configured instances are shared
+ * across connections; create a separate handler for each connection. Authentication handlers must
+ * establish identity before forwarding business requests and preserve the delegate's registration
+ * checks and channel lifecycle callbacks.
*
* @param channel The connected channel opened by the client.
* @param baseMessageHandler The RPC handler for the server.
* @return The base message handler to use for the channel.
*/
BaseMessageHandler doBootstrap(Channel channel, BaseMessageHandler baseMessageHandler);
+
+ /**
+ * Releases resources owned by a configured bootstrap. The transport context owns the shared
+ * bootstrap and closes it after its factories and servers. Caller-supplied instances remain the
+ * caller's responsibility.
+ */
+ @Override
+ default void close() throws Exception {}
Review Comment:
The independent default `close()` methods here and in `TransportClientBootstrap` break plugins that implement both interfaces without overriding `close()`: recompilation fails, and existing binaries can fail during cleanup with `IncompatibleClassChangeError`. That error bypasses the `Exception` catch in `TransportContext.closeBootstraps()`, skipping the remaining bootstrap and SSL cleanup. Move the default method into one shared parent interface inherited by both bootstrap interfaces.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Posted by "Kalvin2077 (via GitHub)" <gi...@apache.org>.
Kalvin2077 commented on PR #3894:
URL: https://github.com/apache/celeborn/pull/3894#issuecomment-6358574591
cc @zaynt4606 @SteNicholas @RexXiong take a look when you get the chance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Posted by "Kalvin2077 (via GitHub)" <gi...@apache.org>.
Kalvin2077 commented on PR #3894:
URL: https://github.com/apache/celeborn/pull/3894#issuecomment-6398198507
@zaynt4606 Thx for your review.
I've clarified the identity contract in docs/security.md: clientId retains Celeborn application-ID semantics, while independent plugin principals and authentication state belong in separate per-connection state.
The ordering test now exercises configured bootstraps followed by actual native SASL. I've also added invalid-credential coverage verifying that the connection closes before any subsequent client bootstrap, native authentication challenge, or business request runs. Existing identity immutability and application authorization checks remain unchanged.
For context, the planned follow-up introduces ConnectionSecurityContext to hold plugin identity and authorization policy. It also changes Celeborn's RPC infrastructure to preserve the receiving connection through dispatch, including one-way calls, so request authorization can use that context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2468] Add module-specific transport bootstrap configuration [celeborn]
Posted by "zaynt4606 (via GitHub)" <gi...@apache.org>.
zaynt4606 commented on code in PR #3894:
URL: https://github.com/apache/celeborn/pull/3894#discussion_r4370249526
##########
docs/security.md:
##########
@@ -143,3 +143,101 @@ celeborn.master.ha.node.3.ratis.port 9872
spark.celeborn.auth.enabled true
```
+## Configurable transport bootstraps
+
+Transport bootstraps let a plugin perform an additional handshake when a connection opens.
+Implement `org.apache.celeborn.common.network.client.TransportClientBootstrap` on the initiating
+side and `org.apache.celeborn.common.network.server.TransportServerBootstrap` on the accepting side.
+Celeborn's existing request authorization checks still apply after the handshake.
+
+### Configuration and deployment
+
+Compile the plugin against the matching Celeborn transport API and put its classes and dependencies
+on the classpath of every participating process. Celeborn uses the thread context class loader.
+Each class must implement the interface for its configured side and expose a public constructor
+taking `TransportConf`, or a public no-argument constructor. The `TransportConf` constructor takes
+precedence when both exist.
+
+Shaded Spark and Flink clients relocate dependencies such as Netty, including types in bootstrap
+method signatures. Build the plugin for the distribution that loads it, using matching relocated
+types for a shaded client. Keep Celeborn API classes out of the plugin JAR so the host supplies them.
+
+Configure comma-separated class names with these properties. Both default to an empty list:
+
+| Property | Meaning |
+| --- | --- |
+| `celeborn.<module>.client.bootstrap.classes` | Client bootstraps, in handshake execution order. |
+| `celeborn.<module>.server.bootstrap.classes` | Server bootstraps, in handshake execution order. |
+
+Bootstrap lists apply only to the exact module name and do not inherit parent module settings.
+Configure matching lists on both sides of each participating transport:
+
+| Transport | Initiating side | Accepting side |
+| --- | --- | --- |
+| LifecycleManager RPC to Master/Worker, with `celeborn.auth.enabled=false` | `rpc_app_lifecyclemanager` client | `rpc_service` server |
+| LifecycleManager RPC to Master/Worker, with `celeborn.auth.enabled=true` | `rpc_service` client | `rpc_service` server |
+| Service RPC between Master/Worker processes | `rpc_service` client | `rpc_service` server |
+| Shuffle push | Application `data` client | Worker `push` server |
+| Shuffle fetch | Application `data` client | Worker `fetch` server |
+| Worker replication | Worker `replicate` client | Worker `replicate` server |
+| RPC within an application | The initiating process's `rpc_app_client` or `rpc_app_lifecyclemanager` client | The receiving process's `rpc_app_client` or `rpc_app_lifecyclemanager` server |
+
+Public and internal Master/Worker RPC listeners both use `rpc_service`. Their bootstrap lists
+cannot be configured separately, so a plugin must support the peers using both listeners.
+
+For example, a plugin protecting shuffle data connections could use these settings.
+Replace the `com.example.security` placeholders with the installed plugin class names:
+
+```properties
+# Application-side Celeborn configuration.
+celeborn.data.client.bootstrap.classes com.example.security.TokenClientBootstrap
+
+# Worker-side Celeborn configuration.
+celeborn.push.server.bootstrap.classes com.example.security.TokenServerBootstrap
+celeborn.fetch.server.bootstrap.classes com.example.security.TokenServerBootstrap
+```
+
+When supplying Celeborn settings through Spark configuration, prefix the names with `spark.`.
+Configure RPC and replication separately if those transports use the plugin.
+
+### Handshake order and handler obligations
+
+For client and server lists `A,B`, a connection completes A's handshake, then B's. Configured client
+bootstraps run before caller-supplied bootstraps. Configured server wrappers receive requests in
+configuration order, before caller-supplied handlers. Caller-supplied lists retain their existing
+ordering behavior.
+
+Configured bootstraps run whether `celeborn.auth.enabled` is enabled or disabled. When native
Review Comment:
[P2] Clarify the application-identity contract when composing with native authentication
Could we document the ownership and application-ID semantics of `TransportClient.clientId` here and in the bootstrap API contract? `BaseMessageHandler.checkAuth()` interprets this field as the Celeborn application ID, and `setClientId()` rejects changing an identity that has already been set.
I reproduced a configured bootstrap authenticating as `plugin` and setting that client ID, followed by native SASL authenticating as `native`: the connection fails with `Client ID has already been set`. Configured and native SASL layers using the same application ID do compose successfully. Plugin authors need this distinction when implementing the composition described here.
Please clarify that an independent plugin principal belongs in separate per-connection state, while `clientId` must retain Celeborn's application-identity semantics. The existing identity immutability and application authorization checks should be preserved.
Could we also add regression coverage for configured bootstraps followed by actual native SASL: successful composition, and invalid plugin credentials closing the connection before any subsequent bootstrap or business request runs? The existing ordering test uses a plain caller callback after the configured SASL bootstrap, so it does not exercise the native-SASL combination.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org