diff --git a/docs/models/ipam/service.md b/docs/models/ipam/service.md index a08267fc8..50579ce6b 100644 --- a/docs/models/ipam/service.md +++ b/docs/models/ipam/service.md @@ -81,4 +81,4 @@ The [IP address(es)](./ipaddress.md) to which this service is bound. If no IP ad ## Bulk Import (CSV) -When importing application services or [application service templates](./servicetemplate.md) via CSV, all port mappings for a row are given in a single `port_mappings` column as a comma-separated list of `protocol/port` pairs enclosed in double quotes. For example, `"tcp/80,tcp/443,udp/53"`. Protocols may be specified in any case. +When importing application services or [application service templates](./servicetemplate.md) via CSV, all port mappings for a row are given in a single `port_mappings` column as a comma-separated list of `protocol/port` pairs enclosed in double quotes. For example, `"tcp/80,tcp/443,udp/53"`. A pair's port may be given as a hyphen range, for example `"tcp/8000-8010"`. Protocols may be entered in uppercase or lowercase. diff --git a/netbox/ipam/forms/bulk_import.py b/netbox/ipam/forms/bulk_import.py index 89dcdb645..488014f02 100644 --- a/netbox/ipam/forms/bulk_import.py +++ b/netbox/ipam/forms/bulk_import.py @@ -9,6 +9,7 @@ from dcim.models import Device, Interface, Site from ipam.choices import * from ipam.constants import * from ipam.models import * +from ipam.utils import expand_port_mapping, split_port_mapping from ipam.validators import validate_port_mappings from netbox.forms import NetBoxModelImportForm, OrganizationalModelImportForm, PrimaryModelImportForm from tenancy.models import Tenant @@ -592,28 +593,36 @@ class VLANTranslationRuleImportForm(NetBoxModelImportForm): class ServicePortMappingsImportMixin(forms.Form): """ Adds a ``port_mappings`` CSV column parsed from a comma-separated list of ``protocol/port`` pairs - (e.g. "tcp/80,udp/53") into the model's flat ``['tcp/80', 'udp/53']`` list. + (e.g. "tcp/80,udp/53") into the model's flat ``['tcp/80', 'udp/53']`` list. A pair's port half may be + a hyphen range (e.g. "tcp/8000-8010"), matching the port syntax the edit form accepts. """ port_mappings = SimpleArrayField( base_field=forms.CharField(), label=_('Port mappings'), required=True, - help_text=_('Comma-separated list of protocol/port pairs in double quotes (e.g. "tcp/80,udp/53").') + help_text=_('Comma-separated list of protocol/port pairs in double quotes (e.g. "tcp/80,udp/53"). ' + 'A port range may be given with a hyphen (e.g. "tcp/8000-8010").') ) def clean_port_mappings(self): mappings = self.cleaned_data.get('port_mappings') if not mappings: return [] - # Strip surrounding whitespace from each CSV token; validate_port_mappings matches the protocol - # case-insensitively and returns the normalized (canonical) list, so protocols may be given in - # any case (e.g. "TCP/80") without folding here. - mappings = [mapping.strip() for mapping in mappings] + # Expand any hyphen range in a pair's port half (tcp/8000-8010 -> tcp/8000, tcp/8001, ...) so the + # CSV accepts the same port syntax as the edit form. validate_port_mappings then normalizes and + # checks each expanded pair, matching the protocol case-insensitively. + expanded = [] + for mapping in mappings: + protocol, ports = split_port_mapping(mapping.strip()) + try: + expanded.extend(expand_port_mapping(protocol, ports)) + except DjangoValidationError as exc: + raise forms.ValidationError(exc.messages) try: - mappings = validate_port_mappings(mappings) + expanded = validate_port_mappings(expanded) except DjangoValidationError as exc: raise forms.ValidationError(exc.messages) - return mappings + return expanded class ServiceTemplateImportForm(ServicePortMappingsImportMixin, PrimaryModelImportForm): diff --git a/netbox/ipam/forms/fields.py b/netbox/ipam/forms/fields.py index 66a03afe1..a8ec23dd9 100644 --- a/netbox/ipam/forms/fields.py +++ b/netbox/ipam/forms/fields.py @@ -77,9 +77,9 @@ class PortMappingField(forms.Field): # already-expanded list, rejects a blank protocol, and preserves a protocol-without-ports # row as a bare 'protocol/' token. Errors are re-raised with the row's position (among the # submitted rows — the widget omits entirely-blank ones), since it renders one row per - # protocol and an unqualified "Select a protocol" gives no clue which row to fix. Errors - # from validate_port_mappings() below are deliberately left unqualified: each quotes the - # offending mapping already, and a duplicate spans two rows. + # protocol and an unqualified "must specify a protocol" gives no clue which row to fix. + # Errors from validate_port_mappings() below are deliberately left unqualified: each quotes + # the offending mapping already, and a duplicate spans two rows. try: mappings.extend(expand_port_mapping(protocol, raw_ports)) except ValidationError as e: diff --git a/netbox/ipam/models/services.py b/netbox/ipam/models/services.py index ebea82ea8..d881f95cb 100644 --- a/netbox/ipam/models/services.py +++ b/netbox/ipam/models/services.py @@ -7,10 +7,11 @@ from django.utils.translation import gettext_lazy as _ from ipam.choices import * from ipam.constants import * -from ipam.utils import legacy_protocol_and_ports, split_port_mapping +from ipam.utils import group_port_mappings, legacy_protocol_and_ports, split_port_mapping from ipam.validators import validate_port_mappings from netbox.models import PrimaryModel from netbox.models.features import ContactsMixin +from utilities.data import array_to_ranges __all__ = ( 'Service', @@ -93,12 +94,22 @@ class ServiceBase(models.Model): @property def port_mappings_list(self): """ - Return a user-friendly list of port mappings, e.g. "TCP/80, TCP/443, UDP/53". + Return a user-friendly list of port mappings, collapsing consecutive ports within a protocol into + a range, e.g. "TCP/80, TCP/443, UDP/53" or "TCP/8000-8100". """ - return ', '.join( - f'{SERVICE_PROTOCOL_LABELS.get(protocol, protocol)}/{port}' - for protocol, port in (split_port_mapping(mapping) for mapping in self.port_mappings) - ) + parts = [] + for protocol, ports in group_port_mappings(self.port_mappings).items(): + label = SERVICE_PROTOCOL_LABELS.get(protocol, protocol) + int_ports = [int(port) for port in ports if port.isdigit()] + for port_range in array_to_ranges(int_ports): + if len(port_range) == 1: + parts.append(f'{label}/{port_range[0]}') + else: + parts.append(f'{label}/{port_range[0]}-{port_range[1]}') + # A port that isn't a plain integer is only reachable via a write that bypassed validation; + # render it verbatim rather than raising, matching sorted_int_ports and normalize_port_mapping. + parts.extend(f'{label}/{port}' for port in ports if not port.isdigit()) + return ', '.join(parts) # Read-only legacy accessors mirroring the deprecated REST/GraphQL protocol/ports fields, retained # for backward compatibility with code that read the old single-protocol fields. A multi-protocol diff --git a/netbox/ipam/tests/test_forms.py b/netbox/ipam/tests/test_forms.py index 7d0ad38f1..4092c2cfd 100644 --- a/netbox/ipam/tests/test_forms.py +++ b/netbox/ipam/tests/test_forms.py @@ -397,11 +397,29 @@ class ServiceTemplateImportFormTestCase(TestCase): self.assertFalse(form.is_valid()) self.assertIn('port_mappings', form.errors) + def test_port_range_expanded(self): + form = ServiceTemplateImportForm(data={'name': 'X', 'port_mappings': 'tcp/8000-8002,udp/53'}) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual( + form.cleaned_data['port_mappings'], + ['tcp/8000', 'tcp/8001', 'tcp/8002', 'udp/53'], + ) + + def test_reversed_port_range_rejected(self): + form = ServiceTemplateImportForm(data={'name': 'X', 'port_mappings': 'tcp/8010-8000'}) + self.assertFalse(form.is_valid()) + self.assertIn('port_mappings', form.errors) + def test_empty_port_rejected(self): form = ServiceTemplateImportForm(data={'name': 'X', 'port_mappings': 'tcp/'}) self.assertFalse(form.is_valid()) self.assertIn('port_mappings', form.errors) + def test_blank_protocol_rejected(self): + form = ServiceTemplateImportForm(data={'name': 'X', 'port_mappings': '/80'}) + self.assertFalse(form.is_valid()) + self.assertIn('port_mappings', form.errors) + class ServiceFilterFormTestCase(TestCase): """ diff --git a/netbox/ipam/tests/test_models.py b/netbox/ipam/tests/test_models.py index 147af9b0a..7842e8b79 100644 --- a/netbox/ipam/tests/test_models.py +++ b/netbox/ipam/tests/test_models.py @@ -2058,6 +2058,31 @@ class ServiceTestCase(TestCase): ) self.assertEqual(service.port_mappings_list, 'TCP/53, UDP/53') + def test_port_mappings_list_collapses_ranges(self): + vm = VirtualMachine.objects.first() + + big = Service.objects.create( + name='big', + parent=vm, + port_mappings=[f'tcp/{port}' for port in range(8000, 8101)], + ) + self.assertEqual(big.port_mappings_list, 'TCP/8000-8100') + + mixed = Service.objects.create( + name='mixed', + parent=vm, + port_mappings=['tcp/82', 'tcp/80', 'tcp/81', 'tcp/443', 'udp/68', 'udp/67'], + ) + self.assertEqual(mixed.port_mappings_list, 'TCP/80-82, TCP/443, UDP/67-68') + + def test_port_mappings_list_tolerates_malformed_ports(self): + service = Service.objects.create( + name='malformed', + parent=VirtualMachine.objects.first(), + port_mappings=['tcp/80', 'tcp/abc'], + ) + self.assertEqual(service.port_mappings_list, 'TCP/80, TCP/abc') + def test_legacy_protocol_ports_properties(self): """The read-only protocol/ports properties expose the deprecated single-protocol representation.""" vm = VirtualMachine.objects.first() diff --git a/netbox/ipam/utils.py b/netbox/ipam/utils.py index 342f14c6e..cfbab053e 100644 --- a/netbox/ipam/utils.py +++ b/netbox/ipam/utils.py @@ -525,10 +525,10 @@ def expand_port_mapping(protocol, ports): # protocol case-insensitively and stores the canonical value. protocol = (protocol or '').strip() if not protocol: - # A row with ports but no protocol (e.g. the initial blank row where the user typed a port but - # never picked a protocol) would otherwise expand to '/80' and surface as a confusing - # "Invalid protocol:" with a blank value. Report the real problem instead. - raise ValidationError(_("Select a protocol for each port mapping.")) + # Ports given with no protocol would otherwise expand to '/80' and surface as a confusing + # "Invalid protocol:" with a blank value. Report the real problem instead, in wording that fits + # all entry paths that route through here (the form widget and CSV import). + raise ValidationError(_("Each port mapping must specify a protocol.")) if isinstance(ports, (list, tuple)): # Already-expanded ports are paired as-is; validate_port_mappings() checks each value's range.