octodns / octodns-cloudflare

Cloudflare DNS provider for octoDNS
MIT License
24 stars 17 forks source link

Ensure unsupported record types are handled correctly #95

Closed kkzo closed 3 months ago

kkzo commented 3 months ago

During the apply update phase, all zone records are iterated and updated as required, regardless of whether the record type is supported by the provider or not. An AttributeError exception is triggered if any unsupported type exists within the zone, regardless of the actual record type being updated. This problem appears to only trigger for zones with existing records that have been onboarded onto OctoDNS (eg: DS records existing within the zone). Attempts to use the octodns.processor.filter.TypeRejectlistFilter processor to ignore the offending type was of no help due to the way the methods are named (_data_for_{_type}) within this provider.

   Traceback (most recent call last):
     ...
     File "./venv/lib/python3.12/site-packages/octodns_cloudflare/__init__.py", line 911, in _apply_Update
       r = self._record_for(zone, name, record['type'], [record], True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     File "./venv/lib/python3.12/site-packages/octodns_cloudflare/__init__.py", line 435, in _record_for
       data_for = getattr(self, f'_data_for_{_type}')
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   AttributeError: 'CloudflareProvider' object has no attribute '_data_for_DS'. Did you mean: '_data_for_NS'?
   exit status 1

This change ensures that only supported record types are evaluated. Unsupported records are logged as a warning and skipped.

kkzo commented 3 months ago

Complete log:

2024-05-17T16:03:28  [8431942336] INFO  Manager __init__: config_file=example.yaml, (octoDNS 1.6.1)
2024-05-17T16:03:28  [8431942336] INFO  Manager _config_executor: max_workers=4
2024-05-17T16:03:28  [8431942336] INFO  Manager _config_include_meta: include_meta=False
2024-05-17T16:03:28  [8431942336] INFO  Manager _config_enable_checksum: enable_checksum=False
2024-05-17T16:03:28  [8431942336] INFO  Manager _config_auto_arpa: auto_arpa=False
2024-05-17T16:03:28  [8431942336] INFO  Manager __init__: global_processors=[]
2024-05-17T16:03:28  [8431942336] INFO  Manager __init__: global_post_processors=[]
2024-05-17T16:03:28  [8431942336] INFO  Manager __init__: provider=cloudflare (octodns_cloudflare 0.0.5)
2024-05-17T16:03:28  [8431942336] INFO  Manager __init__: provider=staging (octodns.provider.yaml 1.6.1)
2024-05-17T16:03:28  [8431942336] INFO  Manager sync: eligible_zones=[], eligible_targets=[], dry_run=False, force=False, plan_output_fh=<stdout>, checksum=None
2024-05-17T16:03:28  [8431942336] INFO  Manager sync:   zone=an.example.zone.
2024-05-17T16:03:28  [8431942336] INFO  Manager sync:     sources=['staging']
2024-05-17T16:03:28  [8431942336] INFO  Manager sync:     processors=[]
2024-05-17T16:03:28  [8431942336] INFO  Manager sync:     targets=['cloudflare']
2024-05-17T16:03:28  [6119190528] INFO  YamlProvider[staging] populate:   found 9 records, exists=False
2024-05-17T16:03:28  [6119190528] INFO  CloudflareProvider[cloudflare] plan: desired=an.example.zone.
2024-05-17T16:03:30  [6119190528] INFO  CloudflareProvider[cloudflare] populate:   found 9 records, exists=True
2024-05-17T16:03:30  [6119190528] INFO  CloudflareProvider[cloudflare] plan:   Creates=0, Updates=1, Deletes=0, Existing Records=9
2024-05-17T16:03:30  [8431942336] INFO  Plan
********************************************************************************
* an.example.zone.
********************************************************************************
* cloudflare (CloudflareProvider)
*   Update
*     <ARecord A 3600, test8.an.example.zone., ['1.1.1.1']> ->
*     <ARecord A 3600, test8.an.example.zone., ['1.0.0.1']> (staging)
*   Summary: Creates=0, Updates=1, Deletes=0, Existing Records=9
********************************************************************************

2024-05-17T16:03:30  [8431942336] INFO  CloudflareProvider[cloudflare] apply: making 1 changes to an.example.zone.
Traceback (most recent call last):
  File "./venv/bin/octodns-sync", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "./venv/lib/python3.12/site-packages/octodns/cmds/sync.py", line 62, in main
    manager.sync(
  File "./venv/lib/python3.12/site-packages/octodns/manager.py", line 857, in sync
    total_changes += target.apply(plan)
                     ^^^^^^^^^^^^^^^^^^
  File "./venv/lib/python3.12/site-packages/octodns/provider/base.py", line 298, in apply
    self._apply(plan)
  File "./venv/lib/python3.12/site-packages/octodns_cloudflare/__init__.py", line 1071, in _apply
    getattr(self, f'_apply_{class_name}')(change)
  File "./venv/lib/python3.12/site-packages/octodns_cloudflare/__init__.py", line 911, in _apply_Update
    r = self._record_for(zone, name, record['type'], [record], True)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "./venv/lib/python3.12/site-packages/octodns_cloudflare/__init__.py", line 435, in _record_for
    data_for = getattr(self, f'_data_for_{_type}')
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'CloudflareProvider' object has no attribute '_data_for_DS'. Did you mean: '_data_for_NS'?
exit status 1
ross commented 3 months ago

One to add DS

Supported added in https://github.com/octodns/octodns-cloudflare/pull/96