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

Bugfix: don't stop proxies when using hostnames to specify listen addresses #631

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

robinbrandt
Copy link
Contributor

Before this change we'd stop existing proxies when populating a set of proxies when the listen addresses were specified using hostnames instead of IP addresses.

This is very surprising as we encourage users to populate proxies at the begin of an application start:

A /populate call can be included for example at application start to ensure
all required proxies exist. It is safe to make this call several times, since
proxies will be untouched as long as their fields are consistent with the new
data.

This is (currently) only partially true as it doesn't work when using hostnames.

For upstream services this isn't a problem because we never store the resolved address.

The change introduces a "Differs" function on a proxy to determine if two proxies are different in terms of listening address and upstream service. This is now used both when changing collections as well as updating individual proxies.

Closes #131.

@robinbrandt robinbrandt force-pushed the fix-hostnames-reset-proxies branch from f9d1f64 to 518377c Compare March 17, 2025 14:17
Copy link
Member

@abecevello abecevello left a comment

Choose a reason for hiding this comment

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

Looks good!

…resses

Before this change we'd stop existing proxies when populating a set of proxies
when the listen addresses were specified using hostnames instead of IP
addresses.

This is very surprising as we encourage users to populate proxies at the begin
of an application start:
> A /populate call can be included for example at application start to ensure
> all required proxies exist. It is safe to make this call several times, since
> proxies will be untouched as long as their fields are consistent with the new
> data.

This is (currently) only partially true as it doesn't work when using hostnames.

For upstream services this isn't a problem because we never store the resolved address.

The change introduces a "Differs" function on a proxy to determine if two
proxies are different in terms of listening address and upstream service. This
is now used both when changing collections as well as updating individual
proxies.

Closes #131.
@robinbrandt robinbrandt force-pushed the fix-hostnames-reset-proxies branch from 518377c to 5ca36b1 Compare March 17, 2025 15:18
@robinbrandt robinbrandt merged commit 64cb722 into main Mar 17, 2025
6 checks passed
@robinbrandt robinbrandt deleted the fix-hostnames-reset-proxies branch March 17, 2025 15:31
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.

Populate doesn't compare hostnames properly
2 participants