Skip to content

Commit 5d2af16

Browse files
committed
cmd/create: Rework download prompt flow for cross-arch verification
Rework the image download prompt flow to support architecture verification before pulling non-native images. The new implementation ensures that the image inspection completes for the non-native creation path before it is pulled, so the image's architecture can be verified. The previous implementation used promptForDownloadError as a control flow mechanism between the first and second download prompts. Replace this with the pullImageDecision enum (pullNo, pullYes, pullUnknown) for clearer three-state signaling. Replaced getImageSizeFromRegistryAsync() with getImageFromRegistryAsync(), which now returns the full skopeo.Image struct instead of just the image size string. It calls skopeo.Inspect() (updated in [1]), making image metadata available throughout the download prompt flow for both size display and architecture verification in a single inspect call. Use Image.GetSizeHuman() (added in [1]) for image size display in the second download prompt, replacing the local size computation. Update showPromptForDownloadFirst() to return (pullImageDecision, *skopeo.Image, error). For non-native architectures, when the user confirms the download, the function now waits for the skopeo inspect to complete (with a spinner) before returning, ensuring that architecture verification can happen before the pull begins. Update pullImage() to verify the image architecture before pulling non-native images by calling VerifyArchitectureMatch() (added in [1]) to catch incompatible single-architecture images. Handle the case where the inspect returns nil (multi-arch manifest has no matching variant) with an explicit error. Detect a missing skopeo binary via exec.ErrNotFound, which is only a soft dependency of the Toolbx package, as it is not required for running non-native containers, and report it through createErrorSkopeoNotFound() added in utils.go. [1] #1784 #1787 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
1 parent 3d41df5 commit 5d2af16

2 files changed

Lines changed: 135 additions & 93 deletions

File tree

src/cmd/create.go

Lines changed: 127 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"os"
24+
"os/exec"
2425
"path/filepath"
2526
"strings"
2627
"time"
@@ -32,22 +33,25 @@ import (
3233
"github.com/containers/toolbox/pkg/skopeo"
3334
"github.com/containers/toolbox/pkg/term"
3435
"github.com/containers/toolbox/pkg/utils"
35-
"github.com/docker/go-units"
3636
"github.com/godbus/dbus/v5"
3737
"github.com/sirupsen/logrus"
3838
"github.com/spf13/cobra"
3939
)
4040

41-
type promptForDownloadError struct {
42-
ImageSize string
43-
}
44-
4541
const (
4642
alpha = `abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ`
4743
num = `0123456789`
4844
alphanum = alpha + num
4945
)
5046

