You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yongjiaw <gi...@git.apache.org> on 2015/10/28 06:58:16 UTC
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
GitHub user yongjiaw opened a pull request:
https://github.com/apache/spark/pull/9321
[SPARK-11354] [Web UI] Expose custom log4j files on executor page for standalone cluster.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yongjiaw/spark log4j
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/9321.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 #9321
----
commit 48be2219fd09c73662f4087f921aaf9cc08e4125
Author: Yongjia Wang <yo...@gmail.com>
Date: 2015-10-28T03:41:11Z
Expose custom log4j files on executor page for standalone cluster.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by yongjiaw <gi...@git.apache.org>.
Github user yongjiaw commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-180017844
yea, explicitly registering custom log files makes sense. It's good to close it for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by yongjiaw <gi...@git.apache.org>.
Github user yongjiaw commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-167131245
@andrewor14 regarding the security concern, yes, I think inside LogPage.scala which is run with the worker process, it's hardcoded to only read from the workerDir, and one can already temper with appId and executorId to potentially read the log from other apps. But only the file "stdout" and "stderr" are allowed to be read.
I agree it's best to check what files are being read, but it's not so easy to access the loggers of the executor from worker process, and one cannot assume the worker has the same log4j config as executors even that's probably the case most of the time. Do you have some suggestions?
On the other hand, I cannot see serious issues by allowing user to potentially read any files under workerDir, it will display the bytecode if reading a jar file but I think the user already have full control access to all the files under workerDir, except for maybe other spark app's jar files. So a security measure is probably better enforced at the application level, not by the file name (even it's still the best practice to check file name).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164617416
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-178270318
I agree, a whitelisting mechanism is preferable. Let's close this PR for now since it's inactive and re-open it later with a different approach if there is still interest.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164617399
@yongjiaw I believe the changes here actually have security implications. Previously the viewers could only read `stderr` and `stdout` files on the executor machines, but now they can read anything. If you can limit the files accessible to the log4j ones then we can proceed safely.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164633753
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-151734301
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164633686
**[Test build #47707 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47707/consoleFull)** for PR 9321 at commit [`48be221`](https://github.com/apache/spark/commit/48be2219fd09c73662f4087f921aaf9cc08e4125).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/9321
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-167644002
> On the other hand, I cannot see serious issues by allowing user to potentially read any files under workerDir, it will display the bytecode if reading a jar file but I think the user already have full control access to all the files under workerDir, except for maybe other spark app's jar files.
What if I have some plain text secret (e.g. private key) in some file? Anyone with access to the UI will be able to read it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164633756
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47707/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-174335619
Ping @yongjiaw, could you please either close this pull request or address our comments? What do you think about adding an explicit whitelisting mechanism?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-164619375
**[Test build #47707 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47707/consoleFull)** for PR 9321 at commit [`48be221`](https://github.com/apache/spark/commit/48be2219fd09c73662f4087f921aaf9cc08e4125).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11354] [Web UI] Expose custom log4j fil...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/9321#issuecomment-171450858
I agree that it's not okay to allow arbitrary file reads inside of the worker directory. What _might_ be okay is a configuration mechanism which lets users register specific custom log files to be displayed in the UI (e.g. an explicit whitelist)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org