Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DietPi-Software | AdGuard Home #4429

Merged
merged 36 commits into from
Jun 16, 2021
Merged

DietPi-Software | AdGuard Home #4429

merged 36 commits into from
Jun 16, 2021

Conversation

Joulinar
Copy link
Collaborator

@Joulinar Joulinar commented May 28, 2021

AdGuard Home - add new software title. Software ID 126

Status: testing

  • AdGuard Home install
  • AdGuard Home uninstall
  • AdGuard Home reinstall
  • install/uninstall/reinstall together with Unbound
  • prevent install/uninstall/reinstall if Pi-hole is present

test device

  • RPi1
  • RPi3B+ 32bit
  • RPi4B 64bit
  • VM amd64 Buster
  • VM amd64 Stretch

Reference: #3778

Commit list/description:

  • DietPi-Software | AdGuard Home - add new software title. create initial configuration
  • DietPi-Services | AdGuard Home - add new service
  • DietPi-Software | AdGuard Home - stop service on reinstall before copying files
  • DietPi-Software | AdGuard Home - adjust product name
  • DietPi-Software | AdGuard Home - change software ID to 126
  • DietPi-Services | AdGuard Home - adjust product name
  • README.md | AdGuard Home - add new software title
  • DietPi-Software | AdGuard Home - change configuration file ID
  • DietPi-Software | AdGuard Home - adjust service configuration file
  • AdGuardHome.yaml | remove non-default DNS filter lists
  • DietPi-Software | AdGuard Home - remove logfiles on uninstall from /var/log/
  • DietPi-Software | AdGuard Home - rename service and keep output on STDOUT (default)
  • DietPi-Services | AdGuard Home - rename service
  • AdGuardHome.yaml | change default port to 8083
  • DietPi-Software | AdGuard Home - remove environment configuration from service description
  • DietPi-Software | AdGuard Home - Avoid having AdGuard Home installed in parallel to Pi-hole
  • DietPi-Software | AdGuard Home - Offer to install Unbound along with AdGuard Home
  • DietPi-Software | Unbound - configure Unbound to work together with AdGuard Home
  • DietPi-Software | Unbound - Configuring AdGuard Home to use Unbound
  • DietPi-Software | AdGuard Home - Unbound: Switch port to 5335 if it was installed before
  • DietPi-Software | AdGuard Home - change configuration to use Unbound if available
  • DietPi-Software | Unbound - Assure AdGuard Home does not resolve via Unbound anymore if Unbound will be uninstalled
  • DietPi-Software | Pi-hole – on uninstallation, switch Unbound port to 53 if Unbound is still installed
  • DietPi-Software | AdGuard Home – on uninstallation, switch Unbound port to 53 if Unbound is still installed
  • DietPi-Software | AdGuard Home - ensure no concurrent installation of AdGuard Home and Pi-hole, even if installation is started from CLI
  • DietPi-Software | Unbound - Show a notification only, instead of a message pop-up if Unbound is being uninstalled
  • DietPi-Software | AdGuard Home: Do Pi-hole concurrency check as part of the dependency function, like we do with Tor Relay and Tor Hotspot. This is called on CLI installs as well while on menu-based installs the additional warnings are great so that users have chance to adjust their selections before starting the installs.
  • DietPi-Software | AdGuard Home: Assure that file modes are kept when copying files to disk and remove files from tmpfs afterwards
  • DietPi-Software | AdGuard Home: Only enable service during install and start it after all installs have been finished, to keep RAM and CPU usage at a minimum. If it was installed already, the service is still running, which is then required to avoid stopping the systems own DNS server.
  • DietPi-Software | Assure that uninstalls do not fail uncleanly when single steps fail, but keep the interactive error prompt. This way it is made clear that something failed, but instead of "Exit", "Ignore" can be selected to proceed with the uninstall.
  • DietPi-Software | Lighttpd - stop service before uninstalling, otherwise an active process will be left
  • DietPi-Software | Lighttpd – remove enabled configuration directory, to avoid other software title trying to disable Lighttpd mods
  • DietPi-Software | Pi-hole – disable Lighttpd mods one by one as using wildcards does not seems to be working
  • DietPi-Software | Pi-hole – correct folder name for available Lighttpd mods to allow correct removal during uninstallation
  • DietPi-Software | Lighttpd - enhanced Lighttpd uninstall process
  • DietPi-Software | AdGuard Home - Allow AdGuardHome to run without superuser privileges
  • DietPi-Software | AdGuard Home - Allow AdGuardHome to run without superuser privileges via service file value AmbientCapabilities
  • AdGuardHome.yaml | reduce query log interval down to 7 days

