From 84cc9c3a83d67b777f9d6165ae5e89255bcb8b0b Mon Sep 17 00:00:00 2001
From: Andrew Cholakian <andrew@andrewvc.com>
Date: Thu, 25 Apr 2019 21:52:34 -0500
Subject: [PATCH] [Heartbeat] Fix NPE on monitor configuration errors (#11910)
 (#11921)

These errors cause the `newMonitor` constructor to return a nil object plus error. Without a nil check we get an NPE.

Fixes https://github.com/elastic/beats/issues/11747

(cherry picked from commit ac37fa396973b587393c0722eb346a7df4820b57)
---
 CHANGELOG.next.asciidoc            |  2 ++
 heartbeat/monitors/mocks_test.go   | 11 +++++++++++
 heartbeat/monitors/monitor.go      |  4 +++-
 heartbeat/monitors/monitor_test.go | 17 +++++++++++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index bcb1ac50150..c293abad716 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -40,6 +40,8 @@ https://github.com/elastic/beats/compare/v7.0.0...7.0[Check the HEAD diff]
 
 *Heartbeat*
 
+- Fix NPE on some monitor configuration errors. {pull}11910[11910]
+
 *Journalbeat*
 
 *Metricbeat*
diff --git a/heartbeat/monitors/mocks_test.go b/heartbeat/monitors/mocks_test.go
index 0ad24a9657f..4effc437e4c 100644
--- a/heartbeat/monitors/mocks_test.go
+++ b/heartbeat/monitors/mocks_test.go
@@ -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
+}
diff --git a/heartbeat/monitors/monitor.go b/heartbeat/monitors/monitor.go
index f50546b520c..b1aaf3c9660 100644
--- a/heartbeat/monitors/monitor.go
+++ b/heartbeat/monitors/monitor.go
@@ -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
 }
 
diff --git a/heartbeat/monitors/monitor_test.go b/heartbeat/monitors/monitor_test.go
index ce86bb16371..4f224edc660 100644
--- a/heartbeat/monitors/monitor_test.go
+++ b/heartbeat/monitors/monitor_test.go
@@ -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))
+}