Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix tar PAX format handling #1414

Merged
merged 2 commits into from Jul 22, 2022
Merged

Fix tar PAX format handling #1414

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7761a6b
fix tar PAX handling on Windows
ondrej-smola Jul 21, 2022
05c84da
always force PAX format
ondrej-smola Jul 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion pkg/v1/mutate/mutate.go
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import (
"io"
"io/ioutil"
"path/filepath"
"runtime"
"strings"
"time"

Expand Down Expand Up @@ -244,6 +245,8 @@ func extract(img v1.Image, w io.Writer) error {
if err != nil {
return fmt.Errorf("retrieving image layers: %w", err)
}

isWin := runtime.GOOS == "windows"
// we iterate through the layers in reverse order because it makes handling
// whiteout layers more efficient, since we can just keep track of the removed
// files as we see .wh. layers and ignore those in previous layers.
Expand All @@ -268,6 +271,14 @@ func extract(img v1.Image, w io.Writer) error {
// name, we may have duplicate entries, which angers tar-split.
header.Name = filepath.Clean(header.Name)

// tar.Next() sometimes mistakenly guesses format as USTAR, which creates a problem:
// if the header.Name is > 100 characters long, WriteHeader() returns an error like
// "archive/tar: cannot encode header: Format specifies USTAR; and USTAR cannot encode Name=...".
// To fix, change format from USTAR to PAX on Windows
if isWin && header.Format == tar.FormatUSTAR {
header.Format = tar.FormatPAX
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. This seems like the kind of thing I'd expect archive/tar to handle for us... :-/

Can you add a link to the issue in the comment, and maybe any other helpful issues you've found while debugging this? That will help remind me better why this is here when I rediscover it in six months, or if I have the same Windows-specific issue elsewhere and come back here to try to remember what the deal was.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also viable path that linuxkit chose - to force PAX on output linuxkit/linuxkit#3141
Maybe force PAX and document it using reasoning in pull request above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like here (with linked pull request)(https://github.com/linuxkit/linuxkit/blob/master/src/cmd/linuxkit/moby/image.go#L216)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be okay with just always setting this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM 👍


basename := filepath.Base(header.Name)
dirname := filepath.Dir(header.Name)
tombstone := strings.HasPrefix(basename, whiteoutPrefix)
Expand Down Expand Up @@ -297,7 +308,9 @@ func extract(img v1.Image, w io.Writer) error {
// any entries with a matching (or child) name
fileMap[name] = tombstone || !(header.Typeflag == tar.TypeDir)
if !tombstone {
tarWriter.WriteHeader(header)
if err := tarWriter.WriteHeader(header); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

return err
}
if header.Size > 0 {
if _, err := io.CopyN(tarWriter, tarReader, header.Size); err != nil {
return err
Expand Down