scylladb / scylla-operator

The Kubernetes Operator for ScyllaDB
https://operator.docs.scylladb.com/
Apache License 2.0
340 stars 175 forks source link

Verify client certs for Prometheus deployments #1186

Open tnozicka opened 1 year ago

tnozicka commented 1 year ago

Is this a bug report or feature request?

What should the feature do: Currently the managed Prometheus that is part of the new monitoring stack doesn't force mTLS certificate verification.

https://github.com/scylladb/scylla-operator/blob/f20887deee7a7b54c89eb2c11a19a1037f7ce18f/assets/monitoring/prometheus/v1/prometheus.yaml#L21-L22

This was done temporarily on purpose because the prometheus-operator sets up probes behind authenticated enpoints, which obviously doesn't work because kubelets don't have the client certs for mTLS. We need to start by creating a simple reproducer and report it to the prometheus-operator.

What is use case behind this feature: Security

fyi @YvanDaSilva (so you are not surprised when this gets fixed)

# Requires
- [ ] https://github.com/prometheus/prometheus/issues/9166
- [ ] https://github.com/scylladb/scylla-operator/issues/2187
- [ ] https://github.com/prometheus-operator/prometheus-operator/issues/5419
rzetelskik commented 1 year ago

Opened https://github.com/prometheus-operator/prometheus-operator/issues/5419 fyi @tnozicka

rzetelskik commented 1 year ago

Tried to follow it up in the most recent PR which tried to address it: https://github.com/prometheus/exporter-toolkit/pull/106. It seems to have lost traction and I haven't received any replies so far. The developers seem to agree on an approach of excluding certain paths from cert verification - although I don't know how exactly they want to achieve that atm. Anyway waiting for a reply there to agree on an approach - I wouldn't want to invest into sending a PR if we don't get anyone to look at it.

rzetelskik commented 1 year ago

Just sent https://github.com/prometheus/exporter-toolkit/pull/151. I will update this issue once (if?) it gets merged.

rzetelskik commented 1 year ago

No response from developers so far. Pinged the maintainers on https://github.com/prometheus/exporter-toolkit/pull/151 and reached out on their slack channel in CNCF workspace. If I don't get a reply in the next couple of days, I'll attend Prometheus Developer Office Hours next Monday.

rzetelskik commented 1 year ago

I added the PR to the Developer Office Hours' agenda today but was only informed by the moderator that we'll just have to wait for the maintainers to reply in the PR. Pinged the maintainers again.

rzetelskik commented 6 months ago

As per https://github.com/prometheus/exporter-toolkit/pull/151#issuecomment-2092752508, this should be discussed on Prometheus Dev Summit today.

Update: it wasn't :melting_face:

scylla-operator-bot[bot] commented 4 months ago

The Scylla Operator project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

/lifecycle stale

rzetelskik commented 4 months ago

/remove-lifecycle stale /triage accepted

rzetelskik commented 1 month ago

Update: it seems like https://github.com/prometheus/exporter-toolkit/pull/151 is likely to finally go through as I've got a first review. @tnozicka can we try accommodating this again in our roadmap?

rzetelskik commented 3 weeks ago

Update: https://github.com/prometheus/exporter-toolkit/pull/151 got an approval and is waiting to be merged. The next step after that would be to update the prometheus-operator issue (or send a PR myself) but we'll have to wait for exporter-toolkit release and for it to propagate to prometheus itself. So at this point this item is blocked.