From c39f684affb2335f27f3fb5cf633266c6f75fc0e Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 19 Dec 2022 09:32:31 -0500 Subject: [PATCH] cmd: Jump to /run/host/$PWD when entering toolbox As a convenience to users, when entering a container toolbx tries to maintain the working directory from the host. This works great if the user is inside their home directory, for instance, since the home directory is shared between host and container. It can be confusing in other cases, though. The issue is that the directory in the container may have the same path as the directory in the host, but have completely different contents. The old contents may actually be in /run/host/$PWD instead. Switching to /run/host/$PWD unconditionally has its own downsides. For one, it's ugly, and also, in common cases, like subdirectories of the home directory, it's unnecessary. This commit tries to find the balance, by making toolbx check first if the directory is shared between host and container, and if not, only then falling back to trying /run/host/$PWD. Closes https://github.com/containers/toolbox/issues/988 --- src/cmd/run.go | 3 +- test/system/104-run.bats | 335 +++++++++++++++++++++++++++++---------- 2 files changed, 256 insertions(+), 82 deletions(-) diff --git a/src/cmd/run.go b/src/cmd/run.go index 7db7d9ef7..d243a343e 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -298,6 +298,7 @@ func runCommandWithFallbacks(container string, command []string, emitEscapeSeque runFallbackCommandsIndex := 0 runFallbackWorkDirsIndex := 0 workDir := workingDirectory + runFallbackWorkDirs := append([]string{"/run/host" + workDir}, runFallbackWorkDirs...) for { execArgs := constructExecArgs(container, command, detachKeysSupported, envOptions, workDir) @@ -522,7 +523,7 @@ func isPathPresent(container, path string) (bool, error) { "exec", "--user", currentUser.Username, container, - "sh", "-c", "test -d \"$1\"", "sh", path, + "sh", "-c", "test -d \"$1\" -a \"$1\" -ef \"/run/host/$1\"", "sh", path, } if err := shell.Run("podman", nil, nil, nil, args...); err != nil { diff --git a/test/system/104-run.bats b/test/system/104-run.bats index 5bb92f8e3..f2e989565 100644 --- a/test/system/104-run.bats +++ b/test/system/104-run.bats @@ -1,4 +1,19 @@ #!/usr/bin/env bats +# +# Copyright © 2021 – 2022 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# load 'libs/bats-support/load' load 'libs/bats-assert/load' @@ -13,98 +28,283 @@ teardown() { cleanup_containers } -@test "run: Try to run a command in the default container with no containers created" { - local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" +@test "run: Smoke test with true(1)" { + create_default_container run $TOOLBOX run true + assert_success + assert_output "" +} + +@test "run: Smoke test with false(1)" { + create_default_container + + run -1 $TOOLBOX run false + assert_failure + assert_output "" +} + +@test "run: Ensure that a login shell is used to invoke the command" { + create_default_container + + cp "$HOME"/.bash_profile "$HOME"/.bash_profile.orig + echo "echo \"~/.bash_profile read\"" >>"$HOME"/.bash_profile + + run $TOOLBOX run true + + mv "$HOME"/.bash_profile.orig "$HOME"/.bash_profile + + assert_success + assert_line --index 0 "~/.bash_profile read" + assert [ ${#lines[@]} -eq 1 ] +} + +@test "run: 'echo \"Hello World\"' inside the default container" { + create_default_container + + run $TOOLBOX --verbose run echo "Hello World" + + assert_success + assert_line --index $((${#lines[@]}-1)) "Hello World" +} + +@test "run: 'echo \"Hello World\"' inside a restarted container" { + create_container running + + start_container running + stop_container running + + run $TOOLBOX --verbose run --container running echo "Hello World" + + assert_success + assert_line --index $((${#lines[@]}-1)) "Hello World" +} + +@test "run: 'sudo id' inside the default container" { + create_default_container + + output="$($TOOLBOX --verbose run sudo id 2>$BATS_TMPDIR/stderr)" + status="$?" + + echo "# stderr" + cat $BATS_TMPDIR/stderr + echo "# stdout" + echo $output + + assert_success + assert_output --partial "uid=0(root)" +} + +@test "run: Ensure that /run/.containerenv exists" { + create_default_container + + run --separate-stderr $TOOLBOX run cat /run/.containerenv + + assert_success + assert [ ${#lines[@]} -gt 0 ] + assert [ ${#stderr_lines[@]} -eq 0 ] +} + +@test "run: Ensure that /run/.toolboxenv exists" { + create_default_container + + run --separate-stderr $TOOLBOX run test -f /run/.toolboxenv + + assert_success + assert [ ${#lines[@]} -eq 0 ] + assert [ ${#stderr_lines[@]} -eq 0 ] +} + +@test "run: Ensure that the default container is used" { + run echo "$name" + + assert_success + assert_output "" + + local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" + create_default_container + create_container other-container + + run --separate-stderr $TOOLBOX run cat /run/.containerenv + + assert_success + assert [ ${#lines[@]} -gt 0 ] + assert [ ${#stderr_lines[@]} -eq 0 ] + + source <(echo "$output") + run echo "$name" + + assert_success + assert_output "$default_container_name" +} + +@test "run: Ensure that a specific container is used" { + run echo "$name" + + assert_success + assert_output "" + + local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" + create_default_container + create_container other-container + + run --separate-stderr $TOOLBOX run --container other-container cat /run/.containerenv + + assert_success + assert [ ${#lines[@]} -gt 0 ] + assert [ ${#stderr_lines[@]} -eq 0 ] + + source <(echo "$output") + run echo "$name" + + assert_success + assert_output "other-container" +} + +@test "run: Ensure that /run/host is used as a fallback working directory" { + local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" + create_default_container + + pushd /etc/kernel + run --separate-stderr $TOOLBOX run pwd + popd + + assert_success + assert_line --index 0 "/run/host/etc/kernel" + assert [ ${#lines[@]} -eq 1 ] + lines=("${stderr_lines[@]}") + assert_line --index $((${#stderr_lines[@]}-2)) "Error: directory /etc/kernel not found in container $default_container_name" + assert_line --index $((${#stderr_lines[@]}-1)) "Using /run/host/etc/kernel instead." + assert [ ${#stderr_lines[@]} -gt 2 ] +} + +@test "run: Pass down 1 additional file descriptor" { + create_default_container + + # File descriptors 3 and 4 are reserved by Bats. + run --separate-stderr $TOOLBOX run --preserve-fds 3 readlink /proc/self/fd/5 5>/dev/null + + assert_success + assert_line --index 0 "/dev/null" + assert [ ${#lines[@]} -eq 1 ] + assert [ ${#stderr_lines[@]} -eq 0 ] +} + +@test "run: Try the non-existent default container with none other present" { + local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" + + run --separate-stderr $TOOLBOX run true + + assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: container $default_container_name not found" assert_line --index 1 "Use the 'create' command to create a toolbox." assert_line --index 2 "Run 'toolbox --help' for usage." + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in the default container when 1 non-default container is present" { +@test "run: Try the non-existent default container with another present" { local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" create_container other-container - run $TOOLBOX run true + run --separate-stderr $TOOLBOX run true assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: container $default_container_name not found" assert_line --index 1 "Use the 'create' command to create a toolbox." assert_line --index 2 "Run 'toolbox --help' for usage." + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in a specific non-existent container" { +@test "run: Try a specific non-existent container with another present" { create_container other-container - run $TOOLBOX run -c wrong-container true + run --separate-stderr $TOOLBOX run --container wrong-container true assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: container wrong-container not found" assert_line --index 1 "Use the 'create' command to create a toolbox." assert_line --index 2 "Run 'toolbox --help' for usage." + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in a container based on unsupported distribution" { +@test "run: Try an unsupported distribution" { local distro="foo" - run $TOOLBOX --assumeyes run --distro "$distro" ls + run --separate-stderr $TOOLBOX --assumeyes run --distro "$distro" ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--distro'" assert_line --index 1 "Distribution $distro is unsupported." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in a container based on Fedora but with wrong version" { - run $TOOLBOX run -d fedora -r foobar ls +@test "run: Try Fedora with an invalid release" { + run --separate-stderr $TOOLBOX run --distro fedora --release foobar ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--release'" assert_line --index 1 "The release must be a positive integer." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] - run $TOOLBOX run --distro fedora --release -3 ls + run --separate-stderr $TOOLBOX run --distro fedora --release -3 ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--release'" assert_line --index 1 "The release must be a positive integer." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in a container based on RHEL but with wrong version" { - run $TOOLBOX run --distro rhel --release 8 ls +@test "run: Try RHEL with an invalid release" { + run --separate-stderr $TOOLBOX run --distro rhel --release 8 ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--release'" assert_line --index 1 "The release must be in the '.' format." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] - run $TOOLBOX run --distro rhel --release 8.2foo ls + run --separate-stderr $TOOLBOX run --distro rhel --release 8.2foo ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--release'" assert_line --index 1 "The release must be in the '.' format." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] - run $TOOLBOX run --distro rhel --release -2.1 ls + run --separate-stderr $TOOLBOX run --distro rhel --release -2.1 ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: invalid argument for '--release'" assert_line --index 1 "The release must be a positive number." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run a command in a container based on non-default distro without providing a version" { +@test "run: Try a non-default distro without a release" { local distro="fedora" local system_id="$(get_system_id)" @@ -112,92 +312,65 @@ teardown() { distro="rhel" fi - run $TOOLBOX run -d "$distro" ls + run --separate-stderr $TOOLBOX run --distro "$distro" ls assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") assert_line --index 0 "Error: option '--release' is needed" assert_line --index 1 "Distribution $distro doesn't match the host." assert_line --index 2 "Run 'toolbox --help' for usage." - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Run echo 'Hello World' inside of the default container" { +@test "run: Smoke test with 'exit 2'" { create_default_container - run $TOOLBOX --verbose run echo -n "Hello World" - - assert_success - assert_line --index $((${#lines[@]}-1)) "Hello World" -} - -@test "run: Run echo 'Hello World' inside a container after being stopped" { - create_container running - - start_container running - stop_container running - - run $TOOLBOX --verbose run --container running echo -n "Hello World" - - assert_success - assert_line --index $((${#lines[@]}-1)) "Hello World" + run -2 $TOOLBOX run /bin/sh -c 'exit 2' + assert_failure + assert_output "" } -@test "run: Run sudo id inside of the default container" { +@test "run: Pass down 1 invalid file descriptor" { + local default_container_name="$(get_system_id)-toolbox-$(get_system_version)" create_default_container - output="$($TOOLBOX --verbose run sudo id 2>$BATS_TMPDIR/stderr)" - status="$?" - - echo "# stderr" - cat $BATS_TMPDIR/stderr - echo "# stdout" - echo $output + # File descriptors 3 and 4 are reserved by Bats. + run -125 --separate-stderr $TOOLBOX run --preserve-fds 3 readlink /proc/self/fd/5 - assert_success - assert_output --partial "uid=0(root)" + assert_failure + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") + assert_line --index 0 "Error: file descriptor 5 is not available - the preserve-fds option requires that file descriptors must be passed" + assert_line --index 1 "Error: failed to invoke 'podman exec' in container $default_container_name" + assert [ ${#stderr_lines[@]} -eq 2 ] } -@test "run: Run command exiting with zero code in the default container" { +@test "run: Try /etc as a command" { create_default_container - run $TOOLBOX run /bin/sh -c 'exit 0' - - assert_success - assert_output "" -} - -@test "run: Run command exiting with non-zero code in the default container" { - create_default_container + run -126 --separate-stderr $TOOLBOX run /etc - run $TOOLBOX run /bin/sh -c 'exit 2' assert_failure - assert [ $status -eq 2 ] - assert_output "" + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") + assert_line --index 0 "bash: line 1: /etc: Is a directory" + assert_line --index 1 "bash: line 1: exec: /etc: cannot execute: Is a directory" + assert_line --index 2 "Error: failed to invoke command /etc in container $(get_latest_container_name)" + assert [ ${#stderr_lines[@]} -eq 3 ] } -@test "run: Try to run non-existent command in the default container" { +@test "run: Try a non-existent command" { local cmd="non-existent-command" create_default_container - run -127 $TOOLBOX run $cmd + run -127 --separate-stderr $TOOLBOX run $cmd assert_failure - assert [ $status -eq 127 ] - assert_line --index 0 "/bin/sh: line 1: exec: $cmd: not found" + assert [ ${#lines[@]} -eq 0 ] + lines=("${stderr_lines[@]}") + assert_line --index 0 "bash: line 1: exec: $cmd: not found" assert_line --index 1 "Error: command $cmd not found in container $(get_latest_container_name)" - assert [ ${#lines[@]} -eq 2 ] -} - -@test "run: Try to run /etc as a command in the deault container" { - create_default_container - - run $TOOLBOX run /etc - - assert_failure - assert [ $status -eq 126 ] - assert_line --index 0 "/bin/sh: line 1: /etc: Is a directory" - assert_line --index 1 "/bin/sh: line 1: exec: /etc: cannot execute: Is a directory" - assert_line --index 2 "Error: failed to invoke command /etc in container $(get_latest_container_name)" - assert [ ${#lines[@]} -eq 3 ] + assert [ ${#stderr_lines[@]} -eq 2 ] }