Add support for arbitrary record types

[benlangfeld]

No description provided.

[benlangfeld]

It's worth noting that this changes the format of the config file used to store zone data, nesting records under a key in the outer hash for each type of record. There are potentially other ways to store this, and I'm open to suggestions if anyone has a particular preference.

As it is, this breaks compatibility with the existing zone format, and these files will need to be deleted and regenerated. Again, I'm open to suggestions on how to manage this more smoothly.

[phinze]

Wow this looks amazing! ❗

I've been hoping to do "proper" record type support for awhile. I'll give this a review tomorrow and get back to you - thanks! 👊

[benlangfeld]

I'm looking into the possibility of revising the store to use BIND zone files to clean this up. Not sure how that'll go yet.

[phinze]

Yeah I'm definitely all for investigating a switch to a standard config format like BIND's

[benlangfeld]

See aeden/dns-zonefile#3

[benlangfeld]

So after a quick shot at aeden/dns-zonefile#3, I've determined that using BIND zonefiles will require more effort than I'm able to commit to this.

Looks like this is where I'm going to let this sit for now.

[benlangfeld]

On further thought, there might be a way to modify some of the assumptions Landrush currently makes in order to support storage as BIND zonefiles, without negatively impacting the overall goal of the project.

Currently, the read and write operations of the storage backend are intrinsically linked. Implementing this exact behaviour on top of BIND zonefiles is difficult because there is not an easy way to read them in (especially arbitrary contents like comments), manipulate them (remove or overwrite entries) and write them back out again. There are libraries available for reading and writing BIND zonefiles (https://github.com/craigw/dns-zonefile for reading, https://github.com/fuCtor/DNSer for writing) but combining these in the same way as we currently use JSON files requires a lot more work.

The part that requires this complex linkage is the ability to overwrite rules and remove rules individually. Perhaps I'm missing something, but this seems somewhat unnecessary. If we were instead to assign zone files one to one against Vagrantfiles, and simply to write the full record set from the Vagrantfile on every invocation, we could do away with this and produce a simpler storage backend that can share components with other projects (the serving of BIND zones via RubyDNS could be extracted as a project of its own).

Thoughts, @phinze?

[phinze]

Interesting!

I spent yesterday poking at the various DNS libraries. I got a small way into writing string/output functionality into https://github.com/craigw/dns-zonefile - but there's much more work left.

If we were instead to assign zone files one to one against Vagrantfiles, and simply to write the full record set from the Vagrantfile on every invocation, we could do away with this and produce a simpler storage backend that can share components with other projects (the serving of BIND zones via RubyDNS could be extracted as a project of its own).

I think we're thinking in the same direction, but I'm not fully clear on what you're suggesting here.

To me it seems like we really have two problems:

1. Landrush's internal concept of DNS records is lacking, and needs to be improved
2. Landrush needs to continue to persist the DNS records for which it is responsible so they can survive daemon restarts

The conclusion I'm tentatively reaching is that (1) is more important than having BIND-style DNS zonefiles for (2).

To that end - this PR is looking more like a good next step for the project. I'm thinking about reviewing it more carefully and pulling it in (possibly with some minor tweaks).

Let me know if your brain is taking you in another direction! 👍 💭

[benlangfeld]

I think what I'm really asking is why does the storage need to be so dynamic such that records for Vagrant boxes are removed when they are shut down? Why not simply maintain the entire zone as long as any box in the Vagrantfile exists, and remove the entire zone once all boxes are destroyed?

[phinze]

Ahh now I'm seeing what that paragraph meant. If we do Zone <-> Vagrantfile, that simplifies our domain model a bit.

Though even if we do that, it doesn't get us BIND-style on-disk storage for free. We'd still need to either extend dns-zonefile or map between dns-zonefile and DNSer.

(the serving of BIND zones via RubyDNS could be extracted as a project of its own).

Sure this now makes more sense.

So really now just the plan of attack is what's left to figure out. What to take on first? I'll think on this a bit and get back to you.

[benlangfeld]

I figure we would replace the parts of landrush which write to storage (the parts that know about the Vagrantfile domain definitions) with writing zonefiles (DNSer), and the DNS server part with something that reads from zonefiles (dns-zonefile). The two parts would not even need to know about each other, with the only interaction being the zonefiles on disk.

I can complete this work if it would be agreeable.

[benlangfeld]

Hell, at that we could even replace the ruby DNS server with BIND itself packaged nicely into vagrant. This would seem to be a nice path to potentially moving (parts of) this plugin into Vagrant core in the future.

[benlangfeld]

@mitchellh Is there any appetite for this plugin (or parts of it) to become a core part of Vagrant in the future?

[mitchellh]

It would certainly be interesting and something we've certainly discussed at HashiCorp, but we don't have any near term plans to integrate this. That can change quickly, though.

[benlangfeld]

@mitchellh Do you have any criteria in mind that this plugin might attempt to satisfy to prepare it for such a thing?

[mitchellh]

I can't say at the moment (because I haven't thought too deeply about it), but I can say its on the right track. :) Functionality and pragmatism trumps all else. Tests and UX we can fix in any merge.

