From 596ad20cc6fb814e9327804cd8a3e4b29c72892c Mon Sep 17 00:00:00 2001
From: Chris McCarthy <chris.mccarthy@methods.co.uk>
Date: Mon, 4 Sep 2023 16:44:29 +0100
Subject: [PATCH] #1800 - Added better logging and error messages to
 AccessControlList class. Updated usage of extra following pydantic deprecated
 warning "`pydantic.config.Extra` is deprecated, use literal values instead
 (e.g. `extra='allow'`)"

---
 src/primaite/simulator/core.py                          | 4 ++--
 src/primaite/simulator/network/hardware/nodes/router.py | 8 ++++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index b7dfcf72..0501bbb2 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -3,7 +3,7 @@ from abc import ABC, abstractmethod
 from typing import Callable, Dict, List, Optional, Union
 from uuid import uuid4
 
-from pydantic import BaseModel, ConfigDict, Extra
+from pydantic import BaseModel, ConfigDict
 
 from primaite import getLogger
 
@@ -126,7 +126,7 @@ class ActionManager:
 class SimComponent(BaseModel):
     """Extension of pydantic BaseModel with additional methods that must be defined by all classes in the simulator."""
 
-    model_config = ConfigDict(arbitrary_types_allowed=True, extra=Extra.allow)
+    model_config = ConfigDict(arbitrary_types_allowed=True, extra="allow")
     """Configure pydantic to allow arbitrary types and to let the instance have attributes not present in model."""
 
     uuid: str
diff --git a/src/primaite/simulator/network/hardware/nodes/router.py b/src/primaite/simulator/network/hardware/nodes/router.py
index a34b83e2..092680a7 100644
--- a/src/primaite/simulator/network/hardware/nodes/router.py
+++ b/src/primaite/simulator/network/hardware/nodes/router.py
@@ -131,6 +131,8 @@ class AccessControlList(SimComponent):
         if isinstance(dst_ip_address, str):
             dst_ip_address = IPv4Address(dst_ip_address)
         if 0 <= position < self.max_acl_rules:
+            if self._acl[position]:
+                self.sys_log.info(f"Overwriting ACL rule at position {position}")
             self._acl[position] = ACLRule(
                 action=action,
                 src_ip_address=src_ip_address,
@@ -140,7 +142,7 @@ class AccessControlList(SimComponent):
                 dst_port=dst_port,
             )
         else:
-            raise ValueError(f"Position {position} is out of bounds.")
+            raise ValueError(f"Cannot add ACL rule, position {position} is out of bounds.")
 
     def remove_rule(self, position: int) -> None:
         """
@@ -150,9 +152,11 @@ class AccessControlList(SimComponent):
         :raises ValueError: When the position is out of bounds.
         """
         if 0 <= position < self.max_acl_rules - 1:
+            rule = self._acl[position]  # noqa
             self._acl[position] = None
+            del rule
         else:
-            raise ValueError(f"Position {position} is out of bounds.")
+            raise ValueError(f"Cannot remove ACL rule, position {position} is out of bounds.")
 
     def is_permitted(
         self,