api/relabelConfig: allow empty values for replacement and separator

Empty values for `replacement` and `separator` fields have special meaning. And it should optional pointer to string. The same logic exists at `vmagent` side and operator must use it as well. Related issue: #1214 Signed-off-by: f41gh7 <[email protected]>
VictoriaMetrics · Jan 13, 2025 · a0883be · a0883be
1 parent b38ee45
commit a0883be
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 23 deletions.
diff --git a/api/operator/v1beta1/common_scrapeparams.go b/api/operator/v1beta1/common_scrapeparams.go
@@ -152,7 +152,7 @@ type RelabelConfig struct {
 	SourceLabels []string `json:"sourceLabels,omitempty" yaml:"-"`
 	// Separator placed between concatenated source label values. default is ';'.
 	// +optional
-	Separator string `json:"separator,omitempty" yaml:"separator,omitempty"`
+	Separator *string `json:"separator,omitempty" yaml:"separator,omitempty"`
 	// Label to which the resulting value is written in a replace action.
 	// It is mandatory for replace actions. Regex capture groups are available.
 	// +optional
@@ -170,7 +170,7 @@ type RelabelConfig struct {
 	// Replacement value against which a regex replace is performed if the
 	// regular expression matches. Regex capture groups are available. Default is '$1'
 	// +optional
-	Replacement string `json:"replacement,omitempty" yaml:"replacement,omitempty"`
+	Replacement *string `json:"replacement,omitempty" yaml:"replacement,omitempty"`
 	// Action to perform based on regex matching. Default is 'replace'
 	// +optional
 	Action string `json:"action,omitempty" yaml:"action,omitempty"`

diff --git a/api/operator/v1beta1/zz_generated.deepcopy.go b/api/operator/v1beta1/zz_generated.deepcopy.go
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -13,6 +13,8 @@ aliases:
 
 ## tip
 
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): properly build `relabelConfigs` with empty string values for `separator` and `replacement` fields. See [this issue](https://github.com/VictoriaMetrics/operator/issues/1214) for details.
+
 ## [v0.51.3](https://github.com/VictoriaMetrics/operator/releases/tag/v0.51.3)
 
 **Release date:** 8 Jan 2025

diff --git a/internal/controller/operator/converter/apis.go b/internal/controller/operator/converter/apis.go
@@ -304,20 +304,12 @@ func ConvertRelabelConfig(promRelabelConfig []promv1.RelabelConfig) []*vmv1beta1
 		return res
 	}
 	for idx, relabel := range promRelabelConfig {
-		var separator string
-		if relabel.Separator != nil {
-			separator = *relabel.Separator
-		}
-		var replacement string
-		if relabel.Replacement != nil {
-			replacement = *relabel.Replacement
-		}
 		relabelCfg = append(relabelCfg, &vmv1beta1.RelabelConfig{
 			SourceLabels: sourceLabelsToStringSlice(relabel.SourceLabels),
-			Separator:    separator,
+			Separator:    relabel.Separator,
 			TargetLabel:  relabel.TargetLabel,
 			Modulus:      relabel.Modulus,
-			Replacement:  replacement,
+			Replacement:  relabel.Replacement,
 			Action:       relabel.Action,
 		})
 		if len(relabel.Regex) > 0 {

diff --git a/internal/controller/operator/converter/v1alpha1/apis_test.go b/internal/controller/operator/converter/v1alpha1/apis_test.go
@@ -4,14 +4,15 @@ import (
 	"fmt"
 	"testing"
 
-	vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
-	"github.com/VictoriaMetrics/operator/internal/config"
 	"github.com/google/go-cmp/cmp"
 	promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	promv1alpha1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
+
+	vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
+	"github.com/VictoriaMetrics/operator/internal/config"
 )
 
 func TestConvertAlertmanagerConfig(t *testing.T) {
@@ -127,13 +128,13 @@ func TestConvertScrapeConfig(t *testing.T) {
 							{
 								Action:      "LabelMap",
 								Regex:       vmv1beta1.StringOrArray{"__meta_kubernetes_pod_label_(.+)"},
-								Replacement: "foo_$1",
+								Replacement: ptr.To("foo_$1"),
 							},
 						},
 						MetricRelabelConfigs: []*vmv1beta1.RelabelConfig{
 							{
 								SourceLabels: []string{"__meta_kubernetes_pod_name", "__meta_kubernetes_pod_container_port_number"},
-								Separator:    ":",
+								Separator:    ptr.To(":"),
 								TargetLabel:  "host_port",
 							},
 						},

diff --git a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go
@@ -1147,8 +1147,8 @@ func generateRelabelConfig(rc *vmv1beta1.RelabelConfig) yaml.MapSlice {
 		relabeling = append(relabeling, yaml.MapItem{Key: "source_labels", Value: rc.SourceLabels})
 	}
 
-	if rc.Separator != "" {
-		relabeling = append(relabeling, yaml.MapItem{Key: "separator", Value: rc.Separator})
+	if rc.Separator != nil {
+		relabeling = append(relabeling, yaml.MapItem{Key: "separator", Value: *rc.Separator})
 	}
 
 	if rc.TargetLabel != "" {
@@ -1168,8 +1168,8 @@ func generateRelabelConfig(rc *vmv1beta1.RelabelConfig) yaml.MapSlice {
 		relabeling = append(relabeling, yaml.MapItem{Key: "modulus", Value: rc.Modulus})
 	}
 
-	if rc.Replacement != "" {
-		relabeling = append(relabeling, yaml.MapItem{Key: "replacement", Value: rc.Replacement})
+	if rc.Replacement != nil {
+		relabeling = append(relabeling, yaml.MapItem{Key: "replacement", Value: *rc.Replacement})
 	}
 
 	if rc.Action != "" {

diff --git a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go
@@ -157,6 +157,29 @@ action: replace
 target_label: address
 action: graphite
 match: foo.*.*.bar
+labels:
+  instance: ${2}:8080
+  job: $1
+`,
+		},
+		{
+			name: "with empty replacement and separator",
+			args: args{rc: &vmv1beta1.RelabelConfig{
+				UnderScoreTargetLabel:  "address",
+				UnderScoreSourceLabels: []string{"__address__"},
+				Action:                 "graphite",
+				Labels:                 map[string]string{"job": "$1", "instance": "${2}:8080"},
+				Match:                  `foo.*.*.bar`,
+				Separator:              ptr.To(""),
+				Replacement:            ptr.To(""),
+			}},
+			want: `source_labels:
+- __address__
+separator: ""
+target_label: address
+replacement: ""
+action: graphite
+match: foo.*.*.bar
 labels:
   instance: ${2}:8080
   job: $1

diff --git a/internal/controller/operator/factory/vmagent/vmagent_test.go b/internal/controller/operator/factory/vmagent/vmagent_test.go
@@ -1081,7 +1081,7 @@ func TestBuildRemoteWrites(t *testing.T) {
 									InsecureSkipVerify: true,
 								},
 								InlineUrlRelabelConfig: []vmv1beta1.RelabelConfig{
-									{TargetLabel: "rw-1", Replacement: "present"},
+									{TargetLabel: "rw-1", Replacement: ptr.To("present")},
 								},
 							},
 							{
@@ -1097,12 +1097,12 @@ func TestBuildRemoteWrites(t *testing.T) {
 									InsecureSkipVerify: true,
 								},
 								InlineUrlRelabelConfig: []vmv1beta1.RelabelConfig{
-									{TargetLabel: "rw-2", Replacement: "present"},
+									{TargetLabel: "rw-2", Replacement: ptr.To("present")},
 								},
 							},
 						},
 						InlineRelabelConfig: []vmv1beta1.RelabelConfig{
-							{TargetLabel: "dst", Replacement: "ok"},
+							{TargetLabel: "dst", Replacement: ptr.To("ok")},
 						},
 					},
 				},
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,6 +13,8 @@ aliases: @@
     ## tip
+    * BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): properly build `relabelConfigs` with empty string values for `separator` and `replacement` fields. See [this issue](https://github.com/VictoriaMetrics/operator/issues/1214) for details.
     ## [v0.51.3](https://github.com/VictoriaMetrics/operator/releases/tag/v0.51.3)
     **Release date:** 8 Jan 2025
@@ Expand Down @@