ok first shot. Quite some thinks to be clarified

done

  • which directory to run on?
    AdGuard Home will be located in /opt
  • default web server port?
    AdGuard Home default web server port will be 8083
  • should we pre-check for already installed PiHole/Unbound?
    pre-check are done during software selection
    if available, Unbound/AdGuard Home will be reconfigured to be able to work together
  • which user to be used on Web UI?
    user admin will be used to login to Web UI
  • configuration
    logs are available via journalctl
  • running AGH without superuser privileges
    done via service file value AmbientCapabilities
  • service file configuration
    restart set to on-failure and set restart attempt to 5 times only
  • tune default config parameter?
    done

Info
Web UI pw = global pw is used (requires apache2-utils)

DietPi-Software |  AdGuardHome - add new software title. create initial configuration
@Joulinar Joulinar marked this pull request as draft May 28, 2021 12:13
@Joulinar Joulinar self-assigned this May 28, 2021
@Joulinar Joulinar added this to the v7.3 milestone May 28, 2021
@Joulinar Joulinar linked an issue May 28, 2021 that may be closed by this pull request
DietPi-Software | AdGuardHome - stop service on reinstall before copying files
dietpi/dietpi-software Outdated Show resolved Hide resolved
dietpi/dietpi-software Outdated Show resolved Hide resolved
dietpi/dietpi-software Outdated Show resolved Hide resolved
dietpi/dietpi-software Outdated Show resolved Hide resolved
DietPi-Software | AdGuardHome - adjust product name
DietPi-Software | AdGuardHome - change software ID to 126
DietPi-Services | AdGuardHome - adjust product name
README.md | AdGuard Home - add new software title
DietPi-Software | AdGuard Home - change configuration file ID
DietPi-Software | AdGuard Home - adjust service configuration file
AdGuardHome.yaml | remove non-default DNS filter lists
DietPi-Software | AdGuard Home - remove logfiles on uninstall from /var/log/
dietpi/dietpi-software Outdated Show resolved Hide resolved
dietpi/dietpi-software Outdated Show resolved Hide resolved
DietPi-Software | AdGuard Home - Fixes
@MichaIng
Copy link
Owner

open points

  1. which directory to run on? /opt
    • As install dir, opt is fine. If it uses the working dir for volatile runtime files, /run/adguardhome could be defined and used instead. Currently install dir = data dir? Probably we can use /mnt/dietpi_userdata/adguardhome for settings and data?
  2. default web server port? 8080
    • Airsonic and Tomcat both use 8080 already. I'll update your Tomcat 8 implementation to Tomcat 9 with next release and change the port then to the last free one. 8081 is Medusa, 8082 mjpg-streamer
    • => 8083 is free
  3. should we pre-check for already installed PiHole/Unbound?
    • Probably. Block AdGuard Home install as long as Pi-hole is installed and the other way round? Otherwise finding a port etc becomes very complicated when Unbound is a possible companion already.
    • Add Unbound as upstream the same way we do it for Pi-hole?
  4. currently AGH is running as root user.
    Should we change this? https://github.com/AdguardTeam/AdGuardHome/wiki/Getting-Started#running-without-superuser
    • I vote for this. The suggested permissions are granted via the executable, which means any user executing it will gain those. Probably we find a way to allow only the adguardhome user binding to port 53. I'll check that.
  5. I did not had a look yet but we might need to tune default config parameter?
    • I did not had a look yet, same here, I'll have a look, when we're otherwise done.
  6. same for service configuration?
    Logs are written to /var/log
    restart set to always
    • I vote for logs to STDOUT only (default), so that journalctl -u adguardhome can be used to check service + process logs together and avoid the need to deal with a log directory and permissions.
    • If there is an internal updater that triggers restarts, then probably we can restart it only when a related exit code/signal is found while leaving it shopped on error. Else I'd skip that. If there is a crash, then a restart is usually not succeeding or making things worse, at least in my experience.
  7. which user to be used on Web UI?
    admin
    • We could think about some consistency for web UI admin users, as currently we use a few different ones. But not important, admin is fine 🙂.

