Stricter Relabel Config Checking for Labeldrop/keep (#2510)

* Minor code cleanup

* Labeldrop/Labelkeep Now *Only* Support Regex

Ref promtheus/prometheus#2368

config/config.go

@@ -584,7 +584,7 @@ func (c *AlertingConfig) UnmarshalYAML(unmarshal func(interface{}) error) error
 	return nil
 }
 
-// AlertmanagersConfig configures how Alertmanagers can be discovered and communicated with.
+// AlertmanagerConfig configures how Alertmanagers can be discovered and communicated with.
 type AlertmanagerConfig struct {
 	// We cannot do proper Go type embedding below as the parser will then parse
 	// values arbitrarily into the overflow maps of further-down types.
@@ -949,8 +949,10 @@ func (c *MarathonSDConfig) UnmarshalYAML(unmarshal func(interface{}) error) erro
 	return nil
 }
 
+// KubernetesRole is role of the service in Kubernetes.
 type KubernetesRole string
 
+// The valid options for KubernetesRole.
 const (
 	KubernetesRoleNode     = "node"
 	KubernetesRolePod      = "pod"
@@ -958,6 +960,7 @@ const (
 	KubernetesRoleEndpoint = "endpoints"
 )
 
+// UnmarshalYAML implements the yaml.Unmarshaler interface.
 func (c *KubernetesRole) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err := unmarshal((*string)(c)); err != nil {
 		return err
@@ -1226,6 +1229,17 @@ func (c *RelabelConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if c.Action == RelabelHashMod && !model.LabelName(c.TargetLabel).IsValid() {
 		return fmt.Errorf("%q is invalid 'target_label' for %s action", c.TargetLabel, c.Action)
 	}
+
+	if c.Action == RelabelLabelDrop || c.Action == RelabelLabelKeep {
+		if c.SourceLabels != nil ||
+			c.TargetLabel != DefaultRelabelConfig.TargetLabel ||
+			c.Modulus != DefaultRelabelConfig.Modulus ||
+			c.Separator != DefaultRelabelConfig.Separator ||
+			c.Replacement != DefaultRelabelConfig.Replacement {
+			return fmt.Errorf("%s action requires only 'regex', and no other fields", c.Action)
+		}
+	}
+
 	return nil
 }
 

config/config_test.go

@@ -538,6 +538,36 @@ var expectedErrors = []struct {
 	}, {
 		filename: "modulus_missing.bad.yml",
 		errMsg:   "relabel configuration for hashmod requires non-zero modulus",
+	}, {
+		filename: "labelkeep.bad.yml",
+		errMsg:   "labelkeep action requires only 'regex', and no other fields",
+	}, {
+		filename: "labelkeep2.bad.yml",
+		errMsg:   "labelkeep action requires only 'regex', and no other fields",
+	}, {
+		filename: "labelkeep3.bad.yml",
+		errMsg:   "labelkeep action requires only 'regex', and no other fields",
+	}, {
+		filename: "labelkeep4.bad.yml",
+		errMsg:   "labelkeep action requires only 'regex', and no other fields",
+	}, {
+		filename: "labelkeep5.bad.yml",
+		errMsg:   "labelkeep action requires only 'regex', and no other fields",
+	}, {
+		filename: "labeldrop.bad.yml",
+		errMsg:   "labeldrop action requires only 'regex', and no other fields",
+	}, {
+		filename: "labeldrop2.bad.yml",
+		errMsg:   "labeldrop action requires only 'regex', and no other fields",
+	}, {
+		filename: "labeldrop3.bad.yml",
+		errMsg:   "labeldrop action requires only 'regex', and no other fields",
+	}, {
+		filename: "labeldrop4.bad.yml",
+		errMsg:   "labeldrop action requires only 'regex', and no other fields",
+	}, {
+		filename: "labeldrop5.bad.yml",
+		errMsg:   "labeldrop action requires only 'regex', and no other fields",
 	}, {
 		filename: "rules.bad.yml",
 		errMsg:   "invalid rule file path",

config/testdata/labeldrop.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - source_labels: [abcdef]
+        action: labeldrop

config/testdata/labeldrop2.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - modulus: 8 
+        action: labeldrop

config/testdata/labeldrop3.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - separator: ',' 
+        action: labeldrop

config/testdata/labeldrop4.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - replacement: yolo-{1} 
+        action: labeldrop

config/testdata/labeldrop5.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - target_label: yolo
+        action: labeldrop

config/testdata/labelkeep.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - source_labels: [abcdef]
+        action: labelkeep

config/testdata/labelkeep2.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - modulus: 8 
+        action: labelkeep

config/testdata/labelkeep3.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - separator: ',' 
+        action: labelkeep

config/testdata/labelkeep4.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - replacement: yolo-{1} 
+        action: labelkeep

config/testdata/labelkeep5.bad.yml

@@ -0,0 +1,5 @@
+scrape_configs:
+  - job_name: prometheus
+    relabel_configs:
+      - target_label: yolo
+        action: labelkeep

relabel/relabel.go

@@ -87,13 +87,13 @@ func relabel(labels model.LabelSet, cfg *config.RelabelConfig) model.LabelSet {
 		}
 		labels = out
 	case config.RelabelLabelDrop:
-		for ln, _ := range labels {
+		for ln := range labels {
 			if cfg.Regex.MatchString(string(ln)) {
 				delete(labels, ln)
 			}
 		}
 	case config.RelabelLabelKeep:
-		for ln, _ := range labels {
+		for ln := range labels {
 			if !cfg.Regex.MatchString(string(ln)) {
 				delete(labels, ln)
 			}