From f96df730939a41e21a24b8c6c9215078c64026dc Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Fri, 11 Apr 2025 09:27:31 -0500 Subject: [PATCH] Closes #8423: Allow assigning Service to FHRP Group, in addition to Device and VirtualMachine (#19005) --- docs/models/ipam/service.md | 9 ++ netbox/dcim/models/devices.py | 8 +- netbox/ipam/api/serializers_/services.py | 29 ++++-- netbox/ipam/constants.py | 6 ++ netbox/ipam/filtersets.py | 71 +++++++++++---- netbox/ipam/forms/bulk_import.py | 69 ++++++++++---- netbox/ipam/forms/filtersets.py | 7 +- netbox/ipam/forms/model_forms.py | 67 +++++++++----- netbox/ipam/graphql/filters.py | 13 ++- netbox/ipam/graphql/types.py | 13 ++- .../0079_add_service_fhrp_group_parent_gfk.py | 29 ++++++ .../0080_populate_service_parent.py | 54 +++++++++++ ...ce_virtual_machine_add_parent_gfk_index.py | 39 ++++++++ netbox/ipam/models/fhrp.py | 6 ++ netbox/ipam/models/services.py | 42 +++------ netbox/ipam/search.py | 2 +- netbox/ipam/tests/test_api.py | 15 ++-- netbox/ipam/tests/test_filtersets.py | 56 ++++++++++-- netbox/ipam/tests/test_views.py | 89 ++++++++++++++++--- netbox/ipam/views.py | 36 ++++++-- netbox/templates/ipam/fhrpgroup.html | 1 + netbox/templates/ipam/service.html | 10 ++- .../virtualization/models/virtualmachines.py | 6 ++ 23 files changed, 537 insertions(+), 140 deletions(-) create mode 100644 netbox/ipam/migrations/0079_add_service_fhrp_group_parent_gfk.py create mode 100644 netbox/ipam/migrations/0080_populate_service_parent.py create mode 100644 netbox/ipam/migrations/0081_remove_service_device_virtual_machine_add_parent_gfk_index.py diff --git a/docs/models/ipam/service.md b/docs/models/ipam/service.md index 316828b61..0d5f12a17 100644 --- a/docs/models/ipam/service.md +++ b/docs/models/ipam/service.md @@ -6,6 +6,15 @@ To aid in the efficient creation of services, users may opt to first create a [s ## Fields +### Parent + +The parent object to which the service is assigned. This must be one of [Device](../dcim/device.md), +[VirtualMachine](../virtualization/virtualmachine.md), or [FHRP Group](./fhrpgroup.md). + +!!! note "Changed in NetBox v4.3" + + Previously, `parent` was a property that pointed to either a Device or Virtual Machine. With the capability to assign services to FHRP groups, this is a unified in a concrete field. + ### Name A service or protocol name. diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index 716b2aaf6..5988f8241 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -3,7 +3,7 @@ import yaml from functools import cached_property -from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.core.exceptions import ValidationError from django.core.files.storage import default_storage from django.core.validators import MaxValueValidator, MinValueValidator @@ -609,6 +609,12 @@ class Device( null=True, help_text=_("GPS coordinate in decimal format (xx.yyyyyy)") ) + services = GenericRelation( + to='ipam.Service', + content_type_field='parent_object_type', + object_id_field='parent_object_id', + related_query_name='device', + ) # Counter fields console_port_count = CounterCacheField( diff --git a/netbox/ipam/api/serializers_/services.py b/netbox/ipam/api/serializers_/services.py index 61b330d01..c7c1bb136 100644 --- a/netbox/ipam/api/serializers_/services.py +++ b/netbox/ipam/api/serializers_/services.py @@ -1,9 +1,13 @@ -from dcim.api.serializers_.devices import DeviceSerializer +from django.contrib.contenttypes.models import ContentType +from drf_spectacular.utils import extend_schema_field +from rest_framework import serializers + from ipam.choices import * +from ipam.constants import SERVICE_ASSIGNMENT_MODELS from ipam.models import IPAddress, Service, ServiceTemplate -from netbox.api.fields import ChoiceField, SerializedPKRelatedField +from netbox.api.fields import ChoiceField, ContentTypeField, SerializedPKRelatedField from netbox.api.serializers import NetBoxModelSerializer -from virtualization.api.serializers_.virtualmachines import VirtualMachineSerializer +from utilities.api import get_serializer_for_model from .ip import IPAddressSerializer __all__ = ( @@ -25,8 +29,6 @@ class ServiceTemplateSerializer(NetBoxModelSerializer): class ServiceSerializer(NetBoxModelSerializer): - device = DeviceSerializer(nested=True, required=False, allow_null=True) - virtual_machine = VirtualMachineSerializer(nested=True, required=False, allow_null=True) protocol = ChoiceField(choices=ServiceProtocolChoices, required=False) ipaddresses = SerializedPKRelatedField( queryset=IPAddress.objects.all(), @@ -35,11 +37,24 @@ class ServiceSerializer(NetBoxModelSerializer): required=False, many=True ) + parent_object_type = ContentTypeField( + queryset=ContentType.objects.filter(SERVICE_ASSIGNMENT_MODELS) + ) + parent = serializers.SerializerMethodField(read_only=True) class Meta: model = Service fields = [ - 'id', 'url', 'display_url', 'display', 'device', 'virtual_machine', 'name', 'protocol', 'ports', - 'ipaddresses', 'description', 'comments', 'tags', 'custom_fields', 'created', 'last_updated', + 'id', 'url', 'display_url', 'display', 'parent_object_type', 'parent_object_id', 'parent', 'name', + 'protocol', 'ports', 'ipaddresses', 'description', 'comments', 'tags', 'custom_fields', + 'created', 'last_updated', ] brief_fields = ('id', 'url', 'display', 'name', 'protocol', 'ports', 'description') + + @extend_schema_field(serializers.JSONField(allow_null=True)) + def get_parent(self, obj): + if obj.parent is None: + return None + serializer = get_serializer_for_model(obj.parent) + context = {'request': self.context['request']} + return serializer(obj.parent, nested=True, context=context).data diff --git a/netbox/ipam/constants.py b/netbox/ipam/constants.py index 6dffd3287..947f1adea 100644 --- a/netbox/ipam/constants.py +++ b/netbox/ipam/constants.py @@ -83,6 +83,12 @@ VLANGROUP_SCOPE_TYPES = ( # Services # +SERVICE_ASSIGNMENT_MODELS = Q( + Q(app_label='dcim', model='device') | + Q(app_label='ipam', model='fhrpgroup') | + Q(app_label='virtualization', model='virtualmachine') +) + # 16-bit port number SERVICE_PORT_MIN = 1 SERVICE_PORT_MAX = 65535 diff --git a/netbox/ipam/filtersets.py b/netbox/ipam/filtersets.py index aaa4b6b1c..554a2808b 100644 --- a/netbox/ipam/filtersets.py +++ b/netbox/ipam/filtersets.py @@ -1150,26 +1150,36 @@ class ServiceTemplateFilterSet(NetBoxModelFilterSet): return queryset.filter(qs_filter) -class ServiceFilterSet(ContactModelFilterSet, NetBoxModelFilterSet): - device_id = django_filters.ModelMultipleChoiceFilter( - queryset=Device.objects.all(), - label=_('Device (ID)'), - ) - device = django_filters.ModelMultipleChoiceFilter( - field_name='device__name', - queryset=Device.objects.all(), - to_field_name='name', +class ServiceFilterSet(NetBoxModelFilterSet): + device = MultiValueCharFilter( + method='filter_device', + field_name='name', label=_('Device (name)'), ) - virtual_machine_id = django_filters.ModelMultipleChoiceFilter( - queryset=VirtualMachine.objects.all(), + device_id = MultiValueNumberFilter( + method='filter_device', + field_name='pk', + label=_('Device (ID)'), + ) + virtual_machine = MultiValueCharFilter( + method='filter_virtual_machine', + field_name='name', + label=_('Virtual machine (name)'), + ) + virtual_machine_id = MultiValueNumberFilter( + method='filter_virtual_machine', + field_name='pk', label=_('Virtual machine (ID)'), ) - virtual_machine = django_filters.ModelMultipleChoiceFilter( - field_name='virtual_machine__name', - queryset=VirtualMachine.objects.all(), - to_field_name='name', - label=_('Virtual machine (name)'), + fhrpgroup = MultiValueCharFilter( + method='filter_fhrp_group', + field_name='name', + label=_('FHRP Group (name)'), + ) + fhrpgroup_id = MultiValueNumberFilter( + method='filter_fhrp_group', + field_name='pk', + label=_('FHRP Group (ID)'), ) ip_address_id = django_filters.ModelMultipleChoiceFilter( field_name='ipaddresses', @@ -1189,7 +1199,7 @@ class ServiceFilterSet(ContactModelFilterSet, NetBoxModelFilterSet): class Meta: model = Service - fields = ('id', 'name', 'protocol', 'description') + fields = ('id', 'name', 'protocol', 'description', 'parent_object_type', 'parent_object_id') def search(self, queryset, name, value): if not value.strip(): @@ -1197,6 +1207,33 @@ class ServiceFilterSet(ContactModelFilterSet, NetBoxModelFilterSet): qs_filter = Q(name__icontains=value) | Q(description__icontains=value) return queryset.filter(qs_filter) + def filter_device(self, queryset, name, value): + devices = Device.objects.filter(**{'{}__in'.format(name): value}) + if not devices.exists(): + return queryset.none() + service_ids = [] + for device in devices: + service_ids.extend(device.services.values_list('id', flat=True)) + return queryset.filter(id__in=service_ids) + + def filter_fhrp_group(self, queryset, name, value): + groups = FHRPGroup.objects.filter(**{'{}__in'.format(name): value}) + if not groups.exists(): + return queryset.none() + service_ids = [] + for group in groups: + service_ids.extend(group.services.values_list('id', flat=True)) + return queryset.filter(id__in=service_ids) + + def filter_virtual_machine(self, queryset, name, value): + virtual_machines = VirtualMachine.objects.filter(**{'{}__in'.format(name): value}) + if not virtual_machines.exists(): + return queryset.none() + service_ids = [] + for vm in virtual_machines: + service_ids.extend(vm.services.values_list('id', flat=True)) + return queryset.filter(id__in=service_ids) + class PrimaryIPFilterSet(django_filters.FilterSet): """ diff --git a/netbox/ipam/forms/bulk_import.py b/netbox/ipam/forms/bulk_import.py index fccf98bd8..d17944674 100644 --- a/netbox/ipam/forms/bulk_import.py +++ b/netbox/ipam/forms/bulk_import.py @@ -559,19 +559,21 @@ class ServiceTemplateImportForm(NetBoxModelImportForm): class ServiceImportForm(NetBoxModelImportForm): - device = CSVModelChoiceField( - label=_('Device'), + parent_object_type = CSVContentTypeField( + queryset=ContentType.objects.filter(SERVICE_ASSIGNMENT_MODELS), + required=True, + label=_('Parent type (app & model)') + ) + parent = CSVModelChoiceField( + label=_('Parent'), queryset=Device.objects.all(), required=False, to_field_name='name', - help_text=_('Required if not assigned to a VM') + help_text=_('Parent object name') ) - virtual_machine = CSVModelChoiceField( - label=_('Virtual machine'), - queryset=VirtualMachine.objects.all(), + parent_object_id = forms.IntegerField( required=False, - to_field_name='name', - help_text=_('Required if not assigned to a device') + help_text=_('Parent object ID'), ) protocol = CSVChoiceField( label=_('Protocol'), @@ -588,15 +590,52 @@ class ServiceImportForm(NetBoxModelImportForm): class Meta: model = Service fields = ( - 'device', 'virtual_machine', 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments', 'tags', + 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments', 'tags', ) - def clean_ipaddresses(self): - parent = self.cleaned_data.get('device') or self.cleaned_data.get('virtual_machine') - for ip_address in self.cleaned_data['ipaddresses']: - if not ip_address.assigned_object or getattr(ip_address.assigned_object, 'parent_object') != parent: + def __init__(self, data=None, *args, **kwargs): + super().__init__(data, *args, **kwargs) + + # Limit parent queryset by assigned parent object type + if data: + match data.get('parent_object_type'): + case 'dcim.device': + self.fields['parent'].queryset = Device.objects.all() + case 'ipam.fhrpgroup': + self.fields['parent'].queryset = FHRPGroup.objects.all() + case 'virtualization.virtualmachine': + self.fields['parent'].queryset = VirtualMachine.objects.all() + + def save(self, *args, **kwargs): + if (parent := self.cleaned_data.get('parent')): + self.instance.parent = parent + + return super().save(*args, **kwargs) + + def clean(self): + super().clean() + + if (parent_ct := self.cleaned_data.get('parent_object_type')): + if (parent := self.cleaned_data.get('parent')): + self.cleaned_data['parent_object_id'] = parent.pk + elif (parent_id := self.cleaned_data.get('parent_object_id')): + parent = parent_ct.model_class().objects.filter(id=parent_id).first() + self.cleaned_data['parent'] = parent + else: + # If a parent object type is passed and we've made it here, then raise a validation + # error since an associated parent object or parent object id has not been passed raise forms.ValidationError( - _("{ip} is not assigned to this device/VM.").format(ip=ip_address) + _("One of parent or parent_object_id must be included with parent_object_type") ) - return self.cleaned_data['ipaddresses'] + # making sure parent is defined. In cases where an import is resulting in an update, the + # import data might not include the parent object and so the above logic might not be + # triggered + parent = self.cleaned_data.get('parent') + for ip_address in self.cleaned_data.get('ipaddresses', []): + if not ip_address.assigned_object or getattr(ip_address.assigned_object, 'parent_object') != parent: + raise forms.ValidationError( + _("{ip} is not assigned to this parent.").format(ip=ip_address) + ) + + return self.cleaned_data diff --git a/netbox/ipam/forms/filtersets.py b/netbox/ipam/forms/filtersets.py index 4f75a015f..da4b1a808 100644 --- a/netbox/ipam/forms/filtersets.py +++ b/netbox/ipam/forms/filtersets.py @@ -612,7 +612,7 @@ class ServiceFilterForm(ContactModelFilterForm, ServiceTemplateFilterForm): fieldsets = ( FieldSet('q', 'filter_id', 'tag'), FieldSet('protocol', 'port', name=_('Attributes')), - FieldSet('device_id', 'virtual_machine_id', name=_('Assignment')), + FieldSet('device_id', 'virtual_machine_id', 'fhrpgroup_id', name=_('Assignment')), FieldSet('contact', 'contact_role', 'contact_group', name=_('Contacts')), ) device_id = DynamicModelMultipleChoiceField( @@ -625,4 +625,9 @@ class ServiceFilterForm(ContactModelFilterForm, ServiceTemplateFilterForm): required=False, label=_('Virtual Machine'), ) + fhrpgroup_id = DynamicModelMultipleChoiceField( + queryset=FHRPGroup.objects.all(), + required=False, + label=_('FHRP Group'), + ) tag = TagFilterField(model) diff --git a/netbox/ipam/forms/model_forms.py b/netbox/ipam/forms/model_forms.py index 37e7ec155..97ce3cd6a 100644 --- a/netbox/ipam/forms/model_forms.py +++ b/netbox/ipam/forms/model_forms.py @@ -21,7 +21,7 @@ from utilities.forms.rendering import FieldSet, InlineFields, ObjectAttribute, T from utilities.forms.utils import get_field_value from utilities.forms.widgets import DatePicker, HTMXSelect from utilities.templatetags.builtins.filters import bettertitle -from virtualization.models import VirtualMachine, VMInterface +from virtualization.models import VMInterface __all__ = ( 'AggregateForm', @@ -759,16 +759,17 @@ class ServiceTemplateForm(NetBoxModelForm): class ServiceForm(NetBoxModelForm): - device = DynamicModelChoiceField( - label=_('Device'), - queryset=Device.objects.all(), - required=False, - selector=True + parent_object_type = ContentTypeChoiceField( + queryset=ContentType.objects.filter(SERVICE_ASSIGNMENT_MODELS), + widget=HTMXSelect(), + required=True, + label=_('Parent type') ) - virtual_machine = DynamicModelChoiceField( - label=_('Virtual machine'), - queryset=VirtualMachine.objects.all(), - required=False, + parent = DynamicModelChoiceField( + label=_('Parent'), + queryset=Device.objects.none(), # Initial queryset + required=True, + disabled=True, selector=True ) ports = NumericArrayField( @@ -792,11 +793,7 @@ class ServiceForm(NetBoxModelForm): fieldsets = ( FieldSet( - TabbedGroups( - FieldSet('device', name=_('Device')), - FieldSet('virtual_machine', name=_('Virtual Machine')), - ), - 'name', + 'parent_object_type', 'parent', 'name', InlineFields('protocol', 'ports', label=_('Port(s)')), 'ipaddresses', 'description', 'tags', name=_('Service') ), @@ -805,9 +802,38 @@ class ServiceForm(NetBoxModelForm): class Meta: model = Service fields = [ - 'device', 'virtual_machine', 'name', 'protocol', 'ports', 'ipaddresses', 'description', 'comments', 'tags', + 'name', 'protocol', 'ports', 'ipaddresses', 'description', 'comments', 'tags', + 'parent_object_type', ] + def __init__(self, *args, **kwargs): + initial = kwargs.get('initial', {}).copy() + + if (instance := kwargs.get('instance', None)) and instance.parent: + initial['parent'] = instance.parent + + kwargs['initial'] = initial + + super().__init__(*args, **kwargs) + + if (parent_object_type_id := get_field_value(self, 'parent_object_type')): + try: + parent_type = ContentType.objects.get(pk=parent_object_type_id) + model = parent_type.model_class() + self.fields['parent'].queryset = model.objects.all() + self.fields['parent'].widget.attrs['selector'] = model._meta.label_lower + self.fields['parent'].disabled = False + self.fields['parent'].label = _(bettertitle(model._meta.verbose_name)) + except ObjectDoesNotExist: + pass + + if self.instance and parent_object_type_id != self.instance.parent_object_type_id: + self.initial['parent'] = None + + def clean(self): + super().clean() + self.instance.parent = self.cleaned_data.get('parent') + class ServiceCreateForm(ServiceForm): service_template = DynamicModelChoiceField( @@ -818,10 +844,7 @@ class ServiceCreateForm(ServiceForm): fieldsets = ( FieldSet( - TabbedGroups( - FieldSet('device', name=_('Device')), - FieldSet('virtual_machine', name=_('Virtual Machine')), - ), + 'parent_object_type', 'parent', TabbedGroups( FieldSet('service_template', name=_('From Template')), FieldSet('name', 'protocol', 'ports', name=_('Custom')), @@ -832,8 +855,8 @@ class ServiceCreateForm(ServiceForm): class Meta(ServiceForm.Meta): fields = [ - 'device', 'virtual_machine', 'service_template', 'name', 'protocol', 'ports', 'ipaddresses', 'description', - 'comments', 'tags', + 'service_template', 'name', 'protocol', 'ports', 'ipaddresses', 'description', + 'comments', 'tags', 'parent_object_type', ] def __init__(self, *args, **kwargs): diff --git a/netbox/ipam/graphql/filters.py b/netbox/ipam/graphql/filters.py index 2f4e185f1..a19262970 100644 --- a/netbox/ipam/graphql/filters.py +++ b/netbox/ipam/graphql/filters.py @@ -19,8 +19,7 @@ from tenancy.graphql.filter_mixins import ContactFilterMixin, TenancyFilterMixin if TYPE_CHECKING: from netbox.graphql.filter_lookups import IntegerArrayLookup, IntegerLookup from core.graphql.filters import ContentTypeFilter - from dcim.graphql.filters import DeviceFilter, SiteFilter - from virtualization.graphql.filters import VirtualMachineFilter + from dcim.graphql.filters import SiteFilter from vpn.graphql.filters import L2VPNFilter from .enums import * @@ -216,16 +215,14 @@ class RouteTargetFilter(TenancyFilterMixin, PrimaryModelFilterMixin): @strawberry_django.filter(models.Service, lookups=True) class ServiceFilter(ContactFilterMixin, ServiceBaseFilterMixin, PrimaryModelFilterMixin): - device: Annotated['DeviceFilter', strawberry.lazy('dcim.graphql.filters')] | None = strawberry_django.filter_field() - device_id: ID | None = strawberry_django.filter_field() - virtual_machine: Annotated['VirtualMachineFilter', strawberry.lazy('virtualization.graphql.filters')] | None = ( - strawberry_django.filter_field() - ) - virtual_machine_id: ID | None = strawberry_django.filter_field() name: FilterLookup[str] | None = strawberry_django.filter_field() ipaddresses: Annotated['IPAddressFilter', strawberry.lazy('ipam.graphql.filters')] | None = ( strawberry_django.filter_field() ) + parent_object_type: Annotated['ContentTypeFilter', strawberry.lazy('core.graphql.filters')] | None = ( + strawberry_django.filter_field() + ) + parent_object_id: ID | None = strawberry_django.filter_field() @strawberry_django.filter(models.ServiceTemplate, lookups=True) diff --git a/netbox/ipam/graphql/types.py b/netbox/ipam/graphql/types.py index 7744445b3..e8f98eabe 100644 --- a/netbox/ipam/graphql/types.py +++ b/netbox/ipam/graphql/types.py @@ -241,17 +241,22 @@ class RouteTargetType(NetBoxObjectType): @strawberry_django.type( models.Service, - fields='__all__', + exclude=('parent_object_type', 'parent_object_id'), filters=ServiceFilter, pagination=True ) class ServiceType(NetBoxObjectType, ContactsMixin): ports: List[int] - device: Annotated["DeviceType", strawberry.lazy('dcim.graphql.types')] | None - virtual_machine: Annotated["VirtualMachineType", strawberry.lazy('virtualization.graphql.types')] | None - ipaddresses: List[Annotated["IPAddressType", strawberry.lazy('ipam.graphql.types')]] + @strawberry_django.field + def parent(self) -> Annotated[Union[ + Annotated["DeviceType", strawberry.lazy('dcim.graphql.types')], + Annotated["VirtualMachineType", strawberry.lazy('virtualization.graphql.types')], + Annotated["FHRPGroupType", strawberry.lazy('ipam.graphql.types')], + ], strawberry.union("ServiceParentType")] | None: + return self.parent + @strawberry_django.type( models.ServiceTemplate, diff --git a/netbox/ipam/migrations/0079_add_service_fhrp_group_parent_gfk.py b/netbox/ipam/migrations/0079_add_service_fhrp_group_parent_gfk.py new file mode 100644 index 000000000..4ae5fd271 --- /dev/null +++ b/netbox/ipam/migrations/0079_add_service_fhrp_group_parent_gfk.py @@ -0,0 +1,29 @@ +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('ipam', '0078_iprange_mark_utilized'), + ] + + operations = [ + migrations.AddField( + model_name='service', + name='parent_object_id', + field=models.PositiveBigIntegerField(blank=True, null=True), + ), + migrations.AddField( + model_name='service', + name='parent_object_type', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name='+', + to='contenttypes.contenttype' + ), + ), + ] diff --git a/netbox/ipam/migrations/0080_populate_service_parent.py b/netbox/ipam/migrations/0080_populate_service_parent.py new file mode 100644 index 000000000..78f3086fc --- /dev/null +++ b/netbox/ipam/migrations/0080_populate_service_parent.py @@ -0,0 +1,54 @@ +from django.db import migrations +from django.db.models import F + + +def populate_service_parent_gfk(apps, schema_config): + Service = apps.get_model('ipam', 'Service') + ContentType = apps.get_model('contenttypes', 'ContentType') + Device = apps.get_model('dcim', 'device') + VirtualMachine = apps.get_model('virtualization', 'virtualmachine') + + Service.objects.filter(device_id__isnull=False).update( + parent_object_type=ContentType.objects.get_for_model(Device), + parent_object_id=F('device_id'), + ) + + Service.objects.filter(virtual_machine_id__isnull=False).update( + parent_object_type=ContentType.objects.get_for_model(VirtualMachine), + parent_object_id=F('virtual_machine_id'), + ) + + +def repopulate_device_and_virtualmachine_relations(apps, schemaconfig): + Service = apps.get_model('ipam', 'Service') + ContentType = apps.get_model('contenttypes', 'ContentType') + Device = apps.get_model('dcim', 'device') + VirtualMachine = apps.get_model('virtualization', 'virtualmachine') + + Service.objects.filter( + parent_object_type=ContentType.objects.get_for_model(Device), + ).update( + device_id=F('parent_object_id') + ) + + Service.objects.filter( + parent_object_type=ContentType.objects.get_for_model(VirtualMachine), + ).update( + virtual_machine_id=F('parent_object_id') + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0202_location_comments_region_comments_sitegroup_comments'), + ('ipam', '0079_add_service_fhrp_group_parent_gfk'), + ('virtualization', '0048_populate_mac_addresses'), + ] + + operations = [ + migrations.RunPython( + populate_service_parent_gfk, + reverse_code=repopulate_device_and_virtualmachine_relations, + ) + ] diff --git a/netbox/ipam/migrations/0081_remove_service_device_virtual_machine_add_parent_gfk_index.py b/netbox/ipam/migrations/0081_remove_service_device_virtual_machine_add_parent_gfk_index.py new file mode 100644 index 000000000..03b63cd12 --- /dev/null +++ b/netbox/ipam/migrations/0081_remove_service_device_virtual_machine_add_parent_gfk_index.py @@ -0,0 +1,39 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('extras', '0126_exporttemplate_file_name'), + ('ipam', '0080_populate_service_parent'), + ] + + operations = [ + migrations.RemoveField( + model_name='service', + name='device', + ), + migrations.RemoveField( + model_name='service', + name='virtual_machine', + ), + migrations.AlterField( + model_name='service', + name='parent_object_id', + field=models.PositiveBigIntegerField(), + ), + migrations.AlterField( + model_name='service', + name='parent_object_type', + field=models.ForeignKey( + on_delete=models.deletion.PROTECT, related_name='+', to='contenttypes.contenttype' + ), + ), + migrations.AddIndex( + model_name='service', + index=models.Index( + fields=['parent_object_type', 'parent_object_id'], name='ipam_servic_parent__563d2b_idx' + ), + ), + ] diff --git a/netbox/ipam/models/fhrp.py b/netbox/ipam/models/fhrp.py index f5982853e..63a04d4d1 100644 --- a/netbox/ipam/models/fhrp.py +++ b/netbox/ipam/models/fhrp.py @@ -48,6 +48,12 @@ class FHRPGroup(PrimaryModel): object_id_field='assigned_object_id', related_query_name='fhrpgroup' ) + services = GenericRelation( + to='ipam.Service', + content_type_field='parent_object_type', + object_id_field='parent_object_id', + related_query_name='fhrpgroup', + ) clone_fields = ('protocol', 'auth_type', 'auth_key', 'description') diff --git a/netbox/ipam/models/services.py b/netbox/ipam/models/services.py index bb4049781..2afd16076 100644 --- a/netbox/ipam/models/services.py +++ b/netbox/ipam/models/services.py @@ -1,5 +1,5 @@ +from django.contrib.contenttypes.fields import GenericForeignKey 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 from django.utils.translation import gettext_lazy as _ @@ -64,21 +64,17 @@ class Service(ContactsMixin, ServiceBase, PrimaryModel): A Service represents a layer-four service (e.g. HTTP or SSH) running on a Device or VirtualMachine. A Service may optionally be tied to one or more specific IPAddresses belonging to its parent. """ - device = models.ForeignKey( - to='dcim.Device', - on_delete=models.CASCADE, - related_name='services', - verbose_name=_('device'), - null=True, - blank=True + parent_object_type = models.ForeignKey( + to='contenttypes.ContentType', + on_delete=models.PROTECT, + related_name='+', ) - virtual_machine = models.ForeignKey( - to='virtualization.VirtualMachine', - on_delete=models.CASCADE, - related_name='services', - null=True, - blank=True + parent_object_id = models.PositiveBigIntegerField() + parent = GenericForeignKey( + ct_field='parent_object_type', + fk_field='parent_object_id' ) + name = models.CharField( max_length=100, verbose_name=_('name') @@ -91,22 +87,12 @@ class Service(ContactsMixin, ServiceBase, PrimaryModel): help_text=_("The specific IP addresses (if any) to which this service is bound") ) - clone_fields = ['protocol', 'ports', 'description', 'device', 'virtual_machine', 'ipaddresses', ] + clone_fields = ['protocol', 'ports', 'description', 'parent', 'ipaddresses', ] class Meta: + indexes = ( + models.Index(fields=('parent_object_type', 'parent_object_id')), + ) ordering = ('protocol', 'ports', 'pk') # (protocol, port) may be non-unique verbose_name = _('service') verbose_name_plural = _('services') - - @property - def parent(self): - return self.device or self.virtual_machine - - def clean(self): - super().clean() - - # A Service must belong to a Device *or* to a VirtualMachine - if self.device and self.virtual_machine: - raise ValidationError(_("A service cannot be associated with both a device and a virtual machine.")) - if not self.device and not self.virtual_machine: - raise ValidationError(_("A service must be associated with either a device or a virtual machine.")) diff --git a/netbox/ipam/search.py b/netbox/ipam/search.py index 6e71d44a5..63437e417 100644 --- a/netbox/ipam/search.py +++ b/netbox/ipam/search.py @@ -123,7 +123,7 @@ class ServiceIndex(SearchIndex): ('description', 500), ('comments', 5000), ) - display_attrs = ('device', 'virtual_machine', 'description') + display_attrs = ('parent', 'description') @register_search diff --git a/netbox/ipam/tests/test_api.py b/netbox/ipam/tests/test_api.py index e9dcacc16..4b9b340c4 100644 --- a/netbox/ipam/tests/test_api.py +++ b/netbox/ipam/tests/test_api.py @@ -1198,27 +1198,30 @@ class ServiceTest(APIViewTestCases.APIViewTestCase): Device.objects.bulk_create(devices) services = ( - 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(parent=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), + Service(parent=devices[0], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2]), + Service(parent=devices[0], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[3]), ) Service.objects.bulk_create(services) cls.create_data = [ { - 'device': devices[1].pk, + 'parent_object_id': devices[1].pk, + 'parent_object_type': 'dcim.device', 'name': 'Service 4', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, 'ports': [4], }, { - 'device': devices[1].pk, + 'parent_object_id': devices[1].pk, + 'parent_object_type': 'dcim.device', 'name': 'Service 5', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, 'ports': [5], }, { - 'device': devices[1].pk, + 'parent_object_id': devices[1].pk, + 'parent_object_type': 'dcim.device', 'name': 'Service 6', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, 'ports': [6], diff --git a/netbox/ipam/tests/test_filtersets.py b/netbox/ipam/tests/test_filtersets.py index e95aa14b1..9c500e002 100644 --- a/netbox/ipam/tests/test_filtersets.py +++ b/netbox/ipam/tests/test_filtersets.py @@ -1258,9 +1258,24 @@ class IPAddressTestCase(TestCase, ChangeLoggedFilterSetTests): IPAddress.objects.bulk_create(ipaddresses) services = ( - Service(name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), - Service(name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), - Service(name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]), + Service( + parent=devices[0], + name='Service 1', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), + Service( + parent=devices[1], + name='Service 2', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), + Service( + parent=devices[2], + name='Service 3', + protocol=ServiceProtocolChoices.PROTOCOL_TCP, + ports=[1], + ), ) Service.objects.bulk_create(services) services[0].ipaddresses.add(ipaddresses[0]) @@ -2329,41 +2344,57 @@ class ServiceTestCase(TestCase, ChangeLoggedFilterSetTests): VirtualMachine(name='Virtual Machine 3', cluster=cluster), ) VirtualMachine.objects.bulk_create(virtual_machines) + fhrp_group = FHRPGroup.objects.create( + name='telnet', + protocol=FHRPGroupProtocolChoices.PROTOCOL_CARP, + group_id=101, + ) services = ( Service( - device=devices[0], + parent=devices[0], name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1001], description='foobar1', ), Service( - device=devices[1], + parent=devices[1], name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1002], description='foobar2', ), - Service(device=devices[2], name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_UDP, ports=[1003]), Service( - virtual_machine=virtual_machines[0], + parent=devices[2], + name='Service 3', + protocol=ServiceProtocolChoices.PROTOCOL_UDP, + ports=[1003] + ), + Service( + parent=virtual_machines[0], name='Service 4', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2001], ), Service( - virtual_machine=virtual_machines[1], + parent=virtual_machines[1], name='Service 5', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[2002], ), Service( - virtual_machine=virtual_machines[2], + parent=virtual_machines[2], name='Service 6', protocol=ServiceProtocolChoices.PROTOCOL_UDP, ports=[2003], ), + Service( + parent=fhrp_group, + name='Service 7', + protocol=ServiceProtocolChoices.PROTOCOL_UDP, + ports=[2004], + ), ) Service.objects.bulk_create(services) services[0].ipaddresses.add(ip_addresses[0]) @@ -2404,6 +2435,13 @@ class ServiceTestCase(TestCase, ChangeLoggedFilterSetTests): params = {'virtual_machine': [vms[0].name, vms[1].name]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + def test_fhrp_group(self): + fhrp_group = FHRPGroup.objects.get() + params = {'fhrpgroup_id': [fhrp_group.pk]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + params = {'fhrpgroup': [fhrp_group.name]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + def test_ip_address(self): ips = IPAddress.objects.all()[:2] params = {'ip_address_id': [ips[0].pk, ips[1].pk]} diff --git a/netbox/ipam/tests/test_views.py b/netbox/ipam/tests/test_views.py index 345f39a51..8ba011d1d 100644 --- a/netbox/ipam/tests/test_views.py +++ b/netbox/ipam/tests/test_views.py @@ -5,11 +5,14 @@ from django.test import override_settings from django.urls import reverse from netaddr import IPNetwork +from core.models import ObjectType from dcim.constants import InterfaceTypeChoices from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Site, Interface from ipam.choices import * from ipam.models import * +from netbox.choices import CSVDelimiterChoices, ImportFormatChoices from tenancy.models import Tenant +from users.models import ObjectPermission from utilities.testing import ViewTestCases, create_tags @@ -1053,6 +1056,8 @@ class ServiceTemplateTestCase(ViewTestCases.PrimaryObjectViewTestCase): class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = Service + # TODO, related to #9816, cannot validate GFK + validation_excluded_fields = ('device',) @classmethod def setUpTestData(cls): @@ -1065,9 +1070,9 @@ class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): interface = Interface.objects.create(device=device, name='Interface 1', type=InterfaceTypeChoices.TYPE_VIRTUAL) services = ( - 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]), + Service(parent=device, name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[101]), + Service(parent=device, name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[102]), + Service(parent=device, name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[103]), ) Service.objects.bulk_create(services) @@ -1080,8 +1085,8 @@ class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): tags = create_tags('Alpha', 'Bravo', 'Charlie') cls.form_data = { - 'device': device.pk, - 'virtual_machine': None, + 'parent_object_type': ContentType.objects.get_for_model(Device).pk, + 'parent': device.pk, 'name': 'Service X', 'protocol': ServiceProtocolChoices.PROTOCOL_TCP, 'ports': '104,105', @@ -1091,10 +1096,10 @@ class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): } cls.csv_data = ( - "device,name,protocol,ports,ipaddresses,description", - "Device 1,Service 1,tcp,1,192.0.2.1/24,First service", - "Device 1,Service 2,tcp,2,192.0.2.2/24,Second service", - "Device 1,Service 3,udp,3,,Third service", + "parent_object_type,parent,name,protocol,ports,ipaddresses,description", + "dcim.device,Device 1,Service 1,tcp,1,192.0.2.1/24,First service", + "dcim.device,Device 1,Service 2,tcp,2,192.0.2.2/24,Second service", + "dcim.device,Device 1,Service 3,udp,3,,Third service", ) cls.csv_update_data = ( @@ -1110,6 +1115,66 @@ class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): 'description': 'New description', } + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) + def test_unassigned_ip_addresses(self): + device = Device.objects.first() + addr = IPAddress.objects.create(address='192.0.2.4/24') + csv_data = ( + "parent_object_type,parent_object_id,name,protocol,ports,ipaddresses,description", + f"dcim.device,{device.pk},Service 11,tcp,10,{addr.address},Eleventh service", + ) + + initial_count = self._get_queryset().count() + data = { + 'data': '\n'.join(csv_data), + 'format': ImportFormatChoices.CSV, + 'csv_delimiter': CSVDelimiterChoices.AUTO, + } + + # Assign model-level permission + obj_perm = ObjectPermission.objects.create(name='Test permission', actions=['add']) + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + # Test POST with permission + response = self.client.post(self._get_url('bulk_import'), data) + + self.assertHttpStatus(response, 200) + form_errors = response.context['form'].errors + self.assertEqual(len(form_errors), 1) + self.assertIn(addr.address, form_errors['__all__'][0]) + self.assertEqual(self._get_queryset().count(), initial_count) + + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) + def test_alternate_csv_import(self): + device = Device.objects.first() + interface = device.interfaces.first() + addr = IPAddress.objects.create(assigned_object=interface, address='192.0.2.3/24') + csv_data = ( + "parent_object_type,parent_object_id,name,protocol,ports,ipaddresses,description", + f"dcim.device,{device.pk},Service 11,tcp,10,{addr.address},Eleventh service", + ) + + initial_count = self._get_queryset().count() + data = { + 'data': '\n'.join(csv_data), + 'format': ImportFormatChoices.CSV, + 'csv_delimiter': CSVDelimiterChoices.AUTO, + } + + # Assign model-level permission + obj_perm = ObjectPermission.objects.create(name='Test permission', actions=['add']) + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + # Test POST with permission + response = self.client.post(self._get_url('bulk_import'), data) + + if response.status_code != 302: + self.assertEqual(response.context['form'].errors, {}) # debugging aid + self.assertHttpStatus(response, 302) + self.assertEqual(self._get_queryset().count(), initial_count + len(csv_data) - 1) + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_create_from_template(self): self.add_permissions('ipam.add_service') @@ -1125,13 +1190,15 @@ class ServiceTestCase(ViewTestCases.PrimaryObjectViewTestCase): request = { 'path': self._get_url('add'), 'data': { - 'device': device.pk, + 'parent_object_type': ContentType.objects.get_for_model(Device).pk, + 'parent': device.pk, 'service_template': service_template.pk, }, } + self.assertHttpStatus(self.client.post(**request), 302) instance = self._get_queryset().order_by('pk').last() - self.assertEqual(instance.device, device) + self.assertEqual(instance.parent, device) self.assertEqual(instance.name, service_template.name) self.assertEqual(instance.protocol, service_template.protocol) self.assertEqual(instance.ports, service_template.ports) diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index fc4406f7d..6dd946a6a 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -8,7 +8,7 @@ from django.utils.translation import gettext_lazy as _ from circuits.models import Provider from dcim.filtersets import InterfaceFilterSet from dcim.forms import InterfaceFilterForm -from dcim.models import Interface, Site +from dcim.models import Device, Interface, Site from ipam.tables import VLANTranslationRuleTable from netbox.views import generic from utilities.query import count_related @@ -16,7 +16,7 @@ from utilities.tables import get_table_ordering from utilities.views import GetRelatedModelsMixin, ViewTab, register_model_view from virtualization.filtersets import VMInterfaceFilterSet from virtualization.forms import VMInterfaceFilterForm -from virtualization.models import VMInterface +from virtualization.models import VirtualMachine, VMInterface from . import filtersets, forms, tables from .choices import PrefixStatusChoices from .constants import * @@ -1161,7 +1161,7 @@ class FHRPGroupListView(generic.ObjectListView): @register_model_view(FHRPGroup) -class FHRPGroupView(generic.ObjectView): +class FHRPGroupView(GetRelatedModelsMixin, generic.ObjectView): queryset = FHRPGroup.objects.all() def get_extra_context(self, request, instance): @@ -1173,6 +1173,18 @@ class FHRPGroupView(generic.ObjectView): members_table.columns.hide('group') return { + 'related_models': self.get_related_models( + request, instance, + extra=( + ( + Service.objects.restrict(request.user, 'view').filter( + parent_object_type=ContentType.objects.get_for_model(FHRPGroup), + parent_object_id=instance.id, + ), + 'fhrpgroup_id' + ), + ), + ), 'members_table': members_table, 'member_count': FHRPGroupAssignment.objects.filter(group=instance).count(), } @@ -1409,7 +1421,7 @@ class ServiceTemplateBulkDeleteView(generic.BulkDeleteView): @register_model_view(Service, 'list', path='', detail=False) class ServiceListView(generic.ObjectListView): - queryset = Service.objects.prefetch_related('device', 'virtual_machine') + queryset = Service.objects.prefetch_related('parent') filterset = filtersets.ServiceFilterSet filterset_form = forms.ServiceFilterForm table = tables.ServiceTable @@ -1419,6 +1431,18 @@ class ServiceListView(generic.ObjectListView): class ServiceView(generic.ObjectView): queryset = Service.objects.all() + def get_extra_context(self, request, instance): + context = {} + match instance.parent: + case Device(): + context['breadcrumb_queryparam'] = 'device_id' + case VirtualMachine(): + context['breadcrumb_queryparam'] = 'virtual_machine_id' + case FHRPGroup(): + context['breadcrumb_queryparam'] = 'fhrpgroup_id' + + return context + @register_model_view(Service, 'add', detail=False) class ServiceCreateView(generic.ObjectEditView): @@ -1445,7 +1469,7 @@ class ServiceBulkImportView(generic.BulkImportView): @register_model_view(Service, 'bulk_edit', path='edit', detail=False) class ServiceBulkEditView(generic.BulkEditView): - queryset = Service.objects.prefetch_related('device', 'virtual_machine') + queryset = Service.objects.prefetch_related('parent') filterset = filtersets.ServiceFilterSet table = tables.ServiceTable form = forms.ServiceBulkEditForm @@ -1453,6 +1477,6 @@ class ServiceBulkEditView(generic.BulkEditView): @register_model_view(Service, 'bulk_delete', path='delete', detail=False) class ServiceBulkDeleteView(generic.BulkDeleteView): - queryset = Service.objects.prefetch_related('device', 'virtual_machine') + queryset = Service.objects.prefetch_related('parent') filterset = filtersets.ServiceFilterSet table = tables.ServiceTable diff --git a/netbox/templates/ipam/fhrpgroup.html b/netbox/templates/ipam/fhrpgroup.html index 943b11bb8..ed4dadb78 100644 --- a/netbox/templates/ipam/fhrpgroup.html +++ b/netbox/templates/ipam/fhrpgroup.html @@ -58,6 +58,7 @@ + {% include 'inc/panels/related_objects.html' %} {% include 'inc/panels/custom_fields.html' %} {% plugin_right_page object %} diff --git a/netbox/templates/ipam/service.html b/netbox/templates/ipam/service.html index 950438f1e..6f1d7c630 100644 --- a/netbox/templates/ipam/service.html +++ b/netbox/templates/ipam/service.html @@ -7,10 +7,12 @@ {% block breadcrumbs %} {{ block.super }} - {% if object.device %} - - {% elif object.virtual_machine %} - + {% if object.parent and breadcrumb_queryparam %} + {% endif %} {% endblock %} diff --git a/netbox/virtualization/models/virtualmachines.py b/netbox/virtualization/models/virtualmachines.py index 1922922e8..aca2a7dbd 100644 --- a/netbox/virtualization/models/virtualmachines.py +++ b/netbox/virtualization/models/virtualmachines.py @@ -126,6 +126,12 @@ class VirtualMachine(ContactsMixin, ImageAttachmentsMixin, RenderConfigMixin, Co blank=True, max_length=50 ) + services = GenericRelation( + to='ipam.Service', + content_type_field='parent_object_type', + object_id_field='parent_object_id', + related_query_name='virtual_machine', + ) # Counter fields interface_count = CounterCacheField(