mirror of
https://github.com/optim-enterprises-bv/vault.git
synced 2025-10-29 17:52:32 +00:00
Fix race in PKI's runUnifiedTransfer (#20701)
* Fix race in PKI's runUnifiedTransfer During this race, we'll sometimes start (or fail to start) an additional unified transfer if the updated last run timestamp was written at the same time as another thread was reading it. Instead, delay this check until we're holding the CAS guard; this will occasionally result in more messages saying that an existing process is already running, but otherwise shouldn't impact the functionality at all. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
@@ -53,15 +53,6 @@ func runUnifiedTransfer(sc *storageContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !status.lastRun.IsZero() {
|
||||
// We have run before, we only run again if we have
|
||||
// been requested to forceRerun, and we haven't run since our
|
||||
// minimum delay
|
||||
if !(status.forceRerun.Load() && time.Since(status.lastRun) < minUnifiedTransferDelay) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if !config.UnifiedCRL {
|
||||
// Feature is disabled, no need to run
|
||||
return
|
||||
@@ -80,6 +71,17 @@ func runUnifiedTransfer(sc *storageContext) {
|
||||
}
|
||||
defer status.isRunning.Store(false)
|
||||
|
||||
// Because access to lastRun is not locked, we need to delay this check
|
||||
// until after we grab the isRunning CAS lock.
|
||||
if !status.lastRun.IsZero() {
|
||||
// We have run before, we only run again if we have
|
||||
// been requested to forceRerun, and we haven't run since our
|
||||
// minimum delay.
|
||||
if !(status.forceRerun.Load() && time.Since(status.lastRun) < minUnifiedTransferDelay) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Reset our flag before we begin, we do this before we start as
|
||||
// we can't guarantee that we can properly parse/fix the error from an
|
||||
// error that comes in from the revoke API after that. This will
|
||||
|
||||
3
changelog/20701.txt
Normal file
3
changelog/20701.txt
Normal file
@@ -0,0 +1,3 @@
|
||||
```release-notes:bug
|
||||
secrets/pki: Fix race during runUnifiedTransfer when deciding to skip re-running a test within a short window.
|
||||
```
|
||||
Reference in New Issue
Block a user