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

Fix rfc5464 date parsing in the syslog input #26419

Merged
merged 5 commits into from
Jun 23, 2021
Merged

Conversation

faec
Copy link
Contributor

@faec faec commented Jun 22, 2021

What does this PR do?

Fixes a reported issue where the Syslog input misparses days of the month that begin with a 0.

Added a test exhibiting the bug, updated the ragel spec, reran go generate, and confirmed that the test now passes.

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.

@faec faec added bug Team:Elastic-Agent Label for the Agent team backport-v7.14.0 Automated backport with mergify labels Jun 22, 2021
@faec faec self-assigned this Jun 22, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jun 22, 2021
@mergify
Copy link
Contributor

mergify bot commented Jun 22, 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 syslog-date upstream/syslog-date
git merge upstream/master
git push upstream syslog-date

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 22, 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 #26419 updated

  • Start Time: 2021-06-23T15:47:45.000+0000

  • Duration: 120 min 42 sec

  • Commit: 3528650

Test stats 🧪

Test Results
Failed 0
Passed 14125
Skipped 2311
Total 16436

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 14125
Skipped 2311
Total 16436

@faec faec requested a review from jsoriano June 22, 2021 21:38
st7:
if (p)++; (p) == (pe) {
if ( p)++; ( p) == ( pe) {
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, why is ragel adding these spaces? 🤔

Copy link
Member

@jsoriano jsoriano Jun 23, 2021

Choose a reason for hiding this comment

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

Looks like linters don't like them.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is interesting, I am not sure why Ragel would add it, I think

@jsoriano
Copy link
Member

make update doesn't work but make check does?

@faec what version of Go do you have locally? Check these issues #26419 #26186.

@faec
Copy link
Contributor Author

faec commented Jun 23, 2021

@faec what version of Go do you have locally? Check these issues #26419 #26186.

I checked the go version, it's 1.16.5 as it should be. The problem is in the build somewhere: make update didn't run gofmt on format_check.go so the linter failed on the whitespace. But make check did run it, and left the corrected file in place, so I was able to commit that.

test := []testRule{
{
title: "Test two-digit mdays",
log: []byte(`<165>1 2003-08-07T05:14:15.000003-07:00 192.0.2.1 myproc 8710 - - %% It's time to make the do-nuts.`),
Copy link
Contributor

Choose a reason for hiding this comment

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

This test made me smile :)

@faec faec merged commit 0ae157e into elastic:master Jun 23, 2021
@faec faec deleted the syslog-date branch June 23, 2021 18:13
mergify bot pushed a commit that referenced this pull request Jun 23, 2021
faec added a commit that referenced this pull request Jun 24, 2021
(cherry picked from commit 0ae157e)

Co-authored-by: Fae Charlton <[email protected]>
mdelapenya added a commit to mdelapenya/beats that referenced this pull request Jun 28, 2021
* master: (32 commits)
  [Metricbeat] Change Account ID to Project ID in `gcp.billing` module (elastic#26412)
  update libbeat fields.ecs.yml file and ecsVersion to 1.10.0 (elastic#26121)
  [Filebeat] Update AWS ELB ingest pipeline (elastic#26441)
  [FIlebeat] add strict_date_optional_time_nanos date format to PanOS module (elastic#26158)
  Fix the irregular and typo on prometheus module. (elastic#25726)
  [Filebeat] Parse additonal debug data fields for Okta module (elastic#25818)
  fix: update MSSQL Server linux image's Docker registry (elastic#26440)
  Update indexing.go godocs (elastic#26408)
  Do not close filestream harvester if an unexpected error is returned when close.on_state_change.* is enabled (elastic#26411)
  Add support for copytruncate method when rotating input logs with an external tool in `filestream` input (elastic#23457)
  Allow fields with ip_range datatype (elastic#26444)
  Add Anomali ThreatStream support to threatintel module (elastic#26350)
  fix: use the right param type (elastic#26469)
  [Automation] Update elastic stack version to 8.0.0-7640093f for testing (elastic#26460)
  Set SM Filebeat modules as GA (elastic#26226)
  Fix rfc5464 date parsing in the syslog input (elastic#26419)
  Add linked account information into billing metricset (elastic#26285)
  [Filebeat] Update HA Proxy log grok patterns (elastic#25835)
  disable metricbeat logstash test_node_stats (elastic#26436)
  chore: pass BEAT_VERSION when running E2E tests (elastic#26291)
  ...
@manarth
Copy link

manarth commented Aug 1, 2021

The fix is reported in the release notes for 7.13.3.

The changes don't appear to be in the code tagged as 7.13.3 or 7.13.4.

Perhaps an issue with the build system, where the release notes are including references to code which isn't included in the build?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify bug Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants