Skip to content

Commit

Permalink
cmd/create: Require -y to pull an image when not connected to a terminal
Browse files Browse the repository at this point in the history
It doesn't make sense to show the image download prompt when the
standard input or output stream is redirected to something other than a
terminal device.

During such non-interactive use, there's no way for the user to see the
prompt and the size of the image and then make a decision based on them.
The decision has to be made differently and earlier.  The user will
either never download or always download or will use 'skopeo inspect'
to decide for themself.

Secondly, when the input and output are not connected to a terminal, the
terminal escape sequences and the terminal-specific ioctl(2) requests
used to show the prompt won't work anyway.

Some changes by Debarshi Ray.

#1428
  • Loading branch information
HarryMichal authored and debarshiray committed Dec 20, 2023
1 parent 8caa7cd commit 55c0e63
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 1 deletion.
10 changes: 10 additions & 0 deletions src/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,16 @@ func pullImage(image, release, authFile string) (bool, error) {
}

if promptForDownload {
if !term.IsTerminal(os.Stdin) || !term.IsTerminal(os.Stdout) {
var builder strings.Builder
fmt.Fprintf(&builder, "image required to create toolbox container.\n")
fmt.Fprintf(&builder, "Use option '--assumeyes' to download the image.\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return false, errors.New(errMsg)
}

shouldPullImage = showPromptForDownload(imageFull)
}

Expand Down
78 changes: 77 additions & 1 deletion test/system/101-create.bats
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

# shellcheck shell=bats
#
# Copyright © 2019 – 2023 Red Hat, Inc.
Expand Down Expand Up @@ -25,7 +26,7 @@ setup() {
}

teardown() {
cleanup_containers
cleanup_all
}

@test "create: Smoke test" {
Expand All @@ -52,6 +53,18 @@ teardown() {
assert_success
}

@test "create: Try without --assumeyes" {
run --keep-empty-lines --separate-stderr "$TOOLBOX" create

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: image required to create toolbox container."
assert_line --index 1 "Use option '--assumeyes' to download the image."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#stderr_lines[@]} -eq 3 ]
}

@test "create: Try with an invalid custom name (using positional argument)" {
run "$TOOLBOX" --assumeyes create "ß[email protected]@m€"

Expand Down Expand Up @@ -83,6 +96,21 @@ teardown() {
assert [ ${#lines[@]} -eq 4 ]
}

@test "create: Try with an invalid custom image (using --assumeyes)" {
local image="ß[email protected]@m€"

run --keep-empty-lines --separate-stderr "$TOOLBOX" --assumeyes create --image "$image"

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: invalid argument for '--image'"
assert_line --index 1 "Container name $image generated from image is invalid."
assert_line --index 2 "Container names must match '[a-zA-Z0-9][a-zA-Z0-9_.-]*'."
assert_line --index 3 "Run 'toolbox --help' for usage."
assert [ ${#stderr_lines[@]} -eq 4 ]
}

@test "create: Arch Linux" {
pull_distro_image arch latest

Expand Down Expand Up @@ -211,6 +239,18 @@ teardown() {
}

@test "create: Try a non-existent image" {
run --keep-empty-lines --separate-stderr "$TOOLBOX" create --image foo.org/bar

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: image required to create toolbox container."
assert_line --index 1 "Use option '--assumeyes' to download the image."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#stderr_lines[@]} -eq 3 ]
}

@test "create: Try a non-existent image (using --assumeyes)" {
run "$TOOLBOX" --assumeyes create --image foo.org/bar

assert_failure
Expand Down Expand Up @@ -639,6 +679,17 @@ teardown() {
}

@test "create: Try using both --distro and --image" {
run --keep-empty-lines --separate-stderr "$TOOLBOX" create --distro fedora --image fedora-toolbox:34

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: options --distro and --image cannot be used together"
assert_line --index 1 "Run 'toolbox --help' for usage."
assert [ ${#stderr_lines[@]} -eq 2 ]
}

@test "create: Try using both --distro and --image (using --assumeyes)" {
pull_distro_image fedora 34

run "$TOOLBOX" --assumeyes create --distro fedora --image fedora-toolbox:34
Expand All @@ -650,6 +701,17 @@ teardown() {
}

@test "create: Try using both --image and --release" {
run --keep-empty-lines --separate-stderr "$TOOLBOX" create --image fedora-toolbox:34 --release 34

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: options --image and --release cannot be used together"
assert_line --index 1 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 2 ]
}

@test "create: Try using both --image and --release (using --assumeyes)" {
pull_distro_image fedora 34

run "$TOOLBOX" --assumeyes create --image fedora-toolbox:34 --release 34
Expand All @@ -672,6 +734,20 @@ teardown() {
assert [ ${#lines[@]} -eq 3 ]
}

@test "create: Try a non-existent authentication file (using --assumeyes)" {
local file="$BATS_RUN_TMPDIR/non-existent-file"

run --keep-empty-lines --separate-stderr "$TOOLBOX" --assumeyes create --authfile "$file"

assert_failure
assert [ ${#lines[@]} -eq 0 ]
lines=("${stderr_lines[@]}")
assert_line --index 0 "Error: file $file not found"
assert_line --index 1 "'podman login' can be used to create the file."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#stderr_lines[@]} -eq 3 ]
}

@test "create: With a custom image that needs an authentication file" {
local authfile="$BATS_RUN_TMPDIR/authfile"
local image="fedora-toolbox:34"
Expand Down

0 comments on commit 55c0e63

Please sign in to comment.