Merge remote-tracking branch 'origin/main' into 18881-Site-Groups-are-missing-VLAN-and-VM-related-objects

netbox/circuits/graphql/types.py

@@ -43,7 +43,7 @@ class ProviderType(NetBoxObjectType, ContactsMixin):
     fields='__all__',
     filters=ProviderAccountFilter
 )
-class ProviderAccountType(NetBoxObjectType):
+class ProviderAccountType(ContactsMixin, NetBoxObjectType):
     provider: Annotated["ProviderType", strawberry.lazy('circuits.graphql.types')]
 
     circuits: List[Annotated["CircuitType", strawberry.lazy('circuits.graphql.types')]]

netbox/dcim/forms/bulk_import.py

@@ -1161,27 +1161,45 @@ class InventoryItemImportForm(NetBoxModelImportForm):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        super().clean()
+        cleaned_data = self.cleaned_data
+        component_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
         device = self.cleaned_data.get("device")
 
-        if not device and hasattr(self, 'instance') and hasattr(self.instance, 'device'):
-            device = self.instance.device
-
-        if not all([device, content_type, component_name]):
-            return None
-
-        model = content_type.model_class()
+        if component_type:
+            if device is None:
+                cleaned_data.pop('component_type', None)
+            if component_name is None:
+                cleaned_data.pop('component_type', None)
+                raise forms.ValidationError(
+                    _("Component name must be specified when component type is specified")
+                )
+            if all([device, component_name]):
                 try:
-            component = model.objects.get(device=device, name=component_name)
-            self.instance.component = component
+                    model = component_type.model_class()
+                    self.instance.component = model.objects.get(device=device, name=component_name)
                 except ObjectDoesNotExist:
+                    cleaned_data.pop('component_type', None)
+                    cleaned_data.pop('component_name', None)
                     raise forms.ValidationError(
                         _("Component not found: {device} - {component_name}").format(
                             device=device, component_name=component_name
                         )
                     )
+            else:
+                cleaned_data.pop('component_type', None)
+                if not component_name:
+                    raise forms.ValidationError(
+                        _("Component name must be specified when component type is specified")
+                    )
+        else:
+            if component_name:
+                raise forms.ValidationError(
+                    _("Component type must be specified when component name is specified")
+                )
+        return cleaned_data
 
 
 #

netbox/extras/dashboard/widgets.py

@@ -9,6 +9,7 @@ import requests
 from django import forms
 from django.conf import settings
 from django.core.cache import cache
+from django.db.models import Model
 from django.template.loader import render_to_string
 from django.urls import NoReverseMatch, resolve, reverse
 from django.utils.translation import gettext as _
@@ -42,6 +43,27 @@ def get_object_type_choices():
     ]
 
 
