Merge pull request #46707 from corhere/replace-xattrs-with-paxrecords

pkg/archive, pkg/tarsum: replace use of Xattrs with PAXRecords

hack/validate/golangci-lint.yml

@@ -113,10 +113,6 @@ issues:
       path: "api/types/(volume|container)/"
       linters:
         - revive
-    # FIXME temporarily suppress these. See #39924
-    - text: "SA1019: .*\\.Xattrs has been deprecated since Go 1.10: Use PAXRecords instead"
-      linters:
-        - staticcheck
     # FIXME temporarily suppress these. See #39926
     - text: "SA1019: httputil.NewClientConn"
       linters:

pkg/archive/archive.go

@@ -481,6 +481,8 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro
 	return hdr, nil
 }
 
+const paxSchilyXattr = "SCHILY.xattr."
+
 // ReadSecurityXattrToTarHeader reads security.capability xattr from filesystem
 // to a tar header
 func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error {
@@ -493,15 +495,16 @@ func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error {
 	)
 	capability, _ := system.Lgetxattr(path, "security.capability")
 	if capability != nil {
-		length := len(capability)
 		if capability[versionOffset] == vfsCapRevision3 {
 			// Convert VFS_CAP_REVISION_3 to VFS_CAP_REVISION_2 as root UID makes no
 			// sense outside the user namespace the archive is built in.
 			capability[versionOffset] = vfsCapRevision2
-			length = xattrCapsSz2
+			capability = capability[:xattrCapsSz2]
 		}
-		hdr.Xattrs = make(map[string]string)
-		hdr.Xattrs["security.capability"] = string(capability[:length])
+		if hdr.PAXRecords == nil {
+			hdr.PAXRecords = make(map[string]string)
+		}
+		hdr.PAXRecords[paxSchilyXattr+"security.capability"] = string(capability)
 	}
 	return nil
 }
@@ -776,8 +779,12 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, o
 	}
 
 	var xattrErrs []string
