Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZoneFileProvider, read and write support added #42

Merged
merged 7 commits into from
Sep 13, 2023
Merged

Conversation

ross
Copy link
Contributor

@ross ross commented Sep 8, 2023

Support for writing zone files out to disk, full read/write provider. Similar to YamlProvider all records are written out each time, i.e. when in target mode the provider assumes the are no existing records.

(env) coho:octodns-bind ross$ cat /tmp/zonefile/unit.tests.
$ORIGIN unit.tests.

@ 3600 IN SOA ns1.unit.tests. webmaster.unit.tests. (
    1694233526 ; Serial
    3600       ; Refresh (1 hour)
    600        ; Retry (10 minutes)
    604800     ; Expire (1 week)
    3600       ; NXDOMAIN ttl (1 hour)
)

@                 300 IN A        1.2.3.4
                  300 IN A        1.2.3.5
                 3600 IN NS       ns1.unit.tests.
                 3600 IN NS       ns2.unit.tests.
                  600 IN SSHFP    1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49
                  600 IN SSHFP    1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73
_25._tcp.mx1     3600 IN TLSA     3 1 1 8a9a70596e869bed72c69d97a8895dfa
_25._tcp.mx2     3600 IN TLSA     3 1 1 c164b2c3f36d068d42a6138e446152f568615f28c69bd96a73e354cac88ed00c
_imap._tcp        600 IN SRV      0 0 0 .
_pop3._tcp        600 IN SRV      0 0 0 .
_srv._tcp         600 IN SRV      10 20 30 foo-1.unit.tests.
                  600 IN SRV      10 20 30 foo-2.unit.tests.
aaaa              600 IN AAAA     2601:644:500:e210:62f8:1dff:feb8:947a
caa              1800 IN CAA      0 iodef "mailto:[email protected]"
                 1800 IN CAA      0 issue "ca.unit.tests"
cname             300 IN CNAME    unit.tests.
included          300 IN CNAME    unit.tests.
loc               300 IN LOC      31 58 52.1 S 115 49 11.7 E 20.0m 10.0m 10.0m 2.0m
                  300 IN LOC      53 14 10.0 N 2 18 26.0 W 20.0m 10.0m 1000.0m 2.0m
mx                300 IN MX       10 smtp-4.unit.tests.
                  300 IN MX       20 smtp-2.unit.tests.
                  300 IN MX       30 smtp-3.unit.tests.
                  300 IN MX       40 smtp-1.unit.tests.
txt               600 IN TXT      "Bah bah black sheep"
                  600 IN TXT      "have you any wool."
                  600 IN TXT      "v=DKIM1\;k=rsa\;s=email\;h=sha256\;p=A/kinda+of/long/string+with+numb3rs"
under            3600 IN NS       ns1.unit.tests.
                 3600 IN NS       ns2.unit.tests.
www               300 IN A        2.2.3.6
wwww.sub          300 IN A        2.2.3.6

/cc Fixes #40 @kabenin
/cc @yzguy for 👀

@ross ross requested a review from yzguy September 8, 2023 21:31
@ross ross self-assigned this Sep 8, 2023
@yzguy
Copy link
Contributor

yzguy commented Sep 12, 2023

Ran it through the paces, took a couple of my zone files and put them through it, loaded them into bind no problem. Serial incremented/changed as expected

Only thing that I hit, which I believe might be because it's written every time is that the Apex NS records cause it to throw the octodns.provider.plan.RootNsChange: Root NS record change, force required error every time

octodns_bind/__init__.py Outdated Show resolved Hide resolved
@ross
Copy link
Contributor Author

ross commented Sep 12, 2023

Only thing that I hit, which I believe might be because it's written every time is that the Apex NS records cause it to throw the octodns.provider.plan.RootNsChange: Root NS record change, force required error every time

Yeah. It's a bit annoying. Maybe we could look at ignoring the RootNSChange exception when creating a zone from scratch. Outside the scope of this PR, but will stick a TODO on the list for octoDNS core about it.

@ross
Copy link
Contributor Author

ross commented Sep 12, 2023

Only thing that I hit, which I believe might be because it's written every time is that the Apex NS records cause it to throw the octodns.provider.plan.RootNsChange: Root NS record change, force required error every time

Yeah. It's a bit annoying. Maybe we could look at ignoring the RootNSChange exception when creating a zone from scratch. Outside the scope of this PR, but will stick a TODO on the list for octoDNS core about it.

Doh. Looks like that's already the case https://github.com/octodns/octodns/blob/ccb4f97a2f857060e654587b35c7d496c4a40cf6/octodns/provider/plan.py#L117-L124.

The problem is with this provider in that it doesn't correctly indicate whether the zone exists or not during populate. Will look into that.

octodns_bind/__init__.py Outdated Show resolved Hide resolved
@ross
Copy link
Contributor Author

ross commented Sep 12, 2023

The problem is with this provider in that it doesn't correctly indicate whether the zone exists or not during populate. Will look into that.

This has been addressed. ZoneFileProvider looks for the existence of the zone file. AxfrPopulate assumes the zone exists since it can't create it anyway.

@yzguy
Copy link
Contributor

yzguy commented Sep 13, 2023

Looks to me! Great new functionality I'll definitely be using for home

@ross ross merged commit 3115464 into main Sep 13, 2023
5 checks passed
@ross ross deleted the zone-file-provider branch September 13, 2023 01:04
@kabenin
Copy link
Contributor

kabenin commented Sep 13, 2023

Thank you very much, @ross!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BIND provider as target?
3 participants