Merge pull request #3320 from nalind/copier-xattrs

copier.Put(): set xattrs after ownership

Author: openshift-merge-robot

copier/copier.go

@@ -1745,14 +1745,6 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 			if err != nil {
 				return errors.Wrapf(err, "copier: put: error creating %q", path)
 			}
-			// restore xattrs
-			if !req.PutOptions.StripXattrs {
-				if err = Lsetxattrs(path, hdr.Xattrs); err != nil { // nolint:staticcheck
-					if !req.PutOptions.IgnoreXattrErrors {
-						return errors.Wrapf(err, "copier: put: error setting extended attributes on %q", path)
-					}
-				}
-			}
 			// set ownership
 			if err = lchown(path, hdr.Uid, hdr.Gid); err != nil {
 				return errors.Wrapf(err, "copier: put: error setting ownership of %q to %d:%d", path, hdr.Uid, hdr.Gid)
@@ -1778,6 +1770,14 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 					return errors.Wrapf(err, "error setting additional permissions on %q to 0%o", path, mode)
 				}
 			}
+			// set xattrs, including some that might have been reset by chown()
+			if !req.PutOptions.StripXattrs {
+				if err = Lsetxattrs(path, hdr.Xattrs); err != nil { // nolint:staticcheck
+					if !req.PutOptions.IgnoreXattrErrors {
+						return errors.Wrapf(err, "copier: put: error setting extended attributes on %q", path)
+					}
+				}
+			}
 			// set time
 			if hdr.AccessTime.IsZero() || hdr.AccessTime.Before(hdr.ModTime) {
 				hdr.AccessTime = hdr.ModTime

tests/copy.bats

@@ -445,3 +445,26 @@ stuff/mystuff"
 
   run_buildah 1 run $from ls -l subdir/sub2.txt
 }
+
+@test "copy-preserving-extended-attributes" {
+  createrandom ${TESTDIR}/randomfile
+  image="registry.fedoraproject.org/fedora-minimal"
+  _prefetch $image
+  run_buildah from --quiet --signature-policy ${TESTSDIR}/policy.json $image
+  first="$output"
+  run_buildah run $first microdnf -y install /usr/bin/getfattr /usr/bin/setfattr /usr/sbin/setcap
+  run_buildah copy $first ${TESTDIR}/randomfile /
+  # set security.capability
+  run buildah run $first setcap cap_setuid=ep /randomfile
+  # set user.something
+  run buildah run $first setfattr user.yeah=butno /randomfile
+  # copy the file to a second container
+  run_buildah from --quiet --signature-policy ${TESTSDIR}/policy.json $image
+  second="$output"
+  run_buildah copy --from $first $second /randomfile /
+  # compare what the extended attributes look like. if we're on a system with SELinux, there's a label in here, too
+  run buildah run $first sh -c "getfattr -d -m . --absolute-names /randomfile | sort"
+  expected="$output"
+  run buildah run $first sh -c "getfattr -d -m . --absolute-names /randomfile | sort"
+  expect_output "$expected"
+}