Filebeat: Have filesets disabled unless explicitly configured

CHANGELOG.next.asciidoc

@@ -85,6 +85,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Add option for S3 input to work without SQS notification {issue}18205[18205] {pull}27332[27332]
 - Fix Crowdstrike ingest pipeline that was creating flattened `process` fields. {issue}27622[27622] {pull}27623[27623]
 - Rename `log.path` to `log.file.path` in filestream to be consistent with `log` input and ECS. {pull}27761[27761]
+- Only filesets that are explicitly configured will be enabled. {issue}17256[17256] {pull}27526[27526]
 
 *Heartbeat*
 - Remove long deprecated `watch_poll` functionality. {pull}27166[27166]

filebeat/autodiscover/builder/hints/logs.go

@@ -201,7 +201,7 @@ func (l *logHints) getFilesets(hints common.MapStr, module string) map[string]*f
 	var configured bool
 	filesets := make(map[string]*filesetConfig)
 
-	moduleFilesets, err := l.registry.ModuleFilesets(module)
+	moduleFilesets, err := l.registry.ModuleAvailableFilesets(module)
 	if err != nil {
 		logp.Err("Error retrieving module filesets: %+v", err)
 		return nil

filebeat/beater/filebeat.go

@@ -113,6 +113,19 @@ func newBeater(b *beat.Beat, plugins PluginFactory, rawConfig *common.Config) (b
 	}
 	if !moduleRegistry.Empty() {
 		logp.Info("Enabled modules/filesets: %s", moduleRegistry.InfoString())
+		for _, mod := range moduleRegistry.ModuleNames() {
+			if mod == "" {
+				continue
+			}
+			filesets, err := moduleRegistry.ModuleConfiguredFilesets(mod)
+			if err != nil {
+				logp.Err("Failed listing filesets for module %s", mod)
+				continue
+			}
+			if len(filesets) == 0 {
+				logp.Warn("Module %s is enabled but has no enabled filesets", mod)
+			}
+		}
 	}
 
 	moduleInputs, err := moduleRegistry.GetInputConfigs()

filebeat/docs/filebeat-modules-options.asciidoc

@@ -75,8 +75,12 @@ The following example shows a configuration that runs the `nginx`,`mysql`, and
 ----
 {beatname_lc}.modules:
 - module: nginx
+  access:
+  error:
 - module: mysql
+  slowlog:
 - module: system
+  auth:
 ----
 
 [[advanced-settings]]

filebeat/fileset/modules.go

@@ -69,33 +69,13 @@ func newModuleRegistry(modulesPath string,
 			return nil, fmt.Errorf("error getting filesets for module %s: %v", mcfg.Module, err)
 		}
 
-		for _, filesetName := range moduleFilesets {
-			fcfg, exists := mcfg.Filesets[filesetName]
-			if !exists {
-				fcfg = &FilesetConfig{}
-			}
+		for filesetName, fcfg := range mcfg.Filesets {
 
 			fcfg, err = applyOverrides(fcfg, mcfg.Module, filesetName, overrides)
 			if err != nil {
 				return nil, fmt.Errorf("error applying overrides on fileset %s/%s: %v", mcfg.Module, filesetName, err)
 			}
 
-			if fcfg.Enabled != nil && !(*fcfg.Enabled) {
-				continue
-			}
-
-			fileset, err := New(modulesPath, filesetName, mcfg, fcfg)
-			if err != nil {
-				return nil, err
-			}
-			if err = fileset.Read(beatInfo); err != nil {
-				return nil, fmt.Errorf("error reading fileset %s/%s: %v", mcfg.Module, filesetName, err)
-			}
-			reg.registry[mcfg.Module][filesetName] = fileset
-		}
-
-		// check that no extra filesets are configured
-		for filesetName, fcfg := range mcfg.Filesets {
 			if fcfg.Enabled != nil && !(*fcfg.Enabled) {
 				continue
 			}
@@ -108,6 +88,15 @@ func newModuleRegistry(modulesPath string,
 			if !found {
 				return nil, fmt.Errorf("fileset %s/%s is configured but doesn't exist", mcfg.Module, filesetName)
 			}
+
+			fileset, err := New(modulesPath, filesetName, mcfg, fcfg)
+			if err != nil {
+				return nil, err
+			}
+			if err = fileset.Read(beatInfo); err != nil {
+				return nil, fmt.Errorf("error reading fileset %s/%s: %v", mcfg.Module, filesetName, err)
+			}
+			reg.registry[mcfg.Module][filesetName] = fileset
 		}
 	}
 
@@ -152,9 +141,30 @@ func NewModuleRegistry(moduleConfigs []*common.Config, beatInfo beat.Info, init
 		return nil, err
 	}
 
+	enableFilesetsFromOverrides(mcfgs, modulesOverrides)
 	return newModuleRegistry(modulesPath, mcfgs, modulesOverrides, beatInfo)
 }
 
+// enableFilesetsFromOverrides enables in mcfgs the filesets mentioned in overrides,
+// so that the overridden configuration can be applied.
+func enableFilesetsFromOverrides(mcfgs []*ModuleConfig, overrides *ModuleOverrides) {
+	if overrides == nil {
+		return
+	}
+	for _, mcfg := range mcfgs {
+		if modOvr, ok := (*overrides)[mcfg.Module]; ok {
+			for fset := range modOvr {
+				if _, ok = mcfg.Filesets[fset]; !ok {
+					if mcfg.Filesets == nil {
+						mcfg.Filesets = make(map[string]*FilesetConfig)
+					}
+					mcfg.Filesets[fset] = &FilesetConfig{}
+				}
+			}
+		}
+	}
+}
+
 func mcfgFromConfig(cfg *common.Config) (*ModuleConfig, error) {
 	var mcfg ModuleConfig
 
@@ -171,11 +181,18 @@ func mcfgFromConfig(cfg *common.Config) (*ModuleConfig, error) {
 	}
 
 	mcfg.Filesets = map[string]*FilesetConfig{}
-	for name, filesetConfig := range dict {
+
+	// This calls cfg.GetFields() instead of iterating over `dict` keys
+	// because cfg.Unpack above doesn't return keys that map to a nil value,
+	// but GetFields() returns all keys. We need to observe filesets that
+	// don't contain any configuration (all default values).
+	for _, name := range cfg.GetFields() {
 		if name == "module" || name == "enabled" || name == "path" {
 			continue
 		}
 
+		filesetConfig, _ := dict[name] // Nil config if name is not present.
+
 		tmpCfg, err := common.NewConfigFrom(filesetConfig)
 		if err != nil {
 			return nil, fmt.Errorf("error creating config from fileset %s/%s: %v", mcfg.Module, name, err)
@@ -400,9 +417,19 @@ func (reg *ModuleRegistry) ModuleNames() []string {
 	return modules
 }
 
-// ModuleFilesets return the list of available filesets for the given module
+// ModuleAvailableFilesets return the list of available filesets for the given module
 // it returns an empty list if the module doesn't exist
-func (reg *ModuleRegistry) ModuleFilesets(module string) ([]string, error) {
+func (reg *ModuleRegistry) ModuleAvailableFilesets(module string) ([]string, error) {
 	modulesPath := paths.Resolve(paths.Home, "module")
 	return getModuleFilesets(modulesPath, module)
 }
+
+// ModuleConfiguredFilesets return the list of configured filesets for the given module
+// it returns an empty list if the module doesn't exist
+func (reg *ModuleRegistry) ModuleConfiguredFilesets(module string) (list []string, err error) {
+	filesets, _ := reg.registry[module]
+	for name := range filesets {
+		list = append(list, name)
+	}
+	return
+}

filebeat/fileset/modules_integration_test.go

@@ -105,7 +105,13 @@ func TestSetupNginx(t *testing.T) {
 	require.NoError(t, err)
 
 	configs := []*ModuleConfig{
-		{Module: "nginx"},
+		{
+			Module: "nginx",
+			Filesets: map[string]*FilesetConfig{
+				"error":  {},
+				"access": {},
+			},
+		},
 	}
 
 	reg, err := newModuleRegistry(modulesPath, configs, nil, makeTestInfo("5.2.0"))

filebeat/fileset/modules_test.go

@@ -45,11 +45,39 @@ func TestNewModuleRegistry(t *testing.T) {
 	modulesPath, err := filepath.Abs("../module")
 	require.NoError(t, err)
 
+	falseVar := false
+
 	configs := []*ModuleConfig{
-		{Module: "nginx"},
-		{Module: "mysql"},
-		{Module: "system"},
-		{Module: "auditd"},
+		{
+			Module: "nginx",
+			Filesets: map[string]*FilesetConfig{
+				"access": {},
+				"error":  {},
+				"ingress_controller": {
+					Enabled: &falseVar,
+				},
+			},
+		},
+		{
+			Module: "mysql",
+			Filesets: map[string]*FilesetConfig{
+				"slowlog": {},
+				"error":   {},
+			},
+		},
+		{
+			Module: "system",
+			Filesets: map[string]*FilesetConfig{
+				"syslog": {},
+				"auth":   {},
+			},
+		},
+		{
+			Module: "auditd",
+			Filesets: map[string]*FilesetConfig{
+				"log": {},
+			},
+		},
 	}
 
 	reg, err := newModuleRegistry(modulesPath, configs, nil, beat.Info{Version: "5.2.0"})
@@ -58,7 +86,7 @@ func TestNewModuleRegistry(t *testing.T) {
 
 	expectedModules := map[string][]string{
 		"auditd": {"log"},
-		"nginx":  {"access", "error", "ingress_controller"},
+		"nginx":  {"access", "error"},
 		"mysql":  {"slowlog", "error"},
 		"system": {"syslog", "auth"},
 	}
@@ -374,6 +402,19 @@ func TestMcfgFromConfig(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "empty fileset (nil)",
+			config: load(t, map[string]interface{}{
+				"module": "nginx",
+				"error":  nil,
+			}),
+			expected: ModuleConfig{
+				Module: "nginx",
+				Filesets: map[string]*FilesetConfig{
+					"error": {},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -451,3 +492,132 @@ func TestInterpretError(t *testing.T) {
 		})
 	}
 }
+
+func TestEnableFilesetsFromOverrides(t *testing.T) {
+	tests := []struct {
+		Name      string
+		Cfg       []*ModuleConfig
+		Overrides *ModuleOverrides
+		Expected  []*ModuleConfig
+	}{
+		{
+			Name: "add fileset",
+			Cfg: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+					},
+				},
+			},
+			Overrides: &ModuleOverrides{
+				"foo": {
+					"baz": nil,
+				},
+			},
+			Expected: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+						"baz": {},
+					},
+				},
+			},
+		},
+		{
+			Name: "defined fileset",
+			Cfg: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {
+							Var: map[string]interface{}{
+								"a": "b",
+							},
+						},
+					},
+				},
+			},
+			Overrides: &ModuleOverrides{
+				"foo": {
+					"bar": nil,
+				},
+			},
+			Expected: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {
+							Var: map[string]interface{}{
+								"a": "b",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			Name: "disabled module",
+			Cfg: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+					},
+				},
+			},
+			Overrides: &ModuleOverrides{
+				"other": {
+					"bar": nil,
+				},
+			},
+			Expected: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+					},
+				},
+			},
+		},
+		{
+			Name: "nil overrides",
+			Cfg: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+					},
+				},
+			},
+			Overrides: nil,
+			Expected: []*ModuleConfig{
+				{
+					Module: "foo",
+					Filesets: map[string]*FilesetConfig{
+						"bar": {},
+					},
+				},
+			},
+		},
+		{
+			Name: "no modules",
+			Cfg:  nil,
+			Overrides: &ModuleOverrides{
+				"other": {
+					"bar": nil,
+				},
+			},
+			Expected: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			enableFilesetsFromOverrides(test.Cfg, test.Overrides)
+			assert.Equal(t, test.Expected, test.Cfg)
+		})
+	}
+
+}