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.

Sign up for GitHub

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

Jump to bottom

Eliminate panic due to nil pointer dereference in Journalbeat #20095

Merged
merged 1 commit into from
Jul 22, 2020
Merged

Eliminate panic due to nil pointer dereference in Journalbeat #20095

merged 1 commit into from
Jul 22, 2020

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jul 21, 2020 •
edited
Loading

What does this PR do?

  • Fix nil pointer dereference in Journalbeat

Why is it important?

The current 7.9 snapshot panics with the default configuration.

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.

Related issues

Closes #20089

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 21, 2020
@kvch kvch added [zube]: In Review needs_backport PR is waiting to be backported to other branches. Team:Services (Deprecated) Label for the former Integrations-Services team and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jul 21, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@kvch kvch requested a review from urso July 21, 2020 12:28
urso
urso reviewed Jul 21, 2020
View reviewed changes
libbeat/tests/system/beat/beat.py Outdated
with open(os.path.join(self.working_dir, output_file, ), "r", encoding="utf_8") as f:
return len([1 for line in f]) == 0
except IOError:
return True
Copy link

Choose a reason for hiding this comment

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

can't we use self.output_has(lines=0)?

The output_has should check for None when testing if lines is set, so we can correctly use 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

output_has returns False if the output does not exist. I added the extra function to avoid getting an exception in this case.

Copy link

Choose a reason for hiding this comment

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

this makes me wonder if output_has should really raise an exception. From the test code POV we only want to know what has been 'published' (or if something has been published). The fact that the file does not exist or is empty is just a detail the test code should not care about.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 21, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20095 updated]

  • Start Time: 2020-07-22T09:47:50.329+0000

  • Duration: 25 min 44 sec

Test stats 🧪

Test Results
Failed 0
Passed 31
Skipped 4
Total 35

@kvch kvch force-pushed the fix-journalbeat-reader-and-tests branch from 17653ef to 5affccf Compare July 21, 2020 13:37
@kvch kvch requested a review from a team as a code owner July 21, 2020 13:37
@botelastic botelastic bot added the Team:Automation Label for the Observability productivity team label Jul 21, 2020
@kvch kvch force-pushed the fix-journalbeat-reader-and-tests branch from c1ecbe7 to caabcda Compare July 22, 2020 09:47
@kvch kvch merged commit 54b1e64 into elastic:master Jul 22, 2020
kvch added a commit to kvch/beats that referenced this pull request Jul 22, 2020
@kvch
Eliminate panic due to nil pointer dereference (elastic#20095)
aebe82c 
(cherry picked from commit 54b1e64)
@kvch kvch mentioned this pull request Jul 22, 2020
Merged
3 tasks
@kvch kvch added v7.10.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jul 22, 2020
kvch added a commit to kvch/beats that referenced this pull request Jul 22, 2020
@kvch
Eliminate panic due to nil pointer dereference (elastic#20095)
1899134 
(cherry picked from commit 54b1e64)
@kvch kvch mentioned this pull request Jul 22, 2020
Merged
3 tasks
@kvch kvch added the v7.9.0 label Jul 22, 2020
@kvch
Copy link
Contributor Author

kvch commented Jul 22, 2020

Fixing the testing of Journalbeat will be done in a follow-up PR.

kvch added a commit that referenced this pull request Jul 22, 2020
@kvch
Eliminate panic due to nil pointer dereference (#20095) (#20132)
ad1371c 
(cherry picked from commit 54b1e64)
kvch added a commit that referenced this pull request Jul 22, 2020
@kvch
Eliminate panic due to nil pointer dereference (#20095) (#20133)
bf8bc0f 
(cherry picked from commit 54b1e64)
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@kvch @melchiormoulin
Eliminate panic due to nil pointer dereference (elastic#20095)
0973bbd
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@kvch
Eliminate panic due to nil pointer dereference (elastic#20095) (elast…
36d28ba 
…ic#20133)

(cherry picked from commit 5445670)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@urso urso urso left review comments

@v1v v1v v1v approved these changes

Assignees
No one assigned
Labels
Team:Automation Label for the Observability productivity team Team:Services (Deprecated) Label for the former Integrations-Services team v7.9.0 v7.10.0 [zube]: In Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Journalbeat 7.9.0-SNAPSHOT panics on start
4 participants
@kvch @elasticmachine @urso @v1v