[Filebeat] backwards compatibility for set processor #20908

leehinman · 2020-09-01T20:42:08Z

What does this PR do?

When loading a pipeline this change checks the elasticsearch version
and if the version is less than 7.9.0 it will replace the
"ignore_empty_value" option with an equivalent if statement on the set
processor.

Why is it important?

This allows filebeat > 7.9.0 to be used with older versions of
elasticsearch. Without it the pipelines fail to load because the
option isn't supported.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

run filebeat modules enable zeek && filebeat -e setup

Logs

Error

2020-09-01T12:42:48.781-0500	ERROR	fileset/setup.go:81	Error loading pipeline: 1 error: Error loading pipeline for fileset zeek/x509: couldn't load pipeline: couldn't load json. Error: 400 Bad Request: {"error":{"root_cause [{"type":"parse_exception","reason":"processor [set] doesn't support one or more provided configuration parameters [ignore_empty_value]",
...

With fix:

2020-09-01T14:04:56.470-0500	DEBUG	[modules] fileset/pipelines.go:277     	in pipeline filebeat-8.0.0-zeek-x509-pipeline replacing unsupported 'ignore_empty_value' with if ctx?.zeek?.x509?.certificate?.subject?.state != null in set processor

elasticmachine · 2020-09-01T20:42:10Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2020-09-01T21:36:46Z

💚 Build Succeeded

Expand to view the summary

Build stats

Build Cause: [Pull request #20908 updated]
Start Time: 2020-09-02T21:28:49.153+0000
Duration: 55 min 17 sec

Test stats 🧪

Test	Results
Failed	0
Passed	5586
Skipped	822
Total	6408

- "ignore_empty_value" option for the set processor only works on elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch.

andrewkroh

Just a few minor suggestions.

andrewkroh · 2020-09-01T22:30:45Z

filebeat/fileset/pipelines.go

+			if !ok {
+				continue
+			}
+			newIf := strings.ReplaceAll(val, "{", "")


How about TrimLeft and TrimRight for these two?

andrewkroh · 2020-09-01T22:31:55Z

filebeat/fileset/pipelines.go

+			newIf = strings.TrimSpace(newIf)
+			newIf = strings.ReplaceAll(newIf, ".", "?.")
+			newIf = "ctx?." + newIf + " != null"
+			logp.Debug("modules", "in pipeline %s replacing unsupported 'ignore_empty_value' with if %s in set processor", pipelineID, newIf)


Suggested change

logp.Debug("modules", "in pipeline %s replacing unsupported 'ignore_empty_value' with if %s in set processor", pipelineID, newIf)

logp.Debug("modules", "In pipeline %s replacing unsupported 'ignore_empty_value' with if %q in set processor", pipelineID, newIf)

andrewkroh · 2020-09-01T22:34:56Z

filebeat/fileset/pipelines_test.go

+						"set": map[string]interface{}{
+							"field":              "rule.name",
+							"value":              "{{panw.panos.ruleset}}",
+							"ignore_empty_value": true,


How about a test case for when the pipeline already has an if and it has ignore_empty_value. Perhaps it should just drop the ignore_empty_value in that case?

andrewkroh · 2020-09-01T22:36:06Z

CHANGELOG.next.asciidoc

@@ -256,6 +256,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix long registry migration times. {pull}20717[20717] {issue}20705[20705]
 - Fix event types and categories in auditd module to comply with ECS {pull}20652[20652]
 - Update documentation in the azure module filebeat. {pull}20815[20815]
+- provide backwards compatibility for set processor and elasticsearch less than 7.9.0 {pull}20908[20908]


Suggested change

- provide backwards compatibility for set processor and elasticsearch less than 7.9.0 {pull}20908[20908]

- Provide backwards compatibility for the `set` processor when Elasticsearch is less than 7.9.0. {pull}20908[20908]

andrewkroh

LGTM. Can you try testing the modules locally against 7.8 if you haven't already.

I think if you export TESTING_ENVIRONMENT=latest and roll back to 7.8 in https://github.com/elastic/beats/blob/master/testing/environments/latest.yml#L6-L29 while running the module tests it will use 7.8.

leehinman · 2020-09-03T16:40:35Z

LGTM. Can you try testing the modules locally against 7.8 if you haven't already.

I think if you export TESTING_ENVIRONMENT=latest and roll back to 7.8 in https://github.com/elastic/beats/blob/master/testing/environments/latest.yml#L6-L29 while running the module tests it will use 7.8.

All tests passed with TESTING_ENVIRONMENT=latest & setting version to 7.8.0 in latest.yml

Also tried loading the zeek module against 7.8.1 & 7.9.0 clusters:

7.8.1 cluster

setup output

2020-09-03T10:00:54.917-0500	DEBUG	[modules]	fileset/pipelines.go:270	In pipeline "filebeat-8.0.0-zeek-x509-pipeline" removing unsupported 'ignore_empty_value' in set processor
2020-09-03T10:00:54.917-0500	DEBUG	[modules]	fileset/pipelines.go:288	In pipeline "filebeat-8.0.0-zeek-x509-pipeline" adding if ctx?.zeek?.x509?.certificate?.subject?.common_name != null to replace 'ignore_empty_value' in set processor
2020-09-03T10:00:55.003-0500	INFO	fileset/pipelines.go:139	Elasticsearch pipeline with ID 'filebeat-8.0.0-zeek-x509-pipeline' loaded

Resulting set processor

{
        "set" : {
          "value" : "{{zeek.x509.certificate.subject.common_name}}",
          "if" : "ctx?.zeek?.x509?.certificate?.subject?.common_name != null",
          "field" : "file.x509.subject.common_name"
        }
},

7.9.0 cluster

setup output

2020-09-03T10:21:33.594-0500	INFO	fileset/pipelines.go:139	Elasticsearch pipeline with ID 'filebeat-8.0.0-zeek-x509-pipeline' loaded

Resulting set processor

{
        "set" : {
          "ignore_empty_value" : true,
          "field" : "file.x509.subject.common_name",
          "value" : "{{zeek.x509.certificate.subject.common_name}}"
        }
},

- "ignore_empty_value" option for the set processor only works on Elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch. (cherry picked from commit 0032c0c)

- "ignore_empty_value" option for the set processor only works on Elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch. (cherry picked from commit 0032c0c) Co-authored-by: Lee Hinman <57081003+leehinman@users.noreply.github.com>

- "ignore_empty_value" option for the set processor only works on Elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch.

…lastic#21084) - "ignore_empty_value" option for the set processor only works on Elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch. (cherry picked from commit b6162c4) Co-authored-by: Lee Hinman <57081003+leehinman@users.noreply.github.com>

leehinman added bug Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. Team:SIEM labels Sep 1, 2020

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 1, 2020

leehinman force-pushed the set_processor_fix branch from b72b450 to 290c0ea Compare September 1, 2020 20:43

backwards compatibility for set processor

84cc638

- "ignore_empty_value" option for the set processor only works on elasticsearch >= 7.9.0. This change removes that option and replaces it with an if statement if pipeline is loaded on an earlier version of elasticsearch.

leehinman force-pushed the set_processor_fix branch from 290c0ea to 84cc638 Compare September 1, 2020 22:17

andrewkroh reviewed Sep 1, 2020

View reviewed changes

leehinman added 2 commits September 2, 2020 16:24

incorporate feedback

0233ae7

update changelog

0799f01

andrewkroh approved these changes Sep 2, 2020

View reviewed changes

leehinman merged commit 0032c0c into elastic:master Sep 3, 2020

leehinman mentioned this pull request Sep 3, 2020

Cherry-pick #20908 to 7.x: [Filebeat] backwards compatibility for set processor #20969

Merged

6 tasks

leehinman added v7.10.0 and removed needs_backport PR is waiting to be backported to other branches. labels Sep 3, 2020

andrewkroh mentioned this pull request Sep 14, 2020

Cherry-pick #20908 to 7.9: [Filebeat] backwards compatibility for set processor #21084

Merged

6 tasks

andrewkroh added the v7.9.2 label Sep 14, 2020

leehinman deleted the set_processor_fix branch October 5, 2020 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Filebeat] backwards compatibility for set processor #20908

[Filebeat] backwards compatibility for set processor #20908

leehinman commented Sep 1, 2020 •

edited

Loading

elasticmachine commented Sep 1, 2020

elasticmachine commented Sep 1, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

andrewkroh left a comment

andrewkroh Sep 1, 2020

andrewkroh Sep 1, 2020

andrewkroh Sep 1, 2020

andrewkroh Sep 1, 2020

andrewkroh left a comment

leehinman commented Sep 3, 2020

	logp.Debug("modules", "in pipeline %s replacing unsupported 'ignore_empty_value' with if %s in set processor", pipelineID, newIf)
	logp.Debug("modules", "In pipeline %s replacing unsupported 'ignore_empty_value' with if %q in set processor", pipelineID, newIf)

	- provide backwards compatibility for set processor and elasticsearch less than 7.9.0 {pull}20908[20908]
	- Provide backwards compatibility for the `set` processor when Elasticsearch is less than 7.9.0. {pull}20908[20908]

[Filebeat] backwards compatibility for set processor #20908

[Filebeat] backwards compatibility for set processor #20908

Conversation

leehinman commented Sep 1, 2020 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Logs

Error

With fix:

elasticmachine commented Sep 1, 2020

elasticmachine commented Sep 1, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

leehinman commented Sep 3, 2020

7.8.1 cluster

setup output

Resulting set processor

7.9.0 cluster

setup output

Resulting set processor

leehinman commented Sep 1, 2020 •

edited

Loading

elasticmachine commented Sep 1, 2020 •

edited by jenkins-beats-ci bot

Loading