feat(auth-keys): rename a label across all keys - #94
Conversation
Add PUT /admin/auth-keys/labels/rename, which renames one label on every in-scope key that carries it (merging with an existing label), plus a Label Management section on the dashboard API Keys page with per-label key counts and a rename dialog. Labels are metadata only: key material, authentication, and key validity are never touched.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
weselben
left a comment
weselben
left a comment
There was a problem hiding this comment.
Review of feat(auth-keys): rename a label across all keys.
Hard-rule validation passed. go test ./... green, make test-dashboard green (643), svelte-check clean.
Findings: 0 red, 2 yellow, 1 blue. The two yellow items are inline.
- 🔵 docs:
docs/features/labelling.mdxdocuments create and edit of API key labels only. The new mass-rename section and thePUT /admin/auth-keys/labels/renameendpoint are missing there and from the endpoint list indocs/advanced/admin-endpoints.mdx. Add a rename section.
Verdict is yours — this is a comment-only review.
| scope := requestScope(c) | ||
| ctx := c.Request().Context() | ||
| renamed := 0 | ||
| for _, view := range h.authKeys.ListViews() { |
There was a problem hiding this comment.
🟡 risk: TOCTOU between the ListViews() snapshot and the full-replace UpdateLabels. A concurrent per-key label edit that lands in that window is silently overwritten. Same shape as the existing full-replace endpoints, but the batch loop widens the window to N keys. No fix required beyond awareness; consider a per-key version check or CAS if the store supports one.
| // The key disappeared between listing and updating; skip it. | ||
| continue | ||
| } | ||
| return handleError(c, authKeyWriteError(err)) |
There was a problem hiding this comment.
🟡 risk: the rename loop is non-atomic. A mid-loop UpdateLabels failure returns an error after earlier keys were already renamed, and the error response says nothing about how far it got. The rename is idempotent, so a retry converges — document this in the handler comment, or return the renamed count even on failure.
1 participant
TL;DR
Renaming a label meant editing every API key that carries it, one by one —
tedious and error-prone with many keys. A new
PUT /admin/auth-keys/labels/renameendpoint renames one label on every in-scope key in one call, and the dashboard
API Keys page gains a Label Management section with per-label key counts and a
rename dialog.
Files to review (14, +524 / -0):
internal/admin/handler_authkeys.go(start here)RenameAuthKeyLabelhandler: scope filter, exact label match, per-keyUpdateLabelsreuse.internal/admin/handler_authkeys_test.gointernal/admin/routes.goPUT /admin/auth-keys/labels/rename.internal/admin/routes_test.goweb/dashboard/src/pages/auth-keys/AuthKeyLabelManager.svelte(new)web/dashboard/src/pages/auth-keys/AuthKeyLabelRenameDialog.svelte(new)web/dashboard/src/pages/auth-keys/authKeys.svelte.jslabelRenamestate and the submit action calling the new endpoint.web/dashboard/src/pages/auth-keys/authKeysLogic.jsdistinctAuthKeyLabelspure function.web/dashboard/src/pages/auth-keys/AuthKeysPage.svelteweb/dashboard/messages/{en,de,pl,zh-CN}.jsonweb/dashboard/tests/auth-keys.test.jsdistinctAuthKeyLabels.How
and replaces the exact label match via the existing per-key
UpdateLabelsservice path. No store interface change.
Key material, authentication, and key validity never change — labels are
metadata only.
from == toand empty values return 400. A label with no matches returnsrenamed: 0, not an error.Reviewer notes
PUT /admin/auth-keys/labels/renamebody{"from": "team-a", "to": "team-b"}→{"from": "...", "to": "...", "renamed": 3}.RenameAuthKeyLabel— reuse of the per-key update path is deliberate.Tests
go test ./...— all packages pass, including the new handler tests.make test-dashboard— 643 pass (newdistinctAuthKeyLabelstests included).This PR description was generated with AI assistance.