fix(snmp): ask for the old community per device, not once - #29
Merged
Conversation
--old prompted once and tested that one string against every device. That was right when a single community was shared across all four. It is not right now: each device has its own predecessor, so one string proves nothing about the three it never belonged to, and there was no way to check a device whose old community differed from the rest. Each device that passed its current-community check is now asked separately. Enter skips one, because the usual case is checking the single device you just rotated. Skipping every device is an error rather than a silent pass — a run that checked nothing must not print "all SNMP targets verified". The prompt reads from /dev/tty explicitly: the loop's stdin is the inventory, so a bare `read` would have consumed a device row instead of waiting for input. The terminal requirement is unchanged, and each community still reaches net-snmp only through a 0600 snmp.conf that is removed on every exit path. Verified against all three branches: a wrong community reports `rejected`, a community the device still accepts is caught as `STILL ACCEPTED` and fails the run, and skipping everything refuses to claim success. Refs #9 Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #28, which noted this as deliberately out of scope.
What changed
--oldprompted once and tested that single string against every device. It nowasks each device separately.
Why
One prompt was correct when a single community was shared across all four
devices. After the rotation each has its own predecessor, so:
belonged to, and
the rest — which is exactly the case for
neo.Enter skips a device, since the usual case is checking the one you just rotated.
Skipping every device is now an error rather than a silent pass: a run that
checked nothing must not print
all SNMP targets verified.Implementation note
The prompt reads from
/dev/ttyexplicitly. The loop’s stdin is the deviceinventory, so a bare
readwould have consumed a device row instead of waitingfor input — the loop would have silently eaten its own work list.
The terminal requirement is unchanged, and each community still reaches net-snmp
only through a 0600
snmp.confinside a 0700 directory removed on every exitpath, including Ctrl-C.
Blast radius
One script and one runbook section. No config, no service, no credential, no
segmentation change.
secrets/*.sops.yamlVerification
All three branches exercised against the live lab, through a pty so the terminal
requirement stayed honest.
A deliberately wrong community — correctly reported as refused:
A community the device does still accept — correctly caught, run fails:
Every device skipped — refuses to claim success:
shellcheckclean,./scripts/validate.shpasses,markdownlint-cli2clean.make validatepassesdocs/runbooks/rotate-snmp-community.md§2.5)Refs #9