From 1e81d98fdde9055cd406739be08f8bd41d1e4485 Mon Sep 17 00:00:00 2001 From: Mya Date: Mon, 20 Dec 2021 13:40:00 -0700 Subject: [PATCH] fix: panic due to no module (#10303) --- internal/snmp/translate.go | 32 ++++++++++++++++---------------- plugins/inputs/snmp/snmp.go | 16 +++++++++------- plugins/inputs/snmp/snmp_test.go | 7 ++----- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/internal/snmp/translate.go b/internal/snmp/translate.go index 7cfccaf8fab78..90e6bac823939 100644 --- a/internal/snmp/translate.go +++ b/internal/snmp/translate.go @@ -121,17 +121,11 @@ func TrapLookup(oid string) (e MibEntry, err error) { // The following is for snmp -func GetIndex(oidNum string, mibPrefix string) (col []string, tagOids map[string]struct{}, err error) { +func GetIndex(oidNum string, mibPrefix string, node gosmi.SmiNode) (col []string, tagOids map[string]struct{}, err error) { // first attempt to get the table's tags tagOids = map[string]struct{}{} // mimcks grabbing INDEX {} that is returned from snmptranslate -Td MibName - node, err := gosmi.GetNodeByOID(types.OidMustFromString(oidNum)) - - if err != nil { - return []string{}, map[string]struct{}{}, fmt.Errorf("getting submask: %w", err) - } - for _, index := range node.GetIndex() { //nolint:staticcheck //assaignment to nil map to keep backwards compatibilty tagOids[mibPrefix+index.Name] = struct{}{} @@ -145,17 +139,23 @@ func GetIndex(oidNum string, mibPrefix string) (col []string, tagOids map[string } //nolint:revive //Too many return variable but necessary -func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) { +func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText string, conversion string, node gosmi.SmiNode, err error) { var out gosmi.SmiNode var end string if strings.ContainsAny(oid, "::") { // split given oid // for example RFC1213-MIB::sysUpTime.0 s := strings.SplitN(oid, "::", 2) - // node becomes sysUpTime.0 + // moduleName becomes RFC1213 + moduleName := s[0] + module, err := gosmi.GetModule(moduleName) + if err != nil { + return oid, oid, oid, oid, gosmi.SmiNode{}, err + } if s[1] == "" { - return "", oid, oid, oid, fmt.Errorf("cannot parse %v\n", oid) + return "", oid, oid, oid, gosmi.SmiNode{}, fmt.Errorf("cannot parse %v\n", oid) } + // node becomes sysUpTime.0 node := s[1] if strings.ContainsAny(node, ".") { s = strings.SplitN(node, ".", 2) @@ -164,9 +164,9 @@ func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText strin end = "." + s[1] } - out, err = gosmi.GetNode(node) + out, err = module.GetNode(node) if err != nil { - return oid, oid, oid, oid, err + return oid, oid, oid, oid, out, err } oidNum = "." + out.RenderNumeric() + end @@ -177,7 +177,7 @@ func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText strin if strings.ContainsAny(s[i], "abcdefghijklmnopqrstuvwxyz") { out, err = gosmi.GetNode(s[i]) if err != nil { - return oid, oid, oid, oid, err + return oid, oid, oid, oid, out, err } s[i] = out.RenderNumeric() } @@ -191,7 +191,7 @@ func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText strin // do not return the err as the oid is numeric and telegraf can continue //nolint:nilerr if err != nil || out.Name == "iso" { - return oid, oid, oid, oid, nil + return oid, oid, oid, oid, out, nil } } @@ -214,10 +214,10 @@ func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText strin oidText = out.RenderQualified() i := strings.Index(oidText, "::") if i == -1 { - return "", oid, oid, oid, fmt.Errorf("not found") + return "", oid, oid, oid, out, fmt.Errorf("not found") } mibName = oidText[:i] oidText = oidText[i+2:] + end - return mibName, oidNum, oidText, conversion, nil + return mibName, oidNum, oidText, conversion, out, nil } diff --git a/plugins/inputs/snmp/snmp.go b/plugins/inputs/snmp/snmp.go index 193332959dbfa..175488858547e 100644 --- a/plugins/inputs/snmp/snmp.go +++ b/plugins/inputs/snmp/snmp.go @@ -12,6 +12,7 @@ import ( "time" "github.com/gosnmp/gosnmp" + "github.com/sleepinggenius2/gosmi" "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/config" @@ -260,7 +261,7 @@ func (f *Field) init() error { // check if oid needs translation or name is not set if strings.ContainsAny(f.Oid, ":abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") || f.Name == "" { - _, oidNum, oidText, conversion, err := SnmpTranslate(f.Oid) + _, oidNum, oidText, conversion, _, err := SnmpTranslate(f.Oid) if err != nil { return fmt.Errorf("translating: %w", err) } @@ -504,7 +505,7 @@ func (t Table) Build(gs snmpConnection, walk bool) (*RTable, error) { // snmptranslate table field value here if f.Translate { if entOid, ok := ent.Value.(string); ok { - _, _, oidText, _, err := SnmpTranslate(entOid) + _, _, oidText, _, _, err := SnmpTranslate(entOid) if err == nil { // If no error translating, the original value for ent.Value should be replaced ent.Value = oidText @@ -827,14 +828,14 @@ func snmpTable(oid string) (mibName string, oidNum string, oidText string, field //nolint:revive //Too many return variable but necessary func snmpTableCall(oid string) (mibName string, oidNum string, oidText string, fields []Field, err error) { - mibName, oidNum, oidText, _, err = SnmpTranslate(oid) + mibName, oidNum, oidText, _, node, err := SnmpTranslate(oid) if err != nil { return "", "", "", nil, fmt.Errorf("translating: %w", err) } mibPrefix := mibName + "::" - col, tagOids, err := snmp.GetIndex(oidNum, mibPrefix) + col, tagOids, err := snmp.GetIndex(oidNum, mibPrefix, node) for _, c := range col { _, isTag := tagOids[mibPrefix+c] @@ -849,6 +850,7 @@ type snmpTranslateCache struct { oidNum string oidText string conversion string + node gosmi.SmiNode err error } @@ -857,7 +859,7 @@ var snmpTranslateCaches map[string]snmpTranslateCache // snmpTranslate resolves the given OID. //nolint:revive //Too many return variable but necessary -func SnmpTranslate(oid string) (mibName string, oidNum string, oidText string, conversion string, err error) { +func SnmpTranslate(oid string) (mibName string, oidNum string, oidText string, conversion string, node gosmi.SmiNode, err error) { snmpTranslateCachesLock.Lock() if snmpTranslateCaches == nil { snmpTranslateCaches = map[string]snmpTranslateCache{} @@ -874,11 +876,11 @@ func SnmpTranslate(oid string) (mibName string, oidNum string, oidText string, c // is worth it. Especially when it would slam the system pretty hard if lots // of lookups are being performed. - stc.mibName, stc.oidNum, stc.oidText, stc.conversion, stc.err = snmp.SnmpTranslateCall(oid) + stc.mibName, stc.oidNum, stc.oidText, stc.conversion, stc.node, stc.err = snmp.SnmpTranslateCall(oid) snmpTranslateCaches[oid] = stc } snmpTranslateCachesLock.Unlock() - return stc.mibName, stc.oidNum, stc.oidText, stc.conversion, stc.err + return stc.mibName, stc.oidNum, stc.oidText, stc.conversion, stc.node, stc.err } diff --git a/plugins/inputs/snmp/snmp_test.go b/plugins/inputs/snmp/snmp_test.go index ee87e2c24ed49..d018376b4eafc 100644 --- a/plugins/inputs/snmp/snmp_test.go +++ b/plugins/inputs/snmp/snmp_test.go @@ -145,9 +145,6 @@ func TestFieldInit(t *testing.T) { {".1.2.3", "foo", "", ".1.2.3", "foo", ""}, {".iso.2.3", "foo", "", ".1.2.3", "foo", ""}, {".1.0.0.0.1.1", "", "", ".1.0.0.0.1.1", "server", ""}, - {"TEST::server", "", "", ".1.0.0.0.1.1", "server", ""}, - {"TEST::server.0", "", "", ".1.0.0.0.1.1.0", "server.0", ""}, - {"TEST::server", "foo", "", ".1.0.0.0.1.1", "foo", ""}, {"IF-MIB::ifPhysAddress.1", "", "", ".1.3.6.1.2.1.2.2.1.6.1", "ifPhysAddress.1", "hwaddr"}, {"IF-MIB::ifPhysAddress.1", "", "none", ".1.3.6.1.2.1.2.2.1.6.1", "ifPhysAddress.1", "none"}, {"BRIDGE-MIB::dot1dTpFdbAddress.1", "", "", ".1.3.6.1.2.1.17.4.3.1.1.1", "dot1dTpFdbAddress.1", "hwaddr"}, @@ -995,7 +992,7 @@ func TestFieldConvert(t *testing.T) { func TestSnmpTranslateCache_miss(t *testing.T) { snmpTranslateCaches = nil oid := "IF-MIB::ifPhysAddress.1" - mibName, oidNum, oidText, conversion, err := SnmpTranslate(oid) + mibName, oidNum, oidText, conversion, _, err := SnmpTranslate(oid) require.Len(t, snmpTranslateCaches, 1) stc := snmpTranslateCaches[oid] require.NotNil(t, stc) @@ -1016,7 +1013,7 @@ func TestSnmpTranslateCache_hit(t *testing.T) { err: fmt.Errorf("e"), }, } - mibName, oidNum, oidText, conversion, err := SnmpTranslate("foo") + mibName, oidNum, oidText, conversion, _, err := SnmpTranslate("foo") require.Equal(t, "a", mibName) require.Equal(t, "b", oidNum) require.Equal(t, "c", oidText)