Bug 242125 - dns/dnsdist: Update to 1.4.0
Summary: dns/dnsdist: Update to 1.4.0
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Carlos J. Puga Medina
URL:
Keywords:
: 242140 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-11-21 01:36 UTC by Jørn Åne de Jong
Modified: 2019-12-12 10:23 UTC (History)
7 users (show)

See Also:
cpm: maintainer-feedback+


Attachments
dns/dnsdist patch 2019112101 (1.49 KB, patch)
2019-11-21 01:36 UTC, Jørn Åne de Jong
no flags Details | Diff
dns/dnsdist patch 2019112102 (1.33 KB, patch)
2019-11-21 01:54 UTC, Jørn Åne de Jong
no flags Details | Diff
dns/dnsdist patch 2019112103 (3.74 KB, patch)
2019-11-21 11:51 UTC, Jørn Åne de Jong
no flags Details | Diff
Update to dnsdist 1.4.0 (3.67 KB, patch)
2019-11-21 15:19 UTC, Ralf van der Enden
no flags Details | Diff
Update to dnsdist 1.4.0 (and enable DoH by default) (3.74 KB, patch)
2019-12-02 08:44 UTC, Ralf van der Enden
tremere: maintainer-approval?
Details | Diff
dnsdist 1.4.0 multi-daemon (6.76 KB, patch)
2019-12-03 14:05 UTC, Leo Vandewoestijne
no flags Details | Diff
removed error from previous patch (8.09 KB, patch)
2019-12-10 13:47 UTC, Leo Vandewoestijne
no flags Details | Diff
same but against rev12 (6.76 KB, patch)
2019-12-12 10:23 UTC, Leo Vandewoestijne
freebsd: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jørn Åne de Jong 2019-11-21 01:36:12 UTC
Created attachment 209303 [details]
dns/dnsdist patch 2019112101

dns/dnsdist: Update to 1.4.0

  * Update dns/dnsdist to version 1.4.0
  * Add DOH option for DNS over HTTPS (default on)
  * Add dependency on www/h2o when DOH is set

Changelog:

   https://dnsdist.org/changelog.html

QA:

  * portlint: OK (looks fine.)
  * testport: OK (poudriere: 120Ramd64, 120Ri386, only default options tested)
Comment 1 Jørn Åne de Jong 2019-11-21 01:54:16 UTC
Created attachment 209305 [details]
dns/dnsdist patch 2019112102

Made configuration flag for DoH dependent on the DOH option.
Comment 2 Peter van Dijk 2019-11-21 08:48:27 UTC
With 1.4.0, you should no longer need https://svnweb.freebsd.org/ports/head/dns/dnsdist/files/patch-dnsdist-lua-vars.cc?revision=496048&view=markup

I do not know if you still need the other patch.
Comment 3 Jørn Åne de Jong 2019-11-21 11:51:41 UTC
Created attachment 209316 [details]
dns/dnsdist patch 2019112103

You're right, it compiles and runs without these patches.

I'm not sure what patch-ext_luawrapper_include_LuaContext.hpp did, so I can't vouch for its removal, but I did test on 121Ramd64 and it runs a DoH server just fine.

QA:

  * portlint: OK (looks fine.)
  * testport: OK (poudriere: 121Ramd64, 121Ri386, 120Ramd64, 120Ri386, 113Ramd64, 113Ri386, with/without DOH tested on 121Ramd64, only default on others)
Comment 4 Ralf van der Enden 2019-11-21 15:19:58 UTC
Created attachment 209320 [details]
Update to dnsdist 1.4.0


- Makefile is formatted using ports-mgmt/portfmt
- Some configure switches changed from --enable to --with
- Rename FSTRM to DNSTAP
- Add DoH support
- Switch to LuaJIT-Openresty (when LuaJIT is selected instead of LUA)
- Remove unnecessary patch (files/patch-dnsdist-lua-vars.cc)
Comment 5 Jørn Åne de Jong 2019-11-21 17:59:03 UTC
Comment on attachment 209316 [details]
dns/dnsdist patch 2019112103

That seems better, thanks!
Comment 6 Carlos J. Puga Medina freebsd_committer 2019-11-23 18:00:43 UTC
(In reply to Ralf van der Enden from comment #4)

Hi Ralf, 

First of all, thank you very much for the patch :)

Second, are you interested in being the new maintainer and keeping it updated?

Last but not least, please can you explain why did you rename FSTRM to DNSTAP?
Comment 7 Carlos J. Puga Medina freebsd_committer 2019-11-23 18:12:03 UTC
*** Bug 242140 has been marked as a duplicate of this bug. ***
Comment 8 Ralf van der Enden 2019-11-24 16:37:18 UTC
Hi Carlos,

I'm interested in taking over maintainership. I also maintain dns/powerdns and dns/powerdns-recursor.

I renamed FSTRM to DNSTAP, because DNSTap is the feature that uses libfstrm (--enable-dnstap for configure). There's more info about it on https://dnstap.info

IMO it made more sense to (re)name the option after the feature than the used library).

