From 63eb064eab92651d647dc54c9a8b3312ad4cc608 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 2 Sep 2020 13:15:38 +0200 Subject: [PATCH 1/3] Avoid generating incomplete configurations in autodiscover (#20898) 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. (cherry picked from commit 99fd545d87df4c66a9f876a7813e3f3c761de18d) --- CHANGELOG.next.asciidoc | 12 ++++++++ libbeat/autodiscover/template/config.go | 16 ++++++++-- libbeat/autodiscover/template/config_test.go | 31 ++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 4570781a08f5..bdccad3ed268 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -12,6 +12,17 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Update to Golang 1.12.1. {pull}11330[11330] - Disable Alibaba Cloud and Tencent Cloud metadata providers by default. {pull}13812[12812] +- The document id fields has been renamed from @metadata.id to @metadata._id {pull}15859[15859] +- Variable substitution from environment variables is not longer supported. {pull}15937{15937} +- Change aws_elb autodiscover provider field name from elb_listener.* to aws.elb.*. {issue}16219[16219] {pull}16402{16402} +- Remove `AddDockerMetadata` and `AddKubernetesMetadata` processors from the `script` processor. They can still be used as normal processors in the configuration. {issue}16349[16349] {pull}16514[16514] +- Introduce APM libbeat instrumentation, active when running the beat with ELASTIC_APM_ACTIVE=true. {pull}17938[17938] +- Remove the non-ECS `agent.hostname` field. Use the `agent.name` or `agent.id` fields for an identifier. {issue}16377[16377] {pull}18328[18328] +- Make error message about locked data path actionable. {pull}18667[18667] +- 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* @@ -54,6 +65,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Change `decode_json_fields` processor, to merge parsed json objects with existing objects in the event instead of fully replacing them. {pull}17958[17958] - 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* diff --git a/libbeat/autodiscover/template/config.go b/libbeat/autodiscover/template/config.go index a34cec104447..a1f87d2bcfc1 100644 --- a/libbeat/autodiscover/template/config.go +++ b/libbeat/autodiscover/template/config.go @@ -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" @@ -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, @@ -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: diff --git a/libbeat/autodiscover/template/config_test.go b/libbeat/autodiscover/template/config_test.go index 87e9ef5592a7..7964ba241269 100644 --- a/libbeat/autodiscover/template/config_test.go +++ b/libbeat/autodiscover/template/config_test.go @@ -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", }) @@ -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 @@ -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. properly { mapping: ` @@ -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 { From ecec6f5147e1055e62fb9d3385f3c29d76541453 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 2 Sep 2020 13:31:08 +0200 Subject: [PATCH 2/3] Fix changelog --- CHANGELOG.next.asciidoc | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index bdccad3ed268..2f9f98fa5b1c 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -12,16 +12,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Update to Golang 1.12.1. {pull}11330[11330] - Disable Alibaba Cloud and Tencent Cloud metadata providers by default. {pull}13812[12812] -- The document id fields has been renamed from @metadata.id to @metadata._id {pull}15859[15859] -- Variable substitution from environment variables is not longer supported. {pull}15937{15937} -- Change aws_elb autodiscover provider field name from elb_listener.* to aws.elb.*. {issue}16219[16219] {pull}16402{16402} -- Remove `AddDockerMetadata` and `AddKubernetesMetadata` processors from the `script` processor. They can still be used as normal processors in the configuration. {issue}16349[16349] {pull}16514[16514] -- Introduce APM libbeat instrumentation, active when running the beat with ELASTIC_APM_ACTIVE=true. {pull}17938[17938] -- Remove the non-ECS `agent.hostname` field. Use the `agent.name` or `agent.id` fields for an identifier. {issue}16377[16377] {pull}18328[18328] -- Make error message about locked data path actionable. {pull}18667[18667] -- 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* From fc73050956f28897b383705d29be1573fc7916b3 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 3 Sep 2020 18:50:18 +0200 Subject: [PATCH 3/3] Fix changelog --- CHANGELOG.next.asciidoc | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 0c926aa5c989..87b46bdb7891 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -51,8 +51,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix `setup.dashboards.index` setting not working. {pull}17749[17749] - Fix Elasticsearch license endpoint URL referenced in error message. {issue}17880[17880] {pull}18030[18030] - Change `decode_json_fields` processor, to merge parsed json objects with existing objects in the event instead of fully replacing them. {pull}17958[17958] -- 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*