Skip to content

Commit 138a23a

Browse files
authored
Fix incorrect usage of hints builder when exposed port is a substring 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.
1 parent 600f5a8 commit 138a23a

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

CHANGELOG.next.asciidoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
241241
- Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]
242242
- Fix compute and pubsub dashboard for googlecloud module. {issue}18962[18962] {pull}18980[18980]
243243
- Fix crash on vsphere module when Host information is not available. {issue}18996[18996] {pull}19078[19078]
244+
- Fix incorrect usage of hints builder when exposed port is a substring of the hint {pull}19052[19052]
244245

245246
*Packetbeat*
246247

metricbeat/autodiscover/builder/hints/metrics.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package hints
1919

2020
import (
2121
"fmt"
22+
"strconv"
2223
"strings"
2324

2425
"github.com/elastic/go-ucfg"
@@ -187,7 +188,7 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
187188
// Only pick hosts that have ${data.port} or the port on current event. This will make
188189
// sure that incorrect meta mapping doesn't happen
189190
for _, h := range thosts {
190-
if strings.Contains(h, "data.port") || strings.Contains(h, fmt.Sprintf(":%d", port)) ||
191+
if strings.Contains(h, "data.port") || m.checkHostPort(h, port) ||
191192
// Use the event that has no port config if there is a ${data.host}:9090 like input
192193
(port == 0 && strings.Contains(h, "data.host")) {
193194
result = append(result, h)
@@ -202,6 +203,27 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
202203
return result, true
203204
}
204205

206+
func (m *metricHints) checkHostPort(h string, p int) bool {
207+
port := strconv.Itoa(p)
208+
209+
index := strings.LastIndex(h, ":"+port)
210+
// Check if host contains :port. If not then return false
211+
if index == -1 {
212+
return false
213+
}
214+
215+
// Check if the host ends with :port. Return true if yes
216+
end := index + len(port) + 1
217+
if end == len(h) {
218+
return true
219+
}
220+
221+
// Check if the character immediately after :port. If its not a number then return true.
222+
// This is to avoid adding :80 as a valid host for an event that has port=8080
223+
// Also ensure that port=3306 and hint="tcp(${data.host}:3306)/" is valid
224+
return h[end] < '0' || h[end] > '9'
225+
}
226+
205227
func (m *metricHints) getNamespace(hints common.MapStr) string {
206228
return builder.GetHintString(hints, m.Key, namespace)
207229
}

metricbeat/autodiscover/builder/hints/metrics_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,45 @@ func TestGenerateHints(t *testing.T) {
290290
"enabled": true,
291291
},
292292
},
293+
{
294+
message: "Module, namespace, host hint shouldn't return when port isn't the same has hint",
295+
event: bus.Event{
296+
"host": "1.2.3.4",
297+
"port": 80,
298+
"hints": common.MapStr{
299+
"metrics": common.MapStr{
300+
"module": "mockmoduledefaults",
301+
"namespace": "test",
302+
"hosts": "${data.host}:8080",
303+
},
304+
},
305+
},
306+
len: 0,
307+
},
308+
{
309+
message: "Non http URLs with valid host port combination should return a valid config",
310+
event: bus.Event{
311+
"host": "1.2.3.4",
312+
"port": 3306,
313+
"hints": common.MapStr{
314+
"metrics": common.MapStr{
315+
"module": "mockmoduledefaults",
316+
"namespace": "test",
317+
"hosts": "tcp(${data.host}:3306)/",
318+
},
319+
},
320+
},
321+
len: 1,
322+
result: common.MapStr{
323+
"module": "mockmoduledefaults",
324+
"namespace": "test",
325+
"metricsets": []string{"default"},
326+
"hosts": []interface{}{"tcp(1.2.3.4:3306)/"},
327+
"timeout": "3s",
328+
"period": "1m",
329+
"enabled": true,
330+
},
331+
},
293332
}
294333
for _, test := range tests {
295334
mockRegister := mb.NewRegister()

0 commit comments

Comments
 (0)