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.

Sign up for GitHub

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

Jump to bottom

reverseproxy: Fix dynamic upstreams ip version #6448

Merged
merged 1 commit into from
Jul 9, 2024
Merged

reverseproxy: Fix dynamic upstreams ip version #6448

merged 1 commit into from
Jul 9, 2024

Conversation

armadi1809
Copy link
Contributor

Closes #6442

@armadi1809 armadi1809 force-pushed the aar-fix-dynamic-upstreams-ip-ver branch from 8d82da4 to cc10b08 Compare July 9, 2024 00:21
@francislavoie
Copy link
Member

francislavoie commented Jul 9, 2024 •
edited
Loading

I think a test would be useful for this if you have time to look into that 🤔 we could refactor this logic to a separate function taking au.Versions (nilable pointer) and returning a tuple of booleans, so it can be tested as a unit.

@armadi1809 armadi1809 force-pushed the aar-fix-dynamic-upstreams-ip-ver branch 3 times, most recently from 23fa658 to fa36179 Compare July 9, 2024 03:33
@armadi1809
Copy link
Contributor Author

@francislavoie I just refactored the logic determining the ip version into its own function and added a test covering all possible combinations of au.Versions.

francislavoie
francislavoie approved these changes Jul 9, 2024
View reviewed changes
Copy link
Member

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

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

Thanks! Looks great 😊

modules/caddyhttp/reverseproxy/upstreams.go Outdated Show resolved Hide resolved
@francislavoie francislavoie changed the title Dynamic upstreams ip version resolving bug fix reverseproxy: Fix dynamic upstreams ip version Jul 9, 2024
@armadi1809 armadi1809 force-pushed the aar-fix-dynamic-upstreams-ip-ver branch from fa36179 to c0aa76f Compare July 9, 2024 04:13
@francislavoie francislavoie merged commit 630c62b into caddyserver:master Jul 9, 2024
23 checks passed
@calvinbrewer
Copy link

Still running into this issue, but it looks like a release hasn't been cut since June 2nd. What is the release cadence for deploying these new changes? Cheers!

@BrewTestBot BrewTestBot mentioned this pull request Dec 31, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francislavoie francislavoie francislavoie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

can't disable IPv4/IPv6 for dynamic reverse proxy from Caddyfile
3 participants
@armadi1809 @francislavoie @calvinbrewer