tkuester / taky

A simple python TAK server
MIT License
188 stars 43 forks source link

Unite "hostname" and "public_ip" with "server_address" #48

Open tkuester opened 2 years ago

tkuester commented 2 years ago

Alright, I think I made a major sin when building taky, and I think it's time to rectify this. Here's the proposal I'd like to make.

The Problem

Most ATAK clients will only look for the DPS on :8443, this is not an easily configurable client setting. This effectively constrains you to a single taky-dps server per IP address. However, if a user runs HAproxy, they can run multiple instances of taky-dps in docker, and use hostname based routing to divert traffic to the correct server.

Unfortunately, taky currently builds all URLs based on the public_ip config setting.

Discussion

From a configuration perspective, public_ip and hostname have different semantic meanings. public_ip should never be set to a DNS hostname, and while hostname could technically be an IP address, it should not.

Theoretically, a user COULD use hostname with an IP address (though the code does not support this yet). However, this would negatively affect end user experience, as the list of servers would no longer have a useful string to help them identify which server they are connecting to.

Proposal

There are a few different ways to rectify this. This is the one I like the best: remove the hostname and public_ip from the configuration file, and replace it with server_address. The user can either specify a fixed IP address, OR they can specify a FQDN. server_address will be used for all certificates, URLs, and networking.

However, to address users who do not have a domain name, and wish to use a static IP address, a new field should be added called server_title. When specified, it will override server_address for the title in the client data package.

However, this is a breaking configuration change. I propose adding this to the 0.9 release of taky. If hostname and/or public_ip are specified, they will be used as they have been in 0.8 -- but a warning will be displayed on screen letting users know they should migrate their configuration. However, if hostname and public_ip are not specified, sever_address and server_title will be used as discussed in this proposal.

Please let me know what you think in discord, or this issue!

tkuester commented 2 years ago

From the discussion today, @dbussert made the excellent observation that explicitly setting a host/ip address was a bit of an anti-pattern. Instead, Flask should present the server's address, protocol, and port in the request object -- and this will simply respond with the same URL root used to access the page.

We should still test this to confirm that it still works through HAproxy -- but for now, an easy win.

tkuester commented 2 years ago

Should be closed with 2aea869.

In addition to the above notes, if server_address is not specified, it is set by socket.getfqdn().

tkuester commented 1 year ago

Bumping this to the next release.

nerdalertdk commented 1 year ago

Still working on this ?

Would love to run multiple taky servers on same host I exclusively use docker with proxy a in front :)