search

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
481 stars 356 forks source link

Add ntfy #1118

Closed sharknoon closed 3 weeks ago

sharknoon commented 1 month ago

App Submission

App name

ntfy

256x256 SVG icon

logo

Gallery images

Bildschirmfoto am 2024-06-02 um 13 58 39 Bildschirmfoto am 2024-06-02 um 14 00 44 Bildschirmfoto am 2024-06-02 um 14 00 30 Bildschirmfoto am 2024-06-02 um 13 59 04 Bildschirmfoto am 2024-06-02 um 13 58 57 Bildschirmfoto am 2024-06-02 um 13 58 52

I have tested my app on:

nmfretz commented 4 weeks ago

Thanks for submitting ntfy @sharknoon! I'll take a look at this tomorrow.

nmfretz commented 4 weeks ago

This is a super fun app @sharknoon, I just played around with it. Excellent job packaging it; we'll start getting gallery images prepared.

I'm hoping to bounce a couple initial thought off you from testing:

1) Instant notifications for iOS users

Unfortunately it looks like instant notifications for iOS clients isn't as seemless as other platforms. Without additional configuration it can take hours for an iOS device to receive a notification: https://docs.ntfy.sh/config/#ios-instant-notifications

To get around this, we can set NTFY_UPSTREAM_BASE_URL to https://ntfy.sh in the compose file. This forwards a poll request (not the actual notification) to the upstream ntfy.sh server, which sends the poll request to your iOS device so that it can download the notification from the self-hosted ntfy instance.

I'm a bit hesitant to force all users to use the centralized https://ntfy.sh server for poll requests, but the app becomes essentially unusable for iOS users without it. What are your thoughts here on including this by default vs providing instructions to add it manually for iOS users?

2) Exposing ntfy to the internet

For receiving notifications outside of a users local network, there are probably two easy ways to do this right now:

But either way there is a hiccup, which is that the environment variable NTFY_BASE_URL must match what's in the client for two features: 1) instant notification on iOS 🤦‍♂️ 2) features like sending attachments (since a download link is sent).

Of course, advanced users can go in and change the NTFY_BASE_URL in the compose file, but I'm wondering if we should try and make this "just work" for most users by detecting whether they have tailscale installed and if so, set NTFY_BASE_URL tohttp://<your-tailscale-short-domain>:13119 , and include instructions in the app description. I'm torn on this.

sharknoon commented 4 weeks ago

Thank you @nmfretz for your comprehensive review 😀 Yes, this app is a bit tricky in many ways. I have spend a lot of hours to create the default user on startup and make it secure by default.

1) Instant notifications for iOS users

According to the docs the default value for NTFY_UPSTREAM_BASE_URL is already https://ntfy.sh, so we do not need to change that manually. I think we should provide the best possible expirience by default, but let the users know (in the description), that we are using this service, due to Apples restrictions.

This way, expert users can still change it if they want to.

2) Exposing ntfy to the internet

This is a tricky one. I don't think there is a good solution at the moment until Umbrel OS brings official support for domains. For now I also think checking for Tailscale is the right solution.

Actions

Questions

highghlow commented 4 weeks ago 
* How do I detect Tailscale? The `exports.sh` file does'nt export a env variable.

I guess you can try pinging of the tailscale's containers tailscale_<name>_1

nmfretz commented 4 weeks ago

I have spend a lot of hours to create the default user on startup and make it secure by default.

I noticed that with your custom entrypoint. Great work on that 👌 I was super impressed.

1) Instant notifications for iOS users

According to the docs the default value for NTFY_UPSTREAM_BASE_URL is already https://ntfy.sh, so we do not need to change that manually.

Ah ya I saw that in that table as well, but it's actually a documentation mistake on their part. When testing I found that it actually isn't the case, and that the default is no upstream base url. We need to include NTFY_UPSTREAM_BASE_URL: https://ntfy.sh in the compose file ourselves for it to work on iOS devices (tested with my iPhone).

I will add a note in the description to explain the iOS instant notification situation and a short guide how to change the upstream base url.

Perfect, thanks for doing that, it looks great 🙏.

2) Exposing ntfy to the internet

This is a tricky one. I don't think there is a good solution at the moment until Umbrel OS brings official support for domains. For now I also think checking for Tailscale is the right solution.

Ya, completely agree with you that there isn't a good solution until domains are supported. In fact, I'm kind of second guessing my suggestion to auto-detect Tailscale and change NTFY_BASE_URL because it may make it even more confusing for users.

Shall we just ship the app as-is for now and see how people do with it?

How do I detect Tailscale? The exports.sh file doesn't export a env variable.

@sharknoon @highghlow here is our current script logic for apps. Anything in here will work on both pre 1.0 and post 1.0 umbrelOS: https://github.com/getumbrel/umbrel/blob/master/packages/umbreld/source/modules/apps/legacy-compat/app-script

So if we wanted to create a pre-start script that checks if Tailscale is installed we could do something like:

if "${UMBREL_ROOT}/scripts/app" ls-installed | grep --quiet 'tailscale'
do something

And we can use the script to pass anything to compose, so if the Tailscale container was up and running we could pass a command like this:

"${UMBREL_ROOT}/scripts/app" compose tailscale exec web <command>

But we'd probably have to include logic to wait until the Tailscale container is up and running (e.g., if umbrelOS is booting and ntfy comes online before tailscale) so that we can query it for the short-domain name. For 99% of users the domain name would be umbrel, but if they have multiple Umbrels connected to their Tailscale then it may be different (e.g., umbrel-1).

sharknoon commented 3 weeks ago

Very good catch, that the ntfy docs are wrong @nmfretz 👍 I have added the NTFY_UPSTREAM_BASE_URL to the compose file.

I agree with you, that adding a manual check for Tailscale is kind of a hack. It doesn't work for other Tunneling / VPN apps like WireGuard or Cloudflare Tunnels.

I propose we leave the NTFY_BASE_URL as it is for now and revisit it as soon as domain support for Umbrel OS is out.

nmfretz commented 3 weeks ago

I propose we leave the NTFY_BASE_URL as it is for now and revisit it as soon as domain support for Umbrel OS is out.

Sounds good! Okay, once the gallery assets are ready we will go live.

nmfretz commented 3 weeks ago

We're going live with ntfy! Thanks again for the huge amount of work you put into this one @sharknoon.

Gallery assets are here: https://github.com/getumbrel/umbrel-apps-gallery/tree/master/ntfy

image
sharknoon commented 3 weeks ago

You are welcome 😀 Thank for the corrections of the umbrel-app.yml, I totally forgot about downgrading the manifest version again after deciding to go with a custom entrypoint instead of hooks.