dockerTools.streamLayeredImage: Store the customisation layer as a ta…

…rball

This fixes as issue described here[1], where permissions set by 'extraCommands'
were ignored by Nix.

[1] #91084 (comment)

nixos/tests/docker-tools.nix

@@ -219,18 +219,11 @@ import ./make-test-python.nix ({ pkgs, ... }: {
         )
 
     with subtest("Ensure correct behavior when no store is needed"):
-        # This check tests two requirements simultaneously
-        #  1. buildLayeredImage can build images that don't need a store.
-        #  2. Layers of symlinks are eliminated by the customization layer.
-        #
+        # This check tests that buildLayeredImage can build images that don't need a store.
         docker.succeed(
             "docker load --input='${pkgs.dockerTools.examples.no-store-paths}'"
         )
 
-        # Busybox will not recognize argv[0] and print an error message with argv[0],
-        # but it confirms that the custom-true symlink is present.
-        docker.succeed("docker run --rm no-store-paths custom-true |& grep custom-true")
-
         # This check may be loosened to allow an *empty* store rather than *no* store.
         docker.succeed("docker run --rm no-store-paths ls /")
         docker.fail("docker run --rm no-store-paths ls /nix/store")

pkgs/build-support/docker/default.nix

@@ -718,28 +718,41 @@ rec {
          architecture = buildPackages.go.GOARCH;
          os = "linux";
       });
-      customisationLayer = runCommand "${name}-customisation-layer" { inherit extraCommands; } ''
-        cp -r ${contentsEnv}/ $out
 
