nhost / hasura-auth

Authentication for Hasura.
https://nhost.io
MIT License
382 stars 114 forks source link

fix: change hasura health check url to make auth server work in gcp #565

Closed itsmurugappan closed 1 week ago

itsmurugappan commented 1 week ago

Google cloud doesnt support "z" urls - https://stackoverflow.com/questions/43380939/where-does-the-convention-of-using-healthz-for-application-health-checks-come-f

And hasura provides an alternate end point for this https://hasura.io/docs/2.0/api-reference/health/ .

Checklist

dbarrosop commented 1 week ago

Hi, thanks for the PR.

Google cloud doesnt support "z" urls - https://stackoverflow.com/questions/43380939/where-does-the-convention-of-using-healthz-for-application-health-checks-come-f

Where does it say that? I couldn't find any reference to it. Do you have any documentation we can reference?

Regarding the change, unfortunately, it isn't good (and doesn't work) for two main reasons:

  1. It is done on the javascript side, which isn't used (we are transitioning to go) so this change basically has no impact.
  2. It is a breaking change as changing the healthz path will break every single deployment

To properly perform this change you will have to:

  1. Add a second healthz endpoint here. Do not change the existing one as any PR modififying it will not be accepted. Adding a second one should be fine.
  2. After you have edited the openapi.yaml you can run go generate ./... to generate some stubs and boilerplate.
  3. You will have to implement the new method added to the server interface (which can just be an alias to this.

Also, don't call the endpoint /hasura/healthz. If the issue is that the endpoint is called healthz we can just add a second one called /health (without the z)

xmlking commented 1 week ago

Where does it say that? I couldn't find any reference to it. Do you have any documentation we can reference?

Workloads in Google Cloud cannot have URL paths with /healthz . here is are some references from stackoverflow

here is the proof (hasrua hosted in google Cloud Run)

image

When hasura-auth container starts, it use this code to check if hasura is UP and healthy. without this check pass, hasura-auth in GCP will not start at all.

I think there is some misunderstanding here:
@itsmurugappan is asking to use Hasura's second health endpoint /hasura/healthz instead of /healthz

[!IMPORTANT]
Hasura include both health-check endpoints in all deployment environments. we don't have to do anything for hasura.

I think this change will not have any side-effect

xmlking commented 1 week ago

hasura docs

image
dbarrosop commented 1 week ago

yes, you are completely right. I totally misinterpreted the change. Thanks for the clarification.

However, if you need this change because you are hosting hasura in GCP, wouldn't you also need an alternative /health endpoint for hasura-auth?

itsmurugappan commented 1 week ago

since hasura auth health check end point is not called through the loadbalancer by any client we dint need it.

xmlking commented 1 week ago

yes, you are completely right. I totally misinterpreted the change. Thanks for the clarification.

However, if you need this change because you are hosting hasura in GCP, wouldn't you also need an alternative /health endpoint for hasura-auth?

Would be nice to have alternative health-check endpoints for all nhost services that won't conflict with GCP. But as minimum viable solution, this PR is good enough making it work in GCP.