You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tooptoop4 <gi...@git.apache.org> on 2018/06/08 23:29:47 UTC
[GitHub] spark pull request #21514: [SPARK-22860] [Core] - hide key password from lin...
GitHub user tooptoop4 opened a pull request:
https://github.com/apache/spark/pull/21514
[SPARK-22860] [Core] - hide key password from linux ps listing
## What changes were proposed in this pull request?
hide password from 'ps' linux command
## How was this patch tested?
Existing tests
Please review http://spark.apache.org/contributing.html before opening a pull request.
I confirm this is my own work
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tooptoop4/spark spark22860
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21514.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #21514
----
commit 380d97d84dda6617f531e29f1d477e2360d29dfd
Author: Toop <re...@...>
Date: 2018-06-08T23:26:02Z
[SPARK-22860] [Core] - hide key password from linux ps listing
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93142 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93142/testReport)** for PR 21514 at commit [`bc8a833`](https://github.com/apache/spark/commit/bc8a83371d6815f6d11431fa5bbffb106d6deec6).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202991814
--- Diff: 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('<a href="https://'))
--- End diff --
`"` instead of `'`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202962837
--- Diff: 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)
+ args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21514
@tooptoop4 the only thing I can see is that you are suing `'` instead of `"` for string. Anyway you can run the scala linter on your local machine too and you can check which part of the code you added to the MasterSuite file causes the failure
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199317575
--- Diff: 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
--- End diff --
please remove this comment
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93104 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93104/testReport)** for PR 21514 at commit [`050b226`](https://github.com/apache/spark/commit/050b226df1340c0c1478bf9f75cdc1fac4142731).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93065/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93172 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93172/testReport)** for PR 21514 at commit [`d4de123`](https://github.com/apache/spark/commit/d4de123e1faee856ea6047229134c2f23458869b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202840687
--- Diff: 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)
+ args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries
--- End diff --
please put the comma at the end of this line instead of the beginning of the next one.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
I just went through the process of getting 'security approval' in my organisation to use Spark in Production. They ran a Qualsys Vulnerability Assessment scanner that among other things picked up a vulnerability called 'presence of string keyStorePassword in process list' so the scan would still fail if the string keyStorePassword appeared at all.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93104/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202841647
--- Diff: 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") {
--- End diff --
nit: we usually put a `:` after the JIRA number, eg. `SPARK-24621:`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93065 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93065/testReport)** for PR 21514 at commit [`825380a`](https://github.com/apache/spark/commit/825380a1cc3ab7733c090bdccfc8abb6db6c3f9d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
@HyukjinKwon can u commit this? i fixed the Scala style
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202841144
--- Diff: 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)
+ args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries
+ , javaOpts.filterNot { opt =>
--- End diff --
we don't put long code inline to function argument usually. Please move this before and store it to a local val used here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199345024
--- Diff: 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")))
--- End diff --
does your PR still end up having the literal 'storePassword' in ps output?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21514
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202990383
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala ---
@@ -99,8 +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)
+ val javaOptsFiltered = javaOpts.filterNot { opt =>
+ opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")}
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202962872
--- Diff: 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)
+ args, sc.executorEnvs, classPathEntries ++ testingClassPath, libraryPathEntries
+ , javaOpts.filterNot { opt =>
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93142/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93172 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93172/testReport)** for PR 21514 at commit [`d4de123`](https://github.com/apache/spark/commit/d4de123e1faee856ea6047229134c2f23458869b).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/21514
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on the issue:
https://github.com/apache/spark/pull/21514
Instead of filtering out the passwords, would it be more useful to blank the passwords or replace them with `******` in the output?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93166/testReport)** for PR 21514 at commit [`39bb2a0`](https://github.com/apache/spark/commit/39bb2a078805dd032ee49d7005f901a96ef7a42d).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [Core] - hide key password from lin...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r194296152
--- Diff: 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")))
--- End diff --
If you really have to do this, I'd have:
```
javaOpts.filterNot { opt =>
opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by pjfanning <gi...@git.apache.org>.
Github user pjfanning commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199344304
--- Diff: 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://"
--- End diff --
Could you try this? It's a bit closer to the usual scala style.
```
val uriScheme = if (conf.getBoolean("spark.ssl.enabled", false)) "https://" else "http://"
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202675020
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala ---
@@ -130,7 +130,12 @@ 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)
+ var uriScheme = "http://"
--- End diff --
this can be:
```
val uriScheme = if (SSL_ENABLED) { "https://" } else { "http://" }
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/21514
ping @tooptoop4
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/21514
Have you tried the config "spark.redaction.regex" ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/21514
I'm going to suggest to close this. The review comments were not addressed more then few months and there's not quite a great point to keep inactive PRs.
Feel free to take over this if any of you here is interested in this. Or @tooptoop4, please recreate a PR after addressing review commnets here.
Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93142 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93142/testReport)** for PR 21514 at commit [`bc8a833`](https://github.com/apache/spark/commit/bc8a83371d6815f6d11431fa5bbffb106d6deec6).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93172/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202962897
--- Diff: 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") {
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202991729
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala ---
@@ -99,8 +99,11 @@ 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)
+ 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)
--- End diff --
wrong indentation here too, missing 2 spaces. Moreover, in such cases, we usually put one argument per line, so:
```
val command = Command("org.apache.spark.executor.CoarseGrainedExecutorBackend",
args,
sc.executorEnvs,
...)
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202977272
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala ---
@@ -130,7 +130,9 @@ 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)
--- End diff --
nit: rename to `sslEnabled`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202991834
--- Diff: 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('<a href="https://'))
+ }
+ } finally {
+ localCluster.stop()
+ }
+ }
+
+ test("SPARK-24621: http urls when ssl disabled") {
+ implicit val formats = org.json4s.DefaultFormats
+ val conf = new SparkConf()
+ conf.set("spark.ssl.enabled", "false")
+ val localCluster = new LocalSparkCluster(2, 2, 512, conf)
+ localCluster.start()
+ try {
+ eventually(timeout(5 seconds), interval(100 milliseconds)) {
+ val json = Source.fromURL(s"http://localhost:${localCluster.masterWebUIPort}/json")
+ .getLines().mkString("\n")
+ assert(!json.contains('<a href="https://'))
--- End diff --
ditto
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [Core] - hide key password from lin...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r194308977
--- Diff: 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")))
--- End diff --
What about `*storePassword`?
Actually I'm thinking of using Hadoop credential provider to store password (https://hadoop.apache.org/docs/current/hadoop-project-dist/hadoop-common/CredentialProviderAPI.html) to avoid plaintext password. I have a local PR for this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93104 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93104/testReport)** for PR 21514 at commit [`050b226`](https://github.com/apache/spark/commit/050b226df1340c0c1478bf9f75cdc1fac4142731).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199345033
--- Diff: 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
--- End diff --
update in latest commit
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202977126
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala ---
@@ -99,8 +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)
+ val javaOptsFiltered = javaOpts.filterNot { opt =>
+ opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")}
--- End diff --
wrong indentation. This should look like:
```
val javaOptsFiltered = javaOpts.filterNot { opt =>
opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93166/testReport)** for PR 21514 at commit [`39bb2a0`](https://github.com/apache/spark/commit/39bb2a078805dd032ee49d7005f901a96ef7a42d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
@mgaido91 - do you know what caused this?
Scalastyle checks failed at following occurrences:
[error] /home/jenkins/workspace/SparkPullRequestBuilder@2/core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala: Expected token RPAREN but got Token(RBRACE,},10480,})
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199345049
--- Diff: 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://"
--- End diff --
woops, i just updated val to var
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21514
**[Test build #93065 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93065/testReport)** for PR 21514 at commit [`825380a`](https://github.com/apache/spark/commit/825380a1cc3ab7733c090bdccfc8abb6db6c3f9d).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202991417
--- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala ---
@@ -99,8 +99,11 @@ 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)
+ val javaOptsFiltered = javaOpts.filterNot { opt =>
+ opt.startsWith("-Dspark.ssl.keyStorePassword") || opt.startsWith("-Dspark.ssl.keyPassword")
--- End diff --
wrong indentation: missing 2 spaces.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199352058
--- Diff: 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://"
--- End diff --
yes, but still please add tests for this, in order to enforce the correctness of your solution. PS I agree with @pjfanning's suggestion..
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93166/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21514
Is this only the place where we need to hide the password? e.g., how about logging about properties in [SparkSubmitArguments](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide key pa...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21514
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r202990353
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala ---
@@ -130,7 +130,9 @@ 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)
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21514: [SPARK-22860] [Core] - hide key password from linux ps l...
Posted by tooptoop4 <gi...@git.apache.org>.
Github user tooptoop4 commented on the issue:
https://github.com/apache/spark/pull/21514
@dbtsai
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21514: [SPARK-22860] [SPARK-24621] [Core] [WebUI] - hide...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21514#discussion_r199317572
--- Diff: 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://"
--- End diff --
have you tested this? This is a val, I doubt this can even compile...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org