-        if [[ -n $extraCommands ]]; then
-          chmod u+w $out
-          (cd $out; eval "$extraCommands")
-        fi
-      '';
-      contentsEnv = symlinkJoin {
-        name = "${name}-bulk-layers";
-        paths = if builtins.isList contents
-          then contents
-          else [ contents ];
+      contentsList = if builtins.isList contents then contents else [ contents ];
+
+      # We store the customisation layer as a tarball, to make sure that
+      # things like permissions set on 'extraCommands' are not overriden
+      # by Nix. Then we precompute the sha256 for performance.
+      customisationLayer = symlinkJoin {
+        name = "${name}-customisation-layer";
+        paths = contentsList;
+        inherit extraCommands;
+        postBuild = ''
+          mv $out old_out
+          (cd old_out; eval "$extraCommands" )
+
+          mkdir $out
+
+          tar \
+            --owner 0 --group 0 --mtime "@$SOURCE_DATE_EPOCH" \
+            --hard-dereference \
+            -C old_out \
+            -cf $out/layer.tar .
+
+          sha256sum $out/layer.tar \
+            | cut -f 1 -d ' ' \
+            > $out/checksum
+        '';
       };
 
-      # NOTE: the `closures` parameter is a list of closures to include.
-      # The TOP LEVEL store paths themselves will never be present in the
-      # resulting image. At this time (2020-06-18) none of these layers
-      # are appropriate to include, as they are all created as
-      # implementation details of dockerTools.
-      closures = [ baseJson contentsEnv ];
-      overallClosure = writeText "closure" (lib.concatStringsSep " " closures);
+      closureRoots = [ baseJson ] ++ contentsList;
+      overallClosure = writeText "closure" (lib.concatStringsSep " " closureRoots);
+
+      # These derivations are only created as implementation details of docker-tools,
+      # so they'll be excluded from the created images.
+      unnecessaryDrvs = [ baseJson overallClosure ];
+
       conf = runCommand "${name}-conf.json" {
         inherit maxLayers created;
         imageName = lib.toLower name;
@@ -751,9 +764,6 @@ rec {
         paths = referencesByPopularity overallClosure;
         buildInputs = [ jq ];
       } ''
-        paths() {
-          cat $paths ${lib.concatMapStringsSep " " (path: "| (grep -v ${path} || true)") (closures ++ [ overallClosure ])}
-        }
         ${if (tag == null) then ''
           outName="$(basename "$out")"
           outHash=$(echo "$outName" | cut -d - -f 1)
@@ -768,6 +778,12 @@ rec {
             created="$(date -Iseconds -d "$created")"
         fi
 
+        paths() {
+          cat $paths ${lib.concatMapStringsSep " "
+                         (path: "| (grep -v ${path} || true)")
+                         unnecessaryDrvs}
+        }
+
         # Create $maxLayers worth of Docker Layers, one layer per store path
         # unless there are more paths than $maxLayers. In that case, create
         # $maxLayers-1 for the most popular layers, and smush the remainaing

pkgs/build-support/docker/examples.nix

@@ -298,21 +298,10 @@ rec {
     name = "no-store-paths";
     tag = "latest";
     extraCommands = ''
-      chmod a+w bin
-
       # This removes sharing of busybox and is not recommended. We do this
       # to make the example suitable as a test case with working binaries.
       cp -r ${pkgs.pkgsStatic.busybox}/* .
     '';
-    contents = [
-      # This layer has no dependencies and its symlinks will be dereferenced
-      # when creating the customization layer.
-      (pkgs.runCommand "layer-to-flatten" {} ''
-        mkdir -p $out/bin
-        ln -s /bin/true $out/bin/custom-true
-      ''
-      )
-    ];
   };
 
   nixLayered = pkgs.dockerTools.buildLayeredImageWithNixDb {
@@ -415,7 +404,7 @@ rec {
     pkgs.dockerTools.buildLayeredImage {
       name = "bash-layered-with-user";
       tag = "latest";
-      contents = [ pkgs.bash pkgs.coreutils (nonRootShadowSetup { uid = 999; user = "somebody"; }) ];
+      contents = [ pkgs.bash pkgs.coreutils ] ++ nonRootShadowSetup { uid = 999; user = "somebody"; };
     };
 
 }

pkgs/build-support/docker/stream_layered_image.py

@@ -33,7 +33,6 @@
 
 import io
 import os
-import re
 import sys
 import json
 import hashlib
@@ -45,21 +44,14 @@
 from collections import namedtuple
 
 
-def archive_paths_to(obj, paths, mtime, add_nix, filter=None):
+def archive_paths_to(obj, paths, mtime):
     """
     Writes the given store paths as a tar file to the given stream.
 
     obj: Stream to write to. Should have a 'write' method.
     paths: List of store paths.
-    add_nix: Whether /nix and /nix/store directories should be
-             prepended to the archive.
-    filter: An optional transformation to be applied to TarInfo
-            objects. Should take a single TarInfo object and return
-            another one. Defaults to identity.
     """
 
-    filter = filter if filter else lambda i: i
-
     # gettarinfo makes the paths relative, this makes them
     # absolute again
     def append_root(ti):
@@ -72,7 +64,7 @@ def apply_filters(ti):
         ti.gid = 0
         ti.uname = "root"
         ti.gname = "root"
-        return filter(ti)
+        return ti
 
     def nix_root(ti):
         ti.mode = 0o0555  # r-xr-xr-x
@@ -85,11 +77,9 @@ def dir(path):
 
     with tarfile.open(fileobj=obj, mode="w|") as tar:
         # To be consistent with the docker utilities, we need to have
-        # these directories first when building layer tarballs. But
-        # we don't need them on the customisation layer.
-        if add_nix:
-            tar.addfile(apply_filters(nix_root(dir("/nix"))))
-            tar.addfile(apply_filters(nix_root(dir("/nix/store"))))
+        # these directories first when building layer tarballs.
+        tar.addfile(apply_filters(nix_root(dir("/nix"))))
+        tar.addfile(apply_filters(nix_root(dir("/nix/store"))))
 
         for path in paths:
             path = pathlib.Path(path)
@@ -136,19 +126,14 @@ def extract(self):
 LayerInfo = namedtuple("LayerInfo", ["size", "checksum", "path", "paths"])
 
 
-def add_layer_dir(tar, paths, mtime, add_nix=True, filter=None):
+def add_layer_dir(tar, paths, mtime):
     """
     Appends given store paths to a TarFile object as a new layer.
 
     tar: 'tarfile.TarFile' object for the new layer to be added to.
     paths: List of store paths.
     mtime: 'mtime' of the added files and the layer tarball.
            Should be an integer representing a POSIX time.
-    add_nix: Whether /nix and /nix/store directories should be
-             added to a layer.
-    filter: An optional transformation to be applied to TarInfo
-            objects inside the layer. Should take a single TarInfo
-            object and return another one. Defaults to identity.
 
     Returns: A 'LayerInfo' object containing some metadata of
              the layer added.
@@ -164,8 +149,6 @@ def add_layer_dir(tar, paths, mtime, add_nix=True, filter=None):
         extract_checksum,
         paths,
         mtime=mtime,
-        add_nix=add_nix,
-        filter=filter
     )
     (checksum, size) = extract_checksum.extract()
 
@@ -182,8 +165,6 @@ def producer():
                 write,
                 paths,
                 mtime=mtime,
-                add_nix=add_nix,
-                filter=filter
             )
             write.close()
 
@@ -199,29 +180,38 @@ def producer():
     return LayerInfo(size=size, checksum=checksum, path=path, paths=paths)
 
 
-def add_customisation_layer(tar, path, mtime):
+def add_customisation_layer(target_tar, customisation_layer, mtime):
     """
-    Adds the contents of the store path as a new layer. This is different
-    than the 'add_layer_dir' function defaults in the sense that the contents
-    of a single store path will be added to the root of the layer. eg (without
-    the /nix/store prefix).
+    Adds the customisation layer as a new layer. This is layer is structured
+    differently; given store path has the 'layer.tar' and corresponding
+    sha256sum ready.
 
     tar: 'tarfile.TarFile' object for the new layer to be added to.
-    path: A store path.
-    mtime: 'mtime' of the added files and the layer tarball. Should be an
-           integer representing a POSIX time.
+    customisation_layer: Path containing the layer archive.
+    mtime: 'mtime' of the added layer tarball.
     """
 
-    def filter(ti):
-        ti.name = re.sub("^/nix/store/[^/]*", "", ti.name)
-        return ti
-    return add_layer_dir(
-        tar,
-        [path],
-        mtime=mtime,
-        add_nix=False,
-        filter=filter
-      )
+    checksum_path = os.path.join(customisation_layer, "checksum")
+    with open(checksum_path) as f:
+        checksum = f.read().strip()
+    assert len(checksum) == 64, f"Invalid sha256 at ${checksum_path}."
+
+    layer_path = os.path.join(customisation_layer, "layer.tar")
+
+    path = f"{checksum}/layer.tar"
+    tarinfo = target_tar.gettarinfo(layer_path)
+    tarinfo.name = path
+    tarinfo.mtime = mtime
+
+    with open(layer_path, "rb") as f:
+        target_tar.addfile(tarinfo, f)
+
+    return LayerInfo(
+      size=None,
+      checksum=checksum,
+      path=path,
+      paths=[customisation_layer]
+    )
 
 
 def add_bytes(tar, path, content, mtime):