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

[packetbeat] Fix interface device parsing for packetbeat protocols #37946

Merged
merged 9 commits into from
Feb 15, 2024
Merged

[packetbeat] Fix interface device parsing for packetbeat protocols #37946

merged 9 commits into from
Feb 15, 2024

Conversation

marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Feb 9, 2024

Proposed commit message

Currently integrations defining interface device name for protocols are failing with an error such as [elastic_agent][error] Unit state changed packet-default-packet-network-533ba19b-0436-4693-b553-7fe84d35872c (STARTING->FAILED): failed to initialize protocol analyzers for foo: can not convert 'object' into 'string' accessing 'interface' due to changes introduced in https://github.com/elastic/beats/pull/36852/files#diff-102cdff9474deb479cd1c099ce54ab013f023692a0783d4bbbf62d2e1fc85b70R174 where interface was assumed to be a string.

This fixes the type of the expected config to the proper object to check device presence.

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.

@marc-gr marc-gr added Packetbeat bugfix backport-v8.12.0 Automated backport with mergify labels Feb 9, 2024
@marc-gr marc-gr requested a review from efd6 February 9, 2024 11:23
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Feb 9, 2024
@mergify mergify bot assigned marc-gr Feb 9, 2024
@marc-gr marc-gr marked this pull request as ready for review February 9, 2024 11:29
@marc-gr marc-gr requested a review from a team as a code owner February 9, 2024 11:29
@marc-gr marc-gr added the Team:Security-Linux Platform Linux Platform Team in Security Solution label Feb 9, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 9, 2024
@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 9, 2024 •
edited
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2024-02-15T12:56:34.494+0000

  • Duration: 47 min 48 sec

Test stats 🧪

Test Results
Failed 0
Passed 2377
Skipped 25
Total 2402

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

efd6
efd6 reviewed Feb 11, 2024
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

This LGTM but it would be nice to have a test get a textual config, parse it and show that this works as expected. This would require that this small part of logic be factored into its own function.

@marc-gr marc-gr requested a review from efd6 February 12, 2024 14:36
efd6
efd6 reviewed Feb 12, 2024
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

nit then good.

packetbeat/protos/protos.go Outdated Show resolved Hide resolved
@marc-gr marc-gr requested a review from efd6 February 13, 2024 08:43
@marc-gr marc-gr added the backport-v8.13.0 Automated backport with mergify label Feb 15, 2024
pkoutsovasilis
pkoutsovasilis approved these changes Feb 15, 2024
View reviewed changes
Copy link
Contributor

@pkoutsovasilis pkoutsovasilis left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @marc-gr

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @marc-gr

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @marc-gr

@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 15, 2024 •
edited
Loading

💔 Build Failed

Failed CI Steps

History

cc @marc-gr

@marc-gr marc-gr enabled auto-merge (squash) February 15, 2024 13:12
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @marc-gr

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @marc-gr

@marc-gr marc-gr merged commit aba91a4 into elastic:main Feb 15, 2024
42 of 43 checks passed
@marc-gr marc-gr deleted the fix-device-definition branch February 15, 2024 13:44
mergify bot pushed a commit that referenced this pull request Feb 15, 2024
@marc-gr @mergify
[packetbeat] Fix interface device parsing for packetbeat protocols (#…
8c0b40a 
…37946)

* Fix interface device parsing for packetbeat protocols

* Add unit test

* Move device check out of validation function

* Add err check on condition for clarity

(cherry picked from commit aba91a4)
mergify bot pushed a commit that referenced this pull request Feb 15, 2024
@marc-gr @mergify
[packetbeat] Fix interface device parsing for packetbeat protocols (#…
00ddb22 
…37946)

* Fix interface device parsing for packetbeat protocols

* Add unit test

* Move device check out of validation function

* Add err check on condition for clarity

(cherry picked from commit aba91a4)
This was referenced Feb 15, 2024
Merged
Merged
marc-gr added a commit that referenced this pull request Feb 15, 2024
@mergify @marc-gr
[packetbeat] Fix interface device parsing for packetbeat protocols (#…
766136e 
…37946) (#38035)

* Fix interface device parsing for packetbeat protocols

* Add unit test

* Move device check out of validation function

* Add err check on condition for clarity

(cherry picked from commit aba91a4)

Co-authored-by: Marc Guasch <marc-gr@users.noreply.github.com>
marc-gr added a commit that referenced this pull request Feb 15, 2024
@mergify @marc-gr
[packetbeat] Fix interface device parsing for packetbeat protocols (#…
f194569 
…37946) (#38036)

* Fix interface device parsing for packetbeat protocols

* Add unit test

* Move device check out of validation function

* Add err check on condition for clarity

(cherry picked from commit aba91a4)

Co-authored-by: Marc Guasch <marc-gr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 approved these changes

@intxgo intxgo intxgo approved these changes

@pkoutsovasilis pkoutsovasilis pkoutsovasilis approved these changes

Assignees

@marc-gr marc-gr

Labels
backport-v8.12.0 Automated backport with mergify backport-v8.13.0 Automated backport with mergify bugfix Packetbeat Team:Security-Linux Platform Linux Platform Team in Security Solution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marc-gr @elasticmachine @pkoutsovasilis @intxgo @efd6