Skip to content

Add a valid Upstream to the DialInfo when doing active health checks #6949

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 1 commit into from
Apr 15, 2025

Conversation

jbro
Copy link
Contributor

@jbro jbro commented Apr 11, 2025

Currently if we extract the DialInfo from a Request Context in a custom Transport during an active health check, then the Upstream in the DialInfo will be nil.

This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overridden in the active health check's config.

This is useful when using a custom Transport for the reverse_proxy module, as it makes GetDialInfo behave similarly in active health check requests and client requests.

Currently if we extract the DialInfo from a Request Context during an active health check, then the Upstream in the DialInfo is nil.

This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overriden in the active health check's config.
@jesper-chainalysis jesper-chainalysis force-pushed the fix/add-upstream-to-dialinfo branch from 3a649d7 to c96f6d7 Compare April 14, 2025 06:20
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks! I think this makes sense, though it's been a while since I've been working with this code. We'll give it a shot and let you know if we have any issues with it :)

@mholt mholt merged commit 6c38ae7 into caddyserver:master Apr 15, 2025
20 checks passed
@mholt mholt added this to the v2.10.0-beta.5 milestone Apr 15, 2025
@jbro jbro deleted the fix/add-upstream-to-dialinfo branch May 12, 2025 19:05
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.

2 participants