+def object_list_widget_supports_model(model: Model) -> bool:
+    """Test whether a model is supported by the ObjectListWidget
+
+    In theory there could be more than one reason why a model isn't supported by the
+    ObjectListWidget, although we've only identified one so far--there's no resolve-able 'list' URL
+    for the model. Add more tests if more conditions arise.
+    """
+    def can_resolve_model_list_view(model: Model) -> bool:
+        try:
+            reverse(get_viewname(model, action='list'))
+            return True
+        except Exception:
+            return False
+
+    tests = [
+        can_resolve_model_list_view,
+    ]
+
+    return all(test(model) for test in tests)
+
+
 def get_bookmarks_object_type_choices():
     return [
         (object_type_identifier(ot), object_type_name(ot))
@@ -234,6 +256,17 @@ class ObjectListWidget(DashboardWidget):
                     raise forms.ValidationError(_("Invalid format. URL parameters must be passed as a dictionary."))
             return data
 
+        def clean_model(self):
+            if model_info := self.cleaned_data['model']:
+                app_label, model_name = model_info.split('.')
+                model = ObjectType.objects.get_by_natural_key(app_label, model_name).model_class()
+                if not object_list_widget_supports_model(model):
+                    raise forms.ValidationError(
+                        _(f"Invalid model selection: {self['model'].data} is not supported.")
+                    )
+
+            return model_info
+
     def render(self, request):
         app_label, model_name = self.config['model'].split('.')
         model = ObjectType.objects.get_by_natural_key(app_label, model_name).model_class()
@@ -257,7 +290,7 @@ class ObjectListWidget(DashboardWidget):
             parameters['per_page'] = page_size
         parameters['embedded'] = True
 
-        if parameters:
+        if parameters and htmx_url is not None:
             try:
                 htmx_url = f'{htmx_url}?{urlencode(parameters, doseq=True)}'
             except ValueError:

netbox/extras/forms/model_forms.py

@@ -14,7 +14,7 @@ from netbox.events import get_event_type_choices
 from netbox.forms import NetBoxModelForm
 from tenancy.models import Tenant, TenantGroup
 from users.models import Group, User
-from utilities.forms import add_blank_choice, get_field_value
+from utilities.forms import get_field_value
 from utilities.forms.fields import (
     CommentField, ContentTypeChoiceField, ContentTypeMultipleChoiceField, DynamicModelChoiceField,
     DynamicModelMultipleChoiceField, JSONField, SlugField,
@@ -687,8 +687,7 @@ class ImageAttachmentForm(forms.ModelForm):
 class JournalEntryForm(NetBoxModelForm):
     kind = forms.ChoiceField(
         label=_('Kind'),
-        choices=add_blank_choice(JournalEntryKindChoices),
-        required=False
+        choices=JournalEntryKindChoices
     )
     comments = CommentField()
 

netbox/extras/management/commands/housekeeping.py

@@ -5,7 +5,6 @@ import requests
 from django.conf import settings
 from django.core.cache import cache
 from django.core.management.base import BaseCommand
-from django.db import DEFAULT_DB_ALIAS
 from django.utils import timezone
 from packaging import version
 
@@ -53,7 +52,7 @@ class Command(BaseCommand):
                         ending=""
                     )
                     self.stdout.flush()
-                ObjectChange.objects.filter(time__lt=cutoff)._raw_delete(using=DEFAULT_DB_ALIAS)
+                ObjectChange.objects.filter(time__lt=cutoff).delete()
                 if options['verbosity']:
                     self.stdout.write("Done.", self.style.SUCCESS)
             elif options['verbosity']:

netbox/extras/migrations/0123_journalentry_kind_default.py

@@ -0,0 +1,25 @@
+from django.db import migrations
+
+from extras.choices import JournalEntryKindChoices
+
+
+def set_kind_default(apps, schema_editor):
+    """
+    Set kind to "info" on any entries with no kind assigned.
+    """
+    JournalEntry = apps.get_model('extras', 'JournalEntry')
+    JournalEntry.objects.filter(kind='').update(kind=JournalEntryKindChoices.KIND_INFO)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('extras', '0122_charfield_null_choices'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=set_kind_default,
+            reverse_code=migrations.RunPython.noop
+        ),
+    ]

netbox/extras/tests/test_dashboard.py

