sweb commented on code in PR #58010:
URL: https://github.com/apache/spark/pull/58010#discussion_r4307559034


##########
docs/security.md:
##########
@@ -183,6 +183,28 @@ Unlike the other SSL settings for the UI, the RPC SSL is 
*not* automatically ena
 `spark.ssl.enabled` is set. It must be explicitly enabled, to ensure a safe 
migration path for users
 upgrading Spark versions.
 
+On Kubernetes, RPC SSL passwords set in the Spark configuration are propagated 
to executor pods
+using environment variables, like the authentication secret described under
+[Kubernetes](#kubernetes) above, so any user who can list pods in the 
namespace can read them. In
+cluster mode they are also stored in plaintext in the driver's ConfigMap. To 
avoid both, store the
+passwords in a Kubernetes Secret and bind the following variables on the 
driver and the executors
+using `spark.kubernetes.driver.secretKeyRef.<name>` and
+`spark.kubernetes.executor.secretKeyRef.<name>`, instead of setting them in 
the Spark configuration:
+
+* `_SPARK_SSL_RPC_KEY_STORE_PASSWORD`
+* `_SPARK_SSL_RPC_KEY_PASSWORD`
+* `_SPARK_SSL_RPC_PRIVATE_KEY_PASSWORD`
+* `_SPARK_SSL_RPC_TRUST_STORE_PASSWORD`
+
+For example:
+`spark.kubernetes.executor.secretKeyRef._SPARK_SSL_RPC_KEY_STORE_PASSWORD=<secret-name>:<key>`.
+
+A variable already bound on the executors, through `secretKeyRef`, 
`spark.executorEnv` or the
+executor pod template, is not overwritten. Passwords the driver reads from a 
Hadoop credential
+provider, from its environment or through variable substitution are not 
propagated and must be

Review Comment:
   addressed in 
https://github.com/apache/spark/pull/58010/commits/ae1664f2d2ad87f4f8c512f864c3365600177965



##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala:
##########
@@ -142,6 +143,32 @@ private[spark] class BasicExecutorFeatureStep(
         case _ => Nil
       }.getOrElse(Nil)
 
+      // spark.ssl.* passwords are withheld from the executor conf, so they go 
through env.
+      // Unlike standalone, a name the user already binds wins; for 
secretKeyRef the skip only
+      // keeps a plaintext copy out of the pod spec. envFrom keys are unknown 
and lose to env,
+      // so any envFrom disables the injection.
+      val container = Option(pod.container)
+      val userBoundEnvNames = kubernetesConf.secretEnvNamesToKeyRefs.keySet ++
+        kubernetesConf.environment.keySet ++
+        container.flatMap(c => Option(c.getEnv))
+          .map(_.asScala.map(_.getName).toSet).getOrElse(Set.empty)
+      val unboundSslRpcPasswords = kubernetesConf.sslRpcPasswordEnvs.filterNot 
{
+        case (name, _) => userBoundEnvNames.contains(name)
+      }
+      val hasEnvFrom = container.flatMap(c => 
Option(c.getEnvFrom)).exists(!_.isEmpty)
+      val sslRpcPasswords = if (hasEnvFrom) {
+        if (unboundSslRpcPasswords.nonEmpty &&
+            BasicExecutorFeatureStep.sslRpcEnvFromWarned.compareAndSet(false, 
true)) {
+          val names = unboundSslRpcPasswords.keys.toSeq.sorted.mkString(", ")
+          logWarning(log"Executor container uses envFrom, so RPC SSL passwords 
are not " +
+            log"injected. Bind ${MDC(LogKeys.EXECUTOR_ENVS, names)} on the 
executors unless " +
+            log"envFrom provides them.")
+        }
+        Seq.empty
+      } else {
+        unboundSslRpcPasswords.toSeq

Review Comment:
   addressed in 
https://github.com/apache/spark/pull/58010/commits/ae1664f2d2ad87f4f8c512f864c3365600177965



-- 
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]

Reply via email to