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

[filebeat] fix crowdstrike ingest pipeline #27623

Merged
merged 4 commits into from
Sep 3, 2021
Merged

[filebeat] fix crowdstrike ingest pipeline #27623

merged 4 commits into from
Sep 3, 2021

Conversation

leandrojmp
Copy link
Contributor

fix process fields that were being created as flattened fields

What does this PR do?

This fix the process fields that were being created as flatenned fields instead of nested.

Why is it important?

Mixing flattened fields and nested fields with similar name is confusing and could lead to errors when running queries or automated processes that expects the nested fields.

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.

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

fix process fields that were being created as flattened fields
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 27, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 27, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-09-03T04:40:16.531+0000

  • Duration: 96 min 34 sec

  • Commit: 1b277eb

Test stats 🧪

Test Results
Failed 0
Passed 8142
Skipped 1201
Total 9343

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 8142
Skipped 1201
Total 9343

@leandrojmp leandrojmp changed the title fix: fix crowdstrike ingest pipeline fix crowdstrike ingest pipeline Aug 27, 2021
@leandrojmp leandrojmp changed the title fix crowdstrike ingest pipeline [filebeat] fix crowdstrike ingest pipeline Aug 27, 2021
@P1llus
Copy link
Member

P1llus commented Sep 1, 2021

/test

@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 1, 2021
@P1llus P1llus self-requested a review September 1, 2021 07:46
@P1llus P1llus self-assigned this Sep 1, 2021
@P1llus
Copy link
Member

P1llus commented Sep 1, 2021

Thanks for your contribution @leandrojmp , would you be able to perform a few small additions to your PR? Specifically:

Please modify the CHANGELOG.NEXT, there should be a section called Bugfixes, and then Filebeat, and just link back to this PR.
Second would be to rerun our test data, which takes raw crowdstrike data and runs the ingest pipelines against it to test.
To test this locally, you need to:

  1. Spin up a local elasticsearch instance:
    docker run -d -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" docker.elastic.co/elasticsearch/elasticsearch:7.14.0
  2. In the ./beats/x-pack/filebeat folder, you can run:
    PYTEST_ADDOPTS="-k test_xpack_modules" GENERATE=1 TESTING_FILEBEAT_MODULES=crowdstrike mage pythonIntegTest

If you are getting issues or are unable to perform the rerun of the test data, please let me know and I can update the PR with an extra commit :)

added line about the pr in the changelog.next file
corrected the links to the issue and pr in changelog
@leandrojmp
Copy link
Contributor Author

Hello @P1llus,

I've changed the CHANGELOG.nex.asciidoc file and ran the tests, the tests were completed without any issue, do I need to do anything else?

@P1llus
Copy link
Member

P1llus commented Sep 1, 2021

Hello @P1llus,

I've changed the CHANGELOG.nex.asciidoc file and ran the tests, the tests were completed without any issue, do I need to do anything else?

Did it also create any changes to a expected.json file? If you could attach that as well

@leandrojmp
Copy link
Contributor Author

leandrojmp commented Sep 1, 2021

Did it also create any changes to a expected.json file? If you could attach that as well

No .json files were created.

In the file falcon-events.log-expected.json, all the fields are flattened, it seems that the expected output does not make distinctions if a field is flattened or nested, so this PR would not change it, I think.

eg:

"process.args": [
            "C:\\Windows\\Explorer.EXE"
        ],
"process.command_line": "C:\\Windows\\Explorer.EXE",
"process.executable": "C:\\Windows\\Explorer.EXE",
"process.name": "explorer.exe",
"process.pid": 38684386611,

Fixed capitalization.
@P1llus
Copy link
Member

P1llus commented Sep 1, 2021

Did it also create any changes to a expected.json file? If you could attach that as well

No .json files were created.

In the file falcon-events.log-expected.json, all the fields are flattened, it seems that the expected output does not make distinctions if a field is flattened or nested, so this PR would not change it, I think.

eg:

"process.args": [
            "C:\\Windows\\Explorer.EXE"
        ],
"process.command_line": "C:\\Windows\\Explorer.EXE",
"process.executable": "C:\\Windows\\Explorer.EXE",
"process.name": "explorer.exe",
"process.pid": 38684386611,

