Add http probe for CLI healthcheck/readiness/liveliness #452
Conversation
bensheldon
force-pushed
the
probe_server
branch
from
e6e4193 to
3edadbd
Compare
bensheldon
force-pushed
the
probe_server
branch
from
3edadbd to
792a288
Compare
bensheldon
force-pushed
the
probe_server
branch
from
792a288 to
f3de849
Compare
| @@ -54,6 +54,7 @@ Gem::Specification.new do |spec| | |||
| spec.add_dependency "fugit", ">= 1.1" | |||
| spec.add_dependency "railties", ">= 5.2.0" | |||
| spec.add_dependency "thor", ">= 0.14.1" | |||
| spec.add_dependency "webrick", ">= 1.3" | |||
There was a problem hiding this comment.
Is there a reason why you went for a very old version rather than a version that contains fixes for vulnerabilities? I assume it's for broad compatibility with all kinds of setups, but since webrick 1.7 supports Ruby 2.3 and above and GoodJob Ruby 2.5 and above, we could set this higher and avoid having people install versions with CVEs?
There was a problem hiding this comment.
I didn't put much thought into it, though I'm not quite sure how I came to v1.3 as it looks like that shipped with Ruby 2.4, which GoodJob does not support.
I think of the gemspec as descriptive of compatibility; I think security should be considered at the application that is consuming the gemspec. Trying to balance:
- if the application is not previously including webrick, that bundler is not defaulting to an insecure version (I don't know how Bundler decides whether to pull the latest version, or just whatever it has lying around locally)
- if the application is previously including webrick, not forcing the application to update the version of webrick because that might be a blocker for the adopting/updating GoodJob in a controlled environment.
The other thing I did consider, rather than pulling in webrick, was just pasting in one of the many "rack-compatible http server in 30 lines of ruby" and packaging it directly. These health check requests are very, very simple and really only intended to be used internally.
| end | ||
|
|
||
| def start | ||
| @handler = Rack::Handler.get(RACK_SERVER) |
There was a problem hiding this comment.
@bensheldon if you use Rack::Handler.default here you don't need the dependency on webrick and the handler can be set with an environment flag—it would be nice if it's user-configurable somehow
There was a problem hiding this comment.
Ooh, that's interesting. When trying that change, it defaults to Puma, but Puma intercepts ctrl-c/Interrupt which becomes really problematic :-(
This isn't intended to be anything more than a container healthcheck, which makes me think that I should explore pasting in a "rack-compatible http server in 30 lines of ruby" snippet of code and avoid the dependency altogether.
There was a problem hiding this comment.
Ah I see, then webrick makes more sense 👍🏻
Connects to #403. Adds separate endpoints that are descriptive of GoodJob's state (started/connected) because I do not want to be opinionated about what constitutes an appropriate probe.
This PR uses WEBrick as the http server. Ruby 3.0 removes webrick from the stdlib and thus it has to be included in the gemspec.