Specify label value in prom-label-proxy config / Wildcards in Labels

[stieglma]

Hey all,

is it by design, that the value of the label to be checked for cannot be specified directly here, and instead I'll need to put another reverse-proxy between prometheus/thanos and grafana? I explicitly don't want to use the datasource query parameters in grafana, because grafana users might have the possibility to change that, and therefore they could themselves change to which resources they have access. I also saw the caddy example, and while that would definitely work I think it would be quite nice to not need to start an additional container for that purpose.

Furthermore it would also be quite nice, if the value given for the label could be a wildcard, e.g. on our cluster we have prefixes for certain namespaces that share the same access rights, so ideally i want to set something like ns-prefix-* as label value. I also tried with url encoding: ns-prefix-%2A but that didn't help either.

If you think these requests are valid, I'm happy to try and add those features as a PR, I did however not work with go up to now, so it might take a while

[stieglma]

Here's a first version of the changes: https://github.com/stieglma/prom-label-proxy/commit/fe2275d3bad3198b9ba6a598f09a038ee4632f7c unit-tests still need to be fixed for the MatchRegex feature, and I need to think if there is any potential security risk involved when having that change. Maybe it should just be another setting that can be enabled, something like --allow-regex

[simonpasquier]

is it by design, that the value of the label to be checked for cannot be specified directly here, and instead I'll need to put another reverse-proxy between prometheus/thanos and grafana?

Not sure if it's something that we've already evaluated...

Furthermore it would also be quite nice, if the value given for the label could be a wildcard

The requirement for allowing multiple label values instead of just one has been discussed in #27. Using regular expressions has been dismissed so far because of security implications (too easy for users to get it wrong) but supporting repeated label values would be ok (see here for more details). I understand that it isn't exactly what you're asking for but would it be enough for your use case? Maybe supporting simple wildcards (e.g. <prefix>-*) might be acceptable but I'm curious to hear feedback from other @prometheus-community/prom-label-proxy maintainers.

[stieglma]

Having repeated label values would work, but needs plenty of more "glue-code" (e.g. when a new namespace with a certain prefix is added, we need to run some kind pipeline which adds another label value, either to the datasource config in grafana, or to our proxy inbetween).

I would also be happy if regex is only allowed when using label regexes is only possible when labels are directly configured in prom-label-proxy. This way you could check the validity of the regex already at startup, and therefore there's no runtime problems.

[fpetkovski]

I would also be interested in having the label value explicitly set when starting the proxy in order to avoid running another component that will enforce the value. I can contribute this change if the maintainers are fine with having it in the project.

[simonpasquier]

:wave: Filip! It makes sense to me that you can setup prom-label-proxy with a static label value. Something like?

./prom-label-proxy -label mytenant -label-value tenantX ...

[fpetkovski]

Hi Simon! :) That makes sense, i've raised a PR to add a capability for a fixed label value: https://github.com/prometheus-community/prom-label-proxy/pull/116

[jeremych1000]

Am I correct in saying that regex labels defined on startup still aren't supported? It would be really useful!

[jeremych1000]

Context: We run a multi-tenant Kubernetes platform, with namespaces as a service. Namespaces are defined as a list in a file in the format <tenant_name>-foobar. It's not feasible for us to copy the list of namespaces across to the definition for prom-label-proxy as the namespace list is generated at the same time as the prom-proxy.

It would be great if we could support regex, i.e. <tenant_name>-.*

[simonpasquier]

You're correct, prom-label-proxy doesn't support regex values. I'd be ok supporting it but only for static label values (e.g. set by CLI argument) provided that it's well separated from non-regex values.

[jeremych1000]

You're correct, prom-label-proxy doesn't support regex values. I'd be ok supporting it but only for static label values (e.g. set by CLI argument) provided that it's well separated from non-regex values.

Yes please, this would be perfect! It's the only blocker for us at the moment.

We deploy using the helm chart, so the following would work (assuming a tenant name of foobar).

