Preserve dir mode and ownership in RUN --mount

add.go

@@ -741,18 +741,18 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				renamedItems := 0
 				writer := io.WriteCloser(pipeWriter)
 				if renameTarget != "" {
-					writer = newTarFilterer(writer, func(hdr *tar.Header) (bool, bool, io.Reader) {
+					writer = newTarFilterer(writer, func(hdr *tar.Header) (tarFilterAction, bool, io.Reader) {
 						hdr.Name = renameTarget
 						renamedItems++
-						return false, false, nil
+						return tarFilterKeep, false, nil
 					})
 				}
 
 				if options.Parents {
 					parentsPrefixToRemove, parentsToSkip := getParentsPrefixToRemoveAndParentsToSkip(src, options.ContextDir)
-					writer = newTarFilterer(writer, func(hdr *tar.Header) (bool, bool, io.Reader) {
+					writer = newTarFilterer(writer, func(hdr *tar.Header) (tarFilterAction, bool, io.Reader) {
 						if slices.Contains(parentsToSkip, hdr.Name) && hdr.Typeflag == tar.TypeDir {
-							return true, false, nil
+							return tarFilterSkip, false, nil
 						}
 						hdr.Name = strings.TrimPrefix(hdr.Name, parentsPrefixToRemove)
 						hdr.Name = strings.TrimPrefix(hdr.Name, "/")
@@ -761,14 +761,14 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 							hdr.Linkname = strings.TrimPrefix(hdr.Linkname, "/")
 						}
 						if hdr.Name == "" {
-							return true, false, nil
+							return tarFilterSkip, false, nil
 						}
-						return false, false, nil
+						return tarFilterKeep, false, nil
 					})
 				}
-				writer = newTarFilterer(writer, func(_ *tar.Header) (bool, bool, io.Reader) {
+				writer = newTarFilterer(writer, func(_ *tar.Header) (tarFilterAction, bool, io.Reader) {
 					itemsCopied++
-					return false, false, nil
+					return tarFilterKeep, false, nil
 				})
 				getOptions := copier.GetOptions{
 					UIDMap:             srcUIDMap,

digester.go

@@ -6,13 +6,22 @@ import (
 	"fmt"
 	"hash"
 	"io"
+	"path"
 	"strings"
 	"sync"
 	"time"
 
 	digest "github.com/opencontainers/go-digest"
 )
 
+type tarFilterAction int
+
+const (
+	tarFilterKeep tarFilterAction = iota
+	tarFilterSkip
+	tarFilterDefer
+)
+
 type digester interface {
 	io.WriteCloser
 	ContentType() string
@@ -95,7 +104,7 @@ func (t *tarFilterer) Close() error {
 // Note: if "filter" indicates that a given item should be skipped, there is no
 // guarantee that there will not be a subsequent item of type TypeLink, which
 // is a hard link, which points to the skipped item as the link target.
-func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (skip, replaceContents bool, replacementContents io.Reader)) io.WriteCloser {
+func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (action tarFilterAction, replaceContents bool, replacementContents io.Reader)) io.WriteCloser {
 	pipeReader, pipeWriter := io.Pipe()
 	tarWriter := tar.NewWriter(writeCloser)
 	filterer := &tarFilterer{
@@ -105,44 +114,75 @@ func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (sk
 		filterer.closedLock.Lock()
 		closed := filterer.closed
 		filterer.closedLock.Unlock()
+		var deferred []*tar.Header
+		var tarReader *tar.Reader
+		writeEntry := func(hdr *tar.Header, replaceContents bool, replacementContents io.Reader) error {
+			if err := tarWriter.WriteHeader(hdr); err != nil {
+				return fmt.Errorf("writing tar header for %q: %w", hdr.Name, err)
+			}
+			if hdr.Size != 0 {
+				var n int64
+				var copyErr error
+				if replaceContents {
+					n, copyErr = io.CopyN(tarWriter, replacementContents, hdr.Size)
+				} else {
+					n, copyErr = io.Copy(tarWriter, tarReader)
+				}
+				if copyErr != nil {
+					return fmt.Errorf("copying content for %q: %w", hdr.Name, copyErr)
+				}
+				if n != hdr.Size {
+					return fmt.Errorf("filtering content for %q: expected %d bytes, got %d bytes", hdr.Name, hdr.Size, n)
+				}
+			}
+			if err := tarWriter.Flush(); err != nil {
+				return fmt.Errorf("flushing tar item padding for %q: %w", hdr.Name, err)
+			}
+			return nil
+		}
 		for !closed {
-			tarReader := tar.NewReader(pipeReader)
+			tarReader = tar.NewReader(pipeReader)
 			hdr, err := tarReader.Next()
 			for err == nil {
-				var skip, replaceContents bool
+				action := tarFilterKeep
+				var replaceContents bool
 				var replacementContents io.Reader
 				if filter != nil {
-					skip, replaceContents, replacementContents = filter(hdr)
+					action, replaceContents, replacementContents = filter(hdr)
 				}
-				if !skip {
-					if err = tarWriter.WriteHeader(hdr); err != nil {
-						err = fmt.Errorf("writing tar header for %q: %w", hdr.Name, err)
-						break
-					}
-					if hdr.Size != 0 {
-						var n int64
-						var copyErr error
-						if replaceContents {
-							n, copyErr = io.CopyN(tarWriter, replacementContents, hdr.Size)
+				switch action {
+				case tarFilterDefer:
+					hdrCopy := *hdr
+					deferred = append(deferred, &hdrCopy)
+				case tarFilterKeep:
+					// Emit deferred ancestors before the child
+					// because tar extractors create missing parent
+					// directories with default ownership otherwise.
+					nameSpec := path.Clean(strings.TrimRight(hdr.Name, "/"))
+					var remaining []*tar.Header
+					for _, d := range deferred {
+						deferredName := path.Clean(strings.TrimRight(d.Name, "/"))
+						if strings.HasPrefix(nameSpec, deferredName+"/") {
+							if err = writeEntry(d, false, nil); err != nil {
+								break
+							}
 						} else {
-							n, copyErr = io.Copy(tarWriter, tarReader)
-						}
-						if copyErr != nil {
-							err = fmt.Errorf("copying content for %q: %w", hdr.Name, copyErr)
-							break
-						}
-						if n != hdr.Size {
-							err = fmt.Errorf("filtering content for %q: expected %d bytes, got %d bytes", hdr.Name, hdr.Size, n)
-							break
+							remaining = append(remaining, d)
 						}
 					}
-					if err = tarWriter.Flush(); err != nil {
-						err = fmt.Errorf("flushing tar item padding for %q: %w", hdr.Name, err)
-						break
+					deferred = remaining
+
+					// Emit the child.
+					if err == nil {
+						err = writeEntry(hdr, replaceContents, replacementContents)
 					}
 				}
+				if err != nil {
+					break
+				}
 				hdr, err = tarReader.Next()
 			}
+			deferred = nil
 			if !errors.Is(err, io.EOF) {
 				filterer.err = fmt.Errorf("reading tar archive: %w", err)
 				break
@@ -174,12 +214,12 @@ type tarDigester struct {
 	tarFilterer io.WriteCloser
 }
 
-func modifyTarHeaderForDigesting(hdr *tar.Header) (skip, replaceContents bool, replacementContents io.Reader) {
+func modifyTarHeaderForDigesting(hdr *tar.Header) (action tarFilterAction, replaceContents bool, replacementContents io.Reader) {
 	zeroTime := time.Time{}
 	hdr.ModTime = zeroTime
 	hdr.AccessTime = zeroTime
 	hdr.ChangeTime = zeroTime
-	return false, false, nil
+	return tarFilterKeep, false, nil
 }
 
 func newTarDigester(contentType string) digester {

digester_test.go

@@ -123,9 +123,9 @@ func TestCompositeDigester(t *testing.T) {
 						}
 						if filtered {
 							// wrap the WriteCloser in another WriteCloser
-							hasher = newTarFilterer(hasher, func(hdr *tar.Header) (bool, bool, io.Reader) {
+							hasher = newTarFilterer(hasher, func(hdr *tar.Header) (tarFilterAction, bool, io.Reader) {
 								hdr.ModTime = zero
-								return false, false, nil
+								return tarFilterKeep, false, nil
 							})
 							require.NotNil(t, hasher, "newTarFilterer returned a null WriteCloser?")
 						}
@@ -192,7 +192,7 @@ func TestTarFilterer(t *testing.T) {
 		name          string
 		input, output map[string]string
 		breakAfter    int
-		filter        func(*tar.Header) (bool, bool, io.Reader)
+		filter        func(*tar.Header) (tarFilterAction, bool, io.Reader)
 	}{
 		{
 			name: "none",
@@ -216,7 +216,7 @@ func TestTarFilterer(t *testing.T) {
 				"file a": "content a",
 				"file b": "content b",
 			},
-			filter: func(*tar.Header) (bool, bool, io.Reader) { return false, false, nil },
+			filter: func(*tar.Header) (tarFilterAction, bool, io.Reader) { return tarFilterKeep, false, nil },
 		},
 		{
 			name: "skip",
@@ -227,7 +227,15 @@ func TestTarFilterer(t *testing.T) {
 			output: map[string]string{
 				"file a": "content a",
 			},
-			filter: func(hdr *tar.Header) (bool, bool, io.Reader) { return hdr.Name == "file b", false, nil },
+			filter: func(hdr *tar.Header) (tarFilterAction, bool, io.Reader) {
+				var action tarFilterAction
+				if hdr.Name == "file b" {
+					action = tarFilterSkip
+				} else {
+					action = tarFilterKeep
+				}
+				return action, false, nil
+			},
 		},
 		{
 			name: "replace",
@@ -242,13 +250,13 @@ func TestTarFilterer(t *testing.T) {
 				"file c": "content c",
 			},
 			breakAfter: 2,
-			filter: func(hdr *tar.Header) (bool, bool, io.Reader) {
+			filter: func(hdr *tar.Header) (tarFilterAction, bool, io.Reader) {
 				if hdr.Name == "file b" {
 					content := "content b+c"
 					hdr.Size = int64(len(content))
-					return false, true, strings.NewReader(content)
+					return tarFilterKeep, true, strings.NewReader(content)
 				}
-				return false, false, nil
+				return tarFilterKeep, false, nil
 			},
 		},
 	}

image.go

@@ -879,6 +879,8 @@ func (i containerImageRef) filterExclusionsByImage(ctx context.Context, exclusio
 				if exclusion.Owner != nil && (int64(exclusion.Owner.UID) != stat.UID && int64(exclusion.Owner.GID) != stat.GID) {
 					continue
 				}
+				exclusion.Mode = &stat.Mode
+				exclusion.Owner = &idtools.IDPair{UID: int(stat.UID), GID: int(stat.GID)}
 				paths = append(paths, exclusion)
 			}
 		}
@@ -1051,6 +1053,7 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, _ *types.SystemC
 		var rc io.ReadCloser
 		var errChan chan error
 		var layerExclusions []copier.ConditionalRemovePath
+		var layerPullUps []copier.EnsureParentPath
 		if i.confidentialWorkload.Convert {
 			// Convert the root filesystem into an encrypted disk image.
 			rc, err = i.extractConfidentialWorkloadFS(i.confidentialWorkload)
@@ -1086,14 +1089,13 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, _ *types.SystemC
 				if layerID == i.layerID {
 					// We need to filter out any mount targets that we created.
 					layerExclusions = append(slices.Clone(i.layerExclusions), i.layerMountTargets...)
-					// And we _might_ need to filter out directories that modified
-					// by creating and removing mount targets, _if_ they were the
-					// same in the base image for this stage.
-					layerPullUps, err := i.filterExclusionsByImage(ctx, i.layerPullUps, i.fromImageID)
+					// Parent directories that were modified by creating and
+					// removing mount targets should have their ownership
+					// and mode corrected rather than being excluded.
+					layerPullUps, err = i.filterExclusionsByImage(ctx, i.layerPullUps, i.fromImageID)
 					if err != nil {
 						return nil, fmt.Errorf("checking which exclusions are in base image %q: %w", i.fromImageID, err)
 					}
-					layerExclusions = append(layerExclusions, layerPullUps...)
 				}
 				// Extract this layer, one of possibly many.
 				rc, err = i.store.Diff("", layerID, diffOptions)
@@ -1137,7 +1139,7 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, _ *types.SystemC
 		// Use specified timestamps in the layer, if we're doing that for history
 		// entries.
 		nestedWriteCloser := ioutils.NewWriteCloserWrapper(writer, writeCloser.Close)
-		writeCloser, err = makeFilteredLayerWriteCloser(nestedWriteCloser, i.layerModTime, i.layerLatestModTime, layerExclusions, i.os == "windows")
+		writeCloser, err = makeFilteredLayerWriteCloser(nestedWriteCloser, i.layerModTime, i.layerLatestModTime, layerExclusions, layerPullUps, i.os == "windows")
 		if err != nil {
 			return nil, fmt.Errorf("creating filter write closer %s: %w", what, err)
 		}
@@ -1422,8 +1424,8 @@ func (i *containerImageRef) makeExtraImageContentDiff(includeFooter bool, timest
 // no later than layerLatestModTime (if a value is provided for it).
 // This implies that if both values are provided, the archive's timestamps will
 // be set to the earlier of the two values.
-func makeFilteredLayerWriteCloser(wc io.WriteCloser, layerModTime, layerLatestModTime *time.Time, exclusions []copier.ConditionalRemovePath, windows bool) (io.WriteCloser, error) {
-	if layerModTime == nil && layerLatestModTime == nil && len(exclusions) == 0 && !windows {
+func makeFilteredLayerWriteCloser(wc io.WriteCloser, layerModTime, layerLatestModTime *time.Time, exclusions []copier.ConditionalRemovePath, pullUps []copier.EnsureParentPath, windows bool) (io.WriteCloser, error) {
+	if layerModTime == nil && layerLatestModTime == nil && len(exclusions) == 0 && len(pullUps) == 0 && !windows {
 		return wc, nil
 	}
 	exclusionsMap := make(map[string]copier.ConditionalRemovePath)
@@ -1434,10 +1436,18 @@ func makeFilteredLayerWriteCloser(wc io.WriteCloser, layerModTime, layerLatestMo
 		}
 		exclusionsMap[pathSpec] = exclusionSpec
 	}
+	pullUpsMap := make(map[string]copier.EnsureParentPath)
+	for _, pullUpSpec := range pullUps {
+		pathSpec := strings.Trim(path.Clean(pullUpSpec.Path), "/")
+		if pathSpec == "" {
+			continue
+		}
+		pullUpsMap[pathSpec] = pullUpSpec
+	}
 	var initialized bool
-	wc = newTarFilterer(wc, func(hdr *tar.Header) (skip, replaceContents bool, replacementContents io.Reader) {
+	wc = newTarFilterer(wc, func(hdr *tar.Header) (action tarFilterAction, replaceContents bool, replacementContents io.Reader) {
 		modTime := hdr.ModTime
-		if layerModTime != nil || layerLatestModTime != nil || len(exclusions) != 0 {
+		if layerModTime != nil || layerLatestModTime != nil || len(exclusions) != 0 || len(pullUps) != 0 {
 			// Changing a zeroed field to a non-zero field can affect the
 			// format that the library uses for writing the header, so only
 			// change fields that are already set to avoid changing the
@@ -1448,8 +1458,21 @@ func makeFilteredLayerWriteCloser(wc io.WriteCloser, layerModTime, layerLatestMo
 				if (conditions.ModTime == nil || conditions.ModTime.Equal(modTime)) &&
 					(conditions.Owner == nil || (conditions.Owner.UID == hdr.Uid && conditions.Owner.GID == hdr.Gid)) &&
 					(conditions.Mode == nil || (*conditions.Mode&os.ModePerm == os.FileMode(hdr.Mode)&os.ModePerm)) {
-					return true, false, nil
+					return tarFilterSkip, false, nil
+				}
+			}
+			// Correct the ownership and mode of pulled-up parent
+			// directories, but defer writing them until a child
+			// entry passes through the filter.
+			if pullUpSpec, ok := pullUpsMap[nameSpec]; ok {
+				if pullUpSpec.Owner != nil {
+					hdr.Uid = pullUpSpec.Owner.UID
+					hdr.Gid = pullUpSpec.Owner.GID
+				}
+				if pullUpSpec.Mode != nil {
+					hdr.Mode = int64(*pullUpSpec.Mode & os.ModePerm)
 				}
+				return tarFilterDefer, false, nil
 			}
 		}
 		if layerModTime != nil {
@@ -1499,7 +1522,7 @@ func makeFilteredLayerWriteCloser(wc io.WriteCloser, layerModTime, layerLatestMo
 				hdr.PAXRecords[keyCreationTime] = fmt.Sprintf("%d.%09d", hdr.ModTime.Unix(), hdr.ModTime.Nanosecond())
 			}
 		}
-		return false, false, nil
+		return tarFilterKeep, false, nil
 	})
 	if windows {
 		// prep the archive by writing the Files/ and Hives/ directories to the writer.
@@ -1588,7 +1611,7 @@ func (b *Builder) makeLinkedLayerInfos(layers []LinkedLayer, layerType string, l
 
 			digester := digest.Canonical.Digester()
 			sizeCountedFile := ioutils.NewWriteCounter(io.MultiWriter(digester.Hash(), f))
-			wc, err := makeFilteredLayerWriteCloser(ioutils.NopWriteCloser(sizeCountedFile), layerModTime, layerLatestModTime, nil, false)
+			wc, err := makeFilteredLayerWriteCloser(ioutils.NopWriteCloser(sizeCountedFile), layerModTime, layerLatestModTime, nil, nil, false)
 			if err != nil {
 				return err
 			}