ODH Monitoring: Label boolean value validation admission policy (#3127)

* [+] odh monitoring label boolean value VAP/B

### Monitoring - Admission Components - Policies - Validations ###

* [+] e2e odh monitoring label const

* [+] odh monitoring label value enforcment e2e tests

Author: bro-adm

internal/controller/services/monitoring/monitoring_controller_actions.go

@@ -25,6 +25,7 @@ import (
 const (
 	// Template files.
 	MonitoringStackTemplate                       = "resources/monitoring-stack.tmpl.yaml"
+	MonitoringAdmissionPoliciesTemplate           = "resources/monitoring-admission-policies.tmpl.yaml"
 	MonitoringStackAlertmanagerRBACTemplate       = "resources/monitoringstack-alertmanager-rbac.tmpl.yaml"
 	TempoMonolithicTemplate                       = "resources/tempo-monolithic.tmpl.yaml"
 	TempoStackTemplate                            = "resources/tempo-stack.tmpl.yaml"
@@ -182,8 +183,20 @@ func updatePrometheusConfigMap(ctx context.Context, rr *odhtypes.ReconciliationR
 // Note: ValidatingAdmissionPolicy is a built-in Kubernetes API resource (not a CRD) available in K8s 1.26+.
 // If the API is not available, the controller setup will fail when trying to create the watch, providing
 // a clear error message to the user that their cluster doesn't support this feature.
-func deployMonitoringAdmissionPolicies(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
-	// TODO: Implement admission policies deployment logic
+func deployMonitoringAdmissionPolicies(_ context.Context, rr *odhtypes.ReconciliationRequest) error {
+	if _, ok := rr.Instance.(*serviceApi.Monitoring); !ok {
+		return errors.New("instance is not of type *services.Monitoring")
+	}
+
+	// no need to validate gvk of validation policy and binding because its a builtin type not a crd
+
+	// Deploy admission policy templates
+	// If the ValidatingAdmissionPolicy API doesn't exist, deployment will fail with a clear error
+	templates := []odhtypes.TemplateInfo{
+		{FS: resourcesFS, Path: MonitoringAdmissionPoliciesTemplate},
+	}
+
+	rr.Templates = append(rr.Templates, templates...)
 	return nil
 }
 

internal/controller/services/monitoring/resources/monitoring-admission-policies.tmpl.yaml

@@ -0,0 +1,34 @@
+# Base admission policies that are always deployed
+# These validate that monitoring label values are boolean strings
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: "monitoring-labels-values-validation"
+spec:
+  failurePolicy: Fail
+  matchConstraints:
+    resourceRules:
+      - apiGroups: ["monitoring.coreos.com"]
+        apiVersions: ["v1"]
+        operations: ["CREATE", "UPDATE"]
+        resources: ["podmonitors", "servicemonitors"]
+      - apiGroups: [""]
+        apiVersions: ["v1"]
+        operations: ["CREATE", "UPDATE"]
+        resources: ["namespaces"]
+  validations:
+    - expression: >
+        !has(object.metadata.labels) ||
+        !('opendatahub.io/monitoring' in object.metadata.labels) ||
+        object.metadata.labels['opendatahub.io/monitoring'] in ['true', 'false']
+      message: "The label 'opendatahub.io/monitoring' must be set to 'true' or 'false'."
+
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: monitoring-labels-values-validation-binding
+spec:
+  policyName: monitoring-labels-values-validation
+  validationActions: ["Deny"]
+

pkg/metadata/labels/types.go

@@ -15,6 +15,7 @@ const (
 	Platform                = "platform"
 	True                    = "true"
 	CustomizedAppNamespace  = "opendatahub.io/application-namespace"
+	ODHLabelMonitoring      = "opendatahub.io/monitoring"
 )
 
 // K8SCommon keeps common kubernetes labels [1]

tests/e2e/monitoring_admission_components_test.go

@@ -4,12 +4,14 @@ import (
 	"testing"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/status"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/testf"
 
@@ -25,8 +27,6 @@ const (
 // createMonitorsEnvironment sets up the namespace and monitors with specific labels.
 // Pre-test cleanup: Ensures resources from previous test runs are deleted before creation.
 // Post-test: Respects --deletion-policy flag (cleanup handled by framework, not this function).
-//
-//nolint:unused // Helper function for future webhook tests - will be used when webhook validation tests are added
 func (tc *MonitoringTestCtx) createMonitorsEnvironment(t *testing.T, namespaceLabels map[string]string, monitorLabels map[string]string) {
 	t.Helper()
 
@@ -50,16 +50,16 @@ func (tc *MonitoringTestCtx) createMonitorsEnvironment(t *testing.T, namespaceLa
 	t.Logf("Pre-test cleanup completed")
 
 	// Helper to apply metadata labels if provided
-	applyLabels := func(labels map[string]string) testf.TransformFn {
+	applyLabels := func(lbls map[string]string) testf.TransformFn {
 		return func(obj *unstructured.Unstructured) error {
-			if len(labels) == 0 {
+			if len(lbls) == 0 {
 				return nil
 			}
 			currentLabels := obj.GetLabels()
 			if currentLabels == nil {
 				currentLabels = make(map[string]string)
 			}
-			for k, v := range labels {
+			for k, v := range lbls {
 				currentLabels[k] = v
 			}
 			obj.SetLabels(currentLabels)
@@ -126,3 +126,165 @@ func (tc *MonitoringTestCtx) ValidateMonitoringWebhookTestsSetup(t *testing.T) {
 
 	t.Logf("Webhook tests setup complete: monitoring is enabled and ready")
 }
+
+// ValidateMonitoringLabelValueEnforcementOnNamespace tests that the validation policy blocks invalid monitoring label values.
+func (tc *MonitoringTestCtx) ValidateMonitoringLabelValueEnforcementOnNamespace(t *testing.T) {
+	t.Helper()
+
+	// Pre-test cleanup: ensure namespace doesn't exist from prior runs
+	tc.DeleteResource(
+		WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: TestNamespaceName}),
+		WithIgnoreNotFound(true),
+		WithWaitForDeletion(true),
+	)
+
+	// Attempt to create namespace with INVALID monitoring label value (not "true" or "false")
+	invalidNamespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: TestNamespaceName,
+			Labels: map[string]string{
+				labels.ODHLabelMonitoring: "invalid-value", // Invalid!
+			},
+		},
+	}
+
+	// Expect this to be BLOCKED by validation policy
+	err := tc.Client().Create(tc.Context(), invalidNamespace)
+	tc.g.Expect(err).To(HaveOccurred(), "Validation policy should block namespace with invalid monitoring label value")
+	tc.g.Expect(err).To(MatchError(ContainSubstring("must be set to 'true' or 'false'")), "Error message should indicate valid values")
+
+	// Explicit cleanup based on deletion policy (runs only if test reaches this point)
+	switch testOpts.deletionPolicy {
+	case DeletionPolicyAlways:
+		t.Logf("Deletion Policy: Always. Cleaning up test namespace %s", TestNamespaceName)
+		tc.DeleteResource(
+			WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: TestNamespaceName}),
+			WithIgnoreNotFound(true),
+			WithWaitForDeletion(true),
+		)
+	case DeletionPolicyOnFailure:
+		if t.Failed() {
+			t.Logf("Test failed. Deletion Policy: On Failure. Cleaning up test namespace %s", TestNamespaceName)
+			tc.DeleteResource(
+				WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: TestNamespaceName}),
+				WithIgnoreNotFound(true),
+				WithWaitForDeletion(true),
+			)
+		}
+	case DeletionPolicyNever:
+		t.Logf("Deletion Policy: Never. Skipping cleanup of test namespace %s", TestNamespaceName)
+	}
+}
+
+// ValidateMonitoringLabelValueEnforcementOnMonitors tests that the validation policy blocks invalid monitoring label values.
+func (tc *MonitoringTestCtx) ValidateMonitoringLabelValueEnforcementOnMonitors(t *testing.T) {
+	t.Helper()
+
+	// Pre-test cleanup: ensure invalid monitors don't exist from prior runs
+	tc.DeleteResource(
+		WithMinimalObject(gvk.CoreosPodMonitor, types.NamespacedName{Name: "test-invalid-podmonitor", Namespace: TestNamespaceName}),
+		WithIgnoreNotFound(true),
+		WithWaitForDeletion(true),
+	)
+	tc.DeleteResource(
+		WithMinimalObject(gvk.CoreosServiceMonitor, types.NamespacedName{Name: "test-invalid-servicemonitor", Namespace: TestNamespaceName}),
+		WithIgnoreNotFound(true),
+		WithWaitForDeletion(true),
+	)
+
+	// 1. Create a valid namespace first (validation usually requires the namespace to exist)
+	tc.createMonitorsEnvironment(t, nil, nil) // Creates namespace & clean monitors. We will ignore the clean monitors.
+
+	// Define the invalid labels we want to test
+	invalidLabels := map[string]string{
+		labels.ODHLabelMonitoring: "invalid-value",
+	}
+
+	// --- Test PodMonitor Validation ---
+
+	// Define an invalid PodMonitor object locally
+	invalidPodMonitor := &unstructured.Unstructured{}
+	invalidPodMonitor.SetGroupVersionKind(gvk.CoreosPodMonitor)
+	invalidPodMonitor.SetName("test-invalid-podmonitor")
+	invalidPodMonitor.SetNamespace(TestNamespaceName)
+	invalidPodMonitor.SetLabels(invalidLabels)
+	// Minimal valid spec so K8s doesn't reject it for schema reasons
+	invalidPodMonitor.Object["spec"] = map[string]interface{}{
+		"selector": map[string]interface{}{
+			"matchLabels": map[string]interface{}{"app": "test"},
+		},
+		"podMetricsEndpoints": []interface{}{
+			map[string]interface{}{"port": "metrics"},
+		},
+	}
+
+	// Attempt to create it - Expect Error
+	err := tc.Client().Create(tc.Context(), invalidPodMonitor)
+	tc.g.Expect(err).To(HaveOccurred(), "Validation policy should block PodMonitor with invalid monitoring label value")
+	tc.g.Expect(err).To(MatchError(ContainSubstring("must be set to 'true' or 'false'")), "Error message should indicate valid values for PodMonitor")
+
+	// --- Test ServiceMonitor Validation ---
+
+	// Define an invalid ServiceMonitor object locally
+	invalidServiceMonitor := &unstructured.Unstructured{}
+	invalidServiceMonitor.SetGroupVersionKind(gvk.CoreosServiceMonitor)
+	invalidServiceMonitor.SetName("test-invalid-servicemonitor")
+	invalidServiceMonitor.SetNamespace(TestNamespaceName)
+	invalidServiceMonitor.SetLabels(invalidLabels)
+	// Minimal valid spec
+	invalidServiceMonitor.Object["spec"] = map[string]interface{}{
+		"selector": map[string]interface{}{
+			"matchLabels": map[string]interface{}{"app": "test"},
+		},
+		"endpoints": []interface{}{
+			map[string]interface{}{"port": "metrics"},
+		},
+	}
+
+	// Attempt to create it - Expect Error
+	err = tc.Client().Create(tc.Context(), invalidServiceMonitor)
+	tc.g.Expect(err).To(HaveOccurred(), "Validation policy should block ServiceMonitor with invalid monitoring label value")
+	tc.g.Expect(err).To(MatchError(ContainSubstring("must be set to 'true' or 'false'")), "Error message should indicate valid values for ServiceMonitor")
+
+	// Explicit cleanup based on deletion policy (runs only if test reaches this point)
+	switch testOpts.deletionPolicy {
+	case DeletionPolicyAlways:
+		t.Logf("Deletion Policy: Always. Cleaning up test resources in namespace %s", TestNamespaceName)
+		tc.DeleteResource(
+			WithMinimalObject(gvk.CoreosPodMonitor, types.NamespacedName{Name: "test-invalid-podmonitor", Namespace: TestNamespaceName}),
+			WithIgnoreNotFound(true),
+			WithWaitForDeletion(true),
+		)
+		tc.DeleteResource(
+			WithMinimalObject(gvk.CoreosServiceMonitor, types.NamespacedName{Name: "test-invalid-servicemonitor", Namespace: TestNamespaceName}),
+			WithIgnoreNotFound(true),
+			WithWaitForDeletion(true),
+		)
+		tc.DeleteResource(
+			WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: TestNamespaceName}),
+			WithIgnoreNotFound(true),
+			WithWaitForDeletion(true),
+		)
+	case DeletionPolicyOnFailure:
+		if t.Failed() {
+			t.Logf("Test failed. Deletion Policy: On Failure. Cleaning up test resources in namespace %s", TestNamespaceName)
+			tc.DeleteResource(
+				WithMinimalObject(gvk.CoreosPodMonitor, types.NamespacedName{Name: "test-invalid-podmonitor", Namespace: TestNamespaceName}),
+				WithIgnoreNotFound(true),
+				WithWaitForDeletion(true),
+			)
+			tc.DeleteResource(
+				WithMinimalObject(gvk.CoreosServiceMonitor, types.NamespacedName{Name: "test-invalid-servicemonitor", Namespace: TestNamespaceName}),
+				WithIgnoreNotFound(true),
+				WithWaitForDeletion(true),
+			)
+			tc.DeleteResource(
+				WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: TestNamespaceName}),
+				WithIgnoreNotFound(true),
+				WithWaitForDeletion(true),
+			)
+		}
+	case DeletionPolicyNever:
+		t.Logf("Deletion Policy: Never. Skipping cleanup of test resources in namespace %s", TestNamespaceName)
+	}
+}

tests/e2e/monitoring_test.go

@@ -142,6 +142,8 @@ func monitoringTestSuite(t *testing.T) {
 	if testOpts.webhookTest {
 		testCases = append(testCases,
 			TestCase{"Setup monitoring admission components tests", monitoringServiceCtx.ValidateMonitoringWebhookTestsSetup},
+			TestCase{"Validate monitoring label value enforcement on namespace", monitoringServiceCtx.ValidateMonitoringLabelValueEnforcementOnNamespace},
+			TestCase{"Validate monitoring label value enforcement on monitors", monitoringServiceCtx.ValidateMonitoringLabelValueEnforcementOnMonitors},
 		)
 	}