From 06595e0a5534b38985534db81a064efb6028e324 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 25 Aug 2026 16:02:52 -0700 Subject: [PATCH] wireguard: Improve reliability functional tests by waiting after ops Tests: - Functional tests run much more reliably. Although there were several failures still observed due to following reasons: - Due to FreedomBox service receiving SIGSEGV (presumably during NM DBus operations). - Error something like Invalid UTF-8: setting not known during removal of a client. - Unknown failures caused by newly added clients not showing up on the page. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- .../wireguard/tests/test_functional.py | 72 ++++++++++----- freedombox/modules/wireguard/utils.py | 91 ++++++++++++++++++- freedombox/modules/wireguard/views.py | 6 +- 3 files changed, 139 insertions(+), 30 deletions(-) diff --git a/freedombox/modules/wireguard/tests/test_functional.py b/freedombox/modules/wireguard/tests/test_functional.py index 55dfe1be3..5a58a7cc1 100644 --- a/freedombox/modules/wireguard/tests/test_functional.py +++ b/freedombox/modules/wireguard/tests/test_functional.py @@ -73,23 +73,35 @@ class TestWireguardApp(functional.BaseAppTests): with functional.wait_for_page_update(browser): start_server_button.first.click() - browser.find_by_css('.btn-add-client').first.click() + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-add-client').first.click() + browser.find_by_id('id_public_key').fill(key) functional.submit(browser, form_class='form-add-client') def _edit_client(self, browser, key1, key2): """Edit a client""" functional.nav_to_module(browser, 'wireguard') - browser.links.find_by_href(self._get_client_href(key1)).first.click() - browser.find_by_css('.btn-edit-client').first.click() + with functional.wait_for_page_update(browser): + browser.links.find_by_href( + self._get_client_href(key1)).first.click() + + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-edit-client').first.click() + browser.find_by_id('id_public_key').fill(key2) functional.submit(browser, form_class='form-edit-client') def _delete_client(self, browser, key): """Delete a client""" functional.nav_to_module(browser, 'wireguard') - browser.links.find_by_href(self._get_client_href(key)).first.click() - browser.find_by_css('.btn-delete-client').first.click() + with functional.wait_for_page_update(browser): + browser.links.find_by_href( + self._get_client_href(key)).first.click() + + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-delete-client').first.click() + functional.submit(browser, form_class='form-delete-client') def test_add_edit_delete_client(self, session_browser): @@ -124,37 +136,38 @@ class TestWireguardApp(functional.BaseAppTests): with functional.wait_for_page_update(session_browser): start_server_button.first.click() - session_browser.find_by_css('.btn-auto-add-client').first.click() + with functional.wait_for_page_update(session_browser): + session_browser.find_by_css('.btn-auto-add-client').first.click() client_pubkey = session_browser.find_by_css( - '.pubkey-val').first.text.strip() + '.pubkey-val').first.text.strip() # Verify private key reveal - privkey_reveal = session_browser.find_by_css( - '.privkey-val') + privkey_reveal = session_browser.find_by_css('.privkey-val') assert privkey_reveal, "Private key reveal should be present" privkey_reveal.click() client_privkey = session_browser.find_by_css( - '.privkey-val').text.splitlines()[1] + '.privkey-val').text.splitlines()[1] assert len(client_privkey) == 44, (("Private key should be base64 " "(44 chars)")) # Verify config download and QR links download_link = session_browser.links.find_by_href( - '/freedombox/apps/wireguard/client/auto-add/action/download/') + '/freedombox/apps/wireguard/client/auto-add/action/download/') qr_link = session_browser.links.find_by_href( - '/freedombox/apps/wireguard/client/auto-add/action/qr/') + '/freedombox/apps/wireguard/client/auto-add/action/qr/') assert download_link, "Download config link should exist" assert qr_link, "QR code link should exist" # Submit to add the client with functional.wait_for_page_update(session_browser): session_browser.find_by_css( - '.btn-auto-add-connection').first.click() + '.btn-auto-add-connection').first.click() # Verify client was added successfully - assert self._client_exists(session_browser, client_pubkey), (( - "Auto-generated client should exist")) + assert self._client_exists( + session_browser, + client_pubkey), (("Auto-generated client should exist")) # Clean up self._delete_client(session_browser, client_pubkey) @@ -179,8 +192,10 @@ class TestWireguardApp(functional.BaseAppTests): return browser.find_by_css(f'tr.{key} > td').first.text functional.nav_to_module(browser, 'wireguard') - href = self._get_server_href(browser, config['peer_public_key']) - href.first.click() + with functional.wait_for_page_update(browser): + href = self._get_server_href(browser, config['peer_public_key']) + href.first.click() + assert get_value('peer-endpoint') == config['peer_endpoint'] assert get_value('peer-public-key') == config['peer_public_key'] assert get_value('server-ip-address-and-network' @@ -192,7 +207,9 @@ class TestWireguardApp(functional.BaseAppTests): def _add_server(browser, config): """Add a server.""" functional.nav_to_module(browser, 'wireguard') - browser.find_by_css('.btn-add-server').first.click() + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-add-server').first.click() + browser.find_by_id('id_peer_endpoint').fill(config['peer_endpoint']) browser.find_by_id('id_peer_public_key').fill( config['peer_public_key']) @@ -205,9 +222,13 @@ class TestWireguardApp(functional.BaseAppTests): def _edit_server(self, browser, config1, config2): """Edit a server.""" functional.nav_to_module(browser, 'wireguard') - self._get_server_href(browser, - config1['peer_public_key']).first.click() - browser.find_by_css('.btn-edit-server').first.click() + with functional.wait_for_page_update(browser): + href = self._get_server_href(browser, config1['peer_public_key']) + href.first.click() + + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-edit-server').first.click() + browser.find_by_id('id_peer_endpoint').fill(config2['peer_endpoint']) browser.find_by_id('id_peer_public_key').fill( config2['peer_public_key']) @@ -220,8 +241,13 @@ class TestWireguardApp(functional.BaseAppTests): def _delete_server(self, browser, config): """Delete a server""" functional.nav_to_module(browser, 'wireguard') - self._get_server_href(browser, config['peer_public_key']).first.click() - browser.find_by_css('.btn-delete-server').first.click() + with functional.wait_for_page_update(browser): + href = self._get_server_href(browser, config['peer_public_key']) + href.first.click() + + with functional.wait_for_page_update(browser): + browser.find_by_css('.btn-delete-server').first.click() + functional.submit(browser, form_class='form-delete-server') def test_add_edit_delete_server(self, session_browser): diff --git a/freedombox/modules/wireguard/utils.py b/freedombox/modules/wireguard/utils.py index 09194a431..9032e73ed 100644 --- a/freedombox/modules/wireguard/utils.py +++ b/freedombox/modules/wireguard/utils.py @@ -79,8 +79,7 @@ def get_nm_info(): settings_ipv6 = connection.get_setting_ip6_config() ip_address, ip_address_and_network = _get_nm_address_info( - settings_ipv4, settings_ipv6 - ) + settings_ipv4, settings_ipv6) info['ip_address'] = ip_address info['ip_address_and_network'] = ip_address_and_network @@ -162,7 +161,11 @@ def delete_connections(): connection.delete() -def _get_public_key_from_private_key(private_key): +def _get_public_key_from_private_key(private_key: str | None) -> str | None: + """Return public key from private key running wg command.""" + if not private_key: + return None + process = subprocess.run(['wg', 'pubkey'], check=True, capture_output=True, input=private_key.encode()) return process.stdout.decode().strip() @@ -207,6 +210,28 @@ def add_server(settings): network.add_connection(settings) + # Wait upto 10 seconds for server to appear + public_key = settings['wireguard']['peer_public_key'] + for _ in range(10): + info = get_info() + if info['my_client'] and info['my_client']['servers']: + found = False + for _, server in info['my_client']['servers'].items(): + for _, peer in server['peers'].items(): + if peer['public_key'] == public_key: + found = True + break + + if found: + break + + if found: + break + + time.sleep(1) + else: + logging.warning('Could not add server') + def edit_server(interface, settings): """Edit information for connecting to a server.""" @@ -220,6 +245,22 @@ def edit_server(interface, settings): network.reactivate_connection(connection.get_uuid()) +def delete_server(interface): + """Delete information for connecting to a server.""" + connection = network.get_connection_by_interface_name(interface) + network.delete_connection(connection.get_uuid()) + + # Wait upto 10 seconds for server to disappear + for _ in range(10): + info = get_nm_info() + if interface not in info: + break + + time.sleep(1) + else: + logging.warning('Could not delete server') + + def setup_server(): """Setup a server connection that clients can connect to.""" app = app_module.App.get('wireguard') @@ -249,6 +290,16 @@ def setup_server(): network.add_connection(settings) logger.info('Created new WireGuard server connection') + # Wait upto 10 seconds for server to appear + for _ in range(10): + info = get_info() + if info['my_server'] and info['my_server']['public_key']: + break + + time.sleep(1) + else: + logging.warning('Could not add server') + def _get_next_available_ip_address(settings): """Get the next available IP address to allocate to a client.""" @@ -311,6 +362,23 @@ def add_client(public_key): connection.commit_changes(True) network.reactivate_connection(connection.get_uuid()) + # Wait upto 10 seconds for client to appear + for _ in range(10): + info = get_info() + if info['my_server'] and info['my_server']['peers']: + found = False + for _, peer in info['my_server']['peers'].items(): + if peer['public_key'] == public_key: + found = True + break + + if found: + break + + time.sleep(1) + else: + logging.warning('Could not add client %s', public_key) + def remove_client(public_key): """Remove permission for a client to connect our server.""" @@ -325,6 +393,23 @@ def remove_client(public_key): connection.commit_changes(True) network.reactivate_connection(connection.get_uuid()) + # Wait upto 10 seconds for client to disappear + for _ in range(10): + info = get_info() + if info['my_server']: + found = False + for _, peer in info['my_server']['peers'].items(): + if peer['public_key'] == public_key: + found = True + break + + if not found: + break + + time.sleep(1) + else: + logging.warning('Could not remove client %s', public_key) + def build_client_config(client_ip: str, client_privkey: str, client_pubkey: str, endpoint: str) -> str: diff --git a/freedombox/modules/wireguard/views.py b/freedombox/modules/wireguard/views.py index 0ff323bc0..085d47ebd 100644 --- a/freedombox/modules/wireguard/views.py +++ b/freedombox/modules/wireguard/views.py @@ -3,8 +3,8 @@ Views for WireGuard application. """ -from io import BytesIO import urllib.parse +from io import BytesIO from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin @@ -14,7 +14,6 @@ from django.urls import reverse_lazy from django.utils.translation import gettext as _ from django.views.generic import FormView, TemplateView, View -from freedombox import network from freedombox.modules.names.components import DomainName from freedombox.views import AppView @@ -381,8 +380,7 @@ class DeleteServerView(SuccessMessageMixin, TemplateView): def post(self, request, interface): """Delete the server.""" - connection = network.get_connection_by_interface_name(interface) - network.delete_connection(connection.get_uuid()) + utils.delete_server(interface) messages.success(request, _('Server deleted.')) return redirect('wireguard:index')