From f97d8963f2767df5ab4fc9879d33c67100ad7003 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 21 Sep 2020 13:21:41 -0400 Subject: [PATCH] Initial work on #2179: Allow a service to have multiple ports --- netbox/ipam/api/nested_serializers.py | 2 +- netbox/ipam/api/serializers.py | 2 +- netbox/ipam/filters.py | 2 +- netbox/ipam/forms.py | 25 ++++++----- .../migrations/0039_service_ports_array.py | 43 +++++++++++++++++++ .../ipam/migrations/0040_service_drop_port.py | 15 +++++++ netbox/ipam/models.py | 23 +++++----- netbox/ipam/tables.py | 7 ++- netbox/ipam/tests/test_api.py | 14 +++--- netbox/ipam/tests/test_filters.py | 18 ++++---- netbox/ipam/tests/test_views.py | 12 +++--- netbox/templates/ipam/service.html | 4 +- netbox/templates/ipam/service_edit.html | 2 +- 13 files changed, 118 insertions(+), 51 deletions(-) create mode 100644 netbox/ipam/migrations/0039_service_ports_array.py create mode 100644 netbox/ipam/migrations/0040_service_drop_port.py diff --git a/netbox/ipam/api/nested_serializers.py b/netbox/ipam/api/nested_serializers.py index 18f42186f..d40c9bb29 100644 --- a/netbox/ipam/api/nested_serializers.py +++ b/netbox/ipam/api/nested_serializers.py @@ -117,4 +117,4 @@ class NestedServiceSerializer(WritableNestedSerializer): class Meta: model = models.Service - fields = ['id', 'url', 'name', 'protocol', 'port'] + fields = ['id', 'url', 'name', 'protocol', 'ports'] diff --git a/netbox/ipam/api/serializers.py b/netbox/ipam/api/serializers.py index 6be0b8a42..074cba9d6 100644 --- a/netbox/ipam/api/serializers.py +++ b/netbox/ipam/api/serializers.py @@ -282,6 +282,6 @@ class ServiceSerializer(TaggedObjectSerializer, CustomFieldModelSerializer): class Meta: model = Service fields = [ - 'id', 'url', 'device', 'virtual_machine', 'name', 'port', 'protocol', 'ipaddresses', 'description', 'tags', + 'id', 'url', 'device', 'virtual_machine', 'name', 'ports', 'protocol', 'ipaddresses', 'description', 'tags', 'custom_fields', 'created', 'last_updated', ] diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index 79fc05334..7b85c7bf3 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -546,7 +546,7 @@ class ServiceFilterSet(BaseFilterSet, CreatedUpdatedFilterSet): class Meta: model = Service - fields = ['id', 'name', 'protocol', 'port'] + fields = ['id', 'name', 'protocol'] def search(self, queryset, name, value): if not value.strip(): diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 1e8e9038a..b10af8c4c 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -10,8 +10,8 @@ from tenancy.forms import TenancyFilterForm, TenancyForm from tenancy.models import Tenant from utilities.forms import ( add_blank_choice, BootstrapMixin, BulkEditNullBooleanSelect, CSVChoiceField, CSVModelChoiceField, CSVModelForm, - DatePicker, DynamicModelChoiceField, DynamicModelMultipleChoiceField, ExpandableIPAddressField, ReturnURLForm, - SlugField, StaticSelect2, StaticSelect2Multiple, TagFilterField, BOOLEAN_WITH_BLANK_CHOICES, + DatePicker, DynamicModelChoiceField, DynamicModelMultipleChoiceField, ExpandableIPAddressField, NumericArrayField, + ReturnURLForm, SlugField, StaticSelect2, StaticSelect2Multiple, TagFilterField, BOOLEAN_WITH_BLANK_CHOICES, ) from virtualization.models import Cluster, VirtualMachine, VMInterface from .choices import * @@ -1155,9 +1155,12 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): # class ServiceForm(BootstrapMixin, CustomFieldModelForm): - port = forms.IntegerField( - min_value=SERVICE_PORT_MIN, - max_value=SERVICE_PORT_MAX + ports = NumericArrayField( + base_field=forms.IntegerField( + min_value=SERVICE_PORT_MIN, + max_value=SERVICE_PORT_MAX + ), + help_text="Comma-separated list of numeric unit IDs. A range may be specified using a hyphen." ) tags = DynamicModelMultipleChoiceField( queryset=Tag.objects.all(), @@ -1167,7 +1170,7 @@ class ServiceForm(BootstrapMixin, CustomFieldModelForm): class Meta: model = Service fields = [ - 'name', 'protocol', 'port', 'ipaddresses', 'description', 'tags', + 'name', 'protocol', 'ports', 'ipaddresses', 'description', 'tags', ] help_texts = { 'ipaddresses': "IP address assignment is optional. If no IPs are selected, the service is assumed to be " @@ -1244,11 +1247,11 @@ class ServiceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, CustomFieldBulkEdit required=False, widget=StaticSelect2() ) - port = forms.IntegerField( - validators=[ - MinValueValidator(1), - MaxValueValidator(65535), - ], + ports = NumericArrayField( + base_field=forms.IntegerField( + min_value=SERVICE_PORT_MIN, + max_value=SERVICE_PORT_MAX + ), required=False ) description = forms.CharField( diff --git a/netbox/ipam/migrations/0039_service_ports_array.py b/netbox/ipam/migrations/0039_service_ports_array.py new file mode 100644 index 000000000..63e592c03 --- /dev/null +++ b/netbox/ipam/migrations/0039_service_ports_array.py @@ -0,0 +1,43 @@ +import django.contrib.postgres.fields +import django.core.validators +from django.db import migrations, models + + +def replicate_ports(apps, schema_editor): + Service = apps.get_model('ipam', 'Service') + # TODO: Figure out how to cast IntegerField to an array so we can use .update() + for service in Service.objects.all(): + Service.objects.filter(pk=service.pk).update(ports=[service.port]) + + +class Migration(migrations.Migration): + + dependencies = [ + ('ipam', '0038_custom_field_data'), + ] + + operations = [ + migrations.AddField( + model_name='service', + name='ports', + field=django.contrib.postgres.fields.ArrayField( + base_field=models.PositiveIntegerField( + validators=[ + django.core.validators.MinValueValidator(1), + django.core.validators.MaxValueValidator(65535) + ] + ), + default=[], + size=None + ), + preserve_default=False, + ), + + migrations.AlterModelOptions( + name='service', + options={'ordering': ('protocol', 'ports', 'pk')}, + ), + migrations.RunPython( + code=replicate_ports + ), + ] diff --git a/netbox/ipam/migrations/0040_service_drop_port.py b/netbox/ipam/migrations/0040_service_drop_port.py new file mode 100644 index 000000000..d1db82678 --- /dev/null +++ b/netbox/ipam/migrations/0040_service_drop_port.py @@ -0,0 +1,15 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('ipam', '0039_service_ports_array'), + ] + + operations = [ + migrations.RemoveField( + model_name='service', + name='port', + ), + ] diff --git a/netbox/ipam/models.py b/netbox/ipam/models.py index 6e7cb0bd4..1146fab55 100644 --- a/netbox/ipam/models.py +++ b/netbox/ipam/models.py @@ -2,6 +2,7 @@ import netaddr from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType +from django.contrib.postgres.fields import ArrayField from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models @@ -1008,12 +1009,14 @@ class Service(ChangeLoggedModel, CustomFieldModel): max_length=50, choices=ServiceProtocolChoices ) - port = models.PositiveIntegerField( - validators=[ - MinValueValidator(SERVICE_PORT_MIN), - MaxValueValidator(SERVICE_PORT_MAX) - ], - verbose_name='Port number' + ports = ArrayField( + base_field=models.PositiveIntegerField( + validators=[ + MinValueValidator(SERVICE_PORT_MIN), + MaxValueValidator(SERVICE_PORT_MAX) + ] + ), + verbose_name='Port numbers' ) ipaddresses = models.ManyToManyField( to='ipam.IPAddress', @@ -1029,13 +1032,13 @@ class Service(ChangeLoggedModel, CustomFieldModel): objects = RestrictedQuerySet.as_manager() - csv_headers = ['device', 'virtual_machine', 'name', 'protocol', 'port', 'description'] + csv_headers = ['device', 'virtual_machine', 'name', 'protocol', 'ports', 'description'] class Meta: - ordering = ('protocol', 'port', 'pk') # (protocol, port) may be non-unique + ordering = ('protocol', 'ports', 'pk') # (protocol, port) may be non-unique def __str__(self): - return '{} ({}/{})'.format(self.name, self.port, self.get_protocol_display()) + return f'{self.name} ({self.get_protocol_display()}/{self.ports})' def get_absolute_url(self): return reverse('ipam:service', args=[self.pk]) @@ -1058,6 +1061,6 @@ class Service(ChangeLoggedModel, CustomFieldModel): self.virtual_machine.name if self.virtual_machine else None, self.name, self.get_protocol_display(), - self.port, + self.ports, self.description, ) diff --git a/netbox/ipam/tables.py b/netbox/ipam/tables.py index d7a64f7db..e454e7850 100644 --- a/netbox/ipam/tables.py +++ b/netbox/ipam/tables.py @@ -623,11 +623,14 @@ class ServiceTable(BaseTable): parent = tables.LinkColumn( order_by=('device', 'virtual_machine') ) + ports = tables.Column( + orderable=False + ) tags = TagColumn( url_name='ipam:service_list' ) class Meta(BaseTable.Meta): model = Service - fields = ('pk', 'name', 'parent', 'protocol', 'port', 'ipaddresses', 'description', 'tags') - default_columns = ('pk', 'name', 'parent', 'protocol', 'port', 'description') + fields = ('pk', 'name', 'parent', 'protocol', 'ports', 'ipaddresses', 'description', 'tags') + default_columns = ('pk', 'name', 'parent', 'protocol', 'ports', 'description') diff --git a/netbox/ipam/tests/test_api.py b/netbox/ipam/tests/test_api.py index 3f2ac470d..4f514aab0 100644 --- a/netbox/ipam/tests/test_api.py +++ b/netbox/ipam/tests/test_api.py @@ -428,7 +428,7 @@ class VLANTest(APIViewTestCases.APIViewTestCase): class ServiceTest(APIViewTestCases.APIViewTestCase): model = Service - brief_fields = ['id', 'name', 'port', 'protocol', 'url'] + brief_fields = ['id', 'name', 'ports', 'protocol', 'url'] @classmethod def setUpTestData(cls): @@ -444,9 +444,9 @@ class ServiceTest(APIViewTestCases.APIViewTestCase): Device.objects.bulk_create(devices) services = ( - Service(device=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=1), - Service(device=devices[0], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=2), - Service(device=devices[0], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=3), + Service(device=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), + Service(device=devices[0], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2]), + Service(device=devices[0], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[3]), ) Service.objects.bulk_create(services) @@ -455,18 +455,18 @@ class ServiceTest(APIViewTestCases.APIViewTestCase): 'device': devices[1].pk, 'name': 'Service 4', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, - 'port': 4, + 'ports': [4], }, { 'device': devices[1].pk, 'name': 'Service 5', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, - 'port': 5, + 'ports': [5], }, { 'device': devices[1].pk, 'name': 'Service 6', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, - 'port': 6, + 'ports': [6], }, ] diff --git a/netbox/ipam/tests/test_filters.py b/netbox/ipam/tests/test_filters.py index 560313f0a..91d878a01 100644 --- a/netbox/ipam/tests/test_filters.py +++ b/netbox/ipam/tests/test_filters.py @@ -742,12 +742,12 @@ class ServiceTestCase(TestCase): VirtualMachine.objects.bulk_create(virtual_machines) services = ( - Service(device=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=1001), - Service(device=devices[1], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=1002), - Service(device=devices[2], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_UDP, port=1003), - Service(virtual_machine=virtual_machines[0], name='Service 4', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=2001), - Service(virtual_machine=virtual_machines[1], name='Service 5', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=2002), - Service(virtual_machine=virtual_machines[2], name='Service 6', protocol=ServiceProtocolChoices.PROTOCOL_UDP, port=2003), + Service(device=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1001]), + Service(device=devices[1], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1002]), + Service(device=devices[2], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_UDP, ports=[1003]), + Service(virtual_machine=virtual_machines[0], name='Service 4', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2001]), + Service(virtual_machine=virtual_machines[1], name='Service 5', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2002]), + Service(virtual_machine=virtual_machines[2], name='Service 6', protocol=ServiceProtocolChoices.PROTOCOL_UDP, ports=[2003]), ) Service.objects.bulk_create(services) @@ -763,9 +763,9 @@ class ServiceTestCase(TestCase): params = {'protocol': ServiceProtocolChoices.PROTOCOL_TCP} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) - def test_port(self): - params = {'port': ['1001', '1002', '1003']} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3) + # def test_port(self): + # params = {'port': ['1001', '1002', '1003']} + # self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3) def test_device(self): devices = Device.objects.all()[:2] diff --git a/netbox/ipam/tests/test_views.py b/netbox/ipam/tests/test_views.py index 7992e4ddf..fc595ac9c 100644 --- a/netbox/ipam/tests/test_views.py +++ b/netbox/ipam/tests/test_views.py @@ -373,9 +373,9 @@ class ServiceTestCase( device = Device.objects.create(name='Device 1', site=site, device_type=devicetype, device_role=devicerole) Service.objects.bulk_create([ - Service(device=device, name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=101), - Service(device=device, name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=102), - Service(device=device, name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, port=103), + Service(device=device, name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[101]), + Service(device=device, name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[102]), + Service(device=device, name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[103]), ]) tags = cls.create_tags('Alpha', 'Bravo', 'Charlie') @@ -385,14 +385,14 @@ class ServiceTestCase( 'virtual_machine': None, 'name': 'Service X', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, - 'port': 999, + 'ports': '104,105', 'ipaddresses': [], 'description': 'A new service', 'tags': [t.pk for t in tags], } cls.csv_data = ( - "device,name,protocol,port,description", + "device,name,protocol,ports,description", "Device 1,Service 1,tcp,1,First service", "Device 1,Service 2,tcp,2,Second service", "Device 1,Service 3,udp,3,Third service", @@ -400,6 +400,6 @@ class ServiceTestCase( cls.bulk_edit_data = { 'protocol': ServiceProtocolChoices.PROTOCOL_UDP, - 'port': 888, + 'ports': '106,107', 'description': 'New description', } diff --git a/netbox/templates/ipam/service.html b/netbox/templates/ipam/service.html index b16e99aa3..f87a7a1a0 100644 --- a/netbox/templates/ipam/service.html +++ b/netbox/templates/ipam/service.html @@ -62,8 +62,8 @@ {{ service.get_protocol_display }} - Port - {{ service.port }} + Ports + {{ service.ports|join:", " }} IP Addresses diff --git a/netbox/templates/ipam/service_edit.html b/netbox/templates/ipam/service_edit.html index 521aec137..48d1b11e3 100644 --- a/netbox/templates/ipam/service_edit.html +++ b/netbox/templates/ipam/service_edit.html @@ -25,7 +25,7 @@
{{ form.protocol }} - {{ form.port }} + {{ form.ports }}
{% render_field form.ipaddresses %}