Fix/556 grab side effect

pkg/stacker/grab.go

@@ -3,7 +3,6 @@ package stacker
 import (
 	"fmt"
 	"io/fs"
-	"os"
 	"path"
 	"path/filepath"
 
@@ -20,24 +19,24 @@ func Grab(sc types.StackerConfig, storage types.Storage, name string, source str
 	}
 	defer c.Close()
 
-	err = c.BindMount(targetDir, types.InternalStackerDir, "")
+	err = SetupBuildContainerConfig(sc, storage, c, types.InternalStackerDir, name)
 	if err != nil {
 		return err
 	}
-	defer os.Remove(path.Join(sc.RootFSDir, name, "rootfs", "stacker"))
 
-	err = SetupBuildContainerConfig(sc, storage, c, types.InternalStackerDir, name)
+	idestdir := filepath.Join(types.InternalStackerDir, "grab")
+	err = c.BindMount(targetDir, idestdir, "")
 	if err != nil {
 		return err
 	}
 
 	bcmd := []string{filepath.Join(types.InternalStackerDir, types.BinStacker), "internal-go"}
 
-	iDestName := filepath.Join(types.InternalStackerDir, path.Base(source))
+	iDestName := filepath.Join(idestdir, path.Base(source))
 	if idest == "" || source[len(source)-1:] != "/" {
 		err = c.Execute(append(bcmd, "cp", source, iDestName), nil)
 	} else {
-		err = c.Execute(append(bcmd, "cp", source, types.InternalStackerDir+"/"), nil)
+		err = c.Execute(append(bcmd, "cp", source, idestdir+"/"), nil)
 	}
 	if err != nil {
 		return err

test/grab.bats

@@ -0,0 +1,49 @@
+load helpers
+
+function setup() {
+    stacker_setup
+}
+
+function teardown() {
+    cleanup
+}
+
+# do a build and a grab in an empty directory and
+# verify that no unexpected files are created.
+@test "grab has no side-effects" {
+    cat > stacker.yaml <<EOF
+layer1:
+    from:
+        type: oci
+        url: $BUSYBOX_OCI
+    imports:
+        - myfile.txt
+    run: |
+        cp /stacker/imports/myfile.txt /my-file
+EOF
+    startdir="$PWD"
+    wkdir="$PWD/work-dir"
+    bdir="$PWD/build-dir"
+    grabdir="$PWD/grab-dir"
+    mkdir "$wkdir" "$grabdir" "$bdir"
+    give_user_ownership "$wkdir" "$grabdir" "$bdir"
+
+    echo "hello world" > myfile.txt
+    expected_sha=$(sha myfile.txt)
+
+    cd "$bdir"
+    stacker "--work-dir=$wkdir" build "--stacker-file=$startdir/stacker.yaml"
+    dir_is_empty . ||
+        test_error "build dir had unexpected files: $_RET_EXTRA"
+
+    cd "$grabdir"
+    stacker "--work-dir=$wkdir" grab layer1:/my-file
+    [ -f my-file ]
+    found_sha=$(sha my-file)
+    [ "${expected_sha}" = "${found_sha}" ]
+
+    dir_has_only . my-file ||
+        test_error "grab produced extra files." \
+            "missing=${_RET_MISSING} extra=${_RET_EXTRA}"
+}
+

test/helpers.bash

@@ -9,7 +9,14 @@ if [ "$(id -u)" != "0" ]; then
 fi
 
 function skip_if_no_unpriv_overlay {
-    run sudo -u $SUDO_USER "${ROOT_DIR}/stacker" --debug internal-go testsuite-check-overlay
+    local wdir=""
+    # use a workdir to ensure no side effects to the caller
+    wdir=$(mktemp -d "$PWD/.skipunpriv.XXXXXX")
+    give_user_ownership "$wdir"
+    run sudo -u $SUDO_USER \
+        "${ROOT_DIR}/stacker" "--work-dir=$wdir" --debug \
+            internal-go testsuite-check-overlay
+    rm -Rf "$wdir"
     echo $output
     [ "$status" -eq 50 ] && skip "need newer kernel for unpriv overlay"
     [ "$status" -eq 0 ]
@@ -73,6 +80,17 @@ function stacker_setup() {
     chown -R $SUDO_USER:$SUDO_USER .
 }
 
+function give_user_ownership() {
+   if [ "$PRIVILEGE_LEVEL" = "priv" ]; then
+      return
+   fi
+   if [ -z "$SUDO_UID" ]; then
+      echo "PRIVILEGE_LEVEL=$PRIVILEGE_LEVEL but empty SUDO_USER"
+      exit 1
+   fi
+   chown -R "$SUDO_USER:$SUDO_USER" "$@"
+ }
+
 function cleanup() {
     cd "$ROOT_DIR/test"
     umount_under "$TEST_TMPDIR"
@@ -225,6 +243,56 @@ function _skopeo() {
     HOME="$home" "$SKOPEO" "$@"
 }
 
+function dir_has_only() {
+    local d="$1" oifs="$IFS" unexpected="" f=""
+    shift
+    _RET_MISSING=""
+    _RET_EXTRA=""
+    unexpected=$(
+        shopt -s nullglob;
+        IFS="/"; allexp="/$*/"; IFS="$oifs"
+        # allexp gets /item/item2/ for all items in args
+        x=""
+        cd "$d" || {
+            echo "dir_has_only could not 'cd $d' from $PWD" 1>&2;
+            exit 1;
+        }
+        for found in * .*; do
+            [ "$found" = "." ] || [ "$found" = ".." ] && continue
+            [ "${allexp#*/$found/}" != "$allexp" ] && continue
+            x="$x $found"
+        done
+        echo "$x"
+    ) || return 1
+    _RET_EXTRA="${unexpected# }"
+    for f in "$@"; do
+        [ -e "$d/$f" -o -L "$d/$f" ] && continue
+        _RET_MISSING="${_RET_MISSING} $f"
+    done
+    _RET_MISSING="${_RET_MISSING# }"
+    [ -z "$_RET_MISSING" -a -z "${_RET_EXTRA}" ]
+    return
+}
+
+function dir_is_empty() {
+    dir_has_only "$1"
+}
+
+# log a failure with ERROR:
+# allows more descriptive error:
+#   [ -f file ] || test_error "missing 'file'"
+# compared to just
+#   [ -f file ]
+function test_error() {
+    local m=""
+    echo "ERROR: $1"
+    shift
+    for m in "$@"; do
+        echo "  $m"
+    done
+    return 1
+}
+
 function test_copy_buffer_size() {
    local buffer_size=$1
    local file_type=$2