dnsdist: allow setting keyLogFile to DoT/DoH backends

[karelbilek]

Short description

This adds keyLogFile setting to DoT and DoH backends.

Checklist

I have:

• read the CONTRIBUTING.md document
• compiled this code
• tested this code - tested with wireshark + google's DoH backend
• included documentation (including possible behaviour changes)
• documented the code
• added or modified regression test(s)
• added or modified unit test(s)

The regression tests just try if the logfiles exist and are non-empty, they don't really check what is in them.

[coveralls]

Pull Request Test Coverage Report for Build 12236041428

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 9 of 11 (81.82%) changed or added relevant lines in 4 files are covered.
• 28 unchanged lines in 9 files lost coverage.
• Overall coverage increased (+0.04%) to 64.743%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pdns/dnsdistdist/doh.cc	0	1	0.0%
pdns/tcpiohandler.cc	3	4	75.0%

Files with Coverage Reduction	New Missed Lines	%
pdns/recursordist/syncres.cc	1	79.38%
pdns/signingpipe.cc	2	83.65%
pdns/recursordist/aggressive_nsec.cc	2	66.25%
modules/lmdbbackend/lmdbbackend.cc	2	72.54%
pdns/recursordist/test-syncres_cc2.cc	3	88.85%
pdns/remote_logger.cc	3	53.81%
pdns/iputils.cc	3	55.76%
pdns/recursordist/test-syncres_cc1.cc	5	89.78%
pdns/recursordist/rec-main.cc	7	62.48%

Totals
Change from base Build 12234956497:	0.04%
Covered Lines:	125883
Relevant Lines:	163529

---

💛 - Coveralls

[rgacogne]

The PR looks good to me, I requested two very small changes for consistency's sake. Thanks a lot!

[karelbilek]

I haven't been able to run the regression tests locally so far, and they don't seem to be run in CIs on PR... should I try more? :)

[rgacogne]

I haven't been able to run the regression tests locally so far, and they don't seem to be run in CIs on PR... should I try more? :)

They are, what makes you think otherwise?

[rgacogne]

(look for Build and test everything / test-dnsdist-regression)

[rgacogne]

Looks great, thanks!

[karelbilek]

@rgacogne just a thought, should I backport it to 1.9? (locally I actually run it with 1.9, it works fine)

[rgacogne]

At this point I would prefer not backporting it: introducing new features in stable branchs is always risky, this one touches code that is quite critical (even though the change is very simple and clean), I'm not sure adding it's worth the risk. If someone really needs it to investigate an issue it's not hard for us to build custom packages backporting the feature.

[karelbilek]

Okay!

…

On Thu 19. 12. 2024 at 10:28, Remi Gacogne ***@***.***> wrote: At this point I would prefer not backporting it: introducing new features in stable branchs is always risky, this one touches code that is quite critical (even though the change is very simple and clean), I'm not sure adding it's worth the risk. If someone really needs it to investigate an issue it's not hard for us to build custom packages backporting the feature. — Reply to this email directly, view it on GitHub <#14938 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZT4JNBQDLFUP5BIYHXFD2GKGTFAVCNFSM6AAAAABTFSRJEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJTGE4DCMZSHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>