From 172f6a333b0e4f9bfa8ff83a5edd75ed34d89fa3 Mon Sep 17 00:00:00 2001
From: Ross McFarland <rwmcfa1@gmail.com>
Date: Fri, 20 Jan 2023 18:11:49 -0800
Subject: [PATCH] fully check auto_arpa and eligible_* usage for safety

---
 octodns/manager.py            | 27 ++++++++++++++++++---------
 tests/test_octodns_manager.py | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/octodns/manager.py b/octodns/manager.py
index 9c3510c..7d05f89 100644
--- a/octodns/manager.py
+++ b/octodns/manager.py
@@ -489,19 +489,28 @@ class Manager(object):
             getattr(plan_output_fh, 'name', plan_output_fh.__class__.__name__),
         )
 
-        if (
-            self.auto_arpa
-            and eligible_zones
-            and any(e.endswith('arpa.') for e in eligible_zones)
-        ):
-            raise ManagerException(
-                'ARPA zones cannot be synced during partial runs when auto_arpa is enabled'
-            )
-
         zones = self.config['zones']
         if eligible_zones:
             zones = IdnaDict({n: zones.get(n) for n in eligible_zones})
 
+        includes_arpa = any(e.endswith('arpa.') for e in zones.keys())
+        if self.auto_arpa and includes_arpa:
+            # it's not safe to mess with auto_arpa when we don't have a complete
+            # picture of records, so if any filtering is happening while arpa
+            # zones are in play we need to abort
+            if any(e.endswith('arpa.') for e in eligible_zones):
+                raise ManagerException(
+                    'ARPA zones cannot be synced during partial runs when auto_arpa is enabled'
+                )
+            if eligible_sources:
+                raise ManagerException(
+                    'eligible_sources is incompatible with auto_arpa'
+                )
+            if eligible_targets:
+                raise ManagerException(
+                    'eligible_targets is incompatible with auto_arpa'
+                )
+
         aliased_zones = {}
         delayed_arpa = []
         futures = []
diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py
index 1344a8a..de29035 100644
--- a/tests/test_octodns_manager.py
+++ b/tests/test_octodns_manager.py
@@ -908,7 +908,7 @@ class TestManager(TestCase):
             str(ctx.exception),
         )
 
-    def test_delayed_arpa(self):
+    def test_auto_arpa(self):
         manager = Manager(get_config_filename('simple-arpa.yaml'))
 
         with TemporaryDirectory() as tmpdir:
@@ -917,7 +917,6 @@ class TestManager(TestCase):
             # we can sync eligible_zones so long as they're not arpa
             tc = manager.sync(dry_run=False, eligible_zones=['unit.tests.'])
             self.assertEqual(22, tc)
-
             # can't do partial syncs that include arpa zones
             with self.assertRaises(ManagerException) as ctx:
                 manager.sync(
@@ -929,6 +928,36 @@ class TestManager(TestCase):
                 str(ctx.exception),
             )
 
+            # same for eligible_sources
+            tc = manager.sync(
+                dry_run=False,
+                eligible_zones=['unit.tests.'],
+                eligible_sources=['in'],
+            )
+            self.assertEqual(22, tc)
+            # can't do partial syncs that include arpa zones
+            with self.assertRaises(ManagerException) as ctx:
+                manager.sync(dry_run=False, eligible_sources=['in'])
+            self.assertEqual(
+                'eligible_sources is incompatible with auto_arpa',
+                str(ctx.exception),
+            )
+
+            # same for eligible_targets
+            tc = manager.sync(
+                dry_run=False,
+                eligible_zones=['unit.tests.'],
+                eligible_targets=['dump'],
+            )
+            self.assertEqual(22, tc)
+            # can't do partial syncs that include arpa zones
+            with self.assertRaises(ManagerException) as ctx:
+                manager.sync(dry_run=False, eligible_targets=['dump'])
+            self.assertEqual(
+                'eligible_targets is incompatible with auto_arpa',
+                str(ctx.exception),
+            )
+
             # full sync with arpa is fine, 2 extra records from it
             tc = manager.sync(dry_run=False)
             self.assertEqual(26, tc)