fix(image): truncate the output file of save and export - #5160
Merged
Conversation
`nerdctl save -o FILE` and `nerdctl export -o FILE` open the output with O_CREATE|O_WRONLY and no O_TRUNC. Writing a smaller archive over a bigger one therefore leaves the tail of the bigger one past the end of the new archive. A tar reader stops at the end-of-archive marker, so `nerdctl load` reads such a file without complaining. What is wrong is the artifact: it is larger than the archive it is supposed to hold, and the extra bytes belong to an unrelated image or container, which shows up in anything that checksums the file, accounts for its size, or ships it somewhere. `docker save` replaces the destination wholesale, writing through a temporary file and renaming it. Add O_TRUNC, so that `-o` replaces the file as a shell redirect does. `pkg/healthcheck/log.go` opens a file with the same flags, but seeks to the end and appends on purpose, so it is left alone. Fixes containerd#5159 Signed-off-by: Eugene Kalinin <e.v.kalinin@gmail.com>
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
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.
nerdctl save -o FILEandnerdctl export -o FILEopen the output withos.O_CREATE|os.O_WRONLYand noos.O_TRUNC. Writing a smaller archive over a bigger onetherefore leaves the tail of the bigger one past the end of the new archive.
wrote 10 bytes, file holds 30: "BBBBBBBBBBAAAAAAAAAAAAAAAAAAAA"This does not break
nerdctl load: a tar reader stops at the end-of-archive marker and neverlooks at what follows, which I checked with both
archive/tarand the systemtar. What iswrong is the artifact — it is larger than the archive it is supposed to hold, and the extra
bytes belong to an unrelated image or container, which shows up in anything that checksums the
file, accounts for its size, or ships it somewhere.
docker savereplaces the destination wholesale, writing through a temporary file and renamingit (
atomicwriter.Newin docker/cli). AddingO_TRUNCgets nerdctl to the same observableresult without a new dependency:
pkg/internal/filesystemis built for state files, withlocking and backups, not for streaming an archive that can be many gigabytes.
pkg/healthcheck/log.goopens a file with the same flags, but seeks to the end and appends onpurpose, so it is left alone.
Tests
TestSaveReplacesExistingFileandTestExportReplacesExistingFilewrite the bigger archivefirst and the smaller one over it, then assert the file shrank. They compare against the size
measured before the overwrite rather than against a separately saved copy, so they do not depend
on two saves of the same image being byte-identical.
Both assert on the size rather than on
loadfailing, since as above a reader would not noticethe tail.
Fixes #5159