diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index f07627e60..94a0c59fc 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -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: diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index efb42ff65..12e91e2a9 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -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() return vpn_client_model = cls.vpn.through @@ -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"): diff --git a/openwisp_controller/config/tests/test_vpn.py b/openwisp_controller/config/tests/test_vpn.py index 211ad4a06..d510df8d8 100644 --- a/openwisp_controller/config/tests/test_vpn.py +++ b/openwisp_controller/config/tests/test_vpn.py @@ -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() + + # 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):