From 67e2fa2dbb400a1e8bc02448ccb56efd53a84725 Mon Sep 17 00:00:00 2001 From: Tudor Golubenco Date: Thu, 27 Jul 2017 15:14:55 +0300 Subject: [PATCH] Fix mixed up modules configuration Fixes #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 | 2 + filebeat/beater/filebeat.go | 3 ++ filebeat/fileset/config.go | 2 - filebeat/fileset/modules.go | 34 ++++++++++++---- filebeat/fileset/modules_test.go | 66 ++++++++++++++++++-------------- 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 0358220bc32..23fec91ea87 100644 --- a/CHANGELOG.asciidoc +++ b/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* diff --git a/filebeat/beater/filebeat.go b/filebeat/beater/filebeat.go index 2b47adcc1ac..b0619722e4d 100644 --- a/filebeat/beater/filebeat.go +++ b/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 { diff --git a/filebeat/fileset/config.go b/filebeat/fileset/config.go index 57dc98b1483..7a20fa158c8 100644 --- a/filebeat/fileset/config.go +++ b/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{} diff --git a/filebeat/fileset/modules.go b/filebeat/fileset/modules.go index d9c364efb8d..398997df139 100644 --- a/filebeat/fileset/modules.go +++ b/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. diff --git a/filebeat/fileset/modules_test.go b/filebeat/fileset/modules_test.go index f5a6e83b68f..da4e34ec3c0 100644 --- a/filebeat/fileset/modules_test.go +++ b/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"}, }, }, }