Skip to content

fix: failed renames in library forms, and two review follow-ups - #32

Merged
floreabogdan merged 3 commits into
mainfrom
fix/library-form-rename
Oct 5, 2026
Merged

floreabogdan merged 3 commits into
mainfrom
fix/library-form-rename

Conversation

@floreabogdan

Copy link
Copy Markdown
Owner

Found by the final review of #30 before the release.

A failed rename in a library form could overwrite another object

A prefix set, AS set, community, RPKI server, BMP station or policy rename that failed validation re-rendered the form posting to the name the operator had typed. When that name belonged to a sibling (the usual reason a rename is refused), the corrected resubmit edited the sibling: it took the first object's contents and the new name, and the first object stayed as it was.

Example: rename prefix set CUST_A to CUST_C, which exists. The save is refused, but the form now posts to /library/prefix-sets/CUST_C/edit. Fix the name to CUST_D and save: CUST_C becomes CUST_D with A's prefixes, and CUST_A is unchanged. C's prefixes are gone.

The six forms now post back to the name the object is stored under (StoredName), as the peer and template forms already do since #21/#30. This bug predates the templates work. It is in v0.5.0 too. The CHANGELOG entry for the peer-form rename fix now covers every form and the overwrite.

Review follow-ups for #30

  • Import from BIRD listed a session named like a peer template as importable, then skipped it on save with no reason. The row now shows can't import and gives the reason, using the same check as the save.
  • Tests for two fix: peer template review follow-ups #30 fixes that had none: a failed template delete only says "detach those peers" when peers are linked, and a community delete is refused when its in-use check cannot run. Neither had a test that fails on revert. sabotageDB changes the test database behind the store's back (an aborting trigger, a renamed table) to reach both paths.

Not changed (noted in review, judged not worth it)

  • Derived names are checked one way only. A template named ebgp_in_X is refused while peer X exists, but creating peer X while that template exists is not refused at save. The render-level duplicate check refuses it before any apply, with a message naming both, and such a template name is contrived.
  • A community can no longer take rpki4, rpki6, bfd1, static_v4, static_v6 or device1, even when that object isn't rendered, so an existing community with one of those names can't be re-saved unchanged. These names are contrived.

Test plan

  • TestFailedLibraryRenamePostsBackToTheStoredName: for each of the six forms, a refused rename onto a sibling's name, then the corrected resubmit to the form's own action. Before the fix, all six renamed the sibling. After it, the right object is renamed and the sibling is untouched.
  • TestSeedPageFlagsASessionNamedLikeATemplate fails before the fix and passes after.
  • TestAnUnlinkedTemplatesFailedDeleteDoesNotSayDetach and TestCommunityDeleteRefusesWhenTheUseCheckFails each fail with its fix reverted.
  • Full local CI replica (Actions is still locked by billing): results in a comment.

🤖 Generated with Claude Code

floreabogdan and others added 3 commits October 5, 2026 13:22
…d name

A prefix set, AS set, community, RPKI server, BMP station or policy
rename that failed validation re-rendered the form posting to the name
the operator had typed. When that name belonged to a sibling (the usual
reason a rename is refused), the corrected resubmit edited the sibling:
it took the first object's contents and the new name, and the first
object stayed as it was. The forms now post back to the name the object
is stored under, as the peer and template forms already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Neither fix had a test that fails when it is reverted: the detach hint
was only checked where it belongs, and nothing made the community in-use
check fail. sabotageDB changes the test database behind the store's back
(a trigger that aborts the delete, a renamed table) to reach both paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Import from BIRD listed such a session as importable, then skipped it on
save with only its name in the "skipped" list. The page now runs the
same template-name check as the save and shows the row as "can't import"
with the reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@floreabogdan

Copy link
Copy Markdown
Owner Author

Local CI replica on 6a01af0 (Actions is still locked by billing), Go 1.26.8: gofmt ✅ · go vet (+ -tags integration) ✅ · golangci-lint v2.1.6 ✅ · go test -race ./... ✅ (all packages) · govulncheck ✅ (no reachable vulnerabilities) · CGO_ENABLED=0 build ✅ · integration against BIRD 2.14 ✅ (TestIntegrationRenderedConfigParsesInBird, TestIntegrationBFDTimersParseInBird).

Independent review found no blockers or major issues. It confirmed:

  • Each of the six forms re-renders only from new, GET-edit and failed-save. No other link, button or script on them builds a URL from the typed name.
  • Each new test fails with its fix reverted.
  • sabotageDB showed no flakiness or races over 1,800 stress runs plus -race and shuffled package runs.

Left as is (minor):

  • If the template-name check errors during an import (a transient DB error between page load and save), the save returns a bare 500, and sessions imported earlier in the same request stay created. This came in with fix: peer template review follow-ups #30. The page now marks such a row "can't import", so the window is narrow.
  • After a failed rename, the title and heading show the typed name, as the peer form does. The form posts to the stored name.

@floreabogdan
floreabogdan merged commit 48afe8b into main Oct 5, 2026
0 of 2 checks passed
@floreabogdan
floreabogdan deleted the fix/library-form-rename branch October 5, 2026 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant