[Filebeat] backwards compatibility for set processor by leehinman · Pull Request #20908 · elastic/beats

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

 - 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 merged 3 commits intoelastic:masterfrom
leehinman:set_processor_fix

leehinman commented Sep 1, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Sep 1, 2020

Uh oh!

elasticmachine commented Sep 1, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Uh oh!

andrewkroh left a comment

Uh oh!

andrewkroh Sep 1, 2020

Uh oh!

andrewkroh Sep 1, 2020

Uh oh!

andrewkroh Sep 1, 2020

Uh oh!

andrewkroh Sep 1, 2020

Uh oh!

andrewkroh left a comment

Uh oh!

leehinman commented Sep 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	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]

Conversation

leehinman commented Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Logs

Error

With fix:

Uh oh!

elasticmachine commented Sep 1, 2020

Uh oh!

elasticmachine commented Sep 1, 2020 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

andrewkroh Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

leehinman commented Sep 3, 2020

7.8.1 cluster

setup output

Resulting set processor

7.9.0 cluster

setup output

Resulting set processor

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

leehinman commented Sep 1, 2020 •

edited

Loading

elasticmachine commented Sep 1, 2020 •

edited by jenkins-beats-ci bot

Loading