Skip to content

Commit

Permalink
Fix incorrect usage of hints builder when exposed port is a substring…
Browse files Browse the repository at this point in the history
… of the hint (elastic#19052)

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.
  • Loading branch information
vjsamuel authored and melchiormoulin committed Oct 14, 2020
1 parent b80c1e3 commit 4d10f34
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down
24 changes: 23 additions & 1 deletion metricbeat/autodiscover/builder/hints/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package hints

import (
"fmt"
"strconv"
"strings"

"github.com/elastic/go-ucfg"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
39 changes: 39 additions & 0 deletions metricbeat/autodiscover/builder/hints/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 4d10f34

Please sign in to comment.