search

DBCDK / morph

NixOS deployment tool
MIT License
839 stars 63 forks source link

Headers in HTTP health checks #65

Closed joepie91 closed 5 years ago

joepie91 commented 5 years ago

A look at the code suggests that there is support for specifying headers to pass along when doing HTTP healthchecks. The Nix option specification however, says that this is not implemented.

It's not clear whether headers are currently supported or not. But if they are not, it would IMO be quite important to support them; especially when setting up a server for which no public DNS records exist yet, you'll often want to test vhosts and such by making requests to the machine's hostname with a spoofed Host header.

What's the current status of header support, and if unsupported, are there any plans to add support for them?

(Aside, I'm quite enjoying morph so far, I'm finding it much easier to reason about than NixOps!)

johanot commented 5 years ago

Hello @joepie91. Thanks for using morph and for reporting this :-) A quick look at the code suggests that we do support http-headers at the moment: https://github.com/DBCDK/morph/blob/master/healthchecks/types.go#L115

In that case, we just need to change the option description. Will do a quick test with headers and PR the doc change asap.

joepie91 commented 5 years ago

Great :)

I did seem to run into issues while testing it out, but because of a deadline I didn't have time to investigate it further. I'm not certain whether it actually works in practice, currently, or whether I just did something wrong myself.

johanot commented 5 years ago

Well.. Looks like you found a bug afterall :-) Headers were set on the Request struct, however that was never used when the actual request was made, so it had no effect. #66 fixes that, and implements special handling (apparently) needed allowing the host-header to be overridden.