Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support path exclusion from basic authentication #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heylongdacoder
Copy link
Contributor

Fixes: prometheus/prometheus#9166

Signed-off-by: heylongdacoder [email protected]

@heylongdacoder
Copy link
Contributor Author

This PR is based on some of the comment from #70

docs/web-configuration.md Outdated Show resolved Hide resolved
@SuperQ SuperQ requested a review from roidelapluie August 30, 2022 08:41
@roidelapluie
Copy link
Member

Can we make the parameter more generic and also skip TLS cert validation?

@heylongdacoder
Copy link
Contributor Author

heylongdacoder commented Sep 19, 2022

@roidelapluie hello, I got two questions:

  1. May I know what do you mean by make the parameter more generic?
  2. Regarding the skip TLS cert validation, you mean when user config client_auth_type as RequireAndVerifyClientCert, but they want Prometheus server to skip the TLS client cert validation only for certain endpoints?

Thanks in advance. :D

@roidelapluie
Copy link
Member

The use case for this is that you can serve /-/health without TLS authentication but the rest with authentication because kube probes do not support certs

@SuperQ
Copy link
Member

SuperQ commented Oct 19, 2022

This needs a rebase.

@heylongdacoder
Copy link
Contributor Author

The use case for this is that you can serve /-/health without TLS authentication but the rest with authentication because kube probes do not support certs

Got it! Let me check how to do this. Thanks :D

@SiiiiiiD
Copy link

Any chance get this merged?

Or, maybe, someone can recommend any workarounds for keeping health check works with mTLS/auth added to prometheus and/or some sensitive exporters?

Even switching health checks to exec of custom wget or curl is not possible in a straight way, since since busybox based images wget not able to handle client certs, while curl is missing at all (this approach also potentially may spawn zombie processes within containers).

@rzetelskik
Copy link

@roidelapluie @SuperQ are you accepting contributions for this issue? Seems like this particular PR lost traction.

@rzetelskik
Copy link

rzetelskik commented Mar 30, 2023

To follow up, I'm particularly interested in being able to skip mTLS for /-/healthy and /-/ready endpoints with "RequireAndVerifyClientCert" due to kubelet probes not supporting certificates (see comments above and prometheus-operator/prometheus-operator#5419). I'd be happy to contribute that part. @roidelapluie did you guys put any thought into how you'd want this implemented?

@heylongdacoder
Copy link
Contributor Author

skip TLS for /-/healthy and /-/ready endpoints with "RequireAndVerifyClientCert"

Hi @rzetelskik, as you may know, this is the part that missing in my PR. I have gone through the HTTP server code a bit regarding this but got no idea how to implement this at this moment. And currently I am busy with my personal life. Please feel free to take this issue 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to disable security on Prometheus health endpoints, /-/healthy and /-/ready
5 participants