From 572dd9eae4ef051b0205a5c5ed3ae41efcb4e1a2 Mon Sep 17 00:00:00 2001 From: Lyncoln Date: Sun, 20 Sep 2026 13:13:28 -0400 Subject: [PATCH] Account for deferred retirements in apply budget --- dns_sync/cli.py | 17 ++++++-- dns_sync/executor.py | 11 +++++ tests/test_cli_reconcile.py | 81 ++++++++++++++++++++++++++++++++++--- tests/test_executor.py | 13 +++++- 4 files changed, 112 insertions(+), 10 deletions(-) diff --git a/dns_sync/cli.py b/dns_sync/cli.py index 170c2b6..ca6c5df 100644 --- a/dns_sync/cli.py +++ b/dns_sync/cli.py @@ -65,7 +65,7 @@ def build_parser(): "--max-changes", type=_nonnegative_int, default=None, - help="Maximum CREATE/UPDATE actions allowed with --apply", + help="Maximum planned mutations allowed with --apply", ) sub.add_parser("plugins", help="Show the configured source/provider plugins") @@ -202,11 +202,21 @@ def run_reconcile(args): for action in plan.actions ) + retirement_count = sum( + len(plan.retirements) + for plan in result.plans + ) + mutation_count = ( + change_count + + retirement_count + ) + print() print( "Plan summary: " f"{create_count} create, " f"{update_count} update, " + f"{retirement_count} retirement, " f"{noop_count} unchanged" ) @@ -221,10 +231,10 @@ def run_reconcile(args): if ( args.max_changes is not None - and change_count > args.max_changes + and mutation_count > args.max_changes ): raise RuntimeError( - f"Refusing to apply {change_count} change(s); " + f"Refusing to apply {mutation_count} mutation(s); " f"--max-changes allows {args.max_changes}" ) @@ -241,6 +251,7 @@ def run_reconcile(args): print( "APPLY COMPLETE: " f"{len(execution.applied)} action(s) applied and verified; " + f"{len(execution.retired)} retirement(s) completed; " f"{len(execution.skipped)} unchanged/skipped." ) return 0 diff --git a/dns_sync/executor.py b/dns_sync/executor.py index 4daa588..0c66df3 100644 --- a/dns_sync/executor.py +++ b/dns_sync/executor.py @@ -42,6 +42,7 @@ class ExecutedAction: class ExecutionResult: applied: tuple[ExecutedAction, ...] skipped: tuple[PlanAction, ...] + retired: tuple[tuple[str, RetireRequest], ...] @dataclass @@ -555,6 +556,9 @@ def execute_endpoint_plans( retired_identities: set[ tuple[str, str, str] ] = set() + retired_requests: list[ + tuple[str, RetireRequest] + ] = [] for endpoint_id, retirement in pending_retirements: retired = retirement.object @@ -580,8 +584,15 @@ def execute_endpoint_plans( ) retire(retirement) + retired_requests.append( + ( + endpoint_id, + retirement, + ) + ) return ExecutionResult( applied=tuple(applied), skipped=tuple(skipped), + retired=tuple(retired_requests), ) diff --git a/tests/test_cli_reconcile.py b/tests/test_cli_reconcile.py index 864dd46..87e7316 100644 --- a/tests/test_cli_reconcile.py +++ b/tests/test_cli_reconcile.py @@ -220,6 +220,7 @@ class ReconcileCliTests(unittest.TestCase): execution = SimpleNamespace( applied=(), skipped=(), + retired=(), ) with tempfile.TemporaryDirectory() as directory: @@ -255,11 +256,14 @@ class ReconcileCliTests(unittest.TestCase): stdout.getvalue(), ) self.assertIn( - "APPLY COMPLETE: 0 action(s) applied and verified", + "APPLY COMPLETE: 0 action(s) applied and verified; " + "0 retirement(s) completed", stdout.getvalue(), ) - def test_apply_budget_blocks_executor_before_mutation(self): + def test_apply_budget_counts_planned_retirement_before_mutation( + self, + ): config = SimpleNamespace( target_ip="192.168.1.101", include_disabled=False, @@ -267,10 +271,18 @@ class ReconcileCliTests(unittest.TestCase): domain=None, ) adapter = object() - action = SimpleNamespace( - operation=SimpleNamespace(value="create"), + + retirement = SimpleNamespace( + object=SimpleNamespace( + object_type="certificates", + logical_key="app.example.com", + local_key="63", + ) + ) + plan = SimpleNamespace( + actions=(), + retirements=(retirement,), ) - plan = SimpleNamespace(actions=(action,)) reconciliation = SimpleNamespace( observations=(), desired=(), @@ -298,7 +310,64 @@ class ReconcileCliTests(unittest.TestCase): with self.assertRaisesRegex( RuntimeError, ( - r"Refusing to apply 1 change\(s\); " + r"Refusing to apply 1 mutation\(s\); " + r"--max-changes allows 0" + ), + ): + run_reconcile( + SimpleNamespace( + state_db=str(state_path), + include_disabled=False, + apply=True, + max_changes=0, + ) + ) + + execute.assert_not_called() + + def test_apply_budget_blocks_executor_before_mutation(self): + config = SimpleNamespace( + target_ip="192.168.1.101", + include_disabled=False, + domain_matching=False, + domain=None, + ) + adapter = object() + action = SimpleNamespace( + operation=SimpleNamespace(value="create"), + ) + plan = SimpleNamespace( + actions=(action,), + retirements=(), + ) + reconciliation = SimpleNamespace( + observations=(), + desired=(), + plans=(plan,), + failures=(), + ) + + with tempfile.TemporaryDirectory() as directory: + state_path = Path(directory) / "state.sqlite3" + + with patch( + "dns_sync.cli.AppConfig.from_env", + return_value=config, + ), patch( + "dns_sync.cli._configured_adapters", + return_value=[adapter], + ), patch( + "dns_sync.cli.discover_reconcile_plan", + return_value=reconciliation, + ), patch( + "dns_sync.cli.print_endpoint_plans", + ), patch( + "dns_sync.cli.execute_endpoint_plans", + ) as execute, redirect_stdout(io.StringIO()): + with self.assertRaisesRegex( + RuntimeError, + ( + r"Refusing to apply 1 mutation\(s\); " r"--max-changes allows 0" ), ): diff --git a/tests/test_executor.py b/tests/test_executor.py index 5b1a87a..9350a2a 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -553,7 +553,7 @@ class ExecutionTests(unittest.TestCase): NOW, ) - execute_endpoint_plans( + result = execute_endpoint_plans( [adapter], [plan], store, @@ -590,6 +590,17 @@ class ExecutionTests(unittest.TestCase): ) ], ) + self.assertEqual( + result.retired, + ( + ( + endpoint.identity, + RetireRequest( + object=old_certificate, + ), + ), + ), + ) def test_failed_verification_is_persisted_and_raises(self): endpoint = Endpoint("verizon", "192.168.1.1:443")