From 92f7c2ed857ee01df179e23628661e2b786c2b24 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 11 Feb 2026 13:04:32 -0600 Subject: [PATCH] fix SNMPv1 walks failing due to unsupported GETBULK use GETNEXT-based WalkAll for v1 devices instead of BulkWalkAll --- snmp.go | 10 +++++- snmp_test.go | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/snmp.go b/snmp.go index c6af8e5..1121919 100644 --- a/snmp.go +++ b/snmp.go @@ -13,6 +13,7 @@ import ( // snmpQuerier abstracts SNMP operations for testability. type snmpQuerier interface { Get(oids []string) (*gosnmp.SnmpPacket, error) + WalkAll(rootOid string) ([]gosnmp.SnmpPDU, error) BulkWalkAll(rootOid string) ([]gosnmp.SnmpPDU, error) } @@ -59,8 +60,15 @@ func executeSnmpJob(job *pb.AgentJob, resultCh chan<- *pb.SnmpResult) { } } case pb.QueryType_WALK: + // SNMPv1 doesn't support GETBULK, use GETNEXT-based WalkAll instead + useV1Walk := dev.Version == "1" || dev.Version == "v1" for _, baseOID := range q.Oids { - results, err := conn.BulkWalkAll(baseOID) + var results []gosnmp.SnmpPDU + if useV1Walk { + results, err = conn.WalkAll(baseOID) + } else { + results, err = conn.BulkWalkAll(baseOID) + } if err != nil { slog.Warn("snmp walk failed", "device", dev.Ip, "oid", baseOID, "error", err) continue diff --git a/snmp_test.go b/snmp_test.go index e1eb9fd..7a40f75 100644 --- a/snmp_test.go +++ b/snmp_test.go @@ -271,16 +271,28 @@ func TestSnmpDialDefault(t *testing.T) { // mockSnmpQuerier implements snmpQuerier for testing. type mockSnmpQuerier struct { - getFunc func(oids []string) (*gosnmp.SnmpPacket, error) - walkFunc func(rootOid string) ([]gosnmp.SnmpPDU, error) - closeCalled bool + getFunc func(oids []string) (*gosnmp.SnmpPacket, error) + walkFunc func(rootOid string) ([]gosnmp.SnmpPDU, error) + bulkWalkFunc func(rootOid string) ([]gosnmp.SnmpPDU, error) + closeCalled bool + walkAllCalled bool + bulkWalkCalled bool } func (m *mockSnmpQuerier) Get(oids []string) (*gosnmp.SnmpPacket, error) { return m.getFunc(oids) } +func (m *mockSnmpQuerier) WalkAll(rootOid string) ([]gosnmp.SnmpPDU, error) { + m.walkAllCalled = true + return m.walkFunc(rootOid) +} + func (m *mockSnmpQuerier) BulkWalkAll(rootOid string) ([]gosnmp.SnmpPDU, error) { + m.bulkWalkCalled = true + if m.bulkWalkFunc != nil { + return m.bulkWalkFunc(rootOid) + } return m.walkFunc(rootOid) } @@ -500,6 +512,78 @@ func TestExecuteSnmpJob(t *testing.T) { } }) + t.Run("WALK v1 uses WalkAll", func(t *testing.T) { + orig := snmpDial + defer func() { snmpDial = orig }() + + mock := &mockSnmpQuerier{ + walkFunc: func(rootOid string) ([]gosnmp.SnmpPDU, error) { + return []gosnmp.SnmpPDU{ + {Name: ".1.3.6.1.2.1.2.2.1.1.1", Type: gosnmp.Integer, Value: 1}, + }, nil + }, + } + snmpDial = func(dev *pb.SnmpDevice) (snmpQuerier, func(), error) { + return mock, func() {}, nil + } + + ch := make(chan *pb.SnmpResult, 1) + executeSnmpJob(&pb.AgentJob{ + JobId: "1", + SnmpDevice: &pb.SnmpDevice{Ip: "10.0.0.1", Version: "1"}, + Queries: []*pb.SnmpQuery{ + {QueryType: pb.QueryType_WALK, Oids: []string{".1.3.6.1.2.1.2.2.1.1"}}, + }, + }, ch) + + result := <-ch + if len(result.OidValues) != 1 { + t.Errorf("got %d oid values, want 1", len(result.OidValues)) + } + if !mock.walkAllCalled { + t.Error("expected WalkAll to be called for v1") + } + if mock.bulkWalkCalled { + t.Error("BulkWalkAll should not be called for v1") + } + }) + + t.Run("WALK v2c uses BulkWalkAll", func(t *testing.T) { + orig := snmpDial + defer func() { snmpDial = orig }() + + mock := &mockSnmpQuerier{ + walkFunc: func(rootOid string) ([]gosnmp.SnmpPDU, error) { + return []gosnmp.SnmpPDU{ + {Name: ".1.3.6.1.2.1.2.2.1.1.1", Type: gosnmp.Integer, Value: 1}, + }, nil + }, + } + snmpDial = func(dev *pb.SnmpDevice) (snmpQuerier, func(), error) { + return mock, func() {}, nil + } + + ch := make(chan *pb.SnmpResult, 1) + executeSnmpJob(&pb.AgentJob{ + JobId: "1", + SnmpDevice: &pb.SnmpDevice{Ip: "10.0.0.1", Version: "2c"}, + Queries: []*pb.SnmpQuery{ + {QueryType: pb.QueryType_WALK, Oids: []string{".1.3.6.1.2.1.2.2.1.1"}}, + }, + }, ch) + + result := <-ch + if len(result.OidValues) != 1 { + t.Errorf("got %d oid values, want 1", len(result.OidValues)) + } + if !mock.bulkWalkCalled { + t.Error("expected BulkWalkAll to be called for v2c") + } + if mock.walkAllCalled { + t.Error("WalkAll should not be called for v2c") + } + }) + t.Run("channel full drops", func(t *testing.T) { orig := snmpDial defer func() { snmpDial = orig }()