New healthcheck path is not compatible with AWS ALB

[hobbsh]

Describe the bug
The new healthcheck path using a basic query is using characters that are not compatible with ALB Target Group Health Check configuration due to character restrictions.

To Reproduce
Steps to reproduce the behavior:

1. Try to add the new healthcheck path (/?query={__typename}) to an ALB Target Group Health Check configuration
2. See the following error:

Health check must begin with a '/' and can only consist of the characters: a-z, A-Z, 0-9, and @:%_+.~#?&/=

Expected behavior
I would expect the healthcheck path to be compatible in any situation.

Output
See above

Additional context
Add any other context about the problem here.

[abernix]

Ah, got it. Thanks for opening this. I think we probably have some work to do here.

At the core of the error message, the query string would indeed need to be URL encoded: meaning {__typename} would become %7B__typename%7D. This would comply with the character requirements they're suggesting in the returned message. Some tooling/clients merely takes care of doing that encoding for you (e.g., browsers, the Fetch API, Kubernetes, etc.) but it seems like ALB requires it to be encoded ahead of time. That's a reasonable requirement that is definitely true from a historical aspect and a tool like this can be done to encode that value properly. It would — in theory — no longer return that particular error after doing so.

We could make the documentation more clear about this since it certainly won't be clear to everyone that the "query" part of a URL (not just the parameter named query, but the whole ?arg1=val1&arg2=val2) is generally subject to those requirements for both its key names and values per RFC3986§3.4.

That said, even if you were to change that to be URL encoded (and we were to fix our documentation), I foresee you running into another constraint immediately (correct me if I end up being wrong): ALB target groups health checks don't seem to let you set any HTTP headers (e.g., content-type, etc.) on their HTTP health checks (as Kubernetes does with httpHeaders).

Ordinarily, setting headers wouldn't be required on simple GET requests like this but due to the CSRF protections that we put in place — even on even GET requests — to reduce the chances of XS Leaks — back in #1039 — at least some headers are needed to be present to durably prevent browsers from considering the operation a CORS simple request. There are various nuances here around CORS and CSRF that unfortunately come into play despite the fact that we are talking about a security aspect that resolves the browser security policy, something which is not directly relevant to health checks, but is incurring this additional requirement.

Even though it's possible to disable CSRF in configuration that is not recommended though it would conceivably fix the header issue that I believe you'll encounter next after URI encoding the query parameter.

We're going to have to do the work to put back in a health check end-point to satisfy those requirements and I've opened #1861 (and re-hashed some of this context in that issue) to capture the work.

Thanks again for filing this!