Skip to content

Commit 4c35d82

Browse files
committed
[fix] Fix serializers vpn cleanup, add Wireguard IP test, update changelog openwisp#1221
- Fix serializers.py: vpn_list was built from old templates so .exclude(vpn__in=vpn_list) never matched the client being removed. Now uses new_vpn_ids from config_templates so clients for removed VPN templates are correctly deleted (including PATCH with empty list). - Add test_wireguard_vpnclient_ip_released_on_template_removal to verify that Wireguard VPN client IP addresses are released on template removal. - Expand API test to cover both "replace with generic" and "empty list" PATCH scenarios. - Add CHANGES.rst entry under Version 1.3.0 [unreleased]. Related to openwisp#1221 Co-Authored-By: mn-ram <mn-ram@users.noreply.github.com>
1 parent 7b202aa commit 4c35d82

File tree

4 files changed

+58
-25
lines changed

4 files changed

+58
-25
lines changed

CHANGES.rst

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ Changelog
44
Version 1.3.0 [unreleased]
55
--------------------------
66

7-
Work in progress.
7+
Bugfixes
8+
~~~~~~~~
9+
10+
- Fixed VPN peer cache desync when VPN template is removed from a device:
11+
``VpnClient`` objects are now deleted via per-instance ``delete()`` so that
12+
``post_delete`` signals fire, ensuring peer cache invalidation, certificate
13+
revocation, and IP address release `#1221
14+
<https://github.com/openwisp/openwisp-controller/issues/1221>`_
815

916
Version 1.2.2 [2026-03-06]
1017
--------------------------

openwisp_controller/config/api/serializers.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,15 @@ def _update_config(self, device, config_data):
200200
old_templates = list(config.templates.values_list("id", flat=True))
201201
if config_templates != old_templates:
202202
with transaction.atomic():
203-
vpn_list = config.templates.filter(type="vpn").values_list("vpn")
204-
if vpn_list:
205-
for vpnclient in (
206-
config.vpnclient_set.select_related("vpn", "cert", "ip")
207-
.exclude(vpn__in=vpn_list)
208-
.iterator()
209-
):
210-
vpnclient.delete()
203+
new_vpn_ids = Template.objects.filter(
204+
pk__in=config_templates, type="vpn"
205+
).values_list("vpn", flat=True)
206+
for vpnclient in (
207+
config.vpnclient_set.select_related("vpn", "cert", "ip")
208+
.exclude(vpn__in=new_vpn_ids)
209+
.iterator()
210+
):
211+
vpnclient.delete()
211212
config.templates.set(config_templates, clear=True)
212213
config.save()
213214
except ValidationError as error:

openwisp_controller/config/tests/test_api.py

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,9 @@ def test_device_patch_with_templates_of_different_org(self):
520520
)
521521

522522
def test_device_patch_vpn_template_removal_triggers_post_delete(self):
523-
"""Regression test for #1221: replacing a VPN template with a generic
524-
one via PATCH API must trigger VpnClient.post_delete so peer cache is
525-
invalidated and the certificate is revoked."""
523+
"""Regression test for #1221: removing VPN template via PATCH API must
524+
trigger VpnClient.post_delete so peer cache is invalidated and the
525+
certificate is revoked. Tests both empty and non-empty replacement."""
526526
org = self._get_org()
527527
vpn = self._create_vpn(organization=org)
528528
t_vpn = self._create_template(
@@ -531,20 +531,33 @@ def test_device_patch_vpn_template_removal_triggers_post_delete(self):
531531
t_generic = self._create_template(name="generic-test", organization=org)
532532
device = self._create_device(organization=org)
533533
config = self._create_config(device=device)
534-
config.templates.add(t_vpn)
535-
vpnclient = config.vpnclient_set.first()
536-
self.assertIsNotNone(vpnclient)
537-
cert_pk = vpnclient.cert.pk
538-
539534
path = reverse("config_api:device_detail", args=[device.pk])
540-
# PATCH replacing VPN template with a generic template
541-
data = {"config": {"templates": [str(t_generic.pk)]}}
542-
with patch.object(Vpn, "_invalidate_peer_cache") as mock_invalidate:
543-
response = self.client.patch(path, data, content_type="application/json")
544-
mock_invalidate.assert_called_once()
545-
self.assertEqual(response.status_code, 200)
546-
self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
547-
self.assertTrue(Cert.objects.get(pk=cert_pk).revoked)
535+
536+
with self.subTest("replace VPN template with generic template"):
537+
config.templates.set([t_vpn])
538+
vpnclient = config.vpnclient_set.first()
539+
self.assertIsNotNone(vpnclient)
540+
cert_pk = vpnclient.cert.pk
541+
data = {"config": {"templates": [str(t_generic.pk)]}}
542+
with patch.object(Vpn, "_invalidate_peer_cache") as mock_invalidate:
543+
response = self.client.patch(path, data, content_type="application/json")
544+
mock_invalidate.assert_called_once()
545+
self.assertEqual(response.status_code, 200)
546+
self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
547+
self.assertTrue(Cert.objects.get(pk=cert_pk).revoked)
548+
549+
with self.subTest("remove all templates (empty list)"):
550+
config.templates.set([t_vpn])
551+
vpnclient = config.vpnclient_set.first()
552+
self.assertIsNotNone(vpnclient)
553+
cert_pk = vpnclient.cert.pk
554+
data = {"config": {"templates": []}}
555+
with patch.object(Vpn, "_invalidate_peer_cache") as mock_invalidate:
556+
response = self.client.patch(path, data, content_type="application/json")
557+
mock_invalidate.assert_called_once()
558+
self.assertEqual(response.status_code, 200)
559+
self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
560+
self.assertTrue(Cert.objects.get(pk=cert_pk).revoked)
548561

549562
def test_device_change_organization_required_templates(self):
550563
org1 = self._create_org(name="org1")

openwisp_controller/config/tests/test_vpn.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,18 @@ def test_trigger_vpn_server_endpoint_invalid_vpn_id(self):
767767
f"VPN Server UUID: {vpn_id} does not exist."
768768
)
769769

770+
def test_wireguard_vpnclient_ip_released_on_template_removal(self):
771+
"""Regression test for #1221: when a Wireguard VPN template is removed,
772+
the allocated IP address must be released (deleted)."""
773+
device, vpn, template = self._create_wireguard_vpn_template()
774+
vpnclient = device.config.vpnclient_set.first()
775+
self.assertIsNotNone(vpnclient)
776+
self.assertIsNotNone(vpnclient.ip)
777+
ip_pk = vpnclient.ip.pk
778+
device.config.templates.remove(template)
779+
self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
780+
self.assertFalse(IpAddress.objects.filter(pk=ip_pk).exists())
781+
770782

771783
class TestWireguardTransaction(BaseTestVpn, TestWireguardVpnMixin, TransactionTestCase):
772784
mock_response = mock.Mock(spec=requests.Response)

0 commit comments

Comments
 (0)