From 50ad58f876b002d7ac6c5fbdbd2643b56d284454 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 7 Aug 2025 19:28:41 +0200 Subject: [PATCH 01/35] feat(models): Add generic source/destination fields for ACL rules Refactors ACLStandardRule and ACLExtendedRule to support generic source and destination fields. Introduces GenericForeignKey and related caching for improved flexibility and performance. BREAKING CHANGE: Updates source/destination field structures; existing ACL data require migration. --- netbox_acls/constants.py | 25 ++ netbox_acls/models/access_list_rules.py | 370 +++++++++++++++++++++--- 2 files changed, 361 insertions(+), 34 deletions(-) diff --git a/netbox_acls/constants.py b/netbox_acls/constants.py index 468845c2..07b9ea00 100644 --- a/netbox_acls/constants.py +++ b/netbox_acls/constants.py @@ -1,14 +1,39 @@ """ Constants for filters """ + from django.db.models import Q +# +# AccessList +# + ACL_HOST_ASSIGNMENT_MODELS = Q( Q(app_label="dcim", model="device") | Q(app_label="dcim", model="virtualchassis") | Q(app_label="virtualization", model="virtualmachine"), ) +# +# ACLInterfaceAssignment +# + ACL_INTERFACE_ASSIGNMENT_MODELS = Q( Q(app_label="dcim", model="interface") | Q(app_label="virtualization", model="vminterface"), ) + +# +# AccessList Rule +# + +ACL_RULE_SOURCE_DESTINATION_MODELS = Q( + Q( + app_label="ipam", + model__in=( + "aggregate", + "ipaddress", + "iprange", + "prefix", + ), + ) +) diff --git a/netbox_acls/models/access_list_rules.py b/netbox_acls/models/access_list_rules.py index c8a46c7b..d7b7b92a 100644 --- a/netbox_acls/models/access_list_rules.py +++ b/netbox_acls/models/access_list_rules.py @@ -2,14 +2,18 @@ Define the django models for this plugin. """ +from django.apps import apps +from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.postgres.fields import ArrayField from django.core.exceptions import ValidationError from django.db import models from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from ipam.models import Aggregate, IPAddress, IPRange, Prefix from netbox.models import NetBoxModel from ..choices import ACLProtocolChoices, ACLRuleActionChoices, ACLTypeChoices +from ..constants import ACL_RULE_SOURCE_DESTINATION_MODELS from .access_lists import AccessList __all__ = ( @@ -21,16 +25,14 @@ # Error message when the action is 'remark', but no remark is provided. ERROR_MESSAGE_NO_REMARK = _("When the action is 'remark', a remark is required.") -# Error message when the action is 'remark', but the source_prefix is set. -ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET = _("When the action is 'remark', the Source Prefix must not be set.") +# Error message when the action is 'remark', but the source is set. +ERROR_MESSAGE_ACTION_REMARK_SOURCE_SET = _("When the action is 'remark', the Source must not be set.") # Error message when the action is 'remark', but the source_ports are set. ERROR_MESSAGE_ACTION_REMARK_SOURCE_PORTS_SET = _("When the action is 'remark', Source Ports must not be set.") -# Error message when the action is 'remark', but the destination_prefix is set. -ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PREFIX_SET = _( - "When the action is 'remark', the Destination Prefix must not be set." -) +# Error message when the action is 'remark', but the destination is set. +ERROR_MESSAGE_ACTION_REMARK_DESTINATION_SET = _("When the action is 'remark', the Destination must not be set.") # Error message when the action is 'remark', but the destination_ports are set. ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PORTS_SET = _("When the action is 'remark', Destination Ports must not be set.") @@ -54,12 +56,9 @@ class ACLRule(NetBoxModel): related_name="rules", verbose_name=_("Access List"), ) + + # Rule index = models.PositiveIntegerField() - remark = models.CharField( - verbose_name=_("Remark"), - max_length=500, - blank=True, - ) description = models.CharField( verbose_name=_("Description"), max_length=500, @@ -70,17 +69,75 @@ class ACLRule(NetBoxModel): max_length=30, choices=ACLRuleActionChoices, ) - source_prefix = models.ForeignKey( - to="ipam.prefix", + + # Remark + remark = models.CharField( + verbose_name=_("Remark"), + max_length=500, + blank=True, + ) + + # Source + source_type = models.ForeignKey( + to="contenttypes.ContentType", on_delete=models.PROTECT, related_name="+", + limit_choices_to=ACL_RULE_SOURCE_DESTINATION_MODELS, + verbose_name=_("Source Type"), + blank=True, + null=True, + ) + source_id = models.PositiveBigIntegerField( + verbose_name=_("Source ID"), + blank=True, + null=True, + ) + source = GenericForeignKey( + ct_field="source_type", + fk_field="source_id", + ) + + # Cached related objects by association name for faster access + _source_aggregate = models.ForeignKey( + to="ipam.aggregate", + on_delete=models.PROTECT, + related_name="_%(class)s_sources", + verbose_name=_("Source Aggregate"), + blank=True, + null=True, + ) + _source_ipaddress = models.ForeignKey( + to="ipam.ipaddress", + on_delete=models.PROTECT, + related_name="_%(class)s_sources", + verbose_name=_("Source IP-Address"), + blank=True, + null=True, + ) + _source_iprange = models.ForeignKey( + to="ipam.iprange", + on_delete=models.PROTECT, + related_name="_%(class)s_sources", + verbose_name=_("Source IP-Range"), + blank=True, + null=True, + ) + _source_prefix = models.ForeignKey( + to="ipam.prefix", + on_delete=models.PROTECT, + related_name="_%(class)s_sources", verbose_name=_("Source Prefix"), blank=True, null=True, ) - clone_fields = ("access_list", "action", "source_prefix") - prerequisite_models = ("netbox_acls.AccessList",) + clone_fields = ( + "access_list", + "action", + "source_id", + "source_type", + ) + prerequisite_models: tuple = ("netbox_acls.AccessList",) class Meta: """ @@ -91,12 +148,56 @@ class Meta: """ abstract = True + indexes = (models.Index(fields=("source_type", "source_id")),) ordering = ["access_list", "index"] unique_together = ["access_list", "index"] def __str__(self): return f"{self.access_list}: Rule {self.index}" + def clean(self): + """ + Override the model's clean method for custom field validation. + """ + # Validate source assignment + if self.source_type and not (self.source or self.source_id): + source_type = self.source_type.model_class() + raise ValidationError( + { + "source": _("Please select a source {source_type}.").format( + source_type=source_type._meta.verbose_name + ) + } + ) + super().clean() + + def save(self, *args, **kwargs): + """ + Saves the current instance to the database. + """ + # Cache the related source objects for faster access + self.cache_related_source_object() + + super().save(*args, **kwargs) + + def cache_related_source_object(self): + """ + Cache the related source objects for faster access. + """ + self._source_aggregate = self._source_ipaddress = self._source_iprange = self._source_prefix = None + if self.source_type: + source_type = self.source_type.model_class() + if source_type == apps.get_model("ipam", "aggregate"): + self._source_aggregate = self.source + elif source_type == apps.get_model("ipam", "ipaddress"): + self._source_ipaddress = self.source + elif source_type == apps.get_model("ipam", "iprange"): + self._source_iprange = self.source + elif source_type == apps.get_model("ipam", "prefix"): + self._source_prefix = self.source + + cache_related_source_object.alters_data = True + def get_absolute_url(self): """ The method is a Django convention; although not strictly required, @@ -140,7 +241,7 @@ def clean(self): Validate the ACL Standard Rule inputs. If the action is 'remark', then the remark field must be provided (non-empty), - and the source_prefix field must be empty. + and the source field must be empty. Conversely, if the remark field is provided, the action must be set to 'remark'. """ @@ -151,8 +252,8 @@ def clean(self): if self.action == ACLRuleActionChoices.ACTION_REMARK: if not self.remark: errors["remark"] = ERROR_MESSAGE_NO_REMARK - if self.source_prefix: - errors["source_prefix"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET + if self.source: + errors["source"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_SET # Validate that the action is "remark", when the remark field is provided elif self.remark: errors["remark"] = ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK @@ -164,7 +265,8 @@ def clean(self): class ACLExtendedRule(ACLRule): """ Inherits ACLRule. - Add ACLExtendedRule specific fields: source_ports, destination_prefix, destination_ports, and protocol + + Add ACLExtendedRule specific fields: source_ports, destination, destination_ports, and protocol """ access_list = models.ForeignKey( @@ -174,39 +276,91 @@ class ACLExtendedRule(ACLRule): limit_choices_to={"type": "extended"}, verbose_name=_("Extended Access List"), ) + + # Protocol + protocol = models.CharField( + verbose_name=_("Protocol"), + max_length=30, + choices=ACLProtocolChoices, + blank=True, + ) + + # Source source_ports = ArrayField( base_field=models.PositiveIntegerField(), verbose_name=_("Source Ports"), blank=True, null=True, ) - destination_prefix = models.ForeignKey( - to="ipam.prefix", + + # Destination + destination_type = models.ForeignKey( + to="contenttypes.ContentType", on_delete=models.PROTECT, related_name="+", - verbose_name=_("Destination Prefix"), + limit_choices_to=ACL_RULE_SOURCE_DESTINATION_MODELS, + verbose_name=_("Destination Type"), + blank=True, + null=True, + ) + destination_id = models.PositiveBigIntegerField( + verbose_name=_("Destination ID"), blank=True, null=True, ) + destination = GenericForeignKey( + ct_field="destination_type", + fk_field="destination_id", + ) destination_ports = ArrayField( base_field=models.PositiveIntegerField(), verbose_name=_("Destination Ports"), blank=True, null=True, ) - protocol = models.CharField( - verbose_name=_("Protocol"), - max_length=30, - choices=ACLProtocolChoices, + + # Cached related objects by association name for faster access + _destination_aggregate = models.ForeignKey( + to="ipam.aggregate", + on_delete=models.PROTECT, + related_name="_%(class)s_destinations", + verbose_name=_("Destination Aggregate"), + blank=True, + null=True, + ) + _destination_ipaddress = models.ForeignKey( + to="ipam.ipaddress", + on_delete=models.PROTECT, + related_name="_%(class)s_destinations", + verbose_name=_("Destination IP-Address"), blank=True, + null=True, + ) + _destination_iprange = models.ForeignKey( + to="ipam.iprange", + on_delete=models.PROTECT, + related_name="_%(class)s_destinations", + verbose_name=_("Destination IP-Range"), + blank=True, + null=True, + ) + _destination_prefix = models.ForeignKey( + to="ipam.prefix", + on_delete=models.PROTECT, + related_name="_%(class)s_destinations", + verbose_name=_("Destination Prefix"), + blank=True, + null=True, ) clone_fields = ( "access_list", "action", - "source_prefix", + "source_id", + "source_type", "source_ports", - "destination_prefix", + "destination_id", + "destination_type", "destination_ports", "protocol", ) @@ -221,6 +375,7 @@ class Meta(ACLRule.Meta): verbose_name = _("ACL Extended Rule") verbose_name_plural = _("ACL Extended Rules") + indexes = (models.Index(fields=("destination_type", "destination_id", "source_type", "source_id")),) def clean(self): """ @@ -228,27 +383,39 @@ def clean(self): When the action is 'remark', the remark field must be provided (non-empty), and the following fields must be empty: - - source_prefix + - source - source_ports - - destination_prefix + - destination - destination_ports - protocol Conversely, if a remark is provided, the action must be set to 'remark'. """ + # Validate destination assignment + if self.destination_type and not (self.destination or self.destination_id): + destination_type = self.destination_type.model_class() + raise ValidationError( + { + "destination": _("Please select a destination {destination_type}.").format( + destination_type=destination_type._meta.verbose_name, + ), + }, + ) + super().clean() + errors = {} # Validate that only the remark field is filled if self.action == ACLRuleActionChoices.ACTION_REMARK: if not self.remark: errors["remark"] = ERROR_MESSAGE_NO_REMARK - if self.source_prefix: - errors["source_prefix"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET + if self.source: + errors["source"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_SET if self.source_ports: errors["source_ports"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PORTS_SET - if self.destination_prefix: - errors["destination_prefix"] = ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PREFIX_SET + if self.destination: + errors["destination"] = ERROR_MESSAGE_ACTION_REMARK_DESTINATION_SET if self.destination_ports: errors["destination_ports"] = ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PORTS_SET if self.protocol: @@ -260,5 +427,140 @@ def clean(self): if errors: raise ValidationError(errors) + def save(self, *args, **kwargs): + """ + Saves the current instance to the database. + """ + # Cache the related destination objects for faster access + self.cache_related_destination_objects() + + super().save(*args, **kwargs) + + def cache_related_destination_objects(self): + """ + Cache the related destination objects for faster access. + """ + self._destination_aggregate = self._destination_ipaddress = self._destination_iprange = ( + self._destination_prefix + ) = None + if self.destination_type: + destination_type = self.destination_type.model_class() + if destination_type == apps.get_model("ipam", "aggregate"): + self._destination_aggregate = self.destination + elif destination_type == apps.get_model("ipam", "ipaddress"): + self._destination_ipaddress = self.destination + elif destination_type == apps.get_model("ipam", "iprange"): + self._destination_iprange = self.destination + elif destination_type == apps.get_model("ipam", "prefix"): + self._destination_prefix = self.destination + + cache_related_destination_objects.alters_data = True + def get_protocol_color(self): return ACLProtocolChoices.colors.get(self.protocol) + + +# +# Generic Relations: ACLStandardRule +# + +# Source Aggregate +GenericRelation( + to=ACLStandardRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_aggregate", +).contribute_to_class(Aggregate, "accesslist_standard_rule_sources") + +# Source IPAddress +GenericRelation( + to=ACLStandardRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_ip_address", +).contribute_to_class(IPAddress, "accesslist_standard_rule_sources") + +# Source IPRange +GenericRelation( + to=ACLStandardRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_ip_range", +).contribute_to_class(IPRange, "accesslist_standard_rule_sources") + +# Source Prefix +GenericRelation( + to=ACLStandardRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_prefix", +).contribute_to_class(Prefix, "accesslist_standard_rule_sources") + + +# +# Generic Relations: ACLExtendedRule +# + +# Source Aggregate +GenericRelation( + to=ACLExtendedRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_aggregate", +).contribute_to_class(Aggregate, "accesslist_extended_rule_sources") + +# Source IPAddress +GenericRelation( + to=ACLExtendedRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_ip_address", +).contribute_to_class(IPAddress, "accesslist_extended_rule_sources") + +# Source IPRange +GenericRelation( + to=ACLExtendedRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_ip_range", +).contribute_to_class(IPRange, "accesslist_extended_rule_sources") + +# Source Prefix +GenericRelation( + to=ACLExtendedRule, + content_type_field="source_type", + object_id_field="source_id", + related_query_name="source_prefix", +).contribute_to_class(Prefix, "accesslist_extended_rule_sources") + +# Destination Aggregate +GenericRelation( + to=ACLExtendedRule, + content_type_field="destination_type", + object_id_field="destination_id", + related_query_name="destination_aggregate", +).contribute_to_class(Aggregate, "accesslist_extended_rule_destinations") + +# Destination IPAddress +GenericRelation( + to=ACLExtendedRule, + content_type_field="destination_type", + object_id_field="destination_id", + related_query_name="destination_ip_address", +).contribute_to_class(IPAddress, "accesslist_extended_rule_destinations") + +# Destination IPRange +GenericRelation( + to=ACLExtendedRule, + content_type_field="destination_type", + object_id_field="destination_id", + related_query_name="destination_ip_range", +).contribute_to_class(IPRange, "accesslist_extended_rule_destinations") + +# Destination Prefix +GenericRelation( + to=ACLExtendedRule, + content_type_field="destination_type", + object_id_field="destination_id", + related_query_name="destination_prefix", +).contribute_to_class(Prefix, "accesslist_extended_rule_destinations") From a92f2e946ed4add39d79b148d2eeff1c1ea17540 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 7 Aug 2025 20:01:23 +0200 Subject: [PATCH 02/35] feat(migrations): Update ACL rule migrations for source/destination Renames and updates fields in ACLStandardRule and ACLExtendedRule to support generic source and destination objects. Introduces new fields, indices, and foreign key relationships for improved flexibility and query performance. BREAKING CHANGE: Requires migration of existing ACL data to the new field structure. --- ...acl_rule_source_and_destination_objects.py | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py diff --git a/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py new file mode 100644 index 00000000..76d57f36 --- /dev/null +++ b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py @@ -0,0 +1,229 @@ +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('extras', '0128_tableconfig'), + ('ipam', '0081_remove_service_device_virtual_machine_add_parent_gfk_index'), + ('netbox_acls', '0004_netbox_acls'), + ] + + operations = [ + migrations.RenameField( + model_name="aclextendedrule", + old_name="destination_prefix", + new_name="_destination_prefix", + ), + migrations.RenameField( + model_name="aclextendedrule", + old_name="source_prefix", + new_name="_source_prefix", + ), + migrations.RenameField( + model_name="aclstandardrule", + old_name="source_prefix", + new_name="_source_prefix", + ), + migrations.AddField( + model_name="aclextendedrule", + name="_destination_aggregate", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_destinations", + to="ipam.aggregate", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="_destination_ipaddress", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_destinations", + to="ipam.ipaddress", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="_destination_iprange", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_destinations", + to="ipam.iprange", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="_source_aggregate", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.aggregate", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="_source_ipaddress", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.ipaddress", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="_source_iprange", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.iprange", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="destination_id", + field=models.PositiveBigIntegerField(blank=True, null=True), + ), + migrations.AddField( + model_name="aclextendedrule", + name="destination_type", + field=models.ForeignKey( + blank=True, + limit_choices_to=models.Q( + models.Q(("app_label", "ipam"), ("model__in", ("aggregate", "ipaddress", "iprange", "prefix"))) + ), + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="contenttypes.contenttype", + ), + ), + migrations.AddField( + model_name="aclextendedrule", + name="source_id", + field=models.PositiveBigIntegerField(blank=True, null=True), + ), + migrations.AddField( + model_name="aclextendedrule", + name="source_type", + field=models.ForeignKey( + blank=True, + limit_choices_to=models.Q( + models.Q(("app_label", "ipam"), ("model__in", ("aggregate", "ipaddress", "iprange", "prefix"))) + ), + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="contenttypes.contenttype", + ), + ), + migrations.AddField( + model_name="aclstandardrule", + name="_source_aggregate", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.aggregate", + ), + ), + migrations.AddField( + model_name="aclstandardrule", + name="_source_ipaddress", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.ipaddress", + ), + ), + migrations.AddField( + model_name="aclstandardrule", + name="_source_iprange", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.iprange", + ), + ), + migrations.AddField( + model_name="aclstandardrule", + name="source_id", + field=models.PositiveBigIntegerField(blank=True, null=True), + ), + migrations.AddField( + model_name="aclstandardrule", + name="source_type", + field=models.ForeignKey( + blank=True, + limit_choices_to=models.Q( + models.Q(("app_label", "ipam"), ("model__in", ("aggregate", "ipaddress", "iprange", "prefix"))) + ), + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="contenttypes.contenttype", + ), + ), + migrations.AlterField( + model_name="aclextendedrule", + name="_destination_prefix", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_destinations", + to="ipam.prefix", + ), + ), + migrations.AlterField( + model_name="aclextendedrule", + name="_source_prefix", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.prefix", + ), + ), + migrations.AlterField( + model_name="aclstandardrule", + name="_source_prefix", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="_%(class)s_sources", + to="ipam.prefix", + ), + ), + migrations.AddIndex( + model_name="aclextendedrule", + index=models.Index( + fields=["destination_type", "destination_id", "source_type", "source_id"], + name="netbox_acls_destina_8f93b4_idx", + ), + ), + migrations.AddIndex( + model_name="aclstandardrule", + index=models.Index(fields=["source_type", "source_id"], name="netbox_acls_source__01d2fa_idx"), + ), + ] From d877eb44f65425a549782d7d9bb5aef43b2b0451 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 8 Aug 2025 18:35:48 +0200 Subject: [PATCH 03/35] feat(migrations): Add prefix assignment migration for ACL rules Introduces a data migration to copy source and destination prefix IDs to the newly added GenericForeignKey fields in ACLStandardRule and ACLExtendedRule. Ensures existing prefix assignments are preserved during schema updates. --- ...acl_rule_source_and_destination_objects.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py index 76d57f36..bb379b0e 100644 --- a/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py +++ b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py @@ -2,6 +2,28 @@ from django.db import migrations, models +def copy_prefix_assignments(apps, schema_editor): + """ + Copy Source and Destination Prefix ForeignKey IDs to the GenericForeignKey + fields. + """ + ContentType = apps.get_model("contenttypes", "ContentType") + Prefix = apps.get_model("ipam", "Prefix") + ACLStandardRule = apps.get_model("netbox_acls", "ACLStandardRule") + ACLExtendedRule = apps.get_model("netbox_acls", "ACLExtendedRule") + + ACLStandardRule.objects.filter(_source_prefix__isnull=False).update( + source_type=ContentType.objects.get_for_model(Prefix), + source_id=models.F("_source_prefix_id"), + ) + ACLExtendedRule.objects.filter(_source_prefix__isnull=False).filter(_destination_prefix__isnull=False).update( + source_type=ContentType.objects.get_for_model(Prefix), + source_id=models.F("_source_prefix_id"), + destination_type=ContentType.objects.get_for_model(Prefix), + destination_id=models.F("_destination_prefix_id"), + ) + + class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), @@ -226,4 +248,6 @@ class Migration(migrations.Migration): model_name="aclstandardrule", index=models.Index(fields=["source_type", "source_id"], name="netbox_acls_source__01d2fa_idx"), ), + # Copy over existing Prefix assignments + migrations.RunPython(code=copy_prefix_assignments, reverse_code=migrations.RunPython.noop), ] From 7387f20ef7e3a162ab90d92ce0e232822e91ed5d Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 9 Aug 2025 17:22:15 +0200 Subject: [PATCH 04/35] feat(tests): Add generic object tests for ACL rules models Extend test coverage for ACLStandardRule and ACLExtendedRule to include validation of generic source and destination objects. Add scenarios for aggregates, IP addresses, IP ranges, and validation of invalid objects. Test updates ensure the robustness and accuracy of the new field structure. --- netbox_acls/tests/models/base.py | 45 ++- .../tests/models/test_extendedrules.py | 359 ++++++++++++++---- .../tests/models/test_standardrules.py | 108 +++++- 3 files changed, 426 insertions(+), 86 deletions(-) diff --git a/netbox_acls/tests/models/base.py b/netbox_acls/tests/models/base.py index 55d5d0b5..7034ab3c 100644 --- a/netbox_acls/tests/models/base.py +++ b/netbox_acls/tests/models/base.py @@ -7,7 +7,8 @@ VirtualChassis, ) from django.test import TestCase -from ipam.models import Prefix +from ipam.models import RIR, Aggregate, IPAddress, IPRange, Prefix +from netaddr import IPNetwork from virtualization.models import Cluster, ClusterType, VirtualMachine @@ -20,12 +21,18 @@ class BaseTestCase(TestCase): def setUpTestData(cls): """ Create base data to test using including - - 1 of each of the following: test site, manufacturer, device type + - 1 of each of the following: rir, site, manufacturer, device type, device role, cluster type, cluster, virtual chassis, and virtual machine - - 2 of each Device, prefix + - 2 of each device, aggregate, ip address, ip range, prefix, service """ + # RIR + rir = RIR.objects.create( + name="RIR 1", + slug="rir-1", + ) + # Sites site = Site.objects.create( name="Site 1", @@ -108,10 +115,38 @@ def setUpTestData(cls): cluster=cluster, ) + # Aggregate + cls.aggregate1 = Aggregate.objects.create( + prefix=IPNetwork("10.0.0.0/8"), + rir=rir, + ) + cls.aggregate2 = Aggregate.objects.create( + prefix=IPNetwork("172.16.0.0/12"), + rir=rir, + ) + + # IP Address + cls.ip_address1 = IPAddress.objects.create( + address=IPNetwork("10.0.0.1/24"), + ) + cls.ip_address2 = IPAddress.objects.create( + address=IPNetwork("10.0.0.2/24"), + ) + + # IP Range + cls.ip_range1 = IPRange.objects.create( + start_address=IPNetwork("10.0.1.1/24"), + end_address=IPNetwork("10.0.1.254/24"), + ) + cls.ip_range2 = IPRange.objects.create( + start_address=IPNetwork("10.0.2.1/24"), + end_address=IPNetwork("10.0.2.254/24"), + ) + # Prefix cls.prefix1 = Prefix.objects.create( - prefix="10.1.0.0/16", + prefix=IPNetwork("10.1.0.0/16"), ) cls.prefix2 = Prefix.objects.create( - prefix="10.2.0.0/16", + prefix=IPNetwork("10.2.0.0/16"), ) diff --git a/netbox_acls/tests/models/test_extendedrules.py b/netbox_acls/tests/models/test_extendedrules.py index b84813b1..9452b3fd 100644 --- a/netbox_acls/tests/models/test_extendedrules.py +++ b/netbox_acls/tests/models/test_extendedrules.py @@ -47,14 +47,14 @@ def test_acl_extended_rule_creation_success(self): index=10, action="permit", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description=( - "Created rule with any source prefix, any source port, " - "any destination prefix, any destination port, and any protocol." + "Created rule with any source, any source port, " + "any destination, any destination port, and any protocol." ), ) created_rule.full_clean() @@ -63,18 +63,108 @@ def test_acl_extended_rule_creation_success(self): self.assertEqual(created_rule.index, 10) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, None) + self.assertEqual(created_rule.source, None) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, None) + self.assertEqual(created_rule.destination, None) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, None) self.assertEqual( created_rule.description, - ( - "Created rule with any source prefix, any source port, " - "any destination prefix, any destination port, and any protocol." - ), + ("Created rule with any source, any source port, any destination, any destination port, and any protocol."), + ) + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_extended_rule_source_aggregate_creation_success(self): + """ + Test that ACLExtendedRule with source aggregate creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=20, + action="permit", + remark="", + source=self.aggregate1, + source_ports=None, + destination=None, + destination_ports=None, + protocol=None, + description="Created rule with source aggregate", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 20) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.aggregate1) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, None) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with source aggregate") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_extended_rule_source_ip_address_creation_success(self): + """ + Test that ACLExtendedRule with source ip address creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=30, + action="permit", + remark="", + source=self.ip_address1, + source_ports=None, + destination=None, + destination_ports=None, + protocol=None, + description="Created rule with source ip address", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 30) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.ip_address1) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, None) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with source ip address") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_extended_rule_source_ip_range_creation_success(self): + """ + Test that ACLExtendedRule with source ip range creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=40, + action="permit", + remark="", + source=self.ip_range1, + source_ports=None, + destination=None, + destination_ports=None, + protocol=None, + description="Created rule with source ip range", ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 40) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.ip_range1) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, None) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with source ip range") self.assertEqual(isinstance(created_rule.access_list, AccessList), True) self.assertEqual(created_rule.access_list.type, self.acl_type) @@ -84,12 +174,12 @@ def test_acl_extended_rule_source_prefix_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=20, + index=50, action="permit", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Created rule with source prefix", @@ -97,12 +187,12 @@ def test_acl_extended_rule_source_prefix_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 20) + self.assertEqual(created_rule.index, 50) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, self.prefix1) + self.assertEqual(created_rule.source, self.prefix1) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, None) + self.assertEqual(created_rule.destination, None) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, None) self.assertEqual(created_rule.description, "Created rule with source prefix") @@ -115,12 +205,12 @@ def test_acl_extended_rule_source_ports_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=30, + index=70, action="permit", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=[22, 443], - destination_prefix=None, + destination=None, destination_ports=None, protocol=self.protocol, description="Created rule with source ports", @@ -128,30 +218,123 @@ def test_acl_extended_rule_source_ports_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 30) + self.assertEqual(created_rule.index, 70) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, self.prefix1) + self.assertEqual(created_rule.source, self.prefix1) self.assertEqual(created_rule.source_ports, [22, 443]) - self.assertEqual(created_rule.destination_prefix, None) + self.assertEqual(created_rule.destination, None) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, self.protocol) self.assertEqual(created_rule.description, "Created rule with source ports") self.assertEqual(isinstance(created_rule.access_list, AccessList), True) self.assertEqual(created_rule.access_list.type, self.acl_type) + def test_acl_extended_rule_destination_aggregate_creation_success(self): + """ + Test that ACLExtendedRule with destination aggregate creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=80, + action="permit", + remark="", + source=None, + source_ports=None, + destination=self.aggregate1, + destination_ports=None, + protocol=None, + description="Created rule with destination aggregate", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 80) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, None) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, self.aggregate1) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with destination aggregate") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_extended_rule_destination_ip_address_creation_success(self): + """ + Test that ACLExtendedRule with destination ip address creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=90, + action="permit", + remark="", + source=None, + source_ports=None, + destination=self.ip_address1, + destination_ports=None, + protocol=None, + description="Created rule with destination ip address", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 90) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, None) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, self.ip_address1) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with destination ip address") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_extended_rule_destination_ip_range_creation_success(self): + """ + Test that ACLExtendedRule with destination ip range creation passes validation. + """ + created_rule = ACLExtendedRule( + access_list=self.extended_acl1, + index=100, + action="permit", + remark="", + source=None, + source_ports=None, + destination=self.ip_range1, + destination_ports=None, + protocol=None, + description="Created rule with destination ip range", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) + self.assertEqual(created_rule.index, 100) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, None) + self.assertEqual(created_rule.source_ports, None) + self.assertEqual(created_rule.destination, self.ip_range1) + self.assertEqual(created_rule.destination_ports, None) + self.assertEqual(created_rule.protocol, None) + self.assertEqual(created_rule.description, "Created rule with destination ip range") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + def test_acl_extended_rule_destination_prefix_creation_success(self): """ Test that ACLExtendedRule with destination prefix creation passes validation. """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=40, + index=110, action="permit", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=self.prefix1, + destination=self.prefix1, destination_ports=None, protocol=None, description="Created rule with destination prefix", @@ -159,12 +342,12 @@ def test_acl_extended_rule_destination_prefix_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 40) + self.assertEqual(created_rule.index, 110) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, None) + self.assertEqual(created_rule.source, None) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, self.prefix1) + self.assertEqual(created_rule.destination, self.prefix1) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, None) self.assertEqual(created_rule.description, "Created rule with destination prefix") @@ -177,12 +360,12 @@ def test_acl_extended_rule_destination_ports_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=50, + index=130, action="permit", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=self.prefix1, + destination=self.prefix1, destination_ports=[22, 443], protocol=self.protocol, description="Created rule with destination ports", @@ -190,12 +373,12 @@ def test_acl_extended_rule_destination_ports_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 50) + self.assertEqual(created_rule.index, 130) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, None) + self.assertEqual(created_rule.source, None) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, self.prefix1) + self.assertEqual(created_rule.destination, self.prefix1) self.assertEqual(created_rule.destination_ports, [22, 443]) self.assertEqual(created_rule.protocol, self.protocol) self.assertEqual(created_rule.description, "Created rule with destination ports") @@ -208,12 +391,12 @@ def test_acl_extended_rule_icmp_protocol_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=60, + index=140, action="permit", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=None, - destination_prefix=self.prefix2, + destination=self.prefix2, destination_ports=None, protocol=ACLProtocolChoices.PROTOCOL_ICMP, description="Created rule with ICMP protocol", @@ -221,12 +404,12 @@ def test_acl_extended_rule_icmp_protocol_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 60) + self.assertEqual(created_rule.index, 140) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, self.prefix1) + self.assertEqual(created_rule.source, self.prefix1) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, self.prefix2) + self.assertEqual(created_rule.destination, self.prefix2) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, ACLProtocolChoices.PROTOCOL_ICMP) self.assertEqual(created_rule.description, "Created rule with ICMP protocol") @@ -239,12 +422,12 @@ def test_acl_extended_rule_complete_params_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=70, + index=150, action="permit", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=[4000, 5000], - destination_prefix=self.prefix2, + destination=self.prefix2, destination_ports=[22, 443], protocol=self.protocol, description="Created rule with complete parameters", @@ -252,12 +435,12 @@ def test_acl_extended_rule_complete_params_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 70) + self.assertEqual(created_rule.index, 150) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, self.prefix1) + self.assertEqual(created_rule.source, self.prefix1) self.assertEqual(created_rule.source_ports, [4000, 5000]) - self.assertEqual(created_rule.destination_prefix, self.prefix2) + self.assertEqual(created_rule.destination, self.prefix2) self.assertEqual(created_rule.destination_ports, [22, 443]) self.assertEqual(created_rule.protocol, self.protocol) self.assertEqual(created_rule.description, "Created rule with complete parameters") @@ -270,12 +453,12 @@ def test_acl_extended_rule_remark_creation_success(self): """ created_rule = ACLExtendedRule( access_list=self.extended_acl1, - index=80, + index=160, action="remark", remark="Test remark", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Created rule with remark", @@ -283,12 +466,12 @@ def test_acl_extended_rule_remark_creation_success(self): created_rule.full_clean() self.assertTrue(isinstance(created_rule, ACLExtendedRule), True) - self.assertEqual(created_rule.index, 80) + self.assertEqual(created_rule.index, 160) self.assertEqual(created_rule.action, "remark") self.assertEqual(created_rule.remark, "Test remark") - self.assertEqual(created_rule.source_prefix, None) + self.assertEqual(created_rule.source, None) self.assertEqual(created_rule.source_ports, None) - self.assertEqual(created_rule.destination_prefix, None) + self.assertEqual(created_rule.destination, None) self.assertEqual(created_rule.destination_ports, None) self.assertEqual(created_rule.protocol, None) self.assertEqual(created_rule.description, "Created rule with remark") @@ -308,12 +491,12 @@ def test_access_list_standard_to_acl_extended_rule_assignment_fail(self): ) extended_rule = ACLExtendedRule( access_list=standard_acl1, - index=80, + index=170, action="remark", remark="Test remark", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Created rule with remark", @@ -346,9 +529,9 @@ def test_acl_extended_rule_action_permit_with_remark_fail(self): index=10, action="permit", remark="Remark", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Invalid rule with action 'permit' and remark", @@ -365,9 +548,9 @@ def test_acl_extended_rule_action_remark_with_no_remark_fail(self): index=10, action="remark", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Invalid rule with action 'remark' and without remark", @@ -384,9 +567,9 @@ def test_acl_extended_rule_action_remark_with_source_prefix_fail(self): index=10, action="remark", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=None, description="Invalid rule with action 'remark' and source prefix", @@ -403,9 +586,9 @@ def test_acl_extended_rule_action_remark_with_source_ports_fail(self): index=10, action="remark", remark="", - source_prefix=self.prefix1, + source=self.prefix1, source_ports=[80, 443], - destination_prefix=None, + destination=None, destination_ports=None, protocol=ACLProtocolChoices.PROTOCOL_TCP, description="Invalid rule with action 'remark' and source ports", @@ -422,9 +605,9 @@ def test_acl_extended_rule_action_remark_with_destination_prefix_fail(self): index=10, action="remark", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=self.prefix1, + destination=self.prefix1, destination_ports=None, protocol=None, description="Invalid rule with action 'remark' and destination prefix", @@ -441,9 +624,9 @@ def test_acl_extended_rule_action_remark_with_destination_ports_fail(self): index=10, action="remark", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=self.prefix1, + destination=self.prefix1, destination_ports=[80, 443], protocol=ACLProtocolChoices.PROTOCOL_TCP, description="Invalid rule with action 'remark' and destination ports", @@ -460,9 +643,9 @@ def test_acl_extended_rule_action_remark_with_protocol_fail(self): index=10, action="remark", remark="", - source_prefix=None, + source=None, source_ports=None, - destination_prefix=None, + destination=None, destination_ports=None, protocol=ACLProtocolChoices.PROTOCOL_ICMP, description="Invalid rule with action 'remark' and ICMP protocol", @@ -470,6 +653,44 @@ def test_acl_extended_rule_action_remark_with_protocol_fail(self): with self.assertRaises(ValidationError): invalid_rule.full_clean() + def test_invalid_aci_extended_rule_source_object(self): + """ + Test ACLExtendedRule source object validation. + """ + invalid_acl_rule_source_object = ACLExtendedRule( + access_list=self.extended_acl1, + index=10, + action="permit", + remark="", + source=self.device1, + source_ports=None, + destination=None, + destination_ports=None, + protocol=ACLProtocolChoices.PROTOCOL_ICMP, + description="Rule with invalid source object.", + ) + with self.assertRaises(ValidationError): + invalid_acl_rule_source_object.full_clean() + + def test_invalid_aci_extended_rule_destination_object(self): + """ + Test ACLExtendedRule destination object validation. + """ + invalid_acl_rule_destination_object = ACLExtendedRule( + access_list=self.extended_acl1, + index=10, + action="permit", + remark="", + source=None, + source_ports=None, + destination=self.device1, + destination_ports=None, + protocol=ACLProtocolChoices.PROTOCOL_ICMP, + description="Rule with invalid destination object.", + ) + with self.assertRaises(ValidationError): + invalid_acl_rule_destination_object.full_clean() + def test_valid_acl_rule_action_choices(self): """ Test ACLExtendedRule action choices using VALID choices. diff --git a/netbox_acls/tests/models/test_standardrules.py b/netbox_acls/tests/models/test_standardrules.py index dcad4968..5bd8600e 100644 --- a/netbox_acls/tests/models/test_standardrules.py +++ b/netbox_acls/tests/models/test_standardrules.py @@ -46,8 +46,8 @@ def test_acl_standard_rule_creation_success(self): index=10, action="permit", remark="", - source_prefix=None, - description="Created rule with any source prefix", + source=None, + description="Created rule with any source", ) created_rule.full_clean() @@ -55,8 +55,8 @@ def test_acl_standard_rule_creation_success(self): self.assertEqual(created_rule.index, 10) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, None) - self.assertEqual(created_rule.description, "Created rule with any source prefix") + self.assertEqual(created_rule.source, None) + self.assertEqual(created_rule.description, "Created rule with any source") self.assertEqual(isinstance(created_rule.access_list, AccessList), True) self.assertEqual(created_rule.access_list.type, self.acl_type) @@ -69,7 +69,7 @@ def test_acl_standard_rule_source_prefix_creation_success(self): index=20, action="permit", remark="", - source_prefix=self.prefix1, + source=self.prefix1, description="Created rule with source prefix", ) created_rule.full_clean() @@ -78,7 +78,7 @@ def test_acl_standard_rule_source_prefix_creation_success(self): self.assertEqual(created_rule.index, 20) self.assertEqual(created_rule.action, "permit") self.assertEqual(created_rule.remark, "") - self.assertEqual(created_rule.source_prefix, self.prefix1) + self.assertEqual(created_rule.source, self.prefix1) self.assertEqual(created_rule.description, "Created rule with source prefix") self.assertEqual(isinstance(created_rule.access_list, AccessList), True) self.assertEqual(created_rule.access_list.type, self.acl_type) @@ -92,7 +92,7 @@ def test_acl_standard_rule_remark_creation_success(self): index=30, action="remark", remark="Test remark", - source_prefix=None, + source=None, description="Created rule with remark", ) created_rule.full_clean() @@ -101,11 +101,80 @@ def test_acl_standard_rule_remark_creation_success(self): self.assertEqual(created_rule.index, 30) self.assertEqual(created_rule.action, "remark") self.assertEqual(created_rule.remark, "Test remark") - self.assertEqual(created_rule.source_prefix, None) + self.assertEqual(created_rule.source, None) self.assertEqual(created_rule.description, "Created rule with remark") self.assertEqual(isinstance(created_rule.access_list, AccessList), True) self.assertEqual(created_rule.access_list.type, self.acl_type) + def test_acl_standard_rule_source_aggregate_creation_success(self): + """ + Test that ACLStandardRule with source aggregate creation passes validation. + """ + created_rule = ACLStandardRule( + access_list=self.standard_acl1, + index=40, + action="permit", + remark="", + source=self.aggregate1, + description="Created rule with source aggregate", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLStandardRule), True) + self.assertEqual(created_rule.index, 40) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.aggregate1) + self.assertEqual(created_rule.description, "Created rule with source aggregate") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_standard_rule_source_ip_address_creation_success(self): + """ + Test that ACLStandardRule with source ip address creation passes validation. + """ + created_rule = ACLStandardRule( + access_list=self.standard_acl1, + index=50, + action="permit", + remark="", + source=self.ip_address1, + description="Created rule with source ip address", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLStandardRule), True) + self.assertEqual(created_rule.index, 50) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.ip_address1) + self.assertEqual(created_rule.description, "Created rule with source ip address") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + + def test_acl_standard_rule_source_ip_range_creation_success(self): + """ + Test that ACLStandardRule with source ip range creation passes validation. + """ + created_rule = ACLStandardRule( + access_list=self.standard_acl1, + index=60, + action="permit", + remark="", + source=self.ip_range1, + description="Created rule with source ip range", + ) + created_rule.full_clean() + + self.assertTrue(isinstance(created_rule, ACLStandardRule), True) + self.assertEqual(created_rule.index, 60) + self.assertEqual(created_rule.action, "permit") + self.assertEqual(created_rule.remark, "") + self.assertEqual(created_rule.source, self.ip_range1) + self.assertEqual(created_rule.description, "Created rule with source ip range") + self.assertEqual(isinstance(created_rule.access_list, AccessList), True) + self.assertEqual(created_rule.access_list.type, self.acl_type) + def test_access_list_extended_to_acl_standard_rule_assignment_fail(self): """ Test that Extended Access List cannot be assigned to ACLStandardRule. @@ -122,7 +191,7 @@ def test_access_list_extended_to_acl_standard_rule_assignment_fail(self): index=30, action="remark", remark="Test remark", - source_prefix=None, + source=None, description="Created rule with remark", ) with self.assertRaises(ValidationError): @@ -153,7 +222,7 @@ def test_acl_standard_rule_action_permit_with_remark_fail(self): index=10, action="permit", remark="Remark", - source_prefix=None, + source=None, description="Invalid rule with action 'permit' and remark", ) with self.assertRaises(ValidationError): @@ -168,7 +237,7 @@ def test_acl_standard_rule_action_remark_with_no_remark_fail(self): index=10, action="remark", remark="", - source_prefix=None, + source=None, description="Invalid rule with action 'remark' and without remark", ) with self.assertRaises(ValidationError): @@ -183,12 +252,27 @@ def test_acl_standard_rule_action_remark_with_source_prefix_fail(self): index=10, action="remark", remark="", - source_prefix=self.prefix1, + source=self.prefix1, description="Invalid rule with action 'remark' and source prefix", ) with self.assertRaises(ValidationError): invalid_rule.full_clean() + def test_invalid_aci_standard_rule_source_object(self): + """ + Test ACLStandardRule source object validation. + """ + invalid_acl_rule_source_object = ACLStandardRule( + access_list=self.standard_acl1, + index=10, + action="permit", + remark="", + source=self.device1, + description="Rule with invalid source object", + ) + with self.assertRaises(ValidationError): + invalid_acl_rule_source_object.full_clean() + def test_valid_acl_rule_action_choices(self): """ Test ACLStandardRule action choices using VALID choices. From f2a1dc8399e8e01c3a58d0aea0277a278fb4afb6 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 9 Aug 2025 17:28:57 +0200 Subject: [PATCH 05/35] feat(tables): Add generic src and dst columns for ACL rules Introduces new columns for source and destination with support for generic objects in ACLStandardRule and ACLExtendedRule tables. Updates field and default column configurations for improved flexibility and alignment with the new data model. --- netbox_acls/tables.py | 46 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/netbox_acls/tables.py b/netbox_acls/tables.py index bc26a23e..d346a124 100644 --- a/netbox_acls/tables.py +++ b/netbox_acls/tables.py @@ -141,6 +141,16 @@ class ACLStandardRuleTable(NetBoxTable): url_name="plugins:netbox_acls:aclstandardrule_list", ) + # Source + source_type = columns.ContentTypeColumn( + verbose_name=_("Source Type"), + ) + source = tables.Column( + verbose_name=_("Source"), + orderable=False, + linkify=True, + ) + class Meta(NetBoxTable.Meta): model = ACLStandardRule fields = ( @@ -152,14 +162,14 @@ class Meta(NetBoxTable.Meta): "remark", "tags", "description", - "source_prefix", + "source", ) default_columns = ( "access_list", "index", "action", "remark", - "source_prefix", + "source", "tags", ) @@ -181,6 +191,26 @@ class ACLExtendedRuleTable(NetBoxTable): ) protocol = ChoiceFieldColumn() + # Source + source_type = columns.ContentTypeColumn( + verbose_name=_("Source Type"), + ) + source = tables.Column( + verbose_name=_("Source"), + orderable=False, + linkify=True, + ) + + # Destination + destination_type = columns.ContentTypeColumn( + verbose_name=_("Destination Type"), + ) + destination = tables.Column( + verbose_name=_("Destination"), + orderable=False, + linkify=True, + ) + class Meta(NetBoxTable.Meta): model = ACLExtendedRule fields = ( @@ -192,9 +222,9 @@ class Meta(NetBoxTable.Meta): "remark", "tags", "description", - "source_prefix", + "source", "source_ports", - "destination_prefix", + "destination", "destination_ports", "protocol", ) @@ -203,10 +233,10 @@ class Meta(NetBoxTable.Meta): "index", "action", "remark", - "tags", - "source_prefix", + "protocol", + "source", "source_ports", - "destination_prefix", + "destination", "destination_ports", - "protocol", + "tags", ) From 3fcafb8fa8ea390930c53a7036c3fe654852d510 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 10 Aug 2025 14:17:08 +0200 Subject: [PATCH 06/35] feat(filtersets): Extend filter support for ACL source/destination Adds support for filtering by aggregates, IP addresses, and IP ranges in both source and destination fields. Updates filter fields for better alignment with the extended ACL rules model. --- netbox_acls/filtersets.py | 149 ++++++++++++++++++++++++++++++++++---- 1 file changed, 135 insertions(+), 14 deletions(-) diff --git a/netbox_acls/filtersets.py b/netbox_acls/filtersets.py index 45b860dc..d1cd773f 100644 --- a/netbox_acls/filtersets.py +++ b/netbox_acls/filtersets.py @@ -7,7 +7,7 @@ from dcim.models import Device, Interface, Region, Site, SiteGroup, VirtualChassis from django.db.models import Q from django.utils.translation import gettext_lazy as _ -from ipam.models import Prefix +from ipam.models import Aggregate, IPAddress, IPRange, Prefix from netbox.filtersets import NetBoxModelFilterSet from virtualization.models import VirtualMachine, VMInterface @@ -203,14 +203,50 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): ) # Source + source_aggregate = django_filters.ModelMultipleChoiceFilter( + field_name="_source_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="name", + label=_("Source Aggregate (name)"), + ) + source_aggregate_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="id", + label=_("Source Aggregate (ID)"), + ) + source_ipaddress = django_filters.ModelMultipleChoiceFilter( + field_name="_source_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="name", + label=_("Source IP-Address (name)"), + ) + source_ipaddress_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="id", + label=_("Source IP-Address (ID)"), + ) + source_iprange = django_filters.ModelMultipleChoiceFilter( + field_name="_source_iprange", + queryset=IPRange.objects.all(), + to_field_name="name", + label=_("Source IP-Range (name)"), + ) + source_iprange_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_iprange", + queryset=IPRange.objects.all(), + to_field_name="id", + label=_("Source IP-Range (ID)"), + ) source_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="source_prefix", + field_name="_source_prefix", queryset=Prefix.objects.all(), to_field_name="name", label=_("Source Prefix (name)"), ) source_prefix_id = django_filters.ModelMultipleChoiceFilter( - field_name="source_prefix", + field_name="_source_prefix", queryset=Prefix.objects.all(), to_field_name="id", label=_("Source Prefix (ID)"), @@ -222,17 +258,20 @@ class Meta: """ model = ACLStandardRule - fields = ("id", "access_list", "index", "action") + fields = ( + "id", + "access_list", + "index", + "action", + "source_type", + "source_id", + ) def search(self, queryset, name, value): """ Override the default search behavior for the django model. """ - query = ( - Q(access_list__name__icontains=value) - | Q(index__icontains=value) - | Q(action__icontains=value) - ) + query = Q(access_list__name__icontains=value) | Q(index__icontains=value) | Q(action__icontains=value) return queryset.filter(query) @@ -254,28 +293,100 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): ) # Source + source_aggregate = django_filters.ModelMultipleChoiceFilter( + field_name="_source_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="name", + label=_("Source Aggregate (name)"), + ) + source_aggregate_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="id", + label=_("Source Aggregate (ID)"), + ) + source_ipaddress = django_filters.ModelMultipleChoiceFilter( + field_name="_source_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="name", + label=_("Source IP-Address (name)"), + ) + source_ipaddress_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="id", + label=_("Source IP-Address (ID)"), + ) + source_iprange = django_filters.ModelMultipleChoiceFilter( + field_name="_source_iprange", + queryset=IPRange.objects.all(), + to_field_name="name", + label=_("Source IP-Range (name)"), + ) + source_iprange_id = django_filters.ModelMultipleChoiceFilter( + field_name="_source_iprange", + queryset=IPRange.objects.all(), + to_field_name="id", + label=_("Source IP-Range (ID)"), + ) source_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="source_prefix", + field_name="_source_prefix", queryset=Prefix.objects.all(), to_field_name="name", label=_("Source Prefix (name)"), ) source_prefix_id = django_filters.ModelMultipleChoiceFilter( - field_name="source_prefix", + field_name="_source_prefix", queryset=Prefix.objects.all(), to_field_name="id", label=_("Source Prefix (ID)"), ) # Destination + destination_aggregate = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="name", + label=_("Destination Aggregate (name)"), + ) + destination_aggregate_id = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_aggregate", + queryset=Aggregate.objects.all(), + to_field_name="id", + label=_("Destination Aggregate (ID)"), + ) + destination_ipaddress = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="name", + label=_("Destination IP-Address (name)"), + ) + destination_ipaddress_id = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_ipaddress", + queryset=IPAddress.objects.all(), + to_field_name="id", + label=_("Destination IP-Address (ID)"), + ) + destination_iprange = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_iprange", + queryset=IPRange.objects.all(), + to_field_name="name", + label=_("Destination IP-Range (name)"), + ) + destination_iprange_id = django_filters.ModelMultipleChoiceFilter( + field_name="_destination_iprange", + queryset=IPRange.objects.all(), + to_field_name="id", + label=_("Destination IP-Range (ID)"), + ) destination_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="destination_prefix", + field_name="_destination_prefix", queryset=Prefix.objects.all(), to_field_name="name", label=_("Destination Prefix (name)"), ) destination_prefix_id = django_filters.ModelMultipleChoiceFilter( - field_name="destination_prefix", + field_name="_destination_prefix", queryset=Prefix.objects.all(), to_field_name="id", label=_("Destination Prefix (ID)"), @@ -287,7 +398,17 @@ class Meta: """ model = ACLExtendedRule - fields = ("id", "access_list", "index", "action", "protocol") + fields = ( + "id", + "access_list", + "index", + "action", + "source_type", + "source_id", + "destination_type", + "destination_id", + "protocol", + ) def search(self, queryset, name, value): """ From 6924365fd14ab0f850e881244831f5a28c35feeb Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 10 Aug 2025 15:03:37 +0200 Subject: [PATCH 07/35] feat(forms): Add generic source and destination fields for ACL rules Integrates generic object handling for source and destination fields in ACLStandardRuleForm and ACLExtendedRuleForm. Introduces support for content types, dynamic queries, and enhanced validation. Improves alignment with the generic source/destination model update. --- netbox_acls/forms/filtersets.py | 158 +++++++++++++++++--- netbox_acls/forms/models.py | 252 +++++++++++++++++++++++++++----- 2 files changed, 355 insertions(+), 55 deletions(-) diff --git a/netbox_acls/forms/filtersets.py b/netbox_acls/forms/filtersets.py index 6998144a..8e97264b 100644 --- a/netbox_acls/forms/filtersets.py +++ b/netbox_acls/forms/filtersets.py @@ -5,7 +5,7 @@ from dcim.models import Device, Interface, Region, Site, SiteGroup, VirtualChassis from django import forms from django.utils.translation import gettext_lazy as _ -from ipam.models import Prefix +from ipam.models import Aggregate, IPAddress, IPRange, Prefix from netbox.forms import NetBoxModelFilterSetForm from utilities.forms.fields import ( DynamicModelChoiceField, @@ -45,17 +45,38 @@ class AccessListFilterForm(NetBoxModelFilterSetForm): model = AccessList fieldsets = ( - FieldSet("q", "tag", name=None), - FieldSet("type", "default_action", name=_("ACL Details")), - FieldSet("region_id", "site_group_id", "site_id", "device_id", name=_("Device Details")), - FieldSet("virtual_chassis_id", name=_("Virtual Chassis Details")), - FieldSet("virtual_machine_id", name=_("Virtual Machine Details")), + FieldSet( + "q", + "tag", + name=None, + ), + FieldSet( + "type", + "default_action", + name=_("ACL Details"), + ), + FieldSet( + "region_id", + "site_group_id", + "site_id", + "device_id", + name=_("Device Details"), + ), + FieldSet( + "virtual_chassis_id", + name=_("Virtual Chassis Details"), + ), + FieldSet( + "virtual_machine_id", + name=_("Virtual Machine Details"), + ), ) - # ACL + # ACL selector type = forms.ChoiceField( choices=add_blank_choice(ACLTypeChoices), required=False, + label=_("Type"), ) default_action = forms.ChoiceField( choices=add_blank_choice(ACLActionChoices), @@ -119,10 +140,29 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm): model = ACLInterfaceAssignment fieldsets = ( - FieldSet("q", "tag", name=None), - FieldSet("access_list_id", "direction", name=_("ACL Details")), - FieldSet("region_id", "site_group_id", "site_id", "device_id", "interface_id", name=_("Device Details")), - FieldSet("virtual_machine_id", "vminterface_id", name=_("Virtual Machine Details")), + FieldSet( + "q", + "tag", + name=None, + ), + FieldSet( + "access_list_id", + "direction", + name=_("ACL Details"), + ), + FieldSet( + "region_id", + "site_group_id", + "site_id", + "device_id", + "interface_id", + name=_("Device Details"), + ), + FieldSet( + "virtual_machine_id", + "vminterface_id", + name=_("Virtual Machine Details"), + ), ) # ACL selector @@ -202,9 +242,24 @@ class ACLStandardRuleFilterForm(NetBoxModelFilterSetForm): model = ACLStandardRule fieldsets = ( - FieldSet("q", "tag", name=None), - FieldSet("access_list_id", "index", "action", name=_("ACL Details")), - FieldSet("source_prefix_id", name=_("Source Details")), + FieldSet( + "q", + "tag", + name=None, + ), + FieldSet( + "access_list_id", + "index", + "action", + name=_("ACL Details"), + ), + FieldSet( + "source_aggregate_id", + "source_ipaddress_id", + "source_iprange_id", + "source_prefix_id", + name=_("Source Details"), + ), ) access_list_id = DynamicModelMultipleChoiceField( @@ -226,6 +281,21 @@ class ACLStandardRuleFilterForm(NetBoxModelFilterSetForm): ) # Source selectors + source_aggregate_id = DynamicModelMultipleChoiceField( + queryset=Aggregate.objects.all(), + required=False, + label=_("Source Aggregate"), + ) + source_ipaddress_id = DynamicModelMultipleChoiceField( + queryset=IPAddress.objects.all(), + required=False, + label=_("Source IP-Address"), + ) + source_iprange_id = DynamicModelMultipleChoiceField( + queryset=IPRange.objects.all(), + required=False, + label=_("Source IP-Range"), + ) source_prefix_id = DynamicModelMultipleChoiceField( queryset=Prefix.objects.all(), required=False, @@ -243,10 +313,32 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): model = ACLExtendedRule fieldsets = ( - FieldSet("q", "tag", name=None), - FieldSet("access_list_id", "index", "action", "protocol", name=_("ACL Details")), - FieldSet("source_prefix_id", name=_("Source Details")), - FieldSet("destination_prefix_id", name=_("Destination Details")), + FieldSet( + "q", + "tag", + name=None, + ), + FieldSet( + "access_list_id", + "index", + "action", + "protocol", + name=_("ACL Details"), + ), + FieldSet( + "source_aggregate_id", + "source_ipaddress_id", + "source_iprange_id", + "source_prefix_id", + name=_("Source Details"), + ), + FieldSet( + "destination_aggregate_id", + "destination_ipaddress_id", + "destination_iprange_id", + "destination_prefix_id", + name=_("Destination Details"), + ), ) access_list_id = DynamicModelMultipleChoiceField( @@ -273,6 +365,21 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): ) # Source selectors + source_aggregate_id = DynamicModelMultipleChoiceField( + queryset=Aggregate.objects.all(), + required=False, + label=_("Source Aggregate"), + ) + source_ipaddress_id = DynamicModelMultipleChoiceField( + queryset=IPAddress.objects.all(), + required=False, + label=_("Source IP-Address"), + ) + source_iprange_id = DynamicModelMultipleChoiceField( + queryset=IPRange.objects.all(), + required=False, + label=_("Source IP-Range"), + ) source_prefix_id = DynamicModelMultipleChoiceField( queryset=Prefix.objects.all(), required=False, @@ -280,6 +387,21 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): ) # Destination selectors + destination_aggregate_id = DynamicModelMultipleChoiceField( + queryset=Aggregate.objects.all(), + required=False, + label=_("Destination Aggregate"), + ) + destination_ipaddress_id = DynamicModelMultipleChoiceField( + queryset=IPAddress.objects.all(), + required=False, + label=_("Destination IP-Address"), + ) + destination_iprange_id = DynamicModelMultipleChoiceField( + queryset=IPRange.objects.all(), + required=False, + label=_("Destination IP-Range"), + ) destination_prefix_id = DynamicModelMultipleChoiceField( queryset=Prefix.objects.all(), required=False, diff --git a/netbox_acls/forms/models.py b/netbox_acls/forms/models.py index 42450a70..6e16bc21 100644 --- a/netbox_acls/forms/models.py +++ b/netbox_acls/forms/models.py @@ -4,13 +4,22 @@ from dcim.models import Device, Interface, Region, Site, SiteGroup, VirtualChassis from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ from ipam.models import Prefix from netbox.forms import NetBoxModelForm -from utilities.forms.fields import CommentField, DynamicModelChoiceField +from utilities.forms import ( + get_field_value, +) +from utilities.forms.fields import ( + CommentField, + ContentTypeChoiceField, + DynamicModelChoiceField, +) from utilities.forms.rendering import FieldSet, TabbedGroups +from utilities.forms.widgets import HTMXSelect +from utilities.templatetags.builtins.filters import bettertitle from virtualization.models import ( Cluster, ClusterGroup, @@ -20,6 +29,7 @@ ) from ..choices import ACLTypeChoices +from ..constants import ACL_RULE_SOURCE_DESTINATION_MODELS from ..models import ( AccessList, ACLExtendedRule, @@ -225,7 +235,7 @@ def clean(self): "__all__": ( "Access Lists must be assigned to one host at a time. Either a device, virtual chassis or " "virtual machine." - ) + ), }, ) @@ -348,21 +358,6 @@ class ACLInterfaceAssignmentForm(NetBoxModelForm): ), ) - def __init__(self, *args, **kwargs): - # Initialize helper selectors - instance = kwargs.get("instance") - initial = kwargs.get("initial", {}).copy() - if instance: - if type(instance.assigned_object) is Interface: - initial["interface"] = instance.assigned_object - initial["device"] = "device" - elif type(instance.assigned_object) is VMInterface: - initial["vminterface"] = instance.assigned_object - initial["virtual_machine"] = "virtual_machine" - kwargs["initial"] = initial - - super().__init__(*args, **kwargs) - class Meta: model = ACLInterfaceAssignment fields = ( @@ -382,6 +377,21 @@ class Meta: ), } + def __init__(self, *args, **kwargs): + # Initialize helper selectors + instance = kwargs.get("instance") + initial = kwargs.get("initial", {}).copy() + if instance: + if type(instance.assigned_object) is Interface: + initial["interface"] = instance.assigned_object + initial["device"] = "device" + elif type(instance.assigned_object) is VMInterface: + initial["vminterface"] = instance.assigned_object + initial["virtual_machine"] = "virtual_machine" + kwargs["initial"] = initial + + super().__init__(*args, **kwargs) + def clean(self): """ Validates form inputs before submitting: @@ -495,11 +505,22 @@ class ACLStandardRuleForm(NetBoxModelForm): ), label="Access List", ) - source_prefix = DynamicModelChoiceField( - queryset=Prefix.objects.all(), + + # Source + source_type = ContentTypeChoiceField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), required=False, + widget=HTMXSelect(), + label=_("Source Type"), help_text=help_text_acl_rule_logic, - label="Source Prefix", + ) + source = DynamicModelChoiceField( + queryset=Prefix.objects.none(), # Initial queryset + selector=True, + required=False, + label=_("Source"), + help_text=help_text_acl_rule_logic, + disabled=True, ) fieldsets = ( @@ -512,10 +533,17 @@ class ACLStandardRuleForm(NetBoxModelForm): FieldSet( "index", "action", - "remark", - "source_prefix", name=_("Rule Definition"), ), + FieldSet( + "remark", + name=_("Remark"), + ), + FieldSet( + "source_type", + "source", + name=_("Source Definition"), + ), ) class Meta: @@ -525,7 +553,7 @@ class Meta: "index", "action", "remark", - "source_prefix", + "source_type", "tags", "description", ) @@ -534,10 +562,54 @@ class Meta: "index": help_text_acl_rule_index, "action": help_text_acl_action, "remark": mark_safe( - "*Note: CANNOT be set if source prefix OR action is set.", + "*Note: CANNOT be set if source OR action is set.", ), } + def __init__(self, *args, **kwargs) -> None: + """ + Initialize the ACLStandardRuleForm. + """ + + # Initialize fields with initial values + instance = kwargs.get("instance") + initial = kwargs.get("initial", {}).copy() + + if instance is not None and instance.source: + # Initialize the source object field + initial["source"] = instance.source + + kwargs["initial"] = initial + + super().__init__(*args, **kwargs) + + if source_type_id := get_field_value(self, "source_type"): + try: + # Retrieve the ContentType model class based on the source type + source_type = ContentType.objects.get(pk=source_type_id) + source_model = source_type.model_class() + + # Configure the queryset and label for the source field + self.fields["source"].queryset = source_model.objects.all() + self.fields["source"].widget.attrs["selector"] = source_model._meta.label_lower + self.fields["source"].disabled = False + self.fields["source"].label = _("Source " + bettertitle(source_model._meta.verbose_name)) + except ObjectDoesNotExist: + pass + + # Clears the source field if the selected type changes + if self.instance and self.instance.pk and source_type_id != self.instance.source_type_id: + self.initial["source"] = None + + def clean(self): + """ + Validate form fields for the ACL Standard Rule form. + """ + super().clean() + + # Ensure the selected source object gets assigned + self.instance.source = self.cleaned_data.get("source") + class ACLExtendedRuleForm(NetBoxModelForm): """ @@ -557,18 +629,40 @@ class ACLExtendedRuleForm(NetBoxModelForm): label="Access List", ) - source_prefix = DynamicModelChoiceField( - queryset=Prefix.objects.all(), + # Source + source_type = ContentTypeChoiceField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), required=False, + widget=HTMXSelect(), + label=_("Source Type"), help_text=help_text_acl_rule_logic, - label="Source Prefix", ) - destination_prefix = DynamicModelChoiceField( - queryset=Prefix.objects.all(), + source = DynamicModelChoiceField( + queryset=Prefix.objects.none(), # Initial queryset + selector=True, required=False, + label=_("Source"), help_text=help_text_acl_rule_logic, - label="Destination Prefix", + disabled=True, ) + + # Destination + destination_type = ContentTypeChoiceField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), + required=False, + widget=HTMXSelect(), + label=_("Destination Type"), + help_text=help_text_acl_rule_logic, + ) + destination = DynamicModelChoiceField( + queryset=Prefix.objects.none(), # Initial queryset + selector=True, + required=False, + label=_("Destination"), + help_text=help_text_acl_rule_logic, + disabled=True, + ) + fieldsets = ( FieldSet( "access_list", @@ -579,13 +673,27 @@ class ACLExtendedRuleForm(NetBoxModelForm): FieldSet( "index", "action", + name=_("Rule Definition"), + ), + FieldSet( "remark", - "source_prefix", + name=_("Remark"), + ), + FieldSet( + "protocol", + name=_("Protocol"), + ), + FieldSet( + "source_type", + "source", "source_ports", - "destination_prefix", + name=_("Source Definition"), + ), + FieldSet( + "destination_type", + "destination", "destination_ports", - "protocol", - name=_("Rule Definition"), + name=_("Destination Definition"), ), ) @@ -596,9 +704,9 @@ class Meta: "index", "action", "remark", - "source_prefix", + "source_type", "source_ports", - "destination_prefix", + "destination_type", "destination_ports", "protocol", "tags", @@ -615,3 +723,73 @@ class Meta: ), "source_ports": help_text_acl_rule_logic, } + + def __init__(self, *args, **kwargs) -> None: + """ + Initialize the ACLExtendedRuleForm. + """ + + # Initialize fields with initial values + instance = kwargs.get("instance") + initial = kwargs.get("initial", {}).copy() + + if instance is not None and instance.source: + # Initialize the source object field + initial["source"] = instance.source + if instance is not None and instance.destination: + # Initialize the destination object field + initial["destination"] = instance.destination + + kwargs["initial"] = initial + + super().__init__(*args, **kwargs) + + # Source + if source_type_id := get_field_value(self, "source_type"): + try: + # Retrieve the ContentType model class based on the source type + source_type = ContentType.objects.get(pk=source_type_id) + source_model = source_type.model_class() + + # Configure the queryset and label for the source field + self.fields["source"].queryset = source_model.objects.all() + self.fields["source"].widget.attrs["selector"] = source_model._meta.label_lower + self.fields["source"].disabled = False + self.fields["source"].label = _("Source " + bettertitle(source_model._meta.verbose_name)) + except ObjectDoesNotExist: + pass + + # Clears the source field if the selected type changes + if self.instance and self.instance.pk and source_type_id != self.instance.source_type_id: + self.initial["source"] = None + + # Destination + if destination_type_id := get_field_value(self, "destination_type"): + try: + # Retrieve the ContentType model class based on the destination type + destination_type = ContentType.objects.get(pk=destination_type_id) + destination_model = destination_type.model_class() + + # Configure the queryset and label for the destination field + self.fields["destination"].queryset = destination_model.objects.all() + self.fields["destination"].widget.attrs["selector"] = destination_model._meta.label_lower + self.fields["destination"].disabled = False + self.fields["destination"].label = _("Destination " + bettertitle(destination_model._meta.verbose_name)) + except ObjectDoesNotExist: + pass + + # Clears the destination field if the selected type changes + if self.instance and self.instance.pk and destination_type_id != self.instance.destination_type_id: + self.initial["destination"] = None + + def clean(self): + """ + Validate form fields for the ACL Extended Rule form. + """ + super().clean() + + # Ensure the selected source object gets assigned + self.instance.source = self.cleaned_data.get("source") + + # Ensure the selected destination object gets assigned + self.instance.destination = self.cleaned_data.get("destination") From 0df9b01cc95f2b4ca637ddcdb9938142e3b6c976 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 10 Aug 2025 15:27:24 +0200 Subject: [PATCH 08/35] feat(templates): Update source/destination field labels Rename labels for source and destination fields in ACL templates to reflect the updated generic model structure. Improves clarity and consistency with recent data model changes. --- netbox_acls/templates/netbox_acls/aclextendedrule.html | 8 ++++---- netbox_acls/templates/netbox_acls/aclstandardrule.html | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/netbox_acls/templates/netbox_acls/aclextendedrule.html b/netbox_acls/templates/netbox_acls/aclextendedrule.html index 3e2bad24..d40c0d42 100644 --- a/netbox_acls/templates/netbox_acls/aclextendedrule.html +++ b/netbox_acls/templates/netbox_acls/aclextendedrule.html @@ -45,16 +45,16 @@

