search

octodns / octodns-cloudflare

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

Provider fails if a TXT record has no content #73

Closed ThisIsntTheWay closed 10 months ago

ThisIsntTheWay commented 10 months ago

I, perhaps erroneously, declared a TXT record as such:

---
'':
  - type: TXT
    values: "v=spf1 mx ~all"
#        ^

Which resulted in the following records being created:

...
*   Update
*     <TxtRecord TXT 1800, <zone>., ['v=spf1 mx ~all']> ->
*     <TxtRecord TXT 600, <zone>., [' ', ' ', '1', '=', 'a', 'f', 'l', 'l', 'm', 'p', 's', 'v', 'x', '~']> (config)
#                                   ^^^  ^^^
...

The consequence, quite the clutter in the zone:

$ dig my-zone.ext TXT
...
;; ANSWER SECTION:
my-zone.ext.             600     IN      TXT     "1"
my-zone.ext.             600     IN      TXT     "="
my-zone.ext.             600     IN      TXT     "a"
my-zone.ext.             600     IN      TXT     "f"
my-zone.ext.             600     IN      TXT     "l"
my-zone.ext.             600     IN      TXT     "m"
my-zone.ext.             600     IN      TXT     "p"
my-zone.ext.             600     IN      TXT     "s"
my-zone.ext.             600     IN      TXT     "v"
my-zone.ext.             600     IN      TXT     "x"
my-zone.ext.             600     IN      TXT     "~"

# Not visible in the query is the record that has NO content.
# > Said record could only be seen in the CloudFlare dashboard.
# (Although OctoDNS created two of those empty records, there was only one)

After correcting my mistake in the YAML (values -> value), consecutive runs failed due to:

2024-01-10T20:16:27  [548384288832] INFO  CloudflareProvider[cloudflare] plan: desired=<zone>.
Traceback (most recent call last):
  File "/home/pi/.local/bin/octodns-sync", line 8, in <module>
    sys.exit(main())
  File "/home/pi/.local/lib/python3.9/site-packages/octodns/cmds/sync.py", line 57, in main
    manager.sync(
  File "/home/pi/.local/lib/python3.9/site-packages/octodns/manager.py", line 697, in sync
    ps, d = future.result()
  File "/home/pi/.local/lib/python3.9/site-packages/octodns/manager.py", line 60, in result
    return self.func(*self.args, **self.kwargs)
  File "/home/pi/.local/lib/python3.9/site-packages/octodns/manager.py", line 460, in _populate_and_plan
    plan = target.plan(zone, processors=processors)
  File "/home/pi/.local/lib/python3.9/site-packages/octodns/provider/base.py", line 226, in plan
    exists = self.populate(existing, target=True, lenient=True)
  File "/home/pi/.local/lib/python3.9/site-packages/octodns_cloudflare/__init__.py", line 487, in populate
    record = self._record_for(
  File "/home/pi/.local/lib/python3.9/site-packages/octodns_cloudflare/__init__.py", line 432, in _record_for
    data = data_for(_type, records)
  File "/home/pi/.local/lib/python3.9/site-packages/octodns_cloudflare/__init__.py", line 217, in _data_for_TXT
    'values': [r['content'].replace(';', '\\;') for r in records],
  File "/home/pi/.local/lib/python3.9/site-packages/octodns_cloudflare/__init__.py", line 217, in <listcomp>
    'values': [r['content'].replace(';', '\\;') for r in records],
KeyError: 'content'

I managed to fix this by removing the empty record in the dashboard:
image

This is admittedly an edge case coupled with user error, but perhaps still worth to look at.

ross commented 10 months ago

Interesting edge case. I think there's a couple potential improvememts/fixes here.

CF needs to deal with the empty value as that's apparently legal.

It might also be possible to cleanly detect when values is a str and it shouldn't be and throw an error.

Will throw these on the TODO list. In the interim I'm guessing manually cleaning out the value is the best work around.

ross commented 10 months ago

https://github.com/octodns/octodns-cloudflare/pull/75 fixes the CloudflareProvider bug

and https://github.com/octodns/octodns/pull/1131 adds a validation for what lead to hitting the bug