Skip to content

Revert to predictable upstream names, optional SHA1 names #1736

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

Merged
merged 3 commits into from
Aug 19, 2021

Conversation

buchdag
Copy link
Member

@buchdag buchdag commented Aug 17, 2021

I decided to backpedal and revert both #1163 and #1725 so that people that relied on predictable upstream blocks names won't have their setup break on them.

As an alternative fix to #1162, I added an optional SHA1_UPSTREAM_NAME environment variable to nginx-proxy which, when set to true, will forces the upstream block's names to SHA1 hashes of the hostnames in the same manner as #1725 did.

@cheald I believe this PR will solve your original issues without breaking stuff for others like the previous solution did.

@akpircher, @tkw1536, @acortelyou, @EricSoroos sorry for the back and forth on this one.

@buchdag buchdag added type/fix PR for a bug fix type/feat PR for a new feature type/test PR that add missing tests or correct existing tests labels Aug 17, 2021
@tkw1536
Copy link
Collaborator

tkw1536 commented Aug 17, 2021

I am fine with the content of this PR, but I'm not sure the word "predictable" in the documentation is correct. IMO the sha1 hash of a domain name is perfectly predictable - maybe unhashed vs sha1 instead?

@buchdag
Copy link
Member Author

buchdag commented Aug 18, 2021

@tkw1536 I think I'll go with Unhashed vs SHA1 upstream names.

@buchdag buchdag force-pushed the predictable-upstream branch from 3226488 to a33af34 Compare August 19, 2021 09:41
@buchdag buchdag merged commit 8adbea8 into main Aug 19, 2021
@buchdag buchdag deleted the predictable-upstream branch August 19, 2021 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feat PR for a new feature type/fix PR for a bug fix type/test PR that add missing tests or correct existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants