Skip to content

Commit

Permalink
[WIP] cmd/create, pkg/podman: Add capabilities for logging to registries
Browse files Browse the repository at this point in the history
Some registries contain private repositories of images and require the
user to log in first to gain access. With this Toolbox tries to
recognize errors when pulling images and offer the user the means to log
in.

The parsing process is quite fragile since Podman does not provide clear
means to distinguish such cases and provided messages differ between
registries. This should distinguish authorization errors with
registry.redhat.io and docker.io.

Testing of this feature is tricky as it relies on network which is
inherently flaky. So, not everything added/changed by this will be
tested. In the future we'll have to start mocking servers to be able to
do so.

#787
  • Loading branch information
HarryMichal committed Jun 8, 2021
1 parent afd427d commit bbcd5e1
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 8 deletions.
26 changes: 25 additions & 1 deletion doc/toolbox-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ toolbox\-create - Create a new toolbox container
Creates a new toolbox container. You can then use the `toolbox enter` command
to interact with the container at any point.

A requested image may live in a registry requiring authentication. Toolbox
tries to recognize such cases and offer the user an interactive prompt for
loging into the registry and then retrying to pull the image.

A toolbox container is an OCI container created from an OCI image. On Fedora,
the default image is known as `fedora-toolbox:N`, where N is the release of
the host. If the image is not present locally, then it is pulled from a
Expand Down Expand Up @@ -63,6 +67,25 @@ containers. Read more about the entry-point in `toolbox-init-container(1)`.

## OPTIONS ##

**--authfile** AUTHFILE

Path of the authentication file. Default is ${XDG\_RUNTIME\_DIR}/containers/auth.json

Note: You can also override the default path of the authentication file by
setting the REGISTRY\_AUTH\_FILE environment variable. `export REGISTRY_AUTH_FILE=path`

Note: This option is equal to one in `podman-create(1)`.

**--creds** USERNAME:PASSWORD

Credentials used to authenticate with a registry if required. Both values have
to be provided beforehand.

Without this option an attempt to log into a registry will not be made if the
global `--assumeyes` option is used.

Note: This option is analogical to one in `podman-pull(1)`.

**--distro** DISTRO, **-d** DISTRO

Create a toolbox container for a different operating system DISTRO than the
Expand Down Expand Up @@ -104,4 +127,5 @@ $ toolbox create --image bar foo

## SEE ALSO

