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

Fix bad gateway error in Seafile-app #1062

Closed ngutech21 closed 1 month ago

ngutech21 commented 2 months ago

This fixes 2 issues:

I have also updated seafile to the latest patch version and tested it on umbrel home 1.1.

nmfretz commented 2 months ago

Thanks for this PR @ngutech21, much appreciated. I'll be taking a look at this late this week / early next week.

ngutech21 commented 1 month ago

@nmfretz Did you find the time to review my PR?

nmfretz commented 1 month ago

Thanks for the ping @ngutech21. And thanks again for the fixes! Really great work.

bad gateway error after starting the app: The seafile container has to use the container-name seafile-mysql to access the database

Great catch. This slipped past in testing because if you only have one app installed with a service called db in the compose file, then Docker will always successfully resolve to the only service named db, but if you have another app with a service called db (there are many in the umbrel app store) then there is a name collision and Docker will resolve db randomly to one of them every time so sometimes it will work and sometimes it won't.

When connecting through one of the desktop seafile apps, there was an internal server error: This was caused by the additional proxy auth, that the apps could not handle so I deactivated it. Seafile already provides an authentification, so this seems redundant anyway.

Good idea to just remove auth, since seafile has it's own strong authentication. If for some reason we want to add back in the app_proxy auth in the future, then the other way we can get around this issue is by using the environment variable PROXY_AUTH_WHITELIST to whitelist whatever endpoint the desktop seafile app is trying to use. See invoice ninja as an example.

I have pushed a couple commits to make it so that existing installs get an app update notification.