[SPARK-15487] [Web UI] Spark Master UI to reverse proxy Application and Workers UI by gurvindersingh · Pull Request #13950 · apache/spark

gurvindersingh · 2016-06-28T19:03:30Z

What changes were proposed in this pull request?

This pull request adds the functionality to enable accessing worker and application UI through master UI itself. Thus helps in accessing SparkUI when running spark cluster in closed networks e.g. Kubernetes. Cluster admin needs to expose only spark master UI and rest of the UIs can be in the private network, master UI will reverse proxy the connection request to corresponding resource. It adds the path for workers/application UIs as

WorkerUI: <http/https>://master-publicIP:/target/workerID/
ApplicationUI: <http/https>://master-publicIP:/target/appID/

This makes it easy for users to easily protect the Spark master cluster access by putting some reverse proxy e.g. https://github.com/bitly/oauth2_proxy

How was this patch tested?

The functionality has been tested manually and there is a unit test too for testing access to worker UI with reverse proxy address.

@pwendell @bomeng @BryanCutler can you please review it, thanks.

ajbozarth · 2016-06-28T19:19:27Z

This looks good and I really like this added functionality, I'll check out your code and give it a test run as soon as I have time.

prb · 2016-06-28T19:19:52Z

+ <li><strong>
+ {
+ if (parent.master.reverseProxy) {
+ <a href={"/target/" + app.id + "/"}>Application Detail UI</a>


Should the "/target/" be a literal or a configuration value?

By all means it can be a configuration value, but there were already some literal conventions eg. "app" so I thought of having it this way. But if community prefer the configured value, then I will add that.

gurvindersingh · 2016-07-01T05:21:41Z

@ajbozarth what are the steps ahead in terms of moving this pr closer to being merged.

ajbozarth · 2016-07-01T18:23:05Z

@srowen @zsxwing @tgravescs What do you guys think of @gurvindersingh proposal?

tnachen · 2016-07-06T05:25:44Z

+<tr>
+ <td><code>spark.ui.reverseProxyUrl</code></td>
+ <td>http://localhost:8080</td>
+ <td>


includeing -> including

tgravescs · 2016-07-06T15:05:49Z

overall proposal seems fine to me if its only affecting standalone mode where things are going through the master. It would be similar to the hadoop Yarn RM proxy although that was mostly for security reasons. You do need to make sure all the authentication and authorization stuff still applies

gurvindersingh · 2016-07-11T08:51:28Z

@tgravescs Yeah the proposal is only for standalone mode where worker & application UI is accessed through master UI now. Looking at the authn/z settings for standalone, I don't see this patch interfere with any of those. The addFilters() function set the ui.filters for '/*' path so it will apply for this case too. Beyond this let me know if I miss anything.

gurvindersingh · 2016-07-18T07:50:24Z

@tgravescs @ajbozarth any update on this PR ?

ajbozarth · 2016-07-19T00:01:52Z

I'll try and take a look at this tomorrow

ajbozarth · 2016-07-20T00:13:28Z

I think changing /target/ to /proxy/ would make a clearer url.
@tgravescs what does the yarn proxy url look like? I don't use yarn outside of testing.

Overall LGTM though

gurvindersingh · 2016-07-20T09:16:20Z

Changed the path from target to proxy Let me know if there is anything else before this PR can be merged.

gurvindersingh · 2016-07-28T06:42:13Z

@ajbozarth ping on this..

ajbozarth · 2016-07-28T18:07:14Z

LGTM, @tgravescs this ready to merge?

gurvindersingh · 2016-08-03T04:13:53Z

@ajbozarth it seems @tgravescs is busy, is anybody else then who you think could help us in merging this.

chandru-ops · 2016-08-03T16:55:36Z

I'm trying to have nginx as reverse proxy for Spark dashboards which doesn't support app/driver logs now. Thank you @gurvindersingh . I will try this.

ajbozarth · 2016-08-03T19:23:10Z

@srowen @JoshRosen could we get some traction on this? Thanks!

vanzin · 2016-08-04T17:47:26Z

ok to test

SparkQA · 2016-08-04T19:54:52Z

Test build #63232 has finished for PR 13950 at commit 22f77c9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2016-08-05T20:50:18Z

 <artifactId>jetty-servlet</artifactId>
 <scope>compile</scope>
 </dependency>
+ <dependency>


These new dependencies need to be added to the copy-dependencies invocation later in this file.

…we are proxying

SparkQA · 2016-08-26T19:48:28Z

Test build #64490 has finished for PR 13950 at commit a524a8b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gurvindersingh · 2016-09-05T11:40:36Z

@ajbozarth @vanzin @JoshRosen it seems to be stalled, any update ?

vanzin · 2016-09-07T23:03:21Z

vanzin · 2016-09-07T23:14:35Z

A few minor things. Looks ok, but I'd still like from someone who's more active in maintaining standalone mode to take a look.

If there's no comments in a few days I'll push it after the comments are addressed.

zsxwing · 2016-09-08T00:39:45Z

Could you add a test for encoded string in the url, such as %2F, %F0%9F%98%84 (the 😄 emoji) ?

gurvindersingh · 2016-09-08T09:40:13Z

@vanzin addressed all your comments and added two extra checks too in test as requested. Let me know if there are any more comments.

SparkQA · 2016-09-08T11:52:14Z

Test build #65099 has finished for PR 13950 at commit 407c1a0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

zsxwing · 2016-09-08T18:53:15Z

nit: private[this]

zsxwing · 2016-09-08T18:54:16Z

LGTM expect one nit!

…nd restricting direct access to application UI

SparkQA · 2016-09-08T22:33:11Z

Test build #65112 has finished for PR 13950 at commit 9f6862e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

zsxwing · 2016-09-09T00:19:47Z

LGTM. Merging to master! Thanks!

prb reviewed Jun 28, 2016
View reviewed changes

tnachen reviewed Jul 6, 2016
View reviewed changes

vanzin reviewed Aug 5, 2016
View reviewed changes

Gurvinder Singh added 3 commits August 26, 2016 19:23

default to master public url, but if proxy url is given use that
c695f3d

fixed syntax and a typo
f435e4d

changed proxy url path from target to proxy to make it explicit that …
f7cec6c
…we are proxying

gurvindersingh force-pushed the rproxy branch from 86f3f84 to a524a8bCompare August 26, 2016 17:23

vanzin reviewed Sep 7, 2016
View reviewed changes

Comment threadcore/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala Outdated
vanzinSep 7, 2016
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val

zsxwing reviewed Sep 8, 2016
View reviewed changes

gurvindersingh force-pushed the rproxy branch from a524a8b to 407c1a0Compare September 8, 2016 09:38

zsxwing reviewed Sep 8, 2016
View reviewed changes

Comment threadcore/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala Outdated
zsxwingSep 8, 2016
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: private[this]

Gurvinder Singh added 4 commits September 8, 2016 22:16

fixed syntax and added test for proxyURI
f649e5d

added test for location header and fixed more syntax
128c4a6

added docs with warning about reverse proxy being global to cluster a…
0fd4477
…nd restricting direct access to application UI

print information about accessing UIs when running in proxy mode
9f6862e

gurvindersingh force-pushed the rproxy branch from 407c1a0 to 9f6862eCompare September 8, 2016 20:19

asfgit closed this in 92ce8d4Sep 9, 2016

gengliangwang mentioned this pull request Dec 15, 2021
[SPARK-34659] Fix that Web UI always correctly get appId #31774
Closed

Uh oh!

Conversation

gurvindersingh commented Jun 28, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

ajbozarth commented Jun 28, 2016

Uh oh!

prbJun 28, 2016

Choose a reason for hiding this comment

Uh oh!

gurvindersinghJun 28, 2016

Choose a reason for hiding this comment

Uh oh!

gurvindersingh commented Jul 1, 2016

Uh oh!

ajbozarth commented Jul 1, 2016

Uh oh!

tnachenJul 6, 2016

Choose a reason for hiding this comment

Uh oh!

tgravescs commented Jul 6, 2016

Uh oh!

gurvindersingh commented Jul 11, 2016

Uh oh!

gurvindersingh commented Jul 18, 2016

Uh oh!

ajbozarth commented Jul 19, 2016

Uh oh!

ajbozarth commented Jul 20, 2016

Uh oh!

gurvindersingh commented Jul 20, 2016

Uh oh!

gurvindersingh commented Jul 28, 2016

Uh oh!

ajbozarth commented Jul 28, 2016

Uh oh!

gurvindersingh commented Aug 3, 2016

Uh oh!

chandru-ops commented Aug 3, 2016

Uh oh!

ajbozarth commented Aug 3, 2016

Uh oh!

vanzin commented Aug 4, 2016

Uh oh!

SparkQA commented Aug 4, 2016

Uh oh!

vanzinAug 5, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 26, 2016

Uh oh!

gurvindersingh commented Sep 5, 2016

Uh oh!

vanzinSep 7, 2016

Choose a reason for hiding this comment

Uh oh!

vanzin commented Sep 7, 2016

Uh oh!

zsxwingSep 8, 2016

Choose a reason for hiding this comment

Uh oh!

gurvindersingh commented Sep 8, 2016

Uh oh!

SparkQA commented Sep 8, 2016

Uh oh!

zsxwingSep 8, 2016

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Sep 8, 2016

Uh oh!

SparkQA commented Sep 8, 2016

Uh oh!

zsxwing commented Sep 9, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone