From 7da43aae2a4120958b862f2391b5f30c9ed4de19 Mon Sep 17 00:00:00 2001
From: Mara Sophie Grosch <littlefox@lf-net.org>
Date: Mon, 15 Nov 2021 22:21:50 +0100
Subject: [PATCH] monitoring: allow overriding monitoring labels

The templates for the mgr-generated ServiceMonitor and PrometheusRule objects included the labels
prometheus and team, making it impossible to override them as user.

This adds a new method `OverwriteApplyToObjectMeta` to pkg/apis/rook.io.Labels,
which, contrary to the existing `ApplyToObjectMeta` method, overwrites existing
labels.

Backport of PR #9180
Signed-off-by: Mara Sophie Grosch <littlefox@lf-net.org>
---
 pkg/apis/rook.io/labels.go                    | 10 ++++
 .../{labels_spec.go => labels_test.go}        | 54 +++++++++++++++++++
 pkg/operator/ceph/cluster/mgr/mgr.go          |  4 +-
 pkg/operator/k8sutil/prometheus_test.go       |  1 +
 4 files changed, 67 insertions(+), 2 deletions(-)
 rename pkg/apis/rook.io/{labels_spec.go => labels_test.go} (69%)

diff --git a/pkg/apis/rook.io/labels.go b/pkg/apis/rook.io/labels.go
index 85a6de000673..5004613d0c2c 100644
--- a/pkg/apis/rook.io/labels.go
+++ b/pkg/apis/rook.io/labels.go
@@ -44,6 +44,16 @@ func (a Labels) ApplyToObjectMeta(t *metav1.ObjectMeta) {
 	}
 }
 
+// OverwriteApplyToObjectMeta adds labels to object meta, overwriting keys that are already defined.
+func (a Labels) OverwriteApplyToObjectMeta(t *metav1.ObjectMeta) {
+	if t.Labels == nil {
+		t.Labels = map[string]string{}
+	}
+	for k, v := range a {
+		t.Labels[k] = v
+	}
+}
+
 // Merge returns a Labels which results from merging the attributes of the
 // original Labels with the attributes of the supplied one. The supplied
 // Labels attributes will override the original ones if defined.
diff --git a/pkg/apis/rook.io/labels_spec.go b/pkg/apis/rook.io/labels_test.go
similarity index 69%
rename from pkg/apis/rook.io/labels_spec.go
rename to pkg/apis/rook.io/labels_test.go
index aec8ce6415ca..7b219335843e 100644
--- a/pkg/apis/rook.io/labels_spec.go
+++ b/pkg/apis/rook.io/labels_test.go
@@ -77,6 +77,60 @@ func TestLabelsApply(t *testing.T) {
 	}
 }
 
+func TestLabelsOverwriteApply(t *testing.T) {
+	tcs := []struct {
+		name     string
+		target   *metav1.ObjectMeta
+		input    Labels
+		expected Labels
+	}{
+		{
+			name:   "it should be able to update meta with no label",
+			target: &metav1.ObjectMeta{},
+			input: Labels{
+				"foo": "bar",
+			},
+			expected: Labels{
+				"foo": "bar",
+			},
+		},
+		{
+			name: "it should keep the original labels when new labels are set",
+			target: &metav1.ObjectMeta{
+				Labels: Labels{
+					"foo": "bar",
+				},
+			},
+			input: Labels{
+				"hello": "world",
+			},
+			expected: Labels{
+				"foo":   "bar",
+				"hello": "world",
+			},
+		},
+		{
+			name: "it should overwrite the existing keys",
+			target: &metav1.ObjectMeta{
+				Labels: Labels{
+					"foo": "bar",
+				},
+			},
+			input: Labels{
+				"foo": "baz",
+			},
+			expected: Labels{
+				"foo": "baz",
+			},
+		},
+	}
+
+	for _, tc := range tcs {
+		tc.input.OverwriteApplyToObjectMeta(tc.target)
+		assert.Equal(t, map[string]string(tc.expected), tc.target.Labels)
+	}
+}
+
 func TestLabelsMerge(t *testing.T) {
 	testLabelsPart1 := Labels{
 		"foo":   "bar",
diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go
index 9f9b5d870bd4..9e0bf9292ab5 100644
--- a/pkg/operator/ceph/cluster/mgr/mgr.go
+++ b/pkg/operator/ceph/cluster/mgr/mgr.go
@@ -469,7 +469,7 @@ func (c *Cluster) EnableServiceMonitor(activeDaemon string) error {
 	}
 	serviceMonitor.SetName(AppName)
 	serviceMonitor.SetNamespace(c.clusterInfo.Namespace)
-	cephv1.GetMonitoringLabels(c.spec.Labels).ApplyToObjectMeta(&serviceMonitor.ObjectMeta)
+	cephv1.GetMonitoringLabels(c.spec.Labels).OverwriteApplyToObjectMeta(&serviceMonitor.ObjectMeta)
 
 	if c.spec.External.Enable {
 		serviceMonitor.Spec.Endpoints[0].Port = controller.ServiceExternalMetricName
@@ -505,7 +505,7 @@ func (c *Cluster) DeployPrometheusRule(name, namespace string) error {
 	if err != nil {
 		return errors.Wrapf(err, "failed to set owner reference to prometheus rule %q", prometheusRule.Name)
 	}
-	cephv1.GetMonitoringLabels(c.spec.Labels).ApplyToObjectMeta(&prometheusRule.ObjectMeta)
+	cephv1.GetMonitoringLabels(c.spec.Labels).OverwriteApplyToObjectMeta(&prometheusRule.ObjectMeta)
 	if _, err := k8sutil.CreateOrUpdatePrometheusRule(prometheusRule); err != nil {
 		return errors.Wrap(err, "prometheus rule could not be deployed")
 	}
diff --git a/pkg/operator/k8sutil/prometheus_test.go b/pkg/operator/k8sutil/prometheus_test.go
index db9ad42cb439..5910bb5a6081 100644
--- a/pkg/operator/k8sutil/prometheus_test.go
+++ b/pkg/operator/k8sutil/prometheus_test.go
@@ -32,6 +32,7 @@ func TestGetServiceMonitor(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, "rook-ceph-mgr", servicemonitor.GetName())
 	assert.Equal(t, "rook-ceph", servicemonitor.GetNamespace())
+	assert.NotNil(t, servicemonitor.GetLabels())
 	assert.NotNil(t, servicemonitor.Spec.NamespaceSelector.MatchNames)
 	assert.NotNil(t, servicemonitor.Spec.Endpoints)
 }