Skip to content

Commit

Permalink
Avoid generating incomplete configurations in autodiscover (elastic#2…
Browse files Browse the repository at this point in the history
…0898)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.
  • Loading branch information
jsoriano authored Sep 2, 2020
1 parent 6d7213f commit 99fd545
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Ensure dynamic template names are unique for the same field. {pull}18849[18849]
- Remove the deprecated `xpack.monitoring.*` settings. Going forward only `monitoring.*` settings may be used. {issue}9424[9424] {pull}18608[18608]
- Added `certificate` TLS verification mode to ignore server name mismatch. {issue}12283[12283] {pull}20293[20293]
- Autodiscover doesn't generate any configuration when a variable is missing. Previously it generated an incomplete configuration. {pull}20898[20898]

*Auditbeat*

Expand Down Expand Up @@ -167,6 +168,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- [Metricbeat][Kubernetes] Change cluster_ip field from ip to keyword. {pull}20571[20571]
- Rename cloud.provider `az` value to `azure` inside the add_cloud_metadata processor. {pull}20689[20689]
- Add missing country_name geo field in `add_host_metadata` and `add_observer_metadata` processors. {issue}20796[20796] {pull}20811[20811]
- Explicitly detect missing variables in autodiscover configuration, log them at the debug level. {issue}20568[20568] {pull}20898[20898]

*Auditbeat*

Expand Down
16 changes: 14 additions & 2 deletions libbeat/autodiscover/template/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
package template

import (
"fmt"

"github.com/elastic/go-ucfg"
"github.com/elastic/go-ucfg/parse"

"github.com/elastic/beats/v7/libbeat/common"
"github.com/elastic/beats/v7/libbeat/common/bus"
Expand Down Expand Up @@ -123,7 +126,16 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config, options ...u
if err != nil {
logp.Err("Error building config: %v", err)
}

opts := []ucfg.Option{
// Catch-all resolve function to log fields not resolved in any other way,
// it needs to be the first resolver added, so it is executed the last one.
// Being the last one, its returned error will be the one returned by `Unpack`,
// this is important to give better feedback in case of failure.
ucfg.Resolve(func(name string) (string, parse.Config, error) {
return "", parse.Config{}, fmt.Errorf("field '%s' not available in event or environment", name)
}),

ucfg.PathSep("."),
ucfg.Env(vars),
ucfg.ResolveEnv,
Expand All @@ -139,9 +151,9 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config, options ...u
}
// Unpack config to process any vars in the template:
var unpacked map[string]interface{}
c.Unpack(&unpacked, opts...)
err = c.Unpack(&unpacked, opts...)
if err != nil {
logp.Err("Error unpacking config: %v", err)
logp.Debug("autodiscover", "Configuration template cannot be resolved: %v", err)
continue
}
// Repack again:
Expand Down
31 changes: 31 additions & 0 deletions libbeat/autodiscover/template/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ import (
"github.com/elastic/beats/v7/libbeat/common"
"github.com/elastic/beats/v7/libbeat/common/bus"
"github.com/elastic/beats/v7/libbeat/keystore"
"github.com/elastic/beats/v7/libbeat/logp"
)

func TestConfigsMapping(t *testing.T) {
logp.TestingSetup()

config, _ := common.NewConfigFrom(map[string]interface{}{
"correct": "config",
})
Expand All @@ -40,6 +43,13 @@ func TestConfigsMapping(t *testing.T) {
"hosts": [1]string{"1.2.3.4:8080"},
})

const envValue = "valuefromenv"
configFromEnv, _ := common.NewConfigFrom(map[string]interface{}{
"correct": envValue,
})

os.Setenv("CONFIGS_MAPPING_TESTENV", envValue)

tests := []struct {
mapping string
event bus.Event
Expand Down Expand Up @@ -79,6 +89,16 @@ func TestConfigsMapping(t *testing.T) {
},
expected: []*common.Config{config},
},
// No condition, value from environment
{
mapping: `
- config:
- correct: ${CONFIGS_MAPPING_TESTENV}`,
event: bus.Event{
"foo": 3,
},
expected: []*common.Config{configFromEnv},
},
// Match config and replace data.host and data.ports.<name> properly
{
mapping: `
Expand Down Expand Up @@ -111,6 +131,17 @@ func TestConfigsMapping(t *testing.T) {
},
expected: []*common.Config{configPorts},
},
// Missing variable, config is not generated
{
mapping: `
- config:
- module: something
hosts: ["${not.exists.host}"]`,
event: bus.Event{
"host": "1.2.3.4",
},
expected: nil,
},
}

for _, test := range tests {
Expand Down

0 comments on commit 99fd545

Please sign in to comment.