hashicorp / vault-helm

Helm chart to install Vault and other associated components.
Mozilla Public License 2.0
1.06k stars 869 forks source link

feat(Prometheus/ServiceMonitor): add configurable tlsConfig & bearerTokenFile for authentication #931

Closed eyenx closed 2 months ago

eyenx commented 11 months ago

This implements a configurable tlsConfig for the serviceMonitor used to grab metrics from the Vault API.

Additionally it adds the possibility to set a bearerTokenFile for authentication purposes.

eyenx commented 9 months ago

@tvoran @swenson @tomhjp

Anyone able to review this pls? It's kinda sad this is laying around without reaction for more than 3 months.

dtrts commented 8 months ago

I would also like to see this feature added to the chart.

If possible the chance to configure bearerTokenSecret would also be welcomed.

eyenx commented 7 months ago

I would also like to see this feature added to the chart.

If possible the chance to configure bearerTokenSecret would also be welcomed.

Would be nice I know, but HashiCorp does not care about contributions from outsiders. This will be laying around forever or they will implement it by themselves.

eyenx commented 4 months ago

eyenx commented 3 months ago

Thank you @tomhjp will look at it today

eyenx commented 3 months ago

@tomhjp ready for a review :)

tomhjp commented 2 months ago

Hey @eyenx, thanks for your patience. I wanted to try writing a proper integration test that exercised these options for real, I've pushed some WIP in (EDIT) https://github.com/hashicorp/vault-helm/commit/c66c9a26fa4bc237ca94c56c0271ec4525977d53.

One thing I learnt while I was looking deeper is that the bearerTokenFile option is deprecated, with users directed to authorization instead: https://prometheus-operator.dev/docs/operator/api/#monitoring.coreos.com/v1.Endpoint. It does seem like a safer and improved pattern, WDYT about supporting authorization instead of bearerTokenFile in this PR?

tomhjp commented 2 months ago

Merged in #1025 - thanks again!