-	for key, value := range hdr.Xattrs {
-		if err := system.Lsetxattr(path, key, []byte(value), 0); err != nil {
+	for key, value := range hdr.PAXRecords {
+		xattr, ok := strings.CutPrefix(key, paxSchilyXattr)
+		if !ok {
+			continue
+		}
+		if err := system.Lsetxattr(path, xattr, []byte(value), 0); err != nil {
 			if bestEffortXattrs && errors.Is(err, syscall.ENOTSUP) || errors.Is(err, syscall.EPERM) {
 				// EPERM occurs if modifying xattrs is not allowed. This can
 				// happen when running in userns with restrictions (ChromeOS).

pkg/archive/archive_linux.go

@@ -41,9 +41,7 @@ func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os
 			return nil, err
 		}
 		if len(opaque) == 1 && opaque[0] == 'y' {
-			if hdr.Xattrs != nil {
-				delete(hdr.Xattrs, "trusted.overlay.opaque")
-			}
+			delete(hdr.PAXRecords, paxSchilyXattr+"trusted.overlay.opaque")
 
 			// create a header for the whiteout file
 			// it should inherit some properties from the parent, but be a regular file

pkg/archive/archive_linux_test.go

@@ -1,6 +1,9 @@
 package archive // import "github.com/docker/docker/pkg/archive"
 
 import (
+	"archive/tar"
+	"bytes"
+	"io"
 	"os"
 	"path/filepath"
 	"syscall"
@@ -8,8 +11,10 @@ import (
 
 	"github.com/containerd/containerd/pkg/userns"
 	"github.com/docker/docker/pkg/system"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"golang.org/x/sys/unix"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
 )
 
@@ -103,11 +108,39 @@ func TestOverlayTarUntar(t *testing.T) {
 		Compression:    Uncompressed,
 		WhiteoutFormat: OverlayWhiteoutFormat,
 	}
-	archive, err := TarWithOptions(src, options)
+	reader, err := TarWithOptions(src, options)
+	assert.NilError(t, err)
+	archive, err := io.ReadAll(reader)
+	reader.Close()
 	assert.NilError(t, err)
-	defer archive.Close()
 
-	err = Untar(archive, dst, options)
+	// The archive should encode opaque directories and file whiteouts
+	// in AUFS format.
+	entries := make(map[string]struct{})
+	rdr := tar.NewReader(bytes.NewReader(archive))
+	for {
+		h, err := rdr.Next()
+		if err == io.EOF {
+			break
+		}
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(h.Devmajor, int64(0)), "unexpected device file in archive")
+		assert.Check(t, is.DeepEqual(h.PAXRecords, map[string]string(nil), cmpopts.EquateEmpty()))
+		entries[h.Name] = struct{}{}
+	}
+
+	assert.DeepEqual(t, entries, map[string]struct{}{
+		"d1/":                         {},
+		"d1/" + WhiteoutOpaqueDir:     {},
+		"d1/f1":                       {},
+		"d2/":                         {},
+		"d2/" + WhiteoutOpaqueDir:     {},
+		"d2/f1":                       {},
+		"d3/":                         {},
+		"d3/" + WhiteoutPrefix + "f1": {},
+	})
+
+	err = Untar(bytes.NewReader(archive), dst, options)
 	assert.NilError(t, err)
 
 	checkFileMode(t, filepath.Join(dst, "d1"), 0o700|os.ModeDir)

pkg/archive/archive_unix_test.go

@@ -254,6 +254,27 @@ func TestTarUntarWithXattr(t *testing.T) {
 	out, err := exec.Command("setcap", "cap_block_suspend+ep", filepath.Join(origin, "2")).CombinedOutput()
 	assert.NilError(t, err, string(out))
 
+	tarball, err := Tar(origin, Uncompressed)
+	assert.NilError(t, err)
+	defer tarball.Close()
+	rdr := tar.NewReader(tarball)
+	for {
+		h, err := rdr.Next()
+		if err == io.EOF {
+			break
+		}
+		assert.NilError(t, err)
+		capability, hasxattr := h.PAXRecords["SCHILY.xattr.security.capability"]
+		switch h.Name {
+		case "2":
+			if assert.Check(t, hasxattr, "tar entry %q should have the 'security.capability' xattr", h.Name) {
+				assert.Check(t, len(capability) > 0, "tar entry %q has a blank 'security.capability' xattr value")
+			}
+		default:
+			assert.Check(t, !hasxattr, "tar entry %q should not have the 'security.capability' xattr", h.Name)
+		}
+	}
+
 	for _, c := range []Compression{
 		Uncompressed,
 		Gzip,

pkg/tarsum/versioning.go

@@ -115,15 +115,18 @@ func v0TarHeaderSelect(h *tar.Header) (orderedHeaders [][2]string) {
 
 func v1TarHeaderSelect(h *tar.Header) (orderedHeaders [][2]string) {
 	// Get extended attributes.
-	xAttrKeys := make([]string, len(h.Xattrs))
-	for k := range h.Xattrs {
-		xAttrKeys = append(xAttrKeys, k)
+	const paxSchilyXattr = "SCHILY.xattr."
+	var xattrs [][2]string
+	for k, v := range h.PAXRecords {
+		if xattr, ok := strings.CutPrefix(k, paxSchilyXattr); ok {
+			xattrs = append(xattrs, [2]string{xattr, v})
+		}
 	}
-	sort.Strings(xAttrKeys)
+	sort.Slice(xattrs, func(i, j int) bool { return xattrs[i][0] < xattrs[j][0] })
 
 	// Make the slice with enough capacity to hold the 11 basic headers
 	// we want from the v0 selector plus however many xattrs we have.
-	orderedHeaders = make([][2]string, 0, 11+len(xAttrKeys))
+	orderedHeaders = make([][2]string, 0, 11+len(xattrs))
 
 	// Copy all headers from v0 excluding the 'mtime' header (the 5th element).
 	v0headers := v0TarHeaderSelect(h)
@@ -131,9 +134,7 @@ func v1TarHeaderSelect(h *tar.Header) (orderedHeaders [][2]string) {
 	orderedHeaders = append(orderedHeaders, v0headers[6:]...)
 
 	// Finally, append the sorted xattrs.
-	for _, k := range xAttrKeys {
-		orderedHeaders = append(orderedHeaders, [2]string{k, h.Xattrs[k]})
-	}
+	orderedHeaders = append(orderedHeaders, xattrs...)
 
 	return
 }