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]
