Bug: directory importing is odd

Question

Bug: directory importing is odd

smoser opened this issue a year ago · 3 comments

smoser commented a year ago

stacker version

v1.0.0-rc4-8e267fc

Describe the bug

I was playing with 'import' using 'path' and 'dest' and it did not behave as I would have expected.

To reproduce

Configuration

build:
  build_only: true
  from: 
    type: docker
    url: ${{DOCKER_BASE:docker://docker.io/library}}/busybox
  import:
    - path: dir1/
      dest: /import1/dirA
    - path: dir2
      dest: /import2/
    - path: file1
      dest: /import3-file
    - path: file2
      dest: /import4-dir/file
  run: |
    #!/bin/sh
    set +e
    
    ERRS=0
    err() { echo "$@"; ERRS=$((ERRS+1)); }
    assertEmptyDir() {
      local n=0
      [ -d "$1" ] || {
        err "$1: not a dir"
        return 1;
      }
      for f in "$1"/*; do
        [ -e "$f" -o -L "$f" ] && n=$((n+1))
      done
      [ $n = 0 ] || err "$1: not empty" "$1/"*
      return $n
    }

    assertFile() {
      [ -f "$1" ] || {
        err "$1: not a file"
        return 1
      }
      return 0
    }

    assertEmptyDir /import1/dirA || ls -l /import1 1>&2
    assertEmptyDir /import2

    assertFile /import3-file
    assertFile /import4-dir/file

    [ "$ERRS" = "0" ]

Client tool used: attached go.sh.txt
Seen error

$ ./go.sh
stacker version v1.0.0-rc4-8e267fc
usernsexec-ing [u 0 1000 1 1 100001 65535 g 0 1000 1 1 100001 65535 -- /usr/local/bin/stacker --internal-userns --debug --work-dir=/tmp/xx/workdir build stacker.yaml]
stacker version v1.0.0-rc4-8e267fc
initializing stacker recipe: stacker.yaml
substituting $STACKER_ROOTFS_DIR to /tmp/xx/workdir/roots
substituting $STACKER_STACKER_DIR to /tmp/xx/workdir/.stacker
substituting $STACKER_OCI_DIR to /tmp/xx/workdir/oci
substituting $STACKER_WORK_DIR to /tmp/xx/workdir
stacker build order:
0 build /tmp/xx/stacker.yaml: requires: []
building: 0 /tmp/xx/stacker.yaml
substituting $STACKER_ROOTFS_DIR to /tmp/xx/workdir/roots
substituting $STACKER_STACKER_DIR to /tmp/xx/workdir/.stacker
substituting $STACKER_OCI_DIR to /tmp/xx/workdir/oci
substituting $STACKER_WORK_DIR to /tmp/xx/workdir
Dependency Order [build]
preparing image build...
copying /tmp/xx/file1
copying /tmp/xx/file2
overlay-dirs, possibly modified after import: [{/tmp/xx/workdir/.stacker/imports-copy/build/2792729892 /import1} {/tmp/xx/workdir/.stacker/imports-copy/build/4272810128 /import2/} {/tmp/xx/workdir/.stacker/imports-copy/build/3094864666 /} {/tmp/xx/workdir/.stacker/imports-copy/build/3894314169 /import4-dir}]
loading docker://docker.io/library/busybox
Copying blob 4b35f584bb4f skipped: already exists  
Copying config b2f7a14676 done  
Writing manifest to image destination
Storing signatures
unpacking to /tmp/xx/workdir/roots/build
overlay dest / is a symlink, patching to 
lxc rootfs overlay arg overlayfs:/tmp/xx/workdir/roots/sha256_7acb01f8c37a28540d12a53220ddc672cf3a84e686516dc4b585c4f5c3b2909b/overlay:/tmp/xx/workdir/roots/sha256_d79bf2a25bb773fd3317f7c1b31faa0b286cb707ea63885f3221a71dc1362426/overlay:/tmp/xx/workdir/roots/sha256_875a4fc0930c9123fbfb1e8573cde947dcd46a024863f51d5d17232bcceef80d/overlay:/tmp/xx/workdir/roots/sha256_270a4ba199049717b06edc7b7fa8c1ddfe177e3d3ab641afe8c5b4984d3c4953/overlay:/tmp/xx/workdir/roots/sha256_4b35f584bb4f862773e2b84b827795b6f01985c7bcebb0696a3eb66318a166a5/overlay:/tmp/xx/workdir/roots/build/overlay
stacker version v1.0.0-rc4-8e267fc
stacker subcommand: [/usr/local/bin/stacker --oci-dir /tmp/xx/workdir/oci --roots-dir /tmp/xx/workdir/roots --stacker-dir /tmp/xx/workdir/.stacker --storage-type overlay --internal-userns --debug internal-go check-aa-profile lxc-container-default-cgns]
bind mounting /tmp/xx/workdir/.stacker/imports/build into container
/import1/dirA: not a dir
total 4
drwxr-xr-x    2 root     root          4096 Apr 17 21:08 dir1
/import2: not empty /import2/dir2
lxc build 20230417210834.377 ERROR    cgroup2_devices - ../src/lxc/cgroups/cgroup2_devices.c:bpf_program_load_kernel:332 - Operation not permitted - Failed to load bpf program: 
lxc build 20230417210834.377 ERROR    cgfsng - ../src/lxc/cgroups/cgfsng.c:__cgfsng_delegate_controllers:3341 - Resource busy - Could not enable "+memory +pids" controllers in the unified cgroup 9
lxc build 20230417210834.377 ERROR    cgfsng - ../src/lxc/cgroups/cgfsng.c:__cgfsng_delegate_controllers:3341 - Resource busy - Could not enable "+memory +pids" controllers in the unified cgroup 9
error: run commands failed: execute failed: exit status 1
stackerbuild.io/stacker/pkg/stacker.(*Builder).build
	/stacker-tree/pkg/stacker/build.go:471
stackerbuild.io/stacker/pkg/stacker.(*Builder).BuildMultiple
	/stacker-tree/pkg/stacker/build.go:568
main.doBuild
	/stacker-tree/cmd/stacker/build.go:117
github.com/urfave/cli.HandleAction
	/stacker-tree/.build/gopath/pkg/mod/github.com/urfave/cli@v1.22.12/app.go:524
github.com/urfave/cli.Command.Run
	/stacker-tree/.build/gopath/pkg/mod/github.com/urfave/cli@v1.22.12/command.go:175
github.com/urfave/cli.(*App).Run
	/stacker-tree/.build/gopath/pkg/mod/github.com/urfave/cli@v1.22.12/app.go:277
main.main
	/stacker-tree/cmd/stacker/main.go:324
runtime.main
	/usr/lib/go/src/runtime/proc.go:250
runtime.goexit
	/usr/lib/go/src/runtime/asm_amd64.s:1598
error: exit status 1
stackerbuild.io/stacker/pkg/container.MaybeRunInNamespace
	/stacker-tree/pkg/container/userns.go:102
main.main.func3
	/stacker-tree/cmd/stacker/main.go:319
github.com/urfave/cli.(*App).Run
	/stacker-tree/.build/gopath/pkg/mod/github.com/urfave/cli@v1.22.12/app.go:264
main.main
	/stacker-tree/cmd/stacker/main.go:324
runtime.main
	/usr/lib/go/src/runtime/proc.go:250
runtime.goexit
	/usr/lib/go/src/runtime/asm_amd64.s:1598

Expected behavior

I want/need/expect to be able to import directories and rename them.

I would expect for the 'dir1/' and 'dir2' imports to result in '/import1/dirA' and '/import2' directories respectively.

Screenshots

No response

Additional context

The same as a file, it is desireable to be able to specify the 'dest' name explicitly.

Answer 1 · 2023-04-18T15:54:38.000Z

I'm just thinking out loud here, but here is what I would suggest for import of a directory:

if the entity being imported is a directory, then 'path' must contain a trailing '/' or the import will fail
'dest' can never include a trailing / - fail on this for both files or directories
the entity imported will exist at 'dest' in the filesystem - no 'basename' behavior is ever used. This is more consistent because value of dest is then not dependent on the value of 'path')

## Assuming existing build directory
## /my/imports
## that has file 'fileA'
import:
 # results in /etc/imports/fileA will be at /etc/imports/fileA
 - path: /my/imports/
   dest: /etc/imports
 # rejected (no trailng / on 'path')
 - path: /my/imports
   dest: /etc/
 # rejected (trailing / on 'dest')
 - path: /my/imports/
   dest: /my/etc/
 # rejected because 'dest' exists (assuming that your fs has '/etc')
 - path: /my/imports/
   dest: /etc

questions:

should links be followed for a 'path' ? If we don't think we need the functionality of importing a link, then right now we could reject imports of a link straight away.

Thoughts?

Answer 2 · 2023-11-20T19:43:46.000Z

Below is a shell script that demonstrates the issue that a directory in a 'import' path.
The behavior changed between 0.4.x and 1.0.0-rc4. the '/stacker/imports' code did not actually change behavior other than moving the from '/stacker' to '/stacker/imports'.

#!/bin/sh
set -xe
rm -Rf tree
mkdir tree
mkdir -p tree/etc
mkdir -p tree/usr/bin
echo "passwd" > tree/etc/passwd
printf "#!/bin/sh\necho hello world\n" > tree/usr/bin/hello
chmod 755 tree/usr/bin/hello

cat > stacker.yaml <<"EOF"
tree:
  from:
    type: docker
    url: docker://busybox:latest
  import:
    - ./tree/
  run: |
    for d in /stacker/imports /stacker ; do
        [ -d "$d" ] && ls -l "$d" && break
    done
    exit 1
EOF

stacker --version
stacker --work-dir="$PWD/work.d" build

The real problem is that a directory named 'tree' doesn't exist inside the build container.

So:

stacker version	import dest
0.4.2	/stacker/tree
1.0.0-rc4	/stacker/tree
1.0.0-rc8	/stacker/imports/usr /stacker/imports/etc

The change is that trailing / on 'tree/' causes the directory to "disappear" in the container.

Answer 3 · 2023-12-14T17:53:23.000Z

#452 contains a request for the regressed behavior.