From 88a9de20d035b4958234831a1098b95d12fb0415 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Tue, 16 Jul 2024 21:53:24 +0200 Subject: [PATCH] feat(models): Restrict cross ACI Tenant assignment Enhances ACIBridgeDomain and ACIVRF assignment validations and provides clearer error messages. Validate assigned ACIBridgeDomain and ACIVRF must belong to same ACITenant or ACITenant 'common' on the model level. Tests are added to verify new assignment rules. --- .../forms/tenant_app_profiles.py | 6 +- netbox_aci_plugin/forms/tenant_networks.py | 5 +- .../models/tenant_app_profiles.py | 17 +++++ netbox_aci_plugin/models/tenant_networks.py | 16 ++++ netbox_aci_plugin/tests/test_models.py | 74 +++++++++++++++++++ 5 files changed, 113 insertions(+), 5 deletions(-) diff --git a/netbox_aci_plugin/forms/tenant_app_profiles.py b/netbox_aci_plugin/forms/tenant_app_profiles.py index b8b3f8e..08b0f90 100644 --- a/netbox_aci_plugin/forms/tenant_app_profiles.py +++ b/netbox_aci_plugin/forms/tenant_app_profiles.py @@ -388,9 +388,9 @@ def clean(self): self.add_error( "aci_bridge_domain", _( - "A Bridge Domain can only be assigned from the same " - "ACI Tenant as the Endpoint Group or ACI Tenant" - "'common'." + "A Bridge Domain can only be assigned belonging to " + "same ACI Tenant as the Application Profile or to the " + "special ACI Tenant 'common'." ), ) diff --git a/netbox_aci_plugin/forms/tenant_networks.py b/netbox_aci_plugin/forms/tenant_networks.py index 23b801d..e7ffa71 100644 --- a/netbox_aci_plugin/forms/tenant_networks.py +++ b/netbox_aci_plugin/forms/tenant_networks.py @@ -752,8 +752,9 @@ def clean(self): self.add_error( "aci_vrf", _( - "A VRF can only be assigned from the same ACI Tenant " - "as the Bridge Domain or ACI Tenant 'common'." + "A VRF can only be assigned belonging to the same ACI " + "Tenant as the Bridge Domain or to the special ACI " + "Tenant 'common'." ), ) diff --git a/netbox_aci_plugin/models/tenant_app_profiles.py b/netbox_aci_plugin/models/tenant_app_profiles.py index 904a112..c85154e 100644 --- a/netbox_aci_plugin/models/tenant_app_profiles.py +++ b/netbox_aci_plugin/models/tenant_app_profiles.py @@ -2,6 +2,7 @@ # # SPDX-License-Identifier: GPL-3.0-or-later +from django.core.exceptions import ValidationError from django.core.validators import MaxLengthValidator from django.db import models from django.urls import reverse @@ -238,6 +239,22 @@ def __str__(self) -> str: """Return string representation of the instance.""" return self.name + def save(self, *args, **kwargs): + """Saves the current instance to the database.""" + # Ensure the assigned ACIBrideDomain belongs to either the same + # ACITenant as the ACIAppProfile or to the special ACITenant 'common'. + if ( + self.aci_bridge_domain.aci_tenant + != self.aci_app_profile.aci_tenant + and self.aci_bridge_domain.aci_tenant.name != "common" + ): + raise ValidationError( + "Assigned ACIBridgeDomain have to belong to the same ACITenant" + "as the ACIAppProfile or to the special ACITenant 'common'." + ) + + super().save(*args, **kwargs) + @property def aci_tenant(self) -> ACITenant: """Return the ACITenant instance of related ACIAppProfile.""" diff --git a/netbox_aci_plugin/models/tenant_networks.py b/netbox_aci_plugin/models/tenant_networks.py index 97aa700..dc72971 100644 --- a/netbox_aci_plugin/models/tenant_networks.py +++ b/netbox_aci_plugin/models/tenant_networks.py @@ -4,6 +4,7 @@ from dcim.fields import MACAddressField from django.contrib.postgres.fields import ArrayField +from django.core.exceptions import ValidationError from django.core.validators import MaxLengthValidator from django.db import models from django.urls import reverse @@ -480,6 +481,21 @@ def __str__(self) -> str: else: return self.name + def save(self, *args, **kwargs): + """Saves the current instance to the database.""" + # Ensure the assigned ACIVRF belongs to either the same ACITenant as + # the ACIBridgeDomain or to the special ACITenant 'common'. + if ( + self.aci_vrf.aci_tenant != self.aci_tenant + and self.aci_vrf.aci_tenant.name != "common" + ): + raise ValidationError( + "Assigned ACIVRF have to belong to the same ACITenant as the" + "ACIBridgeDomain or to the special ACITenant 'common'." + ) + + super().save(*args, **kwargs) + def get_absolute_url(self) -> str: """Return the absolute URL of the instance.""" return reverse( diff --git a/netbox_aci_plugin/tests/test_models.py b/netbox_aci_plugin/tests/test_models.py index 07cb67d..23153ec 100644 --- a/netbox_aci_plugin/tests/test_models.py +++ b/netbox_aci_plugin/tests/test_models.py @@ -806,6 +806,40 @@ def test_invalid_aci_bridge_domain_description_length(self) -> None: with self.assertRaises(ValidationError): bd.full_clean() + def test_valid_aci_bridge_domain_aci_vrf_assignment_form_tenant_common( + self, + ) -> None: + """Test valid assignment of ACI VRF from ACI Tenant 'common'.""" + tenant_common = ACITenant.objects.get_or_create(name="common")[0] + vrf_common = ACIVRF.objects.create( + name="common_vrf", aci_tenant=tenant_common + ) + bd = ACIBridgeDomain( + name="ACIBDTest1", + aci_tenant=self.aci_tenant, + aci_vrf=vrf_common, + ) + bd.full_clean() + bd.save() + self.assertEqual(bd.aci_vrf, vrf_common) + + def test_invalid_aci_bridge_domain_aci_vrf_assignment_from_tenant_other( + self, + ) -> None: + """Test invalid assignment of ACI VRF from ACI Tenant 'other'.""" + tenant_other = ACITenant.objects.get_or_create(name="other")[0] + vrf_other = ACIVRF.objects.create( + name="other_vrf", aci_tenant=tenant_other + ) + bd = ACIBridgeDomain( + name="ACIBDTest1", + aci_tenant=self.aci_tenant, + aci_vrf=vrf_other, + ) + with self.assertRaises(ValidationError): + bd.full_clean() + bd.save() + def test_constraint_unique_aci_bridge_domain_name_per_aci_tenant( self, ) -> None: @@ -1323,6 +1357,46 @@ def test_invalid_aci_endpoint_group_description_length(self) -> None: with self.assertRaises(ValidationError): epg.full_clean() + def test_valid_aci_endpoint_group_aci_bd_assignment_form_tenant_common( + self, + ) -> None: + """Test valid assignment of ACI BD from ACI Tenant 'common'.""" + tenant_common = ACITenant.objects.get_or_create(name="common")[0] + vrf_common = ACIVRF.objects.create( + name="common_vrf", aci_tenant=tenant_common + ) + bd_common = ACIBridgeDomain.objects.create( + name="common_bd", aci_tenant=tenant_common, aci_vrf=vrf_common + ) + epg = ACIEndpointGroup.objects.create( + name="ACIEPGTest1", + aci_app_profile=self.aci_app_profile, + aci_bridge_domain=bd_common, + ) + epg.full_clean() + epg.save() + self.assertEqual(epg.aci_bridge_domain, bd_common) + + def test_invalid_aci_endpoint_group_aci_bd_assignment_from_tenant_other( + self, + ) -> None: + """Test invalid assignment of ACI BD from ACI Tenant 'other'.""" + tenant_other = ACITenant.objects.get_or_create(name="other")[0] + vrf_other = ACIVRF.objects.create( + name="other_vrf", aci_tenant=tenant_other + ) + bd_other = ACIBridgeDomain.objects.create( + name="other_bd", aci_tenant=tenant_other, aci_vrf=vrf_other + ) + epg = ACIEndpointGroup( + name="ACIEPGTest1", + aci_app_profile=self.aci_app_profile, + aci_bridge_domain=bd_other, + ) + with self.assertRaises(ValidationError): + epg.full_clean() + epg.save() + def test_constraint_unique_aci_endpoint_group_name_per_aci_app_profile( self, ) -> None: