refactor: use container image repo client from go-containerregistry (#…

…2018)

Signed-off-by: Kent Rancourt <[email protected]>
Co-authored-by: Hidde Beydals <[email protected]>

go.mod

@@ -13,19 +13,17 @@ require (
 	github.com/bacongobbler/browser v1.1.0
 	github.com/bombsimon/logrusr/v4 v4.1.0
 	github.com/coreos/go-oidc/v3 v3.10.0
-	github.com/distribution/distribution/v3 v3.0.0-20230722181636-7b502560cad4
 	github.com/evanphx/json-patch/v5 v5.9.0
 	github.com/fatih/structtag v1.2.0
 	github.com/gobwas/glob v0.2.3
 	github.com/gogo/protobuf v1.3.2
 	github.com/golang-jwt/jwt/v5 v5.2.1
+	github.com/google/go-containerregistry v0.14.0
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/go-cleanhttp v0.5.2
 	github.com/kelseyhightower/envconfig v1.4.0
 	github.com/klauspost/compress v1.17.8
 	github.com/oklog/ulid/v2 v2.1.0
-	github.com/opencontainers/go-digest v1.0.0
-	github.com/opencontainers/image-spec v1.1.0
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/rs/cors v1.11.0
 	github.com/sirupsen/logrus v1.9.3
@@ -99,7 +97,6 @@ require (
 	github.com/gorilla/mux v1.8.0 // indirect
 	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
 	github.com/hashicorp/go-retryablehttp v0.7.2 // indirect
-	github.com/hashicorp/golang-lru v0.5.4 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
@@ -152,4 +149,10 @@ require (
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )
 
-require github.com/go-jose/go-jose/v4 v4.0.1 // indirect
+require (
+	github.com/distribution/distribution/v3 v3.0.0-20230722181636-7b502560cad4 // indirect
+	github.com/go-jose/go-jose/v4 v4.0.1 // indirect
+	github.com/mitchellh/go-homedir v1.1.0 // indirect
+	github.com/opencontainers/go-digest v1.0.0 // indirect
+	github.com/opencontainers/image-spec v1.1.0 // indirect
+)

go.sum

@@ -57,6 +57,8 @@ github.com/containerd/continuity v0.4.2 h1:v3y/4Yz5jwnvqPKJJ+7Wf93fyWoCB3F5EclWG
 github.com/containerd/continuity v0.4.2/go.mod h1:F6PTNCKepoxEaXLQp3wDAjygEnImnZ/7o4JzpodfroQ=
 github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I=
 github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo=
+github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSkDwbkEETK84kQgEeFwDC+62k=
+github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o=
 github.com/coreos/go-oidc/v3 v3.10.0 h1:tDnXHnLyiTVyT/2zLDGj09pFPkhND8Gl8lnTRhoEaJU=
 github.com/coreos/go-oidc/v3 v3.10.0/go.mod h1:5j11xcw0D3+SGxn6Z/WFADsgcWVMyNAlSQupk0KK3ac=
 github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
@@ -167,6 +169,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/google/go-containerregistry v0.14.0 h1:z58vMqHxuwvAsVwvKEkmVBz2TlgBgH5k6koEXBtlYkw=
+github.com/google/go-containerregistry v0.14.0/go.mod h1:aiJ2fp/SXvkWgmYHioXnbMdlgB8eXiiYOY55gfN91Wk=
 github.com/google/go-github/v56 v56.0.0 h1:TysL7dMa/r7wsQi44BjqlwaHvwlFlqkK8CtBWCX3gb4=
 github.com/google/go-github/v56 v56.0.0/go.mod h1:D8cdcX98YWJvi7TLo7zM4/h8ZTx6u6fwGEkCdisopo0=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
@@ -242,6 +246,8 @@ github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvls
 github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k=
 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1fWh90gTKwiN4QCGoY9TWyyO4=
 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
+github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
+github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg=
 github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc=
 github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78=
@@ -336,6 +342,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/technosophos/moniker v0.0.0-20210218184952-3ea787d3943b h1:fo0GUa0B+vxSZ8bgnL3fpCPHReM/QPlALdak9T/Zw5Y=
 github.com/technosophos/moniker v0.0.0-20210218184952-3ea787d3943b/go.mod h1:O1c8HleITsZqzNZDjSNzirUGsMT0oGu9LhHKoJrqO+A=
+github.com/vbatts/tar-split v0.11.2 h1:Via6XqJr0hceW4wff3QRzD5gAk/tatMw/4ZA7cTlIME=
+github.com/vbatts/tar-split v0.11.2/go.mod h1:vV3ZuO2yWSVsz+pfFzDG/upWH1JhjOiEaWq6kXyQ3VI=
 github.com/withfig/autocomplete-tools/integrations/cobra v1.2.1 h1:+dBg5k7nuTE38VVdoroRsT0Z88fmvdYrI2EjzJst35I=
 github.com/withfig/autocomplete-tools/integrations/cobra v1.2.1/go.mod h1:nmuySobZb4kFgFy6BptpXp/BBw+xFSyvVPP6auoJB4k=
 github.com/xanzy/go-gitlab v0.105.0 h1:3nyLq0ESez0crcaM19o5S//SvezOQguuIHZ3wgX64hM=

internal/controller/warehouses/images.go

@@ -70,7 +70,7 @@ func (r *reconciler) discoverImages(
 		for _, img := range images {
 			discovery := kargoapi.DiscoveredImageReference{
 				Tag:        img.Tag,
-				Digest:     img.Digest.String(),
+				Digest:     img.Digest,
 				GitRepoURL: r.getImageSourceURL(sub.GitRepoURL, img.Tag),
 			}
 			if img.CreatedAt != nil {

internal/image/creds.go

@@ -1,34 +1,12 @@
 package image
 
-import "net/url"
-
 // Credentials represents the credentials for connecting to a private image
-// repository. It implements the
-// distribution/V3/registry/client/auth.CredentialStore interface.
+// repository.
 type Credentials struct {
 	// Username identifies a principal, which combined with the value of the
 	// Password field, can be used for reading from some image repository.
 	Username string
 	// Password, when combined with the principal identified by the Username
 	// field, can be used for reading from some image repository.
-	Password      string
-	refreshTokens map[string]string
-}
-
-// Basic implements distribution/V3/registry/client/auth.CredentialStore.
-func (c Credentials) Basic(*url.URL) (string, string) {
-	return c.Username, c.Password
-}
-
-// RefreshToken implements distribution/V3/registry/client/auth.CredentialStore.
-func (c Credentials) RefreshToken(_ *url.URL, service string) string {
-	return c.refreshTokens[service]
-}
-
-// SetRefreshToken implements
-// distribution/V3/registry/client/auth.CredentialStore.
-func (c Credentials) SetRefreshToken(_ *url.URL, service, token string) {
-	if c.refreshTokens != nil {
-		c.refreshTokens[service] = token
-	}
+	Password string
 }

internal/image/digest_selector.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 	log "github.com/sirupsen/logrus"
 
 	"github.com/akuity/kargo/internal/logging"
@@ -36,52 +38,33 @@ func newDigestSelector(
 
 // Select implements the Selector interface.
 func (d *digestSelector) Select(ctx context.Context) ([]Image, error) {
+	tag := d.constraint
 	logger := logging.LoggerFromContext(ctx).WithFields(log.Fields{
 		"registry":            d.repoClient.registry.name,
-		"image":               d.repoClient.image,
+		"image":               d.repoClient.repoURL,
 		"selectionStrategy":   SelectionStrategyDigest,
+		"tag":                 tag,
 		"platformConstrained": d.platform != nil,
 	})
 	logger.Trace("selecting image")
 
 	ctx = logging.ContextWithLogger(ctx, logger)
 
-	// TODO(hidde): it would be much more efficient to directly attempt
-	// to retrieve the image for the tag, while gracefully handling the
-	// case where it does not exist. This would avoid the need to list
-	// all tags and then iterate over them.
-	tags, err := d.repoClient.getTags(ctx)
+	image, err := d.repoClient.getImageByTag(ctx, tag, d.platform)
 	if err != nil {
-		return nil, fmt.Errorf("error listing tags: %w", err)
-	}
-	if len(tags) == 0 {
-		logger.Trace("found no tags")
-		return nil, nil
-	}
-	logger.Trace("got all tags")
-
-	for _, tag := range tags {
-		if tag != d.constraint {
-			continue
-		}
-		image, err := d.repoClient.getImageByTag(ctx, tag, d.platform)
-		if err != nil {
-			return nil, fmt.Errorf("error retrieving image with tag %q: %w", tag, err)
-		}
-		if image == nil {
-			logger.Tracef(
-				"image with tag %q was found, but did not match platform constraint",
-				tag,
-			)
+		var te *transport.Error
+		if errors.As(err, &te) && te.StatusCode == http.StatusNotFound {
+			logger.Trace("found no image with tag")
 			return nil, nil
 		}
-		logger.WithFields(log.Fields{
-			"tag":    image.Tag,
-			"digest": image.Digest.String(),
-		}).Trace("found image")
-		return []Image{*image}, nil
+		return nil, fmt.Errorf("error retrieving image with tag %q: %w", tag, err)
+	}
+
+	if image == nil {
+		logger.Trace("image with tag did not match platform constraints")
+		return nil, nil
 	}
 
-	logger.Trace("no images matched criteria")
-	return nil, nil
+	logger.Trace("found image with tag")
+	return []Image{*image}, nil
 }

internal/image/image.go

@@ -4,23 +4,22 @@ import (
 	"time"
 
 	"github.com/Masterminds/semver/v3"
-	"github.com/opencontainers/go-digest"
 )
 
 // Image is a representation of a container image.
 type Image struct {
 	Tag       string
-	Digest    digest.Digest
+	Digest    string
 	CreatedAt *time.Time
 	semVer    *semver.Version
 }
 
 // newImage initializes and returns an Image.
-func newImage(tag string, date *time.Time, digest digest.Digest) Image {
+func newImage(tag, digest string, date *time.Time) Image {
 	t := Image{
 		Tag:       tag,
-		CreatedAt: date,
 		Digest:    digest,
+		CreatedAt: date,
 	}
 	// It's ok if the tag doesn't parse as semver, but if it does, store it
 	if sv, err := semver.NewVersion(tag); err == nil {

internal/image/image_test.go

@@ -4,12 +4,11 @@ import (
 	"testing"
 	"time"
 
-	"github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/require"
 )
 
 func TestNewImage(t *testing.T) {
-	testDigest := digest.Digest("fake-digest")
+	const testDigest = "fake-digest"
 	testDate := time.Now().UTC()
 	testCases := []struct {
 		name       string
@@ -42,7 +41,7 @@ func TestNewImage(t *testing.T) {
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			testCase.assertions(t, newImage(testCase.tag, &testDate, testDigest))
+			testCase.assertions(t, newImage(testCase.tag, testDigest, &testDate))
 		})
 	}
 }

internal/image/lexical_selector.go

@@ -43,7 +43,7 @@ func newLexicalSelector(
 func (l *lexicalSelector) Select(ctx context.Context) ([]Image, error) {
 	logger := logging.LoggerFromContext(ctx).WithFields(log.Fields{
 		"registry":            l.repoClient.registry.name,
-		"image":               l.repoClient.image,
+		"image":               l.repoClient.repoURL,
 		"selectionStrategy":   SelectionStrategyLexical,
 		"platformConstrained": l.platform != nil,
 		"discoveryLimit":      l.discoveryLimit,
@@ -82,7 +82,7 @@ func (l *lexicalSelector) Select(ctx context.Context) ([]Image, error) {
 
 		logger.WithFields(log.Fields{
 			"tag":    image.Tag,
-			"digest": image.Digest.String(),
+			"digest": image.Digest,
 		}).Trace("discovered image")
 		images = append(images, *image)
 	}

internal/image/newest_build_selector.go

@@ -44,7 +44,7 @@ func newNewestBuildSelector(
 func (n *newestBuildSelector) Select(ctx context.Context) ([]Image, error) {
 	logger := logging.LoggerFromContext(ctx).WithFields(log.Fields{
 		"registry":            n.repoClient.registry.name,
-		"image":               n.repoClient.image,
+		"image":               n.repoClient.repoURL,
 		"selectionStrategy":   SelectionStrategyNewestBuild,
 		"platformConstrained": n.platform != nil,
 		"discoveryLimit":      n.discoveryLimit,
@@ -67,7 +67,7 @@ func (n *newestBuildSelector) Select(ctx context.Context) ([]Image, error) {
 		for _, image := range images[:limit] {
 			logger.WithFields(log.Fields{
 				"tag":    image.Tag,
-				"digest": image.Digest.String(),
+				"digest": image.Digest,
 			}).Trace("discovered image")
 		}
 		logger.Tracef("discovered %d images", limit)
@@ -86,20 +86,20 @@ func (n *newestBuildSelector) Select(ctx context.Context) ([]Image, error) {
 
 		discoveredImage, err := n.repoClient.getImageByDigest(ctx, image.Digest, n.platform)
 		if err != nil {
-			return nil, fmt.Errorf("error retrieving image with digest %q: %w", image.Digest.String(), err)
+			return nil, fmt.Errorf("error retrieving image with digest %q: %w", image.Digest, err)
 		}
 
 		if discoveredImage == nil {
 			logger.Tracef(
 				"image with digest %q was found, but did not match platform constraint",
-				image.Digest.String(),
+				image.Digest,
 			)
 			continue
 		}
 
 		logger.WithFields(log.Fields{
 			"tag":    image.Tag,
-			"digest": image.Digest.String(),
+			"digest": image.Digest,
 		}).Trace("discovered image")
 
 		discoveredImage.Tag = image.Tag

internal/image/registry.go

@@ -1,23 +1,18 @@
 package image
 
 import (
-	"fmt"
-	"strings"
 	"sync"
 	"time"
 
+	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/patrickmn/go-cache"
 	"go.uber.org/ratelimit"
 )
 
-// dockerRegistry is registry configuration for Docker Hub, whose API endpoint
-// cannot be inferred from an image prefix because its API endpoint is at
-// https://registry-1.docker.io despite Docker Hub images either lacking a
-// prefix entirely or beginning with docker.io
+// dockerRegistry is registry configuration for Docker Hub.
 var dockerRegistry = &registry{
 	name:             "Docker Hub",
-	imagePrefix:      "docker.io",
-	apiAddress:       "https://registry-1.docker.io",
+	imagePrefix:      name.DefaultRegistry,
 	defaultNamespace: "library",
 	imageCache: cache.New(
 		30*time.Minute, // Default ttl for each entry
@@ -43,7 +38,6 @@ var (
 type registry struct {
 	name             string
 	imagePrefix      string
-	apiAddress       string
 	defaultNamespace string
 	imageCache       *cache.Cache
 	rateLimiter      ratelimit.Limiter
@@ -54,7 +48,6 @@ func newRegistry(imagePrefix string) *registry {
 	return &registry{
 		name:        imagePrefix,
 		imagePrefix: imagePrefix,
-		apiAddress:  fmt.Sprintf("https://%s", imagePrefix),
 		imageCache: cache.New(
 			30*time.Minute, // Default ttl for each entry
 			time.Hour,      // Cleanup interval
@@ -77,14 +70,3 @@ func getRegistry(imagePrefix string) *registry {
 	registries[registry.imagePrefix] = registry
 	return registry
 }
-
-// normalizeImageName returns a normalized image name that accounts for the fact
-// that some registries have a default namespace that is used when the image
-// name doesn't specify one. For example on Docker Hub, "debian" officially
-// equates to "library/debian".
-func (r *registry) normalizeImageName(image string) string {
-	if len(strings.Split(image, "/")) == 1 && r.defaultNamespace != "" {
-		return fmt.Sprintf("%s/%s", r.defaultNamespace, image)
-	}
-	return image
-}