diff --git a/accounts/tests/test_audit_rows.py b/accounts/tests/test_audit_rows.py index 775287b3..17b68a65 100644 --- a/accounts/tests/test_audit_rows.py +++ b/accounts/tests/test_audit_rows.py @@ -61,11 +61,12 @@ class AdminAccountRowTests(TestCase): self.client.post(url, {"is_vnc": "on"}) # no change self.client.post(reverse("accounts:user_instance_delete", args=[ui.pk])) self.assertEqual(rows(), [ - ("VM vm owner bob: add", True), - ("VM vm owner bob: change is_vnc", True), - ("VM vm owner bob: remove", True), + ("VM vm: add owner bob", True), + ("VM vm: change owner bob: is_vnc", True), + ("VM vm: remove owner bob", True), ]) - self.assertEqual(set(Logs.objects.values_list("user", "host", "instance")), {("audit-admin", "c1", "vm")}) + self.assertEqual(set(Logs.objects.values_list("user", "host", "instance", "instance_uuid")), + {("audit-admin", "c1", "vm", self.vm.uuid)}) def test_otp_mail_without_an_email(self): from smtplib import SMTPException diff --git a/accounts/views.py b/accounts/views.py index e7983136..5db96b85 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -19,7 +19,7 @@ from django.views.decorators.http import require_POST from accounts.apps import admin_password_path from instances.models import Instance -from logs.audit import audit, changed_fields, failed, snapshot +from logs.audit import audit, audit_vm, changed_fields, failed, snapshot from accounts.forms import EmailOTPForm, ProfileForm, UserSSHKeyForm from accounts.models import User, UserAttributes, UserInstance, UserSSHKey @@ -146,8 +146,7 @@ def user_instance_create(request, user_id): form = forms.UserInstanceForm(request.POST or None, initial={"user": user}) if form.is_valid(): instance = form.cleaned_data["instance"] - subject = f"VM {instance.name} owner {form.cleaned_data['user'].username}" - with audit(request, instance.compute, subject, "add", instance=instance.name): + with audit_vm(request, instance, f"add owner {form.cleaned_data['user'].username}"): form.save() return redirect(reverse("accounts:account", args=[user.id])) @@ -167,11 +166,10 @@ def user_instance_update(request, pk): instance, before = user_instance.instance, snapshot(user_instance) form = forms.UserInstanceForm(request.POST or None, instance=user_instance) if form.is_valid(): - subject = f"VM {instance.name} owner {user_instance.user.username}" - with audit(request, instance.compute, subject, "change", instance=instance.name) as entry: + with audit_vm(request, instance, "change owner") as entry: form.save() fields = changed_fields(before, snapshot(user_instance)) - entry.action = "change " + ", ".join(fields) + entry.action = f"change owner {user_instance.user.username}: " + ", ".join(fields) entry.skip = not fields return redirect(reverse("accounts:account", args=[user_instance.user.id])) @@ -190,8 +188,7 @@ def user_instance_delete(request, pk): user_instance = get_object_or_404(UserInstance, pk=pk) if request.method == "POST": user, instance = user_instance.user, user_instance.instance - with audit(request, instance.compute, f"VM {instance.name} owner {user.username}", "remove", - instance=instance.name): + with audit_vm(request, instance, f"remove owner {user.username}"): user_instance.delete() next_url = request.GET.get("next", None) if next_url and url_has_allowed_host_and_scheme( diff --git a/computes/utils.py b/computes/utils.py index 7a95657e..7c3db362 100644 --- a/computes/utils.py +++ b/computes/utils.py @@ -60,7 +60,7 @@ def lock_ownership(): def _log(inst, what): - write("-", inst.compute.name, "", f"VM {inst.name} ({inst.uuid}): {what}", True) + write("-", inst.compute.name, inst.name, f"VM {inst.name} ({inst.uuid}): {what}", True, uuid=inst.uuid) def _move_ownership(source, target): diff --git a/instances/jobs.py b/instances/jobs.py index ce7a29ab..c6a367fe 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -53,7 +53,7 @@ def _vm_plan(params): claims.instance_key(instance.uuid): Claim.EXCLUSIVE}, resource=claims.instance_key(instance.uuid), label=f"{instance.compute.name}/{instance.name}", - intent={"compute": instance.compute_id}, + intent={"compute": instance.compute_id, "uuid": instance.uuid}, ) @@ -83,9 +83,12 @@ def _libvirt_busy(dom): return dom.isActive() and dom.jobInfo()[0] != VIR_DOMAIN_JOB_NONE -def _log(op, state, message): - compute, _sep, vm = op.resource_label.rpartition("/") # a VM name has no "/" - return compute, vm, message if state == Operation.SUCCEEDED else f"{message} ({state})" +def _log(op, state, action, vm=None): + """The job's row: "VM : ", the end state added unless it succeeded.""" + compute, _sep, label_vm = op.resource_label.rpartition("/") # a VM name has no "/" + vm = vm or label_vm + message = f"VM {vm}: {action}" + return compute, vm, op.intent["uuid"], message if state == Operation.SUCCEEDED else f"{message} ({state})" def _snapshot_free_space(): @@ -283,7 +286,7 @@ class DeleteExternalSnapshot(Kind): "changed; check the VM before deleting the snapshot again or reverting to it.") def log_entry(self, op, state): - return _log(op, state, f"Delete external snapshot: {op.params['name']}") + return _log(op, state, f"delete external snapshot {op.params['name']}") GONE = "The VM is no longer on the compute the job ran on" @@ -335,7 +338,7 @@ class CreateSnapshot(Kind): def log_entry(self, op, state): kind = "external snapshot" if op.params["external"] else "snapshot" - return _log(op, state, f"Create {kind}: {op.params['name']}") + return _log(op, state, f"take {kind} {op.params['name']}") def _current(proxy): @@ -388,7 +391,7 @@ class RevertSnapshot(Kind): def log_entry(self, op, state): kind = "external snapshot" if op.params["external"] else "snapshot" - return _log(op, state, f"Revert {kind}: {op.params['name']}") + return _log(op, state, f"revert to {kind} {op.params['name']}") def _delete_created(proxy, op): @@ -606,9 +609,8 @@ class CloneInstance(Kind): return Outcome(Operation.SUCCEEDED) def log_entry(self, op, state): - compute, _sep, source = op.resource_label.rpartition("/") - message = f"Create a clone of '{source}'" - return compute, op.intent["name"], message if state == Operation.SUCCEEDED else f"{message} ({state})" + source = op.resource_label.rpartition("/")[2] + return _log(op, state, f"clone from {source}", vm=op.intent["name"]) @register @@ -756,9 +758,7 @@ class CreateInstance(Kind): return vm_outcome(op, owner=False, volumes=names if data["hdd_size"] or data["template"] else []) def log_entry(self, op, state): - compute, _sep, name = op.resource_label.rpartition("/") - message = "Instance is created" - return compute, name, message if state == Operation.SUCCEEDED else f"{message} ({state})" + return _log(op, state, "create") MIGRATE_OPTIONS = ("live", "unsafe", "offline", "autoconverge", "compress") diff --git a/instances/templates/instances/stats_tab.html b/instances/templates/instances/stats_tab.html index 4d6a0ea0..c202a5de 100644 --- a/instances/templates/instances/stats_tab.html +++ b/instances/templates/instances/stats_tab.html @@ -9,7 +9,7 @@ @@ -71,11 +71,12 @@ {% trans "Date" %} {% trans "User" %} {% trans "Message" %} + {% trans "Result" %} - {% trans 'None' %}... + {% trans 'None' %}... @@ -85,10 +86,8 @@