Skip to content

Commit

Permalink
[change] Ensure subnet can have multiple subnet division rules
Browse files Browse the repository at this point in the history
The "rule" field of SubnetDivisionIndex will be set to "None" if
the rule is deleted. Subnets related to indexes with rule "None"
will be deleted.
  • Loading branch information
pandafy committed Jan 19, 2022
1 parent 8cb3083 commit 2873932
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 31 deletions.
20 changes: 12 additions & 8 deletions openwisp_controller/subnet_division/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,18 @@ def update_related_objects(self):
)

def delete_provisioned_subnets(self):
# Deleting an object of SubnetDivisionRule will automatically delete
# it's related SubnetDivisionIndex objects due to "on_delete=CASCADE".
# Similarly, deleting a Subnet object will automatically delete IpAddress
# objects related to it.
# Hence, this method only executes query of deleting Subnets provisioned by
# this corresponding SubnetDivisionRule.
# Deleting an object of SubnetDivisionRule will set the rule field
# of related SubnetDivisionIndex to "None" due to "on_delete=SET_NULL".
# These indexes are used delete subnets that were provisioned by the
# deleted rule. Deleting a Subnet object will automatically delete
# related IpAddress objects.
Subnet = swapper.load_model('openwisp_ipam', 'Subnet')
SubnetDivisionIndex = swapper.load_model(
'subnet_division', 'SubnetDivisionIndex'
)

Subnet.objects.filter(
organization_id=self.organization_id, name__startswith=self.label
id__in=SubnetDivisionIndex.objects.filter(rule_id=None).values('subnet_id')
).delete()

@classmethod
Expand Down Expand Up @@ -241,7 +244,8 @@ class AbstractSubnetDivisionIndex(models.Model):
)
rule = models.ForeignKey(
swapper.get_model_name('subnet_division', 'SubnetDivisionRule'),
on_delete=models.CASCADE,
null=True,
on_delete=models.SET_NULL,
)
config = models.ForeignKey(
swapper.get_model_name('config', 'Config'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.1.13 on 2021-09-27 19:25

import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('subnet_division', '0003_related_field_allow_blank'),
]

operations = [
migrations.AlterField(
model_name='subnetdivisionindex',
name='rule',
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.SET_NULL,
to=settings.SUBNET_DIVISION_SUBNETDIVISIONRULE_MODEL,
),
),
]
45 changes: 31 additions & 14 deletions openwisp_controller/subnet_division/rule_types/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,25 @@ def provision_receiver(cls, instance, **kwargs):
def _provision_receiver():
# If any of following operations fail, the database transaction
# should fail/rollback.
provisioned = cls.create_subnets_ips(instance, rule_type, **kwargs)
cls.post_provision_handler(instance, provisioned, **kwargs)
cls.subnet_provisioned_signal_emitter(instance, provisioned)

if kwargs['created'] is False:
# This method is also called by "provision_for_existing_objects"
# which passes the "rule" keyword argument. In such case,
# provisioning should be only triggered for received rule.
if 'rule' in kwargs:
rules = [kwargs['rule']]
else:
try:
rules = cls.get_subnet_division_rules(instance)
except (AttributeError, ObjectDoesNotExist):
return
for rule in rules:
provisioned = cls.create_subnets_ips(instance, rule, **kwargs)
cls.post_provision_handler(instance, provisioned, **kwargs)
cls.subnet_provisioned_signal_emitter(instance, provisioned)

if not cls.should_create_subnets_ips(instance, **kwargs):
return
rule_type = f'{cls.__module__}.{cls.__name__}'

transaction.on_commit(_provision_receiver)

@classmethod
Expand Down Expand Up @@ -99,16 +111,9 @@ def provision_for_existing_objects(cls, rule_obj):
raise NotImplementedError()

@classmethod
def create_subnets_ips(cls, instance, rule_type, **kwargs):
if not cls.should_create_subnets_ips(instance, **kwargs):
return
def create_subnets_ips(cls, instance, division_rule, **kwargs):
try:
organization_id = cls.get_organization(instance)
config = cls.get_config(instance)
subnet = cls.get_subnet(instance)
division_rule = subnet.subnetdivisionrule_set.get(
organization_id__in=(organization_id, None), type=rule_type,
)
except (AttributeError, ObjectDoesNotExist):
return

