getumbrel / umbrel-apps

The official app repository of the Umbrel App Store. Submit apps and updates here. Learn how → https://github.com/getumbrel/umbrel-apps#readme
https://apps.umbrel.com
482 stars 357 forks source link

Add adguardhome #1038

Closed Julienpeps closed 1 month ago

Julienpeps commented 2 months ago

App Submission

Adguard Home

256x256 SVG icon

https://upload.wikimedia.org/wikipedia/commons/4/4c/AdGuard.svg

Gallery images

image image image

I have tested my app on:

lxndrblz commented 2 months ago

@Julienpeps Great work, I was also working on this and came up with a similar solution.

A couple of my screenshots:

Screenshot 2024-03-31 at 18 57 47 Screenshot 2024-03-31 at 18 55 32 Screenshot 2024-03-31 at 18 57 38

My umbrel-app.yml:


manifestVersion: 1.1
id: adguard-home
name: AdGuard Home
category: networking
version: "0.107.46"
tagline: Network-wide ads & trackers blocking DNS server
icon: https://github.com/walkxcode/dashboard-icons/blob/main/png/adguard-home.png?raw=true
description:  >-
  AdGuard Home is a network-wide software for blocking ads and tracking. After you set it up, it'll cover ALL your home devices, and you don't need any client-side software for that.
  It operates as a DNS server that re-routes tracking domains to a “black hole”, thus preventing your devices from connecting to those servers. It's based on software we use for our public AdGuard DNS servers, and both share a lot of code.
developer: AdGuard Team
website: https://adguard.com/en/adguard-home/overview.html
dependencies: []
repo: https://github.com/AdguardTeam/AdguardHome
support: https://github.com/AdguardTeam/AdguardHome/issues
port: 3000
gallery:
  - https://camo.githubusercontent.com/32e0cce4e74cedaa564e1156e9da302fb06c308d96e51239a158370fdaa17fc2/68747470733a2f2f63646e2e6164746964792e6f72672f7075626c69632f416467756172642f436f6d6d6f6e2f616467756172645f686f6d652e676966
path: ""
defaultPassword: ""
releaseNotes: >-
  As promised in the notes for the previous release, this update brings some quality-of-life improvements as well as a few bug fixes 🔧.

  Full changelog

  -  See also the v0.107.46 GitHub milestone.

  Added

  -  Ability to disable the use of system hosts file information for query resolution.

  -  Ability to define custom directories for storage of query log files and statistics.

  Changed

  -  Private rDNS resolution (dns.use_private_ptr_resolvers in YAML configuration) now requires a valid "Private reverse DNS servers", when enabled.

  -  Note: Disabling private rDNS resolution behaves effectively the same as if no private reverse DNS servers provided by user and by the OS.

  Fixed

  -  Statistics for 7 days displayed by day on the dashboard graph.

  -  Missing "served from cache" label on long DNS server strings.

  -  Incorrect tracking of the system hosts file's changes.
submitter: lxndrblz
submission:

The docker-compose.yml:

version: "3.7"

services:
  server:
    image: adguard/adguardhome:v0.107.46
    container_name: adguardhome
    hostname: adguard
    restart: on-failure
    ports:
      #  DNS
      - 53:53/tcp
      - 53:53/udp
      # Setup Interface
      - 3000:3000/tcp
      # DNS-over-TLS
      - 853:853/tcp
      # DNS-over-QUIC
      - 784:784/udp
      - 853:853/udp
      - 8853:8853/udp
    volumes:
      - ${APP_DATA_DIR}/data/adguardhome/work:/opt/adguardhome/work
      - ${APP_DATA_DIR}/data/adguardhome/conf:/opt/adguardhome/conf

This PR closes issue #939

nmfretz commented 2 months ago

Thanks for submitting adguard and for your other update PR as well @Julienpeps! I should be able to get to these late this week / early next week. Sorry for the delay and thanks for your patience.

nmfretz commented 1 month ago

Hey @Julienpeps, thanks again for submitting Adguard Home! I'm taking a look at this now. Luckily, we already made an icon and gallery images for this old PR that we put on hold last year (https://github.com/getumbrel/umbrel-apps/pull/603). So we're ready to launch as soon as your PR here is finalized.

The main reason I held off from merging the Adguard PR last year was that the setup wizard makes it so that users can choose any port for the web interface and inadvertently make it so that clicking on the app icon from the umbrelOS homescreen doesn't take them to the Adguard UI. There is no way to pre-populate this field in the setup wizard, and it instead defaults to port 80... which means users will be required to change it (and potentially mess up):

image

When a user inputs a port for the web interface and finishes the setup wizard, they will be automatically directed to http:<hostname>:<their-chosen-port>. If they choose a port other than the one we set in the umbrel-app.yml then they won't be able to get to the webUI when they click the app icon from the umbrelOS homescreen, which will be confusing to most users. It also means that they might choose a port that will clash with another app they might install in the future. Luckily, in host network mode, Adguard at least helpfully shows a warning and won't let you configure to a port that is already bound on the host, so immediate clashes aren't possible.

I had been hoping that Adguard would make the set up a little bit easier (e.g., keeping the setup wizard and webUI at the same port by default, or allowing the final web UI port to be set via environment variable), but this doesn't seem to be on their roadmap. So let's just try and make it as easy as possible to install and set up Adguard.

Here's what I think we should do:

1) Keep Adguard in host networking mode as you have done 👌. This is the easiest way to configure Adguard for all functionality, and it also means that the actual local IP address of the user's device will be displayed to them within Adguard's in-app instructions. In bridge network mode, only the internal Docker container IP will be displayed which will not be helpful to users, and you won't be able to use Adguard as a DHCP server.

2) Change the default port of the setup wizard. By default it is at port 3000, but this will clash with Thunderhub. If a user tries to install both apps, they will encounter a port clash and the app they try to install second will fail. Unfortunately, Adguard doesn't support environment variables that we can pass to the container. We also don't want to supply a default AdGuardHome.yaml because we then lock ourselves in to making sure that this yaml is compatible with every app update we issue. It is best to allow Adguard itself to generate the config file and allow users to set up their own account, DNS listen interface, etc without locking them in.

We can use the new-ish [command-line argument `--web-addr` ](--web-addr) to set the port for the setup wizard. The user will still have to set the correct port for the webUI during set-up, but at least we can avoid an initial port clash. We can set this by overriding the [command in the Dockerfile](https://github.com/AdguardTeam/AdGuardHome/blob/71c44fa40ca7ca7c2a2d8df0e2d0ae9d8377d31e/docker/Dockerfile#L55-L59) with our own command in the docker-compose.yml and including `--web-addr`. For each update it will just be important to check if the Adguard team has modified the command in anyway and make the equivalent change to the command in the docker-compose.yml.

3) Include instructions in the app description that users need to set the web UI to a specific port (the one that we'll include in the umbrel-app.yml... which will be the same as the setup wizard port). This means when users complete the setup wizard, they will be redirected to the proper page, and clicking on the app icon on the umbrelOS homescreen will take users to the web UI. Since this is such a critical step, it might even be a good idea to change the first gallery image to describe this step. Kind of like what we do here for Portainer.

What do you think of this approach?

Let me give this a quick test and push some changes based on my previous PR and the 3 points I gave above.

nmfretz commented 1 month ago

Here's what we had for gallery assets from last time. I'm thinking we should change out one of them with instructions for setting the web interface correctly, similar to what we do here for Portainer. icon 1 2 3

nmfretz commented 1 month ago

@Julienpeps I have pushed some commits and tested. Can you please take a look and let me know what you think?

Julienpeps commented 1 month ago

Hi @nmfretz, I've tested the changes and everything seems fine. Your proposed solutions seems great, provided the instructions are well-explained.

The logo and gallery images are nice too, except for the absence of the "instructions" image.

nmfretz commented 1 month ago

@Julienpeps the gallery assets are finalized and this is good to go. Thanks again for submitting Adguard Home 🙌!

I have made some final minor tweaks. Going live now.