feat: operator support set basic auth for prometheus server

[Xu-Wentao]

Description

Support set basic auth info from secret and set statefulset probe's httpGet headers. Aim to solve https://github.com/prometheus-operator/prometheus-operator/issues/5836

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

• [ ] CHANGE (fix or feature that would cause existing functionality to not work as expected)
• [x] 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.)

Verification

Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.

[Xu-Wentao]

@simonpasquier hi, i have changed the type of basicAuthUsers to BasicAuth and basis auth info store in secret (should be created maunally first) currently. and when reconcile prometheus, we will set statefulset's probes with basic auth headers automaticly. But there's still have a promblem that, user can get basic auth info by decoding authorization info which in prometheus liveness probe or readiness probe.

[simonpasquier]

There's also the question of the Thanos sidecar and config reloader which require authentication credentials too. Another option instead of picking up a random user/password from the list provided by the user would be to create a special username with a random password and inject the credentials in the generated passwd file.

[Xu-Wentao]

There's also the question of the Thanos sidecar and config reloader which require authentication credentials too. Another option instead of picking up a random user/password from the list provided by the user would be to create a special username with a random password and inject the credentials in the generated passwd file.

ok.I will finish it. bytheway, maybe prometheus and other components can exclued health check api for authicated like the issue https://github.com/prometheus/prometheus/issues/9166 mentioned.

[simonpasquier]

I'm on the fence regarding the API: I wonder if the operator should be in charge of producing the brypt hashes or if it should pass the secret data as-is to Prometheus (and let the user provide the hashes). In particular involving the operator with security concerns is always a risk for the long-term maintenance.

[simonpasquier]

After more thinking, I realize that the simplest API might be for the user to provide a secret containing all the credentials where the username would be the key and the password the value:

apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  prometheus: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: v1
kind: Secret
metadata:
  name: example-webconfig
stringData:
  password: test
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
        username: prometheus
        password:
           key: password
           name: example-webconfig

The operator can fail the reconciliation if it detects invalid hashes in the secret (e.g. using bcrypt.Cost()).

Again sorry for the back and forth but I didn't put a lot of effort in this part before your PR so thanks for doing it!

[Xu-Wentao]

After more thinking, I realize that the simplest API might be for the user to provide a secret containing all the credentials where the username would be the key and the password the value:

apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  prometheus: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: v1
kind: Secret
metadata:
  name: example-webconfig
stringData:
  password: test
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
        username: prometheus
        password:
           key: password
           name: example-webconfig

The operator can fail the reconciliation if it detects invalid hashes in the secret (e.g. using bcrypt.Cost()).

Again sorry for the back and forth but I didn't put a lot of effort in this part before your PR so thanks for doing it!

Ok, I'll try to change the API as you recommended, But the way, when i try to set a correct config of thanos sidecar to make sidecar can access to prometheus server normally, i find out that the sidecar currently set --prometheus.http-client config as a plain text, is that ok? coz if we want to use a config file for --promethes.http-client-config https://thanos.io/tip/components/sidecar.md/#configuration, this a lot of work need to do.

[simonpasquier]

when i try to set a correct config of thanos sidecar to make sidecar can access to prometheus server normally, i find out that the sidecar currently set --prometheus.http-client config as a plain text, is that ok? coz if we want to use a config file for --promethes.http-client-config https://thanos.io/tip/components/sidecar.md/#configuration, this a lot of work need to do.

For sure, we don't want the HTTP client config to be in clear-text if it contains sensitive information like password so yes, we'll need to change this and probably use a dedicated secret mounted in the sidecar. And we need the same for the config reloader sidecar. I'd say that it should be a preliminary PR to this one to facilitate the review process.

For context, both thanos and config-reloader sidecars had to disable TLS verification because they only connect through localhost and we didn't want to bother with configuring the right CA.

[Xu-Wentao]

when i try to set a correct config of thanos sidecar to make sidecar can access to prometheus server normally, i find out that the sidecar currently set --prometheus.http-client config as a plain text, is that ok? coz if we want to use a config file for --promethes.http-client-config https://thanos.io/tip/components/sidecar.md/#configuration, this a lot of work need to do.

