HyukjinKwon commented on code in PR #58612:
URL: https://github.com/apache/spark/pull/58612#discussion_r3974175971
##########
common/network-common/src/main/java/org/apache/spark/network/server/AbstractAuthRpcHandler.java:
##########
@@ -83,7 +86,7 @@ public final StreamCallbackWithID receiveStream(
@Override
public StreamManager getStreamManager() {
- return delegate.getStreamManager();
+ return new AuthCheckingStreamManager(delegate.getStreamManager());
Review Comment:
This flips a default on a maintenance branch: on a channel behind an
authentication bootstrap, `StreamRequest`/`ChunkFetchRequest` that used to be
served now throw `SecurityException` until the channel authenticates. On master
that hardening is clearly the right call, but shipping it in a `[3.5]` patch
release changes runtime behavior operators did not opt into -- anything that
today relies on stream/chunk serving before the handshake completes on such a
channel would start getting rejected after a minor upgrade.
Could we gate the fail-closed path behind a config that defaults to the
current (pre-PR) behavior on branch-3.5 (and can default on in master), so the
patch release stays behavior-compatible and operators opt into the stricter
check? If we would rather not add a flag, let's at least call the behavior
change out explicitly in the description and release notes so it is not a
silent default change in a maintenance line.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
