(spark) branch branch-3.4 updated: [SPARK-50022][CORE][UI][3.4] Fix `MasterPage` to hide App UI links when UI is disabled

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-3.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.4 by this push:
     new 534ad5266dd9 [SPARK-50022][CORE][UI][3.4] Fix `MasterPage` to hide App UI links when UI is disabled
534ad5266dd9 is described below

commit 534ad5266dd9607f503ee0e41c3bfee16387aacd
Author: Dongjoon Hyun <do...@apache.org>
AuthorDate: Fri Oct 18 14:25:44 2024 -0700

    [SPARK-50022][CORE][UI][3.4] Fix `MasterPage` to hide App UI links when UI is disabled
    
    ### What changes were proposed in this pull request?
    
    This PR aims to fix `MasterPage` to hide App UI links when UI is disabled.
    
    Previously, the link leads the following errors if a user clicks it.
    <img width="997" alt="Screenshot 2024-10-17 at 22 06 22" src="https://github.com/user-attachments/assets/e53ba01f-533f-4d42-a488-830afaf40efa">
    
    ### Why are the changes needed?
    
    **1. PREPARATION**
    
    ```
    $ cat conf/spark-defaults.conf
    spark.ui.reverseProxy true
    spark.ui.reverseProxyUrl http://localhost:8080
    
    $ sbin/start-master.sh
    
    $ sbin/start-worker.sh spark://$(hostname):7077
    
    $ bin/spark-shell --master spark://$(hostname):7077 -c spark.ui.enabled=false
    ```
    
    **2. BEFORE**
    
    <img width="340" alt="Screenshot 2024-10-17 at 22 01 16" src="https://github.com/user-attachments/assets/3069e43d-ba8c-4d36-8101-65e10b420879">
    
    **3. AFTER**
    
    <img width="345" alt="Screenshot 2024-10-17 at 22 04 12" src="https://github.com/user-attachments/assets/b9feba47-90fb-4557-803c-94eaa8ce62e1">
    
    ### Does this PR introduce _any_ user-facing change?
    
    The previous behavior shows HTTP 500 error.
    
    ### How was this patch tested?
    
    Pass the CIs with a newly added test case.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    No.
    
    Closes #48550 from dongjoon-hyun/SPARK-50022-3.4.
    
    Authored-by: Dongjoon Hyun <do...@apache.org>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../apache/spark/deploy/master/ui/MasterPage.scala |  3 +-
 .../spark/deploy/master/ui/MasterWebUISuite.scala  |  5 +-
 .../master/ui/ReadOnlyMasterWebUISuite.scala       | 82 ++++++++++++++++++++++
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
index d8753a0ffcb5..078ed102f0bb 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala
@@ -21,6 +21,7 @@ import javax.servlet.http.HttpServletRequest
 
 import scala.xml.Node
 
+import org.apache.commons.lang3.StringUtils
 import org.json4s.JValue
 
 import org.apache.spark.deploy.DeployMessages.{KillDriverResponse, MasterStateResponse, RequestKillDriver, RequestMasterState}
