From e984e2f10f2e2a66e8865960d62293b9083d201a Mon Sep 17 00:00:00 2001
From: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Date: Wed, 3 Jun 2020 17:20:33 +0200
Subject: [PATCH] Fix panic on `metricbeat test modules` (#18797) (#18852)

Since metricbeat light modules support processors (#15923), module
initialization requires a publisher in the beat so modules can attach
their processors. `metricbeat test modules` is not initializing as
normal metricbeat commands, and it is not initializing any output or
publisher pipeline, so metricbeat panics when trying to initialize
modules with the new method.

This change adds a dummy publisher for this case, and fixes also a
condition that was adding a `nil` module option, causing additional
panics. A test that reproduced the issues is also added.

(cherry picked from commit 25b8bf17038c4d659299b8e5b1dd1cafa723ba3f)
---
 CHANGELOG.next.asciidoc               |  1 +
 metricbeat/cmd/test/modules.go        | 17 +++++++++++++++++
 metricbeat/mb/module/configuration.go |  2 +-
 metricbeat/tests/system/test_cmd.py   | 15 +++++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 7c655f3ccead..ed38345fadbb 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -239,6 +239,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Remove specific win32 api errors from events in perfmon. {issue}18292[18292] {pull}18361[18361]
 - Fix application_pool metricset after pdh changes. {pull}18477[18477]
 - Fix tags_filter for cloudwatch metricset in aws. {pull}18524[18524]
+- Fix panic on `metricbeat test modules` when modules are configured in `metricbeat.modules`. {issue}18789[18789] {pull}18797[18797]
 - Fix getting gcp compute instance metadata with partial zone/region in config. {pull}18757[18757]
 - Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]
 
diff --git a/metricbeat/cmd/test/modules.go b/metricbeat/cmd/test/modules.go
index 93950973a5dc..6bf312d17b92 100644
--- a/metricbeat/cmd/test/modules.go
+++ b/metricbeat/cmd/test/modules.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/elastic/beats/v7/libbeat/beat"
 	"github.com/elastic/beats/v7/libbeat/cmd/instance"
+	"github.com/elastic/beats/v7/libbeat/publisher/pipeline"
 	"github.com/elastic/beats/v7/libbeat/testing"
 	"github.com/elastic/beats/v7/metricbeat/beater"
 )
@@ -49,6 +50,8 @@ func GenTestModulesCmd(name, beatVersion string, create beat.Creator) *cobra.Com
 				os.Exit(1)
 			}
 
+			// A publisher is needed for modules that add their own pipelines
+			b.Beat.Publisher = newPublisher()
 			mb, err := create(&b.Beat, b.Beat.BeatConfig)
 			if err != nil {
 				fmt.Fprintf(os.Stderr, "Error initializing metricbeat: %s\n", err)
@@ -78,3 +81,17 @@ func GenTestModulesCmd(name, beatVersion string, create beat.Creator) *cobra.Com
 		},
 	}
 }
+
+type publisher struct {
+	beat.PipelineConnector
+}
+
+// newPublisher returns a functional publisher that does nothing.
+func newPublisher() *publisher {
+	return &publisher{pipeline.NewNilPipeline()}
+}
+
+// SetACKHandler is a dummy implementation of the ack handler for the test publisher.
+func (*publisher) SetACKHandler(beat.PipelineACKHandler) error {
+	return nil
+}
diff --git a/metricbeat/mb/module/configuration.go b/metricbeat/mb/module/configuration.go
index 8b0701c0ae9f..d5049690b487 100644
--- a/metricbeat/mb/module/configuration.go
+++ b/metricbeat/mb/module/configuration.go
@@ -30,7 +30,7 @@ func ConfiguredModules(modulesData []*common.Config, configModulesData *common.C
 	var modules []*Wrapper
 
 	for _, moduleCfg := range modulesData {
-		module, err := NewWrapper(moduleCfg, mb.Registry, nil)
+		module, err := NewWrapper(moduleCfg, mb.Registry, moduleOptions...)
 		if err != nil {
 			return nil, err
 		}
diff --git a/metricbeat/tests/system/test_cmd.py b/metricbeat/tests/system/test_cmd.py
index 740beedb97a9..ad9a507d08c8 100644
--- a/metricbeat/tests/system/test_cmd.py
+++ b/metricbeat/tests/system/test_cmd.py
@@ -124,6 +124,21 @@ def test_modules_test(self):
         assert self.log_contains("cpu...OK")
         assert self.log_contains("memory...OK")
 
+    def test_modules_test_with_module_in_main_config(self):
+        self.render_config_template(reload=False, modules=[{
+            "name": "system",
+            "metricsets": ["cpu", "memory"],
+            "period": "10s",
+        }])
+
+        exit_code = self.run_beat(
+            logging_args=None,
+            extra_args=["test", "modules"])
+
+        assert exit_code == 0
+        assert self.log_contains("cpu...OK")
+        assert self.log_contains("memory...OK")
+
     def test_modules_test_error(self):
         """
         Test test modules command with an error result