syscontainers: do not delete modified files with --system-package=no

Store the checksum of files copied to the host, so that on an update or
an uninstall we can skip the files that have a mismatch in the
checksum.

This reflects in the --system-package=no case what the rpm backend
already does.

Signed-off-by: Giuseppe Scrivano <[email protected]>

Closes: projectatomic#1131
Approved by: ashcrow

Atomic/rpm_host_install.py

@@ -4,6 +4,7 @@
 from . import rpmwriter
 import tempfile
 import shutil
+import hashlib
 
 RPM_NAME_PREFIX = "atomic-container"
 
@@ -12,16 +13,20 @@ class RPMHostInstall(object):
     @staticmethod
     def copyfile(src, dest):
         if os.path.isdir(src):
-            # add the directory only if it is empty, so we don't create directories that
+            # add the directory only if it is empty, so we don't delete directories that
             # weren't added by us.  Anyway a non empty directory would be created by
             # its children.
             if len(os.listdir(src)) == 0:
                 os.mkdir(dest)
+                return True
         elif os.path.islink(src):
             linkto = os.readlink(src)
             os.symlink(linkto, dest)
+            return True
         else:
             shutil.copy2(src, dest)
+            return True
+        return False
 
     @staticmethod
     def _do_rename_path(path, rename_files):
@@ -33,12 +38,31 @@ def _do_rename_path(path, rename_files):
         return path
 
     @staticmethod
-    def rm_add_files_to_host(old_installed_files, exports, prefix="/", files_template=None, values=None, rename_files=None):
+    def file_checksum(path, blocksize=(1<<20)):
+        if not os.path.exists(path) or os.path.isdir(path):
+            return "0"
+
+        h = hashlib.sha256()
+        with open(path, "rb") as f:
+            while True:
+                chunk = f.read(blocksize)
+                if chunk == None or len(chunk) == 0:
+                    break
+                h.update(chunk)
+        return h.hexdigest()
+
+    @staticmethod
+    def rm_add_files_to_host(old_installed_files_checksum, exports, prefix="/", files_template=None, values=None, rename_files=None):
         # if any file was installed on the host delete it
-        if old_installed_files:
-            for i in old_installed_files:
+        if old_installed_files_checksum:
+            for path, checksum in old_installed_files_checksum.items():
+                new_checksum = RPMHostInstall.file_checksum(path)
+                if new_checksum != checksum:
+                    # Do not delete the file if it was modified.
+                    util.write_out("Will not delete %s as it was manually modified." % path, lf="\n")
+                    continue
                 try:
-                    os.remove(i)
+                    os.remove(path)
                 except OSError:
                     pass
 
