-
Notifications
You must be signed in to change notification settings - Fork 121
feat(deletion): Add soft-deletion with recovery and audit trail #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Configurable soft-delete with retention periods - Undelete recovery for accidentally deleted links - DeletedBy audit trail tracking who deleted links - Automatic schema migrations for old databases - /.deleted endpoint for link recovery The soft-deletion pattern provides a foundation for scaling golink across multiple instances with active/passive or active/active deployments. Signed-off-by: Arun Philip <[email protected]>
b06e8f0 to
67dae31
Compare
mikeodr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR and the idea.
There's a lot to unpack here:
- Unused code
- Unrelated changes
- Repetitive test code
This PR isn't ready in its current state, please clean it up and simplify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What or why did the SVG change? It's not called out in the commit description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bunch of formatting changes unrelated to the update here. Can you clean up the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I'm struggling to parse out formatting changes from updates.
| ) | ||
|
|
||
| // newTestDB creates a new SQLiteDB with a test clock for deterministic time control. | ||
| func newTestDB(t *testing.T, initialTime time.Time) (*SQLiteDB, *tstest.Clock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is unused.
| t.Errorf("db.LoadStats got %v, want %v", got, want) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of these tests look repetitive they can be test case driven.
There is far too many sleeps here slowing up the tests.
Users can safely delete links with a configurable recovery window, and a DeletedBy audit trail tracks who deleted each link. Soft-deletion can also provides a foundation for scaling golink across multiple instances (read: running golink as a tailscale service).
Changes
/.deletedendpoint-deleted-retention,-cleanup-interval