🚚 release (#165)

Author: mfiedorowicz

docker/common/requirements-diode-netbox-plugin.txt

@@ -3,6 +3,6 @@ certifi==2024.7.4
 coverage==7.6.0
 grpcio==1.62.1
 protobuf==5.29.6
-pytest==8.0.2
+pytest==9.0.3
 netboxlabs-netbox-branching==0.8.3
 granian[reload]

docker/v4.4.x/requirements-diode-netbox-plugin.txt

@@ -3,5 +3,5 @@ certifi==2024.7.4
 coverage==7.6.0
 grpcio==1.62.1
 protobuf==5.29.6
-pytest==8.0.2
+pytest==9.0.3
 netboxlabs-netbox-branching==0.7.1

docker/v4.5.x/requirements-diode-netbox-plugin.txt

@@ -3,6 +3,6 @@ certifi==2024.7.4
 coverage==7.6.0
 grpcio==1.62.1
 protobuf==5.29.6
-pytest==8.0.2
+pytest==9.0.3
 netboxlabs-netbox-branching==0.8.3
 granian[reload]

netbox_diode_plugin/__init__.py

@@ -43,6 +43,10 @@ class NetBoxDiodePluginConfig(PluginConfig):
         # TTL in seconds for caching find_existing_object results in Redis.
         # Set to 0 to disable caching.
         "find_obj_cache_ttl": 5,
+
+        # Override the displayed Diode target URL without affecting internal
+        # communication (e.g. to show the external ingress address).
+        "diode_target_display": None,
     }
 
 

netbox_diode_plugin/api/matcher.py

@@ -32,14 +32,32 @@ def _get_find_obj_cache_ttl() -> int:
     return get_plugin_config("netbox_diode_plugin", "find_obj_cache_ttl")
 
 
+def _get_active_branch_schema() -> str | None:
+    try:
+        from netbox_branching.contextvars import active_branch
+    except ImportError:
+        return None
+    branch = active_branch.get()
+    if branch is not None:
+        return branch.schema_id
+    return None
+
+
+def _find_obj_rev_key(object_type: str, object_id: int) -> str:
+    branch_schema = _get_active_branch_schema()
+    if branch_schema:
+        return f"diode:fobj:rev:{branch_schema}:{object_type}:{object_id}"
+    return f"diode:fobj:rev:{object_type}:{object_id}"
+
+
 def invalidate_find_obj_entry(object_type: str, object_id: int):
     """
     Delete a cached find_existing_object result by PK.
 
     Uses a reverse-index (PK → cache key) to find and delete the
     lookup cache entry. Call this after updating an existing object.
     """
-    rev_key = f"diode:fobj:rev:{object_type}:{object_id}"
+    rev_key = _find_obj_rev_key(object_type, object_id)
     lookup_key = django_cache.get(rev_key)
     if lookup_key:
         django_cache.delete(lookup_key)
@@ -921,7 +939,11 @@ def _find_obj_cache_key(data: dict, object_type: str) -> str | None:
     if not items:
         return None
 
-    raw = f"{object_type}:{items}"
+    branch_schema = _get_active_branch_schema()
+    if branch_schema:
+        raw = f"{branch_schema}:{object_type}:{items}"
+    else:
+        raw = f"{object_type}:{items}"
     key_hash = hashlib.sha256(raw.encode()).hexdigest()[:20]
     return f"diode:fobj:{key_hash}"
 
@@ -955,8 +977,7 @@ def find_existing_object(data: dict, object_type: str): # noqa: C901
                 # Object deleted since cached — clean up and fall through
                 cache_hit = False
                 django_cache.delete(cache_key)
-                rev_key = f"diode:fobj:rev:{object_type}:{cached_id}"
-                django_cache.delete(rev_key)
+                django_cache.delete(_find_obj_rev_key(object_type, cached_id))
 
     if not cache_hit:
         for matcher in get_model_matchers(model_class):
@@ -973,8 +994,7 @@ def find_existing_object(data: dict, object_type: str): # noqa: C901
 
         if cache_key and result is not None:
             django_cache.set(cache_key, result.id, cache_ttl)
-            rev_key = f"diode:fobj:rev:{object_type}:{result.id}"
-            django_cache.set(rev_key, cache_key, cache_ttl)
+            django_cache.set(_find_obj_rev_key(object_type, result.id), cache_key, cache_ttl)
 
     if ctx:
         ctx.record_timing("find_obj", (time.monotonic() - start) * 1000)

netbox_diode_plugin/tests/v4.4.x/tests/test_matcher_cache.py

@@ -11,6 +11,7 @@
 from netbox_diode_plugin.api.common import UnresolvedReference
 from netbox_diode_plugin.api.matcher import (
     _find_obj_cache_key,
+    _find_obj_rev_key,
     find_existing_object,
     invalidate_find_obj_entry,
 )
@@ -292,3 +293,154 @@ def test_invalidate_does_not_affect_other_entries(self, _mock_ttl):
         # Main should be gone
         cache_key_main = _find_obj_cache_key(data_main, "dcim.manufacturer")
         self.assertIsNone(django_cache.get(cache_key_main))
+
+
+BRANCH_SCHEMA_MOCK = "netbox_diode_plugin.api.matcher._get_active_branch_schema"
+
+
+class BranchAwareCacheKeyTestCase(TestCase):
+    """Tests that cache keys are isolated per branch."""
+
+    def setUp(self):
+        """Clear cache before each test."""
+        django_cache.clear()
+
+    def tearDown(self):
+        """Clear cache after each test."""
+        django_cache.clear()
+
+    def test_different_branch_different_cache_key(self):
+        """Same data under different branches produces different cache keys."""
+        data = {"name": "Cisco"}
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            key_a = _find_obj_cache_key(data, "dcim.manufacturer")
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_b"):
+            key_b = _find_obj_cache_key(data, "dcim.manufacturer")
+        self.assertNotEqual(key_a, key_b)
+
+    def test_same_branch_same_cache_key(self):
+        """Same data under the same branch produces identical cache keys."""
+        data = {"name": "Cisco"}
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            key1 = _find_obj_cache_key(data, "dcim.manufacturer")
+            key2 = _find_obj_cache_key(data, "dcim.manufacturer")
+        self.assertEqual(key1, key2)
+
+    def test_no_branch_unchanged_from_legacy_format(self):
+        """Without active branch, cache key matches the original format."""
+        data = {"name": "Cisco"}
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value=None):
+            key = _find_obj_cache_key(data, "dcim.manufacturer")
+        self.assertTrue(key.startswith("diode:fobj:"))
+
+    def test_branch_vs_no_branch_different_keys(self):
+        """A branched key differs from a non-branched key for the same data."""
+        data = {"name": "Cisco"}
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value=None):
+            key_main = _find_obj_cache_key(data, "dcim.manufacturer")
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            key_branch = _find_obj_cache_key(data, "dcim.manufacturer")
+        self.assertNotEqual(key_main, key_branch)
+
+    def test_different_branch_different_rev_key(self):
+        """Reverse-index keys are isolated per branch."""
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            rev_a = _find_obj_rev_key("dcim.manufacturer", 100)
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_b"):
+            rev_b = _find_obj_rev_key("dcim.manufacturer", 100)
+        self.assertNotEqual(rev_a, rev_b)
+        self.assertIn("branch_a", rev_a)
+        self.assertIn("branch_b", rev_b)
+
+    def test_no_branch_rev_key_unchanged(self):
+        """Without active branch, rev key matches the original format."""
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value=None):
+            rev = _find_obj_rev_key("dcim.manufacturer", 100)
+        self.assertEqual(rev, "diode:fobj:rev:dcim.manufacturer:100")
+
+
+class BranchAwareFindExistingObjectTestCase(TestCase):
+    """Tests that find_existing_object cache does not cross branches."""
+
+    def setUp(self):
+        """Set up test fixtures."""
+        self.manufacturer = Manufacturer.objects.create(
+            name="BranchTestMfr",
+            slug="branch-test-mfr",
+        )
+        django_cache.clear()
+
+    def tearDown(self):
+        """Clean up cache after each test."""
+        django_cache.clear()
+
+    @mock.patch(
+        "netbox_diode_plugin.api.matcher._get_find_obj_cache_ttl",
+        return_value=5,
+    )
+    def test_cache_does_not_cross_branches(self, _mock_ttl):
+        """Cache populated under branch A is a miss under branch B."""
+        data = {"name": "BranchTestMfr", "_object_type": "dcim.manufacturer"}
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            result_a = find_existing_object(data, "dcim.manufacturer")
+            self.assertEqual(result_a.id, self.manufacturer.id)
+            cache_key_a = _find_obj_cache_key(data, "dcim.manufacturer")
+            self.assertIsNotNone(django_cache.get(cache_key_a))
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_b"):
+            cache_key_b = _find_obj_cache_key(data, "dcim.manufacturer")
+            self.assertIsNone(django_cache.get(cache_key_b))
+
+    @mock.patch(
+        "netbox_diode_plugin.api.matcher._get_find_obj_cache_ttl",
+        return_value=5,
+    )
+    def test_invalidate_does_not_cross_branches(self, _mock_ttl):
+        """Invalidation under branch B does not evict branch A's cache."""
+        data = {"name": "BranchTestMfr", "_object_type": "dcim.manufacturer"}
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            find_existing_object(data, "dcim.manufacturer")
+            cache_key_a = _find_obj_cache_key(data, "dcim.manufacturer")
+            self.assertIsNotNone(django_cache.get(cache_key_a))
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_b"):
+            invalidate_find_obj_entry("dcim.manufacturer", self.manufacturer.id)
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            self.assertIsNotNone(django_cache.get(cache_key_a))
+
+    @mock.patch(
+        "netbox_diode_plugin.api.matcher._get_find_obj_cache_ttl",
+        return_value=5,
+    )
+    def test_invalidate_within_same_branch(self, _mock_ttl):
+        """Invalidation under the same branch correctly evicts cache."""
+        data = {"name": "BranchTestMfr", "_object_type": "dcim.manufacturer"}
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            find_existing_object(data, "dcim.manufacturer")
+            cache_key_a = _find_obj_cache_key(data, "dcim.manufacturer")
+            self.assertIsNotNone(django_cache.get(cache_key_a))
+            invalidate_find_obj_entry("dcim.manufacturer", self.manufacturer.id)
+            self.assertIsNone(django_cache.get(cache_key_a))
+
+    @mock.patch(
+        "netbox_diode_plugin.api.matcher._get_find_obj_cache_ttl",
+        return_value=5,
+    )
+    def test_branch_and_main_caches_independent(self, _mock_ttl):
+        """Cache entries for main (no branch) and a branch are independent."""
+        data = {"name": "BranchTestMfr", "_object_type": "dcim.manufacturer"}
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value=None):
+            find_existing_object(data, "dcim.manufacturer")
+            cache_key_main = _find_obj_cache_key(data, "dcim.manufacturer")
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value="branch_a"):
+            cache_key_branch = _find_obj_cache_key(data, "dcim.manufacturer")
+            self.assertIsNone(django_cache.get(cache_key_branch))
+
+        with mock.patch(BRANCH_SCHEMA_MOCK, return_value=None):
+            self.assertIsNotNone(django_cache.get(cache_key_main))

