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 Web basic_auth_users config for Prometheus and Alertmanager #4942

Conversation

heylongdacoder
Copy link
Contributor

Description

Fixes: #4200

Type of change

  • CHANGE (fix or feature that would cause existing functionality to not work as expected)
  • FEATURE (non-breaking change which adds functionality)
  • BUGFIX (non-breaking change which fixes an issue)
  • ENHANCEMENT (non-breaking change which improves existing functionality)
  • NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Added support for Web basic_auth_users config for Prometheus and Alertmanager CRD

@heylongdacoder
Copy link
Contributor Author

Pending Works:

  • config-reload with auth
  • prometheus's thanos sidecar with auth
  • health check probe with auth
  • e2e test case
  • documentation, function name, error message

Fixes: prometheus-operator#4200

Signed-off-by: heylongdacoder <heylongdacoder@gmail.com>
@simonpasquier
Copy link
Contributor

prometheus/exporter-toolkit#70 is of interest for health check probe with auth but I agree that config reloader and Thanos sidecar -> Prometheus authentication has to be sorted out :)

@heylongdacoder
Copy link
Contributor Author

Opened prometheus/exporter-toolkit#106 to tackle the health check probe with auth

@heylongdacoder
Copy link
Contributor Author

Hi @simonpasquier , I saw you suggested to create a user/password for config-reloader and thanos-sidecar. For different "Prometheus" CRD Object, I am thinking to use a fix username("prometheus-operator-managed-user"), generate a random password, and create username and password as "Secret" so that config-reloader and thanos-sidecar can refer this "Secret" as environment variables. Do you think this is ok?

And since we are generating the user/password, do we need to support the rotation of this auth? Thanks :)

@nourspace
Copy link

Pending Works:

  • config-reload with auth
  • prometheus's thanos sidecar with auth
  • health check probe with auth
  • e2e test case
  • documentation, function name, error message

Don't you think you will also need to update the default Grafana data source so it includes the generated credentials?

@heylongdacoder
Copy link
Contributor Author

@nourspace good point! But I think Grafana is out of scope for prometheus-operator. Probably got to do another PR at https://github.com/prometheus-operator/kube-prometheus or https://github.com/prometheus-community/helm-charts/tree/main/charts/kube-prometheus-stack. Maybe I can take a look as well after this PR.

@slashpai
Copy link
Contributor

@heylongdacoder are you still working on this?

@heylongdacoder
Copy link
Contributor Author

hi @slashpai , currently not, bit busy with my current job. FYI, I stucked in this issue(prometheus/exporter-toolkit#106) last time. 😄

@ArthurSens
Copy link
Member

Hey @heylongdacoder, I'm closing this PR for now since you've communicated that you won't be able to continue.

Thanks a lot for the effort so far though! You made things clearer about what needs to be done :)

Feel free to re open if you wish to continue in the future

@ArthurSens ArthurSens closed this Nov 17, 2023
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.

Support basic_auth_users in web configuration for Prometheus and Alertmanager
5 participants