fix(instances): the VM page opens the tab a redirect points to, and host links are for administrators

Redirects to #access (root password, SSH key) and other top-level panes
opened nothing, and some sub-tab ids did not exist. static/js/instance_tabs.js
opens the pane and the tab that holds it from the page structure, and
opens Settings on its first section, without leaving the current tab;
guest OS info is loaded whenever it is the section opened. The bottom bar linked to
host pages, which only administrators may open, and showed host names to
everyone. A stray </li> is removed; the page's inline scripts and the tab
logic are checked with Node.

instances/templates/bottom_bar.html

@@ -8,6 +8,7 @@
         </button>
         <div id="navbar_bottom" class="navbar-collapse collapse">
             <ul class="navbar-nav">
+                {% if request.user.is_superuser %}{# host pages and names are for administrators #}
                 <li class="nav-item">
                     <a class="nav-link" href="{% url 'overview' compute.id %}">
                         {% bs_icon 'server'%} 
@@ -17,6 +18,7 @@
                 <li class="nav-item">
                     <a class="nav-link" href="{% url 'instances' compute.id %}">{% bs_icon 'laptop'%}  {% trans "Instances" %}</a>
                 </li>
+                {% endif %}
                 <li class="nav-item active">
                     <a class="nav-link" href="{% url 'instances:instance' instance.id %}">{% bs_icon 'device-hdd'%}  {{ instance.name }}</a>
                 </li>

instances/templates/instance.html

@@ -675,43 +675,31 @@
         return true;
     });
 </script>
+<script src="{% static 'js/instance_tabs.js' %}"></script>
 <script>