@@ -0,0 +1,48 @@
+from django.test import tag, TestCase
+
+from extras.dashboard.widgets import ObjectListWidget
+
+
+class ObjectListWidgetTests(TestCase):
+    def test_widget_config_form_validates_model(self):
+        model_info = 'extras.notification'
+        form = ObjectListWidget.ConfigForm({'model': model_info})
+        self.assertFalse(form.is_valid())
+
+    @tag('regression')
+    def test_widget_fails_gracefully(self):
+        """
+        Example:
+        '2829fd9b-5dee-4c9a-81f2-5bd84c350a27': {
+            'class': 'extras.ObjectListWidget',
+            'color': 'indigo',
+            'title': 'Object List',
+            'config': {
+                'model': 'extras.notification',
+                'page_size': None,
+                'url_params': None
+            }
+        }
+        """
+        config = {
+            # 'class': 'extras.ObjectListWidget',  # normally popped off, left for clarity
+            'color': 'yellow',
+            'title': 'this should fail',
+            'config': {
+                'model': 'extras.notification',
+                'page_size': None,
+                'url_params': None,
+            },
+        }
+
+        class Request:
+            class User:
+                def has_perm(self, *args, **kwargs):
+                    return True
+
+            user = User()
+
+        mock_request = Request()
+        widget = ObjectListWidget(id='2829fd9b-5dee-4c9a-81f2-5bd84c350a27', **config)
+        rendered = widget.render(mock_request)
+        self.assertTrue('Unable to load content. Invalid view name:' in rendered)

netbox/extras/views.py

@@ -1098,8 +1098,8 @@ class DashboardWidgetAddView(LoginRequiredMixin, View):
         if not request.htmx:
             return redirect('home')
 
-        initial = request.GET or {
-            'widget_class': 'extras.NoteWidget',
+        initial = {
+            'widget_class': request.GET.get('widget_class') or 'extras.NoteWidget',
         }
         widget_form = DashboardWidgetAddForm(initial=initial)
         widget_name = get_field_value(widget_form, 'widget_class')

netbox/ipam/filtersets.py

@@ -232,6 +232,19 @@ class ASNFilterSet(OrganizationalModelFilterSet, TenancyFilterSet):
         to_field_name='slug',
         label=_('RIR (slug)'),
     )
+    site_group_id = TreeNodeMultipleChoiceFilter(
+        queryset=SiteGroup.objects.all(),
+        field_name='sites__group',
+        lookup_expr='in',
+        label=_('Site group (ID)'),
+    )
+    site_group = TreeNodeMultipleChoiceFilter(
+        queryset=SiteGroup.objects.all(),
+        field_name='sites__group',
+        lookup_expr='in',
+        to_field_name='slug',
+        label=_('Site group (slug)'),
+    )
     site_id = django_filters.ModelMultipleChoiceFilter(
         field_name='sites',
         queryset=Site.objects.all(),

netbox/ipam/forms/filtersets.py

@@ -142,7 +142,7 @@ class ASNFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm):
     model = ASN
     fieldsets = (
         FieldSet('q', 'filter_id', 'tag'),
-        FieldSet('rir_id', 'site_id', name=_('Assignment')),
+        FieldSet('rir_id', 'site_group_id', 'site_id', name=_('Assignment')),
         FieldSet('tenant_group_id', 'tenant_id', name=_('Tenant')),
     )
     rir_id = DynamicModelMultipleChoiceField(
@@ -150,6 +150,11 @@ class ASNFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm):
         required=False,
         label=_('RIR')
     )
+    site_group_id = DynamicModelMultipleChoiceField(
+        queryset=SiteGroup.objects.all(),
+        required=False,
+        label=_('Site group')
+    )
     site_id = DynamicModelMultipleChoiceField(
         queryset=Site.objects.all(),
         required=False,
@@ -418,7 +423,7 @@ class FHRPGroupFilterForm(NetBoxModelFilterSetForm):
 class VLANGroupFilterForm(NetBoxModelFilterSetForm):
     fieldsets = (
         FieldSet('q', 'filter_id', 'tag'),
-        FieldSet('region', 'sitegroup', 'site', 'location', 'rack', name=_('Location')),
+        FieldSet('region', 'site_group', 'site', 'location', 'rack', name=_('Location')),
         FieldSet('cluster_group', 'cluster', name=_('Cluster')),
         FieldSet('contains_vid', name=_('VLANs')),
     )
@@ -428,7 +433,7 @@ class VLANGroupFilterForm(NetBoxModelFilterSetForm):
         required=False,
         label=_('Region')
     )
-    sitegroup = DynamicModelMultipleChoiceField(
+    site_group = DynamicModelMultipleChoiceField(
         queryset=SiteGroup.objects.all(),
         required=False,
         label=_('Site group')

netbox/ipam/graphql/types.py

@@ -5,6 +5,7 @@ import strawberry_django
 
 from circuits.graphql.types import ProviderType
 from dcim.graphql.types import SiteType
+from extras.graphql.mixins import ContactsMixin
 from ipam import models
 from netbox.graphql.scalars import BigInt
 from netbox.graphql.types import BaseObjectType, NetBoxObjectType, OrganizationalObjectType
@@ -83,7 +84,7 @@ class ASNRangeType(NetBoxObjectType):
     fields='__all__',
     filters=AggregateFilter
 )
-class AggregateType(NetBoxObjectType, BaseIPAddressFamilyType):
+class AggregateType(NetBoxObjectType, ContactsMixin, BaseIPAddressFamilyType):
     prefix: str
     rir: Annotated["RIRType", strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
@@ -120,7 +121,7 @@ class FHRPGroupAssignmentType(BaseObjectType):
     exclude=('assigned_object_type', 'assigned_object_id', 'address'),
     filters=IPAddressFilter
 )
-class IPAddressType(NetBoxObjectType, BaseIPAddressFamilyType):
+class IPAddressType(NetBoxObjectType, ContactsMixin, BaseIPAddressFamilyType):
     address: str
     vrf: Annotated["VRFType", strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
@@ -144,7 +145,7 @@ class IPAddressType(NetBoxObjectType, BaseIPAddressFamilyType):
     fields='__all__',
     filters=IPRangeFilter
 )
-class IPRangeType(NetBoxObjectType):
+class IPRangeType(NetBoxObjectType, ContactsMixin):
     start_address: str
     end_address: str
     vrf: Annotated["VRFType", strawberry.lazy('ipam.graphql.types')] | None
@@ -157,7 +158,7 @@ class IPRangeType(NetBoxObjectType):
     exclude=('scope_type', 'scope_id', '_location', '_region', '_site', '_site_group'),
     filters=PrefixFilter
 )
-class PrefixType(NetBoxObjectType, BaseIPAddressFamilyType):
+class PrefixType(NetBoxObjectType, ContactsMixin, BaseIPAddressFamilyType):
     prefix: str
     vrf: Annotated["VRFType", strawberry.lazy('ipam.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
@@ -217,7 +218,7 @@ class RouteTargetType(NetBoxObjectType):
     fields='__all__',
     filters=ServiceFilter
 )
-class ServiceType(NetBoxObjectType):
+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

netbox/ipam/tests/test_filtersets.py

@@ -133,10 +133,18 @@ class ASNTestCase(TestCase, ChangeLoggedFilterSetTests):
         )
         ASN.objects.bulk_create(asns)
 
+        site_groups = (
+            SiteGroup(name='Site Group 1', slug='site-group-1'),
+            SiteGroup(name='Site Group 2', slug='site-group-2'),
+            SiteGroup(name='Site Group 3', slug='site-group-3'),
+        )
+        for site_group in site_groups:
+            site_group.save()
+
         sites = [
-            Site(name='Site 1', slug='site-1'),
-            Site(name='Site 2', slug='site-2'),
-            Site(name='Site 3', slug='site-3')
+            Site(name='Site 1', slug='site-1', group=site_groups[0]),
+            Site(name='Site 2', slug='site-2', group=site_groups[1]),
+            Site(name='Site 3', slug='site-3', group=site_groups[2]),
         ]
         Site.objects.bulk_create(sites)
         asns[0].sites.set([sites[0]])
