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

Ensure AgentProxy.snmpVersion is prefixed by v #2780

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

lunkwill42
Copy link
Member

If the snmpVersion argument to AgentProxy is not a version string prefixed by the letter v, AgentProxy will silently default to v2c.

Using 2c as a value works, since it defaults then to v2c. Using 3 also seems to work, as providing further SNMPv3 configuration arguments to the Net-SNMP library seems to automatically set the session version to v3, regardless of the version parameter.

However, using 1 will not work, as that will also silently default the session to v2c, making SNMPv1 communication impossible.

Fixes #2779
Fixes #2772

If the `snmpVersion` argument to `AgentProxy` is not a version string
prefixed by the letter `v`, `AgentProxy` will silently default to `v2c`.

Using `2c` as a value works, since it defaults then to `v2c`.  Using `3`
also seems to work, as providing further SNMPv3 configuration arguments
to the Net-SNMP library seems to automatically set the session version
to `v3`, regardless of the version parameter.

However, using `1` will not work, as that will also silently default
the session to `v2c`, making SNMPv1 communication impossible.
@lunkwill42 lunkwill42 added the snmp label Dec 1, 2023
@lunkwill42 lunkwill42 requested a review from hmpf December 1, 2023 10:43
@lunkwill42 lunkwill42 self-assigned this Dec 1, 2023
@lunkwill42 lunkwill42 changed the title Ensure AgentProxy.snmpVersion is prefixed by v Ensure AgentProxy.snmpVersion is prefixed by v Dec 1, 2023
@lunkwill42
Copy link
Member Author

Copy link

github-actions bot commented Dec 1, 2023

Test results

       8 files         8 suites   4m 7s ⏱️
3 300 tests 3 300 ✔️ 0 💤 0
4 915 runs  4 915 ✔️ 0 💤 0

Results for commit c679e56.

♻️ This comment has been updated with latest results.

johannaengland
johannaengland previously approved these changes Dec 1, 2023
Copy link
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

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

Just....lovely... 😅

Now need to test that the version argument is actually prefixed by `v`
@lunkwill42 lunkwill42 force-pushed the bugfix/snmpv1-agentproxy-fixup branch from 83e9eda to c679e56 Compare December 1, 2023 11:03
Copy link

sonarqubecloud bot commented Dec 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Github needs reaction-gifs.

To be exact: Picard facepalms.

@lunkwill42 lunkwill42 merged commit cec8ad9 into Uninett:5.8.x Dec 1, 2023
10 checks passed
@lunkwill42 lunkwill42 deleted the bugfix/snmpv1-agentproxy-fixup branch December 1, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants