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

Parse SNMP profile version numbers using existing ManagementProfile.snmp_version property #2768

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

lunkwill42
Copy link
Member

This fixes #2767 by moving the responsibility for "parsing" the snmp version value of a profile from the SNMPParameters.factory() method to the ManagementProfile object itself.

Regression tests included.

While all data we have has SNMPv2c profiles store their version
number as either the string or the integer 2, some users report that
they appear to be stored as the string `2c`, which crashes ipdevpoll's
`SNMPParameters.factory()` method.
`ManagementProfile.snmp_version` already does the heavy lifting for
us when it comes to parse the SNMP version value of an SNMP profile,
so let's use that instead to avoid issues like the one described in
Uninett#2767
@lunkwill42 lunkwill42 requested a review from hmpf November 27, 2023 14:20
@lunkwill42 lunkwill42 self-assigned this Nov 27, 2023
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (427276e) 56.01% compared to head (98b67c8) 56.01%.

Additional details and impacted files
@@           Coverage Diff           @@
##            5.8.x    #2768   +/-   ##
=======================================
  Coverage   56.01%   56.01%           
=======================================
  Files         567      567           
  Lines       41279    41277    -2     
=======================================
  Hits        23121    23121           
+ Misses      18158    18156    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Nov 27, 2023

Test results

     12 files       12 suites   11m 34s ⏱️
3 294 tests 3 294 ✔️ 0 💤 0
9 357 runs  9 357 ✔️ 0 💤 0

Results for commit 98b67c8.

♻️ This comment has been updated with latest results.

@johannaengland johannaengland self-requested a review November 27, 2023 14:33
Copy link

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

@lunkwill42 lunkwill42 merged commit cce6311 into Uninett:5.8.x Nov 28, 2023
12 checks passed
@lunkwill42 lunkwill42 deleted the bugfix/allow-v2c-profiles branch November 28, 2023 08:27
@lunkwill42 lunkwill42 linked an issue Nov 28, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ipdevpoll fails to start after upgrade to 5.8.0
3 participants