mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(nwfilters): a filter is changed in place, also while a VM uses it
Every change to a filter (a rule or reference removed or added, the XML edited) undefined it and defined the new XML. libvirt refuses to undefine a filter a running VM uses, so none of these worked on such a filter; on an unused one a refused definition was undone by defining the old XML without its UUID, so the filter came back as another one. The filter is now defined again with its own UUID, which libvirt does in place, while VMs use it too; XML it cannot parse or apply to the running VMs is refused and reported. The XML editor shows the UUID. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
39933fce7f
commit
f9c8c36a6b
3 changed files with 84 additions and 37 deletions
75
nwfilters/tests/test_redefine.py
Normal file
75
nwfilters/tests/test_redefine.py
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
"""A filter is changed by defining it again with its own UUID, which libvirt
|
||||
does in place, also while a running VM uses it (undefining one in use is
|
||||
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 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 libvirt import libvirtError
|
||||
from lxml import etree, html
|
||||
|
||||
UUID = "6d1f7a10-0000-4000-8000-00000000c0de"
|
||||
XML = (f"<filter name='web' chain='root'><uuid>{UUID}</uuid><filterref filter='clean-traffic'/>"
|
||||
"<rule action='accept' direction='in' priority='500'><tcp dstportstart='22'/></rule>"
|
||||
"<rule action='drop' direction='in' priority='900'><all/></rule></filter>")
|
||||
|
||||
|
||||
class FilterRedefineTests(TestCase):
|
||||
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
|
||||
nwfilter.name.return_value = "web"
|
||||
nwfilter.UUIDString.return_value = UUID
|
||||
self.wvm.listNWFilters.return_value = ["web"]
|
||||
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"))
|
||||
self.url = reverse("nwfilter", args=[compute.id, "web"])
|
||||
|
||||
def defined(self, data):
|
||||
response = self.client.post(self.url, data)
|
||||
self.wvm.nwfilterLookupByName.return_value.undefine.assert_not_called()
|
||||
self.wvm.nwfilterDefineXML.assert_called_once()
|
||||
return etree.fromstring(self.wvm.nwfilterDefineXML.call_args.args[0]), response
|
||||
|
||||
def test_a_rule_is_removed_in_place(self):
|
||||
tree, _ = self.defined({"del_nwfilter_rule": "", "action": "drop", "direction": "in", "priority": "900"})
|
||||
self.assertEqual(tree.findtext("uuid"), UUID)
|
||||
self.assertEqual([r.get("action") for r in tree.findall("rule")], ["accept"])
|
||||
|
||||
def test_a_reference_is_removed_in_place(self):
|
||||
tree, _ = self.defined({"del_nwfilter_ref": "", "ref": "clean-traffic"})
|
||||
self.assertEqual((tree.findtext("uuid"), tree.findall("filterref")), (UUID, []))
|
||||
|
||||
def test_a_rule_is_added_in_place(self):
|
||||
rule = "<rule action='accept' direction='in' priority='500'><udp dstportstart='53'/></rule>"
|
||||
tree, response = self.defined({"add_nwfilter_rule": "", "nwfilterrule_xml": rule})
|
||||
self.assertEqual(tree.findtext("uuid"), UUID)
|
||||
self.assertEqual(tree.xpath("rule[@priority='500']/*/@dstportstart"), ["22", "53"])
|
||||
self.assertRedirects(response, self.url, fetch_redirect_response=False)
|
||||
|
||||
def test_a_reference_is_added_in_place(self):
|
||||
tree, response = self.defined({"add_nwfilter_ref": "", "nwfilters_select": "no-spoofing"})
|
||||
self.assertEqual(tree.findtext("uuid"), UUID)
|
||||
self.assertEqual(tree.xpath("filterref/@filter"), ["clean-traffic", "no-spoofing"])
|
||||
self.assertRedirects(response, self.url, fetch_redirect_response=False)
|
||||
|
||||
def test_the_editor_holds_the_uuid_and_saves_in_place(self):
|
||||
page = html.fromstring(self.client.get(self.url).content)
|
||||
edited = page.get_element_by_id("edit_editor").text
|
||||
self.assertIn(UUID, edited)
|
||||
tree, _ = self.defined({"edit_nwfilter": "", "edit_xml": edited})
|
||||
self.assertEqual(tree.findtext("uuid"), UUID)
|
||||
|
||||
def test_a_refused_definition_is_reported_not_undone(self):
|
||||
self.wvm.nwfilterDefineXML.side_effect = libvirtError("XML error: Invalid value for attribute 'action'")
|
||||
_, response = self.defined({"del_nwfilter_rule": "", "action": "drop", "direction": "in", "priority": "900"})
|
||||
self.assertIn("Invalid value for attribute 'action'", " ".join(str(m) for m in get_messages(response.wsgi_request)))
|
||||
|
|
@ -180,12 +180,7 @@ def nwfilter(request, compute_id, nwfltr):
|
|||
new_xml = request.POST.get("edit_xml", "")
|
||||
|
||||
if new_xml:
|
||||
nwfilter.delete()
|
||||
try:
|
||||
conn.create_nwfilter(new_xml)
|
||||
except libvirtError as lib_err:
|
||||
conn.create_nwfilter(xml)
|
||||
raise util.OperationError(lib_err)
|
||||
conn.create_nwfilter(new_xml)
|
||||
|
||||
if "del_nwfilter_rule" in request.POST:
|
||||
action = request.POST.get("action", "")
|
||||
|
|
@ -193,46 +188,26 @@ def nwfilter(request, compute_id, nwfltr):
|
|||
priority = request.POST.get("priority", "")
|
||||
|
||||
new_xml = nwfilter.delete_rule(action, direction, priority)
|
||||
nwfilter.delete()
|
||||
try:
|
||||
conn.create_nwfilter(new_xml)
|
||||
except libvirtError as lib_err:
|
||||
conn.create_nwfilter(xml)
|
||||
raise util.OperationError(lib_err)
|
||||
conn.create_nwfilter(new_xml)
|
||||
|
||||
if "del_nwfilter_ref" in request.POST:
|
||||
ref_name = request.POST.get("ref")
|
||||
new_xml = nwfilter.delete_ref(ref_name)
|
||||
nwfilter.delete()
|
||||
try:
|
||||
conn.create_nwfilter(new_xml)
|
||||
except libvirtError as lib_err:
|
||||
conn.create_nwfilter(xml)
|
||||
raise util.OperationError(lib_err)
|
||||
conn.create_nwfilter(new_xml)
|
||||
|
||||
if "add_nwfilter_rule" in request.POST:
|
||||
rule_xml = request.POST.get("nwfilterrule_xml", "")
|
||||
if not rule_xml:
|
||||
return HttpResponseRedirect(request.get_full_path())
|
||||
new_xml = nwfilter.add_rule(rule_xml)
|
||||
nwfilter.delete()
|
||||
try:
|
||||
conn.create_nwfilter(new_xml)
|
||||
except libvirtError as lib_err:
|
||||
conn.create_nwfilter(xml)
|
||||
raise util.OperationError(lib_err)
|
||||
conn.create_nwfilter(new_xml)
|
||||
|
||||
if "add_nwfilter_ref" in request.POST:
|
||||
ref_name = request.POST.get("nwfilters_select", "")
|
||||
if not ref_name:
|
||||
return HttpResponseRedirect(request.get_full_path())
|
||||
new_xml = nwfilter.add_ref(ref_name)
|
||||
nwfilter.delete()
|
||||
try:
|
||||
conn.create_nwfilter(new_xml)
|
||||
except libvirtError as lib_err:
|
||||
conn.create_nwfilter(xml)
|
||||
raise util.OperationError(lib_err)
|
||||
conn.create_nwfilter(new_xml)
|
||||
|
||||
return HttpResponseRedirect(request.get_full_path())
|
||||
conn.close()
|
||||
|
|
|
|||
|
|
@ -37,14 +37,11 @@ class wvmNWFilter(wvmConnect):
|
|||
def get_name(self):
|
||||
return self.nwfilter.name()
|
||||
|
||||
def delete(self):
|
||||
self.nwfilter.undefine()
|
||||
|
||||
def get_xml(self):
|
||||
tree = ElementTree.fromstring(self._XMLDesc(0))
|
||||
uuid = tree.find("uuid")
|
||||
tree.remove(uuid)
|
||||
return ElementTree.tostring(tree).decode()
|
||||
"""With the UUID: defined again with it, libvirt changes the filter in
|
||||
place, also while a VM uses it. It refuses XML it cannot parse or
|
||||
apply to the running VMs' interfaces."""
|
||||
return self._XMLDesc(0)
|
||||
|
||||
def get_filter_refs(self):
|
||||
tree = ElementTree.fromstring(self._XMLDesc(0))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue