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

Bind the health check service to localhost #391

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

stephen-soltesz
Copy link
Contributor

@stephen-soltesz stephen-soltesz commented Jun 30, 2023

This change updates the default -health_addr to explicitly bind to localhost:8000 to prevent accidentally exposing this service to the public internet. This change should have not effect since the heartbeat service already targets localhost unconditionally

Part of:


This change is Reviewable

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

:lgtm: with one comment, possibly needing no action.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)


ndt-server.go line 41 at r1 (raw file):

	ndt5WsAddr        = flag.String("ndt5_ws_addr", "127.0.0.1:3002", "The address and port to use for the ndt5 WS test")
	ndt5WssAddr       = flag.String("ndt5_wss_addr", ":3010", "The address and port to use for the ndt5 WSS test")
	healthAddr        = flag.String("health_addr", "localhost:8000", "The address and port to use for health checks")

Could this use 127.0.0.1 instead, to be consistent with the value for ndt5WsAddr? Or will using localhost cause it to bind to both the IPv4 and IPv6 loopback addresses?

Copy link
Contributor Author

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade)


ndt-server.go line 41 at r1 (raw file):

Previously, nkinkade wrote…

Could this use 127.0.0.1 instead, to be consistent with the value for ndt5WsAddr? Or will using localhost cause it to bind to both the IPv4 and IPv6 loopback addresses?

That's a very good call. I've updated to use 127.0.0.1.

@stephen-soltesz stephen-soltesz merged commit 8a6fa8a into main Jun 30, 2023
@stephen-soltesz stephen-soltesz deleted the sandbox-soltesz-local-health branch June 30, 2023 21:26
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