From 11203e29dc07d0f937ef09390e8c0fcb97883067 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Thu, 2 Feb 2023 22:39:23 +0000 Subject: [PATCH 01/15] add draft test for plugin models --- netbox_acls/tests/test_models.py | 238 +++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 netbox_acls/tests/test_models.py diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py new file mode 100644 index 0000000..8c50c78 --- /dev/null +++ b/netbox_acls/tests/test_models.py @@ -0,0 +1,238 @@ +from dcim.models import ( + Device, + DeviceRole, + DeviceType, + Interface, + Manufacturer, + Site, + VirtualChassis, +) +from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError +from django.test import TestCase +from ipam.models import Prefix +from netaddr import IPNetwork +from virtualization.models import Cluster, ClusterType, VirtualMachine, VMInterface + +from netbox_acls.choices import * +from netbox_acls.models import * + + +class BaseTestCase(TestCase): + """ + Base test case for netbox_acls models. + """ + + @classmethod + def setUpTestData(cls): + """ + Create base data to test using including: + - 1 of each of the following: test site, manufacturer, device type, device role, cluster type, cluster, virtual_chassis, & virtual machine + - 2 devices, prefixes, 2 interfaces, and 2 vminterfaces + """ + + site = Site.objects.create(name="Site 1", slug="site-1") + manufacturer = Manufacturer.objects.create( + name="Manufacturer 1", + slug="manufacturer-1", + ) + devicetype = DeviceType.objects.create( + manufacturer=manufacturer, + model="Device Type 1", + ) + devicerole = DeviceRole.objects.create( + name="Device Role 1", + slug="device-role-1", + ) + device = Device.objects.create( + name="Device 1", + site=site, + device_type=devicetype, + device_role=devicerole, + ) + virtual_chassis = VirtualChassis.objects.create(name="Virtual Chassis 1") + virtual_chassis_member = Device.objects.create( + name="VC Device", + site=site, + device_type=devicetype, + device_role=devicerole, + virtual_chassis=virtual_chassis, + vc_position=1, + ) + cluster_member = Device.objects.create( + name="Cluster Device", + site=site, + device_type=devicetype, + device_role=devicerole, + ) + clustertype = ClusterType.objects.create(name="Cluster Type 1") + cluster = Cluster.objects.create( + name="Cluster 1", + type=clustertype, + ) + virtual_machine = VirtualMachine.objects.create(name="VirtualMachine 1") + + +class TestAccessList(BaseTestCase): + """ + Test AccessList model. + """ + + def test_alphanumeric_plus_success(self): + """ + Test that AccessList names with alphanumeric characters, '_', or '-' pass validation. + """ + acl_good_name = AccessList( + name="Testacl-good_name-1", + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + type=ACLTypeChoices.TYPE_EXTENDED, + default_action=ACLActionChoices.ACTION_PERMIT, + ) + acl_good_name.full_clean() + # TODO: test_alphanumeric_plus_success - VirtualChassis & Cluster + + def test_duplicate_name_success(self): + """ + Test that AccessList names can be non-unique if associated to different devices. + """ + + params = { + "name": "GOOD-DUPLICATE-ACL", + "type": ACLTypeChoices.TYPE_STANDARD, + "default_action": ACLActionChoices.ACTION_PERMIT, + } + AccessList.objects.create( + **params, + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + ) + vm_acl = AccessList( + **params, + assigned_object_type=ContentType.objects.get_for_model(VirtualMachine), + assigned_object_id=1, + ) + vm_acl.full_clean() + # TODO: test_duplicate_name_success - VirtualChassis & Cluster + #vc_acl = AccessList( + # **params, + # assigned_object_type=ContentType.objects.get_for_model(VirtualChassis), + # assigned_object_id=1, + #) + #vc_acl.full_clean() + + def test_alphanumeric_plus_fail(self): + """ + Test that AccessList names with non-alphanumeric (exluding '_' and '-') characters fail validation. + """ + non_alphanumeric_plus_chars = " !@#$%^&*()[]{};:,./<>?\|~=+" + + for i, char in enumerate(non_alphanumeric_plus_chars, start=1): + bad_acl_name = AccessList( + name=f"Testacl-bad_name_{i}_{char}", + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + type=ACLTypeChoices.TYPE_EXTENDED, + default_action=ACLActionChoices.ACTION_PERMIT, + comments=f'ACL with "{char}" in name', + ) + with self.assertRaises(ValidationError): + bad_acl_name.full_clean() + + def test_duplicate_name_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": 1, + "type": ACLTypeChoices.TYPE_STANDARD, + "default_action": ACLActionChoices.ACTION_PERMIT, + } + acl_1 = AccessList.objects.create(**params) + acl_1.save() + acl_2 = AccessList(**params) + with self.assertRaises(ValidationError): + acl_2.full_clean() + # TODO: test_duplicate_name_fail - VirtualChassis & Cluster + + # TODO: Test choices for AccessList Model + + +class TestACLInterfaceAssignment(BaseTestCase): + """ + Test ACLInterfaceAssignment model. + """ + + @classmethod + def setUpTestData(cls): + """ + Extend BaseTestCase's setUpTestData() to create additional data for testing. + """ + super().setUpTestData() + + #interfaces = Interface.objects.bulk_create( + # ( + # Interface(name="Interface 1", device=device, type="1000baset"), + # Interface(name="Interface 2", device=device, type="1000baset"), + # ) + #) + #vminterfaces = VMInterface.objects.bulk_create( + # ( + # VMInterface(name="Interface 1", virtual_machine=virtual_machine), + # VMInterface(name="Interface 2", virtual_machine=virtual_machine), + # ) + #) + # prefixes = Prefix.objects.bulk_create( + # ( + # Prefix(prefix=IPNetwork("10.0.0.0/24")), + # Prefix(prefix=IPNetwork("192.168.1.0/24")), + # ) + # ) + + 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. + """ + pass + # TODO: test_acl_interface_assignment_success - VM & Device + + def test_acl_interface_assignment_fail(self): + """ + Test that ACLInterfaceAssignment fails validation if the ACL is not assigned to the parent host. + """ + pass + # TODO: test_acl_interface_assignment_fail - VM & Device + + def test_duplicate_assignment_fail(self): + """ + Test that ACLInterfaceAssignment fails validation if the ACL already is assigned to the same interface and direction. + """ + pass + # TODO: test_duplicate_assignment_fail - VM & Device + + def test_acl_already_assinged_fail(self): + """ + Test that ACLInterfaceAssignment fails validation if the interface already has an ACL assigned in the same direction. + """ + pass + ## TODO: test_acl_already_assinged_fail - VM & Device + + # TODO: Test choices for ACLInterfaceAssignment Model + + +# TODO: Investigate a Base model for ACLStandardRule and ACLExtendedRule + +class TestACLStandardRule(BaseTestCase): + """ + Test ACLStandardRule model. + """ + # TODO: Develop tests for ACLStandardRule model + + +class ACLExtendedRule(BaseTestCase): + """ + Test ACLExtendedRule model. + """ + # TODO: Develop tests for ACLExtendedRule model From 9053922ab735cda92ca271a34cd31fa9b7e175d1 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Thu, 2 Feb 2023 22:50:13 +0000 Subject: [PATCH 02/15] Fix AccessList RegexValidator Bug --- .../migrations/0004_alter_accesslist_name.py | 26 +++++++++++++++++++ netbox_acls/models/access_lists.py | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 netbox_acls/migrations/0004_alter_accesslist_name.py diff --git a/netbox_acls/migrations/0004_alter_accesslist_name.py b/netbox_acls/migrations/0004_alter_accesslist_name.py new file mode 100644 index 0000000..3086dac --- /dev/null +++ b/netbox_acls/migrations/0004_alter_accesslist_name.py @@ -0,0 +1,26 @@ +# Generated by Django 4.1.5 on 2023-02-02 22:34 + +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_lists.py b/netbox_acls/models/access_lists.py index 1feb635..e65f765 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.", ) From 8efcfbba30af96df3c4fe4e47fb49b93b10c7a41 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Thu, 2 Feb 2023 23:41:19 +0000 Subject: [PATCH 03/15] add tmp to gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 2f771e8..031820d 100644 --- a/.gitignore +++ b/.gitignore @@ -163,3 +163,7 @@ cython_debug/ .vscode/ # JetBrains .idea/ + +# Temporary files +*.tmp +tmp/ From cd5753695ffb7c0ae5d536398c1efed2a18affc6 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Thu, 2 Feb 2023 23:55:41 +0000 Subject: [PATCH 04/15] update devcontainer --- .devcontainer/devcontainer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 2d3b513..6fa60d8 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -28,9 +28,8 @@ "isort.args": [ "--profile=black" ], - "isort.path": "/opt/netbox/venv/bin/isort", + "isort.path": ["/opt/netbox/venv/bin/isort"], "python.analysis.typeCheckingMode": "strict", - python.Jedi "python.analysis.extraPaths": [ "/opt/netbox/netbox" ], @@ -86,6 +85,7 @@ "aaron-bond.better-comments", "batisteo.vscode-django", "codezombiech.gitignore", + "eamodio.gitlens", "esbenp.prettier-vscode", "formulahendry.auto-rename-tag", "mintlify.document", From d9e8d03f448c8d2658d324e08940e55ef2944765 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 02:56:59 +0000 Subject: [PATCH 05/15] add coverage to gitignore --- .gitignore | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.gitignore b/.gitignore index 031820d..28a01be 100644 --- a/.gitignore +++ b/.gitignore @@ -167,3 +167,11 @@ cython_debug/ # Temporary files *.tmp tmp/ + +# coverage +coverage/ +htmlcov/ +.coverage +.coverage.* +coverage.xml +*.cover From 929fc9e9a062e15ae7cf64f69683885efb9b5c32 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 13:58:09 +0000 Subject: [PATCH 06/15] black formatting --- netbox_acls/forms/models.py | 2 -- .../migrations/0002_alter_accesslist_options_and_more.py | 1 - netbox_acls/migrations/0003_netbox_acls.py | 1 - 3 files changed, 4 deletions(-) diff --git a/netbox_acls/forms/models.py b/netbox_acls/forms/models.py index 8d04d3a..e034da6 100644 --- a/netbox_acls/forms/models.py +++ b/netbox_acls/forms/models.py @@ -149,7 +149,6 @@ class Meta: } def __init__(self, *args, **kwargs): - # Initialize helper selectors instance = kwargs.get("instance") initial = kwargs.get("initial", {}).copy() @@ -324,7 +323,6 @@ class ACLInterfaceAssignmentForm(NetBoxModelForm): comments = CommentField() def __init__(self, *args, **kwargs): - # Initialize helper selectors instance = kwargs.get("instance") initial = kwargs.get("initial", {}).copy() 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"), ] From d70cab503d67e0e912a93456b571d216905c25fa Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 13:58:43 +0000 Subject: [PATCH 07/15] more verbose tests --- Makefile | 2 +- test.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index fb3ba16..5c856fe 100644 --- a/Makefile +++ b/Makefile @@ -76,7 +76,7 @@ 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} + ${VENV_PY_PATH} ${NETBOX_MANAGE_PATH}/manage.py test ${PLUGIN_NAME} -v 2 #relpatch: # $(eval GSTATUS := $(shell git status --porcelain)) diff --git a/test.sh b/test.sh index 5fc2e06..7f16723 100755 --- a/test.sh +++ b/test.sh @@ -17,7 +17,7 @@ doco="docker compose --file docker-compose.yml" test_netbox_unit_tests() { echo "⏱ Running NetBox Unit Tests" $doco run --rm netbox python manage.py makemigrations netbox_acls --check - $doco run --rm netbox python manage.py test netbox_acls + $doco run --rm netbox python manage.py test netbox_acls -v 2 } test_cleanup() { From c062b6338010df373a89ab9909ae77bd3784b41e Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 14:38:28 +0000 Subject: [PATCH 08/15] minimize duplicate code in model tests --- netbox_acls/tests/test_models.py | 65 ++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index 8c50c78..5e3af74 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -71,6 +71,7 @@ def setUpTestData(cls): type=clustertype, ) virtual_machine = VirtualMachine.objects.create(name="VirtualMachine 1") + prefix = Prefix.objects.create(prefix="10.0.0.0/8") class TestAccessList(BaseTestCase): @@ -78,19 +79,35 @@ class TestAccessList(BaseTestCase): Test AccessList model. """ + common_acl_params = { + "assigned_object_id": 1, + "type": ACLTypeChoices.TYPE_EXTENDED, + "default_action": ACLActionChoices.ACTION_PERMIT, + } + + 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), + **self.common_acl_params, + ) + with self.assertRaises(ValidationError): + acl_bad_gfk.full_clean() + def test_alphanumeric_plus_success(self): """ Test that AccessList names with alphanumeric characters, '_', or '-' pass validation. """ acl_good_name = AccessList( - name="Testacl-good_name-1", + name="Testacl-Good_Name-1", assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, - type=ACLTypeChoices.TYPE_EXTENDED, - default_action=ACLActionChoices.ACTION_PERMIT, + **self.common_acl_params, ) acl_good_name.full_clean() - # TODO: test_alphanumeric_plus_success - VirtualChassis & Cluster + # TODO: test_alphanumeric_plus_success - VirtualChassis, VirtualMachine & Cluster def test_duplicate_name_success(self): """ @@ -103,23 +120,23 @@ def test_duplicate_name_success(self): "default_action": ACLActionChoices.ACTION_PERMIT, } AccessList.objects.create( - **params, + name="GOOD-DUPLICATE-ACL", assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, + **self.common_acl_params, ) vm_acl = AccessList( - **params, + name="GOOD-DUPLICATE-ACL", assigned_object_type=ContentType.objects.get_for_model(VirtualMachine), - assigned_object_id=1, + **self.common_acl_params, ) vm_acl.full_clean() - # TODO: test_duplicate_name_success - VirtualChassis & Cluster - #vc_acl = AccessList( - # **params, + # TODO: test_duplicate_name_success - VirtualChassis, VirtualMachine & Cluster + # vc_acl = AccessList( + # "name": "GOOD-DUPLICATE-ACL", # assigned_object_type=ContentType.objects.get_for_model(VirtualChassis), - # assigned_object_id=1, - #) - #vc_acl.full_clean() + # **self.common_acl_params, + # ) + # vc_acl.full_clean() def test_alphanumeric_plus_fail(self): """ @@ -131,10 +148,8 @@ def test_alphanumeric_plus_fail(self): bad_acl_name = AccessList( name=f"Testacl-bad_name_{i}_{char}", assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, - type=ACLTypeChoices.TYPE_EXTENDED, - default_action=ACLActionChoices.ACTION_PERMIT, comments=f'ACL with "{char}" in name', + **self.common_acl_params, ) with self.assertRaises(ValidationError): bad_acl_name.full_clean() @@ -146,9 +161,8 @@ def test_duplicate_name_fail(self): params = { "name": "FAIL-DUPLICATE-ACL", "assigned_object_type": ContentType.objects.get_for_model(Device), + **self.common_acl_params, "assigned_object_id": 1, - "type": ACLTypeChoices.TYPE_STANDARD, - "default_action": ACLActionChoices.ACTION_PERMIT, } acl_1 = AccessList.objects.create(**params) acl_1.save() @@ -172,18 +186,18 @@ def setUpTestData(cls): """ super().setUpTestData() - #interfaces = Interface.objects.bulk_create( + # interfaces = Interface.objects.bulk_create( # ( # Interface(name="Interface 1", device=device, type="1000baset"), # Interface(name="Interface 2", device=device, type="1000baset"), # ) - #) - #vminterfaces = VMInterface.objects.bulk_create( + # ) + # vminterfaces = VMInterface.objects.bulk_create( # ( # VMInterface(name="Interface 1", virtual_machine=virtual_machine), # VMInterface(name="Interface 2", virtual_machine=virtual_machine), # ) - #) + # ) # prefixes = Prefix.objects.bulk_create( # ( # Prefix(prefix=IPNetwork("10.0.0.0/24")), @@ -224,10 +238,12 @@ def test_acl_already_assinged_fail(self): # TODO: Investigate a Base model for ACLStandardRule and ACLExtendedRule + class TestACLStandardRule(BaseTestCase): """ Test ACLStandardRule model. """ + # TODO: Develop tests for ACLStandardRule model @@ -235,4 +251,5 @@ class ACLExtendedRule(BaseTestCase): """ Test ACLExtendedRule model. """ + # TODO: Develop tests for ACLExtendedRule model From 568feb031358267f99042c1ad1d227958d9dd77b Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 22:16:30 +0000 Subject: [PATCH 09/15] build acl models tests --- netbox_acls/tests/test_models.py | 72 ++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index 5e3af74..d7dc2ac 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -1,3 +1,5 @@ +from itertools import cycle + from dcim.models import ( Device, DeviceRole, @@ -14,7 +16,6 @@ from netaddr import IPNetwork from virtualization.models import Cluster, ClusterType, VirtualMachine, VMInterface -from netbox_acls.choices import * from netbox_acls.models import * @@ -81,8 +82,8 @@ class TestAccessList(BaseTestCase): common_acl_params = { "assigned_object_id": 1, - "type": ACLTypeChoices.TYPE_EXTENDED, - "default_action": ACLActionChoices.ACTION_PERMIT, + "type": "extended", + "default_action": "permit", } def test_wrong_assigned_object_type_fail(self): @@ -114,11 +115,6 @@ def test_duplicate_name_success(self): Test that AccessList names can be non-unique if associated to different devices. """ - params = { - "name": "GOOD-DUPLICATE-ACL", - "type": ACLTypeChoices.TYPE_STANDARD, - "default_action": ACLActionChoices.ACTION_PERMIT, - } AccessList.objects.create( name="GOOD-DUPLICATE-ACL", assigned_object_type=ContentType.objects.get_for_model(Device), @@ -171,7 +167,65 @@ def test_duplicate_name_fail(self): acl_2.full_clean() # TODO: test_duplicate_name_fail - VirtualChassis & Cluster - # TODO: Test choices for AccessList Model + def test_valid_acl_choices(self): + """ + Test that AccessList action choices using VALID choices. + """ + valid_acl_default_action_choices = ["permit", "deny"] + valid_acl_types = ["standard", "extended"] + if len(valid_acl_default_action_choices) > len(valid_acl_types): + valid_acl_choices = list( + zip(valid_acl_default_action_choices, cycle(valid_acl_types)) + ) + elif len(valid_acl_default_action_choices) < len(valid_acl_types): + valid_acl_choices = list( + zip(cycle(valid_acl_default_action_choices), valid_acl_types) + ) + else: + valid_acl_choices = list( + zip(valid_acl_default_action_choices, valid_acl_types) + ) + + for default_action, acl_type in valid_acl_choices: + valid_acl_choice = AccessList( + name=f"TestACL_Valid_Choice_{default_action}_{acl_type}", + comments=f"VALID ACL CHOICES USED: {default_action=} {acl_type=}", + type=acl_type, + default_action=default_action, + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + ) + valid_acl_choice.full_clean() + + def test_invalid_acl_choices(self): + """ + Test that AccessList action choices using INVALID choices. + """ + valid_acl_types = ["standard", "extended"] + 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]}", + comments=f"INVALID ACL DEFAULT CHOICE USED: default_action='{invalid_acl_default_action_choice}'", + type=valid_acl_types[0], + default_action=invalid_acl_default_action_choice, + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + ) + with self.assertRaises(ValidationError): + invalid_acl_default_action.full_clean() + + valid_acl_default_action_choices = ["permit", "deny"] + invalid_acl_type = "super-dupper-extended" + invalid_acl_type = AccessList( + name=f"TestACL_Valid_Choice_{valid_acl_default_action_choices[0]}_{invalid_acl_type}", + comments=f"INVALID ACL DEFAULT CHOICE USED: type='{invalid_acl_type}'", + type=invalid_acl_type, + default_action=valid_acl_default_action_choices[0], + assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, + ) + with self.assertRaises(ValidationError): + invalid_acl_type.full_clean() class TestACLInterfaceAssignment(BaseTestCase): From bf155343e69a4f105f35950425181ae15122b033 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Fri, 3 Feb 2023 22:16:38 +0000 Subject: [PATCH 10/15] add coverage --- .devcontainer/requirements-dev.txt | 1 + Makefile | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.devcontainer/requirements-dev.txt b/.devcontainer/requirements-dev.txt index d4a50d5..e772e59 100644 --- a/.devcontainer/requirements-dev.txt +++ b/.devcontainer/requirements-dev.txt @@ -13,3 +13,4 @@ pylint-django wily yapf sourcery-analytics +coverage diff --git a/Makefile b/Makefile index 5c856fe..55bce8a 100644 --- a/Makefile +++ b/Makefile @@ -75,8 +75,15 @@ 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} -v 2 + ${NETBOX_MANAGE_PATH}/manage.py makemigrations ${PLUGIN_NAME} --check + coverage run ${NETBOX_MANAGE_PATH}/manage.py test ${PLUGIN_NAME} -v 2 + +.PHONY: coverage_report +coverage_report: + coverage report + +.PHONY: test_coverage +test_coverage: test coverage_report #relpatch: # $(eval GSTATUS := $(shell git status --porcelain)) From 690d254cce1790f99086eecdef3d0d40d4fcfde9 Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Sun, 5 Feb 2023 13:47:32 +0000 Subject: [PATCH 11/15] test ACLInterfaceAssignment draft --- Makefile | 2 +- netbox_acls/tests/test_models.py | 93 ++++++++++++++++++++++---------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 55bce8a..259948c 100644 --- a/Makefile +++ b/Makefile @@ -76,7 +76,7 @@ rebuild: setup makemigrations migrate collectstatic start .PHONY: test test: setup ${NETBOX_MANAGE_PATH}/manage.py makemigrations ${PLUGIN_NAME} --check - coverage run ${NETBOX_MANAGE_PATH}/manage.py test ${PLUGIN_NAME} -v 2 + coverage run --source "netbox_acls" ${NETBOX_MANAGE_PATH}/manage.py test ${PLUGIN_NAME} -v 2 .PHONY: coverage_report coverage_report: diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index d7dc2ac..945788d 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -81,10 +81,10 @@ class TestAccessList(BaseTestCase): """ common_acl_params = { - "assigned_object_id": 1, "type": "extended", "default_action": "permit", } + # device = Device.objects.first() def test_wrong_assigned_object_type_fail(self): """ @@ -93,6 +93,7 @@ def test_wrong_assigned_object_type_fail(self): acl_bad_gfk = AccessList( name="TestACL_Wrong_GFK", assigned_object_type=ContentType.objects.get_for_model(Prefix), + assigned_object_id=Prefix.objects.first(), **self.common_acl_params, ) with self.assertRaises(ValidationError): @@ -105,6 +106,7 @@ def test_alphanumeric_plus_success(self): acl_good_name = AccessList( name="Testacl-Good_Name-1", assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, # TODO - replace with Device.objects.first() **self.common_acl_params, ) acl_good_name.full_clean() @@ -114,15 +116,16 @@ def test_duplicate_name_success(self): """ Test that AccessList names can be non-unique if associated to different devices. """ - AccessList.objects.create( name="GOOD-DUPLICATE-ACL", assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object_id=1, # TODO - replace with Device.objects.first() **self.common_acl_params, ) vm_acl = AccessList( name="GOOD-DUPLICATE-ACL", assigned_object_type=ContentType.objects.get_for_model(VirtualMachine), + assigned_object_id=1, # TODO - replace with VirtualMachine.objects.first().id, **self.common_acl_params, ) vm_acl.full_clean() @@ -158,7 +161,7 @@ def test_duplicate_name_fail(self): "name": "FAIL-DUPLICATE-ACL", "assigned_object_type": ContentType.objects.get_for_model(Device), **self.common_acl_params, - "assigned_object_id": 1, + "assigned_object_id": 1, # TODO - replace with Device.objects.first() } acl_1 = AccessList.objects.create(**params) acl_1.save() @@ -193,7 +196,7 @@ def test_valid_acl_choices(self): type=acl_type, default_action=default_action, assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, + assigned_object_id=1, # TODO - replace with Device.objects.first() ) valid_acl_choice.full_clean() @@ -209,7 +212,7 @@ def test_invalid_acl_choices(self): type=valid_acl_types[0], default_action=invalid_acl_default_action_choice, assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, + assigned_object_id=1, # TODO - replace with Device.objects.first() ) with self.assertRaises(ValidationError): invalid_acl_default_action.full_clean() @@ -222,7 +225,7 @@ def test_invalid_acl_choices(self): type=invalid_acl_type, default_action=valid_acl_default_action_choices[0], assigned_object_type=ContentType.objects.get_for_model(Device), - assigned_object_id=1, + assigned_object_id=1, # TODO - replace with Device.objects.first() ) with self.assertRaises(ValidationError): invalid_acl_type.full_clean() @@ -239,32 +242,68 @@ def setUpTestData(cls): Extend BaseTestCase's setUpTestData() to create additional data for testing. """ super().setUpTestData() - - # interfaces = Interface.objects.bulk_create( - # ( - # Interface(name="Interface 1", device=device, type="1000baset"), - # Interface(name="Interface 2", device=device, type="1000baset"), - # ) - # ) - # vminterfaces = VMInterface.objects.bulk_create( - # ( - # VMInterface(name="Interface 1", virtual_machine=virtual_machine), - # VMInterface(name="Interface 2", virtual_machine=virtual_machine), - # ) - # ) - # prefixes = Prefix.objects.bulk_create( - # ( - # Prefix(prefix=IPNetwork("10.0.0.0/24")), - # Prefix(prefix=IPNetwork("192.168.1.0/24")), - # ) - # ) + device = Device.objects.first() + interfaces = Interface.objects.bulk_create( + ( + Interface(name="Interface 1", device=device, type="1000baset"), + Interface(name="Interface 2", device=device, type="1000baset"), + ) + ) + virtual_machine = VirtualMachine.objects.first() + vminterfaces = VMInterface.objects.bulk_create( + ( + VMInterface(name="Interface 1", virtual_machine=virtual_machine), + VMInterface(name="Interface 2", virtual_machine=virtual_machine), + ) + ) + prefixes = Prefix.objects.bulk_create( + ( + Prefix(prefix=IPNetwork("10.0.0.0/24")), + Prefix(prefix=IPNetwork("192.168.1.0/24")), + ) + ) 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. """ - pass - # TODO: test_acl_interface_assignment_success - VM & Device + device_acl = AccessList( + name="STANDARD_ACL", + comments="STANDARD_ACL", + type="standard", + default_action="permit", + assigned_object_id=1, + assigned_object_type=ContentType.objects.get_for_model(Device), + ) + device_acl.save() + acl_device_interface = ACLInterfaceAssignment( + access_list_id=device_acl.pk, + direction="ingress", + assigned_object_id=1, + assigned_object_type=ContentType.objects.get_for_model(Interface), + ) + acl_device_interface.full_clean() + + 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", + comments="STANDARD_ACL", + type="standard", + default_action="permit", + assigned_object_id=1, + assigned_object_type=ContentType.objects.get_for_model(VirtualMachine), + ) + vm_acl.save() + acl_vm_interface = ACLInterfaceAssignment( + access_list=vm_acl.pk, + direction="ingress", + assigned_object_id=1, + assigned_object_type=ContentType.objects.get_for_model(VMInterface), + ) + acl_vm_interface.full_clean() def test_acl_interface_assignment_fail(self): """ From 4bf2e096c8e82b59664603308ec6ff574200c1f0 Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Sun, 5 Feb 2023 20:27:56 +0530 Subject: [PATCH 12/15] fixed typo --- netbox_acls/tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index 945788d..bfe8c96 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -298,7 +298,7 @@ def test_acl_vminterface_assignment_success(self): ) vm_acl.save() acl_vm_interface = ACLInterfaceAssignment( - access_list=vm_acl.pk, + access_list=vm_acl, direction="ingress", assigned_object_id=1, assigned_object_type=ContentType.objects.get_for_model(VMInterface), From 36920fb2cadc826e1ec6c3159198f3b5f710961d Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Sun, 5 Feb 2023 17:26:42 +0000 Subject: [PATCH 13/15] correct acl key --- netbox_acls/tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index bfe8c96..76fd701 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -277,7 +277,7 @@ def test_acl_interface_assignment_success(self): ) device_acl.save() acl_device_interface = ACLInterfaceAssignment( - access_list_id=device_acl.pk, + access_list=device_acl, direction="ingress", assigned_object_id=1, assigned_object_type=ContentType.objects.get_for_model(Interface), From eb44575e928c9221ab9844d1efd5dc2272e42ead Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Tue, 7 Feb 2023 05:09:59 +0000 Subject: [PATCH 14/15] draft host logic model --- netbox_acls/models/access_lists.py | 17 +++++++ netbox_acls/tests/test_models.py | 79 ++++++++++++++++++------------ 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/netbox_acls/models/access_lists.py b/netbox_acls/models/access_lists.py index e65f765..31185ef 100644 --- a/netbox_acls/models/access_lists.py +++ b/netbox_acls/models/access_lists.py @@ -5,6 +5,7 @@ from dcim.models import Device, Interface, VirtualChassis from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError from django.core.validators import RegexValidator from django.db import models from django.urls import reverse @@ -146,6 +147,22 @@ def get_absolute_url(self): args=[self.pk], ) + def clean(self): + super().clean() + + # Get the model type of the assigned interface. + if self.assigned_object_type.model_class() == VMInterface: + interface_host = self.assigned_object.virtual_machine + elif self.assigned_object_type.model_class() == Interface: + interface_host = self.assigned_object.device + # Check if the assigned interface's host is the same as the host assigned to the access list. + if interface_host != self.access_list.assigned_object: + raise ValidationError( + { + "assigned_object": "The assigned object must be the same as the device assigned to it." + } + ) + @classmethod def get_prerequisite_models(cls): return [AccessList] diff --git a/netbox_acls/tests/test_models.py b/netbox_acls/tests/test_models.py index 76fd701..8816b84 100644 --- a/netbox_acls/tests/test_models.py +++ b/netbox_acls/tests/test_models.py @@ -51,27 +51,28 @@ def setUpTestData(cls): device_type=devicetype, device_role=devicerole, ) - virtual_chassis = VirtualChassis.objects.create(name="Virtual Chassis 1") - virtual_chassis_member = Device.objects.create( - name="VC Device", - site=site, - device_type=devicetype, - device_role=devicerole, - virtual_chassis=virtual_chassis, - vc_position=1, - ) - cluster_member = Device.objects.create( - name="Cluster Device", - site=site, - device_type=devicetype, - device_role=devicerole, - ) - clustertype = ClusterType.objects.create(name="Cluster Type 1") - cluster = Cluster.objects.create( - name="Cluster 1", - type=clustertype, - ) + # virtual_chassis = VirtualChassis.objects.create(name="Virtual Chassis 1") + # virtual_chassis_member = Device.objects.create( + # name="VC Device", + # site=site, + # device_type=devicetype, + # device_role=devicerole, + # virtual_chassis=virtual_chassis, + # vc_position=1, + # ) + # cluster_member = Device.objects.create( + # name="Cluster Device", + # site=site, + # device_type=devicetype, + # device_role=devicerole, + # ) + # clustertype = ClusterType.objects.create(name="Cluster Type 1") + # cluster = Cluster.objects.create( + # name="Cluster 1", + # type=clustertype, + # ) virtual_machine = VirtualMachine.objects.create(name="VirtualMachine 1") + virtual_machine.save() prefix = Prefix.objects.create(prefix="10.0.0.0/8") @@ -256,12 +257,12 @@ def setUpTestData(cls): VMInterface(name="Interface 2", virtual_machine=virtual_machine), ) ) - prefixes = Prefix.objects.bulk_create( - ( - Prefix(prefix=IPNetwork("10.0.0.0/24")), - Prefix(prefix=IPNetwork("192.168.1.0/24")), - ) - ) + #prefixes = Prefix.objects.bulk_create( + # ( + # Prefix(prefix=IPNetwork("10.0.0.0/24")), + # Prefix(prefix=IPNetwork("192.168.1.0/24")), + # ) + #) def test_acl_interface_assignment_success(self): """ @@ -272,18 +273,36 @@ def test_acl_interface_assignment_success(self): comments="STANDARD_ACL", type="standard", default_action="permit", - assigned_object_id=1, - assigned_object_type=ContentType.objects.get_for_model(Device), + assigned_object=Device.objects.first(), ) device_acl.save() acl_device_interface = ACLInterfaceAssignment( access_list=device_acl, direction="ingress", - assigned_object_id=1, - assigned_object_type=ContentType.objects.get_for_model(Interface), + assigned_object=Interface.objects.first(), ) acl_device_interface.full_clean() + def test_aclinterface_assignment_fail(self): + """ + Test that ACLInterfaceAssignment passes validation if the ACL is assigned to the host and not already assigned to the vminterface and direction. + """ + device_acl = AccessList( + name="STANDARD_ACL", + comments="STANDARD_ACL", + type="standard", + default_action="permit", + assigned_object=Device.objects.first(), + ) + device_acl.save() + acl_vm_interface = ACLInterfaceAssignment( + access_list=device_acl, + direction="ingress", + assigned_object=VMInterface.objects.first(), + ) + with self.assertRaises(ValidationError): + acl_vm_interface.full_clean() + 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. From ed8a56cfffe410042f974e608ca2a7c1097c953c Mon Sep 17 00:00:00 2001 From: ryanmerolle Date: Tue, 7 Feb 2023 05:10:12 +0000 Subject: [PATCH 15/15] lint --- netbox_acls/graphql/schema.py | 1 + netbox_acls/graphql/types.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_acls/graphql/schema.py b/netbox_acls/graphql/schema.py index cdaa8a2..0dc2dd2 100644 --- a/netbox_acls/graphql/schema.py +++ b/netbox_acls/graphql/schema.py @@ -3,6 +3,7 @@ from .types import * + class Query(ObjectType): """ Defines the queries available to this plugin via the graphql api. diff --git a/netbox_acls/graphql/types.py b/netbox_acls/graphql/types.py index 9e05942..f43774c 100644 --- a/netbox_acls/graphql/types.py +++ b/netbox_acls/graphql/types.py @@ -72,4 +72,3 @@ class Meta: model = models.ACLStandardRule fields = "__all__" filterset_class = filtersets.ACLStandardRuleFilterSet -