Support for scrapeclass in VMServiceScrape, VMPodScrape, VMProbe

[ankitdh7]

Use-case

We use prometheus ServiceMonitor, PodMonitor, and Probe as inputs for scraping targets. These resources are translated by the VM operator into VMServiceScrape, VMPodScrape, and VMProbe, which are then scraped by VMagent.

Recently, the Prometheus Operator introduced scrapeClass to standardize common configurations such as relabeling rules, TLS certificates, and authentication. This feature enables strict mTLS support in PodMonitors and Probes (ServiceMonitor already supported it via tlsConfig) for scraping workloads inside an Istio mesh.

While VMPodScrape already supports mTLS through tlsConfig, the issue arises when scraping config comes from ServiceMonitor, PodMonitor, and Probe. When these CRs are converted into their VM equivalents, the TLS settings from scrapeClass are not passed through, leaving us unable to use mTLS with VMPodScrape and VMProbe.

Steps to reproduce

• Enable the scrapeclass in prometheus helm chart like this
• Apply the podmonitor as below

kubectl apply -f - <<EOF
apiVersion: monitoring.coreos.com/v1
kind: PodMonitor
metadata:
 name: test-podmonitor
 labels:
   release: prometheus 
spec:
 scrapeClass: istio-mtls
 podMetricsEndpoints:
 - interval: 60s
   path: /metrics
   port: metrics-port
   relabelings:
   - action: replace
     sourceLabels: [__meta_kubernetes_pod_label_status]
     targetLabel: status
   - action: replace
     sourceLabels: [pod]
     targetLabel: instance
 selector:
   matchLabels:
     app.kubernetes.io/instance: test
EOF

• The pod monitor has been loaded with TLS config in prometheus
• The translated VMPodscrape has been loaded in the VMagent but without TLS config

VM Versions:

• Helm chart - victoria-metrics-k8s-stack-0.27.6
• VMOperator - v0.48.3
• VMAgent - v1.105.0
• App version - v1.105.0

Could you please help if there are plans to support scrapeClass in the VM operator? If not, do you have any recommendations on how to achieve strict mTLS support for VMPodScrape and VMProbe when using ServiceMonitor, PodMonitor, and Probe as inputs? Thanks in advance!

[endesapt]

I would like to try to work on that feature. As for me the best way to implement this is to copy Prometheus Operator approach: add scrapeClass field to PodScrape, ServiceScrape, SCrapeConfig, Probe . Add scrapeClasses field to VMAgent similar to Prometheus one. I guess it is to complicated and unnecessary to copy scrapeClasses from Prometheus CRD: sometimes you will not be able to solve the problem with certs mounted to Prometheus/prometheus image itself. So i guess configuration of scrapeclasses of VMAgent should be on the user side. IF VMAgent dont have scrapeclass of Any CRD it collected, it will also ignore it ,like prometheus do with an event:

ServiceMonitor example-service-monitor was rejected due to invalid configuration:  
scrapeClass "istio" not found in Prometheus scrapeClasses

[f41gh7]

@endesapt thanks for the initial implementation. I created a small follow-up commit, which changes a bit field names at scrapeClass, most notable change are relabelings and metricRelabelings rename. It has a different names compared to the Prometheus, but I think, it's better to align it with our current naming at Scrape Objects.

Also, I added OAuth2 and BasicAuth to the ScrapeClass. Feel free to provide any feedback on it.

96d9c34#diff-da13336d6df8ab1b3324185381d6d1ae3cb55ab3fafa2da17738cfd138ff088a

[f41gh7]

It looks like scrapeClass merge logic should be a bit adjusted. Secret or ConfigMap auth options must have priority for *File related auth configurations.

As an example:

scrapeClass: tlsConfig.Cert.Secret must have priority over podScrape.spec.endpoint.tlsConfig.CertFile.

It improves security for VMAgent. See linked issue.

[endesapt]

scrapeClass: tlsConfig.Cert.Secret must have priority over podScrape.spec.endpoint.tlsConfig.CertFile.

1. Won't there be a situation if we have VMAgent with default scrapeClass and tlsConfig.Cert.Secret, and for example, we want to override it with CertFile (on some VMProbe resource), but we cant do that because of the priority?

2. I think it is a little bit confusing that specs of resources sometimes will be overrided even if they exist, especially because if i understand it right, Prometheus Operator has the same behaviour we have now, and changing it may confuse users that migrate from it.

So in general i think it is great, that TLS is not overrided by scrapeClass if it exists on resource side, even if it is not as secure, it gives flexibility to end user.

If i missed something, correct me

[f41gh7]

The feature was included into v0.64.0 release