refactor: Add pkg/fileutil and move fstab writes onto it - #314
Draft
lauri-amd wants to merge 1 commit into
Draft
Conversation
Replacing a file in place truncates it for the duration of the write, so an interrupt in that window leaves it empty. fstab.go had grown a private atomicWriteFile to avoid exactly that, and an in-flight fix for the ephemeral ssh key handling needs the same guarantee for authorized_keys, where an emptied file locks the operator out of the node. Promote the helper into pkg/fileutil rather than keep a second copy of it, and add the one capability fstab never needed: WriteAtomicallyOwned sets uid and gid, because authorized_keys has to keep its owner. WriteAtomically is the mode-only form, and /etc/fstab and its backup now use it. Two details changed in the move. The mode is applied after the write rather than before, so content is never briefly readable at a wider mode than intended, and the temporary file is removed on every error path instead of only those before the rename. The tests assert the guarantee deterministically instead of racing the writer: a reader that opened the file beforehand must still see the complete old content, and the target must be a different inode afterwards. pkg/fileutil carries no build tag, because pkg/ssh is untagged and so cannot import the linux-only runtime package. The ssh key handling moves onto pkg/fileutil in a follow-up PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replacing a file in place truncates it for the duration of the write, so an interrupt in that window leaves it empty. fstab.go had grown a private atomicWriteFile to avoid exactly that, and an in-flight fix for the ephemeral ssh key handling needs the same guarantee for authorized_keys, where an emptied file locks the operator out of the node.
Promote the helper into pkg/fileutil rather than keep a second copy of it, and add the one capability fstab never needed: WriteAtomicallyOwned sets uid and gid, because authorized_keys has to keep its owner. WriteAtomically is the mode-only form, and /etc/fstab and its backup now use it.
Two details changed in the move. The mode is applied after the write rather than before, so content is never briefly readable at a wider mode than intended, and the temporary file is removed on every error path instead of only those before the rename. The tests assert the guarantee deterministically instead of racing the writer: a reader that opened the file beforehand must still see the complete old content, and the target must be a different inode afterwards.
pkg/fileutil carries no build tag, because pkg/ssh is untagged and so cannot import the linux-only runtime package. The ssh key handling moves onto pkg/fileutil in a follow-up PR.