-    // Open the tab (and sub-tab) a redirect points to with #<pane id>
+    // Open the tab whose control targets "#<pane id>"
     function show_tab(target) {
-        var button = document.querySelector('button[data-bs-target="' + target + '"]');
-        if (button) {
-            bootstrap.Tab.getOrCreateInstance(button).show();
-        }
-    }
-    var hash = location.hash;
-    var parents = {
-        '#poweron': '#power', '#poweroff': '#power', '#powercycle': '#power', '#suspend': '#power', '#resume': '#power',
-        '#resizevm_cpu': '#resize', '#resizevm_mem': '#resize', '#resizevm_disk': '#resize',
-        '#boot_opt': '#settings', '#disks': '#settings', '#network': '#settings', '#clone': '#settings',
-        '#xmledit': '#settings', '#vncsettings': '#settings', '#migrate': '#settings', '#options': '#settings',
-        '#users': '#settings',
-        '#takesnapshot': '#snapshots', '#takeextsnapshot': '#snapshots', '#managesnapshot': '#snapshots'
-    };
-    if (hash === '#resize') {
-        show_tab('#resize');
-    } else if (parents[hash]) {
-        show_tab(parents[hash]);
-        show_tab(hash);
-    }
-    // Settings opens on its first section the user has (Boot is superuser-only)
-    var settings_tabs = document.querySelector('#settings .nav-tabs');
-    if (settings_tabs && !settings_tabs.querySelector('.nav-link.active')) {
-        var first = settings_tabs.querySelector('.nav-link:not(#osinfo-tab)');
-        if (first) {
-            bootstrap.Tab.getOrCreateInstance(first).show();
+        var controls = document.querySelectorAll('[data-bs-target], a[data-bs-toggle][href]');
+        for (var i = 0; i < controls.length; i++) {
+            var c = controls[i];
+            if ((c.getAttribute('data-bs-target') || c.getAttribute('href')) === target) {
+                bootstrap.Tab.getOrCreateInstance(c).show();
+                return;
+            }
         }
     }
+    // after the whole page is parsed: load_osinfo is defined further down
+    document.addEventListener('DOMContentLoaded', function () {
+        restoreInstanceTab(document, location.hash, show_tab, load_osinfo);
+    });
 </script>
 <script>
     function get_osinfo() {
         show_tab('#settings');
         show_tab('#osinfo');
-
+        load_osinfo();
+    }
+    function load_osinfo() {
         $.getJSON("{% url 'instances:osinfo' instance.id %}", function (data) {
             $.each(data, function() {
                 $('#oshostname').text(data['host-name']);

instances/templates/instances/resize_tab.html

@@ -17,7 +17,6 @@
                 {% trans "Disk" %}
             </button>
         </li>
-        </li>
     </ul>
     <!-- Tab panes -->
     <div class="tab-content">

instances/test_page_controls.py

@@ -8,6 +8,9 @@ or with is_change; snapshots with snapshot_instances and is_change, on a
 template only for staff. is_staff and the global view_instances permission
 alone are read-only.
 """
+import shutil
+import subprocess
+import unittest
 from unittest.mock import PropertyMock, patch
 
 from django.contrib.auth import get_user_model
@@ -30,7 +33,7 @@ SUPERUSER_ONLY = {"vcpu hotplug", "video model", "guest agent"}
 class PageControls:
     def setUp(self):
         AppSettings.objects.filter(
-            key__in=["SHOW_ACCESS_ROOT_PASSWORD", "SHOW_ACCESS_SSH_KEYS"]
+            key__in=["SHOW_ACCESS_ROOT_PASSWORD", "SHOW_ACCESS_SSH_KEYS", "VIEW_INSTANCE_DETAIL_BOTTOM_BAR"]
         ).update(value="True")
         self.compute = Compute.objects.create(
             name="ctl-compute", hostname="127.0.0.1:1", login="root", password="", type=1
@@ -72,6 +75,7 @@ class PageControls:
             proxy.get_status.return_value = status
             response = self.client.get(reverse("instances:instance", args=[self.instance.id]))
         self.assertEqual(response.status_code, 200)
+        self.last_page = response.content.decode()
         doc = lxml_html.fromstring(response.content)
         pk = self.instance.id
 
@@ -89,6 +93,8 @@ class PageControls:
             "resize": form("resize_memory"),
             "options": form("change_options"),
             "console settings": form("update_console"),
+            # host pages (superuser-only views)
+            "host links": bool(doc.xpath("//a[@href=$a]", a=reverse("overview", args=[self.compute.id]))),
             # a link to the destroy confirmation page
             "destroy": bool(doc.xpath("//a[@href=$a]", a=reverse("instances:destroy", args=[pk]))),
             "vcpu hotplug": form("set_vcpu"),
@@ -102,11 +108,26 @@ class PageControls:
 
 
 class PageControlsTestCase(PageControls, TestCase):
+    @unittest.skipUnless(shutil.which("node"), "node is not installed")
+    def test_the_page_scripts_parse(self):
+        superuser = get_user_model().objects.create_superuser("ctl_js", "ctl_js@example.com", "x")
+        viewer = self.user("ctl_js_viewer", perms=("view_instances",))
+        for user in (superuser, viewer):
+            self.controls(user)
+            scripts = lxml_html.fromstring(self.last_page).xpath("//script[not(@src)]/text()")
+            for number, script in enumerate(scripts):
+                with self.subTest(user=user.username, script=number):
+                    result = subprocess.run(
+                        ["node", "--check", "-"], input=script, capture_output=True, text=True, timeout=30
+                    )
+                    self.assertEqual(result.returncode, 0, result.stderr)
+
     def test_superuser_sees_every_control(self):
         superuser = get_user_model().objects.create_superuser("ctl_super", "ctl_super@example.com", "x")
         self.assertEqual(
             self.controls(superuser),
-            CONSOLE | POWER | CHANGE | SUPERUSER_ONLY | {"clone", "snapshots", "console settings", "destroy"},
+            CONSOLE | POWER | CHANGE | SUPERUSER_ONLY
+            | {"clone", "snapshots", "console settings", "destroy", "host links"},
         )
 
     def test_global_viewers_see_nothing_to_act_on(self):

static/js/instance_tabs.js

@@ -0,0 +1,40 @@
+// The tab the VM page opens on.
+//
+// A redirect points at a pane (#access, #resize, #options, ...): open the tab
+// that holds it first, then the pane itself; the page structure tells which
+// tab holds which pane. Settings opens on its first section the user has;
+// guest OS info is loaded whenever it is the section opened.
+//
+// show(target) opens the tab whose control targets "#id"; loadInfo() fetches
+// the guest OS info.
+function restoreInstanceTab(doc, hash, show, loadInfo) {
+    var pane = hash && hash.length > 1 ? doc.getElementById(hash.slice(1)) : null;
+    if (pane && pane.classList.contains("tab-pane")) {
+        var holder = pane.parentElement;
+        while (holder && !holder.classList.contains("tab-pane")) {
+            holder = holder.parentElement;
+        }
+        if (holder) {
+            show("#" + holder.id);
+        }
+        show(hash);
+        if (hash === "#osinfo") {
+            loadInfo();
+        }
+    }
+    var settings = doc.getElementById("settings");
+    var tabs = settings ? settings.querySelector(".nav-tabs") : null;
+    if (tabs && !tabs.querySelector(".nav-link.active")) {
+        var first = tabs.querySelector(".nav-link:not(#osinfo-tab)");
+        if (first) {
+            show(first.getAttribute("data-bs-target"));
+        } else if (tabs.querySelector("#osinfo-tab")) {
+            show("#osinfo");
+            loadInfo();
+        }
+    }
+}
+
+if (typeof module !== "undefined") {
+    module.exports = {restoreInstanceTab: restoreInstanceTab};
+}

webvirtcloud/js_tests/instance_tabs.test.js

@@ -0,0 +1,77 @@
+// node instance_tabs.test.js path/to/static/js/instance_tabs.js
+const assert = require("assert");
+const {restoreInstanceTab} = require(process.argv[2]);
+
+// A minimal DOM: elements with an id, classes, a parent, children, and the
+// few selectors restoreInstanceTab uses.
+function el(id, classes, children = [], attrs = {}) {
+    const node = {id, classList: new Set(classes), children, attrs, parentElement: null};
+    node.classList.contains = (c) => node.classList.has(c);
+    node.getAttribute = (name) => attrs[name];
+    for (const child of children) child.parentElement = node;
+    node.querySelector = (selector) => find(node, selector);
+    return node;
+}
+
+function* walk(node) {
+    for (const child of node.children) {
+        yield child;
+        yield* walk(child);
+    }
+}
+
+function matches(node, selector) {
+    if (selector === ".nav-tabs") return node.classList.has("nav-tabs");
+    if (selector === ".nav-link.active") return node.classList.has("nav-link") && node.classList.has("active");
+    if (selector === ".nav-link:not(#osinfo-tab)") return node.classList.has("nav-link") && node.id !== "osinfo-tab";
+    if (selector === "#osinfo-tab") return node.id === "osinfo-tab";
+    throw new Error("unsupported selector " + selector);
+}
+
+function find(root, selector) {
+    for (const node of walk(root)) if (matches(node, selector)) return node;
+    return null;
+}
+
+function link(target, extra = []) {
+    return el(target === "#osinfo" ? "osinfo-tab" : "", ["nav-link", ...extra], [], {"data-bs-target": target});
+}
+
+// The VM page: top-level panes, Settings with its own tabs and panes.
+function page(settingsLinks) {
+    const settings = el("settings", ["tab-pane"], [
+        el("", ["nav-tabs"], settingsLinks),
+        el("", ["tab-content"], [el("osinfo", ["tab-pane"]), el("options", ["tab-pane"]), el("vncsettings", ["tab-pane"])]),
+    ]);
+    const root = el("", [], [el("", ["tab-content"], [el("power", ["tab-pane", "active"]), el("access", ["tab-pane"]), settings])]);
+    root.getElementById = (id) => {
+        for (const node of walk(root)) if (node.id === id) return node;
+        return null;
+    };
+    return root;
+}
+
+function run(doc, hash) {
+    const shown = [];
+    let loaded = 0;
+    restoreInstanceTab(doc, hash, (target) => shown.push(target), () => loaded++);
+    return {shown, loaded};
+}
+
+const settled = () => page([link("#options", ["active"])]);
+
+// a top-level pane opens directly
+assert.deepStrictEqual(run(settled(), "#access").shown, ["#access"]);
+// a pane inside Settings opens Settings first
+assert.deepStrictEqual(run(settled(), "#options").shown, ["#settings", "#options"]);
+// no hash, an unknown one, or one with odd characters opens nothing
+for (const hash of ["", "#", "#nosuchpane", '#a"]b']) {
+    assert.deepStrictEqual(run(settled(), hash).shown, [], hash);
+}
+// Settings without an active section opens its first one, not OS info
+assert.deepStrictEqual(run(page([link("#osinfo"), link("#vncsettings")]), "").shown, ["#vncsettings"]);
+// OS info as the only section is selected and loaded
+assert.deepStrictEqual(run(page([link("#osinfo")]), ""), {shown: ["#osinfo"], loaded: 1});
+// a redirect to OS info loads it
+assert.deepStrictEqual(run(settled(), "#osinfo"), {shown: ["#settings", "#osinfo"], loaded: 1});
+console.log("ok");

webvirtcloud/test_instance_tabs_js.py

@@ -0,0 +1,23 @@
+"""The VM page opens the tab a redirect points to (static/js/instance_tabs.js).
+Its behaviour is checked with Node."""
+
+import shutil
+import subprocess
+import unittest
+from pathlib import Path
+
+from django.conf import settings
+
+BASE = Path(settings.BASE_DIR)
+
+
+class InstanceTabsTestCase(unittest.TestCase):
+    @unittest.skipUnless(shutil.which("node"), "node is not installed")
+    def test_instance_tabs_js(self):
+        result = subprocess.run(
+            ["node", str(BASE / "webvirtcloud/js_tests/instance_tabs.test.js"), str(BASE / "static/js/instance_tabs.js")],
+            capture_output=True,
+            text=True,
+            timeout=60,
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)