VictoriaMetrics / VictoriaMetrics/operator

RBAC: remove the monitoring.coreos.com wildcard, keep the narrower markers

Open
#2,628 0 comments 0 reactions 1 assignee View on GitHub

@AndrewChubatiuk is already working on this.

Since Sep 17, 2026.

enhancement
Dominant language
Go
Stars
589
Forks
229
Avg merge
1d 9h
Merged PRs (30d)
77

Description

What

Two markers grant the operator write access to every monitoring.coreos.com resource, which it never uses:

  • internal/controller/operator/vmagent_controller.go:78
  • internal/controller/operator/vmsingle_controller.go:84
// +kubebuilder:rbac:groups=monitoring.coreos.com,resources=*,verbs=*

That wildcard also subsumes the 12 narrower markers in prom*_controller.go, so config/rbac/role.yaml ends up with three rules where the first makes the other two dead:

- apiGroups: [monitoring.coreos.com]
  resources: ['*']
  verbs: ['*']                      # subsumes both rules below
- apiGroups: [monitoring.coreos.com]
  resources: [alertmanagerconfigs, podmonitors, probes, prometheusrules, scrapeconfigs, servicemonitors]
  verbs: [get, list, watch]
- apiGroups: [monitoring.coreos.com]
  resources: [alertmanagerconfigs/status, podmonitors/status, probes/status, prometheusrules/status, scrapeconfigs/status, servicemonitors/status]
  verbs: [get, patch, update]

Nothing misbehaves today - RBAC is additive. This is an over-grant, not a bug.

Why fix it

The operator only reads Prometheus objects and updates their /status. Verified on master:

  • promrule_controller.go:67,96 - var instance promv1.PrometheusRule, For(&promv1.PrometheusRule{})
  • zero rclient.Create / Update / Patch / Delete calls on any promv1 or promv1alpha1 type outside tests

So the wildcard is the part to drop. The narrower markers already describe real usage accurately and should stay.

One thing the wildcard does supply

The Prometheus converters set BlockOwnerDeletion: true on the converted VM CRD, with the Prometheus object as owner:

  • factory/converter/apis.go:76 (PrometheusRule), :139 (ServiceMonitor), :417 (PodMonitor), :501 (Probe)
  • factory/converter/v1alpha1/apis.go:126 (AlertmanagerConfig), :182 (ScrapeConfig)

Under the OwnerReferencesPermissionEnforcement admission plugin, setting such an ownerRef requires update on the owner's /finalizers subresource - prometheusrules/finalizers, servicemonitors/finalizers, and so on. No marker declares those; only the wildcard supplies them today.

This is gated on EnabledPrometheusConverterOwnerReferences (internal/config/config.go:753), which defaults to false. So it affects only users who set VM_ENABLEDPROMETHEUSCONVERTEROWNERREFERENCES=true and run that admission plugin - but for them, removing the wildcard alone would break conversion.

Proposed change

  1. Remove the two resources=*,verbs=* markers.
  2. Add explicit finalizer markers for the six convertible owner types, so the case above keeps working:
// +kubebuilder:rbac:groups=monitoring.coreos.com,resources=alertmanagerconfigs/finalizers;podmonitors/finalizers;probes/finalizers;prometheusrules/finalizers;scrapeconfigs/finalizers;servicemonitors/finalizers,verbs=update
  1. make manifests, confirm config/rbac/role.yaml keeps read + /status + /finalizers and loses the wildcard.
  2. Exercise conversion with VM_ENABLEDPROMETHEUSCONVERTEROWNERREFERENCES=true on a cluster running OwnerReferencesPermissionEnforcement. No existing test covers that path, and a missing finalizer or watch permission does not surface as a clean 403 - a denied watch parks the controller-runtime informer, so the operator hangs silently.

Context

Raised in review of VictoriaMetrics/helm-charts#3207, which derives the chart's ClusterRole from this generated artifact. The redundancy was spotted there first; the chart cannot fix it, since .github/workflows/crds.yaml copies this file verbatim on release, so the correction has to happen in the markers here.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.