Cheers,
Ralf
Comment 9 Peter van Dijk 2019-11-24 16:48:21 UTC
Hello from PowerDNS upstream! Ralf has been great about keeping dns/powerdns and dns/powerdns-recursor updated, checking with us if anything breaks, and making sure the patches the ports need end up in our upstream tree eventually. Indeed, when I pointed him to this ticket, he mentioned he already had an updated dnsdist port sitting on his disk, as he has shown in this thread.

So, we'd fully support Ralf taking ownership here. Thank you!

(also, yes, calling the option DNSTAP instead of FSTRM makes sense.)
Comment 10 Leo Vandewoestijne 2019-11-25 10:02:59 UTC
As I didn't expect anyone being such a rapid submitting the 1.4.0 update, I've submitted a duplicate PR: PR242140.

I don't know if it's the right moment to suggest a rc script that ignites multiple daemons, which would allow to keep config simple, and so prevent going slowing down.
For example if you run Bind with views, then you don't want to have a rule with countless times "if src addr then / else if addr then / etc".

In a single dnsdist I've used:

function luarule(dq)
  str = dq.localaddr:toString()
  if     string.find("10.0.0.1   2a03::1",   str) then return DNSAction.Pool, "v001"    -- mind that 2a03::1 also matches 2a03::11 and 2a03::111
  elseif string.find("10.0.0.2   2a03::2",   str) then return DNSAction.Pool, "v002"    -- mind that 10.0.0.2 also matches 10.0.0.22 and 10.0.0.222
  elseif string.find("10.0.0.249 2a03::249", str) then return DNSAction.Pool, "v249"    -- so string.find is not the most accurate function to use here 
                                                  else return DNSAction.Pool, "v250"
  end
end
addAction(AllRule(), LuaAction(luarule))

(I'm aware it's not the best code, but it served my need have something working).
In stead of improving config I tougth a multi-daemon RC was a better solution.

This patch contains what I was playing with:
https://bz-attachments.freebsd.org/attachment.cgi?id=209318
consider it code for inspiration: I'm not fully sure I've truly used it, plus wonder if it really does what I wish to accomplish (or if it's the correct way to do so).

I will try to do some testing later today.
If you have input in the meanwhile - or know of better methods/examples, then RSVP.
Comment 11 Leo Vandewoestijne 2019-11-26 16:21:39 UTC
Still working on it / verifying if it does what it should.

Ahead of that I'd like to mention the rc script could already be improved with minor changes:

+actual_command="%%PREFIX%%/sbin/${name} -u ${dnsdist_priv_user} -g ${dnsdist_priv_group} --supervised"
 command=/usr/sbin/daemon
-actual_command=/usr/local/sbin/${name}
-command_args="-c -f -r -P ${pidfile} ${actual_command} -u ${dnsdist_priv_user} -g ${dnsdist_priv_group} --supervised"
+command_args="-c -f -r -P ${pidfile} -- ${actual_command}"

Such that you unlikely will send wrong parameters to the other command.
Comment 12 Sascha Biberhofer 2019-12-01 13:24:19 UTC
(In reply to Ralf van der Enden from comment #4)

This patch has been working fine for me. Is DoH support going to stay a non-default option? I'd really prefer to enable DoH by default since dnsdist is one of the few projects able to serve DNS-over-TLS as well as DNS-over-HTTPS, especially since powerdns for example delegates this functionality pretty explicitly to dnsdist.
Comment 13 Ralf van der Enden 2019-12-02 08:44:34 UTC
Created attachment 209591 [details]
Update to dnsdist 1.4.0 (and enable DoH by default)
Comment 14 Ralf van der Enden 2019-12-02 08:46:42 UTC
Also cleaned up the Makefile a bit. DoH was enabled by default, but the OPTIONS knob for DoH pulled in the necessary library (h2o) if enabled.
Comment 15 Leo Vandewoestijne 2019-12-03 14:05:37 UTC
Created attachment 209658 [details]
dnsdist 1.4.0 multi-daemon

+ includes everything of patch-id 209591 of Ralf (I applied it before starting my changes)
+ maintainer change
+ minor improvements in the rc script
+ a major improvement in the rc script; allowing multiple daemons
+ start using -C and %%ETCDIR%% causing the default path for dnsdist.conf changes to usually /usr/local/etc/dnsdist/ in which you can store single or multiple config files, includes, key-files for DNSCrypt, key/certs for DoH, etc.

For the multi-daemon part I peaked at the NSD port maintained by Jaap Akkerhuis.

Of course I've tested it running it single and multiple, and checked (succesfully) if all processes / pid.files all started/stopped cleanly.
Comment 16 Leo Vandewoestijne 2019-12-10 13:47:26 UTC
Created attachment 209826 [details]
removed error from previous patch

While adding my own patch to my production environments I found a mistake; GNU_CONFIGURE went missing for unknown reason.
This patch keeps it as intended by Ralf.
Comment 17 Leo Vandewoestijne 2019-12-12 10:23:01 UTC
Created attachment 209886 [details]
same but against rev12

I applied my own patch and got rejected, then discovered PORTREVISION 12,
so this is all the same as before, but adapted to that.