Skip to content

Commit 981c890

Browse files
pandafynemesifier
authored andcommitted
[fix] Invalidate Config checksum after subnet provisioning
Bug: Previously, the `post_provision_handler` in BaseSubnetDivisionRuleType did not update the associated Config checksum after subnets or IP addresses were provisioned. This could lead to inconsistent state where the Config's checksum and checksum_db diverged, especially when VPN templates were switched or updated, causing change detection and dependent logic to fail. Fix: - Converted `post_provision_handler` to a classmethod so subclasses can extend it safely. - Invalidate the backend instance cache for the Config. - Update `checksum_db` if the current checksum differs. - In VPNSubnetDivisionRuleType, call the superclass handler to ensure checksum and cache are updated automatically when a VPN template is provisioned. [backport 1.2] (cherry picked from commit 76e1527)
1 parent d6a4727 commit 981c890

File tree

3 files changed

+168
-13
lines changed

3 files changed

+168
-13
lines changed

openwisp_controller/subnet_division/rule_types/base.py

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,36 @@ def _provision_receiver():
7979
def destroyer_receiver(cls, instance, **kwargs):
8080
cls.destroy_provisioned_subnets_ips(instance, **kwargs)
8181

82-
@staticmethod
83-
def post_provision_handler(instance, provisioned, **kwargs):
82+
@classmethod
83+
def post_provision_handler(cls, instance, provisioned, **kwargs):
8484
"""
85-
This method should be overridden in inherited rule types to
86-
perform any operation on provisioned subnets and IP addresses.
87-
:param instance: object that triggered provisioning
88-
:param provisioned: dictionary containing subnets and IP addresses
89-
provisioned, None if nothing is provisioned
85+
Hook for post-provisioning actions on subnets and IP addresses.
86+
87+
This method is intended to be extended by subclasses of rule types
88+
to perform custom operations after subnets and IPs are provisioned.
89+
90+
Subnet provisioning is executed asynchronously in Celery workers.
91+
If the device configuration references variables provided by the
92+
subnet division rule, the current checksum may have been computed
93+
using variable names instead of their provisioned values. In such cases,
94+
`Config.checksum_db` (which tracks persisted configuration changes)
95+
must be updated to reflect the actual provisioned values, and the
96+
checksum cache invalidated to avoid stale data.
97+
98+
:param instance: The object that triggered the provisioning.
99+
:param provisioned: Dictionary containing provisioned subnets and IPs,
100+
or None if no provisioning occurred.
90101
"""
91-
pass
102+
if not provisioned:
103+
return
104+
config = cls.get_config(instance)
105+
config._invalidate_backend_instance_cache()
106+
current_checksum = config.checksum
107+
if current_checksum != config.checksum_db:
108+
# Update checksum using the UPDATE query to avoid sending
109+
# unnecessary signals that may be triggered by `save()` method.
110+
config._update_checksum_db(current_checksum)
111+
config.invalidate_checksum_cache()
92112

93113
@staticmethod
94114
def subnet_provisioned_signal_emitter(instance, provisioned):

openwisp_controller/subnet_division/rule_types/vpn.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ def provision_for_existing_objects(cls, rule_obj):
4141
for vpn_client in qs:
4242
cls.provision_receiver(instance=vpn_client, created=True)
4343

44-
@staticmethod
45-
def post_provision_handler(instance, provisioned, **kwargs):
44+
@classmethod
45+
def post_provision_handler(cls, instance, provisioned, **kwargs):
46+
super().post_provision_handler(instance, provisioned, **kwargs)
4647
# Assign the first provisioned IP address to the VPNClient
4748
# only when subnets and IPs have been provisioned
4849
if provisioned and provisioned["ip_addresses"]:

openwisp_controller/subnet_division/tests/test_admin.py

Lines changed: 137 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
from unittest.mock import patch
22

3-
from django.test import TestCase
3+
from django.test import TestCase, TransactionTestCase
44
from django.urls import reverse
55
from swapper import load_model
66

7-
from openwisp_controller.config.tests.utils import TestWireguardVpnMixin
7+
from openwisp_controller.config.tests.test_admin import TestDeviceAdminMixin
8+
from openwisp_controller.config.tests.utils import (
9+
TestVpnX509Mixin,
10+
TestWireguardVpnMixin,
11+
)
812
from openwisp_users.tests.utils import TestMultitenantAdminMixin
913

10-
from .helpers import SubnetDivisionAdminTestMixin
14+
from .helpers import SubnetDivisionAdminTestMixin, SubnetDivisionTestMixin
1115

1216
Subnet = load_model("openwisp_ipam", "Subnet")
1317
Device = load_model("config", "Device")
18+
Config = load_model("config", "Config")
1419

1520

