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

feat: split ips; internal local ip when on LAN, external ip otherwise #10307

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

midzelis
Copy link
Contributor

New feature for selfhosters:

The environment variable:
PUBLIC_SERVER_URLS

If you run Immich on your local network, you want to access it using the fast local network when you are at home, on a LAN, but still access it when you are away from home, using the public address.

Lets say your public address is 10.10.10.1 (at immich.example.com) and your internal address is 192.168.1.1.

If you specify PUBLIC_SERVER_URLS=http://192.168.1.1:2283/api,http://immich.example.com then when the immich frontend code loads, it will try to access the API server first at 192.168.1.1, then at immich.example.com.

If you on a LAN, the first ip will resolve and you'll access it on a fast internal network.

Once you leave your home network, the first request will fail, and will fallback to the external domain, which will access the server over your public (slower) internet uplink.

This PR also adds a loading/stencil during the time its searching for the right server. The spinner only appears after .3 seconds, so it doesn't flicker on very short loads.

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Did you try this in a production build? I'm pretty sure it just won't work. None of this code is executed server side so accessing the environment variable is not really feasible afaik

@zackpollard
Copy link
Contributor

zackpollard commented Jun 14, 2024

Aside from the problems @jrasm91 mentioned, we also have had this issue come up before and have always said that this should just be resolved with split DNS. This isn't something we want to handle within Immich itself

@midzelis midzelis marked this pull request as draft June 15, 2024 14:07
@midzelis
Copy link
Contributor Author

Just a few comments - I put this PR back into draft, as its really not ready, and there are a few things I want to change. However, thank you for your feedback!

Split Horizon DNS could work, but there are a bunch of problems with it.

  1. Its really hard for beginners to setup, as it requires router changes, DHCP settings, etc,
    • especially since we really want to focus on making immich as easy as possible to install and run.
  2. Caching issues with DNS records
  3. Make things like DNSSEC difficult.
  4. Generally, hard to diagnose problems when you start mucking around with DNS
    • its very hard to understand, even for experts!
    • is it dns?

However, theres another thing that this feature also enables, besides just getting a more efficient route to the server.

  1. Hosting the web frontend code on a separate server/domain than the api backend.
    • You could host the frontend on an CDN or edge server like github pages, cloudflare, or fly.io - but still access my photos from the backend

The code in the PR up till now is making some unrelated changes which I'll split out. However, it also requires some config changes enable CORs on the /api backend server.

I've started prototype work on what it would take, cookies over cross-domain CORs will require SSL. I think its a best practice to avoid SSL/CORs from the backend application code, and leave that up to the load-balancer/proxies. Additionally, the load-balancer can statically serve the SPA web application.

My new approach would be to make most of those changes in the load-balancer/reverse-proxy, so they become configuration instead of code. Most of this can be done via docker-compose file. This won't eliminate app code changes, but they will should be minimal.

I'm starting to think that our example docker-compose files should configure caddy/nginx to do SSL termination, serving the SPA frontend code statically (taking load off of express backend service), managing CORs. Ideally, for security, running in a non-root, unprivileged container.

It is a best practice to avoid handling traffic in a container, since containers are not security barriers - its relatively easy to break out of a docker container. I need to research caddy a bit more - but maybe it can be run unprivileged. Once we figure out a secure way to host it, I think we should/could provide this setup as an example. We should be secure-by-default, and should encourage security best practices for all those that run this at home.

I'm all for reducing the number of container we require - but containers are just processes after all, and I think it would be very appropriate to have a single secure container be the ingress for the server.

To @jrasm91's question - using import { env } from '$env/dynamic/public'; will generate a env.js file during the build. If you are using the prebuilt images, this file would be empty, since the env var would not be there during the build.

You could build the image yourself, and it will be included if you add it to the ENV during build.
However, I'd imagine the better way would be to simply use the pre-build images and do a bind mount:

[snip]
    volumes:
      - ./my-env.js:/usr/src/app/build/_app/env.js

@bo0tzz
Copy link
Member

bo0tzz commented Jun 15, 2024

Hosting the web frontend code on a separate server/domain than the api backend.

We have the .well-known/immich endpoint that is used for the mobile app to find where the backend lives. I would expect that to be used by the web as well, but I'm not sure whether it actually is. That would be able to cover this same case as well.

We used to have an nginx container in the stack that routed between a web container and the api backend. I don't think we should reintroduce something like that, also because people run many different configurations (most with their own reverse proxy already) and so shipping our own proxy just interferes with that.

Tbqh I find mounting a js file to be a pretty unappealing way of passing these env vars.

@jrasm91
Copy link
Contributor

jrasm91 commented Jun 15, 2024

Also the API serves the web, so the API path is always going to be /api. I don't see any benefit to running the web on a separate process or container. That introduces complexity self hosted people don't seem to need or want. Now you start getting into cors, connectivity and networking.

@alextran1502
Copy link
Contributor

I think the issue with split IPs should be handled on the mobile code base since it is the most applicable use case, right?

@GewoonJaap
Copy link

GewoonJaap commented Jun 24, 2024

I think the issue with split IPs should be handled on the mobile code base since it is the most applicable use case, right?

I think it would be great for both mobile and web. In the Home Assistant app you configure your internal URL and Wifi SSID and then the app switches URLS accordingly. Something similar for the Immich mobile app would be great. For the web maybe let the user configure a list of URLs which the frontend will try to reach

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.

None yet

7 participants