Health checks do not seem to take Basic Auth into consideration

[jangie]

From my read of the code, I believe that the health checks against members in a cluster do not take into account the configured HTTPBasicAuthUser and HTTPBasicPassword on performing the health check.

To explain why I believe this is the case, cluster.go's healthCheckNode uses http.Client.Get (https://golang.org/src/net/http/client.go ), which does not allow for the attachment of custom headers in a similar way as http.Client.NewRequest and a following http.Client.Do would, nor does cluster.go seem to have a reference to the credentials at any point. client.go does showcase usage of the credentials as well as utilizing http.Client.NewRequest within buildAPIRequest.

If this is the case, this is problematic for consumers of this library who utilize Marathon's basic auth as basic auth seems to protect the ping endpoint in at least Marathon v1.4.2; once cluster's markDown is called, that member will not be able to be marked as up as it will only receive a 401 from Marathon at the /ping path.

[timoreimann]

@jangie thanks for filing a bug report.

I believe your analysis is perfectly correct: Credentials are not taken into consideration when doing health checks against unavailable cluster nodes. To me it seems like we should create a custom struct (maybe a custom HTTP client implementation?) holding HTTP-related parameters (including the credentials) and use it whenever we try to make a request against the Marathon API.

WDYT?

[jangie]

@timoreimann thank you for validating, and that sounds like a great approach. I will put together a PR to do this and will ensure it has a proper amount of test coverage.

Thanks!

[timoreimann]

Thanks @jangie, appreciated. 👍