Fix mixed up modules configuration

Fixes elastic#4761. Due to combining pointer and reference passing we ended up with
passing the same module pointer to multiple filesets. The pointer was correct
during initialization, but wrong during run time.

Also added an Info with the enabled modules / filesets.

CHANGELOG.asciidoc

@@ -30,6 +30,8 @@ https://github.com/elastic/beats/compare/v6.0.0-beta1...master[Check the HEAD di
 
 *Filebeat*
 
+- Fix issue where the `fileset.module` could have the wrong value. {issue}4761[4761]
+
 *Heartbeat*
 
 *Metricbeat*

filebeat/beater/filebeat.go

@@ -56,6 +56,9 @@ func New(b *beat.Beat, rawConfig *common.Config) (beat.Beater, error) {
 	if err != nil {
 		return nil, err
 	}
+	if !moduleRegistry.Empty() {
+		logp.Info("Enabled modules/filesets: %s", moduleRegistry.InfoString())
+	}
 
 	moduleProspectors, err := moduleRegistry.GetProspectorConfigs()
 	if err != nil {

filebeat/fileset/config.go

@@ -15,5 +15,3 @@ type FilesetConfig struct {
 	Var        map[string]interface{} `config:"var"`
 	Prospector map[string]interface{} `config:"prospector"`
 }
-
-var defaultFilesetConfig = FilesetConfig{}

filebeat/fileset/modules.go

@@ -22,7 +22,7 @@ type ModuleRegistry struct {
 
 // newModuleRegistry reads and loads the configured module into the registry.
 func newModuleRegistry(modulesPath string,
-	moduleConfigs []ModuleConfig,
+	moduleConfigs []*ModuleConfig,
 	overrides *ModuleOverrides,
 	beatVersion string) (*ModuleRegistry, error) {
 
@@ -43,7 +43,7 @@ func newModuleRegistry(modulesPath string,
 		for _, filesetName := range moduleFilesets {
 			fcfg, exists := mcfg.Filesets[filesetName]
 			if !exists {
-				fcfg = &defaultFilesetConfig
+				fcfg = &FilesetConfig{}
 			}
 
 			fcfg, err = applyOverrides(fcfg, mcfg.Module, filesetName, overrides)
@@ -55,7 +55,7 @@ func newModuleRegistry(modulesPath string,
 				continue
 			}
 
-			fileset, err := New(modulesPath, filesetName, &mcfg, fcfg)
+			fileset, err := New(modulesPath, filesetName, mcfg, fcfg)
 			if err != nil {
 				return nil, err
 			}
@@ -100,13 +100,13 @@ func NewModuleRegistry(moduleConfigs []*common.Config, beatVersion string) (*Mod
 	if err != nil {
 		return nil, err
 	}
-	mcfgs := []ModuleConfig{}
+	mcfgs := []*ModuleConfig{}
 	for _, moduleConfig := range moduleConfigs {
 		mcfg, err := mcfgFromConfig(moduleConfig)
 		if err != nil {
 			return nil, fmt.Errorf("Error unpacking module config: %v", err)
 		}
-		mcfgs = append(mcfgs, *mcfg)
+		mcfgs = append(mcfgs, mcfg)
 	}
 	mcfgs, err = appendWithoutDuplicates(mcfgs, modulesCLIList)
 	if err != nil {
@@ -209,7 +209,7 @@ func applyOverrides(fcfg *FilesetConfig,
 
 // appendWithoutDuplicates appends basic module configuration for each module in the
 // modules list, unless the same module is not already loaded.
-func appendWithoutDuplicates(moduleConfigs []ModuleConfig, modules []string) ([]ModuleConfig, error) {
+func appendWithoutDuplicates(moduleConfigs []*ModuleConfig, modules []string) ([]*ModuleConfig, error) {
 	if len(modules) == 0 {
 		return moduleConfigs, nil
 	}
@@ -226,7 +226,7 @@ func appendWithoutDuplicates(moduleConfigs []ModuleConfig, modules []string) ([]
 	// add the non duplicates to the list
 	for _, module := range modules {
 		if _, exists := modulesMap[module]; !exists {
-			moduleConfigs = append(moduleConfigs, ModuleConfig{Module: module})
+			moduleConfigs = append(moduleConfigs, &ModuleConfig{Module: module})
 		}
 	}
 	return moduleConfigs, nil
@@ -285,6 +285,26 @@ func (reg *ModuleRegistry) LoadPipelines(esClient PipelineLoader) error {
 	return nil
 }
 
+// InfoString returns the enabled modules and filesets in a single string, ready to
+// be shown to the user
+func (reg *ModuleRegistry) InfoString() string {
+	var result string
+	for module, filesets := range reg.registry {
+		var filesetNames string
+		for name, _ := range filesets {
+			if filesetNames != "" {
+				filesetNames += ", "
+			}
+			filesetNames += name
+		}
+		if result != "" {
+			result += ", "
+		}
+		result += fmt.Sprintf("%s (%s)", module, filesetNames)
+	}
+	return result
+}
+
 // checkAvailableProcessors calls the /_nodes/ingest API and verifies that all processors listed
 // in the requiredProcessors list are available in Elasticsearch. Returns nil if all required
 // processors are available.

filebeat/fileset/modules_test.go

@@ -26,11 +26,11 @@ func TestNewModuleRegistry(t *testing.T) {
 	modulesPath, err := filepath.Abs("../module")
 	assert.NoError(t, err)
 
-	configs := []ModuleConfig{
-		{Module: "nginx"},
-		{Module: "mysql"},
-		{Module: "system"},
-		{Module: "auditd"},
+	configs := []*ModuleConfig{
+		&ModuleConfig{Module: "nginx"},
+		&ModuleConfig{Module: "mysql"},
+		&ModuleConfig{Module: "system"},
+		&ModuleConfig{Module: "auditd"},
 	}
 
 	reg, err := newModuleRegistry(modulesPath, configs, nil, "5.2.0")
@@ -58,8 +58,16 @@ func TestNewModuleRegistry(t *testing.T) {
 
 	for module, filesets := range reg.registry {
 		for name, fileset := range filesets {
-			_, err = fileset.getProspectorConfig()
+			cfg, err := fileset.getProspectorConfig()
 			assert.NoError(t, err, fmt.Sprintf("module: %s, fileset: %s", module, name))
+
+			moduleName, err := cfg.String("_module_name", -1)
+			assert.NoError(t, err)
+			assert.Equal(t, module, moduleName)
+
+			filesetName, err := cfg.String("_fileset_name", -1)
+			assert.NoError(t, err)
+			assert.Equal(t, name, filesetName)
 		}
 	}
 }
@@ -70,8 +78,8 @@ func TestNewModuleRegistryConfig(t *testing.T) {
 
 	falseVar := false
 
-	configs := []ModuleConfig{
-		{
+	configs := []*ModuleConfig{
+		&ModuleConfig{
 			Module: "nginx",
 			Filesets: map[string]*FilesetConfig{
 				"access": {
@@ -84,7 +92,7 @@ func TestNewModuleRegistryConfig(t *testing.T) {
 				},
 			},
 		},
-		{
+		&ModuleConfig{
 			Module:  "mysql",
 			Enabled: &falseVar,
 		},
@@ -191,24 +199,24 @@ func TestAppendWithoutDuplicates(t *testing.T) {
 	falseVar := false
 	tests := []struct {
 		name     string
-		configs  []ModuleConfig
+		configs  []*ModuleConfig
 		modules  []string
-		expected []ModuleConfig
+		expected []*ModuleConfig
 	}{
 		{
 			name:    "just modules",
-			configs: []ModuleConfig{},
+			configs: []*ModuleConfig{},
 			modules: []string{"moduleA", "moduleB", "moduleC"},
-			expected: []ModuleConfig{
-				{Module: "moduleA"},
-				{Module: "moduleB"},
-				{Module: "moduleC"},
+			expected: []*ModuleConfig{
+				&ModuleConfig{Module: "moduleA"},
+				&ModuleConfig{Module: "moduleB"},
+				&ModuleConfig{Module: "moduleC"},
 			},
 		},
 		{
 			name: "eliminate a duplicate, no override",
-			configs: []ModuleConfig{
-				{
+			configs: []*ModuleConfig{
+				&ModuleConfig{
 					Module: "moduleB",
 					Filesets: map[string]*FilesetConfig{
 						"fileset": {
@@ -220,8 +228,8 @@ func TestAppendWithoutDuplicates(t *testing.T) {
 				},
 			},
 			modules: []string{"moduleA", "moduleB", "moduleC"},
-			expected: []ModuleConfig{
-				{
+			expected: []*ModuleConfig{
+				&ModuleConfig{
 					Module: "moduleB",
 					Filesets: map[string]*FilesetConfig{
 						"fileset": {
@@ -231,14 +239,14 @@ func TestAppendWithoutDuplicates(t *testing.T) {
 						},
 					},
 				},
-				{Module: "moduleA"},
-				{Module: "moduleC"},
+				&ModuleConfig{Module: "moduleA"},
+				&ModuleConfig{Module: "moduleC"},
 			},
 		},
 		{
 			name: "disabled config",
-			configs: []ModuleConfig{
-				{
+			configs: []*ModuleConfig{
+				&ModuleConfig{
 					Module:  "moduleB",
 					Enabled: &falseVar,
 					Filesets: map[string]*FilesetConfig{
@@ -251,8 +259,8 @@ func TestAppendWithoutDuplicates(t *testing.T) {
 				},
 			},
 			modules: []string{"moduleA", "moduleB", "moduleC"},
-			expected: []ModuleConfig{
-				{
+			expected: []*ModuleConfig{
+				&ModuleConfig{
 					Module:  "moduleB",
 					Enabled: &falseVar,
 					Filesets: map[string]*FilesetConfig{
@@ -263,9 +271,9 @@ func TestAppendWithoutDuplicates(t *testing.T) {
 						},
 					},
 				},
-				{Module: "moduleA"},
-				{Module: "moduleB"},
-				{Module: "moduleC"},
+				&ModuleConfig{Module: "moduleA"},
+				&ModuleConfig{Module: "moduleB"},
+				&ModuleConfig{Module: "moduleC"},
 			},
 		},
 	}