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: panic due to no module #10303

Merged
merged 5 commits into from
Dec 20, 2021
Merged

fix: panic due to no module #10303

merged 5 commits into from
Dec 20, 2021

Conversation

MyaLongmire
Copy link
Contributor

@MyaLongmire MyaLongmire commented Dec 18, 2021

Required for all PRs:

resolves #10290

THIS IS A DRAFT! Loaded module in to prevent panic on IETF mibs. Has been tested by community members to fix their panics. Now loads in a module and passes the gosmi.SmiNode to other function so the node does not get lost and cause a panic.

internal/snmp/translate.go Outdated Show resolved Hide resolved
internal/snmp/translate.go Outdated Show resolved Hide resolved
plugins/inputs/snmp/snmp_test.go Outdated Show resolved Hide resolved
@MyaLongmire MyaLongmire marked this pull request as ready for review December 20, 2021 16:10
Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

Same remark as maintainer of gosmi, You know the module name, but aren't doing anything with it, so you traverse all the loaded mibs in order to find one node that might happen to have the same name.

internal/snmp/translate.go Outdated Show resolved Hide resolved
internal/snmp/translate.go Outdated Show resolved Hide resolved
@MyaLongmire
Copy link
Contributor Author

Added module but remove the server mib test as I hand wrote the mib and believe it is invalid. There is a reason people do not handwrite mibs as they are very complex. The tests were based on invalid mibs to begin with, I kept all of the ieft valid mibs to keep the validity of the tests.

@telegraf-tiger
Copy link
Contributor

@MyaLongmire MyaLongmire merged commit 1e81d98 into master Dec 20, 2021
@MyaLongmire MyaLongmire deleted the snmp-panic-no-module branch December 20, 2021 20:40
powersj pushed a commit that referenced this pull request Jan 5, 2022
nward added a commit to nward/telegraf that referenced this pull request Jan 12, 2022
powersj pushed a commit to powersj/telegraf that referenced this pull request Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/snmp fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Telegraf 1.21.0 - issue parsing mibs and panic with SNMP plugin
3 participants