Skip to content

Commit

Permalink
[Heartbeat] Fix NPE on monitor configuration errors (elastic#11910)
Browse files Browse the repository at this point in the history
These errors cause the `newMonitor` constructor to return a nil object plus error. Without a nil check we get an NPE.

Fixes elastic#11747
  • Loading branch information
andrewvc authored Apr 24, 2019
1 parent 1d94462 commit ac37fa3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d

*Heartbeat*

- Fix NPE on some monitor configuration errors. {pull}11910[11910]

*Journalbeat*

*Metricbeat*
Expand Down
11 changes: 11 additions & 0 deletions heartbeat/monitors/mocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,14 @@ func mockPluginConf(t *testing.T, id string, schedule string, url string) *commo

return conf
}

func mockInvalidPluginConf(t *testing.T) *common.Config {
confMap := map[string]interface{}{
"hoeutnheou": "oueanthoue",
}

conf, err := common.NewConfigFrom(confMap)
require.NoError(t, err)

return conf
}
4 changes: 3 additions & 1 deletion heartbeat/monitors/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ func (m *Monitor) String() string {

func checkMonitorConfig(config *common.Config, registrar *pluginsReg, allowWatches bool) error {
m, err := newMonitor(config, registrar, nil, nil, allowWatches, nil)
m.Stop() // Stop the monitor to free up the ID from uniqueness checks
if m != nil {
m.Stop() // Stop the monitor to free up the ID from uniqueness checks
}
return err
}

Expand Down
17 changes: 17 additions & 0 deletions heartbeat/monitors/monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,20 @@ func TestDuplicateMonitorIDs(t *testing.T) {
_, m3Err := makeTestMon()
assert.NoError(t, m3Err)
}

func TestCheckInvalidConfig(t *testing.T) {
serverMonConf := mockInvalidPluginConf(t)
reg := mockPluginsReg()
pipelineConnector := &MockPipelineConnector{}

sched := scheduler.New(1)
err := sched.Start()
require.NoError(t, err)
defer sched.Stop()

m, err := newMonitor(serverMonConf, reg, pipelineConnector, sched, false, nil)
// This could change if we decide the contract for newMonitor should always return a monitor
require.Nil(t, m, "For this test to work we need a nil value for the monitor.")

require.Error(t, checkMonitorConfig(serverMonConf, reg, false))
}

0 comments on commit ac37fa3

Please sign in to comment.