Add snmpd system tests with Valgrind and fix memory leak

.containerignore

@@ -0,0 +1,6 @@
+.git
+.vscode
+build
+**/__pycache__
+**/*.pyc
+**/*.so

.github/workflows/system-tests.yml

@@ -0,0 +1,83 @@
+name: System tests
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+
+permissions:
+  contents: read
+
+jobs:
+  snmpd-valgrind:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y \
+            build-essential \
+            libsnmp-dev \
+            python3-dev \
+            python3-setuptools \
+            snmp \
+            snmpd \
+            valgrind
+
+      - name: Run API system tests and Valgrind
+        run: sh netsnmp/tests/system/run.sh
+
+  coverage:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y \
+            build-essential \
+            libsnmp-dev \
+            python3-dev \
+            python3-setuptools \
+            snmp \
+            snmpd
+
+      - name: Run API system tests with C coverage
+        run: sh netsnmp/tests/system/run-coverage.sh
+
+      - name: Upload C coverage report
+        uses: actions/upload-artifact@v4
+        with:
+          name: client-intf-coverage
+          path: |
+            client_intf-coverage.txt
+            client_intf.c.gcov
+
+  asan-ubsan:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y \
+            build-essential \
+            libsnmp-dev \
+            python3-dev \
+            python3-setuptools \
+            snmp \
+            snmpd
+
+      - name: Run API system tests with ASan and UBSan
+        run: sh netsnmp/tests/system/run-sanitizers.sh

Containerfile.system-tests

@@ -0,0 +1,20 @@
+FROM ubuntu:latest
+
+ENV DEBIAN_FRONTEND=noninteractive
+
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+        build-essential \
+        libsnmp-dev \
+        python3 \
+        python3-dev \
+        python3-setuptools \
+        snmp \
+        snmpd \
+        valgrind \
+    && rm -rf /var/lib/apt/lists/*
+
+WORKDIR /src
+COPY . .
+
+CMD ["sh", "netsnmp/tests/system/run.sh"]

netsnmp/tests/system/README.md

@@ -0,0 +1,24 @@
+# System tests
+
+The system tests start a private SNMPv2c agent on `127.0.0.1:1161`, exercise
+all convenience functions and `Session` methods, and repeat the test suite
+under Valgrind. Each SNMP operation has a separate `test_*.py` file, and the
+runner discovers new test files automatically.
+
+The normal and Valgrind suites each run all discovered tests in one process.
+CI also builds the extension with GCC coverage and with ASan/UBSan. The
+coverage job uploads the full C coverage report.
+
+Coverage also includes SNMPv1, multiple varbinds, protocol error paths,
+repeated session and walk lifecycles, and the Python `Varbind` and `VarList`
+containers.
+
+Run the same Ubuntu environment locally with Podman:
+
+```sh
+podman build -f Containerfile.system-tests -t python3-netsnmp-system-tests .
+podman run --rm python3-netsnmp-system-tests
+```
+
+Valgrind returns a non-zero status for memory errors and definite leaks.
+GitHub Actions runs the same test script for every pull request.

netsnmp/tests/system/common.py

@@ -0,0 +1,21 @@
+HOST = '127.0.0.1:1161'
+READ_ARGS = {
+    'Version': 2,
+    'DestHost': HOST,
+    'Community': 'public',
+    'Timeout': 1000000,
+    'Retries': 0,
+}
+WRITE_ARGS = READ_ARGS.copy()
+WRITE_ARGS['Community'] = 'private'
+
+SYS_DESCR = '.1.3.6.1.2.1.1.1'
+SYS_UPTIME = '.1.3.6.1.2.1.1.3'
+SYS_LOCATION = '.1.3.6.1.2.1.1.6'
+SYSTEM = '.1.3.6.1.2.1.1'
+
+
+def assert_value(test_case, values):
+    test_case.assertIsInstance(values, tuple)
+    test_case.assertTrue(values)
+    test_case.assertIsNotNone(values[0])

netsnmp/tests/system/run-coverage.sh

@@ -0,0 +1,17 @@
+#!/bin/sh
+set -eu
+
+ROOT=$(CDPATH= cd -- "$(dirname "$0")/../../.." && pwd)
+cd "$ROOT"
+
+rm -rf build
+rm -f netsnmp/client_intf*.so
+
+CFLAGS='--coverage -O0' \
+LDFLAGS='--coverage' \
+RUN_VALGRIND=0 \
+    sh netsnmp/tests/system/run.sh
+
+OBJECT_DIR=$(dirname "$(find build -name client_intf.o -print -quit)")
+gcov -b -f -o "$OBJECT_DIR" netsnmp/client_intf.c \
+    | tee client_intf-coverage.txt

netsnmp/tests/system/run-sanitizers.sh

@@ -0,0 +1,17 @@
+#!/bin/sh
+set -eu
+
+ROOT=$(CDPATH= cd -- "$(dirname "$0")/../../.." && pwd)
+cd "$ROOT"
+
+rm -rf build
+rm -f netsnmp/client_intf*.so
+
+SANITIZERS=address,undefined
+CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=$SANITIZERS" \
+LDFLAGS="-fsanitize=$SANITIZERS" \
+LD_PRELOAD=$(gcc -print-file-name=libasan.so) \
+ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 \
+UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 \
+RUN_VALGRIND=0 \
+    sh netsnmp/tests/system/run.sh

netsnmp/tests/system/run.sh

@@ -0,0 +1,60 @@
+#!/bin/sh
+set -eu
+
+ROOT=$(CDPATH= cd -- "$(dirname "$0")/../../.." && pwd)
+RUNTIME_DIR=$(mktemp -d)
+SNMPD_PID=
+
+cleanup() {
+    if [ -n "$SNMPD_PID" ]; then
+        kill "$SNMPD_PID" 2>/dev/null || true
+        wait "$SNMPD_PID" 2>/dev/null || true
+    fi
+    rm -rf "$RUNTIME_DIR"
+}
+trap cleanup EXIT INT TERM
+
+cd "$ROOT"
+python3 setup.py build_ext --inplace
+
+SNMPD_CONFIG="$RUNTIME_DIR/snmpd.conf"
+cat "$ROOT/netsnmp/tests/system/snmpd.conf" >"$SNMPD_CONFIG"
+printf '\npersistentDir %s\n' "$RUNTIME_DIR" >>"$SNMPD_CONFIG"
+
+snmpd -f -Lo -C \
+    -c "$SNMPD_CONFIG" \
+    -p "$RUNTIME_DIR/snmpd.pid" \
+    >"$RUNTIME_DIR/snmpd.log" 2>&1 &
+SNMPD_PID=$!
+
+attempt=0
+until snmpget -v2c -c public -t 1 -r 0 \
+        127.0.0.1:1161 .1.3.6.1.2.1.1.1.0 >/dev/null 2>&1; do
+    attempt=$((attempt + 1))
+    if [ "$attempt" -ge 20 ] || ! kill -0 "$SNMPD_PID" 2>/dev/null; then
+        cat "$RUNTIME_DIR/snmpd.log"
+        exit 1
+    fi
+done
+
+python3 -m unittest discover -v -s netsnmp/tests/system -p 'test_*.py'
+
+if [ "${RUN_VALGRIND:-1}" = 0 ]; then
+    exit 0
+fi
+
+valgrind \
+    --error-exitcode=99 \
+    --leak-check=full \
+    --show-leak-kinds=definite \
+    --errors-for-leak-kinds=definite \
+    --track-origins=yes \
+    --log-file="$RUNTIME_DIR/valgrind.log" \
+    python3 -m unittest discover -v -s netsnmp/tests/system \
+        -p 'test_*.py' || {
+        status=$?
+        cat "$RUNTIME_DIR/valgrind.log"
+        exit "$status"
+    }
+
+cat "$RUNTIME_DIR/valgrind.log"

netsnmp/tests/system/snmpd.conf

@@ -0,0 +1,3 @@
+agentAddress udp:127.0.0.1:1161
+rocommunity public 127.0.0.1
+rwcommunity private 127.0.0.1

netsnmp/tests/system/test_errors.py

@@ -0,0 +1,39 @@
+import unittest
+
+import netsnmp
+
+from netsnmp.tests.system.common import READ_ARGS, SYS_DESCR, SYS_LOCATION
+
+
+class ErrorPathTests(unittest.TestCase):
+    def test_unknown_oid_returns_no_value(self):
+        varbind = netsnmp.Varbind('.1.3.6.1.2.1.1.999', '0')
+        values = netsnmp.snmpget(varbind, **READ_ARGS)
+
+        self.assertEqual(values, (None,))
+        self.assertIn(varbind.type, ('NOSUCHOBJECT', 'NOSUCHINSTANCE'))
+
+    def test_read_only_set_is_rejected(self):
+        session = netsnmp.Session(**READ_ARGS)
+        varlist = netsnmp.VarList(
+            netsnmp.Varbind(
+                SYS_LOCATION, '0', b'not-written', 'OCTETSTR'))
+
+        self.assertEqual(session.set(varlist), 0)
+        self.assertTrue(session.ErrorStr)
+        self.assertNotEqual(session.ErrorNum, 0)
+
+    def test_timeout_updates_session_error(self):
+        timeout_args = READ_ARGS.copy()
+        timeout_args['DestHost'] = '127.0.0.1:1162'
+        timeout_args['Timeout'] = 100000
+        session = netsnmp.Session(**timeout_args)
+        varlist = netsnmp.VarList(netsnmp.Varbind(SYS_DESCR, '0'))
+
+        self.assertEqual(session.get(varlist), (None,))
+        self.assertTrue(session.ErrorStr)
+        self.assertNotEqual(session.ErrorInd, 0)
+
+
+if __name__ == '__main__':
+    unittest.main()

netsnmp/tests/system/test_get.py

@@ -0,0 +1,21 @@
+import unittest
+
+import netsnmp
+
+from netsnmp.tests.system.common import READ_ARGS, SYS_DESCR, assert_value
+
+
+class GetTests(unittest.TestCase):
+    def test_convenience_function(self):
+        values = netsnmp.snmpget(
+            netsnmp.Varbind(SYS_DESCR, '0'), **READ_ARGS)
+        assert_value(self, values)
+
+    def test_session_method(self):
+        session = netsnmp.Session(**READ_ARGS)
+        varlist = netsnmp.VarList(netsnmp.Varbind(SYS_DESCR, '0'))
+        assert_value(self, session.get(varlist))
+
+
+if __name__ == '__main__':
+    unittest.main()

netsnmp/tests/system/test_getbulk.py

@@ -0,0 +1,21 @@
+import unittest
+
+import netsnmp
+
+from netsnmp.tests.system.common import READ_ARGS, SYSTEM, assert_value
+
+
+class GetBulkTests(unittest.TestCase):
+    def test_convenience_function(self):
+        values = netsnmp.snmpgetbulk(
+            0, 4, netsnmp.Varbind(SYSTEM), **READ_ARGS)
+        assert_value(self, values)
+
+    def test_session_method(self):
+        session = netsnmp.Session(**READ_ARGS)
+        varlist = netsnmp.VarList(netsnmp.Varbind(SYSTEM))
+        assert_value(self, session.getbulk(0, 4, varlist))
+
+
+if __name__ == '__main__':
+    unittest.main()

netsnmp/tests/system/test_getnext.py

@@ -0,0 +1,21 @@
+import unittest
+
+import netsnmp
+
+from netsnmp.tests.system.common import READ_ARGS, SYSTEM, assert_value
+
+
+class GetNextTests(unittest.TestCase):
+    def test_convenience_function(self):
+        varbind = netsnmp.Varbind(SYSTEM)
+        assert_value(self, netsnmp.snmpgetnext(varbind, **READ_ARGS))
+        self.assertTrue(varbind.tag)
+
+    def test_session_method(self):
+        session = netsnmp.Session(**READ_ARGS)
+        varlist = netsnmp.VarList(netsnmp.Varbind(SYSTEM))
+        assert_value(self, session.getnext(varlist))
+
+
+if __name__ == '__main__':
+    unittest.main()

netsnmp/tests/system/test_lifecycle.py

@@ -0,0 +1,31 @@
+import unittest
+
+import netsnmp
+
+from netsnmp.tests.system.common import (
+    READ_ARGS,
+    SYS_DESCR,
+    SYSTEM,
+    assert_value,
+)
+
+
+class SessionLifecycleTests(unittest.TestCase):
+    def test_repeated_session_creation(self):
+        for _ in range(25):
+            session = netsnmp.Session(**READ_ARGS)
+            varlist = netsnmp.VarList(netsnmp.Varbind(SYS_DESCR, '0'))
+            assert_value(self, session.get(varlist))
+            del session
+
+    def test_repeated_walks(self):
+        session = netsnmp.Session(**READ_ARGS)
+
+        for _ in range(10):
+            varlist = netsnmp.VarList(netsnmp.Varbind(SYSTEM))
+            assert_value(self, session.walk(varlist))
+            self.assertGreater(len(varlist), 1)
+
+
+if __name__ == '__main__':
+    unittest.main()