config:
  label: namespace
  extraArgs:
    - "--label-value=foobar-.*"     <-- or --label-value-regex to be explicit

I'm guessing that would translate into namespace=~"foobar-.*"?

[stieglma]

@jeremych1000 if you feel comfortable with it, you can use the state at https://github.com/stieglma/prom-label-proxy/tree/ikor-customization I am using it in that version since two years already. You can just build the docker image yourself from that branch, and use it.

These are the args in my deployment:

args:
        - --label=namespace
        - --labelValue=portals-.*
        - --upstream=http://monitoring-prometheus.monitoring.svc.cluster.local:9090/
        - --insecure-listen-address=0.0.0.0:8080
        - --unsafe-passthrough-paths=/-/ready,/api/v1/metadata
        - --enable-label-apis=true

[jeremych1000]

Hm I ran make build in your branch and pushed a docker image, however the pod fails to be ready and says

Readiness probe failed: HTTP probe failed with statuscode: 404

Is this off v0.7.0? I see you have --labelValue and not --label-value

I have

# config for prom label proxy
config:
  # -- listen address
  listenAddress: 0.0.0.0:8080
  # -- The upstream URL to proxy to
  upstream: "http://kube-prometheus-stack-prometheus.monitoring.svc.cluster.local:9090/"
  # -- The label to enforce in all proxies PromQL queries.
  label: "namespace"
  # -- Additional arguments for prom-label-proxy
  extraArgs:
    - "--labelValue=foobar-.*"
    - "--enable-label-apis=true"
    - "--unsafe-passthrough-paths=/-/ready,/api/v1/metadata"

[jeremych1000]

I've overridden the readiness probe and it boots. So far so good.

[stieglma]

I didn't update the branch for almost two years, so I guess it is quite old, but it does everything we need. I have indeed not configured any liveness/readiness probes. Not sure if I forgot them, or if this is a feature that didn't exist at that time for the project

[jeremych1000]

I didn't update the branch for almost two years, so I guess it is quite old, but it does everything we need. I have indeed not configured any liveness/readiness probes. Not sure if I forgot them, or if this is a feature that didn't exist at that time for the project

It was indeed added later.

I've modified your code to add https://github.com/prometheus-community/prom-label-proxy/blob/f44a33ca7eeeae46c4a39e347ef970bf1887aff5/injectproxy/routes.go#L324-L328, and also imported encoding/json.

[jeremych1000]

I didn't update the branch for almost two years, so I guess it is quite old, but it does everything we need. I have indeed not configured any liveness/readiness probes. Not sure if I forgot them, or if this is a feature that didn't exist at that time for the project

Mind rebasing with the healthz addition above? Then we can make this the PR that @simonpasquier can review?

[stieglma]

I tried to, but it's not that easy, the whole label handling has been rewritten and there are quite some merge conflicts. I would need to take some time and integrate it again with the new "multi label" thing that exists now. I don't have time for that at the moment and in the forseeable future. I guess it will be easier if you just fork my fork, and add the healthz endpoint yourself.

@simonpasquier if it is wanted (and once I have time) I could try to add my changes for regex labels as a pull request. But I'm not very familiar with go, so that will surely take a while. I assume not before sometime around October

[jeremych1000]

I tried to, but it's not that easy, the whole label handling has been rewritten and there are quite some merge conflicts. I would need to take some time and integrate it again with the new "multi label" thing that exists now. I don't have time for that at the moment and in the forseeable future. I guess it will be easier if you just fork my fork, and add the healthz endpoint yourself.

@simonpasquier if it is wanted (and once I have time) I could try to add my changes for regex labels as a pull request. But I'm not very familiar with go, so that will surely take a while. I assume not before sometime around October

Yeah that's what I've done for now.

Will ask my colleague if he has time to create the PR - he knows Golang better than myself.

[sathieu]

I created https://github.com/prometheus-community/prom-label-proxy/pull/171.

[jeremych1000]

I created #171.

Thanks for this - looking forward to having it merged!