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

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

Open
tkuester opened this issue Nov 21, 2021 · 4 comments
Open

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

tkuester opened this issue Nov 21, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request fix-committed
Milestone

Comments

@tkuester
Copy link
Owner

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 tkuester added enhancement New feature or request question Further information is requested labels Nov 21, 2021
@tkuester tkuester added this to the Release 0.9 milestone Nov 21, 2021
@tkuester tkuester self-assigned this Nov 21, 2021
tkuester added a commit that referenced this issue Nov 22, 2021
In efforts to decrease the complexity of server configuration (see #48)
and to increase flexibility with DNS or IP addresses, we decided to
experiment with letting gunicorn/flask return with the URL root that was
used to make the request.
@tkuester
Copy link
Owner Author

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
Copy link
Owner Author

Should be closed with 2aea869.

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

@tkuester tkuester added fix-committed and removed question Further information is requested labels Dec 31, 2021
@tkuester
Copy link
Owner Author

Bumping this to the next release.

@tkuester tkuester modified the milestones: Release 0.9, Release 1.0 Jan 21, 2023
@nerdalertdk
Copy link

nerdalertdk commented Aug 10, 2023

Still working on this ?

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fix-committed
Projects
None yet
Development

No branches or pull requests

2 participants