You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2019/09/05 22:53:45 UTC

[impala] 02/02: IMPALA-8897 (part 2): Fix javascript to work with Knox integration

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

tmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit fdde66987b4c05aed1f0d848c6f408110a3fcdcc
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Fri Aug 30 10:26:38 2019 -0700

    IMPALA-8897 (part 2): Fix javascript to work with Knox integration
    
    This patch qualifies all urls accessed via javascript in the webui
    with the appropriate host:port in order to allow these urls to work
    when the connection to the webui is being proxied through Apache Knox.
    
    It accomplishes this with the function make_url(), which takes the
    href of a link from the page, which may have been rewritten by Knox,
    and appends a path to it.
    
    This patch also fixes on issue on the /admissions page, where
    resetting a pool's stats could fail due to the page being reloaded
    before the reset is executed. Fixed by moving the call to reload to
    the completion callback for the 'reset' endpoint.
    
    Testing:
    - Added a regex to test_knox_compatability that performs a rough check
      for places in the tmpl files where urls are used in javascript
      without calling make_url().
    
    Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581
    Reviewed-on: http://gerrit.cloudera.org:8080/14173
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 tests/webserver/test_web_pages.py |  7 +++++--
 www/admission_controller.tmpl     | 12 ++++++++----
 www/common-header.tmpl            | 20 +++++++++++++++++++-
 www/query_plan.tmpl               |  2 +-
 www/query_summary.tmpl            |  2 +-
 www/rpcz.tmpl                     |  8 ++++----
 6 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/tests/webserver/test_web_pages.py b/tests/webserver/test_web_pages.py
index beca9f5..f3c8545 100644
--- a/tests/webserver/test_web_pages.py
+++ b/tests/webserver/test_web_pages.py
@@ -21,6 +21,7 @@ from tests.common.skip import SkipIfBuildType
 from tests.common.impala_cluster import ImpalaCluster
 from tests.common.impala_test_suite import ImpalaTestSuite
 import json
+import os
 import pytest
 import re
 import requests
@@ -667,7 +668,9 @@ class TestWebPage(ImpalaTestSuite):
     href_regex = "<a .*? href=['\"](?!({{ __common__.host-url }})|#)"
     # Matches all 'form' tags that are not followed by including the hidden inputs.
     form_regex = "<form [^{]*?>(?!{{>www/form-hidden-inputs.tmpl}})"
-    regex = "(%s)|(%s)" % (href_regex, form_regex)
-    results = grep_dir("/home/thomas/Impala/www", regex, ".*\.tmpl")
+    # Matches XMLHttpRequest.open() in javascript that are not followed with make_url().
+    javascript_regex = "open\(['\"]GET['\"], (?!make_url)"
+    regex = "(%s)|(%s)|(%s)" % (href_regex, form_regex, javascript_regex)
+    results = grep_dir(os.path.join(os.environ['IMPALA_HOME'], "www"), regex, ".*\.tmpl")
     assert len(results) == 0, \
         "All links on the webui must include the webserver host: %s" % results
diff --git a/www/admission_controller.tmpl b/www/admission_controller.tmpl
index cf72dc6..848d4ed 100644
--- a/www/admission_controller.tmpl
+++ b/www/admission_controller.tmpl
@@ -180,17 +180,21 @@ function formatMemory(val) {
 function reset_all() {
   if (!confirm('Are you sure you want to reset stats for all resource pools ?')) return;
   var xhr = new XMLHttpRequest();
-  xhr.open('GET', "/resource_pool_reset", true);
+  xhr.onload = function(ignored_arg) {
+    window.location.reload(true);
+  }
+  xhr.open('GET', make_url("/resource_pool_reset"), true);
   xhr.send();
-  window.location.reload(true);
 }
 
 function reset_method(pool_name) {
   if (!confirm('Are you sure you want to reset stats for ' + pool_name +' ?')) return;
   var xhr = new XMLHttpRequest();
-  xhr.open('GET', "/resource_pool_reset?pool_name=" + pool_name, true);
+  xhr.onload = function(ignored_arg) {
+    window.location.reload(true);
+  }
+  xhr.open('GET', make_url("/resource_pool_reset?pool_name=" + pool_name), true);
   xhr.send();
-  window.location.reload(true);
 }
 </script>
 
diff --git a/www/common-header.tmpl b/www/common-header.tmpl
index eb15343..00287e7 100644
--- a/www/common-header.tmpl
+++ b/www/common-header.tmpl
@@ -54,7 +54,7 @@ common-footer.tmpl) }}
     <header class="navbar navbar-default navbar-expand bg-light navbar-static-top" id="top" role="banner">
       <div id="nav-options" class="container">
         <div class="navbar-header">
-          <a class='navbar-brand' href='{{ __common__.host-url }}/'>{{ __common__.process-name }}</a>
+          <a class='navbar-brand' href='{{ __common__.host-url }}/' id='root-link'>{{ __common__.process-name }}</a>
         </div>
         <nav class="collapse navbar-collapse bs-navbar-collapse" role="navigation">
           <ul class="nav navbar-nav">
@@ -72,3 +72,21 @@ common-footer.tmpl) }}
   {{.}}
 </div>
 {{/error}}
+<script>
+// For Apache Knox compatibility, all urls that are accessed by javascript should have
+// their path wrapped with this.
+function make_url(path) {
+  var root_link = document.getElementById('root-link');
+  var s  = root_link.href.split("?");
+  url = s[0] + path;
+  if (s.length > 1) {
+    if (path.includes("?")) {
+      url += "&"
+    } else {
+      url += "?";
+    }
+    url += s[1];
+  }
+  return url;
+}
+</script>
\ No newline at end of file
diff --git a/www/query_plan.tmpl b/www/query_plan.tmpl
index f3e5257..a644257 100644
--- a/www/query_plan.tmpl
+++ b/www/query_plan.tmpl
@@ -215,7 +215,7 @@ function renderGraph(ignored_arg) {
 function refresh() {
   req = new XMLHttpRequest();
   req.onload = renderGraph;
-  req.open("GET", "/query_plan?query_id={{query_id}}&json", true);
+  req.open("GET", make_url("/query_plan?query_id={{query_id}}&json"), true);
   req.send();
 }
 
diff --git a/www/query_summary.tmpl b/www/query_summary.tmpl
index de55176..8ed6e9b 100644
--- a/www/query_summary.tmpl
+++ b/www/query_summary.tmpl
@@ -50,7 +50,7 @@ function refresh() {
     document.getElementById("last-updated").textContent = new Date();
   }
   xhr.ontimeout = function(){ }
-  xhr.open('GET', "/query_summary?query_id={{query_id}}&json", true);
+  xhr.open('GET', make_url("/query_summary?query_id={{query_id}}&json"), true);
   xhr.send();
 }
 
diff --git a/www/rpcz.tmpl b/www/rpcz.tmpl
index 298cc89..42d1f46 100644
--- a/www/rpcz.tmpl
+++ b/www/rpcz.tmpl
@@ -145,19 +145,19 @@ under the License.
 <script>
 function reset_all() {
   var xhr = new XMLHttpRequest();
-  xhr.open('GET', "/rpcz_reset", true);
+  xhr.open('GET', make_url("/rpcz_reset"), true);
   xhr.send();
 }
 
 function reset_method(server, method) {
   var xhr = new XMLHttpRequest();
-  xhr.open('GET', "/rpcz_reset?server=" + server + "&method=" + method, true);
+  xhr.open('GET', make_url("/rpcz_reset?server=" + server + "&method=" + method), true);
   xhr.send();
 }
 
 function reset_server(server) {
   var xhr = new XMLHttpRequest();
-  xhr.open('GET', "/rpcz_reset?server=" + server, true);
+  xhr.open('GET', make_url("/rpcz_reset?server=" + server), true);
   xhr.send();
 }
 
@@ -257,7 +257,7 @@ function refresh() {
   }
 
   xhr.ontimeout = function() {}
-  xhr.open('GET', "/rpcz?json", true);
+  xhr.open('GET', make_url("/rpcz?json"), true);
   xhr.send();
 }