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] GCP module enhancement - Populate orchestrator.* fields for K8S logs #25368

Merged
merged 11 commits into from
Jun 2, 2021
Merged

Conversation

TonioRyo
Copy link
Contributor

What does this PR do?

This PR add the ability to populate orchestrator.* fields for GCP K8S logs. The cluster_name field from the original message was not kept in the logs, this field is important to analyze security events. I also tried to populate other orchestrator.* fields with information available in other fields of the original message.

Why is it important?

The cluster_name field from the original message was not kept in the logs, this field is important to analyze security events.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 28, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 28, 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #25368 updated

  • Start Time: 2021-06-01T11:52:52.571+0000

  • Duration: 102 min 53 sec

  • Commit: 9b88e3d

Test stats 🧪

Test Results
Failed 0
Passed 7194
Skipped 1193
Total 8387

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 7194
Skipped 1193
Total 8387

@exekias exekias added the Team:Integrations Label for the Integrations team label Apr 28, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 28, 2021
@TonioRyo
Copy link
Contributor Author

A discussion is opened on discuss.elastic.co regarding this PR: https://discuss.elastic.co/t/gcp-module-add-cluster-name-for-gke-k8s-io-logs/271278

@legoguy1000
Copy link
Contributor

This should also be done for the other modules that can get K8S logs.

@legoguy1000
Copy link
Contributor

legoguy1000 commented Apr 28, 2021

You also need to add sample K8S logs to the x-pack/filebeat/module/gcp/audit/test/audit-log-entries.json.log file and then run the below to update the generated data.

cd beats/x-pack/filebeat
GENERATE=true TESTING_FILEBEAT_MODULES=gcp TESTING_FILEBEAT_FILESETS=audit mage -v pythonIntegTest

@TonioRyo
Copy link
Contributor Author

You also need to add sample K8S logs to the x-pack/filebeat/module/gcp/audit/test/audit-log-entries.json.log file and then run the below to update the generated data.

cd beats/x-pack/filebeat
GENERATE=true TESTING_FILEBEAT_MODULES=gcp TESTING_FILEBEAT_FILESETS=audit mage -v pythonIntegTest

I tried to do that but got an error FAILED tests/system/test_xpack_modules.py::XPackTest::test_fileset_file_0_gcp - Exception: Key 'orchestrator.cluster.name' found in event is not documented!.
I think it's because the orchestrator.* field set is not available in ECS 1.9, it will only be available in ECS 1.10, and FileBeat is not yet build with this ECS version.
Do you have any direction on how I should handle that ?

@TonioRyo
Copy link
Contributor Author

This should also be done for the other modules that can get K8S logs.

Agreed but I do not have access to K8S systems from other providers (Azure Kubernetes Service, Amazon EKS, on premise K8S...) to do that.

@legoguy1000
Copy link
Contributor

You also need to add sample K8S logs to the x-pack/filebeat/module/gcp/audit/test/audit-log-entries.json.log file and then run the below to update the generated data.

cd beats/x-pack/filebeat
GENERATE=true TESTING_FILEBEAT_MODULES=gcp TESTING_FILEBEAT_FILESETS=audit mage -v pythonIntegTest

I tried to do that but got an error FAILED tests/system/test_xpack_modules.py::XPackTest::test_fileset_file_0_gcp - Exception: Key 'orchestrator.cluster.name' found in event is not documented!.
I think it's because the orchestrator.* field set is not available in ECS 1.9, it will only be available in ECS 1.10, and FileBeat is not yet build with this ECS version.
Do you have any direction on how I should handle that ?

Thats a good question. for the development team. I've never tried to use fields in a future ECS spec version.

@TonioRyo TonioRyo marked this pull request as ready for review April 29, 2021 14:05
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine
Copy link
Collaborator

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

@mergify
Copy link
Contributor

mergify bot commented Apr 29, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b master upstream/master
git merge upstream/master
git push upstream master

@sorantis
Copy link
Contributor

sorantis commented May 3, 2021

cc @kaiyan-sheng, @jsoriano, @exekias. Looks like we need to align with the Inventory schema work here.

@kaiyan-sheng
Copy link
Contributor

@sorantis Thanks for pinging here! orchestrator.* fields will be published in ECS 1.10 and the three fields we defined for orchestrator in inventory schema already matches the ones in ECS.

@andrewkroh andrewkroh added needs_integration_sync Changes in this PR need synced to elastic/integrations. and removed Team:Security-External Integrations labels May 3, 2021
@andrewkroh
Copy link
Member

This change will also need applied to elastic/integrations.

@ChrsMark
Copy link
Member

ChrsMark commented May 6, 2021

Thanks for your contribution @TonioRyo! Regarding the ECS version how about trying to update it at

?

@kaiyan-sheng @andrewkroh this change looks good to me regarding the orchestrator.* fields that it adds. I'm ok with merging this when golden files are updated too. We will need to open a separate issue for elastic/integrations too.

@mergify
Copy link
Contributor

mergify bot commented May 25, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b master upstream/master
git merge upstream/master
git push upstream master

@P1llus
Copy link
Member

P1llus commented Jun 1, 2021

@TonioRyo Would you be able to merge with master because I added the ECS fields to the beats definition today. If you could also bump the ECS version to 1.10.0 that would be great!

@TonioRyo
Copy link
Contributor Author

TonioRyo commented Jun 1, 2021

Thanks @P1llus, I was able to generate the expected results based on sample logs thanks to your work!

@P1llus P1llus removed the needs_integration_sync Changes in this PR need synced to elastic/integrations. label Jun 1, 2021
@P1llus
Copy link
Member

P1llus commented Jun 1, 2021

Removing integration sync as I added the same changes to packages here: elastic/integrations#1045

@P1llus
Copy link
Member

P1llus commented Jun 1, 2021

Awesome @TonioRyo . Il check to see if someone can merge it then!

@P1llus
Copy link
Member

P1llus commented Jun 1, 2021

jenkins run tests

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@kaiyan-sheng kaiyan-sheng added needs_backport PR is waiting to be backported to other branches. needs_integration_sync Changes in this PR need synced to elastic/integrations. labels Jun 1, 2021
@P1llus P1llus removed the needs_integration_sync Changes in this PR need synced to elastic/integrations. label Jun 1, 2021
@ChrsMark ChrsMark merged commit 2d04bf7 into elastic:master Jun 2, 2021
ChrsMark pushed a commit to ChrsMark/beats that referenced this pull request Jun 2, 2021
@ChrsMark ChrsMark added v7.14.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jun 2, 2021
ChrsMark added a commit that referenced this pull request Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Integrations Label for the Integrations team v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants