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 for using HTTPS in the Prometheus web UI is incomplete #4273

Open
jphx opened this issue Sep 21, 2021 · 12 comments · Fixed by #4276
Open

Support for using HTTPS in the Prometheus web UI is incomplete #4273

jphx opened this issue Sep 21, 2021 · 12 comments · Fixed by #4276

Comments

@jphx
Copy link

jphx commented Sep 21, 2021

What happened?

I've been trying to use the feature of having the Prometheus web UI use HTTPS, via config in the Prometheus resource like the following:

web:
  tlsConfig:
    keySecret:
      name: cert-secret
      key: tls.key
    cert:
      secret:
        name: cert-secret
        key: tls.crt

This appears to partially work. It does configure Prometheus to use HTTPS, and it does change the readiness probe for the Prometheus container to use HTTPS. But it doesn't change the config-reloader container to use HTTPS when it contacts the Prometheus container (via the --reload-url option), and it doesn't change the thanos-sidecar container to use HTTPS when it contacts the Prometheus container (via the --prometheus.url option).

The config-reloader log fills up with messages like:

function failed. Retrying in next tick" err="trigger reload: received non-200 response: 400 Bad Request; have you set "--web.enable-lifecycle" Prometheus flag?

and the thanos-sidecar log fills up with messages like:

failed to get Prometheus flags. Is Prometheus running? Retrying" err="got non-200 response code: 400, response: Client sent an HTTP request to an HTTPS server.

I've reproduced this with version v0.50.0 of the Prometheus Operator.

Did you expect to see something different?

The config-reloader and thanos-sidecar containers should be configured to contact Prometheus using HTTPS instead of HTTP, when the Prometheus web UI is using HTTPS.

One other important point: My setup uses a self-signed certificate for the Prometheus web UI, so the config-loader and thanos-sidecar would have to (at least optionally) suppress certificate validation when they make their connections.

How to reproduce it (as minimally and precisely as possible):

Use some config lines as above in the Prometheus resource to ask for HTTPS support in Prometheus.

Environment:

  • Prometheus Operator version: v0.50.0

  • Kubernetes version information: v1.20.10

  • Kubernetes cluster kind: IBM Cloud Kubernetes cluster

@jphx jphx added the kind/bug label Sep 21, 2021
@fpetkovski
Copy link
Contributor

fpetkovski commented Sep 22, 2021

This is indeed a bug, but changing the config reloader to use https can be problematic for certificates issued by an untrusted CA. We either need to mount the CA's certificate in /etc/ssl/certs/ or add a feature to config-reloader for accept insecure connections.

We can also fix the bug for trusted CAs only and treat untrusted CAs as a separate issue.

@simonpasquier @philipgough thoughts on this one?

@philipgough
Copy link
Contributor

@fpetkovski personally, I would be in favour of fixing the reported bug and treating support for self-signed certificates as an enhancement.

@simonpasquier
Copy link
Contributor

Ah interesting case :) Agreed that it doesn't hurt to adjust the URI scheme but I expect that for most (all?) impacted users, it won't fix the issue (e.g. TLS verification will fail further down the road).
IIUC we depend on Thanos sidecar (+ Thanos reloader package) for this. Maybe skipping the TLS verification is enough in our case since the sidecar and reloader connect to the localhost address?

@fpetkovski
Copy link
Contributor

If we decide to skip TLS verification, this will need to be implemented in the thanos sidecar and config reloader components first. Then we can configure these components with the insecure flag. Another alternative would be to allow Prometheus to listen on both HTTP and HTTPS at the same time. This way internal requests can go to the http endpoint on localhost and external ones can go to https.

@simonpasquier
Copy link
Contributor

If we decide to skip TLS verification, this will need to be implemented in the thanos sidecar and config reloader components first.

yes this is what I meant too :)

Another alternative would be to allow Prometheus to listen on both HTTP and HTTPS at the same time. This way internal requests can go to the http endpoint on localhost and external ones can go to https.

Possible too but I suspect that it will be more complicated to implement... Worth checking with upstream though.

@simonpasquier
Copy link
Contributor

Reopening since #4276 only fixed the HTTP scheme but the TLS verification is still an issue.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had any activity in the last 60 days. Thank you for your contributions.

@simonpasquier
Copy link
Contributor

The fix should be to pass down the proper TLS settings to Thanos sidecar and config reloader:

  • The Thanos sidecar supports HTTP client configuration (including TLS) since v0.24.0 with the --prometheus.http-client flag.
  • The same capabilities should be added to the config reloader.

The operator could reuse spec.web.tlsConfig.cert as the CA for the sidecars but it wouldn't work in case of client certificate authentication AFAICT. Which means that the Prometheus CRD needs a new field to specify the HTTP configuration applying to the sidecars.

@raptorsun
Copy link
Contributor

The fix should be to pass down the proper TLS settings to Thanos sidecar and config reloader:

  • The Thanos sidecar supports HTTP client configuration (including TLS) since v0.24.0 with the --prometheus.http-client flag.
  • The same capabilities should be added to the config reloader.

The operator could reuse spec.web.tlsConfig.cert as the CA for the sidecars but it wouldn't work in case of client certificate authentication AFAICT. Which means that the Prometheus CRD needs a new field to specify the HTTP configuration applying to the sidecars.

Do we need the prometheus-operater generate a client certificate dedicated to Thanos sidecar and config reloader? or do we expect the user to set a certificate in the Prometheus CRD?

@fpetkovski
Copy link
Contributor

In this case the certificate should be defined by the user.

@simonpasquier
Copy link
Contributor

+1 to what @fpetkovski said.

Thinking more about this issue, it probably makes sense to disable TLS verification for the config reloader and the Thanos sidecar. Both talk to the Prometheus container via the local address so there's little advantage into verifying the peer's identity (if something manages to spoof/hijack the Prometheus port, you have probably bigger problems).
FWIW how Kubernetes works with HTTPS probes today (see https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#http-probes)

@NLRemco
Copy link
Contributor

NLRemco commented Feb 22, 2022

Also +1 for what @fpetkovski said. Currently having issues with offering Prometheus over HTTPS with the operator, but the config reloader not trusting said certificate since its self-signed. Prefer to keep using the self managed certificate or to skip TLS verify.

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

Successfully merging a pull request may close this issue.

6 participants