`toolbox(1)`, `toolbox-init-container(1)`, `podman(1)`, `podman-create(1)`
`toolbox(1)`, `toolbox-init-container(1)`, `podman(1)`, `podman-create(1)`,
`podman-pull(1)`
101 changes: 99 additions & 2 deletions src/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ const (

var (
createFlags struct {
authfile string
container string
creds string
distro string
image string
release string
Expand All @@ -66,12 +68,22 @@ var createCmd = &cobra.Command{
func init() {
flags := createCmd.Flags()

flags.StringVar(&createFlags.authfile,
"authfile",
"",
"Path of the authentication file")

flags.StringVarP(&createFlags.container,
"container",
"c",
"",
"Assign a different name to the toolbox container")

flags.StringVar(&createFlags.creds,
"creds",
"",
"Credentials (USERNAME:PASSWORD) to use for authenticating to a registry")

flags.StringVarP(&createFlags.distro,
"distro",
"d",
Expand Down Expand Up @@ -115,6 +127,17 @@ func create(cmd *cobra.Command, args []string) error {
return errors.New("options --image and --release cannot be used together")
}

if cmd.Flag("creds").Changed {
if strings.Count(createFlags.creds, ":") != 1 {
return errors.New("option --creds accepts values in format USERNAME:PASSWORD")
}

creds := strings.Split(createFlags.creds, ":")
if creds[0] == "" || creds[1] == "" {
return errors.New("option --creds requires both parts of value to have at least 1 character")
}
}

var container string
var containerArg string

Expand Down Expand Up @@ -656,6 +679,8 @@ func isPathReadWrite(path string) (bool, error) {
}

func pullImage(image, release string) (bool, error) {
var didLogin bool = false

if _, err := utils.ImageReferenceCanBeID(image); err == nil {
logrus.Debugf("Looking for image %s", image)

Expand Down Expand Up @@ -720,18 +745,90 @@ func pullImage(image, release string) (bool, error) {

logrus.Debugf("Pulling image %s", imageFull)

pull_image:
stdoutFd := os.Stdout.Fd()
stdoutFdInt := int(stdoutFd)
var s *spinner.Spinner = spinner.New(spinner.CharSets[9], 500*time.Millisecond)
if logLevel := logrus.GetLevel(); logLevel < logrus.DebugLevel && terminal.IsTerminal(stdoutFdInt) {
s := spinner.New(spinner.CharSets[9], 500*time.Millisecond)
s.Prefix = fmt.Sprintf("Pulling %s: ", imageFull)
s.Writer = os.Stdout
s.Start()
defer s.Stop()
}

if err := podman.Pull(imageFull); err != nil {
return false, fmt.Errorf("failed to pull image %s", imageFull)
if errors.Is(err, podman.ErrUnknownImage) {
var builder strings.Builder
fmt.Fprintf(&builder, "The requested image does not seem to exist\n")
fmt.Fprintf(&builder, "Make sure the image URI is correct.")
errMsg := errors.New(builder.String())
return false, errMsg
}

if errors.Is(err, podman.ErrNotAuthorized) {
var builder strings.Builder

// This error should not be encountered for the second time after
// logging into a registry
if didLogin {
return false, fmt.Errorf("failed to pull image %s: unexpected unauthorized access", imageFull)
}

fmt.Fprintf(&builder, "Could not pull image %s\n", imageFull)
fmt.Fprintf(&builder, "The registry might require logging in but not necessarily.\n")

if rootFlags.assumeYes {
// We don't want to block when no credentials were provided
if createFlags.creds == "" {
fmt.Fprintf(&builder, "See 'podman login --help' on how to login into a registry.")
errMsg := errors.New(builder.String())
return false, errMsg
}

s.Stop()
fmt.Fprintf(&builder, "Credentials were provided. Trying to log into %s\n", domain)
fmt.Fprintf(os.Stderr, builder.String())
creds := strings.Split(createFlags.creds, ":")
args := []string{"--username", creds[0], "--password", creds[1]}
if err = podman.Login(domain, args...); err != nil {
return false, err
}

fmt.Fprintf(os.Stderr, "Retrying to pull image %s\n", imageFull)
didLogin = true
goto pull_image
}

if !rootFlags.assumeYes {
s.Stop()
fmt.Fprintf(os.Stderr, builder.String())

if !utils.AskForConfirmation("Do you want to log into the registry and try to pull the image again? [y/N]") {
return false, nil
}

var args []string

if createFlags.authfile != "" {
args = append(args, "--authfile", createFlags.authfile)
}

if createFlags.creds != "" {
creds := strings.Split(createFlags.creds, ":")
args = append(args, "--username", creds[0], "--password", creds[1])
}

if err = podman.Login(domain, args...); err != nil {
return false, err
}

fmt.Fprintf(os.Stderr, "Retrying to pull image %s\n", imageFull)
didLogin = true
goto pull_image
}
}

return false, fmt.Errorf("failed to pull image %s: %w", imageFull, err)
}

return true, nil
Expand Down
77 changes: 72 additions & 5 deletions src/pkg/podman/podman.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"errors"
"fmt"
"io"
"sort"
"os"
"strings"

"github.com/HarryMichal/go-version"
Expand All @@ -38,6 +38,11 @@ var (
LogLevel = logrus.ErrorLevel
)

var (
ErrNotAuthorized = errors.New("possibly not authorized to registry")
ErrUnknownImage = errors.New("the pulled image is not known")
)

// CheckVersion compares provided version with the version of Podman.
//
// Takes in one string parameter that should be in the format that is used for versioning (eg. 1.0.0, 2.5.1-dev).
Expand Down Expand Up @@ -229,13 +234,75 @@ func IsToolboxImage(image string) (bool, error) {
return true, nil
}

// Pull pulls an image
func Login(registry string, extraArgs ...string) error {
var stderr bytes.Buffer

logLevelString := LogLevel.String()
args := []string{"--log-level", logLevelString, "login", registry}
args = append(args, extraArgs...)

if err := shell.Run("podman", os.Stdin, os.Stdout, &stderr, args...); err != nil {
if stderr.Len() == 0 {
return fmt.Errorf("failed to login to registry %s: %w", registry, err)
}

err := parseErrorMsg(&stderr)
return fmt.Errorf("failed to login to registry %s: %w", registry, err.Err)
}

return nil
}

// Pull pulls an image. Wraps around command 'podman pull'.
//
// The image pull can fail for many reasons. Few of those are:
// - unknown image (ErrUnknownImage)
// - unauthorized access to registry (ErrNotAuthorized)
// - general network issue
//
// The wrapper tries to discern these errors to some extent and provide means
// for their handling. The provided errors can not be relied on completely as
// they are created based on error message parsing that differs based on used
// registry.
func Pull(imageName string) error {
var stderr bytes.Buffer

logLevelString := LogLevel.String()
args := []string{"--log-level", logLevelString, "pull", imageName}
args := []string{"--log-level", logLevelString, "pull", imageName, "--quiet"}

if err := shell.Run("podman", nil, nil, nil, args...); err != nil {
return err
if err := shell.Run("podman", nil, nil, &stderr, args...); err != nil {
if stderr.Len() == 0 {
return fmt.Errorf("failed to pull image %s: %w", imageName, err)
}

err := parseErrorMsg(&stderr)
// The following error handling is very fragile as it relies on
// specific strings in error messages which may change in the future.
// Some registries provide so useless error message that the cause of
// the problem is close to impossible to discern.

// Unknown image handling
// "name unknown" and "Repo not found" are part of error message when
// trying to pull an unknown image from registry.access.redhat.com
// "manifest unknown" is part of error message when trying to pull an
// unknown image from registry.fedoraproject.org, registry.centos.org and
// docker.io (after loging in)
// "Not Found" is part of error message when trying to pull an unknown
// image from registry.redhat.io (after loging in)
if err.Is(errors.New("name unknown")) || err.Is(errors.New("Repo not found")) ||
err.Is(errors.New("manifest unknown")) || err.Is(errors.New("Not Found")) {
return ErrUnknownImage
}

// Unathorized access handling
// "unauthorized" is part of error message when trying to pull from registry.redhat.io
// and quay.io
// "authentication required" is part of error message when trying to pull from docker.io
if err.Is(errors.New("unauthorized")) || err.Is(errors.New("authentication required")) {
return ErrNotAuthorized
}

return fmt.Errorf("failed to pull image %s: %w", imageName, err.Err)
}

return nil
Expand Down
39 changes: 39 additions & 0 deletions test/system/101-create.bats
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,26 @@ teardown() {
cleanup_containers
}

@test "create: Run command with invalid --creds flag (no colon)" {
run $TOOLBOX create --creds invalid

assert_failure
assert_output "Error: option --creds accepts values in format USERNAME:PASSWORD"
}

@test "create: Run command with invalid --creds flag (several colons)" {
run $TOOLBOX create --creds invalid:invalid:invalid

assert_failure
assert_output "Error: option --creds accepts values in format USERNAME:PASSWORD"
}

@test "create: Run command with invalid --creds flags (only colon)" {
run $TOOLBOX crreate --creds :

assert_failure
assert_output "Error: option --creds requires both parts of value to have at least 1 character"
}

@test "create: Create the default container" {
pull_default_image
Expand Down Expand Up @@ -69,3 +89,22 @@ teardown() {

assert_output --regexp "Created[[:blank:]]+fedora-toolbox-32"
}

@test "create: Try to create container based on image from private registry" {
run $TOOLBOX -y create -i registry.redhat.io/ubi8/ubi

assert_failure
if [ "${lines[0]}" != "Error: Could not pull image registry.redhat.io/ubi8/ubi" ]; then skip $skipreason_network; fi
if [ "${lines[1]}" != "The registry might require logging in but not necessarily." ]; then skip $skipreason_network; fi
if [ "${lines[2]}" != "See 'podman login' for more information." ]; then skip $skipreason_network; fi
}

@test "create: Try to create container based on image from private registry (provide false credentials)" {
run $TOOLBOX -y create -i registry.redhat.io/ubi8/ubi --creds wrong:wrong

assert_failure
if [ "${lines[0]}" != "Error: Could not pull image registry.redhat.io/ubi8/ubi" ]; then skip $skipreason_network; fi
if [ "${lines[1]}" != "The registry might require logging in but not necessarily." ]; then skip $skipreason_network; fi
if [ "${lines[2]}" != "Credentials were provided. Trying to log into registry.redhat.io" ]; then skip $skipreason_network; fi
if [ ! "${lines[4]}" =~ "Error: failed to login to registry.redhat.io"]; then skip $skipreason_network; fi
}
3 changes: 3 additions & 0 deletions test/system/libs/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ readonly SKOPEO=$(command -v skopeo)
readonly PROJECT_DIR=${PWD}
readonly IMAGE_CACHE_DIR="${PROJECT_DIR}/image-cache"

# Reasons for skipping tests
readonly skipreason_network="Possibly an network error occured. Skipping test."

# Images
declare -Ag IMAGES=([busybox]="docker.io/library/busybox" \
[fedora]="registry.fedoraproject.org/fedora-toolbox" \
Expand Down

0 comments on commit bbcd5e1

Please sign in to comment.