For sure, we don't want the HTTP client config to be in clear-text if it contains sensitive information like password so yes, we'll need to change this and probably use a dedicated secret mounted in the sidecar. And we need the same for the config reloader sidecar. I'd say that it should be a preliminary PR to this one to facilitate the review process.

For context, both thanos and config-reloader sidecars had to disable TLS verification because they only connect through localhost and we didn't want to bother with configuring the right CA.

I find that the config-reloader's ReloaderURL is defined here https://github.com/prometheus-operator/prometheus-operator/blob/main/pkg/prometheus/common.go#L456 and did not support config basic auth headers, should i change the thanos config reloader to support this param?

[simonpasquier]

For the config reloader, the idea should be to add CLI arguments to pass the basic-auth user/password in https://github.com/prometheus-operator/prometheus-operator/blob/main/cmd/prometheus-config-reloader/main.go and use something like https://pkg.go.dev/k8s.io/[email protected]/transport#NewBasicAuthRoundTripper to inject the header in the request.

[Xu-Wentao]

@simonpasquier Add a new PR for thanos sidecar to support save config in secret, plz reivew this PR first.

[Xu-Wentao]

After more thinking, I realize that the simplest API might be for the user to provide a secret containing all the credentials where the username would be the key and the password the value:经过更多思考，我意识到最简单的 API 可能是让用户提供一个包含所有凭据的秘密，其中用户名是密钥，密码是值：

apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  prometheus: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: v1
kind: Secret
metadata:
  name: example-webconfig
stringData:
  password: test
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
        username: prometheus
        password:
           key: password
           name: example-webconfig

The operator can fail the reconciliation if it detects invalid hashes in the secret (e.g. using bcrypt.Cost()).如果操作员检测到秘密中的无效哈希值（例如使用bcrypt.Cost() ），则协调可能会失败。

Again sorry for the back and forth but I didn't put a lot of effort in this part before your PR so thanks for doing it!再次对来来回回表示歉意，但在你的 PR 之前我没有在这部分投入很多精力，所以感谢你这样做！

how about like this, this podCredentialUser must be in the secret. User can specify which user info is used for pod probe check. the operator will check if the podCredentialUser in secret when reconcile.

apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  prometheus: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentialUser: prometheus

[simonpasquier]

There's already an identity for the "pod user" which is the service account (SA) name. To make things simple, we could require that the secret contains a key equal to the SA name (or default if the resource doesn't define a SA). If the key isn't present, the reconciliation fails.

[Xu-Wentao]

There's already an identity for the "pod user" which is the service account (SA) name. To make things simple, we could require that the secret contains a key equal to the SA name (or default if the resource doesn't define a SA). If the key isn't present, the reconciliation fails.

If we use the SA name as the username of basic auth for pod probe check, how to get the password of it, from another secret specified by user? or create a secrete to save password automatically?

[simonpasquier]

Right, we need both the hashed password and the password in clear text. Something like this?

spec:
  web:
    basicAuthUsers:
      secretName: web-users # secret
      serviceAccountPassword:
        name: credentials
        key: prometheus-web-password

[Xu-Wentao]

After more thinking, I realize that the simplest API might be for the user to provide a secret containing all the credentials where the username would be the key and the password the value:

apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  prometheus: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: v1
kind: Secret
metadata:
  name: example-webconfig
stringData:
  password: test
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
        username: prometheus
        password:
           key: password
           name: example-webconfig

The operator can fail the reconciliation if it detects invalid hashes in the secret (e.g. using bcrypt.Cost()).

Again sorry for the back and forth but I didn't put a lot of effort in this part before your PR so thanks for doing it!

I finally used this desgin, coz it's more clearly for user to setup the podCredentials which is used for pod health check. and if we use the ServiceAccount as username, when we build the web-config, it did not have the prometheusSpec param, it may make it code more complex.

[simonpasquier]

I finally used this desgin, coz it's more clearly for user to setup the podCredentials which is used for pod health check. and if we use the ServiceAccount as username, when we build the web-config, it did not have the prometheusSpec param, it may make it code more complex.

I would like to avoid adding more fields than necessary to the API. Let's see if we can get rid of the username somehow.

[Xu-Wentao]

I finally used this desgin, coz it's more clearly for user to setup the podCredentials which is used for pod health check. and if we use the ServiceAccount as username, when we build the web-config, it did not have the prometheusSpec param, it may make it code more complex.

I would like to avoid adding more fields than necessary to the API. Let's see if we can get rid of the username somehow.

How about that? (maybe we can generate the passowrd automatically, user no need to create it)

---
apiVersion: v1
kind: Secret
metadata:
  name: web-users
stringData:
  foo: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
  bar: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay
---
apiVersion: v1
kind: Secret
metadata:
  name: example-webconfig
stringData:
  password: test
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
        password:
           key: password
           name: example-webconfig
---
# generate by operator
apiVersion: v1
kind: Secret
metadata:
  name: pod-credentials
data:
  # username and password is used for health check probe/thanos-sidecar/config-reloader
  username: serviceAccountName
  password: test
  # hashedValue is used for prometheus server merged with `web-users` and save into `web-config`
  hashedValue: "serviceAccountName: $2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay"

[simonpasquier]

I would simplify the struct a bit:

apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: example
spec:
  serviceAccountName: prometheus
  web:
    basicAuthUsers:
      secretName: web-users
      podCredentials:
         key: password
         name: example-webconfig

Indeed the operator could merge the user-provided user list (web-users secret) with the bcrypt hash from the SA password though I'm a bit nervous of dealing with security/crypto topics. As a first approach, I would make it mandatory to have an entry in web-users for the service account (and fail reconciliation if it's not the case).

In all cases, we still need https://github.com/prometheus/exporter-toolkit/pull/151 before proceeding.

[Xu-Wentao]

情况

Agree with that, and i was tried that way. But there is a unwanted thing happend: the username:password will be bcrypted by operator automatically, and when the secret created (merged with web-user secret), it will cause trigger another reconcile, and then a different bcryped hashed value will gerneated, and then cause one another reconcile and so on.

There's some thing i think we can work on it: when we try to create a secret web-config, we can compare the current user list if we need to update.

[simonpasquier]

But there is a unwanted thing happend: the username:password will be bcrypted by operator automatically, and when the secret created (merged with web-user secret), it will cause trigger another reconcile, and then a different bcryped hashed value will gerneated, and then cause one another reconcile and so on.

I'm not sure to understand what's happening here. Also my suggestion would be not having the operator to deal with password encryption.

[Xu-Wentao]

@simonpasquier sorry for update late. Currently the basic auth is designed like this.

---
apiVersion: v1
kind: Secret
metadata:
  name: basic-auth-users
stringData:
  foo: "$2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay"
  bar: "$2b$12$hNf2lSsxfm0.i4a.1kVpSOVyBCfIB51VRjgBUyv6kdnyTlgWj81Ay"
  prometheus: "$2a$12$6H/IeGCSIqdl2Bg1Mk3D5ebqbc6qESkmIwBXiJLLg/nDN3OVQBF76"
---
apiVersion: v1
kind: Secret
metadata:
  name: basic-auth-pod-credentials
stringData:
  password: "prometheus-password"
---
apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: prometheus
  labels:
    prometheus: basic-auth
spec:
  replicas: 1
  thanos:
    logLevel: info
  serviceAccountName: prometheus
  serviceMonitorSelector: {}
  web:
    basicAuthUsers:
      secretName: basic-auth-users
      podCredentials:
        key: password
        name: basic-auth-pod-credentials

User need to createe a Secret to store all basic-auth-users info, included the name from ServiceAccount (in this example is prometheus), and create a Secret to store the password for user name from ServiceAccount. Is this design like waht you mentoned?

[simonpasquier]

User need to createe a Secret to store all basic-auth-users info, included the name from ServiceAccount (in this example is prometheus), and create a Secret to store the password for user name from ServiceAccount. Is this design like waht you mentoned?

yes this is what i had in mind.

[Xu-Wentao]

User need to createe a Secret to store all basic-auth-users info, included the name from ServiceAccount (in this example is prometheus), and create a Secret to store the password for user name from ServiceAccount. Is this design like waht you mentoned?

yes this is what i had in mind.

I have completed the writing of relevant code logic. Please take a look at which logic we can continue to adjust and optimize.

[Xu-Wentao]

@simonpasquier hi, i have use object reference to secrets as you mentioned, is there any other things i can to to move on？