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

@@ -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,
         )

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("")
 

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

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"])