Server port and admin server port can end up with the same value

[wolfgangwalther]

As found out in #3424 (comment).

Do this:

PGRST_SERVER_PORT=3000 PGRST_SERVER_HOST="localhost" PGRST_ADMIN_SERVER_PORT=3000 postgrest-with-postgresql-16 postgrest-run

and observe:

11/May/2024:21:30:27 +0200: Admin server listening on port 3000
11/May/2024:21:30:27 +0200: Listening on port 3000

One of them is listening on ipv4, the other on ipv6.

We should probably reject this case on the config level? Admin port should always be different from server port?

[develop7]

One of them is listening on ipv4, the other on ipv6.

Generally, a user expects us to stick to one protocol, right? Meanwhile, one way to reduce confusion would be reporting URLs of apps to reach them at instead of just port numbers (and we do have all the info required for it).

[develop7]

The problem here is the app server binds to [::1]:3000 and admin server binds to 127.0.0.1:3000 because bindPortTCP binds to whatever address available and localhost resolves in both [::1] and 127.0.0.1 addresses.

Anyway, with #3512 admin server is bound to the same address as main, so specifying the same port results in a runtime error:

❯  PGRST_SERVER_PORT=3000 PGRST_SERVER_HOST='localhost' PGRST_ADMIN_SERVER_PORT=3000 postgrest-with-postgresql-16 postgrest-run
postgrest-with-postgresql-16: You can connect with: psql 'postgres:///postgres?host=/run/user/1000/postgrest/postgrest-with-postgresql-16-BBy/socket' -U postgres
postgrest-with-postgresql-16: You can tail the logs with: tail -f /run/user/1000/postgrest/postgrest-with-postgresql-16-BBy/db.log
postgrest: Network.Socket.bind: resource busy (Address already in use)
Temporary directory kept at: /run/user/1000/postgrest/postgrest-with-postgresql-16-BBy

Although the above runtime error is rather uninformative. I wonder if I should validate the config and fail if port numbers match, or just add logs, like binding main app to <hostname>:<port>; binding admin app to <hostname>:<port>? The latter could be useful to admins either way, and the runtime error would make perfect sense either way: when ports are equal — the problem is with the PostgREST config; otherwise there's some other service occupying the same port.

[develop7]

Okay, we actually do diagnostic messages "[Admin server] listening on port <port>", but in the case of the port conflict neither of those is shown before the runtime error.

[wolfgangwalther]

Generally, a user expects us to stick to one protocol, right?

This sounds very plausible - but I'm not 100% sure.

I can easily imagine scenarios where I don't want this to be the case.

Isn't the whole problem here that we only have server-host, but not admin-server-host?

I think it's quite reasonable to have the admin server listen on 127.0.0.1 only, but the regular server on 0.0.0.0.

So even if it's the same protocol, we probably still need a way to allow setting different hosts.

Meanwhile, one way to reduce confusion would be reporting URLs of apps to reach them at instead of just port numbers (and we do have all the info required for it).

Agreed, that would be really good.

Overall, even given both admin-server-host and the display of URLs, I think we should still prevent using the same port number on both on the config level already. This only leads to confusion and ambiguity.

[develop7]

So, to summarize:

• an admin-server-host setting should be introduced, defaulting to server-host value
• we have to make sure the host & port of both app and admin servers are not completely equal
• we need to log both host & port either app got bound to (right now only the port is logged, which leads to confusion)

I'll cook a PR

[wolfgangwalther]

we have to make sure the host & port of both app and admin servers are not completely equal

I think it would be fine to block on the same port only. This leads to less confusion and is also much easier to do, because you know the port beforehand. The host could be "localhost" and "127.0.0.1" - and then it becomes much harder to tell whether those are equal or not. You could actually only check after binding to the interface. So I'd like the port-only check at startup, way before trying to open any sockets, more.