From d8b4995ddd9a63bfc4a2c2708d3be55a5cc0b808 Mon Sep 17 00:00:00 2001
From: Brian Brazil <brian.brazil@robustperception.io>
Date: Fri, 9 Jun 2017 16:18:19 +0100
Subject: [PATCH] Check target labels are valid. Check for address after
 relabelling.

Fixes #2822
Fixes #2819
---
 retrieval/target.go             | 13 +++--
 retrieval/targetmanager_test.go | 94 ++++++++++++++++++++++++++++++---
 2 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/retrieval/target.go b/retrieval/target.go
index 66813604a..7c037a5b9 100644
--- a/retrieval/target.go
+++ b/retrieval/target.go
@@ -266,9 +266,6 @@ func (app *countingAppender) Append(s *model.Sample) error {
 // Returns a nil label set if the target is dropped during relabeling.
 func populateLabels(lset model.LabelSet, cfg *config.ScrapeConfig) (res, orig model.LabelSet, err error) {
 	lset = lset.Clone()
-	if _, ok := lset[model.AddressLabel]; !ok {
-		return nil, nil, fmt.Errorf("no address")
-	}
 	// Copy labels into the labelset for the target if they are not
 	// set already. Apply the labelsets in order of decreasing precedence.
 	scrapeLabels := model.LabelSet{
@@ -295,6 +292,9 @@ func populateLabels(lset model.LabelSet, cfg *config.ScrapeConfig) (res, orig mo
 	if lset == nil {
 		return nil, nil, nil
 	}
+	if _, ok := lset[model.AddressLabel]; !ok {
+		return nil, nil, fmt.Errorf("no address")
+	}
 
 	// addPort checks whether we should add a default port to the address.
 	// If the address is not valid, we don't append a port either.
@@ -327,9 +327,14 @@ func populateLabels(lset model.LabelSet, cfg *config.ScrapeConfig) (res, orig mo
 
 	// Meta labels are deleted after relabelling. Other internal labels propagate to
 	// the target which decides whether they will be part of their label set.
-	for ln := range lset {
+	for ln, lv := range lset {
 		if strings.HasPrefix(string(ln), model.MetaLabelPrefix) {
 			delete(lset, ln)
+			continue
+		}
+		// Check label values are valid, drop the target if not.
+		if !lv.IsValid() {
+			return nil, nil, fmt.Errorf("invalid label value for %q: %q", ln, lv)
 		}
 	}
 
diff --git a/retrieval/targetmanager_test.go b/retrieval/targetmanager_test.go
index a8b629204..b1380dee4 100644
--- a/retrieval/targetmanager_test.go
+++ b/retrieval/targetmanager_test.go
@@ -14,6 +14,7 @@
 package retrieval
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -35,6 +36,7 @@ func TestPopulateLabels(t *testing.T) {
 		cfg     *config.ScrapeConfig
 		res     model.LabelSet
 		resOrig model.LabelSet
+		err     error
 	}{
 		// Regular population of scrape config options.
 		{
@@ -117,11 +119,24 @@ func TestPopulateLabels(t *testing.T) {
 				model.JobLabel:         "job",
 			},
 		},
-		// Apply relabeling.
+		// Address label missing.
 		{
 			in: model.LabelSet{
-				model.AddressLabel: "1.2.3.4:1000",
-				"custom":           "value",
+				"custom": "value",
+			},
+			cfg: &config.ScrapeConfig{
+				Scheme:      "https",
+				MetricsPath: "/metrics",
+				JobName:     "job",
+			},
+			res:     nil,
+			resOrig: nil,
+			err:     fmt.Errorf("no address"),
+		},
+		// Address label missing, but added in relabelling.
+		{
+			in: model.LabelSet{
+				"custom": "host:1234",
 			},
 			cfg: &config.ScrapeConfig{
 				Scheme:      "https",
@@ -129,21 +144,84 @@ func TestPopulateLabels(t *testing.T) {
 				JobName:     "job",
 				RelabelConfigs: []*config.RelabelConfig{
 					{
-						Action:       config.RelabelDrop,
-						Regex:        mustNewRegexp(".*"),
-						SourceLabels: model.LabelNames{"job"},
+						Action:       config.RelabelReplace,
+						Regex:        mustNewRegexp("(.*)"),
+						SourceLabels: model.LabelNames{"custom"},
+						Replacement:  "${1}",
+						TargetLabel:  string(model.AddressLabel),
 					},
 				},
 			},
+			res: model.LabelSet{
+				model.AddressLabel:     "host:1234",
+				model.InstanceLabel:    "host:1234",
+				model.SchemeLabel:      "https",
+				model.MetricsPathLabel: "/metrics",
+				model.JobLabel:         "job",
+				"custom":               "host:1234",
+			},
+			resOrig: model.LabelSet{
+				model.SchemeLabel:      "https",
+				model.MetricsPathLabel: "/metrics",
+				model.JobLabel:         "job",
+				"custom":               "host:1234",
+			},
+		},
+		// Address label missing, but added in relabelling.
+		{
+			in: model.LabelSet{
+				"custom": "host:1234",
+			},
+			cfg: &config.ScrapeConfig{
+				Scheme:      "https",
+				MetricsPath: "/metrics",
+				JobName:     "job",
+				RelabelConfigs: []*config.RelabelConfig{
+					{
+						Action:       config.RelabelReplace,
+						Regex:        mustNewRegexp("(.*)"),
+						SourceLabels: model.LabelNames{"custom"},
+						Replacement:  "${1}",
+						TargetLabel:  string(model.AddressLabel),
+					},
+				},
+			},
+			res: model.LabelSet{
+				model.AddressLabel:     "host:1234",
+				model.InstanceLabel:    "host:1234",
+				model.SchemeLabel:      "https",
+				model.MetricsPathLabel: "/metrics",
+				model.JobLabel:         "job",
+				"custom":               "host:1234",
+			},
+			resOrig: model.LabelSet{
+				model.SchemeLabel:      "https",
+				model.MetricsPathLabel: "/metrics",
+				model.JobLabel:         "job",
+				"custom":               "host:1234",
+			},
+		},
+		// Invalid UTF-8 in label.
+		{
+			in: model.LabelSet{
+				model.AddressLabel: "1.2.3.4:1000",
+				"custom":           "\xbd",
+			},
+			cfg: &config.ScrapeConfig{
+				Scheme:      "https",
+				MetricsPath: "/metrics",
+				JobName:     "job",
+			},
 			res:     nil,
 			resOrig: nil,
+			err:     fmt.Errorf("invalid label value for \"custom\": \"\\xbd\""),
 		},
 	}
 	for i, c := range cases {
 		in := c.in.Clone()
 		res, orig, err := populateLabels(c.in, c.cfg)
-		if err != nil {
-			t.Fatalf("case %d: %s", i, err)
+		if !reflect.DeepEqual(err, c.err) {
+			t.Fatalf("case %d: wanted %v error, got %v", i, c.err, err)
 		}
 		if !reflect.DeepEqual(c.in, in) {
 			t.Errorf("case %d: input lset was changed was\n\t%+v\n now\n\t%+v", i, in, c.in)