refactor: cleaner image pulls (#2460)

## Description

Image pulls needed some love.

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/.github/CONTRIBUTING.md#developer-workflow)
followed

---------

Signed-off-by: razzle <harry@razzle.cloud>
Co-authored-by: Philip Laine <philip.laine@gmail.com>
Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com>
Co-authored-by: Lucas Rodriguez <lucas.rodriguez@defenseunicorns.com>

go.mod

@@ -46,6 +46,7 @@ require (
 	github.com/stretchr/testify v1.9.0
 	github.com/xeipuuv/gojsonschema v1.2.0
 	golang.org/x/crypto v0.23.0
+	golang.org/x/sync v0.7.0
 	golang.org/x/term v0.20.0
 	helm.sh/helm/v3 v3.14.2
 	k8s.io/api v0.29.1
@@ -476,7 +477,6 @@ require (
 	golang.org/x/mod v0.14.0 // indirect
 	golang.org/x/net v0.25.0 // indirect
 	golang.org/x/oauth2 v0.20.0 // indirect
-	golang.org/x/sync v0.7.0 // indirect
 	golang.org/x/sys v0.20.0 // indirect
 	golang.org/x/text v0.15.0 // indirect
 	golang.org/x/time v0.5.0 // indirect

src/cmd/tools/crane.go

@@ -13,6 +13,7 @@ import (
 	"github.com/defenseunicorns/zarf/src/cmd/common"
 	"github.com/defenseunicorns/zarf/src/config"
 	"github.com/defenseunicorns/zarf/src/config/lang"
+	"github.com/defenseunicorns/zarf/src/internal/packager/images"
 	"github.com/defenseunicorns/zarf/src/pkg/cluster"
 	"github.com/defenseunicorns/zarf/src/pkg/message"
 	"github.com/defenseunicorns/zarf/src/pkg/transform"
@@ -85,12 +86,12 @@ func init() {
 	craneCopy := craneCmd.NewCmdCopy(&craneOptions)
 
 	registryCmd.AddCommand(craneCopy)
-	registryCmd.AddCommand(zarfCraneCatalog(&craneOptions))
-	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdList, &craneOptions, lang.CmdToolsRegistryListExample, 0))
-	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPush, &craneOptions, lang.CmdToolsRegistryPushExample, 1))
-	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPull, &craneOptions, lang.CmdToolsRegistryPullExample, 0))
-	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDelete, &craneOptions, lang.CmdToolsRegistryDeleteExample, 0))
-	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDigest, &craneOptions, lang.CmdToolsRegistryDigestExample, 0))
+	registryCmd.AddCommand(zarfCraneCatalog(craneOptions))
+	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdList, craneOptions, lang.CmdToolsRegistryListExample, 0))
+	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPush, craneOptions, lang.CmdToolsRegistryPushExample, 1))
+	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdPull, craneOptions, lang.CmdToolsRegistryPullExample, 0))
+	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDelete, craneOptions, lang.CmdToolsRegistryDeleteExample, 0))
+	registryCmd.AddCommand(zarfCraneInternalWrapper(craneCmd.NewCmdDigest, craneOptions, lang.CmdToolsRegistryDigestExample, 0))
 	registryCmd.AddCommand(pruneCmd)
 	registryCmd.AddCommand(craneCmd.NewCmdVersion())
 
@@ -103,8 +104,8 @@ func init() {
 }
 
 // Wrap the original crane catalog with a zarf specific version
-func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command {
-	craneCatalog := craneCmd.NewCmdCatalog(cranePlatformOptions)
+func zarfCraneCatalog(cranePlatformOptions []crane.Option) *cobra.Command {
+	craneCatalog := craneCmd.NewCmdCatalog(&cranePlatformOptions)
 
 	craneCatalog.Example = lang.CmdToolsRegistryCatalogExample
 	craneCatalog.Args = nil
@@ -135,8 +136,8 @@ func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command {
 		}
 
 		// Add the correct authentication to the crane command options
-		authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PullUsername, zarfState.RegistryInfo.PullPassword)
-		*cranePlatformOptions = append(*cranePlatformOptions, authOption)
+		authOption := images.WithPullAuth(zarfState.RegistryInfo)
+		cranePlatformOptions = append(cranePlatformOptions, authOption)
 
 		if tunnel != nil {
 			message.Notef(lang.CmdToolsRegistryTunnel, registryEndpoint, zarfState.RegistryInfo.Address)
@@ -151,8 +152,8 @@ func zarfCraneCatalog(cranePlatformOptions *[]crane.Option) *cobra.Command {
 }
 
 // Wrap the original crane list with a zarf specific version
-func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command, cranePlatformOptions *[]crane.Option, exampleText string, imageNameArgumentIndex int) *cobra.Command {
-	wrappedCommand := commandToWrap(cranePlatformOptions)
+func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command, cranePlatformOptions []crane.Option, exampleText string, imageNameArgumentIndex int) *cobra.Command {
+	wrappedCommand := commandToWrap(&cranePlatformOptions)
 
 	wrappedCommand.Example = exampleText
 	wrappedCommand.Args = nil
@@ -190,8 +191,8 @@ func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command
 		}
 
 		// Add the correct authentication to the crane command options
-		authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PushUsername, zarfState.RegistryInfo.PushPassword)
-		*cranePlatformOptions = append(*cranePlatformOptions, authOption)
+		authOption := images.WithPushAuth(zarfState.RegistryInfo)
+		cranePlatformOptions = append(cranePlatformOptions, authOption)
 
 		if tunnel != nil {
 			message.Notef(lang.CmdToolsRegistryTunnel, tunnel.Endpoint(), zarfState.RegistryInfo.Address)
@@ -245,7 +246,7 @@ func pruneImages(_ *cobra.Command, _ []string) error {
 }
 
 func doPruneImagesForPackages(zarfState *types.ZarfState, zarfPackages []types.DeployedPackage, registryEndpoint string) error {
-	authOption := config.GetCraneAuthOption(zarfState.RegistryInfo.PushUsername, zarfState.RegistryInfo.PushPassword)
+	authOption := images.WithPushAuth(zarfState.RegistryInfo)
 
 	spinner := message.NewProgressSpinner(lang.CmdToolsRegistryPruneLookup)
 	defer spinner.Stop()

src/config/config.go

@@ -5,20 +5,15 @@
 package config
 
 import (
-	"crypto/tls"
 	"embed"
 	"fmt"
-	"net/http"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
 
 	"github.com/defenseunicorns/zarf/src/types"
-	"github.com/google/go-containerregistry/pkg/authn"
-	"github.com/google/go-containerregistry/pkg/crane"
-	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
 // Zarf Global Configuration Constants.
@@ -121,43 +116,6 @@ func GetDataInjectionMarker() string {
 	return fmt.Sprintf(dataInjectionMarker, operationStartTime)
 }
 
-// GetCraneOptions returns a crane option object with the correct options & platform.
-func GetCraneOptions(insecure bool, archs ...string) []crane.Option {
-	var options []crane.Option
-
-	// Handle insecure registry option
-	if insecure {
-		roundTripper := http.DefaultTransport.(*http.Transport).Clone()
-		roundTripper.TLSClientConfig = &tls.Config{
-			InsecureSkipVerify: true,
-		}
-		options = append(options, crane.Insecure, crane.WithTransport(roundTripper))
-	}
-
-	if archs != nil {
-		options = append(options, crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: GetArch(archs...)}))
-	}
-
-	options = append(options,
-		crane.WithUserAgent("zarf"),
-		crane.WithNoClobber(true),
-		// TODO: (@WSTARR) this is set to limit pushes to registry pods and reduce the likelihood that crane will get stuck.
-		// We should investigate this further in the future to dig into more of what is happening (see https://github.com/defenseunicorns/zarf/issues/1568)
-		crane.WithJobs(1),
-	)
-
-	return options
-}
-
-// GetCraneAuthOption returns a crane auth option with the provided credentials.
-func GetCraneAuthOption(username string, secret string) crane.Option {
-	return crane.WithAuth(
-		authn.FromConfig(authn.AuthConfig{
-			Username: username,
-			Password: secret,
-		}))
-}
-
 // GetAbsCachePath gets the absolute cache path for images and git repos.
 func GetAbsCachePath() string {
 	return GetAbsHomePath(CommonOptions.CachePath)

src/config/lang/english.go

@@ -729,11 +729,10 @@ const (
 
 // Collection of reusable error messages.
 var (
-	ErrInitNotFound         = errors.New("this command requires a zarf-init package, but one was not found on the local system. Re-run the last command again without '--confirm' to download the package")
-	ErrUnableToCheckArch    = errors.New("unable to get the configured cluster's architecture")
-	ErrInterrupt            = errors.New("execution cancelled due to an interrupt")
-	ErrUnableToGetPackages  = errors.New("unable to load the Zarf Package data from the cluster")
-	ErrUnsupportedImageType = errors.New("zarf does not currently support image indexes or docker manifest lists")
+	ErrInitNotFound        = errors.New("this command requires a zarf-init package, but one was not found on the local system. Re-run the last command again without '--confirm' to download the package")
+	ErrUnableToCheckArch   = errors.New("unable to get the configured cluster's architecture")
+	ErrInterrupt           = errors.New("execution cancelled due to an interrupt")
+	ErrUnableToGetPackages = errors.New("unable to load the Zarf Package data from the cluster")
 )
 
 // Collection of reusable warn messages.

src/internal/packager/images/common.go

@@ -5,23 +5,111 @@
 package images
 
 import (
+	"net/http"
+	"time"
+
+	"github.com/defenseunicorns/pkg/helpers"
+	"github.com/defenseunicorns/zarf/src/config"
+	"github.com/defenseunicorns/zarf/src/pkg/message"
 	"github.com/defenseunicorns/zarf/src/pkg/transform"
 	"github.com/defenseunicorns/zarf/src/types"
+	"github.com/google/go-containerregistry/pkg/authn"
+	"github.com/google/go-containerregistry/pkg/crane"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
-// ImageConfig is the main struct for managing container images.
-type ImageConfig struct {
-	ImagesPath string
+// PullConfig is the configuration for pulling images.
+type PullConfig struct {
+	DestinationDirectory string
+
+	ImageList []transform.Image
+
+	Arch string
+
+	RegistryOverrides map[string]string
+
+	CacheDirectory string
+}
+
+// PushConfig is the configuration for pushing images.
+type PushConfig struct {
+	SourceDirectory string
 
 	ImageList []transform.Image
 
 	RegInfo types.RegistryInfo
 
 	NoChecksum bool
 
-	Insecure bool
+	Arch string
+
+	Retries int
+}
 
-	Architectures []string
+// NoopOpt is a no-op option for crane.
+func NoopOpt(*crane.Options) {}
 
-	RegistryOverrides map[string]string
+// WithGlobalInsecureFlag returns an option for crane that configures insecure
+// based upon Zarf's global --insecure flag.
+func WithGlobalInsecureFlag() []crane.Option {
+	if config.CommonOptions.Insecure {
+		return []crane.Option{crane.Insecure}
+	}
+	// passing a nil option will cause panic
+	return []crane.Option{NoopOpt}
+}
+
+// WithArchitecture sets the platform option for crane.
+//
+// This option is actually a slight mis-use of the platform option, as it is
+// setting the architecture only and hard coding the OS to linux.
+func WithArchitecture(arch string) crane.Option {
+	return crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: arch})
+}
+
+// CommonOpts returns a set of common options for crane under Zarf.
+func CommonOpts(arch string) []crane.Option {
+	opts := WithGlobalInsecureFlag()
+	opts = append(opts, WithArchitecture(arch))
+
+	opts = append(opts,
+		crane.WithUserAgent("zarf"),
+		crane.WithNoClobber(true),
+		crane.WithJobs(1),
+	)
+	return opts
+}
+
+// WithBasicAuth returns an option for crane that sets basic auth.
+func WithBasicAuth(username, password string) crane.Option {
+	return crane.WithAuth(authn.FromConfig(authn.AuthConfig{
+		Username: username,
+		Password: password,
+	}))
+}
+
+// WithPullAuth returns an option for crane that sets pull auth from a given registry info.
+func WithPullAuth(ri types.RegistryInfo) crane.Option {
+	return WithBasicAuth(ri.PullUsername, ri.PullPassword)
+}
+
+// WithPushAuth returns an option for crane that sets push auth from a given registry info.
+func WithPushAuth(ri types.RegistryInfo) crane.Option {
+	return WithBasicAuth(ri.PushUsername, ri.PushPassword)
+}
+
+func createPushOpts(cfg PushConfig, pb *message.ProgressBar) []crane.Option {
+	opts := CommonOpts(cfg.Arch)
+	opts = append(opts, WithPushAuth(cfg.RegInfo))
+
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.TLSClientConfig.InsecureSkipVerify = config.CommonOptions.Insecure
+	// TODO (@WSTARR) This is set to match the TLSHandshakeTimeout to potentially mitigate effects of https://github.com/defenseunicorns/zarf/issues/1444
+	transport.ResponseHeaderTimeout = 10 * time.Second
+
+	transportWithProgressBar := helpers.NewTransport(transport, pb)
+
+	opts = append(opts, crane.WithTransport(transportWithProgressBar))
+
+	return opts
 }