add_process_metadata processor adds container id even if process metadata not accessible by jtinkus · Pull Request #19767 · elastic/beats

jtinkus · 2020-07-09T07:52:52Z

Enhancement

What does this PR do?

Changed add_process_metadata processor to get both process metadata and container id and not error out if only one is available.

Why is it important?

If container is non-privileged, then process metadata for external processes is not fully readable (no access to /proc/pid/exe and /proc/pid/cwd) and code errors out before even trying to get container id. Same time container id is still accessible in /proc/pid/cgroup file. Now process metadata is skipped for such processes, but container id is still added.

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.

Minor fix, probably no need for doc or changelog.

Author's Checklist

[ ]

How to test this PR locally

Run AuditBeat in non-privileged container in k8s cluster.

Configure add_process_metadata processor in yaml file, for example:

- add_process_metadata:
     match_pids: [process.ppid]
     target: process.parent
     cgroup_regex: \/.+\/.+\/.+\/([0-9a-f]{64}).*

This should add process.parent.container.id field to event even if other process metadata for given ppid is not accessible due non-privileged container rights. In such case there is no other process metadata added except container.id.

Related issues

Use cases

Screenshots

Logs

elasticmachine · 2020-07-09T07:52:54Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2020-07-09T07:52:54Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2020-07-09T07:56:50Z

💚 Build Succeeded

Expand to view the summary

Build stats

Build Cause: [ChrsMark commented: jenkins run the tests please]
Start Time: 2020-08-03T12:53:27.196+0000
Duration: 66 min 17 sec

Test stats 🧪

Test	Results
Failed	0
Passed	14469
Skipped	1373
Total	15842

jtinkus · 2020-07-09T07:56:51Z

@exekias proposal for minor fix in the code you have reviewed before.

elasticmachine · 2020-07-09T12:45:55Z

Pinging @elastic/integrations-platforms (Team:Platforms)

exekias

Thank you so much for contributing 🎉 ! I left a question, could you also please add a CHANGELOG entry?

@jsoriano This is one of the places where privileged mode was needed, note anymore if this goes in 🙂

libbeat/processors/add_process_metadata/add_process_metadata.go

ChrsMark

LGTM, extra thanks for the unit tests!

ChrsMark · 2020-08-03T12:53:03Z

jenkins run the tests please

jtinkus · 2020-08-04T06:24:09Z

Seems that PR is now waiting approval from @exekias

ChrsMark · 2020-08-04T07:08:43Z

@exekias will be out for some days so I'm merging this one to avoid waiting.
Thank you @jtinkus for contributing this! Could you also share your manual testing process on the description of this PR (How to test this PR locally)?

