From 0270cf1495489e6a4b661928421c9c28c8b5f8e9 Mon Sep 17 00:00:00 2001 From: Arthur Hanson Date: Mon, 3 Aug 2026 07:58:01 -0700 Subject: [PATCH] #20285 - Support multiple protocols per application service via port mappings (#22692) --- docs/models/ipam/service.md | 56 +- docs/models/ipam/servicetemplate.md | 8 +- netbox/ipam/api/serializers_/services.py | 133 ++++- netbox/ipam/filtersets.py | 145 ++++- netbox/ipam/forms/__init__.py | 2 + netbox/ipam/forms/bulk_edit.py | 23 +- netbox/ipam/forms/bulk_import.py | 46 +- netbox/ipam/forms/fields.py | 103 ++++ netbox/ipam/forms/filtersets.py | 20 +- netbox/ipam/forms/model_forms.py | 63 +-- netbox/ipam/forms/widgets.py | 63 +++ netbox/ipam/graphql/enums.py | 2 +- netbox/ipam/graphql/filter_mixins.py | 25 - netbox/ipam/graphql/filters.py | 140 ++++- netbox/ipam/graphql/types.py | 31 +- .../0095_multi_protocol_services.py | 126 +++++ netbox/ipam/models/services.py | 137 +++-- netbox/ipam/tables/services.py | 26 +- .../templates/ipam/widgets/port_mappings.html | 66 +++ netbox/ipam/tests/test_api.py | 523 +++++++++++++++++- netbox/ipam/tests/test_filtersets.py | 335 +++++++---- netbox/ipam/tests/test_forms.py | 232 +++++++- netbox/ipam/tests/test_models.py | 147 +++-- netbox/ipam/tests/test_views.py | 167 ++++-- netbox/ipam/ui/panels.py | 6 +- netbox/ipam/utils.py | 274 +++++++++ netbox/ipam/validators.py | 56 ++ netbox/ipam/views.py | 19 +- netbox/netbox/views/generic/bulk_views.py | 13 + netbox/project-static/dist/netbox.css | 2 +- netbox/project-static/dist/netbox.js | 8 +- netbox/project-static/dist/netbox.js.map | 8 +- netbox/project-static/src/forms/index.ts | 9 +- .../project-static/src/forms/portMappings.ts | 232 ++++++++ .../project-static/styles/custom/_misc.scss | 20 + netbox/utilities/forms/utils.py | 23 +- netbox/utilities/tests/test_forms.py | 60 ++ 37 files changed, 2957 insertions(+), 392 deletions(-) create mode 100644 netbox/ipam/forms/fields.py create mode 100644 netbox/ipam/forms/widgets.py delete mode 100644 netbox/ipam/graphql/filter_mixins.py create mode 100644 netbox/ipam/migrations/0095_multi_protocol_services.py create mode 100644 netbox/ipam/templates/ipam/widgets/port_mappings.html create mode 100644 netbox/project-static/src/forms/portMappings.ts diff --git a/docs/models/ipam/service.md b/docs/models/ipam/service.md index fc6ab73d2..a08267fc8 100644 --- a/docs/models/ipam/service.md +++ b/docs/models/ipam/service.md @@ -23,14 +23,62 @@ The parent object to which the application service is assigned. This must be one A service or protocol name. -### Protocol +### Port Mappings -The wire protocol on which the service runs. Choices include UDP, TCP, and SCTP. +The protocols and ports on which the service runs. A service may expose the same port on multiple protocols — for example, DNS listening on both `tcp/53` and `udp/53`. In the UI, ports for a given protocol may be entered together using commas and/or hyphens (e.g. `80,8001-8003`). -### Ports +In the REST and GraphQL APIs, port mappings are represented as a flat list of `protocol/port` strings — matching how they are stored: -One or more numeric ports to which the service is bound. Multiple ports can be expressed using commas and/or hyphens. For example, `80,8001-8003` specifies ports 80, 8001, 8002, and 8003. +```json +[ + "tcp/80", + "tcp/443", + "udp/53" +] +``` + +!!! note "Changed in NetBox v4.7" + + The single-protocol `protocol` and `ports` fields have been replaced by the unified `port_mappings` field, which supports multiple protocols per service. For backward compatibility, the REST and GraphQL APIs still expose the legacy `protocol` and `ports` fields, and the REST API still accepts them on write as an alternative to `port_mappings`. They are populated for single-protocol services; a service with multiple protocols cannot be represented in the legacy format and returns `null` for both, while a service with no mappings returns `protocol: null` and `ports: []`. In other words, `ports: null` specifically signals "multiple protocols — read `port_mappings` instead." **These legacy fields are deprecated and will be removed in NetBox v5.0; use `port_mappings` instead.** + + On write, `port_mappings` and the legacy `protocol`/`ports` fields may be submitted together only when they agree — as in a full-object round-trip that echoes back a read. A request whose legacy fields contradict `port_mappings` (for example, an edited `port_mappings` sent alongside the stale `protocol`/`ports` from the original read) is rejected as ambiguous; send `port_mappings` alone, or keep the legacy fields consistent with it. + + At the ORM level (custom scripts and plugins), `protocol` and `ports` are now **read-only** properties derived from `port_mappings`. Assign `port_mappings` directly — e.g. `Service(parent=device, name='http', port_mappings=['tcp/80'])` — since passing `protocol=`/`ports=` to the model raises `TypeError` and setting `service.ports = [...]` raises `AttributeError`. + +### Filtering by Port Mapping, Protocol, and Port + +`port_mappings`, `protocol`, and `port` are all filtered against the `port_mappings` array. Each accepts multiple values (matching any of them), and `port` supports the usual numeric lookups: + +| Parameter | Matches services having a mapping… | +|---|---| +| `?port_mappings=tcp/80` | that is exactly `tcp/80` | +| `?port_mappings__n=tcp/80` | *(negated)* that is exactly `tcp/80` | +| `?protocol=tcp` | whose protocol is TCP | +| `?protocol__n=tcp` | *(negated)* whose protocol is TCP | +| `?port=80` | whose port is 80 | +| `?port__n=80` | *(negated)* whose port is 80 | +| `?port__gt=` / `?port__gte=` / `?port__lt=` / `?port__lte=` | whose port is above/below the given value | + +`port_mappings` is the most direct way to ask "which services expose this exact protocol and port?" — `?port_mappings=tcp/80` will not match a service that exposes only `udp/80`. Protocols may be given in any case, and leading zeros are ignored, so `?port_mappings=TCP/080` finds `tcp/80`. A value naming an unknown protocol or a malformed pair simply matches nothing rather than returning an error. + +When `protocol` and one or more `port` lookups are combined, they must all be satisfied by a **single** mapping. So `?protocol=tcp&port__gt=1000` does not match a service whose only TCP mapping is `tcp/80` (even if it also exposes `udp/9999`), and `?port__gte=1000&port__lte=2000` does not match a service exposing only ports 500 and 5000. Each `port_mappings` value already names one complete pair, so it needs no such correlation and is simply combined with the other parameters. + +All of these parameters are available as GraphQL filters too, under the same names — `port_mappings`, `protocol`, `port`, `port__gt`, `port__gte`, `port__lt`, `port__lte` — each accepting a list of values. For example, `filters: {port_mappings: ["tcp/80"]}` or `filters: {protocol: [TCP], port__gt: [1000]}`. The single-mapping correlation rule described above applies identically. + +!!! warning "GraphQL filter change in NetBox v4.7" + + The GraphQL filters for `Service` and `ServiceTemplate` have changed shape. The former `protocol` lookup and `ports` integer lookup (which nested their comparisons, e.g. `ports: {gt: 1000}`) are replaced by the flat `protocol`, `port`, `port__gt`, `port__gte`, `port__lt`, `port__lte`, and `port_mappings` parameters, each accepting a list of values and spelled the same way as the corresponding REST query parameter. Rewrite `ports: {gt: 1000}` as `port__gt: [1000]`, and `ports: {exact: 80}` as `port: [80]`. The `range` and `i_exact` lookups previously offered by the integer lookup have no direct equivalent; express a range as `port__gte`/`port__lte`, which — unlike the old lookup — requires a single mapping to satisfy both bounds. + + The members of the `ServiceProtocolEnum` used by the `protocol` filter have also been renamed to drop a spurious `ROLE_` prefix: `ROLE_TCP`, `ROLE_UDP`, and `ROLE_SCTP` are now `TCP`, `UDP`, and `SCTP`. + +!!! warning "REST filter change in NetBox v4.7" + + Because `protocol` is now filtered against the `port_mappings` array rather than a dedicated model field, the character-based lookup variants previously auto-generated for it — `protocol__ic`, `protocol__nic`, `protocol__isw`, `protocol__empty`, etc. — are no longer available; `protocol` and `protocol__n` remain. The `port__empty` lookup is likewise gone, as a service always has at least one port mapping. As with any unrecognized query parameter, the REST API silently ignores a removed lookup rather than raising an error, so update any saved filters or scripts that relied on them. ### IP Addresses The [IP address(es)](./ipaddress.md) to which this service is bound. If no IP addresses are bound, the service is assumed to be reachable via any assigned IP address. + +## 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. diff --git a/docs/models/ipam/servicetemplate.md b/docs/models/ipam/servicetemplate.md index 9dd69b3c4..84d0bf80b 100644 --- a/docs/models/ipam/servicetemplate.md +++ b/docs/models/ipam/servicetemplate.md @@ -12,10 +12,10 @@ Application service templates can be used to instantiate [application services]( A service or protocol name. -### Protocol +### Port Mappings -The wire protocol on which the service runs. Choices include UDP, TCP, and SCTP. +The protocols and ports on which the service runs. See [Port Mappings](./service.md#port-mappings) on the application service model for details. -### Ports +## Bulk Import (CSV) -One or more numeric ports to which the service is bound. Multiple ports can be expressed using commas and/or hyphens. For example, `80,8001-8003` specifies ports 80, 8001, 8002, and 8003. +Application service templates are imported via CSV using the same `port_mappings` column format as application services. See [Bulk Import (CSV)](./service.md#bulk-import-csv) on the application service model for details. diff --git a/netbox/ipam/api/serializers_/services.py b/netbox/ipam/api/serializers_/services.py index 9d075dfb6..266213024 100644 --- a/netbox/ipam/api/serializers_/services.py +++ b/netbox/ipam/api/serializers_/services.py @@ -1,8 +1,13 @@ from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError as DjangoValidationError +from django.utils.translation import gettext as _ +from rest_framework import serializers from ipam.choices import * -from ipam.constants import SERVICE_ASSIGNMENT_MODELS +from ipam.constants import SERVICE_ASSIGNMENT_MODELS, SERVICE_PORT_MAX, SERVICE_PORT_MIN from ipam.models import IPAddress, Service, ServiceTemplate +from ipam.utils import legacy_protocol_and_ports +from ipam.validators import validate_port_mappings from netbox.api.fields import ChoiceField, ContentTypeField, SerializedPKRelatedField from netbox.api.gfk_fields import GFKSerializerField from netbox.api.serializers import PrimaryModelSerializer @@ -15,20 +20,126 @@ __all__ = ( ) -class ServiceTemplateSerializer(PrimaryModelSerializer): - protocol = ChoiceField(choices=ServiceProtocolChoices, required=False) +class PortMappingsField(serializers.ListField): + """ + A service's port mappings as a flat list of ``protocol/port`` strings (e.g. ``["tcp/80", "udp/53"]``), + matching how they are stored. Each entry is validated (and normalized) on write. + """ + child = serializers.CharField() + + def to_internal_value(self, data): + mappings = super().to_internal_value(data) + try: + return validate_port_mappings(mappings) + except DjangoValidationError as exc: + raise serializers.ValidationError(exc.messages) + + +class PortMappingsSerializerMixin(serializers.Serializer): + """ + Shared port-mapping handling for the Service and ServiceTemplate serializers, including backward + compatibility for the legacy single-protocol ``protocol``/``ports`` representation. + + Read: alongside the ``port_mappings`` list, a service that uses a single protocol also reports the + legacy ``protocol`` and ``ports`` fields; a multi-protocol service reports ``null`` for both (it + cannot be expressed in the old single-protocol format). + + Write: either format is accepted. When the legacy ``protocol``/``ports`` pair is supplied (and + ``port_mappings`` is not), it is translated into ``port_mappings``. Supplying both together is + accepted only when the legacy fields agree with what ``port_mappings`` implies (e.g. a full-object + round-trip that echoes back the read representation); a genuine conflict is rejected as ambiguous. + + Subclassing ``serializers.Serializer`` (rather than a plain mixin) lets DRF's metaclass collect the + fields declared here into the inheriting serializers. + """ + port_mappings = PortMappingsField(required=False) + + # Legacy single-protocol fields, retained for backward compatibility. They are read straight off the + # model's protocol/ports properties (which share these field names), so DRF sources them directly — + # matching the {"value", "label"} shape every other choice field uses. default=None applies only on + # write, where validate() consumes them. + # TODO: Remove protocol/ports in v5.0 along with the legacy handling in validate(). + protocol = ChoiceField( + choices=ServiceProtocolChoices, + required=False, + allow_null=True, + default=None, + help_text=_("Deprecated; use port_mappings. Reported only for single-protocol services."), + ) + ports = serializers.ListField( + child=serializers.IntegerField(min_value=SERVICE_PORT_MIN, max_value=SERVICE_PORT_MAX), + required=False, + allow_null=True, + default=None, + help_text=_("Deprecated; use port_mappings. Reported only for single-protocol services."), + ) + + def validate(self, data): + # Consume the legacy fields and translate them into port_mappings *before* calling super(), + # which instantiates the model (via full_clean()) and would choke on these now-nonexistent kwargs. + legacy_protocol = data.pop('protocol', None) + legacy_ports = data.pop('ports', None) + # protocol/ports carry default=None, so an omitted field arrives as None; an explicitly-supplied + # value (including a falsy ports=[]) is a legacy write and must be handled — checking `is not None` + # rather than truthiness so an intentional empty list isn't silently dropped. + if legacy_protocol is not None or legacy_ports is not None: + # `port_mappings` and `protocol`/`ports` are mutually exclusive as *representations*, but a + # full-object round-trip (GET then PUT/PATCH) legitimately resubmits port_mappings alongside + # the legacy protocol/ports the read emitted. Only reject a genuine *conflict*: when the legacy + # fields agree with what port_mappings already implies they're merely redundant, so accept the + # request and let port_mappings win. + if 'port_mappings' in data: + expected_protocol, expected_ports = legacy_protocol_and_ports(data['port_mappings']) + protocol_agrees = legacy_protocol is None or legacy_protocol == expected_protocol + ports_agree = legacy_ports is None or sorted(legacy_ports) == (expected_ports or []) + if not (protocol_agrees and ports_agree): + raise serializers.ValidationError(_( + "Specify either 'port_mappings' or the deprecated 'protocol'/'ports' fields, not both." + )) + return super().validate(data) + # The old API accepted an empty ports list (the ArrayField had no minimum length); the new + # model requires at least one mapping. Report that directly — both fields may have been + # supplied, so the "both are required" message below would be misleading. + if legacy_ports == []: + raise serializers.ValidationError( + {'ports': _("At least one port mapping is required.")} + ) + # The legacy API let either field be updated on its own (e.g. a PATCH that adjusts only the + # port list). Preserve that by backfilling the omitted field from the instance's current + # single-protocol representation. + if not (legacy_protocol and legacy_ports): + legacy_protocol = legacy_protocol or (self.instance.protocol if self.instance else None) + if legacy_ports is None: + legacy_ports = self.instance.ports if self.instance else None + # If the pair still can't be resolved — a create, or an existing multi-protocol service that + # has no single-protocol form — the request can't be expressed in the legacy format. + if not (legacy_protocol and legacy_ports): + raise serializers.ValidationError(_( + "Both 'protocol' and 'ports' are required when writing via the deprecated legacy " + "format; use port_mappings instead." + )) + try: + data['port_mappings'] = validate_port_mappings( + [f'{legacy_protocol}/{port}' for port in legacy_ports] + ) + except DjangoValidationError as exc: + raise serializers.ValidationError({'ports': exc.messages}) + + return super().validate(data) + + +class ServiceTemplateSerializer(PortMappingsSerializerMixin, PrimaryModelSerializer): class Meta: model = ServiceTemplate fields = [ - 'id', 'url', 'display_url', 'display', 'name', 'protocol', 'ports', 'description', 'owner', 'comments', - 'tags', 'custom_fields', 'created', 'last_updated', + 'id', 'url', 'display_url', 'display', 'name', 'port_mappings', 'protocol', 'ports', 'description', + 'owner', 'comments', 'tags', 'custom_fields', 'created', 'last_updated', ] - brief_fields = ('id', 'url', 'display', 'name', 'protocol', 'ports', 'description') + brief_fields = ('id', 'url', 'display', 'name', 'port_mappings', 'description') -class ServiceSerializer(PrimaryModelSerializer): - protocol = ChoiceField(choices=ServiceProtocolChoices, required=False) +class ServiceSerializer(PortMappingsSerializerMixin, PrimaryModelSerializer): ipaddresses = SerializedPKRelatedField( queryset=IPAddress.objects.all(), serializer=IPAddressSerializer, @@ -45,7 +156,7 @@ class ServiceSerializer(PrimaryModelSerializer): model = Service fields = [ 'id', 'url', 'display_url', 'display', 'parent_object_type', 'parent_object_id', 'parent', 'name', - 'protocol', 'ports', 'ipaddresses', 'description', 'owner', 'comments', 'tags', 'custom_fields', - 'created', 'last_updated', + 'port_mappings', 'protocol', 'ports', 'ipaddresses', 'description', 'owner', 'comments', 'tags', + 'custom_fields', 'created', 'last_updated', ] - brief_fields = ('id', 'url', 'display', 'name', 'protocol', 'ports', 'description') + brief_fields = ('id', 'url', 'display', 'name', 'port_mappings', 'description') diff --git a/netbox/ipam/filtersets.py b/netbox/ipam/filtersets.py index 97abd274a..6bcd3d4f5 100644 --- a/netbox/ipam/filtersets.py +++ b/netbox/ipam/filtersets.py @@ -22,7 +22,6 @@ from utilities.filters import ( MultiValueCharFilter, MultiValueContentTypeFilter, MultiValueNumberFilter, - NumericArrayFilter, TreeNodeMultipleChoiceFilter, ) from utilities.filtersets import register_filterset @@ -31,6 +30,7 @@ from vpn.models import L2VPN from .choices import * from .models import * +from .utils import normalize_port_mapping, port_mapping_q __all__ = ( 'ASNFilterSet', @@ -1214,16 +1214,139 @@ class VLANTranslationRuleFilterSet(NetBoxModelFilterSet): return queryset.filter(qs_filter) -@register_filterset -class ServiceTemplateFilterSet(PrimaryModelFilterSet): - port = NumericArrayFilter( - field_name='ports', - lookup_expr='contains' +# Service/ServiceTemplate port filter name -> the port lookup it applies, in the order the conditions +# are built. See ServicePortMappingFilterMixin and ipam.utils.PORT_MAPPING_LOOKUPS. +SERVICE_PORT_FILTERS = { + 'port': 'exact', + 'port__gt': 'gt', + 'port__gte': 'gte', + 'port__lt': 'lt', + 'port__lte': 'lte', +} + + +class ServicePortMappingFilterMixin(django_filters.FilterSet): + """ + Shared ``port_mappings``, ``protocol`` and ``port`` filtering for Service and ServiceTemplate, all + operating on the ``port_mappings`` array. ``protocol`` and every active ``port`` lookup are + correlated: they must all be satisfied by one single mapping, so ``?protocol=tcp&port__gt=1000`` does + not match a service whose only tcp mapping is tcp/80, and ``?port__gte=1000&port__lte=2000`` does not + match a service exposing only ports 500 and 5000. See ``ipam.utils.port_mapping_q``. + """ + # Whole-mapping lookup, e.g. ?port_mappings=tcp/80. Each value already names one complete + # protocol/port pair, so this needs none of the protocol/port correlation machinery below and is + # simply ANDed with the other filters. + port_mappings = MultiValueCharFilter( + method='filter_port_mappings', + label=_('Port mapping (protocol/port)'), ) + port_mappings__n = MultiValueCharFilter( + method='filter_port_mappings_negated', + label=_('Port mapping (protocol/port)'), + ) + protocol = django_filters.MultipleChoiceFilter( + choices=ServiceProtocolChoices, + method='filter_noop', + ) + # Negation lookup retained from when `protocol` was a model field: method-based filters don't get + # the char-based lookups (protocol__n, __ic, ...) auto-generated, and silently dropping protocol__n + # would widen existing saved filters/scripts rather than error. The __ic/__nic/__empty variants were + # never meaningful on a small fixed choice set and are intentionally left gone. + protocol__n = django_filters.MultipleChoiceFilter( + choices=ServiceProtocolChoices, + method='filter_protocol_negated', + ) + # `port` and its range lookups. These are declared explicitly because a method-based filter gets no + # auto-generated lookups (BaseFilterSet.get_additional_lookups() skips filters with a method), and + # they must be correlated with `protocol` rather than applied independently. `port__empty` is + # intentionally absent: port_mappings is never empty on a validated object, so it was never + # meaningful. See ipam.utils.PORT_MAPPING_LOOKUPS for the lookup -> SQL operator mapping. + # + # `protocol` above and every `port*` lookup below (except the negations, which stand alone) are + # deliberately no-ops: because they must be correlated with one another they cannot be applied as each + # filter runs. filter_queryset() applies them together, once, after super() has applied the rest. + port = MultiValueNumberFilter( + method='filter_noop', + ) + port__n = MultiValueNumberFilter( + method='filter_port_negated', + ) + port__gt = MultiValueNumberFilter( + method='filter_noop', + ) + port__gte = MultiValueNumberFilter( + method='filter_noop', + ) + port__lt = MultiValueNumberFilter( + method='filter_noop', + ) + port__lte = MultiValueNumberFilter( + method='filter_noop', + ) + + def filter_queryset(self, queryset): + """ + Apply `protocol` and every active `port*` lookup as a single correlated predicate. + + These can't be applied per-filter the way django-filter normally works: they must all be satisfied + by one single mapping, and a query combining N of them would otherwise emit N independent (and + redundant) copies of the same scan. So the individual filters are no-ops and the combined + predicate is built here, from the cleaned data, exactly once per call. + """ + queryset = super().filter_queryset(queryset) + + cleaned_data = self.form.cleaned_data + protocols = cleaned_data.get('protocol') or [] + port_tests = [ + (lookup, values) + for lookup, values in ( + (lookup, cleaned_data.get(name) or []) + for name, lookup in SERVICE_PORT_FILTERS.items() + ) + if values + ] + if not protocols and not port_tests: + return queryset + + return queryset.filter(port_mapping_q(protocols, port_tests)) + + def filter_noop(self, queryset, name, value): + # See filter_queryset(), which applies `protocol` and the port lookups as one correlated predicate. + return queryset + + def filter_port_mappings(self, queryset, name, value: list[str]): + # Array overlap (&&) is served by the GIN index on port_mappings and gives the multi-value OR + # semantics used throughout NetBox: ?port_mappings=tcp/80&port_mappings=udp/53 matches either. + if not value: + return queryset + return queryset.filter(port_mappings__overlap=[normalize_port_mapping(v) for v in value]) + + def filter_port_mappings_negated(self, queryset, name, value: list[str]): + if not value: + return queryset + return queryset.exclude(port_mappings__overlap=[normalize_port_mapping(v) for v in value]) + + def filter_protocol_negated(self, queryset, name, value: list[str]): + # Exclude services exposing any of the given protocols (negation of the protocol-only lookup). + if not value: + return queryset + return queryset.exclude(port_mapping_q(value)) + + def filter_port_negated(self, queryset, name, value: list[int]): + # Exclude services exposing any of the given ports. Correlated with `protocol` when supplied, so + # ?protocol=tcp&port__n=80 excludes only services exposing tcp/80 (not those exposing udp/80). + if not value: + return queryset + protocols = self.form.cleaned_data.get('protocol') or [] + return queryset.exclude(port_mapping_q(protocols, [('exact', value)])) + + +@register_filterset +class ServiceTemplateFilterSet(ServicePortMappingFilterMixin, PrimaryModelFilterSet): class Meta: model = ServiceTemplate - fields = ('id', 'name', 'protocol', 'description') + fields = ('id', 'name', 'description') def search(self, queryset, name, value): if not value.strip(): @@ -1236,7 +1359,7 @@ class ServiceTemplateFilterSet(PrimaryModelFilterSet): @register_filterset -class ServiceFilterSet(ContactModelFilterSet, PrimaryModelFilterSet): +class ServiceFilterSet(ServicePortMappingFilterMixin, ContactModelFilterSet, PrimaryModelFilterSet): parent_object_type = MultiValueContentTypeFilter() device = MultiValueCharFilter( method='filter_device', @@ -1279,14 +1402,10 @@ class ServiceFilterSet(ContactModelFilterSet, PrimaryModelFilterSet): to_field_name='address', label=_('IP address'), ) - port = NumericArrayFilter( - field_name='ports', - lookup_expr='contains' - ) class Meta: model = Service - fields = ('id', 'name', 'protocol', 'description', 'parent_object_type', 'parent_object_id') + fields = ('id', 'name', 'description', 'parent_object_type', 'parent_object_id') def search(self, queryset, name, value): if not value.strip(): diff --git a/netbox/ipam/forms/__init__.py b/netbox/ipam/forms/__init__.py index 5cec11aac..f5ae3bca5 100644 --- a/netbox/ipam/forms/__init__.py +++ b/netbox/ipam/forms/__init__.py @@ -1,5 +1,7 @@ from .bulk_create import * from .bulk_edit import * from .bulk_import import * +from .fields import * from .filtersets import * from .model_forms import * +from .widgets import * diff --git a/netbox/ipam/forms/bulk_edit.py b/netbox/ipam/forms/bulk_edit.py index 8ba91fa0a..fe16c143b 100644 --- a/netbox/ipam/forms/bulk_edit.py +++ b/netbox/ipam/forms/bulk_edit.py @@ -6,6 +6,7 @@ from dcim.forms.mixins import ScopedBulkEditForm from dcim.models import Region, Site, SiteGroup from ipam.choices import * from ipam.constants import * +from ipam.forms.fields import PortMappingField from ipam.models import * from ipam.models import ASN from netbox.forms import NetBoxModelBulkEditForm, OrganizationalModelBulkEditForm, PrimaryModelBulkEditForm @@ -16,7 +17,6 @@ from utilities.forms.fields import ( DynamicModelChoiceField, DynamicModelMultipleChoiceField, GenericObjectChoiceField, - NumericArrayField, NumericRangeArrayField, ) from utilities.forms.rendering import FieldSet @@ -476,23 +476,20 @@ class VLANTranslationRuleBulkEditForm(NetBoxModelBulkEditForm): class ServiceTemplateBulkEditForm(PrimaryModelBulkEditForm): - protocol = ChoiceField( - label=_('Protocol'), - choices=add_blank_choice(ServiceProtocolChoices), - required=False + add_port_mappings = PortMappingField( + label=_('Add port mappings'), + required=False, + help_text=_("Port mappings to add to each selected object"), ) - ports = NumericArrayField( - label=_('Ports'), - base_field=forms.IntegerField( - min_value=SERVICE_PORT_MIN, - max_value=SERVICE_PORT_MAX - ), - required=False + remove_port_mappings = PortMappingField( + label=_('Remove port mappings'), + required=False, + help_text=_("Port mappings to remove from each selected object (if present)"), ) model = ServiceTemplate fieldsets = ( - FieldSet('protocol', 'ports', 'description'), + FieldSet('add_port_mappings', 'remove_port_mappings', 'description'), ) nullable_fields = ('description', 'comments') diff --git a/netbox/ipam/forms/bulk_import.py b/netbox/ipam/forms/bulk_import.py index 43a679f84..89dcdb645 100644 --- a/netbox/ipam/forms/bulk_import.py +++ b/netbox/ipam/forms/bulk_import.py @@ -1,5 +1,7 @@ from django import forms from django.contrib.contenttypes.models import ContentType +from django.contrib.postgres.forms import SimpleArrayField +from django.core.exceptions import ValidationError as DjangoValidationError from django.utils.translation import gettext_lazy as _ from dcim.forms.mixins import ScopedImportForm @@ -7,6 +9,7 @@ from dcim.models import Device, Interface, Site from ipam.choices import * from ipam.constants import * from ipam.models import * +from ipam.validators import validate_port_mappings from netbox.forms import NetBoxModelImportForm, OrganizationalModelImportForm, PrimaryModelImportForm from tenancy.models import Tenant from utilities.forms.fields import ( @@ -586,19 +589,41 @@ class VLANTranslationRuleImportForm(NetBoxModelImportForm): fields = ('policy', 'local_vid', 'remote_vid') -class ServiceTemplateImportForm(PrimaryModelImportForm): - protocol = CSVChoiceField( - label=_('Protocol'), - choices=ServiceProtocolChoices, - help_text=_('IP protocol') +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. + """ + 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").') ) + 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] + try: + mappings = validate_port_mappings(mappings) + except DjangoValidationError as exc: + raise forms.ValidationError(exc.messages) + return mappings + + +class ServiceTemplateImportForm(ServicePortMappingsImportMixin, PrimaryModelImportForm): + class Meta: model = ServiceTemplate - fields = ('name', 'protocol', 'ports', 'description', 'owner', 'comments', 'tags') + fields = ('name', 'port_mappings', 'description', 'owner', 'comments', 'tags') -class ServiceImportForm(PrimaryModelImportForm): +class ServiceImportForm(ServicePortMappingsImportMixin, PrimaryModelImportForm): parent_object_type = CSVContentTypeField( queryset=ContentType.objects.filter(SERVICE_ASSIGNMENT_MODELS), required=True, @@ -615,11 +640,6 @@ class ServiceImportForm(PrimaryModelImportForm): required=False, help_text=_('Parent object ID'), ) - protocol = CSVChoiceField( - label=_('Protocol'), - choices=ServiceProtocolChoices, - help_text=_('IP protocol') - ) ipaddresses = CSVModelMultipleChoiceField( queryset=IPAddress.objects.all(), required=False, @@ -630,7 +650,7 @@ class ServiceImportForm(PrimaryModelImportForm): class Meta: model = Service fields = ( - 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'owner', 'comments', 'tags', + 'ipaddresses', 'name', 'port_mappings', 'description', 'owner', 'comments', 'tags', ) def __init__(self, data=None, *args, **kwargs): diff --git a/netbox/ipam/forms/fields.py b/netbox/ipam/forms/fields.py new file mode 100644 index 000000000..66a03afe1 --- /dev/null +++ b/netbox/ipam/forms/fields.py @@ -0,0 +1,103 @@ +import json + +from django import forms +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + +from ipam.forms.widgets import PortMappingWidget +from ipam.utils import expand_port_mapping, group_port_mapping_rows +from ipam.validators import validate_port_mappings + +__all__ = ( + 'PortMappingField', +) + + +class PortMappingField(forms.Field): + """ + A form field for editing a service's port mappings. Presents one row per protocol (each with a + comma/range list of ports) but cleans to the model's flat list of ``protocol/port`` strings, e.g. + ``['tcp/80', 'tcp/443', 'udp/53']``. + """ + widget = PortMappingWidget + + def prepare_value(self, value): + # Group the flat ['tcp/80', 'tcp/443', 'udp/53'] list back into per-protocol rows for the widget. + if value in (None, ''): + return '[]' + if isinstance(value, str): + # An already-grouped JSON string (e.g. re-rendering a bound form) is passed through. A bare + # 'protocol/port' string arrives when cloning a single-mapping object: the querystring + # single-value collapse (normalize_querydict) yields a str rather than a list, so group it + # like the list case instead of handing the widget unparseable JSON (which blanks the row). + try: + json.loads(value) + except (TypeError, ValueError): + return json.dumps(group_port_mapping_rows([value])) + return value + return json.dumps(group_port_mapping_rows(value)) + + def to_python(self, value): + if value in (None, ''): + return [] + # A list is assumed to already be the flat ['tcp/80', ...] form (e.g. set programmatically) + if isinstance(value, list): + mappings = value + else: + try: + rows = json.loads(value) + except (TypeError, ValueError): + raise ValidationError(_("Invalid port mapping data.")) + if not isinstance(rows, list): + raise ValidationError(_("Invalid port mapping data.")) + + mappings = [] + for position, row in enumerate(rows, start=1): + # The widget's JS always submits a list of {protocol, ports} objects, but the hidden + # input is just POST data: a hand-crafted payload can put anything here, so validate the + # shape rather than letting a non-dict row raise AttributeError (a 500) on .get() below. + if not isinstance(row, dict): + raise ValidationError(_("Invalid port mapping data.")) + protocol = row.get('protocol') + raw_ports = row.get('ports') + # Likewise `protocol` is only ever a string, and `ports` either a string (the widget's + # comma/range format) or a list of ports (set programmatically); anything else would reach + # expand_port_mapping() and fail there on .strip(). + if ( + (protocol is not None and not isinstance(protocol, str)) + or (raw_ports is not None and not isinstance(raw_ports, (str, list))) + ): + raise ValidationError(_("Invalid port mapping data.")) + if isinstance(raw_ports, str): + raw_ports = raw_ports.strip() + # Ignore entirely-empty rows (e.g. the default blank row on an untouched form) + if not protocol and not raw_ports: + continue + # Expand via the shared helper, which accepts either the widget's comma/range string or an + # 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. + try: + mappings.extend(expand_port_mapping(protocol, raw_ports)) + except ValidationError as e: + raise ValidationError([ + _("Row {position}: {error}").format(position=position, error=message) + for message in e.messages + ]) + + # Shared validation returns the canonical (normalized) list of protocol/port strings + return validate_port_mappings(mappings) + + def has_changed(self, initial, data): + # Compare the parsed mappings rather than raw strings, so cosmetic differences (row/port + # ordering, whitespace) don't register as a change. + def normalize(value): + try: + return sorted(self.to_python(value)) + except ValidationError: + return None + + return normalize(self.prepare_value(initial)) != normalize(data) diff --git a/netbox/ipam/forms/filtersets.py b/netbox/ipam/forms/filtersets.py index 0bd60ef4f..925a153e9 100644 --- a/netbox/ipam/forms/filtersets.py +++ b/netbox/ipam/forms/filtersets.py @@ -640,12 +640,24 @@ class ServiceTemplateFilterForm(PrimaryModelFilterSetForm): model = ServiceTemplate fieldsets = ( FieldSet('q', 'filter_id', 'tag'), - FieldSet('protocol', 'port', name=_('Attributes')), + FieldSet('port_mappings', 'protocol', 'port', name=_('Attributes')), FieldSet('owner_group_id', 'owner_id', name=_('Ownership')), ) - protocol = forms.ChoiceField( + # A complete protocol/port pair, matched as a whole. Unlike `protocol` and `port` (which are + # correlated but independently specified), this is a single free-text value: an unknown protocol or + # malformed pair simply matches nothing, so no client-side validation is needed here. + port_mappings = forms.CharField( + label=_('Port mapping'), + required=False, + widget=forms.TextInput( + attrs={ + 'placeholder': 'e.g. tcp/80', + } + ) + ) + protocol = forms.MultipleChoiceField( label=_('Protocol'), - choices=add_blank_choice(ServiceProtocolChoices), + choices=ServiceProtocolChoices, required=False ) port = forms.IntegerField( @@ -659,7 +671,7 @@ class ServiceFilterForm(ContactModelFilterForm, ServiceTemplateFilterForm): model = Service fieldsets = ( FieldSet('q', 'filter_id', 'tag'), - FieldSet('protocol', 'port', name=_('Attributes')), + FieldSet('port_mappings', 'protocol', 'port', name=_('Attributes')), FieldSet('device_id', 'virtual_machine_id', 'fhrpgroup_id', name=_('Assignment')), FieldSet('owner_group_id', 'owner_id', name=_('Ownership')), FieldSet('contact', 'contact_role', 'contact_group', name=_('Contacts')), diff --git a/netbox/ipam/forms/model_forms.py b/netbox/ipam/forms/model_forms.py index ed88519cc..4c0e0933a 100644 --- a/netbox/ipam/forms/model_forms.py +++ b/netbox/ipam/forms/model_forms.py @@ -9,6 +9,7 @@ from dcim.models import Device, Interface, Site, SiteGroup from ipam.choices import * from ipam.constants import * from ipam.formfields import IPNetworkFormField +from ipam.forms.fields import PortMappingField from ipam.models import * from netbox.forms import NetBoxModelForm, OrganizationalModelForm, PrimaryModelForm from tenancy.forms import TenancyForm @@ -19,11 +20,10 @@ from utilities.forms.fields import ( DynamicModelChoiceField, DynamicModelMultipleChoiceField, GenericObjectChoiceField, - NumericArrayField, NumericRangeArrayField, TypedChoiceField, ) -from utilities.forms.rendering import FieldSet, InlineFields, ObjectAttribute, TabbedGroups +from utilities.forms.rendering import FieldSet, ObjectAttribute, TabbedGroups from utilities.forms.widgets import DatePicker from virtualization.models import VirtualMachine, VMInterface @@ -808,34 +808,31 @@ class VLANTranslationRuleForm(NetBoxModelForm): ] -class ServiceTemplateForm(PrimaryModelForm): - protocol = ChoiceField( - label=_('Protocol'), - choices=ServiceProtocolChoices, - ) - ports = NumericArrayField( - label=_('Ports'), - base_field=forms.IntegerField( - min_value=SERVICE_PORT_MIN, - max_value=SERVICE_PORT_MAX +class ServicePortMappingsMixin(forms.Form): + """ + Adds a ``port_mappings`` field (protocol + ports rows) to a Service/ServiceTemplate form. The field + maps directly to the model's ``port_mappings`` ArrayField, so no custom save handling is required. + """ + port_mappings = PortMappingField( + label=_('Port Mappings'), + help_text=_( + "One protocol per row, each with one or more port numbers. A range may be specified using a " + "hyphen (e.g. 80,443,8000-8010)." ), - help_text=_("Comma-separated list of one or more port numbers. A range may be specified using a hyphen.") ) + +class ServiceTemplateForm(ServicePortMappingsMixin, PrimaryModelForm): fieldsets = ( - FieldSet('name', 'protocol', 'ports', 'description', 'tags', name=_('Application Service Template')), + FieldSet('name', 'port_mappings', 'description', 'tags', name=_('Application Service Template')), ) class Meta: model = ServiceTemplate - fields = ('name', 'protocol', 'ports', 'description', 'owner', 'comments', 'tags') + fields = ('name', 'port_mappings', 'description', 'owner', 'comments', 'tags') -class ServiceForm(GenericObjectFormMixin, PrimaryModelForm): - protocol = ChoiceField( - label=_('Protocol'), - choices=ServiceProtocolChoices, - ) +class ServiceForm(ServicePortMappingsMixin, GenericObjectFormMixin, PrimaryModelForm): parent = GenericObjectChoiceField( label=_('Parent'), content_type_queryset=ContentType.objects.filter(SERVICE_ASSIGNMENT_MODELS), @@ -843,14 +840,6 @@ class ServiceForm(GenericObjectFormMixin, PrimaryModelForm): selector=True, hx_target_id='service', ) - ports = NumericArrayField( - label=_('Ports'), - base_field=forms.IntegerField( - min_value=SERVICE_PORT_MIN, - max_value=SERVICE_PORT_MAX - ), - help_text=_("Comma-separated list of one or more port numbers. A range may be specified using a hyphen.") - ) ipaddresses = DynamicModelMultipleChoiceField( queryset=IPAddress.objects.all(), required=False, @@ -859,8 +848,7 @@ class ServiceForm(GenericObjectFormMixin, PrimaryModelForm): fieldsets = ( FieldSet( - 'parent', 'name', - InlineFields('protocol', 'ports', label=_('Port(s)')), + 'parent', 'name', 'port_mappings', 'ipaddresses', 'description', 'tags', name=_('Application Service'), html_id='service', ), @@ -869,7 +857,7 @@ class ServiceForm(GenericObjectFormMixin, PrimaryModelForm): class Meta: model = Service fields = [ - 'name', 'protocol', 'ports', 'ipaddresses', 'description', 'owner', 'comments', 'tags', + 'name', 'port_mappings', 'ipaddresses', 'description', 'owner', 'comments', 'tags', ] def __init__(self, *args, **kwargs): @@ -898,7 +886,7 @@ class ServiceCreateForm(ServiceForm): 'parent', TabbedGroups( FieldSet('service_template', name=_('From Template')), - FieldSet('name', 'protocol', 'ports', name=_('Custom')), + FieldSet('name', 'port_mappings', name=_('Custom')), ), 'ipaddresses', 'description', 'tags', name=_('Application Service'), html_id='service', @@ -907,7 +895,7 @@ class ServiceCreateForm(ServiceForm): class Meta(ServiceForm.Meta): fields = [ - 'service_template', 'name', 'protocol', 'ports', 'ipaddresses', 'description', + 'service_template', 'name', 'port_mappings', 'ipaddresses', 'description', 'comments', 'tags', ] @@ -915,7 +903,7 @@ class ServiceCreateForm(ServiceForm): super().__init__(*args, **kwargs) # Fields which may be populated from a ServiceTemplate are not required - for field in ('name', 'protocol', 'ports'): + for field in ('name', 'port_mappings'): self.fields[field].required = False self.fields[field].widget.is_required = False @@ -925,11 +913,10 @@ class ServiceCreateForm(ServiceForm): # Create a new Service from the specified template service_template = self.cleaned_data['service_template'] self.cleaned_data['name'] = service_template.name - self.cleaned_data['protocol'] = service_template.protocol - self.cleaned_data['ports'] = service_template.ports + self.cleaned_data['port_mappings'] = list(service_template.port_mappings) if not self.cleaned_data['description']: self.cleaned_data['description'] = service_template.description - elif not all(self.cleaned_data[f] for f in ('name', 'protocol', 'ports')): + elif not self.cleaned_data.get('name') or not self.cleaned_data.get('port_mappings'): raise forms.ValidationError( - _("Must specify name, protocol, and port(s) if not using an application service template.") + _("Must specify name and port mapping(s) if not using an application service template.") ) diff --git a/netbox/ipam/forms/widgets.py b/netbox/ipam/forms/widgets.py new file mode 100644 index 000000000..3054bb9b9 --- /dev/null +++ b/netbox/ipam/forms/widgets.py @@ -0,0 +1,63 @@ +import json + +from django import forms +from django.forms.utils import flatatt + +from ipam.choices import ServiceProtocolChoices + +__all__ = ( + 'PortMappingWidget', +) + + +class PortMappingWidget(forms.Widget): + """ + Renders a dynamic set of (protocol, ports) rows. The rows are serialized to a JSON string held in a + single hidden input (client-side JS keeps the hidden input in sync as rows are added/removed). Each + row's ``ports`` value is a raw comma/range string (e.g. "80,443,8000-8010"); the server expands it. + """ + template_name = 'ipam/widgets/port_mappings.html' + + # aria-* attributes which render_field_with_aria() sets per-field, and which must be copied onto the + # row controls: the wrapping
isn't a form control, so assistive technology ignores them there. + CONTROL_ATTRS = ('aria-describedby', 'aria-invalid') + + def id_for_label(self, id_): + # The field's