From 268dca43ac4c1ca94e11d16bd7f742c0b65b2d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Tue, 13 Nov 2018 19:08:57 +0100 Subject: [PATCH 1/8] Add Central Management blacklisting This change allows to define local blacklist for configurations coming from Central Management. If a configuration block matches the given regular expession, it will be ignored: For example: ``` management: blacklist: output: console metricbeat.modules.module: k.{18}s ``` --- x-pack/libbeat/management/blacklist.go | 188 ++++++++++++ x-pack/libbeat/management/blacklist_test.go | 311 ++++++++++++++++++++ x-pack/libbeat/management/config.go | 5 + x-pack/libbeat/management/manager.go | 46 ++- 4 files changed, 534 insertions(+), 16 deletions(-) create mode 100644 x-pack/libbeat/management/blacklist.go create mode 100644 x-pack/libbeat/management/blacklist_test.go diff --git a/x-pack/libbeat/management/blacklist.go b/x-pack/libbeat/management/blacklist.go new file mode 100644 index 00000000000..bc8b7abafcb --- /dev/null +++ b/x-pack/libbeat/management/blacklist.go @@ -0,0 +1,188 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package management + +import ( + "fmt" + "regexp" + "strings" + + "github.com/pkg/errors" + + "github.com/elastic/beats/libbeat/common" + "github.com/elastic/beats/libbeat/logp" + "github.com/elastic/beats/x-pack/libbeat/management/api" +) + +// ConfigBlacklist takes a ConfigBlocks object and filter it based on the given +// blacklist settings +type ConfigBlacklist struct { + patterns map[string]*regexp.Regexp +} + +// ConfigBlacklistSettings holds a list of fields and regular expressions to blacklist +type ConfigBlacklistSettings map[string]string + +// Unpack unpacks nested fields set with dot notation like foo.bar into the proper nesting +// in a nested map/slice structure. +func (f ConfigBlacklistSettings) Unpack(to interface{}) error { + m, ok := to.(map[string]interface{}) + if !ok { + return fmt.Errorf("wrong type, expect map") + } + + var expand func(key string, value interface{}) + + expand = func(key string, value interface{}) { + switch v := value.(type) { + case map[string]interface{}: + for k, val := range v { + expand(fmt.Sprintf("%v.%v", key, k), val) + } + case []interface{}: + for i := range v { + expand(fmt.Sprintf("%v.%v", key, i), v[i]) + } + default: + m[key] = fmt.Sprintf("%s", value) + } + } + + for k, val := range m { + expand(k, val) + } + return nil +} + +// NewConfigBlacklist filters configs from CM according to a given blacklist +func NewConfigBlacklist(patterns ConfigBlacklistSettings) (*ConfigBlacklist, error) { + list := ConfigBlacklist{ + patterns: map[string]*regexp.Regexp{}, + } + + for field, pattern := range patterns { + exp, err := regexp.Compile(pattern) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("Given expression is not a valid regexp: %s", pattern)) + } + + list.patterns[field] = exp + } + + return &list, nil +} + +// Filter returns a copy of the given ConfigBlocks with the +func (c *ConfigBlacklist) Filter(configBlocks api.ConfigBlocks) api.ConfigBlocks { + var result api.ConfigBlocks + + for _, configs := range configBlocks { + newConfigs := api.ConfigBlocksWithType{Type: configs.Type} + + for _, block := range configs.Blocks { + if c.blacklisted(configs.Type, block) { + logp.Err("Got a blacklisted configuration, ignoring it") + continue + } + + newConfigs.Blocks = append(newConfigs.Blocks, block) + } + + if len(newConfigs.Blocks) > 0 { + result = append(result, newConfigs) + } + } + + return result +} + +func (c *ConfigBlacklist) blacklisted(blockType string, block *api.ConfigBlock) bool { + cfg, err := block.ConfigWithMeta() + if err != nil { + return false + } + + for field, pattern := range c.patterns { + prefix := blockType + if strings.Contains(field, ".") { + prefix += "." + } + + if strings.HasPrefix(field, prefix) { + // This pattern affects a field on this block type + field = field[len(prefix):] + var segments []string + if len(field) > 0 { + segments = strings.Split(field, ".") + } + if c.blockMatches(pattern, segments, cfg.Config) { + return true + } + } + } + + return false +} + +func (c *ConfigBlacklist) blockMatches(pattern *regexp.Regexp, segments []string, current *common.Config) bool { + if current.IsDict() { + switch len(segments) { + case 0: + for _, field := range current.GetFields() { + if pattern.MatchString(field) { + return true + } + } + + case 1: + // Check field in the dict + val, err := current.String(segments[0], -1) + if err == nil { + return pattern.MatchString(val) + } + // not a string, traverse + child, _ := current.Child(segments[0], -1) + return child != nil && c.blockMatches(pattern, segments[1:], child) + + default: + // traverse the tree + child, _ := current.Child(segments[0], -1) + return child != nil && c.blockMatches(pattern, segments[1:], child) + + } + } + + if current.IsArray() { + switch len(segments) { + case 0: + // List of elements, match strings + for count, _ := current.CountField(""); count > 0; count-- { + val, err := current.String("", count-1) + if err == nil && pattern.MatchString(val) { + return true + } + + // not a string, traverse + child, _ := current.Child("", count-1) + if child != nil { + if c.blockMatches(pattern, segments, child) { + return true + } + } + } + + default: + // List of elements, explode traversal to all of them + for count, _ := current.CountField(""); count > 0; count-- { + child, _ := current.Child("", count-1) + if child != nil && c.blockMatches(pattern, segments, child) { + return true + } + } + } + } + + return false +} diff --git a/x-pack/libbeat/management/blacklist_test.go b/x-pack/libbeat/management/blacklist_test.go new file mode 100644 index 00000000000..83781fd72ad --- /dev/null +++ b/x-pack/libbeat/management/blacklist_test.go @@ -0,0 +1,311 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package management + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/elastic/beats/x-pack/libbeat/management/api" +) + +func TestConfigBlacklist(t *testing.T) { + tests := []struct { + name string + patterns map[string]string + blocks api.ConfigBlocks + blacklisted bool + + // only fill if blacklisted == true + expected api.ConfigBlocks + }{ + { + name: "No patterns", + blacklisted: false, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "output", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "output": "console", + }, + }, + }, + }, + }, + }, + { + name: "Blacklisted dict key", + blacklisted: true, + patterns: map[string]string{ + "output": "^console$", + }, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "output", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "console": map[string]interface{}{ + "pretty": "true", + }, + }, + }, + { + Raw: map[string]interface{}{ + "elasticsearch": map[string]interface{}{ + "host": "localhost", + }, + }, + }, + }, + }, + }, + expected: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "output", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "elasticsearch": map[string]interface{}{ + "host": "localhost", + }, + }, + }, + }, + }, + }, + }, + { + name: "Blacklisted value key", + blacklisted: true, + patterns: map[string]string{ + "metricbeat.modules.module": "k.{8}s", + }, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "metricbeat.modules", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "hosts": "localhost:10255", + }, + }, + }, + }, + }, + }, + { + name: "Blacklisted value in a list", + blacklisted: true, + patterns: map[string]string{ + "metricbeat.modules.metricsets": "event", + }, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "metricbeat.modules", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "metricsets": []string{ + "event", + "default", + }, + }, + }, + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "metricsets": []string{ + "default", + }, + }, + }, + }, + }, + }, + expected: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "metricbeat.modules", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "metricsets": []string{ + "default", + }, + }, + }, + }, + }, + }, + }, + { + name: "Blacklisted value in a deep list", + blacklisted: true, + patterns: map[string]string{ + "filebeat.inputs.containers.ids": "1ffeb0dbd13", + }, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "metricbeat.modules", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "metricsets": []string{ + "event", + "default", + }, + }, + }, + }, + }, + api.ConfigBlocksWithType{ + Type: "filebeat.inputs", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "type": "docker", + "containers": map[string]interface{}{ + "ids": []string{ + "1ffeb0dbd13", + }, + }, + }, + }, + { + Raw: map[string]interface{}{ + "type": "docker", + "containers": map[string]interface{}{ + "ids": []string{ + "256425931c2", + }, + }, + }, + }, + }, + }, + }, + expected: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "metricbeat.modules", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "module": "kubernetes", + "metricsets": []string{ + "event", + "default", + }, + }, + }, + }, + }, + api.ConfigBlocksWithType{ + Type: "filebeat.inputs", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "type": "docker", + "containers": map[string]interface{}{ + "ids": []string{ + "256425931c2", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "Blacklisted dict key in a list", + blacklisted: true, + patterns: map[string]string{ + "list.of.elements": "forbidden", + "list.of.elements.disallowed": "yes", + }, + blocks: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "list", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "of": map[string]interface{}{ + "elements": []interface{}{ + map[string]interface{}{ + "forbidden": "yes", + }, + }, + }, + }, + }, + { + Raw: map[string]interface{}{ + "of": map[string]interface{}{ + "elements": []interface{}{ + map[string]interface{}{ + "allowed": "yes", + }, + }, + }, + }, + }, + { + Raw: map[string]interface{}{ + "of": map[string]interface{}{ + "elements": []interface{}{ + map[string]interface{}{ + "disallowed": "yes", + }, + }, + }, + }, + }, + }, + }, + }, + expected: api.ConfigBlocks{ + api.ConfigBlocksWithType{ + Type: "list", + Blocks: []*api.ConfigBlock{ + { + Raw: map[string]interface{}{ + "of": map[string]interface{}{ + "elements": []interface{}{ + map[string]interface{}{ + "allowed": "yes", + }, + }, + }, + }, + }, + }, + }}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + bl, err := NewConfigBlacklist(test.patterns) + if err != nil { + t.Fatal(err) + } + + result := bl.Filter(test.blocks) + equal := api.ConfigBlocksEqual(result, test.blocks) + assert.Equal(t, test.blacklisted, !equal) + + if test.blacklisted { + assert.Equal(t, test.expected, result) + } + }) + } +} diff --git a/x-pack/libbeat/management/config.go b/x-pack/libbeat/management/config.go index 552525cf6b3..093b05549c0 100644 --- a/x-pack/libbeat/management/config.go +++ b/x-pack/libbeat/management/config.go @@ -77,11 +77,16 @@ type Config struct { AccessToken string `config:"access_token" yaml:"access_token"` Kibana *kibana.ClientConfig `config:"kibana" yaml:"kibana"` + + Blacklist ConfigBlacklistSettings `config:"blacklist" yaml:"blacklist"` } func defaultConfig() *Config { return &Config{ Period: 60 * time.Second, + Blacklist: ConfigBlacklistSettings{ + "output": "console|file", + }, } } diff --git a/x-pack/libbeat/management/manager.go b/x-pack/libbeat/management/manager.go index 9c494fd8996..2a6c6c8cca0 100644 --- a/x-pack/libbeat/management/manager.go +++ b/x-pack/libbeat/management/manager.go @@ -31,14 +31,15 @@ func init() { // ConfigManager handles internal config updates. By retrieving // new configs from Kibana and applying them to the Beat type ConfigManager struct { - config *Config - cache *Cache - logger *logp.Logger - client *api.Client - beatUUID uuid.UUID - done chan struct{} - registry *reload.Registry - wg sync.WaitGroup + config *Config + cache *Cache + logger *logp.Logger + client *api.Client + beatUUID uuid.UUID + done chan struct{} + registry *reload.Registry + wg sync.WaitGroup + blacklist *ConfigBlacklist } // NewConfigManager returns a X-Pack Beats Central Management manager @@ -56,9 +57,17 @@ func NewConfigManager(config *common.Config, registry *reload.Registry, beatUUID func NewConfigManagerWithConfig(c *Config, registry *reload.Registry, beatUUID uuid.UUID) (management.ConfigManager, error) { var client *api.Client var cache *Cache + var blacklist *ConfigBlacklist + if c.Enabled { var err error + // Initialize configs blacklist + blacklist, err = NewConfigBlacklist(c.Blacklist) + if err != nil { + return nil, errors.Wrap(err, "wrong settings for configurations blacklist") + } + // Initialize central management settings cache cache = &Cache{ ConfigOK: true, @@ -74,16 +83,18 @@ func NewConfigManagerWithConfig(c *Config, registry *reload.Registry, beatUUID u if err != nil { return nil, errors.Wrap(err, "initializing kibana client") } + } return &ConfigManager{ - config: c, - cache: cache, - logger: logp.NewLogger(management.DebugK), - client: client, - done: make(chan struct{}), - beatUUID: beatUUID, - registry: registry, + config: c, + cache: cache, + blacklist: blacklist, + logger: logp.NewLogger(management.DebugK), + client: client, + done: make(chan struct{}), + beatUUID: beatUUID, + registry: registry, }, nil } @@ -187,8 +198,11 @@ func (cm *ConfigManager) apply() { missing[name] = true } + // Filter unwanted configs from the list + configs := cm.blacklist.Filter(cm.cache.Configs) + // Reload configs - for _, b := range cm.cache.Configs { + for _, b := range configs { err := cm.reload(b.Type, b.Blocks) configOK = configOK && err == nil missing[b.Type] = false From 8bc8b2490c59d3ffc8cb22a3ee78f8c0d056e959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 15 Nov 2018 22:29:24 +0100 Subject: [PATCH 2/8] Rename signatures after comments --- x-pack/libbeat/management/blacklist.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/libbeat/management/blacklist.go b/x-pack/libbeat/management/blacklist.go index bc8b7abafcb..fdceabbaf89 100644 --- a/x-pack/libbeat/management/blacklist.go +++ b/x-pack/libbeat/management/blacklist.go @@ -82,7 +82,7 @@ func (c *ConfigBlacklist) Filter(configBlocks api.ConfigBlocks) api.ConfigBlocks newConfigs := api.ConfigBlocksWithType{Type: configs.Type} for _, block := range configs.Blocks { - if c.blacklisted(configs.Type, block) { + if c.isBlacklisted(configs.Type, block) { logp.Err("Got a blacklisted configuration, ignoring it") continue } @@ -98,7 +98,7 @@ func (c *ConfigBlacklist) Filter(configBlocks api.ConfigBlocks) api.ConfigBlocks return result } -func (c *ConfigBlacklist) blacklisted(blockType string, block *api.ConfigBlock) bool { +func (c *ConfigBlacklist) isBlacklisted(blockType string, block *api.ConfigBlock) bool { cfg, err := block.ConfigWithMeta() if err != nil { return false @@ -117,7 +117,7 @@ func (c *ConfigBlacklist) blacklisted(blockType string, block *api.ConfigBlock) if len(field) > 0 { segments = strings.Split(field, ".") } - if c.blockMatches(pattern, segments, cfg.Config) { + if c.isBlacklistedBlock(pattern, segments, cfg.Config) { return true } } @@ -126,7 +126,7 @@ func (c *ConfigBlacklist) blacklisted(blockType string, block *api.ConfigBlock) return false } -func (c *ConfigBlacklist) blockMatches(pattern *regexp.Regexp, segments []string, current *common.Config) bool { +func (c *ConfigBlacklist) isBlacklistedBlock(pattern *regexp.Regexp, segments []string, current *common.Config) bool { if current.IsDict() { switch len(segments) { case 0: @@ -144,12 +144,12 @@ func (c *ConfigBlacklist) blockMatches(pattern *regexp.Regexp, segments []string } // not a string, traverse child, _ := current.Child(segments[0], -1) - return child != nil && c.blockMatches(pattern, segments[1:], child) + return child != nil && c.isBlacklistedBlock(pattern, segments[1:], child) default: // traverse the tree child, _ := current.Child(segments[0], -1) - return child != nil && c.blockMatches(pattern, segments[1:], child) + return child != nil && c.isBlacklistedBlock(pattern, segments[1:], child) } } @@ -167,7 +167,7 @@ func (c *ConfigBlacklist) blockMatches(pattern *regexp.Regexp, segments []string // not a string, traverse child, _ := current.Child("", count-1) if child != nil { - if c.blockMatches(pattern, segments, child) { + if c.isBlacklistedBlock(pattern, segments, child) { return true } } @@ -177,7 +177,7 @@ func (c *ConfigBlacklist) blockMatches(pattern *regexp.Regexp, segments []string // List of elements, explode traversal to all of them for count, _ := current.CountField(""); count > 0; count-- { child, _ := current.Child("", count-1) - if child != nil && c.blockMatches(pattern, segments, child) { + if child != nil && c.isBlacklistedBlock(pattern, segments, child) { return true } } From e91a84456a279798689470893e6309aaa60fec23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Fri, 16 Nov 2018 12:49:55 +0100 Subject: [PATCH 3/8] Disable blocklisting in existing tests --- x-pack/libbeat/tests/system/test_management.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/libbeat/tests/system/test_management.py b/x-pack/libbeat/tests/system/test_management.py index dee3b133dea..da05f0021b0 100644 --- a/x-pack/libbeat/tests/system/test_management.py +++ b/x-pack/libbeat/tests/system/test_management.py @@ -93,7 +93,11 @@ def test_fetch_configs(self): ]) # Start beat - proc = self.start_beat(extra_args=["-E", "management.period=1s"]) + proc = self.start_beat(extra_args=[ + "-E", "management.period=1s", + # do not blacklist file output + "-E", "management.blacklist.output='.*'", + ]) # Wait for beat to apply new conf self.wait_log_contains("Applying settings for output") @@ -163,6 +167,8 @@ def test_configs_cache(self): proc = self.start_beat(extra_args=[ "-E", "management.kibana.host=wronghost", "-E", "management.kibana.timeout=0.5s", + # do not blacklist file output + "-E", "management.blacklist.output='.*'", ]) self.wait_until(cond=lambda: self.output_has( 1, output_file=output_file)) From 0f802c31e957f84e2fc28642938bc26e2b1625d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Tue, 27 Nov 2018 18:33:25 +0100 Subject: [PATCH 4/8] Fix config unpacking --- x-pack/libbeat/management/blacklist.go | 35 ++++--------- x-pack/libbeat/management/blacklist_test.go | 56 ++++++++++++++++++++- x-pack/libbeat/management/config.go | 4 +- 3 files changed, 69 insertions(+), 26 deletions(-) diff --git a/x-pack/libbeat/management/blacklist.go b/x-pack/libbeat/management/blacklist.go index fdceabbaf89..c12f378e175 100644 --- a/x-pack/libbeat/management/blacklist.go +++ b/x-pack/libbeat/management/blacklist.go @@ -23,46 +23,33 @@ type ConfigBlacklist struct { } // ConfigBlacklistSettings holds a list of fields and regular expressions to blacklist -type ConfigBlacklistSettings map[string]string +type ConfigBlacklistSettings struct { + Patterns map[string]string `yaml:",inline"` +} // Unpack unpacks nested fields set with dot notation like foo.bar into the proper nesting // in a nested map/slice structure. -func (f ConfigBlacklistSettings) Unpack(to interface{}) error { - m, ok := to.(map[string]interface{}) +func (f *ConfigBlacklistSettings) Unpack(from interface{}) error { + m, ok := from.(map[string]interface{}) if !ok { - return fmt.Errorf("wrong type, expect map") + return fmt.Errorf("wrong type, map is expected") } - var expand func(key string, value interface{}) - - expand = func(key string, value interface{}) { - switch v := value.(type) { - case map[string]interface{}: - for k, val := range v { - expand(fmt.Sprintf("%v.%v", key, k), val) - } - case []interface{}: - for i := range v { - expand(fmt.Sprintf("%v.%v", key, i), v[i]) - } - default: - m[key] = fmt.Sprintf("%s", value) - } + f.Patterns = map[string]string{} + for k, v := range common.MapStr(m).Flatten() { + f.Patterns[k] = fmt.Sprintf("%s", v) } - for k, val := range m { - expand(k, val) - } return nil } // NewConfigBlacklist filters configs from CM according to a given blacklist -func NewConfigBlacklist(patterns ConfigBlacklistSettings) (*ConfigBlacklist, error) { +func NewConfigBlacklist(cfg ConfigBlacklistSettings) (*ConfigBlacklist, error) { list := ConfigBlacklist{ patterns: map[string]*regexp.Regexp{}, } - for field, pattern := range patterns { + for field, pattern := range cfg.Patterns { exp, err := regexp.Compile(pattern) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("Given expression is not a valid regexp: %s", pattern)) diff --git a/x-pack/libbeat/management/blacklist_test.go b/x-pack/libbeat/management/blacklist_test.go index 83781fd72ad..6b35ba3897a 100644 --- a/x-pack/libbeat/management/blacklist_test.go +++ b/x-pack/libbeat/management/blacklist_test.go @@ -9,9 +9,60 @@ import ( "github.com/stretchr/testify/assert" + "github.com/elastic/beats/libbeat/common" "github.com/elastic/beats/x-pack/libbeat/management/api" ) +func TestConfigBlacklistSettingsUnpack(t *testing.T) { + tests := []struct { + name string + config *common.Config + error bool + expected ConfigBlacklistSettings + }{ + { + name: "Simple config", + config: common.MustNewConfigFrom(map[string]interface{}{ + "foo": "bar", + }), + expected: ConfigBlacklistSettings{ + Patterns: map[string]string{ + "foo": "bar", + }, + }, + }, + { + name: "Wrong config", + config: common.MustNewConfigFrom([]string{"a", "b"}), + error: true, + }, + { + name: "Tree config", + config: common.MustNewConfigFrom(map[string]interface{}{ + "foo": map[string]interface{}{ + "bar": "baz", + }, + }), + expected: ConfigBlacklistSettings{ + Patterns: map[string]string{ + "foo.bar": "baz", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var result ConfigBlacklistSettings + err := test.config.Unpack(&result) + if test.error { + assert.Error(t, err) + } + assert.Equal(t, test.expected, result) + }) + } +} + func TestConfigBlacklist(t *testing.T) { tests := []struct { name string @@ -294,7 +345,10 @@ func TestConfigBlacklist(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - bl, err := NewConfigBlacklist(test.patterns) + cfg := ConfigBlacklistSettings{ + Patterns: test.patterns, + } + bl, err := NewConfigBlacklist(cfg) if err != nil { t.Fatal(err) } diff --git a/x-pack/libbeat/management/config.go b/x-pack/libbeat/management/config.go index 093b05549c0..3781938f59c 100644 --- a/x-pack/libbeat/management/config.go +++ b/x-pack/libbeat/management/config.go @@ -85,7 +85,9 @@ func defaultConfig() *Config { return &Config{ Period: 60 * time.Second, Blacklist: ConfigBlacklistSettings{ - "output": "console|file", + Patterns: map[string]string{ + "output": "console|file", + }, }, } } From 0ac34dc9b60bc4ef6b1383ac4e78d49d06702a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Wed, 28 Nov 2018 12:09:34 +0100 Subject: [PATCH 5/8] Fix tests --- x-pack/libbeat/tests/system/test_management.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/libbeat/tests/system/test_management.py b/x-pack/libbeat/tests/system/test_management.py index da05f0021b0..44af6beefda 100644 --- a/x-pack/libbeat/tests/system/test_management.py +++ b/x-pack/libbeat/tests/system/test_management.py @@ -96,7 +96,7 @@ def test_fetch_configs(self): proc = self.start_beat(extra_args=[ "-E", "management.period=1s", # do not blacklist file output - "-E", "management.blacklist.output='.*'", + "-E", "management.blacklist.output='elasticsearch'", ]) # Wait for beat to apply new conf @@ -155,7 +155,10 @@ def test_configs_cache(self): output_file = os.path.join("output", "mockbeat_managed") # Start beat - proc = self.start_beat() + proc = self.start_beat(extra_args=[ + # do not blacklist file output + "-E", "management.blacklist.output='elasticsearch'", + ]) self.wait_until(cond=lambda: self.output_has( 1, output_file=output_file)) proc.check_kill_and_wait() @@ -168,7 +171,7 @@ def test_configs_cache(self): "-E", "management.kibana.host=wronghost", "-E", "management.kibana.timeout=0.5s", # do not blacklist file output - "-E", "management.blacklist.output='.*'", + "-E", "management.blacklist.output='elasticsearch'", ]) self.wait_until(cond=lambda: self.output_has( 1, output_file=output_file)) From 9254f79f66cf23685f360599c7792b75a2e70392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Wed, 28 Nov 2018 13:10:59 +0100 Subject: [PATCH 6/8] Test blacklist ignores bad configs --- .../libbeat/tests/system/test_management.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/x-pack/libbeat/tests/system/test_management.py b/x-pack/libbeat/tests/system/test_management.py index 44af6beefda..f09834228e0 100644 --- a/x-pack/libbeat/tests/system/test_management.py +++ b/x-pack/libbeat/tests/system/test_management.py @@ -177,6 +177,45 @@ def test_configs_cache(self): 1, output_file=output_file)) proc.check_kill_and_wait() + def test_blocklist(self): + """ + Config cache is used if Kibana is not available + """ + # Enroll the beat + config_path = os.path.join(self.working_dir, "mockbeat.yml") + self.render_config_template("mockbeat", config_path) + exit_code = self.enroll(KIBANA_PASSWORD) + assert exit_code == 0 + + # Update output configuration + self.create_and_assing_tag([ + { + "type": "output", + "configs": [ + { + "output": "file", + "file": { + "path": os.path.join(self.working_dir, "output"), + "filename": "mockbeat_managed", + } + } + ] + } + ]) + + output_file = os.path.join("output", "mockbeat_managed") + + # Start beat + proc = self.start_beat(extra_args=[ + # do not blacklist file output + "-E", "management.blacklist.output='output'", + ]) + + self.wait_until( + cond=lambda: self.log_contains("Got a blacklisted configuration, ignoring it")) + proc.check_kill_and_wait() + assert not os.path.isfile(os.path.join(self.working_dir, output_file)) + def enroll(self, password): return self.run_beat( extra_args=["enroll", self.get_kibana_url(), From 7dd370eefaa6bf968b9b5a004a61f00abbf218d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 10 Dec 2018 23:12:29 +0100 Subject: [PATCH 7/8] Do not apply any settings when a block is blacklisted --- x-pack/libbeat/management/blacklist.go | 21 ++--- x-pack/libbeat/management/blacklist_test.go | 89 +------------------ x-pack/libbeat/management/manager.go | 10 ++- .../libbeat/tests/system/test_management.py | 11 +-- 4 files changed, 19 insertions(+), 112 deletions(-) diff --git a/x-pack/libbeat/management/blacklist.go b/x-pack/libbeat/management/blacklist.go index c12f378e175..677faf9ac48 100644 --- a/x-pack/libbeat/management/blacklist.go +++ b/x-pack/libbeat/management/blacklist.go @@ -9,10 +9,10 @@ import ( "regexp" "strings" + "github.com/joeshaw/multierror" "github.com/pkg/errors" "github.com/elastic/beats/libbeat/common" - "github.com/elastic/beats/libbeat/logp" "github.com/elastic/beats/x-pack/libbeat/management/api" ) @@ -61,28 +61,19 @@ func NewConfigBlacklist(cfg ConfigBlacklistSettings) (*ConfigBlacklist, error) { return &list, nil } -// Filter returns a copy of the given ConfigBlocks with the -func (c *ConfigBlacklist) Filter(configBlocks api.ConfigBlocks) api.ConfigBlocks { - var result api.ConfigBlocks +// Filter an error if any of the given config blocks is blacklisted +func (c *ConfigBlacklist) Filter(configBlocks api.ConfigBlocks) error { + var errs multierror.Errors for _, configs := range configBlocks { - newConfigs := api.ConfigBlocksWithType{Type: configs.Type} - for _, block := range configs.Blocks { if c.isBlacklisted(configs.Type, block) { - logp.Err("Got a blacklisted configuration, ignoring it") - continue + errs = append(errs, fmt.Errorf("Config for '%s' is blacklisted", configs.Type)) } - - newConfigs.Blocks = append(newConfigs.Blocks, block) - } - - if len(newConfigs.Blocks) > 0 { - result = append(result, newConfigs) } } - return result + return errs.Err() } func (c *ConfigBlacklist) isBlacklisted(blockType string, block *api.ConfigBlock) bool { diff --git a/x-pack/libbeat/management/blacklist_test.go b/x-pack/libbeat/management/blacklist_test.go index 6b35ba3897a..b6fb8cd70b3 100644 --- a/x-pack/libbeat/management/blacklist_test.go +++ b/x-pack/libbeat/management/blacklist_test.go @@ -69,9 +69,6 @@ func TestConfigBlacklist(t *testing.T) { patterns map[string]string blocks api.ConfigBlocks blacklisted bool - - // only fill if blacklisted == true - expected api.ConfigBlocks }{ { name: "No patterns", @@ -116,20 +113,6 @@ func TestConfigBlacklist(t *testing.T) { }, }, }, - expected: api.ConfigBlocks{ - api.ConfigBlocksWithType{ - Type: "output", - Blocks: []*api.ConfigBlock{ - { - Raw: map[string]interface{}{ - "elasticsearch": map[string]interface{}{ - "host": "localhost", - }, - }, - }, - }, - }, - }, }, { name: "Blacklisted value key", @@ -181,21 +164,6 @@ func TestConfigBlacklist(t *testing.T) { }, }, }, - expected: api.ConfigBlocks{ - api.ConfigBlocksWithType{ - Type: "metricbeat.modules", - Blocks: []*api.ConfigBlock{ - { - Raw: map[string]interface{}{ - "module": "kubernetes", - "metricsets": []string{ - "default", - }, - }, - }, - }, - }, - }, }, { name: "Blacklisted value in a deep list", @@ -244,37 +212,6 @@ func TestConfigBlacklist(t *testing.T) { }, }, }, - expected: api.ConfigBlocks{ - api.ConfigBlocksWithType{ - Type: "metricbeat.modules", - Blocks: []*api.ConfigBlock{ - { - Raw: map[string]interface{}{ - "module": "kubernetes", - "metricsets": []string{ - "event", - "default", - }, - }, - }, - }, - }, - api.ConfigBlocksWithType{ - Type: "filebeat.inputs", - Blocks: []*api.ConfigBlock{ - { - Raw: map[string]interface{}{ - "type": "docker", - "containers": map[string]interface{}{ - "ids": []string{ - "256425931c2", - }, - }, - }, - }, - }, - }, - }, }, { name: "Blacklisted dict key in a list", @@ -323,23 +260,6 @@ func TestConfigBlacklist(t *testing.T) { }, }, }, - expected: api.ConfigBlocks{ - api.ConfigBlocksWithType{ - Type: "list", - Blocks: []*api.ConfigBlock{ - { - Raw: map[string]interface{}{ - "of": map[string]interface{}{ - "elements": []interface{}{ - map[string]interface{}{ - "allowed": "yes", - }, - }, - }, - }, - }, - }, - }}, }, } @@ -353,13 +273,8 @@ func TestConfigBlacklist(t *testing.T) { t.Fatal(err) } - result := bl.Filter(test.blocks) - equal := api.ConfigBlocksEqual(result, test.blocks) - assert.Equal(t, test.blacklisted, !equal) - - if test.blacklisted { - assert.Equal(t, test.expected, result) - } + err = bl.Filter(test.blocks) + assert.Equal(t, test.blacklisted, err != nil) }) } } diff --git a/x-pack/libbeat/management/manager.go b/x-pack/libbeat/management/manager.go index 2a6c6c8cca0..81aff95bc5d 100644 --- a/x-pack/libbeat/management/manager.go +++ b/x-pack/libbeat/management/manager.go @@ -199,10 +199,14 @@ func (cm *ConfigManager) apply() { } // Filter unwanted configs from the list - configs := cm.blacklist.Filter(cm.cache.Configs) + errors := cm.blacklist.Filter(cm.cache.Configs) + if errors != nil { + cm.logger.Error(errors) + return + } // Reload configs - for _, b := range configs { + for _, b := range cm.cache.Configs { err := cm.reload(b.Type, b.Blocks) configOK = configOK && err == nil missing[b.Type] = false @@ -216,7 +220,7 @@ func (cm *ConfigManager) apply() { } if !configOK { - logp.Info("Failed to apply settings, reporting error on next fetch") + cm.logger.Info("Failed to apply settings, reporting error on next fetch") } // Update configOK flag with the result of this apply diff --git a/x-pack/libbeat/tests/system/test_management.py b/x-pack/libbeat/tests/system/test_management.py index f09834228e0..ba2e8e3dff3 100644 --- a/x-pack/libbeat/tests/system/test_management.py +++ b/x-pack/libbeat/tests/system/test_management.py @@ -177,9 +177,9 @@ def test_configs_cache(self): 1, output_file=output_file)) proc.check_kill_and_wait() - def test_blocklist(self): + def test_blacklist(self): """ - Config cache is used if Kibana is not available + Blacklist blocks bad configs """ # Enroll the beat config_path = os.path.join(self.working_dir, "mockbeat.yml") @@ -206,13 +206,10 @@ def test_blocklist(self): output_file = os.path.join("output", "mockbeat_managed") # Start beat - proc = self.start_beat(extra_args=[ - # do not blacklist file output - "-E", "management.blacklist.output='output'", - ]) + proc = self.start_beat() self.wait_until( - cond=lambda: self.log_contains("Got a blacklisted configuration, ignoring it")) + cond=lambda: self.log_contains("Config for 'output' is blacklisted")) proc.check_kill_and_wait() assert not os.path.isfile(os.path.join(self.working_dir, output_file)) From b3dcca54321da668db4e17b705696188b4d1c5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Tue, 11 Dec 2018 01:10:25 +0100 Subject: [PATCH 8/8] fix test --- x-pack/libbeat/tests/system/test_management.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/libbeat/tests/system/test_management.py b/x-pack/libbeat/tests/system/test_management.py index ba2e8e3dff3..0345d1ce8e4 100644 --- a/x-pack/libbeat/tests/system/test_management.py +++ b/x-pack/libbeat/tests/system/test_management.py @@ -95,8 +95,8 @@ def test_fetch_configs(self): # Start beat proc = self.start_beat(extra_args=[ "-E", "management.period=1s", - # do not blacklist file output - "-E", "management.blacklist.output='elasticsearch'", + # do not blacklist file/elasticsearch outputs + "-E", "management.blacklist.output='foo'", ]) # Wait for beat to apply new conf