dietpi/dietpi-software Outdated Show resolved Hide resolved
Joulinar and others added 4 commits May 28, 2021 18:56
DietPi-Software | AdGuard Home - rename service and keep output on STDOUT (default)
DietPi-Services | AdGuard Home - rename service
AdGuardHome.yaml | change default port to 8083
DietPi-Software | AdGuard Home - code fixe
+ DietPi-Software | Satisfy shellsheck: Shouldn't be SC2016 satisfied automatically when $ signs are already escaped explicitly? :(
Copy link
Collaborator Author

@Joulinar Joulinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is close to cheating 😉

Joulinar and others added 2 commits May 28, 2021 23:06
DietPi-Software | AdGuard Home - remove environment configuration from service description
META | Update AdGuardHome branch
@MichaIng MichaIng marked this pull request as ready for review May 29, 2021 22:34
@MichaIng MichaIng changed the title DietPi-Software | AdGuardHome DietPi-Software | AdGuard Home May 30, 2021
@Joulinar
Copy link
Collaborator Author

quick and dirty tried this

[Service]
User=adguardhome
CapabilityBoundingSet=CAP_NET_BIND_SERVICE
WorkingDirectory=/opt/AdGuardHome
ExecStart=/opt/AdGuardHome/AdGuardHome -s run

But getting permission denied still

Jun 10 15:22:27 DietPi4 AdGuardHome[5138]: 2021/06/10 15:22:27.982242 [fatal] couldn't start forwarding DNS server: couldn't listen to UDP socket, cause: listen udp 0.0.0.0:53: bind: permission denied
Jun 10 15:22:27 DietPi4 systemd[1]: adguardhome.service: Main process exited, code=exited, status=1/FAILURE

@Joulinar
Copy link
Collaborator Author

for now I will push my commit, even if it is not the final solution. Just to be able to work on.

DietPi-Software | AdGuard Home - Allow AdGuardHome to run without superuser privileges
@Joulinar
Copy link
Collaborator Author

btw: setcap is already part of the initial image and installed by default. It can be used right from the beginning 😉

DietPi-Software | AdGuard Home - change permission on home and working directory
@MichaIng
Copy link
Owner

There is the AmbientCapabilities directive. Ah, lol and indeed that is the one to "grant" capabilities, while the other one limits them: https://unix.stackexchange.com/a/581337

@MichaIng MichaIng mentioned this pull request Jun 11, 2021
@Joulinar
Copy link
Collaborator Author

Will play with it later the evening.

DietPi-Software | AdGuard Home - Allow AdGuardHome to run without superuser privileges via service value `AmbientCapabilities`
AdGuardHome.yaml | reduce query log interval down to 7 days
@MichaIng
Copy link
Owner

Works pretty well, also behind frp 😄, but I found a few things we can enhance.

To consequently have identical lower-case name for user, service and dirs:

  • /opt/AdGuardHome => /opt/adguardhome

