[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-08 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/13950


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-08 Thread zsxwing
Github user zsxwing commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r78067335
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala ---
@@ -34,6 +38,7 @@ class MasterWebUI(
 
   val masterEndpointRef = master.self
   val killEnabled = master.conf.getBoolean("spark.ui.killEnabled", true)
+  val proxyHandlers = new HashMap[String, ServletContextHandler]
--- End diff --

nit: private[this]


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread zsxwing
Github user zsxwing commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77929992
  
--- Diff: core/src/test/scala/org/apache/spark/ui/UISuite.scala ---
@@ -190,6 +193,36 @@ class UISuite extends SparkFunSuite {
 }
   }
 
+  test("verify proxy rewrittenURI") {
--- End diff --

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77921327
  
--- Diff: docs/configuration.md ---
@@ -635,6 +635,20 @@ Apart from these, the following properties are also 
available, and may be useful
   
 
 
+  spark.ui.reverseProxy
+  false
+  
+Enable running Spark Master as reverse proxy for worker and 
application UI. In this mode, Spark master will reverse proxy the worker and 
application UIs to enable access without requiring direct access to their UIs. 
Use it with caution, as worker and application UI will not be accessible 
directly, you will only be able to access it through spark master/proxy public 
URL. This setting affects all the workers and application UIs running in the 
cluster and must be set on all the workers, drivers and master.
--- End diff --

everywhere: "UIs" (since you're saying "worker an application")

"...without requiring direct access to their hosts."

"...you will only be able to access them..."

"...workers, drivers and masters." (Yes there can be more than one.)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77921118
  
--- Diff: core/src/test/scala/org/apache/spark/ui/UISuite.scala ---
@@ -190,6 +193,36 @@ class UISuite extends SparkFunSuite {
 }
   }
 
+  test("verify proxy rewrittenURI") {
+val prefix = "/proxy/worker-id"
+val target = "http://localhost:8081;
+val path = "/proxy/worker-id/json"
+var rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
null)
+assert(rewrittenURI.toString().equals("http://localhost:8081/json;))
--- End diff --

use `===` instead of `equals`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77921124
  
--- Diff: core/src/test/scala/org/apache/spark/ui/UISuite.scala ---
@@ -190,6 +193,36 @@ class UISuite extends SparkFunSuite {
 }
   }
 
+  test("verify proxy rewrittenURI") {
+val prefix = "/proxy/worker-id"
+val target = "http://localhost:8081;
+val path = "/proxy/worker-id/json"
+var rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
null)
+assert(rewrittenURI.toString().equals("http://localhost:8081/json;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
"test=done")
+
assert(rewrittenURI.toString().equals("http://localhost:8081/json?test=done;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-id", null)
+assert(rewrittenURI.toString().equals("http://localhost:8081;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-noid/json", null)
+assert(rewrittenURI == null)
--- End diff --

`===`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77920947
  
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -510,4 +510,15 @@ private[spark] object UIUtils extends Logging {
 
   def getTimeZoneOffset() : Int =
 TimeZone.getDefault().getOffset(System.currentTimeMillis()) / 1000 / 60
+
+  /**
+  * Return the correct Href after checking if master is running in the
+  * reverse proxy mode or not.
+  */
+  def makeHref(proxy: Boolean, id: String, origHref: String): String = {
+if (proxy) {
--- End diff --

Similar to previous comment.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77920924
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -332,6 +375,47 @@ private[spark] object JettyUtils extends Logging {
 redirectHandler
   }
 
+  def createProxyURI(prefix: String, target: String, path: String, query: 
String): URI = {
+if (!path.startsWith(prefix)) {
+  return null
+}
+
+val uri = new StringBuilder(target)
+val rest = path.substring(prefix.length())
+
+if (!rest.isEmpty()) {
+  if (!rest.startsWith("/")) {
+uri.append("/")
+  }
+  uri.append(rest)
+}
+
+val rewrittenURI = URI.create(uri.toString())
+if (query != null) {
+  return new URI(
+  rewrittenURI.getScheme(),
+  rewrittenURI.getAuthority(),
+  rewrittenURI.getPath(),
+  query,
+  rewrittenURI.getFragment()
+).normalize()
+}
+rewrittenURI.normalize()
+  }
+
+  def createProxyLocationHeader(
+  prefix: String,
+  headerValue: String,
+  clientRequest: HttpServletRequest,
+  targetUri: URI): String = {
+val toReplace = targetUri.getScheme() + "://" + 
targetUri.getAuthority()
+if (headerValue.startsWith(toReplace)) {
+  return clientRequest.getScheme() + "://" + 
clientRequest.getHeader("host") +
--- End diff --

In these cases, instead of `return`, it's preferred to just do:

```
if (foo) {
  bar
} else {
  baz
}
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77920328
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -100,19 +105,21 @@ private[ui] class ApplicationPage(parent: 
MasterWebUI) extends WebUIPage("app")
   }
 
   private def executorRow(executor: ExecutorDesc): Seq[Node] = {
+var workerUrlRef = UIUtils.makeHref(parent.master.reverseProxy,
--- End diff --

val


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-09-07 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r77920399
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -77,7 +77,12 @@ private[ui] class ApplicationPage(parent: MasterWebUI) 
extends WebUIPage("app")
 State: {app.state}
 {
   if (!app.isFinished) {
-Application Detail 
UI
+
+  {
--- End diff --

Are the braces really necessary? (Here and in other parts of your change 
too.)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75186873
  
--- Diff: 
repl/scala-2.11/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -43,7 +43,11 @@ class SparkILoop(in0: Option[BufferedReader], out: 
JPrintWriter)
   }
 @transient val sc = {
   val _sc = spark.sparkContext
-  _sc.uiWebUrl.foreach(webUrl => println(s"Spark context Web UI 
available at ${webUrl}"))
+  if (_sc.getConf.getBoolean("spark.ui.reverseProxy", false)) {
+println(s"Spark Context Web UI is available at Spark Master 
Public URL")
+  } else {
+_sc.uiWebUrl.foreach(webUrl => println(s"Spark context Web UI 
available at ${webUrl}"))
--- End diff --

nit: `.foreach { webUrl => ... }`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75186435
  
--- Diff: docs/configuration.md ---
@@ -627,6 +627,20 @@ Apart from these, the following properties are also 
available, and may be useful
   
 
 
+  spark.ui.reverseProxy
+  false
+  
+To enable running Spark Master, worker and application UI behined a 
reverse proxy. In this mode, Spark master will reverse proxy the worker and 
application UIs to enable access. Use it with caution, as worker and 
application UI will not be accessible directly, you will only be able to access 
it through spark master/proxy public URL. This setting affects all the workers 
and application UIs running in the cluster.
--- End diff --

"Worker"
"behind"
"Master"
"Spark"


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75186503
  
--- Diff: docs/configuration.md ---
@@ -627,6 +627,20 @@ Apart from these, the following properties are also 
available, and may be useful
   
 
 
+  spark.ui.reverseProxy
+  false
+  
+To enable running Spark Master, worker and application UI behined a 
reverse proxy. In this mode, Spark master will reverse proxy the worker and 
application UIs to enable access. Use it with caution, as worker and 
application UI will not be accessible directly, you will only be able to access 
it through spark master/proxy public URL. This setting affects all the workers 
and application UIs running in the cluster.
+  
+
+
+  spark.ui.reverseProxyUrl
+  http://localhost:8080
--- End diff --

There is no default. The default is the master's address computed at 
runtime.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185921
  
--- Diff: core/src/test/scala/org/apache/spark/ui/UISuite.scala ---
@@ -190,6 +193,42 @@ class UISuite extends SparkFunSuite {
 }
   }
 
+  test("verify proxy rewrittenURI") {
+val prefix = "/proxy/worker-id"
+val target = "http://localhost:8081;
+val path = "/proxy/worker-id/json"
+var rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
null)
+assert(rewrittenURI.toString().equals("http://localhost:8081/json;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
"test=done")
+
assert(rewrittenURI.toString().equals("http://localhost:8081/json?test=done;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-id", null)
+assert(rewrittenURI.toString().equals("http://localhost:8081;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-noid/json", null)
+assert(rewrittenURI == null)
+  }
+
+  test("verify rewriting location header for reverse proxy") {
+val clientRequest = mock(classOf[HttpServletRequest])
+var headerValue = "http://localhost:4040/jobs;
+val prefix = "/proxy/worker-id"
+val targetUri = URI.create("http://localhost:4040;)
+when(clientRequest.getScheme()).thenReturn("http")
+when(clientRequest.getHeader("host")).thenReturn("localhost:8080")
+var newHeader = JettyUtils.createProxyLocationHeader(
+  prefix,
+  headerValue,
+  clientRequest,
+  targetUri)
+
assert(newHeader.toString().equals("http://localhost:8080/proxy/worker-id/jobs;))
+headerValue = "http://localhost:4041/jobs;
+newHeader = JettyUtils.createProxyLocationHeader(
+  prefix,
--- End diff --

indentation


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185903
  
--- Diff: core/src/test/scala/org/apache/spark/ui/UISuite.scala ---
@@ -190,6 +193,42 @@ class UISuite extends SparkFunSuite {
 }
   }
 
+  test("verify proxy rewrittenURI") {
+val prefix = "/proxy/worker-id"
+val target = "http://localhost:8081;
+val path = "/proxy/worker-id/json"
+var rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
null)
+assert(rewrittenURI.toString().equals("http://localhost:8081/json;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, path, 
"test=done")
+
assert(rewrittenURI.toString().equals("http://localhost:8081/json?test=done;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-id", null)
+assert(rewrittenURI.toString().equals("http://localhost:8081;))
+rewrittenURI = JettyUtils.createProxyURI(prefix, target, 
"/proxy/worker-noid/json", null)
+assert(rewrittenURI == null)
+  }
+
+  test("verify rewriting location header for reverse proxy") {
+val clientRequest = mock(classOf[HttpServletRequest])
+var headerValue = "http://localhost:4040/jobs;
+val prefix = "/proxy/worker-id"
+val targetUri = URI.create("http://localhost:4040;)
+when(clientRequest.getScheme()).thenReturn("http")
+when(clientRequest.getHeader("host")).thenReturn("localhost:8080")
+var newHeader = JettyUtils.createProxyLocationHeader(
+  prefix,
--- End diff --

indentation


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185483
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,50 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val rewrittenURI = createProxyURI(
+prefix,
+target,
+request.getRequestURI(),
+request.getQueryString())
+if (rewrittenURI == null) {
+  return null
+}
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+rewrittenURI.toString()
+  }
+
+  override def filterServerResponseHeader(
+  clientRequest: HttpServletRequest,
+  serverResponse: Response,
+  headerName: String,
+  headerValue: String): String = {
+if (headerName.equalsIgnoreCase("location")) {
+  val newHeader = createProxyLocationHeader(
+  prefix,
+  headerValue,
+  clientRequest,
+  serverResponse.getRequest().getURI())
+  if (newHeader != null) return newHeader
--- End diff --

```
if (...) {
   return ...
}
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185375
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,50 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val rewrittenURI = createProxyURI(
+prefix,
+target,
+request.getRequestURI(),
+request.getQueryString())
+if (rewrittenURI == null) {
+  return null
+}
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+rewrittenURI.toString()
+  }
+
+  override def filterServerResponseHeader(
+  clientRequest: HttpServletRequest,
+  serverResponse: Response,
+  headerName: String,
+  headerValue: String): String = {
+if (headerName.equalsIgnoreCase("location")) {
+  val newHeader = createProxyLocationHeader(
+  prefix,
--- End diff --

indentation


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185349
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,50 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val rewrittenURI = createProxyURI(
+prefix,
--- End diff --

indentation is wrong, should be 4 spaces (= 2 indent levels).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75185088
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala ---
@@ -176,7 +176,13 @@ private[ui] class MasterPage(parent: MasterWebUI) 
extends WebUIPage("") {
   private def workerRow(worker: WorkerInfo): Seq[Node] = {
 
   
-{worker.id}
+{
+  if (parent.master.reverseProxy) {
--- End diff --

This pattern is being used in a lot of places and it's kinda becoming 
noisy... perhaps a better approach would be to have a method somewhere that 
takes both the proxied and non-proxied versions, and returns which one to use. 
e.g.

def href(normal: String, proxied: String): String = if (reverseProxy) 
s"/proxy/$proxied" else normal



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-17 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r75183834
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -100,19 +108,24 @@ private[ui] class ApplicationPage(parent: 
MasterWebUI) extends WebUIPage("app")
   }
 
   private def executorRow(executor: ExecutorDesc): Seq[Node] = {
+var workerUrlRef = if (parent.master.reverseProxy) {
--- End diff --

Indentation here is odd.

```
val foo = if (...) {
  // code
} else {
  // code
}
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74840528
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala ---
@@ -157,6 +157,34 @@ class MasterSuite extends SparkFunSuite
 }
   }
 
+  test("master/worker web ui available with reverseProxy") {
+implicit val formats = org.json4s.DefaultFormats
+val reverseProxyUrl = "http://localhost:8080;
+val conf = new SparkConf()
+conf.set("spark.ui.reverseProxy", "true")
+conf.set("spark.ui.reverseProxyUrl", reverseProxyUrl)
+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")
+val JArray(workers) = (parse(json) \ "workers")
+workers.size should be (2)
+workers.foreach { workerSummaryJson =>
+  val JString(workerId) = workerSummaryJson \ "id"
+  val url = 
s"http://localhost:${localCluster.masterWebUIPort}/proxy/${workerId}/json;
+  val workerResponse = parse(Source.fromURL(url)
+.getLines().mkString("\n"))
--- End diff --

nit: seems like this fits in the previous line


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74839612
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -332,6 +378,34 @@ private[spark] object JettyUtils extends Logging {
 redirectHandler
   }
 
+  def createProxyURI (prefix: String, target: String, path: String, query: 
String): URI = {
--- End diff --

nit: no space before `(`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74838680
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,50 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val rewrittenURI = createProxyURI(
+  prefix,
+  target,
+  request.getRequestURI(),
+  request.getQueryString())
+if (rewrittenURI == null) {
+  return null
+}
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+rewrittenURI.toString();
--- End diff --

nit: no semi-colons.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74838557
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,50 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val rewrittenURI = createProxyURI(
+  prefix,
--- End diff --

nit: style. If arguments don't fit in one line, they should be indented two 
levels (= 4 spaces).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74837912
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala ---
@@ -48,6 +53,21 @@ class MasterWebUI(
 attachHandler(createRedirectHandler(
   "/driver/kill", "/", masterPage.handleDriverKillRequest, httpMethods 
= Set("POST")))
   }
+
+  def addProxyTargets(id: String, target: String): Unit = {
+var endTarget: String = target
+if (endTarget.endsWith("/")) {
+  endTarget = endTarget.substring(0, endTarget.length()-1)
+}
+val handler = createProxyHandler("/proxy/" + id, endTarget)
+attachHandler(handler)
+proxyHandlers(id) = handler
+  }
+
+  def removeProxyTargets(id: String): Unit = {
+detachHandler(proxyHandlers.get(id).get)
--- End diff --

`proxyHandlers.remove(id).foreach(detachHandler)`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74837226
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala ---
@@ -48,6 +53,21 @@ class MasterWebUI(
 attachHandler(createRedirectHandler(
   "/driver/kill", "/", masterPage.handleDriverKillRequest, httpMethods 
= Set("POST")))
   }
+
+  def addProxyTargets(id: String, target: String): Unit = {
+var endTarget: String = target
--- End diff --

`val endTarget = target.stripSuffix("/")`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74836512
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala ---
@@ -244,7 +254,12 @@ private[ui] class MasterPage(parent: MasterWebUI) 
extends WebUIPage("") {
 
   {driver.id} {killLink}
   {driver.submitDate}
-  {driver.worker.map(w => {w.id.toString}).getOrElse("None")}
+  {driver.worker.map(w =>
+if (parent.master.reverseProxy) {
+  {w.id.toString}
--- End diff --

nit: `toString` is unnecessary.

Also, interpolation would look better: `s"/proxy/${w.id}/"`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-15 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74835979
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -100,19 +108,21 @@ private[ui] class ApplicationPage(parent: 
MasterWebUI) extends WebUIPage("app")
   }
 
   private def executorRow(executor: ExecutorDesc): Seq[Node] = {
+var workerUrlRef = executor.worker.webUiAddress
--- End diff --

```
 val workerUrlRef = if (parent.master.reverseProxy) ... else ...
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-13 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r74690638
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
--- End diff --

added a test under UISuite, let me know if that is not the right place for 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73759385
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty()) {
+  if (!rest.startsWith("/")) {
+uri.append("/")
+  }
+  uri.append(rest)
+}
+
+val query = request.getQueryString()
+if (query != null) {
+  // Is there at least one path segment ?
+  val separator = "://"
+  if (uri.indexOf("/", uri.indexOf(separator) + 
separator.length()) < 0) {
+uri.append("/")
+  }
+  uri.append("?").append(query)
+}
+val rewrittenURI = URI.create(uri.toString()).normalize()
+
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+
+rewrittenURI.toString();
+  }
+  override def filterServerResponseHeader(
+clientRequest: HttpServletRequest,
+serverResponse: Response,
+headerName: String,
+headerValue: String): String = {
+if (headerName.equalsIgnoreCase("location")) {
--- End diff --

It would also be nice to have targeted unit tests for this code.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73759305
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
--- End diff --

+1 to targeted tests of string manipulation code.

Move the string manipulation part into its own method, and write targeted 
unit tests for 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73758964
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
--- End diff --

Seems like it would be better to normalize `target` and `prefix` before 
instantiating the servlet, so that they conform to whatever you expect, and 
avoid these extra checks on every invocation of the method.

e.g., make sure `target` does not end with a slash, and `prefix` does.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73758157
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty()) {
+  if (!rest.startsWith("/")) {
+uri.append("/")
+  }
+  uri.append(rest)
+}
+
+val query = request.getQueryString()
+if (query != null) {
+  // Is there at least one path segment ?
+  val separator = "://"
+  if (uri.indexOf("/", uri.indexOf(separator) + 
separator.length()) < 0) {
--- End diff --

Any reason not to use `java.net.URI` here instead of manual parsing code?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73758012
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty()) {
+  if (!rest.startsWith("/")) {
+uri.append("/")
+  }
+  uri.append(rest)
+}
+
+val query = request.getQueryString()
+if (query != null) {
+  // Is there at least one path segment ?
+  val separator = "://"
+  if (uri.indexOf("/", uri.indexOf(separator) + 
separator.length()) < 0) {
+uri.append("/")
+  }
+  uri.append("?").append(query)
+}
+val rewrittenURI = URI.create(uri.toString()).normalize()
+
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+
+rewrittenURI.toString();
+  }
+  override def filterServerResponseHeader(
+clientRequest: HttpServletRequest,
--- End diff --

style: multi-line argument lists need to be indented one extra level.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73757885
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
--- End diff --

nit: no semi-colons in scala code


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73757842
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,66 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty()) {
+  if (!rest.startsWith("/")) {
+uri.append("/")
+  }
+  uri.append(rest)
+}
+
+val query = request.getQueryString()
+if (query != null) {
+  // Is there at least one path segment ?
+  val separator = "://"
+  if (uri.indexOf("/", uri.indexOf(separator) + 
separator.length()) < 0) {
+uri.append("/")
+  }
+  uri.append("?").append(query)
+}
+val rewrittenURI = URI.create(uri.toString()).normalize()
+
+if (!validateDestination(rewrittenURI.getHost(), 
rewrittenURI.getPort())) {
+  return null
+}
+
+rewrittenURI.toString();
+  }
+  override def filterServerResponseHeader(
--- End diff --

nit: add empty line between methods


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73757480
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
@@ -753,6 +757,7 @@ private[deploy] class Master(
 workers += worker
 idToWorker(worker.id) = worker
 addressToWorker(workerAddress) = worker
+if (reverseProxy) webUi.addProxyTargets(worker.id, worker.webUiAddress)
--- End diff --

style: please use the "long form" of if statements (unless they're used as 
a ternary operator)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-08-05 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r73757257
  
--- Diff: core/pom.xml ---
@@ -127,6 +127,16 @@
 
 
   org.eclipse.jetty
+  jetty-proxy
--- End diff --

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-06 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69706266
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -502,6 +502,9 @@ class SparkContext(config: SparkConf) extends Logging 
with ExecutorAllocationCli
 _applicationId = _taskScheduler.applicationId()
 _applicationAttemptId = taskScheduler.applicationAttemptId()
 _conf.set("spark.app.id", _applicationId)
+if (_conf.getBoolean("spark.ui.reverseProxy", false)) {
+  System.setProperty("spark.ui.proxyBase", "/target/" + _applicationId)
--- End diff --

O, so what about target/proxy, target is fine or should I change it to 
proxy ?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-06 Thread tnachen
Github user tnachen commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69690304
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -502,6 +502,9 @@ class SparkContext(config: SparkConf) extends Logging 
with ExecutorAllocationCli
 _applicationId = _taskScheduler.applicationId()
 _applicationAttemptId = taskScheduler.applicationAttemptId()
 _conf.set("spark.app.id", _applicationId)
+if (_conf.getBoolean("spark.ui.reverseProxy", false)) {
+  System.setProperty("spark.ui.proxyBase", "/target/" + _applicationId)
--- End diff --

ah I see, sorry I thought it will be just opaque ids, but worker- and app- 
prefixes looks fine to me.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-06 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69688222
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -502,6 +502,9 @@ class SparkContext(config: SparkConf) extends Logging 
with ExecutorAllocationCli
 _applicationId = _taskScheduler.applicationId()
 _applicationAttemptId = taskScheduler.applicationAttemptId()
 _conf.set("spark.app.id", _applicationId)
+if (_conf.getBoolean("spark.ui.reverseProxy", false)) {
+  System.setProperty("spark.ui.proxyBase", "/target/" + _applicationId)
--- End diff --

I am not sure if it would have any benefit, as currently it will look like 
/target/worker-Id or /target/app-id so url does have information about whether 
it is application or worker. Regarding proxy or target name, I have no 
preference. As to me it does convey that we are accessing worker/app page. I 
can make it a configurable value if that's preferred.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-06 Thread tnachen
Github user tnachen commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69678895
  
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -502,6 +502,9 @@ class SparkContext(config: SparkConf) extends Logging 
with ExecutorAllocationCli
 _applicationId = _taskScheduler.applicationId()
 _applicationAttemptId = taskScheduler.applicationAttemptId()
 _conf.set("spark.app.id", _applicationId)
+if (_conf.getBoolean("spark.ui.reverseProxy", false)) {
+  System.setProperty("spark.ui.proxyBase", "/target/" + _applicationId)
--- End diff --

I wonder if this is better named /proxy/app/applicationId for applications, 
and /proxy/worker/workerId, so it's more clear what the destination target is?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-05 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69678617
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty())
+{
--- End diff --

Both are fixed 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-05 Thread tnachen
Github user tnachen commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69676891
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
+  prefix: String,
+  target: String): ServletContextHandler = {
+val servlet = new ProxyServlet {
+  override def rewriteTarget(request: HttpServletRequest): String = {
+val path = request.getRequestURI();
+if (!path.startsWith(prefix)) return null
+
+val uri = new StringBuilder(target)
+if (target.endsWith("/")) uri.setLength(uri.length() - 1)
+val rest = path.substring(prefix.length())
+if (!rest.isEmpty())
+{
--- End diff --

Move { to previous line 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-07-05 Thread tnachen
Github user tnachen commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r69676869
  
--- Diff: docs/configuration.md ---
@@ -598,6 +598,20 @@ Apart from these, the following properties are also 
available, and may be useful
   
 
 
+  spark.ui.reverseProxy
+  false
+  
+To enable running Spark Master, worker and application UI behined a 
reverse proxy. In this mode, Spark master will reverse proxy the worker and 
application UIs to enable access.
+  
+
+
+  spark.ui.reverseProxyUrl
+  http://localhost:8080
+  
+This is the URL where your proxy is running. Make sure this is a 
complete URL includeing scheme (http/https) and port to reach your proxy.
--- End diff --

includeing -> including


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68886506
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
@@ -127,7 +128,14 @@ 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
+if (reverseProxy) {
+  masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", null)
+  if (masterWebUiUrl == null) {
+   throw new SparkException("spark.ui.reverseProxyUrl must be 
provided")
--- End diff --

Updated the code now to remove the exception and use public address as 
default and if reverseproxyURL is given then override it. Should solve the 
issue you seeing.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68886448
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
@@ -127,7 +128,14 @@ 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
+if (reverseProxy) {
+  masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", null)
--- End diff --

It is used in case when you are running spark master itself behind a proxy 
e.g. Oauth2 to provide authentication/authorization. Its to make sure "Back to 
Master"  link works when you are on workers UI.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread ajbozarth
Github user ajbozarth commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68842668
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
@@ -127,7 +128,14 @@ 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
+if (reverseProxy) {
+  masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", null)
+  if (masterWebUiUrl == null) {
+   throw new SparkException("spark.ui.reverseProxyUrl must be 
provided")
--- End diff --

When this is thrown the Spark Master still starts, but workings and 
applications can't connect to 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread ajbozarth
Github user ajbozarth commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68842364
  
--- Diff: core/src/main/scala/org/apache/spark/deploy/master/Master.scala 
---
@@ -127,7 +128,14 @@ 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
+if (reverseProxy) {
+  masterWebUiUrl = conf.get("spark.ui.reverseProxyUrl", null)
--- End diff --

Is there a reason we need this config? Would there ever be a case where the 
proxy URL is different from the master URL? Also from my testing this config is 
completely ignored (as long as it exists)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68826828
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
--- End diff --

I might have overlooked the tests for other handlers, can you point me to 
them if there is any. I can add test for this handler too then.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread gurvindersingh
Github user gurvindersingh commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68826228
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -77,7 +77,15 @@ private[ui] class ApplicationPage(parent: MasterWebUI) 
extends WebUIPage("app")
 State: {app.state}
 {
   if (!app.isFinished) {
-Application Detail 
UI
+
+  {
+if (parent.master.reverseProxy) {
+  Application 
Detail UI
--- End diff --

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.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread prb
Github user prb commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68825733
  
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -186,6 +188,67 @@ private[spark] object JettyUtils extends Logging {
 contextHandler
   }
 
+  /** Create a handler for proxying request to Workers and Application 
Drivers */
+  def createProxyHandler(
--- End diff --

Always a little scary to see string manipulation code with no tests.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread prb
Github user prb commented on a diff in the pull request:

https://github.com/apache/spark/pull/13950#discussion_r68824409
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala ---
@@ -77,7 +77,15 @@ private[ui] class ApplicationPage(parent: MasterWebUI) 
extends WebUIPage("app")
 State: {app.state}
 {
   if (!app.isFinished) {
-Application Detail 
UI
+
+  {
+if (parent.master.reverseProxy) {
+  Application 
Detail UI
--- End diff --

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #13950: [SPARK-15487] [Web UI] Spark Master UI to reverse...

2016-06-28 Thread gurvindersingh
GitHub user gurvindersingh opened a pull request:

https://github.com/apache/spark/pull/13950

[SPARK-15487] [Web UI] Spark Master UI to reverse proxy Application and 
Workers UI

## 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: ://master-publicIP:/target/workerID/
ApplicationUI: ://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.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/gurvindersingh/spark rproxy

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/13950.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 #13950


commit 19c8dc6fdc2d1de11cedb7e706a7ce1c04f2f06a
Author: Gurvinder Singh 
Date:   2016-05-26T21:14:05Z

added support for reverse proxying request to workers and application UI. 
This helps in accessing SparkUI when running spark cluster in closed networks.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org