Skip to content

Commit e07a9b2

Browse files
committed
[SPARK-59296][CORE][FOLLOWUP] Address review: bind local credential callback to this SparkEnv and document the driver-side early-startup window
Addresses review feedback on the local-mode parity change: - LocalSchedulerBackend.setupUserCredentialManager now captures this backend's SparkEnv once and uses it in both the propagation callback and the initial credential store, instead of looking up the global SparkEnv.get on every callback. stop() only waits a bounded time for the renewal thread, so a renewal that outlives this SparkContext could otherwise write this application's credentials into a different SparkContext's store created later in the same JVM (notebook, test, or Spark Connect session), where the restarted version counter would then reject the new application's own renewals. Binding to this env confines a late renewal to this (stopped) application's store. - Correct the applyProviderProperties scaladoc: running the selection phase in local mode extends the same driver-side early-startup window that already exists in cluster mode. Provider wiring is applied during SparkContext construction but credentials are not resolved until scheduler-backend start, so driver-side access to a wired scheme during construction (spark.jars / spark.files / spark.archives / spark.checkpoint.dir on e.g. s3a://) runs before credentials exist. The earlier wording ("points at credentials that are actually populated") was inaccurate for this window. The OIDC security docs (separate docs PR) document this for local mode alongside cluster mode. This keeps credential resolution late (parity with HadoopDelegationTokenManager and with cluster mode); moving resolution earlier in local mode only would diverge local from cluster and break that design intent, so the window is documented rather than closed.
1 parent f3d9b5a commit e07a9b2

2 files changed

Lines changed: 20 additions & 7 deletions

File tree

core/src/main/scala/org/apache/spark/deploy/security/UserCredentialManager.scala

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -541,10 +541,16 @@ private[spark] object UserCredentialManager extends Logging {
541541
*
542542
* This phase runs in local mode as well: `LocalSchedulerBackend` starts a
543543
* [[UserCredentialManager]] (for parity with `HadoopDelegationTokenManager`, which also runs
544-
* in `LocalSchedulerBackend`), so a resolution phase follows and the wiring applied here
545-
* points at credentials that are actually populated. (Before SPARK-59296's follow-up, local
546-
* mode had no resolution phase, so this selection phase was skipped there to avoid wiring a
547-
* provider whose credentials would never be resolved.)
544+
* in `LocalSchedulerBackend`), so a resolution phase follows and populates the credentials the
545+
* wiring points at. As in cluster mode, the resolution phase runs later than this selection
546+
* phase (at scheduler-backend start), so there is a driver-side early-startup window: any
547+
* driver-side access to a wired scheme during `SparkContext` construction -- for example
548+
* fetching `spark.jars` / `spark.files` / `spark.archives`, or a `spark.checkpoint.dir` on such
549+
* a scheme -- happens before the credentials exist and therefore cannot use them. This is the
550+
* same driver-side window that exists in cluster mode; prefer `local://` for such resources.
551+
* (Before SPARK-59296's follow-up, local mode had no resolution phase; the wiring applied here
552+
* would never have been backed by resolved credentials, so this phase returned early in local
553+
* mode.)
548554
*
549555
* Scheme selection is limited to schemes for which a provider is UNAMBIGUOUSLY selected:
550556
* either an explicitly-configured scheme (`spark.security.oidc.provider.<scheme>`) or a

core/src/main/scala/org/apache/spark/scheduler/local/LocalSchedulerBackend.scala

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,22 +155,29 @@ private[spark] class LocalSchedulerBackend(
155155
* driver's Hadoop Configuration.
156156
*/
157157
private def setupUserCredentialManager(): Unit = {
158+
// Capture this backend's SparkEnv once, rather than looking up the global SparkEnv.get on
159+
// every callback. stop() only waits a bounded time for the renewal thread, so a renewal that
160+
// outlives this SparkContext must not write into a *different* SparkContext's credential
161+
// store (e.g. a new context created in the same JVM by a notebook, test, or Spark Connect
162+
// session). Binding to this env ensures a late renewal updates only this application's store,
163+
// which is harmless once this env is stopped. (CoarseGrainedSchedulerBackend avoids the issue
164+
// differently, by routing updates through its own already-stopped driverEndpoint.)
165+
val env = SparkEnv.get
158166
// Reuse the loader from SparkContext's selection phase (Some when OIDC is enabled, None
159167
// otherwise). Passing the Option straight through keeps SparkContext as the single owner of
160168
// the loader: create() enforces that an enabled configuration has a loader rather than
161169
// silently allocating one here that no one would close.
162170
userCredentialManager = UserCredentialManager.create(conf, { (version, credentials) =>
163171
// No remote executors in local mode; update the shared credential store directly so that
164172
// subsequently dispatched tasks (and driver-side access) observe the new credentials.
165-
VersionedCredentials.updateIfNewer(SparkEnv.get.userCredentials, version, credentials)
173+
VersionedCredentials.updateIfNewer(env.userCredentials, version, credentials)
166174
}, scheduler.sc.userCredentialProviderLoader)
167175
userCredentialManager.foreach { manager =>
168176
val (version, initialCredentials) = manager.start()
169177
// Store initial credentials synchronously so they are available for TaskDescription
170178
// (task dispatch) immediately. The onCredentialsUpdate callback above also runs the same
171179
// updateIfNewer, so this is idempotent.
172-
VersionedCredentials.updateIfNewer(
173-
SparkEnv.get.userCredentials, version, initialCredentials)
180+
VersionedCredentials.updateIfNewer(env.userCredentials, version, initialCredentials)
174181
}
175182
}
176183

0 commit comments

Comments
 (0)