To use the intended data/config dir:

  • ExecStart=/opt/AdGuardHome/AdGuardHome -s run => ExecStart=/opt/AdGuardHome/AdGuardHome -w /mnt/dietpi_userdata/adguardhome
    • -s run is undocumented and looks like a dummy mode, to run it in foreground instead of background, as we want it for systemd. But running it without service mode does exactly the same.
  • WorkingDirectory=/opt/AdGuardHome => WorkingDirectory=/mnt/dietpi_userdata/adguardhome
    • WorkingDirectory has not effect and is not requires with above -w option.
  • /opt/AdGuardHome/AdGuardHome.yaml => /mnt/dietpi_userdata/adguardhome/AdGuardHome.yaml

With this, /opt/adguardhome can be removed completely on reinstalls. I'm not sure why, but the install dir comes with 777 mode. Since the user does not required any write access to the install dir, when using userdata for data and config, all we need to do about it is:

  • chmod 755 /opt/adguardhome

To allow DHCP server scanning:

  • AmbientCapabilities=CAP_NET_BIND_SERVICE => AmbientCapabilities=CAP_NET_BIND_SERVICE CAP_NET_RAW

To allow reloading the configuration without full server restart:

  • ExecReload=/opt/adguardhome/AdGuardHome -s reload

@Joulinar
Copy link
Collaborator Author

Joulinar commented Jun 16, 2021

but the install dir comes with 777 mode.

Main executable is set to 777 as well. Should we reduce it same way?

root@DietPi4:/opt/adguardhome# ls -la
total 20380
drwxr-xr-x 2 adguardhome adguardhome     4096 May 19 15:26 .
drwxr-xr-x 3 root        root            4096 Jun 16 22:54 ..
-rwxrwxrwx 1 adguardhome adguardhome 20774912 May 19 15:26 AdGuardHome
-rw-rw-rw- 1 adguardhome adguardhome      331 May 19 15:26 AdGuardHome.sig
-rw-r--r-- 1 adguardhome adguardhome    16827 May 19 15:26 CHANGELOG.md
-rw-r--r-- 1 adguardhome adguardhome    35149 May 19 15:26 LICENSE.txt
-rw-r--r-- 1 adguardhome adguardhome    22689 May 19 15:26 README.md
root@DietPi4:/opt/adguardhome#

like this chmod 755 /opt/adguardhome{,/AdGuardHome}

@MichaIng
Copy link
Owner

like this chmod 755 /opt/adguardhome{,/AdGuardHome}

🚀

DietPi-Software | AdGuard Home - consequently have identical lower-case name for user, service and directories
DietPi-Software | AdGuard Home – change working directory to dietpi_userdata
DietPi-Software | AdGuard Home – move config files and data directory to dietpi_userdata
DietPi-Software | AdGuard Home – reduce permission of install directory /opt/adguardhome to 755
DietPi-Software | AdGuard Home – allow DHCP server scanning by setting CAP_NET_RAW within service file
DietPi-Software | AdGuard Home – add reload option within service file
@Joulinar
Copy link
Collaborator Author

ok did the changes, pls can you have another look.

dietpi/dietpi-software Outdated Show resolved Hide resolved
smaller change
dietpi/dietpi-software Outdated Show resolved Hide resolved
@MichaIng
Copy link
Owner

Aside of the minor notes, LGTM. I just made a test install, reinstall and uninstall. Read to be merged from my end.

MichaIng
MichaIng previously approved these changes Jun 16, 2021
smaller cosmetics
MichaIng
MichaIng previously approved these changes Jun 16, 2021
@Joulinar
Copy link
Collaborator Author

should I merge or will you do once you are ready with reviewing the others?

+ CHANGELOG | AdGuard Home: This DNS sinkhole ad blocker for your LAN, similar to Pi-hole, has been added with software ID 126.
@MichaIng MichaIng merged commit 5bb8d60 into dev Jun 16, 2021
@MichaIng MichaIng deleted the AdGuardHome branch June 16, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DietPi-Software | AdGuard Home
3 participants