[phinze]

Cool! My primary goal with this is to make it useful as a plugin. If it lands in core someday that's just a bonus. IMHO that should be the thought process for all Vagrant plugins.

Going to get this landed one way or another this weekend. 🙆‍♀️

[benlangfeld]

@phinze I was still waiting for some further comments regarding #62 (comment) if you have any ideas/preferences there. I figure if we can simplify and separate concerns, we'll get ourselves on an even more solid platform.

[phinze]

I was still waiting for some further comments regarding #62 (comment) if you have any ideas/preferences there.

I reread and reprocessed and I am fully with you! I'm planning on picking this up tomorrow and running with it. Landrush will be Vagrant interop, zonefile writing, and daemon pinging. Separate library will be zonefile reading -> rubydns dnsing.

I'll keep posted on progress here! If you want to work on it together you can ping me too. Who knows maybe we can pair! 😀

[benlangfeld]

Awesome! If you wanted to take the former, perhaps I could do the latter.

[phinze]

Sounds like a plan! 📃 Starting a branch now that writes out to zonefiles.

[benlangfeld]

@phinze Have you made any progress on writing zonefiles? Do you have any notes on what paths at which the server component should read them from? Do you envision that the server should reload zones on every request, or should store them in memory until sent SIGHUP or similar?

[phinze]

I did make a bit of progress!

I was looking at https://github.com/boesemar/zonefile which actually has the ability to both read and write zonefile.

The thing I got caught up on was the fact that I think I'd like to significantly modify the Landrush DSL to support the full host of DNS records. So that's where my current WIP is focused.

Do you have any notes on what paths at which the server component should read them from?

Probably some file in ~/.vagrant.d/data/ makes sense. Landrush maintains a dir of state/config in there. But I'm willing to consider alternatives.

Do you have any notes on what paths at which the server component should read them from?

Definitely a signal-based reload makes more sense here.

Will keep you posted once I have some code to show!

(Wish I had more time to work on this.... silly $DAYJOB) 💼 😦

[bexelbie]

@benlangfeld Can you rebase this and let's look at trying to get this committed.

[benlangfeld]

@bexelbie Is this project alive again? If I rebase this is it likely to get merged?

[bexelbie]

@benlangfeld alive and kicking :) We had our first maintainers meeting and have a basic Roadmap to get us rolling again

[benlangfeld]

@bexelbie Rebased, but this modifies CNAME support and removes reverse DNS. I'm not convinced either were implemented adequately and were done so in a way that makes generic record type support hard. That those PRs were merged before this one I think was a mistake; it looks like a suggestion for a long-term change in approach was interpreted as a suggestion that this medium-term solution not be merged, which was not my intent.

I now believe a ground-up rethink of what landrush is supposed to be, along the lines of the notes above, is necessary before moving forward. Complexity has added and now it's time to step back and re-evaluate. I've made my preference for the approach clear. If you're in agreement, then all that remains is to decide the API Landrush is to provide for manual record insertion and which records should be inserted automatically, both to generate a BIND zone file. That this functionality and the DNS server are coupled has over time encouraged strange behaviours and a very loose specification of intent.

[bexelbie]

@benlangfeld I will take some time to re-read the history of this PR before I comment. I am updating to let you know that I saw your update.

[hferentschik]

@benlangfeld, even worse. I merged now also my changes to make Landrush work on Windows. They don't directly affect your code, but since I touched a lot of files, there are plenty of merge issues. Not sure if you to try to sort them out. If so, it would be nice if you also create a issue for this and add the issue key to the commit messages.

The new issue could also serve as a discussion ground. AFAIU you also think that we first need to define the overall goal. For me it is not quite clear yet which benefit supporting BIND zone files has? What do we really gain here, compared to the simple config we have now? Could additional DNS entry types also supported by sticking to a simple JSON notation?

[hferentschik]

I guess some explanation would be needed here what this actually means. They make no sense to me atm. I guess they are some sort of BIND syntax.

[benlangfeld]

@hferentschik I don't use Landrush currently; in fact, right now I have no need for anything landrush-like. Given my other project load, I probably won't get back to this issue. If I ever do have a need in the future and the point of this PR becomes important, I will probably build it out separately from Landrush to avoid integration effort.

[hferentschik]

I don't use Landrush currently; in fact, right now I have no need for anything landrush-like.

Fair enough.