1621
class TestSubnetAdmin(
@@ -257,3 +262,132 @@ def test_delete_device(self):
257262
)
258263
self.assertEqual(subnet_response.status_code, 200)
259264
self.assertContains(subnet_response, self.config.device.name, 1)
265+
266+
267+
class TestTransactionDeviceAdmin(
268+
SubnetDivisionTestMixin,
269+
TestVpnX509Mixin,
270+
TestDeviceAdminMixin,
271+
TransactionTestCase,
272+
):
273+
ipam_label = "openwisp_ipam"
274+
config_label = "config"
275+
276+
def test_vpn_template_switch_checksum_db(self):
277+
admin = self._create_admin()
278+
self.client.force_login(admin)
279+
org = self._get_org()
280+
vpn1_subnet = self._get_master_subnet(organization=org, subnet="10.0.0.0/24")
281+
self._get_vpn_subdivision_rule(
282+
number_of_ips=1,
283+
number_of_subnets=1,
284+
organization=org,
285+
master_subnet=vpn1_subnet,
286+
label="VPN1",
287+
)
288+
vpn1 = self._create_vpn(name="vpn1", organization=org, subnet=vpn1_subnet)
289+
vpn2_subnet = self._get_master_subnet(organization=org, subnet="10.0.1.0/24")
290+
self._get_vpn_subdivision_rule(
291+
number_of_ips=1,
292+
number_of_subnets=1,
293+
organization=org,
294+
master_subnet=vpn2_subnet,
295+
label="VPN2",
296+
)
297+
vpn2 = self._create_vpn(name="vpn2", organization=org, subnet=vpn2_subnet)
298+
vpn1_template = self._create_template(
299+
organization=org,
300+
name="vpn1-template",
301+
type="vpn",
302+
vpn=vpn1,
303+
default_values={
304+
"VPN1_subnet1_ip1": "10.0.0.1",
305+
"VPN1_prefix": "24",
306+
"ifname": "tun0",
307+
},
308+
auto_cert=True,
309+
config={},
310+
)
311+
vpn1_template.config["openvpn"][0]["dev"] = "{{ ifname }}"
312+
vpn1_template.config.update(
313+
{
314+
"network": [
315+
{
316+
"config_name": "interface",
317+
"config_value": "lan",
318+
"ipaddr": "{{ VPN1_subnet1_ip1 }}",
319+
"netmask": "255.255.255.240",
320+
}
321+
],
322+
}
323+
)
324+
vpn1_template.full_clean()
325+
vpn1_template.save()
326+
vpn2_template = self._create_template(
327+
organization=org,
328+
name="vpn2-template",
329+
type="vpn",
330+
vpn=vpn2,
331+
default_values={
332+
"VPN2_subnet1_ip1": "10.0.1.1",
333+
"VPN2_prefix": "32",
334+
"ifname": "tun1",
335+
},
336+
auto_cert=True,
337+
config={},
338+
)
339+
vpn2_template.config["openvpn"][0]["dev"] = "{{ ifname }}"
340+
vpn2_template.config.update(
341+
{
342+
"network": [
343+
{
344+
"config_name": "interface",
345+
"config_value": "lan",
346+
"ipaddr": "{{ VPN2_subnet1_ip1 }}",
347+
"netmask": "255.255.255.240",
348+
}
349+
],
350+
}
351+
)
352+
vpn2_template.full_clean()
353+
vpn2_template.save()
354+
default_template = self._create_template(
355+
name="default-template",
356+
default=True,
357+
)
358+
path = reverse(f"admin:{self.config_label}_device_add")
359+
params = self._get_device_params(org=org)
360+
params.update(
361+
{"config-0-templates": f"{default_template.pk},{vpn1_template.pk}"}
362+
)
363+
response = self.client.post(path, data=params, follow=True)
364+
self.assertEqual(response.status_code, 200)
365+
config = Config.objects.get(device__name=params["name"])
366+
config.refresh_from_db()
367+
config._invalidate_backend_instance_cache()
368+
initial_checksum = config.checksum
369+
self.assertEqual(config.checksum_db, initial_checksum)
370+
self.assertEqual(config.vpnclient_set.count(), 1)
371+
self.assertEqual(config.vpnclient_set.first().vpn, vpn1)
372+
373+
path = reverse(
374+
f"admin:{self.config_label}_device_change", args=[config.device_id]
375+
)
376+
params.update(
377+
{
378+
"config-0-templates": f"{default_template.pk},{vpn2_template.pk}",
379+
"config-0-id": str(config.pk),
380+
"config-0-device": str(config.device_id),
381+
"config-INITIAL_FORMS": 1,
382+
"_continue": True,
383+
}
384+
)
385+
response = self.client.post(path, data=params, follow=True)
386+
self.assertEqual(response.status_code, 200)
387+
config.refresh_from_db()
388+
config._invalidate_backend_instance_cache()
389+
self.assertEqual(config.status, "modified")
390+
self.assertEqual(config.vpnclient_set.count(), 1)
391+
self.assertEqual(config.vpnclient_set.first().vpn, vpn2)
392+
self.assertNotEqual(config.checksum, initial_checksum)
393+
self.assertEqual(config.checksum, config.checksum_db)

0 commit comments

Comments
 (0)