#20285: Accept port ranges in the service port_mappings CSV import
The port-mappings CSV column expanded only comma-separated individual protocol/port pairs, while the edit form's port field already accepted hyphen ranges (e.g. tcp/8000-8010). Route the CSV column through the same expand_port_mapping() helper so both entry paths accept identical port syntax. Parsing uses the shared split_port_mapping() helper, and the blank-protocol error is worded to fit every entry path rather than only the form widget's dropdown.
This commit is contained in:
parent
feffda99d7
commit
af59d71642
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue