From e29a79c76d91a6544c2fb5683cab2bb80f5f2319 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 26 Oct 2026 12:11:58 +0300 Subject: [PATCH] fix(instances): internal snapshot names are checked, snapshot dates carry the year Only external snapshots had their name checked. An internal one could be taken with an empty name: libvirt named it, the job and its log row named nothing, and a recovery looked for "" and marked the job failed though the snapshot existed. Both kinds now pass the same check when queued, and the internal form asks for a name as the external one does. Snapshot dates showed no year, so last year's snapshot looked like this year's in the list one reverts from; they now read 2026-10-13 23:04:00 under "Date (time zone)", as on the Jobs page. --- instances/jobs.py | 3 ++- instances/templates/instances/snapshots_tab.html | 12 +++++++----- instances/tests/test_snapshot_jobs.py | 7 +++++++ vrtManager/instance.py | 10 ++++++---- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/instances/jobs.py b/instances/jobs.py index d3170397..565eda74 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -13,7 +13,7 @@ from libvirt import (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, VIR_DOMAIN_JOB_NONE, from vrtManager import util from vrtManager.connection import wvmConnect from vrtManager.create import wvmCreate -from vrtManager.instance import wvmInstance +from vrtManager.instance import check_snapshot_name, wvmInstance from accounts.models import UserInstance, UserSSHKey from appsettings.settings import app_settings @@ -305,6 +305,7 @@ class CreateSnapshot(Kind): "memory": bool(raw.get("memory")), "quiesce": bool(raw.get("quiesce"))} def validate(self, user, params): + check_snapshot_name(params["name"]) # recovery counts a listed snapshot as taken by the job if params["name"] in _proxy(_authorized(user, params["instance"])).instance.snapshotListNames(0): raise util.OperationError(f"A snapshot named {params['name']} already exists") diff --git a/instances/templates/instances/snapshots_tab.html b/instances/templates/instances/snapshots_tab.html index d21797a2..075c34bc 100755 --- a/instances/templates/instances/snapshots_tab.html +++ b/instances/templates/instances/snapshots_tab.html @@ -1,4 +1,6 @@ {% load i18n %} +{% load tz %} +{% get_current_timezone as TIME_ZONE %} {% load bootstrap_icons %} {% load operations %}
@@ -38,7 +40,7 @@
{% csrf_token %}{% submission_key %}
- + | {% if instance.external_snapshots or busy or held %} @@ -90,7 +92,7 @@ - + @@ -99,7 +101,7 @@ {% for snap in instance.internal_snapshots %} - +
{% trans "Date" %}{% blocktrans %}Date ({{ TIME_ZONE }}){% endblocktrans %} {% trans "Name" %} {% trans "Description" %} {% trans "Action" %}
{{ snap.date|date:"M d H:i:s" }}{{ snap.date|date:"Y-m-d H:i:s" }} {{ snap.name }} {{ snap.description }} @@ -138,7 +140,7 @@ - + @@ -147,7 +149,7 @@ {% for ext_snap in instance.external_snapshots %} - +
{% trans "Date" %}{% blocktrans %}Date ({{ TIME_ZONE }}){% endblocktrans %} {% trans "Name" %} {% trans "Description" %} {% trans "Action" %}
{{ ext_snap.date|date:"M d H:i:s" }}{{ ext_snap.date|date:"Y-m-d H:i:s" }} {{ ext_snap.name }} {% if ext_snap.current %}{% trans "current" %}{% endif %} diff --git a/instances/tests/test_snapshot_jobs.py b/instances/tests/test_snapshot_jobs.py index 69b03d7d..fa98a9de 100644 --- a/instances/tests/test_snapshot_jobs.py +++ b/instances/tests/test_snapshot_jobs.py @@ -62,6 +62,13 @@ class SnapshotJobsTestCase(TransactionTestCase): {"name": "a", "submission_key": "key-snap0001"}) self.assertFalse(Operation.objects.exists()) + def test_a_name_that_is_empty_or_not_a_snapshot_name_is_refused_on_the_page(self): + for name in ("", ".hidden", "a b"): + response = self.client.post(reverse("instances:snapshot", args=[self.instance.pk]), + {"name": name, "submission_key": "key-snap0001"}, follow=True) + self.assertContains(response, "Snapshot names are 1-64 letters") + self.assertFalse(Operation.objects.exists()) + def test_create_recovery(self): op = Operation(params={"instance": self.instance.pk, "name": "a", "external": True}, intent={"compute": self.instance.compute_id}) diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 451c6e18..0c5f0b95 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -348,6 +348,11 @@ class wvmInstances(wvmConnect): ) from error +def check_snapshot_name(name): + if not SNAPSHOT_NAME.fullmatch(name): + raise util.OperationError("Snapshot names are 1-64 letters, digits, '.', '_' or '-' and do not start with '.'") + + def agent_command(dom, command, arguments=None): """The answer of one guest agent command that waits AGENT_TIMEOUT seconds at most: libvirt's own calls for it wait for a hung agent indefinitely, @@ -1618,10 +1623,7 @@ class wvmInstance(wvmConnect): accepts it only without memory, which keeps the whole state anyway). free_percent, free_min_gib: the free space each pool keeps (see _check_snapshot_space).""" - if not SNAPSHOT_NAME.fullmatch(name): - raise util.OperationError( - "Snapshot names are 1-64 letters, digits, '.', '_' or '-' and do not start with '.'" - ) + check_snapshot_name(name) if quiesce: if memory: raise util.OperationError("A snapshot with memory is not quiesced; it keeps the whole guest state")