Only suppress "noted" items when not squashing

When suppressing what we think are pulled-up directories at commit-time,
only do that when we're _not_ squashing the image, in which case we
really do need to output it into the one layer that our output image
will have.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

image.go

@@ -50,11 +50,14 @@ const (
 	// containerExcludesDir is the subdirectory of the container data
 	// directory where we drop exclusions
 	containerExcludesDir = "commit-excludes"
+	// containerPulledUpDir is the subdirectory of the container
+	// data directory where we drop exclusions when we're not squashing
+	containerPulledUpDir = "commit-pulled-up"
 	// containerExcludesSubstring is the suffix of files under
-	// $cdir/containerExcludesDir which should be ignored, as they only
-	// exist because we use CreateTemp() to create uniquely-named files,
-	// but we don't want to try to use their contents until after they've
-	// been written to
+	// $cdir/containerExcludesDir and $cdir/containerPulledUpDir which
+	// should be ignored, as they only exist because we use CreateTemp() to
+	// create uniquely-named files, but we don't want to try to use their
+	// contents until after they've been written to
 	containerExcludesSubstring = ".tmp"
 )
 
@@ -1440,10 +1443,18 @@ func (b *Builder) makeContainerImageRef(options CommitOptions) (*containerImageR
 		return nil, fmt.Errorf("getting the per-container data directory for %q: %w", b.ContainerID, err)
 	}
 
-	excludesFiles, err := filepath.Glob(filepath.Join(cdir, containerExcludesDir, "*"))
+	mountTargetFiles, err := filepath.Glob(filepath.Join(cdir, containerExcludesDir, "*"))
 	if err != nil {
 		return nil, fmt.Errorf("checking for commit exclusions for %q: %w", b.ContainerID, err)
 	}
