From 84c1efbef1e2b158274f826ff7e557a3e025c76a Mon Sep 17 00:00:00 2001 From: Mya Date: Thu, 27 Jan 2022 13:38:03 -0800 Subject: [PATCH] fix: collapsed fields by calling more indepth function (#10430) --- internal/snmp/testdata/mibs/testmib | 22 +++++ internal/snmp/translate.go | 27 +++--- internal/snmp/translate_test.go | 89 ++++++++++++++++++++ plugins/inputs/snmp_trap/snmp_trap_test.go | 94 --------------------- plugins/inputs/snmp_trap/testdata/test.mib | 40 --------- plugins/inputs/snmp_trap/testdata/test2 | 97 ---------------------- 6 files changed, 128 insertions(+), 241 deletions(-) create mode 100644 internal/snmp/testdata/mibs/testmib create mode 100644 internal/snmp/translate_test.go delete mode 100644 plugins/inputs/snmp_trap/testdata/test.mib delete mode 100644 plugins/inputs/snmp_trap/testdata/test2 diff --git a/internal/snmp/testdata/mibs/testmib b/internal/snmp/testdata/mibs/testmib new file mode 100644 index 0000000000000..ce44a135b272c --- /dev/null +++ b/internal/snmp/testdata/mibs/testmib @@ -0,0 +1,22 @@ +TGTEST-MIB DEFINITIONS ::= BEGIN + +org OBJECT IDENTIFIER ::= { iso 3 } -- "iso" = 1 +dod OBJECT IDENTIFIER ::= { org 6 } +internet OBJECT IDENTIFIER ::= { dod 1 } +mgmt OBJECT IDENTIFIER ::= { internet 2 } +mibs OBJECT IDENTIFIER ::= { mgmt 1 } +system OBJECT IDENTIFIER ::= { mibs 1 } +systemUpTime OBJECT IDENTIFIER ::= { system 3 } +sysUpTimeInstance OBJECT IDENTIFIER ::= { systemUpTime 0 } + +private OBJECT IDENTIFIER ::= { internet 4 } +enterprises OBJECT IDENTIFIER ::= { private 1 } + +snmpV2 OBJECT IDENTIFIER ::= { internet 6 } +snmpModules OBJECT IDENTIFIER ::= { snmpV2 3 } +snmpMIB OBJECT IDENTIFIER ::= { snmpModules 1 } +snmpMIBObjects OBJECT IDENTIFIER ::= { snmpMIB 1 } +snmpTraps OBJECT IDENTIFIER ::= { snmpMIBObjects 5 } +coldStart OBJECT IDENTIFIER ::= { snmpTraps 1 } + +END diff --git a/internal/snmp/translate.go b/internal/snmp/translate.go index c6d779d8a2206..da87f131bb8d8 100644 --- a/internal/snmp/translate.go +++ b/internal/snmp/translate.go @@ -111,22 +111,29 @@ type MibEntry struct { } func TrapLookup(oid string) (e MibEntry, err error) { - var node gosmi.SmiNode - node, err = gosmi.GetNodeByOID(types.OidMustFromString(oid)) + var givenOid types.Oid + if givenOid, err = types.OidFromString(oid); err != nil { + return e, fmt.Errorf("could not convert OID %s: %w", oid, err) + } - // ensure modules are loaded or node will be empty (might not error) - if err != nil { + // Get node name + var node gosmi.SmiNode + if node, err = gosmi.GetNodeByOID(givenOid); err != nil { return e, err } + e.OidText = node.Name - e.OidText = node.RenderQualified() + // Add not found OID part + if !givenOid.Equals(node.Oid) { + e.OidText += "." + givenOid[len(node.Oid):].String() + } - i := strings.Index(e.OidText, "::") - if i == -1 { - return e, fmt.Errorf("not found") + // Get module name + module := node.GetModule() + if module.Name != "" { + e.MibName = module.Name } - e.MibName = e.OidText[:i] - e.OidText = e.OidText[i+2:] + return e, nil } diff --git a/internal/snmp/translate_test.go b/internal/snmp/translate_test.go new file mode 100644 index 0000000000000..75555386b7939 --- /dev/null +++ b/internal/snmp/translate_test.go @@ -0,0 +1,89 @@ +package snmp + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/influxdata/telegraf/testutil" +) + +func TestTrapLookup(t *testing.T) { + tests := []struct { + name string + oid string + expected MibEntry + }{ + { + name: "Known trap OID", + oid: ".1.3.6.1.6.3.1.1.5.1", + expected: MibEntry{ + MibName: "TGTEST-MIB", + OidText: "coldStart", + }, + }, + { + name: "Known trap value OID", + oid: ".1.3.6.1.2.1.1.3.0", + expected: MibEntry{ + MibName: "TGTEST-MIB", + OidText: "sysUpTimeInstance", + }, + }, + { + name: "Unknown enterprise sub-OID", + oid: ".1.3.6.1.4.1.0.1.2.3", + expected: MibEntry{ + MibName: "TGTEST-MIB", + OidText: "enterprises.0.1.2.3", + }, + }, + { + name: "Unknown MIB", + oid: ".1.2.3", + expected: MibEntry{OidText: "iso.2.3"}, + }, + } + + // Load the MIBs + require.NoError(t, LoadMibsFromPath([]string{"testdata/mibs"}, testutil.Logger{})) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Run the actual test + actual, err := TrapLookup(tt.oid) + require.NoError(t, err) + require.Equal(t, tt.expected, actual) + }) + } +} + +func TestTrapLookupFail(t *testing.T) { + tests := []struct { + name string + oid string + expected string + }{ + { + name: "New top level OID", + oid: ".3.6.1.3.0", + expected: "Could not find node for OID 3.6.1.3.0", + }, + { + name: "Malformed OID", + oid: ".1.3.dod.1.3.0", + expected: "could not convert OID .1.3.dod.1.3.0: strconv.ParseUint: parsing \"dod\": invalid syntax", + }, + } + + // Load the MIBs + require.NoError(t, LoadMibsFromPath([]string{"testdata/mibs"}, testutil.Logger{})) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Run the actual test + _, err := TrapLookup(tt.oid) + require.EqualError(t, err, tt.expected) + }) + } +} diff --git a/plugins/inputs/snmp_trap/snmp_trap_test.go b/plugins/inputs/snmp_trap/snmp_trap_test.go index 6c7c7df33e20f..804e5a34c1ff3 100644 --- a/plugins/inputs/snmp_trap/snmp_trap_test.go +++ b/plugins/inputs/snmp_trap/snmp_trap_test.go @@ -3,7 +3,6 @@ package snmp_trap import ( "fmt" "net" - "path/filepath" "strconv" "strings" "testing" @@ -1312,96 +1311,3 @@ func TestReceiveTrap(t *testing.T) { }) } } - -func TestGosmiSingleMib(t *testing.T) { - // We would prefer to specify port 0 and let the network - // stack choose an unused port for us but TrapListener - // doesn't have a way to return the autoselected port. - // Instead, we'll use an unusual port and hope it's - // unused. - const port = 12399 - - // Hook into the trap handler so the test knows when the - // trap has been received - received := make(chan int) - wrap := func(f gosnmp.TrapHandlerFunc) gosnmp.TrapHandlerFunc { - return func(p *gosnmp.SnmpPacket, a *net.UDPAddr) { - f(p, a) - received <- 0 - } - } - - fakeTime := time.Unix(456456456, 456) - now := uint32(123123123) - - testDataPath, err := filepath.Abs("./testdata") - require.NoError(t, err) - - trap := gosnmp.SnmpTrap{ - Variables: []gosnmp.SnmpPDU{ - { - Name: ".1.3.6.1.2.1.1.3.0", - Type: gosnmp.TimeTicks, - Value: now, - }, - { - Name: ".1.3.6.1.6.3.1.1.4.1.0", // SNMPv2-MIB::snmpTrapOID.0 - Type: gosnmp.ObjectIdentifier, - Value: ".1.3.6.1.6.3.1.1.5.1", // coldStart - }, - }, - } - - metrics := []telegraf.Metric{ - testutil.MustMetric( - "snmp_trap", // name - map[string]string{ // tags - "oid": ".1.3.6.1.6.3.1.1.5.1", - "name": "coldStart", - "mib": "SNMPv2-MIB", - "version": "2c", - "source": "127.0.0.1", - "community": "public", - }, - map[string]interface{}{ // fields - "sysUpTimeInstance": now, - }, - fakeTime, - ), - } - - // Set up the service input plugin - s := &SnmpTrap{ - ServiceAddress: "udp://:" + strconv.Itoa(port), - makeHandlerWrapper: wrap, - timeFunc: func() time.Time { - return fakeTime - }, - lookupFunc: snmp.TrapLookup, - Log: testutil.Logger{}, - Version: "2c", - Path: []string{testDataPath}, - } - require.NoError(t, s.Init()) - - var acc testutil.Accumulator - require.Nil(t, s.Start(&acc)) - defer s.Stop() - - goSNMP := newGoSNMP(gosnmp.Version2c, port) - - // Send the trap - sendTrap(t, goSNMP, trap) - - // Wait for trap to be received - select { - case <-received: - case <-time.After(2 * time.Second): - t.Fatal("timed out waiting for trap to be received") - } - - // Verify plugin output - testutil.RequireMetricsEqual(t, - metrics, acc.GetTelegrafMetrics(), - testutil.SortMetrics()) -} diff --git a/plugins/inputs/snmp_trap/testdata/test.mib b/plugins/inputs/snmp_trap/testdata/test.mib deleted file mode 100644 index d8ff17af04eba..0000000000000 --- a/plugins/inputs/snmp_trap/testdata/test.mib +++ /dev/null @@ -1,40 +0,0 @@ -SNMPv2-MIB DEFINITIONS ::= BEGIN - -IMPORTS - NOTIFICATION-TYPE, NOTIFICATION-GROUP - FROM test2; - - -snmpMIB MODULE-IDENTITY - LAST-UPDATED "2021060900Z" - ORGANIZATION "testing" - CONTACT-INFO - "EMail: testing@emai.com" - DESCRIPTION - "MIB module for testing snmp_trap plugin - for telegraf - " - ::={ coldStart 1 } - -snmpMIBObjects OBJECT IDENTIFIER ::= { snmpMIB 1 } - -system OBJECT IDENTIFIER ::= { sysUpTimeInstance 1 } - -coldStart NOTIFICATION-TYPE - STATUS current - DESCRIPTION - "A coldStart trap signifies that the SNMP entity, - supporting a notification originator application, is - reinitializing itself and that its configuration may - have been altered." - ::= { snmpTraps 1 } - -snmpBasicNotificationsGroup NOTIFICATION-GROUP - NOTIFICATIONS { coldStart, authenticationFailure } - STATUS current - DESCRIPTION - "The basic notifications implemented by an SNMP entity - supporting command responder applications." - ::= { snmpMIBGroups 7 } - -END diff --git a/plugins/inputs/snmp_trap/testdata/test2 b/plugins/inputs/snmp_trap/testdata/test2 deleted file mode 100644 index e4950b902d803..0000000000000 --- a/plugins/inputs/snmp_trap/testdata/test2 +++ /dev/null @@ -1,97 +0,0 @@ -SNMPv2-MIB DEFINITIONS ::= BEGIN - -org OBJECT IDENTIFIER ::= { iso 3 } -- "iso" = 1 -dod OBJECT IDENTIFIER ::= { org 6 } -internet OBJECT IDENTIFIER ::= { dod 1 } - -directory OBJECT IDENTIFIER ::= { internet 1 } - -mgmt OBJECT IDENTIFIER ::= { internet 2 } -sysUpTimeInstance OBJECT IDENTIFIER ::= { mgmt 1 } -transmission OBJECT IDENTIFIER ::= { sysUpTimeInstance 10 } - -experimental OBJECT IDENTIFIER ::= { internet 3 } - -private OBJECT IDENTIFIER ::= { internet 4 } -enterprises OBJECT IDENTIFIER ::= { private 1 } - -security OBJECT IDENTIFIER ::= { internet 5 } - -snmpV2 OBJECT IDENTIFIER ::= { internet 6 } - --- transport domains -snmpDomains OBJECT IDENTIFIER ::= { snmpV2 1 } - --- transport proxies -snmpProxys OBJECT IDENTIFIER ::= { snmpV2 2 } - --- module identities -coldStart OBJECT IDENTIFIER ::= { snmpV2 3 } - -NOTIFICATION-TYPE MACRO ::= -BEGIN - TYPE NOTATION ::= - ObjectsPart - "STATUS" Status - "DESCRIPTION" Text - ReferPart - - VALUE NOTATION ::= - value(VALUE NotificationName) - - ObjectsPart ::= - "OBJECTS" "{" Objects "}" - | empty - Objects ::= - Object - - | Objects "," Object - Object ::= - value(ObjectName) - - Status ::= - "current" - | "deprecated" - | "obsolete" - - ReferPart ::= - "REFERENCE" Text - | empty - - -- a character string as defined in section 3.1.1 - Text ::= value(IA5String) -END - -NOTIFICATION-GROUP MACRO ::= -BEGIN - TYPE NOTATION ::= - NotificationsPart - "STATUS" Status - "DESCRIPTION" Text - ReferPart - - VALUE NOTATION ::= - value(VALUE OBJECT IDENTIFIER) - - NotificationsPart ::= - "NOTIFICATIONS" "{" Notifications "}" - Notifications ::= - Notification - | Notifications "," Notification - Notification ::= - value(NotificationName) - - Status ::= - "current" - | "deprecated" - | "obsolete" - - ReferPart ::= - "REFERENCE" Text - | empty - - -- a character string as defined in [2] - Text ::= value(IA5String) -END - -END \ No newline at end of file