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

Improve http_service.checks documentation #1862

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Conversation

scottohara
Copy link
Contributor

Summary of changes

For the method and protocol options, indicate that the default values (if omitted) are get and http, respectively.

Additionally, add a warning section that warns about common issues with redirects (or any non-200 response).

One such example is that, by default, a standard Rails application includes config.force_ssl = true in config/environments/production.rb, which causes a health check request to http://0.0.0.0:3000/up to receive a 301 redirect to https://0.0.0.0:3000/up (and the default puma config for Rails is not configured to accept SSL requests).

The warning includes some guidance about adding an X-Forwarded-Proto = "https" header to the health check to avoid this sort of redirect.

Related Fly.io community and GitHub links

https://community.fly.io/t/http-service-checks-random-questions/22424

For the `method` and `protocol` options, indicate that the default values (if omitted) are `get` and `http`, respectively.

Additionally, add a warning section that warns about common issues with redirects (or any non-200 response).

One such example is that, by default, a standard Rails application includes `config.force_ssl = true` in `config/environments/production.rb`, which causes a health check request to http://0.0.0.0:3000/up to receive a 301 redirect to https://0.0.0.0:3000/up (and the default puma config for Rails is not configured to accept SSL requests).

The warning includes some guidance about adding an `X-Forwarded-Proto = "https"` header to the health check to avoid this sort of redirect.
Linter complains about http and https, so hoping that wrapping these in backticks is enough to silence these warnings.
@scottohara
Copy link
Contributor Author

Just bumping this low priority PR as it may have slipped off everybody’s radar.

There was initially some linter errors on the first build attempt, but I believe they have been addressed so this seems to just be awaiting an approval to rerun the workflow.

Thanks.

@mrkurt mrkurt merged commit b9fc20c into superfly:main Dec 16, 2024
@scottohara scottohara deleted the patch-1 branch December 16, 2024 20:57
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