Merge pull request #22900 from netbox-community/22896-merge-main-into-feature

Merge main to feature
This commit is contained in:
bctiemann 2026-08-10 20:32:09 -04:00 committed by GitHub
commit bc879dc48f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
108 changed files with 9293 additions and 5690 deletions

View File

@ -33,7 +33,8 @@ jobs:
fetch-depth: 1
- name: Run Claude Code
id: claude
uses: anthropics/claude-code-action@11a9dadd198803a0cea6bd53da3e0e8a762fc6ea # v1.0.108
uses: anthropics/claude-code-action@be7b93b1907a4abad570368f3c74b6fe3807510b # v1.0.183
with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
github_token: ${{ secrets.GITHUB_TOKEN }}
claude_args: --model claude-opus-5

View File

@ -2,7 +2,7 @@
"openapi": "3.0.3",
"info": {
"title": "NetBox REST API",
"version": "4.6.6",
"version": "4.6.7",
"license": {
"name": "Apache v2 License"
}
@ -555,6 +555,30 @@
"explode": true,
"style": "form"
},
{
"in": "query",
"name": "member_type_id",
"schema": {
"type": "array",
"items": {
"type": "integer"
}
},
"explode": true,
"style": "form"
},
{
"in": "query",
"name": "member_type_id__n",
"schema": {
"type": "array",
"items": {
"type": "integer"
}
},
"explode": true,
"style": "form"
},
{
"in": "query",
"name": "modified_by_request",

View File

@ -30,6 +30,15 @@ Marking a field as required will force the user to provide a value for the field
A custom field must be assigned to one or more object types, or models, in NetBox. Once created, custom fields will automatically appear as part of these models in the web UI and REST API. Note that not all models support custom fields.
!!! info "This behavior changed in NetBox v4.6.8."
To improve performance when creating custom fields, empty field values are no longer pre-provisioned.
Unless the field has been assigned a default value, creating a custom field does not write a value to the objects which already exist. An object which has never been assigned a value simply stores nothing for the field, and reports the field as having no value in the web UI, REST API, GraphQL API, and exports, exactly as if it stored an explicit null.
This matters only if you query the underlying `custom_field_data` JSON directly, for example in a custom script. The field's key is absent from an object's data until a value is assigned to it, so read it with `obj.cf['field_name']` or `obj.custom_field_data.get('field_name')` rather than by direct subscript.
Assigning a default value, by contrast, does write that value to every existing object at the time the field is created, so that objects can be filtered by it immediately. On a model with a very large number of objects, this can take some time. Note that a default added to a field which already exists is _not_ backfilled: objects with no value continue to report none until they are next saved.
### Filtering
The filter logic controls how values are matched when filtering objects by the custom field. Loose filtering (the default) matches on a partial value, whereas exact matching requires a complete match of the given string to a field's value. For example, exact filtering with the string "red" will only match the exact value "red", whereas loose filtering will match on the values "red", "red-orange", or "bored". Setting the filter logic to "disabled" disables filtering by the field entirely.

View File

@ -23,6 +23,9 @@ Custom scripts are Python code which exists outside the NetBox code base, so the
## Writing Custom Scripts
!!! warning "Choose a unique file name"
A script file's name (without the `.py` extension) becomes its Python module name when the script is loaded. A script file must not share its name with a NetBox application (e.g. `circuits.py` or `dcim.py`) or any other installed Python module: the script will shadow that module in Python's import system and can break unrelated functionality. Choose a unique, descriptive file name, such as `circuit_maintenance.py`.
All custom scripts must inherit from the `extras.scripts.Script` base class. This class provides the functionality necessary to generate forms and log activity.
```python
@ -105,7 +108,7 @@ class MyScript(Script):
### `commit_default`
The checkbox to commit database changes when executing a script is checked by default. Set `commit_default` to False under the script's Meta class to leave this option unchecked by default.
The checkbox to commit database changes when executing a script is checked by default. Set `commit_default` to False under the script's Meta class to leave this option unchecked by default. This setting controls only the initial state of the execution form.
```python
commit_default = False
@ -117,7 +120,9 @@ By default, a script can be scheduled for execution at a later time. Setting `sc
### `notifications_default`
By default, a notification is generated for the requesting user each time a script finishes running. This attribute sets the initial value for the notifications field when running a script. Valid values are `always` (default), `on_failure`, and `never`.
By default, a notification is generated for the user associated with the script's job each time the script finishes running. This attribute sets the initial value for the notifications field when running a script. Valid values are `always` (default), `on_failure`, and `never`.
Scripts run from an event rule or the `runscript` management command use this value as their notification policy. For an event rule, the notification goes to the user associated with the triggering event, if there is one.
```python
notifications_default = 'on_failure'
@ -131,7 +136,7 @@ notifications_default = 'on_failure'
### `job_timeout`
Set the maximum allowed runtime for the script. If not set, `RQ_DEFAULT_TIMEOUT` will be used.
Set the maximum allowed runtime for the script. If not set, `RQ_DEFAULT_TIMEOUT` will be used. Scripts run from an event rule use this value as their execution timeout.
## Accessing Request Data

View File

@ -32,7 +32,12 @@ The events which will trigger the webhook. At least one event type must be selec
### URL
The URL to which the webhook HTTP request will be made.
The URL to which the webhook HTTP request will be made. Must be `http://` or `https://`, though
part or all of the value may be a Jinja2 template rendered at send time (e.g.
`http://{{ data.name }}.example.com/hook`, or `{{ data.custom_fields.callback_url }}` if the whole
URL comes from a template). A literal scheme is always validated as such, even if the rest of the
URL is templated; otherwise the value is checked only for valid Jinja2 syntax, since its rendered
value isn't known until the webhook actually fires.
### HTTP Method

View File

@ -4,6 +4,8 @@
Tenant groups may be nested recursively to achieve a multi-level hierarchy. For example, you might have a group called "Customers" containing subgroups of individual tenants grouped by product or account team.
A tenant group cannot be deleted if ungrouping its tenants, including those of any nested groups, would result in duplicate tenant names or slugs among ungrouped tenants.
## Fields
### Parent

View File

@ -1,5 +1,21 @@
# NetBox v4.6
## v4.6.7 (2026-07-30)
### Performance Improvements
* [#22810](https://github.com/netbox-community/netbox/issues/22810) - Skip cached scope rebuild for sites and locations when scope fields are unchanged
* [#22813](https://github.com/netbox-community/netbox/issues/22813) - Avoid extraneous database queries when fetching custom field data via the GraphQL API
* [#22822](https://github.com/netbox-community/netbox/issues/22822) - Avoid an extra database query when including rack reservation units via the GraphQL API
* [#22823](https://github.com/netbox-community/netbox/issues/22823) - Avoid extraneous database queries when fetching the IP address or prefix family via the GraphQL API
### Bug Fixes
* [#22738](https://github.com/netbox-community/netbox/issues/22738) - Correctly evaluate IP availability for users whose permissions are constrained by a custom field on a related object
* [#22800](https://github.com/netbox-community/netbox/issues/22800) - Filter circuit group assignments by member type to avoid displaying assignments belonging to a virtual circuit with the same ID
---
## v4.6.6 (2026-07-28)
### Enhancements

View File

@ -405,6 +405,11 @@ class CircuitGroupAssignmentFilterSet(NetBoxModelFilterSet):
label=_('Search'),
)
member_type = MultiValueContentTypeFilter()
member_type_id = django_filters.ModelMultipleChoiceFilter(
field_name='member_type',
queryset=ContentType.objects.all(),
distinct=False,
)
circuit = MultiValueCharFilter(
method='filter_circuit',
field_name='cid',
@ -450,7 +455,7 @@ class CircuitGroupAssignmentFilterSet(NetBoxModelFilterSet):
class Meta:
model = CircuitGroupAssignment
fields = ('id', 'member_id', 'priority')
fields = ('id', 'member_type_id', 'member_id', 'priority')
def search(self, queryset, name, value):
if not value.strip():

View File

@ -5,7 +5,9 @@ import strawberry_django
from circuits import models
from dcim.graphql.mixins import CabledObjectMixin
from dcim.models import Location, Region, Site, SiteGroup
from extras.graphql.mixins import ContactsMixin, CustomFieldsMixin, TagsMixin
from netbox.graphql.optimization import build_gfk_prefetch
from netbox.graphql.types import BaseObjectType, ObjectType, OrganizationalObjectType, PrimaryObjectType, register_type
from tenancy.graphql.types import TenantType
@ -74,7 +76,19 @@ class ProviderNetworkType(PrimaryObjectType):
class CircuitTerminationType(CustomFieldsMixin, TagsMixin, CabledObjectMixin, ObjectType):
circuit: Annotated['CircuitType', strawberry.lazy('circuits.graphql.types')]
@strawberry_django.field(prefetch_related='termination')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'termination',
[
Location,
Region,
SiteGroup,
Site,
models.ProviderNetwork,
],
),
only=['termination_type', 'termination_id'],
)
def termination(self) -> Annotated[
Annotated['LocationType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RegionType', strawberry.lazy('dcim.graphql.types')]
@ -133,7 +147,16 @@ class CircuitGroupType(OrganizationalObjectType):
class CircuitGroupAssignmentType(TagsMixin, BaseObjectType):
group: Annotated['CircuitGroupType', strawberry.lazy('circuits.graphql.types')]
@strawberry_django.field(prefetch_related='member')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'member',
[
models.Circuit,
models.VirtualCircuit,
],
),
only=['member_type', 'member_id'],
)
def member(self) -> Annotated[
Annotated['CircuitType', strawberry.lazy('circuits.graphql.types')]
| Annotated['VirtualCircuitType', strawberry.lazy('circuits.graphql.types')],

View File

@ -1,24 +1,24 @@
{
"circuit:api_list_objects": 16,
"circuit:list_objects_with_permission": 22,
"circuitgroup:api_list_objects": 13,
"circuitgroup:list_objects_with_permission": 20,
"circuit:api_list_objects": 15,
"circuit:list_objects_with_permission": 19,
"circuitgroup:api_list_objects": 12,
"circuitgroup:list_objects_with_permission": 17,
"circuitgroupassignment:api_list_objects": 17,
"circuitgroupassignment:list_objects_with_permission": 26,
"circuittermination:api_list_objects": 18,
"circuittermination:list_objects_with_permission": 24,
"circuittype:api_list_objects": 13,
"circuittype:list_objects_with_permission": 20,
"provider:api_list_objects": 15,
"provider:list_objects_with_permission": 20,
"provideraccount:api_list_objects": 14,
"provideraccount:list_objects_with_permission": 21,
"providernetwork:api_list_objects": 14,
"providernetwork:list_objects_with_permission": 21,
"virtualcircuit:api_list_objects": 16,
"virtualcircuit:list_objects_with_permission": 24,
"virtualcircuittermination:api_list_objects": 17,
"virtualcircuittermination:list_objects_with_permission": 23,
"virtualcircuittype:api_list_objects": 13,
"virtualcircuittype:list_objects_with_permission": 20
"circuitgroupassignment:list_objects_with_permission": 23,
"circuittermination:api_list_objects": 17,
"circuittermination:list_objects_with_permission": 21,
"circuittype:api_list_objects": 12,
"circuittype:list_objects_with_permission": 17,
"provider:api_list_objects": 14,
"provider:list_objects_with_permission": 17,
"provideraccount:api_list_objects": 13,
"provideraccount:list_objects_with_permission": 18,
"providernetwork:api_list_objects": 13,
"providernetwork:list_objects_with_permission": 18,
"virtualcircuit:api_list_objects": 15,
"virtualcircuit:list_objects_with_permission": 21,
"virtualcircuittermination:api_list_objects": 16,
"virtualcircuittermination:list_objects_with_permission": 20,
"virtualcircuittype:api_list_objects": 12,
"virtualcircuittype:list_objects_with_permission": 17
}

View File

@ -1,3 +1,4 @@
from django.contrib.contenttypes.models import ContentType
from django.test import TestCase
from circuits.choices import *
@ -779,6 +780,30 @@ class CircuitGroupAssignmentTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
params = {'virtual_circuit': [virtual_circuits[0].cid, virtual_circuits[1].cid]}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
def test_member_type(self):
params = {'member_type': ['circuits.circuit']}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
params = {'member_type_id': [ContentType.objects.get_for_model(Circuit).pk]}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
params = {'member_type_id': [ContentType.objects.get_for_model(VirtualCircuit).pk]}
self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
def test_member(self):
"""The member type and ID are matched together, so a matching ID of another type is excluded."""
circuit = Circuit.objects.first()
circuit_type = ContentType.objects.get_for_model(Circuit)
virtual_circuit_type = ContentType.objects.get_for_model(VirtualCircuit)
expected = self.queryset.get(member_type=circuit_type, member_id=circuit.pk)
# A virtual circuit assignment sharing the circuit's object ID must not match
group = CircuitGroup.objects.create(name='Circuit Group 4', slug='circuit-group-4')
CircuitGroupAssignment.objects.create(
group=group, member_type=virtual_circuit_type, member_id=circuit.pk
)
params = {'member_type_id': [circuit_type.pk], 'member_id': [circuit.pk]}
self.assertEqual(list(self.filterset(params, self.queryset).qs), [expected])
def test_provider(self):
providers = Provider.objects.all()[:2]
params = {'provider_id': [providers[0].pk, providers[1].pk]}

View File

@ -1,8 +1,8 @@
{
"datafile:api_list_objects": 10,
"datafile:list_objects_with_permission": 18,
"datasource:api_list_objects": 12,
"datasource:list_objects_with_permission": 20,
"datafile:list_objects_with_permission": 17,
"datasource:api_list_objects": 11,
"datasource:list_objects_with_permission": 17,
"job:api_list_objects": 12,
"job:list_objects_with_permission": 19
}

View File

@ -9,6 +9,7 @@ from core.graphql.mixins import ChangelogMixin
from dcim import models
from extras.graphql.mixins import ConfigContextMixin, ContactsMixin, ImageAttachmentsMixin
from ipam.graphql.mixins import IPAddressesMixin, VLANGroupsMixin
from netbox.graphql.optimization import build_gfk_prefetch
from netbox.graphql.scalars import BigInt
from netbox.graphql.types import (
BaseObjectType,
@ -21,7 +22,7 @@ from netbox.graphql.types import (
)
from users.graphql.mixins import OwnerMixin
from utilities.querysets import RestrictedPrefetch
from virtualization.models import Cluster
from virtualization.models import Cluster, VMInterface
from .filters import *
from .mixins import CabledObjectMixin, PathEndpointMixin
@ -159,7 +160,25 @@ class CableBundleType(PrimaryObjectType):
)
class CableTerminationType(NetBoxObjectType):
cable: Annotated['CableType', strawberry.lazy('dcim.graphql.types')] | None
termination: Annotated[
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'termination',
[
CircuitTermination,
models.ConsolePort,
models.ConsoleServerPort,
models.FrontPort,
models.Interface,
models.PowerFeed,
models.PowerOutlet,
models.PowerPort,
models.RearPort,
],
),
only=['termination_type', 'termination_id'],
)
def termination(self) -> Annotated[
Annotated['CircuitTerminationType', strawberry.lazy('circuits.graphql.types')]
| Annotated['ConsolePortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['ConsoleServerPortType', strawberry.lazy('dcim.graphql.types')]
@ -170,7 +189,8 @@ class CableTerminationType(NetBoxObjectType):
| Annotated['PowerPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RearPortType', strawberry.lazy('dcim.graphql.types')],
strawberry.union('CableTerminationTerminationType'),
] | None
] | None:
return self.termination
@register_type(
@ -343,22 +363,38 @@ class InventoryItemTemplateType(LtreeNodeMixin, ComponentTemplateType):
role: Annotated['InventoryItemRoleType', strawberry.lazy('dcim.graphql.types')] | None
manufacturer: Annotated['ManufacturerType', strawberry.lazy('dcim.graphql.types')]
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(prefetch_related='parent', only=['parent_id'])
def parent(self) -> Annotated['InventoryItemTemplateType', strawberry.lazy('dcim.graphql.types')] | None:
return self.parent
child_items: list[Annotated['InventoryItemTemplateType', strawberry.lazy('dcim.graphql.types')]]
component: Annotated[
Annotated['ConsolePortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['ConsoleServerPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['FrontPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['InterfaceType', strawberry.lazy('dcim.graphql.types')]
| Annotated['PowerOutletType', strawberry.lazy('dcim.graphql.types')]
| Annotated['PowerPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RearPortType', strawberry.lazy('dcim.graphql.types')],
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'component',
[
models.ConsolePortTemplate,
models.ConsoleServerPortTemplate,
models.FrontPortTemplate,
models.InterfaceTemplate,
models.PowerOutletTemplate,
models.PowerPortTemplate,
models.RearPortTemplate,
],
),
only=['component_type', 'component_id'],
)
def component(self) -> Annotated[
Annotated['ConsolePortTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['ConsoleServerPortTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['FrontPortTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['InterfaceTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['PowerOutletTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['PowerPortTemplateType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RearPortTemplateType', strawberry.lazy('dcim.graphql.types')],
strawberry.union('InventoryItemTemplateComponentType'),
] | None
] | None:
return self.component
@register_type(
@ -450,7 +486,16 @@ class FrontPortTemplateType(ModularComponentTemplateType):
class MACAddressType(PrimaryObjectType):
mac_address: str
@strawberry_django.field(prefetch_related='assigned_object')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'assigned_object',
[
models.Interface,
VMInterface,
],
),
only=['assigned_object_type', 'assigned_object_id'],
)
def assigned_object(self) -> Annotated[
Annotated['InterfaceType', strawberry.lazy('dcim.graphql.types')]
| Annotated['VMInterfaceType', strawberry.lazy('virtualization.graphql.types')],
@ -516,11 +561,26 @@ class InventoryItemType(LtreeNodeMixin, ComponentType):
child_items: list[Annotated['InventoryItemType', strawberry.lazy('dcim.graphql.types')]]
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(prefetch_related='parent', only=['parent_id'])
def parent(self) -> Annotated['InventoryItemType', strawberry.lazy('dcim.graphql.types')] | None:
return self.parent
component: Annotated[
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'component',
[
models.ConsolePort,
models.ConsoleServerPort,
models.FrontPort,
models.Interface,
models.PowerOutlet,
models.PowerPort,
models.RearPort,
],
),
only=['component_type', 'component_id'],
)
def component(self) -> Annotated[
Annotated['ConsolePortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['ConsoleServerPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['FrontPortType', strawberry.lazy('dcim.graphql.types')]
@ -529,7 +589,8 @@ class InventoryItemType(LtreeNodeMixin, ComponentType):
| Annotated['PowerPortType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RearPortType', strawberry.lazy('dcim.graphql.types')],
strawberry.union('InventoryItemComponentType'),
] | None
] | None:
return self.component
@register_type(
@ -633,7 +694,7 @@ class ModuleBayType(LtreeNodeMixin, ModularComponentType):
children: list[Annotated["ModuleBayType", strawberry.lazy('dcim.graphql.types')]]
module_bay_types: list[Annotated["ModuleBayTypeType", strawberry.lazy('dcim.graphql.types')]]
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(prefetch_related='parent', only=['parent_id'])
def parent(self) -> Annotated["ModuleBayType", strawberry.lazy('dcim.graphql.types')] | None:
return self.parent
@ -936,11 +997,15 @@ class RackReservationType(PrimaryObjectType):
@classmethod
def get_queryset(cls, queryset, info, **kwargs):
queryset = super().get_queryset(queryset, info, **kwargs)
# Annotate unit_count here so RackReservationFilter.unit_count can reference it. The field below
# is resolved from `units` rather than from this annotation, which is not applied on every path
# by which a RackReservation may be resolved.
return queryset.annotate(
unit_count=Func('units', function='CARDINALITY', output_field=IntegerField())
)
@strawberry.field
# Ensure `units` is fetched when `unit_count` is requested
@strawberry_django.field(only=['units'])
def unit_count(self) -> int:
return len(self.units)
@ -992,7 +1057,7 @@ class RegionType(VLANGroupsMixin, ContactsMixin, NestedLtreeGroupObjectType):
sites: list[Annotated["SiteType", strawberry.lazy('dcim.graphql.types')]]
children: list[Annotated["RegionType", strawberry.lazy('dcim.graphql.types')]]
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(prefetch_related='parent', only=['parent_id'])
def parent(self) -> Annotated["RegionType", strawberry.lazy('dcim.graphql.types')] | None:
return self.parent
@ -1069,7 +1134,7 @@ class SiteGroupType(VLANGroupsMixin, ContactsMixin, NestedLtreeGroupObjectType):
sites: list[Annotated["SiteType", strawberry.lazy('dcim.graphql.types')]]
children: list[Annotated["SiteGroupType", strawberry.lazy('dcim.graphql.types')]]
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(prefetch_related='parent', only=['parent_id'])
def parent(self) -> Annotated["SiteGroupType", strawberry.lazy('dcim.graphql.types')] | None:
return self.parent

View File

@ -812,18 +812,22 @@ class CablePath(models.Model):
super().save(*args, **kwargs)
# Record a direct reference to this CablePath on its originating object(s)
# Record a direct reference to this CablePath on its originating object(s). Only PathEndpoint
# subclasses carry the denormalized `_path` back-reference; other valid origins (e.g.
# CircuitTermination) do not, so skip the update for them.
origin_model = self.origin_type.model_class()
origin_ids = [decompile_path_node(node)[1] for node in self.path[0]]
chunked_update(origin_model.objects.filter(pk__in=origin_ids), _path=self.pk)
if issubclass(origin_model, PathEndpoint):
origin_ids = [decompile_path_node(node)[1] for node in self.path[0]]
chunked_update(origin_model.objects.filter(pk__in=origin_ids), _path=self.pk)
def delete(self, *args, **kwargs):
# Mirror save() - clear _path on origins to prevent stale references
# in table views that render _path.destinations
# in table views that render _path.destinations. Only PathEndpoint subclasses carry `_path`.
if self.path:
origin_model = self.origin_type.model_class()
origin_ids = [decompile_path_node(node)[1] for node in self.path[0]]
chunked_update(origin_model.objects.filter(pk__in=origin_ids, _path=self.pk), _path=None)
if issubclass(origin_model, PathEndpoint):
origin_ids = [decompile_path_node(node)[1] for node in self.path[0]]
chunked_update(origin_model.objects.filter(pk__in=origin_ids, _path=self.pk), _path=None)
super().delete(*args, **kwargs)

View File

@ -1,98 +1,98 @@
{
"cable:api_list_objects": 24,
"cable:list_objects_with_permission": 24,
"cablebundle:api_list_objects": 13,
"cablebundle:list_objects_with_permission": 20,
"cable:api_list_objects": 23,
"cable:list_objects_with_permission": 21,
"cablebundle:api_list_objects": 12,
"cablebundle:list_objects_with_permission": 17,
"cabletermination:api_list_objects": 16,
"consoleconnection:list_objects_with_permission": 29,
"consoleport:api_list_objects": 14,
"consoleport:list_objects_with_permission": 21,
"consoleport:api_list_objects": 13,
"consoleport:list_objects_with_permission": 18,
"consoleporttemplate:api_list_objects": 11,
"consoleserverport:api_list_objects": 14,
"consoleserverport:list_objects_with_permission": 21,
"consoleserverport:api_list_objects": 13,
"consoleserverport:list_objects_with_permission": 18,
"consoleserverporttemplate:api_list_objects": 11,
"coolingfeed:api_list_objects": 15,
"coolingfeed:list_objects_with_permission": 22,
"coolingintake:api_list_objects": 14,
"coolingintake:list_objects_with_permission": 21,
"coolingfeed:api_list_objects": 14,
"coolingfeed:list_objects_with_permission": 19,
"coolingintake:api_list_objects": 13,
"coolingintake:list_objects_with_permission": 18,
"coolingintaketemplate:api_list_objects": 11,
"coolingoutflow:api_list_objects": 14,
"coolingoutflow:list_objects_with_permission": 22,
"coolingoutflow:api_list_objects": 13,
"coolingoutflow:list_objects_with_permission": 19,
"coolingoutflowtemplate:api_list_objects": 11,
"coolingsource:api_list_objects": 15,
"coolingsource:list_objects_with_permission": 22,
"device:api_list_objects": 20,
"device:list_objects_with_permission": 25,
"devicebay:api_list_objects": 14,
"devicebay:list_objects_with_permission": 21,
"coolingsource:api_list_objects": 14,
"coolingsource:list_objects_with_permission": 19,
"device:api_list_objects": 19,
"device:list_objects_with_permission": 22,
"devicebay:api_list_objects": 13,
"devicebay:list_objects_with_permission": 18,
"devicebaytemplate:api_list_objects": 11,
"devicerole:api_list_objects": 13,
"devicerole:list_objects_with_permission": 20,
"devicetype:api_list_objects": 14,
"devicetype:list_objects_with_permission": 21,
"frontport:api_list_objects": 15,
"frontport:list_objects_with_permission": 25,
"devicerole:api_list_objects": 12,
"devicerole:list_objects_with_permission": 17,
"devicetype:api_list_objects": 13,
"devicetype:list_objects_with_permission": 18,
"frontport:api_list_objects": 14,
"frontport:list_objects_with_permission": 22,
"frontporttemplate:api_list_objects": 12,
"interface:api_list_objects": 23,
"interface:list_objects_with_permission": 21,
"interface:api_list_objects": 22,
"interface:list_objects_with_permission": 18,
"interfaceconnection:list_objects_with_permission": 41,
"interfacetemplate:api_list_objects": 11,
"inventoryitem:api_list_objects": 20,
"inventoryitem:list_objects_with_permission": 23,
"inventoryitemrole:api_list_objects": 13,
"inventoryitemrole:list_objects_with_permission": 20,
"inventoryitem:api_list_objects": 19,
"inventoryitem:list_objects_with_permission": 20,
"inventoryitemrole:api_list_objects": 12,
"inventoryitemrole:list_objects_with_permission": 17,
"inventoryitemtemplate:api_list_objects": 13,
"location:api_list_objects": 15,
"location:list_objects_with_permission": 22,
"macaddress:api_list_objects": 17,
"macaddress:list_objects_with_permission": 24,
"manufacturer:api_list_objects": 13,
"manufacturer:list_objects_with_permission": 20,
"module:api_list_objects": 20,
"module:list_objects_with_permission": 24,
"modulebay:api_list_objects": 16,
"modulebay:list_objects_with_permission": 21,
"location:api_list_objects": 14,
"location:list_objects_with_permission": 19,
"macaddress:api_list_objects": 16,
"macaddress:list_objects_with_permission": 21,
"manufacturer:api_list_objects": 12,
"manufacturer:list_objects_with_permission": 17,
"module:api_list_objects": 19,
"module:list_objects_with_permission": 21,
"modulebay:api_list_objects": 15,
"modulebay:list_objects_with_permission": 18,
"modulebaytemplate:api_list_objects": 12,
"modulebaytype:api_list_objects": 14,
"modulebaytype:list_objects_with_permission": 21,
"moduletype:api_list_objects": 15,
"moduletype:list_objects_with_permission": 22,
"moduletypeprofile:api_list_objects": 13,
"moduletypeprofile:list_objects_with_permission": 20,
"platform:api_list_objects": 13,
"platform:list_objects_with_permission": 21,
"modulebaytype:api_list_objects": 13,
"modulebaytype:list_objects_with_permission": 18,
"moduletype:api_list_objects": 14,
"moduletype:list_objects_with_permission": 19,
"moduletypeprofile:api_list_objects": 12,
"moduletypeprofile:list_objects_with_permission": 17,
"platform:api_list_objects": 12,
"platform:list_objects_with_permission": 18,
"powerconnection:list_objects_with_permission": 29,
"powerfeed:api_list_objects": 15,
"powerfeed:list_objects_with_permission": 22,
"poweroutlet:api_list_objects": 14,
"poweroutlet:list_objects_with_permission": 22,
"powerfeed:api_list_objects": 14,
"powerfeed:list_objects_with_permission": 19,
"poweroutlet:api_list_objects": 13,
"poweroutlet:list_objects_with_permission": 19,
"poweroutlettemplate:api_list_objects": 11,
"powerpanel:api_list_objects": 15,
"powerpanel:list_objects_with_permission": 22,
"powerport:api_list_objects": 14,
"powerport:list_objects_with_permission": 21,
"powerpanel:api_list_objects": 14,
"powerpanel:list_objects_with_permission": 19,
"powerport:api_list_objects": 13,
"powerport:list_objects_with_permission": 18,
"powerporttemplate:api_list_objects": 11,
"rack:api_list_objects": 17,
"rack:list_objects_with_permission": 29,
"rackgroup:api_list_objects": 13,
"rackgroup:list_objects_with_permission": 20,
"rackreservation:api_list_objects": 15,
"rackreservation:list_objects_with_permission": 23,
"rackrole:api_list_objects": 13,
"rackrole:list_objects_with_permission": 20,
"racktype:api_list_objects": 14,
"racktype:list_objects_with_permission": 21,
"rearport:api_list_objects": 15,
"rearport:list_objects_with_permission": 22,
"rack:api_list_objects": 16,
"rack:list_objects_with_permission": 26,
"rackgroup:api_list_objects": 12,
"rackgroup:list_objects_with_permission": 17,
"rackreservation:api_list_objects": 14,
"rackreservation:list_objects_with_permission": 20,
"rackrole:api_list_objects": 12,
"rackrole:list_objects_with_permission": 17,
"racktype:api_list_objects": 13,
"racktype:list_objects_with_permission": 18,
"rearport:api_list_objects": 14,
"rearport:list_objects_with_permission": 19,
"rearporttemplate:api_list_objects": 12,
"region:api_list_objects": 13,
"region:list_objects_with_permission": 20,
"site:api_list_objects": 17,
"site:list_objects_with_permission": 22,
"sitegroup:api_list_objects": 13,
"sitegroup:list_objects_with_permission": 20,
"virtualchassis:api_list_objects": 16,
"virtualchassis:list_objects_with_permission": 21,
"virtualdevicecontext:api_list_objects": 14,
"virtualdevicecontext:list_objects_with_permission": 20
"region:api_list_objects": 12,
"region:list_objects_with_permission": 17,
"site:api_list_objects": 16,
"site:list_objects_with_permission": 19,
"sitegroup:api_list_objects": 12,
"sitegroup:list_objects_with_permission": 17,
"virtualchassis:api_list_objects": 15,
"virtualchassis:list_objects_with_permission": 18,
"virtualdevicecontext:api_list_objects": 13,
"virtualdevicecontext:list_objects_with_permission": 17
}

View File

@ -2680,6 +2680,45 @@ class LegacyCablePathTestCase(BaseCablePathTestCase):
)
self.assertEqual(CablePath.objects.count(), 2)
def test_225_circuittermination_origin_passive_network(self):
"""
[CT1] --C1-- [RP1] [FP1]
A CircuitTermination cabled into a passive (FrontPort/RearPort-only) device can become a
CablePath origin. Unlike PathEndpoint origins, CircuitTermination has no `_path` back-reference
field, so saving and deleting such a path must not attempt to write it (see #22825).
"""
rearport1 = RearPort.objects.create(device=self.device, name='Rear Port 1')
frontport1 = FrontPort.objects.create(device=self.device, name='Front Port 1')
PortMapping.objects.create(
device=self.device, front_port=frontport1, front_port_position=1,
rear_port=rearport1, rear_port_position=1,
)
circuittermination1 = CircuitTermination.objects.create(
circuit=self.circuit,
termination=self.site,
term_side='A'
)
cable1 = Cable(
a_terminations=[circuittermination1],
b_terminations=[rearport1]
)
cable1.save()
# Re-fetch so the in-memory instance reflects the cable set above (from_origin reads .cable).
circuittermination1.refresh_from_db()
# A path traced from the CircuitTermination origin must save without raising FieldDoesNotExist
# on the missing `_path` field.
cablepath = CablePath.from_origin([circuittermination1])
cablepath.save()
self.assertEqual(cablepath.origin_type.model_class(), CircuitTermination)
self.assertEqual(cablepath.origins, [circuittermination1])
# Deleting the path must likewise not attempt to clear a nonexistent `_path` field.
cablepath.delete()
self.assertIsNone(CablePath.objects.filter(pk=cablepath.pk).first())
def test_301_create_path_via_existing_cable(self):
"""
[IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- [IF2]

View File

@ -211,9 +211,15 @@ class PathTraceView(generic.ObjectView):
# Get the total length of the cable and whether the length is definitive (fully defined)
total_length, is_definitive = path.get_total_length() if path else (None, False)
# Determine the path to the SVG trace image
api_viewname = f"{path.origin_type.app_label}-api:{path.origin_type.model}-trace"
svg_url = f"{reverse(api_viewname, kwargs={'pk': path.origins[0].pk})}?render=svg"
# Determine the path to the SVG trace image. The `-trace` API action (and the SVG renderer,
# which calls origin.trace()) exist only for PathEndpoint origins. Other valid origins such as
# CircuitTermination have no such action, so omit the SVG for them.
origin_model = path.origin_type.model_class()
if issubclass(origin_model, PathEndpoint):
api_viewname = f"{path.origin_type.app_label}-api:{path.origin_type.model}-trace"
svg_url = f"{reverse(api_viewname, kwargs={'pk': path.origins[0].pk})}?render=svg"
else:
svg_url = None
return {
'path': path,

View File

@ -19,7 +19,8 @@ __all__ = (
class CustomFieldChoiceSetSerializer(OwnerMixin, ChangeLogMessageSerializer, ValidatedModelSerializer):
base_choices = ChoiceField(
choices=CustomFieldChoiceSetBaseChoices,
required=False
required=False,
allow_null=True,
)
extra_choices = serializers.ListField(
child=serializers.ListField(

View File

@ -71,6 +71,8 @@ class ScriptAction(EventRuleAction):
'name': script.name,
'user': event_context['user'],
'data': action_data,
'notifications': script.notifications_default,
'job_timeout': script.job_timeout,
}
if 'snapshots' in event_context:
params['snapshots'] = event_context['snapshots']

View File

@ -1,13 +1,111 @@
from functools import cache
import django_filters
from django.db.models import Q
from .models import Tag
__all__ = (
'MissingKeyAwareFilterMixin',
'TagFilter',
'TagIDFilter',
'missing_key_aware_filter_factory',
)
class MissingKeyAwareFilterMixin:
"""
Treat a JSON key which is absent as equivalent to one holding a null value: an object storing
no value for a custom field must filter identically however that absence is represented.
Custom field data materializes a key only once a value is assigned to it (see
CustomField.populate_initial_data()), so an object predating a field carries no key for it at
all, whereas one whose value has been cleared holds a JSON null. Postgres treats the two
differently, in two places:
* Django compiles `exclude(custom_field_data__foo='x')` to a bare `NOT (data -> 'foo' = 'x')`.
A row which does not carry the key yields SQL NULL there, so the negation evaluates to NULL
and the row is discarded. A row holding a JSON null fares no better under any of the text
lookups (icontains, istartswith, etc.), which compare `data ->> 'foo'` and so are NULL for a
JSON null as well.
* The null sentinel (`?cf_foo=null`; see FILTERS_NULL_CHOICE_VALUE) asks for the objects holding
no value. MultipleChoiceFilter.filter() translates it to None and hands it to
get_filter_predicate(), which builds a lookup matching a JSON null only -- silently omitting
every object which predates the field.
Both directions are handled: the sentinel is mapped onto "holds no value" rather than onto a
predicate of its own, and a negation is built explicitly so that valueless rows are admitted.
Two constraints on where this may be mixed in, both satisfied by every filter class
CustomField.to_filter() can select:
* filter() is reimplemented rather than delegated to, so any custom filter() on the base class
is bypassed. Do not mix this into a class which overrides filter() (e.g.
MultiValueMACAddressFilter, MultiValueContentTypeFilter).
missing_key_aware_filter_factory() rejects such classes.
* `conjoined` is not honored: multiple values are always OR'ed. Passing it raises TypeError.
"""
def __init__(self, *args, **kwargs):
if kwargs.get('conjoined'):
raise TypeError(
f"{type(self).__name__} does not support conjoined filtering: multiple values are "
f"always OR'ed."
)
super().__init__(*args, **kwargs)
def filter(self, qs, value):
if not value:
return super().filter(qs, value)
# `<key>__isnull` matches only a missing key and `<key>=None` only a JSON null, so together
# they select exactly the objects holding no value. Both are null-safe, which is what makes
# them usable inside the negation below.
unset = Q(**{f'{self.field_name}__isnull': True}) | Q(**{self.field_name: None})
values = set(value)
match_unset = self.null_value in values
values.discard(self.null_value)
q = Q()
for v in values:
q |= Q(**self.get_filter_predicate(v))
if match_unset:
q |= unset
if self.exclude:
# Negate explicitly rather than deferring to exclude(), whose bare NOT discards the
# rows carrying no key. Those rows are admitted, unless holding no value is itself one
# of the things being excluded.
q = ~q if match_unset else ~q | unset
qs = qs.filter(q)
return qs.distinct() if self.distinct else qs
@cache
def missing_key_aware_filter_factory(filter_class):
"""
Return a subclass of the given filter class which treats an absent JSON key as equivalent to a
null one. Results are cached so that each filter class yields a single stable subclass.
The class must inherit MultipleChoiceFilter.filter() unmodified: the mixin reimplements it, so a
filter() of its own (and with it any custom predicate or short-circuit) would be silently
bypassed, yielding a wrong result set rather than an error.
"""
if filter_class.filter is not django_filters.MultipleChoiceFilter.filter:
raise TypeError(
f"{filter_class.__name__} cannot be made missing-key aware: it defines its own "
f"filter(), which MissingKeyAwareFilterMixin would bypass."
)
return type(
f'MissingKeyAware{filter_class.__name__}',
(MissingKeyAwareFilterMixin, filter_class),
{}
)
class TagFilter(django_filters.ModelMultipleChoiceFilter):
"""
Match on one or more assigned tags. If multiple tags are specified (e.g. ?tag=foo&tag=bar), the queryset is filtered

View File

@ -52,11 +52,16 @@ class CustomFieldsMixin:
@strawberry_django.field(only=['custom_field_data'])
def custom_fields(self) -> strawberry.scalars.JSON:
data = dict(self.custom_field_data)
for cf in CustomField.objects.get_for_model(type(self)):
if cf.name in data:
data[cf.name] = cf.resolve_selection_value(data[cf.name])
return data
# Emit a key for every custom field assigned to the model, as the REST API does, rather than
# returning the stored data verbatim. A key is materialized only once a value is assigned
# (see CustomField.populate_initial_data()), so an object which predates a field carries no
# key for it; without this, such a field would be absent from the response instead of null.
# CustomFieldManager.get_for_model() is served from the per-request cache, so this costs one
# query per model rather than one per object.
return {
cf.name: cf.resolve_selection_value(self.custom_field_data.get(cf.name))
for cf in CustomField.objects.get_for_model(self)
}
@strawberry.type

View File

@ -85,6 +85,7 @@ class Command(BaseCommand):
form.cleaned_data.pop('_schedule_at')
form.cleaned_data.pop('_interval')
form.cleaned_data.pop('_commit')
notifications = form.cleaned_data.pop('_notifications')
# Execute the script.
job = ScriptJob.enqueue(
@ -92,6 +93,7 @@ class Command(BaseCommand):
user=user,
immediate=True,
data=form.cleaned_data,
notifications=notifications,
request=NetBoxFakeRequest({
'META': {},
'COOKIES': {},

View File

@ -10,7 +10,6 @@ from django.conf import settings
from django.core.validators import RegexValidator, ValidationError
from django.db import models
from django.db.models import F, Func, Value
from django.db.models.expressions import RawSQL
from django.urls import reverse
from django.utils.html import escape
from django.utils.safestring import mark_safe
@ -70,10 +69,12 @@ class CustomFieldManager(models.Manager.from_queryset(RestrictedQuerySet)):
"""
Return all CustomFields assigned to the given model.
"""
# Check the request cache before hitting the database
# Check the request cache before hitting the database. Test the cached value against None
# rather than for truthiness: a model with no custom fields caches an empty QuerySet, which
# would otherwise be treated as a miss and re-queried on every call.
cache = query_cache.get()
if cache is not None:
if custom_fields := cache['custom_fields'].get(model._meta.model):
if (custom_fields := cache['custom_fields'].get(model._meta.model)) is not None:
return custom_fields
content_type = ObjectType.objects.get_for_model(model._meta.concrete_model)
@ -348,12 +349,16 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
"""
Populate initial custom field data upon either a) the creation of a new CustomField, or
b) the assignment of an existing CustomField to new object types.
Only a non-null default is written. A field with no default has no value to record, and an
absent key is equivalent to a null one everywhere the data is read (see CustomFieldsMixin),
so materializing a JSON null on every object would be a very expensive no-op: on a large
table it can outlast the request. Objects without the key simply report no value until one
is assigned.
"""
if self.default is None:
# We have to convert None to a JSON null for jsonb_set()
value = RawSQL("'null'::jsonb", [])
else:
value = Value(self.default, models.JSONField())
return
value = Value(self.default, models.JSONField())
for ct in content_types:
if model := ct.model_class():
chunked_update(
@ -370,11 +375,15 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
"""
Delete custom field data which is no longer relevant (either because the CustomField is
no longer assigned to a model, or because it has been deleted).
Only objects which actually hold a value for the field are rewritten. Because keys are
materialized only when a value is set (see populate_initial_data()), this typically
excludes the bulk of the table.
"""
for ct in content_types:
if model := ct.model_class():
chunked_update(
model.objects.all(),
model.objects.filter(custom_field_data__has_key=self.name),
custom_field_data=F('custom_field_data') - self.name
)
@ -386,14 +395,15 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
for ct in self.object_types.all():
if model := ct.model_class():
chunked_update(
model.objects.all(),
model.objects.filter(custom_field_data__has_key=old_name),
custom_field_data=Func(
F('custom_field_data') - old_name,
Value([new_name]),
Func(
F('custom_field_data'),
function='jsonb_extract_path_text',
template=f"to_jsonb(%(expressions)s -> '{old_name}')"
Value(old_name),
function='jsonb_extract_path',
output_field=models.JSONField()
),
function='jsonb_set')
)
@ -698,6 +708,9 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
:param lookup_expr: Custom lookup expression (optional)
"""
# Imported locally as extras.filters imports extras.models
from extras.filters import missing_key_aware_filter_factory
kwargs = {
'field_name': f'custom_field_data__{self.name}'
}
@ -763,6 +776,11 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
else:
return None
# A negated lookup must match objects which carry no key for this field at all; see
# MissingKeyAwareFilterMixin. BooleanFilter is never negated, so it is left alone.
if not issubclass(filter_class, django_filters.BooleanFilter):
filter_class = missing_key_aware_filter_factory(filter_class)
filter_instance = filter_class(**kwargs)
filter_instance.custom_field = self

View File

@ -1,4 +1,5 @@
import json
import re
import urllib.parse
from pathlib import Path
@ -38,7 +39,7 @@ from netbox.models.features import (
from netbox.models.mixins import OwnerMixin
from netbox.settings_utils import parse_job_timeout
from utilities.html import clean_html
from utilities.jinja2 import render_jinja2, sanitize_http_header
from utilities.jinja2 import JINJA2_TEMPLATE_RE, render_jinja2, sanitize_http_header, validate_jinja2_syntax
from utilities.querydict import dict_to_querydict
from utilities.querysets import RestrictedQuerySet
from utilities.tables import get_table_for_model
@ -55,6 +56,11 @@ __all__ = (
'Webhook',
)
# Matches a literal URL scheme (RFC 3986), independent of urlsplit()'s netloc parsing -- which can
# raise ValueError on a malformed host -- so a payload_url's scheme can always be read even when
# its host is templated or malformed.
LITERAL_SCHEME_RE = re.compile(r'^([a-zA-Z][a-zA-Z0-9+.-]*):')
class EventRule(CustomFieldsMixin, ExportTemplatesMixin, OwnerMixin, TagsMixin, ChangeLoggedModel):
"""
@ -220,8 +226,9 @@ class Webhook(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, OwnerMixin, Ch
max_length=500,
verbose_name=_('URL'),
help_text=_(
"This URL will be called using the HTTP method defined when the webhook is called. Jinja2 template "
"processing is supported with the same context as the request body."
"This URL will be called using the HTTP method defined when the webhook is called. Must be "
"http:// or https://. Jinja2 template processing is supported (with the same context as the "
"request body) for part or all of the URL."
)
)
http_method = models.CharField(
@ -322,11 +329,40 @@ class Webhook(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, OwnerMixin, Ch
def clean(self):
super().clean()
errors = {}
# CA file path requires SSL verification enabled
if not self.ssl_verification and self.ca_file_path:
raise ValidationError({
'ca_file_path': _('Do not specify a CA certificate file if SSL verification is disabled.')
})
errors['ca_file_path'] = _('Do not specify a CA certificate file if SSL verification is disabled.')
# payload_url may be a literal URL or a Jinja2 template (see its help_text). Skipped when
# blank; clean_fields() already flags that.
if self.payload_url:
if JINJA2_TEMPLATE_RE.search(self.payload_url):
# A literal, disallowed scheme (e.g. "file://") can never resolve no matter what
# else in the value is templated; anything else is checked for template syntax
# only, since its rendered result isn't known here.
match = LITERAL_SCHEME_RE.match(self.payload_url)
if match and match.group(1).lower() not in ('http', 'https'):
errors['payload_url'] = _("Enter a valid URL, beginning with http:// or https://.")
else:
try:
validate_jinja2_syntax(self.payload_url)
except ValidationError as e:
errors['payload_url'] = e
else:
# Fully literal -- validate directly rather than via URLValidator, which rejects
# single-label and underscore hosts that `requests` accepts fine. urlsplit() can
# raise ValueError for a malformed netloc (e.g. an unbalanced IPv6 bracket).
try:
scheme, netloc = urllib.parse.urlsplit(self.payload_url)[:2]
except ValueError:
scheme, netloc = '', ''
if scheme not in ('http', 'https') or not netloc:
errors['payload_url'] = _("Enter a valid URL, beginning with http:// or https://.")
if errors:
raise ValidationError(errors)
# A timeout which meets or exceeds the background job timeout leaves no room for the request's own timeout
# to apply: the worker will terminate the job first. (Staying below the job timeout does not guarantee that

View File

@ -26,20 +26,33 @@ from .utils import run_validators
#
def handle_cf_added_obj_types(instance, action, pk_set, **kwargs):
def handle_cf_object_types_changed(instance, action, pk_set, reverse, **kwargs):
"""
Handle the population of default/null values when a CustomField is added to one or more ContentTypes.
Handle the stored data of a CustomField as it is assigned to or unassigned from object types.
Only the forward direction is handled: every action below operates on the CustomField, whereas
the reverse of this relation (ContentType.custom_fields) reports the ContentType as the sender's
instance. Nothing in NetBox assigns object types that way.
"""
if reverse or action not in ('pre_clear', 'post_add', 'post_remove'):
return
if action == 'pre_clear':
# clear() unassigns every object type at once. It must be handled before the fact: no
# pk_set is reported for a clear, so the assignments have to be read while they still
# exist. (Note that set() diffs via remove()/add() by default, so it does not land here.)
instance.remove_stale_data(instance.object_types.all())
return
object_types = ContentType.objects.filter(pk__in=pk_set)
if action == 'post_add':
instance.populate_initial_data(ContentType.objects.filter(pk__in=pk_set))
# Populate the field's default value (if any) on all existing objects
instance.populate_initial_data(object_types)
def handle_cf_removed_obj_types(instance, action, pk_set, **kwargs):
"""
Handle the cleanup of old custom field data when a CustomField is removed from one or more ContentTypes.
"""
if action == 'post_remove':
instance.remove_stale_data(ContentType.objects.filter(pk__in=pk_set))
else:
# Remove the field's stored data from objects to which it no longer applies
instance.remove_stale_data(object_types)
def handle_cf_renamed(instance, created, **kwargs):
@ -59,8 +72,7 @@ def handle_cf_deleted(instance, **kwargs):
post_save.connect(handle_cf_renamed, sender=CustomField)
pre_delete.connect(handle_cf_deleted, sender=CustomField)
m2m_changed.connect(handle_cf_added_obj_types, sender=CustomField.object_types.through)
m2m_changed.connect(handle_cf_removed_obj_types, sender=CustomField.object_types.through)
m2m_changed.connect(handle_cf_object_types_changed, sender=CustomField.object_types.through)
#

View File

@ -4,7 +4,7 @@
"configcontext:api_list_objects": 22,
"configcontext:list_objects_with_permission": 16,
"configcontextprofile:api_list_objects": 12,
"configcontextprofile:list_objects_with_permission": 19,
"configcontextprofile:list_objects_with_permission": 16,
"configtemplate:api_list_objects": 10,
"configtemplate:list_objects_with_permission": 17,
"customfield:api_list_objects": 10,
@ -13,14 +13,14 @@
"customfieldchoiceset:list_objects_with_permission": 16,
"customlink:api_list_objects": 10,
"customlink:list_objects_with_permission": 18,
"eventrule:api_list_objects": 16,
"eventrule:list_objects_with_permission": 23,
"eventrule:api_list_objects": 15,
"eventrule:list_objects_with_permission": 20,
"exporttemplate:api_list_objects": 10,
"exporttemplate:list_objects_with_permission": 19,
"imageattachment:api_list_objects": 11,
"imageattachment:list_objects_with_permission": 21,
"journalentry:api_list_objects": 16,
"journalentry:list_objects_with_permission": 24,
"journalentry:api_list_objects": 15,
"journalentry:list_objects_with_permission": 21,
"notification:api_list_objects": 12,
"notificationgroup:api_list_objects": 11,
"notificationgroup:list_objects_with_permission": 18,
@ -32,6 +32,6 @@
"tag:api_list_objects": 10,
"tag:list_objects_with_permission": 18,
"taggeditem:api_list_objects": 12,
"webhook:api_list_objects": 13,
"webhook:list_objects_with_permission": 20
"webhook:api_list_objects": 12,
"webhook:list_objects_with_permission": 17
}

View File

@ -425,6 +425,36 @@ class CustomFieldChoiceSetTestCase(APIViewTestCases.APIViewTestCase):
response = self.client.post(self._get_list_url(), data, format='json', **self.header)
self.assertEqual(response.status_code, 400)
def test_null_base_choices(self):
"""
A null value for base_choices should be accepted, as returned by the API for a choice set which defines
only extra choices.
"""
self.add_permissions('extras.add_customfieldchoiceset', 'extras.change_customfieldchoiceset')
data = {
'name': 'test',
'base_choices': None,
'extra_choices': [
['choice1', 'Choice 1'],
],
}
response = self.client.post(self._get_list_url(), data, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_201_CREATED)
self.assertIsNone(response.data['base_choices'])
choice_set = CustomFieldChoiceSet.objects.get(pk=response.data['id'])
self.assertIsNone(choice_set.base_choices)
# A choice set with base choices assigned can be reverted to null
choice_set.base_choices = CustomFieldChoiceSetBaseChoices.IATA
choice_set.save()
response = self.client.patch(
self._get_detail_url(choice_set), {'base_choices': None}, format='json', **self.header
)
self.assertHttpStatus(response, status.HTTP_200_OK)
choice_set.refresh_from_db()
self.assertIsNone(choice_set.base_choices)
def test_invalid_choice_color(self):
self.add_permissions('extras.add_customfieldchoiceset')
data = {

View File

@ -1,9 +1,13 @@
import datetime
import json
from collections import defaultdict
from decimal import Decimal
from unittest.mock import patch
import django_filters
from django.core.exceptions import ValidationError
from django.db import connection
from django.db.models import QuerySet
from django.test import override_settings, tag
from django.test.utils import CaptureQueriesContext
from django.urls import reverse
@ -13,12 +17,15 @@ from core.models import ObjectChange, ObjectType
from dcim.filtersets import SiteFilterSet
from dcim.forms import SiteImportForm
from dcim.models import Manufacturer, Rack, Site
from dcim.tables import SiteTable
from extras.choices import *
from extras.filters import MissingKeyAwareFilterMixin, missing_key_aware_filter_factory
from extras.models import CustomField, CustomFieldChoiceSet
from ipam.models import VLAN
from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
from netbox.context import query_cache
from netbox.tables.columns import CustomFieldColumn
from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter
from utilities.testing import APITestCase, TestCase
from virtualization.models import VirtualMachine
@ -50,7 +57,7 @@ class CustomFieldTestCase(TestCase):
def test_text_field(self):
value = 'Foobar!'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='text_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
@ -58,7 +65,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -119,7 +126,7 @@ class CustomFieldTestCase(TestCase):
def test_longtext_field(self):
value = 'A' * 256
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='longtext_field',
type=CustomFieldTypeChoices.TYPE_LONGTEXT,
@ -127,7 +134,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -143,7 +150,7 @@ class CustomFieldTestCase(TestCase):
def test_integer_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='integer_field',
type=CustomFieldTypeChoices.TYPE_INTEGER,
@ -151,7 +158,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (123456, 0, -123456):
@ -169,7 +176,7 @@ class CustomFieldTestCase(TestCase):
def test_decimal_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='decimal_field',
type=CustomFieldTypeChoices.TYPE_DECIMAL,
@ -177,7 +184,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (123456.54, 0, -123456.78):
@ -195,7 +202,7 @@ class CustomFieldTestCase(TestCase):
def test_boolean_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='boolean_field',
type=CustomFieldTypeChoices.TYPE_INTEGER,
@ -203,7 +210,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (True, False):
@ -222,7 +229,7 @@ class CustomFieldTestCase(TestCase):
def test_date_field(self):
value = datetime.date(2016, 6, 23)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='date_field',
type=CustomFieldTypeChoices.TYPE_DATE,
@ -230,7 +237,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = cf.serialize(value)
@ -247,7 +254,7 @@ class CustomFieldTestCase(TestCase):
def test_datetime_field(self):
value = datetime.datetime(2016, 6, 23, 9, 45, 0)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='date_field',
type=CustomFieldTypeChoices.TYPE_DATETIME,
@ -255,7 +262,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = cf.serialize(value)
@ -272,7 +279,7 @@ class CustomFieldTestCase(TestCase):
def test_url_field(self):
value = 'http://example.com/'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='url_field',
type=CustomFieldTypeChoices.TYPE_URL,
@ -280,7 +287,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -297,7 +304,7 @@ class CustomFieldTestCase(TestCase):
def test_json_field(self):
value = '{"foo": 1, "bar": 2}'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='json_field',
type=CustomFieldTypeChoices.TYPE_JSON,
@ -305,7 +312,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -387,7 +394,7 @@ class CustomFieldTestCase(TestCase):
extra_choices=CHOICES
)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='select_field',
type=CustomFieldTypeChoices.TYPE_SELECT,
@ -396,7 +403,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -424,7 +431,7 @@ class CustomFieldTestCase(TestCase):
extra_choices=CHOICES
)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='multiselect_field',
type=CustomFieldTypeChoices.TYPE_MULTISELECT,
@ -433,7 +440,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -594,7 +601,7 @@ class CustomFieldTestCase(TestCase):
def test_object_field(self):
value = VLAN.objects.create(name='VLAN 1', vid=1).pk
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='object_field',
type=CustomFieldTypeChoices.TYPE_OBJECT,
@ -603,7 +610,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -626,7 +633,7 @@ class CustomFieldTestCase(TestCase):
VLAN.objects.bulk_create(vlans)
value = [vlan.pk for vlan in vlans]
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='object_field',
type=CustomFieldTypeChoices.TYPE_MULTIOBJECT,
@ -635,7 +642,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -710,13 +717,227 @@ class CustomFieldTestCase(TestCase):
0
)
# Removal: the key is stripped from every existing object when the field is deleted
# Removal: deleting the field strips the key from every existing object
cf.delete()
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='renamed_field').count(),
0
)
def test_provisioning_writes_nothing_without_a_default(self):
"""
A field with no default has no value to record, so creating one must not touch any object.
"""
cf = CustomField.objects.create(
name='unset_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
with CaptureQueriesContext(connection) as queries:
cf.object_types.set([self.object_type])
# No object data is written at all -- the cost of adding a field no longer scales with the
# number of objects it applies to
self.assertFalse([
q['sql'] for q in queries.captured_queries
if q['sql'].lstrip().upper().startswith('UPDATE "DCIM_SITE"'.upper())
])
self.assertEqual(Site.objects.filter(custom_field_data__has_key='unset_field').count(), 0)
for site in Site.objects.all():
self.assertEqual(site.custom_field_data, {})
self.assertIsNone(site.cf['unset_field'])
def test_provisioning_applies_a_default_immediately(self):
"""
A default value, by contrast, must be recorded on every existing object as soon as the
field is created -- it has to be filterable straight away, so it cannot be deferred.
"""
cf = CustomField.objects.create(
name='defaulted_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='bar'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__defaulted_field='bar').count(),
Site.objects.count()
)
def test_rename_touches_only_objects_holding_a_value(self):
"""
Renaming rewrites the key only where a value is actually stored. This is what keeps a
rename cheap now that objects are no longer provisioned with a placeholder each.
"""
cf = CustomField.objects.create(
name='sparse_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
cf.object_types.set([self.object_type])
site = Site.objects.first()
site.custom_field_data['sparse_field'] = 'value'
site.save()
cf.name = 'sparse_renamed'
cf.save()
self.assertEqual(
list(
Site.objects.filter(custom_field_data__has_key='sparse_renamed')
.values_list('pk', flat=True)
),
[site.pk]
)
self.assertEqual(Site.objects.filter(custom_field_data__has_key='sparse_field').count(), 0)
site.refresh_from_db()
self.assertEqual(site.custom_field_data['sparse_renamed'], 'value')
def test_removal_from_object_type_purges_data(self):
"""
Unassigning a field from an object type removes its data from those objects.
"""
cf = CustomField.objects.create(
name='unassigned_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='baz'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='unassigned_field').count(),
Site.objects.count()
)
cf.object_types.remove(self.object_type)
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='unassigned_field').count(),
0
)
def test_clearing_object_types_purges_data(self):
"""
clear() unassigns every object type at once and reports no pk_set, so it must be handled
before the fact. Its data is removed just as remove()'s is.
"""
cf = CustomField.objects.create(
name='cleared_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='baz'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='cleared_field').count(),
Site.objects.count()
)
cf.object_types.clear()
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='cleared_field').count(),
0
)
def test_batch_update_excludes_rows_which_no_longer_match(self):
"""
A caller's filters must constrain the UPDATE as well as the selection of each batch.
rename_object_data() builds a jsonb_set() expression which evaluates to NULL for a row not
holding the key being renamed, so a row which loses it between the two statements would
otherwise have its entire custom_field_data column nulled out.
"""
cf = CustomField.objects.create(
name='drifting_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
cf.object_types.set([self.object_type])
sites = list(Site.objects.order_by('pk'))
holder, bystander = sites[0], sites[-1]
Site.objects.filter(pk=holder.pk).update(custom_field_data={'drifting_field': 'value'})
Site.objects.filter(pk=bystander.pk).update(custom_field_data={'other': 'untouched'})
# Simulate a concurrent write: the batch selection yields a pk which no longer satisfies
# the has_key filter by the time the UPDATE is issued.
select_pks = QuerySet.values_list
injected = []
def inject_stale_pk(self, *args, **kwargs):
result = select_pks(self, *args, **kwargs)
if self.model is Site and args == ('pk',) and kwargs.get('flat') and not injected:
injected.append(bystander.pk)
return [*result, bystander.pk]
return result
with patch.object(QuerySet, 'values_list', inject_stale_pk):
cf.name = 'drifted_field'
cf.save()
self.assertEqual(injected, [bystander.pk], "the stale pk was never injected")
# The renamed value landed, and the bystander was left entirely alone
holder.refresh_from_db()
self.assertEqual(holder.custom_field_data, {'drifted_field': 'value'})
bystander.refresh_from_db()
self.assertEqual(bystander.custom_field_data, {'other': 'untouched'})
@staticmethod
def order_sites_by(*aliases):
"""
Order a SiteTable by the given column aliases and return the underlying QuerySet.
"""
table = SiteTable(Site.objects.all())
table.order_by = aliases
return table.data.data
def test_table_ordering_breaks_ties_by_primary_key(self):
"""
Rows tying on the sort value -- every object holding no value ties on both sort keys --
must still be totally ordered, or paginated results may skip or repeat rows between
page requests.
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
# None of these hold a value for the field, so all of them tie
Site.objects.bulk_create([
Site(name=f'Tied Site {i}', slug=f'tied-site-{i}') for i in range(1, 11)
])
for alias in ('cf_sort_field', '-cf_sort_field'):
ordered = self.order_sites_by(alias)
self.assertEqual(
ordered.query.order_by[-1],
'pk',
"the primary key must be applied as the final sort key"
)
# Paging through the results must yield each object exactly once
expected = [site.pk for site in ordered]
paginated = []
for offset in range(0, len(expected), 4):
paginated.extend(site.pk for site in ordered[offset:offset + 4])
self.assertEqual(paginated, expected)
def test_table_ordering_tolerates_a_repeated_sort_alias(self):
"""
The sort parameter is read with getlist(), so the same custom field column can appear in
the ordering more than once, applying the same annotation to the queryset twice.
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
table = SiteTable(Site.objects.all())
table.order_by = ['cf_sort_field', '-cf_sort_field']
self.assertEqual(len(list(table.rows)), Site.objects.count())
def test_default_value_validation(self):
choiceset = CustomFieldChoiceSet.objects.create(
name="Test Choice Set",
@ -873,6 +1094,22 @@ class CustomFieldManagerTestCase(TestCase):
self.assertEqual(CustomField.objects.get_for_model(Site).count(), 1)
self.assertEqual(CustomField.objects.get_for_model(VirtualMachine).count(), 0)
def test_get_for_model_caches_models_with_no_custom_fields(self):
"""
A model with no custom fields assigned must be served from the request cache like any other.
An empty QuerySet is falsy, so testing the cached value for truthiness would treat it as a
miss and re-query on every call.
"""
token = query_cache.set(defaultdict(dict))
self.addCleanup(query_cache.reset, token)
# Site has one custom field assigned, VirtualMachine none
for model in (Site, VirtualMachine):
# Prime the cache, iterating so that the QuerySet's own result cache is populated too
list(CustomField.objects.get_for_model(model))
with self.assertNumQueries(0):
list(CustomField.objects.get_for_model(model))
class CustomFieldAPITestCase(APITestCase):
@ -2058,6 +2295,77 @@ class CustomFieldModelTestCase(TestCase):
site.custom_field_data['baz'] = 'def'
site.clean()
def test_required_field_enforced_on_existing_objects(self):
"""
Adding a required custom field invalidates the objects which already exist, whether they
carry no key for it -- the normal state now that empty values are not provisioned -- or an
explicit null. Both are rejected, as they were before: every object then held a materialized
null, which CustomField.validate() rejects for a required field.
"""
site = Site.objects.create(name='Test Site', slug='test-site')
cf = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, name='req', required=True)
cf.save()
cf.object_types.set([ObjectType.objects.get_for_model(Site)])
# No value was provisioned onto the existing object
site.refresh_from_db()
self.assertNotIn('req', site.custom_field_data)
with self.assertRaises(ValidationError):
site.clean()
# An explicit null is rejected identically
site.custom_field_data['req'] = None
with self.assertRaises(ValidationError):
site.clean()
site.custom_field_data['req'] = 'value'
site.clean()
class MissingKeyAwareFilterTestCase(TestCase):
"""
MissingKeyAwareFilterMixin reimplements MultipleChoiceFilter.filter() for the negated case, so
it may only be mixed into a class which inherits that method unmodified and which does not
filter conjoined. Both constraints are enforced, as violating either would yield a wrong result
set rather than an error.
"""
def test_factory_rejects_a_class_which_defines_filter(self):
# MultiValueMACAddressFilter overrides filter() to swallow ValidationError
with self.assertRaises(TypeError):
missing_key_aware_filter_factory(MultiValueMACAddressFilter)
# BooleanFilter does not inherit MultipleChoiceFilter.filter() at all
with self.assertRaises(TypeError):
missing_key_aware_filter_factory(django_filters.BooleanFilter)
def test_factory_accepts_a_class_which_inherits_filter(self):
filter_class = missing_key_aware_filter_factory(MultiValueCharFilter)
self.assertTrue(issubclass(filter_class, MissingKeyAwareFilterMixin))
self.assertTrue(issubclass(filter_class, MultiValueCharFilter))
# The factory is cached, so a class yields a single stable subclass
self.assertIs(filter_class, missing_key_aware_filter_factory(MultiValueCharFilter))
def test_conjoined_filtering_is_rejected(self):
filter_class = missing_key_aware_filter_factory(MultiValueCharFilter)
filter_class(field_name='custom_field_data__foo')
filter_class(field_name='custom_field_data__foo', conjoined=False)
with self.assertRaises(TypeError):
filter_class(field_name='custom_field_data__foo', conjoined=True)
def test_every_supported_custom_field_type_satisfies_the_constraints(self):
"""
The filter classes CustomField.to_filter() selects must all remain admissible.
"""
for cf_type in CustomFieldTypeChoices.values():
with self.subTest(cf_type):
cf = CustomField(name='test', type=cf_type)
# Raises TypeError if the selected filter class violates a constraint
cf.to_filter()
cf.to_filter(lookup_expr='empty')
class CustomFieldModelFilterTestCase(TestCase):
queryset = Site.objects.all()
@ -2214,12 +2522,14 @@ class CustomFieldModelFilterTestCase(TestCase):
'cf11': manufacturers[2].pk,
'cf12': [manufacturers[2].pk, manufacturers[3].pk],
}),
# Carries no custom field data at all. Negated lookups ("is not x") match it, as they
# do an object holding an explicit null; see MissingKeyAwareFilterMixin.
Site(name='Site 4', slug='site-4'),
])
def test_filter_integer(self):
self.assertEqual(self.filterset({'cf_cf1': [100, 200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__n': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__n': [200]}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf1__gt': [200]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf1__gte': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__lt': [200]}, self.queryset).qs.count(), 1)
@ -2228,7 +2538,7 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_decimal(self):
self.assertEqual(self.filterset({'cf_cf2': [100.1, 200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__n': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__n': [200.2]}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf2__gt': [200.2]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf2__gte': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__lt': [200.2]}, self.queryset).qs.count(), 1)
@ -2241,15 +2551,15 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_text_strict(self):
self.assertEqual(self.filterset({'cf_cf4': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__n': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__n': ['foo']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf4__ic': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nic': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nic': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__isw': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nisw': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nisw': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__iew': ['bar']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__niew': ['bar']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__niew': ['bar']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__ie': ['FOO']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nie': ['FOO']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nie': ['FOO']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf4__empty': True}, self.queryset).qs.count(), 1)
def test_filter_text_loose(self):
@ -2257,7 +2567,7 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_date(self):
self.assertEqual(self.filterset({'cf_cf6': ['2016-06-26', '2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__n': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__n': ['2016-06-27']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf6__gt': ['2016-06-27']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf6__gte': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__lt': ['2016-06-27']}, self.queryset).qs.count(), 1)
@ -2269,20 +2579,108 @@ class CustomFieldModelFilterTestCase(TestCase):
self.filterset({'cf_cf7': ['http://a.example.com', 'http://b.example.com']}, self.queryset).qs.count(),
2
)
self.assertEqual(self.filterset({'cf_cf7__n': ['http://b.example.com']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__n': ['http://b.example.com']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__ic': ['b']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__nic': ['b']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__nic': ['b']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__isw': ['http://']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__nisw': ['http://']}, self.queryset).qs.count(), 0)
self.assertEqual(self.filterset({'cf_cf7__nisw': ['http://']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__iew': ['.com']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__niew': ['.com']}, self.queryset).qs.count(), 0)
self.assertEqual(self.filterset({'cf_cf7__niew': ['.com']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__ie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__empty': True}, self.queryset).qs.count(), 1)
def test_filter_url_loose(self):
self.assertEqual(self.filterset({'cf_cf8': ['example.com']}, self.queryset).qs.count(), 3)
def test_filter_negation_matches_unset_values(self):
"""
A negated lookup must match an object which holds no value for the field, whether that is
recorded as an explicit null or by the absence of the key; see MissingKeyAwareFilterMixin.
"""
no_key = Site.objects.get(slug='site-4')
explicit_null = Site.objects.create(name='Site 5', slug='site-5', custom_field_data={
'cf1': None,
'cf4': None,
'cf6': None,
'cf7': None,
})
for filter_name, value in (
('cf_cf1__n', 100),
('cf_cf4__n', 'foo'),
('cf_cf4__nic', 'foo'),
('cf_cf4__nisw', 'foo'),
('cf_cf4__niew', 'bar'),
('cf_cf4__nie', 'FOO'),
('cf_cf6__n', '2016-06-26'),
('cf_cf7__n', 'http://a.example.com'),
('cf_cf7__nic', 'a'),
('cf_cf7__nisw', 'http://'),
('cf_cf7__niew', '.com'),
):
with self.subTest(filter_name):
pks = set(
self.filterset({filter_name: [value]}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertIn(no_key.pk, pks, "an object carrying no key must match")
self.assertIn(explicit_null.pk, pks, "an object holding a null must match")
def test_filter_null_sentinel_matches_unset_values(self):
"""
The null sentinel (FILTERS_NULL_CHOICE_VALUE) asks for the objects holding no value, which
must include those carrying no key as well as those holding an explicit null. Negating it
must therefore return exactly the objects which do hold a value -- and in particular must
not return the ones it is being asked to exclude.
Only string-backed field types are exercised: a numeric or date field rejects 'null' during
form validation ("Enter a whole number"), so the sentinel never reaches the filter at all.
That is a property of multivalue_field_factory() and is unaffected by this behavior.
"""
no_key = Site.objects.get(slug='site-4')
explicit_null = Site.objects.create(name='Site 5', slug='site-5', custom_field_data={
'cf4': None,
'cf7': None,
'cf9': None,
})
has_value = set(
Site.objects.filter(slug__in=('site-1', 'site-2', 'site-3')).values_list('pk', flat=True)
)
for filter_name in ('cf_cf4', 'cf_cf7', 'cf_cf9'):
with self.subTest(filter_name):
pks = set(
self.filterset({filter_name: ['null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertEqual(pks, {no_key.pk, explicit_null.pk})
pks = set(
self.filterset({f'{filter_name}__n': ['null']}, self.queryset)
.qs.values_list('pk', flat=True)
)
self.assertEqual(pks, has_value)
def test_filter_null_sentinel_combined_with_a_value(self):
"""
The sentinel may be passed alongside real values, in which case it widens the match rather
than replacing it. Under negation the valueless objects are then excluded, as they are among
the values being negated.
"""
no_key = Site.objects.get(slug='site-4')
site_1 = Site.objects.get(slug='site-1')
pks = set(
self.filterset({'cf_cf4': ['foo', 'null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertIn(site_1.pk, pks, "an object holding the value must match")
self.assertIn(no_key.pk, pks, "an object holding no value must match")
pks = set(
self.filterset({'cf_cf4__n': ['foo', 'null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertNotIn(site_1.pk, pks, "an object holding the value must be excluded")
self.assertNotIn(no_key.pk, pks, "an object holding no value must be excluded")
def test_filter_select(self):
self.assertEqual(self.filterset({'cf_cf9': ['A', 'B']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf9__empty': True}, self.queryset).qs.count(), 1)
@ -2290,7 +2688,8 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_multiselect(self):
self.assertEqual(self.filterset({'cf_cf10': ['A']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf10': ['A', 'C']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 1) # Contains a literal null
# Matches both the object holding a literal null and the one carrying no key, as `empty` does
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf10__empty': True}, self.queryset).qs.count(), 2)
def test_filter_object(self):

View File

@ -16,14 +16,14 @@ from PIL import Image
from requests import Session
from rest_framework import status
from core.choices import ManagedFileRootPathChoices
from core.choices import JobNotificationChoices, ManagedFileRootPathChoices
from core.events import *
from core.models import Job, ObjectType
from dcim.choices import SiteStatusChoices
from dcim.models import DeviceType, Interface, Manufacturer, Site
from extras.choices import EventRuleActionChoices
from extras.events import enqueue_event, flush_events, process_event_rules, serialize_for_event
from extras.models import EventRule, Script, ScriptModule, Tag, Webhook
from extras.models import EventRule, Notification, Script, ScriptModule, Tag, Webhook
from extras.scripts import Script as ScriptBase
from extras.signals import process_job_end_event_rules
from extras.webhooks import generate_signature, send_webhook
@ -904,6 +904,79 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
self.assertEqual(script_job.status, "completed")
self.assertEqual(script_job.data.get('output', ''), "finished successfully")
@tag('regression') # Issue #22852
def test_eventrule_script_action_honors_script_defaults(self):
"""A script run from an event rule uses the notification policy and job timeout from its Meta class."""
class DummyScript(ScriptBase):
class Meta:
name = 'Dummy Defaults Script'
notifications_default = JobNotificationChoices.NOTIFICATION_ON_FAILURE
job_timeout = 600
def run(self, data, commit=True):
return 'finished successfully'
dummy_script = DummyScript()
with patch.object(ScriptModule, 'sync_classes'):
module = ScriptModule.objects.create(
file_root=ManagedFileRootPathChoices.SCRIPTS,
file_path='dummy_defaults_script.py',
)
script = Script.objects.create(
module=module,
name=dummy_script.name,
is_executable=True,
)
event_rule = EventRule.objects.create(
name='Test Script Defaults Event Rule',
event_types=[OBJECT_CREATED],
action_type=EventRuleActionChoices.SCRIPT,
action_object_type=ObjectType.objects.get_for_model(Script),
action_object_id=script.pk,
)
event_rule.object_types.set([ObjectType.objects.get_for_model(DeviceType)])
manufacturer = Manufacturer.objects.create(name='Test Manufacturer', slug='test-manufacturer')
self.add_permissions('dcim.add_devicetype')
with patch.object(Script, 'python_class') as mock:
mock.return_value = dummy_script
with self.captureOnCommitCallbacks(execute=True):
response = self.client.post(
reverse('dcim-api:devicetype-list'),
{
'manufacturer': manufacturer.pk,
'model': 'Test DeviceType',
'slug': 'test-devicetype',
},
format='json',
**self.header,
)
self.assertHttpStatus(response, status.HTTP_201_CREATED)
self.assertEqual(self.queue.count, 1)
self.assertEqual(self.queue.jobs[0].timeout, 600)
script_job = Job.objects.get(name=dummy_script.name)
self.assertEqual(script_job.notifications, JobNotificationChoices.NOTIFICATION_ON_FAILURE)
# silence rqworker (cleaner output) and trigger job execution
rq_logger = logging.getLogger('rq.worker')
self.addCleanup(rq_logger.setLevel, rq_logger.level)
rq_logger.setLevel(logging.ERROR)
self.run_rq_jobs('default')
script_job.refresh_from_db()
self.assertEqual(script_job.status, "completed")
self.assertFalse(
Notification.objects.filter(
user=self.user,
object_type=ObjectType.objects.get_for_model(Job),
object_id=script_job.pk,
).exists()
)
@tag('regression')
def test_eventrule_webhook_action_with_object_image_files(self):
"""

View File

@ -8,11 +8,13 @@ from django.core.management.base import CommandError
from django.db.models import F
from django.test import TestCase
from core.choices import JobNotificationChoices
from dcim.choices import InterfaceTypeChoices
from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site
from extras.management.commands import renaturalize, webhook_receiver
from extras.management.commands.webhook_receiver import WebhookHandler
from extras.models import ConfigContext, ImageAttachment
from extras.scripts import Script, StringVar
from extras.tests.test_models import OverwriteStyleMemoryStorage, UnreadableSizeMemoryStorage
from users.models import User
from utilities.fields import NaturalOrderingField
@ -257,20 +259,14 @@ class RunScriptTestCase(TestCase):
)
def test_enqueues_script_job(self):
class TestScript:
full_name = 'test.Script'
class TestScript(Script):
value = StringVar()
def as_form(self, data, files):
form = MagicMock()
form.is_valid.return_value = True
form.cleaned_data = {
'_schedule_at': None,
'_interval': None,
'_commit': None,
'name': data['name'],
}
form.errors.get_json_data.return_value = {}
return form
class Meta:
notifications_default = JobNotificationChoices.NOTIFICATION_ON_FAILURE
def run(self, data, commit):
return None
script_obj = SimpleNamespace(python_class=TestScript)
job = SimpleNamespace(duration='0 seconds')
@ -290,7 +286,7 @@ class RunScriptTestCase(TestCase):
'runscript',
'test.Script',
user='admin',
data='{"name": "test"}',
data='{"value": "test"}',
stdout=StringIO(),
)
@ -300,8 +296,9 @@ class RunScriptTestCase(TestCase):
self.assertEqual(kwargs['instance'], script_obj)
self.assertEqual(kwargs['user'], self.user)
self.assertTrue(kwargs['immediate'])
self.assertEqual(kwargs['data'], {'name': 'test'})
self.assertEqual(kwargs['data'], {'value': 'test'})
self.assertFalse(kwargs['commit'])
self.assertEqual(kwargs['notifications'], JobNotificationChoices.NOTIFICATION_ON_FAILURE)
def test_invalid_script_data_raises_error_without_enqueueing_job(self):
class TestScript:
@ -353,6 +350,7 @@ class RunScriptTestCase(TestCase):
'_schedule_at': None,
'_interval': None,
'_commit': None,
'_notifications': JobNotificationChoices.NOTIFICATION_ALWAYS,
}
form.errors.get_json_data.return_value = {}
return form
@ -393,6 +391,7 @@ class RunScriptTestCase(TestCase):
'_schedule_at': None,
'_interval': None,
'_commit': None,
'_notifications': JobNotificationChoices.NOTIFICATION_ALWAYS,
}
form.errors.get_json_data.return_value = {}
return form

View File

@ -1582,6 +1582,95 @@ class ExportTemplateRenderTestCase(TestCase):
self.assertEqual(response.content.decode(), 'Site A\nSite B\nSite C\n')
class WebhookPayloadUrlValidationTestCase(TestCase):
"""Tests for Webhook.clean()'s validation of payload_url (#22828)."""
def test_payload_url_accepts_literal_url(self):
webhook = Webhook(name='Webhook 1', payload_url='http://example.com/hook')
webhook.clean()
def test_payload_url_rejects_non_url(self):
webhook = Webhook(name='Webhook 1', payload_url='not-a-url-at-all')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_payload_url_rejects_disallowed_scheme(self):
webhook = Webhook(name='Webhook 1', payload_url='file:///etc/passwd')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_payload_url_accepts_jinja2_template(self):
"""A templated payload_url must not be rejected merely for not being a literal URL."""
webhook = Webhook(name='Webhook 1', payload_url='http://{{ data.name }}.example.com/hook')
webhook.clean()
def test_payload_url_accepts_template_using_a_registered_filter(self):
webhook = Webhook(name='Webhook 1', payload_url="http://example.com/{{ 'HOME' | env }}")
webhook.clean()
def test_payload_url_rejects_malformed_template_syntax(self):
webhook = Webhook(name='Webhook 1', payload_url='http://{{ data.name }.example.com/hook')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_payload_url_rejects_template_with_unregistered_filter(self):
webhook = Webhook(
name='Webhook 1', payload_url='http://example.com/{{ data.name | totally_unregistered_filter }}'
)
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_payload_url_accepts_single_label_host(self):
"""A Docker/Kubernetes-style internal service name is a legitimate webhook target (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='http://webhook-receiver:8080/hook')
webhook.clean()
def test_payload_url_accepts_underscore_in_hostname(self):
"""requests accepts an underscore in a hostname even though Django's URLValidator does not (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='http://my_host.example.com/hook')
webhook.clean()
def test_payload_url_rejects_missing_host(self):
webhook = Webhook(name='Webhook 1', payload_url='http:///hook')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_payload_url_rejects_templated_disallowed_scheme(self):
"""A literal, disallowed scheme must be rejected even when the rest of the URL is templated (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='file:///{{ data.name }}')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
def test_blank_payload_url_produces_a_single_error(self):
"""clean() must not add its own error on top of clean_fields()'s for a blank value (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='')
with self.assertRaises(ValidationError) as cm:
webhook.full_clean()
self.assertEqual(cm.exception.message_dict['payload_url'], ['This field cannot be blank.'])
def test_none_payload_url_does_not_raise_typeerror(self):
webhook = Webhook(name='Webhook 1', payload_url=None)
webhook.clean()
def test_payload_url_accepts_fully_templated_value(self):
"""A value with no literal scheme at all (the scheme itself is templated) must still be usable (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='{{ data.custom_fields.callback_url }}')
webhook.clean()
def test_payload_url_rejects_malformed_bracketed_host_gracefully(self):
"""A malformed netloc must raise ValidationError, not an uncaught ValueError from urlsplit() (#22832)."""
webhook = Webhook(name='Webhook 1', payload_url='http://[2001:db8::1/hook')
with self.assertRaises(ValidationError) as cm:
webhook.clean()
self.assertIn('payload_url', cm.exception.message_dict)
class EventRuleTestCase(TestCase):
def test_action_data_clean_accepts_dict(self):

View File

@ -81,8 +81,8 @@ class CustomFieldDeletedSignalTestCase(TestCase):
class CustomFieldObjectTypeSignalTestCase(TestCase):
"""
Verify extras.signals.handle_cf_added_obj_types and handle_cf_removed_obj_types
populate or strip default values when a CustomField's object_types m2m changes.
Verify extras.signals.handle_cf_object_types_changed populates or strips default values when a
CustomField's object_types m2m changes.
"""
def test_adding_object_type_populates_default_value(self):

View File

@ -1,6 +1,7 @@
from django.contrib.auth import get_user_model
from django.contrib.auth.context_processors import PermWrapper
from django.test import RequestFactory, TestCase
from django.utils.html import escape
from core.models import ObjectType
from dcim.models import Site
@ -139,6 +140,12 @@ class CustomLinkRenderErrorEscapingTest(TestCase):
XSS_NAME = '<img src=x onerror=alert(1)>'
ESCAPED_NAME = '&lt;img src=x onerror=alert(1)&gt;'
# Subscripting a string with a nonexistent attribute yields an Undefined, and operating on it raises
# UndefinedError. These tests depend on Jinja2 quoting the subscript verbatim in that message (currently
# "'str object' has no attribute '<payload>'"); a change to Jinja2's message format would break them.
XSS_PAYLOAD = '" ></span><script>alert(1)</script>'
FAILING_TEMPLATE = f"{{{{ ''['{XSS_PAYLOAD}'] + 1 }}}}"
@classmethod
def setUpTestData(cls):
cls.site = Site.objects.create(name='Site 1', slug='site-1')
@ -189,3 +196,33 @@ class CustomLinkRenderErrorEscapingTest(TestCase):
rendered = self.render(self.make_user_with_view_permission('user2'))
self.assertNotIn(self.XSS_NAME, rendered)
self.assertIn(self.ESCAPED_NAME, rendered)
def test_render_error_escapes_exception_message(self):
# The exception message reproduces the (attacker-controlled) template code, so it must be escaped
# in the error fallback as well (NB-3311).
custom_link = CustomLink.objects.create(
name='Custom Link 1',
enabled=True,
link_text=self.FAILING_TEMPLATE,
link_url='http://example.com/',
)
custom_link.object_types.set([ObjectType.objects.get_for_model(Site)])
rendered = self.render(self.make_user_with_view_permission('user3'))
self.assertNotIn(self.XSS_PAYLOAD, rendered)
self.assertIn(escape(self.XSS_PAYLOAD), rendered)
def test_render_error_escapes_grouped_exception_message(self):
# The grouped-link error fallback must likewise escape the exception message (NB-3311).
custom_link = CustomLink.objects.create(
name='Custom Link 1',
enabled=True,
group_name='Group 1',
link_text=self.FAILING_TEMPLATE,
link_url='http://example.com/',
)
custom_link.object_types.set([ObjectType.objects.get_for_model(Site)])
rendered = self.render(self.make_user_with_view_permission('user4'))
self.assertNotIn(self.XSS_PAYLOAD, rendered)
self.assertIn(escape(self.XSS_PAYLOAD), rendered)

View File

@ -6,6 +6,7 @@ from django.contrib.contenttypes.models import ContentType
from django.contrib.messages import get_messages
from django.test import tag
from django.urls import reverse
from django.utils.html import escape
from core.choices import JobStatusChoices, ManagedFileRootPathChoices
from core.events import *
@ -245,6 +246,30 @@ class CustomLinkRenderingTestCase(TestCase):
self.assertEqual(response.status_code, 200)
self.assertNotIn(f'FOO {site.name} BAR', str(response.content))
def test_list_view_custom_link_column_escapes_render_error(self):
# Jinja2 includes the invalid key verbatim in UndefinedError; this test intentionally depends on that format.
payload = '" ></span><script>alert(1)</script>'
customlink = CustomLink(
name='Test',
link_text=f"{{{{ ''['{payload}'] + 1 }}}}",
link_url='http://example.com/',
new_window=False
)
customlink.save()
customlink.object_types.set([ObjectType.objects.get_for_model(Site)])
site = Site(name='Test Site', slug='test-site')
site.save()
response = self.client.get(f"{reverse('dcim:site_list')}?include_columns=cl_Test")
self.assertEqual(response.status_code, 200)
content = response.content.decode()
# The error element must be present, but the payload must appear only in escaped form
self.assertIn('<span class="text-danger" title="', content)
self.assertNotIn(payload, content)
self.assertIn(escape(payload), content)
class SavedFilterTestCase(ViewTestCases.PrimaryObjectViewTestCase):
model = SavedFilter

View File

@ -5,8 +5,10 @@ import strawberry_django
from circuits.graphql.types import ProviderType
from dcim.graphql.types import SiteType
from dcim.models import Device, Interface, Location, Rack, RackGroup, Region, Site, SiteGroup
from extras.graphql.mixins import ContactsMixin
from ipam import models
from netbox.graphql.optimization import build_gfk_prefetch
from netbox.graphql.scalars import BigInt
from netbox.graphql.types import (
BaseObjectType,
@ -15,6 +17,7 @@ from netbox.graphql.types import (
PrimaryObjectType,
register_type,
)
from virtualization.models import Cluster, ClusterGroup, VirtualMachine, VMInterface
from .filters import *
from .mixins import IPAddressesMixin
@ -24,6 +27,7 @@ if TYPE_CHECKING:
DeviceType,
InterfaceType,
LocationType,
RackGroupType,
RackType,
RegionType,
SiteGroupType,
@ -57,23 +61,16 @@ __all__ = (
@strawberry.type
class IPAddressFamilyType:
"""
The address family (4 or 6) of a model's IP address or prefix column. Each type exposing this
declares its own `family` resolver, hinted with the column backing it so that the query optimizer
does not defer that column. `value` is non-null because those columns are: the models' `family`
properties return None only for an unsaved instance with no address assigned.
"""
value: int
label: str
@strawberry.type
class BaseIPAddressFamilyType:
"""
Base type for models that need to expose their IPAddress family type.
"""
@strawberry.field
def family(self) -> IPAddressFamilyType:
# Note that self, is an instance of models.IPAddress
# thus resolves to the address family value.
return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
@register_type(
models.ASN,
fields='__all__',
@ -109,11 +106,16 @@ class ASNRangeType(OrganizationalObjectType):
filters=AggregateFilter,
pagination=True
)
class AggregateType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
class AggregateType(ContactsMixin, PrimaryObjectType):
prefix: str
rir: Annotated["RIRType", strawberry.lazy('ipam.graphql.types')] | None
tenant: Annotated["TenantType", strawberry.lazy('tenancy.graphql.types')] | None
# Note that self is the Django model instance, so self.family resolves to the model's family property
@strawberry_django.field(only=['prefix'])
def family(self) -> IPAddressFamilyType:
return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
@register_type(
models.FHRPGroup,
@ -134,7 +136,16 @@ class FHRPGroupType(IPAddressesMixin, PrimaryObjectType):
class FHRPGroupAssignmentType(BaseObjectType):
group: Annotated['FHRPGroupType', strawberry.lazy('ipam.graphql.types')]
@strawberry_django.field(prefetch_related='interface')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'interface',
[
Interface,
VMInterface,
],
),
only=['interface_type', 'interface_id'],
)
def interface(self) -> Annotated[
Annotated['InterfaceType', strawberry.lazy('dcim.graphql.types')]
| Annotated['VMInterfaceType', strawberry.lazy('virtualization.graphql.types')],
@ -149,17 +160,31 @@ class FHRPGroupAssignmentType(BaseObjectType):
filters=IPAddressFilter,
pagination=True
)
class IPAddressType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
class IPAddressType(ContactsMixin, PrimaryObjectType):
address: str
vrf: Annotated['VRFType', strawberry.lazy('ipam.graphql.types')] | None
tenant: Annotated['TenantType', strawberry.lazy('tenancy.graphql.types')] | None
nat_inside: Annotated['IPAddressType', strawberry.lazy('ipam.graphql.types')] | None
nat_outside: list[Annotated['IPAddressType', strawberry.lazy('ipam.graphql.types')]]
tunnel_terminations: list[Annotated['TunnelTerminationType', strawberry.lazy('vpn.graphql.types')]]
services: list[Annotated['ServiceType', strawberry.lazy('ipam.graphql.types')]]
@strawberry_django.field(prefetch_related='assigned_object')
# Note that self is the Django model instance, so self.family resolves to the model's family property
@strawberry_django.field(only=['address'])
def family(self) -> IPAddressFamilyType:
return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'assigned_object',
[
models.FHRPGroup,
Interface,
VMInterface,
],
),
only=['assigned_object_type', 'assigned_object_id'],
)
def assigned_object(self) -> Annotated[
Annotated['InterfaceType', strawberry.lazy('dcim.graphql.types')]
| Annotated['FHRPGroupType', strawberry.lazy('ipam.graphql.types')]
@ -189,14 +214,30 @@ class IPRangeType(ContactsMixin, PrimaryObjectType):
filters=PrefixFilter,
pagination=True
)
class PrefixType(ContactsMixin, BaseIPAddressFamilyType, PrimaryObjectType):
class PrefixType(ContactsMixin, PrimaryObjectType):
prefix: str
vrf: Annotated['VRFType', strawberry.lazy('ipam.graphql.types')] | None
tenant: Annotated['TenantType', strawberry.lazy('tenancy.graphql.types')] | None
vlan: Annotated['VLANType', strawberry.lazy('ipam.graphql.types')] | None
role: Annotated['RoleType', strawberry.lazy('ipam.graphql.types')] | None
@strawberry_django.field(prefetch_related='scope')
# Note that self is the Django model instance, so self.family resolves to the model's family property
@strawberry_django.field(only=['prefix'])
def family(self) -> IPAddressFamilyType:
return IPAddressFamilyType(value=self.family, label=f'IPv{self.family}')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'scope',
[
Region,
SiteGroup,
Site,
Location,
],
),
only=['scope_type', 'scope_id'],
)
def scope(self) -> Annotated[
Annotated['LocationType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RegionType', strawberry.lazy('dcim.graphql.types')]
@ -273,7 +314,17 @@ class ServiceType(ContactsMixin, PrimaryObjectType):
def ports(self) -> list[int] | None:
return self.ports
@strawberry_django.field(prefetch_related='parent')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'parent',
[
Device,
VirtualMachine,
models.FHRPGroup,
],
),
only=['parent_object_type', 'parent_object_id'],
)
def parent(self) -> Annotated[
Annotated['DeviceType', strawberry.lazy('dcim.graphql.types')]
| Annotated['VirtualMachineType', strawberry.lazy('virtualization.graphql.types')]
@ -320,7 +371,7 @@ class VLANType(PrimaryObjectType):
interfaces_as_tagged: list[Annotated["InterfaceType", strawberry.lazy('dcim.graphql.types')]]
vminterfaces_as_tagged: list[Annotated["VMInterfaceType", strawberry.lazy('virtualization.graphql.types')]]
@strawberry_django.field(prefetch_related='qinq_svlan')
@strawberry_django.field(prefetch_related='qinq_svlan', only=['qinq_svlan_id'])
def qinq_svlan(self) -> Annotated["VLANType", strawberry.lazy('ipam.graphql.types')] | None:
return self.qinq_svlan
@ -338,11 +389,27 @@ class VLANGroupType(OrganizationalObjectType):
total_vlan_ids: BigInt
tenant: Annotated['TenantType', strawberry.lazy('tenancy.graphql.types')] | None
@strawberry_django.field(prefetch_related='scope')
@strawberry_django.field(
prefetch_related=build_gfk_prefetch(
'scope',
[
Cluster,
ClusterGroup,
Location,
Rack,
RackGroup,
Region,
Site,
SiteGroup,
],
),
only=['scope_type', 'scope_id'],
)
def scope(self) -> Annotated[
Annotated['ClusterType', strawberry.lazy('virtualization.graphql.types')]
| Annotated['ClusterGroupType', strawberry.lazy('virtualization.graphql.types')]
| Annotated['LocationType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RackGroupType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RackType', strawberry.lazy('dcim.graphql.types')]
| Annotated['RegionType', strawberry.lazy('dcim.graphql.types')]
| Annotated['SiteType', strawberry.lazy('dcim.graphql.types')]

View File

@ -13,5 +13,9 @@ class IPAddressManager(Manager.from_queryset(IPAddressQuerySet)):
address. We can use HOST() to extract just the host portion of the address (ignoring its mask), but we must
then re-cast this value to INET() so that records will be ordered properly. We are essentially re-casting each
IP address as a /32 or /128.
Host addresses are not unique, so we must also order by primary key to guarantee a stable, total ordering.
Without this tiebreaker, PostgreSQL is free to return tied rows in a different order from one query to the
next, which causes objects to be duplicated or omitted across paginated requests.
"""
return super().get_queryset().order_by(Inet(Host('address')))
return super().get_queryset().order_by(Inet(Host('address')), 'pk')

View File

@ -0,0 +1,33 @@
import django.db.models.functions.comparison
from django.db import migrations, models
import ipam.fields
import ipam.lookups
class Migration(migrations.Migration):
dependencies = [
('ipam', '0093_alter_prefix__region_alter_prefix__site_group'),
]
operations = [
# Replace the host address index with a composite index which also covers the primary key, so that it can
# satisfy the default ordering of IPAddress outright. Note that the existing index must be dropped rather
# than retained alongside the new one: with both present, PostgreSQL continues to select the narrower index
# and applies an incremental sort atop it.
migrations.RemoveIndex(
model_name='ipaddress',
name='ipam_ipaddress_host',
),
migrations.AddIndex(
model_name='ipaddress',
index=models.Index(
django.db.models.functions.comparison.Cast(
ipam.lookups.Host('address'),
output_field=ipam.fields.IPAddressField(),
),
models.F('id'),
name='ipam_ipaddress_host',
),
),
]

View File

@ -0,0 +1,14 @@
# Generated by Django 6.0.7
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('ipam', '0096_ipaddress_host_index'),
('ipam', '0095_multi_protocol_services'),
]
operations = [
]

View File

@ -1023,8 +1023,13 @@ class IPAddress(ContactsMixin, PrimaryModel):
class Meta:
ordering = ('address', 'pk') # address may be non-unique
indexes = (
models.Index(fields=('address', 'id')), # Default ordering
models.Index(Cast(Host('address'), output_field=IPAddressField()), name='ipam_ipaddress_host'),
models.Index(fields=('address', 'id')),
# Default ordering (see IPAddressManager). The primary key must be included so that the index can
# satisfy the ordering outright; without it PostgreSQL falls back to an incremental sort, which
# measurably slows deep pagination.
models.Index(
Cast(Host('address'), output_field=IPAddressField()), F('id'), name='ipam_ipaddress_host'
),
models.Index(fields=('assigned_object_type', 'assigned_object_id')),
)
verbose_name = _('IP address')

View File

@ -1,37 +1,37 @@
{
"aggregate:api_list_objects": 14,
"aggregate:list_objects_with_permission": 24,
"asn:api_list_objects": 18,
"asn:list_objects_with_permission": 31,
"asnrange:api_list_objects": 15,
"asnrange:list_objects_with_permission": 22,
"fhrpgroup:api_list_objects": 14,
"fhrpgroup:list_objects_with_permission": 21,
"aggregate:api_list_objects": 13,
"aggregate:list_objects_with_permission": 21,
"asn:api_list_objects": 17,
"asn:list_objects_with_permission": 28,
"asnrange:api_list_objects": 14,
"asnrange:list_objects_with_permission": 19,
"fhrpgroup:api_list_objects": 13,
"fhrpgroup:list_objects_with_permission": 18,
"fhrpgroupassignment:api_list_objects": 18,
"ipaddress:api_list_objects": 14,
"ipaddress:list_objects_with_permission": 21,
"iprange:api_list_objects": 13,
"iprange:list_objects_with_permission": 20,
"prefix:api_list_objects": 13,
"prefix:list_objects_with_permission": 29,
"rir:api_list_objects": 13,
"rir:list_objects_with_permission": 20,
"role:api_list_objects": 13,
"role:list_objects_with_permission": 20,
"routetarget:api_list_objects": 13,
"routetarget:list_objects_with_permission": 21,
"service:api_list_objects": 16,
"service:list_objects_with_permission": 21,
"servicetemplate:api_list_objects": 13,
"servicetemplate:list_objects_with_permission": 20,
"vlan:api_list_objects": 15,
"vlan:list_objects_with_permission": 24,
"vlangroup:api_list_objects": 13,
"vlangroup:list_objects_with_permission": 25,
"ipaddress:api_list_objects": 13,
"ipaddress:list_objects_with_permission": 18,
"iprange:api_list_objects": 12,
"iprange:list_objects_with_permission": 17,
"prefix:api_list_objects": 12,
"prefix:list_objects_with_permission": 26,
"rir:api_list_objects": 12,
"rir:list_objects_with_permission": 17,
"role:api_list_objects": 12,
"role:list_objects_with_permission": 17,
"routetarget:api_list_objects": 12,
"routetarget:list_objects_with_permission": 18,
"service:api_list_objects": 15,
"service:list_objects_with_permission": 18,
"servicetemplate:api_list_objects": 12,
"servicetemplate:list_objects_with_permission": 17,
"vlan:api_list_objects": 14,
"vlan:list_objects_with_permission": 21,
"vlangroup:api_list_objects": 12,
"vlangroup:list_objects_with_permission": 22,
"vlantranslationpolicy:api_list_objects": 12,
"vlantranslationpolicy:list_objects_with_permission": 20,
"vlantranslationpolicy:list_objects_with_permission": 17,
"vlantranslationrule:api_list_objects": 12,
"vlantranslationrule:list_objects_with_permission": 21,
"vrf:api_list_objects": 15,
"vrf:list_objects_with_permission": 20
"vlantranslationrule:list_objects_with_permission": 18,
"vrf:api_list_objects": 14,
"vrf:list_objects_with_permission": 17
}

View File

@ -198,3 +198,46 @@ class IPAddressOrderingTestCase(OrderingTestBase):
# Test
self._compare(IPAddress.objects.all(), addresses)
def test_duplicate_address_ordering(self):
"""
Host addresses are not unique, so tied addresses must be ordered by primary key to yield a stable, total
ordering. Without a tiebreaker the database may return tied rows in a different order from one query to the
next, duplicating or omitting objects across paginated requests.
"""
# Create several duplicates of each address, interleaved so that primary key order does not follow
# address order.
addresses = [
IPAddress(status=IPAddressStatusChoices.STATUS_ACTIVE, address=netaddr.IPNetwork(f'10.0.{i}.1/24'))
for _ in range(4)
for i in range(100)
]
IPAddress.objects.bulk_create(addresses)
pks = list(IPAddress.objects.values_list('pk', flat=True))
expected = [
ip.pk for ip in sorted(IPAddress.objects.all(), key=lambda ip: (ip.address.ip, ip.pk))
]
self.assertEqual(pks, expected)
def test_duplicate_address_pagination(self):
"""
Paginating over duplicate addresses must not return the same object on two pages, nor omit any object.
"""
addresses = [
IPAddress(status=IPAddressStatusChoices.STATUS_ACTIVE, address=netaddr.IPNetwork(f'10.0.{i}.1/24'))
for _ in range(4)
for i in range(100)
]
IPAddress.objects.bulk_create(addresses)
queryset = IPAddress.objects.values_list('pk', flat=True)
page_size = 37
paginated = []
for offset in range(0, len(addresses), page_size):
paginated.extend(queryset[offset:offset + page_size])
self.assertEqual(len(paginated), len(addresses))
self.assertEqual(len(set(paginated)), len(addresses))
self.assertEqual(paginated, list(queryset))

View File

@ -1,4 +1,5 @@
import datetime
from unittest.mock import patch
from django.contrib.contenttypes.models import ContentType
from django.db.backends.postgresql.psycopg_any import NumericRange
@ -10,13 +11,16 @@ from core.choices import ObjectChangeActionChoices
from core.models import ObjectChange, ObjectType
from dcim.constants import InterfaceTypeChoices
from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site
from extras.models import SavedFilter
from extras.choices import CustomFieldTypeChoices
from extras.models import CustomField, SavedFilter
from ipam import filtersets
from ipam.choices import *
from ipam.models import *
from ipam.views import AggregatePrefixesView
from ipam.utils import AvailableIPSpace
from ipam.views import AggregatePrefixesView, PrefixPrefixesView
from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
from tenancy.models import Tenant
from users.models import ObjectPermission
from users.models import Group, ObjectPermission
from utilities.testing import ViewTestCases, create_tags, post_data
@ -420,37 +424,175 @@ class AggregateTestCase(ViewTestCases.PrimaryObjectViewTestCase):
self.assertContains(response, '203.0.114.0/26')
self.assertNotContains(response, '203.0.114.64/26')
def test_children_are_filtered_fallback(self):
"""_children_are_filtered() rebuilds the queryset when prep_table_data() has not cached a result."""
def test_aggregate_prefixes_custom_field_constraint_shows_available(self):
"""A tenant custom-field permission constraint does not suppress available-prefix rows."""
cf = CustomField.objects.create(name='integerCustomField', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(Tenant)])
tenant = Tenant.objects.create(
name='Agg CF Tenant', slug='agg-cf-tenant', custom_field_data={'integerCustomField': 1}
)
aggregate = Aggregate.objects.create(prefix=IPNetwork('198.51.100.0/24'), rir=RIR.objects.first())
child = Prefix.objects.create(prefix=IPNetwork('198.51.100.0/26'), tenant=tenant)
self.add_permissions('ipam.view_aggregate')
obj_perm = ObjectPermission(
name='View prefixes', actions=['view'], constraints={'tenant__custom_field_data__integerCustomField': 1}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(Prefix))
self.assertIn(child, Prefix.objects.restrict(self.user, 'view'))
url = reverse('ipam:aggregate_prefixes', kwargs={'pk': aggregate.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertTrue(response.context['show_available'])
self.assertGreater(len(response.context['table'].data), 1)
def has_active_child_filters(self, **params):
"""Run the child filter detector on a fresh view for the given query parameters."""
view = AggregatePrefixesView()
request = RequestFactory().get('/', params)
request.user = self.user
return view._has_active_child_filters(request)
def test_has_active_child_filters_declared_filters(self):
"""A declared filter with a real value counts as active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
tenant = Tenant.objects.create(name='Declared Tenant', slug='declared-tenant')
self.assertTrue(self.has_active_child_filters(tenant_id=tenant.pk))
self.assertTrue(self.has_active_child_filters(q='test'))
# A boolean false is a value, not an absent filter.
self.assertTrue(self.has_active_child_filters(is_pool='false'))
def test_has_active_child_filters_lookup_variants(self):
"""Lookup variants generated by get_filters() count as active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
aggregate = Aggregate.objects.create(
prefix=IPNetwork('203.0.115.0/24'),
rir=RIR.objects.first()
self.assertTrue(self.has_active_child_filters(status__n=PrefixStatusChoices.STATUS_ACTIVE))
self.assertTrue(self.has_active_child_filters(description__empty='true'))
def test_has_active_child_filters_custom_field_filters(self):
"""Custom field filters registered on the filterset instance count as active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
cf = CustomField.objects.create(name='edge_cf', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(Prefix)])
self.assertTrue(self.has_active_child_filters(cf_edge_cf='1'))
self.assertTrue(self.has_active_child_filters(cf_edge_cf__gte='1'))
def test_has_active_child_filters_saved_filter(self):
"""A populated saved filter counts as active filtering by slug or by id."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
tenant = Tenant.objects.create(name='Saved Ref Tenant', slug='saved-ref-tenant')
saved_filter = SavedFilter.objects.create(
name='Saved ref', slug='saved-ref', parameters={'tenant_id': [str(tenant.pk)]}
)
tenant = Tenant.objects.create(name='Aggregate Fallback Tenant', slug='aggregate-fallback-tenant')
Prefix.objects.create(prefix=IPNetwork('203.0.115.0/26'), tenant=tenant)
Prefix.objects.create(prefix=IPNetwork('203.0.115.64/26'))
saved_filter.object_types.add(ObjectType.objects.get_for_model(Prefix))
# No cached value: the fallback path rebuilds the filtered queryset and detects the filter.
self.assertTrue(self.has_active_child_filters(filter=saved_filter.slug))
self.assertTrue(self.has_active_child_filters(filter_id=saved_filter.pk))
def test_has_active_child_filters_unresolved_saved_filter(self):
"""A saved filter reference that expands to nothing is not active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
saved_filter = SavedFilter.objects.create(name='Empty', slug='empty-saved-filter', parameters={})
saved_filter.object_types.add(ObjectType.objects.get_for_model(Prefix))
self.assertFalse(self.has_active_child_filters(filter_id=saved_filter.pk))
self.assertFalse(self.has_active_child_filters(filter_id='99999999'))
def test_has_active_child_filters_without_values(self):
"""A request with no parameters or only empty ones is not active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
self.assertFalse(self.has_active_child_filters())
self.assertFalse(self.has_active_child_filters(tenant_id=''))
self.assertFalse(self.has_active_child_filters(q=''))
def test_has_active_child_filters_invalid_value(self):
"""A filter value that fails validation is not applied, so it is not active filtering."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
self.assertFalse(self.has_active_child_filters(tenant_id='99999999'))
def test_has_active_child_filters_valid_beside_invalid_value(self):
"""A valid filter is still active when another submitted value is invalid."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
tenant = Tenant.objects.create(name='Mixed Tenant', slug='mixed-tenant')
self.assertTrue(self.has_active_child_filters(tenant_id=tenant.pk, status='not-a-valid-status'))
def test_has_active_child_filters_non_filter_params(self):
"""Unknown parameters, display toggles, and table controls are not filters."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
self.assertFalse(self.has_active_child_filters(not_a_filter='x'))
self.assertFalse(self.has_active_child_filters(show_available='false'))
self.assertFalse(self.has_active_child_filters(show_assigned='true'))
self.assertFalse(self.has_active_child_filters(page='2', per_page='100', sort='prefix', tableconfig_id='1'))
def test_has_active_child_filters_control_beside_filter(self):
"""A table control submitted alongside a filter does not mask the filter."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
tenant = Tenant.objects.create(name='Control Tenant', slug='control-tenant')
self.assertTrue(self.has_active_child_filters(page='2', tenant_id=tenant.pk))
def test_has_active_child_filters_without_filterset(self):
"""A view without a filterset reports no active filters."""
view = AggregatePrefixesView()
request = RequestFactory().get('/', {'tenant_id': tenant.pk})
view.filterset = None
request = RequestFactory().get('/', {'tenant_id': '1'})
request.user = self.user
self.assertFalse(hasattr(view, '_child_queryset_is_filtered'))
self.assertTrue(view._children_are_filtered(request, aggregate))
self.assertFalse(view._has_active_child_filters(request))
def test_has_active_child_filters_reuses_view_filterset(self):
"""The detector evaluates the FilterSet already bound by the view, not a fresh one."""
aggregate = Aggregate.objects.create(prefix=IPNetwork('203.0.115.0/24'), rir=RIR.objects.first())
tenant = Tenant.objects.create(name='Reuse Tenant', slug='reuse-tenant')
# No cached value and no filter: the fallback path reports no filtering.
view = AggregatePrefixesView()
request = RequestFactory().get('/')
request.user = self.user
self.assertFalse(view._children_are_filtered(request, aggregate))
# A cached value takes precedence over the actual request state.
view = AggregatePrefixesView()
view._set_children_filtered(False)
request = RequestFactory().get('/', {'tenant_id': tenant.pk})
# The bound FilterSet is authoritative: it reports a filter the request itself does not carry.
view.filterset_instance = AggregatePrefixesView.filterset(
{'tenant_id': [str(tenant.pk)]}, view.get_children(request, aggregate), request=request
)
request = RequestFactory().get('/', {'page': '2'})
request.user = self.user
self.assertFalse(view._children_are_filtered(request, aggregate))
self.assertTrue(view._has_active_child_filters(request))
def test_child_tab_binds_filterset_once(self):
"""A filtered child tab binds the FilterSet once; the detector reuses it instead of rebuilding."""
self.add_permissions('ipam.view_aggregate', 'ipam.view_prefix')
aggregate = Aggregate.objects.create(prefix=IPNetwork('203.0.116.0/24'), rir=RIR.objects.first())
tenant = Tenant.objects.create(name='Bind Once Tenant', slug='bind-once-tenant')
Prefix.objects.create(prefix=IPNetwork('203.0.116.0/26'), tenant=tenant)
# Count only data-bound instantiations: the filter form separately builds an unbound
# FilterSet to resolve field modifiers, which is unrelated to filter detection.
bound = []
original_init = filtersets.PrefixFilterSet.__init__
def counting_init(fs, *args, **kwargs):
if args or 'data' in kwargs:
bound.append(fs)
original_init(fs, *args, **kwargs)
url = reverse('ipam:aggregate_prefixes', kwargs={'pk': aggregate.pk})
with patch.object(filtersets.PrefixFilterSet, '__init__', counting_init):
response = self.client.get(url, {'tenant_id': tenant.pk})
self.assertHttpStatus(response, 200)
self.assertEqual(len(bound), 1)
self.assertFalse(response.context['show_available'])
class RoleTestCase(ViewTestCases.OrganizationalObjectViewTestCase):
@ -878,18 +1020,437 @@ class PrefixTestCase(ViewTestCases.PrimaryObjectViewTestCase):
self.assertContains(response, '192.0.2.1/24')
self.assertNotContains(response, '192.0.2.2/24')
def assertIPAvailabilityShown(self, response, visible_ip):
"""The permitted IP renders as a real row and synthetic available-space rows are present."""
records = list(response.context['table'].data)
rendered_ip_pks = {r.pk for r in records if isinstance(r, IPAddress)}
self.assertIn(visible_ip.pk, rendered_ip_pks)
self.assertTrue(any(isinstance(r, AvailableIPSpace) for r in records))
def test_prefix_ipaddresses_unfiltered_shows_available_space(self):
"""An unfiltered IP Addresses tab injects synthetic available-space rows."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress', 'ipam.view_iprange')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertGreater(len(response.context['table'].data), 1)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_custom_field_constraint_shows_available(self):
"""A permission constraint on a related object's custom field data does not suppress the available-IP rows."""
cf = CustomField.objects.create(name='integerCustomField', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(Tenant)])
tenant = Tenant.objects.create(name='CF Tenant', slug='cf-tenant', custom_field_data={'integerCustomField': 1})
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'), tenant=tenant)
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), tenant=tenant)
# The issue reports the JSON string "1", but an integer custom field stores an int, so a
# string constraint would not match and would hide the parent. Use 1 and prove access below.
constraint = {'tenant__custom_field_data__integerCustomField': 1}
for model in (Prefix, IPAddress):
obj_perm = ObjectPermission(
name=f'View {model._meta.verbose_name}', actions=['view'], constraints=constraint
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(model))
# Self-verifying: the constraint must actually grant access to both objects, or the
# availability assertion could pass through an unrestricted re-query instead of the fix.
self.assertIn(prefix, Prefix.objects.restrict(self.user, 'view'))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_related_field_constraint_shows_available(self):
"""A permission constraint on a related object field does not suppress the available-IP rows."""
tenant = Tenant.objects.create(name='Plain Constraint Tenant', slug='plain-constraint-tenant')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'), tenant=tenant)
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), tenant=tenant)
constraint = {'tenant__slug': 'plain-constraint-tenant'}
for model in (Prefix, IPAddress):
obj_perm = ObjectPermission(
name=f'View {model._meta.verbose_name}', actions=['view'], constraints=constraint
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(model))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_constraint_with_direct_filter_suppresses(self):
"""A direct filter still suppresses available-IP rows for a constrained user."""
tenants = (
Tenant(name='Constraint Direct 1', slug='constraint-direct-1'),
Tenant(name='Constraint Direct 2', slug='constraint-direct-2'),
)
Tenant.objects.bulk_create(tenants)
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/24'))
ip1 = IPAddress.objects.create(address=IPNetwork('192.0.2.1/24'), tenant=tenants[0])
ip2 = IPAddress.objects.create(address=IPNetwork('192.0.2.2/24'), tenant=tenants[1])
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs', actions=['view'], constraints={'tenant__slug__startswith': 'constraint-direct'}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
# Both IPs are visible under the constraint, so suppression below is due to the filter, not permissions.
self.assertIn(ip1, IPAddress.objects.restrict(self.user, 'view'))
self.assertIn(ip2, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'tenant_id': tenants[0].pk})
self.assertHttpStatus(response, 200)
self.assertEqual(len(response.context['table'].data), 1)
self.assertContains(response, '192.0.2.1/24')
self.assertNotContains(response, '192.0.2.2/24')
def test_prefix_ipaddresses_constraint_with_saved_filter_suppresses(self):
"""A saved filter still suppresses available-IP rows for a constrained user."""
tenants = (
Tenant(name='Constraint Saved 1', slug='constraint-saved-1'),
Tenant(name='Constraint Saved 2', slug='constraint-saved-2'),
)
Tenant.objects.bulk_create(tenants)
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/24'))
ip1 = IPAddress.objects.create(address=IPNetwork('192.0.2.1/24'), tenant=tenants[0])
ip2 = IPAddress.objects.create(address=IPNetwork('192.0.2.2/24'), tenant=tenants[1])
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs', actions=['view'], constraints={'tenant__slug__startswith': 'constraint-saved'}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
# Both IPs are visible under the constraint, so suppression below is due to the filter, not permissions.
self.assertIn(ip1, IPAddress.objects.restrict(self.user, 'view'))
self.assertIn(ip2, IPAddress.objects.restrict(self.user, 'view'))
saved_filter = SavedFilter.objects.create(
name='Constraint saved tenant 1', slug='constraint-saved-tenant-1',
parameters={'tenant_id': [str(tenants[0].pk)]},
)
saved_filter.object_types.add(ObjectType.objects.get_for_model(IPAddress))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'filter_id': saved_filter.pk})
self.assertHttpStatus(response, 200)
self.assertEqual(len(response.context['table'].data), 1)
self.assertContains(response, '192.0.2.1/24')
self.assertNotContains(response, '192.0.2.2/24')
def test_prefix_ipaddresses_direct_field_constraint_shows_available(self):
"""A permission constraint on the IP Address status field does not suppress the available-IP rows."""
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), status=IPAddressStatusChoices.STATUS_ACTIVE)
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs', actions=['view'], constraints={'status': IPAddressStatusChoices.STATUS_ACTIVE}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_own_custom_field_constraint_shows_available(self):
"""A permission constraint on the IP Address custom field data does not suppress the available-IP rows."""
cf = CustomField.objects.create(name='ip_cf', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(IPAddress)])
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), custom_field_data={'ip_cf': 1})
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(name='View IPs', actions=['view'], constraints={'custom_field_data__ip_cf': 1})
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_multikey_constraint_shows_available(self):
"""A permission constraint combining a direct and a related field does not suppress the available-IP rows."""
tenant = Tenant.objects.create(name='Multi Key Tenant', slug='multi-key-tenant')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(
address=IPNetwork('192.0.2.1/29'), status=IPAddressStatusChoices.STATUS_ACTIVE, tenant=tenant
)
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs',
actions=['view'],
constraints={'status': IPAddressStatusChoices.STATUS_ACTIVE, 'tenant__slug': 'multi-key-tenant'},
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_or_constraint_shows_available(self):
"""A permission granting access through either of two constraints does not suppress the available-IP rows."""
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), status=IPAddressStatusChoices.STATUS_ACTIVE)
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs',
actions=['view'],
constraints=[
{'status': IPAddressStatusChoices.STATUS_ACTIVE},
{'status': IPAddressStatusChoices.STATUS_RESERVED},
],
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_address_startswith_constraint_shows_available(self):
"""The #22539 address__startswith constraint does not suppress the available-IP rows."""
prefix = Prefix.objects.create(prefix=IPNetwork('192.168.0.0/24'))
ip = IPAddress.objects.create(address=IPNetwork('192.168.0.1/24'))
self.add_permissions('ipam.view_prefix')
obj_perm = ObjectPermission(
name='View IPs', actions=['view'], constraints={'address__startswith': '192.168.0.'}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_prefixes_show_available_false_skips_filter_detection(self):
"""With availability already off, the filter detector never runs."""
self.add_permissions('ipam.view_prefix')
parent = Prefix.objects.create(prefix=IPNetwork('198.51.104.0/24'))
Prefix.objects.create(prefix=IPNetwork('198.51.104.0/26'))
view = PrefixPrefixesView()
request = RequestFactory().get('/', {'show_available': 'false'})
request.user = self.user
view.prep_table_data(request, view.get_children(request, parent), parent)
self.assertFalse(hasattr(view, '_active_child_filters'))
def test_prefix_ipaddresses_valid_plus_invalid_filter_suppresses(self):
"""A valid filter is applied and suppresses synthetic rows even when another filter value is invalid."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress')
tenant = Tenant.objects.create(name='VPI Tenant', slug='vpi-tenant')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/24'))
matching = IPAddress.objects.create(address=IPNetwork('192.0.2.1/24'), tenant=tenant)
IPAddress.objects.create(address=IPNetwork('192.0.2.2/24'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'tenant_id': tenant.pk, 'status': 'not-a-valid-status'})
self.assertHttpStatus(response, 200)
records = list(response.context['table'].data)
self.assertFalse(any(isinstance(r, AvailableIPSpace) for r in records))
self.assertEqual({r.pk for r in records if isinstance(r, IPAddress)}, {matching.pk})
def test_prefix_ipaddresses_htmx_direct_filter_suppresses(self):
"""An HTMX table refresh with a direct filter suppresses available-IP rows."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress')
tenants = (
Tenant(name='HTMX Tenant 1', slug='htmx-tenant-1'),
Tenant(name='HTMX Tenant 2', slug='htmx-tenant-2'),
)
Tenant.objects.bulk_create(tenants)
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/24'))
IPAddress.objects.create(address=IPNetwork('192.0.2.1/24'), tenant=tenants[0])
IPAddress.objects.create(address=IPNetwork('192.0.2.2/24'), tenant=tenants[1])
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'tenant_id': tenants[0].pk}, HTTP_HX_REQUEST='true')
self.assertHttpStatus(response, 200)
self.assertEqual(len(response.context['table'].data), 1)
def test_prefix_ipaddresses_htmx_unfiltered_shows_available(self):
"""An HTMX table refresh with no filter still injects available-IP rows."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, HTTP_HX_REQUEST='true')
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_group_constraint_shows_available(self):
"""A constraint granted through a group does not suppress the available-IP rows."""
self.add_permissions('ipam.view_prefix')
tenant = Tenant.objects.create(name='Group Grant', slug='group-grant')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), tenant=tenant)
group = Group.objects.create(name='IP Viewers')
self.user.groups.add(group)
obj_perm = ObjectPermission(name='View IPs', actions=['view'], constraints={'tenant__slug': 'group-grant'})
obj_perm.save()
obj_perm.groups.add(group)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
# The group grant must resolve, or the assertion below would be vacuous.
self.assertIn(ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_ipaddresses_partial_visibility_shows_available(self):
"""A constraint that hides one child IP keeps availability rows and omits the hidden IP."""
self.add_permissions('ipam.view_prefix')
visible_tenant = Tenant.objects.create(name='Partial Vis', slug='partial-vis')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
visible_ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'), tenant=visible_tenant)
hidden_ip = IPAddress.objects.create(address=IPNetwork('192.0.2.6/29'))
obj_perm = ObjectPermission(name='View IPs', actions=['view'], constraints={'tenant__slug': 'partial-vis'})
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPAddress))
# Exactly one child IP is visible, so the constraint is doing real work.
self.assertIn(visible_ip, IPAddress.objects.restrict(self.user, 'view'))
self.assertNotIn(hidden_ip, IPAddress.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
# The hidden IP is omitted and its slot is counted as available space.
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, visible_ip)
records = list(response.context['table'].data)
self.assertNotIn(hidden_ip.pk, {r.pk for r in records if isinstance(r, IPAddress)})
self.assertEqual(sum(r.size for r in records if isinstance(r, AvailableIPSpace)), 5)
def test_prefix_ipaddresses_partial_visibility_omits_hidden_range(self):
"""A constraint on child ranges keeps the permitted range and omits the hidden one."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
visible_range = IPRange.objects.create(
start_address=IPNetwork('192.0.2.2/29'),
end_address=IPNetwork('192.0.2.3/29'),
size=2,
mark_populated=True,
description='visible',
)
hidden_range = IPRange.objects.create(
start_address=IPNetwork('192.0.2.4/29'),
end_address=IPNetwork('192.0.2.5/29'),
size=2,
mark_populated=True,
)
obj_perm = ObjectPermission(name='View ranges', actions=['view'], constraints={'description': 'visible'})
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(IPRange))
# Exactly one child range is visible, so the constraint is doing real work.
self.assertIn(visible_range, IPRange.objects.restrict(self.user, 'view'))
self.assertNotIn(hidden_range, IPRange.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
records = list(response.context['table'].data)
self.assertEqual({r.pk for r in records if isinstance(r, IPRange)}, {visible_range.pk})
self.assertEqual(sum(r.size for r in records if isinstance(r, AvailableIPSpace)), 3)
def test_prefix_ipaddresses_sorted_suppresses_available(self):
"""A sorted IP Addresses tab drops synthetic rows so ordering stays in SQL."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress', 'ipam.view_iprange')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'sort': 'address'})
self.assertHttpStatus(response, 200)
records = list(response.context['table'].data)
self.assertEqual({r.pk for r in records if isinstance(r, IPAddress)}, {ip.pk})
self.assertFalse(any(isinstance(r, AvailableIPSpace) for r in records))
def test_prefix_ipaddresses_empty_sort_shows_available(self):
"""An empty sort value is not an ordering, so synthetic rows survive."""
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress', 'ipam.view_iprange')
prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/29'))
ip = IPAddress.objects.create(address=IPNetwork('192.0.2.1/29'))
url = reverse('ipam:prefix_ipaddresses', kwargs={'pk': prefix.pk})
response = self.client.get(url, {'sort': ''})
self.assertHttpStatus(response, 200)
self.assertIPAvailabilityShown(response, ip)
def test_prefix_prefixes_unfiltered_shows_available_prefixes(self):
"""An unfiltered Child Prefixes tab injects synthetic available-prefix rows."""
@ -904,6 +1465,74 @@ class PrefixTestCase(ViewTestCases.PrimaryObjectViewTestCase):
self.assertHttpStatus(response, 200)
self.assertGreater(len(response.context['table'].data), 1)
def test_prefix_prefixes_custom_field_constraint_shows_available(self):
"""A tenant custom-field permission constraint does not suppress available child-prefix rows."""
cf = CustomField.objects.create(name='integerCustomField', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(Tenant)])
tenant = Tenant.objects.create(
name='Child CF Tenant', slug='child-cf-tenant', custom_field_data={'integerCustomField': 1}
)
parent = Prefix.objects.create(prefix=IPNetwork('198.51.100.0/24'), tenant=tenant)
child = Prefix.objects.create(prefix=IPNetwork('198.51.100.0/26'), tenant=tenant)
obj_perm = ObjectPermission(
name='View prefixes', actions=['view'], constraints={'tenant__custom_field_data__integerCustomField': 1}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(Prefix))
self.assertIn(child, Prefix.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_prefixes', kwargs={'pk': parent.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertTrue(response.context['show_available'])
self.assertGreater(len(response.context['table'].data), 1)
def test_prefix_prefixes_available_only_shows_available(self):
"""The Available button's parameters render synthetic rows and no assigned rows."""
self.add_permissions('ipam.view_prefix')
parent = Prefix.objects.create(prefix=IPNetwork('198.51.104.0/24'))
Prefix.objects.create(prefix=IPNetwork('198.51.104.0/26'))
url = reverse('ipam:prefix_prefixes', kwargs={'pk': parent.pk})
response = self.client.get(url, {'show_assigned': 'false', 'show_available': 'true'})
self.assertHttpStatus(response, 200)
rendered = list(response.context['table'].data)
self.assertTrue([p for p in rendered if p.pk is None])
self.assertFalse([p for p in rendered if p.pk is not None])
def test_prefix_prefixes_partial_visibility_shows_available(self):
"""A constraint that hides one child prefix does not suppress the available-prefix rows."""
visible_tenant = Tenant.objects.create(name='PP Partial', slug='pp-partial')
# The parent carries the visible tenant, so the single constrained grant covers the tab itself.
parent = Prefix.objects.create(prefix=IPNetwork('198.51.100.0/24'), tenant=visible_tenant)
visible_child = Prefix.objects.create(prefix=IPNetwork('198.51.100.0/26'), tenant=visible_tenant)
hidden_child = Prefix.objects.create(prefix=IPNetwork('198.51.100.64/26'))
obj_perm = ObjectPermission(
name='View prefixes', actions=['view'], constraints={'tenant__slug': 'pp-partial'}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(Prefix))
self.assertIn(visible_child, Prefix.objects.restrict(self.user, 'view'))
self.assertNotIn(hidden_child, Prefix.objects.restrict(self.user, 'view'))
url = reverse('ipam:prefix_prefixes', kwargs={'pk': parent.pk})
response = self.client.get(url)
# Pins that a constraint does not suppress availability, not how the hidden child is handled.
self.assertHttpStatus(response, 200)
self.assertTrue(response.context['show_available'])
self.assertTrue([p for p in response.context['table'].data if p.pk is None])
def test_prefix_ipaddresses_with_single_address_range(self):
self.add_permissions('ipam.view_prefix', 'ipam.view_ipaddress', 'ipam.view_iprange')
# The IP Addresses tab annotates child IP addresses alongside any
@ -1486,6 +2115,73 @@ class VLANGroupTestCase(ViewTestCases.OrganizationalObjectViewTestCase):
self.assertHttpStatus(response, 200)
self.assertGreater(len(response.context['table'].data), 1)
def test_vlans_custom_field_constraint_shows_available(self):
"""A tenant custom-field permission constraint does not suppress available-VLAN rows."""
cf = CustomField.objects.create(name='integerCustomField', type=CustomFieldTypeChoices.TYPE_INTEGER)
cf.object_types.set([ObjectType.objects.get_for_model(Tenant)])
tenant = Tenant.objects.create(
name='VLAN CF Tenant', slug='vlan-cf-tenant', custom_field_data={'integerCustomField': 1}
)
group = VLANGroup.objects.create(name='CF VLAN Group', slug='cf-vlan-group')
vlan = VLAN.objects.create(group=group, vid=10, name='VLAN0010', tenant=tenant)
self.add_permissions('ipam.view_vlangroup')
obj_perm = ObjectPermission(
name='View VLANs', actions=['view'], constraints={'tenant__custom_field_data__integerCustomField': 1}
)
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(VLAN))
self.assertIn(vlan, VLAN.objects.restrict(self.user, 'view'))
url = reverse('ipam:vlangroup_vlans', kwargs={'pk': group.pk})
response = self.client.get(url)
self.assertHttpStatus(response, 200)
self.assertGreater(len(response.context['table'].data), 1)
def test_vlans_partial_visibility_shows_available(self):
"""A constraint that hides one VLAN does not suppress the available-VLAN rows."""
self.add_permissions('ipam.view_vlangroup')
visible_tenant = Tenant.objects.create(name='VLAN Partial', slug='vlan-partial')
group = VLANGroup.objects.create(name='Partial VLAN Group', slug='partial-vlan-group')
visible_vlan = VLAN.objects.create(group=group, vid=10, name='VLAN0010', tenant=visible_tenant)
hidden_vlan = VLAN.objects.create(group=group, vid=20, name='VLAN0020')
obj_perm = ObjectPermission(name='View VLANs', actions=['view'], constraints={'tenant__slug': 'vlan-partial'})
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(VLAN))
self.assertIn(visible_vlan, VLAN.objects.restrict(self.user, 'view'))
self.assertNotIn(hidden_vlan, VLAN.objects.restrict(self.user, 'view'))
url = reverse('ipam:vlangroup_vlans', kwargs={'pk': group.pk})
response = self.client.get(url)
# Pins that a constraint does not suppress availability, not how the hidden child is handled.
self.assertHttpStatus(response, 200)
rendered = list(response.context['table'].data)
self.assertIn(visible_vlan.vid, {r.vid for r in rendered if isinstance(r, VLAN)})
self.assertTrue([r for r in rendered if isinstance(r, dict)]) # synthetic available VLANs present
def test_vlans_sorted_suppresses_available(self):
"""A sorted VLANs tab drops synthetic available-VLAN rows."""
self.add_permissions('ipam.view_vlangroup', 'ipam.view_vlan')
group = VLANGroup.objects.create(name='Sorted VLAN Group', slug='sorted-vlan-group')
vlan = VLAN.objects.create(group=group, vid=10, name='VLAN0010')
url = reverse('ipam:vlangroup_vlans', kwargs={'pk': group.pk})
response = self.client.get(url, {'sort': 'vid'})
self.assertHttpStatus(response, 200)
rendered = list(response.context['table'].data)
self.assertEqual({r.vid for r in rendered if isinstance(r, VLAN)}, {vlan.vid})
self.assertFalse([r for r in rendered if isinstance(r, dict)]) # no synthetic available VLANs
class VLANTestCase(ViewTestCases.PrimaryObjectViewTestCase):
model = VLAN

View File

@ -79,14 +79,26 @@ def add_requested_prefixes(parent, prefix_list, show_available=True, show_assign
return child_prefixes
def annotate_ip_space(prefix):
def annotate_ip_space(prefix, *, ip_addresses=None, ip_ranges=None):
"""
Return a prefix's child ranges and IPs interleaved with available space records.
:param prefix: Parent Prefix instance
:param ip_addresses: Child IP addresses queryset (defaults to all child IPs)
:param ip_ranges: Child IP ranges queryset (defaults to all populated child ranges)
"""
if ip_addresses is None:
ip_addresses = prefix.get_child_ips()
if ip_ranges is None:
ip_ranges = prefix.get_child_ranges(mark_populated=True)
# Compile child objects
records = []
records.extend([
(iprange.start_address.ip, iprange) for iprange in prefix.get_child_ranges(mark_populated=True)
(iprange.start_address.ip, iprange) for iprange in ip_ranges
])
records.extend([
(ip.address.ip, ip) for ip in prefix.get_child_ips()
(ip.address.ip, ip) for ip in ip_addresses
])
records = sorted(records, key=lambda x: x[0])

View File

@ -1,10 +1,11 @@
import django_filters
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import EmptyResultSet
from django.db.models import Prefetch
from django.db.models.expressions import RawSQL
from django.shortcuts import get_object_or_404, redirect, render
from django.urls import reverse
from django.utils.translation import gettext_lazy as _
from django_filters.constants import EMPTY_VALUES
from circuits.models import Provider
from dcim.filtersets import InterfaceFilterSet
@ -564,62 +565,63 @@ class AggregateView(generic.ObjectView):
class ChildAvailabilityMixin:
"""
Mixin for ObjectChildrenView subclasses that render synthetic "available" rows
(available IP space, prefixes, or VLANs) and must suppress them when the child
queryset has been narrowed by a direct or saved filter.
(available IP space, prefixes, or VLANs) and must suppress them when the request
activates a child object filter, so objects excluded by the filter are not
misrepresented as available space.
"""
@staticmethod
def _where_signature(queryset):
# Compare compiled SQL rather than str(query.where): the WHERE tree embeds default
# object reprs (memory addresses) for permission-constraint subqueries, so two
# otherwise identical querysets built via restrict() never match (#22539).
try:
return queryset.query.get_compiler(using=queryset.db).as_sql()
except EmptyResultSet:
def _get_detection_filterset(self, request):
"""
Return the bound FilterSet used to evaluate the request, or None if the view declares no
filterset. ObjectChildrenView.get() has already built one and validated its form while
resolving the child queryset, so reuse it rather than paying for a second FilterSet:
get_filters() regenerates every dynamic lookup variant and NetBoxModelFilterSet.__init__
queries the custom fields for the model. Build one only for direct calls where get() has
not run.
"""
if self.filterset_instance is not None:
return self.filterset_instance
if self.filterset is None:
return None
def _set_children_filtered(self, is_filtered):
self._child_queryset_is_filtered = is_filtered
return is_filtered
return self.filterset(request.GET, request=request)
def _queryset_is_filtered(self, request, queryset, parent):
def _has_active_child_filters(self, request):
"""
Return True if the filtered child queryset differs from the unfiltered one.
Compares WHERE clauses rather than testing queryset.query.where for truthiness,
because child querysets are already scoped to their parent object and carry WHERE
clauses before any user filter is applied. The result is cached on the view instance
so get_extra_context() can reuse it without rebuilding the queryset.
Return True if the request supplies a valid, non-empty value for any filter declared by
the view's filterset. Saved filters are expanded during filterset instantiation and
dynamic custom field filters are registered on the instance, so both are detected.
Permission constraints and parent scoping never appear in the request, so they cannot
affect the result. The result is memoized because a single request evaluates it from
both prep_table_data and get_extra_context.
"""
if self.filterset is None:
return self._set_children_filtered(False)
if hasattr(self, '_active_child_filters'):
return self._active_child_filters
unfiltered = self.get_children(request, parent)
self._active_child_filters = False
return self._set_children_filtered(
self._where_signature(queryset) != self._where_signature(unfiltered)
)
# An empty request cannot activate a filter, so skip validating a form for nothing.
if not request.GET:
return False
def _children_are_filtered(self, request, parent):
"""
Return whether child objects are filtered.
filterset = self._get_detection_filterset(request)
if filterset is None:
return False
In the normal ObjectChildrenView flow prep_table_data() runs first and caches the
result, so this returns the cached value. Fall back to rebuilding the queryset for
direct calls where prep_table_data() has not run.
"""
if hasattr(self, '_child_queryset_is_filtered'):
return self._child_queryset_is_filtered
# A non-empty cleaned value means the request activated a declared filter. Emptiness
# follows each filter's own semantics, so absent multi-value fields stay inactive.
# This is a no-op for a reused FilterSet: .qs validated the form to build the queryset.
filterset.form.is_valid()
for name, value in filterset.form.cleaned_data.items():
if isinstance(filterset.filters[name], django_filters.MultipleChoiceFilter):
if value: # mirrors MultipleChoiceFilter.filter()
self._active_child_filters = True
break
elif value not in EMPTY_VALUES: # mirrors Filter.filter()
self._active_child_filters = True
break
if self.filterset is None:
return self._set_children_filtered(False)
unfiltered = self.get_children(request, parent)
filtered = self.filterset(request.GET, unfiltered, request=request).qs
return self._set_children_filtered(
self._where_signature(filtered) != self._where_signature(unfiltered)
)
return self._active_child_filters
@register_model_view(Aggregate, 'prefixes')
@ -647,7 +649,7 @@ class AggregatePrefixesView(ChildAvailabilityMixin, generic.ObjectChildrenView):
show_available = bool(request.GET.get('show_available', 'true') == 'true')
show_assigned = bool(request.GET.get('show_assigned', 'true') == 'true')
if self._queryset_is_filtered(request, queryset, parent):
if show_available and self._has_active_child_filters(request):
show_available = False
return add_requested_prefixes(parent.prefix, queryset, show_available, show_assigned)
@ -655,7 +657,7 @@ class AggregatePrefixesView(ChildAvailabilityMixin, generic.ObjectChildrenView):
def get_extra_context(self, request, instance):
show_available = (
bool(request.GET.get('show_available', 'true') == 'true') and
not self._children_are_filtered(request, instance)
not self._has_active_child_filters(request)
)
return {
@ -880,7 +882,7 @@ class PrefixPrefixesView(ChildAvailabilityMixin, generic.ObjectChildrenView):
show_available = bool(request.GET.get('show_available', 'true') == 'true')
show_assigned = bool(request.GET.get('show_assigned', 'true') == 'true')
if self._queryset_is_filtered(request, queryset, parent):
if show_available and self._has_active_child_filters(request):
show_available = False
return add_requested_prefixes(parent.prefix, queryset, show_available, show_assigned)
@ -888,7 +890,7 @@ class PrefixPrefixesView(ChildAvailabilityMixin, generic.ObjectChildrenView):
def get_extra_context(self, request, instance):
show_available = (
bool(request.GET.get('show_available', 'true') == 'true') and
not self._children_are_filtered(request, instance)
not self._has_active_child_filters(request)
)
return {
@ -945,8 +947,11 @@ class PrefixIPAddressesView(ChildAvailabilityMixin, generic.ObjectChildrenView):
return parent.get_child_ips().restrict(request.user, 'view').prefetch_related('vrf', 'tenant', 'tenant__group')
def prep_table_data(self, request, queryset, parent):
if not self._queryset_is_filtered(request, queryset, parent) and not get_table_ordering(request, self.table):
return annotate_ip_space(parent)
# Ordering is checked first: it reads request.GET directly, so a sorted request never
# builds the detection filterset.
if not get_table_ordering(request, self.table) and not self._has_active_child_filters(request):
ip_ranges = parent.get_child_ranges(mark_populated=True).restrict(request.user, 'view')
return annotate_ip_space(parent, ip_addresses=queryset, ip_ranges=ip_ranges)
return super().prep_table_data(request, queryset, parent)
@ -1419,7 +1424,7 @@ class VLANGroupVLANsView(ChildAvailabilityMixin, generic.ObjectChildrenView):
def prep_table_data(self, request, queryset, parent):
# Skip synthetic available rows under active filters: filtered-out VLANs would otherwise look available.
if not self._queryset_is_filtered(request, queryset, parent) and not get_table_ordering(request, self.table):
if not get_table_ordering(request, self.table) and not self._has_active_child_filters(request):
return add_available_vlans(queryset, parent)
return super().prep_table_data(request, queryset, parent)

View File

@ -0,0 +1,43 @@
from collections.abc import Callable, Sequence
from django.contrib.contenttypes.prefetch import GenericPrefetch
from django.db.models import Model, QuerySet
from strawberry.types import Info
from strawberry_django.optimizer import optimize
from strawberry_django.optimizer import optimizer as optimizer_ctx
__all__ = (
'build_gfk_prefetch',
'optimize_prefetch_queryset',
)
def optimize_prefetch_queryset(queryset: QuerySet, info: Info) -> QuerySet:
"""
Apply strawberry-django's query optimizer to a queryset used inside a GenericForeignKey prefetch.
"""
if ext := optimizer_ctx.get():
return ext.optimize(queryset, info)
return optimize(queryset, info)
def build_gfk_prefetch(
lookup: str,
models: Sequence[type[Model]],
) -> Callable[[Info], GenericPrefetch]:
"""
Return a selection-aware GenericPrefetch for a GenericForeignKey field.
Each model gets its own queryset, optimized according to the client's GraphQL selection set.
"""
def prefetch(info: Info) -> GenericPrefetch:
querysets = [
optimize_prefetch_queryset(model.objects.all(), info)
for model in models
]
return GenericPrefetch(lookup, querysets)
return prefetch

View File

@ -1,12 +1,15 @@
import strawberry
from django.db import DEFAULT_DB_ALIAS
from django.db.models.functions import DenseRank
from strawberry.types.unset import UNSET
from strawberry_django.pagination import _QS, apply
from strawberry_django.pagination import _QS, _PaginationWindow, _resolve_limit, apply
from netbox.config import get_config
__all__ = (
'OffsetPaginationInfo',
'OffsetPaginationInput',
'apply_distinct_window_pagination',
'apply_pagination',
)
@ -26,6 +29,58 @@ class OffsetPaginationInput(OffsetPaginationInfo):
pass
def apply_distinct_window_pagination(
queryset: _QS,
*,
related_field_id: str,
offset: int = 0,
limit: int | None = UNSET,
) -> _QS:
"""
Replacement for strawberry-django's `apply_window_pagination()` for a queryset which has `DISTINCT`
enabled, as is the case when a list field is filtered across a to-many relation with `DISTINCT: true`.
SQL evaluates window functions before `DISTINCT`, so the `ROW_NUMBER()` annotation which
strawberry-django uses to paginate a prefetched relation assigns a unique value to each of the
duplicate rows produced by the join, and `DISTINCT` can never collapse them. `DENSE_RANK()` instead
assigns the same rank to every row which compares equal under the window ordering, leaving the
duplicate rows identical so that `DISTINCT` deduplicates them as intended. And because the rank is
incremented only once per distinct row, the rows are numbered as if the duplicates were never there,
keeping the pagination limit meaningful.
"""
limit = _resolve_limit(limit)
order_by = [
expr
for expr, _ in queryset.query.get_compiler(
using=queryset._db or DEFAULT_DB_ALIAS
).get_order_by()
]
# Order by the primary key as well, to ensure that two rows representing *different* objects can
# never be assigned the same rank (and hence be counted only once against the limit).
order_by.append('pk')
# Note that we omit the `_strawberry_total_count` annotation which strawberry-django adds, as it
# cannot be made accurate here: window functions are evaluated before `DISTINCT`, so it would count
# the duplicate rows. strawberry-django's `get_total_count()` already disregards the annotation for
# a queryset with `DISTINCT` enabled and falls back to `count()`, so computing it would be wasted
# work: an extra window aggregate over every joined row.
queryset = queryset.annotate(
_strawberry_row_number=_PaginationWindow(
DenseRank(),
partition_by=related_field_id,
order_by=order_by,
),
)
if offset:
queryset = queryset.filter(_strawberry_row_number__gt=offset)
if limit is not None and limit >= 0:
queryset = queryset.filter(_strawberry_row_number__lte=offset + limit)
return queryset
def apply_pagination(
self,
queryset: _QS,
@ -52,11 +107,33 @@ def apply_pagination(
# Enforce MAX_PAGE_SIZE on the pagination limit
max_page_size = get_config().MAX_PAGE_SIZE
if max_page_size:
# A limit is meaningless for a field which returns at most one object, and synthesizing one for a
# prefetched to-one relation is actively harmful. strawberry-django deliberately leaves `pagination`
# as None there so that the prefetch remains a plain `WHERE id IN (...)` query; making it non-None
# switches the prefetch to a window function partitioned by the parent ID. Every partition then
# holds exactly one row, so ROW_NUMBER() is 1 throughout and the row number filter discards nothing,
# causing the join back to the parent table to return every row which shares the related object.
# See strawberry-graphql/strawberry-django#719.
returns_single_object = not (self.is_list or self.is_paginated or self.is_connection)
if pagination is None:
pagination = OffsetPaginationInput(limit=max_page_size)
# Note that `pagination` is never None for a single-object field unless it is a prefetched
# relation: strawberry-django populates it with an implicit limit of its own beforehand.
if not returns_single_object:
pagination = OffsetPaginationInput(limit=max_page_size)
elif pagination.limit in (None, UNSET) or pagination.limit > max_page_size:
pagination.limit = max_page_size
elif pagination.limit <= 0:
pagination.limit = max_page_size
# A prefetched relation is paginated with a window function, which is incompatible with the
# `DISTINCT` applied by the filter layer. Fall back to our own implementation in that case.
if pagination is not None and related_field_id is not None and queryset.query.distinct:
return apply_distinct_window_pagination(
queryset,
related_field_id=related_field_id,
offset=pagination.offset,
limit=pagination.limit,
)
return apply(pagination, queryset, related_field_id=related_field_id)

View File

@ -646,6 +646,10 @@ SERIALIZATION_MODULES = {
DEBUG_TOOLBAR_CONFIG = {
'SHOW_TOOLBAR_CALLBACK': 'utilities.debug.show_toolbar',
# The GraphiQL integration provided by strawberry-django locates the toolbar via
# document.getElementById('djDebug'), which fails when the toolbar is rendered inside a
# shadow root (the default as of django-debug-toolbar v7.0).
'USE_SHADOW_DOM': False,
}

View File

@ -4,6 +4,7 @@ import importlib
import importlib.util
import os
import sys
import threading
import warnings
from typing import NamedTuple
@ -138,32 +139,48 @@ def _import_module(name):
raise
# Serializes cache checks, module execution, and the temporary sys.path change.
# Reentrant because configuration code may load another path-based module.
_import_lock = threading.RLock()
def _import_from_path(module_name, path):
"""Load a configuration module from an explicit file path.
The module is registered in sys.modules (and removed again if execution fails), and the
file's directory is placed on sys.path for the duration of execution so the module can
import siblings, matching normal import semantics closely enough for configuration files.
The module is registered in sys.modules while it executes, and the file's directory is
placed on sys.path for that duration so the module can import siblings, matching normal
import semantics closely enough for configuration files. A module already loaded under the
same name from the same path is reused, while the same name from a different path replaces
it. A failed load leaves the previous entry in place. Loading is serialized so that a
concurrent caller cannot observe a module mid-execution.
"""
path = os.path.abspath(path)
module_dir = os.path.dirname(path)
spec = importlib.util.spec_from_file_location(module_name, path)
if spec is None or spec.loader is None:
raise ImproperlyConfigured(f"Unable to load configuration file {path}")
module = importlib.util.module_from_spec(spec)
sys.modules[module_name] = module
sys.path.insert(0, module_dir)
try:
spec.loader.exec_module(module)
except Exception:
if sys.modules.get(module_name) is module:
del sys.modules[module_name]
raise
finally:
# Remove only the entry this helper inserted at index 0.
if sys.path and sys.path[0] == module_dir:
sys.path.pop(0)
return module
with _import_lock:
existing = sys.modules.get(module_name)
existing_path = getattr(existing, '__file__', None)
if existing_path and os.path.abspath(existing_path) == path:
return existing
module_dir = os.path.dirname(path)
spec = importlib.util.spec_from_file_location(module_name, path)
if spec is None or spec.loader is None:
raise ImproperlyConfigured(f"Unable to load configuration file {path}")
module = importlib.util.module_from_spec(spec)
sys.modules[module_name] = module
sys.path.insert(0, module_dir)
try:
spec.loader.exec_module(module)
except Exception:
if sys.modules.get(module_name) is module:
if existing is None:
del sys.modules[module_name]
else:
sys.modules[module_name] = existing
raise
finally:
# Remove only the entry this helper inserted at index 0.
if sys.path and sys.path[0] == module_dir:
sys.path.pop(0)
return module
def get_configuration_dir(module):

View File

@ -6,7 +6,7 @@ import django_tables2 as tables
from django.conf import settings
from django.contrib.auth.context_processors import auth
from django.contrib.auth.models import AnonymousUser
from django.db.models import Case, DateField, DateTimeField, F, IntegerField, Value, When
from django.db.models import Case, DateField, DateTimeField, IntegerField, Q, Value, When
from django.db.models.fields.json import KeyTextTransform
from django.template import Context, Template
from django.urls import reverse
@ -526,14 +526,80 @@ class CustomFieldColumn(tables.Column):
CustomFieldTypeChoices.TYPE_MULTIOBJECT
):
kwargs['orderable'] = False
else:
kwargs.setdefault('order_by', (
self.unset_alias,
f'custom_field_data__{customfield.name}',
))
super().__init__(*args, **kwargs)
@property
def unset_alias(self):
"""
Return the name of the annotation which groups together the objects holding no value for
this field (see get_ordering_annotation()).
The annotation is named for the custom field so that ordering by two custom field columns
cannot produce a duplicate alias. Field names are validated to contain only alphanumerics
and underscores, so the alias is always a legal identifier.
"""
return f'_cf_{self.customfield.name}_unset'
def get_ordering_annotation(self):
"""
Return the annotation by which objects holding no value for this field are sorted together,
as the leading sort key for the column. (BaseTable applies it to the queryset when ordering
by this column.)
An object can lack a value either by storing a JSON null or by carrying no key for the
field at all -- the latter being the normal state for objects which predate it, as data is
no longer provisioned onto existing objects (see CustomField.populate_initial_data()).
Postgres sorts those two apart: a JSON null is the lowest jsonb value, whereas a missing
key yields SQL NULL and sorts last, so the "empty" rows would otherwise land at both ends
of the same column. This key (the `empty` lookup covers both states) groups them at one
end, matching how SQL NULLs are ordered for an ordinary column: last when ascending, first
when descending. The column's second sort key then orders by the raw value, so that numeric
and date fields still sort by type rather than lexically.
"""
return {
self.unset_alias: Q(**{f'custom_field_data__{self.customfield.name}__empty': True})
}
def order(self, queryset, is_descending):
# Order by the underlying JSON value, honoring the custom field's null placement preference.
# A missing key or a JSON null value is extracted as SQL NULL via the ->> (text) operator,
# whereas the -> (JSONB) operator used for value ordering treats JSON null as a sortable value.
# We therefore annotate an explicit rank to control null placement independently of JSONB sorting.
"""
Override get_ordering_annotation()'s default (SQL-standard, direction-coupled) null
placement to honor the custom field's nulls_first attribute instead: the empty group's
position is fixed by admin preference, independent of ascending/descending. Returning
(queryset, True) here signals django-tables2 to use this ordering as-is, bypassing the
generic annotation set up by get_ordering_annotation() (which still runs, but its result
goes unused for this column since only its alias name -- referenced by unset_alias --
needs to exist, not the SQL-standard placement it would otherwise apply).
A missing key or a JSON null value is extracted as SQL NULL via the ->> (text) operator,
whereas the -> (JSONB) operator used for value ordering treats JSON null as a sortable
value. We therefore annotate an explicit rank to control null placement independently of
JSONB sorting.
Ordering is expressed as plain string keys (not F()-based OrderBy expressions): NetBox's
BaseTable._apply_ordering_tie_breaker() inspects self.data.data.query.order_by afterward
and wraps each entry in django-tables2's own (string-only) OrderBy helper, which raises
TypeError on a raw expression object.
Trade-off: returning (queryset, True) here is django-tables2's signal that this column
has fully handled ordering itself, which takes priority over -- and discards -- any other
columns' sort keys requested in the same multi-column sort (see TableQuerysetData.order_by()
in django_tables2/data.py: the loop applies whichever column's order() last returns
modified=True and returns immediately, never combining it with sibling columns'
contributions). A CustomFieldColumn can therefore not currently be composed with other
columns in a single sort; it is always the sole and final sort key when included. Preserving
nulls_first (an existing, widely-integrated per-field admin setting) was judged to matter
more than gaining composability for this specific column, since django-tables2's per-key
ascending/descending toggle is applied uniformly across an entire order_by tuple and cannot
keep one key's effective placement constant while another flips -- so nulls_first and
multi-column composition cannot both be expressed through the generic annotation mechanism
for the same column.
"""
name = self.customfield.name
text_value = f'_cf_{name}_text'
null_rank = f'_cf_{name}_nullrank'
@ -547,8 +613,8 @@ class CustomFieldColumn(tables.Column):
output_field=IntegerField(),
),
})
value = F(f'custom_field_data__{name}')
ordering = (null_rank, value.desc() if is_descending else value.asc())
value_field = f'custom_field_data__{name}'
ordering = (null_rank, f'-{value_field}' if is_descending else value_field)
return queryset.order_by(*ordering), True
@staticmethod
@ -654,7 +720,9 @@ class CustomLinkColumn(tables.Column):
return mark_safe(f'<a href="{rendered["link"]}"{rendered["link_target"]}>{rendered["text"]}</a>')
except Exception as e:
error_text = _('Error')
return mark_safe(f'<span class="text-danger" title="{e}"><i class="mdi mdi-alert"></i> {error_text}</span>')
return format_html(
'<span class="text-danger" title="{}"><i class="mdi mdi-alert"></i> {}</span>', e, error_text
)
return ''
def value(self, record, table, **kwargs):

View File

@ -12,6 +12,7 @@ from django.urls.exceptions import NoReverseMatch
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _
from django_tables2.data import TableQuerysetData
from django_tables2.utils import OrderBy
from core.models import ObjectType
from extras.choices import *
@ -158,6 +159,68 @@ class BaseTable(tables.Table):
prefetch_fields.append('__'.join(prefetch_path))
self.data.data = self.data.data.prefetch_related(*prefetch_fields)
def _get_custom_field_ordering_columns(self, order_by):
"""
Return the custom field columns among those named by the given ordering.
Args:
order_by: An iterable (or comma-separated string) of order by aliases.
"""
order_by = order_by.split(',') if isinstance(order_by, str) else order_by or ()
ordering_columns = []
for alias in order_by:
name = OrderBy(alias).bare
# Ignore any aliases which django-tables2 will itself discard
if name not in self.columns or not self.columns[name].orderable:
continue
if isinstance(column := self.columns[name].column, columns.CustomFieldColumn):
ordering_columns.append(column)
return ordering_columns
def _apply_ordering_annotations(self, ordering_columns):
"""
Dynamically annotate the table's QuerySet with the expressions needed to sort by the given
custom field columns. These are applied only for the columns actually being ordered by, to
avoid burdening every query with expressions it has no use for.
"""
annotations = {}
for column in ordering_columns:
annotations.update(column.get_ordering_annotation())
# Skip any annotations already applied, as when the ordering is set more than once
if annotations := {
name: expr for name, expr in annotations.items()
if name not in self.data.data.query.annotations
}:
self.data.data = self.data.data.annotate(**annotations)
def _apply_ordering_tie_breaker(self):
"""
Append the primary key to the table's ordering as a final sort key, so that the ordering is
total. Rows tying on every preceding key -- and every object holding no value for a custom
field ties on both of that column's keys -- are otherwise free to come back in a different
order for each query, which would cause paginated results to skip or repeat rows from one
page to the next.
"""
ordering = self.data.data.query.order_by
if ordering and not any(OrderBy(o).bare in ('pk', 'id') for o in ordering):
self.data.data = self.data.data.order_by(*ordering, 'pk')
@tables.Table.order_by.setter
def order_by(self, value):
"""
Extend the ordering of the table's data with the support needed by custom field columns.
"""
if not isinstance(self.data, TableQuerysetData):
tables.Table.order_by.fset(self, value)
return
if ordering_columns := self._get_custom_field_ordering_columns(value):
self._apply_ordering_annotations(ordering_columns)
tables.Table.order_by.fset(self, value)
if ordering_columns:
self._apply_ordering_tie_breaker()
def configure(self, request):
"""
Configure the table for a specific request context. This performs pagination and records

View File

@ -9,15 +9,18 @@ from django.db import connection
from django.test import override_settings
from django.test.utils import CaptureQueriesContext
from django.urls import reverse
from django.utils import timezone
from rest_framework import status
from strawberry.extensions import QueryDepthLimiter
from strawberry.schema.config import StrawberryConfig
from core.models import ObjectType
from dcim.choices import LocationStatusChoices
from dcim.models import (
Device,
DeviceRole,
DeviceType,
Interface,
Location,
Manufacturer,
Rack,
@ -25,13 +28,23 @@ from dcim.models import (
Site,
VirtualChassis,
)
from extras.models import TableConfig, Tag
from extras.choices import CustomFieldTypeChoices
from extras.models import CustomField, TableConfig, Tag
from ipam.models import RIR, Aggregate, IPAddress, Prefix
from netbox.graphql.pagination import apply_distinct_window_pagination
from netbox.graphql.scalars import BigInt, BigIntScalar
from netbox.graphql.schema import Query, get_schema_extensions, schema
from users.models import Token, User
from utilities.tables import get_table_for_model
from utilities.testing import APITestCase, APIViewTestCases, TestCase, disable_warnings
def count_primary_table_queries(queries, table):
"""Count queries that read from `table` as the primary relation (not only as a join)."""
pattern = re.compile(rf'FROM "{re.escape(table)}"')
return sum(1 for query_record in queries if pattern.search(query_record['sql']))
class GraphQLTestCase(TestCase):
def _schema_extension_instances(self):
@ -429,6 +442,31 @@ class GraphQLAPITestCase(APITestCase):
self.assertNotIn('errors', data)
self.assertEqual(int(data['data']['table_config']['object_type']['id']), site_ct.pk)
def test_graphql_custom_fields_include_unset_fields(self):
"""
CustomFieldsMixin.custom_fields must emit a key for every custom field assigned to the model,
as the REST API does, rather than returning the stored data verbatim. A key is materialized
only once a value is assigned, so an object predating a field carries none; without this such
a field would be absent from the response instead of null. Stale data for a field which no
longer applies is likewise omitted.
"""
self.add_permissions('dcim.view_site')
url = reverse('graphql')
cf = CustomField.objects.create(name='cf1', type=CustomFieldTypeChoices.TYPE_TEXT)
cf.object_types.set([ObjectType.objects.get_for_model(Site)])
site = Site.objects.get(slug='site-1')
self.assertNotIn('cf1', site.custom_field_data)
Site.objects.filter(pk=site.pk).update(custom_field_data={'stale': 'value'})
query = '{ site(id: ' + str(site.pk) + ') { custom_fields } }'
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(data['data']['site']['custom_fields'], {'cf1': None})
@override_settings(LOGIN_REQUIRED=True)
def test_graphql_device_list_tags_are_prefetched(self):
"""
@ -486,6 +524,149 @@ class GraphQLAPITestCase(APITestCase):
msg=f'Expected batched tag prefetch, got {tag_queries} tag queries for 10 devices',
)
def test_graphql_ip_address_list_assigned_object(self):
"""
Requesting assigned_object should batch prefetch related objects.
"""
self.add_permissions('ipam.view_ipaddress', 'dcim.view_interface', 'dcim.view_device')
site = Site.objects.first()
manufacturer = Manufacturer.objects.create(name='Assigned Object Manufacturer', slug='assigned-object-mfg')
device_type = DeviceType.objects.create(
manufacturer=manufacturer,
model='Assigned Object Model',
slug='assigned-object-model',
)
device_role = DeviceRole.objects.create(name='Assigned Object Role', slug='assigned-object-role')
device = Device.objects.create(
name='Assigned Object Device',
site=site,
device_type=device_type,
role=device_role,
)
interface = Interface.objects.create(name='eth0', device=device, type='1000baset')
ip_addresses = IPAddress.objects.bulk_create([
IPAddress(address=f'192.0.2.{index}/24', assigned_object=interface)
for index in range(1, 6)
])
ip_ids = json.dumps([str(ip.pk) for ip in ip_addresses])
query = f"""
{{
ip_address_list(filters: {{id: {{in_list: {ip_ids}}}}}) {{
address
assigned_object {{
... on InterfaceType {{
name
device {{
name
}}
}}
}}
}}
}}
"""
url = reverse('graphql')
with CaptureQueriesContext(connection) as context:
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['ip_address_list']), len(ip_addresses))
device_queries = count_primary_table_queries(context.captured_queries, 'dcim_device')
self.assertLessEqual(
device_queries,
2,
msg=f'Expected batched assigned_object prefetch, got {device_queries} device queries for 5 IP addresses',
)
def test_graphql_ip_address_list_assigned_object_nested_site(self):
"""
Nested assigned_object selections should be optimized on the GFK prefetch queryset.
"""
self.add_permissions(
'ipam.view_ipaddress',
'dcim.view_interface',
'dcim.view_device',
'dcim.view_site',
)
site = Site.objects.first()
manufacturer = Manufacturer.objects.create(
name='Nested Site Manufacturer',
slug='nested-site-mfg',
)
device_type = DeviceType.objects.create(
manufacturer=manufacturer,
model='Nested Site Model',
slug='nested-site-model',
)
device_role = DeviceRole.objects.create(name='Nested Site Role', slug='nested-site-role')
interfaces = []
for index in range(5):
device = Device.objects.create(
name=f'Nested Site Device {index}',
site=site,
device_type=device_type,
role=device_role,
)
interfaces.append(Interface.objects.create(
name=f'eth{index}',
device=device,
type='1000baset',
))
ip_addresses = IPAddress.objects.bulk_create([
IPAddress(address=f'192.0.2.{index}/24', assigned_object=interfaces[index - 1])
for index in range(1, 6)
])
ip_ids = json.dumps([str(ip.pk) for ip in ip_addresses])
query = f"""
{{
ip_address_list(filters: {{id: {{in_list: {ip_ids}}}}}) {{
address
assigned_object {{
... on InterfaceType {{
name
device {{
name
site {{
name
}}
}}
}}
}}
}}
}}
"""
url = reverse('graphql')
with CaptureQueriesContext(connection) as context:
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['ip_address_list']), len(ip_addresses))
for ip_data in data['data']['ip_address_list']:
self.assertEqual(ip_data['assigned_object']['device']['site']['name'], site.name)
device_queries = count_primary_table_queries(context.captured_queries, 'dcim_device')
site_queries = count_primary_table_queries(context.captured_queries, 'dcim_site')
self.assertLessEqual(
device_queries,
2,
msg=f'Expected batched device prefetch, got {device_queries} device queries for 5 IP addresses',
)
self.assertLessEqual(
site_queries,
2,
msg=f'Expected optimized site join, got {site_queries} site queries for 5 IP addresses',
)
def test_offset_pagination(self):
self.add_permissions('dcim.view_site')
url = reverse('graphql')
@ -644,6 +825,239 @@ class GraphQLAPITestCase(APITestCase):
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['site_list']), 2)
def test_to_one_relation_prefetch(self):
"""
A prefetched to-one relation should be fetched with a plain `WHERE id IN (...)` query, rather than
with a window function partitioned by the parent ID (which returns every row sharing the related
object, regardless of the requested page size).
"""
self.add_permissions('dcim.view_device', 'dcim.view_site')
url = reverse('graphql')
site = Site.objects.first()
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
Device.objects.bulk_create([
Device(name=f'Device {i}', site=site, device_type=device_type, role=role)
for i in range(1, 21)
])
# Request two of the twenty devices at the site
query = """
{
device_list(pagination: {limit: 2}) {
name
site { name }
}
}
"""
with CaptureQueriesContext(connection) as ctx:
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['device_list']), 2)
self.assertEqual(data['data']['device_list'][0]['site']['name'], site.name)
# The site should have been fetched by exactly one query. (Asserting that it exists keeps the
# assertions below from silently passing if the site is ever fetched some other way.)
site_queries = [q['sql'] for q in ctx.captured_queries if 'FROM "dcim_site"' in q['sql']]
self.assertEqual(len(site_queries), 1, msg=f'Expected one query against dcim_site, got {site_queries}')
# That query should not apply window pagination, nor join back to the devices table (which would
# return one row per device at the site)
self.assertNotIn('ROW_NUMBER', site_queries[0])
self.assertNotIn('dcim_device', site_queries[0])
@override_settings(MAX_PAGE_SIZE=3)
def test_max_page_size_nested_list(self):
"""
MAX_PAGE_SIZE should still be enforced on a nested list relation.
"""
self.add_permissions('dcim.view_device', 'dcim.view_site')
url = reverse('graphql')
site = Site.objects.first()
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
Device.objects.bulk_create([
Device(name=f'Device {i}', site=site, device_type=device_type, role=role)
for i in range(1, 6)
])
query = """
{
site_list(pagination: {limit: 1}) {
name
devices { name }
}
}
"""
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['site_list'][0]['devices']), 3)
def test_distinct_nested_list(self):
"""
The `DISTINCT` filter should deduplicate a nested list field which is filtered across a to-many
relation, just as it does for the equivalent top-level list field.
"""
self.add_permissions('dcim.view_device', 'dcim.view_site')
url = reverse('graphql')
site = Site.objects.get(slug='site-1')
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
devices = Device.objects.bulk_create([
Device(name=f'Device {i}', site=site, device_type=device_type, role=role)
for i in range(1, 3)
])
Interface.objects.bulk_create([
Interface(device=device, name=f'eth{i}', type='1000base-t')
for device in devices
for i in range(3)
])
# Each device should be returned exactly once, despite having three matching interfaces
query = """
{
site_list(filters: {slug: {exact: "site-1"}}) {
name
devices(filters: {DISTINCT: true, interfaces: {name: {starts_with: "eth"}}}) {
name
}
}
}
"""
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(
[device['name'] for device in data['data']['site_list'][0]['devices']],
['Device 1', 'Device 2']
)
# The equivalent top-level query should return the same devices
query = """
{
device_list(filters: {DISTINCT: true, interfaces: {name: {starts_with: "eth"}}}) {
name
}
}
"""
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(
[device['name'] for device in data['data']['device_list']],
['Device 1', 'Device 2']
)
@override_settings(MAX_PAGE_SIZE=2)
def test_distinct_nested_list_max_page_size(self):
"""
MAX_PAGE_SIZE should still be enforced on a deduplicated nested list field, and should be applied
to the number of distinct objects returned (not to the number of joined rows).
"""
self.add_permissions('dcim.view_device', 'dcim.view_site')
url = reverse('graphql')
site = Site.objects.get(slug='site-1')
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
devices = Device.objects.bulk_create([
Device(name=f'Device {i}', site=site, device_type=device_type, role=role)
for i in range(1, 5)
])
Interface.objects.bulk_create([
Interface(device=device, name=f'eth{i}', type='1000base-t')
for device in devices
for i in range(3)
])
query = """
{
site_list(filters: {slug: {exact: "site-1"}}) {
name
devices(filters: {DISTINCT: true, interfaces: {name: {starts_with: "eth"}}}) {
name
}
}
}
"""
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(
[device['name'] for device in data['data']['site_list'][0]['devices']],
['Device 1', 'Device 2']
)
# An explicit offset should likewise be applied to the distinct objects
query = """
{
site_list(filters: {slug: {exact: "site-1"}}) {
name
devices(
pagination: {offset: 1, limit: 2},
filters: {DISTINCT: true, interfaces: {name: {starts_with: "eth"}}}
) {
name
}
}
}
"""
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(
[device['name'] for device in data['data']['site_list'][0]['devices']],
['Device 2', 'Device 3']
)
def test_distinct_window_pagination_tied_ordering(self):
"""
Two rows which represent *different* objects must never be assigned the same rank, even when they
compare equal under the queryset's ordering. `DENSE_RANK()` ties such rows by definition, so the
primary key is appended to the window ordering to separate them; without it every device below
would be assigned rank 1 and the limit of two would return all four of them.
"""
site = Site.objects.get(slug='site-1')
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
devices = Device.objects.bulk_create([
Device(name=f'Device {i}', site=site, device_type=device_type, role=role)
for i in range(1, 5)
])
Interface.objects.bulk_create([
Interface(device=device, name=f'eth{i}', type='1000base-t')
for device in devices
for i in range(3)
])
# Order by a column whose value is identical for every device, so that the ordering alone cannot
# distinguish them. Each device additionally matches three interfaces, so the join emits three
# duplicate rows per device which DISTINCT must still collapse.
queryset = Device.objects.filter(
site=site, interfaces__name__startswith='eth'
).order_by('status').distinct()
queryset = apply_distinct_window_pagination(queryset, related_field_id='site_id', limit=2)
results = list(queryset)
self.assertEqual(sorted(device.name for device in results), ['Device 1', 'Device 2'])
self.assertEqual(sorted(device._strawberry_row_number for device in results), [1, 2])
def test_pagination_conflict(self):
url = reverse('graphql')
query = """
@ -660,6 +1074,209 @@ class GraphQLAPITestCase(APITestCase):
self.assertEqual(data['errors'][0]['message'], 'Cannot specify both `start` and `offset` in pagination.')
class GraphQLDeferredColumnTestCase(APITestCase):
"""
A GraphQL field backed by a custom resolver is opaque to the query optimizer, which narrows column
selection with .only() based on the fields named in the GraphQL document. Any column such a resolver
reads must therefore be declared via an `only` hint; otherwise the column is deferred and reading it
reloads the row from the database once per object returned (see #22813).
Each test below asserts both that no single-row reload occurs and that the total query count does not
grow with the number of objects returned.
"""
OBJECT_COUNT = 10
@classmethod
def setUpTestData(cls):
site = Site.objects.create(name='Site 1', slug='site-1')
# Devices, for CustomFieldsMixin.custom_fields
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1', slug='device-type-1')
device_role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
custom_field = CustomField.objects.create(name='cf1', type=CustomFieldTypeChoices.TYPE_TEXT)
custom_field.object_types.set([ObjectType.objects.get_for_model(Device)])
Device.objects.bulk_create([
Device(
name=f'Device {i}',
device_type=device_type,
role=device_role,
site=site,
custom_field_data={'cf1': f'value {i}'},
)
for i in range(cls.OBJECT_COUNT)
])
# Rack reservations, for RackReservationType.unit_count. Reservations within a rack may not claim
# overlapping units, so each is allocated a distinct run of them. The length of each run varies so
# that unit_count is asserted per object rather than against a single expected value.
rack = Rack.objects.create(name='Rack 1', site=site)
user = User.objects.create(username='Reservation user')
reservations, next_unit = [], 1
for i in range(cls.OBJECT_COUNT):
unit_count = i % 3 + 1
reservations.append(RackReservation(
rack=rack,
units=list(range(next_unit, next_unit + unit_count)),
user=user,
description=f'Reservation {i}',
))
next_unit += unit_count
cls.expected_unit_counts = {
reservation.pk: len(reservation.units)
for reservation in RackReservation.objects.bulk_create(reservations)
}
# IPAM objects, for the `family` field of each type which exposes one
IPAddress.objects.bulk_create([
IPAddress(address=f'10.0.0.{i + 1}/24') for i in range(cls.OBJECT_COUNT)
])
Prefix.objects.bulk_create([
Prefix(prefix=f'10.{i}.0.0/16') for i in range(cls.OBJECT_COUNT)
])
rir = RIR.objects.create(name='RIR 1', slug='rir-1')
Aggregate.objects.bulk_create([
Aggregate(prefix=f'{i + 20}.0.0.0/8', rir=rir) for i in range(cls.OBJECT_COUNT)
])
def _execute(self, query):
url = reverse('graphql')
with CaptureQueriesContext(connection) as context:
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
return data['data'], context.captured_queries
def assertNoDeferredColumnReloads(self, query_template, list_field, table, validate):
"""
Execute `query_template` (which must accept a `limit` interpolation) for a single object and for
OBJECT_COUNT objects, asserting that no row of `table` is re-fetched by primary key and that the
total query count is identical for both. `validate` is called with the returned objects.
"""
# Token authentication updates Token.last_used at most once per minute, so the first API request
# made by a test issues an additional UPDATE (see netbox/api/authentication.py). Stamp the token
# up front to keep that one-off write out of the counts compared below.
Token.objects.filter(pk=self.token.pk).update(last_used=timezone.now())
query_counts = []
for limit in (1, self.OBJECT_COUNT):
data, queries = self._execute(query_template % {'limit': limit})
objects = data[list_field]
self.assertEqual(len(objects), limit)
validate(objects)
reloads = [q['sql'] for q in queries if f'FROM "{table}" WHERE "{table}"."id" = ' in q['sql']]
self.assertEqual(
reloads, [], msg=f'{len(reloads)} deferred-column reload(s) for {limit} object(s): {reloads[:1]}'
)
query_counts.append(len(queries))
self.assertEqual(
query_counts[0],
query_counts[1],
msg=(
f'Query count grew from {query_counts[0]} to {query_counts[1]} when the number of objects '
f'returned grew from 1 to {self.OBJECT_COUNT}'
)
)
def test_custom_fields(self):
"""
Regression test for #22813: CustomFieldsMixin.custom_fields must not defer `custom_field_data`.
"""
self.add_permissions('dcim.view_device')
query = """
{
device_list(pagination: {limit: %(limit)s}) {
id
custom_fields
}
}
"""
expected_values = {f'value {i}' for i in range(self.OBJECT_COUNT)}
def validate(devices):
for device in devices:
self.assertEqual(list(device['custom_fields']), ['cf1'])
self.assertIn(device['custom_fields']['cf1'], expected_values)
self.assertNoDeferredColumnReloads(query, 'device_list', 'dcim_device', validate)
def test_rack_reservation_unit_count(self):
"""
Regression test for #22822: RackReservationType.unit_count must not defer `units`.
"""
self.add_permissions('dcim.view_rackreservation')
query = """
{
rack_reservation_list(pagination: {limit: %(limit)s}) {
id
unit_count
}
}
"""
def validate(reservations):
for reservation in reservations:
self.assertEqual(
reservation['unit_count'], self.expected_unit_counts[int(reservation['id'])]
)
self.assertNoDeferredColumnReloads(query, 'rack_reservation_list', 'dcim_rackreservation', validate)
def test_ip_address_family(self):
"""
Regression test for #22823: IPAddressType.family must not defer `address`.
"""
self.add_permissions('ipam.view_ipaddress')
query = """
{
ip_address_list(pagination: {limit: %(limit)s}) {
id
family { value label }
}
}
"""
self.assertNoDeferredColumnReloads(
query, 'ip_address_list', 'ipam_ipaddress', self._validate_ipv4_family
)
def test_prefix_family(self):
"""
Regression test for #22823: PrefixType.family must not defer `prefix`.
"""
self.add_permissions('ipam.view_prefix')
query = """
{
prefix_list(pagination: {limit: %(limit)s}) {
id
family { value label }
}
}
"""
self.assertNoDeferredColumnReloads(query, 'prefix_list', 'ipam_prefix', self._validate_ipv4_family)
def test_aggregate_family(self):
"""
Regression test for #22823: AggregateType.family must not defer `prefix`.
"""
self.add_permissions('ipam.view_aggregate')
query = """
{
aggregate_list(pagination: {limit: %(limit)s}) {
id
family { value label }
}
}
"""
self.assertNoDeferredColumnReloads(query, 'aggregate_list', 'ipam_aggregate', self._validate_ipv4_family)
def _validate_ipv4_family(self, objects):
for obj in objects:
self.assertEqual(obj['family'], {'value': 4, 'label': 'IPv4'})
class GraphQLSchemaCoverageTestCase(APIViewTestCases.GraphQLSchemaCoverageTestCase):
pass

View File

@ -133,6 +133,52 @@ class LoadConfigurationTest(SimpleTestCase):
finally:
sys.path[:] = saved
def test_import_from_path_reuses_module_loaded_from_same_path(self):
"""A repeated load of the same path returns the first module and runs the file only once."""
with tempfile.TemporaryDirectory() as root:
marker = os.path.join(root, 'executions')
path = os.path.join(root, 'cached_cfg.py')
with open(path, 'w') as handle:
handle.write(f'with open({marker!r}, "a") as handle:\n handle.write("x")\n')
self.addCleanup(sys.modules.pop, 'netbox_test_cached_cfg', None)
first = settings_utils._import_from_path('netbox_test_cached_cfg', path)
second = settings_utils._import_from_path('netbox_test_cached_cfg', path)
self.assertIs(second, first)
with open(marker) as handle:
self.assertEqual(handle.read(), 'x')
def test_import_from_path_replaces_module_loaded_from_another_path(self):
"""The same module name at a different path is loaded fresh, not served from the cache."""
with tempfile.TemporaryDirectory() as root:
first_path = os.path.join(root, 'first_cfg.py')
second_path = os.path.join(root, 'second_cfg.py')
with open(first_path, 'w') as handle:
handle.write('ALLOWED_HOSTS = ["first"]\n')
with open(second_path, 'w') as handle:
handle.write('ALLOWED_HOSTS = ["second"]\n')
self.addCleanup(sys.modules.pop, 'netbox_test_switched_cfg', None)
settings_utils._import_from_path('netbox_test_switched_cfg', first_path)
module = settings_utils._import_from_path('netbox_test_switched_cfg', second_path)
self.assertEqual(module.ALLOWED_HOSTS, ['second'])
self.assertIs(sys.modules['netbox_test_switched_cfg'], module)
def test_import_from_path_restores_previous_module_on_failure(self):
"""A failed replacement does not evict the previously loaded module."""
with tempfile.TemporaryDirectory() as root:
module_name = 'netbox_test_restore_cfg'
first_path = os.path.join(root, 'first_cfg.py')
broken_path = os.path.join(root, 'broken_cfg.py')
with open(first_path, 'w') as handle:
handle.write('ALLOWED_HOSTS = ["first"]\n')
with open(broken_path, 'w') as handle:
handle.write('raise RuntimeError("Simulated configuration error")\n')
self.addCleanup(sys.modules.pop, module_name, None)
first = settings_utils._import_from_path(module_name, first_path)
with self.assertRaisesMessage(RuntimeError, 'Simulated configuration error'):
settings_utils._import_from_path(module_name, broken_path)
self.assertIs(sys.modules[module_name], first)
self.assertIs(settings_utils._import_from_path(module_name, first_path), first)
def test_wheel_both_configs_present_warns_and_prefers_conf(self):
with tempfile.TemporaryDirectory() as root:
conf = os.path.join(root, 'conf')
@ -291,6 +337,16 @@ class LoadLdapConfigTest(SimpleTestCase):
self.assertEqual(module.AUTH_LDAP_SERVER_URI, 'ldaps://example')
self.assertIs(sys.modules['netbox.ldap_config'], module)
def test_repeated_calls_reuse_the_sibling_module(self):
"""Two calls with an unchanged sibling ldap_config.py return the same module object."""
with tempfile.TemporaryDirectory() as conf_dir:
with open(os.path.join(conf_dir, 'ldap_config.py'), 'w') as handle:
handle.write('AUTH_LDAP_SERVER_URI = "ldaps://example"\n')
self.addCleanup(sys.modules.pop, 'netbox.ldap_config', None)
first = settings_utils.load_ldap_config(conf_dir)
second = settings_utils.load_ldap_config(conf_dir)
self.assertIs(second, first)
def test_legacy_fallback_loads_historical_module_with_warning(self):
legacy = ModuleType('netbox.ldap_config')
legacy.AUTH_LDAP_SERVER_URI = 'ldaps://legacy'

View File

@ -98,12 +98,14 @@ class ObjectChildrenView(ObjectView, ActionsMixin, TableMixin):
table: The django-tables2 Table class used to render the child objects list
filterset: A django-filter FilterSet that is applied to the queryset
filterset_form: The form class used to render filter options
filterset_instance: The bound FilterSet built for the current request (set during get())
actions: An iterable of ObjectAction subclasses (see ActionsMixin)
"""
child_model = None
table = None
filterset = None
filterset_form = None
filterset_instance = None
actions = (CloneObject, EditObject, DeleteObject, BulkEdit, BulkDelete)
template_name = 'generic/object_children.html'
@ -142,7 +144,10 @@ class ObjectChildrenView(ObjectView, ActionsMixin, TableMixin):
child_objects = self.get_children(request, instance)
if self.filterset:
child_objects = self.filterset(request.GET, child_objects, request=request).qs
# Retain the bound FilterSet so that prep_table_data() and get_extra_context() can
# inspect the request's validated filter data without rebuilding it.
self.filterset_instance = self.filterset(request.GET, child_objects, request=request)
child_objects = self.filterset_instance.qs
# Determine the available actions
actions = self.get_permitted_actions(request.user, model=self.child_model)

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -1,5 +1,5 @@
import type { RecursivePartial, TomOption, TomSettings, TomInput } from 'tom-select/dist/cjs/types';
import { addClasses } from 'tom-select/src/vanilla.ts';
import { addClasses, removeClasses } from 'tom-select/src/vanilla.ts';
import queryString from 'query-string';
import type { Stringifiable } from 'query-string';
import { DynamicParamsMap } from './dynamicParamsMap';
@ -20,6 +20,18 @@ export class DynamicTomSelect extends NetBoxTomSelect {
private readonly dynamicParams: DynamicParamsMap = new DynamicParamsMap();
private readonly pathValues: PathFilter = new Map();
// Incremented on every load() call. Lets us detect and discard stale responses: if a
// newer load() has started (e.g. because two dependencies changed in quick succession)
// before an older request's response arrives, the older response is out of date and
// must not be allowed to overwrite state set by the newer one.
private loadSequence = 0;
// Tracks a previous selection that still needs to be restored once a settled request wins.
// Stored on the instance rather than only as a `load()` parameter -- if the request carrying
// it is itself superseded by a later cascading load() call before it resolves, the value
// isn't lost; whichever request's response ultimately wins can still attempt to restore it.
private pendingRestoreValue?: string | string[];
/**
* Overrides
*/
@ -75,8 +87,33 @@ export class DynamicTomSelect extends NetBoxTomSelect {
load(value: string, preserveValue?: string | string[]) {
const self = this;
// No API endpoint is configured yet (e.g. a generic object selector before a content type is chosen).
// Record which request this is. Incremented unconditionally, before any early return
// below, so that an already-in-flight request from a previous call is always correctly
// invalidated by any newer call to load() -- even one that itself aborts early (e.g. no
// valid URL). If another load() call starts before this one's response comes back,
// `self.loadSequence` will have moved on and this response is stale -- it must be
// discarded rather than applied.
self.loadSequence += 1;
const sequence = self.loadSequence;
// Remember any value that still needs to be restored, without erasing a value captured
// by an earlier, still-in-flight call. If this particular call has nothing new to
// preserve (e.g. its dependency was already cleared by a cascaded change), an earlier
// call's pending value should still get a chance to be restored by whichever request
// ends up winning. An empty array (e.g. a multi-select cleared by clear()) doesn't count
// as something worth preserving.
const hasValue = Array.isArray(preserveValue)
? preserveValue.length > 0
: preserveValue !== undefined;
if (hasValue) {
self.pendingRestoreValue = preserveValue;
}
// No API endpoint is configured yet (e.g. a generic object selector before a content type
// is chosen). No options can be shown under this state, so any pending value carried from
// an earlier call is no longer relevant to restore here.
if (!self.api_url) {
self.pendingRestoreValue = undefined;
return;
}
@ -89,9 +126,12 @@ export class DynamicTomSelect extends NetBoxTomSelect {
self.addOption(self.nullOption);
}
// Get the API request URL. If none is provided, abort as no request can be made.
// Get the API request URL. If none is provided, abort as no request can be made. No
// options can be shown for this field under its current (invalid) filter, so any
// pending value carried from an earlier call is no longer relevant to restore here.
const url = self.getRequestUrl(value);
if (!url) {
self.pendingRestoreValue = undefined;
return;
}
@ -112,17 +152,32 @@ export class DynamicTomSelect extends NetBoxTomSelect {
})
// Pass the options to the callback function
.then(options => {
// A newer load() has since been issued (e.g. two dependencies changed in quick
// succession). This response is stale; applying it now would risk clobbering
// state already set by the newer, still-in-flight or already-resolved request.
if (sequence !== self.loadSequence) {
self.finalizeStaleLoad();
return;
}
self.loadCallback(options, []);
// Restore the previous selection if it is still valid under the new filter.
if (preserveValue !== undefined) {
const values = Array.isArray(preserveValue) ? preserveValue : [preserveValue];
if (self.pendingRestoreValue !== undefined) {
const values = Array.isArray(self.pendingRestoreValue)
? self.pendingRestoreValue
: [self.pendingRestoreValue];
const validValues = values.filter(v => v !== '' && v in self.options);
if (validValues.length > 0) {
self.setValue(validValues.length === 1 ? validValues[0] : validValues, true);
}
self.pendingRestoreValue = undefined;
}
})
.catch(() => {
if (sequence !== self.loadSequence) {
self.finalizeStaleLoad();
return;
}
self.pendingRestoreValue = undefined;
self.loadCallback([], []);
});
}
@ -131,6 +186,17 @@ export class DynamicTomSelect extends NetBoxTomSelect {
* Custom methods
*/
// Finalizes Tom Select's loading state after a superseded (stale) response settles: clears
// the loading counter and, once it reaches zero, removes the wrapper's loading class and
// refreshes the dropdown to drop any stale loading indicator rendered internally.
private finalizeStaleLoad(): void {
this.loading = Math.max(this.loading - 1, 0);
if (!this.loading) {
removeClasses(this.wrapper, this.settings.loadingClass);
this.refreshOptions(false);
}
}
// Formulate and return the complete URL for an API request, including any query parameters.
getRequestUrl(search: string): string {
if (!this.api_url) {

View File

@ -18,6 +18,11 @@ $table-cell-padding-y: 0.5rem;
// Ensure active nav-pill has a background color in dark mode
$nav-pills-link-active-bg: rgba(var(--tblr-secondary-rgb), 0.15);
// Tabler borders unchecked form controls with the translucent token, which drops to
// 1.1:1 against the dark-mode control fill. Restate it as a solid grey in dark mode.
$form-check-input-border: var(--tblr-border-width) var(--tblr-border-style)
light-dark(var(--tblr-border-color-translucent), var(--tblr-gray-600));
// Brand colors
$rich-black: #001423;
$rich-black-light: #081B2A;

View File

@ -160,6 +160,15 @@ html[data-bs-theme='dark'] {
--tblr-table-hover-bg: inherit;
--tblr-table-hover-color: inherit;
}
// Tabler bakes a white glyph into the checked data URI, which reads at 1.4:1 against
// the teal dark-mode primary. A data URI is an isolated document, so the color cannot
// come from a custom property and has to be baked in at build time.
.form-check-input:checked[type='checkbox'] {
--tblr-form-check-bg-image: #{escape-svg(
url("data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16' width='16' height='16'><path fill='none' stroke='#{$rich-black}' stroke-linecap='round' stroke-linejoin='round' stroke-width='2' d='M4 8.5l2.5 2.5l5.5 -5.5'/></svg>")
)};
}
}
// Do not apply padding to <code> elements inside a <pre>

View File

@ -1,3 +1,4 @@
version: "4.6.6"
version: "4.6.8"
edition: "Community"
published: "2026-07-28"
build: "rc2"
published: "2026-08-04"

View File

@ -17,6 +17,7 @@
{# Cable trace SVG & options #}
<div class="col col-md-5">
{% if path %}
{% if svg_url %}
<div class="text-center my-3">
<object data="{{ svg_url }}" class="rack_elevation"></object>
<div>
@ -25,6 +26,7 @@
</a>
</div>
</div>
{% endif %}
<div class="trace-end">
{% if path.is_split and path.get_asymmetric_nodes %}
<h3 class="text-danger">{% trans "Asymmetric Path" %}!</h3>

View File

@ -1,5 +1,6 @@
from django.contrib.postgres.indexes import GistIndex
from django.db import models
from django.db.models import Count, ProtectedError, Q
from django.utils.translation import gettext_lazy as _
from netbox.models import NestedLtreeGroupModel, PrimaryModel
@ -37,6 +38,24 @@ class TenantGroup(NestedLtreeGroupModel):
verbose_name = _('tenant group')
verbose_name_plural = _('tenant groups')
def delete(self, *args, **kwargs):
# Ungrouping the tenants of this group and its descendants can violate tenant name and slug uniqueness.
ungrouped = Tenant.objects.filter(
Q(group__isnull=True) | Q(group__in=self.get_descendants(include_self=True))
)
duplicate_names = ungrouped.values('name').annotate(count=Count('pk')).filter(count__gt=1).values('name')
duplicate_slugs = ungrouped.values('slug').annotate(count=Count('pk')).filter(count__gt=1).values('slug')
if conflicts := set(ungrouped.filter(Q(name__in=duplicate_names) | Q(slug__in=duplicate_slugs))):
raise ProtectedError(
_(
"Unable to delete tenant group {tenant_group}. Ungrouping its tenants, including those of any "
"nested groups, would create duplicate tenant names or slugs."
).format(tenant_group=self),
conflicts,
)
return super().delete(*args, **kwargs)
class Tenant(ContactsMixin, PrimaryModel):
"""

View File

@ -1,14 +1,14 @@
{
"contact:api_list_objects": 14,
"contact:list_objects_with_permission": 21,
"contactassignment:api_list_objects": 17,
"contactassignment:list_objects_with_permission": 25,
"contactgroup:api_list_objects": 14,
"contactgroup:list_objects_with_permission": 20,
"contactrole:api_list_objects": 13,
"contactrole:list_objects_with_permission": 20,
"tenant:api_list_objects": 14,
"tenant:list_objects_with_permission": 21,
"tenantgroup:api_list_objects": 14,
"tenantgroup:list_objects_with_permission": 20
"contact:api_list_objects": 13,
"contact:list_objects_with_permission": 18,
"contactassignment:api_list_objects": 16,
"contactassignment:list_objects_with_permission": 22,
"contactgroup:api_list_objects": 13,
"contactgroup:list_objects_with_permission": 17,
"contactrole:api_list_objects": 12,
"contactrole:list_objects_with_permission": 17,
"tenant:api_list_objects": 13,
"tenant:list_objects_with_permission": 18,
"tenantgroup:api_list_objects": 13,
"tenantgroup:list_objects_with_permission": 17
}

View File

@ -1,9 +1,14 @@
import json
import logging
from django.test import tag
from django.urls import reverse
from rest_framework import status
from dcim.models import Site
from tenancy.choices import *
from tenancy.models import *
from utilities.testing import APITestCase, APIViewTestCases
from utilities.testing import APITestCase, APIViewTestCases, disable_logging
class AppTestCase(APITestCase):
@ -60,6 +65,52 @@ class TenantGroupTestCase(APIViewTestCases.APIViewTestCase):
},
]
@tag('regression') # Ref: #22821
def test_delete_tenant_group_with_conflicting_tenants(self):
"""
Attempt and fail to delete a tenant group whose tenants cannot be ungrouped.
"""
group = TenantGroup.objects.create(name='Tenant Group 7', slug='tenant-group-7')
Tenant.objects.create(name='Tenant 1', slug='tenant-1a', group=group)
Tenant.objects.create(name='Tenant 1', slug='tenant-1b')
self.add_permissions('tenancy.delete_tenantgroup')
url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': group.pk})
with disable_logging(level=logging.WARNING):
response = self.client.delete(url, **self.header)
self.assertHttpStatus(response, status.HTTP_409_CONFLICT)
content = json.loads(response.content.decode('utf-8'))
self.assertIn('detail', content)
self.assertTrue(content['detail'].startswith('Unable to delete object.'))
self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
@tag('regression') # Ref: #22821
def test_bulk_delete_tenant_groups_with_conflicting_tenants(self):
"""
Attempt and fail to bulk delete two tenant groups whose tenants conflict only once both are
ungrouped, leaving every group and assignment intact.
"""
group1 = TenantGroup.objects.create(name='Tenant Group 8', slug='tenant-group-8')
group2 = TenantGroup.objects.create(name='Tenant Group 9', slug='tenant-group-9')
tenant1 = Tenant.objects.create(name='Tenant 2', slug='tenant-2a', group=group1)
tenant2 = Tenant.objects.create(name='Tenant 2', slug='tenant-2b', group=group2)
self.add_permissions('tenancy.delete_tenantgroup')
data = [{'id': group1.pk}, {'id': group2.pk}]
with disable_logging(level=logging.WARNING):
response = self.client.delete(self._get_list_url(), data, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_409_CONFLICT)
# The rolled back batch must not leave the first group deleted
self.assertEqual(TenantGroup.objects.filter(pk__in=(group1.pk, group2.pk)).count(), 2)
tenant1.refresh_from_db()
tenant2.refresh_from_db()
self.assertEqual(tenant1.group, group1)
self.assertEqual(tenant2.group, group2)
class TenantTestCase(APIViewTestCases.APIViewTestCase):
model = Tenant

View File

@ -1,6 +1,104 @@
from django.test import TestCase
from django.db.models import ProtectedError
from django.test import TestCase, tag
from tenancy.models import Contact, ContactGroup
from tenancy.models import Contact, ContactGroup, Tenant, TenantGroup
class TenantGroupTestCase(TestCase):
@tag('regression') # Ref: #22821
def test_tenantgroup_deletion_blocked_by_duplicate_ungrouped_name(self):
"""
Deleting a tenant group must raise ProtectedError when ungrouping its tenant would duplicate
the name of an already ungrouped tenant.
"""
group = TenantGroup.objects.create(name='Tenant Group 1', slug='tenant-group-1')
tenant1 = Tenant.objects.create(name='Tenant 1', slug='tenant-1a', group=group)
tenant2 = Tenant.objects.create(name='Tenant 1', slug='tenant-1b')
with self.assertRaises(ProtectedError) as cm:
group.delete()
self.assertEqual(
cm.exception.args[0],
'Unable to delete tenant group Tenant Group 1. Ungrouping its tenants, including those of any nested '
'groups, would create duplicate tenant names or slugs.'
)
self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
# The failed deletion must leave the group and its tenant assignment intact
self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
tenant1.refresh_from_db()
self.assertEqual(tenant1.group, group)
@tag('regression') # Ref: #22821
def test_tenantgroup_deletion_blocked_by_duplicate_ungrouped_slug(self):
"""
Deleting a tenant group must raise ProtectedError for a slug collision alone, when the
colliding tenants have differing names.
"""
group = TenantGroup.objects.create(name='Tenant Group 2', slug='tenant-group-2')
tenant1 = Tenant.objects.create(name='Tenant 2', slug='duplicate-slug', group=group)
tenant2 = Tenant.objects.create(name='Tenant 3', slug='duplicate-slug')
with self.assertRaises(ProtectedError) as cm:
group.delete()
self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
@tag('regression') # Ref: #22821
def test_tenantgroup_deletion_blocked_by_duplicate_name_in_descendants(self):
"""
Deleting a parent tenant group must raise ProtectedError when ungrouping the tenants of its
descendant groups would duplicate a name.
"""
parent = TenantGroup.objects.create(name='Parent Group', slug='parent-group')
child1 = TenantGroup.objects.create(name='Child Group 1', slug='child-group-1', parent=parent)
child2 = TenantGroup.objects.create(name='Child Group 2', slug='child-group-2', parent=parent)
tenant1 = Tenant.objects.create(name='Tenant 4', slug='tenant-4a', group=child1)
tenant2 = Tenant.objects.create(name='Tenant 4', slug='tenant-4b', group=child2)
with self.assertRaises(ProtectedError) as cm:
parent.delete()
self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
self.assertTrue(TenantGroup.objects.filter(pk=parent.pk).exists())
self.assertEqual(TenantGroup.objects.filter(pk__in=(child1.pk, child2.pk)).count(), 2)
def test_tenantgroup_deletion_ungroups_tenants(self):
"""
Deleting a tenant group whose tenants can be ungrouped safely must succeed and clear the group
assignment across the whole subtree.
"""
parent = TenantGroup.objects.create(name='Parent Group', slug='parent-group')
child = TenantGroup.objects.create(name='Child Group', slug='child-group', parent=parent)
tenant1 = Tenant.objects.create(name='Tenant 5', slug='tenant-5', group=parent)
tenant2 = Tenant.objects.create(name='Tenant 6', slug='tenant-6', group=child)
parent.delete()
self.assertFalse(TenantGroup.objects.filter(pk__in=(parent.pk, child.pk)).exists())
tenant1.refresh_from_db()
tenant2.refresh_from_db()
self.assertIsNone(tenant1.group)
self.assertIsNone(tenant2.group)
def test_tenantgroup_deletion_ignores_tenants_in_unrelated_groups(self):
"""
Deleting a tenant group must succeed when a tenant outside its subtree shares a name and slug
with one of its own tenants, as that tenant remains grouped.
"""
group = TenantGroup.objects.create(name='Tenant Group 3', slug='tenant-group-3')
unrelated = TenantGroup.objects.create(name='Unrelated Group', slug='unrelated-group')
tenant = Tenant.objects.create(name='Tenant 7', slug='tenant-7', group=group)
Tenant.objects.create(name='Tenant 7', slug='tenant-7', group=unrelated)
group.delete()
self.assertFalse(TenantGroup.objects.filter(pk=group.pk).exists())
tenant.refresh_from_db()
self.assertIsNone(tenant.group)
class ContactGroupTestCase(TestCase):

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -1,14 +1,14 @@
{
"group:api_list_objects": 10,
"group:list_objects_with_permission": 17,
"group:list_objects_with_permission": 16,
"objectpermission:api_list_objects": 14,
"objectpermission:list_objects_with_permission": 18,
"objectpermission:list_objects_with_permission": 17,
"owner:api_list_objects": 11,
"owner:list_objects_with_permission": 19,
"owner:list_objects_with_permission": 18,
"ownergroup:api_list_objects": 9,
"ownergroup:list_objects_with_permission": 17,
"ownergroup:list_objects_with_permission": 16,
"token:api_list_objects": 10,
"token:list_objects_with_permission": 17,
"user:api_list_objects": 12,
"user:list_objects_with_permission": 17
"user:list_objects_with_permission": 16
}

View File

@ -3,7 +3,10 @@ import os
import re
from django.apps import apps
from django.core.exceptions import ValidationError
from django.utils.translation import gettext_lazy as _
from jinja2 import BaseLoader, TemplateNotFound
from jinja2.exceptions import TemplateSyntaxError
from jinja2.meta import find_referenced_templates
from jinja2.sandbox import SandboxedEnvironment
@ -12,16 +15,23 @@ from netbox.registry import registry
__all__ = (
'DEFAULT_JINJA2_FILTERS',
'HTTP_HEADER_INVALID_CHARS_RE',
'JINJA2_TEMPLATE_RE',
'DataFileLoader',
'env_filter',
'render_jinja2',
'sanitize_http_header',
'validate_jinja2_syntax',
)
# Control characters (C0 range plus DEL) which are invalid in an HTTP header value. Notably, this includes the
# carriage return and line feed characters used to smuggle additional headers (CR/LF injection).
HTTP_HEADER_INVALID_CHARS_RE = re.compile(r'[\x00-\x1f\x7f]')
# Matches the start of a Jinja2 expression, statement, or comment ({{, {%, {#), to detect whether a
# template-capable field (e.g. Webhook.payload_url) is being used as a literal value or a template.
JINJA2_TEMPLATE_RE = re.compile(r'\{[{%#]')
def env_filter(name):
"""
@ -87,6 +97,23 @@ class DataFileLoader(BaseLoader):
# Utility functions
#
def _jinja2_filters(filters=None):
"""
Build the Jinja2 filter table: default < plugin-registered < instance JINJA_FILTERS < filters
passed for this call, in increasing precedence. Instance-level config wins over
plugin-registered filters so site admins can override anything. Filters passed for this call
take precedence over all of them, so that context-specific (e.g. sanitization) filters cannot
be shadowed. Shared by render_jinja2() and validate_jinja2_syntax() so both see an identical
filter table.
"""
return {
**DEFAULT_JINJA2_FILTERS,
**registry['plugins'].get('jinja_filters', {}),
**get_config().JINJA_FILTERS,
**(filters or {}),
}
def render_jinja2(template_code, context, environment_params=None, data_file=None, debug=False, filters=None):
"""
Render a Jinja2 template with the provided context. Return the rendered content.
@ -115,21 +142,25 @@ def render_jinja2(template_code, context, environment_params=None, data_file=Non
environment_params['loader'] = loader
environment = SandboxedEnvironment(**environment_params)
# Build filter table: default < plugin-registered < instance JINJA_FILTERS < per-render filters.
# Instance-level config wins over plugin-registered filters so site admins can override anything.
# Filters passed for this render take precedence over all of them, so that context-specific
# (e.g. sanitization) filters cannot be shadowed.
all_filters = {
**DEFAULT_JINJA2_FILTERS,
**registry['plugins'].get('jinja_filters', {}),
**get_config().JINJA_FILTERS,
**(filters or {}),
}
environment.filters.update(all_filters)
environment.filters.update(_jinja2_filters(filters))
if data_file:
template = environment.get_template(data_file.path)
else:
template = environment.from_string(source=template_code)
return template.render(**context)
def validate_jinja2_syntax(template_code, filters=None):
"""
Validate that template_code is syntactically well-formed Jinja2 -- including that any filters
it references are registered -- without rendering it, so no context data is required. Pass the
same `filters` used at render time (see render_jinja2()) for an identical filter table. Raises
django.core.exceptions.ValidationError on failure.
"""
environment = SandboxedEnvironment(loader=BaseLoader())
environment.filters.update(_jinja2_filters(filters))
try:
environment.compile(template_code)
except TemplateSyntaxError as e:
raise ValidationError(_("Invalid template: {error}").format(error=e))

Some files were not shown because too many files have changed in this diff Show More