roczei commented on code in PR #532:
URL: https://github.com/apache/livy/pull/532#discussion_r3655011251


##########
server/src/main/scala/org/apache/livy/utils/SparkApp.scala:
##########
@@ -89,6 +93,53 @@ object SparkApp {
     } else {
       sparkConf
     }
+    enrichHiveMetastoreSslConf(baseConf, livyConf)
+  }
+
+  /**
+   * Resolves Hive Metastore SSL credentials from the local JCEKS credential 
provider
+   * (reachable only on the Livy server host) and injects them as explicit 
plaintext
+   * spark.hadoop.* properties so the driver/executors on YARN don't need 
access to
+   * the local file. The provider path itself is blanked afterward so it isn't
+   * propagated to nodes where it can't be resolved.

Review Comment:
   It looks like the comment here is outdated. The code in SparkApp.scala 
doesn't actually blank the provider path. deleteRecursively seems to only exist 
in SparkAppSpec.scala.



##########
server/src/main/scala/org/apache/livy/utils/SparkApp.scala:
##########
@@ -89,6 +93,53 @@ object SparkApp {
     } else {
       sparkConf
     }
+    enrichHiveMetastoreSslConf(baseConf, livyConf)
+  }
+
+  /**
+   * Resolves Hive Metastore SSL credentials from the local JCEKS credential 
provider
+   * (reachable only on the Livy server host) and injects them as explicit 
plaintext
+   * spark.hadoop.* properties so the driver/executors on YARN don't need 
access to
+   * the local file. The provider path itself is blanked afterward so it isn't
+   * propagated to nodes where it can't be resolved.
+   *
+   * No-op if no credential provider path is configured, or if it resolves to
+   * unrelated (non-HMS) secrets; in either case the original conf is returned
+   * untouched so any user-supplied provider path (e.g. an HDFS-backed one used
+   * for other credentials) is preserved.
+   */
+  private def enrichHiveMetastoreSslConf(
+      conf: Map[String, String],
+      livyConf: LivyConf): Map[String, String] = {
+    val credentialProviderPath = 
livyConf.get(LivyConf.HADOOP_CREDENTIAL_PROVIDER_PATH)
+    if (credentialProviderPath == null || credentialProviderPath.isEmpty) {
+      return conf
+    }
+    val hadoopConf = new Configuration()
+    hadoopConf.set("hadoop.security.credential.provider.path", 
credentialProviderPath)
+    val hmsSslKeys = Seq("hive.metastore.keystore.password", 
"hive.metastore.truststore.password")

Review Comment:
   The entire point of using a JCEKS credential provider is to avoid exposing 
passwords in plaintext configs. By injecting them directly as Spark properties, 
they will very likely show up on the /environment page in both the Spark UI and 
History Server UI (unless redacted by spark.redaction.regex, which should be 
enabled by default). Could you double-check how these appear on those UIs and 
also ensure that the passwords aren't being printed anywhere in the logs?



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

Reply via email to