@@ -178,6 +186,13 @@ class ASNTestCase(TestCase, ChangeLoggedFilterSetTests):
         params = {'rir': [rirs[0].slug, rirs[1].slug]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_site_group(self):
+        site_groups = SiteGroup.objects.all()[:2]
+        params = {'site_group_id': [site_groups[0].pk, site_groups[1].pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+        params = {'site_group': [site_groups[0].slug, site_groups[1].slug]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
+
     def test_site(self):
         sites = Site.objects.all()[:2]
         params = {'site_id': [sites[0].pk, sites[1].pk]}

netbox/netbox/authentication/__init__.py

@@ -28,7 +28,7 @@ AUTH_BACKEND_ATTRS = {
     'bitbucket-oauth2': ('BitBucket', 'bitbucket'),
     'digitalocean': ('DigitalOcean', 'digital-ocean'),
     'docker': ('Docker', 'docker'),
-    'github': ('GitHub', 'docker'),
+    'github': ('GitHub', 'github'),
     'github-app': ('GitHub', 'github'),
     'github-org': ('GitHub', 'github'),
     'github-team': ('GitHub', 'github'),

netbox/tenancy/graphql/types.py

@@ -3,7 +3,7 @@ from typing import Annotated, List
 import strawberry
 import strawberry_django
 
-from extras.graphql.mixins import CustomFieldsMixin, TagsMixin
+from extras.graphql.mixins import CustomFieldsMixin, TagsMixin, ContactsMixin
 from netbox.graphql.types import BaseObjectType, OrganizationalObjectType, NetBoxObjectType
 from tenancy import models
 from .mixins import ContactAssignmentsMixin
@@ -28,7 +28,7 @@ __all__ = (
     fields='__all__',
     filters=TenantFilter
 )
-class TenantType(NetBoxObjectType):
+class TenantType(ContactsMixin, NetBoxObjectType):
     group: Annotated["TenantGroupType", strawberry.lazy('tenancy.graphql.types')] | None
 
     asns: List[Annotated["ASNType", strawberry.lazy('ipam.graphql.types')]]

netbox/virtualization/api/serializers_/virtualmachines.py

@@ -112,10 +112,27 @@ class VMInterfaceSerializer(NetBoxModelSerializer):
         brief_fields = ('id', 'url', 'display', 'virtual_machine', 'name', 'description')
 
     def validate(self, data):
-
         # Validate many-to-many VLAN assignments
+        virtual_machine = None
+        tagged_vlans = []
+
+        # #18887
+        # There seem to be multiple code paths coming through here. Previously, we might either get
+        # the VirtualMachine instance from self.instance or from incoming data. However, #18887
+        # illustrated that this is also being called when a custom field pointing to an object_type
+        # of VMInterface is on the right side of a custom-field assignment coming in from an API
+        # request. As such, we need to check a third way to access the VirtualMachine
+        # instance--where `data` is the VMInterface instance itself and we can get the associated
+        # VirtualMachine via attribute access.
+        if isinstance(data, dict):
             virtual_machine = self.instance.virtual_machine if self.instance else data.get('virtual_machine')
-        for vlan in data.get('tagged_vlans', []):
+            tagged_vlans = data.get('tagged_vlans', [])
+        elif isinstance(data, VMInterface):
+            virtual_machine = data.virtual_machine
+            tagged_vlans = data.tagged_vlans.all()
+
+        if virtual_machine:
+            for vlan in tagged_vlans:
                 if vlan.site not in [virtual_machine.site, None]:
                     raise serializers.ValidationError({
                         'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent virtual "

netbox/virtualization/graphql/types.py

@@ -33,7 +33,7 @@ class ComponentType(NetBoxObjectType):
     exclude=('scope_type', 'scope_id', '_location', '_region', '_site', '_site_group'),
     filters=ClusterFilter
 )
-class ClusterType(VLANGroupsMixin, NetBoxObjectType):
+class ClusterType(ContactsMixin, VLANGroupsMixin, NetBoxObjectType):
     type: Annotated["ClusterTypeType", strawberry.lazy('virtualization.graphql.types')] | None
     group: Annotated["ClusterGroupType", strawberry.lazy('virtualization.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
@@ -55,7 +55,7 @@ class ClusterType(VLANGroupsMixin, NetBoxObjectType):
     fields='__all__',
     filters=ClusterGroupFilter
 )
-class ClusterGroupType(VLANGroupsMixin, OrganizationalObjectType):
+class ClusterGroupType(ContactsMixin, VLANGroupsMixin, OrganizationalObjectType):
 
     clusters: List[Annotated["ClusterType", strawberry.lazy('virtualization.graphql.types')]]
 

netbox/virtualization/tests/test_api.py

@@ -1,11 +1,15 @@
+from django.test import tag
 from django.urls import reverse
+from netaddr import IPNetwork
 from rest_framework import status
 
+from core.models import ObjectType
 from dcim.choices import InterfaceModeChoices
 from dcim.models import Site
-from extras.models import ConfigTemplate
+from extras.choices import CustomFieldTypeChoices
+from extras.models import ConfigTemplate, CustomField
 from ipam.choices import VLANQinQRoleChoices
-from ipam.models import VLAN, VRF
+from ipam.models import Prefix, VLAN, VRF
 from utilities.testing import APITestCase, APIViewTestCases, create_test_device, create_test_virtualmachine
 from virtualization.choices import *
 from virtualization.models import *
@@ -350,6 +354,39 @@ class VMInterfaceTest(APIViewTestCases.APIViewTestCase):
             },
         ]
 
+    @tag('regression')
+    def test_set_vminterface_as_object_in_custom_field(self):
+        cf = CustomField.objects.create(
+            name='associated_interface',
+            type=CustomFieldTypeChoices.TYPE_OBJECT,
+            related_object_type=ObjectType.objects.get_for_model(VMInterface),
+            required=False
+        )
+        cf.object_types.set([ObjectType.objects.get_for_model(Prefix)])
+        cf.save()
+
+        prefix = Prefix.objects.create(prefix=IPNetwork('10.0.0.0/12'))
+        vmi = VMInterface.objects.first()
+
+        url = reverse('ipam-api:prefix-detail', kwargs={'pk': prefix.pk})
+        data = {
+            'custom_fields': {
+                'associated_interface': vmi.id,
+            },
+        }
+
+        self.add_permissions('ipam.change_prefix')
+
+        response = self.client.patch(url, data, format='json', **self.header)
+        self.assertEqual(response.status_code, 200)
+
+        prefix_data = response.json()
+        self.assertEqual(prefix_data['custom_fields']['associated_interface']['id'], vmi.id)
+
+        reloaded_prefix = Prefix.objects.get(pk=prefix.pk)
+        self.assertEqual(prefix.pk, reloaded_prefix.pk)
+        self.assertNotEqual(reloaded_prefix.cf['associated_interface'], None)
+
     def test_bulk_delete_child_interfaces(self):
         interface1 = VMInterface.objects.get(name='Interface 1')
         virtual_machine = interface1.virtual_machine

netbox/vpn/graphql/types.py

@@ -27,7 +27,7 @@ __all__ = (
     fields='__all__',
     filters=TunnelGroupFilter
 )
-class TunnelGroupType(OrganizationalObjectType):
+class TunnelGroupType(ContactsMixin, OrganizationalObjectType):
 
     tunnels: List[Annotated["TunnelType", strawberry.lazy('vpn.graphql.types')]]
 
@@ -48,7 +48,7 @@ class TunnelTerminationType(CustomFieldsMixin, TagsMixin, ObjectType):
     fields='__all__',
     filters=TunnelFilter
 )
-class TunnelType(NetBoxObjectType):
+class TunnelType(ContactsMixin, NetBoxObjectType):
     group: Annotated["TunnelGroupType", strawberry.lazy('vpn.graphql.types')] | None
     ipsec_profile: Annotated["IPSecProfileType", strawberry.lazy('vpn.graphql.types')] | None
     tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None