47+
type pullImageDecision int
48+
49+
const (
50+
pullNo pullImageDecision = iota // 0 - User declined or default
51+
pullYes // 1 - User confirmed
52+
pullUnknown // 2 - Need second prompt
53+
)
54+
5155
var (
5256
createFlags struct {
5357
arch string
@@ -540,6 +544,13 @@ func createContainer(container, image, release, authFile string, archConfig arch
540544
return nil
541545
}
542546

547+
func boolToPullDecision(shouldPull bool) pullImageDecision {
548+
if shouldPull {
549+
return pullYes
550+
}
551+
return pullNo
552+
}
553+
543554
func createHelp(cmd *cobra.Command, args []string) {
544555
if utils.IsInsideContainer() {
545556
if !utils.IsInsideToolboxContainer() {
@@ -599,42 +610,19 @@ func getEnterCommand(container string) string {
599610
return enterCommand
600611
}
601612

602-
func getImageSizeFromRegistry(ctx context.Context, imageFull string) (string, error) {
603-
image, err := skopeo.Inspect(ctx, imageFull, architecture.HostArchID, "")
604-
if err != nil {
605-
return "", err
606-
}
607-
608-
if image.LayersData == nil {
609-
return "", errors.New("'skopeo inspect' did not have LayersData")
610-
}
611-
612-
var imageSizeFloat float64
613-
614-
for _, layer := range image.LayersData {
615-
if layerSize, err := layer.Size.Float64(); err != nil {
616-
return "", err
617-
} else {
618-
imageSizeFloat += layerSize
619-
}
620-
}
621-
622-
imageSizeHuman := units.HumanSize(imageSizeFloat)
623-
return imageSizeHuman, nil
624-
}
625-
626-
func getImageSizeFromRegistryAsync(ctx context.Context, imageFull string) (<-chan string, <-chan error) {
627-
retValCh := make(chan string)
613+
func getImageFromRegistryAsync(ctx context.Context, imageFull string, archID int, authFile string) (<-chan *skopeo.Image, <-chan error) {
614+
retValCh := make(chan *skopeo.Image)
628615
errCh := make(chan error)
629616

630617
go func() {
631-
imageSize, err := getImageSizeFromRegistry(ctx, imageFull)
618+
image, err := skopeo.Inspect(ctx, imageFull, archID, authFile)
619+
632620
if err != nil {
633621
errCh <- err
634622
return
635623
}
636624

637-
retValCh <- imageSize
625+
retValCh <- image
638626
}()
639627

640628
return retValCh, errCh
@@ -745,10 +733,18 @@ func pullImage(image, release, authFile string, archID int) (bool, bool, error)
745733

746734
promptForDownload := true
747735
var shouldPullImage bool
736+
var imageInfo *skopeo.Image
737+
var imageInspectErr error
748738

749739
if rootFlags.assumeYes || domain == "localhost" {
750740
promptForDownload = false
751741
shouldPullImage = true
742+
743+
if isNonNativeArch {
744+
s := startSpinner("Fetching non-native architecture image info: ")
745+
imageInfo, imageInspectErr = skopeo.Inspect(context.Background(), imageFull, archID, authFile)
746+
stopSpinner(s)
747+
}
752748
}
753749

754750
if promptForDownload {
@@ -762,13 +758,22 @@ func pullImage(image, release, authFile string, archID int) (bool, bool, error)
762758
return false, false, errors.New(errMsg)
763759
}
764760

765-
shouldPullImage = showPromptForDownload(imageFull)
761+
shouldPullImage, imageInfo, imageInspectErr = showPromptForDownload(imageFull, archID, authFile)
766762
}
767763

768764
if !shouldPullImage {
769765
return false, false, nil
770766
}
771767

768+
if imageInspectErr != nil && isNonNativeArch {
769+
if errors.Is(imageInspectErr, exec.ErrNotFound) {
770+
return false, false, createErrorSkopeoNotFound(imageFull, archID)
771+
}
772+
773+
// For now, log and continue (imageInfo will be nil)
774+
logrus.Debugf("Failed to inspect image: %s", imageInspectErr)
775+
}
776+
772777
logrus.Debugf("Pulling image %s", imageFull)
773778

774779
s := startSpinner(fmt.Sprintf("Pulling %s: ", imageFull))
@@ -783,6 +788,17 @@ func pullImage(image, release, authFile string, archID int) (bool, bool, error)
783788
} else {
784789
logrus.Debugf("'skopeo copy' is used for pulling non-native architecture image %s", imageFull)
785790

791+
if imageInfo == nil {
792+
// Multi-arch image mismatch
793+
expectedArchName := architecture.GetArchNameOCI(archID)
794+
return false, false, fmt.Errorf("failed to verify: image %s does not support architecture %s or the image does not exists at all",
795+
imageFull, expectedArchName)
796+
}
797+
798+
if err := imageInfo.VerifyArchitectureMatch(archID); err != nil {
799+
return false, false, err
800+
}
801+
786802
if err := skopeo.CopyOverrideArch(imageFull, imageFullWithArch, archID, authFile); err != nil {
787803
return false, false, createErrorImagePull(imageFull, domain)
788804
}
@@ -802,57 +818,79 @@ func createPromptForDownload(imageFull, imageSize string) string {
802818
return prompt
803819
}
804820

805-
func showPromptForDownloadFirst(imageFull string) (bool, error) {
821+
func showPromptForDownloadFirst(imageFull string, archID int, authFile string) (pullImageDecision, *skopeo.Image, error) {
806822
prompt := createPromptForDownload(imageFull, " ... MB")
823+
isNonnativeArch := !architecture.HasContainerNativeArch(archID)
807824

808825
parentCtx := context.Background()
809826
askCtx, askCancel := context.WithCancelCause(parentCtx)
810827
defer askCancel(errors.New("clean-up"))
811828

812829
askCh, askErrCh := askForConfirmationAsync(askCtx, prompt, nil)
813830

814-
imageSizeCtx, imageSizeCancel := context.WithCancelCause(parentCtx)
815-
defer imageSizeCancel(errors.New("clean-up"))
831+
imageCtx, imageCancel := context.WithCancelCause(parentCtx)
832+
defer imageCancel(errors.New("clean-up"))
816833

817-
imageSizeCh, imageSizeErrCh := getImageSizeFromRegistryAsync(imageSizeCtx, imageFull)
834+
imageCh, imageErrCh := getImageFromRegistryAsync(imageCtx, imageFull, archID, authFile)
818835

819-
var imageSize string
820-
var shouldPullImage bool
836+
var image *skopeo.Image = nil
837+
var shouldPullImage pullImageDecision = pullNo
838+
var imageInspectErr error = nil
821839

822840
select {
823841
case val := <-askCh:
824-
shouldPullImage = val
825-
cause := fmt.Errorf("%w: received confirmation without image size", context.Canceled)
826-
imageSizeCancel(cause)
842+
shouldPullImage = boolToPullDecision(val)
843+
844+
if isNonnativeArch {
845+
if shouldPullImage == pullNo {
846+
return pullNo, nil, nil
847+
}
848+
849+
s := startSpinner("Fetching non-native architecture image info: ")
850+
851+
select {
852+
case img := <-imageCh:
853+
stopSpinner(s)
854+
image = img
855+
return shouldPullImage, image, nil
856+
case err := <-imageErrCh:
857+
stopSpinner(s)
858+
return shouldPullImage, nil, err
859+
}
860+
} else {
861+
cause := fmt.Errorf("%w: received confirmation without image info", context.Canceled)
862+
imageCancel(cause)
863+
}
827864
case err := <-askErrCh:
828-
shouldPullImage = false
829-
cause := fmt.Errorf("failed to ask for confirmation without image size: %w", err)
830-
imageSizeCancel(cause)
831-
case val := <-imageSizeCh:
832-
imageSize = val
833-
cause := fmt.Errorf("%w: received image size", context.Canceled)
865+
shouldPullImage = pullNo
866+
cause := fmt.Errorf("failed to ask for confirmation without image info: %w", err)
867+
imageCancel(cause)
868+
case val := <-imageCh:
869+
image = val
870+
cause := fmt.Errorf("%w: received image info", context.Canceled)
834871
askCancel(cause)
835-
case err := <-imageSizeErrCh:
836-
cause := fmt.Errorf("failed to get image size: %w", err)
872+
case err := <-imageErrCh:
873+
imageInspectErr = err
874+
cause := fmt.Errorf("failed to get image info: %w", err)
837875
askCancel(cause)
838876
}
839877

840-
if imageSizeCtx.Err() != nil && askCtx.Err() == nil {
841-
cause := context.Cause(imageSizeCtx)
842-
logrus.Debugf("Show prompt for download: image size canceled: %s", cause)
843-
return shouldPullImage, nil
878+
if imageCtx.Err() != nil && askCtx.Err() == nil {
879+
cause := context.Cause(imageCtx)
880+
logrus.Debugf("Show prompt for download: image info canceled: %s", cause)
881+
return shouldPullImage, nil, nil
844882
}
845883

846884
var done bool
847885

848-
if imageSizeCtx.Err() == nil && askCtx.Err() != nil {
886+
if imageCtx.Err() == nil && askCtx.Err() != nil {
849887
select {
850888
case val := <-askCh:
851-
logrus.Debugf("Show prompt for download: received pending confirmation without image size")
852-
shouldPullImage = val
889+
logrus.Debugf("Show prompt for download: received pending confirmation without image info")
890+
shouldPullImage = boolToPullDecision(val)
853891
done = true
854892
case err := <-askErrCh:
855-
logrus.Debugf("Show prompt for download: failed to ask for confirmation without image size: %s",
893+
logrus.Debugf("Show prompt for download: failed to ask for confirmation without image info: %s",
856894
err)
857895
}
858896
} else {
@@ -863,13 +901,13 @@ func showPromptForDownloadFirst(imageFull string) (bool, error) {
863901
logrus.Debugf("Show prompt for download: ask canceled: %s", cause)
864902

865903
if done {
866-
return shouldPullImage, nil
904+
return shouldPullImage, image, imageInspectErr
905+
} else {
906+
return pullUnknown, image, imageInspectErr
867907
}
868-
869-
return false, &promptForDownloadError{imageSize}
870908
}
871909

872-
func showPromptForDownloadSecond(imageFull string, errFirst *promptForDownloadError) bool {
910+
func showPromptForDownloadSecond(imageFull, imageSize string) bool {
873911
oldState, err := term.GetState(os.Stdin)
874912
if err != nil {
875913
logrus.Debugf("Show prompt for download: failed to get terminal state: %s", err)
@@ -895,12 +933,7 @@ func showPromptForDownloadSecond(imageFull string, errFirst *promptForDownloadEr
895933

896934
discardCh, discardErrCh := discardInputAsync(discardCtx)
897935

898-
var prompt string
899-
if errors.Is(errFirst, context.Canceled) {
900-
prompt = createPromptForDownload(imageFull, errFirst.ImageSize)
901-
} else {
902-
prompt = createPromptForDownload(imageFull, "")
903-
}
936+
prompt := createPromptForDownload(imageFull, imageSize)
904937

905938
fmt.Printf("\r")
906939

@@ -981,22 +1014,37 @@ func showPromptForDownloadSecond(imageFull string, errFirst *promptForDownloadEr
9811014
return shouldPullImage
9821015
}
9831016

984-
func showPromptForDownload(imageFull string) bool {
1017+
func showPromptForDownload(imageFull string, archID int, authFile string) (bool, *skopeo.Image, error) {
9851018
fmt.Println("Image required to create Toolbx container.")
9861019

987-
shouldPullImage, err := showPromptForDownloadFirst(imageFull)
988-
if err == nil {
989-
return shouldPullImage
1020+
var shouldPullImageFirst pullImageDecision
1021+
var image *skopeo.Image
1022+
var imageInspectErr error
1023+
1024+
shouldPullImageFirst, image, imageInspectErr = showPromptForDownloadFirst(imageFull, archID, authFile)
1025+
1026+
switch shouldPullImageFirst {
1027+
case pullYes:
1028+
return true, image, imageInspectErr
1029+
case pullNo:
1030+
return false, image, imageInspectErr
9901031
}
9911032

992-
var errPromptForDownload *promptForDownloadError
993-
if !errors.As(err, &errPromptForDownload) {
994-
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
995-
panic(panicMsg)
1033+
var imageSize string
1034+
var shouldPullImageSecond bool
1035+
var getSizeErr error
1036+
1037+
if image == nil {
1038+
imageSize = "n/a"
1039+
} else {
1040+
imageSize, getSizeErr = image.GetSizeHuman()
1041+
if getSizeErr != nil {
1042+
imageSize = "n/a"
1043+
}
9961044
}
9971045

998-
shouldPullImage = showPromptForDownloadSecond(imageFull, errPromptForDownload)
999-
return shouldPullImage
1046+
shouldPullImageSecond = showPromptForDownloadSecond(imageFull, imageSize)
1047+
return shouldPullImageSecond, image, imageInspectErr
10001048
}
10011049

10021050
func startSpinner(message string) *spinner.Spinner {
@@ -1042,17 +1090,3 @@ func systemdPathBusEscape(path string) string {
10421090
}
10431091
return string(n)
10441092
}
1045-
1046-
func (err *promptForDownloadError) Error() string {
1047-
innerErr := err.Unwrap()
1048-
errMsg := innerErr.Error()
1049-
return errMsg
1050-
}
1051-
1052-
func (err *promptForDownloadError) Unwrap() error {
1053-
if err.ImageSize == "" {
1054-
return errors.New("failed to get image size")
1055-
}
1056-
1057-
return context.Canceled
1058-
}

src/cmd/utils.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,14 @@ func createErrorProfileDNotFound() error {
367367
return errors.New(errMsg)
368368
}
369369

370+
func createErrorSkopeoNotFound(imageFull string, archID int) error {
371+
archName := architecture.GetArchNameOCI(archID)
372+
return fmt.Errorf(
373+
"Cannot inspect image %s for architecture %s: skopeo is not installed.\n"+
374+
"Skopeo is required for creating non-native architecture containers.",
375+
imageFull, archName)
376+
}
377+
370378
func createErrorSudoersDNotFound() error {
371379
const sudoersD = "/etc/sudoers.d"
372380

0 commit comments

Comments
 (0)