net-dns/dnscrypt-proxy: simplify init, follow upstream recommendations. #5346
Conversation
gentoo-repo-qa-bot
added
the
assigned
|
Looking at the differences between |
|
My thoughts we that since I rework the ebuild significantly I'd bump a version with current approach and introduce my changes with -r1 to discuss here. |
|
Yeah, please do that. |
gyakovlev
force-pushed
the
dnscrypt_simplification
branch
3 times, most recently
from
348ab36
to
02ea860
Compare
|
done. i don't like confusing names of the updated files in the files dir, but I guess i have no choice until 1.9.4 ebuild is deleted. |
gyakovlev
force-pushed
the
dnscrypt_simplification
branch
from
02ea860
to
58beb82
Compare
|
Please name the ebuild |
| DEPEND="${RDEPEND} | ||
| virtual/pkgconfig" | ||
|
|
||
| DOCS="AUTHORS ChangeLog NEWS README* THANKS *txt" |
There was a problem hiding this comment.
Since EAPI-6 wildcards are not allowed in global scope. If you really need the wildcards, move the DOCS variable into src_install() and make it local. You could also consider to make it an array.
There was a problem hiding this comment.
renamed the ebuild, I was going to change it as I squashed but lost that change during squashing.
also moved DOCS into src_install and made it an array.
gyakovlev
force-pushed
the
dnscrypt_simplification
branch
from
58beb82
to
f47723d
Compare
|
I've made DOCS a local and made it an array. My previous comment here vanished with another rebase. |
|
|
||
| ## Write the PID number to a file | ||
|
|
||
| PidFile /var/run/dnscrypt-proxy.pid |
There was a problem hiding this comment.
I suggest leaving the PidFile out of the configuration file, and instead passing it on the command-line (which would override the one in the config file anyway). The reason is, in the init script, you define
pidfile="/var/run/${SVCNAME}.pid"
But what happens if the user changed the PidFile line in the config file? The init script stops working =(
Don't ask why, but people actually do that. Instead, I would pass the existing $pidfile variable to the daemon:
command_args="${DNSCRYPT_OPTS} --pidfile="${pidfile}"
That way the PID file that start-stop-daemon uses is guaranteed to be the one that dnscrypt-proxy uses.
An unrelated note: the /var/run directory is being migrated to simply /run, so now is as good a time as any to make that change.
| ## run the server as a less-privileged system user. | ||
| ## The value for this parameter is a user name. | ||
|
|
||
| User dnscrypt |
There was a problem hiding this comment.
Since your ebuild creates the dnscrypt user, you might consider forcing the daemon to run as that user (by passing it on the command line). Then you could get rid of this setting so that people don't change it and break the init script. For example,
command_args="${DNSCRYPT_OPTS} --pidfile="${pidfile} --user=dnscrypt"
I'm not nearly as sure about this one though, so it's up to you.
There was a problem hiding this comment.
@orlitzky That's not possible.
Dnscrypt will only take either config file or command line parameters. Not both.
That's why I had to create a config file. It allows to use cache, bypass domains and more.
I don't know if it's on purpose or just temporary limitation, I'll ask upstream and check their github issues.
It makes a perfect sense to override pid file and user in conf.d but unfortunately not possible now.
There was a problem hiding this comment.
found.
DNSCrypt/dnscrypt-proxy#607
so it's intended.
It's actually possible to enable cache and other features using command line by passing magic parameters but it quickly becomes quite cumbersome.
Upstream advises to use config file by default, so I decided to ship a sensible working config with random secure resolver. It works out of the box, just need to update /etc/resolv.conf to point to 127.0.0.1.
There was a problem hiding this comment.
Ok, thanks, I didn't know that it was one-or-the-other. Forget everything I said =)
There was a problem hiding this comment.
sidenote:
It's still possible to run dual resolvers with config file.
One can symlink the initscript, copy conf.d file and point it to another config file with unique PidFile, LocalAddress and optionally syslog prefix.
That's not ideal but works.
I'll push a change placing pidfile it to /run/. Makes sense.
gyakovlev
force-pushed
the
dnscrypt_simplification
branch
from
f47723d
to
e84b257
Compare
|
ping |
|
Meanwhile |
Greatly simplify initscript to allow symlinking of unit files for openrc. This approach follows upstream recommendation to use config file instead of command line args. Also proper systemd unit with socket activation from upstream. Fixes 588462 Bug: https://bugs.gentoo.org/show_bug.cgi?id=588462
Request maintanership. Add my proxy as well.
gyakovlev
force-pushed
the
dnscrypt_simplification
branch
from
e84b257
to
49b82ff
Compare
|
😞 The QA check for this pull request has found the following issues: Issues inherited from Gentoo (may be modified by PR): |
mgorny
removed
the
assigned
|
Pull Request assignment Areas affected: ebuilds net-dns/dnscrypt-proxy: @gentoo/proxy-maint (maintainer needed) Bugs linked: 588462 |
gentoo-repo-qa-bot
added
maintainer-needed
|
@Polynomial-C ping, I've rebased for a clean merge. |
Request maintanership. Add my proxy as well. Closes: #5346
Modify dnscrypt-proxy ebuild to follow upstream recommendation of using config file over command line arguments.
Currently only with config file one can specify advanced options like Cache, Forward domains and more.
This greatly simplifies the init script removing unnecessary bashism and boilerplate.
Allows to make symlinks to run several instances.
Adds proper socket activation for systemd as upstream intended.
Ships with a config file that works out of the box and selects random provider that does not log queries and supports dnssec.
Adding myself as a secondary maintainer as discussed with @mgorny on irc.
Also ping my proxy @Polynomial-C