sl: Fix bug with password set timestamp#28855
Conversation
This fixes a bug where if a user updates a Shadow Link with the same password the set at timestamp clears to Unix Epoch. Signed-off-by: Michael Boquard <michael@redpanda.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in Shadow Link password timestamp handling where updating a Shadow Link with the same password would incorrectly clear the password set timestamp to Unix Epoch. The fix ensures that when the password remains unchanged, the existing timestamp is preserved.
Key Changes:
- Modified the timestamp update logic to preserve the original timestamp when passwords match
- Added test coverage to verify the timestamp preservation behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/v/redpanda/admin/services/shadow_link/converter.cc | Added assignment to preserve password timestamp before checking if password changed |
| tests/rptest/tests/cluster_linking_topic_syncing_test.py | Added test case to verify password timestamp preservation when updating with same password |
| c.password_last_updated = from_creds.password_last_updated; | ||
| if (from_creds.password != c.password) { | ||
| c.password_last_updated = model::timestamp::now(); | ||
| return; |
There was a problem hiding this comment.
The assignment at line 1187 is unconditionally executed before the conditional check. If the password has changed (line 1188), the timestamp is set twice: first to from_creds.password_last_updated and then to now(). Consider moving line 1187 into an else block after the password comparison to avoid the redundant assignment when passwords differ.
| c.password_last_updated = from_creds.password_last_updated; | |
| if (from_creds.password != c.password) { | |
| c.password_last_updated = model::timestamp::now(); | |
| return; | |
| if (from_creds.password != c.password) { | |
| c.password_last_updated = model::timestamp::now(); | |
| return; | |
| } else { | |
| c.password_last_updated = from_creds.password_last_updated; |
Retry command for Build#77362please wait until all jobs are finished before running the slash command |
|
/ci-repeat 1 |
|
/backport v25.3.x |
This fixes a bug where if a user updates a Shadow Link with the same password the set at timestamp clears to Unix Epoch.
Backports Required
Release Notes
Bug Fixes