…data not accessible (elastic#19767) (cherry picked from commit 99191e9)

…ner id even if process metadata not accessible (#20417) * add_process_metadata processor adds container id even if process metadata not accessible (#19767) (cherry picked from commit 99191e9) * Update CHANGELOG.next.asciidoc Co-authored-by: jtinkus <35308202+jtinkus@users.noreply.github.com>

…ne-2.0 * upstream/master: [docs] Promote ingest management to beta (elastic#20295) Upgrade elasticsearch client library used in tests (elastic#20405) Disable logging when pulling on python integration tests (elastic#20397) Remove pillow from testing requirements.txt (elastic#20407) [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440) [Ingest Manager] Send datastreams fields (elastic#20402) Add event.ingested to all Filebeat modules (elastic#20386) [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426) Improve cgroup_regex docs with examples (elastic#20425) Makes `metrics` config option required in app_insights (elastic#20406) Ensure install scripts only install if needed (elastic#20349) Update container name for the azure filesets (elastic#19899) Group same timestamp metrics values in app_insights metricset (elastic#20403) add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767) Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547) [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396) Update Suricata dashboards (elastic#20394) [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359) Prepare home directories for docker images in a different stage (elastic#20356)

…allation * upstream/master: (23 commits) [docs] Promote ingest management to beta (elastic#20295) Upgrade elasticsearch client library used in tests (elastic#20405) Disable logging when pulling on python integration tests (elastic#20397) Remove pillow from testing requirements.txt (elastic#20407) [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440) [Ingest Manager] Send datastreams fields (elastic#20402) Add event.ingested to all Filebeat modules (elastic#20386) [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426) Improve cgroup_regex docs with examples (elastic#20425) Makes `metrics` config option required in app_insights (elastic#20406) Ensure install scripts only install if needed (elastic#20349) Update container name for the azure filesets (elastic#19899) Group same timestamp metrics values in app_insights metricset (elastic#20403) add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767) Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547) [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396) Update Suricata dashboards (elastic#20394) [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359) Prepare home directories for docker images in a different stage (elastic#20356) New multiline mode in Filebeat: while_pattern (elastic#19662) ...

…data not accessible (elastic#19767)

Jako Tinkus added 4 commits June 30, 2020 16:51

Initial container id fix

80b1927

Merge remote-tracking branch 'upstream/master' into jako_dev

16680ff

Polished code and added tests

b1ba706

Fix typo

84d7cad

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 9, 2020

exekias added enhancement review Team:Platforms Label for the Integrations - Platforms team labels Jul 9, 2020

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 9, 2020

exekias suggested changes Jul 9, 2020

View reviewed changes

libbeat/processors/add_process_metadata/add_process_metadata.go Show resolved Hide resolved

andresrc added [zube]: Inbox [zube]: In Review and removed [zube]: Inbox labels Jul 13, 2020

Jako Tinkus added 2 commits August 3, 2020 13:49

Merge remote-tracking branch 'upstream/master' into jako_dev

3b81af7

Update changelog

fc50aa2

jtinkus requested a review from exekias August 3, 2020 12:19

ChrsMark self-requested a review August 3, 2020 12:46

ChrsMark approved these changes Aug 3, 2020

View reviewed changes

ChrsMark added test-plan Add this PR to be manual test plan and removed test-plan Add this PR to be manual test plan labels Aug 4, 2020

ChrsMark merged commit 99191e9 into elastic:master Aug 4, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Aug 4, 2020

ChrsMark mentioned this pull request Aug 4, 2020

Cherry-pick #19767 to 7.x: add_process_metadata processor adds container id even if process metadata not accessible #20417

Merged

3 tasks

ChrsMark pushed a commit to ChrsMark/beats that referenced this pull request Aug 4, 2020

add_process_metadata processor adds container id even if process meta…

6bd6054

…data not accessible (elastic#19767) (cherry picked from commit 99191e9)

ChrsMark added the v7.10.0 label Aug 4, 2020

andresrc added the test-plan-added This PR has been added to the test plan label Oct 3, 2020

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020

add_process_metadata processor adds container id even if process meta…

183d020

…data not accessible (elastic#19767)

zube bot removed the [zube]: Done label Nov 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add_process_metadata processor adds container id even if process metadata not accessible#19767

add_process_metadata processor adds container id even if process metadata not accessible#19767
ChrsMark merged 6 commits intoelastic:masterfrom
jtinkus:jako_dev

jtinkus commented Jul 9, 2020 •

edited by zube bot

Loading

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Uh oh!

jtinkus commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

exekias left a comment

Uh oh!

Uh oh!

ChrsMark left a comment

Uh oh!

ChrsMark commented Aug 3, 2020

Uh oh!

jtinkus commented Aug 4, 2020

Uh oh!

ChrsMark commented Aug 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jtinkus commented Jul 9, 2020 • edited by zube bot 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

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 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!

jtinkus commented Jul 9, 2020

Uh oh!

elasticmachine commented Jul 9, 2020

Uh oh!

exekias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Aug 3, 2020

Uh oh!

jtinkus commented Aug 4, 2020

Uh oh!

ChrsMark commented Aug 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jtinkus commented Jul 9, 2020 •

edited by zube bot

Loading

elasticmachine commented Jul 9, 2020 •

edited by jenkins-beats-ci bot

Loading