netbox_diode_plugin/tests/v4.4.x/tests/test_views.py

@@ -76,6 +76,53 @@ def test_settings_created_if_not_found(self):
             self.assertIn("grpc://localhost:8080/diode", str(response.content))
 
 
+    def test_settings_display_with_diode_target_display(self):
+        """Test that diode_target_display overrides the displayed target."""
+        self.request.user = User.objects.create_user("foo", password="pass")
+        self.add_permissions(self.request.user, "netbox_diode_plugin.view_setting")
+
+        def mock_get_plugin_config(plugin, key):
+            if key == "diode_target_display":
+                return "grpcs://external.example.com/diode"
+            if key == "diode_target_override":
+                return None
+            if key == "diode_target":
+                return "grpc://localhost:8080/diode"
+            return None
+
+        with mock.patch(
+            "netbox_diode_plugin.views.get_plugin_config",
+            side_effect=mock_get_plugin_config,
+        ):
+            response = self.view.get(self.request)
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertIn("grpcs://external.example.com/diode", str(response.content))
+            self.assertNotIn("grpc://localhost:8080/diode", str(response.content))
+
+    def test_diode_target_display_takes_precedence_over_override(self):
+        """Test that diode_target_display takes precedence over diode_target_override for display."""
+        self.request.user = User.objects.create_user("foo", password="pass")
+        self.add_permissions(self.request.user, "netbox_diode_plugin.view_setting")
+
+        def mock_get_plugin_config(plugin, key):
+            if key == "diode_target_display":
+                return "grpcs://external.example.com/diode"
+            if key == "diode_target_override":
+                return "grpc://internal-override:8080/diode"
+            if key == "diode_target":
+                return "grpc://localhost:8080/diode"
+            return None
+
+        with mock.patch(
+            "netbox_diode_plugin.views.get_plugin_config",
+            side_effect=mock_get_plugin_config,
+        ):
+            response = self.view.get(self.request)
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertIn("grpcs://external.example.com/diode", str(response.content))
+            self.assertNotIn("grpc://internal-override:8080/diode", str(response.content))
+
+
 class SettingsEditViewTestCase(TestCase):
     """Test case for the SettingsEditView."""