Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion openwisp_controller/config/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,14 @@ def _update_config(self, device, config_data):
with transaction.atomic():
vpn_list = config.templates.filter(type="vpn").values_list("vpn")
if vpn_list:
config.vpnclient_set.exclude(vpn__in=vpn_list).delete()
# Per-instance delete ensures post_delete signals fire
# (cache invalidation, cert revocation, IP release).
for vpnclient in (
config.vpnclient_set.exclude(vpn__in=vpn_list)
.select_related("vpn", "cert", "ip")
.iterator()
):
vpnclient.delete()
config.templates.set(config_templates, clear=True)
config.save()
except ValidationError as error:
Expand Down
22 changes: 18 additions & 4 deletions openwisp_controller/config/base/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,13 @@ def manage_vpn_clients(cls, action, instance, pk_set, **kwargs):
if instance.is_deactivating_or_deactivated():
# If the device is deactivated or in the process of deactivating, then
# delete all vpn clients and return.
instance.vpnclient_set.all().delete()
# Per-instance delete ensures post_delete signals fire
# (cache invalidation, cert revocation, IP release).
with transaction.atomic():
for vpnclient in instance.vpnclient_set.select_related(
"vpn", "cert", "ip"
).iterator():
vpnclient.delete()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return

vpn_client_model = cls.vpn.through
Expand Down Expand Up @@ -370,9 +376,17 @@ def manage_vpn_clients(cls, action, instance, pk_set, **kwargs):
# signal is triggered again—after all templates, including the required
# ones, have been fully added. At that point, we can identify and
# delete VpnClient objects not linked to the final template set.
instance.vpnclient_set.exclude(
template_id__in=instance.templates.values_list("id", flat=True)
).delete()
# Per-instance delete ensures post_delete signals fire
# (cache invalidation, cert revocation, IP release).
with transaction.atomic():
for vpnclient in (
instance.vpnclient_set.exclude(
template_id__in=instance.templates.values_list("id", flat=True)
)
.select_related("vpn", "cert", "ip")
.iterator()
):
vpnclient.delete()

if action == "post_add":
for template in templates.filter(type="vpn"):
Expand Down
39 changes: 39 additions & 0 deletions openwisp_controller/config/tests/test_vpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,45 @@ def test_vpn_peers_changed(self):
device.config.templates.remove(template)
handler.assert_called_once()

def test_template_removal_fires_post_delete_signals(self):
"""Regression test for #1221: removing a VPN template must trigger
VpnClient.post_delete so that the peer cache is invalidated,
and IP addresses are released."""
device, vpn, template = self._create_wireguard_vpn_template()
vpn_client = device.config.vpnclient_set.first()
self.assertIsNotNone(vpn_client)
vpn_client_pk = vpn_client.pk
ip_pk = vpn_client.ip.pk

with self.subTest("peer cache invalidated on template removal"):
with catch_signal(vpn_peers_changed) as handler:
device.config.templates.remove(template)
handler.assert_called_once()

with self.subTest("VpnClient deleted"):
self.assertFalse(VpnClient.objects.filter(pk=vpn_client_pk).exists())

with self.subTest("IP address released"):
self.assertFalse(IpAddress.objects.filter(pk=ip_pk).exists())

def test_deactivation_fires_post_delete_signals(self):
"""Regression test for #1221: deactivating a device must trigger
VpnClient.post_delete for each client (not bulk QuerySet.delete)."""
device, vpn, template = self._create_wireguard_vpn_template()
vpn_client = device.config.vpnclient_set.first()
self.assertIsNotNone(vpn_client)
ip_pk = vpn_client.ip.pk

with catch_signal(vpn_peers_changed) as handler:
device.deactivate()
handler.assert_called_once()

Comment thread
coderabbitai[bot] marked this conversation as resolved.
# Verify cleanup happened
self.assertEqual(device.config.vpnclient_set.count(), 0)

with self.subTest("IP address released"):
self.assertFalse(IpAddress.objects.filter(pk=ip_pk).exists())


class TestVxlan(BaseTestVpn, TestVxlanWireguardVpnMixin, TestCase):
def test_vxlan_config_creation(self):
Expand Down
Loading