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

[Auditbeat][System] Fix error handling around go-sysinfo.Host #17569

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

adriansr
Copy link
Contributor

@adriansr adriansr commented Apr 7, 2020

Before a system dataset is loaded, host information is loaded to populate entity_id fields.

Error handling around it was incorrect and resulted in a panic when fetching host info failed, which afaik it's limited to errors accessing /proc/stat file.

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.
@adriansr adriansr added bug review needs_backport PR is waiting to be backported to other branches. Auditbeat Team:SIEM labels Apr 7, 2020
@adriansr adriansr requested a review from a team as a code owner April 7, 2020 11:23
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@@ -55,19 +55,20 @@ func NewModule(base mb.BaseModule) (mb.Module, error) {
log := logp.NewLogger(moduleName)

var hostID string
hostInfo, err := sysinfo.Host()
if hostInfo != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What was going on in here is when sysinfo.Host() errors, it returns a non-nil interface that wraps a nil pointer, causing the nil check to pass but a panic inside the hostInfo.Info() call below.

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

Code LGTM

Does it need a Changelog entry?

@adriansr adriansr merged commit 97750c8 into elastic:master Apr 7, 2020
adriansr added a commit to adriansr/beats that referenced this pull request Apr 8, 2020
…c#17569)

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.

(cherry picked from commit 97750c8)
@adriansr adriansr added v7.8.0 and removed needs_backport PR is waiting to be backported to other branches. labels Apr 8, 2020
adriansr added a commit to adriansr/beats that referenced this pull request Apr 8, 2020
…c#17569)

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.

(cherry picked from commit 97750c8)
@adriansr adriansr added the v7.7.0 label Apr 8, 2020
adriansr added a commit that referenced this pull request Apr 9, 2020
#17602)

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.

(cherry picked from commit 97750c8)
adriansr added a commit that referenced this pull request Apr 9, 2020
#17603)

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.

(cherry picked from commit 97750c8)
@adriansr adriansr deleted the fix_system_hostinfo_err branch May 15, 2020 12:56
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…c#17569) (elastic#17603)

Before a system dataset is loaded, host information is loaded to
populate `entity_id` fields.

Error handling around it was incorrect and resulted in a panic when
fetching host info failed, which afaik it's limited to errors accessing
/proc/stat file.

(cherry picked from commit 7602e79)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants