From 7a48a266b675d7531234c66b54f31e7c71bcd341 Mon Sep 17 00:00:00 2001
From: Bryan Boreham <bjboreham@gmail.com>
Date: Wed, 3 May 2023 11:59:27 +0100
Subject: [PATCH] labels: respect Set after Del in Builder (#12322)

* labels: respect Set after Del in Builder

The implementations are not symmetric between `Set()` and `Del()`, so
we must be careful. Add tests for this, both in labels and in relabel
where the issue was reported.

Also make the slice implementation consistent re `slices.Contains`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
---
 model/labels/labels.go        |  9 ++++-----
 model/labels/labels_string.go |  7 ++++---
 model/labels/labels_test.go   |  7 +++++++
 model/relabel/relabel_test.go | 28 ++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/model/labels/labels.go b/model/labels/labels.go
index 93524ddcf..9ac0e5b53 100644
--- a/model/labels/labels.go
+++ b/model/labels/labels.go
@@ -533,16 +533,15 @@ func (b *Builder) Set(n, v string) *Builder {
 }
 
 func (b *Builder) Get(n string) string {
-	for _, d := range b.del {
-		if d == n {
-			return ""
-		}
-	}
+	// Del() removes entries from .add but Set() does not remove from .del, so check .add first.
 	for _, a := range b.add {
 		if a.Name == n {
 			return a.Value
 		}
 	}
+	if slices.Contains(b.del, n) {
+		return ""
+	}
 	return b.base.Get(n)
 }
 
diff --git a/model/labels/labels_string.go b/model/labels/labels_string.go
index ff46103eb..6d54e98ab 100644
--- a/model/labels/labels_string.go
+++ b/model/labels/labels_string.go
@@ -593,14 +593,15 @@ func (b *Builder) Set(n, v string) *Builder {
 }
 
 func (b *Builder) Get(n string) string {
-	if slices.Contains(b.del, n) {
-		return ""
-	}
+	// Del() removes entries from .add but Set() does not remove from .del, so check .add first.
 	for _, a := range b.add {
 		if a.Name == n {
 			return a.Value
 		}
 	}
+	if slices.Contains(b.del, n) {
+		return ""
+	}
 	return b.base.Get(n)
 }
 
diff --git a/model/labels/labels_test.go b/model/labels/labels_test.go
index 9e60c2251..108d8b0de 100644
--- a/model/labels/labels_test.go
+++ b/model/labels/labels_test.go
@@ -607,6 +607,13 @@ func TestBuilder(t *testing.T) {
 			require.Equal(t, tcase.want.BytesWithoutLabels(nil, "aaa", "bbb"), b.Labels().Bytes(nil))
 		})
 	}
+	t.Run("set_after_del", func(t *testing.T) {
+		b := NewBuilder(FromStrings("aaa", "111"))
+		b.Del("bbb")
+		b.Set("bbb", "222")
+		require.Equal(t, FromStrings("aaa", "111", "bbb", "222"), b.Labels())
+		require.Equal(t, "222", b.Get("bbb"))
+	})
 }
 
 func TestScratchBuilder(t *testing.T) {
diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go
index d277d778d..b50ff4010 100644
--- a/model/relabel/relabel_test.go
+++ b/model/relabel/relabel_test.go
@@ -397,6 +397,34 @@ func TestRelabel(t *testing.T) {
 				"foo":              "bar",
 			}),
 		},
+		{ // From https://github.com/prometheus/prometheus/issues/12283
+			input: labels.FromMap(map[string]string{
+				"__meta_kubernetes_pod_container_port_name":         "foo",
+				"__meta_kubernetes_pod_annotation_XXX_metrics_port": "9091",
+			}),
+			relabel: []*Config{
+				{
+					Regex:  MustNewRegexp("^__meta_kubernetes_pod_container_port_name$"),
+					Action: LabelDrop,
+				},
+				{
+					SourceLabels: model.LabelNames{"__meta_kubernetes_pod_annotation_XXX_metrics_port"},
+					Regex:        MustNewRegexp("(.+)"),
+					Action:       Replace,
+					Replacement:  "metrics",
+					TargetLabel:  "__meta_kubernetes_pod_container_port_name",
+				},
+				{
+					SourceLabels: model.LabelNames{"__meta_kubernetes_pod_container_port_name"},
+					Regex:        MustNewRegexp("^metrics$"),
+					Action:       Keep,
+				},
+			},
+			output: labels.FromMap(map[string]string{
+				"__meta_kubernetes_pod_annotation_XXX_metrics_port": "9091",
+				"__meta_kubernetes_pod_container_port_name":         "metrics",
+			}),
+		},
 		{
 			input: labels.FromMap(map[string]string{
 				"a":  "foo",