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

allow fastrouter subscriptions to use hostnames #2012

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

terencehonles
Copy link
Contributor

@terencehonles terencehonles commented May 7, 2019

Update: Backported in #2013

The following patch allows the following hostname subscriptions:

--subscribe-to NODE_HOSTNAME:3031=ROUTER_HOSTNAME:2626:FASTROUTER_KEY
--subscribe2 addr=NODE_HOSTNAME:3031,server=ROUTER_HOSTNAME:2626,key=FASTROUTER_KEY

instead of manually resolving the IP addresses:

--subscribe-to 10.0.0.2:3031=10.0.0.1:2626:FASTROUTER_KEY
--subscribe2 addr=10.0.0.2:3031,server=10.0.0.1:2626,key=FASTROUTER_KEY

Addresses are resolved by the fastrouter client rather than the server so I didn't have to update the request verification on the server (signing). This also means that new clients can understand hostnames without having to restart the server. If server side address resolution is preferred I can look at updating fastrouter/corerouter, but an initial investigation suggested it was not a simple change.

In addition to above, I also made the following changes:

  • I clarified the subscribe-to parsing by noting that addr can actually be specified as long as = isn't the first character of the "named socket" (and as mentioned above I added hostname resolution on that code path). NOTE: should this be updated in the documentation? Currently the docs suggest you need to use subscribe2, but instead of closing the undocumented behavior I updated it to match the handling I was adding to subscribe2
  • I updated the variable "socket_name" to "address" in the functions that are populating the uwsgi subscription request buffer so the variable name matches the key being populated ("address").

@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch 2 times, most recently from 3b4739e to 310951c Compare May 8, 2019 17:18
@terencehonles
Copy link
Contributor Author

@xrmx Any chance this can get reviewed?

@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 310951c to 4f81d6f Compare July 8, 2020 23:58
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 4f81d6f to 3604ab0 Compare November 1, 2022 10:52
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 3604ab0 to d639098 Compare September 7, 2023 11:51
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from d639098 to 8396089 Compare November 16, 2023 02:50
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 8396089 to 3d71183 Compare February 9, 2024 09:31
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 3d71183 to fb38663 Compare April 26, 2024 08:44
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from fb38663 to 8322fa0 Compare June 5, 2024 18:57
@terencehonles terencehonles force-pushed the update-fastrouter-subscriptions-to-allow-hostnames branch from 8322fa0 to 8055cda Compare October 10, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant