[Bug]: ACL Interface Assignment Should Be Unique per Interface and Direction

[pheus]

NetBox access-list plugin version

v1.8.1

NetBox version

v4.2.5

Steps to Reproduce

1. Create a Device (with its dependencies).
2. Create an Interface for the Device.
3. Create an Access List "ACL1" (type: "standard") bound to the Device.
4. Create an ACL Interface Assignment for the Interface with direction set to "ingress."
5. Create another Access List "ACL2" (type: "standard") bound to the same Device.
6. Attempt to assign "ACL2" to the same interface with the same direction via nb_shell.

Reproduction via nb_shell:

from dcim.models import (
    Device,
    DeviceRole,
    DeviceType,
    Manufacturer,
    Site,
    VirtualChassis,
)
from netbox_acls.models import AccessList, ACLInterfaceAssignment
from ipam.models import Prefix

# Create Site
site = Site.objects.create(
    name="Site 1",
    slug="site-1",
)

# Create Manufacturer and Device Type
manufacturer = Manufacturer.objects.create(
    name="Manufacturer 1",
    slug="manufacturer-1",
)
device_type = DeviceType.objects.create(
    manufacturer=manufacturer,
    model="Device Type 1",
)

# Create Device Role
device_role = DeviceRole.objects.create(
    name="Device Role 1",
    slug="device-role-1",
)

# Create Device and Interface
device1 = Device.objects.create(
    name="Device 1",
    site=site,
    device_type=device_type,
    role=device_role,
)
device_interface1 = Interface.objects.create(
    name="Interface 1",
    device=device1,
    type="1000baset",
)

# Create and Assign Access List 1
device_acl1 = AccessList.objects.create(
    name="STANDARD_ACL1",
    assigned_object=device1,
    type="standard",
    default_action="permit",
    comments="STANDARD_ACL",
)
ACLInterfaceAssignment.objects.create(
    access_list=device_acl1,
    direction="ingress",
    assigned_object=device_interface1,
)

# Create and Attempt to Assign Access List 2
device_acl2 = AccessList.objects.create(
    name="STANDARD_ACL2",
    assigned_object=device1,
    type="standard",
    default_action="permit",
    comments="STANDARD_ACL",
)
acl_device_interface2 = ACLInterfaceAssignment(
    access_list=device_acl2,
    direction="ingress",
    assigned_object=device_interface1,
)
acl_device_interface2.full_clean()  # Expected to fail, but it succeeds
acl_device_interface2.save()

Expected Behavior

The assignment should fail, as only one Access List should be assignable to an interface per direction.

Observed Behavior

The assignment via nb_shell succeeds, even though the Web UI correctly enforces this restriction.

[pheus]

I encountered this issue while developing a test case to validate the uniqueness of ACL assignments. The root cause appears to be an incorrect unique_together constraint in the model configuration.

Currently, the uniqueness constraint is enforced on (assigned_object_type, assigned_object_id, direction, access_list) (e.g., Interface + direction + AccessList), which allows multiple ACL assignments with the same direction on the same interface, as long as the access lists are different.

However, the correct constraint should be on (assigned_object_type, assigned_object_id, direction) (e.g., Interface + direction) alone. Once an ACL assignment exists for a given interface and direction, no other ACL should be assignable in that direction on the same interface.

This misconfiguration allows nb_shell to bypass the expected uniqueness check, whereas the Web UI correctly prevents the duplicate assignment.

[cruse1977]

I'm a little torn on this one @pheus, what if say you had an ipv4 and ipv6 acl against the same interface which is a really common use case ?

[pheus]

Thanks for bringing that up, @cruse1977! You’re absolutely right that a lot of folks want to bind both an IPv4 ACL and an IPv6 ACL on the same interface/direction.

I’m going to remove the bugfix from PR #257 for now so I can think through a more flexible solution. I’ll follow up here once I have a proposal - any feedback in the meantime is welcome!

[pheus]

Proposal

To support this while still enforcing uniqueness, one possible solution is to introduce an address_family field to ACLInterfaceAssignment, with values like "ipv4" or "ipv6". This field could be explicitly set or derived from the associated AccessList.

The uniqueness constraint would then be updated to:

unique_together = (
    "assigned_object_type",
    "assigned_object_id",
    "direction",
    "address_family",
)

This approach allows one ACL per address family in a given direction on a given interface. For example, it would permit both an ingress IPv4 ACL and an ingress IPv6 ACL on the same interface, while preventing multiple assignments for the same direction and address family.

Let me know if this seems like a reasonable direction or if you'd prefer a different approach. If this falls outside the scope of the issue, I'm happy to open a separate feature request to explore it further.

[cruse1977]

Let me know if this seems like a reasonable direction or if you'd prefer a different approach. If this falls outside the scope of the issue, I'm happy to open a separate feature request to explore it further.

I like this approach - although it will have follow ups to ensure addresses families of added prefixes to a particular rule + we'd need to be careful on any default due to existing acls and ensuring we don't break things.

It also allows for systems whereas the config stanza is slightly different based on the family - ie ip access-list and ipv6 access-list