Fix incorrect usage of hints builder when exposed port is a substring of the hint (elastic#19052)

vjsamuel · web-flow · commit 138a23a19ca0 · 2020-06-12T17:31:34.000+02:00
This PR makes sure that if a user exposes port 80 and has 8080 on the
metric hint, then we don't use that port to do the autodiscover as the port
could be exposed on another container.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -241,6 +241,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]
 - Fix compute and pubsub dashboard for googlecloud module. {issue}18962[18962] {pull}18980[18980]
 - Fix crash on vsphere module when Host information is not available. {issue}18996[18996] {pull}19078[19078]
+- Fix incorrect usage of hints builder when exposed port is a substring of the hint {pull}19052[19052]
 
 *Packetbeat*
 
diff --git a/metricbeat/autodiscover/builder/hints/metrics.go b/metricbeat/autodiscover/builder/hints/metrics.go
@@ -19,6 +19,7 @@ package hints
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/elastic/go-ucfg"
@@ -187,7 +188,7 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
 	// Only pick hosts that have ${data.port} or the port on current event. This will make
 	// sure that incorrect meta mapping doesn't happen
 	for _, h := range thosts {
-		if strings.Contains(h, "data.port") || strings.Contains(h, fmt.Sprintf(":%d", port)) ||
+		if strings.Contains(h, "data.port") || m.checkHostPort(h, port) ||
 			// Use the event that has no port config if there is a ${data.host}:9090 like input
 			(port == 0 && strings.Contains(h, "data.host")) {
 			result = append(result, h)
@@ -202,6 +203,27 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
 	return result, true
 }
 
+func (m *metricHints) checkHostPort(h string, p int) bool {
+	port := strconv.Itoa(p)
+
+	index := strings.LastIndex(h, ":"+port)
+	// Check if host contains :port. If not then return false
+	if index == -1 {
+		return false
+	}
+
+	// Check if the host ends with :port. Return true if yes
+	end := index + len(port) + 1
+	if end == len(h) {
+		return true
+	}
+
+	// Check if the character immediately after :port. If its not a number then return true.
+	// This is to avoid adding :80 as a valid host for an event that has port=8080
+	// Also ensure that port=3306 and hint="tcp(${data.host}:3306)/" is valid
+	return h[end] < '0' || h[end] > '9'
+}
+
 func (m *metricHints) getNamespace(hints common.MapStr) string {
 	return builder.GetHintString(hints, m.Key, namespace)
 }
diff --git a/metricbeat/autodiscover/builder/hints/metrics_test.go b/metricbeat/autodiscover/builder/hints/metrics_test.go
@@ -290,6 +290,45 @@ func TestGenerateHints(t *testing.T) {
 				"enabled":    true,
 			},
 		},
+		{
+			message: "Module, namespace, host hint shouldn't return when port isn't the same has hint",
+			event: bus.Event{
+				"host": "1.2.3.4",
+				"port": 80,
+				"hints": common.MapStr{
+					"metrics": common.MapStr{
+						"module":    "mockmoduledefaults",
+						"namespace": "test",
+						"hosts":     "${data.host}:8080",
+					},
+				},
+			},
+			len: 0,
+		},
+		{
+			message: "Non http URLs with valid host port combination should return a valid config",
+			event: bus.Event{
+				"host": "1.2.3.4",
+				"port": 3306,
+				"hints": common.MapStr{
+					"metrics": common.MapStr{
+						"module":    "mockmoduledefaults",
+						"namespace": "test",
+						"hosts":     "tcp(${data.host}:3306)/",
+					},
+				},
+			},
+			len: 1,
+			result: common.MapStr{
+				"module":     "mockmoduledefaults",
+				"namespace":  "test",
+				"metricsets": []string{"default"},
+				"hosts":      []interface{}{"tcp(1.2.3.4:3306)/"},
+				"timeout":    "3s",
+				"period":     "1m",
+				"enabled":    true,
+			},
+		},
 	}
 	for _, test := range tests {
 		mockRegister := mb.NewRegister()
