diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
index a3fc26f..2d3b513 100644
--- a/.devcontainer/devcontainer.json
+++ b/.devcontainer/devcontainer.json
@@ -78,7 +78,10 @@
"extensions": [
"DavidAnson.vscode-markdownlint",
"GitHub.codespaces",
+ "GitHub.copilot",
+ "GitHub.copilot-labs",
"GitHub.vscode-pull-request-github",
+ "Gruntfuggly.todo-tree",
"Tyriar.sort-lines",
"aaron-bond.better-comments",
"batisteo.vscode-django",
diff --git a/.devcontainer/requirements-dev.txt b/.devcontainer/requirements-dev.txt
index 13a832d..d4a50d5 100644
--- a/.devcontainer/requirements-dev.txt
+++ b/.devcontainer/requirements-dev.txt
@@ -12,3 +12,4 @@ pylint
pylint-django
wily
yapf
+sourcery-analytics
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9ebc05c..efe8170 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -19,11 +19,13 @@ repos:
- id: isort
args:
- "--profile=black"
+ exclude: ^.devcontainer/
- repo: https://github.com/psf/black
rev: 22.12.0
hooks:
- id: black
language_version: python3
+ exclude: ^.devcontainer/
- repo: https://github.com/asottile/add-trailing-comma
rev: v2.4.0
hooks:
@@ -34,6 +36,7 @@ repos:
rev: 6.0.0
hooks:
- id: flake8
+ exclude: ^.devcontainer/
- repo: https://github.com/asottile/pyupgrade
rev: v3.3.1
hooks:
@@ -44,6 +47,12 @@ repos:
rev: v1.29.0
hooks:
- id: yamllint
+ - repo: https://github.com/econchick/interrogate
+ rev: 1.5.0
+ hooks:
+ - id: interrogate
+ args: [--fail-under=90, --verbose]
+ exclude: (^.devcontainer/|^netbox_acls/migrations/)
#- repo: https://github.com/Lucas-C/pre-commit-hooks-nodejs
# rev: v1.1.2
# hooks:
diff --git a/Makefile b/Makefile
index aa6c2f9..fb3ba16 100644
--- a/Makefile
+++ b/Makefile
@@ -70,9 +70,13 @@ start:
.PHONY: all ## Run all PLUGIN DEV targets
all: setup makemigrations migrate collectstatic initializers start
-#.PHONY: test
-#test:
-# ${VENV_PY_PATH} /opt/netbox/netbox/manage.py runserver test ${PLUGIN_NAME}
+.PHONY: rebuild ## Run PLUGIN DEV targets to rebuild
+rebuild: setup makemigrations migrate collectstatic start
+
+.PHONY: test
+test: setup
+ ${VENV_PY_PATH} ${NETBOX_MANAGE_PATH}/manage.py makemigrations ${PLUGIN_NAME} --check
+ ${VENV_PY_PATH} ${NETBOX_MANAGE_PATH}/manage.py test ${PLUGIN_NAME}
#relpatch:
# $(eval GSTATUS := $(shell git status --porcelain))
diff --git a/TODO b/TODO
new file mode 100644
index 0000000..5218b1d
--- /dev/null
+++ b/TODO
@@ -0,0 +1,7 @@
+- TODO: ACL Form Bubble/ICON Extended/Standard
+- TODO: Add an Access List to an Interface Custom Fields after comments - DONE
+- TODO: ACL rules, look at last number and increment to next 10
+- TODO: Clone for ACL Interface should include device
+- TODO: Inconsistent errors for add/edit (where model is using a generic page)
+- TODO: Check Constants across codebase for consistency.
+- TODO: Test API, Forms, & Models - https://github.com/k01ek/netbox-bgp/tree/main/netbox_bgp/tests , https://github.com/DanSheps/netbox-secretstore/tree/develop/netbox_secretstore/tests & https://github.com/FlxPeters/netbox-plugin-prometheus-sd/tree/main/netbox_prometheus_sd/tests
diff --git a/netbox_acls/api/serializers.py b/netbox_acls/api/serializers.py
index b84a0bd..46f24cb 100644
--- a/netbox_acls/api/serializers.py
+++ b/netbox_acls/api/serializers.py
@@ -19,6 +19,7 @@
ACLExtendedRule,
ACLInterfaceAssignment,
ACLStandardRule,
+ BaseACLRule,
)
from .nested_serializers import NestedAccessListSerializer
@@ -29,18 +30,40 @@
"ACLExtendedRuleSerializer",
]
+# TODO: Check Constants across codebase for consistency.
# 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."
+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 = (
+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 not set to remark, but no remark is set.
-error_message_remark_without_action_remark = (
+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 to an ACL of the same type.
-error_message_acl_type = "Provided parent Access List is not of right type."
+ERROR_MESSAGE_ACL_TYPE = "Provided parent Access List is not of right type."
+
+
+def validate_gfk(data):
+ """
+ Check that the GFK object is valid.
+ """
+ # TODO: This can removed after https://github.com/netbox-community/netbox/issues/10221 is fixed.
+ try:
+ assigned_object = data[ # noqa: F841
+ "assigned_object_type"
+ ].get_object_for_this_type(
+ id=data["assigned_object_id"],
+ )
+ except ObjectDoesNotExist as e:
+ error_message_invalid_gfk = f"Invalid assigned_object {data['assigned_object_type']} ID {data['assigned_object_id']}"
+ raise serializers.ValidationError(
+ {
+ "assigned_object_type": [error_message_invalid_gfk],
+ "assigned_object_id": [error_message_invalid_gfk],
+ }
+ ) from e
class AccessListSerializer(NetBoxModelSerializer):
@@ -83,6 +106,9 @@ class Meta:
@swagger_serializer_method(serializer_or_field=serializers.DictField)
def get_assigned_object(self, obj):
+ """
+ Returns the assigned object for the Access List.
+ """
serializer = get_serializer_for_model(
obj.assigned_object,
prefix=NESTED_SERIALIZER_PREFIX,
@@ -93,25 +119,13 @@ def get_assigned_object(self, obj):
def validate(self, data):
"""
Validates api inputs before processing:
- - Check that the GFK object is valid.
- - Check if Access List has no existing rules before change the Access List's type.
+ - Check that the GFK object is valid.
+ - Check if Access List has no existing rules before change the Access List's type.
"""
error_message = {}
# Check that the GFK object is valid.
- if "assigned_object_type" in data and "assigned_object_id" in data:
- # TODO: This can removed after https://github.com/netbox-community/netbox/issues/10221 is fixed.
- try:
- assigned_object = data[ # noqa: F841
- "assigned_object_type"
- ].get_object_for_this_type(
- id=data["assigned_object_id"],
- )
- except ObjectDoesNotExist:
- # Sets a standard error message for invalid GFK
- error_message_invalid_gfk = f"Invalid assigned_object {data['assigned_object_type']} ID {data['assigned_object_id']}"
- error_message["assigned_object_type"] = [error_message_invalid_gfk]
- error_message["assigned_object_id"] = [error_message_invalid_gfk]
+ assigned_object = validate_gfk(data)
# Check if Access List has no existing rules before change the Access List's type.
if (
@@ -119,9 +133,9 @@ def validate(self, data):
and self.instance.type != data.get("type")
and self.instance.rule_count > 0
):
- error_message["type"] = [
- "This ACL has ACL rules associated, CANNOT change ACL type.",
- ]
+ raise serializers.ValidationError(
+ {"type": ["This ACL has ACL rules associated, CANNOT change ACL type."]}
+ )
if error_message:
raise serializers.ValidationError(error_message)
@@ -166,6 +180,9 @@ class Meta:
@swagger_serializer_method(serializer_or_field=serializers.DictField)
def get_assigned_object(self, obj):
+ """
+ Returns the assigned object for the ACLInterfaceAssignment.
+ """
serializer = get_serializer_for_model(
obj.assigned_object,
prefix=NESTED_SERIALIZER_PREFIX,
@@ -173,51 +190,51 @@ def get_assigned_object(self, obj):
context = {"request": self.context["request"]}
return serializer(obj.assigned_object, context=context).data
+ def _validate_get_interface_host(self, data, assigned_object):
+ """
+ Check that the associated interface's parent host has the selected ACL defined.
+ """
+ MODEL_MAPPING = {
+ "interface": "device",
+ "vminterface": "virtual_machine",
+ }
+
+ assigned_object_model = data["assigned_object_type"].model
+
+ return getattr(assigned_object, MODEL_MAPPING.get(assigned_object_model, None))
+
+ def _validate_acl_host(self, acl_host, interface_host):
+ """
+ Check that the associated interface's parent host has the selected ACL defined.
+ """
+ if acl_host == interface_host:
+ return {}
+
+ error_acl_not_assigned_to_host = (
+ "Access List not present on the selected interface's host."
+ )
+ return {
+ "access_list": [error_acl_not_assigned_to_host],
+ "assigned_object_id": [error_acl_not_assigned_to_host],
+ }
+
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.
+ - Check that the GFK object is valid.
+ - Check that the associated interface's parent host has the selected ACL defined.
"""
+
+ # Check that the GFK object is valid.
+ assigned_object = validate_gfk(data)
+
error_message = {}
acl_host = data["access_list"].assigned_object
- # Check that the GFK object is valid.
- if "assigned_object_type" in data and "assigned_object_id" in data:
- # TODO: This can removed after https://github.com/netbox-community/netbox/issues/10221 is fixed.
- try:
- assigned_object = data[ # noqa: F841
- "assigned_object_type"
- ].get_object_for_this_type(
- id=data["assigned_object_id"],
- )
- except ObjectDoesNotExist:
- # Sets a standard error message for invalid GFK
- error_message_invalid_gfk = f"Invalid assigned_object {data['assigned_object_type']} ID {data['assigned_object_id']}"
- error_message["assigned_object_type"] = [error_message_invalid_gfk]
- error_message["assigned_object_id"] = [error_message_invalid_gfk]
-
- 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]
+ interface_host = self._validate_get_interface_host(data, assigned_object)
+ acl_host = data["access_list"].assigned_object
+
+ error_message |= self._validate_acl_host(acl_host, interface_host)
if error_message:
raise serializers.ValidationError(error_message)
@@ -225,14 +242,11 @@ def validate(self, data):
return super().validate(data)
-class ACLStandardRuleSerializer(NetBoxModelSerializer):
+class BaseACLRuleSerializer(NetBoxModelSerializer):
"""
- Defines the serializer for the django ACLStandardRule model & associates it to a view.
+ Defines the serializer for the django BaseACLRule model & associates it to a view.
"""
- url = serializers.HyperlinkedIdentityField(
- view_name="plugins-api:netbox_acls-api:aclstandardrule-detail",
- )
access_list = NestedAccessListSerializer()
source_prefix = NestedPrefixSerializer(
required=False,
@@ -242,10 +256,11 @@ class ACLStandardRuleSerializer(NetBoxModelSerializer):
class Meta:
"""
- Associates the django model ACLStandardRule & fields to the serializer.
+ Associates the django model BaseACLRule & fields to the serializer.
"""
- model = ACLStandardRule
+ abstract = True
+ model = BaseACLRule
fields = (
"id",
"url",
@@ -264,19 +279,19 @@ class Meta:
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.
+ Validate the BaseACLRule 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.
"""
error_message = {}
# Check if action set to remark, but no remark set.
if data.get("action") == "remark" and data.get("remark") is None:
- error_message["remark"] = [error_message_no_remark]
+ 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,
+ ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET,
]
if error_message:
@@ -285,7 +300,24 @@ def validate(self, data):
return super().validate(data)
-class ACLExtendedRuleSerializer(NetBoxModelSerializer):
+class ACLStandardRuleSerializer(BaseACLRuleSerializer):
+ """
+ Defines the serializer for the django ACLStandardRule model & associates it to a view.
+ """
+
+ url = serializers.HyperlinkedIdentityField(
+ view_name="plugins-api:netbox_acls-api:aclstandardrule-detail",
+ )
+
+ class Meta(BaseACLRuleSerializer.Meta):
+ """
+ Associates the django model ACLStandardRule & fields to the serializer.
+ """
+
+ model = ACLStandardRule
+
+
+class ACLExtendedRuleSerializer(BaseACLRuleSerializer):
"""
Defines the serializer for the django ACLExtendedRule model & associates it to a view.
"""
@@ -293,37 +325,21 @@ class ACLExtendedRuleSerializer(NetBoxModelSerializer):
url = serializers.HyperlinkedIdentityField(
view_name="plugins-api:netbox_acls-api:aclextendedrule-detail",
)
- access_list = NestedAccessListSerializer()
- source_prefix = NestedPrefixSerializer(
- required=False,
- allow_null=True,
- default=None,
- )
destination_prefix = NestedPrefixSerializer(
required=False,
allow_null=True,
default=None,
)
- class Meta:
+ class Meta(BaseACLRuleSerializer.Meta):
"""
Associates the django model ACLExtendedRule & fields to the serializer.
"""
model = ACLExtendedRule
- fields = (
- "id",
- "url",
- "display",
- "access_list",
- "index",
- "action",
- "tags",
- "description",
- "created",
- "custom_fields",
- "last_updated",
- "source_prefix",
+
+ # Add the additional fields to the serializer to support Extended ACL Rules.
+ fields = BaseACLRuleSerializer.Meta.fields + (
"source_ports",
"destination_prefix",
"destination_ports",
@@ -334,44 +350,33 @@ class Meta:
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_ports set.
- - Check if action set to remark, but destination_prefix 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.
+ - 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_ports set.
+ - Check if action set to remark, but destination_prefix 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.
"""
error_message = {}
+ rule_attributes = [
+ "source_prefix",
+ "source_ports",
+ "destination_prefix",
+ "destination_ports",
+ "protocol",
+ ]
# Check if action set to remark, but no remark set.
if data.get("action") == "remark" and data.get("remark") is None:
- 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 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"] = [
- "Action is set to remark, Destination Prefix CANNOT be set.",
- ]
- # Check if action set to remark, but destination_ports set.
- if data.get("destination_ports"):
- error_message["destination_ports"] = [
- "Action is set to remark, Destination Ports CANNOT be set.",
- ]
- # Check if action set to remark, but protocol set.
- if data.get("protocol"):
- error_message["protocol"] = [
- "Action is set to remark, Protocol CANNOT be set.",
- ]
+ error_message["remark"] = [ERROR_MESSAGE_NO_REMARK]
+
+ # Check if action set to remark, but other fields set.
+ for attribute in rule_attributes:
+ if data.get(attribute):
+ error_message[attribute] = [
+ f'Action is set to remark, {attribute.replace("_", " ").title()} CANNOT be set.'
+ ]
if error_message:
raise serializers.ValidationError(error_message)
diff --git a/netbox_acls/forms/constants.py b/netbox_acls/forms/constants.py
new file mode 100644
index 0000000..dac7a8e
--- /dev/null
+++ b/netbox_acls/forms/constants.py
@@ -0,0 +1,25 @@
+"""
+Constants for forms
+"""
+from django.utils.safestring import mark_safe
+
+# Sets a standard mark_safe help_text value to be used by the various classes
+HELP_TEXT_ACL_RULE_LOGIC = mark_safe(
+ "*Note: CANNOT be set if action is set to remark.",
+)
+
+# Sets a standard help_text value to be used by the various classes for acl action
+HELP_TEXT_ACL_ACTION = "Action the rule will take (remark, deny, or allow)."
+
+# Sets a standard help_text value to be used by the various classes for acl index
+HELP_TEXT_ACL_RULE_INDEX = (
+ "Determines the order of the rule in the ACL processing. AKA Sequence Number."
+)
+
+# 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 not set to remark, but no remark is set.
+ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK = (
+ "CANNOT set remark unless action is set to remark."
+)
diff --git a/netbox_acls/forms/filtersets.py b/netbox_acls/forms/filtersets.py
index f227e3f..a833e7b 100644
--- a/netbox_acls/forms/filtersets.py
+++ b/netbox_acls/forms/filtersets.py
@@ -27,6 +27,7 @@
ACLExtendedRule,
ACLInterfaceAssignment,
ACLStandardRule,
+ BaseACLRule,
)
__all__ = (
@@ -37,11 +38,18 @@
)
-class AccessListFilterForm(NetBoxModelFilterSetForm):
+class BaseACLFilterForm(NetBoxModelFilterSetForm):
"""
- GUI filter form to search the django AccessList model.
+ GUI filter inherited base form to search the django ACL and ACL Interface Assignment models.
"""
+ class Meta:
+ """
+ Sets the parent class as an abstract class to be inherited by other classes.
+ """
+
+ abstract = True
+
model = AccessList
region = DynamicModelChoiceField(
queryset=Region.objects.all(),
@@ -70,6 +78,14 @@ class AccessListFilterForm(NetBoxModelFilterSetForm):
queryset=VirtualMachine.objects.all(),
required=False,
)
+
+
+class AccessListFilterForm(BaseACLFilterForm):
+ """
+ GUI filter form to search the django AccessList model.
+ """
+
+ model = AccessList
virtual_chassis = DynamicModelChoiceField(
queryset=VirtualChassis.objects.all(),
required=False,
@@ -94,43 +110,20 @@ class AccessListFilterForm(NetBoxModelFilterSetForm):
"site_group",
"site",
"device",
- "virtual_chassis",
"virtual_machine",
+ "virtual_chassis",
),
),
("ACL Details", ("type", "default_action")),
)
-class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm):
+class ACLInterfaceAssignmentFilterForm(BaseACLFilterForm):
"""
GUI filter form to search the django AccessList model.
"""
model = ACLInterfaceAssignment
- region = DynamicModelChoiceField(
- queryset=Region.objects.all(),
- required=False,
- )
- site_group = DynamicModelChoiceField(
- queryset=SiteGroup.objects.all(),
- required=False,
- label="Site Group",
- )
- site = DynamicModelChoiceField(
- queryset=Site.objects.all(),
- required=False,
- query_params={"region_id": "$region", "group_id": "$site_group"},
- )
- device = DynamicModelChoiceField(
- queryset=Device.objects.all(),
- query_params={
- "region_id": "$region",
- "group_id": "$site_group",
- "site_id": "$site",
- },
- required=False,
- )
interface = DynamicModelChoiceField(
queryset=Interface.objects.all(),
required=False,
@@ -138,25 +131,13 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm):
"device_id": "$device",
},
)
- virtual_machine = DynamicModelChoiceField(
- queryset=VirtualMachine.objects.all(),
- required=False,
- label="Virtual Machine",
- )
vminterface = DynamicModelChoiceField(
queryset=VMInterface.objects.all(),
required=False,
query_params={
"virtual_machine_id": "$virtual_machine",
},
- label="Interface",
- )
- access_list = DynamicModelChoiceField(
- queryset=AccessList.objects.all(),
- query_params={
- "assigned_object": "$device",
- },
- label="Access List",
+ label="VM Interface",
)
direction = ChoiceField(
choices=add_blank_choice(ACLAssignmentDirectionChoices),
@@ -164,63 +145,75 @@ class ACLInterfaceAssignmentFilterForm(NetBoxModelFilterSetForm):
)
tag = TagFilterField(model)
- # fieldsets = (
- # (None, ('q', 'tag')),
- # ('Host Details', ('region', 'site_group', 'site', 'device')),
- # ('ACL Details', ('type', 'default_action')),
- # )
+ fieldsets = (
+ (None, ("q", "tag")),
+ (
+ "Host Details",
+ (
+ "region",
+ "site_group",
+ "site",
+ "device",
+ "virtual_machine",
+ ),
+ ),
+ ("Interface Details", ("interface", "vminterface")),
+ )
-class ACLStandardRuleFilterForm(NetBoxModelFilterSetForm):
+class BaseACLRuleFilterForm(NetBoxModelFilterSetForm):
"""
- GUI filter form to search the django ACLStandardRule model.
+ GUI filter inherited base form to search the django ACL rule models.
"""
- model = ACLStandardRule
- tag = TagFilterField(model)
+ class Meta:
+ """
+ Sets the parent class as an abstract class to be inherited by other classes.
+ """
+
+ abstract = True
+
+ model = BaseACLRule
+ index = forms.IntegerField(
+ required=False,
+ )
access_list = DynamicModelMultipleChoiceField(
queryset=AccessList.objects.all(),
required=False,
)
+ action = ChoiceField(
+ choices=add_blank_choice(ACLRuleActionChoices),
+ required=False,
+ )
source_prefix = DynamicModelMultipleChoiceField(
queryset=Prefix.objects.all(),
required=False,
label="Source Prefix",
)
- action = ChoiceField(
- choices=add_blank_choice(ACLRuleActionChoices),
- required=False,
- )
fieldsets = (
(None, ("q", "tag")),
("Rule Details", ("access_list", "action", "source_prefix")),
)
-class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm):
+class ACLStandardRuleFilterForm(BaseACLRuleFilterForm):
+ """
+ GUI filter form to search the django ACLStandardRule model.
+ """
+
+ model = ACLStandardRule
+
+ tag = TagFilterField(model)
+
+
+class ACLExtendedRuleFilterForm(BaseACLRuleFilterForm):
"""
GUI filter form to search the django ACLExtendedRule model.
"""
model = ACLExtendedRule
- index = forms.IntegerField(
- required=False,
- )
tag = TagFilterField(model)
- access_list = DynamicModelMultipleChoiceField(
- queryset=AccessList.objects.all(),
- required=False,
- )
- action = ChoiceField(
- choices=add_blank_choice(ACLRuleActionChoices),
- required=False,
- )
- source_prefix = DynamicModelMultipleChoiceField(
- queryset=Prefix.objects.all(),
- required=False,
- label="Source Prefix",
- )
- desintation_prefix = DynamicModelMultipleChoiceField(
+ destination_prefix = DynamicModelMultipleChoiceField(
queryset=Prefix.objects.all(),
required=False,
label="Destination Prefix",
@@ -229,17 +222,9 @@ class ACLExtendedRuleFilterForm(NetBoxModelFilterSetForm):
choices=add_blank_choice(ACLProtocolChoices),
required=False,
)
-
- fieldsets = (
- (None, ("q", "tag")),
+ fieldsets = BaseACLRuleFilterForm.fieldsets[:-1] + (
(
"Rule Details",
- (
- "access_list",
- "action",
- "source_prefix",
- "desintation_prefix",
- "protocol",
- ),
+ BaseACLRuleFilterForm.fieldsets[-1][1] + ("destination_prefix", "protocol"),
),
)
diff --git a/netbox_acls/forms/models.py b/netbox_acls/forms/models.py
index 4dc1738..8c1c4c0 100644
--- a/netbox_acls/forms/models.py
+++ b/netbox_acls/forms/models.py
@@ -23,6 +23,14 @@
ACLExtendedRule,
ACLInterfaceAssignment,
ACLStandardRule,
+ BaseACLRule,
+)
+from .constants import (
+ ERROR_MESSAGE_NO_REMARK,
+ ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK,
+ HELP_TEXT_ACL_ACTION,
+ HELP_TEXT_ACL_RULE_INDEX,
+ HELP_TEXT_ACL_RULE_LOGIC,
)
__all__ = (
@@ -32,28 +40,6 @@
"ACLExtendedRuleForm",
)
-# Sets a standard mark_safe help_text value to be used by the various classes
-help_text_acl_rule_logic = mark_safe(
- "*Note: CANNOT be set if action is set to remark.",
-)
-# Sets a standard help_text value to be used by the various classes for acl action
-help_text_acl_action = "Action the rule will take (remark, deny, or allow)."
-# Sets a standard help_text value to be used by the various classes for acl index
-help_text_acl_rule_index = (
- "Determines the order of the rule in the ACL processing. AKA Sequence Number."
-)
-
-# 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 not set to remark, but no remark is set.
-error_message_remark_without_action_remark = (
- "CANNOT set remark unless action is set to remark."
-)
-
class AccessListForm(NetBoxModelForm):
"""
@@ -133,6 +119,10 @@ class AccessListForm(NetBoxModelForm):
comments = CommentField()
class Meta:
+ """
+ Defines the Model and fields to be used by the form.
+ """
+
model = AccessList
fields = (
"region",
@@ -156,16 +146,19 @@ class Meta:
}
def __init__(self, *args, **kwargs):
+ """
+ Initializes the form
+ """
# Initialize helper selectors
instance = kwargs.get("instance")
initial = kwargs.get("initial", {}).copy()
if instance:
- if type(instance.assigned_object) is Device:
+ if isinstance(instance.assigned_object, Device):
initial["device"] = instance.assigned_object
- elif type(instance.assigned_object) is VirtualChassis:
+ elif isinstance(instance.assigned_object, VirtualChassis):
initial["virtual_chassis"] = instance.assigned_object
- elif type(instance.assigned_object) is VirtualMachine:
+ elif isinstance(instance.assigned_object, VirtualMachine):
initial["virtual_machine"] = instance.assigned_object
kwargs["initial"] = initial
@@ -174,52 +167,69 @@ def __init__(self, *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.
+ - 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.
"""
+ # TODO: Refactor this method to fix error message logic.
cleaned_data = super().clean()
- error_message = {}
if self.errors.get("name"):
return cleaned_data
+
name = cleaned_data.get("name")
acl_type = cleaned_data.get("type")
- device = cleaned_data.get("device")
- virtual_chassis = cleaned_data.get("virtual_chassis")
- virtual_machine = 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)
- ):
+ host_types = self._get_host_types()
+
+ # Check if no hosts selected.
+ self._clean_check_host_types(host_types)
+
+ host_type, host = host_types[0]
+
+ # Check if duplicate entry.
+ self._clean_check_duplicate_entry(name, host_type, host)
+ # Check if Access List has no existing rules before change the Access List's type.
+ self._clean_check_acl_type_change(acl_type)
+
+ return cleaned_data
+
+ def _get_host_types(self):
+ """
+ Get host type assigned to the Access List.
+ """
+ device = self.cleaned_data.get("device")
+ virtual_chassis = self.cleaned_data.get("virtual_chassis")
+ virtual_machine = self.cleaned_data.get("virtual_machine")
+ host_types = [
+ ("device", device),
+ ("virtual_chassis", virtual_chassis),
+ ("virtual_machine", virtual_machine),
+ ]
+ return [x for x in host_types if x[1]]
+
+ def _clean_check_host_types(self, host_types):
+ """
+ Used by parent class's clean method. Check number of host types selected.
+ """
+ if len(host_types) > 1:
raise forms.ValidationError(
"Access Lists must be assigned to one host (either a device, virtual chassis or virtual machine) at a time.",
)
# Check if no hosts selected.
- if not device and not virtual_chassis and not virtual_machine:
+ if not host_types:
raise forms.ValidationError(
"Access Lists must be assigned to a device, virtual chassis or virtual machine.",
)
- 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()
- else:
- host_type = "virtual_chassis"
- existing_acls = AccessList.objects.filter(
- name=name,
- virtual_chassis=virtual_chassis,
- ).exists()
-
+ def _clean_check_duplicate_entry(self, name, host_type, host):
+ """
+ Used by parent class's clean method. Check if duplicate entry. (Because of GFK.)
+ """
+ existing_acls = AccessList.objects.filter(
+ name=name, **{host_type: host}
+ ).exists()
# Check if duplicate entry.
if (
"name" in self.changed_data or host_type in self.changed_data
@@ -227,30 +237,45 @@ def clean(self):
error_same_acl_name = (
"An ACL with this name is already associated to this host."
)
- error_message |= {
- host_type: [error_same_acl_name],
- "name": [error_same_acl_name],
- }
+ raise forms.ValidationError(
+ {
+ host_type: [error_same_acl_name],
+ "name": [error_same_acl_name],
+ }
+ )
+
+ def _clean_check_acl_type_change(self, acl_type):
+ """
+ Used by parent class's clean method. Check if Access List has no existing rules before change the Access List's type.
+ """
if self.instance.pk:
- # Check if Access List has no existing rules before change the Access List's type.
+ ERROR_MESSAGE_EXISTING_RULES = (
+ "This ACL has ACL rules associated, CANNOT change ACL type.",
+ )
if (
acl_type == ACLTypeChoices.TYPE_EXTENDED
and self.instance.aclstandardrules.exists()
- ) or (
+ ):
+ raise forms.ValidationError(
+ {
+ "type": ERROR_MESSAGE_EXISTING_RULES,
+ }
+ )
+
+ if (
acl_type == ACLTypeChoices.TYPE_STANDARD
and self.instance.aclextendedrules.exists()
):
- error_message["type"] = [
- "This ACL has ACL rules associated, CANNOT change ACL type.",
- ]
-
- if error_message:
- raise forms.ValidationError(error_message)
-
- return cleaned_data
+ raise forms.ValidationError(
+ {
+ "type": ERROR_MESSAGE_EXISTING_RULES,
+ }
+ )
def save(self, *args, **kwargs):
- # Set assigned object
+ """
+ Set assigned object
+ """
self.instance.assigned_object = (
self.cleaned_data.get("device")
or self.cleaned_data.get("virtual_chassis")
@@ -315,15 +340,17 @@ class ACLInterfaceAssignmentForm(NetBoxModelForm):
comments = CommentField()
def __init__(self, *args, **kwargs):
+ """
+ Initialize helper selectors
+ """
- # Initialize helper selectors
instance = kwargs.get("instance")
initial = kwargs.get("initial", {}).copy()
if instance:
- if type(instance.assigned_object) is Interface:
+ if isinstance(instance.assigned_object, Interface):
initial["interface"] = instance.assigned_object
initial["device"] = "device"
- elif type(instance.assigned_object) is VMInterface:
+ elif isinstance(instance.assigned_object, VMInterface):
initial["vminterface"] = instance.assigned_object
initial["virtual_machine"] = "virtual_machine"
kwargs["initial"] = initial
@@ -331,6 +358,10 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
class Meta:
+ """
+ Defines the Model and fields to be used by the form.
+ """
+
model = ACLInterfaceAssignment
fields = (
"access_list",
@@ -351,109 +382,149 @@ class Meta:
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.
+ - 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.
"""
+ # Call the parent class's `clean` method
cleaned_data = super().clean()
- error_message = {}
- access_list = cleaned_data.get("access_list")
- direction = cleaned_data.get("direction")
- interface = cleaned_data.get("interface")
- vminterface = 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."
+
+ # Get the interface types assigned to the Access List
+ interface_types = self._clean_get_interface_types()
+
+ # Initialize an error message variable
+ self._clean_check_interface_types(interface_types)
+
+ # Get the assigned interface & interface type
+ assigned_object_type, assigned_object = interface_types[0]
+
+ # Get the parent host (device or virtual machine) of the assigned interface
+ if assigned_object_type == "interface":
+ assigned_object_id = Interface.objects.get(pk=assigned_object.pk).pk
+ else:
+ assigned_object_id = VMInterface.objects.get(pk=assigned_object.pk).pk
+
+ # Get the ContentType id for the assigned object
+ assigned_object_type_id = ContentType.objects.get_for_model(assigned_object).pk
+
+ # Check if the parent host is assigned to the Access List
+ self._clean_check_if_interface_parent_is_assigned_to_access_list(
+ cleaned_data.get("access_list"), assigned_object_type, assigned_object
+ )
+
+ # Check for duplicate entries in the Access List
+ self._clean_check_if_interface_already_has_acl_in_direction(
+ cleaned_data.get("access_list"),
+ assigned_object_id,
+ assigned_object_type,
+ assigned_object_type_id,
+ cleaned_data.get("direction"),
+ )
+
+ return cleaned_data
+
+ def _clean_get_interface_types(self):
+ """
+ Used by parent class's clean method. Get interface type/model assigned to the Access List.
+ """
+ interface = self.cleaned_data.get("interface")
+ vminterface = self.cleaned_data.get("vminterface")
+ interface_types = [
+ ("interface", interface),
+ ("vminterface", vminterface),
+ ]
+ return [x for x in interface_types if x[1]]
+
+ def _clean_check_interface_types(self, interface_types):
+ """
+ Used by parent class's clean method. Check if number of interface type selected is 1.
+ """
+ # Check if more than 1 hosts selected.
+ if len(interface_types) > 1:
+ raise forms.ValidationError(
+ "Assignment can only be to one interface at a time (either a interface or vminterface)."
)
- error_message |= {
- "interface": [error_no_interface],
- "vminterface": [error_no_interface],
- }
+ # Check if no hosts selected.
+ elif not interface_types:
+ raise forms.ValidationError("No interface or vminterface selected.")
+
+ def _clean_check_if_interface_parent_is_assigned_to_access_list(
+ self, access_list, assigned_object_type, assigned_object
+ ):
+ """
+ Used by parent class's clean method. Check that an interface's parent device/virtual_machine is assigned to the Access List.
+ """
+ access_list_host = AccessList.objects.get(pk=access_list.pk).assigned_object
+ host_type = (
+ "device" if assigned_object_type == "interface" else "virtual_machine"
+ )
+ if assigned_object_type == "interface":
+ host = Interface.objects.get(pk=assigned_object.pk).device
else:
- if interface:
- assigned_object = interface
- assigned_object_type = "interface"
- host_type = "device"
- host = Interface.objects.get(pk=assigned_object.pk).device
- assigned_object_id = Interface.objects.get(pk=assigned_object.pk).pk
- 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 = VMInterface.objects.get(pk=assigned_object.pk).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.
- if ACLInterfaceAssignment.objects.filter(
- access_list=access_list,
- assigned_object_id=assigned_object_id,
- assigned_object_type=assigned_object_type_id,
- direction=direction,
- ).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],
+ host = VMInterface.objects.get(pk=assigned_object.pk).virtual_machine
+
+ if access_list_host != host:
+ ERROR_ACL_NOT_ASSIGNED_TO_HOST = "Access List not present on selected host."
+ raise forms.ValidationError(
+ {
+ "access_list": [ERROR_ACL_NOT_ASSIGNED_TO_HOST],
+ host_type: [ERROR_ACL_NOT_ASSIGNED_TO_HOST],
+ assigned_object_type: [ERROR_ACL_NOT_ASSIGNED_TO_HOST],
}
- # Check that the interface does not have an existing ACL applied in the direction already.
- 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 |= {
+ )
+
+ def _clean_check_if_interface_already_has_acl_in_direction(
+ self,
+ access_list,
+ assigned_object_id,
+ assigned_object_type,
+ assigned_object_type_id,
+ direction,
+ ):
+ """
+ Check that the interface does not have an existing ACL applied in the direction already.
+ """
+
+ # Check for duplicate entry. (Because of GFK)
+ if ACLInterfaceAssignment.objects.filter(
+ access_list=access_list,
+ assigned_object_id=assigned_object_id,
+ assigned_object_type=assigned_object_type_id,
+ direction=direction,
+ ).exists():
+ raise forms.ValidationError({"access_list": ["Duplicate entry."]})
+
+ # Check that the interface does not have an existing ACL applied in the direction already.
+ 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."
+ )
+ raise forms.ValidationError(
+ {
"direction": [error_interface_already_assigned],
assigned_object_type: [error_interface_already_assigned],
}
-
- if error_message:
- raise forms.ValidationError(error_message)
- return cleaned_data
+ )
def save(self, *args, **kwargs):
- # Set assigned object
+ """
+ Set assigned object
+ """
self.instance.assigned_object = self.cleaned_data.get(
"interface",
) or self.cleaned_data.get("vminterface")
return super().save(*args, **kwargs)
-class ACLStandardRuleForm(NetBoxModelForm):
- """
- GUI form to add or edit Standard Access List.
- Requires an access_list, an index, and ACL rule type.
- See the clean function for logic on other field requirements.
- """
+class BaseACLRuleForm(NetBoxModelForm):
+ """GUI form to add or edit Access List Rules to be inherited by other classes"""
access_list = DynamicModelChoiceField(
queryset=AccessList.objects.all(),
@@ -461,14 +532,14 @@ class ACLStandardRuleForm(NetBoxModelForm):
"type": ACLTypeChoices.TYPE_STANDARD,
},
help_text=mark_safe(
- "*Note: This field will only display Standard ACLs.",
+ "*Note: This field will only display Standard ACLs."
),
label="Access List",
)
source_prefix = DynamicModelChoiceField(
queryset=Prefix.objects.all(),
required=False,
- help_text=help_text_acl_rule_logic,
+ help_text=HELP_TEXT_ACL_RULE_LOGIC,
label="Source Prefix",
)
@@ -478,55 +549,98 @@ class ACLStandardRuleForm(NetBoxModelForm):
)
class Meta:
- model = ACLStandardRule
+ """Defines the Model and fields to be used by the form."""
+
+ abstract = True
+ model = BaseACLRule
fields = (
"access_list",
"index",
"action",
"remark",
- "source_prefix",
"tags",
"description",
)
+ # Adds a help text to the form fields where the field is not defined in the form class.
help_texts = {
- "index": help_text_acl_rule_index,
- "action": help_text_acl_action,
+ "action": HELP_TEXT_ACL_ACTION,
+ "destination_ports": HELP_TEXT_ACL_RULE_LOGIC,
+ "index": HELP_TEXT_ACL_RULE_INDEX,
+ "protocol": HELP_TEXT_ACL_RULE_LOGIC,
"remark": mark_safe(
- "*Note: CANNOT be set if source prefix OR action is set.",
+ "*Note: CANNOT be set if action is not set to remark."
),
+ "source_ports": HELP_TEXT_ACL_RULE_LOGIC,
}
- def clean(self):
+ def clean_check_remark_rule(self, cleaned_data, error_message, rule_type):
"""
- Validates form inputs before submitting:
- - Check if action set to remark, but no remark set.
- - Check if action set to remark, but source_prefix set.
- - Check remark set, but action not set to remark.
+ Used by parent class's clean method. Checks form inputs before submitting:
+ - Check if action set to remark, but no remark set.
+ - Check if action set to remark, but other rule attributes set.
"""
+ # Check if action set to remark, but no remark set.
+ if not cleaned_data.get("remark"):
+ error_message["remark"] = [ERROR_MESSAGE_NO_REMARK]
+
+ # list all the fields of a rule besides the remark
+ if rule_type == "extended":
+ rule_attributes = [
+ "source_prefix",
+ "source_ports",
+ "destination_prefix",
+ "destination_ports",
+ "protocol",
+ ]
+ else:
+ rule_attributes = ["source_prefix"]
+
+ # Check if action set to remark, but other fields set.
+ for attribute in rule_attributes:
+ if cleaned_data.get(attribute):
+ error_message[attribute] = [
+ f'Action is set to remark, {attribute.replace("_", " ").title()} CANNOT be set.'
+ ]
+
+ if error_message:
+ forms.ValidationError(error_message)
+
+
+class ACLStandardRuleForm(BaseACLRuleForm):
+ """
+ GUI form to add or edit Standard Access List.
+ Requires an access_list, an index, and ACL rule type.
+ See the clean function for logic on other field requirements.
+ """
+
+ class Meta(BaseACLRuleForm.Meta):
+ """Defines the Model and fields to be used by the form."""
+
+ model = ACLStandardRule
+ # Need to add source_prefix to the tuple here instead of base or it will cause a ValueError
+ fields = BaseACLRuleForm.Meta.fields + ("source_prefix",)
+
+ def clean(self):
cleaned_data = super().clean()
error_message = {}
- # No need to check for unique_together since there is no usage of GFK
-
+ # Check if action set to remark, but no remark set OR other fields set.
if cleaned_data.get("action") == "remark":
- # Check if action set to remark, but no remark set.
- if not cleaned_data.get("remark"):
- error_message["remark"] = [error_message_no_remark]
- # Check if action set to remark, but source_prefix set.
- if cleaned_data.get("source_prefix"):
- error_message["source_prefix"] = [
- error_message_action_remark_source_prefix_set,
- ]
+ BaseACLRuleForm.clean_check_remark_rule(
+ self, cleaned_data, error_message, rule_type="standard"
+ )
+
# Check remark set, but action not set to remark.
elif cleaned_data.get("remark"):
- error_message["remark"] = [error_message_remark_without_action_remark]
+ error_message["remark"] = [ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK]
if error_message:
raise forms.ValidationError(error_message)
+
return cleaned_data
-class ACLExtendedRuleForm(NetBoxModelForm):
+class ACLExtendedRuleForm(BaseACLRuleForm):
"""
GUI form to add or edit Extended Access List.
Requires an access_list, an index, and ACL rule type.
@@ -535,120 +649,54 @@ class ACLExtendedRuleForm(NetBoxModelForm):
access_list = DynamicModelChoiceField(
queryset=AccessList.objects.all(),
- query_params={
- "type": ACLTypeChoices.TYPE_EXTENDED,
- },
+ query_params={"type": ACLTypeChoices.TYPE_EXTENDED},
help_text=mark_safe(
"*Note: This field will only display Extended ACLs.",
),
label="Access List",
)
- source_prefix = DynamicModelChoiceField(
- queryset=Prefix.objects.all(),
- required=False,
- help_text=help_text_acl_rule_logic,
- label="Source Prefix",
- )
destination_prefix = DynamicModelChoiceField(
queryset=Prefix.objects.all(),
required=False,
- help_text=help_text_acl_rule_logic,
+ help_text=HELP_TEXT_ACL_RULE_LOGIC,
label="Destination Prefix",
)
- fieldsets = (
- ("Access List Details", ("access_list", "description", "tags")),
+ fieldsets = BaseACLRuleForm.fieldsets[:-1] + (
(
"Rule Definition",
- (
- "index",
- "action",
- "remark",
- "source_prefix",
- "source_ports",
- "destination_prefix",
- "destination_ports",
- "protocol",
- ),
+ BaseACLRuleForm.fieldsets[-1][1]
+ + ("source_ports", "destination_prefix", "destination_ports", "protocol"),
),
)
- class Meta:
+ class Meta(BaseACLRuleForm.Meta):
+ """Defines the Model and fields to be used by the form."""
+
model = ACLExtendedRule
- fields = (
- "access_list",
- "index",
- "action",
- "remark",
+ fields = BaseACLRuleForm.Meta.fields + (
"source_prefix",
"source_ports",
"destination_prefix",
"destination_ports",
"protocol",
- "tags",
- "description",
)
- help_texts = {
- "action": help_text_acl_action,
- "destination_ports": help_text_acl_rule_logic,
- "index": help_text_acl_rule_index,
- "protocol": help_text_acl_rule_logic,
- "remark": mark_safe(
- "*Note: CANNOT be set if action is not set to remark.",
- ),
- "source_ports": help_text_acl_rule_logic,
- }
def clean(self):
- """
- Validates form inputs before submitting:
- - 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_ports set.
- - Check if action set to remark, but destination_prefix set.
- - Check if action set to remark, but destination_ports set.
- - Check if action set to remark, but destination_ports set.
- - Check if action set to remark, but protocol set.
- - Check remark set, but action not set to remark.
- """
cleaned_data = super().clean()
error_message = {}
- # No need to check for unique_together since there is no usage of GFK
-
+ # Check if action set to remark, but no remark set OR other fields set.
if cleaned_data.get("action") == "remark":
- # Check if action set to remark, but no remark set.
- if not cleaned_data.get("remark"):
- error_message["remark"] = [error_message_no_remark]
- # Check if action set to remark, but source_prefix set.
- if cleaned_data.get("source_prefix"):
- error_message["source_prefix"] = [
- error_message_action_remark_source_prefix_set,
- ]
- # Check if action set to remark, but source_ports set.
- if cleaned_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 cleaned_data.get("destination_prefix"):
- error_message["destination_prefix"] = [
- "Action is set to remark, Destination Prefix CANNOT be set.",
- ]
- # Check if action set to remark, but destination_ports set.
- if cleaned_data.get("destination_ports"):
- error_message["destination_ports"] = [
- "Action is set to remark, Destination Ports CANNOT be set.",
- ]
- # Check if action set to remark, but protocol set.
- if cleaned_data.get("protocol"):
- error_message["protocol"] = [
- "Action is set to remark, Protocol CANNOT be set.",
- ]
- # Check if action not set to remark, but remark set.
+ BaseACLRuleForm.clean_check_remark_rule(
+ self, cleaned_data, error_message, rule_type="extended"
+ )
+
+ # Check remark set, but action not set to remark.
elif cleaned_data.get("remark"):
- error_message["remark"] = [error_message_remark_without_action_remark]
+ error_message["remark"] = [ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK]
if error_message:
raise forms.ValidationError(error_message)
+
return cleaned_data
diff --git a/netbox_acls/migrations/0002_alter_accesslist_options_and_more.py b/netbox_acls/migrations/0002_alter_accesslist_options_and_more.py
index ea5ab86..902248f 100644
--- a/netbox_acls/migrations/0002_alter_accesslist_options_and_more.py
+++ b/netbox_acls/migrations/0002_alter_accesslist_options_and_more.py
@@ -6,7 +6,6 @@
class Migration(migrations.Migration):
-
dependencies = [
("contenttypes", "0002_remove_content_type_name"),
("netbox_acls", "0001_initial"),
diff --git a/netbox_acls/migrations/0003_netbox_acls.py b/netbox_acls/migrations/0003_netbox_acls.py
index 562e0f3..0af5aef 100644
--- a/netbox_acls/migrations/0003_netbox_acls.py
+++ b/netbox_acls/migrations/0003_netbox_acls.py
@@ -5,7 +5,6 @@
class Migration(migrations.Migration):
-
dependencies = [
("netbox_acls", "0002_alter_accesslist_options_and_more"),
]
diff --git a/netbox_acls/migrations/0004_netbox_acls.py b/netbox_acls/migrations/0004_netbox_acls.py
new file mode 100644
index 0000000..aeac436
--- /dev/null
+++ b/netbox_acls/migrations/0004_netbox_acls.py
@@ -0,0 +1,26 @@
+# Generated by Django 4.1.5 on 2023-02-01 22:47
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+ dependencies = [
+ ("netbox_acls", "0003_netbox_acls"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="accesslist",
+ name="name",
+ field=models.CharField(
+ max_length=500,
+ validators=[
+ django.core.validators.RegexValidator(
+ "^[a-zA-Z0-9-_]+$",
+ "Only alphanumeric, hyphens, and underscores characters are allowed.",
+ )
+ ],
+ ),
+ ),
+ ]
diff --git a/netbox_acls/models/access_list_rules.py b/netbox_acls/models/access_list_rules.py
index 5656720..465f562 100644
--- a/netbox_acls/models/access_list_rules.py
+++ b/netbox_acls/models/access_list_rules.py
@@ -12,13 +12,13 @@
from .access_lists import AccessList
__all__ = (
- "ACLRule",
+ "BaseACLRule",
"ACLStandardRule",
"ACLExtendedRule",
)
-class ACLRule(NetBoxModel):
+class BaseACLRule(NetBoxModel):
"""
Abstract model for ACL Rules.
Inherrited by both ACLStandardRule and ACLExtendedRule.
@@ -55,13 +55,22 @@ class ACLRule(NetBoxModel):
clone_fields = ("access_list", "action", "source_prefix")
def __str__(self):
+ """
+ Return a string representation of the model.
+ """
return f"{self.access_list}: Rule {self.index}"
def get_action_color(self):
+ """
+ Return the color for the action.
+ """
return ACLRuleActionChoices.colors.get(self.action)
@classmethod
def get_prerequisite_models(cls):
+ """
+ Return a list of prerequisite models for this model.
+ """
return [apps.get_model("ipam.Prefix"), AccessList]
class Meta:
@@ -77,9 +86,9 @@ class Meta:
unique_together = ["access_list", "index"]
-class ACLStandardRule(ACLRule):
+class ACLStandardRule(BaseACLRule):
"""
- Inherits ACLRule.
+ Inherits BaseACLRule.
"""
access_list = models.ForeignKey(
@@ -99,9 +108,12 @@ def get_absolute_url(self):
@classmethod
def get_prerequisite_models(cls):
+ """
+ Return a list of prerequisite models for this model.
+ """
return [AccessList]
- class Meta(ACLRule.Meta):
+ class Meta(BaseACLRule.Meta):
"""
Define the model properties adding to or overriding the inherited class:
- default_related_name for any FK relationships
@@ -113,9 +125,9 @@ class Meta(ACLRule.Meta):
verbose_name_plural = "ACL Standard Rules"
-class ACLExtendedRule(ACLRule):
+class ACLExtendedRule(BaseACLRule):
"""
- Inherits ACLRule.
+ Inherits BaseACLRule.
Add ACLExtendedRule specific fields: source_ports, desintation_prefix, destination_ports, and protocol
"""
@@ -123,7 +135,7 @@ class ACLExtendedRule(ACLRule):
on_delete=models.CASCADE,
to=AccessList,
verbose_name="Extended Access List",
- limit_choices_to={"type": "extended"},
+ limit_choices_to={"type": ACLTypeChoices.TYPE_EXTENDED},
related_name="aclextendedrules",
)
source_ports = ArrayField(
@@ -160,13 +172,19 @@ def get_absolute_url(self):
return reverse("plugins:netbox_acls:aclextendedrule", args=[self.pk])
def get_protocol_color(self):
+ """
+ Return the color for the protocol.
+ """
return ACLProtocolChoices.colors.get(self.protocol)
@classmethod
def get_prerequisite_models(cls):
+ """
+ Return a list of prerequisite models for this model.
+ xs"""
return [apps.get_model("ipam.Prefix"), AccessList]
- class Meta(ACLRule.Meta):
+ class Meta(BaseACLRule.Meta):
"""
Define the model properties adding to or overriding the inherited class:
- default_related_name for any FK relationships
diff --git a/netbox_acls/models/access_lists.py b/netbox_acls/models/access_lists.py
index 1feb635..33c6112 100644
--- a/netbox_acls/models/access_lists.py
+++ b/netbox_acls/models/access_lists.py
@@ -21,7 +21,7 @@
alphanumeric_plus = RegexValidator(
- r"^[0-9a-zA-Z,-,_]*$",
+ r"^[a-zA-Z0-9-_]+$",
"Only alphanumeric, hyphens, and underscores characters are allowed.",
)
@@ -65,12 +65,19 @@ class AccessList(NetBoxModel):
)
class Meta:
+ """
+ Define the ordering, unique constraints, and verbose names for the model.
+ """
+
unique_together = ["assigned_object_type", "assigned_object_id", "name"]
ordering = ["assigned_object_type", "assigned_object_id", "name"]
verbose_name = "Access List"
verbose_name_plural = "Access Lists"
def __str__(self):
+ """
+ Return a string representation of the model.
+ """
return self.name
def get_absolute_url(self):
@@ -81,9 +88,15 @@ def get_absolute_url(self):
return reverse("plugins:netbox_acls:accesslist", args=[self.pk])
def get_default_action_color(self):
+ """
+ Define the color of the default action.
+ """
return ACLActionChoices.colors.get(self.default_action)
def get_type_color(self):
+ """
+ Define the color of the type.
+ """
return ACLTypeChoices.colors.get(self.type)
@@ -121,6 +134,10 @@ class ACLInterfaceAssignment(NetBoxModel):
clone_fields = ("access_list", "direction")
class Meta:
+ """
+ Define the ordering, unique constraints, and verbose names for the model.
+ """
+
unique_together = [
"assigned_object_type",
"assigned_object_id",
@@ -148,9 +165,15 @@ def get_absolute_url(self):
@classmethod
def get_prerequisite_models(cls):
+ """
+ Return a list of models that must be defined before this model.
+ """
return [AccessList]
def get_direction_color(self):
+ """
+ Define the color of the direction.
+ """
return ACLAssignmentDirectionChoices.colors.get(self.direction)
diff --git a/netbox_acls/tables.py b/netbox_acls/tables.py
index 8552d78..a47988f 100644
--- a/netbox_acls/tables.py
+++ b/netbox_acls/tables.py
@@ -54,6 +54,10 @@ class AccessListTable(NetBoxTable):
)
class Meta(NetBoxTable.Meta):
+ """
+ Defines the table view for the AccessList model.
+ """
+
model = AccessList
fields = (
"pk",
@@ -103,6 +107,10 @@ class ACLInterfaceAssignmentTable(NetBoxTable):
)
class Meta(NetBoxTable.Meta):
+ """
+ Defines the table view for the ACLInterfaceAssignment model.
+ """
+
model = ACLInterfaceAssignment
fields = (
"pk",
@@ -140,6 +148,10 @@ class ACLStandardRuleTable(NetBoxTable):
)
class Meta(NetBoxTable.Meta):
+ """
+ Defines the table view for the ACLStandardRule model.
+ """
+
model = ACLStandardRule
fields = (
"pk",
@@ -182,6 +194,10 @@ class ACLExtendedRuleTable(NetBoxTable):
protocol = ChoiceFieldColumn()
class Meta(NetBoxTable.Meta):
+ """
+ Defines the table view for the ACLExtendedRule model.
+ """
+
model = ACLExtendedRule
fields = (
"pk",
diff --git a/netbox_acls/templates/inc/aclrule.html b/netbox_acls/templates/inc/aclrule.html
new file mode 100644
index 0000000..7ef1278
--- /dev/null
+++ b/netbox_acls/templates/inc/aclrule.html
@@ -0,0 +1,84 @@
+{% extends 'generic/object.html' %}
+
+{% block content %}
+
+
+
+
+
+
+ {{ object|meta:"verbose_name"|bettertitle }}
+
+ | Access List |
+
+ {{ object.access_list }}
+ |
+
+
+ | Index |
+ {{ object.index }} |
+
+
+ | Description |
+ {{ object.description|placeholder }} |
+
+
+
+
+ {% include 'inc/panels/custom_fields.html' %}
+ {% include 'inc/panels/tags.html' %}
+
+
+
+
+
+
+ Details
+
+ | Remark |
+ {{ object.get_remark_display|placeholder }} |
+
+
+ | Protocol |
+ {{ object.get_protocol_display|placeholder }} |
+
+
+ | Source Prefix |
+
+ {% if object.source_prefix %}
+ {{ object.source_prefix }}
+ {% else %}
+ {{ ''|placeholder }}
+ {% endif %}
+ |
+
+ {% if "extended" in object|meta:"verbose_name"|lower %}
+
+ | Source Ports |
+ {{ object.source_ports|join:", "|placeholder }} |
+
+
+ | Destination Prefix |
+
+ {% if object.destination_prefix %}
+ {{ object.destination_prefix }}
+ {% else %}
+ {{ ''|placeholder }}
+ {% endif %}
+ |
+
+
+ | Destination Ports |
+ {{ object.destination_ports|join:", "|placeholder }} |
+
+ {% endif %}
+
+ | Action |
+ {% badge object.get_action_display bg_color=object.get_action_color %} |
+
+
+
+
+
+
+
Custom Fields
+ {% render_custom_fields form %}
+
+{% endif %}
+
+
Comments
+ {% render_field form.comments %}
+
diff --git a/netbox_acls/templates/netbox_acls/accesslist_edit.html b/netbox_acls/templates/netbox_acls/accesslist_edit.html
index a197daa..4edd576 100644
--- a/netbox_acls/templates/netbox_acls/accesslist_edit.html
+++ b/netbox_acls/templates/netbox_acls/accesslist_edit.html
@@ -74,16 +74,5 @@ Host Assignment
-
-
Comments
- {% render_field form.comments %}
-
-{% if form.custom_fields %}
-
-
-
- {% render_custom_fields form %}
-
-
-
-
-
-
-
- ACL Extended Rule
-
- | Access List |
-
- {{ object.access_list }}
- |
-
-
- | Description |
- {{ object.description|placeholder }} |
-
-
- | Index |
- {{ object.index }} |
-
-
-
-
- {% include 'inc/panels/custom_fields.html' %}
- {% include 'inc/panels/tags.html' %}
-
-
-
-
-
-
- Details
-
- | Remark |
- {{ object.get_remark_display|placeholder }} |
-
-
- | Protocol |
- {{ object.get_protocol_display|placeholder }} |
-
-
- | Source Prefix |
-
- {% if object.source_prefix %}
- {{ object.source_prefix }}
- {% else %}
- {{ ''|placeholder }}
- {% endif %}
- |
-
-
- | Source Ports |
- {{ object.source_ports|join:", "|placeholder }} |
-
-
- | Destination Prefix |
-
- {% if object.destination_prefix %}
- {{ object.destination_prefix }}
- {% else %}
- {{ ''|placeholder }}
- {% endif %}
- |
-
-
- | Destination Ports |
- {{ object.destination_ports|join:", "|placeholder }} |
-
-
- | Action |
- {% badge object.get_action_display bg_color=object.get_action_color %} |
-
-
-
-
-
-
Interface Assignment
-
-
Comments
- {% render_field form.comments %}
-
-{% if form.custom_fields %}
-
-
-
- {% render_custom_fields form %}
-
-
-
-
-
-
-
- ACL Standard Rule
-
- | Access List |
-
- {{ object.access_list }}
- |
-
-
- | Index |
- {{ object.index }} |
-
-
- | Description |
- {{ object.description|placeholder }} |
-
-
-
-
- {% include 'inc/panels/custom_fields.html' %}
- {% include 'inc/panels/tags.html' %}
-
-
-
-
-
-
- Details
-
- | Remark |
- {{ object.get_remark_display|placeholder }} |
-
-
- | Protocol |
- {{ object.get_protocol_display|placeholder }} |
-
-
- | Source Prefix |
-
- {% if object.source_prefix %}
- {{ object.source_prefix }}
- {% else %}
- {{ ''|placeholder }}
- {% endif %}
- |
-
-
- | Action |
- {% badge object.get_action_display bg_color=object.get_action_color %} |
-
-
-
-
-
-