@@ -49,7 +73,7 @@ def rm_add_files_to_host(old_installed_files, exports, prefix="/", files_templat
 
         # if there is a directory hostfs/ under exports, copy these files to the host file system.
         hostfs = os.path.join(exports, "hostfs")
-        new_installed_files = []
+        new_installed_files_checksum = {}
         if os.path.exists(hostfs):
             for root, dirs, files in os.walk(hostfs):
                 rel_root_path = os.path.relpath(root, hostfs)
@@ -71,18 +95,21 @@ def rm_add_files_to_host(old_installed_files, exports, prefix="/", files_templat
 
                     if not os.path.exists(os.path.dirname(dest_path)):
                         os.makedirs(os.path.dirname(dest_path))
+
+                    created = False
                     if rel_dest_path in templates_set:
                         with open(src_file, 'r') as src_file_obj:
                             data = src_file_obj.read()
                         util.write_template(src_file, data, values or {}, dest_path)
                         shutil.copystat(src_file, dest_path)
+                        created = True
                     else:
-                        RPMHostInstall.copyfile(src_file, dest_path)
+                        created = RPMHostInstall.copyfile(src_file, dest_path)
 
-                    new_installed_files.append(rel_dest_path)
-            new_installed_files.sort()  # just for an aesthetic reason in the info file output
+                    if created:
+                        new_installed_files_checksum[rel_dest_path] = RPMHostInstall.file_checksum(dest_path)
 
-        return new_installed_files
+        return new_installed_files_checksum
 
 
     @staticmethod
@@ -175,7 +202,8 @@ def generate_rpm(name, image_id, labels, exports, destination, values=None, inst
             rootfs = os.path.join(rpm_content, "usr/lib/containers/atomic", name)
             os.makedirs(rootfs)
             if installed_files is None:
-                installed_files = RPMHostInstall.rm_add_files_to_host(None, exports, rpm_content, files_template=installed_files_template, values=values, rename_files=rename_files)
+                checksums = RPMHostInstall.rm_add_files_to_host(None, exports, rpm_content, files_template=installed_files_template, values=values, rename_files=rename_files)
+                installed_files = checksums.keys()
             rpm_root = RPMHostInstall.generate_rpm_from_rootfs(destination, temp_dir, name, image_id, labels, include_containers_file=False, installed_files=installed_files, defaultversion=defaultversion)
             rpm_file = RPMHostInstall.find_rpm(rpm_root)
             if rpm_file:

Atomic/syscontainers.py

@@ -529,7 +529,7 @@ def _resolve_remote_path(self, remote_path):
             raise ValueError("The container's rootfs is set to remote, but the remote rootfs does not exist")
         return real_path
 
-    def _checkout(self, repo, name, img, deployment, upgrade, values=None, destination=None, extract_only=False, remote=None, prefix=None, installed_files=None, system_package='no'):
+    def _checkout(self, repo, name, img, deployment, upgrade, values=None, destination=None, extract_only=False, remote=None, prefix=None, installed_files_checksum=None, system_package='no'):
         destination = destination or os.path.join(self._get_system_checkout_path(), "{}.{}".format(name, deployment))
         unitfileout, tmpfilesout = self._get_systemd_destination_files(name, prefix)
 
@@ -539,8 +539,8 @@ def _checkout(self, repo, name, img, deployment, upgrade, values=None, destinati
                     raise ValueError("The file %s already exists." % f)
 
         try:
-            return self._do_checkout(repo, name, img, upgrade, deployment, values, destination, unitfileout, tmpfilesout, extract_only, remote, prefix, installed_files=installed_files,
-                                     system_package=system_package)
+            return self._do_checkout(repo, name, img, upgrade, deployment, values, destination, unitfileout, tmpfilesout, extract_only, remote, prefix,
+                                     installed_files_checksum=installed_files_checksum, system_package=system_package)
         except (GLib.Error, ValueError, OSError, subprocess.CalledProcessError, KeyboardInterrupt) as e:
             try:
                 if not extract_only and not upgrade:
@@ -747,7 +747,7 @@ def _canonicalize_location(self, destination):
 
 
     def _do_checkout(self, repo, name, img, upgrade, deployment, values, destination, unitfileout,
-                     tmpfilesout, extract_only, remote, prefix=None, installed_files=None, system_package='no'):
+                     tmpfilesout, extract_only, remote, prefix=None, installed_files_checksum=None, system_package='no'):
         """
         Actually do the checkout.
 
@@ -926,10 +926,11 @@ def _do_checkout(self, repo, name, img, upgrade, deployment, values, destination
             labels = {k.lower() : v for k, v in img_obj.get('Labels', {}).items()}
             (rpm_installed, rpm_file, _) = RPMHostInstall.generate_rpm(name, image_id, labels, exports, destination, values=values, installed_files_template=installed_files_template, rename_files=rename_files, defaultversion=deployment)
         if rpm_installed or system_package == 'absent':
-            new_installed_files = []
+            new_installed_files_checksum = {}
         else:
-            new_installed_files = RPMHostInstall.rm_add_files_to_host(installed_files, exports, prefix or "/", files_template=installed_files_template, values=values, rename_files=rename_files)
+            new_installed_files_checksum = RPMHostInstall.rm_add_files_to_host(installed_files_checksum, exports, prefix or "/", files_template=installed_files_template, values=values, rename_files=rename_files)
 
+        new_installed_files = list(new_installed_files_checksum.keys())
         try:
             with open(os.path.join(destination, "info"), 'w') as info_file:
                 info = {"image" : img,
@@ -939,6 +940,7 @@ def _do_checkout(self, repo, name, img, upgrade, deployment, values, destination
                         "values" : values,
                         "has-container-service" : has_container_service,
                         "installed-files": new_installed_files,
+                        "installed-files-checksum": new_installed_files_checksum,
                         "installed-files-template": installed_files_template,
                         "rename-installed-files" : rename_files,
                         "rpm-installed" : rpm_installed,
@@ -1126,7 +1128,10 @@ def update_container(self, name, setvalues=None, rebase=None):
         image = rebase or info["image"]
         values = info["values"]
         revision = info["revision"] if "revision" in info else None
-        installed_files = info["installed-files"] if "installed-files" in info else None
+        installed_files_checksum = info["installed-files-checksum"] if "installed-files-checksum" in info else None
+        if installed_files_checksum is None:
+            installed_files = info["installed-files"] if "installed-files" in info else None
+            installed_files_checksum = {k : "" for k in installed_files}
         rpm_installed = info["rpm-installed"] if "rpm-installed" in info else None
         system_package = info["system-package"] if "system-package" in info else None
         runtime = info["runtime"] if "runtime" in info else None
@@ -1159,7 +1164,8 @@ def update_container(self, name, setvalues=None, rebase=None):
             self._runtime_from_info_file = runtime
         if system_package is None:
             system_package = 'yes' if rpm_installed else 'no'
-        self._checkout(repo, name, image, next_deployment, True, values, remote=self.args.remote, installed_files=installed_files, system_package=system_package)
+        self._checkout(repo, name, image, next_deployment, True, values, remote=self.args.remote,
+                       installed_files_checksum=installed_files_checksum, system_package=system_package)
         return
 
     def rollback(self, name):
@@ -1180,7 +1186,10 @@ def rollback(self, name):
         with open(os.path.join(self._get_system_checkout_path(), name, "info"), "r") as info_file:
             info = json.loads(info_file.read())
             rpm_installed = info["rpm-installed"] if "rpm-installed" in info else None
-            installed_files = info["installed-files"] if "installed-files" in info and rpm_installed is None else None
+            installed_files_checksum = info["installed-files-checksum"] if "installed-files-checksum" in info else None
+            if installed_files_checksum is None:
+                installed_files = info["installed-files"] if "installed-files" in info else None
+                installed_files_checksum = {k : "" for k in installed_files}
             installed_files_template = info["installed-files-template"] if "installed-files-template" in info and rpm_installed is None else None
             has_container_service = info["has-container-service"] if "has-container-service" in info else True
             rename_files = info["rename-installed-files"] if "rename-installed-files" in info else None
@@ -1213,8 +1222,8 @@ def rollback(self, name):
         if (os.path.exists(tmpfiles)):
             shutil.copyfile(tmpfiles, tmpfilesout)
 
-        if installed_files:
-            RPMHostInstall.rm_add_files_to_host(installed_files, os.path.join(destination, "rootfs/exports"), files_template=installed_files_template, rename_files=rename_files)
+        if installed_files_checksum:
+            RPMHostInstall.rm_add_files_to_host(installed_files_checksum, os.path.join(destination, "rootfs/exports"), files_template=installed_files_template, rename_files=rename_files)
 
         os.unlink(path)
         os.symlink(destination, path)
@@ -1646,9 +1655,12 @@ def uninstall(self, name):
         installed_files = None
         with open(os.path.join(checkout, name,  "info"), 'r') as info_file:
             info = json.loads(info_file.read())
-            installed_files = info["installed-files"] if "installed-files" in info else None
-        if installed_files:
-            RPMHostInstall.rm_add_files_to_host(installed_files, None)
+            installed_files_checksum = info["installed-files-checksum"] if "installed-files-checksum" in info else None
+            installed_files = info["installed-files"] if "installed-files" in info else []
+            if installed_files_checksum == None:
+                installed_files_checksum = {k: "" for k in installed_files}
+        if installed_files_checksum:
+            RPMHostInstall.rm_add_files_to_host(installed_files_checksum, None)
 
         if rpm_installed:
             try:

tests/integration/test_system_containers_rpm.sh

@@ -81,6 +81,7 @@ teardown () {
     ${ATOMIC} uninstall --storage ostree atomic-test-system-hostfs || true
     rm -rf /etc/systemd/system/atomic-test-system-*.service /etc/tmpfiles.d/atomic-test-system-*.conf
     ostree --repo=${ATOMIC_OSTREE_REPO} refs --delete ociimage &> /dev/null || true
+    rm -f /usr/local/lib/secret-message
 }
 trap teardown EXIT
 
@@ -149,3 +150,11 @@ for i in /usr/local/lib/renamed-atomic-test-system-hostfs /usr/local/lib/secret-
 do
     test -e $i
 done
+
+echo "This message will not be deleted" > /usr/local/lib/secret-message
+
+ATOMIC_OSTREE_TEST_FORCE_IMAGE_ID=NEW-ID ${ATOMIC} containers update atomic-test-system-hostfs
+
+test -e /usr/local/lib/secret-message
+
+assert_matches "This message will not be deleted" /usr/local/lib/secret-message