This is an automated email from the ASF dual-hosted git repository.
srowen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new c324e1d [SPARK-27122][CORE] Jetty classes must not be return via
getters in org.apache.spark.ui.WebUI
c324e1d is described below
commit c324e1da9d0519fc13220202136307d4547b7426
Author: Ajith <ajith2...@gmail.com>
AuthorDate: Sun Mar 17 06:44:02 2019 -0500
[SPARK-27122][CORE] Jetty classes must not be return via getters in
org.apache.spark.ui.WebUI
## What changes were proposed in this pull request?
When we run YarnSchedulerBackendSuite, the class path seems to be made from
the classes folder(resource-managers/yarn/target/scala-2.12/classes) instead of
jar (resource-managers/yarn/target/spark-yarn_2.12-3.0.0-SNAPSHOT.jar) .
ui.getHandlers is in spark-core and its loaded from spark-core.jar which is
shaded and hence refers to org.spark_project.jetty.servlet.ServletContextHandler
Here in org.apache.spark.scheduler.cluster.YarnSchedulerBackend, as its
not shaded, it expects org.eclipse.jetty.servlet.ServletContextHandler
Refer discussion
https://issues.apache.org/jira/browse/SPARK-27122?focusedCommentId=16792318&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16792318
Hence as a fix, org.apache.spark.ui.WebUI must only return a wrapper class
instance or references so that Jetty classes can be avoided in getters which
are accessed outside spark-core
## How was this patch tested?
Existing UT can pass
Closes #24088 from ajithme/shadebug.
Authored-by: Ajith <ajith2...@gmail.com>
Signed-off-by: Sean Owen <sean.o...@databricks.com>
---
.../src/main/scala/org/apache/spark/ui/WebUI.scala | 47 +++++++++++++++++++++-
.../scheduler/cluster/YarnSchedulerBackend.scala | 16 ++------
.../cluster/YarnSchedulerBackendSuite.scala | 10 ++---
3 files changed, 51 insertions(+), 22 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/ui/WebUI.scala
b/core/src/main/scala/org/apache/spark/ui/WebUI.scala
index 8165942..54ae258 100644
--- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala
+++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala
@@ -17,13 +17,15 @@
package org.apache.spark.ui
-import javax.servlet.http.HttpServletRequest
+import java.util.EnumSet
+import javax.servlet.DispatcherType
+import javax.servlet.http.{HttpServlet, HttpServletRequest}
import scala.collection.mutable.ArrayBuffer
import scala.collection.mutable.HashMap
import scala.xml.Node
-import org.eclipse.jetty.servlet.ServletContextHandler
+import org.eclipse.jetty.servlet.{FilterHolder, FilterMapping,
ServletContextHandler, ServletHolder}
import org.json4s.JsonAST.{JNothing, JValue}
import org.apache.spark.{SecurityManager, SparkConf, SSLOptions}
@@ -59,6 +61,10 @@ private[spark] abstract class WebUI(
def getTabs: Seq[WebUITab] = tabs
def getHandlers: Seq[ServletContextHandler] = handlers
+ def getDelegatingHandlers: Seq[DelegatingServletContextHandler] = {
+ handlers.map(new DelegatingServletContextHandler(_))
+ }
+
/** Attaches a tab to this UI, along with all of its attached pages. */
def attachTab(tab: WebUITab): Unit = {
tab.pages.foreach(attachPage)
@@ -95,6 +101,14 @@ private[spark] abstract class WebUI(
serverInfo.foreach(_.addHandler(handler, securityManager))
}
+ /** Attaches a handler to this UI. */
+ def attachHandler(contextPath: String, httpServlet: HttpServlet, pathSpec:
String): Unit = {
+ val ctx = new ServletContextHandler()
+ ctx.setContextPath(contextPath)
+ ctx.addServlet(new ServletHolder(httpServlet), pathSpec)
+ attachHandler(ctx)
+ }
+
/** Detaches a handler from this UI. */
def detachHandler(handler: ServletContextHandler): Unit = synchronized {
handlers -= handler
@@ -193,3 +207,32 @@ private[spark] abstract class WebUIPage(var prefix:
String) {
def render(request: HttpServletRequest): Seq[Node]
def renderJson(request: HttpServletRequest): JValue = JNothing
}
+
+private[spark] class DelegatingServletContextHandler(handler:
ServletContextHandler) {
+
+ def prependFilterMapping(
+ filterName: String,
+ spec: String,
+ types: EnumSet[DispatcherType]): Unit = {
+ val mapping = new FilterMapping()
+ mapping.setFilterName(filterName)
+ mapping.setPathSpec(spec)
+ mapping.setDispatcherTypes(types)
+ handler.getServletHandler.prependFilterMapping(mapping)
+ }
+
+ def addFilter(
+ filterName: String,
+ className: String,
+ filterParams: Map[String, String]): Unit = {
+ val filterHolder = new FilterHolder()
+ filterHolder.setName(filterName)
+ filterHolder.setClassName(className)
+ filterParams.foreach { case (k, v) => filterHolder.setInitParameter(k, v) }
+ handler.getServletHandler.addFilter(filterHolder)
+ }
+
+ def filterCount(): Int = {
+ handler.getServletHandler.getFilters.length
+ }
+}
diff --git
a/resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
b/resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
index a8472b4..dda8172 100644
---
a/resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
+++
b/resource-managers/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackend.scala
@@ -180,19 +180,9 @@ private[spark] abstract class YarnSchedulerBackend(
}
conf.set(UI_FILTERS, allFilters)
- ui.getHandlers.map(_.getServletHandler()).foreach { h =>
- val holder = new FilterHolder()
- holder.setName(filterName)
- holder.setClassName(filterName)
- filterParams.foreach { case (k, v) => holder.setInitParameter(k,
v) }
- h.addFilter(holder)
-
- val mapping = new FilterMapping()
- mapping.setFilterName(filterName)
- mapping.setPathSpec("/*")
- mapping.setDispatcherTypes(EnumSet.allOf(classOf[DispatcherType]))
-
- h.prependFilterMapping(mapping)
+ ui.getDelegatingHandlers.foreach { h =>
+ h.addFilter(filterName, filterName, filterParams)
+ h.prependFilterMapping(filterName, "/*",
EnumSet.allOf(classOf[DispatcherType]))
}
}
}
diff --git
a/resource-managers/yarn/src/test/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackendSuite.scala
b/resource-managers/yarn/src/test/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackendSuite.scala
index 5836944..70f86aa 100644
---
a/resource-managers/yarn/src/test/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackendSuite.scala
+++
b/resource-managers/yarn/src/test/scala/org/apache/spark/scheduler/cluster/YarnSchedulerBackendSuite.scala
@@ -101,9 +101,9 @@ class YarnSchedulerBackendSuite extends SparkFunSuite with
MockitoSugar with Loc
yarnSchedulerBackend.addWebUIFilter(classOf[TestFilter2].getName(),
Map("responseCode" -> HttpServletResponse.SC_NOT_ACCEPTABLE.toString),
"")
- sc.ui.get.getHandlers.foreach { h =>
+ sc.ui.get.getDelegatingHandlers.foreach { h =>
// Two filters above + security filter.
- assert(h.getServletHandler().getFilters().length === 3)
+ assert(h.filterCount() === 3)
}
// The filter should have been added first in the chain, so we should get
SC_NOT_ACCEPTABLE
@@ -117,11 +117,7 @@ class YarnSchedulerBackendSuite extends SparkFunSuite with
MockitoSugar with Loc
}
}
- val ctx = new ServletContextHandler()
- ctx.setContextPath("/new-handler")
- ctx.addServlet(new ServletHolder(servlet), "/")
-
- sc.ui.get.attachHandler(ctx)
+ sc.ui.get.attachHandler("/new-handler", servlet, "/")
val newUrl = new URL(sc.uiWebUrl.get + "/new-handler/")
assert(TestUtils.httpResponseCode(newUrl) ===
HttpServletResponse.SC_NOT_ACCEPTABLE)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
