Log an error on SNMPv3 requests where authentication fails (#7929)#8215
Log an error on SNMPv3 requests where authentication fails (#7929)#8215DouglasHeriot wants to merge 1 commit intoinfluxdata:masterfrom
Conversation
…a#7929) gosnmp does not return any error when an SNMP get request fails due to authentication. The SNMP agent will return a "report" PDU instead of a "get-response" PDU in this case. A check has been added to verify the packet's PDUType is not "Report"
|
@sjwang90 or @ssoroka Can you link issue #7929 to this PR? |
|
Hello @DouglasHeriot, Are you still planning to add more commits? Since this is still a PR in Draft state, it will not be considered for merging. If there is no response soon, the PR will be closed as we are trying to clean up the back-log. |
|
Thanks for your response, but shouldn’t there still be some logging if credentials are not correct? |
|
see #7929 - Issue still happens, and yes I think a log message would indeed be expected in telegraf.log |
|
Issue is not fixed, but this PR will. |
Hipska
left a comment
There was a problem hiding this comment.
I tested the code and would suggest this change for more meaningful error message.
| if pkt, err := gs.Get([]string{oid}); err != nil { | ||
| return nil, fmt.Errorf("performing get on field %s: %w", f.Name, err) | ||
| } else if pkt != nil && pkt.PDUType == gosnmp.Report { | ||
| return nil, fmt.Errorf("received SNMP report instead of response for field %s: check authentication?", f.Name) |
There was a problem hiding this comment.
| return nil, fmt.Errorf("received SNMP report instead of response for field %s: check authentication?", f.Name) | |
| _, _, oidText, _, _ := SnmpTranslate(pkt.Variables[0].Name) | |
| return nil, fmt.Errorf("recieved %s report", strings.TrimSuffix(oidText, ".0")) |
|
@DouglasHeriot Are you willing to pick this up? Otherwise I will create my own PR. I want this to get resolved ASAP. |
gosnmp does not return any error when an SNMP get request fails due to authentication.
The SNMP agent will return a "report" PDU instead of a "get-response" PDU in this case.
A check has been added to verify the packet's PDUType is not "Report"
I have not yet added documentation or unit tests.
Required for all PRs: