wangyum commented on code in PR #57525:
URL: https://github.com/apache/spark/pull/57525#discussion_r4000077779
##########
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala:
##########
@@ -62,6 +63,10 @@ class CoarseGrainedSchedulerBackend(scheduler:
TaskSchedulerImpl, val rpcEnv: Rp
// Total number of executors that are currently registered
protected val totalRegisteredExecutors = new AtomicInteger(0)
protected val conf = scheduler.sc.conf
+ private[spark] val driverInstanceToken = UUID.randomUUID().toString
+ // Propagate the token as an environment variable to executor processes
(YARN, Kubernetes).
+ // Standalone mode adds it to Command.environment directly in
StandaloneSchedulerBackend.
+ conf.setExecutorEnv(EXECUTOR_DRIVER_INSTANCE_TOKEN, driverInstanceToken)
Review Comment:
Is this must fix? This is the AI review comments:
**Why a Kubernetes Secret Alone Is Insufficient**
Replacing a literal Pod environment value with `secretKeyRef` would hide the
value from ordinary Pod inspection, but it would not establish isolation if
another application can:
- Read the Secret.
- Execute commands in the target executor.
- Create a Pod that mounts the Secret.
- Access the target application’s authentication credentials through another
path.
A different service-account name also does not help unless its permissions
enforce the intended boundary.
**A Complete Fix**
1. **Keep the instance ID as a non-secret correctness check.**
It distinguishes driver instances and rejects accidental stale
connections. Do not make it an authentication credential.
2. **Require a unique RPC authentication secret per application.**
Authentication must reject an executor from application A before it can
invoke B’s bootstrap endpoint. A shared cluster-wide secret does not provide
this property.
3. **Distribute that secret through a protected Kubernetes mechanism.**
Use the existing supported RPC-secret/file mechanism where possible,
backed by a Secret or equivalent protected mount. Do not expose the value
through literal Pod environment entries, JVM arguments, public attributes, or
session configuration.
4. **Enforce an application isolation boundary.**
Namespace/service-account/RBAC policies must prevent other applications
from reading or mounting the secret or executing in the protected Pods.
Applications allowed to create arbitrary Pods in the same namespace cannot
generally be isolated merely by hiding Secret contents from `get` requests.
5. **Test authentication rejection, not just absence from Pod YAML.**
Verify that an executor with A’s credentials cannot retrieve B’s
bootstrap credentials or register with B, **even when it knows B’s public
instance ID**. Also verify that generated Pod specifications contain no literal
authentication secret.
If instead we turn the UUID itself into a bearer secret, the change becomes
much broader: it must also be removed from executor attributes, configuration
responses, SQL/Connect access, and mismatch errors. **The current mismatch
error includes the expected driver ID**, so hiding it only from Pod
specifications would accomplish little.
**Recommendation For This PR**
I would not add a Kubernetes-only secret-distribution mechanism to
SPARK-58322. It would mix a stale-executor correctness fix with a substantial
authentication and deployment-policy change.
The practical choices are:
| Goal | Action |
|---|---|
| Fix accidental driver attachment | Keep the current design and obtain
sunchao’s explicit agreement that malicious-peer isolation is outside this PR |
| Implement the comment’s isolation requirement | Design application-scoped
authentication and protected distribution as a separate change, including its
deployment prerequisites |
**There is no small Pod-specification edit that fully satisfies the
comment.** If the reviewer requires malicious-replay protection in this PR, we
need to agree on the authentication and Kubernetes permission model before
implementing it.
--
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]
