Fixes #39368 Fix race during concurrent autosign requests#945
Fixes #39368 Fix race during concurrent autosign requests#945alexjfisher wants to merge 1 commit into
Conversation
Concurrent autosign requests could race while updating the token whitelist. Although writes were locked, the read and modification happened before the write lock was taken, so one request could observe an empty file or overwrite another request's update.
adamruzicka
left a comment
adamruzicka
left a comment
There was a problem hiding this comment.
This looks sane. At what scale did you start seeing those conflicts?
| # These helpers must only be called with a locked file handle from #lock. | ||
| def unsafe_read(file) | ||
| file.rewind | ||
| YAML.safe_load(file.read, fallback: []) |
There was a problem hiding this comment.
Since we're trying to be as defensive as possible, should we also check the type we get and reset to empty array if we get something unexpected?
There was a problem hiding this comment.
If the file is empty, it seemed reasonable to return an empty array, but I'm not too sure about silently fixing corrupt data??
~ 50 hosts. In our use-case the API is being called at the terraform plan phase, with the actual concurrency being governed by terraform's (I'm separately considering a follow-up PR that'd do some pruning of expired, but never used tokens). |
2 participants
Concurrent autosign requests could race while updating the token whitelist. Although writes were locked, the read and modification happened before the write lock was taken, so one request could observe an empty file or overwrite another request's update.