octodns / octodns-bind

RFC compliant (Bind9) provider for octoDNS
MIT License
7 stars 12 forks source link

TXT records lose space symbol #38

Closed hellbot closed 1 year ago

hellbot commented 1 year ago

If you attempt to update TXT record it won't fail but will split txt line into parts by spaces. And this leads to lost all spaces in txt records. @ IN TXT "A custom zone!" will became @ IN TXT "Acustomzone!" and octodns will always attempt to update it

* ns2 (Rfc2136Provider)
*   Update
*     <TxtRecord TXT 3600, unit.tests., ['Acustomzone!']> ->
*     <TxtRecord TXT 3600, unit.tests., ['A custom zone!']> ()

This happens due to dnspython rdata tokenizer (which parses a string) it counts spaces by delimiters and to avoid it you have to surround your rdata string with quotes. But there is no way to set quotes for txt record in octodns configuration files (I've tried yaml & zones) - it always strips them.

hellbot commented 1 year ago

A dirty solution is to custom process TXT records in octodns_bind:_apply

    def _apply(self, plan):
        desired = plan.desired
        auth_params = self._auth_params()
        update = DnsUpdate(desired.name, **auth_params)

        for change in plan.changes:
            record = change.record
            name, ttl, _type, rdatas = record.rrs

            # Special handling for TXT records. 255 is a maximum length of one TEXT record chunk
            if _type == 'TXT':
                for index, value in enumerate(rdatas):
                    chunks = []
                    for chunk in [value[i:i + 255] for i in range(0, len(value), 255)]:
                        chunks.append(f'"{chunk}"')
                    rdatas[index] = " ".join(chunks)

            if isinstance(change, Create):
                update.add(name, ttl, _type, *rdatas)
            elif isinstance(change, Update):
                update.replace(name, ttl, _type, *rdatas)
            else:  # isinstance(change, Delete):
                update.delete(name, _type, *rdatas)

        r: dns.message.Message = dns.query.tcp(
            update, self.host, port=self.port, timeout=self.timeout
        )
        if r.rcode() != dns.rcode.NOERROR:
            raise Rfc2136ProviderUpdateFailed(dns.rcode.to_text(r.rcode()))

        self.log.debug(
            '_apply: zone=%s, num_records=%d', name, len(plan.changes)
        )

        return True

This also will deal with octodns/octodns#1088 but I'm unsure it is a proper way to do it

ross commented 1 year ago

This happens due to dnspython rdata tokenizer (which parses a string) it counts spaces by delimiters and to avoid it you have to surround your rdata string with quotes.

Will have to dig into this. My first though is that something would likely need tweaking as things go in/out of dns python to get the desired behavior.

ross commented 1 year ago

There's a good chance https://github.com/octodns/octodns/pull/1089 will address this in addition to the stuff I was working on when I created the PR. Will have to test...

ross commented 1 year ago
spaces:
  type: TXT
  value: A value with spaces

with latest octoDNS release, 1.2.1

2023-10-15T17:46:19  [4402265600] INFO  Manager __init__: config_file=config/dev.yaml, (octoDNS 1.2.1)
...
********************************************************************************
* exxampled.com.
********************************************************************************
* rfc2136 (Rfc2136Provider)
*   Update
*     <TxtRecord TXT 3600, spaces.exxampled.com., ['Avaluewithspaces']> ->
*     <TxtRecord TXT 3600, spaces.exxampled.com., ['A value with spaces']> (config)
*   Summary: Creates=0, Updates=1, Deletes=0, Existing Records=9
********************************************************************************

With octodns HEAD:

2023-10-15T17:47:37  [4784848384] INFO  Manager __init__: config_file=config/dev.yaml, (octoDNS 1.2.1+ded53023)
...
********************************************************************************
No changes were planned
********************************************************************************

So We're good here once that lands.

ross commented 1 year ago

/cc Fixed by https://github.com/octodns/octodns/pull/1089

paulgear commented 1 year ago

Hi @ross,

Thanks so much for your work on this.

I'm still seeing the space deletion behaviour after updating to 1.2.1:

$ pip list|grep octodns
octodns                1.2.1
octodns-bind           0.0.5
octodns-cloudflare     0.0.3
octodns-digitalocean   0.0.2
$ octodns-sync --config-file config/main.yml --doit
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: config_file=config/main.yml, (octoDNS 1.2.1)
2023-11-05T14:57:50  [140298585202688] INFO  Manager _config_executor: max_workers=2
2023-11-05T14:57:50  [140298585202688] INFO  Manager _config_include_meta: include_meta=False
2023-11-05T14:57:50  [140298585202688] INFO  Manager _config_auto_arpa: auto_arpa=False
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: global_processors=[]
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: global_post_processors=[]
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=cloudflare (octodns_cloudflare 0.0.3)
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=digitalocean (octodns_digitalocean 0.0.2)
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=publicdns (octodns_bind 0.0.5)
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=writeyaml (octodns.provider.yaml 1.2.1)
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=yaml (octodns.provider.yaml 1.2.1)
2023-11-05T14:57:50  [140298585202688] INFO  Manager __init__: provider=zonefile (octodns_bind 0.0.5)
2023-11-05T14:57:50  [140298585202688] INFO  Manager sync: eligible_zones=[], eligible_targets=[], dry_run=False, force=False, plan_output_fh=<stdout>
2023-11-05T14:57:50  [140298585202688] INFO  Manager sync:   zone=libertysys.com.au.
2023-11-05T14:57:50  [140298585202688] INFO  Manager sync:   sources=['yaml']
2023-11-05T14:57:50  [140298585202688] INFO  Manager sync:   targets=['digitalocean', 'publicdns']
2023-11-05T14:57:50  [140298550244928] INFO  YamlProvider[yaml] populate:   found 55 records, exists=False
2023-11-05T14:57:50  [140298550244928] INFO  DigitalOceanProvider[digitalocean] plan: desired=libertysys.com.au.
2023-11-05T14:57:52  [140298550244928] WARNING DigitalOceanProvider[digitalocean] populate: skipping unsupported SOA record
2023-11-05T14:57:52  [140298550244928] INFO  DigitalOceanProvider[digitalocean] populate:   found 55 records, exists=True
2023-11-05T14:57:52  [140298550244928] INFO  DigitalOceanProvider[digitalocean] plan:   No changes
2023-11-05T14:57:52  [140298550244928] INFO  Rfc2136Provider[publicdns] plan: desired=libertysys.com.au.
2023-11-05T14:57:52  [140298550244928] INFO  Rfc2136Provider[publicdns] populate:   found 55 records
2023-11-05T14:57:52  [140298550244928] INFO  Rfc2136Provider[publicdns] plan:   Creates=0, Updates=8, Deletes=0, Existing Records=55
2023-11-05T14:57:52  [140298585202688] INFO  Plan 
********************************************************************************
* libertysys.com.au.
********************************************************************************
* publicdns (Rfc2136Provider)
*   Update
*     <TxtRecord TXT 36000, libertysys.com.au., ['MS=ms25174473', 'google-site-verification=nZuh78aD9jRcXp35KjuOYPO9ZlSOUGzujXhzvHBBT9k', 'have-i-been-pwned-verification=0cc6128613eb8cb2b2a24b8f72e4d9ef', 'v=spf1mxip4:150.101.178.79-all']> ->
*     <TxtRecord TXT 36000, libertysys.com.au., ['MS=ms25174473', 'google-site-verification=nZuh78aD9jRcXp35KjuOYPO9ZlSOUGzujXhzvHBBT9k', 'have-i-been-pwned-verification=0cc6128613eb8cb2b2a24b8f72e4d9ef', 'v=spf1 mx ip4:150.101.178.79 -all']> (yaml)
*   Update
*     <TxtRecord TXT 36000, 2016._domainkey.libertysys.com.au., ['v=DKIM1\;h=sha256\;k=rsa\;p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDOZm88HcPmUkLeIDtCRzw0I1CjX6DlYq4Hdsx7nAy1zCZFJIpHNcxNOR6StwJ5fIe1zxE2wddqwfwZKrusHYDXUK98duRTC51PN4zybLYsSgR6N91bQjAEqO1PXv9zoBKkDn1Wb9yAWUmIpjoPCLPppuCY60F1WrkH/91df36quwIDAQAB']> ->
*     <TxtRecord TXT 36000, 2016._domainkey.libertysys.com.au., ['v=DKIM1\; h=sha256\; k=rsa\; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDOZm88HcPmUkLeIDtCRzw0I1CjX6DlYq4Hdsx7nAy1zCZFJIpHNcxNOR6StwJ5fIe1zxE2wddqwfwZKrusHYDXUK98duRTC51PN4zybLYsSgR6N91bQjAEqO1PXv9zoBKkDn1Wb9yAWUmIpjoPCLPppuCY60F1WrkH/91df36quwIDAQAB']> (yaml)
*   Update
*     <TxtRecord TXT 36000, _dmarc.libertysys.com.au., ['v=DMARC1\;p=quarantine\;rua=mailto:km8l45t9@ag.au.dmarcian.com\;ruf=mailto:km8l45t9@fr.au.dmarcian.com\;fo=1\;sp=quarantine\;']> ->
*     <TxtRecord TXT 36000, _dmarc.libertysys.com.au., ['v=DMARC1\; p=quarantine\; rua=mailto:km8l45t9@ag.au.dmarcian.com\; ruf=mailto:km8l45t9@fr.au.dmarcian.com\; fo=1\; sp=quarantine\;']> (yaml)
...
*   Summary: Creates=0, Updates=8, Deletes=0, Existing Records=55
********************************************************************************
2023-11-05T14:57:52  [140298585202688] INFO  Rfc2136Provider[publicdns] apply: making 8 changes to libertysys.com.au.
2023-11-05T14:57:52  [140298585202688] INFO  Manager sync:   8 total changes
$ octodns-sync --config-file config/main.yml
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: config_file=config/main.yml, (octoDNS 1.2.1)
2023-11-05T14:59:17  [139854406455296] INFO  Manager _config_executor: max_workers=2
2023-11-05T14:59:17  [139854406455296] INFO  Manager _config_include_meta: include_meta=False
2023-11-05T14:59:17  [139854406455296] INFO  Manager _config_auto_arpa: auto_arpa=False
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: global_processors=[]
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: global_post_processors=[]
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=cloudflare (octodns_cloudflare 0.0.3)
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=digitalocean (octodns_digitalocean 0.0.2)
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=publicdns (octodns_bind 0.0.5)
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=writeyaml (octodns.provider.yaml 1.2.1)
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=yaml (octodns.provider.yaml 1.2.1)
2023-11-05T14:59:17  [139854406455296] INFO  Manager __init__: provider=zonefile (octodns_bind 0.0.5)
2023-11-05T14:59:17  [139854406455296] INFO  Manager sync: eligible_zones=[], eligible_targets=[], dry_run=True, force=False, plan_output_fh=<stdout>
2023-11-05T14:59:17  [139854406455296] INFO  Manager sync:   zone=libertysys.com.au.
2023-11-05T14:59:17  [139854406455296] INFO  Manager sync:   sources=['yaml']
2023-11-05T14:59:17  [139854406455296] INFO  Manager sync:   targets=['digitalocean', 'publicdns']
2023-11-05T14:59:17  [139854372402752] INFO  YamlProvider[yaml] populate:   found 55 records, exists=False
2023-11-05T14:59:17  [139854372402752] INFO  DigitalOceanProvider[digitalocean] plan: desired=libertysys.com.au.
2023-11-05T14:59:20  [139854372402752] WARNING DigitalOceanProvider[digitalocean] populate: skipping unsupported SOA record
2023-11-05T14:59:20  [139854372402752] INFO  DigitalOceanProvider[digitalocean] populate:   found 55 records, exists=True
2023-11-05T14:59:20  [139854372402752] INFO  DigitalOceanProvider[digitalocean] plan:   No changes
2023-11-05T14:59:20  [139854372402752] INFO  Rfc2136Provider[publicdns] plan: desired=libertysys.com.au.
2023-11-05T14:59:20  [139854372402752] INFO  Rfc2136Provider[publicdns] populate:   found 55 records
2023-11-05T14:59:20  [139854372402752] INFO  Rfc2136Provider[publicdns] plan:   Creates=0, Updates=8, Deletes=0, Existing Records=55
2023-11-05T14:59:20  [139854406455296] INFO  Plan 
********************************************************************************
* libertysys.com.au.
********************************************************************************
* publicdns (Rfc2136Provider)
*   Update
*     <TxtRecord TXT 36000, libertysys.com.au., ['MS=ms25174473', 'google-site-verification=nZuh78aD9jRcXp35KjuOYPO9ZlSOUGzujXhzvHBBT9k', 'have-i-been-pwned-verification=0cc6128613eb8cb2b2a24b8f72e4d9ef', 'v=spf1mxip4:150.101.178.79-all']> ->
*     <TxtRecord TXT 36000, libertysys.com.au., ['MS=ms25174473', 'google-site-verification=nZuh78aD9jRcXp35KjuOYPO9ZlSOUGzujXhzvHBBT9k', 'have-i-been-pwned-verification=0cc6128613eb8cb2b2a24b8f72e4d9ef', 'v=spf1 mx ip4:150.101.178.79 -all']> (yaml)
*   Update
*     <TxtRecord TXT 36000, 2016._domainkey.libertysys.com.au., ['v=DKIM1\;h=sha256\;k=rsa\;p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDOZm88HcPmUkLeIDtCRzw0I1CjX6DlYq4Hdsx7nAy1zCZFJIpHNcxNOR6StwJ5fIe1zxE2wddqwfwZKrusHYDXUK98duRTC51PN4zybLYsSgR6N91bQjAEqO1PXv9zoBKkDn1Wb9yAWUmIpjoPCLPppuCY60F1WrkH/91df36quwIDAQAB']> ->
*     <TxtRecord TXT 36000, 2016._domainkey.libertysys.com.au., ['v=DKIM1\; h=sha256\; k=rsa\; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDOZm88HcPmUkLeIDtCRzw0I1CjX6DlYq4Hdsx7nAy1zCZFJIpHNcxNOR6StwJ5fIe1zxE2wddqwfwZKrusHYDXUK98duRTC51PN4zybLYsSgR6N91bQjAEqO1PXv9zoBKkDn1Wb9yAWUmIpjoPCLPppuCY60F1WrkH/91df36quwIDAQAB']> (yaml)
*   Update
*     <TxtRecord TXT 36000, _dmarc.libertysys.com.au., ['v=DMARC1\;p=quarantine\;rua=mailto:km8l45t9@ag.au.dmarcian.com\;ruf=mailto:km8l45t9@fr.au.dmarcian.com\;fo=1\;sp=quarantine\;']> ->
*     <TxtRecord TXT 36000, _dmarc.libertysys.com.au., ['v=DMARC1\; p=quarantine\; rua=mailto:km8l45t9@ag.au.dmarcian.com\; ruf=mailto:km8l45t9@fr.au.dmarcian.com\; fo=1\; sp=quarantine\;']> (yaml)
...
*   Summary: Creates=0, Updates=8, Deletes=0, Existing Records=55
********************************************************************************

Is there some other dependency I need to upgrade to get the correct behaviour?

ross commented 1 year ago

Looks like the TXT fix hasn't made it into a release yet, it'll land in 1.3.0 🔜