oauth2-proxy / oauth2-proxy

A reverse proxy that provides authentication with Google, Azure, OpenID Connect and many more identity providers.
https://oauth2-proxy.github.io/oauth2-proxy
MIT License
9.69k stars 1.58k forks source link

[Feature]: Docker: Add HEALTHCHECK command #2555

Open alexdutton opened 7 months ago

alexdutton commented 7 months ago

Motivation

oauth2-proxy currently supports HTTP-based health checks (--ping-path/--ready-path), but the official Docker image doesn't contain or support a way to run a command-based health check. This means that when run in environments like AWS ECS and docker compose (which don't support HTTP-based health checks), it's difficult to set up a health check.

Possible solution

Either:

  1. the Docker image should contain a second executable that parses the configuration to query the --ready-path endpoint and return an appropriate exit code, or
  2. the oauth2-proxy binary should be given a new command-line option --health-check that does the above

In both cases, an appropriate HEALTHCHECK Dockerfile instruction should be added.

I could have a go at implementing this if such a feature were welcomed.

Provider

None

alexdutton commented 7 months ago

I've just realised that the alpine images — as contrasted the distroless ones — can be fairly easily extended with e.g.:

FROM quay.io/oauth2-proxy/oauth2-proxy:v7.6.0-alpine

RUN apk --no-cache add curl
HEALTHCHECK --interval=30s --timeout=5s CMD ["/usr/bin/curl", "-f", "http://localhost/oauth2/ready"]

This meets our immediate needs, but maybe this feature request is still valid for out-of-the-box HEALTHCHECK support.

CesarStef commented 7 months ago

Hi, The alpine image has wget , have you tried to use it for healthcheck? If you need a guide, this should work: https://stackoverflow.com/a/47722899

alexdutton commented 7 months ago

Yes, I did end up using the alpine image. It's enough for our purposes, but it might still be helpful to have it available as part of the base Docker image so that folks don't need to extend it.

wollomatic commented 6 months ago

I avoid having shells, downloaders, or package managers in prod images for security purposes. So a health check option, as initially proposed, would make sense, imho.

github-actions[bot] commented 4 months ago

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

wollomatic commented 4 months ago

bump issue

ski7777 commented 3 months ago

this feature would be awesome

tuunit commented 3 months ago

I avoid having shells, downloaders, or package managers in prod images for security purposes. So a health check option, as initially proposed, would make sense, imho.

That is exactly why we introduced the distroless base image. To isolate oauth2-proxy as much as possible and avoid linux package related CVEs.

tuunit commented 3 months ago

Resource: https://github.com/GoogleContainerTools/distroless/issues/183

I don't know if I like the idea of adding back wget to the distroless image

alexdutton commented 3 months ago

I completely agree with not adding wget or any other CLI tool to the distroless image. My earlier comment was how I'd met our immediate requirement using the alpine image, not how I think it should be done upstream here — sorry if that wasn't clear!

I still think both the distress and alpine images should have an additional Go binary or way to invoke oauth2-proxy that provides the healthcheck functionality.

The healthcheck would use the same config to find where oauth2-proxy is listening, make the HTTP healthcheck call and exit with an appropriate exit code.

wollomatic commented 3 months ago

I use a small go binary for the healthcheck in another project. I suppose this could work similar with oauth2-proxy: https://github.com/wollomatic/socket-proxy/blob/main/cmd/healthcheck/main.go

alexdutton commented 3 months ago

I've just realised this now has a "help wanted" label, so I'll start to put together a PR.

@tuunit If that's okay, can you assign this issue to me?

tuunit commented 3 months ago

@alexdutton thanks for offering to take this up. Do you already have in mind how you would want to implement the healthcheck? Additional binary? Concerning the size of the images I would prefer to have a healthcheck flag in the main application that invokes the healthcheck routine instead of starting the proxy.

alexdutton commented 3 months ago

@tuunit I was thinking of adding --ping-check and --ready-check options to the main binary after config file parsing. These would use the BindAddress/SecureBindAddress and PingPath/ReadyPath to construct the URL to make an HTTP request (exiting 0 or 1 as appropriate).

Edit: Have ignored the ready path, as the Docker HEALTHCHECK most closely correlates to a liveness check. Being unable to connect to the session store doesn't mean that the oauth2-proxy container itself is unhealthy.

This would work with config provided by environment variables, but anyone extending the image to add in a config file and change the CMD to use it would need to also update the HEALTHCHECK.

I had proposed https://github.com/oauth2-proxy/oauth2-proxy/issues/2554 as a way to provide a YAML config file via environment variable, which this healthcheck would also benefit from. At that point it'd be possible to configure oauth2-proxy entirely via environment variable, with a working healthcheck, without having to extend the base image.

alexdutton commented 2 months ago

Just to say there's now a draft PR to add the healthcheck, but also that I'm going to be on leave for the next 2½ weeks :-)

github-actions[bot] commented 3 weeks ago

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

niklasweimann commented 3 weeks ago

Bump issue