Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/19631#discussion_r152117915
--- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
@@ -542,7 +496,54 @@ private[spark] class SecurityManager(
* Gets the secret key.
* @return the secret key as a String if authentication is enabled,
otherwise returns null
*/
- def getSecretKey(): String = secretKey
+ def getSecretKey(): String = {
+ if (isAuthenticationEnabled) {
+ Option(sparkConf.getenv(ENV_AUTH_SECRET))
--- End diff --
It seems that now for the driver we are using the conf
(SPARK_AUTH_SECRET_CONF) just as a holding point for yarn. To me this
introduces a bit of risk that it more easily gets out to the user. we are
filtering it out from the spark conf written for executors but that seems more
brittle then if its just not in there.
I realize this makes the code a bit more common for the other modes, but
the other modes aren't really secure. I would almost rather keep the in memory
secretKey variable as storage on yarn.
I think this also makes the secret key available for the user to get on the
driver side (sc.getConf.get..) which I think it would be better to hide.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
