From fa84ce81132e703ace1003c53912a2f57241e168 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 26 Oct 2026 12:10:27 +0300 Subject: [PATCH] fix(instances): a console save changes and logs only the fields edited The saved console form had no initial values, so every posted field counted as changed: saving only a password also logged the keymap and the listen address again, each time. A listen address the form does not list (set in the XML editor) was not a choice, so the select showed localhost and a save moved the console there. The form now takes its initial values from the VM, on the page and on the save, and offers the VM's own address. --- instances/forms.py | 16 ++++++++++++++-- instances/tests/test_console_listener.py | 5 ++++- instances/tests/test_vm_audit_rows.py | 15 +++++++++++++++ instances/views.py | 12 ++---------- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/instances/forms.py b/instances/forms.py index 987bd45b..4b517ce5 100644 --- a/instances/forms.py +++ b/instances/forms.py @@ -28,7 +28,15 @@ class ConsoleForm(forms.Form): clear_keymap = forms.BooleanField(label=_("Clear keymap"), required=False) keymap = forms.ChoiceField(label=_("Keymap"), required=False) - def __init__(self, *args, can_set_listener=True, **kwargs): + def __init__(self, *args, instance, can_set_listener, **kwargs): + # the VM's settings as the initial values: a save changes only the fields edited + listener = instance.config_console_listener_address + kwargs["initial"] = { + "console_type": instance.console_type, + "listen_on": listener, + "password": instance.config_console_passwd, + "keymap": instance.config_console_keymap or "auto", + } super(ConsoleForm, self).__init__(*args, **kwargs) types = [("vnc", "VNC")] if settings.SERIAL_CONSOLE_ENABLED: @@ -38,9 +46,13 @@ class ConsoleForm(forms.Form): # Only administrators choose the listen address: beyond localhost the # console is reachable from the network. A disabled field ignores # posted values. + listeners = list(QEMU_CONSOLE_LISTENER_ADDRESSES) + # an address set elsewhere (the XML editor) is offered, so a save keeps it + if listener and listener not in dict(listeners): + listeners.append((listener, listener)) self.fields["listen_on"] = forms.ChoiceField( label=_("Listen on"), - choices=QEMU_CONSOLE_LISTENER_ADDRESSES, + choices=listeners, disabled=not can_set_listener, required=can_set_listener, ) diff --git a/instances/tests/test_console_listener.py b/instances/tests/test_console_listener.py index 8bbcb80a..9553570d 100644 --- a/instances/tests/test_console_listener.py +++ b/instances/tests/test_console_listener.py @@ -28,6 +28,9 @@ class ConsoleListenerTests(TestCase): with patch("instances.models.wvmInstance") as wvm: proxy = wvm.return_value proxy.console_listens_locally.return_value = local + proxy.get_console_listener_addr.return_value = "127.0.0.1" if local else "0.0.0.0" + proxy.get_console_passwd.return_value = "Old-pass-1" + proxy.get_console_keymap.return_value = "" proxy.set_console_passwd.return_value = True response = self.client.post(reverse("instances:update_console", args=[self.vm.id]), data) return proxy, [str(m) for m in get_messages(response.wsgi_request)] @@ -50,7 +53,7 @@ class ConsoleListenerTests(TestCase): proxy.set_console_passwd.assert_called_once_with("s3cret-pass") def test_superuser_can_open_the_console_to_all_interfaces(self): - proxy, _ = self.post(self.admin, {"listen_on": "0.0.0.0", "clear_password": "on", "keymap": "auto"}, local=False) + proxy, _ = self.post(self.admin, {"listen_on": "0.0.0.0", "clear_password": "on", "keymap": "auto"}) proxy.set_console_listener_addr.assert_called_once_with("0.0.0.0") proxy.set_console_passwd.assert_called_once_with("") diff --git a/instances/tests/test_vm_audit_rows.py b/instances/tests/test_vm_audit_rows.py index 7c36e430..ab4fb703 100644 --- a/instances/tests/test_vm_audit_rows.py +++ b/instances/tests/test_vm_audit_rows.py @@ -101,6 +101,9 @@ class VMRowTests(TestCase): def test_a_console_setting_that_changed_nothing_writes_no_row(self): self.proxy.get_console_type.return_value = "vnc" + self.proxy.get_console_listener_addr.return_value = "127.0.0.1" + self.proxy.get_console_passwd.return_value = "" + self.proxy.get_console_keymap.return_value = "" self.proxy.set_console_keymap.return_value = False # no graphics device self.proxy.set_console_listener_addr.return_value = False # a socket, no address self.post("update_console", {"listen_on": "0.0.0.0", "keymap": "de"}) @@ -110,6 +113,18 @@ class VMRowTests(TestCase): self.assertEqual(self.rows(), [("VM vm: set console keymap de", True), ("VM vm: set console listen address 0.0.0.0", True)]) + def test_a_console_save_logs_only_what_changed(self): + # an address the form does not list stays: the select offers it + self.proxy.get_console_type.return_value = "vnc" + self.proxy.get_console_listener_addr.return_value = "192.0.2.10" + self.proxy.get_console_keymap.return_value = "de" + self.proxy.get_console_passwd.return_value = "Vnc-pass-1" + form = {"listen_on": "192.0.2.10", "keymap": "de"} + self.post("update_console", {**form, "password": "Vnc-pass-1"}) + self.post("update_console", {**form, "password": "Vnc-pass-2"}) + self.assertEqual(self.rows(), [("VM vm: set console password", True)]) + self.proxy.set_console_listener_addr.assert_not_called() + def test_a_deleted_vm_and_each_of_its_disks_have_their_own_row(self): self.proxy.split_disk_paths_by_use.return_value = (["/pool/a.qcow2", "/pool/b.qcow2"], []) volume = self.proxy.get_volume_by_path.return_value diff --git a/instances/views.py b/instances/views.py index d9ebffbe..f3b4a231 100755 --- a/instances/views.py +++ b/instances/views.py @@ -240,15 +240,7 @@ def _instance_page(request, pk, instance, compute): can_manage_console = utils.can_manage_console(request.user, instance) console_form = None if can_manage_console: - console_form = ConsoleForm( - initial={ - "console_type": instance.console_type, - "listen_on": instance.config_console_listener_address, - "password": instance.config_console_passwd, - "keymap": instance.config_console_keymap, - }, - can_set_listener=request.user.is_superuser, - ) + console_form = ConsoleForm(instance=instance, can_set_listener=request.user.is_superuser) console_listener_addresses = settings.QEMU_CONSOLE_LISTENER_ADDRESSES bottom_bar = app_settings.VIEW_INSTANCE_DETAIL_BOTTOM_BAR try: @@ -1937,7 +1929,7 @@ def update_console(request, pk): if not utils.can_manage_console(request.user, instance): raise PermissionDenied - form = ConsoleForm(request.POST or None, can_set_listener=request.user.is_superuser) + form = ConsoleForm(request.POST, instance=instance, can_set_listener=request.user.is_superuser) if form.is_valid() and form.cleaned_data["console_type"] not in ("", instance.console_type): with audit_vm(request, instance, f"set console type {form.cleaned_data['console_type']}"): instance.proxy.set_console_type(form.cleaned_data["console_type"])