Skip to content

feat(snmp): add SNMPv3 noAuthNoPriv support with backend test#6552

Merged
CommanderStorm merged 19 commits intolouislam:masterfrom
querysmith-sys:feat/snmp-v3
Jan 18, 2026
Merged

feat(snmp): add SNMPv3 noAuthNoPriv support with backend test#6552
CommanderStorm merged 19 commits intolouislam:masterfrom
querysmith-sys:feat/snmp-v3

Conversation

@querysmith-sys
Copy link
Copy Markdown
Contributor

Summary

Adds basic SNMPv3 support using the noAuthNoPriv security level.

Changes

  • Backend support for SNMPv3 (noAuthNoPriv)
  • UI option to select SNMPv3 and provide username
  • Database field for SNMPv3 username
  • Backend test using node:test to verify SNMPv3 session creation (stubs net-snmp)

Testing

  • Verified UI changes locally
  • All tests pass locally

Related to #6515

Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left a few comments below, needs a bit of cleanup

*/

describe("SNMP Monitor Type", () => {
test("SNMPv3 noAuthNoPriv uses createV3Session", async() => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an incredibly brittle test.
Can we instead use a testcontainer, spin something up with snmv3 and test this way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

— this makes sense.
I’ll revisit the test approach and follow up (likely with a more robust integration-style test) tomorrow.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll replace it with an integration-style test using testcontainers. The plan is to spin up a real SNMPv3 agent (e.g. polinux/snmpd) configured for noAuthNoPriv, run the SNMP monitor against it, and assert that the monitor successfully completes the check.
what do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great!

Copy link
Copy Markdown
Contributor Author

@querysmith-sys querysmith-sys Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SNMPv3 noAuthNoPriv – testing update
Quick update on the SNMPv3 testing work:
Tried replacing the mock with real integration tests using testcontainers:
polinux/snmpd
a custom Net-SNMP container (VACM configured, non-privileged port, SNMPv3 user)
In all cases, the Node net-snmp client consistently hits:
"RequestTimedOutError"
even though the agent starts correctly and binds to UDP.
This seems related to SNMPv3 engineID discovery / engineBoots + VACM behavior in short-lived CI containers, where agent state resets on each start and requests get silently dropped.
It is solvable with more setup (persistent engine state, fixed engineID, long-running agent), but that feels heavy and brittle for CI.
Reference I looked into:
https://network-king.net/fix-snmp-timeout-errors-complete-snmp-v2-vs-v3-solution-guide/
Would you prefer:
Keeping SNMPv3 covered by unit tests + docs, or
Using SNMPv2c for CI integration tests and documenting SNMPv3 CI limitations?

ok to go with whatever you think makes the most sense.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea about SNMP.

If you think SNMPv3 is too britte for CI, then lets just add SNMPv2c for CI.
I would love to have some way of knowing that this works though, even if it is an older version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that SNMPv3 noAuthNoPriv does work end-to-end when running on Linux; the failures I was seeing were specific to Windows (Docker/Testcontainers UDP limitations), so adding skip: win32 makes the test pass locally.
That said, SNMPv3 integration still depends on engineID discovery and persistent USM state, which makes it more timing- and environment-sensitive in CI, so I’ve kept SNMPv3 covered via a unit test and used SNMPv2c for CI-safe integration coverage as discussed.

@CommanderStorm CommanderStorm added the pr:please address review comments this PR needs a bit more work to be mergable label Dec 31, 2025
@CommanderStorm CommanderStorm marked this pull request as draft January 1, 2026 07:00
@querysmith-sys querysmith-sys marked this pull request as ready for review January 4, 2026 03:18
CommanderStorm

This comment was marked as resolved.

@querysmith-sys
Copy link
Copy Markdown
Contributor Author

I have addressed the changes you pointed, if any further changes required tell me.

@querysmith-sys querysmith-sys marked this pull request as ready for review January 14, 2026 07:13
Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@CommanderStorm CommanderStorm enabled auto-merge (squash) January 17, 2026 10:52
@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 17, 2026
@CommanderStorm CommanderStorm removed this from the 2.1.0 milestone Jan 17, 2026
Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is still some work to do. Could you fix the testcases?

test at test/backend-test/test-snmp.js:9:5
✖ check() sets heartbeat to UP when SNMP agent responds (1.532224ms)
  TypeError: Cannot read properties of undefined (reading 'toString')
      at GenericContainer.withExposedPorts (/home/runner/work/uptime-kuma/uptime-kuma/node_modules/testcontainers/build/generic-container/generic-container.js:260:67)
      at TestContext.<anonymous> (/home/runner/work/uptime-kuma/uptime-kuma/test/backend-test/test-snmp.js:16:75)
      at Test.runInAsyncScope (node:async_hooks:214:14)
      at Test.run (node:internal/test_runner/test:1106:25)
      at Test.start (node:internal/test_runner/test:1003:17)
      at node:internal/test_runner/test:1516:71
      at node:internal/per_context/primordials:466:82
      at new Promise (<anonymous>)
      at new SafePromise (node:internal/per_context/primordials:435:3)
      at node:internal/per_context/primordials:466:9
node:internal/errors:985
  const err = new Error(message);
              ^

Error: Command failed: npm run test-backend-22
    at genericNodeError (node:internal/errors:985:15)
    at wrappedFn (node:internal/errors:539:14)
    at checkExecSyncError (node:child_process:925:11)
    at Module.execSync (node:child_process:997:15)
    at file:///home/runner/work/uptime-kuma/uptime-kuma/test/test-backend.mjs:11:18
    at ModuleJob.run (node:internal/modules/esm/module_job:413:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:660:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:101:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 2798,
  stdout: null,
  stderr: null
}

@CommanderStorm CommanderStorm marked this pull request as draft January 17, 2026 14:28
auto-merge was automatically disabled January 17, 2026 14:28

Pull request was converted to draft

@querysmith-sys
Copy link
Copy Markdown
Contributor Author

querysmith-sys commented Jan 17, 2026

i need your opinion that as testcontainer version 10.13.1 does not support UDP at all, i tried many ways still failed and it seems that only version above 11.5.0 have the way to do it in simplest and with easy syntax and more maintainable(i tried with v11.5.0).

there is another way which is bypass "testcontainers and use Docker CLI" directly (it works but very brittle,

  • Less maintainable (uses child_process.exec for docker commands)
  • Requires Docker CLI in PATH
  • Manual cleanup handling).

i recommend to go with 11.5.0+ version. also the earlier failed test case was because 10.13.1 does not have native support for port to be "161/udp", which version 11.5.0 have.

reference:
testcontainers/testcontainers-node#556

@CommanderStorm
Copy link
Copy Markdown
Collaborator

fair, lets upgrade that then.

@querysmith-sys querysmith-sys marked this pull request as ready for review January 17, 2026 19:06
@github-actions github-actions bot added pr:needs review this PR needs a review by maintainers or other community members and removed pr:please address review comments this PR needs a bit more work to be mergable labels Jan 17, 2026
@querysmith-sys
Copy link
Copy Markdown
Contributor Author

Thanks for confirming 👍
I’ve upgraded testcontainers to 11.5.0, fixed the SNMP UDP test accordingly, and all CI checks are now passing.

Could you please take another look and update the review when you have time?

Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@CommanderStorm CommanderStorm merged commit 30ee8ce into louislam:master Jan 18, 2026
26 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

@querysmith-sys congrats on your first contribution to Uptime Kuma! 🐻
We hope you enjoy contributing to our project and look forward to seeing more of your work in the future! If you want to see your contribution in action, please see our nightly builds here.

@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 18, 2026
@querysmith-sys
Copy link
Copy Markdown
Contributor Author

Thanks for the review and merge! Really appreciate it 🙌
Had a great time learning through this — totally worth it.

@querysmith-sys querysmith-sys deleted the feat/snmp-v3 branch January 18, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:needs review this PR needs a review by maintainers or other community members

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants