Merge branch 'develop' into develop-2.8

docs/release-notes/version-2.7.md

@@ -9,6 +9,7 @@
 
 ### Bug Fixes
 
+* [#2769](https://github.com/netbox-community/netbox/issues/2769) - Improve `prefix_length` validation on available-prefixes API
 * [#4340](https://github.com/netbox-community/netbox/issues/4340) - Enforce unique constraints for device and virtual machine names in the API
 * [#4343](https://github.com/netbox-community/netbox/issues/4343) - Fix Markdown support for tables
 * [#4365](https://github.com/netbox-community/netbox/issues/4365) - Fix exception raised on IP address bulk add view

netbox/extras/api/serializers.py

@@ -13,7 +13,7 @@ from extras.constants import *
 from extras.models import (
     ConfigContext, ExportTemplate, Graph, ImageAttachment, ObjectChange, ReportResult, Tag,
 )
-from extras.utils import FeatureQuerySet
+from extras.utils import FeatureQuery
 from tenancy.api.nested_serializers import NestedTenantSerializer, NestedTenantGroupSerializer
 from tenancy.models import Tenant, TenantGroup
 from users.api.nested_serializers import NestedUserSerializer
@@ -32,7 +32,7 @@ from .nested_serializers import *
 
 class GraphSerializer(ValidatedModelSerializer):
     type = ContentTypeField(
-        queryset=ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset()),
+        queryset=ContentType.objects.filter(FeatureQuery('graphs').get_query()),
     )
 
     class Meta:
@@ -68,7 +68,7 @@ class RenderedGraphSerializer(serializers.ModelSerializer):
 
 class ExportTemplateSerializer(ValidatedModelSerializer):
     content_type = ContentTypeField(
-        queryset=ContentType.objects.filter(FeatureQuerySet('export_templates').get_queryset()),
+        queryset=ContentType.objects.filter(FeatureQuery('export_templates').get_query()),
     )
     template_language = ChoiceField(
         choices=TemplateLanguageChoices,

netbox/extras/migrations/0039_update_features_content_types.py

@@ -15,26 +15,26 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='customfield',
             name='obj_type',
-            field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuerySet('custom_fields'), related_name='custom_fields', to='contenttypes.ContentType'),
+            field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuery('custom_fields'), related_name='custom_fields', to='contenttypes.ContentType'),
         ),
         migrations.AlterField(
             model_name='customlink',
             name='content_type',
-            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('custom_links'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
+            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('custom_links'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
         ),
         migrations.AlterField(
             model_name='exporttemplate',
             name='content_type',
-            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('export_templates'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
+            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('export_templates'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
         ),
         migrations.AlterField(
             model_name='graph',
             name='type',
-            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('graphs'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
+            field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('graphs'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'),
         ),
         migrations.AlterField(
             model_name='webhook',
             name='obj_type',
-            field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuerySet('webhooks'), related_name='webhooks', to='contenttypes.ContentType'),
+            field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuery('webhooks'), related_name='webhooks', to='contenttypes.ContentType'),
         ),
     ]

netbox/extras/models.py

@@ -22,7 +22,7 @@ from utilities.utils import deepmerge, render_jinja2
 from .choices import *
 from .constants import *
 from .querysets import ConfigContextQuerySet
-from .utils import FeatureQuerySet
+from .utils import FeatureQuery
 
 
 __all__ = (
@@ -59,7 +59,7 @@ class Webhook(models.Model):
         to=ContentType,
         related_name='webhooks',
         verbose_name='Object types',
-        limit_choices_to=FeatureQuerySet('webhooks'),
+        limit_choices_to=FeatureQuery('webhooks'),
         help_text="The object(s) to which this Webhook applies."
     )
     name = models.CharField(
@@ -224,7 +224,7 @@ class CustomField(models.Model):
         to=ContentType,
         related_name='custom_fields',
         verbose_name='Object(s)',
-        limit_choices_to=FeatureQuerySet('custom_fields'),
+        limit_choices_to=FeatureQuery('custom_fields'),
         help_text='The object(s) to which this field applies.'
     )
     type = models.CharField(
@@ -471,7 +471,7 @@ class CustomLink(models.Model):
     content_type = models.ForeignKey(
         to=ContentType,
         on_delete=models.CASCADE,
-        limit_choices_to=FeatureQuerySet('custom_links')
+        limit_choices_to=FeatureQuery('custom_links')
     )
     name = models.CharField(
         max_length=100,
@@ -519,7 +519,7 @@ class Graph(models.Model):
     type = models.ForeignKey(
         to=ContentType,
         on_delete=models.CASCADE,
-        limit_choices_to=FeatureQuerySet('graphs')
+        limit_choices_to=FeatureQuery('graphs')
     )
     weight = models.PositiveSmallIntegerField(
         default=1000
@@ -580,7 +580,7 @@ class ExportTemplate(models.Model):
     content_type = models.ForeignKey(
         to=ContentType,
         on_delete=models.CASCADE,
-        limit_choices_to=FeatureQuerySet('export_templates')
+        limit_choices_to=FeatureQuery('export_templates')
     )
     name = models.CharField(
         max_length=100

netbox/extras/tests/test_api.py

@@ -9,7 +9,7 @@ from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Platform,
 from extras.api.views import ScriptViewSet
 from extras.models import ConfigContext, Graph, ExportTemplate, Tag
 from extras.scripts import BooleanVar, IntegerVar, Script, StringVar
-from extras.utils import FeatureQuerySet
+from extras.utils import FeatureQuery
 from tenancy.models import Tenant, TenantGroup
 from utilities.testing import APITestCase
 

netbox/extras/tests/test_filters.py

@@ -4,7 +4,7 @@ from django.test import TestCase
 from dcim.models import DeviceRole, Platform, Region, Site
 from extras.choices import *
 from extras.filters import *
-from extras.utils import FeatureQuerySet
+from extras.utils import FeatureQuery
 from extras.models import ConfigContext, ExportTemplate, Graph
 from tenancy.models import Tenant, TenantGroup
 from virtualization.models import Cluster, ClusterGroup, ClusterType
@@ -18,7 +18,7 @@ class GraphTestCase(TestCase):
     def setUpTestData(cls):
 
         # Get the first three available types
-        content_types = ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset())[:3]
+        content_types = ContentType.objects.filter(FeatureQuery('graphs').get_query())[:3]
 
         graphs = (
             Graph(name='Graph 1', type=content_types[0], template_language=TemplateLanguageChoices.LANGUAGE_DJANGO, source='http://example.com/1'),
@@ -32,7 +32,7 @@ class GraphTestCase(TestCase):
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
     def test_type(self):
-        content_type = ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset()).first()
+        content_type = ContentType.objects.filter(FeatureQuery('graphs').get_query()).first()
         params = {'type': content_type.pk}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
 

netbox/extras/utils.py

@@ -42,7 +42,7 @@ registry = Registry()
 
 
 @deconstructible
-class FeatureQuerySet:
+class FeatureQuery:
     """
     Helper class that delays evaluation of the registry contents for the functionaility store
     until it has been populated.
@@ -52,9 +52,9 @@ class FeatureQuerySet:
         self.feature = feature
 
     def __call__(self):
-        return self.get_queryset()
+        return self.get_query()
 
-    def get_queryset(self):
+    def get_query(self):
         """
         Given an extras feature, return a Q object for content type lookup
         """

netbox/extras/webhooks.py

@@ -8,7 +8,7 @@ from extras.models import Webhook
 from utilities.api import get_serializer_for_model
 from .choices import *
 from .constants import *
-from .utils import FeatureQuerySet
+from .utils import FeatureQuery
 
 
 def generate_signature(request_body, secret):
@@ -30,7 +30,7 @@ def enqueue_webhooks(instance, user, request_id, action):
     """
     obj_type = ContentType.objects.get_for_model(instance.__class__)
 
-    webhook_models = ContentType.objects.filter(FeatureQuerySet('webhooks').get_queryset())
+    webhook_models = ContentType.objects.filter(FeatureQuery('webhooks').get_query())
     if obj_type not in webhook_models:
         return
 

netbox/ipam/api/serializers.py

@@ -154,6 +154,33 @@ class PrefixSerializer(TaggitSerializer, CustomFieldModelSerializer):
         read_only_fields = ['family']
 
 
+class PrefixLengthSerializer(serializers.Serializer):
+
+    prefix_length = serializers.IntegerField()
+
+    def to_internal_value(self, data):
+        requested_prefix = data.get('prefix_length')
+        if requested_prefix is None:
+            raise serializers.ValidationError({
+                'prefix_length': 'this field can not be missing'
+            })
+        if not isinstance(requested_prefix, int):
+            raise serializers.ValidationError({
+                'prefix_length': 'this field must be int type'
+            })
+
+        prefix = self.context.get('prefix')
+        if prefix.family == 4 and requested_prefix > 32:
+            raise serializers.ValidationError({
+                'prefix_length': 'Invalid prefix length ({}) for IPv4'.format((requested_prefix))
+            })
+        elif prefix.family == 6 and requested_prefix > 128:
+            raise serializers.ValidationError({
+                'prefix_length': 'Invalid prefix length ({}) for IPv6'.format((requested_prefix))
+            })
+        return data
+
+
 class AvailablePrefixSerializer(serializers.Serializer):
     """
     Representation of a prefix which does not exist in the database.

netbox/ipam/api/views.py

@@ -91,45 +91,25 @@ class PrefixViewSet(CustomFieldModelViewSet):
             if not request.user.has_perm('ipam.add_prefix'):
                 raise PermissionDenied()
 
-            # Normalize to a list of objects
+            # Validate Requested Prefixes' length
-            requested_prefixes = request.data if isinstance(request.data, list) else [request.data]
+            serializer = serializers.PrefixLengthSerializer(
+                data=request.data if isinstance(request.data, list) else [request.data],
+                many=True,
+                context={
+                    'request': request,
+                    'prefix': prefix,
+                }
+            )
+            if not serializer.is_valid():
+                return Response(
+                    serializer.errors,
+                    status=status.HTTP_400_BAD_REQUEST
+                )
 
+            requested_prefixes = serializer.validated_data
             # Allocate prefixes to the requested objects based on availability within the parent
             for i, requested_prefix in enumerate(requested_prefixes):
 
-                # Validate requested prefix size
-                prefix_length = requested_prefix.get('prefix_length')
-                if prefix_length is None:
-                    return Response(
-                        {
-                            "detail": "Item {}: prefix_length field missing".format(i)
-                        },
-                        status=status.HTTP_400_BAD_REQUEST
-                    )
-                try:
-                    prefix_length = int(prefix_length)
-                except ValueError:
-                    return Response(
-                        {
-                            "detail": "Item {}: Invalid prefix length ({})".format(i, prefix_length),
-                        },
-                        status=status.HTTP_400_BAD_REQUEST
-                    )
-                if prefix.family == 4 and prefix_length > 32:
-                    return Response(
-                        {
-                            "detail": "Item {}: Invalid prefix length ({}) for IPv4".format(i, prefix_length),
-                        },
-                        status=status.HTTP_400_BAD_REQUEST
-                    )
-                elif prefix.family == 6 and prefix_length > 128:
-                    return Response(
-                        {
-                            "detail": "Item {}: Invalid prefix length ({}) for IPv6".format(i, prefix_length),
-                        },
-                        status=status.HTTP_400_BAD_REQUEST
-                    )
-
                 # Find the first available prefix equal to or larger than the requested size
                 for available_prefix in available_prefixes.iter_cidrs():
                     if requested_prefix['prefix_length'] >= available_prefix.prefixlen:

netbox/ipam/tests/test_api.py

@@ -586,10 +586,15 @@ class PrefixTest(APITestCase):
             self.assertEqual(response.data['description'], data['description'])
 
         # Try to create one more prefix
-        response = self.client.post(url, {'prefix_length': 30}, **self.header)
+        response = self.client.post(url, {'prefix_length': 30}, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT)
         self.assertIn('detail', response.data)
 
+        # Try to create invalid prefix type
+        response = self.client.post(url, {'prefix_length': '30'}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertIn('prefix_length', response.data[0])
+
     def test_create_multiple_available_prefixes(self):
 
         prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/28'), is_pool=True)