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

Fix Filebeat autodiscover hints default settings #12348

Merged
merged 1 commit into from
May 29, 2019
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
21 changes: 14 additions & 7 deletions filebeat/autodiscover/builder/hints/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@ type config struct {
func defaultConfig() config {
defaultCfgRaw := map[string]interface{}{
"type": "container",
"containers": map[string]interface{}{
"paths": []string{
// To be able to use this builder with CRI-O replace paths with:
// /var/log/pods/${data.kubernetes.pod.uid}/${data.kubernetes.container.name}/*.log
"/var/lib/docker/containers/${data.container.id}/*-json.log",
},
"paths": []string{
// To be able to use this builder with CRI-O replace paths with:
// /var/log/pods/${data.kubernetes.pod.uid}/${data.kubernetes.container.name}/*.log
"/var/lib/docker/containers/${data.container.id}/*-json.log",
},
}
defaultCfg, _ := common.NewConfigFrom(defaultCfgRaw)
Expand All @@ -53,7 +51,16 @@ func (c *config) Unpack(from *common.Config) error {
}

if config, err := from.Child("default_config", -1); err == nil {
c.DefaultConfig = config
fields := config.GetFields()
if len(fields) == 1 && fields[0] == "enabled" {
// only enabling/disabling default config:
if err := c.DefaultConfig.Merge(config); err != nil {
return nil
}
} else {
// full config provided, discard default
c.DefaultConfig = config
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a full config is provided, but also with the enabled setting set to true?
Would this work?

          hints.default_config:
            type: container
            enabled: true
            paths:
              - "/var/log/pods/${data.kubernetes.pod.uid}/${data.kubernetes.container.name}/*.log"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, as I'm checking number of fields == 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right

}

c.Key = tmpConfig.Key
Expand Down
4 changes: 3 additions & 1 deletion filebeat/autodiscover/builder/hints/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,10 @@ func (l *logHints) CreateConfig(event bus.Event) []*common.Config {
return []*common.Config{}
}

// Clone original config
// Clone original config, enable it if disabled
config, _ := common.NewConfigFrom(l.config.DefaultConfig)
config.Remove("enabled", -1)

host, _ := event["host"].(string)
if host == "" {
return []*common.Config{}
Expand Down
134 changes: 111 additions & 23 deletions filebeat/autodiscover/builder/hints/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,105 @@ import (
)

func TestGenerateHints(t *testing.T) {
customCfg := common.MustNewConfigFrom(map[string]interface{}{
"default_config": map[string]interface{}{
"type": "docker",
"containers": map[string]interface{}{
"ids": []string{
"${data.container.id}",
},
},
"close_timeout": "true",
},
})

defaultCfg := common.NewConfig()

defaultDisabled := common.MustNewConfigFrom(map[string]interface{}{
"default_config": map[string]interface{}{
"enabled": "false",
},
})

tests := []struct {
msg string
config *common.Config
event bus.Event
len int
result common.MapStr
}{
{
msg: "Hints without host should return nothing",
msg: "Default config is correct",
config: defaultCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
},
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
},
len: 1,
result: common.MapStr{
"paths": []interface{}{"/var/lib/docker/containers/abc/*-json.log"},
"type": "container",
},
},
{
msg: "Config disabling works",
config: defaultDisabled,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
},
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
},
len: 0,
},
{
msg: "Hint to enable when disabled by default works",
config: defaultDisabled,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
},
"container": common.MapStr{
"name": "foobar",
"id": "abc",
},
"hints": common.MapStr{
"logs": common.MapStr{
"enabled": "true",
"exclude_lines": "^test2, ^test3",
},
},
},
len: 1,
result: common.MapStr{
"type": "container",
"paths": []interface{}{"/var/lib/docker/containers/abc/*-json.log"},
"exclude_lines": []interface{}{"^test2", "^test3"},
},
},
{
msg: "Hints without host should return nothing",
config: customCfg,
event: bus.Event{
"hints": common.MapStr{
"metrics": common.MapStr{
Expand All @@ -48,7 +139,8 @@ func TestGenerateHints(t *testing.T) {
result: common.MapStr{},
},
{
msg: "Hints with logs.disable should return nothing",
msg: "Hints with logs.disable should return nothing",
config: customCfg,
event: bus.Event{
"hints": common.MapStr{
"logs": common.MapStr{
Expand All @@ -60,7 +152,8 @@ func TestGenerateHints(t *testing.T) {
result: common.MapStr{},
},
{
msg: "Empty event hints should return default config",
msg: "Empty event hints should return default config",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand All @@ -84,7 +177,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with include|exclude_lines must be part of the input config",
msg: "Hint with include|exclude_lines must be part of the input config",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -116,7 +210,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with multiline config must have a multiline in the input config",
msg: "Hint with multiline config must have a multiline in the input config",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -152,7 +247,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with inputs config as json must be accepted",
msg: "Hint with inputs config as json must be accepted",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -184,7 +280,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with processors config must have a processors in the input config",
msg: "Hint with processors config must have a processors in the input config",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -230,7 +327,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with module should attach input to its filesets",
msg: "Hint with module should attach input to its filesets",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -277,7 +375,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with module should honor defined filesets",
msg: "Hint with module should honor defined filesets",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -325,7 +424,8 @@ func TestGenerateHints(t *testing.T) {
},
},
{
msg: "Hint with module should honor defined filesets with streams",
msg: "Hint with module should honor defined filesets with streams",
config: customCfg,
event: bus.Event{
"host": "1.2.3.4",
"kubernetes": common.MapStr{
Expand Down Expand Up @@ -376,25 +476,13 @@ func TestGenerateHints(t *testing.T) {
}

for _, test := range tests {
cfg, _ := common.NewConfigFrom(map[string]interface{}{
"default_config": map[string]interface{}{
"type": "docker",
"containers": map[string]interface{}{
"ids": []string{
"${data.container.id}",
},
},
"close_timeout": "true",
},
})

// Configure path for modules access
abs, _ := filepath.Abs("../../..")
err := paths.InitPaths(&paths.Path{
Home: abs,
})

l, err := NewLogHints(cfg)
l, err := NewLogHints(test.config)
if err != nil {
t.Fatal(err)
}
Expand Down