Merge pull request #1161 from wjgauthier/auto_arpa_priority

Add a priority option to AutoArpa

CHANGELOG.md

@@ -3,6 +3,7 @@
 * Support for specifying per-zone change thresholds, to allow for zones
   where lots of changes are expected frequently to live along side zones
   where little or no churn is expected.
+* AutoArpa gained support for prioritizing values
 
 ## v1.6.1 - 2024-03-17 - Didn't we do this already
 

docs/auto_arpa.md

@@ -19,6 +19,8 @@ manager:
     populate_should_replace: false
     # Explicitly set the TTL of auto-created records, default is 3600s, 1hr
     ttl: 1800
+    # Set how many PTR records will be created for the same IP, default: 999
+    max_auto_arpa: 1
 ```
 
 Once enabled, a singleton `AutoArpa` instance, `auto-arpa`, will be added to the pool of providers and globally configured to run as the very last global processor so that it will see all records as they will be seen by targets. Further all zones ending with `arpa.` will be held back and processed after all other zones have been completed so that all `A` and `AAAA` records will have been seen prior to planning the `arpa.` zones.

docs/records.md

@@ -94,6 +94,17 @@ octoDNS is fairly strict in terms of standards compliance and is opinionated in
 
 It's best to think of the `lenient` flag as "I know what I'm doing and accept any problems I run across." The main reason being is that some providers may allow the non-compliant setup and others may not. The behavior of the non-compliant records may even vary from one provider to another. Caveat emptor.
 
+#### Record priority for AutoArpa
+When multiple A or AAAA records point to the same IP, it is possible to set an optional priority on each record. The records with the lowest priority will have the highest preference when being processed by AutoArpa. The AutoArpa provider will create PTR records in order of preference, up to a set limit defined by the `max_auto_arpa` option in the provider configuration.
+
+```yaml
+test:
+- type: A
+  value: 1.2.3.4
+  octodns:
+    auto_arpa_priority: 1
+```
+
 #### octodns-dump
 
 If you're trying to import a zone into octoDNS config file using `octodns-dump` which fails due to validation errors you can supply the `--lenient` argument to tell octoDNS that you acknowledge that things aren't lining up with its expectations, but you'd like it to go ahead anyway. This will do its best to populate the zone and dump the results out into an octoDNS zone file and include the non-compliant bits. If you go to use that config file octoDNS will again complain about the validation problems. You can correct them in cases where that makes sense, but if you need to preserve the non-compliant records read on for options.

octodns/processor/arpa.py

@@ -11,17 +11,21 @@ from .base import BaseProcessor
 
 
 class AutoArpa(BaseProcessor):
-    def __init__(self, name, ttl=3600, populate_should_replace=False):
+    def __init__(
+        self, name, ttl=3600, populate_should_replace=False, max_auto_arpa=999
+    ):
         super().__init__(name)
         self.log = getLogger(f'AutoArpa[{name}]')
         self.log.info(
-            '__init__: ttl=%d, populate_should_replace=%s',
+            '__init__: ttl=%d, populate_should_replace=%s, max_auto_arpa=%d',
             ttl,
             populate_should_replace,
+            max_auto_arpa,
         )
         self.ttl = ttl
         self.populate_should_replace = populate_should_replace
-        self._records = defaultdict(set)
+        self.max_auto_arpa = max_auto_arpa
+        self._records = defaultdict(list)
 
     def process_source_zone(self, desired, sources):
         for record in desired.records:
@@ -37,10 +41,29 @@ class AutoArpa(BaseProcessor):
 
                 for ip in ips:
                     ptr = ip_address(ip).reverse_pointer
-                    self._records[f'{ptr}.'].add(record.fqdn)
+                    auto_arpa_priority = record.octodns.get(
+                        'auto_arpa_priority', 999
+                    )
+                    self._records[f'{ptr}.'].append(
+                        (auto_arpa_priority, record.fqdn)
+                    )
 
         return desired
 
+    def _order_and_unique_fqdns(self, fqdns, max_auto_arpa):
+        seen = set()
+        # order the fqdns making a copy so we can reset the list below
+        ordered = sorted(fqdns)
+        fqdns = []
+        for _, fqdn in ordered:
+            if fqdn in seen:
+                continue
+            fqdns.append(fqdn)
+            seen.add(fqdn)
+            if len(seen) >= max_auto_arpa:
+                break
+        return fqdns
+
     def populate(self, zone, target=False, lenient=False):
         self.log.debug(
             'populate: name=%s, target=%s, lenient=%s',
@@ -56,7 +79,8 @@ class AutoArpa(BaseProcessor):
         for arpa, fqdns in self._records.items():
             if arpa.endswith(f'.{zone_name}'):
                 name = arpa[:-n]
-                fqdns = sorted(fqdns)
+                # Note: this takes a list of (priority, fqdn) tuples and returns the ordered and uniqified list of fqdns.
+                fqdns = self._order_and_unique_fqdns(fqdns, self.max_auto_arpa)
                 record = Record.new(
                     zone,
                     name,

tests/test_octodns_processor_arpa.py

@@ -27,7 +27,7 @@ class TestAutoArpa(TestCase):
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
         self.assertEqual(
-            {'4.3.2.1.in-addr.arpa.': {'a.unit.tests.'}}, aa._records
+            {'4.3.2.1.in-addr.arpa.': [(999, 'a.unit.tests.')]}, aa._records
         )
 
         # matching zone
@@ -56,8 +56,8 @@ class TestAutoArpa(TestCase):
         aa.process_source_zone(zone, [])
         self.assertEqual(
             {
-                '4.3.2.1.in-addr.arpa.': {'a.unit.tests.'},
-                '5.3.2.1.in-addr.arpa.': {'a.unit.tests.'},
+                '4.3.2.1.in-addr.arpa.': [(999, 'a.unit.tests.')],
+                '5.3.2.1.in-addr.arpa.': [(999, 'a.unit.tests.')],
             },
             aa._records,
         )
@@ -81,7 +81,7 @@ class TestAutoArpa(TestCase):
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
         ip6_arpa = '2.0.0.0.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.c.0.0.0.f.f.0.0.ip6.arpa.'
-        self.assertEqual({ip6_arpa: {'aaaa.unit.tests.'}}, aa._records)
+        self.assertEqual({ip6_arpa: [(999, 'aaaa.unit.tests.')]}, aa._records)
 
         # matching zone
         arpa = Zone('c.0.0.0.f.f.0.0.ip6.arpa.', [])
@@ -117,13 +117,13 @@ class TestAutoArpa(TestCase):
         aa.process_source_zone(zone, [])
         self.assertEqual(
             {
-                '1.1.1.1.in-addr.arpa.': {'geo.unit.tests.'},
-                '2.2.2.2.in-addr.arpa.': {'geo.unit.tests.'},
-                '3.3.3.3.in-addr.arpa.': {'geo.unit.tests.'},
-                '4.4.4.4.in-addr.arpa.': {'geo.unit.tests.'},
-                '5.5.5.5.in-addr.arpa.': {'geo.unit.tests.'},
-                '4.3.2.1.in-addr.arpa.': {'geo.unit.tests.'},
-                '5.3.2.1.in-addr.arpa.': {'geo.unit.tests.'},
+                '4.3.2.1.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '5.3.2.1.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '1.1.1.1.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '2.2.2.2.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '3.3.3.3.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '4.4.4.4.in-addr.arpa.': [(999, 'geo.unit.tests.')],
+                '5.5.5.5.in-addr.arpa.': [(999, 'geo.unit.tests.')],
             },
             aa._records,
         )
@@ -165,16 +165,18 @@ class TestAutoArpa(TestCase):
         zone.add_record(record)
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
-        self.assertEqual(
-            {
-                '3.3.3.3.in-addr.arpa.': {'dynamic.unit.tests.'},
-                '4.4.4.4.in-addr.arpa.': {'dynamic.unit.tests.'},
-                '5.5.5.5.in-addr.arpa.': {'dynamic.unit.tests.'},
-                '4.3.2.1.in-addr.arpa.': {'dynamic.unit.tests.'},
-                '5.3.2.1.in-addr.arpa.': {'dynamic.unit.tests.'},
-            },
-            aa._records,
-        )
+        zones = [
+            '4.3.2.1.in-addr.arpa.',
+            '5.3.2.1.in-addr.arpa.',
+            '3.3.3.3.in-addr.arpa.',
+            '4.4.4.4.in-addr.arpa.',
+            '5.5.5.5.in-addr.arpa.',
+        ]
+        for zone in zones:
+            unique_values = aa._order_and_unique_fqdns(
+                aa._records[f'{zone}'], 999
+            )
+            self.assertEqual([('dynamic.unit.tests.')], unique_values)
 
     def test_multiple_names(self):
         zone = Zone('unit.tests.', [])
@@ -188,9 +190,15 @@ class TestAutoArpa(TestCase):
         zone.add_record(record2)
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
+        sorted_records = sorted(aa._records['4.3.2.1.in-addr.arpa.'])
         self.assertEqual(
-            {'4.3.2.1.in-addr.arpa.': {'a1.unit.tests.', 'a2.unit.tests.'}},
-            aa._records,
+            {
+                '4.3.2.1.in-addr.arpa.': [
+                    (999, 'a1.unit.tests.'),
+                    (999, 'a2.unit.tests.'),
+                ]
+            },
+            {'4.3.2.1.in-addr.arpa.': sorted_records},
         )
 
         # matching zone
@@ -211,7 +219,7 @@ class TestAutoArpa(TestCase):
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
         self.assertEqual(
-            {'4.3.20.10.in-addr.arpa.': {'a.unit.tests.'}}, aa._records
+            {'4.3.20.10.in-addr.arpa.': [(999, 'a.unit.tests.')]}, aa._records
         )
 
         # matching zone
@@ -251,7 +259,7 @@ class TestAutoArpa(TestCase):
         aa = AutoArpa('auto-arpa')
         aa.process_source_zone(zone, [])
         self.assertEqual(
-            {'4.3.2.1.in-addr.arpa.': {'a with spaces.unit.tests.'}},
+            {'4.3.2.1.in-addr.arpa.': [(999, 'a with spaces.unit.tests.')]},
             aa._records,
         )
 
@@ -263,3 +271,47 @@ class TestAutoArpa(TestCase):
         self.assertEqual('4.3.2.1.in-addr.arpa.', ptr.fqdn)
         self.assertEqual(record.fqdn, ptr.value)
         self.assertEqual(3600, ptr.ttl)
+
+    def test_arpa_priority(self):
+        aa = AutoArpa('auto-arpa')
+
+        duplicate_values = [(999, 'a.unit.tests.'), (1, 'a.unit.tests.')]
+        self.assertEqual(
+            ['a.unit.tests.'],
+            aa._order_and_unique_fqdns(duplicate_values, max_auto_arpa=999),
+        )
+
+        duplicate_values = [
+            (50, 'd.unit.tests.'),
+            (999, 'dup.unit.tests.'),
+            (3, 'a.unit.tests.'),
+            (1, 'dup.unit.tests.'),
+            (2, 'c.unit.tests.'),
+        ]
+        self.assertEqual(
+            [
+                'dup.unit.tests.',
+                'c.unit.tests.',
+                'a.unit.tests.',
+                'd.unit.tests.',
+            ],
+            aa._order_and_unique_fqdns(duplicate_values, max_auto_arpa=999),
+        )
+
+        duplicate_values_2 = [(999, 'a.unit.tests.'), (999, 'a.unit.tests.')]
+        self.assertEqual(
+            ['a.unit.tests.'],
+            aa._order_and_unique_fqdns(duplicate_values_2, max_auto_arpa=999),
+        )
+
+        ordered_values = [(999, 'a.unit.tests.'), (1, 'b.unit.tests.')]
+        self.assertEqual(
+            ['b.unit.tests.', 'a.unit.tests.'],
+            aa._order_and_unique_fqdns(ordered_values, max_auto_arpa=999),
+        )
+
+        max_one_value = [(999, 'a.unit.tests.'), (1, 'b.unit.tests.')]
+        self.assertEqual(
+            ['b.unit.tests.'],
+            aa._order_and_unique_fqdns(max_one_value, max_auto_arpa=1),
+        )