From a0883be38bc63178c45859c820ca2dcbc0ea43fb Mon Sep 17 00:00:00 2001 From: f41gh7 Date: Mon, 13 Jan 2025 11:39:53 +0100 Subject: [PATCH] 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: https://github.com/VictoriaMetrics/operator/issues/1214 Signed-off-by: f41gh7 --- api/operator/v1beta1/common_scrapeparams.go | 4 ++-- api/operator/v1beta1/zz_generated.deepcopy.go | 10 ++++++++ docs/CHANGELOG.md | 2 ++ .../controller/operator/converter/apis.go | 12 ++-------- .../operator/converter/v1alpha1/apis_test.go | 9 ++++---- .../factory/vmagent/vmagent_scrapeconfig.go | 8 +++---- .../vmagent/vmagent_scrapeconfig_test.go | 23 +++++++++++++++++++ .../operator/factory/vmagent/vmagent_test.go | 6 ++--- 8 files changed, 51 insertions(+), 23 deletions(-) diff --git a/api/operator/v1beta1/common_scrapeparams.go b/api/operator/v1beta1/common_scrapeparams.go index c2df7b7a..78400aae 100644 --- 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 index cb5ac2ec..df3ba4db 100644 --- a/api/operator/v1beta1/zz_generated.deepcopy.go +++ b/api/operator/v1beta1/zz_generated.deepcopy.go @@ -2292,11 +2292,21 @@ func (in *RelabelConfig) DeepCopyInto(out *RelabelConfig) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.Separator != nil { + in, out := &in.Separator, &out.Separator + *out = new(string) + **out = **in + } if in.Regex != nil { in, out := &in.Regex, &out.Regex *out = make(StringOrArray, len(*in)) copy(*out, *in) } + if in.Replacement != nil { + in, out := &in.Replacement, &out.Replacement + *out = new(string) + **out = **in + } if in.If != nil { in, out := &in.If, &out.If *out = make(StringOrArray, len(*in)) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ca92813b..4114bfa3 100644 --- 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 index bfd4d716..28ab4c8c 100644 --- 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 index b9eb578b..5043ff81 100644 --- 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 index a6ad9861..a09ada50 100644 --- 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 index c50972e5..eda14db8 100644 --- 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 index 62ba6163..fb04ae89 100644 --- 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")}, }, }, },