Expand All @@ -132,6 +137,15 @@ def get_organization(cls, instance):
def get_subnet(cls, instance):
return attrgetter(cls.subnet_path)(instance)

@classmethod
def get_subnet_division_rules(cls, instance):
rule_type = f'{cls.__module__}.{cls.__name__}'
organization_id = cls.get_organization(instance)
subnet = cls.get_subnet(instance)
return subnet.subnetdivisionrule_set.filter(
organization_id__in=(organization_id, None), type=rule_type,
).iterator()

@classmethod
def get_config(cls, instance):
if cls.config_path == 'self':
Expand Down Expand Up @@ -232,5 +246,8 @@ def destroy_provisioned_subnets_ips(cls, instance, **kwargs):
# Deleting related subnets automatically deletes related IpAddress
# and SubnetDivisionIndex objects
config = cls.get_config(instance)
subnet_ids = config.subnetdivisionindex_set.values_list('subnet_id')
rule_type = f'{cls.__module__}.{cls.__name__}'
subnet_ids = config.subnetdivisionindex_set.filter(
rule__type=rule_type
).values_list('subnet_id')
Subnet.objects.filter(id__in=subnet_ids).delete()
17 changes: 10 additions & 7 deletions openwisp_controller/subnet_division/rule_types/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ class DeviceSubnetDivisionRuleType(BaseSubnetDivisionRuleType):

@classmethod
def get_subnet(cls, instance):
return instance.device.organization.subnetdivisionrule_set.get(
type=f'{cls.__module__}.{cls.__name__}'
).master_subnet
pass

@classmethod
def get_subnet_division_rules(cls, instance):
rule_type = f'{cls.__module__}.{cls.__name__}'
return instance.device.organization.subnetdivisionrule_set.filter(
type=rule_type
).iterator()

@classmethod
def should_create_subnets_ips(cls, instance, **kwargs):
return 'created' in kwargs and kwargs['created']
return kwargs.get('created', False)

@staticmethod
def destroy_provisioned_subnets_ips(instance, **kwargs):
Expand All @@ -44,6 +49,4 @@ def provision_for_existing_objects(cls, rule_obj):
.filter(device__organization_id=rule_obj.organization_id)
.iterator()
):
cls.provision_receiver(
config, created=True,
)
cls.provision_receiver(config, created=True, rule=rule_obj)
2 changes: 1 addition & 1 deletion openwisp_controller/subnet_division/rule_types/vpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class VpnSubnetDivisionRuleType(BaseSubnetDivisionRuleType):

@classmethod
def should_create_subnets_ips(cls, instance, **kwargs):
return kwargs['created']
return kwargs.get('created', False)

@classmethod
def provision_for_existing_objects(cls, rule_obj):
Expand Down
41 changes: 41 additions & 0 deletions openwisp_controller/subnet_division/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,47 @@ def test_subnet_division_index_validation(self):
index.config = self.config
index.full_clean()

def test_single_subnet_multiple_vpn_rule(self):
rule1 = self._get_vpn_subdivision_rule()
rule2 = self._get_vpn_subdivision_rule(label='VPN')
self.config.templates.add(self.template)
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
rule1.number_of_subnets + rule2.number_of_subnets,
)
rule2.delete()
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
rule1.number_of_subnets,
)

def test_single_subnet_multiple_device_rule(self):
rule1 = self._get_device_subdivision_rule(label='LAN1')
rule2 = self._get_device_subdivision_rule(label='LAN2')
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
rule1.number_of_subnets + rule2.number_of_subnets,
)
rule2.delete()
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
rule1.number_of_subnets,
)

def test_single_subnet_vpn_device_rule(self):
device_rule = self._get_device_subdivision_rule(label='LAN')
vpn_rule = self._get_vpn_subdivision_rule(label='VPN')
self.config.templates.add(self.template)
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
device_rule.number_of_subnets + vpn_rule.number_of_subnets,
)
self.config.templates.remove(self.template)
self.assertEqual(
self.subnet_query.exclude(id=self.master_subnet.id).count(),
device_rule.number_of_subnets,
)


class TestCeleryTasks(TestCase):
def test_subnet_division_rule_does_not_exist(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ class Migration(migrations.Migration):
(
'rule',
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
null=True,
on_delete=django.db.models.deletion.SET_NULL,
to='sample_subnet_division.subnetdivisionrule',
),
),
Expand Down

0 comments on commit 2873932

Please sign in to comment.