From 0014b5f240ee3dbf0cfac48e0c0197d7902b5758 Mon Sep 17 00:00:00 2001 From: catborise Date: Thu, 29 Oct 2026 00:21:00 +0300 Subject: [PATCH] feat(nwfilters): the network filter pages on Bootstrap's cards, tables and dialogs As the network and interface pages; plan reviewed with Codex. - The list is one table with a search: each filter's chain, priority, number of rules, the filters it references, and how many VMs and filters use it directly; a row's menu clones it (one shared dialog, the copy's name filled in) or deletes it, Delete disabled and saying so while it is in use. Without the VMs (a host that fails to list them) the list is not shown: it would offer to delete filters in use. - New filter is one dialog with the filter as XML in the Ace editor, from a starter filter, with what in, out and priority mean and a link to libvirt's format. - A filter's page: a Filter card (UUID, chain, priority, the VMs that use it directly, linked, and the filters that reference it, linked); its references with their parameters (one to a filter gone from the host marked as missing, not linked), removed per row, added from a select of the filters it can reference; its rules with the action as a badge, the direction as to or from the VM, the priority (500 where none is given), every protocol element with its attributes, a rule of no protocol as any traffic of the chain, and when it does not track connections; Add rule as an XML dialog; the XML, collapsed, saved in place. - The editors size themselves when shown and keep their label for screen readers; tables keep their hidden header text inside them, so a phone does not scroll the page sideways. add_nwf_rule.html goes (the dialog is on the page). --- nwfilters/templates/add_nwf_rule.html | 33 -- .../templates/create_nwfilter_block.html | 50 +-- nwfilters/templates/nwfilter.html | 305 ++++++++++-------- nwfilters/templates/nwfilters.html | 170 +++++----- nwfilters/tests/test_redefine.py | 50 ++- nwfilters/views.py | 12 + vrtManager/nwfilters.py | 10 +- 7 files changed, 367 insertions(+), 263 deletions(-) delete mode 100644 nwfilters/templates/add_nwf_rule.html diff --git a/nwfilters/templates/add_nwf_rule.html b/nwfilters/templates/add_nwf_rule.html deleted file mode 100644 index 55403877..00000000 --- a/nwfilters/templates/add_nwf_rule.html +++ /dev/null @@ -1,33 +0,0 @@ -{% load i18n %} -{% load bootstrap_icons %} -{% if request.user.is_superuser %} - - - - -{% endif %} \ No newline at end of file diff --git a/nwfilters/templates/create_nwfilter_block.html b/nwfilters/templates/create_nwfilter_block.html index bc7e0482..6e015abf 100644 --- a/nwfilters/templates/create_nwfilter_block.html +++ b/nwfilters/templates/create_nwfilter_block.html @@ -1,33 +1,39 @@ {% load i18n %} {% load bootstrap_icons %} {% if request.user.is_superuser %} - - - {% endif %} diff --git a/nwfilters/templates/nwfilter.html b/nwfilters/templates/nwfilter.html index b3e584ac..3aef4827 100644 --- a/nwfilters/templates/nwfilter.html +++ b/nwfilters/templates/nwfilter.html @@ -3,161 +3,210 @@ {% load static %} {% load bootstrap_icons %} -{% block title %}{% trans "NWFilter" %}: {{ name }}{% endblock %} +{% block title %}{% trans "Network filter" %} - {{ name }}{% endblock %} -{% block page_heading %}{% trans "NWFilter" %}: {{ name }}{% endblock page_heading %} +{% block page_heading %}{{ compute.name }} - {% trans "Network filter" %}: {{ name }}{% endblock page_heading %} {% block content %} -
-
- {% include "computes/host_nav.html" with section="nwfilters" detail=True %} -
+ {% include "computes/host_nav.html" with section="nwfilters" detail=True %} + + {% if uuid %} +
+
{% trans "Filter" %}
+
    +
  • + {% trans "UUID" %}{{ uuid }} +
  • +
  • + {% trans "Chain" %}{{ chain|default:"–" }} +
  • +
  • + {% trans "Priority" %}{% if priority is not None %}{{ priority }}{% else %}–{% endif %} +
  • +
  • + {% trans "Directly used by" %} + {% for vm in vms %}{% if vm.id %}{{ vm.name }}{% else %}{{ vm.name }}{% endif %}{% if not forloop.last %}, {% endif %}{% empty %}–{% endfor %} +
  • +
  • + {% trans "Referenced by" %} + {% for other in referenced_by %}{{ other }}{% if not forloop.last %}, {% endif %}{% empty %}–{% endfor %} +
  • +
