fix(rbac): permit managed ServiceMonitor reconciliation (#479)

dc-tec · web-flow · commit 693a43cd6f3d · 2026-05-24T22:27:42.000+02:00
Signed-off-by: Roel de Cort &lt;roel.decort@adfinis.com&gt;
diff --git a/charts/openbao-operator/templates/admission/lock-managed-resources.yaml b/charts/openbao-operator/templates/admission/lock-managed-resources.yaml
@@ -144,6 +144,55 @@ spec:
     - name: is_service_request
       expression: >-
         request.kind.group == "" && request.kind.kind == "Service"
+    - name: is_service_monitor_request
+      expression: >-
+        request.kind.group == "monitoring.coreos.com" && request.kind.kind == "ServiceMonitor"
+    - name: current_service_monitor_is_operator_owned
+      expression: >-
+        object != null &&
+        has(object.metadata) &&
+        request.namespace != "" &&
+        object.metadata.name.endsWith("-metrics") &&
+        has(object.metadata.labels) &&
+        ("app.kubernetes.io/managed-by" in object.metadata.labels) &&
+        object.metadata.labels["app.kubernetes.io/managed-by"] == "openbao-operator" &&
+        ("app.kubernetes.io/component" in object.metadata.labels) &&
+        object.metadata.labels["app.kubernetes.io/component"] == "metrics" &&
+        ("openbao.org/component" in object.metadata.labels) &&
+        object.metadata.labels["openbao.org/component"] == "metrics" &&
+        ("openbao.org/cluster" in object.metadata.labels) &&
+        object.metadata.labels["openbao.org/cluster"] != "" &&
+        object.metadata.name == object.metadata.labels["openbao.org/cluster"] + "-metrics" &&
+        has(object.metadata.ownerReferences) &&
+        object.metadata.ownerReferences.exists(ref,
+          ref.apiVersion == "openbao.org/v1alpha1" &&
+          ref.kind == "OpenBaoCluster" &&
+          ref.name == object.metadata.labels["openbao.org/cluster"] &&
+          has(ref.controller) &&
+          ref.controller == true)
+    - name: old_service_monitor_is_operator_owned
+      expression: >-
+        oldObject != null &&
+        has(oldObject.metadata) &&
+        request.namespace != "" &&
+        oldObject.metadata.name.endsWith("-metrics") &&
+        has(oldObject.metadata.labels) &&
+        ("app.kubernetes.io/managed-by" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["app.kubernetes.io/managed-by"] == "openbao-operator" &&
+        ("app.kubernetes.io/component" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["app.kubernetes.io/component"] == "metrics" &&
+        ("openbao.org/component" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["openbao.org/component"] == "metrics" &&
+        ("openbao.org/cluster" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["openbao.org/cluster"] != "" &&
+        oldObject.metadata.name == oldObject.metadata.labels["openbao.org/cluster"] + "-metrics" &&
+        has(oldObject.metadata.ownerReferences) &&
+        oldObject.metadata.ownerReferences.exists(ref,
+          ref.apiVersion == "openbao.org/v1alpha1" &&
+          ref.kind == "OpenBaoCluster" &&
+          ref.name == oldObject.metadata.labels["openbao.org/cluster"] &&
+          has(ref.controller) &&
+          ref.controller == true)
     - name: is_kubelet_node
       expression: >-
         request.userInfo.username.startsWith("system:node:")
@@ -295,6 +344,15 @@ spec:
           check("maintenance").
           allowed()
   validations:
+    - expression: >-
+        !(variables.is_operator_controller || variables.is_operator_provisioner) ||
+        !variables.is_service_monitor_request ||
+        (
+          (request.operation == "CREATE" && variables.current_service_monitor_is_operator_owned) ||
+          (request.operation == "UPDATE" && variables.current_service_monitor_is_operator_owned && variables.old_service_monitor_is_operator_owned) ||
+          (request.operation == "DELETE" && variables.old_service_monitor_is_operator_owned)
+        )
+      message: The operator can only create, update, or delete ServiceMonitors that match the OpenBao metrics ownership shape.
     - expression: >-
         !variables.is_managed ||
         variables.is_operator_controller ||
diff --git a/charts/openbao-operator/templates/admission/provisioner-rbac.yaml b/charts/openbao-operator/templates/admission/provisioner-rbac.yaml
@@ -194,6 +194,15 @@ spec:
               rule.verbs != null &&
               rule.verbs.all(v, v in ['create', 'delete', 'get', 'list', 'patch', 'update', 'watch'])
             ) ||
+            (
+              rule.apiGroups.size() == 1 &&
+              rule.apiGroups[0] == 'monitoring.coreos.com' &&
+              rule.resources != null &&
+              rule.resources.size() == 1 &&
+              rule.resources[0] == 'servicemonitors' &&
+              rule.verbs != null &&
+              rule.verbs.all(v, v in ['create', 'delete', 'get', 'patch'])
+            ) ||
             (
               rule.apiGroups.size() == 1 &&
               rule.apiGroups[0] == 'rbac.authorization.k8s.io' &&
diff --git a/charts/openbao-operator/templates/rbac/single-tenant-clusterrole.yaml b/charts/openbao-operator/templates/rbac/single-tenant-clusterrole.yaml
@@ -164,6 +164,15 @@ rules:
       - update
       - watch{{- end }}
 
+  - apiGroups:
+      - monitoring.coreos.com
+    resources:
+      - servicemonitors
+    verbs:
+      - create
+      - delete
+      - get
+      - patch
   - apiGroups:
       - rbac.authorization.k8s.io
     resources:
diff --git a/config/overlays/single-tenant-custom-identity/single_tenant_clusterrole.yaml b/config/overlays/single-tenant-custom-identity/single_tenant_clusterrole.yaml
@@ -23,10 +23,7 @@ rules:
       - create
       - delete
       - get
-      - list
       - patch
-      - update
-      - watch
   - apiGroups:
       - openbao.org
     resources:
@@ -168,6 +165,15 @@ rules:
       - patch
       - update
       - watch
+  - apiGroups:
+      - monitoring.coreos.com
+    resources:
+      - servicemonitors
+    verbs:
+      - create
+      - delete
+      - get
+      - patch
   - apiGroups:
       - rbac.authorization.k8s.io
     resources:
diff --git a/config/overlays/single-tenant/single_tenant_clusterrole.yaml b/config/overlays/single-tenant/single_tenant_clusterrole.yaml
@@ -23,10 +23,7 @@ rules:
       - create
       - delete
       - get
-      - list
       - patch
-      - update
-      - watch
   - apiGroups:
       - openbao.org
     resources:
@@ -168,6 +165,15 @@ rules:
       - patch
       - update
       - watch
+  - apiGroups:
+      - monitoring.coreos.com
+    resources:
+      - servicemonitors
+    verbs:
+      - create
+      - delete
+      - get
+      - patch
   - apiGroups:
       - rbac.authorization.k8s.io
     resources:
diff --git a/config/policy/openbao-lock-managed-resource-mutations.yaml b/config/policy/openbao-lock-managed-resource-mutations.yaml
@@ -141,6 +141,55 @@ spec:
     - name: is_service_request
       expression: >-
         request.kind.group == "" && request.kind.kind == "Service"
+    - name: is_service_monitor_request
+      expression: >-
+        request.kind.group == "monitoring.coreos.com" && request.kind.kind == "ServiceMonitor"
+    - name: current_service_monitor_is_operator_owned
+      expression: >-
+        object != null &&
+        has(object.metadata) &&
+        request.namespace != "" &&
+        object.metadata.name.endsWith("-metrics") &&
+        has(object.metadata.labels) &&
+        ("app.kubernetes.io/managed-by" in object.metadata.labels) &&
+        object.metadata.labels["app.kubernetes.io/managed-by"] == "openbao-operator" &&
+        ("app.kubernetes.io/component" in object.metadata.labels) &&
+        object.metadata.labels["app.kubernetes.io/component"] == "metrics" &&
+        ("openbao.org/component" in object.metadata.labels) &&
+        object.metadata.labels["openbao.org/component"] == "metrics" &&
+        ("openbao.org/cluster" in object.metadata.labels) &&
+        object.metadata.labels["openbao.org/cluster"] != "" &&
+        object.metadata.name == object.metadata.labels["openbao.org/cluster"] + "-metrics" &&
+        has(object.metadata.ownerReferences) &&
+        object.metadata.ownerReferences.exists(ref,
+          ref.apiVersion == "openbao.org/v1alpha1" &&
+          ref.kind == "OpenBaoCluster" &&
+          ref.name == object.metadata.labels["openbao.org/cluster"] &&
+          has(ref.controller) &&
+          ref.controller == true)
+    - name: old_service_monitor_is_operator_owned
+      expression: >-
+        oldObject != null &&
+        has(oldObject.metadata) &&
+        request.namespace != "" &&
+        oldObject.metadata.name.endsWith("-metrics") &&
+        has(oldObject.metadata.labels) &&
+        ("app.kubernetes.io/managed-by" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["app.kubernetes.io/managed-by"] == "openbao-operator" &&
+        ("app.kubernetes.io/component" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["app.kubernetes.io/component"] == "metrics" &&
+        ("openbao.org/component" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["openbao.org/component"] == "metrics" &&
+        ("openbao.org/cluster" in oldObject.metadata.labels) &&
+        oldObject.metadata.labels["openbao.org/cluster"] != "" &&
+        oldObject.metadata.name == oldObject.metadata.labels["openbao.org/cluster"] + "-metrics" &&
+        has(oldObject.metadata.ownerReferences) &&
+        oldObject.metadata.ownerReferences.exists(ref,
+          ref.apiVersion == "openbao.org/v1alpha1" &&
+          ref.kind == "OpenBaoCluster" &&
+          ref.name == oldObject.metadata.labels["openbao.org/cluster"] &&
+          has(ref.controller) &&
+          ref.controller == true)
     - name: is_kubelet_node
       expression: >-
         request.userInfo.username.startsWith("system:node:")
@@ -292,6 +341,15 @@ spec:
           check("maintenance").
           allowed()
   validations:
+    - expression: >-
+        !(variables.is_operator_controller || variables.is_operator_provisioner) ||
+        !variables.is_service_monitor_request ||
+        (
+          (request.operation == "CREATE" && variables.current_service_monitor_is_operator_owned) ||
+          (request.operation == "UPDATE" && variables.current_service_monitor_is_operator_owned && variables.old_service_monitor_is_operator_owned) ||
+          (request.operation == "DELETE" && variables.old_service_monitor_is_operator_owned)
+        )
+      message: The operator can only create, update, or delete ServiceMonitors that match the OpenBao metrics ownership shape.
     - expression: >-
         !variables.is_managed ||
         variables.is_operator_controller ||
diff --git a/config/policy/openbao-restrict-provisioner-rbac.yaml b/config/policy/openbao-restrict-provisioner-rbac.yaml
@@ -191,6 +191,15 @@ spec:
               rule.verbs != null &&
               rule.verbs.all(v, v in ['create', 'delete', 'get', 'list', 'patch', 'update', 'watch'])
             ) ||
+            (
+              rule.apiGroups.size() == 1 &&
+              rule.apiGroups[0] == 'monitoring.coreos.com' &&
+              rule.resources != null &&
+              rule.resources.size() == 1 &&
+              rule.resources[0] == 'servicemonitors' &&
+              rule.verbs != null &&
+              rule.verbs.all(v, v in ['create', 'delete', 'get', 'patch'])
+            ) ||
             (
               rule.apiGroups.size() == 1 &&
               rule.apiGroups[0] == 'rbac.authorization.k8s.io' &&
diff --git a/config/rbac/single_tenant_clusterrole.yaml b/config/rbac/single_tenant_clusterrole.yaml
@@ -190,6 +190,16 @@ rules:
       - patch
       - update
       - watch
+  # Observability
+  - apiGroups:
+      - monitoring.coreos.com
+    resources:
+      - servicemonitors
+    verbs:
+      - create
+      - delete
+      - get
+      - patch
   # RBAC for OpenBao pod discovery
   - apiGroups:
       - rbac.authorization.k8s.io
diff --git a/docs/security/infrastructure/rbac.md b/docs/security/infrastructure/rbac.md
@@ -150,7 +150,7 @@ The provisioner writes the RBAC that grants the controller access, but it does n
     {
       cells: [
         'Manage tenant-scoped workload resources',
-        'StatefulSets, Services, ConfigMaps, Jobs, and related resources are the normal lifecycle surface.',
+        'StatefulSets, Services, ConfigMaps, Jobs, ServiceMonitors, and related resources are the normal lifecycle surface.',
         'This access only exists where the provisioner already introduced the controller via RoleBinding.',
       ],
     },
diff --git a/docs/user-guide/openbaocluster/configuration/observability.md b/docs/user-guide/openbaocluster/configuration/observability.md
@@ -303,6 +303,12 @@ spec:
   Keep the first alert set small. Availability, backup freshness, sustained read-pool degradation, and sustained reconcile failure are the highest-value starting signals.
 </CommandBlock>
 
+<Callout type="note" title="Alert rules are not operator-managed">
+
+The OpenBaoCluster controller manages the workload `ServiceMonitor` when `spec.observability.metrics` enables it. `PrometheusRule` resources are user-applied observability objects, so the ServiceAccount or GitOps controller applying the alert rules needs `monitoring.coreos.com` `prometheusrules` permissions in that namespace.
+
+</Callout>
+
 ## Dashboards, logs, and health
 
 <CommandBlock
diff --git a/internal/service/networking/metrics.go b/internal/service/networking/metrics.go
@@ -113,11 +113,15 @@ func buildWorkloadServiceMonitor(cluster *openbaov1alpha1.OpenBaoCluster) (*unst
 
 	serviceMonitor := cluster.Spec.Observability.Metrics.ServiceMonitor
 	labels := metricsResourceLabels(cluster)
+	requiredLabels := metricsResourceLabels(cluster)
 	if serviceMonitor != nil {
 		for key, value := range serviceMonitor.Labels {
 			labels[key] = value
 		}
 	}
+	for key, value := range requiredLabels {
+		labels[key] = value
+	}
 
 	endpoint, err := buildServiceMonitorEndpoint(cluster, serviceMonitor)
 	if err != nil {
diff --git a/internal/service/networking/metrics_test.go b/internal/service/networking/metrics_test.go
@@ -8,6 +8,7 @@ import (
 	"k8s.io/utils/ptr"
 
 	openbaov1alpha1 "github.com/dc-tec/openbao-operator/api/v1alpha1"
+	"github.com/dc-tec/openbao-operator/internal/platform/constants"
 )
 
 func TestBuildWorkloadServiceMonitor_IncludesAuthTLSAndSelector(t *testing.T) {
@@ -106,6 +107,42 @@ func TestBuildWorkloadServiceMonitor_IncludesAuthTLSAndSelector(t *testing.T) {
 	}
 }
 
+func TestBuildWorkloadServiceMonitor_PreservesOperatorIdentityLabels(t *testing.T) {
+	cluster := newMinimalCluster("metrics-cluster", "security")
+	cluster.Spec.Observability = &openbaov1alpha1.ObservabilityConfig{
+		Metrics: &openbaov1alpha1.MetricsConfig{
+			Enabled: true,
+			ServiceMonitor: &openbaov1alpha1.ServiceMonitorConfig{
+				Enabled: true,
+				Labels: map[string]string{
+					constants.LabelAppManagedBy:   "user",
+					constants.LabelOpenBaoCluster: "other-cluster",
+					constants.LabelAppComponent:   "custom",
+					"release":                     "kube-prometheus-stack",
+				},
+			},
+		},
+	}
+
+	monitor, err := buildWorkloadServiceMonitor(cluster)
+	if err != nil {
+		t.Fatalf("buildWorkloadServiceMonitor() error = %v", err)
+	}
+	labels := monitor.GetLabels()
+	if got := labels[constants.LabelAppManagedBy]; got != constants.LabelValueAppManagedByOpenBaoOperator {
+		t.Fatalf("managed-by label = %q, want %q", got, constants.LabelValueAppManagedByOpenBaoOperator)
+	}
+	if got := labels[constants.LabelOpenBaoCluster]; got != cluster.Name {
+		t.Fatalf("cluster label = %q, want %q", got, cluster.Name)
+	}
+	if got := labels[constants.LabelAppComponent]; got != metricsComponentLabelValue {
+		t.Fatalf("component label = %q, want %q", got, metricsComponentLabelValue)
+	}
+	if got := labels["release"]; got != "kube-prometheus-stack" {
+		t.Fatalf("user label = %q", got)
+	}
+}
+
 func TestBuildWorkloadServiceMonitor_RejectsInvalidTLSCAConfig(t *testing.T) {
 	cluster := newMinimalCluster("metrics-invalid", "default")
 	cluster.Spec.Observability = &openbaov1alpha1.ObservabilityConfig{
diff --git a/internal/service/provisioner/manager_test.go b/internal/service/provisioner/manager_test.go
@@ -328,8 +328,9 @@ func TestEnsureTenantRBAC_UpdatesRoleWhenRulesChange(t *testing.T) {
 	}
 
 	// Verify rules were updated
-	if len(role.Rules) != 15 {
-		t.Errorf("Role rules count = %v, want 15", len(role.Rules))
+	expectedRules := len(GenerateTenantRole(namespace).Rules)
+	if len(role.Rules) != expectedRules {
+		t.Errorf("Role rules count = %v, want %v", len(role.Rules), expectedRules)
 	}
 
 	// Verify at least one rule has the expected OpenBaoCluster permissions
diff --git a/internal/service/provisioner/rbac.go b/internal/service/provisioner/rbac.go
@@ -8,13 +8,14 @@ import (
 )
 
 var (
-	verbsReadOnly     = []string{"get", "list", "watch"}
-	verbsManage       = []string{"create", "delete", "get", "list", "patch", "update", "watch"}
-	verbsEventWrite   = []string{"create", "patch"}
-	verbsPodManage    = []string{"delete", "get", "list", "patch", "update", "watch"}
-	verbsSecretRead   = []string{"get"}
-	verbsSecretCreate = []string{"create"}
-	verbsSecretMutate = []string{"delete", "get", "patch", "update"}
+	verbsReadOnly             = []string{"get", "list", "watch"}
+	verbsManage               = []string{"create", "delete", "get", "list", "patch", "update", "watch"}
+	verbsEventWrite           = []string{"create", "patch"}
+	verbsPodManage            = []string{"delete", "get", "list", "patch", "update", "watch"}
+	verbsServiceMonitorManage = []string{"create", "delete", "get", "patch"}
+	verbsSecretRead           = []string{"get"}
+	verbsSecretCreate         = []string{"create"}
+	verbsSecretMutate         = []string{"delete", "get", "patch", "update"}
 )
 
 func cloneStrings(in []string) []string {
@@ -110,6 +111,11 @@ func GenerateTenantRole(namespace string) *rbacv1.Role {
 			Resources: []string{"httproutes", "tlsroutes", "backendtlspolicies"},
 			Verbs:     cloneStrings(verbsManage),
 		},
+		{
+			APIGroups: []string{"monitoring.coreos.com"},
+			Resources: []string{"servicemonitors"},
+			Verbs:     cloneStrings(verbsServiceMonitorManage),
+		},
 		{
 			APIGroups: []string{"rbac.authorization.k8s.io"},
 			Resources: []string{"roles", "rolebindings"},
diff --git a/internal/service/provisioner/rbac_test.go b/internal/service/provisioner/rbac_test.go
diff --git a/test/integration/kustomize_contract_test.go b/test/integration/kustomize_contract_test.go
diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go
diff --git a/test/integration/vap_lock_managed_rbac_test.go b/test/integration/vap_lock_managed_rbac_test.go
diff --git a/test/manifests/prometheus-operator/v0.91.0/README.md b/test/manifests/prometheus-operator/v0.91.0/README.md
diff --git a/test/manifests/prometheus-operator/v0.91.0/crds/monitoring.coreos.com_servicemonitors.yaml b/test/manifests/prometheus-operator/v0.91.0/crds/monitoring.coreos.com_servicemonitors.yaml
diff --git a/test/utils/policy_restrict_provisioner_rbac_test.go b/test/utils/policy_restrict_provisioner_rbac_test.go

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ The provisioner writes the RBAC that grants the controller access, but it does n`
`150`	`150`	`{`
`151`	`151`	`cells: [`
`152`	`152`	`'Manage tenant-scoped workload resources',`
`153`		`- 'StatefulSets, Services, ConfigMaps, Jobs, and related resources are the normal lifecycle surface.',`
	`153`	`+ 'StatefulSets, Services, ConfigMaps, Jobs, ServiceMonitors, and related resources are the normal lifecycle surface.',`
`154`	`154`	`'This access only exists where the provisioner already introduced the controller via RoleBinding.',`
`155`	`155`	`],`
`156`	`156`	`},`