+	pulledUpFiles, err := filepath.Glob(filepath.Join(cdir, containerPulledUpDir, "*"))
+	if err != nil {
+		return nil, fmt.Errorf("checking for commit pulled-up items for %q: %w", b.ContainerID, err)
+	}
+	excludesFiles := slices.Clone(mountTargetFiles)
+	if !options.ConfidentialWorkloadOptions.Convert && !options.Squash {
+		excludesFiles = append(excludesFiles, pulledUpFiles...)
+	}
 	var layerExclusions []copier.ConditionalRemovePath
 	for _, excludesFile := range excludesFiles {
 		if strings.Contains(excludesFile, containerExcludesSubstring) {

run_common.go

@@ -2152,23 +2152,28 @@ func (b *Builder) createMountTargets(spec *specs.Spec) ([]copier.ConditionalRemo
 	if err != nil {
 		return nil, fmt.Errorf("finding working container bookkeeping directory: %w", err)
 	}
-	if err := os.Mkdir(filepath.Join(cdir, containerExcludesDir), 0o700); err != nil && !errors.Is(err, os.ErrExist) {
-		return nil, fmt.Errorf("creating exclusions directory: %w", err)
+	for excludesDir, exclusions := range map[string][]copier.ConditionalRemovePath{
+		containerExcludesDir: remove,
+		containerPulledUpDir: noted,
+	} {
+		if err := os.Mkdir(filepath.Join(cdir, excludesDir), 0o700); err != nil && !errors.Is(err, os.ErrExist) {
+			return nil, fmt.Errorf("creating exclusions directory: %w", err)
+		}
+		encoded, err := json.Marshal(exclusions)
+		if err != nil {
+			return nil, fmt.Errorf("encoding list of items to exclude at commit-time: %w", err)
+		}
+		f, err := os.CreateTemp(filepath.Join(cdir, excludesDir), "filter*"+containerExcludesSubstring)
+		if err != nil {
+			return nil, fmt.Errorf("creating exclusions file: %w", err)
+		}
+		defer os.Remove(f.Name())
+		defer f.Close()
+		if err := ioutils.AtomicWriteFile(strings.TrimSuffix(f.Name(), containerExcludesSubstring), encoded, 0o600); err != nil {
+			return nil, fmt.Errorf("writing exclusions file: %w", err)
+		}
 	}
-	encoded, err := json.Marshal(append(slices.Clone(noted), remove...))
-	if err != nil {
-		return nil, fmt.Errorf("encoding list of items to exclude at commit-time: %w", err)
-	}
-	f, err := os.CreateTemp(filepath.Join(cdir, containerExcludesDir), "filter*"+containerExcludesSubstring)
-	if err != nil {
-		return nil, fmt.Errorf("creating exclusions file: %w", err)
-	}
-	defer os.Remove(f.Name())
-	defer f.Close()
-	if err := ioutils.AtomicWriteFile(strings.TrimSuffix(f.Name(), containerExcludesSubstring), encoded, 0o600); err != nil {
-		return nil, fmt.Errorf("writing exclusions file: %w", err)
-	}
-	// return that set of paths directly, in case the caller would prefer
-	// to clear them out before commit-time
+	// return the set of to-remove-now paths directly, in case the caller would prefer
+	// to clear them out itself now instead of waiting until commit-time
 	return remove, nil
 }

tests/bud.bats

@@ -8481,7 +8481,7 @@ _EOF
 }
 
 @test "bud --layers should not include pulled up parent directories of mount points" {
-  _prefetch busybox
+  _prefetch quay.io/libpod/busybox
   local contextdir=${TEST_SCRATCH_DIR}/context
   mkdir $contextdir
   mkdir $contextdir/dev
@@ -8495,6 +8495,8 @@ COPY --from=quay.io/libpod/busybox / /
 RUN rm -f /Dockerfile
 RUN --mount=type=bind,ro,src=/Dockerfile,target=/var/spool/mail/tmpfile touch /newfile
 _EOF
+
+  # the most recent layer should only have the one newly-added file in it
   run_buildah build --layers -t oci:${TEST_SCRATCH_DIR}/oci-image ${contextdir}
   lastlayer=$(oci_image_last_diff ${TEST_SCRATCH_DIR}/oci-image)
   run tar tf ${TEST_SCRATCH_DIR}/oci-image/"${lastlayer}"
@@ -8502,3 +8504,34 @@ _EOF
   assert "$status" = "0"
   assert "${#lines[*]}" = "1"
 }
+
+@test "bud should include excluded pulled up parent directories in squashed images" {
+  _prefetch quay.io/libpod/busybox
+  local contextdir=${TEST_SCRATCH_DIR}/context
+  mkdir $contextdir
+  mkdir $contextdir/dev
+  mkdir $contextdir/etc
+  mkdir $contextdir/proc
+  mkdir $contextdir/sys
+
+  cat > $contextdir/Dockerfile << _EOF
+FROM scratch
+COPY / /
+COPY --from=quay.io/libpod/busybox / /
+RUN rm -f /Dockerfile
+RUN --mount=type=bind,ro,src=/Dockerfile,target=/etc/removed-file --mount=type=bind,ro,src=/Dockerfile,target=/var/spool/mail/tmpfile touch /newfile
+_EOF
+
+  local source_date_epoch=$(date +%s)
+  # build a copy of the image that's all squashed
+  run_buildah build --no-cache --squash --source-date-epoch=$source_date_epoch --rewrite-timestamp -t dir:${TEST_SCRATCH_DIR}/squashed-image ${contextdir}
+  # build a copy of the image that's "normal"
+  run_buildah build --no-cache --layers --source-date-epoch=$source_date_epoch --rewrite-timestamp -t not-squashed-image ${contextdir}
+  # now squash it, with no record of needing to exclude anything carrying over
+  run_buildah from --name not-squashed-container not-squashed-image
+  run_buildah commit --squash not-squashed-container dir:${TEST_SCRATCH_DIR}/squashed-later-image
+  # find the diffs for the two versions, which should look the same
+  local squashed=${TEST_SCRATCH_DIR}/squashed-image/$(dir_image_last_diff ${TEST_SCRATCH_DIR}/squashed-image)
+  local squashedlater=${TEST_SCRATCH_DIR}/squashed-later-image/$(dir_image_last_diff ${TEST_SCRATCH_DIR}/squashed-later-image)
+  cmp ${squashed} ${squashedlater}
+}