@@ -289,7 +290,7 @@ private[ui] class MasterPage(parent: MasterWebUI) extends WebUIPage("") {
       </td>
       <td>
         {
-          if (app.isFinished) {
+          if (app.isFinished || StringUtils.isBlank(app.desc.appUiUrl)) {
             app.desc.name
           } else {
             <a href={UIUtils.makeHref(parent.master.reverseProxy,
diff --git a/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
index 40265a12af93..bda3309ad820 100644
--- a/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
@@ -35,6 +35,7 @@ import org.apache.spark.rpc.{RpcEndpointRef, RpcEnv}
 import org.apache.spark.util.Utils
 
 class MasterWebUISuite extends SparkFunSuite {
+  import MasterWebUISuite._
 
   val conf = new SparkConf().set(DECOMMISSION_ENABLED, true)
   val securityMgr = new SecurityManager(conf)
@@ -113,12 +114,14 @@ class MasterWebUISuite extends SparkFunSuite {
   private def convPostDataToString(data: Map[String, String]): String = {
     convPostDataToString(data.toSeq)
   }
+}
 
+object MasterWebUISuite {
   /**
    * Send an HTTP request to the given URL using the method and the body specified.
    * Return the connection object.
    */
-  private def sendHttpRequest(
+  private[ui] def sendHttpRequest(
       url: String,
       method: String,
       body: String = ""): HttpURLConnection = {
diff --git a/core/src/test/scala/org/apache/spark/deploy/master/ui/ReadOnlyMasterWebUISuite.scala b/core/src/test/scala/org/apache/spark/deploy/master/ui/ReadOnlyMasterWebUISuite.scala
new file mode 100644
index 000000000000..9fd5431418aa
--- /dev/null
+++ b/core/src/test/scala/org/apache/spark/deploy/master/ui/ReadOnlyMasterWebUISuite.scala
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy.master.ui
+
+import java.util.Date
+import javax.servlet.http.HttpServletResponse.SC_OK
+
+import scala.io.Source
+
+import org.mockito.Mockito.{mock, when}
+
+import org.apache.spark.{SecurityManager, SparkConf, SparkFunSuite}
+import org.apache.spark.deploy.DeployMessages.{MasterStateResponse, RequestMasterState}
+import org.apache.spark.deploy.master._
+import org.apache.spark.deploy.master.ui.MasterWebUISuite._
+import org.apache.spark.internal.config.DECOMMISSION_ENABLED
+import org.apache.spark.internal.config.UI.UI_KILL_ENABLED
+import org.apache.spark.rpc.{RpcEndpointRef, RpcEnv}
+import org.apache.spark.util.Utils
+
+class ReadOnlyMasterWebUISuite extends SparkFunSuite {
+
+  import org.apache.spark.deploy.DeployTestUtils._
+
+  val conf = new SparkConf()
+    .set(UI_KILL_ENABLED, false)
+    .set(DECOMMISSION_ENABLED, false)
+  val securityMgr = new SecurityManager(conf)
+  val rpcEnv = mock(classOf[RpcEnv])
+  val master = mock(classOf[Master])
+  val masterEndpointRef = mock(classOf[RpcEndpointRef])
+  when(master.securityMgr).thenReturn(securityMgr)
+  when(master.conf).thenReturn(conf)
+  when(master.rpcEnv).thenReturn(rpcEnv)
+  when(master.self).thenReturn(masterEndpointRef)
+  val desc1 = createAppDesc().copy(name = "WithUI")
+  val desc2 = desc1.copy(name = "WithoutUI", appUiUrl = "")
+  val app1 = new ApplicationInfo(new Date().getTime, "app1", desc1, new Date(), null, Int.MaxValue)
+  val app2 = new ApplicationInfo(new Date().getTime, "app2", desc2, new Date(), null, Int.MaxValue)
+  val state = new MasterStateResponse(
+    "host", 8080, None, Array.empty, Array(app1, app2), Array.empty,
+    Array.empty, Array.empty, RecoveryState.ALIVE)
+  when(masterEndpointRef.askSync[MasterStateResponse](RequestMasterState)).thenReturn(state)
+  val masterWebUI = new MasterWebUI(master, 0)
+
+  override def beforeAll(): Unit = {
+    super.beforeAll()
+    masterWebUI.bind()
+  }
+
+  override def afterAll(): Unit = {
+    try {
+      masterWebUI.stop()
+    } finally {
+      super.afterAll()
+    }
+  }
+
+  test("SPARK-50022: Fix 'MasterPage' to hide App UI links when UI is disabled") {
+    val url = s"http://${Utils.localHostNameForURI()}:${masterWebUI.boundPort}/"
+    val conn = sendHttpRequest(url, "GET")
+    assert(conn.getResponseCode === SC_OK)
+    val result = Source.fromInputStream(conn.getInputStream).mkString
+    assert(result.contains("<a href=\"appUiUrl\">WithUI</a>"))
+    assert(result.contains("  WithoutUI\n"))
+  }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org