From 05cf9b0eebb7f8b3f9ec7ac39c960134934fc1b9 Mon Sep 17 00:00:00 2001
From: Scott Moser <smoser@brickies.net>
Date: Thu, 30 Nov 2023 09:46:48 -0800
Subject: [PATCH] fix: Remove the bin/ dir that was created as a side-effect of
 grab.

'stacker grab' was creating a 'bin/' dir in the current working
directory.  This fixes that and adds a test to ensure that neither
grab nor build have side effects.

Fixes #566.

Signed-off-by: Scott Moser <smoser@brickies.net>
---
 pkg/stacker/grab.go | 11 +++++-----
 test/grab.bats      | 49 ++++++++++++++++++++++++++++++++++++++++++++
 test/helpers.bash   | 50 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100644 test/grab.bats

diff --git a/pkg/stacker/grab.go b/pkg/stacker/grab.go
index 9efca86d..666de41e 100644
--- a/pkg/stacker/grab.go
+++ b/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
diff --git a/test/grab.bats b/test/grab.bats
new file mode 100644
index 00000000..c5416747
--- /dev/null
+++ b/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}"
+}
+
diff --git a/test/helpers.bash b/test/helpers.bash
index 18798fe2..baa52727 100644
--- a/test/helpers.bash
+++ b/test/helpers.bash
@@ -225,6 +225,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