Merge pull request #22921 from netbox-community/20285-beta-qa

#20285: Pre-release QA
This commit is contained in:
bctiemann 2026-08-13 19:28:00 -04:00 committed by GitHub
commit 3666eeb859
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 85 additions and 22 deletions

View File

@ -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.

View File

@ -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):

View File

@ -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:

View File

@ -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

View File

@ -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):
"""

View File

@ -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()

View File

@ -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.