From b88cc964c1ea2b3d0204a9230c8b6792577f7e5c 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 or tell the user that the requested image/manifest does not exist. The parsing process relies on error messages that match the OCI specification[0]. Testing of this feature is tricky as it relies on network which is inherently flaky. So, as part of the setup two local image registries are created and these features can be tested against them. To prevent the removal of the registries during testing a flag --namespace is used in Podman during their creation. [0] https://github.com/opencontainers/distribution-spec/blob/main/spec.md#error-codes https://github.com/containers/toolbox/pull/787 --- doc/toolbox-create.1.md | 26 +++++++++- src/cmd/create.go | 98 ++++++++++++++++++++++++++++++++++- src/pkg/podman/podman.go | 73 ++++++++++++++++++++++++-- test/system/000-setup.bats | 50 ++++++++++++++++++ test/system/101-create.bats | 68 ++++++++++++++++++++++++ test/system/999-teardown.bats | 8 +++ test/system/libs/helpers.bash | 3 +- 7 files changed, 318 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..e3f601522 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) @@ -718,12 +743,13 @@ func pullImage(image, release string) (bool, error) { return false, nil } +pull_image: logrus.Debugf("Pulling image %s", imageFull) 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,75 @@ 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.ErrNameUnknown) || errors.Is(err, podman.ErrManifestUnknown) { + var builder strings.Builder + fmt.Fprintf(&builder, "The requested image does not 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.ErrUnauthorized) { + 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 requires logging in.\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 + } + } + + 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, err } return true, nil diff --git a/src/pkg/podman/podman.go b/src/pkg/podman/podman.go index 3f6998466..816dd8d07 100644 --- a/src/pkg/podman/podman.go +++ b/src/pkg/podman/podman.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "os" "strings" "github.com/HarryMichal/go-version" @@ -37,6 +38,12 @@ var ( LogLevel = logrus.ErrorLevel ) +var ( + ErrUnauthorized = errors.New("not authorized to access resources in registry") + ErrManifestUnknown = errors.New("registry does not know the requested manifest") + ErrNameUnknown = errors.New("registry does not know the requested repository name") +) + // 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). @@ -228,13 +235,71 @@ 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. The recognized reasons are: +// - manifest unknown (ErrManifestUnknown) +// - name unknown (ErrNameUnknown) +// - unauthorized (ErrUnauthorized) +// +// To learn more about OCI API error codes, see: https://github.com/opencontainers/distribution-spec/blob/main/spec.md +// +// 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) + + // This error is returned when the manifest, identified by name and tag + // is unknown to the repository. + if err.Is(errors.New("manifest unknown")) { + return ErrManifestUnknown + } + + // This is returned if the name used during an operation is unknown to the + // registry. + if err.Is(errors.New("name unknown")) { + return ErrNameUnknown + } + + // The access controller was unable to authenticate the client. Often this + // will be accompanied by a Www-Authenticate HTTP response header + // indicating how to authenticate. + if err.Is(errors.New("unauthorized")) { + return ErrUnauthorized + } + + return fmt.Errorf("failed to pull image %s: %w", imageName, err.Err) } return nil diff --git a/test/system/000-setup.bats b/test/system/000-setup.bats index 67eeae0e1..c8b7c6deb 100644 --- a/test/system/000-setup.bats +++ b/test/system/000-setup.bats @@ -9,4 +9,54 @@ load 'libs/helpers' _pull_and_cache_distro_image fedora 32 || die _pull_and_cache_distro_image rhel 8.4 || die _pull_and_cache_distro_image busybox || die + + # Prepare localy hosted image registries + # The registries need to live in a separate instance of Podman to prevent + # them from being removed when cleaning state between test cases. + + # Create certificates for HTTPS + mkdir -p "$BATS_RUN_TMPDIR"/certs + run openssl req \ + -newkey rsa:4096 \ + -nodes -sha256 \ + -keyout "$BATS_RUN_TMPDIR"/certs/domain.key \ + -addext "subjectAltName = DNS:localhost" \ + -x509 \ + -days 365 \ + -subj '/' \ + -out "$BATS_RUN_TMPDIR"/certs/domain.crt + assert_success + + # Create testing registry user + mkdir -p "$BATS_RUN_TMPDIR"/auth + run $PODMAN --namespace registries run \ + --entrypoint htpasswd \ + httpd:2 -Bbn testuser testpassword > "$BATS_RUN_TMPDIR"/auth/htpasswd + assert_success + + # Create a Docker registry without authentication + run $PODMAN --namespace registries run -d \ + --name docker-registry-noauth \ + -v "$BATS_RUN_TMPDIR"/certs:/certs \ + -e REGISTRY_HTTP_ADDR=0.0.0.0:443 \ + -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/domain.crt \ + -e REGISTRY_HTTP_TLS_KEY=/certs/domain.key \ + -p 5000:443 \ + docker.io/library/registry:2 + assert_success + + # Create a Docker registry with authentication + $PODMAN --namespace registries run -d \ + --name docker-registry-auth \ + -v "$BATS_RUN_TMPDIR"/auth:/auth \ + -e REGISTRY_AUTH=htpasswd \ + -e REGISTRY_AUTH_HTPASSWD_REALM="Registry Realm" \ + -e REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd \ + -v "$BATS_RUN_TMPDIR"/certs:/certs \ + -e REGISTRY_HTTP_ADDR=0.0.0.0:443 \ + -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/domain.crt \ + -e REGISTRY_HTTP_TLS_KEY=/certs/domain.key \ + -p 5001:443 \ + docker.io/library/registry:2 + assert_success } diff --git a/test/system/101-create.bats b/test/system/101-create.bats index dfb4d89e1..3453db116 100644 --- a/test/system/101-create.bats +++ b/test/system/101-create.bats @@ -12,6 +12,26 @@ teardown() { cleanup_containers } +@test "create: Try to 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: Try to 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: Try to 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,51 @@ teardown() { assert_output --regexp "Created[[:blank:]]+fedora-toolbox-32" } + +@test "create: Try to create container based on non-existent image" { + run $TOOLBOX create -i localhost:5000/unknownimage + + assert_failure + assert_line --index 0 "Error: The requested image does not exist" + assert_line --index 1 "Make sure the image URI is correct." +} + +@test "create: Try to create container based on image from private registry" { + run $TOOLBOX -y create -i localhost:5001/ubi8/ubi + + assert_failure + assert_line --index 0 "Error: Could not pull image localhost:5001/ubi8/ubi" + assert_line --index 1 "The registry requires logging in." + 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)" { + run $TOOLBOX -y create -i localhost:5001/ubi8/ubi --creds wrong:wrong + + assert_failure + assert_line --index 0 "Error: Could not pull image localhost:5001/ubi8/ubi" + assert_line --index 1 "The registry requires logging in." + assert_line --index 2 "Credentials were provided. Trying to log into localhost:5001" + assert_line --index 3 --partial "Error: failed to login to localhost:5001" +} + +@test "create: Create container based on image from private registry (provide correct credentials)" { + run $TOOLBOX -y create -i localhost:5001/ubi8/ubi --creds testuser:testpassword + + assert_success + assert_line --index 0 "Error: Could not pull image localhost:5001/ubi8/ubi" + assert_line --index 1 "The registry requires logging in." + assert_line --index 2 "Credentials were provided. Trying to log into localhost:5001" +} + +@test "create: Create container based on image from private registry (log in beforehand)" { + run $PODMAN login localhost:5001 --username testuser --password testpassword + + assert_success + + run $TOOLBOX -y create my-ubi -i localhost:5001/ubi8/ubi + + assert_success + assert_line --index 0 "Created container: my-ubi" + assert_line --index 1 "Enter with: toolbox enter my-ubi" +} diff --git a/test/system/999-teardown.bats b/test/system/999-teardown.bats index 4ed580c8a..188d4d89f 100644 --- a/test/system/999-teardown.bats +++ b/test/system/999-teardown.bats @@ -4,4 +4,12 @@ load 'libs/helpers' @test "test suite: Teardown" { _clean_cached_images + + # Remove containers & with registries + run $PODMAN --namespace registries rm -a + run $PODMAN --namepsace registries rmi -a + + # Remove support files/dirs + rm -rf "$BATS_RUN_TMPDIR"/auth + rm -rf "$BATS_RUN_TMPDIR"/certs } diff --git a/test/system/libs/helpers.bash b/test/system/libs/helpers.bash index dbfd5cc14..923011a90 100644 --- a/test/system/libs/helpers.bash +++ b/test/system/libs/helpers.bash @@ -18,7 +18,8 @@ declare -Ag IMAGES=([busybox]="docker.io/library/busybox" \ function cleanup_all() { - $PODMAN system reset --force >/dev/null + $PODMAN rm --all --force >/dev/null + $PODMAN rmi --all --force >/dev/null }