graham/towerops-agent
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix SNMPv1 walks failing due to unsupported GETBULK

Browse source 
use GETNEXT-based WalkAll for v1 devices instead of BulkWalkAll
This commit is contained in:
Graham McIntire 2026-02-11 13:04:32 -06:00
parent 01cd109d10
commit 92f7c2ed85
No known key found for this signature in database
2 changed files with 96 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
snmp.go
View file

@ -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

90
snmp_test.go
View file

@ -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 }()