-
-
{% trans "UUID" %}:
-
{{ uuid }}
-
{% trans "Name" %}:
-
{{ name }}
-
- -
-
- -
-
-
{% csrf_token %} -
- - -
- -
-
-
-
-
-
-
-
{% trans "Filter References" %} -
{% csrf_token %} -
- - - -
+
+
+ {% trans "References" %} + {% if ref_choices %} + {% csrf_token %} + + -
+ {% endif %}
-
- -
-
-
- - - - - - - - - - {% for ref in refs %} - - - - - - {% endfor %} - -
#{% trans "Reference" %}{% trans "Action" %}
{{ forloop.counter }}{{ ref.filter }}{% for param, value in ref.parameters %} {{ param }}={{ value }}{% endfor %} -
{% csrf_token %} - - - -
-
-
-
-
- - - {% include 'add_nwf_rule.html' %} -
{% trans "Rules" %}
- -
- + {% if refs %} +
+
- - + + + + + + + {% for ref in refs %} + + + + + + {% endfor %} + +
{% trans "Rule" %}{% trans "Action Type" %}{% trans "Filter" %}{% trans "Parameters" %}{% trans "Actions" %}
+ {% if ref.filter in filter_names %}{{ ref.filter }} + {% else %}{{ ref.filter }} {% trans "Missing" %}{% endif %} + {% for param, value in ref.parameters %}
{{ param }}={{ value }}
{% empty %}–{% endfor %}
+
{% csrf_token %} + + + +
+
+
+ {% else %} +
{% trans "It references no other filter." %}
+ {% endif %} +
+ +
+
+ {% trans "Rules" %} + +
+ {% if rules %} +
+ + + + + - - - + + {% for rule in rules %} - - - - - + + + + - {% endfor %} - -
#{% trans "Action" %} {% trans "Direction" %} {% trans "Priority" %}{% trans "Statematch" %}{% trans "Directives" %}{% trans "Action" %}{% trans "Matches" %}{% trans "Actions" %}
{{ forloop.counter }}{{ rule.action }}{{ rule.direction }}{{ rule.priority }}{{ rule.statematch }}{% for protocol in rule.protocols %}
{{ protocol.name }}{% for attribute, value in protocol.attributes %} {{ attribute }}={{ value }}{% endfor %}
{% endfor %}
-
{% csrf_token %} + {% if rule.action == "accept" %}{% trans "Accept" %} + {% elif rule.action == "drop" or rule.action == "reject" %}{{ rule.action|capfirst }} + {% else %}{{ rule.action|capfirst }}{% endif %} +
{% if rule.direction == "in" %}{% trans "To the VM" %}{% elif rule.direction == "out" %}{% trans "From the VM" %}{% elif rule.direction == "inout" %}{% trans "Both ways" %}{% else %}{{ rule.direction }}{% endif %}{% if rule.priority is not None %}{{ rule.priority }}{% else %}500{% endif %} + {% for protocol in rule.protocols %} +
{{ protocol.name }}{% for attribute, value in protocol.attributes %} {{ attribute }}={{ value }}{% endfor %}
+ {% empty %} + {% trans "Any traffic of the chain" %} + {% endfor %} + {% if rule.statematch == "false" or rule.statematch == "0" %}
{% trans "Without connection tracking" %}
{% endif %} +
+ {% csrf_token %} - +
+ {% endfor %} + +
+ {% else %} +
{% trans "The filter has no rules of its own." %}
+ {% endif %} +
+ +
+
+

+ +

+
+
{% csrf_token %} +

{% trans "A change applies at once, also to the running VMs that use the filter; it keeps the filter's name and UUID." %}

+ +
+
+ +
+
+
+
+
+ + + {% endif %} {% endblock %} {% block script %} - - + - + document.getElementById(shownBy).addEventListener(shownEvent, () => editor.resize()); + } + xmlEditor("edit_editor", "edit_xml", "nwfilter-xml", "shown.bs.collapse"); + xmlEditor("rule_editor", "nwfilterrule_xml", "AddNWFilterRule", "shown.bs.modal"); + +{% endif %} {% endblock %} diff --git a/nwfilters/templates/nwfilters.html b/nwfilters/templates/nwfilters.html index 847116df..69e37c90 100644 --- a/nwfilters/templates/nwfilters.html +++ b/nwfilters/templates/nwfilters.html @@ -3,106 +3,126 @@ {% load static %} {% load bootstrap_icons %} -{% block title %}{% trans "NWFilters" %} - {{ compute.name }}{% endblock %} +{% block title %}{% trans "Network filters" %} - {{ compute.name }}{% endblock %} -{% block page_heading %}{{ compute.name }} - {% trans "NWFilters" %}{% endblock page_heading %} +{% block page_heading %}{{ compute.name }} - {% trans "Network filters" %}{% endblock page_heading %} -{% block page_heading_extra %} -{% include 'create_nwfilter_block.html' %} -{% include 'search_block.html' %} -{% endblock page_heading_extra %} +{% block page_heading_extra %}{% include 'create_nwfilter_block.html' %}{% endblock page_heading_extra %} {% block content %} -
-
- {% include "computes/host_nav.html" with section="nwfilters" %} -
-
+ {% include "computes/host_nav.html" with section="nwfilters" %} -
-
- {% if nwfilters %} -
- + {% if not nwfilters %} +
+ {% bs_icon 'exclamation-triangle' %} {% trans "This host has no network filters." %} +
+ {% else %} +
+
+ {% trans "Filters" %} {{ nwfilters|length }} + +
+
+
- - - + + + + + + - {% for nwfilter in nwfilters %} - - - - - - - + {% for nwf in nwfilters %} + + + + + + + + + {% endfor %}
#{% trans "UUID" %} {% trans "Name" %}{% trans "Action" %}{% trans "Chain" %}{% trans "Priority" %}{% trans "Rules" %}{% trans "References" %}{% trans "Directly used by" %}{% trans "Actions" %}
{{ forloop.counter }}{{ nwfilter.uuid }}{{ nwfilter.name }} - - - -
{% csrf_token %} - - -
-
{{ nwf.name }}{{ nwf.chain|default:"–" }}{% if nwf.priority is not None %}{{ nwf.priority }}{% else %}–{% endif %}{{ nwf.rules }}{{ nwf.refs|join:", "|default:"–" }} + {% if nwf.vms or nwf.referenced_by %} + {% if nwf.vms %}{% blocktrans count counter=nwf.vms|length %}{{ counter }} VM{% plural %}{{ counter }} VMs{% endblocktrans %}{% endif %}{% if nwf.vms and nwf.referenced_by %}, {% endif %} + {% if nwf.referenced_by %}{% blocktrans count counter=nwf.referenced_by|length %}{{ counter }} filter{% plural %}{{ counter }} filters{% endblocktrans %}{% endif %} + {% else %}–{% endif %} + +
+ + +
+
- {% else %} -
-
-
- {% bs_icon 'exclamation-triangle' %} {% trans "Warning" %}: {% trans "Hypervisor doesn't have any NWFilters" %} -
-
+
+
- {% endif %}
-
+ + {# one dialog for every filter's clone: its row's Clone fills in the filter #} + + {% endif %} {% endblock %} {% block script %} +{% if nwfilters %} + +{% endif %} {% endblock %} diff --git a/nwfilters/tests/test_redefine.py b/nwfilters/tests/test_redefine.py index fa31eab6..37c3553f 100644 --- a/nwfilters/tests/test_redefine.py +++ b/nwfilters/tests/test_redefine.py @@ -4,13 +4,14 @@ refused). A definition libvirt refuses is reported, not undone by an undefine and another define. libvirt's test driver has no filters, so the connection is a mock.""" -from unittest.mock import patch +from unittest.mock import MagicMock, patch from computes.models import Compute from django.contrib.auth import get_user_model from django.contrib.messages import get_messages from django.test import TestCase from django.urls import reverse +from instances.models import Instance from libvirt import libvirtError from lxml import etree, html from vrtManager.nwfilters import element_text @@ -133,3 +134,50 @@ class RuleDeleteTests(TestCase): response = self.delete(index, rule) self.assertIn("reload the page", " ".join(str(m) for m in get_messages(response.wsgi_request))) self.wvm.nwfilterDefineXML.assert_not_called() + + +class PageTests(TestCase): + """The list says what uses each filter, and does not offer to delete one in + use; a filter's page links the VMs that use it and shows its references' + parameters.""" + + def setUp(self): + patcher = patch("vrtManager.connection.connection_manager.get_connection") + self.wvm = patcher.start().return_value + self.addCleanup(patcher.stop) + nwfilter = self.wvm.nwfilterLookupByName.return_value + nwfilter.XMLDesc.return_value = XML.replace("", + "") + nwfilter.name.return_value = "web" + nwfilter.UUIDString.return_value = UUID + self.wvm.listAllNWFilters.return_value = [nwfilter] + self.wvm.listNWFilters.return_value = ["web", "clean-traffic"] + vm = MagicMock(**{"name.return_value": "web-1", "isActive.return_value": False, "XMLDesc.return_value": + ""}) + self.wvm.listAllDomains.return_value = [vm] + self.compute = Compute.objects.create(name="nwf", hostname="localhost", login="", password="", type=4) + self.client.force_login(get_user_model().objects.create_superuser("nwf", "nwf@example.com", "pw")) + + def test_a_filter_in_use_is_not_offered_for_delete(self): + page = html.fromstring(self.client.get(reverse("nwfilters", args=[self.compute.id])).content) + [delete] = page.xpath("//button[@name='del_nwfilter']") + self.assertIsNotNone(delete.get("disabled")) + self.assertIn("1 VM", page.xpath("//tbody/tr/td[6]")[0].text_content()) + + def test_the_filter_page_links_its_vms_and_shows_parameters(self): + instance = Instance.objects.create(compute=self.compute, name="web-1", uuid="6d1f7a10-0000-4000-8000-00000000a001") + page = html.fromstring(self.client.get(reverse("nwfilter", args=[self.compute.id, "web"])).content) + self.assertEqual(page.xpath("//a[text()='web-1']/@href"), [reverse("instances:instance", args=[instance.id])]) + self.assertIn("IP=192.0.2.10", page.text_content()) + + def test_without_the_vms_the_list_is_not_shown(self): + # it would offer to delete filters in use + self.wvm.listAllDomains.side_effect = libvirtError("connection reset") + page = html.fromstring(self.client.get(reverse("nwfilters", args=[self.compute.id])).content) + self.assertEqual(page.xpath("//button[@name='del_nwfilter']"), []) + + def test_a_reference_to_a_filter_gone_is_not_a_link(self): + self.wvm.listNWFilters.return_value = ["web"] + page = html.fromstring(self.client.get(reverse("nwfilter", args=[self.compute.id, "web"])).content) + self.assertEqual(page.xpath("//a[text()='clean-traffic']"), []) + self.assertIn("Missing", page.xpath("//td[contains(., 'clean-traffic')]")[0].text_content()) diff --git a/nwfilters/views.py b/nwfilters/views.py index 8555018c..6aac34ef 100644 --- a/nwfilters/views.py +++ b/nwfilters/views.py @@ -4,6 +4,7 @@ from django.contrib import messages from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import reverse +from instances.models import Instance from libvirt import libvirtError from logs.audit import audit from vrtManager import util @@ -39,7 +40,11 @@ def nwfilters(request, compute_id): conn.delete_nwfilter(name) return HttpResponseRedirect(request.get_full_path()) + # the VMs first: a list without them would offer to delete filters in use + users = conn.get_users() nwfilters_all = conn.get_nwfilters_info() + for nwf in nwfilters_all: + nwf["vms"] = users.get(nwf["name"], []) except libvirtError as lib_err: # OperationError too: a name in use, a filter in use messages.error(request, lib_err) if request.method == "POST": # a host not reached as well: nothing was changed @@ -84,9 +89,16 @@ def nwfilter(request, compute_id, nwfltr): name = nwfilter.get_name() uuid = nwfilter.get_uuid() xml = nwfilter.get_xml() + chain = nwfilter.tree.get("chain") + priority = nwfilter.tree.get("priority") rules = nwfilter.get_rules() refs = nwfilter.get_refs() ref_choices = nwfilter.get_ref_choices() + filter_names = nwfilter.get_nwfilters() + referenced_by = [nwf["name"] for nwf in nwfilter.get_nwfilters_info() if name in nwf["refs"]] + vm_names = nwfilter.get_users().get(name, []) + vm_ids = dict(Instance.objects.filter(compute=compute, name__in=vm_names).values_list("name", "id")) + vms = [{"name": vm_name, "id": vm_ids.get(vm_name)} for vm_name in vm_names] except libvirtError as lib_err: messages.error(request, lib_err) if request.method == "POST": # a filter gone meanwhile, a host not reached: nothing was changed diff --git a/vrtManager/nwfilters.py b/vrtManager/nwfilters.py index 73d54168..27ab44d0 100644 --- a/vrtManager/nwfilters.py +++ b/vrtManager/nwfilters.py @@ -34,10 +34,12 @@ def filter_info(nwfilter, tree): class wvmNWFilters(wvmConnect): def get_nwfilters_info(self): - """Every filter of the host (filter_info) by name: one listing, one XML - read per filter.""" + """Every filter of the host (filter_info), with the filters that + reference it, by name: one listing, one XML read per filter.""" filters = [filter_info(nwfilter, etree.fromstring(nwfilter.XMLDesc(0))) for nwfilter in self.wvm.listAllNWFilters(0)] + for nwfilter in filters: + nwfilter["referenced_by"] = [other["name"] for other in filters if nwfilter["name"] in other["refs"]] return sorted(filters, key=lambda nwfilter: nwfilter["name"]) def get_users(self): @@ -101,14 +103,14 @@ class wvmNWFilters(wvmConnect): self.get_nwfilter(name).undefine() -class wvmNWFilter(wvmConnect): +class wvmNWFilter(wvmNWFilters): """A filter of the host. A page reads its XML once (tree); a change is the filter defined again with its UUID, which libvirt does in place, also while a VM uses it, and refuses for XML it cannot parse or apply to the running VMs' interfaces.""" def __init__(self, host, login, passwd, conn, nwfiltername): - wvmConnect.__init__(self, host, login, passwd, conn) + wvmNWFilters.__init__(self, host, login, passwd, conn) self.nwfilter = self.get_nwfilter(nwfiltername) @cached_property