Details

{{ object.get_protocol_display|placeholder }} - Source Prefix - {{ object.source_prefix|linkify|placeholder }} + Source + {{ object.source|linkify|placeholder }} Source Ports {{ object.source_ports|join:", "|placeholder }} - Destination Prefix - {{ object.destination_prefix|linkify|placeholder }} + Destination + {{ object.destination|linkify|placeholder }} Destination Ports diff --git a/netbox_acls/templates/netbox_acls/aclstandardrule.html b/netbox_acls/templates/netbox_acls/aclstandardrule.html index 7d26f8ef..80129940 100644 --- a/netbox_acls/templates/netbox_acls/aclstandardrule.html +++ b/netbox_acls/templates/netbox_acls/aclstandardrule.html @@ -41,8 +41,8 @@

Details

{{ object.remark|placeholder }} - Source Prefix - {{ object.source_prefix|linkify|placeholder }} + Source + {{ object.source|linkify|placeholder }} From 2f1c7476947901b20520792518b49b2110601285 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 10 Aug 2025 15:51:36 +0200 Subject: [PATCH 09/35] feat(views): Update prefetch fields for ACL rules Replaces source_prefix and destination_prefix with generic source and destination prefetches in views for ACLStandardRule and ACLExtendedRule. Aligns view logic with the updated generic source/destination model for better flexibility and consistency. --- netbox_acls/views.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/netbox_acls/views.py b/netbox_acls/views.py index d6825edf..1a6b6528 100644 --- a/netbox_acls/views.py +++ b/netbox_acls/views.py @@ -323,8 +323,8 @@ class ACLStandardRuleView(generic.ObjectView): queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) @@ -336,8 +336,8 @@ class ACLStandardRuleListView(generic.ObjectListView): queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) table = tables.ACLStandardRuleTable filterset = filtersets.ACLStandardRuleFilterSet @@ -353,8 +353,8 @@ class ACLStandardRuleEditView(generic.ObjectEditView): queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) form = forms.ACLStandardRuleForm @@ -376,8 +376,8 @@ class ACLStandardRuleDeleteView(generic.ObjectDeleteView): queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) @@ -385,8 +385,8 @@ class ACLStandardRuleDeleteView(generic.ObjectDeleteView): class ACLStandardRuleBulkDeleteView(generic.BulkDeleteView): queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) filterset = filtersets.ACLStandardRuleFilterSet table = tables.ACLStandardRuleTable @@ -405,9 +405,9 @@ class ACLExtendedRuleView(generic.ObjectView): queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) @@ -419,9 +419,9 @@ class ACLExtendedRuleListView(generic.ObjectListView): queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) table = tables.ACLExtendedRuleTable filterset = filtersets.ACLExtendedRuleFilterSet @@ -437,9 +437,9 @@ class ACLExtendedRuleEditView(generic.ObjectEditView): queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) form = forms.ACLExtendedRuleForm @@ -461,9 +461,9 @@ class ACLExtendedRuleDeleteView(generic.ObjectDeleteView): queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) @@ -471,9 +471,9 @@ class ACLExtendedRuleDeleteView(generic.ObjectDeleteView): class ACLExtendedRuleBulkDeleteView(generic.BulkDeleteView): queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) filterset = filtersets.ACLExtendedRuleFilterSet table = tables.ACLExtendedRuleTable From 52a8f1329e57074e5092ac0bb99c9d8dc28e02d3 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 13 Aug 2025 18:06:18 +0200 Subject: [PATCH 10/35] feat(serializers): Add generic source/destination support Replaces source_prefix and destination_prefix with generic source and destination fields in ACL serializers. Introduces support for content types, dynamic querying, and enhanced validation in ACLStandardRule and ACLExtendedRule. Aligns serializers with the generic source/destination model for improved flexibility and consistency. --- netbox_acls/api/serializers.py | 121 +++++++++++++++++++++++++-------- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/netbox_acls/api/serializers.py b/netbox_acls/api/serializers.py index 39cd253a..56102748 100644 --- a/netbox_acls/api/serializers.py +++ b/netbox_acls/api/serializers.py @@ -5,13 +5,16 @@ from django.contrib.contenttypes.models import ContentType from drf_spectacular.utils import extend_schema_field -from ipam.api.serializers import PrefixSerializer from netbox.api.fields import ContentTypeField from netbox.api.serializers import NetBoxModelSerializer from rest_framework import serializers from utilities.api import get_serializer_for_model -from ..constants import ACL_HOST_ASSIGNMENT_MODELS, ACL_INTERFACE_ASSIGNMENT_MODELS +from ..constants import ( + ACL_HOST_ASSIGNMENT_MODELS, + ACL_INTERFACE_ASSIGNMENT_MODELS, + ACL_RULE_SOURCE_DESTINATION_MODELS, +) from ..models import ( AccessList, ACLExtendedRule, @@ -28,8 +31,8 @@ # Sets a standard error message for ACL rules with an action of remark, but no remark set. error_message_no_remark = "Action is set to remark, you MUST add a remark." -# Sets a standard error message for ACL rules with an action of remark, but no source_prefix is set. -error_message_action_remark_source_prefix_set = "Action is set to remark, Source Prefix CANNOT be set." +# Sets a standard error message for ACL rules with an action of remark, but no source is set. +error_message_action_remark_source_set = "Action is set to remark, Source CANNOT be set." # Sets a standard error message for ACL rules with an action not set to remark, but no remark is set. error_message_remark_without_action_remark = "CANNOT set remark unless action is set to remark." # Sets a standard error message for ACL rules no associated with an ACL of the same type. @@ -186,12 +189,18 @@ class ACLStandardRuleSerializer(NetBoxModelSerializer): view_name="plugins-api:netbox_acls-api:aclstandardrule-detail", ) access_list = AccessListSerializer(nested=True, required=True) - source_prefix = PrefixSerializer( - nested=True, + source_type = ContentTypeField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), required=False, + default=None, allow_null=True, + ) + source_id = serializers.IntegerField( + required=False, default=None, + allow_null=True, ) + source = serializers.SerializerMethodField(read_only=True) class Meta: """ @@ -207,20 +216,36 @@ class Meta: "index", "action", "remark", - "source_prefix", + "source_type", + "source_id", + "source", "description", "tags", "created", "custom_fields", "last_updated", ) - brief_fields = ("id", "url", "display", "access_list", "index") + brief_fields = ( + "id", + "url", + "display", + "access_list", + "index", + ) + + @extend_schema_field(serializers.JSONField(allow_null=True)) + def get_source(self, obj): + if obj.source_id is None: + return None + serializer = get_serializer_for_model(obj.source) + context = {"request": self.context["request"]} + return serializer(obj.source, nested=True, context=context).data def validate(self, data): """ Validate the ACLStandardRule django model's inputs before allowing it to update the instance: - Check if action set to remark, but no remark set. - - Check if action set to remark, but source_prefix set. + - Check if action set to remark, but source set. """ error_message = {} @@ -230,10 +255,10 @@ def validate(self, data): error_message["remark"] = [ error_message_no_remark, ] - # Check if action set to remark, but source_prefix set. - if data.get("source_prefix"): - error_message["source_prefix"] = [ - error_message_action_remark_source_prefix_set, + # Check if action set to remark, but the source set. + if data.get("source"): + error_message["source"] = [ + error_message_action_remark_source_set, ] if error_message: @@ -251,18 +276,30 @@ class ACLExtendedRuleSerializer(NetBoxModelSerializer): view_name="plugins-api:netbox_acls-api:aclextendedrule-detail", ) access_list = AccessListSerializer(nested=True, required=True) - source_prefix = PrefixSerializer( - nested=True, + source_type = ContentTypeField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), required=False, + default=None, allow_null=True, + ) + source_id = serializers.IntegerField( + required=False, default=None, + allow_null=True, ) - destination_prefix = PrefixSerializer( - nested=True, + source = serializers.SerializerMethodField(read_only=True) + destination_type = ContentTypeField( + queryset=ContentType.objects.filter(ACL_RULE_SOURCE_DESTINATION_MODELS), required=False, + default=None, allow_null=True, + ) + destination_id = serializers.IntegerField( + required=False, default=None, + allow_null=True, ) + destination = serializers.SerializerMethodField(read_only=True) class Meta: """ @@ -279,9 +316,13 @@ class Meta: "action", "remark", "protocol", - "source_prefix", + "source_type", + "source_id", + "source", "source_ports", - "destination_prefix", + "destination_type", + "destination_id", + "destination", "destination_ports", "description", "tags", @@ -289,15 +330,37 @@ class Meta: "custom_fields", "last_updated", ) - brief_fields = ("id", "url", "display", "access_list", "index") + brief_fields = ( + "id", + "url", + "display", + "access_list", + "index", + ) + + @extend_schema_field(serializers.JSONField(allow_null=True)) + def get_source(self, obj): + if obj.source_id is None: + return None + serializer = get_serializer_for_model(obj.source) + context = {"request": self.context["request"]} + return serializer(obj.source, nested=True, context=context).data + + @extend_schema_field(serializers.JSONField(allow_null=True)) + def get_destination(self, obj): + if obj.destination_id is None: + return None + serializer = get_serializer_for_model(obj.destination) + context = {"request": self.context["request"]} + return serializer(obj.destination, nested=True, context=context).data def validate(self, data): """ Validate the ACLExtendedRule django model's inputs before allowing it to update the instance: - Check if action set to remark, but no remark set. - - Check if action set to remark, but source_prefix set. + - Check if action set to remark, but source set. - Check if action set to remark, but source_ports set. - - Check if action set to remark, but destination_prefix set. + - Check if action set to remark, but destination set. - Check if action set to remark, but destination_ports set. - Check if action set to remark, but protocol set. - Check if action set to remark, but protocol set. @@ -310,19 +373,19 @@ def validate(self, data): error_message["remark"] = [ error_message_no_remark, ] - # Check if action set to remark, but source_prefix set. - if data.get("source_prefix"): - error_message["source_prefix"] = [ - error_message_action_remark_source_prefix_set, + # Check if action set to remark, but the source set. + if data.get("source"): + error_message["source"] = [ + error_message_action_remark_source_set, ] # Check if action set to remark, but source_ports set. if data.get("source_ports"): error_message["source_ports"] = [ "Action is set to remark, Source Ports CANNOT be set.", ] - # Check if action set to remark, but destination_prefix set. - if data.get("destination_prefix"): - error_message["destination_prefix"] = [ + # Check if action set to remark, but destination set. + if data.get("destination"): + error_message["destination"] = [ "Action is set to remark, Destination Prefix CANNOT be set.", ] # Check if action set to remark, but destination_ports set. From 97900439786a64fdb83a2d07c8354ba46fd45293 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 13 Aug 2025 18:17:58 +0200 Subject: [PATCH 11/35] feat(views): Improve view docstrings and adjust prefetch fields Updated docstrings for clarity by replacing "&" with "and" in multiple view classes. Revised prefetch fields in ACLStandardRule and ACLExtendedRule to align with the new generic source/destination model changes for better consistency. --- netbox_acls/api/views.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/netbox_acls/api/views.py b/netbox_acls/api/views.py index cdff48d1..5c7c17b2 100644 --- a/netbox_acls/api/views.py +++ b/netbox_acls/api/views.py @@ -25,7 +25,7 @@ class AccessListViewSet(NetBoxModelViewSet): """ - Defines the view set for the django AccessList model & associates it to a view. + Defines the view set for the django AccessList model and associates it with a view. """ queryset = ( @@ -41,7 +41,7 @@ class AccessListViewSet(NetBoxModelViewSet): class ACLInterfaceAssignmentViewSet(NetBoxModelViewSet): """ - Defines the view set for the django ACLInterfaceAssignment model & associates it to a view. + Defines the view set for the django ACLInterfaceAssignment model and associates it with a view. """ queryset = models.ACLInterfaceAssignment.objects.prefetch_related( @@ -54,13 +54,13 @@ class ACLInterfaceAssignmentViewSet(NetBoxModelViewSet): class ACLStandardRuleViewSet(NetBoxModelViewSet): """ - Defines the view set for the django ACLStandardRule model & associates it to a view. + Defines the view set for the django ACLStandardRule model and associates it with a view. """ queryset = models.ACLStandardRule.objects.prefetch_related( "access_list", + "source", "tags", - "source_prefix", ) serializer_class = ACLStandardRuleSerializer filterset_class = filtersets.ACLStandardRuleFilterSet @@ -68,14 +68,14 @@ class ACLStandardRuleViewSet(NetBoxModelViewSet): class ACLExtendedRuleViewSet(NetBoxModelViewSet): """ - Defines the view set for the django ACLExtendedRule model & associates it to a view. + Defines the view set for the django ACLExtendedRule model and associates it with a view. """ queryset = models.ACLExtendedRule.objects.prefetch_related( "access_list", + "source", + "destination", "tags", - "source_prefix", - "destination_prefix", ) serializer_class = ACLExtendedRuleSerializer filterset_class = filtersets.ACLExtendedRuleFilterSet From dcf96d0cc3fd6af4418c8fc9e780642407959961 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 14 Aug 2025 20:03:09 +0200 Subject: [PATCH 12/35] feat(graphql): Add generic source/destination field support Introduces generic source and destination fields in GraphQL types for ACLStandardRule and ACLExtendedRule. Adds support for ContentType relationships, caching related objects, and enhanced flexibility in object handling. Aligns GraphQL implementation with the updated generic model structure for consistency. --- netbox_acls/graphql/types.py | 115 +++++++++++++++++++++++++++++++++-- 1 file changed, 110 insertions(+), 5 deletions(-) diff --git a/netbox_acls/graphql/types.py b/netbox_acls/graphql/types.py index d7888d56..18aad285 100644 --- a/netbox_acls/graphql/types.py +++ b/netbox_acls/graphql/types.py @@ -2,15 +2,20 @@ Define the object types and queries available via the graphql api. """ -from typing import Annotated, List, Union +from typing import Annotated, List, Union, TYPE_CHECKING import strawberry import strawberry_django -from netbox.graphql.types import NetBoxObjectType +from netbox.graphql.types import ContentTypeType, NetBoxObjectType from .. import models from . import filters +if TYPE_CHECKING: + from dcim.graphql.types import DeviceType, InterfaceType + from ipam.graphql.types import AggregateType, IPAddressType, IPRangeType, PrefixType + from virtualization.graphql.types import VirtualMachineType, VMInterfaceType + @strawberry_django.type( models.AccessList, @@ -74,6 +79,10 @@ class ACLInterfaceAssignmentType(NetBoxObjectType): @strawberry_django.type( models.ACLStandardRule, fields="__all__", + exclude=[ + "source_id", + "source_type", + ], filters=filters.ACLStandardRuleFilter, ) class ACLStandardRuleType(NetBoxObjectType): @@ -83,12 +92,48 @@ class ACLStandardRuleType(NetBoxObjectType): # Model fields access_list: Annotated["AccessListType", strawberry.lazy("netbox_acls.graphql.types")] - source_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None + source_type: Annotated["ContentTypeType", strawberry.lazy("netbox.graphql.types")] | None + source: ( + Annotated[ + Union[ + Annotated[ + "AggregateType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPAddressType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPRangeType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "PrefixType", + strawberry.lazy("ipam.graphql.types"), + ], + ], + strawberry.union("ACLStandardRuleObjectType"), + ] + | None + ) + + # Cached related source objects + _source_aggregate: Annotated["AggregateType", strawberry.lazy("ipam.graphql.types")] | None + _source_ipaddress: Annotated["IPAddressType", strawberry.lazy("ipam.graphql.types")] | None + _source_iprange: Annotated["IPRangeType", strawberry.lazy("ipam.graphql.types")] | None + _source_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None @strawberry_django.type( models.ACLExtendedRule, fields="__all__", + exclude=[ + "source_id", + "source_type", + "destination_id", + "destination_type", + ], filters=filters.ACLExtendedRuleFilter, ) class ACLExtendedRuleType(NetBoxObjectType): @@ -98,7 +143,67 @@ class ACLExtendedRuleType(NetBoxObjectType): # Model fields access_list: Annotated["AccessListType", strawberry.lazy("netbox_acls.graphql.types")] - source_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None + source_type: Annotated["ContentTypeType", strawberry.lazy("netbox.graphql.types")] | None + source: ( + Annotated[ + Union[ + Annotated[ + "AggregateType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPAddressType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPRangeType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "PrefixType", + strawberry.lazy("ipam.graphql.types"), + ], + ], + strawberry.union("ACLStandardRuleObjectType"), + ] + | None + ) source_ports: List[int] | None - destination_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None + destination_type: Annotated["ContentTypeType", strawberry.lazy("netbox.graphql.types")] | None + destination: ( + Annotated[ + Union[ + Annotated[ + "AggregateType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPAddressType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "IPRangeType", + strawberry.lazy("ipam.graphql.types"), + ], + Annotated[ + "PrefixType", + strawberry.lazy("ipam.graphql.types"), + ], + ], + strawberry.union("ACLStandardRuleObjectType"), + ] + | None + ) destination_ports: List[int] | None + + # Cached related source objects + _source_aggregate: Annotated["AggregateType", strawberry.lazy("ipam.graphql.types")] | None + _source_ipaddress: Annotated["IPAddressType", strawberry.lazy("ipam.graphql.types")] | None + _source_iprange: Annotated["IPRangeType", strawberry.lazy("ipam.graphql.types")] | None + _source_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None + + # Cached related destination objects + _destination_aggregate: Annotated["AggregateType", strawberry.lazy("ipam.graphql.types")] | None + _destination_ipaddress: Annotated["IPAddressType", strawberry.lazy("ipam.graphql.types")] | None + _destination_iprange: Annotated["IPRangeType", strawberry.lazy("ipam.graphql.types")] | None + _destination_prefix: Annotated["PrefixType", strawberry.lazy("ipam.graphql.types")] | None From 6647390a05d6b496524461e2b4b14bda6a02601b Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 14 Aug 2025 20:39:11 +0200 Subject: [PATCH 13/35] feat(graphql): Add ContentType filters for ACL rules Introduces ContentType filtering for generic source and destination fields in ACLStandardRule and ACLExtendedRule GraphQL filters. Adds source_type, source_id, destination_type, and destination_id fields to enhance filtering flexibility and alignment with generic models. --- .../graphql/filters/access_list_rules.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/netbox_acls/graphql/filters/access_list_rules.py b/netbox_acls/graphql/filters/access_list_rules.py index be1e0b2d..50be41db 100644 --- a/netbox_acls/graphql/filters/access_list_rules.py +++ b/netbox_acls/graphql/filters/access_list_rules.py @@ -3,6 +3,7 @@ import strawberry import strawberry_django +from core.graphql.filters import ContentTypeFilter from netbox.graphql.filter_mixins import NetBoxModelFilterMixin from strawberry.scalars import ID from strawberry_django import FilterLookup @@ -10,7 +11,6 @@ from ... import models if TYPE_CHECKING: - from ipam.graphql.filters import PrefixFilter from netbox.graphql.filter_lookups import IntegerArrayLookup, IntegerLookup from ..enums import ( @@ -39,14 +39,19 @@ class ACLRuleFilterMixin(NetBoxModelFilterMixin): index: Annotated["IntegerLookup", strawberry.lazy("netbox.graphql.filter_lookups")] | None = ( strawberry_django.filter_field() ) - remark: FilterLookup[str] | None = strawberry_django.filter_field() description: FilterLookup[str] | None = strawberry_django.filter_field() action: Annotated["ACLRuleActionEnum", strawberry.lazy("netbox_acls.graphql.enums")] | None = ( strawberry_django.filter_field() ) - source_prefix: Annotated["PrefixFilter", strawberry.lazy("ipam.graphql.filters")] | None = ( + + # Remark + remark: FilterLookup[str] | None = strawberry_django.filter_field() + + # Source + source_type: Annotated["ContentTypeFilter", strawberry.lazy("core.graphql.filters")] | None = ( strawberry_django.filter_field() ) + source_id: ID | None = strawberry_django.filter_field() @strawberry_django.filter(models.ACLStandardRule, lookups=True) @@ -64,15 +69,21 @@ class ACLExtendedRuleFilter(ACLRuleFilterMixin): GraphQL filter definition for the ACLExtendedRule model. """ + # Source source_ports: Annotated["IntegerArrayLookup", strawberry.lazy("netbox.graphql.filter_lookups")] | None = ( strawberry_django.filter_field() ) - destination_prefix: Annotated["PrefixFilter", strawberry.lazy("ipam.graphql.filters")] | None = ( + + # Destination + destination_type: Annotated["ContentTypeFilter", strawberry.lazy("core.graphql.filters")] | None = ( strawberry_django.filter_field() ) + destination_id: ID | None = strawberry_django.filter_field() destination_ports: Annotated["IntegerArrayLookup", strawberry.lazy("netbox.graphql.filter_lookups")] | None = ( strawberry_django.filter_field() ) + + # Protocol protocol: Annotated["ACLProtocolEnum", strawberry.lazy("netbox_acls.graphql.enums")] | None = ( strawberry_django.filter_field() ) From 2b2f628071287b14cff59dea2f83dc8576e5d340 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 15 Aug 2025 10:28:02 +0200 Subject: [PATCH 14/35] feat(tests): Update ACL rule tests for generic fields Replaces `source_prefix` and `destination_prefix` with generic `source` and `destination` fields in ACL rule tests. Updates test cases to include `source_type`, `source_id`, `destination_type`, and `destination_id` for better alignment with the updated generic model. --- .../tests/api/test_access_list_rules.py | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/netbox_acls/tests/api/test_access_list_rules.py b/netbox_acls/tests/api/test_access_list_rules.py index ace6d079..d4e83e72 100644 --- a/netbox_acls/tests/api/test_access_list_rules.py +++ b/netbox_acls/tests/api/test_access_list_rules.py @@ -3,8 +3,13 @@ from utilities.testing import APIViewTestCases from virtualization.models import Cluster, ClusterType, VirtualMachine -from netbox_acls.choices import * -from netbox_acls.models import * +from netbox_acls.choices import ( + ACLActionChoices, + ACLProtocolChoices, + ACLRuleActionChoices, + ACLTypeChoices, +) +from netbox_acls.models import AccessList, ACLExtendedRule, ACLStandardRule class ACLStandardRuleAPIViewTestCase(APIViewTestCases.APIViewTestCase): @@ -98,7 +103,7 @@ def setUpTestData(cls): index=10, description="Rule 10", action=ACLRuleActionChoices.ACTION_PERMIT, - source_prefix=prefix1, + source=prefix1, ), ACLStandardRule( access_list=access_list_device, @@ -112,7 +117,7 @@ def setUpTestData(cls): index=10, description="Rule 10", action=ACLRuleActionChoices.ACTION_DENY, - source_prefix=prefix2, + source=prefix2, ), ) ACLStandardRule.objects.bulk_create(acl_standard_rules) @@ -123,14 +128,16 @@ def setUpTestData(cls): "index": 30, "description": "Rule 30", "action": ACLRuleActionChoices.ACTION_DENY, - "source_prefix": prefix2.id, + "source_type": "ipam.prefix", + "source_id": prefix2.id, }, { "access_list": access_list_vm.id, "index": 20, "description": "Rule 30", "action": ACLRuleActionChoices.ACTION_PERMIT, - "source_prefix": prefix1.id, + "source_type": "ipam.prefix", + "source_id": prefix1.id, }, { "access_list": access_list_vm.id, @@ -237,9 +244,9 @@ def setUpTestData(cls): description="Rule 10", action=ACLRuleActionChoices.ACTION_PERMIT, protocol=ACLProtocolChoices.PROTOCOL_TCP, - source_prefix=prefix1, + source=prefix1, source_ports=[22, 443], - destination_prefix=prefix1, + destination=prefix1, destination_ports=[22, 443], ), ACLExtendedRule( @@ -254,8 +261,8 @@ def setUpTestData(cls): index=10, description="Rule 10", action=ACLRuleActionChoices.ACTION_DENY, - source_prefix=prefix2, - destination_prefix=prefix1, + source=prefix2, + destination=prefix1, ), ) ACLExtendedRule.objects.bulk_create(acl_extended_rules) @@ -267,9 +274,11 @@ def setUpTestData(cls): "description": "Rule 30", "action": ACLRuleActionChoices.ACTION_DENY, "protocol": ACLProtocolChoices.PROTOCOL_UDP, - "source_prefix": prefix2.id, + "source_type": "ipam.prefix", + "source_id": prefix2.id, "source_ports": [53], - "destination_prefix": prefix2.id, + "destination_type": "ipam.prefix", + "destination_id": prefix2.id, "destination_ports": [53], }, { @@ -278,8 +287,10 @@ def setUpTestData(cls): "description": "Rule 30", "action": ACLRuleActionChoices.ACTION_PERMIT, "protocol": ACLProtocolChoices.PROTOCOL_ICMP, - "source_prefix": prefix1.id, - "destination_prefix": prefix2.id, + "source_type": "ipam.prefix", + "source_id": prefix1.id, + "destination_type": "ipam.prefix", + "destination_id": prefix2.id, }, { "access_list": access_list_vm.id, From 373281908bb04a9351deaa647cd9fe19be1e1282 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Mon, 18 Aug 2025 12:30:13 +0200 Subject: [PATCH 15/35] fix(choices): Correct typos in docstrings for ACL choices Fixes multiple instances of "availble" to "available" in docstrings for ACL action, rule action, type, and protocol choices. Ensures documentation accuracy and readability. --- netbox_acls/choices.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/netbox_acls/choices.py b/netbox_acls/choices.py index 4b1c5b2a..19793589 100644 --- a/netbox_acls/choices.py +++ b/netbox_acls/choices.py @@ -16,7 +16,7 @@ class ACLActionChoices(ChoiceSet): """ - Defines the choices availble for the Access Lists plugin specific to ACL default_action. + Defines the choices available for the Access Lists plugin specific to ACL default_action. """ ACTION_DENY = "deny" @@ -32,7 +32,7 @@ class ACLActionChoices(ChoiceSet): class ACLRuleActionChoices(ChoiceSet): """ - Defines the choices availble for the Access Lists plugin specific to ACL rule actions. + Defines the choices available for the Access Lists plugin specific to ACL rule actions. """ ACTION_DENY = "deny" @@ -62,7 +62,7 @@ class ACLAssignmentDirectionChoices(ChoiceSet): class ACLTypeChoices(ChoiceSet): """ - Defines the choices availble for the Access Lists plugin specific to ACL type. + Defines the choices available for the Access Lists plugin specific to ACL type. """ TYPE_STANDARD = "standard" @@ -76,7 +76,7 @@ class ACLTypeChoices(ChoiceSet): class ACLProtocolChoices(ChoiceSet): """ - Defines the choices availble for the Access Lists plugin specific to ACL Rule protocol. + Defines the choices available for the Access Lists plugin specific to ACL Rule protocol. """ PROTOCOL_ICMP = "icmp" From 5b8390aec150b52146534b8412c97c19a8270b5d Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 23 Aug 2025 16:19:17 +0200 Subject: [PATCH 16/35] fix(filtersets): Update filter fields for ACL source/destination Refines field names and query attributes for source and destination filters. Ensures consistency with related models and improves filtering accuracy for aggregates, IP ranges, and IP addresses. --- netbox_acls/filtersets.py | 48 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/netbox_acls/filtersets.py b/netbox_acls/filtersets.py index d1cd773f..3cd3c945 100644 --- a/netbox_acls/filtersets.py +++ b/netbox_acls/filtersets.py @@ -204,9 +204,9 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): # Source source_aggregate = django_filters.ModelMultipleChoiceFilter( - field_name="_source_aggregate", + field_name="_source_aggregate__prefix", queryset=Aggregate.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Source Aggregate (name)"), ) source_aggregate_id = django_filters.ModelMultipleChoiceFilter( @@ -216,9 +216,9 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): label=_("Source Aggregate (ID)"), ) source_ipaddress = django_filters.ModelMultipleChoiceFilter( - field_name="_source_ipaddress", + field_name="_source_ipaddress__address", queryset=IPAddress.objects.all(), - to_field_name="name", + to_field_name="address", label=_("Source IP-Address (name)"), ) source_ipaddress_id = django_filters.ModelMultipleChoiceFilter( @@ -228,9 +228,9 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): label=_("Source IP-Address (ID)"), ) source_iprange = django_filters.ModelMultipleChoiceFilter( - field_name="_source_iprange", + field_name="_source_iprange__start_address", queryset=IPRange.objects.all(), - to_field_name="name", + to_field_name="start_address", label=_("Source IP-Range (name)"), ) source_iprange_id = django_filters.ModelMultipleChoiceFilter( @@ -240,9 +240,9 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): label=_("Source IP-Range (ID)"), ) source_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="_source_prefix", + field_name="_source_prefix__prefix", queryset=Prefix.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Source Prefix (name)"), ) source_prefix_id = django_filters.ModelMultipleChoiceFilter( @@ -294,9 +294,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): # Source source_aggregate = django_filters.ModelMultipleChoiceFilter( - field_name="_source_aggregate", + field_name="_source_aggregate__prefix", queryset=Aggregate.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Source Aggregate (name)"), ) source_aggregate_id = django_filters.ModelMultipleChoiceFilter( @@ -306,9 +306,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Source Aggregate (ID)"), ) source_ipaddress = django_filters.ModelMultipleChoiceFilter( - field_name="_source_ipaddress", + field_name="_source_ipaddress__address", queryset=IPAddress.objects.all(), - to_field_name="name", + to_field_name="address", label=_("Source IP-Address (name)"), ) source_ipaddress_id = django_filters.ModelMultipleChoiceFilter( @@ -318,9 +318,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Source IP-Address (ID)"), ) source_iprange = django_filters.ModelMultipleChoiceFilter( - field_name="_source_iprange", + field_name="_source_iprange__start_address", queryset=IPRange.objects.all(), - to_field_name="name", + to_field_name="start_address", label=_("Source IP-Range (name)"), ) source_iprange_id = django_filters.ModelMultipleChoiceFilter( @@ -330,9 +330,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Source IP-Range (ID)"), ) source_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="_source_prefix", + field_name="_source_prefix__prefix", queryset=Prefix.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Source Prefix (name)"), ) source_prefix_id = django_filters.ModelMultipleChoiceFilter( @@ -344,9 +344,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): # Destination destination_aggregate = django_filters.ModelMultipleChoiceFilter( - field_name="_destination_aggregate", + field_name="_destination_aggregate__prefix", queryset=Aggregate.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Destination Aggregate (name)"), ) destination_aggregate_id = django_filters.ModelMultipleChoiceFilter( @@ -356,9 +356,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Destination Aggregate (ID)"), ) destination_ipaddress = django_filters.ModelMultipleChoiceFilter( - field_name="_destination_ipaddress", + field_name="_destination_ipaddress__address", queryset=IPAddress.objects.all(), - to_field_name="name", + to_field_name="address", label=_("Destination IP-Address (name)"), ) destination_ipaddress_id = django_filters.ModelMultipleChoiceFilter( @@ -368,9 +368,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Destination IP-Address (ID)"), ) destination_iprange = django_filters.ModelMultipleChoiceFilter( - field_name="_destination_iprange", + field_name="_destination_iprange__start_address", queryset=IPRange.objects.all(), - to_field_name="name", + to_field_name="start_address", label=_("Destination IP-Range (name)"), ) destination_iprange_id = django_filters.ModelMultipleChoiceFilter( @@ -380,9 +380,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): label=_("Destination IP-Range (ID)"), ) destination_prefix = django_filters.ModelMultipleChoiceFilter( - field_name="_destination_prefix", + field_name="_destination_prefix__prefix", queryset=Prefix.objects.all(), - to_field_name="name", + to_field_name="prefix", label=_("Destination Prefix (name)"), ) destination_prefix_id = django_filters.ModelMultipleChoiceFilter( From 1fc9b60254b0aa58429e8930b11bea0ff8ac3d8b Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 23 Aug 2025 17:06:24 +0200 Subject: [PATCH 17/35] feat(filtersets): Add remark and port filters for ACL rules Introduces `remark`, `source_port`, and `destination_port` fields to ACL filtersets for enhanced filtering capabilities. Aligns with the generic source/destination model updates and improves flexibility in rule definition. --- netbox_acls/filtersets.py | 24 ++++++++++++++++++++++++ netbox_acls/forms/filtersets.py | 20 ++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/netbox_acls/filtersets.py b/netbox_acls/filtersets.py index 3cd3c945..e90bcab2 100644 --- a/netbox_acls/filtersets.py +++ b/netbox_acls/filtersets.py @@ -9,6 +9,7 @@ from django.utils.translation import gettext_lazy as _ from ipam.models import Aggregate, IPAddress, IPRange, Prefix from netbox.filtersets import NetBoxModelFilterSet +from utilities.filters import ContentTypeFilter, NumericArrayFilter from virtualization.models import VirtualMachine, VMInterface from .choices import ACLTypeChoices @@ -203,6 +204,9 @@ class ACLStandardRuleFilterSet(NetBoxModelFilterSet): ) # Source + source_type = ContentTypeFilter( + label=_("Source Type"), + ) source_aggregate = django_filters.ModelMultipleChoiceFilter( field_name="_source_aggregate__prefix", queryset=Aggregate.objects.all(), @@ -263,6 +267,7 @@ class Meta: "access_list", "index", "action", + "remark", "source_type", "source_id", ) @@ -293,6 +298,9 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): ) # Source + source_type = ContentTypeFilter( + label=_("Source Type"), + ) source_aggregate = django_filters.ModelMultipleChoiceFilter( field_name="_source_aggregate__prefix", queryset=Aggregate.objects.all(), @@ -341,8 +349,16 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): to_field_name="id", label=_("Source Prefix (ID)"), ) + source_port = NumericArrayFilter( + field_name="source_ports", + lookup_expr="contains", + label=_("Source Port"), + ) # Destination + destination_type = ContentTypeFilter( + label=_("Destination Type"), + ) destination_aggregate = django_filters.ModelMultipleChoiceFilter( field_name="_destination_aggregate__prefix", queryset=Aggregate.objects.all(), @@ -391,6 +407,11 @@ class ACLExtendedRuleFilterSet(NetBoxModelFilterSet): to_field_name="id", label=_("Destination Prefix (ID)"), ) + destination_port = NumericArrayFilter( + field_name="destination_ports", + lookup_expr="contains", + label=_("Destination Port"), + ) class Meta: """ @@ -403,10 +424,13 @@ class Meta: "access_list", "index", "action", + "remark", "source_type", "source_id", + "source_port", "destination_type", "destination_id", + "destination_port", "protocol", ) diff --git a/netbox_acls/forms/filtersets.py b/netbox_acls/forms/filtersets.py index 8e97264b..c1392f97 100644 --- a/netbox_acls/forms/filtersets.py +++ b/netbox_acls/forms/filtersets.py @@ -251,6 +251,7 @@ class ACLStandardRuleFilterForm(NetBoxModelFilterSetForm): "access_list_id", "index", "action", + "remark", name=_("ACL Details"), ), FieldSet( @@ -279,6 +280,10 @@ class ACLStandardRuleFilterForm(NetBoxModelFilterSetForm): required=False, label=_("Action"), ) + remark = forms.CharField( + required=False, + label=_("Remark"), + ) # Source selectors source_aggregate_id = DynamicModelMultipleChoiceField( @@ -322,6 +327,7 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): "access_list_id", "index", "action", + "remark", "protocol", name=_("ACL Details"), ), @@ -330,6 +336,7 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): "source_ipaddress_id", "source_iprange_id", "source_prefix_id", + "source_port", name=_("Source Details"), ), FieldSet( @@ -337,6 +344,7 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): "destination_ipaddress_id", "destination_iprange_id", "destination_prefix_id", + "destination_port", name=_("Destination Details"), ), ) @@ -358,6 +366,10 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): required=False, label=_("Action"), ) + remark = forms.CharField( + required=False, + label=_("Remark"), + ) protocol = forms.ChoiceField( choices=add_blank_choice(ACLProtocolChoices), required=False, @@ -385,6 +397,10 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): required=False, label=_("Source Prefix"), ) + source_port = forms.IntegerField( + label=_("Source Port"), + required=False, + ) # Destination selectors destination_aggregate_id = DynamicModelMultipleChoiceField( @@ -407,6 +423,10 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm): required=False, label=_("Destination Prefix"), ) + destination_port = forms.IntegerField( + label=_("Destination Port"), + required=False, + ) # Tag selector tag = TagFilterField(model) From dc69807ecf4a0675c6d9092b53ee4a3b0f659fe2 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 30 Aug 2025 19:37:01 +0200 Subject: [PATCH 18/35] fix(migrations): Ensure database alias is used in ACL rule updates Adds `using(db_alias)` to queries for ACLStandardRule and ACLExtendedRule to ensure the correct database alias is used during migration. Also adjusts formatting for dependency tuples to enhance consistency. --- ...05_acl_rule_source_and_destination_objects.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py index bb379b0e..52e32349 100644 --- a/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py +++ b/netbox_acls/migrations/0005_acl_rule_source_and_destination_objects.py @@ -7,16 +7,20 @@ def copy_prefix_assignments(apps, schema_editor): Copy Source and Destination Prefix ForeignKey IDs to the GenericForeignKey fields. """ + + db_alias = schema_editor.connection.alias ContentType = apps.get_model("contenttypes", "ContentType") Prefix = apps.get_model("ipam", "Prefix") ACLStandardRule = apps.get_model("netbox_acls", "ACLStandardRule") ACLExtendedRule = apps.get_model("netbox_acls", "ACLExtendedRule") - ACLStandardRule.objects.filter(_source_prefix__isnull=False).update( + ACLStandardRule.objects.using(db_alias).filter(_source_prefix__isnull=False).update( source_type=ContentType.objects.get_for_model(Prefix), source_id=models.F("_source_prefix_id"), ) - ACLExtendedRule.objects.filter(_source_prefix__isnull=False).filter(_destination_prefix__isnull=False).update( + ACLExtendedRule.objects.using(db_alias).filter(_source_prefix__isnull=False).filter( + _destination_prefix__isnull=False + ).update( source_type=ContentType.objects.get_for_model(Prefix), source_id=models.F("_source_prefix_id"), destination_type=ContentType.objects.get_for_model(Prefix), @@ -26,10 +30,10 @@ def copy_prefix_assignments(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('contenttypes', '0002_remove_content_type_name'), - ('extras', '0128_tableconfig'), - ('ipam', '0081_remove_service_device_virtual_machine_add_parent_gfk_index'), - ('netbox_acls', '0004_netbox_acls'), + ("contenttypes", "0002_remove_content_type_name"), + ("extras", "0128_tableconfig"), + ("ipam", "0081_remove_service_device_virtual_machine_add_parent_gfk_index"), + ("netbox_acls", "0004_netbox_acls"), ] operations = [ From 4a86457730c9022460c481f6a2b4460bbd43a358 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sat, 23 Aug 2025 19:18:07 +0200 Subject: [PATCH 19/35] refactor(models): Unify ACL assignment and update validation Consolidates ACLInterfaceAssignment into a single ACLAssignment model, ensuring support for devices, virtual chassis, VMs, and interfaces. Improves validation logic by enforcing unique constraints per object type and direction. Also enhances maintainability by centralizing assignment-related logic. BREAKING CHANGE: Legacy ACLInterfaceAssignment is replaced with ACLAssignment. --- netbox_acls/choices.py | 19 ++- netbox_acls/constants.py | 30 ++-- netbox_acls/models/access_lists.py | 226 +++++++++++++++++++++++------ 3 files changed, 219 insertions(+), 56 deletions(-) diff --git a/netbox_acls/choices.py b/netbox_acls/choices.py index 19793589..654d9f7d 100644 --- a/netbox_acls/choices.py +++ b/netbox_acls/choices.py @@ -7,6 +7,7 @@ __all__ = ( "ACLActionChoices", "ACLAssignmentDirectionChoices", + "ACLAssignmentDirectionUIChoices", "ACLProtocolChoices", "ACLRuleActionChoices", "ACLTypeChoices", @@ -46,17 +47,33 @@ class ACLRuleActionChoices(ChoiceSet): ] +class ACLAssignmentDirectionUIChoices(ChoiceSet): + """ + Defines the application direction of the ACL on an associated interface (UI version). + """ + + DIRECTION_INGRESS = "ingress" + DIRECTION_EGRESS = "egress" + + CHOICES = [ + (DIRECTION_INGRESS, "Ingress", "blue"), + (DIRECTION_EGRESS, "Egress", "purple"), + ] + + class ACLAssignmentDirectionChoices(ChoiceSet): """ - Defines the direction of the application of the ACL on an associated interface. + Defines the application direction of the ACL on an associated interface. """ DIRECTION_INGRESS = "ingress" DIRECTION_EGRESS = "egress" + DIRECTION_NONE = "none" CHOICES = [ (DIRECTION_INGRESS, "Ingress", "blue"), (DIRECTION_EGRESS, "Egress", "purple"), + (DIRECTION_NONE, "N/A", "darkgray"), ] diff --git a/netbox_acls/constants.py b/netbox_acls/constants.py index 07b9ea00..08759573 100644 --- a/netbox_acls/constants.py +++ b/netbox_acls/constants.py @@ -5,21 +5,25 @@ from django.db.models import Q # -# AccessList +# AccessList Assignments # -ACL_HOST_ASSIGNMENT_MODELS = Q( - Q(app_label="dcim", model="device") - | Q(app_label="dcim", model="virtualchassis") - | Q(app_label="virtualization", model="virtualmachine"), -) - -# -# ACLInterfaceAssignment -# - -ACL_INTERFACE_ASSIGNMENT_MODELS = Q( - Q(app_label="dcim", model="interface") | Q(app_label="virtualization", model="vminterface"), +ACL_ASSIGNMENT_MODELS = Q( + Q( + app_label="dcim", + model__in=( + "device", + "interface", + "virtualchassis", + ), + ) + | Q( + app_label="virtualization", + model__in=( + "virtualmachine", + "vminterface", + ), + ) ) # diff --git a/netbox_acls/models/access_lists.py b/netbox_acls/models/access_lists.py index 5035a8df..36d76284 100644 --- a/netbox_acls/models/access_lists.py +++ b/netbox_acls/models/access_lists.py @@ -14,11 +14,11 @@ from virtualization.models import VirtualMachine, VMInterface from ..choices import ACLActionChoices, ACLAssignmentDirectionChoices, ACLTypeChoices -from ..constants import ACL_HOST_ASSIGNMENT_MODELS, ACL_INTERFACE_ASSIGNMENT_MODELS +from ..constants import ACL_ASSIGNMENT_MODELS __all__ = ( "AccessList", - "ACLInterfaceAssignment", + "ACLAssignment", ) @@ -38,17 +38,6 @@ class AccessList(NetBoxModel): max_length=500, validators=[alphanumeric_plus], ) - assigned_object_type = models.ForeignKey( - to=ContentType, - on_delete=models.PROTECT, - limit_choices_to=ACL_HOST_ASSIGNMENT_MODELS, - verbose_name=_("Assigned Object Type"), - ) - assigned_object_id = models.PositiveBigIntegerField() - assigned_object = GenericForeignKey( - ct_field="assigned_object_type", - fk_field="assigned_object_id", - ) type = models.CharField( verbose_name=_("Type"), max_length=30, @@ -70,14 +59,25 @@ class AccessList(NetBoxModel): ) class Meta: - unique_together = ["assigned_object_type", "assigned_object_id", "name"] - ordering = ["assigned_object_type", "assigned_object_id", "name"] + ordering = ("name",) verbose_name = _("Access List") verbose_name_plural = _("Access Lists") def __str__(self): + """ + Returns the string representation of the object. + """ return self.name + def __init__(self, *args, **kwargs): + """ + Initializes a new instance of the class. + """ + super().__init__(*args, **kwargs) + + # Save a copy of the ACL name for validation in clean() + self._original_name = self.__dict__.get("name") + def get_absolute_url(self): """ The method is a Django convention; although not strictly required, @@ -85,16 +85,65 @@ def get_absolute_url(self): """ return reverse("plugins:netbox_acls:accesslist", args=[self.pk]) + def clean(self): + """ + Override the model's clean method for custom validation. + """ + super().clean() + + # Validate that uniqueness of the AccessList name per assigned host type + # (device, virtual chassis, or virtual machine) during renaming. + if self.pk and self._original_name and self._original_name != self.name: + host_assigned_object_types = [ + ContentType.objects.get_for_model(Device), + ContentType.objects.get_for_model(VirtualChassis), + ContentType.objects.get_for_model(VirtualMachine), + ] + acl_assignments = ACLAssignment.objects.filter( + access_list=self, + assigned_object_type__in=host_assigned_object_types, + ) + for acl_assignment in acl_assignments: + conflicting_acl_assignments = ACLAssignment.objects.filter( + access_list__name=self.name, + assigned_object_type=acl_assignment.assigned_object_type, + assigned_object_id=acl_assignment.assigned_object_id, + ).exclude(pk=acl_assignment.pk) + if conflicting_acl_assignments.exists(): + assigned_object_model = acl_assignment.assigned_object_type.model_class() + raise ValidationError( + { + "name": _( + "An Access List with the name '{access_list}' " + "is already assigned to the {assigned_object} " + "'{assigned_object_name}'.".format( + access_list=self.name, + assigned_object=assigned_object_model._meta.verbose_name, + assigned_object_name=acl_assignment.assigned_object.name, + ) + ) + } + ) + def get_default_action_color(self): + """ + Retrieves the default action color from the ACLActionChoices. + """ return ACLActionChoices.colors.get(self.default_action) def get_type_color(self): + """ + Retrieves the type color from the ACLTypeChoices. + """ return ACLTypeChoices.colors.get(self.type) -class ACLInterfaceAssignment(NetBoxModel): +class ACLAssignment(NetBoxModel): """ - Model definition for Access Lists associations with other Host interfaces: + Model definition for Access Lists associations with objects: + - device + - virtual chassis + - virtual machine - VM interfaces - device interface """ @@ -102,17 +151,13 @@ class ACLInterfaceAssignment(NetBoxModel): access_list = models.ForeignKey( to=AccessList, on_delete=models.CASCADE, + related_name="aclassignments", verbose_name=_("Access List"), ) - direction = models.CharField( - verbose_name=_("Direction"), - max_length=30, - choices=ACLAssignmentDirectionChoices, - ) assigned_object_type = models.ForeignKey( to=ContentType, on_delete=models.PROTECT, - limit_choices_to=ACL_INTERFACE_ASSIGNMENT_MODELS, + limit_choices_to=ACL_ASSIGNMENT_MODELS, verbose_name=_("Assigned Object Type"), ) assigned_object_id = models.PositiveBigIntegerField() @@ -120,6 +165,11 @@ class ACLInterfaceAssignment(NetBoxModel): ct_field="assigned_object_type", fk_field="assigned_object_id", ) + direction = models.CharField( + verbose_name=_("Direction"), + max_length=30, + choices=ACLAssignmentDirectionChoices, + ) comments = models.TextField( blank=True, ) @@ -140,11 +190,14 @@ class Meta: "access_list", "direction", ] - verbose_name = _("ACL Interface Assignment") - verbose_name_plural = _("ACL Interface Assignments") + verbose_name = _("ACL Assignment") + verbose_name_plural = _("ACL Assignments") def __str__(self): - return f"{self.access_list}: Interface {self.assigned_object}" + """ + Returns the string representation of the object. + """ + return f"{self.access_list}: Object {self.assigned_object}" def get_absolute_url(self): """ @@ -152,57 +205,146 @@ def get_absolute_url(self): it conveniently returns the absolute URL for any particular object. """ return reverse( - "plugins:netbox_acls:aclinterfaceassignment", + "plugins:netbox_acls:aclassignment", args=[self.pk], ) - def save(self, *args, **kwargs): - """Saves the current instance to the database.""" - # Ensure the assigned interface's host matches the host assigned to the access list. - if self.assigned_object.parent_object != self.access_list.assigned_object: + def clean(self) -> None: + """ + Override the model's clean method for custom validation. + """ + + # Validate object assignment before validation of any other fields + if self.assigned_object_type and not (self.assigned_object or self.assigned_object_id): + assigned_object_model = self.assigned_object_type.model_class() raise ValidationError( { - "assigned_object": "The assigned object must be the same as the device assigned to it." + "assigned_object": _( + "The {assigned_object} field is required,if an assigned object type is selected.".format( + assigned_object=assigned_object_model._meta.verbose_name + ) + ) } ) + super().clean() + + # Validate that uniqueness of the AccessList name per assigned host type + # (device, virtual chassis, or virtual machine) + host_assigned_object_types = [ + ContentType.objects.get_for_model(Device), + ContentType.objects.get_for_model(VirtualChassis), + ContentType.objects.get_for_model(VirtualMachine), + ] + if self.assigned_object_type in host_assigned_object_types: + self._validate_unique_acl_name_per_assigned_object() + else: + self._validate_unique_acl_assignment_per_assigned_interface() + + def _validate_unique_acl_name_per_assigned_object(self) -> None: + """ + Validates that there is no Access List with the same name for + the assigned object type and object ID. + This ensures each ACL name is unique for the same object. + """ + conflicting_acl_assignments = ACLAssignment.objects.filter( + access_list__name=self.access_list.name, + assigned_object_type=self.assigned_object_type, + assigned_object_id=self.assigned_object_id, + ) + if self.pk: + conflicting_acl_assignments = conflicting_acl_assignments.exclude(pk=self.pk) + + if conflicting_acl_assignments.exists(): + assigned_object_model = self.assigned_object_type.model_class() + raise ValidationError( + { + "access_list": _( + "An Access List with the name '{access_list}' " + "already exists for the specified " + "{assigned_object}.".format( + access_list=self.access_list.name, assigned_object=assigned_object_model._meta.verbose_name + ) + ) + } + ) + + def _validate_unique_acl_assignment_per_assigned_interface(self) -> None: + conflicting_acl_assignments = ACLAssignment.objects.filter( + assigned_object_type=self.assigned_object_type, + assigned_object_id=self.assigned_object_id, + direction=self.direction, + ) + if self.pk: + conflicting_acl_assignments = conflicting_acl_assignments.exclude(pk=self.pk) + if conflicting_acl_assignments.exists(): + assigned_object_model = self.assigned_object_type.model_class() + raise ValidationError( + { + "direction": _( + "An ACL Assignment with the same direction already exists for the specified " + "{assigned_object}.".format(assigned_object=assigned_object_model._meta.verbose_name) + ) + } + ) + + def save(self, *args, **kwargs): + """ + Saves the current instance to the database. + """ + host_assigned_object_types = [ + ContentType.objects.get_for_model(Device), + ContentType.objects.get_for_model(VirtualChassis), + ContentType.objects.get_for_model(VirtualMachine), + ] + + # If the assigned object is a host type (device, virtual chassis, + # or virtual machine), directional semantics (ingress/egress) are + # not applicable. + # Therefore, the direction field is set to "none" in these cases. + if self.assigned_object_type in host_assigned_object_types: + self.direction = ACLAssignmentDirectionChoices.DIRECTION_NONE + super().save(*args, **kwargs) def get_direction_color(self): + """ + Retrieves the direction color from the ACLAssignmentDirectionChoices. + """ return ACLAssignmentDirectionChoices.colors.get(self.direction) GenericRelation( - to=ACLInterfaceAssignment, + to=ACLAssignment, content_type_field="assigned_object_type", object_id_field="assigned_object_id", related_query_name="interface", -).contribute_to_class(Interface, "accesslistassignments") +).contribute_to_class(Interface, "aclassignments") GenericRelation( - to=ACLInterfaceAssignment, + to=ACLAssignment, content_type_field="assigned_object_type", object_id_field="assigned_object_id", related_query_name="vminterface", -).contribute_to_class(VMInterface, "accesslistassignments") +).contribute_to_class(VMInterface, "aclassignments") GenericRelation( - to=AccessList, + to=ACLAssignment, content_type_field="assigned_object_type", object_id_field="assigned_object_id", related_query_name="device", -).contribute_to_class(Device, "accesslists") +).contribute_to_class(Device, "aclassignments") GenericRelation( - to=AccessList, + to=ACLAssignment, content_type_field="assigned_object_type", object_id_field="assigned_object_id", related_query_name="virtual_chassis", -).contribute_to_class(VirtualChassis, "accesslists") +).contribute_to_class(VirtualChassis, "aclassignments") GenericRelation( - to=AccessList, + to=ACLAssignment, content_type_field="assigned_object_type", object_id_field="assigned_object_id", related_query_name="virtual_machine", -).contribute_to_class(VirtualMachine, "accesslists") +).contribute_to_class(VirtualMachine, "aclassignments") From ea323af65b1a94826ff350ed44ec1ba566ed35f4 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 26 Sep 2025 11:19:53 +0200 Subject: [PATCH 20/35] refactor(migrations): Rename and update ACL assignments model Renames `ACLInterfaceAssignment` to `ACLAssignment` and adjusts related model fields and constraints. Removes legacy `assigned_object_id` and `assigned_object_type` fields from `AccessList`. Adds data migration for copying over existing host assignments to the updated model. --- .../migrations/0006_acl_assignments.py | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 netbox_acls/migrations/0006_acl_assignments.py diff --git a/netbox_acls/migrations/0006_acl_assignments.py b/netbox_acls/migrations/0006_acl_assignments.py new file mode 100644 index 00000000..eb2af35e --- /dev/null +++ b/netbox_acls/migrations/0006_acl_assignments.py @@ -0,0 +1,75 @@ +import django.db.models.deletion +from django.db import migrations, models + + +def copy_host_assignments(apps, schema_editor): + """ + Copies host assignments from the AccessList model to ACLAssignment model. + """ + + db_alias = schema_editor.connection.alias + AccessList = apps.get_model("netbox_acls", "AccessList") + ACLAssignment = apps.get_model("netbox_acls", "ACLAssignment") + + for acl in AccessList.objects.using(db_alias).all(): + ACLAssignment.objects.using(db_alias).create( + access_list=acl, + assigned_object_type=acl.assigned_object_type, + assigned_object_id=acl.assigned_object_id, + direction="none", + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("netbox_acls", "0005_acl_rule_source_and_destination_objects"), + ] + + operations = [ + migrations.RenameModel( + old_name="ACLInterfaceAssignment", + new_name="ACLAssignment", + ), + migrations.AlterModelOptions( + name="accesslist", + options={"ordering": ("name",)}, + ), + migrations.AlterUniqueTogether( + name="accesslist", + unique_together=set(), + ), + migrations.AlterField( + model_name="aclassignment", + name="access_list", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="aclassignments", + to="netbox_acls.accesslist", + ), + ), + migrations.AlterField( + model_name="aclassignment", + name="assigned_object_type", + field=models.ForeignKey( + limit_choices_to=models.Q( + models.Q( + models.Q(("app_label", "dcim"), ("model__in", ("device", "interface", "virtualchassis"))), + models.Q(("app_label", "virtualization"), ("model__in", ("virtualmachine", "vminterface"))), + _connector="OR", + ), + ), + on_delete=django.db.models.deletion.PROTECT, + to="contenttypes.contenttype", + ), + ), + # Copy over existing Host assignments + migrations.RunPython(code=copy_host_assignments, reverse_code=migrations.RunPython.noop), + migrations.RemoveField( + model_name="accesslist", + name="assigned_object_id", + ), + migrations.RemoveField( + model_name="accesslist", + name="assigned_object_type", + ), + ] From 150570b32bf147b133228edb06753fc685e69a50 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 24 Aug 2025 11:07:44 +0200 Subject: [PATCH 21/35] refactor(tests): Remove legacy assigned object tests for AccessList Removes tests for `assigned_object` in AccessList, aligning with the migration to centralized ACLAssignment. Updates tests to focus on ACLAssignment validation scenarios across different object types. --- netbox_acls/tests/models/test_accesslists.py | 140 +--------- .../tests/models/test_aclassignments.py | 244 ++++++++++++++++++ .../models/test_aclinterfaceassignments.py | 190 -------------- .../tests/models/test_extendedrules.py | 3 - .../tests/models/test_standardrules.py | 3 - 5 files changed, 250 insertions(+), 330 deletions(-) create mode 100644 netbox_acls/tests/models/test_aclassignments.py delete mode 100644 netbox_acls/tests/models/test_aclinterfaceassignments.py diff --git a/netbox_acls/tests/models/test_accesslists.py b/netbox_acls/tests/models/test_accesslists.py index 579bd719..7bf20935 100644 --- a/netbox_acls/tests/models/test_accesslists.py +++ b/netbox_acls/tests/models/test_accesslists.py @@ -1,10 +1,6 @@ from itertools import cycle -from dcim.models import Device, VirtualChassis -from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError -from ipam.models import Prefix -from virtualization.models import VirtualMachine from netbox_acls.models import AccessList @@ -29,7 +25,6 @@ def test_accesslist_standard_creation(self): created_acl = AccessList( name=acl_name, - assigned_object=self.device1, type="standard", default_action="deny", ) @@ -38,8 +33,6 @@ def test_accesslist_standard_creation(self): self.assertEqual(created_acl.name, acl_name) self.assertEqual(created_acl.type, "standard") self.assertEqual(created_acl.default_action, "deny") - self.assertEqual(isinstance(created_acl.assigned_object, Device), True) - self.assertEqual(created_acl.assigned_object, self.device1) def test_accesslist_extended_creation(self): """ @@ -49,7 +42,6 @@ def test_accesslist_extended_creation(self): created_acl = AccessList( name=acl_name, - assigned_object=self.device2, type="extended", default_action="permit", ) @@ -58,60 +50,6 @@ def test_accesslist_extended_creation(self): self.assertEqual(created_acl.name, acl_name) self.assertEqual(created_acl.type, "extended") self.assertEqual(created_acl.default_action, "permit") - self.assertEqual(isinstance(created_acl.assigned_object, Device), True) - self.assertEqual(created_acl.assigned_object, self.device2) - - def test_accesslist_creation_with_virtual_chassis(self): - """ - Test that AccessList creation with an assigned virtual chassis passes validation. - """ - acl_name = "Test-ACL-with-Virtual-Machine" - - created_acl = AccessList( - name=acl_name, - assigned_object=self.virtual_chassis1, - **self.common_acl_params, - ) - - self.assertTrue(isinstance(created_acl, AccessList)) - self.assertEqual(created_acl.name, acl_name) - self.assertEqual(created_acl.type, "extended") - self.assertEqual(created_acl.default_action, "permit") - self.assertEqual(isinstance(created_acl.assigned_object, VirtualChassis), True) - self.assertEqual(created_acl.assigned_object, self.virtual_chassis1) - - def test_accesslist_creation_with_virtual_machine(self): - """ - Test that AccessList creation with an assigned virtual machine passes validation. - """ - acl_name = "Test-ACL-with-Virtual-Machine" - - created_acl = AccessList( - name=acl_name, - assigned_object=self.virtual_machine1, - **self.common_acl_params, - ) - - self.assertTrue(isinstance(created_acl, AccessList)) - self.assertEqual(created_acl.name, acl_name) - self.assertEqual(created_acl.type, "extended") - self.assertEqual(created_acl.default_action, "permit") - self.assertEqual(isinstance(created_acl.assigned_object, VirtualMachine), True) - self.assertEqual(created_acl.assigned_object, self.virtual_machine1) - - def test_wrong_assigned_object_type_fail(self): - """ - Test that AccessList cannot be assigned to an object type other than Device, VirtualChassis, VirtualMachine, - or Cluster. - """ - acl_bad_gfk = AccessList( - name="TestACL_Wrong_GFK", - assigned_object_type=ContentType.objects.get_for_model(Prefix), - assigned_object_id=self.prefix1.id, - **self.common_acl_params, - ) - with self.assertRaises(ValidationError): - acl_bad_gfk.full_clean() def test_alphanumeric_plus_success(self): """ @@ -119,104 +57,41 @@ def test_alphanumeric_plus_success(self): """ acl_good_name = AccessList( name="Test-ACL-Good_Name-1", - assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=self.device1.id, **self.common_acl_params, ) acl_good_name.full_clean() def test_duplicate_name_success(self): """ - Test that AccessList names can be non-unique if associated with different devices. + Test that AccessList names can be non-unique. """ - # Device - device_acl = AccessList( + acl1 = AccessList( name="GOOD-DUPLICATE-ACL", - assigned_object=self.device1, **self.common_acl_params, ) - device_acl.full_clean() + acl1.full_clean() - # Virtual Chassis - vc_acl = AccessList( + acl2 = AccessList( name="GOOD-DUPLICATE-ACL", - assigned_object=self.virtual_chassis1, **self.common_acl_params, ) - vc_acl.full_clean() - - # Virtual Machine - vm_acl = AccessList( - name="GOOD-DUPLICATE-ACL", - assigned_object=self.virtual_machine1, - **self.common_acl_params, - ) - vm_acl.full_clean() + acl2.full_clean() def test_alphanumeric_plus_fail(self): """ Test that AccessList names with non-alphanumeric (excluding '_' and '-') characters fail validation. """ - non_alphanumeric_plus_chars = " !@#$%^&*()[]{};:,./<>?\|~=+" + non_alphanumeric_plus_chars = r" !@#$%^&*()[]{};:,./<>?\|~=+" for i, char in enumerate(non_alphanumeric_plus_chars, start=1): bad_acl_name = AccessList( name=f"Test-ACL-bad_name_{i}_{char}", - assigned_object=self.device1, comments=f'ACL with "{char}" in name', **self.common_acl_params, ) with self.assertRaises(ValidationError): bad_acl_name.full_clean() - def test_duplicate_name_per_device_fail(self): - """ - Test that AccessList names must be unique per device. - """ - params = { - "name": "FAIL-DUPLICATE-ACL", - "assigned_object_type": ContentType.objects.get_for_model(Device), - "assigned_object_id": self.device1.id, - **self.common_acl_params, - } - acl_1 = AccessList.objects.create(**params) - acl_1.save() - acl_2 = AccessList(**params) - with self.assertRaises(ValidationError): - acl_2.full_clean() - - def test_duplicate_name_per_virtual_chassis_fail(self): - """ - Test that AccessList names must be unique per virtual chassis. - """ - params = { - "name": "FAIL-DUPLICATE-ACL", - "assigned_object_type": ContentType.objects.get_for_model(VirtualChassis), - "assigned_object_id": self.virtual_chassis1.id, - **self.common_acl_params, - } - acl_1 = AccessList.objects.create(**params) - acl_1.save() - acl_2 = AccessList(**params) - with self.assertRaises(ValidationError): - acl_2.full_clean() - - def test_duplicate_name_per_virtual_machine_fail(self): - """ - Test that AccessList names must be unique per virtual machine. - """ - params = { - "name": "FAIL-DUPLICATE-ACL", - "assigned_object_type": ContentType.objects.get_for_model(VirtualMachine), - "assigned_object_id": self.virtual_machine1.id, - **self.common_acl_params, - } - acl_1 = AccessList.objects.create(**params) - acl_1.save() - acl_2 = AccessList(**params) - with self.assertRaises(ValidationError): - acl_2.full_clean() - def test_valid_acl_choices(self): """ Test that AccessList action choices using VALID choices. @@ -233,7 +108,6 @@ def test_valid_acl_choices(self): for default_action, acl_type in valid_acl_choices: valid_acl_choice = AccessList( name=f"TestACL_Valid_Choice_{default_action}_{acl_type}", - assigned_object=self.device1, type=acl_type, default_action=default_action, comments=f"VALID ACL CHOICES USED: {default_action=} {acl_type=}", @@ -248,7 +122,6 @@ def test_invalid_acl_choices(self): invalid_acl_default_action_choice = "log" invalid_acl_default_action = AccessList( name=f"TestACL_Valid_Choice_{invalid_acl_default_action_choice}_{valid_acl_types[0]}", - assigned_object=self.device1, type=valid_acl_types[0], default_action=invalid_acl_default_action_choice, comments=f"INVALID ACL DEFAULT CHOICE USED: default_action='{invalid_acl_default_action_choice}'", @@ -260,7 +133,6 @@ def test_invalid_acl_choices(self): invalid_acl_type = "super-dupper-extended" invalid_acl_type = AccessList( name=f"TestACL_Valid_Choice_{valid_acl_default_action_choices[0]}_{invalid_acl_type}", - assigned_object=self.device1, type=invalid_acl_type, default_action=valid_acl_default_action_choices[0], comments=f"INVALID ACL DEFAULT CHOICE USED: type='{invalid_acl_type}'", diff --git a/netbox_acls/tests/models/test_aclassignments.py b/netbox_acls/tests/models/test_aclassignments.py new file mode 100644 index 00000000..e051f18d --- /dev/null +++ b/netbox_acls/tests/models/test_aclassignments.py @@ -0,0 +1,244 @@ +from django.core.exceptions import ValidationError +from virtualization.models import VirtualMachine +from dcim.models import Device, VirtualChassis, Interface +from virtualization.models import VMInterface + +from netbox_acls.models import AccessList, ACLAssignment + +from .base import BaseTestCase + + +class TestACLAssignment(BaseTestCase): + """ + Test ACLAssignment model. + """ + + @classmethod + def setUpTestData(cls): + """ + Extend BaseTestCase's setUpTestData() to create additional data for testing. + """ + super().setUpTestData() + + interface_type = "1000baset" + + # Device Interfaces + cls.device_interface1 = Interface.objects.create( + name="Interface 1", + device=cls.device1, + type=interface_type, + ) + cls.device_interface2 = Interface.objects.create( + name="Interface 2", + device=cls.device1, + type=interface_type, + ) + + # Virtual Machine Interfaces + cls.vm_interface1 = VMInterface.objects.create( + name="Interface 1", + virtual_machine=cls.virtual_machine1, + ) + cls.vm_interface2 = VMInterface.objects.create( + name="Interface 2", + virtual_machine=cls.virtual_machine1, + ) + + # Standard ACLs + cls.acl_standard1 = AccessList.objects.create( + name="STANDARD_ACL_1", + type="standard", + default_action="permit", + comments="STANDARD_ACL", + ) + cls.acl_standard2 = AccessList.objects.create( + name="STANDARD_ACL_2", + type="standard", + default_action="permit", + comments="STANDARD_ACL", + ) + + # Extended ACLs + cls.acl_extended1 = AccessList.objects.create( + name="EXTENDED_ACL_1", + type="extended", + default_action="permit", + comments="EXTENDED_ACL", + ) + cls.acl_extended2 = AccessList.objects.create( + name="EXTENDED_ACL_2", + type="extended", + default_action="permit", + comments="EXTENDED_ACL", + ) + + def test_acl_assignment_success(self): + """ + Test that ACLAssignment passes validation if the ACL is assigned to the host + and not already assigned to the interface and direction. + """ + acl_device_interface = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.device_interface1, + ) + acl_device_interface.full_clean() + + def test_acl_assignment_with_device(self): + """ + Test that ACLAssignment creation with an assigned device passes validation. + """ + device_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.device1, + ) + + self.assertTrue(isinstance(device_assignment, ACLAssignment)) + self.assertEqual(device_assignment.access_list.name, self.acl_standard1.name) + self.assertEqual(isinstance(device_assignment.assigned_object, Device), True) + self.assertEqual(device_assignment.assigned_object, self.device1) + self.assertEqual(device_assignment.direction, "ingress") + + def test_acl_assignment_with_device_interface(self): + """ + Test that ACLAssignment creation with an assigned interface passes validation. + """ + interface_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.device_interface1, + ) + + self.assertTrue(isinstance(interface_assignment, ACLAssignment)) + self.assertEqual(interface_assignment.access_list.name, self.acl_standard1.name) + self.assertEqual(isinstance(interface_assignment.assigned_object, Interface), True) + self.assertEqual(interface_assignment.assigned_object, self.device_interface1) + self.assertEqual(interface_assignment.direction, "ingress") + + def test_acl_assignment_with_virtual_chassis(self): + """ + Test that ACLAssignment creation with an assigned virtual chassis passes validation. + """ + virtual_chassis_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.virtual_chassis1, + ) + + self.assertTrue(isinstance(virtual_chassis_assignment, ACLAssignment)) + self.assertEqual(virtual_chassis_assignment.access_list.name, self.acl_standard1.name) + self.assertEqual(isinstance(virtual_chassis_assignment.assigned_object, VirtualChassis), True) + self.assertEqual(virtual_chassis_assignment.assigned_object, self.virtual_chassis1) + self.assertEqual(virtual_chassis_assignment.direction, "ingress") + + def test_acl_assignment_with_virtual_machine(self): + """ + Test that ACLAssignment creation with an assigned virtual machine passes validation. + """ + virtual_machine_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.virtual_machine1, + ) + + self.assertTrue(isinstance(virtual_machine_assignment, ACLAssignment)) + self.assertEqual(virtual_machine_assignment.access_list.name, self.acl_standard1.name) + self.assertEqual(isinstance(virtual_machine_assignment.assigned_object, VirtualMachine), True) + self.assertEqual(virtual_machine_assignment.assigned_object, self.virtual_machine1) + self.assertEqual(virtual_machine_assignment.direction, "ingress") + + def test_acl_assignment_with_virtual_machine_interface(self): + """ + Test that ACLAssignment creation with an assigned virtual machine interface passes validation. + """ + virtual_machine_interface_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.vm_interface1, + ) + + self.assertTrue(isinstance(virtual_machine_interface_assignment, ACLAssignment)) + self.assertEqual(virtual_machine_interface_assignment.access_list.name, self.acl_standard1.name) + self.assertEqual(isinstance(virtual_machine_interface_assignment.assigned_object, VMInterface), True) + self.assertEqual(virtual_machine_interface_assignment.assigned_object, self.vm_interface1) + self.assertEqual(virtual_machine_interface_assignment.direction, "ingress") + + def test_acl_assignment_with_duplicate_name_per_device_fail(self): + """ + Test that AccessList names must be unique per device. + """ + acl1_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction="none", + assigned_object=self.device1, + ) + acl1_assignment.full_clean() + acl1_assignment.save() + + acl2 = AccessList( + name="STANDARD_ACL_1", + type="standard", + default_action="permit", + comments="Same Name ACL", + ) + acl2.save() + acl2_assignment = ACLAssignment( + access_list=acl2, + direction="none", + assigned_object=self.device1, + ) + + with self.assertRaises(ValidationError): + acl2_assignment.full_clean() + acl2_assignment.save() + + def test_acl_assignment_with_duplicate_interface_and_direction_fail(self): + """ + Test that ACLAssignment fails validation + if the ACL already is assigned to the same interface and direction. + """ + acl_device_interface1 = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.device_interface1, + ) + acl_device_interface1.full_clean() + acl_device_interface1.save() + acl_device_interface2 = ACLAssignment( + access_list=self.acl_standard1, + direction="ingress", + assigned_object=self.device_interface1, + ) + with self.assertRaises(ValidationError): + acl_device_interface2.full_clean() + + def test_valid_acl_interface_assignment_choices(self): + """ + Test that ACLAssignment action choices using VALID choices. + """ + valid_acl_assignment_direction_choices = ["ingress", "egress"] + + for direction_choice in valid_acl_assignment_direction_choices: + valid_acl_assignment = ACLAssignment( + access_list=self.acl_standard1, + direction=direction_choice, + assigned_object=self.device_interface1, + comments=f"VALID ACL ASSIGNMENT CHOICES USED: direction={direction_choice}", + ) + valid_acl_assignment.full_clean() + + def test_invalid_acl_choices(self): + """ + Test that ACLAssignment action choices using INVALID choices. + """ + invalid_acl_assignment_direction_choice = "both" + + invalid_acl_assignment_direction = ACLAssignment( + access_list=self.acl_standard1, + direction=invalid_acl_assignment_direction_choice, + assigned_object=self.device_interface1, + comments=f"INVALID ACL DEFAULT CHOICE USED: default_action='{invalid_acl_assignment_direction_choice}'", + ) + with self.assertRaises(ValidationError): + invalid_acl_assignment_direction.full_clean() diff --git a/netbox_acls/tests/models/test_aclinterfaceassignments.py b/netbox_acls/tests/models/test_aclinterfaceassignments.py deleted file mode 100644 index bd2cb0f9..00000000 --- a/netbox_acls/tests/models/test_aclinterfaceassignments.py +++ /dev/null @@ -1,190 +0,0 @@ -from dcim.models import Interface -from django.core.exceptions import ValidationError -from virtualization.models import VMInterface - -from netbox_acls.models import AccessList, ACLInterfaceAssignment - -from .base import BaseTestCase - - -class TestACLInterfaceAssignment(BaseTestCase): - """ - Test ACLInterfaceAssignment model. - """ - - @classmethod - def setUpTestData(cls): - """ - Extend BaseTestCase's setUpTestData() to create additional data for testing. - """ - super().setUpTestData() - - interface_type = "1000baset" - - # Device Interfaces - cls.device_interface1 = Interface.objects.create( - name="Interface 1", - device=cls.device1, - type=interface_type, - ) - cls.device_interface2 = Interface.objects.create( - name="Interface 2", - device=cls.device1, - type=interface_type, - ) - - # Virtual Machine Interfaces - cls.vm_interface1 = VMInterface.objects.create( - name="Interface 1", - virtual_machine=cls.virtual_machine1, - ) - cls.vm_interface2 = VMInterface.objects.create( - name="Interface 2", - virtual_machine=cls.virtual_machine1, - ) - - def test_acl_interface_assignment_success(self): - """ - Test that ACLInterfaceAssignment passes validation if the ACL is assigned to the host - and not already assigned to the interface and direction. - """ - device_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.device1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - device_acl.save() - acl_device_interface = ACLInterfaceAssignment( - access_list=device_acl, - direction="ingress", - assigned_object=self.device_interface1, - ) - acl_device_interface.full_clean() - - def test_acl_interface_assignment_fail(self): - """ - Test that ACLInterfaceAssignment fails validation if the ACL is not - assigned to the parent host. - """ - device_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.device1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - device_acl.save() - acl_vm_interface = ACLInterfaceAssignment( - access_list=device_acl, - direction="ingress", - assigned_object=self.vm_interface1, - ) - with self.assertRaises(ValidationError): - acl_vm_interface.full_clean() - acl_vm_interface.save() - - def test_acl_vminterface_assignment_success(self): - """ - Test that ACLInterfaceAssignment passes validation if the ACL is assigned to the host - and not already assigned to the vminterface and direction. - """ - vm_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.virtual_machine1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - vm_acl.save() - acl_vm_interface = ACLInterfaceAssignment( - access_list=vm_acl, - direction="ingress", - assigned_object=self.vm_interface1, - ) - acl_vm_interface.full_clean() - - def test_duplicate_assignment_fail(self): - """ - Test that ACLInterfaceAssignment fails validation - if the ACL already is assigned to the same interface and direction. - """ - device_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.device1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - device_acl.save() - acl_device_interface1 = ACLInterfaceAssignment( - access_list=device_acl, - direction="ingress", - assigned_object=self.device_interface1, - ) - acl_device_interface1.full_clean() - acl_device_interface1.save() - acl_device_interface2 = ACLInterfaceAssignment( - access_list=device_acl, - direction="ingress", - assigned_object=self.device_interface1, - ) - with self.assertRaises(ValidationError): - acl_device_interface2.full_clean() - - def test_acl_already_assigned_fail(self): - """ - Test that ACLInterfaceAssignment fails validation - if the interface already has an ACL assigned in the same direction. - """ - pass - # TODO: test_acl_already_assigned_fail - VM & Device - - def test_valid_acl_interface_assignment_choices(self): - """ - Test that ACLInterfaceAssignment action choices using VALID choices. - """ - valid_acl_assignment_direction_choices = ["ingress", "egress"] - - test_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.device1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - test_acl.save() - - for direction_choice in valid_acl_assignment_direction_choices: - valid_acl_assignment = ACLInterfaceAssignment( - access_list=test_acl, - direction=direction_choice, - assigned_object=self.device_interface1, - comments=f"VALID ACL ASSIGNMENT CHOICES USED: direction={direction_choice}", - ) - valid_acl_assignment.full_clean() - - def test_invalid_acl_choices(self): - """ - Test that ACLInterfaceAssignment action choices using INVALID choices. - """ - invalid_acl_assignment_direction_choice = "both" - - test_acl = AccessList( - name="STANDARD_ACL", - assigned_object=self.device1, - type="standard", - default_action="permit", - comments="STANDARD_ACL", - ) - test_acl.save() - - invalid_acl_assignment_direction = ACLInterfaceAssignment( - access_list=test_acl, - direction=invalid_acl_assignment_direction_choice, - assigned_object=self.device_interface1, - comments=f"INVALID ACL DEFAULT CHOICE USED: default_action='{invalid_acl_assignment_direction_choice}'", - ) - with self.assertRaises(ValidationError): - invalid_acl_assignment_direction.full_clean() diff --git a/netbox_acls/tests/models/test_extendedrules.py b/netbox_acls/tests/models/test_extendedrules.py index 9452b3fd..4867fcc3 100644 --- a/netbox_acls/tests/models/test_extendedrules.py +++ b/netbox_acls/tests/models/test_extendedrules.py @@ -25,14 +25,12 @@ def setUpTestData(cls): # AccessLists cls.extended_acl1 = AccessList.objects.create( name="EXTENDED_ACL", - assigned_object=cls.device1, type=cls.acl_type, default_action=cls.default_action, comments="EXTENDED_ACL", ) cls.extended_acl2 = AccessList.objects.create( name="EXTENDED_ACL", - assigned_object=cls.virtual_machine1, type=cls.acl_type, default_action=cls.default_action, comments="EXTENDED_ACL", @@ -484,7 +482,6 @@ def test_access_list_standard_to_acl_extended_rule_assignment_fail(self): """ standard_acl1 = AccessList.objects.create( name="STANDARD_ACL", - assigned_object=self.device1, type=ACLTypeChoices.TYPE_STANDARD, default_action=self.default_action, comments="STANDARD_ACL", diff --git a/netbox_acls/tests/models/test_standardrules.py b/netbox_acls/tests/models/test_standardrules.py index 5bd8600e..dc90b35b 100644 --- a/netbox_acls/tests/models/test_standardrules.py +++ b/netbox_acls/tests/models/test_standardrules.py @@ -24,14 +24,12 @@ def setUpTestData(cls): # AccessLists cls.standard_acl1 = AccessList.objects.create( name="STANDARD_ACL", - assigned_object=cls.device1, type=cls.acl_type, default_action=cls.default_action, comments="STANDARD_ACL", ) cls.standard_acl2 = AccessList.objects.create( name="STANDARD_ACL", - assigned_object=cls.virtual_machine1, type=cls.acl_type, default_action=cls.default_action, comments="STANDARD_ACL", @@ -181,7 +179,6 @@ def test_access_list_extended_to_acl_standard_rule_assignment_fail(self): """ extended_acl1 = AccessList.objects.create( name="EXTENDED_ACL", - assigned_object=self.device1, type=ACLTypeChoices.TYPE_EXTENDED, default_action=self.default_action, comments="EXTENDED_ACL", From 08a140b26dc72ec1877f129f24d6f81fa8b2a774 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 24 Aug 2025 11:15:50 +0200 Subject: [PATCH 22/35] refactor(tables): Update tables to align with ACLAssignment model Replaces `ACLInterfaceAssignment` with `ACLAssignment` across table definitions. Updates associated columns to use the centralized model fields and modern column types from `netbox.tables`. --- netbox_acls/tables.py | 65 +++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/netbox_acls/tables.py b/netbox_acls/tables.py index d346a124..7e595839 100644 --- a/netbox_acls/tables.py +++ b/netbox_acls/tables.py @@ -4,27 +4,18 @@ import django_tables2 as tables from django.utils.translation import gettext_lazy as _ -from netbox.tables import ChoiceFieldColumn, NetBoxTable, columns +from netbox.tables import NetBoxTable, columns -from .models import AccessList, ACLExtendedRule, ACLInterfaceAssignment, ACLStandardRule +from .models import AccessList, ACLExtendedRule, ACLAssignment, ACLStandardRule __all__ = ( "AccessListTable", - "ACLInterfaceAssignmentTable", + "ACLAssignmentTable", "ACLStandardRuleTable", "ACLExtendedRuleTable", ) -COL_HOST_ASSIGNMENT = """ - {% if record.assigned_object.device %} - {{ record.assigned_object.device|placeholder }} - {% else %} - {{ record.assigned_object.virtual_machine|placeholder }} - {% endif %} - """ - - class AccessListTable(NetBoxTable): """ Defines the table view for the AccessList model. @@ -34,19 +25,14 @@ class AccessListTable(NetBoxTable): id = tables.Column( linkify=True, ) - assigned_object = tables.Column( - verbose_name=_("Assigned Host"), - orderable=False, - linkify=True, - ) name = tables.Column( linkify=True, ) device = tables.Column( linkify=True, ) - type = ChoiceFieldColumn() - default_action = ChoiceFieldColumn() + type = columns.ChoiceFieldColumn() + default_action = columns.ChoiceFieldColumn() rule_count = tables.Column( verbose_name=_("Rule Count"), ) @@ -60,7 +46,6 @@ class Meta(NetBoxTable.Meta): "pk", "id", "name", - "assigned_object", "type", "rule_count", "default_action", @@ -70,7 +55,6 @@ class Meta(NetBoxTable.Meta): ) default_columns = ( "name", - "assigned_object", "type", "rule_count", "default_action", @@ -78,7 +62,7 @@ class Meta(NetBoxTable.Meta): ) -class ACLInterfaceAssignmentTable(NetBoxTable): +class ACLAssignmentTable(NetBoxTable): """ Defines the table view for the AccessList model. """ @@ -90,37 +74,46 @@ class ACLInterfaceAssignmentTable(NetBoxTable): access_list = tables.Column( linkify=True, ) - direction = ChoiceFieldColumn() - host = tables.TemplateColumn( - template_code=COL_HOST_ASSIGNMENT, - orderable=False, + assigned_object_type = columns.ContentTypeColumn( + linkify=True, ) assigned_object = tables.Column( - verbose_name=_("Assigned Interface"), + verbose_name=_("Assigned Object"), orderable=False, linkify=True, ) + direction = columns.ChoiceFieldColumn() tags = columns.TagColumn( - url_name="plugins:netbox_acls:aclinterfaceassignment_list", + url_name="plugins:netbox_acls:aclassignment_list", + ) + type = tables.Column( + accessor=tables.A("access_list__type"), + orderable=False, + verbose_name=_("Type"), + ) + default_action = tables.Column( + accessor=tables.A("access_list__default_action"), + orderable=False, + verbose_name=_("Default Action"), ) class Meta(NetBoxTable.Meta): - model = ACLInterfaceAssignment + model = ACLAssignment fields = ( "pk", "id", "access_list", - "direction", - "host", + "assigned_object_type", "assigned_object", + "direction", "tags", ) default_columns = ( "id", "access_list", - "direction", - "host", + "type", "assigned_object", + "direction", "tags", ) @@ -136,7 +129,7 @@ class ACLStandardRuleTable(NetBoxTable): index = tables.Column( linkify=True, ) - action = ChoiceFieldColumn() + action = columns.ChoiceFieldColumn() tags = columns.TagColumn( url_name="plugins:netbox_acls:aclstandardrule_list", ) @@ -185,11 +178,11 @@ class ACLExtendedRuleTable(NetBoxTable): index = tables.Column( linkify=True, ) - action = ChoiceFieldColumn() + action = columns.ChoiceFieldColumn() tags = columns.TagColumn( url_name="plugins:netbox_acls:aclextendedrule_list", ) - protocol = ChoiceFieldColumn() + protocol = columns.ChoiceFieldColumn() # Source source_type = columns.ContentTypeColumn( From e343fcd70cb6e26ed730d5e99d9191d7cfe4e241 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 24 Aug 2025 14:52:17 +0200 Subject: [PATCH 23/35] refactor(filtersets): Update to use ACLAssignment model Replaces `ACLInterfaceAssignmentFilterSet` with `ACLAssignmentFilterSet` to align with the consolidated ACLAssignment model. Updates filtersets to support devices, virtual chassis, VMs, and interfaces using new fields. Enhances query and filter logic for improved maintainability and functionality. --- netbox_acls/filtersets.py | 148 ++++++++++++++++++++------------------ 1 file changed, 80 insertions(+), 68 deletions(-) diff --git a/netbox_acls/filtersets.py b/netbox_acls/filtersets.py index e90bcab2..ec3a981d 100644 --- a/netbox_acls/filtersets.py +++ b/netbox_acls/filtersets.py @@ -13,12 +13,12 @@ from virtualization.models import VirtualMachine, VMInterface from .choices import ACLTypeChoices -from .models import AccessList, ACLExtendedRule, ACLInterfaceAssignment, ACLStandardRule +from .models import AccessList, ACLExtendedRule, ACLAssignment, ACLStandardRule __all__ = ( "AccessListFilterSet", "ACLStandardRuleFilterSet", - "ACLInterfaceAssignmentFilterSet", + "ACLAssignmentFilterSet", "ACLExtendedRuleFilterSet", ) @@ -28,6 +28,51 @@ class AccessListFilterSet(NetBoxModelFilterSet): Define the filter set for the django model AccessList. """ + class Meta: + """ + Associates the django model AccessList & fields to the filter set. + """ + + model = AccessList + fields = ( + "id", + "name", + "type", + "default_action", + "comments", + ) + + def search(self, queryset, name, value): + """ + Override the default search behavior for the django model. + """ + query = ( + Q(name__icontains=value) + | Q(type__icontains=value) + | Q(default_action__icontains=value) + | Q(comments__icontains=value) + ) + return queryset.filter(query) + + +class ACLAssignmentFilterSet(NetBoxModelFilterSet): + """ + Define the filter set for the django model ACLAssignment. + """ + + # Access List + access_list = django_filters.ModelMultipleChoiceFilter( + queryset=AccessList.objects.all(), + to_field_name="name", + label=_("Access List (name)"), + ) + access_list_id = django_filters.ModelMultipleChoiceFilter( + queryset=AccessList.objects.all(), + to_field_name="id", + label=_("Access List (ID)"), + ) + + # Organization region = django_filters.ModelMultipleChoiceFilter( field_name="device__site__region", queryset=Region.objects.all(), @@ -46,6 +91,8 @@ class AccessListFilterSet(NetBoxModelFilterSet): to_field_name="id", label="Site", ) + + # Device device = django_filters.ModelMultipleChoiceFilter( field_name="device__name", queryset=Device.objects.all(), @@ -57,6 +104,21 @@ class AccessListFilterSet(NetBoxModelFilterSet): queryset=Device.objects.all(), label="Device (ID)", ) + + # Interface + interface = django_filters.ModelMultipleChoiceFilter( + field_name="interface__name", + queryset=Interface.objects.all(), + to_field_name="name", + label="Interface (name)", + ) + interface_id = django_filters.ModelMultipleChoiceFilter( + field_name="interface", + queryset=Interface.objects.all(), + label="Interface (ID)", + ) + + # Virtual Chassis virtual_chassis = django_filters.ModelMultipleChoiceFilter( field_name="virtual_chassis__name", queryset=VirtualChassis.objects.all(), @@ -68,6 +130,8 @@ class AccessListFilterSet(NetBoxModelFilterSet): queryset=VirtualChassis.objects.all(), label="Virtual Chassis (ID)", ) + + # Virtual Machine virtual_machine = django_filters.ModelMultipleChoiceFilter( field_name="virtual_machine__name", queryset=VirtualMachine.objects.all(), @@ -80,71 +144,7 @@ class AccessListFilterSet(NetBoxModelFilterSet): label="Virtual machine (ID)", ) - class Meta: - """ - Associates the django model AccessList & fields to the filter set. - """ - - model = AccessList - fields = ( - "id", - "name", - "device", - "device_id", - "virtual_chassis", - "virtual_chassis_id", - "virtual_machine", - "virtual_machine_id", - "type", - "default_action", - "comments", - "site", - "site_group", - "region", - ) - - def search(self, queryset, name, value): - """ - Override the default search behavior for the django model. - """ - query = ( - Q(name__icontains=value) - | Q(device__name__icontains=value) - | Q(virtual_chassis__name__icontains=value) - | Q(virtual_machine__name__icontains=value) - | Q(type__icontains=value) - | Q(default_action__icontains=value) - | Q(comments__icontains=value) - ) - return queryset.filter(query) - - -class ACLInterfaceAssignmentFilterSet(NetBoxModelFilterSet): - """ - Define the filter set for the django model ACLInterfaceAssignment. - """ - - access_list = django_filters.ModelMultipleChoiceFilter( - queryset=AccessList.objects.all(), - to_field_name="name", - label=_("Access List (name)"), - ) - access_list_id = django_filters.ModelMultipleChoiceFilter( - queryset=AccessList.objects.all(), - to_field_name="id", - label=_("Access List (ID)"), - ) - interface = django_filters.ModelMultipleChoiceFilter( - field_name="interface__name", - queryset=Interface.objects.all(), - to_field_name="name", - label="Interface (name)", - ) - interface_id = django_filters.ModelMultipleChoiceFilter( - field_name="interface", - queryset=Interface.objects.all(), - label="Interface (ID)", - ) + # Virtual Machine Interface vminterface = django_filters.ModelMultipleChoiceFilter( field_name="vminterface__name", queryset=VMInterface.objects.all(), @@ -162,10 +162,19 @@ class Meta: Associates the django model ACLInterfaceAssignment & fields to the filter set. """ - model = ACLInterfaceAssignment + model = ACLAssignment fields = ( "id", "access_list", + "site", + "site_group", + "region", + "device", + "device_id", + "virtual_chassis", + "virtual_chassis_id", + "virtual_machine", + "virtual_machine_id", "direction", "interface", "interface_id", @@ -182,6 +191,9 @@ def search(self, queryset, name, value): | Q(direction__icontains=value) | Q(interface__name__icontains=value) | Q(vminterface__name__icontains=value) + | Q(device__name__icontains=value) + | Q(virtual_chassis__name__icontains=value) + | Q(virtual_machine__name__icontains=value) ) return queryset.filter(query) From d861b972de7dbf28efc26d79a9540290b65679bb Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Sun, 24 Aug 2025 16:26:21 +0200 Subject: [PATCH 24/35] refactor(forms): Migrate forms to use ACLAssignment model Replaces `ACLInterfaceAssignmentForm` with `ACLAssignmentForm` across forms to align with the unified ACLAssignment model. Removes legacy fields and validation logic, introducing centralized validation for assigned objects and directions. Simplifies fieldsets and enhances maintainability by leveraging the consolidated model structure. --- netbox_acls/forms/filtersets.py | 85 ++---- netbox_acls/forms/models.py | 479 +++++++------------------------- 2 files changed, 124 insertions(+), 440 deletions(-) diff --git a/netbox_acls/forms/filtersets.py b/netbox_acls/forms/filtersets.py index c1392f97..2a7a21ee 100644 --- a/netbox_acls/forms/filtersets.py +++ b/netbox_acls/forms/filtersets.py @@ -26,13 +26,13 @@ from ..models import ( AccessList, ACLExtendedRule, - ACLInterfaceAssignment, + ACLAssignment, ACLStandardRule, ) __all__ = ( "AccessListFilterForm", - "ACLInterfaceAssignmentFilterForm", + "ACLAssignmentFilterForm", "ACLStandardRuleFilterForm", "ACLExtendedRuleFilterForm", ) @@ -55,21 +55,6 @@ class AccessListFilterForm(NetBoxModelFilterSetForm): "default_action", name=_("ACL Details"), ), - FieldSet( - "region_id", - "site_group_id", - "site_id", - "device_id", - name=_("Device Details"), - ), - FieldSet( - "virtual_chassis_id", - name=_("Virtual Chassis Details"), - ), - FieldSet( - "virtual_machine_id", - name=_("Virtual Machine Details"), - ), ) # ACL selector @@ -84,61 +69,16 @@ class AccessListFilterForm(NetBoxModelFilterSetForm): label=_("Default Action"), ) - # Device selector - region_id = DynamicModelChoiceField( - queryset=Region.objects.all(), - required=False, - label=_("Region"), - ) - site_group_id = DynamicModelChoiceField( - queryset=SiteGroup.objects.all(), - required=False, - label=_("Site Group"), - ) - site_id = DynamicModelChoiceField( - queryset=Site.objects.all(), - required=False, - query_params={ - "region_id": "$region_id", - "group_id": "$site_group_id", - }, - label=_("Site"), - ) - device_id = DynamicModelChoiceField( - queryset=Device.objects.all(), - query_params={ - "region_id": "$region_id", - "group_id": "$site_group_id", - "site_id": "$site_id", - }, - required=False, - label=_("Device"), - ) - - # Virtual Chassis selector - virtual_chassis_id = DynamicModelChoiceField( - queryset=VirtualChassis.objects.all(), - required=False, - label=_("Virtual Chassis"), - ) - - # Virtual Machine selector - virtual_machine_id = DynamicModelChoiceField( - queryset=VirtualMachine.objects.all(), - required=False, - label=_("Virtual Machine"), - ) - # Tag selector tag = TagFilterField(model) -class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm): +class ACLAssignmentFilterForm(NetBoxModelFilterSetForm): """ - GUI filter form to search the django AccessList model. + GUI filter form to search the django ACLAssignment model. """ - model = ACLInterfaceAssignment + model = ACLAssignment fieldsets = ( FieldSet( "q", @@ -158,6 +98,10 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm): "interface_id", name=_("Device Details"), ), + FieldSet( + "virtual_chassis_id", + name=_("Virtual Chassis Details"), + ), FieldSet( "virtual_machine_id", "vminterface_id", @@ -177,7 +121,7 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm): label=_("Direction"), ) - # Device Interface selector + # Device and Interface selectors region_id = DynamicModelChoiceField( queryset=Region.objects.all(), required=False, @@ -216,7 +160,14 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm): label=_("Device Interface"), ) - # Virtual Machine Interface selector + # Virtual Chassis selector + virtual_chassis_id = DynamicModelChoiceField( + queryset=VirtualChassis.objects.all(), + required=False, + label=_("Virtual Chassis"), + ) + + # Virtual Machine and VM Interface selectors virtual_machine_id = DynamicModelChoiceField( queryset=VirtualMachine.objects.all(), required=False, diff --git a/netbox_acls/forms/models.py b/netbox_acls/forms/models.py index 6e16bc21..9e434eb6 100644 --- a/netbox_acls/forms/models.py +++ b/netbox_acls/forms/models.py @@ -2,7 +2,8 @@ Defines each django model's GUI form to add or edit objects for each django model. """ -from dcim.models import Device, Interface, Region, Site, SiteGroup, VirtualChassis +from django import forms +from dcim.models import Device, Interface from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.utils.safestring import mark_safe @@ -11,35 +12,30 @@ from netbox.forms import NetBoxModelForm from utilities.forms import ( get_field_value, + add_blank_choice, ) -from utilities.forms.fields import ( - CommentField, - ContentTypeChoiceField, - DynamicModelChoiceField, -) -from utilities.forms.rendering import FieldSet, TabbedGroups +from utilities.forms.fields import CommentField, DynamicModelChoiceField, ContentTypeChoiceField +from utilities.forms.rendering import FieldSet from utilities.forms.widgets import HTMXSelect from utilities.templatetags.builtins.filters import bettertitle -from virtualization.models import ( - Cluster, - ClusterGroup, - ClusterType, - VirtualMachine, - VMInterface, -) +from virtualization.models import VMInterface -from ..choices import ACLTypeChoices -from ..constants import ACL_RULE_SOURCE_DESTINATION_MODELS +from ..constants import ACL_ASSIGNMENT_MODELS, ACL_RULE_SOURCE_DESTINATION_MODELS +from ..choices import ( + ACLAssignmentDirectionChoices, + ACLAssignmentDirectionUIChoices, + ACLTypeChoices, +) from ..models import ( AccessList, ACLExtendedRule, - ACLInterfaceAssignment, + ACLAssignment, ACLStandardRule, ) __all__ = ( "AccessListForm", - "ACLInterfaceAssignmentForm", + "ACLAssignmentForm", "ACLStandardRuleForm", "ACLExtendedRuleForm", ) @@ -60,67 +56,6 @@ class AccessListForm(NetBoxModelForm): Requires a device, a name, a type, and a default_action. """ - # Device selector - region = DynamicModelChoiceField( - queryset=Region.objects.all(), - required=False, - initial_params={ - "sites": "$site", - }, - ) - site_group = DynamicModelChoiceField( - queryset=SiteGroup.objects.all(), - required=False, - label="Site Group", - initial_params={"sites": "$site"}, - ) - site = DynamicModelChoiceField( - queryset=Site.objects.all(), - required=False, - query_params={"region_id": "$region", "group_id": "$site_group"}, - ) - device = DynamicModelChoiceField( - queryset=Device.objects.all(), - required=False, - query_params={ - "region_id": "$region", - "group_id": "$site_group", - "site_id": "$site", - }, - ) - - # Virtual Chassis selector - virtual_chassis = DynamicModelChoiceField( - queryset=VirtualChassis.objects.all(), - required=False, - label="Virtual Chassis", - ) - - # Virtual Machine selector - cluster_type = DynamicModelChoiceField( - queryset=ClusterType.objects.all(), - required=False, - ) - cluster_group = DynamicModelChoiceField( - queryset=ClusterGroup.objects.all(), - required=False, - query_params={"type_id": "$cluster_type"}, - ) - cluster = DynamicModelChoiceField( - queryset=Cluster.objects.all(), - required=False, - query_params={"type_id": "$cluster_type", "group_id": "$cluster_group"}, - ) - - virtual_machine = DynamicModelChoiceField( - queryset=VirtualMachine.objects.all(), - required=False, - query_params={ - "cluster_id": "$cluster", - "cluster_type_id": "$cluster_type", - "cluster_group_id": "$cluster_group", - }, - ) comments = CommentField() fieldsets = ( @@ -131,40 +66,11 @@ class AccessListForm(NetBoxModelForm): "tags", name=_("Access List Details"), ), - FieldSet( - TabbedGroups( - FieldSet( - "region", - "site_group", - "site", - "device", - name=_("Device"), - ), - FieldSet( - "virtual_chassis", - name=_("Virtual Chassis"), - ), - FieldSet( - "cluster_type", - "cluster_group", - "cluster", - "virtual_machine", - name=_("Virtual Machine"), - ), - ), - name=_("Host Assignment"), - ), ) class Meta: model = AccessList fields = ( - "region", - "site_group", - "site", - "device", - "virtual_machine", - "virtual_chassis", "name", "type", "default_action", @@ -173,319 +79,146 @@ class Meta: ) help_texts = { - "default_action": "The default behavior of the ACL.", - "name": "The name uniqueness per device is case insensitive.", + "default_action": _("The default behavior of the ACL."), + "name": _("The name uniqueness per device is case insensitive."), "type": mark_safe( - "*Note: CANNOT be changed if ACL Rules are associated to this Access List.", + _("*Note: CANNOT be changed if ACL Rules are associated to this Access List."), ), } - def __init__(self, *args, **kwargs): - # Initialize helper selectors - instance = kwargs.get("instance") - initial = kwargs.get("initial", {}).copy() - if instance: - if isinstance(instance.assigned_object, Device): - initial["device"] = instance.assigned_object - if instance.assigned_object.site: - initial["site"] = instance.assigned_object.site - if instance.assigned_object.site.group: - initial["site_group"] = instance.assigned_object.site.group - - if instance.assigned_object.site.region: - initial["region"] = instance.assigned_object.site.region - elif isinstance(instance.assigned_object, VirtualMachine): - initial["virtual_machine"] = instance.assigned_object - if instance.assigned_object.cluster: - initial["cluster"] = instance.assigned_object.cluster - if instance.assigned_object.cluster.group: - initial["cluster_group"] = instance.assigned_object.cluster.group - - if instance.assigned_object.cluster.type: - initial["cluster_type"] = instance.assigned_object.cluster.type - elif isinstance(instance.assigned_object, VirtualChassis): - initial["virtual_chassis"] = instance.assigned_object - - kwargs["initial"] = initial - super().__init__(*args, **kwargs) - def clean(self): """ - Validates form inputs before submitting: - - Check if more than one host type selected. - - Check if no hosts selected. - - Check if duplicate entry. (Because of GFK.) - - Check if Access List has no existing rules before change the Access List's type. + Validates and cleans the input data for the current object. + + Ensures that the type of the Access Control List (ACL) cannot be + altered if there are existing rules associated with the current ACL. """ super().clean() - if self.errors.get("name"): - return - - name = self.cleaned_data.get("name") acl_type = self.cleaned_data.get("type") - device = self.cleaned_data.get("device") - virtual_chassis = self.cleaned_data.get("virtual_chassis") - virtual_machine = self.cleaned_data.get("virtual_machine") - - # Check if more than one host type selected. - if (device and virtual_chassis) or (device and virtual_machine) or (virtual_chassis and virtual_machine): - raise ValidationError( - { - "__all__": ( - "Access Lists must be assigned to one host at a time. Either a device, virtual chassis or " - "virtual machine." - ), - }, - ) - - # Check if no hosts selected. - if not device and not virtual_chassis and not virtual_machine: - raise ValidationError( - {"__all__": "Access Lists must be assigned to a device, virtual chassis or virtual machine."} - ) - - existing_acls = None - host_type = None - if device: - host_type = "device" - existing_acls = AccessList.objects.filter(name=name, device=device).exists() - elif virtual_machine: - host_type = "virtual_machine" - existing_acls = AccessList.objects.filter(name=name, virtual_machine=virtual_machine).exists() - elif virtual_chassis: - host_type = "virtual_chassis" - existing_acls = AccessList.objects.filter(name=name, virtual_chassis=virtual_chassis).exists() - - # Check if duplicate entry. - if ("name" in self.changed_data or host_type in self.changed_data) and existing_acls: - error_same_acl_name = "An ACL with this name is already associated to this host." - raise ValidationError({host_type: [error_same_acl_name], "name": [error_same_acl_name]}) - - # Check if Access List has no existing rules before change the Access List's type. + + # Check if Access List has no existing rules before change the + # Access List's type. if self.instance.pk and ( (acl_type == ACLTypeChoices.TYPE_EXTENDED and self.instance.aclstandardrules.exists()) or (acl_type == ACLTypeChoices.TYPE_STANDARD and self.instance.aclextendedrules.exists()) ): - raise ValidationError({"type": ["This ACL has ACL rules associated, CANNOT change ACL type."]}) - - def save(self, *args, **kwargs): - # Set assigned object - self.instance.assigned_object = ( - self.cleaned_data.get("device") - or self.cleaned_data.get("virtual_chassis") - or self.cleaned_data.get("virtual_machine") - ) - - return super().save(*args, **kwargs) + raise ValidationError({"type": _("This ACL has ACL rules associated, CANNOT change ACL type.")}) -class ACLInterfaceAssignmentForm(NetBoxModelForm): +class ACLAssignmentForm(NetBoxModelForm): """ - GUI form to add or edit ACL Host Object assignments + GUI form to add or edit ACL assignments Requires an access_list, a name, a type, and a default_action. """ - device = DynamicModelChoiceField( - queryset=Device.objects.all(), - required=False, - # query_params={ - # Need to pass ACL device to it - # }, - ) - interface = DynamicModelChoiceField( - queryset=Interface.objects.all(), - required=False, - query_params={ - "device_id": "$device", - }, - ) - virtual_machine = DynamicModelChoiceField( - queryset=VirtualMachine.objects.all(), - required=False, - # query_params={ - # Need to pass ACL device to it - # }, - label="Virtual Machine", - ) - vminterface = DynamicModelChoiceField( - queryset=VMInterface.objects.all(), - required=False, - query_params={ - "virtual_machine_id": "$virtual_machine", - }, - label="VM Interface", - ) - # virtual_chassis = DynamicModelChoiceField( - # queryset=VirtualChassis.objects.all(), - # required=False, - # label='Virtual Chassis', - # ) access_list = DynamicModelChoiceField( queryset=AccessList.objects.all(), - # query_params={ - # 'assigned_object': '$device', - # 'assigned_object': '$virtual_machine', - # }, label="Access List", help_text=mark_safe( "*Note: Access List must be present on the device already.", ), ) + assigned_object_type = ContentTypeChoiceField( + queryset=ContentType.objects.filter(ACL_ASSIGNMENT_MODELS), + widget=HTMXSelect(), + label=_("Assignment Object Type"), + ) + assigned_object = DynamicModelChoiceField( + queryset=Device.objects.none(), # Initial queryset + selector=True, + label=_("Assignment Object"), + disabled=True, + ) + direction = forms.ChoiceField( + choices=add_blank_choice(ACLAssignmentDirectionChoices), + required=False, + label=_("Direction"), + help_text=_( + "The ACL assignment direction field is only enabled for " + "Device Interface or Virtual Machine Interface objects. " + "For other types (such as Device, Virtual Chassis, or Virtual Machine), " + "this field is disabled. " + "*Note: CANNOT assign 2 ACLs to the same interface & direction." + ), + disabled=True, + ) comments = CommentField() fieldsets = ( FieldSet( "access_list", - "direction", "tags", name=_("Access List Details"), ), FieldSet( - TabbedGroups( - FieldSet( - "device", - "interface", - name=_("Device"), - ), - FieldSet( - "virtual_machine", - "vminterface", - name=_("Virtual Machine"), - ), - ), - name=_("Interface Assignment"), + "assigned_object_type", + "assigned_object", + "direction", + name=_("Assignment"), ), ) - class Meta: - model = ACLInterfaceAssignment - fields = ( - "access_list", - "direction", - "device", - "interface", - "virtual_machine", - "vminterface", - "comments", - "tags", - ) - - help_texts = { - "direction": mark_safe( - "*Note: CANNOT assign 2 ACLs to the same interface & direction.", - ), - } + def __init__(self, *args, **kwargs) -> None: + """ + Initialize the ACL Assignment form. + """ - def __init__(self, *args, **kwargs): - # Initialize helper selectors + # Initialize fields with initial values instance = kwargs.get("instance") initial = kwargs.get("initial", {}).copy() - if instance: - if type(instance.assigned_object) is Interface: - initial["interface"] = instance.assigned_object - initial["device"] = "device" - elif type(instance.assigned_object) is VMInterface: - initial["vminterface"] = instance.assigned_object - initial["virtual_machine"] = "virtual_machine" + initial["direction"] = ACLAssignmentDirectionChoices.DIRECTION_NONE + + if instance is not None and instance.assigned_object: + # Initialize the assigned object field + initial["assigned_object"] = instance.assigned_object + initial["direction"] = instance.direction + kwargs["initial"] = initial super().__init__(*args, **kwargs) - def clean(self): - """ - Validates form inputs before submitting: - - Check if both interface and vminterface are set. - - Check if neither interface nor vminterface are set. - - Check that an interface's parent device/virtual_machine is assigned to the Access List. - - Check that an interface's parent device/virtual_machine is assigned to the Access List. - - Check for duplicate entry. (Because of GFK) - - Check that the interface does not have an existing ACL applied in the direction already. - """ + if assigned_object_type_id := get_field_value(self, "assigned_object_type"): + try: + # Retrieve the ContentType model class based on the assigned object type + assigned_object_type = ContentType.objects.get(pk=assigned_object_type_id) + assigned_object_model = assigned_object_type.model_class() + + # Configure the queryset and label for the assigned_object field + self.fields["assigned_object"].queryset = assigned_object_model.objects.all() + self.fields["assigned_object"].widget.attrs["selector"] = assigned_object_model._meta.label_lower + self.fields["assigned_object"].disabled = False + self.fields["assigned_object"].label = _(bettertitle(assigned_object_model._meta.verbose_name)) + if assigned_object_model == Interface or assigned_object_model == VMInterface: + self.fields["direction"].disabled = False + self.fields["direction"].required = True + self.fields["direction"].choices = add_blank_choice(ACLAssignmentDirectionUIChoices) + else: + self.fields["direction"].disabled = True + self.fields["direction"].widget.attrs["value"] = "None" + except ObjectDoesNotExist: + pass + + # Clears the assigned_object field if the selected type changes + if self.instance and self.instance.pk and assigned_object_type_id != self.instance.assigned_object_type_id: + self.initial["assigned_object"] = None + + def clean(self) -> None: + """Validate form fields for the ACL Assignment form.""" super().clean() - error_message = {} - access_list = self.cleaned_data.get("access_list") - direction = self.cleaned_data.get("direction") - interface = self.cleaned_data.get("interface") - vminterface = self.cleaned_data.get("vminterface") - - # Check if both interface and vminterface are set. - if interface and vminterface: - error_too_many_interfaces = ( - "Access Lists must be assigned to one type of interface at a time (VM interface or physical interface)" - ) - error_message |= { - "interface": [error_too_many_interfaces], - "vminterface": [error_too_many_interfaces], - } - elif not (interface or vminterface): - error_no_interface = "An Access List assignment but specify an Interface or VM Interface." - error_message |= { - "interface": [error_no_interface], - "vminterface": [error_no_interface], - } - else: - # Define assigned_object, assigned_object_type, host_type, and host based on interface or vminterface - if interface: - assigned_object = interface - assigned_object_type = "interface" - host_type = "device" - host = Interface.objects.get(pk=assigned_object.pk).device - else: - assigned_object = vminterface - assigned_object_type = "vminterface" - host_type = "virtual_machine" - host = VMInterface.objects.get(pk=assigned_object.pk).virtual_machine - - assigned_object_id = assigned_object.pk - assigned_object_type_id = ContentType.objects.get_for_model(assigned_object).pk - access_list_host = AccessList.objects.get(pk=access_list.pk).assigned_object - - # Check that an interface's parent device/virtual_machine is assigned to the Access List. - if access_list_host != host: - error_acl_not_assigned_to_host = "Access List not present on selected host." - error_message |= { - "access_list": [error_acl_not_assigned_to_host], - assigned_object_type: [error_acl_not_assigned_to_host], - host_type: [error_acl_not_assigned_to_host], - } - - # Check for duplicate entry and existing ACL in the direction. - existing_acl = ACLInterfaceAssignment.objects.filter( - access_list=access_list, - assigned_object_id=assigned_object_id, - assigned_object_type=assigned_object_type_id, - direction=direction, - ) - if existing_acl.exists(): - error_duplicate_entry = "An ACL with this name is already associated to this interface & direction." - error_message |= { - "access_list": [error_duplicate_entry], - "direction": [error_duplicate_entry], - assigned_object_type: [error_duplicate_entry], - } - - if ACLInterfaceAssignment.objects.filter( - assigned_object_id=assigned_object_id, - assigned_object_type=assigned_object_type_id, - direction=direction, - ).exists(): - error_interface_already_assigned = "Interfaces can only have 1 Access List assigned in each direction." - error_message |= { - "direction": [error_interface_already_assigned], - assigned_object_type: [error_interface_already_assigned], - } - - if error_message: - raise ValidationError(error_message) - - def save(self, *args, **kwargs): - # Set assigned object - self.instance.assigned_object = self.cleaned_data.get( - "interface", - ) or self.cleaned_data.get("vminterface") - return super().save(*args, **kwargs) + # Ensure the selected object gets assigned + self.instance.assigned_object = self.cleaned_data.get("assigned_object") + + class Meta: + model = ACLAssignment + fields = ( + "access_list", + "assigned_object_type", + "direction", + "comments", + "tags", + ) class ACLStandardRuleForm(NetBoxModelForm): From 1a2eff5d9eb54b2669e5cc13fba62a10c94158df Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Mon, 25 Aug 2025 19:47:11 +0200 Subject: [PATCH 25/35] refactor(templates): Update templates to use ACLAssignment model Replaces references to `ACLInterfaceAssignment` with `ACLAssignment` in templates, aligning with the unified model. Updates template structure to streamline assigned object display and improve maintainability. Removes legacy elements no longer applicable after the migration. --- netbox_acls/templates/inc/view_tab.html | 29 ++++--------------- .../templates/netbox_acls/accesslist.html | 4 --- ...faceassignment.html => aclassignment.html} | 25 +++++++--------- 3 files changed, 16 insertions(+), 42 deletions(-) rename netbox_acls/templates/netbox_acls/{aclinterfaceassignment.html => aclassignment.html} (70%) diff --git a/netbox_acls/templates/inc/view_tab.html b/netbox_acls/templates/inc/view_tab.html index e86cb3b3..511ba2ba 100644 --- a/netbox_acls/templates/inc/view_tab.html +++ b/netbox_acls/templates/inc/view_tab.html @@ -1,30 +1,11 @@ -{% extends 'generic/object.html' %} -{% load buttons %} +{% extends 'generic/object_children.html' %} {% load helpers %} -{% load plugins %} -{% load render_table from django_tables2 %} +{% load i18n %} {% block extra_controls %} - {% if perms.netbox_todo.add_accesslist %} - - Add Access List + {% if perms.netbox_acls.add_accesslist %} + + {% trans "Assign an ACL" %} {% endif %} {% endblock extra_controls %} - -{% block content %} - {% include 'inc/table_controls_htmx.html' with table_modal=table_config %} -
- {% csrf_token %} -
-
- {% include 'htmx/table.html' %} -
-
-
-{% endblock content %} - -{% block modals %} - {{ block.super }} - {% table_config_form table %} -{% endblock modals %} diff --git a/netbox_acls/templates/netbox_acls/accesslist.html b/netbox_acls/templates/netbox_acls/accesslist.html index 27528f69..1c050b15 100644 --- a/netbox_acls/templates/netbox_acls/accesslist.html +++ b/netbox_acls/templates/netbox_acls/accesslist.html @@ -42,10 +42,6 @@

Access List

{% endif %} - - Assigned Host - {{ object.assigned_object|linkify }} - {% include 'inc/panels/custom_fields.html' %} diff --git a/netbox_acls/templates/netbox_acls/aclinterfaceassignment.html b/netbox_acls/templates/netbox_acls/aclassignment.html similarity index 70% rename from netbox_acls/templates/netbox_acls/aclinterfaceassignment.html rename to netbox_acls/templates/netbox_acls/aclassignment.html index 677e2f9e..be174a4b 100644 --- a/netbox_acls/templates/netbox_acls/aclinterfaceassignment.html +++ b/netbox_acls/templates/netbox_acls/aclassignment.html @@ -3,33 +3,30 @@ {% block breadcrumbs %} {{ block.super }} -
  • {{ object.access_list }}
    • +
  • {{ object.access_list }}
    • {% endblock breadcrumbs %} {% block content %}
    -

    ACL Interface Assignment

    +

    ACL Assignment

    - + + + + + - - - - - - - - From 5c00a57d2993707b03aa819bc7897c15caaf45dc Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Tue, 26 Aug 2025 20:14:38 +0200 Subject: [PATCH 26/35] refactor(views): Migrate views to ACLAssignment model Replaces `ACLInterfaceAssignment` views with updated `ACLAssignment` views. Introduces unified children views for devices, interfaces, VMs, and virtual chassis, improving maintainability and consistency. Enhances tab logic and table configurations to reflect the consolidated model structure. --- netbox_acls/views.py | 327 +++++++++++++++++++++++++------------------ 1 file changed, 190 insertions(+), 137 deletions(-) diff --git a/netbox_acls/views.py b/netbox_acls/views.py index 1a6b6528..6c70a8ff 100644 --- a/netbox_acls/views.py +++ b/netbox_acls/views.py @@ -3,6 +3,8 @@ Specifically, all the various interactions with a client. """ +from django.contrib.contenttypes.models import ContentType +from django.utils.translation import gettext_lazy as _ from dcim.models import Device, Interface, VirtualChassis from django.db.models import Count from netbox.views import generic @@ -17,11 +19,11 @@ "AccessListEditView", "AccessListDeleteView", "AccessListBulkDeleteView", - "ACLInterfaceAssignmentView", - "ACLInterfaceAssignmentListView", - "ACLInterfaceAssignmentEditView", - "ACLInterfaceAssignmentDeleteView", - "ACLInterfaceAssignmentBulkDeleteView", + "ACLAssignmentView", + "ACLAssignmentListView", + "ACLAssignmentEditView", + "ACLAssignmentDeleteView", + "ACLAssignmentBulkDeleteView", "ACLStandardRuleView", "ACLStandardRuleListView", "ACLStandardRuleEditView", @@ -34,6 +36,46 @@ "ACLExtendedRuleBulkDeleteView", ) +# +# Base children views +# + + +class ACLAssignmentChildrenView(generic.ObjectChildrenView): + """Base children view for attaching a tab of ACL Assignments.""" + + child_model = models.ACLAssignment + filterset = filtersets.ACLAssignmentFilterSet + tab = ViewTab( + label=_("Access Lists"), + badge=lambda obj: obj.aclassignments.count(), + permission="netbox_acls.view_aclassignment", + weight=1100, + ) + table = tables.ACLAssignmentTable + + def get_children(self, request, parent): + """ + Return all objects of ACLAssignment. + """ + return models.ACLAssignment.objects.restrict(request.user, "view").prefetch_related( + "access_list", + "assigned_object_type", + "assigned_object", + "tags", + ) + + def get_extra_context(self, request, instance) -> dict: + """ + Return ContentType as extra context. + """ + assigned_object_type = ContentType.objects.get_for_model(self.queryset.model) + + return super().get_extra_context(request, instance) | { + "add_url": "plugins:netbox_acls:aclassignment_add", + "content_type_id": assigned_object_type.id, + } + # # AccessList views @@ -112,118 +154,50 @@ class AccessListBulkDeleteView(generic.BulkDeleteView): table = tables.AccessListTable -class AccessListChildView(generic.ObjectChildrenView): - """ - Defines the child view for the AccessLists model. - """ - - child_model = models.AccessList - table = tables.AccessListTable - filterset = filtersets.AccessListFilterSet - template_name = "inc/view_tab.html" - - def get_extra_context(self, request, instance): - return { - "table_config": self.table.__name__, - "model_type": self.queryset.model._meta.verbose_name.replace(" ", "_"), - "add_url": "plugins:netbox_acls:accesslist_add", - } - - def prep_table_data(self, request, queryset, parent): - return queryset.annotate( - rule_count=Count("aclextendedrules") + Count("aclstandardrules"), - ) - - -@register_model_view(Device, "access_lists") -class DeviceAccessListView(AccessListChildView): - queryset = Device.objects.prefetch_related("tags") - tab = ViewTab( - label="Access Lists", - badge=lambda obj: models.AccessList.objects.filter(device=obj).count(), - permission="netbox_acls.view_accesslist", - ) - - def get_children(self, request, parent): - return self.child_model.objects.restrict(request.user, "view").filter( - device=parent, - ) - - -@register_model_view(VirtualChassis, "access_lists") -class VirtualChassisAccessListView(AccessListChildView): - queryset = VirtualChassis.objects.prefetch_related("tags") - tab = ViewTab( - label="Access Lists", - badge=lambda obj: models.AccessList.objects.filter(virtual_chassis=obj).count(), - permission="netbox_acls.view_accesslist", - ) - - def get_children(self, request, parent): - return self.child_model.objects.restrict(request.user, "view").filter( - virtual_chassis=parent, - ) - - -@register_model_view(VirtualMachine, "access_lists") -class VirtualMachineAccessListView(AccessListChildView): - queryset = VirtualMachine.objects.prefetch_related("tags") - tab = ViewTab( - label="Access Lists", - badge=lambda obj: models.AccessList.objects.filter(virtual_machine=obj).count(), - permission="netbox_acls.view_accesslist", - ) - - def get_children(self, request, parent): - return self.child_model.objects.restrict(request.user, "view").filter( - virtual_machine=parent, - ) - - # -# ACLInterfaceAssignment views +# ACLAssignment views # -@register_model_view(models.ACLInterfaceAssignment) -class ACLInterfaceAssignmentView(generic.ObjectView): +@register_model_view(models.ACLAssignment) +class ACLAssignmentView(generic.ObjectView): """ - Defines the view for the ACLInterfaceAssignments django model. + Defines the view for the ACLAssignments django model. """ - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) -@register_model_view(models.ACLInterfaceAssignment, "list", path="", detail=False) -class ACLInterfaceAssignmentListView(generic.ObjectListView): +@register_model_view(models.ACLAssignment, "list", path="", detail=False) +class ACLAssignmentListView(generic.ObjectListView): """ - Defines the list view for the ACLInterfaceAssignments django model. + Defines the list view for the ACLAssignments django model. """ - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) - table = tables.ACLInterfaceAssignmentTable - filterset = filtersets.ACLInterfaceAssignmentFilterSet - filterset_form = forms.ACLInterfaceAssignmentFilterForm + table = tables.ACLAssignmentTable + filterset = filtersets.ACLAssignmentFilterSet + filterset_form = forms.ACLAssignmentFilterForm -@register_model_view(models.ACLInterfaceAssignment, "add", detail=False) -@register_model_view(models.ACLInterfaceAssignment, "edit") -class ACLInterfaceAssignmentEditView(generic.ObjectEditView): +@register_model_view(models.ACLAssignment, "add", detail=False) +@register_model_view(models.ACLAssignment, "edit") +class ACLAssignmentEditView(generic.ObjectEditView): """ - Defines the edit view for the ACLInterfaceAssignments django model. + Defines the edit view for the ACLAssignments django model. """ - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) - form = forms.ACLInterfaceAssignmentForm + form = forms.ACLAssignmentForm def get_extra_addanother_params(self, request): """ @@ -236,78 +210,157 @@ def get_extra_addanother_params(self, request): } -@register_model_view(models.ACLInterfaceAssignment, "delete") -class ACLInterfaceAssignmentDeleteView(generic.ObjectDeleteView): +@register_model_view(models.ACLAssignment, "delete") +class ACLAssignmentDeleteView(generic.ObjectDeleteView): """ - Defines delete view for the ACLInterfaceAssignments django model. + Defines delete view for the ACLAssignments django model. """ - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) -@register_model_view(models.ACLInterfaceAssignment, "bulk_delete", path="delete", detail=False) -class ACLInterfaceAssignmentBulkDeleteView(generic.BulkDeleteView): - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( +@register_model_view(models.ACLAssignment, "bulk_delete", path="delete", detail=False) +class ACLAssignmentBulkDeleteView(generic.BulkDeleteView): + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) - filterset = filtersets.ACLInterfaceAssignmentFilterSet - table = tables.ACLInterfaceAssignmentTable + filterset = filtersets.ACLAssignmentFilterSet + table = tables.ACLAssignmentTable -class ACLInterfaceAssignmentChildView(generic.ObjectChildrenView): +@register_model_view(Device, "aclassignments", path="access-lists") +class DeviceACLAssignmentView(ACLAssignmentChildrenView): """ - Defines the child view for the ACLInterfaceAssignments model. + Children view of ACL Assignment of Devices. """ - child_model = models.ACLInterfaceAssignment - table = tables.ACLInterfaceAssignmentTable - filterset = filtersets.ACLInterfaceAssignmentFilterSet + queryset = Device.objects.all() template_name = "inc/view_tab.html" - def get_extra_context(self, request, instance): - return { - "table_config": self.table.__name__, - "model_type": self.queryset.model._meta.verbose_name.replace(" ", "_"), - "add_url": "plugins:netbox_acls:aclinterfaceassignment_add", - } + def get_children(self, request, parent): + """Return all children objects to the current parent object.""" + return super().get_children(request, parent).filter(device=parent.pk) + def get_table(self, *args, **kwargs): + """Return the table with the assigned object colum hidden.""" + table = super().get_table(*args, **kwargs) -@register_model_view(Interface, "acl_interface_assignments") -class InterfaceACLInterfaceAssignmentView(ACLInterfaceAssignmentChildView): - queryset = Interface.objects.prefetch_related("device", "tags") - tab = ViewTab( - label="ACL Interface Assignments", - badge=lambda obj: models.ACLInterfaceAssignment.objects.filter( - interface=obj, - ).count(), - permission="netbox_acls.view_aclinterfaceassignment", - ) + # Hide the assigned object type column + table.columns.hide("assigned_object_type") + # Hide the assigned object column + table.columns.hide("assigned_object") + # Hide the direction column + table.columns.hide("direction") + + return table + + +@register_model_view(Interface, "aclassignments", path="access-lists") +class InterfaceACLAssignmentView(ACLAssignmentChildrenView): + """ + Children view of ACL Assignment of Interfaces. + """ + + queryset = Interface.objects.all() + template_name = "inc/view_tab.html" def get_children(self, request, parent): - return self.child_model.objects.restrict(request.user, "view").filter( - interface=parent, - ) + """Return all children objects to the current parent object.""" + return super().get_children(request, parent).filter(interface=parent.pk) + def get_table(self, *args, **kwargs): + """Return the table with the assigned object colum hidden.""" + table = super().get_table(*args, **kwargs) + + # Hide the assigned object type column + table.columns.hide("assigned_object_type") + # Hide the assigned object column + table.columns.hide("assigned_object") + + return table -@register_model_view(VMInterface, "acl_interface_assignments") -class VirtualMachineInterfaceACLInterfaceAssignmentView(ACLInterfaceAssignmentChildView): - queryset = VMInterface.objects.prefetch_related("virtual_machine", "tags") - tab = ViewTab( - label="ACL Interface Assignments", - badge=lambda obj: models.ACLInterfaceAssignment.objects.filter( - vminterface=obj, - ).count(), - permission="netbox_acls.view_aclinterfaceassignment", - ) + +@register_model_view(VirtualChassis, "aclassignments", path="access-lists") +class VirtualChassisACLAssignmentView(ACLAssignmentChildrenView): + """ + Children view of ACL Assignment of VirtualChassiss. + """ + + queryset = VirtualChassis.objects.all() + template_name = "inc/view_tab.html" def get_children(self, request, parent): - return self.child_model.objects.restrict(request.user, "view").filter( - vminterface=parent, - ) + """Return all children objects to the current parent object.""" + return super().get_children(request, parent).filter(virtual_chassis=parent.pk) + + def get_table(self, *args, **kwargs): + """Return the table with the assigned object colum hidden.""" + table = super().get_table(*args, **kwargs) + + # Hide the assigned object type column + table.columns.hide("assigned_object_type") + # Hide the assigned object column + table.columns.hide("assigned_object") + # Hide the direction column + table.columns.hide("direction") + + return table + + +@register_model_view(VirtualMachine, "aclassignments", path="access-lists") +class VirtualMachineACLAssignmentView(ACLAssignmentChildrenView): + """ + Children view of ACL Assignment of VirtualMachines. + """ + + queryset = VirtualMachine.objects.all() + template_name = "inc/view_tab.html" + + def get_children(self, request, parent): + """Return all children objects to the current parent object.""" + return super().get_children(request, parent).filter(virtual_machine=parent.pk) + + def get_table(self, *args, **kwargs): + """Return the table with the assigned object colum hidden.""" + table = super().get_table(*args, **kwargs) + + # Hide the assigned object type column + table.columns.hide("assigned_object_type") + # Hide the assigned object column + table.columns.hide("assigned_object") + # Hide the direction column + table.columns.hide("direction") + + return table + + +@register_model_view(VMInterface, "aclassignments", path="access-lists") +class VMInterfaceACLAssignmentView(ACLAssignmentChildrenView): + """ + Children view of ACL Assignment of VMInterfaces. + """ + + queryset = VMInterface.objects.all() + template_name = "inc/view_tab.html" + + def get_children(self, request, parent): + """Return all children objects to the current parent object.""" + return super().get_children(request, parent).filter(vminterface=parent.pk) + + def get_table(self, *args, **kwargs): + """Return the table with the assigned object colum hidden.""" + table = super().get_table(*args, **kwargs) + + # Hide the assigned object type column + table.columns.hide("assigned_object_type") + # Hide the assigned object column + table.columns.hide("assigned_object") + + return table # From 31e64abdef2ddf3930fe89bdf5ee4642fe1a046a Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Tue, 26 Aug 2025 20:18:06 +0200 Subject: [PATCH 27/35] refactor(urls): Update routes to align with ACLAssignment model Replaces `ACLInterfaceAssignment` routes with `ACLAssignment` routes in URLs. Shortens path names for clarity and consistency with the unified model. --- netbox_acls/urls.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/netbox_acls/urls.py b/netbox_acls/urls.py index 24f8bdf9..2e521af8 100644 --- a/netbox_acls/urls.py +++ b/netbox_acls/urls.py @@ -19,12 +19,12 @@ ), # Access List Interface Assignments path( - "interface-assignments/", - include(get_model_urls("netbox_acls", "aclinterfaceassignment", detail=False)), + "assignments/", + include(get_model_urls("netbox_acls", "aclassignment", detail=False)), ), path( - "interface-assignments//", - include(get_model_urls("netbox_acls", "aclinterfaceassignment")), + "assignments//", + include(get_model_urls("netbox_acls", "aclassignment")), ), # Standard Access List Rules path( From 400df2b856d16bd9aa3af6e78556cddd2f023376 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 27 Aug 2025 18:12:33 +0200 Subject: [PATCH 28/35] refactor(serializers): Update serializers to use ACLAssignment model Replaces `ACLInterfaceAssignmentSerializer` with `ACLAssignmentSerializer` to align with the unified ACLAssignment model. Removes legacy fields and validation logic that depended on outdated models. Enhances maintainability by consolidating serializers under the updated model structure. BREAKING CHANGE: Legacy ACLInterfaceAssignmentSerializer is replaced with ACLAssignmentSerializer. --- netbox_acls/api/serializers.py | 65 +++++----------------------------- 1 file changed, 9 insertions(+), 56 deletions(-) diff --git a/netbox_acls/api/serializers.py b/netbox_acls/api/serializers.py index 56102748..f7eadcdd 100644 --- a/netbox_acls/api/serializers.py +++ b/netbox_acls/api/serializers.py @@ -10,21 +10,17 @@ from rest_framework import serializers from utilities.api import get_serializer_for_model -from ..constants import ( - ACL_HOST_ASSIGNMENT_MODELS, - ACL_INTERFACE_ASSIGNMENT_MODELS, - ACL_RULE_SOURCE_DESTINATION_MODELS, -) +from ..constants import ACL_ASSIGNMENT_MODELS, ACL_RULE_SOURCE_DESTINATION_MODELS from ..models import ( AccessList, ACLExtendedRule, - ACLInterfaceAssignment, + ACLAssignment, ACLStandardRule, ) __all__ = [ "AccessListSerializer", - "ACLInterfaceAssignmentSerializer", + "ACLAssignmentSerializer", "ACLStandardRuleSerializer", "ACLExtendedRuleSerializer", ] @@ -48,10 +44,6 @@ class AccessListSerializer(NetBoxModelSerializer): view_name="plugins-api:netbox_acls-api:accesslist-detail", ) rule_count = serializers.IntegerField(read_only=True) - assigned_object_type = ContentTypeField( - queryset=ContentType.objects.filter(ACL_HOST_ASSIGNMENT_MODELS), - ) - assigned_object = serializers.SerializerMethodField(read_only=True) class Meta: """ @@ -64,9 +56,6 @@ class Meta: "url", "display", "name", - "assigned_object_type", - "assigned_object_id", - "assigned_object", "type", "default_action", "comments", @@ -78,14 +67,6 @@ class Meta: ) brief_fields = ("id", "url", "display", "name") - @extend_schema_field(serializers.JSONField(allow_null=True)) - def get_assigned_object(self, obj): - if obj.assigned_object is None: - return None - serializer = get_serializer_for_model(obj.assigned_object) - context = {"request": self.context["request"]} - return serializer(obj.assigned_object, nested=True, context=context).data - def validate(self, data): """ Validates api inputs before processing: @@ -106,26 +87,26 @@ def validate(self, data): return super().validate(data) -class ACLInterfaceAssignmentSerializer(NetBoxModelSerializer): +class ACLAssignmentSerializer(NetBoxModelSerializer): """ - Defines the serializer for the django ACLInterfaceAssignment model and associates it with a view. + Defines the serializer for the django ACLAssignment model and associates it with a view. """ url = serializers.HyperlinkedIdentityField( - view_name="plugins-api:netbox_acls-api:aclinterfaceassignment-detail", + view_name="plugins-api:netbox_acls-api:aclassignment-detail", ) access_list = AccessListSerializer(nested=True, required=True) assigned_object_type = ContentTypeField( - queryset=ContentType.objects.filter(ACL_INTERFACE_ASSIGNMENT_MODELS), + queryset=ContentType.objects.filter(ACL_ASSIGNMENT_MODELS), ) assigned_object = serializers.SerializerMethodField(read_only=True) class Meta: """ - Associates the django model ACLInterfaceAssignment & fields to the serializer. + Associates the django model ACLAssignment & fields to the serializer. """ - model = ACLInterfaceAssignment + model = ACLAssignment fields = ( "id", "url", @@ -151,34 +132,6 @@ def get_assigned_object(self, obj): context = {"request": self.context["request"]} return serializer(obj.assigned_object, nested=True, context=context).data - def validate(self, data): - """ - Validate the AccessList django model's inputs before allowing it to update the instance. - - Check that the GFK object is valid. - - Check that the associated interface's parent host has the selected ACL defined. - """ - error_message = {} - acl_host = data["access_list"].assigned_object - - if data["assigned_object_type"].model == "interface": - interface_host = data["assigned_object_type"].get_object_for_this_type(id=data["assigned_object_id"]).device - elif data["assigned_object_type"].model == "vminterface": - interface_host = ( - data["assigned_object_type"].get_object_for_this_type(id=data["assigned_object_id"]).virtual_machine - ) - else: - interface_host = None - # Check that the associated interface's parent host has the selected ACL defined. - if acl_host != interface_host: - error_acl_not_assigned_to_host = "Access List not present on the selected interface's host." - error_message["access_list"] = [error_acl_not_assigned_to_host] - error_message["assigned_object_id"] = [error_acl_not_assigned_to_host] - - if error_message: - raise serializers.ValidationError(error_message) - - return super().validate(data) - class ACLStandardRuleSerializer(NetBoxModelSerializer): """ From 9427e2734771ba7818d00dd55836b36a66dd97c9 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 27 Aug 2025 19:42:38 +0200 Subject: [PATCH 29/35] refactor(views): Replace ACLInterfaceAssignment with ACLAssignment Renames `ACLInterfaceAssignmentViewSet` to `ACLAssignmentViewSet` and updates associated classes, serializers, and queryset references. Aligns with the unified `ACLAssignment` model for improved consistency and maintainability. --- netbox_acls/api/views.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/netbox_acls/api/views.py b/netbox_acls/api/views.py index 5c7c17b2..eec43c45 100644 --- a/netbox_acls/api/views.py +++ b/netbox_acls/api/views.py @@ -11,14 +11,14 @@ from .serializers import ( AccessListSerializer, ACLExtendedRuleSerializer, - ACLInterfaceAssignmentSerializer, + ACLAssignmentSerializer, ACLStandardRuleSerializer, ) __all__ = [ "AccessListViewSet", "ACLStandardRuleViewSet", - "ACLInterfaceAssignmentViewSet", + "ACLAssignmentViewSet", "ACLExtendedRuleViewSet", ] @@ -39,17 +39,17 @@ class AccessListViewSet(NetBoxModelViewSet): filterset_class = filtersets.AccessListFilterSet -class ACLInterfaceAssignmentViewSet(NetBoxModelViewSet): +class ACLAssignmentViewSet(NetBoxModelViewSet): """ Defines the view set for the django ACLInterfaceAssignment model and associates it with a view. """ - queryset = models.ACLInterfaceAssignment.objects.prefetch_related( + queryset = models.ACLAssignment.objects.prefetch_related( "access_list", "tags", ) - serializer_class = ACLInterfaceAssignmentSerializer - filterset_class = filtersets.ACLInterfaceAssignmentFilterSet + serializer_class = ACLAssignmentSerializer + filterset_class = filtersets.ACLAssignmentFilterSet class ACLStandardRuleViewSet(NetBoxModelViewSet): From 8c1f07fcaa4d8492a835910c11ec46b42a7f37da Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 27 Aug 2025 19:49:01 +0200 Subject: [PATCH 30/35] refactor(api): Update URLs to use ACLAssignment routes Replaces `interface-assignments` route with `assignments` to align with the unified `ACLAssignment` model. Enhances consistency and maintainability across the API by simplifying route names. --- netbox_acls/api/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_acls/api/urls.py b/netbox_acls/api/urls.py index 3d292f80..40ff58c0 100644 --- a/netbox_acls/api/urls.py +++ b/netbox_acls/api/urls.py @@ -10,7 +10,7 @@ router = NetBoxRouter() router.register("access-lists", views.AccessListViewSet) -router.register("interface-assignments", views.ACLInterfaceAssignmentViewSet) +router.register("assignments", views.ACLAssignmentViewSet) router.register("standard-acl-rules", views.ACLStandardRuleViewSet) router.register("extended-acl-rules", views.ACLExtendedRuleViewSet) From d7ab05f0f57126f92cb286c13d5df7931b8f2121 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 28 Aug 2025 18:44:53 +0200 Subject: [PATCH 31/35] refactor(graphql): Replace ACLInterfaceAssignment with ACLAssignment Renames `ACLInterfaceAssignmentFilter` to `ACLAssignmentFilter` and updates related imports and fields. Removes legacy fields from the filter and adjusts field placement for consistency with the unified model structure. --- netbox_acls/graphql/filters/__init__.py | 4 ++-- netbox_acls/graphql/filters/access_lists.py | 18 +++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/netbox_acls/graphql/filters/__init__.py b/netbox_acls/graphql/filters/__init__.py index fe0a8281..8873eda9 100644 --- a/netbox_acls/graphql/filters/__init__.py +++ b/netbox_acls/graphql/filters/__init__.py @@ -1,9 +1,9 @@ from .access_list_rules import ACLExtendedRuleFilter, ACLStandardRuleFilter -from .access_lists import AccessListFilter, ACLInterfaceAssignmentFilter +from .access_lists import AccessListFilter, ACLAssignmentFilter __all__ = ( "AccessListFilter", "ACLExtendedRuleFilter", - "ACLInterfaceAssignmentFilter", + "ACLAssignmentFilter", "ACLStandardRuleFilter", ) diff --git a/netbox_acls/graphql/filters/access_lists.py b/netbox_acls/graphql/filters/access_lists.py index 9801b996..efd1ff1f 100644 --- a/netbox_acls/graphql/filters/access_lists.py +++ b/netbox_acls/graphql/filters/access_lists.py @@ -19,7 +19,7 @@ __all__ = ( "AccessListFilter", - "ACLInterfaceAssignmentFilter", + "ACLAssignmentFilter", ) @@ -30,10 +30,6 @@ class AccessListFilter(NetBoxModelFilterMixin): """ name: FilterLookup[str] | None = strawberry_django.filter_field() - assigned_object_type: Annotated["ContentTypeFilter", strawberry.lazy("core.graphql.filters")] | None = ( - strawberry_django.filter_field() - ) - assigned_object_id: ID | None = strawberry_django.filter_field() type: Annotated["ACLTypeEnum", strawberry.lazy("netbox_acls.graphql.enums")] | None = ( strawberry_django.filter_field() ) @@ -42,20 +38,20 @@ class AccessListFilter(NetBoxModelFilterMixin): ) -@strawberry_django.filter(models.ACLInterfaceAssignment, lookups=True) -class ACLInterfaceAssignmentFilter(NetBoxModelFilterMixin): +@strawberry_django.filter(models.ACLAssignment, lookups=True) +class ACLAssignmentFilter(NetBoxModelFilterMixin): """ - GraphQL filter definition for the ACLInterfaceAssignment model. + GraphQL filter definition for the ACLAssignment model. """ access_list: Annotated["AccessListFilter", strawberry.lazy("netbox_acls.graphql.filters")] | None = ( strawberry_django.filter_field() ) access_list_id: ID | None = strawberry_django.filter_field() - direction: Annotated["ACLAssignmentDirectionEnum", strawberry.lazy("netbox_acls.graphql.enums")] | None = ( - strawberry_django.filter_field() - ) assigned_object_type: Annotated["ContentTypeFilter", strawberry.lazy("core.graphql.filters")] | None = ( strawberry_django.filter_field() ) assigned_object_id: ID | None = strawberry_django.filter_field() + direction: Annotated["ACLAssignmentDirectionEnum", strawberry.lazy("netbox_acls.graphql.enums")] | None = ( + strawberry_django.filter_field() + ) From 0ffbf3fc848bfdd5efa67a98aec86ae5e62590d4 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 28 Aug 2025 19:57:26 +0200 Subject: [PATCH 32/35] refactor(graphql): Replace ACLInterfaceAssignment with ACLAssignment Updates GraphQL types to use the unified `ACLAssignment` model, removing legacy fields and adding support for additional object types like virtual chassis. Enhances alignment with the consolidated model structure for improved maintainability and consistency. --- netbox_acls/graphql/types.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/netbox_acls/graphql/types.py b/netbox_acls/graphql/types.py index 18aad285..342d426a 100644 --- a/netbox_acls/graphql/types.py +++ b/netbox_acls/graphql/types.py @@ -12,7 +12,7 @@ from . import filters if TYPE_CHECKING: - from dcim.graphql.types import DeviceType, InterfaceType + from dcim.graphql.types import DeviceType, InterfaceType, VirtualChassisType from ipam.graphql.types import AggregateType, IPAddressType, IPRangeType, PrefixType from virtualization.graphql.types import VirtualMachineType, VMInterfaceType @@ -20,7 +20,6 @@ @strawberry_django.type( models.AccessList, fields="__all__", - exclude=["assigned_object_type", "assigned_object_id"], filters=filters.AccessListFilter, ) class AccessListType(NetBoxObjectType): @@ -28,16 +27,6 @@ class AccessListType(NetBoxObjectType): Defines the object type for the django model AccessList. """ - # Model fields - assigned_object_type: Annotated["ContentTypeType", strawberry.lazy("netbox.graphql.types")] - assigned_object: Annotated[ - Union[ - Annotated["DeviceType", strawberry.lazy("dcim.graphql.types")], - Annotated["VirtualMachineType", strawberry.lazy("virtualization.graphql.types")], - ], - strawberry.union("ACLAssignmentType"), - ] - # Related models aclstandardrules: List[ Annotated[ @@ -51,15 +40,21 @@ class AccessListType(NetBoxObjectType): strawberry.lazy("netbox_acls.graphql.types"), ] ] + aclassignments: List[ + Annotated[ + "ACLAssignmentType", + strawberry.lazy("netbox_acls.graphql.types"), + ] + ] @strawberry_django.type( - models.ACLInterfaceAssignment, + models.ACLAssignment, fields="__all__", exclude=["assigned_object_type", "assigned_object_id"], - filters=filters.ACLInterfaceAssignmentFilter, + filters=filters.ACLAssignmentFilter, ) -class ACLInterfaceAssignmentType(NetBoxObjectType): +class ACLAssignmentType(NetBoxObjectType): """ Defines the object type for the django model ACLInterfaceAssignment. """ @@ -69,10 +64,13 @@ class ACLInterfaceAssignmentType(NetBoxObjectType): assigned_object_type: Annotated["ContentTypeType", strawberry.lazy("netbox.graphql.types")] assigned_object: Annotated[ Union[ + Annotated["DeviceType", strawberry.lazy("dcim.graphql.types")], Annotated["InterfaceType", strawberry.lazy("dcim.graphql.types")], + Annotated["VirtualChassisType", strawberry.lazy("dcim.graphql.types")], + Annotated["VirtualMachineType", strawberry.lazy("virtualization.graphql.types")], Annotated["VMInterfaceType", strawberry.lazy("virtualization.graphql.types")], ], - strawberry.union("ACLInterfaceAssignedObjectType"), + strawberry.union("ACLAssignedObjectType"), ] From 681b57b62ec9208c39d95a83143b8732fadaa9f4 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 28 Aug 2025 20:00:42 +0200 Subject: [PATCH 33/35] refactor(graphql): Replace ACLInterfaceAssignment with ACLAssignment Updates GraphQL schema to rename `ACLInterfaceAssignmentType` to `ACLAssignmentType`. Adjusts related fields and lists to align with the unified `ACLAssignment` model. Enhances consistency and maintainability within the GraphQL layer. --- netbox_acls/graphql/schema.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox_acls/graphql/schema.py b/netbox_acls/graphql/schema.py index 3516604d..ddfe92a8 100644 --- a/netbox_acls/graphql/schema.py +++ b/netbox_acls/graphql/schema.py @@ -6,7 +6,7 @@ from .types import ( AccessListType, ACLExtendedRuleType, - ACLInterfaceAssignmentType, + ACLAssignmentType, ACLStandardRuleType, ) @@ -26,5 +26,5 @@ class NetBoxACLSQuery: acl_standard_rule: ACLStandardRuleType = strawberry_django.field() acl_standard_rule_list: List[ACLStandardRuleType] = strawberry_django.field() - acl_interface_assignment: ACLInterfaceAssignmentType = strawberry_django.field() - acl_interface_assignment_list: List[ACLInterfaceAssignmentType] = strawberry_django.field() + acl_assignment: ACLAssignmentType = strawberry_django.field() + acl_assignment_list: List[ACLAssignmentType] = strawberry_django.field() From 89f66a16217c2f0c5ec2e5ff67011dee27b13b4c Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 26 Sep 2025 11:47:18 +0200 Subject: [PATCH 34/35] refactor(tests): Update test cases to use ACLAssignment model Replaces `ACLInterfaceAssignment` with `ACLAssignment` in test cases for access lists. Removes legacy object creation for sites, devices, and VMs, simplifying test data setup. Enhances test maintainability by aligning with the unified model structure. --- .../tests/api/test_access_list_rules.py | 98 ------------------- netbox_acls/tests/api/test_access_lists.py | 95 ++++-------------- 2 files changed, 17 insertions(+), 176 deletions(-) diff --git a/netbox_acls/tests/api/test_access_list_rules.py b/netbox_acls/tests/api/test_access_list_rules.py index d4e83e72..5e6e9251 100644 --- a/netbox_acls/tests/api/test_access_list_rules.py +++ b/netbox_acls/tests/api/test_access_list_rules.py @@ -1,7 +1,5 @@ -from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Site from ipam.models import Prefix from utilities.testing import APIViewTestCases -from virtualization.models import Cluster, ClusterType, VirtualMachine from netbox_acls.choices import ( ACLActionChoices, @@ -21,70 +19,22 @@ class ACLStandardRuleAPIViewTestCase(APIViewTestCases.APIViewTestCase): view_namespace = "plugins-api:netbox_acls" brief_fields = ["access_list", "display", "id", "index", "url"] user_permissions = ( - "dcim.view_site", - "dcim.view_manufacturer", - "dcim.view_devicetype", - "dcim.view_device", "ipam.view_prefix", - "virtualization.view_cluster", - "virtualization.view_clustergroup", - "virtualization.view_clustertype", - "virtualization.view_virtualmachine", "netbox_acls.view_accesslist", ) @classmethod def setUpTestData(cls): """Set up ACL Standard Rule for API view testing.""" - site = Site.objects.create( - name="Site 1", - slug="site-1", - ) - - # Device - manufacturer = Manufacturer.objects.create( - name="Manufacturer 1", - slug="manufacturer-1", - ) - device_type = DeviceType.objects.create( - manufacturer=manufacturer, - model="Device Type 1", - ) - device_role = DeviceRole.objects.create( - name="Device Role 1", - slug="device-role-1", - ) - device = Device.objects.create( - name="Device 1", - site=site, - device_type=device_type, - role=device_role, - ) - - # Virtual Machine - cluster_type = ClusterType.objects.create( - name="Cluster Type 1", - slug="cluster-type-1", - ) - cluster = Cluster.objects.create( - name="Cluster 1", - type=cluster_type, - ) - virtual_machine = VirtualMachine.objects.create( - name="VM 1", - cluster=cluster, - ) # AccessList access_list_device = AccessList.objects.create( name="testacl1", - assigned_object=device, type=ACLTypeChoices.TYPE_STANDARD, default_action=ACLActionChoices.ACTION_DENY, ) access_list_vm = AccessList.objects.create( name="testacl2", - assigned_object=virtual_machine, type=ACLTypeChoices.TYPE_STANDARD, default_action=ACLActionChoices.ACTION_PERMIT, ) @@ -161,70 +111,22 @@ class ACLExtendedRuleAPIViewTestCase(APIViewTestCases.APIViewTestCase): view_namespace = "plugins-api:netbox_acls" brief_fields = ["access_list", "display", "id", "index", "url"] user_permissions = ( - "dcim.view_site", - "dcim.view_manufacturer", - "dcim.view_devicetype", - "dcim.view_device", "ipam.view_prefix", - "virtualization.view_cluster", - "virtualization.view_clustergroup", - "virtualization.view_clustertype", - "virtualization.view_virtualmachine", "netbox_acls.view_accesslist", ) @classmethod def setUpTestData(cls): """Set up ACL Extended Rule for API view testing.""" - site = Site.objects.create( - name="Site 1", - slug="site-1", - ) - - # Device - manufacturer = Manufacturer.objects.create( - name="Manufacturer 1", - slug="manufacturer-1", - ) - device_type = DeviceType.objects.create( - manufacturer=manufacturer, - model="Device Type 1", - ) - device_role = DeviceRole.objects.create( - name="Device Role 1", - slug="device-role-1", - ) - device = Device.objects.create( - name="Device 1", - site=site, - device_type=device_type, - role=device_role, - ) - - # Virtual Machine - cluster_type = ClusterType.objects.create( - name="Cluster Type 1", - slug="cluster-type-1", - ) - cluster = Cluster.objects.create( - name="Cluster 1", - type=cluster_type, - ) - virtual_machine = VirtualMachine.objects.create( - name="VM 1", - cluster=cluster, - ) # AccessList access_list_device = AccessList.objects.create( name="testacl1", - assigned_object=device, type=ACLTypeChoices.TYPE_EXTENDED, default_action=ACLActionChoices.ACTION_DENY, ) access_list_vm = AccessList.objects.create( name="testacl2", - assigned_object=virtual_machine, type=ACLTypeChoices.TYPE_EXTENDED, default_action=ACLActionChoices.ACTION_PERMIT, ) diff --git a/netbox_acls/tests/api/test_access_lists.py b/netbox_acls/tests/api/test_access_lists.py index 48639af7..776311a8 100644 --- a/netbox_acls/tests/api/test_access_lists.py +++ b/netbox_acls/tests/api/test_access_lists.py @@ -9,7 +9,7 @@ ACLAssignmentDirectionChoices, ACLTypeChoices, ) -from netbox_acls.models import AccessList, ACLInterfaceAssignment +from netbox_acls.models import AccessList, ACLAssignment class AccessListAPIViewTestCase(APIViewTestCases.APIViewTestCase): @@ -20,76 +20,23 @@ class AccessListAPIViewTestCase(APIViewTestCases.APIViewTestCase): model = AccessList view_namespace = "plugins-api:netbox_acls" brief_fields = ["display", "id", "name", "url"] - user_permissions = ( - "dcim.view_site", - "dcim.view_devicetype", - "dcim.view_device", - "virtualization.view_cluster", - "virtualization.view_clustergroup", - "virtualization.view_clustertype", - "virtualization.view_virtualmachine", - ) @classmethod def setUpTestData(cls): """Set up Access List for API view testing.""" - site = Site.objects.create( - name="Site 1", - slug="site-1", - ) - - # Device - manufacturer = Manufacturer.objects.create( - name="Manufacturer 1", - slug="manufacturer-1", - ) - device_type = DeviceType.objects.create( - manufacturer=manufacturer, - model="Device Type 1", - ) - device_role = DeviceRole.objects.create( - name="Device Role 1", - slug="device-role-1", - ) - device = Device.objects.create( - name="Device 1", - site=site, - device_type=device_type, - role=device_role, - ) - - # Virtual Machine - cluster_type = ClusterType.objects.create( - name="Cluster Type 1", - slug="cluster-type-1", - ) - cluster = Cluster.objects.create( - name="Cluster 1", - type=cluster_type, - ) - virtual_machine = VirtualMachine.objects.create( - name="VM 1", - cluster=cluster, - ) - access_lists = ( AccessList( name="testacl1", - assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=device.id, type=ACLTypeChoices.TYPE_STANDARD, default_action=ACLActionChoices.ACTION_DENY, ), AccessList( name="testacl2", - assigned_object=device, type=ACLTypeChoices.TYPE_EXTENDED, default_action=ACLActionChoices.ACTION_PERMIT, ), AccessList( name="testacl3", - assigned_object_type=ContentType.objects.get_for_model(VirtualMachine), - assigned_object_id=virtual_machine.id, type=ACLTypeChoices.TYPE_EXTENDED, default_action=ACLActionChoices.ACTION_DENY, ), @@ -99,22 +46,16 @@ def setUpTestData(cls): cls.create_data = [ { "name": "testacl4", - "assigned_object_type": "dcim.device", - "assigned_object_id": device.id, "type": ACLTypeChoices.TYPE_STANDARD, "default_action": ACLActionChoices.ACTION_DENY, }, { "name": "testacl5", - "assigned_object_type": "dcim.device", - "assigned_object_id": device.id, "type": ACLTypeChoices.TYPE_EXTENDED, "default_action": ACLActionChoices.ACTION_DENY, }, { "name": "testacl6", - "assigned_object_type": "virtualization.virtualmachine", - "assigned_object_id": virtual_machine.id, "type": ACLTypeChoices.TYPE_STANDARD, "default_action": ACLActionChoices.ACTION_PERMIT, }, @@ -124,12 +65,12 @@ def setUpTestData(cls): } -class ACLInterfaceAssignmentAPIViewTestCase(APIViewTestCases.APIViewTestCase): +class ACLAssignmentAPIViewTestCase(APIViewTestCases.APIViewTestCase): """ - API view test case for ACLInterfaceAssignment. + API view test case for ACLAssignment. """ - model = ACLInterfaceAssignment + model = ACLAssignment view_namespace = "plugins-api:netbox_acls" brief_fields = ["access_list", "display", "id", "url"] user_permissions = ( @@ -215,55 +156,53 @@ def setUpTestData(cls): ) # AccessList - access_list_device = AccessList.objects.create( + acl1 = AccessList.objects.create( name="testacl1", - assigned_object=device, type=ACLTypeChoices.TYPE_STANDARD, default_action=ACLActionChoices.ACTION_DENY, ) - access_list_vm = AccessList.objects.create( + acl2 = AccessList.objects.create( name="testacl2", - assigned_object=virtual_machine, type=ACLTypeChoices.TYPE_EXTENDED, default_action=ACLActionChoices.ACTION_PERMIT, ) - acl_interface_assignments = ( - ACLInterfaceAssignment( - access_list=access_list_device, + acl_assignments = ( + ACLAssignment( + access_list=acl1, direction=ACLAssignmentDirectionChoices.DIRECTION_INGRESS, assigned_object_type=ContentType.objects.get_for_model(Interface), assigned_object_id=device_interface1.id, ), - ACLInterfaceAssignment( - access_list=access_list_device, + ACLAssignment( + access_list=acl1, direction=ACLAssignmentDirectionChoices.DIRECTION_EGRESS, assigned_object=device_interface2, ), - ACLInterfaceAssignment( - access_list=access_list_vm, + ACLAssignment( + access_list=acl2, direction=ACLAssignmentDirectionChoices.DIRECTION_EGRESS, assigned_object_type=ContentType.objects.get_for_model(VMInterface), assigned_object_id=virtual_machine_interface1.id, ), ) - ACLInterfaceAssignment.objects.bulk_create(acl_interface_assignments) + ACLAssignment.objects.bulk_create(acl_assignments) cls.create_data = [ { - "access_list": access_list_device.id, + "access_list": acl1.id, "assigned_object_type": "dcim.interface", "assigned_object_id": device_interface3.id, "direction": ACLAssignmentDirectionChoices.DIRECTION_EGRESS, }, { - "access_list": access_list_vm.id, + "access_list": acl2.id, "assigned_object_type": "virtualization.vminterface", "assigned_object_id": virtual_machine_interface2.id, "direction": ACLAssignmentDirectionChoices.DIRECTION_INGRESS, }, { - "access_list": access_list_vm.id, + "access_list": acl2.id, "assigned_object_type": "virtualization.vminterface", "assigned_object_id": virtual_machine_interface3.id, "direction": ACLAssignmentDirectionChoices.DIRECTION_EGRESS, From 186093d27f5f73455880e3800971aae2af67770e Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 29 Aug 2025 22:10:14 +0200 Subject: [PATCH 35/35] refactor(navigation): Replace ACLInterfaceAssignment with ACLAssignment Updates navigation to use the unified `ACLAssignment` model. Replaces `Interface Assignments` menu items with `Assignments`, aligning with the consolidated model structure. Enhances consistency and maintainability across the plugin interface. --- netbox_acls/navigation.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/netbox_acls/navigation.py b/netbox_acls/navigation.py index 1144cf1d..3ed9f391 100644 --- a/netbox_acls/navigation.py +++ b/netbox_acls/navigation.py @@ -56,17 +56,17 @@ ), ) -# ACL Interface Assignment -aclinterfaceassignment_item = PluginMenuItem( - link="plugins:netbox_acls:aclinterfaceassignment_list", - link_text="Interface Assignments", - permissions=["netbox_acls.view_aclinterfaceassignment"], +# ACL Assignment +aclassignment_item = PluginMenuItem( + link="plugins:netbox_acls:aclassignment_list", + link_text="Assignments", + permissions=["netbox_acls.view_aclassignment"], buttons=( PluginMenuButton( - link="plugins:netbox_acls:aclinterfaceassignment_add", + link="plugins:netbox_acls:aclassignment_add", title="Add", icon_class="mdi mdi-plus-thick", - permissions=["netbox_acls.add_aclinterfaceassignment"], + permissions=["netbox_acls.add_aclassignment"], ), ), ) @@ -78,7 +78,10 @@ groups=( ( "Access Lists", - (accesslist_item,), + ( + accesslist_item, + aclassignment_item, + ), ), ( "Rules", @@ -87,17 +90,13 @@ aclextendedrule_item, ), ), - ( - "Assignments", - (aclinterfaceassignment_item,), - ), ), icon_class="mdi mdi-lock", ) else: menu_items = ( accesslist_item, + aclassignment_item, aclstandardrule_item, aclextendedrule_item, - aclinterfaceassignment_item, )
    HostAccess List{{ object.access_list|linkify }}
    Assigned Object {% if object.assigned_object.device %} - {{ object.assigned_object.device|linkify|placeholder }} - {% else %} - {{ object.assigned_object.virtual_machine|linkify|placeholder }} + {{ object.assigned_object.device|linkify }} / + {% elif object.assigned_object.virtual_machine %} + {{ object.assigned_object.virtual_machine|linkify }} / {% endif %} + {{ object.assigned_object|linkify|placeholder }}
    Interface{{ object.assigned_object|linkify|placeholder }}
    Access List{{ object.access_list|linkify }}
    Direction {% badge object.get_direction_display bg_color=object.get_direction_color %}