From 9f5e40ae482af5c3e3daa7d0db088594bdccace4 Mon Sep 17 00:00:00 2001 From: Sven Luethi Date: Wed, 12 Aug 2026 16:01:55 +0200 Subject: [PATCH] Replace global IntegerKeyValueSetting with per-view CatalogZone model Track the catalog zone SOA serial per view instead of a single global value. The new CatalogZone model has a 1:1 FK to netbox_dns.models.View and its own soa_serial field; each view's catalog zone now increments independently. The existing global serial is preserved during migration by copying it into every view's new CatalogZone row. Change CatalogZoneMemberIdentifier.name from globally unique to unique per catalog zone (UniqueConstraint(name, catalog_zone)) and add a catalog_zone FK to enforce it. Remove the dns-settings management command, which was a generic interface over the now-removed IntegerKeyValueSetting model. Increment both catalog zone serials when a zone moves between views: the old view's catalog zone loses a member and the new view's gains one, so both must be bumped. Also refresh the member identifier's catalog_zone FK on a view move so it does not go stale. The pre_save handler now captures _old_view_id and the post_save handler detects view changes, increments the old view's serial inside on_commit (before the new view's), and re-creates the identifier. AI generated, code manually verified. --- CHANGELOG.md | 11 ++ .../management/commands/dns-settings.py | 51 ------- .../catalog_zone_manager.py | 52 ++++--- ...004_catalogzone_and_per_zone_identifier.py | 134 ++++++++++++++++++ netbox_dns_bridge/models.py | 28 +++- netbox_dns_bridge/signals/netbox_dns_zones.py | 35 ++++- netbox_dns_bridge/signals/notify_handler.py | 16 ++- .../netbox_dns_bridge/catz_overview.html | 23 ++- netbox_dns_bridge/tests/conftest.py | 9 +- .../tests/test_catalog_zone_manager.py | 39 ++--- .../tests/test_management_commands.py | 56 +------- netbox_dns_bridge/tests/test_models.py | 90 ++++++++---- .../tests/test_notify_handler.py | 8 +- netbox_dns_bridge/tests/test_signals.py | 87 ++++++++---- netbox_dns_bridge/tests/test_smoke.py | 4 +- netbox_dns_bridge/tests/test_views.py | 17 ++- netbox_dns_bridge/views.py | 9 +- 17 files changed, 429 insertions(+), 240 deletions(-) delete mode 100644 netbox_dns_bridge/management/commands/dns-settings.py create mode 100644 netbox_dns_bridge/migrations/0004_catalogzone_and_per_zone_identifier.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3887554..98ae9a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,15 @@ ## Unreleased +- Replace the global `IntegerKeyValueSetting` model (single `catalog-zone-soa-serial` + value shared by all catalog zones) with a new `CatalogZone` model that has a 1:1 + foreign key to `netbox_dns.models.View` and its own `soa_serial` field. Each view's + catalog zone now tracks and increments its SOA serial independently. Existing serials + are preserved during migration by copying the old global value into every view's new + `CatalogZone` row. +- Change the `CatalogZoneMemberIdentifier.name` uniqueness from globally unique to + unique per catalog zone (`UniqueConstraint(name, catalog_zone)`). A `catalog_zone` + foreign key has been added to the identifier model to enforce this. +- Remove the `dns-settings` management command, which was a generic get/set/list + interface over `IntegerKeyValueSetting` (now removed). - Defer the catalog zone SOA serial increment on zone deletion to `transaction.on_commit`, matching the create/modify path. Previously the increment ran immediately at `post_delete` time, so a rollback of the delete transaction would have left the catalog serial bumped anyway. diff --git a/netbox_dns_bridge/management/commands/dns-settings.py b/netbox_dns_bridge/management/commands/dns-settings.py deleted file mode 100644 index 050aea9..0000000 --- a/netbox_dns_bridge/management/commands/dns-settings.py +++ /dev/null @@ -1,51 +0,0 @@ -from django.core.management.base import BaseCommand, CommandError -from netbox_dns_bridge.models import IntegerKeyValueSetting - - -class Command(BaseCommand): - help = "Get or set integer key-value settings" - - def add_arguments(self, parser): - parser.add_argument( - "action", choices=["get", "set", "list"], help="Action to perform" - ) - parser.add_argument("key", nargs="?", help="Setting key") - parser.add_argument( - "value", nargs="?", type=int, help="Value to set (required for 'set')" - ) - - def handle(self, *args, **options): - action = options["action"] - key = options.get("key") - value = options.get("value") - - if action == "list": - settings = IntegerKeyValueSetting.objects.all() - if not settings.exists(): - self.stdout.write("No settings found.") - return - for setting in settings: - self.stdout.write(str(setting)) - - elif action == "get": - if not key: - raise CommandError("A key is required for GET") - try: - setting = IntegerKeyValueSetting.objects.get(key=key) - self.stdout.write(f"{setting.key}: {setting.value}") - except IntegerKeyValueSetting.DoesNotExist: - raise CommandError(f"No setting was found for {key}") - - elif action == "set": - if not key: - raise CommandError("A setting is required for SET command") - if value is None: - raise CommandError("A value is required for SET command") - try: - setting = IntegerKeyValueSetting.objects.get(key=key) - old_value = setting.value - setting.value = value - setting.save() - self.stdout.write(f"Updated {key} from {old_value} to {value}") - except IntegerKeyValueSetting.DoesNotExist: - raise CommandError(f"No setting found for {key}") diff --git a/netbox_dns_bridge/management/commands/dns_transfer_endpoint/catalog_zone_manager.py b/netbox_dns_bridge/management/commands/dns_transfer_endpoint/catalog_zone_manager.py index a0efa49..0617268 100644 --- a/netbox_dns_bridge/management/commands/dns_transfer_endpoint/catalog_zone_manager.py +++ b/netbox_dns_bridge/management/commands/dns_transfer_endpoint/catalog_zone_manager.py @@ -3,8 +3,8 @@ import dns.zone import dns.rdatatype import dns.rdataclass from netbox_dns_bridge.utils import get_logger -from netbox_dns.models import Zone as NBZone -from netbox_dns_bridge.models import IntegerKeyValueSetting, CatalogZoneMemberIdentifier +from netbox_dns.models import Zone as NBZone, View as NBView +from netbox_dns_bridge.models import CatalogZone, CatalogZoneMemberIdentifier from uuid import uuid4 from base64 import b32encode from django.db import transaction, close_old_connections, OperationalError @@ -13,31 +13,32 @@ from django.db import transaction, close_old_connections, OperationalError LOGGER = get_logger(__name__) -def increment_soa_serial() -> int: +def increment_soa_serial(view: NBView) -> int: with transaction.atomic(): try: ( soa_serial_obj, _, - ) = IntegerKeyValueSetting.objects.select_for_update().get_or_create( - key="catalog-zone-soa-serial", - defaults={"value": 1}, + ) = CatalogZone.objects.select_for_update().get_or_create( + view=view, + defaults={"soa_serial": 1}, ) - new_soa_serial = soa_serial_obj.value + 1 + new_soa_serial = soa_serial_obj.soa_serial + 1 if new_soa_serial > 0xFFFFFFFF: LOGGER.warning( - f"Catalog serial {soa_serial_obj.value} reached max — wrapping back to 1" + f"Catalog serial {soa_serial_obj.soa_serial} reached max — wrapping back to 1" ) new_soa_serial = 1 - soa_serial_obj.value = new_soa_serial + soa_serial_obj.soa_serial = new_soa_serial soa_serial_obj.save() soa_serial_obj.refresh_from_db() LOGGER.debug( - f"Catalog zone SOA serial number is now {soa_serial_obj.value}" + f"Catalog zone SOA serial for view '{view.name}' " + f"is now {soa_serial_obj.soa_serial}" ) - return soa_serial_obj.value + return soa_serial_obj.soa_serial except OperationalError as e: LOGGER.error( f"ERROR: Failed to increment the Catalog Zone's SOA serial: {e}" @@ -69,8 +70,11 @@ def create_zone(name, view_name) -> dns.zone.Zone: try: catz_identifier = nb_zone.catz_identifier except NBZone.catz_identifier.RelatedObjectDoesNotExist: + catalog_zone = _get_or_create_catalog_zone(nb_zone.view) catz_identifier = CatalogZoneMemberIdentifier.objects.create( - zone=nb_zone, name=_generate_member_identifier() + zone=nb_zone, + name=_generate_member_identifier(), + catalog_zone=catalog_zone, ) p_name = catz_identifier.name @@ -100,7 +104,8 @@ def create_zone(name, view_name) -> dns.zone.Zone: zone_root_node = zone.find_node(origin, create=True) # SOA record - soa_rdataset = _create_soa_rdataset() + view = NBView.objects.get(name=view_name) + soa_rdataset = _create_soa_rdataset(view) zone_root_node.rdatasets.append(soa_rdataset) # NS record @@ -126,23 +131,34 @@ def _generate_member_identifier() -> str: return b32encode(uuid4().bytes)[0:26].lower().decode("UTF-8") +def _get_or_create_catalog_zone(view: NBView) -> CatalogZone: + catalog_zone, _ = CatalogZone.objects.get_or_create( + view=view, defaults={"soa_serial": 1} + ) + return catalog_zone + + def update_member_identifier(zone: NBZone) -> None: try: + catalog_zone = _get_or_create_catalog_zone(zone.view) CatalogZoneMemberIdentifier.objects.update_or_create( zone=zone, - defaults={"name": _generate_member_identifier()}, + defaults={ + "name": _generate_member_identifier(), + "catalog_zone": catalog_zone, + }, ) except OperationalError as e: LOGGER.error(f"ERROR: Failed to update Catalog Zone member identifier: {e}") close_old_connections() -def _create_soa_rdataset() -> dns.rdataset: +def _create_soa_rdataset(view: NBView) -> dns.rdataset: try: - serial_obj, _ = IntegerKeyValueSetting.objects.get_or_create( - key="catalog-zone-soa-serial", defaults={"value": 1} + serial_obj, _ = CatalogZone.objects.get_or_create( + view=view, defaults={"soa_serial": 1} ) - serial = serial_obj.value + serial = serial_obj.soa_serial # SOA Record components ttl = 0 diff --git a/netbox_dns_bridge/migrations/0004_catalogzone_and_per_zone_identifier.py b/netbox_dns_bridge/migrations/0004_catalogzone_and_per_zone_identifier.py new file mode 100644 index 0000000..ead66eb --- /dev/null +++ b/netbox_dns_bridge/migrations/0004_catalogzone_and_per_zone_identifier.py @@ -0,0 +1,134 @@ +import django.db.models.deletion +from django.db import migrations, models + + +def copy_serials_and_link_identifiers(apps, schema_editor): + """Seed one CatalogZone per view from the old global serial and link identifiers.""" + IntegerKeyValueSetting = apps.get_model("netbox_dns_bridge", "IntegerKeyValueSetting") + CatalogZone = apps.get_model("netbox_dns_bridge", "CatalogZone") + CatalogZoneMemberIdentifier = apps.get_model( + "netbox_dns_bridge", "CatalogZoneMemberIdentifier" + ) + + # Preserve the current global serial; default to 1 when never initialized. + seed_serial = 1 + kv = IntegerKeyValueSetting.objects.filter(key="catalog-zone-soa-serial").first() + if kv is not None: + seed_serial = kv.value + + # Create a CatalogZone for every view that currently has at least one + # member identifier, seeding each with the old global serial. + view_ids = set( + CatalogZoneMemberIdentifier.objects.exclude(zone__view__isnull=True) + .values_list("zone__view_id", flat=True) + ) + + view_id_to_catalog_zone = {} + for view_id in view_ids: + cz, _ = CatalogZone.objects.get_or_create( + view_id=view_id, defaults={"soa_serial": seed_serial} + ) + view_id_to_catalog_zone[view_id] = cz + + # Link each identifier to the CatalogZone matching its zone's view. + for ident in CatalogZoneMemberIdentifier.objects.select_related("zone__view"): + cz = view_id_to_catalog_zone.get(ident.zone.view_id) + if cz is None: + # Defensive: identifier existed without a resolvable view. Create + # the CatalogZone on demand so the NOT NULL constraint can be set. + cz, _ = CatalogZone.objects.get_or_create( + view_id=ident.zone.view_id, defaults={"soa_serial": seed_serial} + ) + view_id_to_catalog_zone[ident.zone.view_id] = cz + ident.catalog_zone = cz + ident.save(update_fields=["catalog_zone"]) + + +def reverse_migration(apps, schema_editor): + """Best-effort reverse: restore the KV row and null the FK.""" + IntegerKeyValueSetting = apps.get_model("netbox_dns_bridge", "IntegerKeyValueSetting") + CatalogZone = apps.get_model("netbox_dns_bridge", "CatalogZone") + CatalogZoneMemberIdentifier = apps.get_model( + "netbox_dns_bridge", "CatalogZoneMemberIdentifier" + ) + + # Pick a representative serial (first catalog zone) to write back. + first = CatalogZone.objects.order_by("pk").first() + serial = first.soa_serial if first is not None else 1 + IntegerKeyValueSetting.objects.update_or_create( + key="catalog-zone-soa-serial", defaults={"value": serial} + ) + + CatalogZoneMemberIdentifier.objects.update(catalog_zone=None) + + +class Migration(migrations.Migration): + + dependencies = [ + ("netbox_dns", "0030_dnsseckeytemplate_comments_dnsseckeytemplate_owner_and_more"), + ("netbox_dns_bridge", "0003_integerkeyvaluesetting_key_unique"), + ] + + operations = [ + migrations.CreateModel( + name="CatalogZone", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False + ), + ), + ("soa_serial", models.IntegerField(default=1)), + ( + "view", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + related_name="catalog_zone", + to="netbox_dns.view", + ), + ), + ], + options={ + "ordering": ("view__name",), + }, + ), + migrations.AddField( + model_name="catalogzonememberidentifier", + name="catalog_zone", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="member_identifiers", + to="netbox_dns_bridge.catalogzone", + ), + ), + migrations.AlterField( + model_name="catalogzonememberidentifier", + name="name", + field=models.CharField(max_length=26), + ), + migrations.RunPython( + copy_serials_and_link_identifiers, + reverse_code=reverse_migration, + ), + migrations.AlterField( + model_name="catalogzonememberidentifier", + name="catalog_zone", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="member_identifiers", + to="netbox_dns_bridge.catalogzone", + ), + ), + migrations.AddConstraint( + model_name="catalogzonememberidentifier", + constraint=models.UniqueConstraint( + fields=("name", "catalog_zone"), + name="unique_name_per_catalog_zone", + ), + ), + migrations.DeleteModel( + name="IntegerKeyValueSetting", + ), + ] diff --git a/netbox_dns_bridge/models.py b/netbox_dns_bridge/models.py index 09aa5a1..056d3b2 100644 --- a/netbox_dns_bridge/models.py +++ b/netbox_dns_bridge/models.py @@ -4,14 +4,21 @@ from django.db import models import netbox_dns.models -class IntegerKeyValueSetting(models.Model): +class CatalogZone(models.Model): _netbox_private = True - key = models.CharField(max_length=64, unique=True) - value = models.IntegerField() + view = models.OneToOneField( + to=netbox_dns.models.View, + on_delete=models.CASCADE, + related_name="catalog_zone", + ) + soa_serial = models.IntegerField(default=1) + + class Meta: + ordering = ("view__name",) def __str__(self): - return f"{self.key}: {str(self.value)}" + return f"{self.view.name}: {self.soa_serial}" class CatalogZoneMemberIdentifier(models.Model): @@ -19,7 +26,6 @@ class CatalogZoneMemberIdentifier(models.Model): name = models.CharField( max_length=26, - unique=True, ) zone = models.OneToOneField( @@ -28,8 +34,20 @@ class CatalogZoneMemberIdentifier(models.Model): related_name="catz_identifier", ) + catalog_zone = models.ForeignKey( + to=CatalogZone, + on_delete=models.CASCADE, + related_name="member_identifiers", + ) + class Meta: ordering = ("name",) + constraints = [ + models.UniqueConstraint( + fields=["name", "catalog_zone"], + name="unique_name_per_catalog_zone", + ), + ] def __str__(self): return self.name diff --git a/netbox_dns_bridge/signals/netbox_dns_zones.py b/netbox_dns_bridge/signals/netbox_dns_zones.py index fcb1747..f8c33be 100644 --- a/netbox_dns_bridge/signals/netbox_dns_zones.py +++ b/netbox_dns_bridge/signals/netbox_dns_zones.py @@ -3,7 +3,7 @@ from django.dispatch import receiver from django.db import transaction from django.conf import settings from netbox.context import current_request -from netbox_dns.models import Zone +from netbox_dns.models import Zone, View from netbox_dns_bridge.management.commands.dns_transfer_endpoint import catalog_zone_manager as catzm from . import notify_handler @@ -14,15 +14,18 @@ def zone_pre_save(sender, instance, **kwargs): zone = instance if zone.pk: try: - old = sender.objects.only("name", "soa_serial").get(pk=zone.pk) + old = sender.objects.only("name", "soa_serial", "view_id").get(pk=zone.pk) zone._old_name = old.name zone._old_soa_serial = old.soa_serial + zone._old_view_id = old.view_id except sender.DoesNotExist: zone._old_name = None zone._old_soa_serial = None + zone._old_view_id = None else: zone._old_name = None zone._old_soa_serial = None + zone._old_view_id = None @receiver(post_save, sender=Zone) @@ -32,6 +35,8 @@ def zone_post_save(sender, instance, created, **kwargs): # Quit if this is not the first signal in this session for this zone. try: request = current_request.get() + if request is None: + raise LookupError handled_zones = getattr(request, "_handled_zones", set()) if zone.pk in handled_zones: return @@ -46,9 +51,25 @@ def zone_post_save(sender, instance, created, **kwargs): new_soa_serial = getattr(zone, "soa_serial", None) soa_serial_changed = created or (old_soa_serial != new_soa_serial) + # Detect whether the zone moved between views. When it does, the old + # view's catalog zone loses a member and the new view's gains one, so + # both serials must be incremented. + old_view_id = getattr(zone, "_old_view_id", None) + view_changed = ( + not created + and old_view_id is not None + and old_view_id != zone.view_id + ) + # On commit, increment soa serial and if notify is enabled, schedule bg job. def _on_commit(): - catzm.increment_soa_serial() + if view_changed: + try: + old_view = View.objects.get(pk=old_view_id) + catzm.increment_soa_serial(old_view) + except View.DoesNotExist: + pass + catzm.increment_soa_serial(zone.view) if SETTINGS.get("notify_clients", False): notify_handler.schedule_client_notify(zone) @@ -64,16 +85,18 @@ def zone_post_save(sender, instance, created, **kwargs): # If zone was created, create catz member identifier record for this zone. catzm.update_member_identifier(zone) else: - # Check if zone name has changed and change the catz member identifier if so. + # Re-create the catz member identifier when the zone name or its view + # changed. A view move leaves the identifier pointing at the wrong + # catalog zone, so it must be refreshed. old_name = getattr(zone, "_old_name", None) - if old_name and zone.name != old_name: + if (old_name and zone.name != old_name) or view_changed: catzm.update_member_identifier(zone) @receiver(post_delete, sender=Zone) def zone_post_delete(sender, instance, **kwargs): def _on_commit(): - catzm.increment_soa_serial() + catzm.increment_soa_serial(instance.view) transaction.on_commit(_on_commit) diff --git a/netbox_dns_bridge/signals/notify_handler.py b/netbox_dns_bridge/signals/notify_handler.py index c0ebbe4..8a1507e 100644 --- a/netbox_dns_bridge/signals/notify_handler.py +++ b/netbox_dns_bridge/signals/notify_handler.py @@ -12,7 +12,7 @@ from django.db import close_old_connections, OperationalError from netbox.jobs import JobRunner from netbox_dns.models import View, Zone, Record from netbox_dns_bridge.utils import get_logger -from netbox_dns_bridge.models import SeenTransferClients, IntegerKeyValueSetting +from netbox_dns_bridge.models import SeenTransferClients, CatalogZone LOGGER = get_logger(__name__) SETTINGS = settings.PLUGINS_CONFIG.get("netbox_dns_bridge", {}) @@ -51,9 +51,12 @@ class SendClientDNSNotify(JobRunner): view_name = kwargs["view_name"] try: - catz_serial = IntegerKeyValueSetting.objects.get( - key="catalog-zone-soa-serial" - ).value + view = View.objects.get(name=view_name) + try: + catz_serial = view.catalog_zone.soa_serial + except CatalogZone.DoesNotExist as e: + LOGGER.error(f"Failed to get catalog zone serial: {e}") + return zones = [ (kwargs["zone_name"], kwargs["soa_serial"]), @@ -61,7 +64,6 @@ class SendClientDNSNotify(JobRunner): (f"{view_name}.catz", catz_serial), ] - view = View.objects.get(name=view_name) tsig_key = _load_tsig_key(view_name) cutoff_hours = SETTINGS.get("notify_client_alive_threshold_hours", 24) seen_cutoff = timezone.now() - timezone.timedelta(hours=cutoff_hours) @@ -103,8 +105,8 @@ class SendClientDNSNotify(JobRunner): f" {view_name}/{zone_name} {soa_serial}: {e}" ) - except IntegerKeyValueSetting.DoesNotExist as e: - LOGGER.error(f"Failed to get catalog zone serial: {e}") + except View.DoesNotExist as e: + LOGGER.error(f"Failed to get view for NOTIFY: {e}") except OperationalError as e: LOGGER.error(f"NOTIFY failed due to unexpected error: {e}") diff --git a/netbox_dns_bridge/templates/netbox_dns_bridge/catz_overview.html b/netbox_dns_bridge/templates/netbox_dns_bridge/catz_overview.html index efb6c02..581bbe1 100644 --- a/netbox_dns_bridge/templates/netbox_dns_bridge/catz_overview.html +++ b/netbox_dns_bridge/templates/netbox_dns_bridge/catz_overview.html @@ -7,12 +7,27 @@
-
{% trans "Catalog Zone SOA Serial Number" %}
+
{% trans "Catalog Zone SOA Serial Numbers" %}
- {% if soa_serial is not None %} - {{ soa_serial }} + {% if catalog_zones %} + + + + + + + + + {% for cz in catalog_zones %} + + + + + {% endfor %} + +
{% trans "View" %}{% trans "SOA Serial" %}
{{ cz.view.name }}{{ cz.soa_serial }}
{% else %} - {% trans "Not yet initialized." %} + {% trans "No catalog zones initialized." %} {% endif %}
diff --git a/netbox_dns_bridge/tests/conftest.py b/netbox_dns_bridge/tests/conftest.py index 0f71b2f..29de1ac 100644 --- a/netbox_dns_bridge/tests/conftest.py +++ b/netbox_dns_bridge/tests/conftest.py @@ -180,11 +180,12 @@ def make_record(db): @pytest.fixture def catz_serial(db): - from netbox_dns_bridge.models import IntegerKeyValueSetting + """Initial SOA serial value (1) for freshly created catalog zones. - IntegerKeyValueSetting.objects.update_or_create( - key="catalog-zone-soa-serial", defaults={"value": 1} - ) + A ``CatalogZone`` is auto-created with ``soa_serial=1`` by the zone + post_save signal (via ``update_member_identifier``) whenever a zone is + created. Tests compare against this initial value. + """ return 1 diff --git a/netbox_dns_bridge/tests/test_catalog_zone_manager.py b/netbox_dns_bridge/tests/test_catalog_zone_manager.py index c42d016..2d8b1e2 100644 --- a/netbox_dns_bridge/tests/test_catalog_zone_manager.py +++ b/netbox_dns_bridge/tests/test_catalog_zone_manager.py @@ -8,27 +8,28 @@ from netbox_dns_bridge.management.commands.dns_transfer_endpoint import ( catalog_zone_manager as catzm, ) from netbox_dns_bridge.models import ( - IntegerKeyValueSetting, + CatalogZone, CatalogZoneMemberIdentifier, ) @pytest.mark.django_db class TestIncrementSoaSerial: - def test_creates_setting_if_not_exists(self): - IntegerKeyValueSetting.objects.filter( - key="catalog-zone-soa-serial" - ).delete() - result = catzm.increment_soa_serial() - assert result == 2 # created with value=1, then incremented to 2 + def test_creates_setting_if_not_exists(self, make_view): + view = make_view(name="cz-create-view") + CatalogZone.objects.filter(view=view).delete() + result = catzm.increment_soa_serial(view) + assert result == 2 # created with soa_serial=1, then incremented to 2 - def test_increments_existing(self, catz_serial): - result = catzm.increment_soa_serial() + def test_increments_existing(self, catz_serial, make_view): + view = make_view(name="cz-inc-view") + result = catzm.increment_soa_serial(view) assert result == catz_serial + 1 - def test_consecutive_increments(self, catz_serial): - a = catzm.increment_soa_serial() - b = catzm.increment_soa_serial() + def test_consecutive_increments(self, catz_serial, make_view): + view = make_view(name="cz-cons-view") + a = catzm.increment_soa_serial(view) + b = catzm.increment_soa_serial(view) assert b == a + 1 def test_wraps_at_max(self, catz_serial): @@ -37,17 +38,17 @@ class TestIncrementSoaSerial: from unittest.mock import patch, MagicMock mock_obj = MagicMock() - mock_obj.value = 0xFFFFFFFF + mock_obj.soa_serial = 0xFFFFFFFF mock_qs = MagicMock() mock_qs.select_for_update.return_value.get_or_create.return_value = ( mock_obj, False ) with patch( "netbox_dns_bridge.management.commands.dns_transfer_endpoint." - "catalog_zone_manager.IntegerKeyValueSetting.objects", + "catalog_zone_manager.CatalogZone.objects", mock_qs, ): - result = catzm.increment_soa_serial() + result = catzm.increment_soa_serial(MagicMock()) assert result == 1 def test_wrap_logs_warning(self, catz_serial, caplog): @@ -55,18 +56,18 @@ class TestIncrementSoaSerial: from unittest.mock import patch, MagicMock mock_obj = MagicMock() - mock_obj.value = 0xFFFFFFFF + mock_obj.soa_serial = 0xFFFFFFFF mock_qs = MagicMock() mock_qs.select_for_update.return_value.get_or_create.return_value = ( mock_obj, False ) with patch( "netbox_dns_bridge.management.commands.dns_transfer_endpoint." - "catalog_zone_manager.IntegerKeyValueSetting.objects", + "catalog_zone_manager.CatalogZone.objects", mock_qs, ): with caplog.at_level(logging.WARNING, logger="dns-bridge"): - catzm.increment_soa_serial() + catzm.increment_soa_serial(MagicMock()) assert any("reached max" in r.message for r in caplog.records) @@ -190,7 +191,7 @@ class TestCreateZone: assert zone.rdclass == dns.rdataclass.IN def test_empty_view_returns_zone_with_soa_ns_version(self, make_view): - view = make_view(name="cz-empty-view") + make_view(name="cz-empty-view") zone = catzm.create_zone("catz", "cz-empty-view") assert zone is not None origin = dns.name.from_text("catz", dns.name.root) diff --git a/netbox_dns_bridge/tests/test_management_commands.py b/netbox_dns_bridge/tests/test_management_commands.py index 32d30fd..8866e92 100644 --- a/netbox_dns_bridge/tests/test_management_commands.py +++ b/netbox_dns_bridge/tests/test_management_commands.py @@ -1,4 +1,4 @@ -"""Tests for management commands: dns-settings, dns-export-zone, dns-export-zones, +"""Tests for management commands: dns-export-zone, dns-export-zones, dns-transfer-endpoint.""" import os import tempfile @@ -9,60 +9,6 @@ import pytest from django.core.management import call_command from django.core.management.base import CommandError -from netbox_dns_bridge.models import IntegerKeyValueSetting - - -@pytest.mark.django_db -class TestDnsSettingsCommand: - def test_list_empty(self): - IntegerKeyValueSetting.objects.all().delete() - out = StringIO() - call_command("dns-settings", "list", stdout=out) - assert "No settings found" in out.getvalue() - - def test_list_with_entries(self): - IntegerKeyValueSetting.objects.create(key="k1", value=10) - IntegerKeyValueSetting.objects.create(key="k2", value=20) - out = StringIO() - call_command("dns-settings", "list", stdout=out) - output = out.getvalue() - assert "k1: 10" in output - assert "k2: 20" in output - - def test_get_existing(self): - IntegerKeyValueSetting.objects.create(key="serial", value=42) - out = StringIO() - call_command("dns-settings", "get", "serial", stdout=out) - assert "serial: 42" in out.getvalue() - - def test_get_missing_raises(self): - with pytest.raises(CommandError, match="No setting"): - call_command("dns-settings", "get", "nonexistent") - - def test_get_without_key_raises(self): - with pytest.raises(CommandError, match="key is required"): - call_command("dns-settings", "get") - - def test_set_existing(self): - IntegerKeyValueSetting.objects.create(key="counter", value=5) - out = StringIO() - call_command("dns-settings", "set", "counter", 99, stdout=out) - assert "Updated counter from 5 to 99" in out.getvalue() - assert IntegerKeyValueSetting.objects.get(key="counter").value == 99 - - def test_set_missing_key_raises(self): - with pytest.raises(CommandError, match="setting is required"): - call_command("dns-settings", "set") - - def test_set_without_value_raises(self): - IntegerKeyValueSetting.objects.create(key="needval", value=1) - with pytest.raises(CommandError, match="value is required"): - call_command("dns-settings", "set", "needval") - - def test_set_nonexistent_setting_raises(self): - with pytest.raises(CommandError, match="No setting found"): - call_command("dns-settings", "set", "ghost", 1) - @pytest.mark.django_db class TestDnsExportZoneCommand: diff --git a/netbox_dns_bridge/tests/test_models.py b/netbox_dns_bridge/tests/test_models.py index db45036..02810fc 100644 --- a/netbox_dns_bridge/tests/test_models.py +++ b/netbox_dns_bridge/tests/test_models.py @@ -2,29 +2,46 @@ import pytest from netbox_dns_bridge.models import ( - IntegerKeyValueSetting, + CatalogZone, CatalogZoneMemberIdentifier, SeenTransferClients, ) @pytest.mark.django_db -class TestIntegerKeyValueSetting: - def test_str_representation(self): - obj = IntegerKeyValueSetting.objects.create(key="my-key", value=42) - assert str(obj) == "my-key: 42" +class TestCatalogZone: + def test_str_representation(self, make_view): + view = make_view(name="cz-str-view") + cz = CatalogZone.objects.create(view=view, soa_serial=42) + assert str(cz) == "cz-str-view: 42" - def test_unique_key_constraint(self): - IntegerKeyValueSetting.objects.create(key="dup-key", value=1) - from django.db import IntegrityError + def test_default_soa_serial(self, make_view): + view = make_view(name="cz-default-view") + cz = CatalogZone.objects.create(view=view) + assert cz.soa_serial == 1 - with pytest.raises(IntegrityError): - IntegerKeyValueSetting.objects.create(key="dup-key", value=2) + def test_one_to_one_relationship(self, make_view): + view = make_view(name="cz-oto-view") + cz = CatalogZone.objects.create(view=view, soa_serial=7) + assert view.catalog_zone == cz - def test_create_and_retrieve(self): - IntegerKeyValueSetting.objects.create(key="catz-serial", value=99) - obj = IntegerKeyValueSetting.objects.get(key="catz-serial") - assert obj.value == 99 + def test_create_and_retrieve(self, make_view): + view = make_view(name="cz-retrieve-view") + CatalogZone.objects.create(view=view, soa_serial=99) + cz = CatalogZone.objects.get(view=view) + assert cz.soa_serial == 99 + + def test_ordering_by_view_name(self, make_view): + v1 = make_view(name="z-view") + v2 = make_view(name="a-view") + v3 = make_view(name="m-view") + CatalogZone.objects.create(view=v1, soa_serial=1) + CatalogZone.objects.create(view=v2, soa_serial=2) + CatalogZone.objects.create(view=v3, soa_serial=3) + names = list( + CatalogZone.objects.values_list("view__name", flat=True) + ) + assert names == ["a-view", "m-view", "z-view"] @pytest.mark.django_db @@ -34,19 +51,35 @@ class TestCatalogZoneMemberIdentifier: m = CatalogZoneMemberIdentifier.objects.get(zone=zone) assert str(m) == m.name - def test_unique_name_constraint(self, make_zone): - zone = make_zone(name="uniq.test") - existing = CatalogZoneMemberIdentifier.objects.get(zone=zone) - existing.name = "shared-name" - existing.save() + def test_unique_name_per_catalog_zone(self, make_zone, make_view): + view = make_view(name="shared-view") + zone1 = make_zone(name="uniq1.test", view=view) + zone2 = make_zone(name="uniq2.test", view=view) - other = make_zone(name="other.test") - other_ident = CatalogZoneMemberIdentifier.objects.get(zone=other) + ident1 = CatalogZoneMemberIdentifier.objects.get(zone=zone1) + ident1.name = "shared-name" + ident1.save() + + ident2 = CatalogZoneMemberIdentifier.objects.get(zone=zone2) from django.db import IntegrityError with pytest.raises(IntegrityError): - other_ident.name = "shared-name" - other_ident.save() + ident2.name = "shared-name" + ident2.save() + + def test_same_name_allowed_in_different_catalog_zones(self, make_zone, make_view): + view1 = make_view(name="view-a") + view2 = make_view(name="view-b") + zone1 = make_zone(name="diff1.test", view=view1) + zone2 = make_zone(name="diff2.test", view=view2) + + ident1 = CatalogZoneMemberIdentifier.objects.get(zone=zone1) + ident1.name = "shared-name" + ident1.save() + + ident2 = CatalogZoneMemberIdentifier.objects.get(zone=zone2) + ident2.name = "shared-name" + ident2.save() def test_cascade_delete_when_zone_deleted(self, make_zone): zone = make_zone(name="cascade.test") @@ -60,10 +93,11 @@ class TestCatalogZoneMemberIdentifier: m = CatalogZoneMemberIdentifier.objects.get(zone=zone) assert zone.catz_identifier == m - def test_ordering_by_name(self, make_zone): - z1 = make_zone(name="z1.test") - z2 = make_zone(name="z2.test") - z3 = make_zone(name="z3.test") + def test_ordering_by_name(self, make_zone, make_view): + view = make_view(name="ord-view") + z1 = make_zone(name="z1.test", view=view) + z2 = make_zone(name="z2.test", view=view) + z3 = make_zone(name="z3.test", view=view) CatalogZoneMemberIdentifier.objects.filter(zone=z3).update(name="gamma") CatalogZoneMemberIdentifier.objects.filter(zone=z1).update(name="alpha") CatalogZoneMemberIdentifier.objects.filter(zone=z2).update(name="beta") @@ -78,8 +112,6 @@ class TestCatalogZoneMemberIdentifier: @pytest.mark.django_db class TestSeenTransferClients: def test_str_representation(self, make_view): - from netbox_dns.models import View - view = make_view(name="str-view") client = SeenTransferClients.objects.create( source_ip="192.0.2.1", diff --git a/netbox_dns_bridge/tests/test_notify_handler.py b/netbox_dns_bridge/tests/test_notify_handler.py index 5718a7e..e128f09 100644 --- a/netbox_dns_bridge/tests/test_notify_handler.py +++ b/netbox_dns_bridge/tests/test_notify_handler.py @@ -203,12 +203,12 @@ class TestScheduleNsNotify: class TestSendClientDNSNotifyRun: def test_sends_notify_to_seen_clients(self, make_zone, make_view, monkeypatch): import netbox_dns_bridge.signals.notify_handler as nh - from netbox_dns_bridge.models import SeenTransferClients, IntegerKeyValueSetting + from netbox_dns_bridge.models import SeenTransferClients, CatalogZone from django.utils import timezone view = make_view(name="notify-run-view") - IntegerKeyValueSetting.objects.update_or_create( - key="catalog-zone-soa-serial", defaults={"value": 42} + CatalogZone.objects.update_or_create( + view=view, defaults={"soa_serial": 42} ) SeenTransferClients.objects.create( source_ip="192.0.2.99", view=view, last_seen=timezone.now() @@ -236,7 +236,7 @@ class TestSendClientDNSNotifyRun: assert mock_udp.call_count >= 3 # zone + catz + view.catz def test_handles_missing_serial_setting(self, monkeypatch, make_view): - # When the IntegerKeyValueSetting for catz serial doesn't exist, + # When the CatalogZone for the view doesn't exist, # the job should log an error and not send any notifies view = make_view(name="missing-serial-view") diff --git a/netbox_dns_bridge/tests/test_signals.py b/netbox_dns_bridge/tests/test_signals.py index 6e2ae01..03e80b3 100644 --- a/netbox_dns_bridge/tests/test_signals.py +++ b/netbox_dns_bridge/tests/test_signals.py @@ -12,7 +12,7 @@ from django.test.testcases import TestCase as _DjangoTestCase captureOnCommitCallbacks = _DjangoTestCase.captureOnCommitCallbacks from netbox_dns_bridge.models import ( - IntegerKeyValueSetting, + CatalogZone, CatalogZoneMemberIdentifier, ) @@ -42,13 +42,13 @@ class TestZonePostSaveCreate: def test_increments_soa_serial_on_commit(self, make_zone, catz_serial): with captureOnCommitCallbacks(execute=True): zone = make_zone(name="serial-inc.test") - obj = IntegerKeyValueSetting.objects.get(key="catalog-zone-soa-serial") - assert obj.value == catz_serial + 1 + obj = CatalogZone.objects.get(view=zone.view) + assert obj.soa_serial == catz_serial + 1 def test_does_not_increment_without_on_commit(self, make_zone, catz_serial): - make_zone(name="no-commit.test") - obj = IntegerKeyValueSetting.objects.get(key="catalog-zone-soa-serial") - assert obj.value == catz_serial + zone = make_zone(name="no-commit.test") + obj = CatalogZone.objects.get(view=zone.view) + assert obj.soa_serial == catz_serial def test_schedule_client_notify_called_when_enabled(self, make_zone, patch_settings): patch_settings({"notify_clients": True}) @@ -114,24 +114,63 @@ class TestZonePostSaveUpdate: with captureOnCommitCallbacks(execute=True): zone.soa_serial = zone.soa_serial + 1 zone.save() - obj = IntegerKeyValueSetting.objects.get(key="catalog-zone-soa-serial") - assert obj.value == catz_serial + 1 + obj = CatalogZone.objects.get(view=zone.view) + assert obj.soa_serial == catz_serial + 1 @pytest.mark.django_db class TestZonePostDelete: def test_increments_soa_serial_on_commit_on_delete(self, make_zone, catz_serial): zone = make_zone(name="delete-serial.test") + view = zone.view with captureOnCommitCallbacks(execute=True): zone.delete() - obj = IntegerKeyValueSetting.objects.get(key="catalog-zone-soa-serial") - assert obj.value == catz_serial + 1 + obj = CatalogZone.objects.get(view=view) + assert obj.soa_serial == catz_serial + 1 def test_does_not_increment_without_on_commit(self, make_zone, catz_serial): zone = make_zone(name="delete-no-commit.test") + view = zone.view zone.delete() - obj = IntegerKeyValueSetting.objects.get(key="catalog-zone-soa-serial") - assert obj.value == catz_serial + obj = CatalogZone.objects.get(view=view) + assert obj.soa_serial == catz_serial + + +@pytest.mark.django_db +class TestZoneViewChange: + def test_increments_both_catalog_zone_serials(self, make_zone, make_view, catz_serial): + view_a = make_view(name="view-a") + view_b = make_view(name="view-b") + # view_b has no zones yet, so no CatalogZone was auto-created for it. + CatalogZone.objects.create(view=view_b, soa_serial=5) + + zone = make_zone(name="move.test", view=view_a) + + serial_a_before = CatalogZone.objects.get(view=view_a).soa_serial + serial_b_before = CatalogZone.objects.get(view=view_b).soa_serial + + with captureOnCommitCallbacks(execute=True): + zone.view = view_b + zone.save() + + serial_a_after = CatalogZone.objects.get(view=view_a).soa_serial + serial_b_after = CatalogZone.objects.get(view=view_b).soa_serial + assert serial_a_after == serial_a_before + 1 + assert serial_b_after == serial_b_before + 1 + + def test_updates_identifier_catalog_zone_fk(self, make_zone, make_view): + view_a = make_view(name="fk-view-a") + view_b = make_view(name="fk-view-b") + zone = make_zone(name="fk-move.test", view=view_a) + + ident = CatalogZoneMemberIdentifier.objects.get(zone=zone) + assert ident.catalog_zone == CatalogZone.objects.get(view=view_a) + + zone.view = view_b + zone.save() + + ident.refresh_from_db() + assert ident.catalog_zone == CatalogZone.objects.get(view=view_b) @pytest.mark.django_db @@ -141,9 +180,9 @@ class TestSignalDedup: from django.contrib.auth import get_user_model zone = make_zone(name="dedup.test") - original_serial = IntegerKeyValueSetting.objects.get( - key="catalog-zone-soa-serial" - ).value + original_serial = CatalogZone.objects.get( + view=zone.view + ).soa_serial User = get_user_model() user = User(username="dedup-user", is_superuser=True) @@ -163,16 +202,16 @@ class TestSignalDedup: zone.soa_serial = zone.soa_serial + 2 zone.save() - new_serial = IntegerKeyValueSetting.objects.get( - key="catalog-zone-soa-serial" - ).value + new_serial = CatalogZone.objects.get( + view=zone.view + ).soa_serial assert new_serial == original_serial + 1 def test_no_dedup_without_request(self, make_zone): zone = make_zone(name="nodedup.test") - original_serial = IntegerKeyValueSetting.objects.get( - key="catalog-zone-soa-serial" - ).value + original_serial = CatalogZone.objects.get( + view=zone.view + ).soa_serial with captureOnCommitCallbacks(execute=True): zone.soa_serial = zone.soa_serial + 1 @@ -181,9 +220,9 @@ class TestSignalDedup: zone.soa_serial = zone.soa_serial + 2 zone.save() - new_serial = IntegerKeyValueSetting.objects.get( - key="catalog-zone-soa-serial" - ).value + new_serial = CatalogZone.objects.get( + view=zone.view + ).soa_serial assert new_serial == original_serial + 2 diff --git a/netbox_dns_bridge/tests/test_smoke.py b/netbox_dns_bridge/tests/test_smoke.py index 9ef5abc..4fdfe7b 100644 --- a/netbox_dns_bridge/tests/test_smoke.py +++ b/netbox_dns_bridge/tests/test_smoke.py @@ -13,11 +13,11 @@ def test_plugin_importable(): def test_models_importable(): from netbox_dns_bridge.models import ( - IntegerKeyValueSetting, + CatalogZone, CatalogZoneMemberIdentifier, SeenTransferClients, ) - assert IntegerKeyValueSetting._meta.model_name + assert CatalogZone._meta.model_name assert CatalogZoneMemberIdentifier._meta.model_name assert SeenTransferClients._meta.model_name diff --git a/netbox_dns_bridge/tests/test_views.py b/netbox_dns_bridge/tests/test_views.py index ec49b7e..d39dc05 100644 --- a/netbox_dns_bridge/tests/test_views.py +++ b/netbox_dns_bridge/tests/test_views.py @@ -43,18 +43,21 @@ class TestCatzOverviewView: resp = client.get("/plugins/dns-bridge/catz/") assert resp.status_code == 200 - def test_context_contains_soa_serial(self, client, superuser, catz_serial): + def test_context_contains_catalog_zones(self, client, superuser, catz_serial, make_zone): + zone = make_zone(name="ctx.test") client.login(username="admin", password="testpass") resp = client.get("/plugins/dns-bridge/catz/") - assert "soa_serial" in resp.context - assert resp.context["soa_serial"] == catz_serial + assert "catalog_zones" in resp.context + cz_list = list(resp.context["catalog_zones"]) + assert len(cz_list) >= 1 + cz = next(c for c in cz_list if c.view_id == zone.view_id) + assert cz.soa_serial == catz_serial - def test_context_soa_serial_none_when_not_set(self, client, superuser): - from netbox_dns_bridge.models import IntegerKeyValueSetting - IntegerKeyValueSetting.objects.filter(key="catalog-zone-soa-serial").delete() + def test_context_no_catalog_zones_when_empty(self, client, superuser): client.login(username="admin", password="testpass") resp = client.get("/plugins/dns-bridge/catz/") - assert resp.context["soa_serial"] is None + assert "catalog_zones" in resp.context + assert list(resp.context["catalog_zones"]) == [] @pytest.mark.django_db diff --git a/netbox_dns_bridge/views.py b/netbox_dns_bridge/views.py index f5e645f..c5c751d 100644 --- a/netbox_dns_bridge/views.py +++ b/netbox_dns_bridge/views.py @@ -8,7 +8,7 @@ from netbox.views.generic import ObjectListView from .filtersets import ZoneFilterSet from .forms import CatalogZoneFilterForm -from .models import IntegerKeyValueSetting, SeenTransferClients +from .models import CatalogZone, SeenTransferClients from .tables import CatalogZoneTable @@ -27,10 +27,9 @@ class CatzOverviewView(ObjectListView): def get_extra_context(self, request): context = super().get_extra_context(request) - soa_serial_obj = IntegerKeyValueSetting.objects.filter( - key="catalog-zone-soa-serial" - ).first() - context["soa_serial"] = soa_serial_obj.value if soa_serial_obj else None + context["catalog_zones"] = CatalogZone.objects.select_related( + "view" + ).order_by("view__name") return context