Can't use HTTP health check in front proxy to vhosted server

[pingles]

Title: Upstream health checks fail when the upstream doesn't recognise the host cluster_name.

Description:
Currently envoy allows HTTP health checks to be performed against upstreams configured for the cluster. We're trying to use envoy as a load-balancing proxy which is connecting to some nginx processes that sit behind different ELBs. These nginx processes are configured with multiple virtual hosts that don't match the service name (but instead match the address we use).

Config
For example,

  clusters:
  - name: some_service
    type: STRICT_DNS
    health_checks:
      timeout: 5s
      interval: 30s
      unhealthy_threshold: 3
      healthy_threshold: 1
      http_health_check:
        path: "/"
    hosts: 
    - socket_address: { address: "app.elb1.domain", port_value: 443 }
    - socket_address: { address: "app.elb2.domain", port_value: 443 }
    - socket_address: { address: "app.elb3.domain", port_value: 443 }

In this situation I was expecting the health check to be performed having set Host to be the address used (app.elb1.domain).

It's difficult to know whether this is just a total misuse of envoy (sorry if so ;), or whether its a topology that should/could be supported. If so, I'd imagine the health check would need some kind of host_rewrite option so that it'd use the address of one of the configured hosts, or perhaps something like this:

http_health_check:
  path: /
  host: ADDRESS

As I said, not really sure whether this is really an issue or just misplaced expectation :) Let me know if I can help more.

[mattklein123]

In the current implementation Envoy sets the host header for HTTP/1.1 health checks to the name of the cluster, since health checking is distinct from service discovery. We could easily add an override for what to set host to. This is a feature request so I will mark it help wanted (it's also easy so will mark it beginner if you are interested).

[pingles]

I’ve been looking for an excuse to try and get into a bit of modern C++ ;) Never done any professionally but will give it a try- envoy seems like a great place to start!!

Any thoughts/preferences on how to express the override?

[mattklein123]

Hah. I was about to tell you to make a data-plane-api change, but it's already there, just not implemented: https://github.com/envoyproxy/data-plane-api/blob/master/envoy/api/v2/health_check.proto#L60.

You just need to implement this field which is already in the proto. The host is set here:
https://github.com/envoyproxy/envoy/blob/master/source/common/upstream/health_checker_impl.cc#L336

So it's basically just reading that field from the config, if not empty, using it (and adding a test here https://github.com/envoyproxy/envoy/blob/master/test/common/upstream/health_checker_impl_test.cc)

[pingles]

Excellent- yeah saw the health check proto and wondered whether it was about to be supported. Thanks for the pointers, I'll try and get something going and get back to you.

…

On Thu, Jan 25, 2018 at 11:14 PM, Matt Klein ***@***.***> wrote: Hah. I was about to tell you to make a data-plane-api change, but it's already there, just not implemented: https://github.com/envoyproxy/ data-plane-api/blob/master/envoy/api/v2/health_check.proto#L60. You just need to implement this field which is already in the proto. The host is set here: https://github.com/envoyproxy/envoy/blob/master/source/ common/upstream/health_checker_impl.cc#L336 So it's basically just reading that field from the config, if not empty, using it (and adding a test here https://github.com/envoyproxy/ envoy/blob/master/test/common/upstream/health_checker_impl_test.cc) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAEfhMTlYgDXv0Jhm59vbmJ6Ipjcd_rks5tOQrvgaJpZM4Rs7qx> .

[dio]

@pingles I have it working in my local using this commit, it was required for a use-case in my org. But since I found that making an additional test to health_checker_impl_test.cc is not trivial for me (guess we need to have a facility to inspect the request header [to check if the sent Host value from the HC agent is correct as defined inside the config] via a decoder a test filter, but yeah I haven't found a way to do so).

[pingles]

Ah cool, I haven’t had a chance to take a look (been distracted with some other things at work). I’ll try and take a look next week and see if I can help, thanks for mentioning it!

…

On Sat, 10 Feb 2018 at 07:19, Dhi Aurrahman ***@***.***> wrote: @pingles <https://github.com/pingles> I have it working in my local using this commit <dio@4944380>, it was required for a use-case in my org. But since I found that making an additional test to health_checker_impl_test.cc <https://github.com/envoyproxy/envoy/blob/master/test/common/upstream/health_checker_impl_test.cc> is not trivial for me (guess we need to have a facility to inspect the request header [to check if the sent Host value from the HC agent is correct as defined inside the config] via a decoder a test filter, but yeah I haven't found a way to do so). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAEfujcW68DrPMsuxec4Mtyfqw7qqdDks5tTUMYgaJpZM4Rs7qx> .

[dio]

@pingles as communicated, allow me to do the PR 🙇

[pingles]

@dio please do!