mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(instances): host errors on the instance list are for administrators
The instance list showed every user an alert for each compute that was not connected, with the compute's name and libvirt's error text. That text names the host and how it is reached (for example "ssh: connect to host 10.20.30.132 port 22: No route to host"), while the list hides host names from users who are not administrators. - Superusers and users with the global view_instances permission see each unreachable host and its error, as before. - Other users see one alert without host details, and only when a host of their own VMs is down. It explains why those VMs show as unknown. - The alerts' close button works (data-bs-dismiss was "modal"). Checked against the test setup with an unreachable compute that holds a user's VM: the user sees the VM as unknown and the alert, but no host name, address or error text; a superuser still sees them. The tests for the user fail on the previous code.
This commit is contained in:
parent
5e26f5a473
commit
a3d3cca477
3 changed files with 88 additions and 10 deletions
|
|
@ -15,14 +15,21 @@
|
|||
{% endblock page_heading_extra %}
|
||||
|
||||
{% block content %}
|
||||
{% for compute in computes %}
|
||||
{% if compute.status is not True %}
|
||||
<div class="alert alert-danger alert-dismissible fade show" role="alert">
|
||||
{% trans 'Problem occurred with host' %} {{ compute.name }}: {{ compute.connection_error }}
|
||||
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
|
||||
</div>
|
||||
{% endif %}
|
||||
{% endfor %}
|
||||
{% if show_hosts %}
|
||||
{% for compute in computes %}
|
||||
{% if compute.status is not True %}
|
||||
<div class="alert alert-danger alert-dismissible fade show" role="alert">
|
||||
{% trans 'Problem occurred with host' %} {{ compute.name }}: {{ compute.connection_error }}
|
||||
<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>
|
||||
</div>
|
||||
{% endif %}
|
||||
{% endfor %}
|
||||
{% elif own_hosts_down %}
|
||||
<div class="alert alert-warning alert-dismissible fade show" role="alert">
|
||||
{% trans 'A host of some of your virtual machines cannot be reached right now.' %}
|
||||
<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>
|
||||
</div>
|
||||
{% endif %}
|
||||
<div class="col-lg-12">
|
||||
{% if app_settings.VIEW_INSTANCES_LIST_STYLE == 'grouped' and request.user.is_superuser %}
|
||||
{% include 'allinstances_index_grouped.html' %}
|
||||
|
|
|
|||
63
instances/test_index_host_alerts.py
Normal file
63
instances/test_index_host_alerts.py
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
"""Host connection errors on the instance list: administrators see which host
|
||||
and why; other users only learn that a host of their own VMs is unreachable."""
|
||||
|
||||
from unittest.mock import PropertyMock, patch
|
||||
|
||||
from accounts.models import UserInstance
|
||||
from computes.models import Compute
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.models import Permission
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
from instances.models import Instance
|
||||
|
||||
USER_MESSAGE = "A host of some of your virtual machines cannot be reached right now."
|
||||
|
||||
|
||||
class IndexHostAlertTests(TestCase):
|
||||
def setUp(self):
|
||||
User = get_user_model()
|
||||
self.user = User.objects.create_user(username="tenant", password="pwd")
|
||||
self.mine = Compute.objects.create(name="mine-host", hostname="10.0.0.1", type=1, login="u", password="p")
|
||||
self.other = Compute.objects.create(name="other-host", hostname="10.0.0.2", type=1, login="u", password="p")
|
||||
vm = Instance.objects.create(compute=self.mine, name="tenant-vm", uuid="11111111-1111-1111-1111-111111111111")
|
||||
UserInstance.objects.create(instance=vm, user=self.user)
|
||||
|
||||
def get(self, user, down):
|
||||
def status(compute):
|
||||
return compute.name not in down
|
||||
|
||||
def error(compute):
|
||||
return None if status(compute) else f"Connection Failed: no route to {compute.hostname}"
|
||||
|
||||
self.client.force_login(user)
|
||||
with patch("instances.views.utils.refr"), patch.object(Compute, "status", property(status)), patch.object(
|
||||
Compute, "connection_error", property(error)
|
||||
), patch.object(Instance, "info", new_callable=PropertyMock, return_value=None):
|
||||
response = self.client.get(reverse("instances:index"))
|
||||
self.assertEqual(response.status_code, 200)
|
||||
return response
|
||||
|
||||
def test_user_sees_no_infrastructure_details(self):
|
||||
response = self.get(self.user, down={"mine-host", "other-host"})
|
||||
self.assertContains(response, USER_MESSAGE, count=1)
|
||||
for text in ("mine-host", "other-host", "10.0.0.1", "10.0.0.2", "Connection Failed"):
|
||||
self.assertNotContains(response, text)
|
||||
|
||||
def test_user_is_not_told_about_hosts_without_their_vms(self):
|
||||
response = self.get(self.user, down={"other-host"})
|
||||
self.assertNotContains(response, USER_MESSAGE)
|
||||
self.assertNotContains(response, "other-host")
|
||||
|
||||
def test_superuser_sees_each_host_and_its_error(self):
|
||||
admin = get_user_model().objects.create_superuser(username="root", password="pwd", email="r@example.com")
|
||||
response = self.get(admin, down={"mine-host", "other-host"})
|
||||
self.assertContains(response, "Connection Failed: no route to 10.0.0.1")
|
||||
self.assertContains(response, "Connection Failed: no route to 10.0.0.2")
|
||||
self.assertNotContains(response, USER_MESSAGE)
|
||||
|
||||
def test_global_viewer_sees_each_host_and_its_error(self):
|
||||
viewer = get_user_model().objects.create_user(username="viewer", password="pwd")
|
||||
viewer.user_permissions.add(Permission.objects.get(codename="view_instances"))
|
||||
response = self.get(viewer, down={"other-host"})
|
||||
self.assertContains(response, "Connection Failed: no route to 10.0.0.2")
|
||||
|
|
@ -58,15 +58,23 @@ def index(request):
|
|||
utils.refr(compute)
|
||||
prefetch_related_objects(computes, "instance_set__userinstance_set")
|
||||
|
||||
if request.user.is_superuser or request.user.has_perm("instances.view_instances"):
|
||||
show_hosts = request.user.is_superuser or request.user.has_perm("instances.view_instances")
|
||||
own_hosts_down = False
|
||||
if show_hosts:
|
||||
instances = Instance.objects.all().prefetch_related("userinstance_set")
|
||||
else:
|
||||
instances = Instance.objects.filter(
|
||||
userinstance__user=request.user
|
||||
).prefetch_related("userinstance_set")
|
||||
# other users get no host names or connection errors, only whether
|
||||
# a host of their own VMs is down
|
||||
own = set(instances.values_list("compute_id", flat=True))
|
||||
own_hosts_down = any(c.status is not True for c in computes if c.pk in own)
|
||||
|
||||
return render(
|
||||
request, "allinstances.html", {"computes": computes, "instances": instances}
|
||||
request,
|
||||
"allinstances.html",
|
||||
{"computes": computes, "instances": instances, "show_hosts": show_hosts, "own_hosts_down": own_hosts_down},
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue