From 4ab65771a19dda1dfc6ed498d0411d8bfb6e796d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20M=C3=ADchal?= Date: Mon, 7 Jun 2021 21:46:45 +0200 Subject: [PATCH] cmd/create, pkg/podman: Add capabilities for logging to registries 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. https://github.com/containers/toolbox/pull/787 --- doc/toolbox-create.1.md | 26 ++++++++- src/cmd/create.go | 101 +++++++++++++++++++++++++++++++++- src/pkg/podman/podman.go | 77 ++++++++++++++++++++++++-- test/system/101-create.bats | 43 +++++++++++++++ test/system/libs/helpers.bash | 3 + 5 files changed, 242 insertions(+), 8 deletions(-) diff --git a/doc/toolbox-create.1.md b/doc/toolbox-create.1.md index 6ba9f9fea..1e1856d58 100644 --- a/doc/toolbox-create.1.md +++ b/doc/toolbox-create.1.md @@ -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 @@ -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 @@ -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)` diff --git a/src/cmd/create.go b/src/cmd/create.go index ad75e4983..0ddaa85eb 100644 --- a/src/cmd/create.go +++ b/src/cmd/create.go @@ -42,7 +42,9 @@ const ( var ( createFlags struct { + authfile string container string + creds string distro string image string release string @@ -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", @@ -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 @@ -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) @@ -720,10 +745,11 @@ 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() @@ -731,7 +757,78 @@ func pullImage(image, release string) (bool, error) { } 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 diff --git a/src/pkg/podman/podman.go b/src/pkg/podman/podman.go index 3d2f25f84..464f40509 100644 --- a/src/pkg/podman/podman.go +++ b/src/pkg/podman/podman.go @@ -22,7 +22,7 @@ import ( "errors" "fmt" "io" - "sort" + "os" "strings" "github.com/HarryMichal/go-version" @@ -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). @@ -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 diff --git a/test/system/101-create.bats b/test/system/101-create.bats index dfb4d89e1..95f6b78b1 100644 --- a/test/system/101-create.bats +++ b/test/system/101-create.bats @@ -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 create --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 @@ -69,3 +89,26 @@ teardown() { assert_output --regexp "Created[[:blank:]]+fedora-toolbox-32" } + +@test "create: Try to create container based on image from private registry" { + skip "Depends on unreliable network resources" + + run $TOOLBOX -y create -i registry.redhat.io/ubi8/ubi + + assert_failure + assert_line --index 0 "Error: Could not pull image registry.redhat.io/ubi8/ubi" + assert_line --index 1 "The registry might require logging in but not necessarily." + assert_line --index 2 "See 'podman login --help' on how to login into a registry." +} + +@test "create: Try to create container based on image from private registry (provide false credentials)" { + skip "Depends on unreliable network resources" + + run $TOOLBOX -y create -i registry.redhat.io/ubi8/ubi --creds wrong:wrong + + assert_failure + assert_line --index 0 "Error: Could not pull image registry.redhat.io/ubi8/ubi" + assert_line --index 1 "The registry might require logging in but not necessarily." + assert_line --index 2 "Credentials were provided. Trying to log into registry.redhat.io" + assert_line --index 3 --partial "Error: failed to login to registry.redhat.io" +} diff --git a/test/system/libs/helpers.bash b/test/system/libs/helpers.bash index dbfd5cc14..5c3550de5 100644 --- a/test/system/libs/helpers.bash +++ b/test/system/libs/helpers.bash @@ -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" \