bucache: save and restore xattrs, ACLs

bin/ch-completion.bash

@@ -112,9 +112,9 @@ _image_build_opts="-b --bind --build-arg -f --file --force
 
 _image_common_opts="-a --arch --always-download --auth --cache
                     --cache-large --dependencies -h --help
-                    --no-cache --no-lock --profile --rebuild
-                    --password-many -s --storage --tls-no-verify
-                    -v --verbose --version"
+                    --no-cache --no-lock --no-xattrs --profile
+                    --rebuild --password-many -s --storage
+                    --tls-no-verify -v --verbose --version"
 
 _image_subcommands="build build-cache delete gestalt
                     import list pull push reset undelete"

bin/ch-convert

@@ -50,7 +50,8 @@ cv_chimage_dir () {
     # (2) it allows a progress meter (issue #1332). A very quick, very dirty,
     # completely half-assed performance test suggests that's about the same.
     dir_make "$2"
-    tar_ "$img" | tar xf - -pC "$2"
+    # shellcheck disable=SC2086
+    tar_ "$img" | tar xf - $tar_xattrs -pC "$2"
     dir_fixup "$2"
 }
 
@@ -111,7 +112,8 @@ cv_dir_squash () {
     mount_points_ensure "$1" "$pflist"
     # Exclude build cache metadata. 64kiB block size based on Shane's
     # experiments.
-    mksquashfs "$1" "$2" -b 65536 -noappend -all-root -pf "$pflist" \
+    # shellcheck disable=SC2086
+    mksquashfs "$1" "$2" $squash_xattrs -b 65536 -noappend -all-root -pf "$pflist" \
                          -e "$1"/.git -e "$1"/.gitignore -e "$1"/ch/git.pickle
     # Zero the archive's internal modification time at bytes 8–11, 0-indexed
     # [1]. Newer SquashFS-Tools ≥4.3 have option “-fstime 0” to do this, but
@@ -313,11 +315,11 @@ cv_tar_dir () {
     ex2="/${ex1}"
     ex3="./${ex1}"
     VERBOSE "exclude patterns: ${ex1} ${ex2} ${ex3}"
-    #shellcheck disable=SC2094
+    #shellcheck disable=SC2094,SC2086
       pv_ -s "$(stat -c%s "$1")" < "$1" \
     | tar x"$(tar_decompress_arg "$1")" -pC "$2" -f - \
           --xform 's|^\./||x' --strip-components=$strip_ct \
-          --anchored --no-wildcards-match-slash \
+          --anchored --no-wildcards-match-slash $tar_xattrs \
           --exclude="$ex1" --exclude="$ex2" --exclude="$ex3"
     dir_fixup "$2"
 }
@@ -422,7 +424,8 @@ dockman_to_tar () {
             --format='{{range .Config.Env}}{{println .}}{{end}}' > "$tmpenv"
     # The tar flavor Docker gives us does not support UIDs or GIDs greater
     # than 2**21, so use 0/0 rather than what’s on the filesystem. See #1573.
-    tar rf "$tmptar" -b1 -P --owner=0 --group=0 \
+    # shellcheck disable=SC2086
+    tar rf "$tmptar" -b1 -P --owner=0 --group=0 $tar_xattrs \
         --xform="s|${tmpenv}|ch/environment|" "$tmpenv"
     INFO 'compressing ...'
     pv_ < "$tmptar" | gzip_ -6 > "$3"
@@ -707,7 +710,9 @@ path_noclobber () {
 # Tar $1 and emit the result on stdout, excluding build cache metadata.
 # Produce a tarbomb because Docker requires tarbombs.
 tar_ () {
-    ( cd "$1" && tar cf - --exclude=./.git \
+    # shellcheck disable=SC2086
+    ( cd "$1" && tar cf - $tar_xattrs \
+                          --exclude=./.git \
                           --exclude=./.gitignore \
                           --exclude=./ch/git.pickle . ) | pv_
 }
@@ -832,6 +837,9 @@ while true; do
             --no-clobber)
                 no_clobber=yes
                 ;;
+            --no-xattrs)
+                no_xattrs=yes
+                ;;
             -o|--out-fmt)
                 shift
                 out_fmt=$1
@@ -857,6 +865,13 @@ done
 if [ "$#" -ne 2 ]; then
     usage
 fi
+if [ -n "$no_xattrs" ]; then
+    tar_xattrs=
+    squash_xattrs=
+else
+    tar_xattrs=--xattrs-include='user.*'
+    squash_xattrs=-xattrs
+fi
 in_desc=$1
 out_desc=$2
 VERBOSE "verbose level: ${verbose}"

bin/ch-image.py.in

@@ -122,6 +122,9 @@ def main():
            [["--no-lock"],
             { "action": "store_true",
               "help": "allow concurrent storage directory access (risky!)" }],
+           [["--no-xattrs"],
+            { "action": "store_true",
+              "help": "disable xattrs and ACLs from being saved/restored via build cache"}],
            [["--password-many"],
             { "action": "store_true",
               "help": "re-prompt each time a registry password is needed" }],

bin/ch-run-oci.py.in

@@ -94,6 +94,7 @@ def args_parse():
    args_.profile = False
    args_.storage = None
    args_.tls_no_verify = False
+   args_.no_xattrs = False
    ch.init(args_)
 
    if len(sys.argv) < 2:

doc/ch-convert.rst

@@ -49,6 +49,9 @@ producing the final format actually needed.
   :code:`--no-clobber`
     Error if :code:`OUT` already exists, rather than replacing it.
 
+  :code:`--no-xattrs`
+    Convert image without preserving xattrs or ACLs.
+
   :code:`-o`, :code:`--out-fmt FMT`
     Output image format is :code:`FMT`; inferred if omitted.
 

doc/ch-image.rst

@@ -85,6 +85,9 @@ Common options placed before or after the sub-command:
     :code:`ch-image` instances as you want against the same storage directory,
     which risks corruption but may be OK for some workloads.
 
+  :code:`--no-xattrs`
+    Disable xattrs and ALCs from being saved or restored by the build cache.
+
   :code:`--profile`
     Dump profile to files :code:`/tmp/chofile.p` (:code:`cProfile` dump
     format) and :code:`/tmp/chofile.txt` (text summary). You can convert the
@@ -290,10 +293,16 @@ it’s often cheaper to retrieve their results from cache instead.
 
 The build cache uses a relatively new Git under the hood; see the installation
 instructions for version requirements. Charliecloud implements workarounds for
-Git’s various storage limitations, so things like file metadata and Git
-repositories within the image should work. **Important exception**: No files
-named :code:`.git*` or other Git metadata are permitted in the image’s root
-directory.
+Git’s various storage limitations, so things like file metadata, access control
+lists, and Git repositories within the image should work. **Important
+exception**: No files named :code:`.git*` or other Git metadata are permitted in
+the image’s root directory.
+
+`Extended attributes <https://man7.org/linux/man-pages/man7/xattr.7.html>`_
+belonging to unprivileged namespaces (e.g. :code:`user`) are also saved and
+restored by the cache by default. Notably, extended attributes in privileged
+namespaces (e.g. :code:`trusted`) cannot be read by :code:`ch-image` and will be
+lost without warning.
 
 The cache has three modes, *enabled*, *disabled*, and a hybrid mode called
 *rebuild* where the cache is fully enabled for :code:`FROM` instructions, but

lib/build_cache.py

@@ -243,6 +243,8 @@ class File_Metadata:
    #   hardlink_to ... If non-None, file is a hard link to this other file,
    #                   stored as a Path object relative to the image root.
    #
+   #   xattrs ........ Extended attributes of the file, stored as a dictionary.
+   #
    # Attributes not stored (recomputed on unpickle):
    #
    #   image_root .... Absolute path to the image directory to which path is
@@ -269,6 +271,10 @@ def __init__(self, image_root, path):
       self.dont_restore = False
       self.hardlink_to = None
       self.large_name = None
+      self.xattrs = dict()
+      if ch.xattrs:
+         for xattr in os.listxattr(self.path_abs, follow_symlinks=False):
+            self.xattrs[xattr] = os.getxattr(self.path_abs, xattr, follow_symlinks=False)
 
    def __getstate__(self):
       return { a:v for (a,v) in self.__dict__.items()
@@ -472,6 +478,11 @@ def git_restore(self, quick):
          ch.ossafe(os.mkfifo, "can’t make FIFO: %s" % self.path, self.path_abs)
       elif (self.path.git_incompatible_p):
          self.path_abs.git_escaped.rename_(self.path_abs)
+      # Restore extended attributes
+      if ch.xattrs:
+         for xattr, val in self.xattrs.items():
+            ch.ossafe(os.setxattr, "unable to restore xattr: %s" % xattr,
+                     self.path_abs, xattr, val, follow_symlinks=False)
       # Recurse children.
       if (len(self.children) > 0):
          for child in self.children.values():
@@ -548,11 +559,13 @@ def str_for_log(self):
 
    def unpickle_fix(self, image_root, path):
       "Does no I/O."
-      # old: large_name, size: no such attribute
+      # old: large_name, size, xattrs: no such attribute
       if (not (hasattr(self, "large_name"))):
          self.large_name = None
       if (not (hasattr(self, "size"))):
          self.size = -1
+      if (not (hasattr(self, "xattrs"))):
+         self.xattrs = dict()
       # old: hardlink_to: stored as string
       if (isinstance(self.hardlink_to, str)):
          self.hardlink_to = fs.Path(self.hardlink_to)

lib/charliecloud.py

@@ -599,9 +599,10 @@ def exit(code):
 
 def init(cli):
    # logging
-   global log_festoon, log_fp, trace_fatal, verbose
+   global log_festoon, log_fp, trace_fatal, verbose, xattrs
    assert (0 <= cli.verbose <= 3)
    verbose = cli.verbose
+   xattrs = (not cli.no_xattrs)
    trace_fatal = (cli.debug or bool(os.environ.get("CH_IMAGE_DEBUG", False)))
    if ("CH_LOG_FESTOON" in os.environ):
       log_festoon = True

test/build/55_cache.bats

@@ -1399,3 +1399,32 @@ EOF
     [[ $output = *'deleting, see issue #1351: var/lib/rpm/__db.001'* ]]
     [[ ! -e $CH_IMAGE_STORAGE/img/tmpimg/var/lib/rpm/__db.001 ]]
 }
+
+@test "${tag}: restore ACLs, xattrs" { # issue #1287
+    ch-image build-cache --reset
+    ch-image build -t tmpimg - <<'EOF'
+FROM almalinux:8
+RUN dnf install -y --setopt=install_weak_deps=false attr
+RUN touch /home/foo
+RUN setfattr -n user.foo -v bar /home/foo
+RUN setfacl -m u:root:r /home/foo
+EOF
+    ch-image delete tmpimg
+    ch-image build -t tmpimg - <<'EOF'
+FROM almalinux:8
+RUN dnf install -y --setopt=install_weak_deps=false attr
+RUN touch /home/foo
+RUN setfattr -n user.foo -v bar /home/foo
+RUN setfacl -m u:root:r /home/foo
+EOF
+    run ch-run tmpimg -- getfattr home/foo
+    echo "$output"
+    [[ $status -eq 0 ]]
+    [[ $output = *'# file: home/foo'* ]]
+    [[ $output = *'user.foo'* ]]
+
+    run ch-run tmpimg -- getfacl home/foo
+    echo "$output"
+    [[ $status -eq 0 ]]
+    [[ $output = *"user:$USER:r--"* ]]
+}

test/make-auto.d/build_custom.bats.in

@@ -18,7 +18,12 @@ source common.bash  # for ShellCheck; removed by ch-test
             tarball=${tarballs[0]}
             # Convert to SquashFS if needed.
             if [[ $CH_TEST_PACK_FMT = squash* ]]; then
-                ch-convert "$tarball" "${tarball/tar.?z/sqfs}"
+                # janky fix for centos7xz tar permission errors
+                if [[ $tarball = *centos7xz* ]]; then
+                    ch-convert --no-xattrs "$tarball" "${tarball/tar.?z/sqfs}"
+                else
+                    ch-convert "$tarball" "${tarball/tar.?z/sqfs}"
+                fi
                 rm "$tarball"
             fi
         elif [[ -d $out ]]; then                             # directory