From 380d97d84dda6617f531e29f1d477e2360d29dfd Mon Sep 17 00:00:00 2001 From: Toop Date: Sat, 9 Jun 2018 09:26:02 +1000 Subject: [PATCH 1/7] [SPARK-22860] [Core] - hide key password from linux ps listing --- .../spark/scheduler/cluster/StandaloneSchedulerBackend.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala index f73a58ff5d48c..9c0e9d3d32903 100644 --- a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala +++ b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala @@ -100,7 +100,7 @@ private[spark] class StandaloneSchedulerBackend( val sparkJavaOpts = Utils.sparkJavaOpts(conf, SparkConf.isExecutorStartupConf) val javaOpts = sparkJavaOpts ++ extraJavaOpts val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend", - args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries, javaOpts) + args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries, javaOpts.filterNot(_.startsWith("-Dspark.ssl.keyStorePassword")).filterNot(_.startsWith("-Dspark.ssl.keyPassword"))) val webUrl = sc.ui.map(_.webUrl).getOrElse("") val coresPerExecutor = conf.getOption("spark.executor.cores").map(_.toInt) // If we're using dynamic allocation, set our initial executor limit to 0 for now. From f38de2bbc7e5538720c7a49c4c6326e243ab5841 Mon Sep 17 00:00:00 2001 From: Toop Date: Sat, 30 Jun 2018 16:18:00 +1000 Subject: [PATCH 2/7] SPARK-24621 --- .../scala/org/apache/spark/deploy/master/Master.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala index 2c78c15773af2..3675e566c7c2e 100644 --- a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala +++ b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala @@ -130,7 +130,13 @@ private[deploy] class Master( logInfo(s"Running Spark version ${org.apache.spark.SPARK_VERSION}") webUi = new MasterWebUI(this, webUiPort) webUi.bind() - masterWebUiUrl = "http://" + masterPublicAddress + ":" + webUi.boundPort + val SSL_ENABLED = conf.getBoolean("spark.ssl.enabled", false) + val uriScheme = "http://" + if (SSL_ENABLED) { + uriScheme = "https://" + } + masterWebUiUrl = uriScheme + masterPublicAddress + ":" + webUi.boundPort + //masterWebUiUrl = "http://" + masterPublicAddress + ":" + webUi.boundPort if (reverseProxy) { masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", masterWebUiUrl) webUi.addProxy() From 825380a1cc3ab7733c090bdccfc8abb6db6c3f9d Mon Sep 17 00:00:00 2001 From: Toop Date: Sun, 1 Jul 2018 19:37:50 +1000 Subject: [PATCH 3/7] updated according to review --- .../src/main/scala/org/apache/spark/deploy/master/Master.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala index 3675e566c7c2e..0bcbad0fd9c2b 100644 --- a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala +++ b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala @@ -131,12 +131,11 @@ private[deploy] class Master( webUi = new MasterWebUI(this, webUiPort) webUi.bind() val SSL_ENABLED = conf.getBoolean("spark.ssl.enabled", false) - val uriScheme = "http://" + var uriScheme = "http://" if (SSL_ENABLED) { uriScheme = "https://" } masterWebUiUrl = uriScheme + masterPublicAddress + ":" + webUi.boundPort - //masterWebUiUrl = "http://" + masterPublicAddress + ":" + webUi.boundPort if (reverseProxy) { masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", masterWebUiUrl) webUi.addProxy() From 050b226df1340c0c1478bf9f75cdc1fac4142731 Mon Sep 17 00:00:00 2001 From: Toop Date: Mon, 16 Jul 2018 22:52:51 +1000 Subject: [PATCH 4/7] style guide <100 char line of code --- .../spark/scheduler/cluster/StandaloneSchedulerBackend.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala index 9c0e9d3d32903..a059f06c10ab7 100644 --- a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala +++ b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala @@ -100,7 +100,9 @@ private[spark] class StandaloneSchedulerBackend( val sparkJavaOpts = Utils.sparkJavaOpts(conf, SparkConf.isExecutorStartupConf) val javaOpts = sparkJavaOpts ++ extraJavaOpts val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend", - args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries, javaOpts.filterNot(_.startsWith("-Dspark.ssl.keyStorePassword")).filterNot(_.startsWith("-Dspark.ssl.keyPassword"))) + args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries + , javaOpts.filterNot { opt => + opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")}) val webUrl = sc.ui.map(_.webUrl).getOrElse("") val coresPerExecutor = conf.getOption("spark.executor.cores").map(_.toInt) // If we're using dynamic allocation, set our initial executor limit to 0 for now. From bc8a83371d6815f6d11431fa5bbffb106d6deec6 Mon Sep 17 00:00:00 2001 From: Toop Date: Tue, 17 Jul 2018 08:04:45 +1000 Subject: [PATCH 5/7] added UT and changed 4 lines to 1 line --- .../apache/spark/deploy/master/Master.scala | 5 +-- .../spark/deploy/master/MasterSuite.scala | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala index 0bcbad0fd9c2b..c972cec8ab033 100644 --- a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala +++ b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala @@ -131,10 +131,7 @@ private[deploy] class Master( webUi = new MasterWebUI(this, webUiPort) webUi.bind() val SSL_ENABLED = conf.getBoolean("spark.ssl.enabled", false) - var uriScheme = "http://" - if (SSL_ENABLED) { - uriScheme = "https://" - } + val uriScheme = if (SSL_ENABLED) { "https://" } else { "http://" } masterWebUiUrl = uriScheme + masterPublicAddress + ":" + webUi.boundPort if (reverseProxy) { masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", masterWebUiUrl) diff --git a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala index 84b3a29b58bf4..9176b21ed051e 100644 --- a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala +++ b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala @@ -282,6 +282,40 @@ class MasterSuite extends SparkFunSuite } } + test("SPARK-24621 https urls when ssl enabled") { + implicit val formats = org.json4s.DefaultFormats + val conf = new SparkConf() + conf.set("spark.ssl.enabled", "true") + val localCluster = new LocalSparkCluster(2, 2, 512, conf) + localCluster.start() + try { + eventually(timeout(5 seconds), interval(100 milliseconds)) { + val json = Source.fromURL(s"https://localhost:${localCluster.masterWebUIPort}/json") + .getLines().mkString("\n") + assert(json.contains(' Date: Tue, 17 Jul 2018 19:57:15 +1000 Subject: [PATCH 6/7] style comments --- .../scheduler/cluster/StandaloneSchedulerBackend.scala | 8 ++++---- .../org/apache/spark/deploy/master/MasterSuite.scala | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala index a059f06c10ab7..924266a1b6aef 100644 --- a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala +++ b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala @@ -99,10 +99,10 @@ private[spark] class StandaloneSchedulerBackend( // Start executors with a few necessary configs for registering with the scheduler val sparkJavaOpts = Utils.sparkJavaOpts(conf, SparkConf.isExecutorStartupConf) val javaOpts = sparkJavaOpts ++ extraJavaOpts - val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend", - args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries - , javaOpts.filterNot { opt => - opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")}) + val javaOptsFiltered = javaOpts.filterNot { opt => + opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")} + val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend", args, + sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries, javaOptsFiltered) val webUrl = sc.ui.map(_.webUrl).getOrElse("") val coresPerExecutor = conf.getOption("spark.executor.cores").map(_.toInt) // If we're using dynamic allocation, set our initial executor limit to 0 for now. diff --git a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala index 9176b21ed051e..cf4240da128bd 100644 --- a/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala +++ b/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala @@ -282,7 +282,7 @@ class MasterSuite extends SparkFunSuite } } - test("SPARK-24621 https urls when ssl enabled") { + test("SPARK-24621: https urls when ssl enabled") { implicit val formats = org.json4s.DefaultFormats val conf = new SparkConf() conf.set("spark.ssl.enabled", "true") @@ -299,7 +299,7 @@ class MasterSuite extends SparkFunSuite } } - test("SPARK-24621 http urls when ssl disabled") { + test("SPARK-24621: http urls when ssl disabled") { implicit val formats = org.json4s.DefaultFormats val conf = new SparkConf() conf.set("spark.ssl.enabled", "false") From d4de123e1faee856ea6047229134c2f23458869b Mon Sep 17 00:00:00 2001 From: Toop Date: Tue, 17 Jul 2018 21:46:49 +1000 Subject: [PATCH 7/7] nit pickin' --- .../main/scala/org/apache/spark/deploy/master/Master.scala | 4 ++-- .../spark/scheduler/cluster/StandaloneSchedulerBackend.scala | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala index c972cec8ab033..1df2b98ee2c93 100644 --- a/core/src/main/scala/org/apache/spark/deploy/master/Master.scala +++ b/core/src/main/scala/org/apache/spark/deploy/master/Master.scala @@ -130,8 +130,8 @@ private[deploy] class Master( logInfo(s"Running Spark version ${org.apache.spark.SPARK_VERSION}") webUi = new MasterWebUI(this, webUiPort) webUi.bind() - val SSL_ENABLED = conf.getBoolean("spark.ssl.enabled", false) - val uriScheme = if (SSL_ENABLED) { "https://" } else { "http://" } + val sslEnabled = conf.getBoolean("spark.ssl.enabled", false) + val uriScheme = if (sslEnabled) { "https://" } else { "http://" } masterWebUiUrl = uriScheme + masterPublicAddress + ":" + webUi.boundPort if (reverseProxy) { masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", masterWebUiUrl) diff --git a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala index 924266a1b6aef..390af3384eb1f 100644 --- a/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala +++ b/core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala @@ -100,7 +100,8 @@ private[spark] class StandaloneSchedulerBackend( val sparkJavaOpts = Utils.sparkJavaOpts(conf, SparkConf.isExecutorStartupConf) val javaOpts = sparkJavaOpts ++ extraJavaOpts val javaOptsFiltered = javaOpts.filterNot { opt => - opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")} + opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword") + } val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend", args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries, javaOptsFiltered) val webUrl = sc.ui.map(_.webUrl).getOrElse("")