[feature] Allow shared subnets to have non shared child subnets #90

Closes #90

openwisp_ipam/base/models.py

@@ -5,6 +5,7 @@
 import xlrd
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.db.models import Q
 from django.utils.translation import gettext_lazy as _
 from openwisp_users.mixins import ShareableOrgMixin
 from openwisp_utils.base import TimeStampedEditableModel
@@ -50,6 +51,7 @@ def clean(self):
             return
         self._validate_multitenant_uniqueness()
         self._validate_multitenant_master_subnet()
+        self._validate_multitenant_unique_child_subnet()
         self._validate_overlapping_subnets()
         self._validate_master_subnet_consistency()
 
@@ -81,20 +83,35 @@ def _validate_multitenant_master_subnet(self):
             return
         if self.master_subnet.organization:
             self._validate_org_relation('master_subnet', field_error='master_subnet')
-        elif self.organization:
+
+    def _validate_multitenant_unique_child_subnet(self):
+        if self.master_subnet is None or self.master_subnet.organization_id is not None:
+            return
+        qs = self._meta.model.objects.exclude(id=self.pk).filter(subnet=self.subnet)
+        if qs.exists():
             raise ValidationError(
                 {
-                    'master_subnet': _(
-                        'Please ensure that the organization of this subnet and '
-                        'the organization of the related subnet match.'
+                    'subnet': _(
+                        'This subnet is already assigned to another organization.'
                     )
                 }
             )
 
     def _validate_overlapping_subnets(self):
-        qs = self._meta.model.objects.filter(organization=self.organization).only(
-            'subnet'
-        )
+        organization_query = Q(organization_id=self.organization_id)
+        error_message = _('Subnet overlaps with {0}.')
+        if self.master_subnet:
+            if self.master_subnet.organization_id is None:
+                # The execution of above code implicitly ensures that
+                # organization of both master_subnet and current subnet are
+                # same. Otherwise, self._validate_multitenant_master_subnet
+                # would have raised a validation error
+                organization_query = Q()
+                error_message = _(
+                    'Subnet overlaps with a subnet of another organization.'
+                )
+
+        qs = self._meta.model.objects.filter(organization_query).only('subnet')
         # exclude parent subnets
         exclude = [self.pk]
         parent_subnet = self.master_subnet
@@ -105,9 +122,7 @@ def _validate_overlapping_subnets(self):
         qs = qs.exclude(pk__in=exclude).exclude(subnet=self.subnet)
         for subnet in qs.iterator():
             if ip_network(self.subnet).overlaps(subnet.subnet):
-                raise ValidationError(
-                    {'subnet': _('Subnet overlaps with %s.') % subnet.subnet}
-                )
+                raise ValidationError({'subnet': error_message.format(subnet.subnet)})
 
     def _validate_master_subnet_consistency(self):
         if not self.master_subnet:

openwisp_ipam/tests/test_models.py

@@ -188,21 +188,51 @@ def test_master_subnet_validation(self):
                     master_subnet=master, subnet='10.0.0.0/24', organization=None
                 )
             message_dict = context_manager.exception.message_dict
-            self.assertIn('master_subnet', message_dict)
             self.assertIn(error_message, message_dict['master_subnet'])
 
-        with self.subTest('shared master, org1 children: rejected'):
+        with self.subTest('shared master, org1 children: ok'):
             org1 = master.organization
             master.organization = None
             master.full_clean()
             master.save()
+            org1_subnet = self._create_subnet(
+                master_subnet=master, subnet='10.0.0.0/24', organization=org1,
+            )
+            org1_subnet.delete()
+
+        with self.subTest(
+            'shared master, org1 children, org2 overlapping children: reject'
+        ):
+            overlap_error_message = (
+                'Subnet overlaps with a subnet of another organization.'
+            )
+            duplicate_error_message = (
+                'This subnet is already assigned to another organization.'
+            )
+            org1 = self._get_org()
+            org2 = self._create_org(name='test')
+            master.organization = None
+            master.full_clean()
+            master.save()
+            org1_subnet = self._create_subnet(
+                master_subnet=master, subnet='10.0.0.0/24', organization=org1,
+            )
+            # Tests for overlapping subnets
             with self.assertRaises(ValidationError) as context_manager:
                 self._create_subnet(
-                    master_subnet=master, subnet='10.0.0.0/24', organization=org1,
+                    master_subnet=master, subnet='10.0.0.0/25', organization=org2
                 )
             message_dict = context_manager.exception.message_dict
-            self.assertIn('master_subnet', message_dict)
-            self.assertIn(error_message, message_dict['master_subnet'])
+            self.assertIn(overlap_error_message, message_dict['subnet'])
+            # Tests for duplicate subnet
+            with self.assertRaises(ValidationError) as context_manager:
+                self._create_subnet(
+                    master_subnet=master, subnet='10.0.0.0/24', organization=org2
+                )
+            message_dict = context_manager.exception.message_dict
+            self.assertIn(duplicate_error_message, message_dict['subnet'])
+
+            org1_subnet.delete()
 
     def test_valid_subnet_relation_tree(self):
         subnet1 = self._create_subnet(subnet='12.0.56.0/24')