alexliesenfeld / health

A simple and flexible health check library for Go.
MIT License
775 stars 38 forks source link

How to differentiate output based on auth status? #2

Closed panta closed 3 years ago

panta commented 3 years ago

Commit 739c4a012ae74f2a6b7b2c9efebc7dcee916116a removed WithCustomAuth and WithBasicAuth.

What's the recommended way to obtain the same behaviour now?

alexliesenfeld commented 3 years ago

It was removed because authentication is not a primary concern of a health check library. The health check handler is just a standard http.Handler though, so you can use it with many middleware implementations, such as this or this .

panta commented 3 years ago

Yes, but it's still useful to be able to return a more limited result to non-authenticated clients, instead of returning a 401 or 403. I can implement a middleware calling two different health handlers based on auth status, but in that case I'd have two separate but similar/identical copies of health checkers running in background, no?

alexliesenfeld commented 3 years ago

Ah, I understand. Yes, this functionality was removed. As a replacement, I planned to add a more generic function to disable details on a request basis. This way you can decide in which situations you would like to disable details yourself. To do this, you would need to add a marker to the request context by calling a health.DisableResponseDetails(request.Context()) function whenever you want to return the reduced response. This could be called by your middleware. Would this be sufficient for you? This is just a rough sketch, I'm open for other ideas.

panta commented 3 years ago

The marker/option would be nice. I was also thinking of separating and exposing the checks machinery from the handler(s), thus also allowing to further customize the output and the strategy of handling. I've seen this approach adopted in hellofresh/health-go for example. What would you think?

panta commented 3 years ago

I've provided an implementation in panta/feature/standalone-checkers (just a matter of making the checker -> Checker interface public, add a NewChecker() function, and some other very minor adjustments).

If you feel this might be useful to others I'll send a PR.

alexliesenfeld commented 3 years ago

Thanks. I looks interesting, indeed! Happy to get a PR for this. I'll look into it tonight.

alexliesenfeld commented 3 years ago

Merged with #5

alexliesenfeld commented 3 years ago

In case you're still interested in a solution: The new version 0.5.0 has a BasicAuth middleware that should do what you want.

panta commented 3 years ago

Actually I need to use it with a different auth mechanism, but the stand-alone checkers are perfect for that! Thanks!