From 631e991fc3a82eec1a4a6eade735064b1345f6c5 Mon Sep 17 00:00:00 2001 From: Ajith Date: Thu, 14 Mar 2019 12:49:31 +0530 Subject: [PATCH 1/4] avoid exposi shaded classes outside spark-core --- .../scala/org/apache/spark/ui/WebUI.scala | 47 ++++++++++++++++++- .../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 81659426792c..0522a0fb2a73 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(handler => new DelegatingServletContextHandler(handler)) + } + /** 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,16 @@ private[spark] abstract class WebUI( serverInfo.foreach(_.addHandler(handler, securityManager)) } + /** Attaches a handler to this UI. */ + def attachNewHandler(contextPath: String, + httpServlet: HttpServlet, + pathSpec: String): Unit = synchronized { + val ctx = new ServletContextHandler() + ctx.setContextPath("/new-handler") + ctx.addServlet(new ServletHolder(httpServlet), "/") + attachHandler(ctx) + } + /** Detaches a handler from this UI. */ def detachHandler(handler: ServletContextHandler): Unit = synchronized { handlers -= handler @@ -193,3 +209,30 @@ private[spark] abstract class WebUIPage(var prefix: String) { def render(request: HttpServletRequest): Seq[Node] def renderJson(request: HttpServletRequest): JValue = JNothing } + +private[spark] class DelegatingServletContextHandler(servletContextHandler: ServletContextHandler) { + + def prependFilterMapping(filterName: String, + spec: String, + types: EnumSet[DispatcherType]): Unit = { + val mapping = new FilterMapping() + mapping.setFilterName(filterName) + mapping.setPathSpec(spec) + mapping.setDispatcherTypes(EnumSet.allOf(classOf[DispatcherType])) + servletContextHandler.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) } + servletContextHandler.getServletHandler.addFilter(filterHolder) + } + + def filterSize(): Int = { + servletContextHandler.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 a8472b49ae27..dda8172fb636 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 583694412322..009997838ffc 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.filterSize() === 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.attachNewHandler("/new-handler", servlet, "/") val newUrl = new URL(sc.uiWebUrl.get + "/new-handler/") assert(TestUtils.httpResponseCode(newUrl) === HttpServletResponse.SC_NOT_ACCEPTABLE) From 20e17b2085ed1dd04caa23b3d4ee174e66adbf55 Mon Sep 17 00:00:00 2001 From: Ajith Date: Fri, 15 Mar 2019 06:39:26 +0530 Subject: [PATCH 2/4] review comments --- .../scala/org/apache/spark/ui/WebUI.scala | 38 +++++++++---------- .../cluster/YarnSchedulerBackendSuite.scala | 4 +- 2 files changed, 21 insertions(+), 21 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 0522a0fb2a73..c5973923b605 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -62,7 +62,7 @@ private[spark] abstract class WebUI( def getHandlers: Seq[ServletContextHandler] = handlers def getDelegatingHandlers: Seq[DelegatingServletContextHandler] = { - handlers.map(handler => new DelegatingServletContextHandler(handler)) + handlers.map(new DelegatingServletContextHandler(_)) } /** Attaches a tab to this UI, along with all of its attached pages. */ @@ -102,12 +102,12 @@ private[spark] abstract class WebUI( } /** Attaches a handler to this UI. */ - def attachNewHandler(contextPath: String, - httpServlet: HttpServlet, - pathSpec: String): Unit = synchronized { - val ctx = new ServletContextHandler() - ctx.setContextPath("/new-handler") - ctx.addServlet(new ServletHolder(httpServlet), "/") + def attachHandler(contextPath: String, + httpServlet: HttpServlet, + pathSpec: String): Unit = synchronized { + val ctx = new ServletContextHandler + ctx.setContextPath(contextPath) + ctx.addServlet(new ServletHolder(httpServlet), pathSpec) attachHandler(ctx) } @@ -210,29 +210,29 @@ private[spark] abstract class WebUIPage(var prefix: String) { def renderJson(request: HttpServletRequest): JValue = JNothing } -private[spark] class DelegatingServletContextHandler(servletContextHandler: ServletContextHandler) { +private[spark] class DelegatingServletContextHandler(handler: ServletContextHandler) { def prependFilterMapping(filterName: String, - spec: String, - types: EnumSet[DispatcherType]): Unit = { - val mapping = new FilterMapping() + spec: String, + types: EnumSet[DispatcherType]): Unit = { + val mapping = new FilterMapping mapping.setFilterName(filterName) mapping.setPathSpec(spec) - mapping.setDispatcherTypes(EnumSet.allOf(classOf[DispatcherType])) - servletContextHandler.getServletHandler.prependFilterMapping(mapping) + mapping.setDispatcherTypes(types) + handler.getServletHandler.prependFilterMapping(mapping) } def addFilter(filterName: String, - className: String, - filterParams: Map[String, String]): Unit = { - val filterHolder = new FilterHolder() + 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) } - servletContextHandler.getServletHandler.addFilter(filterHolder) + handler.getServletHandler.addFilter(filterHolder) } - def filterSize(): Int = { - servletContextHandler.getServletHandler.getFilters.length + def filterCount(): Int = { + handler.getServletHandler.getFilters.length } } 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 009997838ffc..70f86aaa72f6 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 @@ -103,7 +103,7 @@ class YarnSchedulerBackendSuite extends SparkFunSuite with MockitoSugar with Loc sc.ui.get.getDelegatingHandlers.foreach { h => // Two filters above + security filter. - assert(h.filterSize() === 3) + assert(h.filterCount() === 3) } // The filter should have been added first in the chain, so we should get SC_NOT_ACCEPTABLE @@ -117,7 +117,7 @@ class YarnSchedulerBackendSuite extends SparkFunSuite with MockitoSugar with Loc } } - sc.ui.get.attachNewHandler("/new-handler", servlet, "/") + sc.ui.get.attachHandler("/new-handler", servlet, "/") val newUrl = new URL(sc.uiWebUrl.get + "/new-handler/") assert(TestUtils.httpResponseCode(newUrl) === HttpServletResponse.SC_NOT_ACCEPTABLE) From aab4a9a26f8094675f2d61a71d55c106d509c475 Mon Sep 17 00:00:00 2001 From: Ajith Date: Fri, 15 Mar 2019 14:51:30 +0530 Subject: [PATCH 3/4] update as per style --- .../scala/org/apache/spark/ui/WebUI.scala | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 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 c5973923b605..612d45e9a1fa 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -102,14 +102,13 @@ private[spark] abstract class WebUI( } /** Attaches a handler to this UI. */ - def attachHandler(contextPath: String, - httpServlet: HttpServlet, - pathSpec: String): Unit = synchronized { - val ctx = new ServletContextHandler - ctx.setContextPath(contextPath) - ctx.addServlet(new ServletHolder(httpServlet), pathSpec) - attachHandler(ctx) - } + def attachHandler(contextPath: String, httpServlet: HttpServlet, pathSpec: String): Unit = + synchronized { + 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 { @@ -212,9 +211,10 @@ private[spark] abstract class WebUIPage(var prefix: String) { private[spark] class DelegatingServletContextHandler(handler: ServletContextHandler) { - def prependFilterMapping(filterName: String, - spec: String, - types: EnumSet[DispatcherType]): Unit = { + def prependFilterMapping( + filterName: String, + spec: String, + types: EnumSet[DispatcherType]): Unit = { val mapping = new FilterMapping mapping.setFilterName(filterName) mapping.setPathSpec(spec) @@ -222,9 +222,10 @@ private[spark] class DelegatingServletContextHandler(handler: ServletContextHand handler.getServletHandler.prependFilterMapping(mapping) } - def addFilter(filterName: String, - className: String, - filterParams: Map[String, String]): Unit = { + def addFilter( + filterName: String, + className: String, + filterParams: Map[String, String]): Unit = { val filterHolder = new FilterHolder filterHolder.setName(filterName) filterHolder.setClassName(className) From 3f4b83153ed7a1299fb4adc154aede10d8300721 Mon Sep 17 00:00:00 2001 From: Ajith Date: Fri, 15 Mar 2019 16:08:12 +0530 Subject: [PATCH 4/4] update review comments --- .../main/scala/org/apache/spark/ui/WebUI.scala | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 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 612d45e9a1fa..54ae258ba565 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -102,13 +102,12 @@ private[spark] abstract class WebUI( } /** Attaches a handler to this UI. */ - def attachHandler(contextPath: String, httpServlet: HttpServlet, pathSpec: String): Unit = - synchronized { - val ctx = new ServletContextHandler - ctx.setContextPath(contextPath) - ctx.addServlet(new ServletHolder(httpServlet), pathSpec) - attachHandler(ctx) - } + 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 { @@ -215,7 +214,7 @@ private[spark] class DelegatingServletContextHandler(handler: ServletContextHand filterName: String, spec: String, types: EnumSet[DispatcherType]): Unit = { - val mapping = new FilterMapping + val mapping = new FilterMapping() mapping.setFilterName(filterName) mapping.setPathSpec(spec) mapping.setDispatcherTypes(types) @@ -226,7 +225,7 @@ private[spark] class DelegatingServletContextHandler(handler: ServletContextHand filterName: String, className: String, filterParams: Map[String, String]): Unit = { - val filterHolder = new FilterHolder + val filterHolder = new FilterHolder() filterHolder.setName(filterName) filterHolder.setClassName(className) filterParams.foreach { case (k, v) => filterHolder.setInitParameter(k, v) }