reverseproxy: Active health checks request body option

[jbro]

Changes

Added the field Body to the ActiveHealthChecks struct, which optionally sets the body of the request used for the health check requests.

Also added Caddyfile support for the same option under the name health_request_body.

Rational

After adding the option to setting the HTTP method used for active health checks it turns out it's also helpful to set the body. Eg when using the POST method on a JSON RPC service with an empty body may return a 400 Bad Request, instead of a 200 OK. Being able to set the body to a valid NOOP RPC call enables a "deeper" health check, than just accepting the 400 status.

[mohammed90]

It may be useful to run it through the replacer 🤔

[francislavoie]

Agreed. Though in reality it would only let {env.*} and {file.*} work, not much else. There's no request context available here in active health checks because they happen in their own goroutines.

[jbro]

Oh yeah I did think about doing that, but then I guess I forgot all about it again :-)

I'll give it go.

[jbro]

OK, I've add a replacer to the body in my latest commit. I've moved the initialization of replacer used for the headers up, and reused that. Is that a good solution ?

[mholt]

Thank you for working on this!

It feels a little weird to set a body without a Content-Type, but maybe we can see if anyone needs that feature and then add a way to customize Headers. (Honestly it's starting to feel a lot like the respond directive, i.e. the static_response middleware, and I wonder if we would benefit from just using that instead. But that's a matter for another discussion.)

[francislavoie]

@mholt we have health_headers already, actually. Users setting a body can (and likely should) also set Content-Type that way.

[jbro]

It feels a little weird to set a body without a Content-Type, but maybe we can see if anyone needs that feature and then add a way to customize Headers. (Honestly it's starting to feel a lot like the respond directive, i.e. the static_response middleware, and I wonder if we would benefit from just using that instead. But that's a matter for another discussion.)

Hmm that's a very good point, and you are probably right that in a weeks time I'll probably find out that I need to set a Content-Type, luckily there is already an option to add headers to the request :-)

I just checked http.Request, and I don't think anything of value can't be set after this PR is merged. I mean the protocol version is not something I usually touch, and we probably don't want to explicitly set the content length either. The only maybe could be trailers?

[mholt]

@francislavoie Ah you're right, I thought that was for headers to check for healthy status, but it seems we are using that to set them.