From 478871838aa51ed8d5f8ded8bea1d8640493c390 Mon Sep 17 00:00:00 2001 From: Jonas Braathen Date: Sun, 27 Sep 2026 17:21:00 +0200 Subject: [PATCH] fix(zone_sync): freeze before writing and keep serials above dynamic updates --- mdb/management/commands/zone_sync.py | 82 ++++++++++++++++----- mdb/services/zone_sync.py | 62 +++++++++++----- tests/test_zone_sync.py | 50 +++++++++++++ tests/test_zone_sync_command.py | 105 ++++++++++++++++++++++++--- 4 files changed, 248 insertions(+), 51 deletions(-) diff --git a/mdb/management/commands/zone_sync.py b/mdb/management/commands/zone_sync.py index 5ad1ae5..32442ab 100644 --- a/mdb/management/commands/zone_sync.py +++ b/mdb/management/commands/zone_sync.py @@ -50,19 +50,45 @@ class Command(BaseCommand): if zone is None: self.stderr.write("error: zone '%s' not found\n" % zone_name) raise SystemExit(1) - service.sync_zone(zone) + zones = [zone] else: - service.sync_all_zones() + zones = service.pending_zones() - if service.changes and not debug: - self.reload_and_notify(service, service.changes) + if not zones: + return + + if debug: + for zone in zones: + service.sync_zone(zone) + return + + self.freeze(service, zones) + try: + for zone in zones: + service.sync_zone(zone) + except BaseException: + service.reload() + raise + self.reload_and_notify(service, service.changes) + + def freeze(self, service, zones): + self.stdout.write("freezing zones... ") + success, failed_command, output = service.freeze() + if not success: + self.stdout.write("fail\n") + # A failed freeze may have frozen some zones before stopping. + service.run_commands([service.thaw_command]) + self.notify_failure( + "Failed to freeze zones, nothing was written", + failed_command, + output, + [z.domain_name for z in zones], + ) + self.stdout.write("ok\n") def reload_and_notify(self, service, changes): self.stdout.write("reloading zones... ") - success, failed_command, output = service.run_reload_commands() - - zones = list(changes.keys()) - zone_list = "\n".join("- " + z for z in zones) + success, failed_command, output = service.reload() if not success: self.stdout.write("fail\n") @@ -70,19 +96,14 @@ class Command(BaseCommand): [v["errors"] for v in changes.values() if v["errors"]] ) diffs = "\n\n\n".join([v["diff"] for v in changes.values() if v["diff"]]) - message = ( - "Failed to reload zones, please inspect...\n\n" - "%s@%s$ %s\n%s\n%s" - % (os.getlogin(), os.uname()[1], failed_command, output, zone_list) + self.notify_failure( + "Failed to reload zones", + failed_command, + output, + list(changes.keys()), + errors, + diffs, ) - if errors: - message += "\n\n" + errors - discord.notify( - subject="⚠️ FAIL: bind", - message=message, - preformatted=diffs or None, - ) - raise SystemExit(1) self.stdout.write("ok\n") for zone, info in changes.items(): @@ -94,3 +115,24 @@ class Command(BaseCommand): message=message, preformatted=info["diff"] or None, ) + + def notify_failure( + self, summary, failed_command, output, zones, errors="", diffs="" + ): + zone_list = "\n".join("- " + z for z in zones) + message = "%s, please inspect...\n\n%s@%s$ %s\n%s\n%s" % ( + summary, + os.getlogin(), + os.uname()[1], + failed_command, + output, + zone_list, + ) + if errors: + message += "\n\n" + errors + discord.notify( + subject="⚠️ FAIL: bind", + message=message, + preformatted=diffs or None, + ) + raise SystemExit(1) diff --git a/mdb/services/zone_sync.py b/mdb/services/zone_sync.py index 318fc56..c4a1501 100644 --- a/mdb/services/zone_sync.py +++ b/mdb/services/zone_sync.py @@ -1,4 +1,5 @@ import os +import re import shutil import sys import tempfile @@ -11,10 +12,9 @@ from mdb.models import Domain, Ip4Subnet, Ip6Subnet class ZoneSyncService: rndc_bin = shutil.which("rndc") or "/usr/sbin/rndc" - reload_command = "%s reload" % rndc_bin freeze_command = "%s freeze" % rndc_bin + reload_command = "%s reload" % rndc_bin thaw_command = "%s thaw" % rndc_bin - commands_in_order = [freeze_command, reload_command, thaw_command] checkzone_bin = shutil.which("named-checkzone") or "/usr/sbin/named-checkzone" compilezone_bin = shutil.which("named-compilezone") or "/usr/sbin/named-compilezone" checkzone_dir = "/tmp/mdb-checkzone" @@ -31,18 +31,30 @@ class ZoneSyncService: print("ERROR: no such file %s, exiting..." % f) sys.exit(1) - def run_reload_commands(self): - """Execute freeze/reload/thaw in order. + def run_commands(self, commands): + """Execute commands in order, stopping at the first failure. Returns (success, failed_command, output). On success, failed_command and output are empty strings. """ - for command in self.commands_in_order: + for command in commands: status, output = getstatusoutput(command) if status != 0: return False, command, output return True, "", "" + def freeze(self): + """Stop dynamic updates and flush each dynamic zone's journal to its file. + + Must run before zone files are written: freeze dumps the zone BIND + holds in memory, which would overwrite a file written earlier. + """ + return self.run_commands([self.freeze_command]) + + def reload(self): + """Load the written zone files and resume dynamic updates.""" + return self.run_commands([self.reload_command, self.thaw_command]) + def find_zone(self, name): for model in (Domain, Ip4Subnet, Ip6Subnet): try: @@ -51,18 +63,31 @@ class ZoneSyncService: pass return None - def sync_all_zones(self): - for zone in chain( - Domain.objects.all(), Ip4Subnet.objects.all(), Ip6Subnet.objects.all() - ): - if zone.domain_serial == zone.domain_active_serial and not self.force: - continue - if self.sync_zone(zone): - self.changes[zone.domain_name]["success"] = True + def pending_zones(self): + return [ + zone + for zone in chain( + Domain.objects.all(), Ip4Subnet.objects.all(), Ip6Subnet.objects.all() + ) + if zone.domain_serial != zone.domain_active_serial or self.force + ] def sync_zone(self, zone): zonetype = getattr(zone._meta, "verbose_name", None) or zone._meta.model_name + try: + with open(zone.domain_filename, "r") as f: + old = f.read() + except IOError: + old = "" + + # Dynamic updates (ACME dns-01 challenges) raise the serial in the + # frozen file past the one mdb generated. Secondaries ignore a zone + # whose serial does not increase. + live_serial = self.read_serial(old) + if live_serial is not None and live_serial >= zone.domain_serial: + zone.domain_serial = live_serial + 1 + self.changes[zone.domain_name] = { "old_serial": zone.domain_active_serial, "new_serial": zone.domain_serial, @@ -85,12 +110,6 @@ class ZoneSyncService: # generated content always use the same format. new = self.normalize_zone(zone, zone.zone_file_contents()) - try: - with open(zone.domain_filename, "r") as f: - old = f.read() - except IOError: - old = "" - self.changes[zone.domain_name]["diff"] = self.calculate_diff(new, old, zone) sys.stdout.write("\t- validating... ") @@ -121,6 +140,11 @@ class ZoneSyncService: self.changes[zone.domain_name]["success"] = True return True + def read_serial(self, contents): + """Return the SOA serial of a zone file, or None if there is none.""" + match = re.search(r"^\s*(\d+)\s*;\s*serial", contents, re.MULTILINE) + return int(match.group(1)) if match else None + def normalize_zone(self, zone, contents): """Convert zone file contents to BIND's canonical format. diff --git a/tests/test_zone_sync.py b/tests/test_zone_sync.py index d11651a..2a35b1c 100644 --- a/tests/test_zone_sync.py +++ b/tests/test_zone_sync.py @@ -160,6 +160,27 @@ def test_check_zone_accepts_zone_file_contents(db_domain, service): assert output.endswith("OK") +def test_read_serial_from_normalized_zone(): + assert ZoneSyncService().read_serial(EXPECTED_NORMALIZED) == 2024010101 + + +def test_read_serial_from_generated_zone(): + assert ZoneSyncService().read_serial(SAMPLE_ZONE) == 2024010101 + + +def test_read_serial_none_without_soa(): + assert ZoneSyncService().read_serial("") is None + + +def test_pending_zones_skips_synced_zones(db_domain, service): + assert service.pending_zones() == [db_domain] + db_domain.domain_active_serial = db_domain.domain_serial + db_domain.save() + assert service.pending_zones() == [] + service.force = True + assert service.pending_zones() == [db_domain] + + def test_find_zone_returns_matching_domain(db_domain, service): assert service.find_zone("example.org") == db_domain @@ -193,6 +214,35 @@ def test_sync_zone_records_success_and_diff(db_domain, tmp_path): assert change["new_serial"] == 2024010101 +def test_sync_zone_raises_serial_past_live_zone(db_domain, tmp_path): + """Dynamic updates can leave the live serial above the one mdb generated.""" + with open(db_domain.domain_filename, "w") as f: + f.write(EXPECTED_NORMALIZED.replace("2024010101", "2024010137")) + svc = ZoneSyncService(debug=False) + svc.checkzone_dir = str(tmp_path) + + svc.sync_zone(db_domain) + + db_domain.refresh_from_db() + assert db_domain.domain_serial == 2024010138 + assert db_domain.domain_active_serial == 2024010138 + assert svc.changes["example.org"]["new_serial"] == 2024010138 + with open(db_domain.domain_filename) as f: + assert svc.read_serial(f.read()) == 2024010138 + + +def test_sync_zone_keeps_serial_above_live_zone(db_domain, tmp_path): + with open(db_domain.domain_filename, "w") as f: + f.write(EXPECTED_NORMALIZED.replace("2024010101", "2024010099")) + svc = ZoneSyncService(debug=False) + svc.checkzone_dir = str(tmp_path) + + svc.sync_zone(db_domain) + + db_domain.refresh_from_db() + assert db_domain.domain_serial == 2024010101 + + def add_host(domain, hostname, allow_email, ip4address=None): host_type = HostType.objects.create(host_type="server", description="") operating_system = OperatingSystem.objects.create( diff --git a/tests/test_zone_sync_command.py b/tests/test_zone_sync_command.py index 5cf3318..910ae4b 100644 --- a/tests/test_zone_sync_command.py +++ b/tests/test_zone_sync_command.py @@ -1,8 +1,10 @@ +import os from io import StringIO from unittest.mock import MagicMock, patch import pytest from mdb.management.commands.zone_sync import Command as CommandClass +from mdb.services.zone_sync import ZoneSyncService @pytest.fixture @@ -18,30 +20,45 @@ def make_service(changes=None): svc.changes = changes or { "example.org": {"errors": None, "diff": "- old\n+ new", "success": True} } - svc.run_reload_commands.return_value = (True, "", "") + svc.reload.return_value = (True, "", "") return svc -# --- run_reload_commands --- +# --- run_commands, freeze, reload --- -def test_run_reload_commands_calls_all_on_success(service): +def test_run_commands_calls_all_on_success(service): with patch("mdb.services.zone_sync.getstatusoutput", return_value=(0, "")) as mock: - success, _, _ = service.run_reload_commands() + success, _, _ = service.run_commands(["a", "b", "c"]) assert success is True assert mock.call_count == 3 -def test_run_reload_commands_stops_on_first_failure(service): +def test_run_commands_stops_on_first_failure(service): with patch( "mdb.services.zone_sync.getstatusoutput", side_effect=[(0, ""), (1, "connection refused"), (0, "")], ) as mock: - success, failed_cmd, output = service.run_reload_commands() + success, failed_cmd, output = service.run_commands(["a", "b", "c"]) assert success is False - assert "reload" in failed_cmd + assert failed_cmd == "b" assert output == "connection refused" - assert mock.call_count == 2 # thaw never reached + assert mock.call_count == 2 + + +def test_freeze_runs_only_freeze(service): + with patch("mdb.services.zone_sync.getstatusoutput", return_value=(0, "")) as mock: + service.freeze() + assert [c.args[0] for c in mock.call_args_list] == [service.freeze_command] + + +def test_reload_runs_reload_then_thaw(service): + with patch("mdb.services.zone_sync.getstatusoutput", return_value=(0, "")) as mock: + service.reload() + assert [c.args[0] for c in mock.call_args_list] == [ + service.reload_command, + service.thaw_command, + ] # --- reload_and_notify --- @@ -79,7 +96,7 @@ def test_reload_and_notify_success_passes_diff_as_preformatted(cmd): def test_reload_and_notify_failure_writes_fail_and_exits(cmd): svc = make_service() - svc.run_reload_commands.return_value = (False, "/usr/sbin/rndc freeze", "timed out") + svc.reload.return_value = (False, "/usr/sbin/rndc reload", "timed out") with patch("mdb.management.commands.zone_sync.discord"): with patch("os.getlogin", return_value="testuser"): with pytest.raises(SystemExit) as exc: @@ -90,7 +107,7 @@ def test_reload_and_notify_failure_writes_fail_and_exits(cmd): def test_reload_and_notify_failure_discord_includes_command_and_output(cmd): svc = make_service() - svc.run_reload_commands.return_value = (False, "/usr/sbin/rndc freeze", "timed out") + svc.reload.return_value = (False, "/usr/sbin/rndc reload", "timed out") with patch("mdb.management.commands.zone_sync.discord") as mock_discord: with patch("os.getlogin", return_value="testuser"): with pytest.raises(SystemExit): @@ -98,7 +115,7 @@ def test_reload_and_notify_failure_discord_includes_command_and_output(cmd): subject = mock_discord.notify.call_args.kwargs["subject"] message = mock_discord.notify.call_args.kwargs["message"] assert "⚠️" in subject - assert "rndc freeze" in message + assert "rndc reload" in message assert "timed out" in message assert "- example.org" in message @@ -123,6 +140,70 @@ def test_handle_skips_reload_in_debug_mode(cmd, db_domain): @pytest.mark.django_db def test_handle_calls_reload_and_notify_when_changes_exist(cmd, db_domain): - with patch.object(cmd, "reload_and_notify") as mock_reload: + with ( + patch.object(cmd, "freeze"), + patch.object(cmd, "reload_and_notify") as mock_reload, + ): cmd.handle(debug=False, force=True, zone="example.org") mock_reload.assert_called_once() + + +@pytest.mark.django_db +def test_handle_does_nothing_without_pending_zones(cmd): + with patch.object(ZoneSyncService, "freeze") as mock_freeze: + cmd.handle(debug=False, force=False, zone=None) + mock_freeze.assert_not_called() + + +@pytest.mark.django_db +def test_handle_freezes_before_writing_and_reloads_after(cmd, db_domain): + """rndc freeze dumps the in-memory zone, so it must not run after the write.""" + seen = [] + + def freeze(): + seen.append(("freeze", os.path.exists(db_domain.domain_filename))) + return True, "", "" + + def reload(): + seen.append(("reload", os.path.exists(db_domain.domain_filename))) + return True, "", "" + + with ( + patch.object(ZoneSyncService, "freeze", side_effect=freeze), + patch.object(ZoneSyncService, "reload", side_effect=reload), + patch("mdb.management.commands.zone_sync.discord"), + ): + cmd.handle(debug=False, force=False, zone=None) + + assert seen == [("freeze", False), ("reload", True)] + + +@pytest.mark.django_db +def test_handle_writes_nothing_when_freeze_fails(cmd, db_domain): + with ( + patch.object( + ZoneSyncService, "freeze", return_value=(False, "rndc freeze", "no") + ), + patch.object(ZoneSyncService, "run_commands") as mock_run, + patch("mdb.management.commands.zone_sync.discord") as mock_discord, + patch("os.getlogin", return_value="testuser"), + pytest.raises(SystemExit), + ): + cmd.handle(debug=False, force=False, zone=None) + + assert not os.path.exists(db_domain.domain_filename) + mock_run.assert_called_once_with([ZoneSyncService.thaw_command]) + assert "- example.org" in mock_discord.notify.call_args.kwargs["message"] + + +@pytest.mark.django_db +def test_handle_thaws_when_sync_raises(cmd, db_domain): + with ( + patch.object(ZoneSyncService, "freeze", return_value=(True, "", "")), + patch.object(ZoneSyncService, "reload") as mock_reload, + patch.object(ZoneSyncService, "sync_zone", side_effect=RuntimeError), + pytest.raises(RuntimeError), + ): + cmd.handle(debug=False, force=False, zone=None) + + mock_reload.assert_called_once()