BerlinVagrant / vagrant-dns

A plugin to manage DNS records for vagrant environments
MIT License
489 stars 50 forks source link

Support Linux with resolved #75

Closed mattiasb closed 1 year ago

mattiasb commented 1 year ago

Support Linux with systemd-resolved. The systemd project's work on standardizing the Linux user space makes it possible to target Linux as a platform.

mattiasb commented 1 year ago

This took a while. Christmas of course, but I also had to make sure I understood systemd-resolved correctly.

The philosophy behind this PR is: supporting any and all Linux distributions out there is a dead end. Support those that are converging on systemd (which on the desktop is pretty much everyone).

The systemd-resolved DNS proxy is enabled by default since a while on both Ubuntu and Fedora and it provides a way to configure split DNS the way vagrant-dns seems to want it. Actually implementing the support in vagrant-dns wasn't very hard.

The issues I struggled with on the way:

Some questions:

mattiasb commented 1 year ago

I'll rebase against master now.

mattiasb commented 1 year ago

I saw that you reformatted the config file multiline string here. Do you want me to keep that formatting?

fnordfish commented 1 year ago

Thanks so much for your work!

The strategy of adding symlinks from config files to a file within /home// won't work since the user that runs systemd-resolved doesn't have permission to read there. This means that the user will have to run vagrant dns --install whenever the IP address, port or the list of TLD's is changed.

I guess that's fine. The mac version has the same limitation when changing TLDs (https://github.com/BerlinVagrant/vagrant-dns/blob/master/PLATFORM_SUPPORT.md#design-goals)

Re TLD overwriting:

I think we should "cache" the TLD configs in a config file (like we do for the name to IP matching config) and generate the system config file(s) from there.

I try to allocate some time to make the necessary changes in vagrant-dns.

fnordfish commented 1 year ago

I saw that you reformatted the config file multiline string here. Do you want me to keep that formatting?

No, you can keep it this way, I'll probably have a major version bump anyways. This was simply to keep compatibility to old ruby versions (< 2.3) and have nicer indention - also it makes line endings explicit :)

mattiasb commented 1 year ago

Thanks for the swift review! I'll get on this tomorrow! :)

mattiasb commented 1 year ago

I saw that you reformatted the config file multiline string here. Do you want me to keep that formatting?

No, you can keep it this way, I'll probably have a major version bump anyways. This was simply to keep compatibility to old ruby versions (< 2.3) and have nicer indention - also it makes line endings explicit :)

Ah gotcha! Ruby compatibility is outside my scope of knowledge. :)

mattiasb commented 1 year ago

NOTE to self: this is how I get vagrant-dns built on my machine:

$ export TEST_RUBY_VERSION=3.1.3 TEST_VAGRANT_VERSION=v2.3.4 rm pkg/vagrant-dns-2.2.3.gem ; bundle install && bundle exec rake build
fnordfish commented 1 year ago

@mattiasb I've just added a TLD managing config and moved some code around in https://github.com/BerlinVagrant/vagrant-dns/tree/feature/linux - would be great, if you could give it a try on your machine.

fnordfish commented 1 year ago

BTW: Testes on fresh Ubuntu 22.04 Desktop install (inside a VMware Box) and works just fine.

mattiasb commented 1 year ago

BTW: Testes on fresh Ubuntu 22.04 Desktop install (inside a VMware Box) and works just fine.

The fact that it's both a different OS than me and a different provider is reassuring!

I'll test and take a look at your code in a bit!

mattiasb commented 1 year ago

One issue I'm stumbling into while trying to get this up and running on my personal laptop is this:

$ vagrant destroy -f mgmt.test 
==> mgmt.test: Domain is not created. Please run `vagrant up` first.
/usr/share/ruby/pstore.rb:124:in `initialize': directory /home/mattiasb/.vagrant.d/tmp/dns does not exist (PStore::Error)
    from /usr/share/ruby/yaml/store.rb:58:in `initialize'
    from /home/mattiasb/Code/git.company.example.com/SRE/AnsiblePlaybooks/.vagrant/plugins/gems/3.1.3/gems/vagrant-dns-2.2.3/lib/vagrant-dns/registry.rb:9:in `new'
       [ … ]

That is: if I run vagrant destroy on a machine that isn't yet created and I haven't gotten vagrant-dns doing its thing yet it spits out a stacktrace. This should probably be handled since it's expected that the file might be missing at this point.

mattiasb commented 1 year ago

Except for that it works fine!

mattiasb commented 1 year ago

Did a quick review. All looked good to me and I only had nits and commit hygienic comments (which is a very subjective topic anyways).

The important thing (to me) is that the code seems to work fine! (except for that stacktrace).

fnordfish commented 1 year ago

manually merged

fnordfish commented 1 year ago

released in 2.3.0