You are correct it seems, so it would not change much in the test output, looking at the output in our elastic-agent integration, there seems that one or more fields are indeed flattened by the script you changed:
https://github.com/elastic/integrations/blob/master/packages/crowdstrike/data_stream/falcon/_dev/test/pipeline/test-falcon-events.log-expected.json#L10

Did you test it yourself locally to confirm it's working or?

@leandrojmp
Copy link
Contributor Author

leandrojmp commented Sep 1, 2021

Did you test it yourself locally to confirm it's working or?

Yes, I'm running in production, this is my current output:

"process" : {
  "args" : [
    "\"C:\\Program",
    "Files",
    "(x86)\\Pritunl\\post_install.exe\""
  ],
  "parent" : {
    "executable" : """\Device\HarddiskVolume3\Users\infra\AppData\Local\Temp\is-V09UA.tmp\Pritunl.tmp""",
    "command_line" : "\"C:\\Users\\infra\\AppData\\Local\\Temp\\is-V09UA.tmp\\Pritunl.tmp\" /SL5=\"$20EF0,74804998,790528,C:\\Users\\REDACTED\\Downloads\\Pritunl.exe\" /SPAWNWND=$20E7A /NOTIFYWND=$9C0CA2 "
  },
  "name" : "post_install.exe",
  "pid" : 581697116246,
  "command_line" : "\"C:\\Program Files (x86)\\Pritunl\\post_install.exe\"",
  "executable" : "\"C:\\Program"
}

I've made the changes in the ingest pipeline located here: https://github.com/elastic/beats/tree/master/x-pack/filebeat/module/crowdstrike/falcon/ingest, the pipelines.yml file.

The output from that integration seems to be running a different ingest pipeline.

For what I understood, this integration is part of the elastic agent, I'm running Filebeat.

@P1llus
Copy link
Member

P1llus commented Sep 3, 2021

Ah yeah I was just comparing it with elastic-agent, which starts with same or similar pipelines in most cases, since the expected.json generated there is not nested, so it was easier to spot the nested fields happening there.

I will go ahead and start the tests again then, and see if the build passes :)

@P1llus
Copy link
Member

P1llus commented Sep 3, 2021

/test

@P1llus P1llus merged commit 825bfb2 into elastic:master Sep 3, 2021
@P1llus P1llus added backport-v7.16.0 Automated backport with mergify needs_integration_sync Changes in this PR need synced to elastic/integrations. labels Sep 3, 2021
mergify bot pushed a commit that referenced this pull request Sep 3, 2021
* fix: fix crowdstrike ingest pipeline

fix process fields that were being created as flattened fields

* docs: add line to changelog.next

added line about the pr in the changelog.next file

* docs: add links in changelog

corrected the links to the issue and pr in changelog

* Update CHANGELOG.next.asciidoc

Fixed capitalization.

(cherry picked from commit 825bfb2)
@P1llus
Copy link
Member

P1llus commented Sep 3, 2021

Thanks for the contribution @leandrojmp :) It has now been merged and will be automatically backported to the next available version (7.16)

@leandrojmp
Copy link
Contributor Author

Thanks! I will wait for the merge to update my stack.

@leandrojmp leandrojmp deleted the fix/crowdstrike-ingest-pipeline branch September 3, 2021 13:03
P1llus added a commit that referenced this pull request Oct 25, 2021
* fix: fix crowdstrike ingest pipeline

fix process fields that were being created as flattened fields

* docs: add line to changelog.next

added line about the pr in the changelog.next file

* docs: add links in changelog

corrected the links to the issue and pr in changelog

* Update CHANGELOG.next.asciidoc

Fixed capitalization.

(cherry picked from commit 825bfb2)

Co-authored-by: Leandro Maciel <[email protected]>
Co-authored-by: Marius Iversen <[email protected]>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
* fix: fix crowdstrike ingest pipeline

fix process fields that were being created as flattened fields

* docs: add line to changelog.next

added line about the pr in the changelog.next file

* docs: add links in changelog

corrected the links to the issue and pr in changelog

* Update CHANGELOG.next.asciidoc

Fixed capitalization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify needs_integration_sync Changes in this PR need synced to elastic/integrations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat] Crowdstrike ingest pipeline wrongly creates process flattened fields.
3 participants