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

Add netsnmp-cffi as alternative SNMP polling back-end #394

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

lunkwill42
Copy link
Member

Scope and purpose

Fixes the polling bits of #383 (replaces #391)

This refactors the SNMP agent back-end to be even more amenable to replacement, and moves on to add an alternative implementation based on the new netsnmp-cffi library.

It does not (currently) replace the trapd back-end, which continues to work alongside these changese. This is potentially a separate PR, but the drawback is currently that the two back-ends use a different set of MIB files (the original PySNMP back-end vendors MIB files translated to Python code, while this back-end introduces a directory of vendored MIB files in their original format).

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Zino can be found in the
README.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with black, ruff and isort, easiest by using pre-commit
  • The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done

In preparation for a new upcoming SNMP back-end, this moves the pysnmp-
specific back-end to a submodule of `zino.snmp` so an interchangeable
module can be made more easily.

The _mib_value_to_python function was imported outside of the original
module, so it was renamed to be public instead of internal in order
for imports to keep working.

Tests and patches that referenced old names were updated, and all
mibdump files were also moved.
These classes and type definitions aren't specific for the PySNMP
back-end and are therefore moved to a new module from where they
can be shared.
The poll device `hcounters` attribute is really what drives the SNMP
version selection in Zino.  Unfortunately, the default is SNMP v1, and
this has worked because PySNMP silently allowed v2c requests like
BULK-GET to be sent over sessions that were otherwise v1.

Net-SNMP does not allow the sending of v2c PDUs over v1 sessions, so
we need to be more strict about this.  It also makes no sense to default
to v1 in 2025, so this sets `hcounters`' default value to True and
fixes a test that implicitly assumed v1 exceptions to be raised.
I don't generally think it is a good idea to check for very specific
log output.  In this case, the non-int object type returned by the
underlying library may depend on the library.  netsnmpy-cffi does not
generally translate octet strings to `str` unless they have a
textual convention of `DisplayString`.  That has really no bearing on
the actual functionality in this case, so it's quite weird to test
for such specific log messages.
The underlying netsnmpy session object will not work with mock values
for addresses and community string, so its better to use an actual
mocked PollDevice object rather than a 100% Mock.
This ensures all MIB lookups work properly when running the test suite.
MIBs should be vendored verbatim.
This vendors in all the MIB definitions required to run Zino.  PySNMP
will work with versions translated to Python modules, Net-SNMP needs the
original definitions.
The netsnmp-cffi package i still in pre-alpha and has no proper
release yet.  It will be moved to GitHub and released on PyPI in
due time.
This specifically reaches into the PySNMP-based back-end for handling
traps, thereby making the test suite green again.
The workflow may need to build netsnmp-cffi from scratch if no pre-made
wheels are available.
I.e. have Net-SNMP load MIBs from its defined locations
There is no need to get a new SNMP session here, the instance
already has one.
Before this, every Task instance would create its own SNMP session
instance.  This might not mean much under PySNMP, but is wasteful
using netsnmp-cffi, as each open session takes up a socket resource.
Having multiple simultaneous sessions/sockets per poll device makes
no sense, so `get_snmp_session()` utilizes a `WeakValueDictionary`
to keep track of the existing session for each device and returns
an existing session if there is one.
These test patches will not work now that everything uses
`zino.snmp.get_snmp_session()` - the SNMP class needs to be patched
in the correct location.
The new backend needs more or less the same set of tests as the old
back-end.  This copies the tests for the PySNMP back-end as a baseline
for modification.
This rewrites the tests copied from the PySNMP back-end so they
instead test the netsnmpy backend.
Some of the PySNMP back-end only had implicit test coverage, which was
lost in the rewrite to a multi-backend system.  This adds explicit
coverage to some of the bits that lost implicit coverage, to keep the
coverage numbers up.
@lunkwill42 lunkwill42 self-assigned this Feb 14, 2025
Copy link

Copy link

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 54 0 1.13s
✅ PYTHON isort 54 0 0.5s
✅ PYTHON ruff 54 0 0.02s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

Copy link

Test results

    3 files      3 suites   1m 23s ⏱️
  639 tests   639 ✅ 0 💤 0 ❌
1 869 runs  1 867 ✅ 2 💤 0 ❌

Results for commit 6e91f30.

@lunkwill42 lunkwill42 changed the title Feature/use netsnmpy cffi Add netsnmp-cffi as alternative SNMP polling back-end Feb 14, 2025
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.

1 participant