Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick #4772 to 6.0: Fix mixed up modules configuration #4784

Merged
merged 1 commit into from
Aug 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
3 changes: 3 additions & 0 deletions filebeat/beater/filebeat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions filebeat/fileset/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ type FilesetConfig struct {
Var map[string]interface{} `config:"var"`
Prospector map[string]interface{} `config:"prospector"`
}

var defaultFilesetConfig = FilesetConfig{}
34 changes: 27 additions & 7 deletions filebeat/fileset/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions filebeat/fileset/modules_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func TestSetupNginx(t *testing.T) {
modulesPath, err := filepath.Abs("../module")
assert.NoError(t, err)

configs := []ModuleConfig{
{Module: "nginx"},
configs := []*ModuleConfig{
&ModuleConfig{Module: "nginx"},
}

reg, err := newModuleRegistry(modulesPath, configs, nil, "5.2.0")
Expand Down
66 changes: 37 additions & 29 deletions filebeat/fileset/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -70,8 +78,8 @@ func TestNewModuleRegistryConfig(t *testing.T) {

falseVar := false

configs := []ModuleConfig{
{
configs := []*ModuleConfig{
&ModuleConfig{
Module: "nginx",
Filesets: map[string]*FilesetConfig{
"access": {
Expand All @@ -84,7 +92,7 @@ func TestNewModuleRegistryConfig(t *testing.T) {
},
},
},
{
&ModuleConfig{
Module: "mysql",
Enabled: &falseVar,
},
Expand Down Expand Up @@ -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": {
Expand All @@ -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": {
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -263,9 +271,9 @@ func TestAppendWithoutDuplicates(t *testing.T) {
},
},
},
{Module: "moduleA"},
{Module: "moduleB"},
{Module: "moduleC"},
&ModuleConfig{Module: "moduleA"},
&ModuleConfig{Module: "moduleB"},
&ModuleConfig{Module: "moduleC"},
},
},
}
Expand Down