Switch from `crane` package to `remote`

`crane` package is the highest level of abstraction that GGCR provides,
it's easy to use, however it doesn't give user much control.
This change moves `OCIRepository` controller logic to a lower-level
`remote` package and makes handling of references more explicit with
`name.Repository`, `name.Digest` and `name.Tag`.
It also simplifies options builder, as there is no need to have separate
sets of options for cosign and crane.

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>

internal/controller/ocirepository_controller.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	cryptotls "crypto/tls"
 	"errors"
 	"fmt"
 	"io"
@@ -31,9 +32,9 @@ import (
 	"github.com/Masterminds/semver/v3"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/authn/k8schain"
-	"github.com/google/go-containerregistry/pkg/crane"
 	"github.com/google/go-containerregistry/pkg/name"
 	gcrv1 "github.com/google/go-containerregistry/pkg/v1"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -369,10 +370,10 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 		return sreconcile.ResultEmpty, e
 	}
 
-	opts := makeRemoteOptions(ctx, obj, transport, keychain, auth)
+	opts := makeRemoteOptions(ctx, transport, keychain, auth)
 
 	// Determine which artifact revision to pull
-	url, err := r.getArtifactURL(obj, opts.craneOpts)
+	ref, err := r.getArtifactRef(obj, opts)
 	if err != nil {
 		if _, ok := err.(invalidOCIURLError); ok {
 			e := serror.NewStalling(
@@ -390,7 +391,8 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	}
 
 	// Get the upstream revision from the artifact digest
-	revision, err := r.getRevision(url, opts.craneOpts)
+	// TODO: getRevision resolves the digest, which may change before image is fetched, so it should probaly update ref
+	revision, err := r.getRevision(ref, opts)
 	if err != nil {
 		e := serror.NewGeneric(
 			fmt.Errorf("failed to determine artifact digest: %w", err),
@@ -405,7 +407,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	// Mark observations about the revision on the object
 	defer func() {
 		if !obj.GetArtifact().HasRevision(revision) {
-			message := fmt.Sprintf("new revision '%s' for '%s'", revision, url)
+			message := fmt.Sprintf("new revision '%s' for '%s'", revision, ref)
 			if obj.GetArtifact() != nil {
 				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
 			}
@@ -428,7 +430,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 		conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation ||
 		conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) {
 
-		err := r.verifySignature(ctx, obj, url, opts.verifyOpts...)
+		err := r.verifySignature(ctx, obj, ref, opts...)
 		if err != nil {
 			provider := obj.Spec.Verify.Provider
 			if obj.Spec.Verify.SecretRef == nil {
@@ -453,7 +455,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	}
 
 	// Pull artifact from the remote container registry
-	img, err := crane.Pull(url, opts.craneOpts...)
+	img, err := remote.Image(ref, opts...)
 	if err != nil {
 		e := serror.NewGeneric(
 			fmt.Errorf("failed to pull artifact from '%s': %w", obj.Spec.URL, err),
@@ -573,37 +575,31 @@ func (r *OCIRepositoryReconciler) selectLayer(obj *ociv1.OCIRepository, image gc
 
 // getRevision fetches the upstream digest, returning the revision in the
 // format '<tag>@<digest>'.
-func (r *OCIRepositoryReconciler) getRevision(url string, options []crane.Option) (string, error) {
+func (r *OCIRepositoryReconciler) getRevision(ref name.Reference, options []remote.Option) (string, error) {
-	ref, err := name.ParseReference(url)
+	switch ref := ref.(type) {
+	case name.Digest:
+		digest, err := v1.NewHash(ref.DigestStr())
 		if err != nil {
 			return "", err
 		}
+		return digest.String(), nil
+	case name.Tag:
+		var digest v1.Hash
 
-	repoTag := ""
+		desc, err := remote.Head(ref, options...)
-	repoName := strings.TrimPrefix(url, ref.Context().RegistryStr())
+		if err == nil {
-	if s := strings.Split(repoName, ":"); len(s) == 2 && !strings.Contains(repoName, "@") {
+			digest = desc.Digest
-		repoTag = s[1]
+		} else {
-	}
+			rdesc, err := remote.Get(ref, options...)
-
-	if repoTag == "" && !strings.Contains(repoName, "@") {
-		repoTag = "latest"
-	}
-
-	digest, err := crane.Digest(url, options...)
 			if err != nil {
 				return "", err
 			}
-
+			digest = rdesc.Descriptor.Digest
-	digestHash, err := gcrv1.NewHash(digest)
-	if err != nil {
-		return "", err
 		}
-
+		return fmt.Sprintf("%s@%s", ref.TagStr(), digest.String()), nil
-	revision := digestHash.String()
+	default:
-	if repoTag != "" {
+		return "", fmt.Errorf("unsupported reference type: %T", ref)
-		revision = fmt.Sprintf("%s@%s", repoTag, revision)
 	}
-	return revision, nil
 }
 
 // digestFromRevision extracts the digest from the revision string.
@@ -615,7 +611,7 @@ func (r *OCIRepositoryReconciler) digestFromRevision(revision string) string {
 // verifySignature verifies the authenticity of the given image reference URL.
 // First, it tries to use a key if a Secret with a valid public key is provided.
 // If not, it falls back to a keyless approach for verification.
-func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv1.OCIRepository, url string, opt ...remote.Option) error {
+func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv1.OCIRepository, ref name.Reference, opt ...remote.Option) error {
 	ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
 	defer cancel()
 
@@ -626,15 +622,6 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv
 			soci.WithRemoteOptions(opt...),
 		}
 
-		var nameOpts []name.Option
-		if obj.Spec.Insecure {
-			nameOpts = append(nameOpts, name.Insecure)
-		}
-		ref, err := name.ParseReference(url, nameOpts...)
-		if err != nil {
-			return err
-		}
-
 		// get the public keys from the given secret
 		if secretRef := obj.Spec.Verify.SecretRef; secretRef != nil {
 			certSecretName := types.NamespacedName{
@@ -669,7 +656,7 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv
 			}
 
 			if !signatureVerified {
-				return fmt.Errorf("no matching signatures were found for '%s'", url)
+				return fmt.Errorf("no matching signatures were found for '%s'", ref)
 			}
 
 			return nil
@@ -691,71 +678,72 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv
 			return nil
 		}
 
-		return fmt.Errorf("no matching signatures were found for '%s'", url)
+		return fmt.Errorf("no matching signatures were found for '%s'", ref)
 	}
 
 	return nil
 }
 
-// parseRepositoryURL validates and extracts the repository URL.
+// parseRepository validates and extracts the repository URL.
-func (r *OCIRepositoryReconciler) parseRepositoryURL(obj *ociv1.OCIRepository) (string, error) {
+func (r *OCIRepositoryReconciler) parseRepository(obj *ociv1.OCIRepository) (name.Repository, error) {
 	if !strings.HasPrefix(obj.Spec.URL, ociv1.OCIRepositoryPrefix) {
-		return "", fmt.Errorf("URL must be in format 'oci://<domain>/<org>/<repo>'")
+		return name.Repository{}, fmt.Errorf("URL must be in format 'oci://<domain>/<org>/<repo>'")
 	}
 
 	url := strings.TrimPrefix(obj.Spec.URL, ociv1.OCIRepositoryPrefix)
-	ref, err := name.ParseReference(url)
+
+	options := []name.Option{}
+	if obj.Spec.Insecure {
+		options = append(options, name.Insecure)
+	}
+	repo, err := name.NewRepository(url, options...)
 	if err != nil {
-		return "", err
+		return name.Repository{}, err
 	}
 
-	imageName := strings.TrimPrefix(url, ref.Context().RegistryStr())
+	imageName := strings.TrimPrefix(url, repo.RegistryStr())
 	if s := strings.Split(imageName, ":"); len(s) > 1 {
-		return "", fmt.Errorf("URL must not contain a tag; remove ':%s'", s[1])
+		return name.Repository{}, fmt.Errorf("URL must not contain a tag; remove ':%s'", s[1])
 	}
 
-	return ref.Context().Name(), nil
+	return repo, nil
 }
 
-// getArtifactURL determines which tag or revision should be used and returns the OCI artifact FQN.
+// getArtifactRef determines which tag or revision should be used and returns the OCI artifact FQN.
-func (r *OCIRepositoryReconciler) getArtifactURL(obj *ociv1.OCIRepository, options []crane.Option) (string, error) {
+func (r *OCIRepositoryReconciler) getArtifactRef(obj *ociv1.OCIRepository, options []remote.Option) (name.Reference, error) {
-	url, err := r.parseRepositoryURL(obj)
+	repo, err := r.parseRepository(obj)
 	if err != nil {
-		return "", invalidOCIURLError{err}
+		return nil, invalidOCIURLError{err}
 	}
 
 	if obj.Spec.Reference != nil {
 		if obj.Spec.Reference.Digest != "" {
-			return fmt.Sprintf("%s@%s", url, obj.Spec.Reference.Digest), nil
+			return repo.Digest(obj.Spec.Reference.Digest), nil
 		}
 
 		if obj.Spec.Reference.SemVer != "" {
-			tag, err := r.getTagBySemver(url, obj.Spec.Reference.SemVer, options)
+			return r.getTagBySemver(repo, obj.Spec.Reference.SemVer, options)
-			if err != nil {
-				return "", err
-			}
-			return fmt.Sprintf("%s:%s", url, tag), nil
 		}
 
 		if obj.Spec.Reference.Tag != "" {
-			return fmt.Sprintf("%s:%s", url, obj.Spec.Reference.Tag), nil
+			return repo.Tag(obj.Spec.Reference.Tag), nil
 		}
 	}
 
-	return url, nil
+	return repo.Tag(name.DefaultTag), nil
 }
 
 // getTagBySemver call the remote container registry, fetches all the tags from the repository,
 // and returns the latest tag according to the semver expression.
-func (r *OCIRepositoryReconciler) getTagBySemver(url, exp string, options []crane.Option) (string, error) {
+func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp string, options []remote.Option) (name.Reference, error) {
-	tags, err := crane.ListTags(url, options...)
+	tags, err := remote.List(repo, options...)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	constraint, err := semver.NewConstraint(exp)
 	if err != nil {
-		return "", fmt.Errorf("semver '%s' parse error: %w", exp, err)
+		return nil, fmt.Errorf("semver '%s' parse error: %w", exp, err)
 	}
 
 	var matchingVersions []*semver.Version
@@ -771,11 +759,11 @@ func (r *OCIRepositoryReconciler) getTagBySemver(url, exp string, options []cran
 	}
 
 	if len(matchingVersions) == 0 {
-		return "", fmt.Errorf("no match found for semver: %s", exp)
+		return nil, fmt.Errorf("no match found for semver: %s", exp)
 	}
 
 	sort.Sort(sort.Reverse(semver.Collection(matchingVersions)))
-	return matchingVersions[0].Original(), nil
+	return repo.Tag(matchingVersions[0].Original()), nil
 }
 
 // keychain generates the credential keychain based on the resource
@@ -825,9 +813,16 @@ func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *ociv1.OCIRe
 
 // transport clones the default transport from remote and when a certSecretRef is specified,
 // the returned transport will include the TLS client and/or CA certificates.
-func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIRepository) (http.RoundTripper, error) {
+func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIRepository) (*http.Transport, error) {
+	transport := remote.DefaultTransport.(*http.Transport).Clone()
+
 	if obj.Spec.CertSecretRef == nil || obj.Spec.CertSecretRef.Name == "" {
-		return nil, nil
+		if obj.Spec.Insecure {
+			transport.TLSClientConfig = &cryptotls.Config{
+				InsecureSkipVerify: true,
+			}
+		}
+		return transport, nil
 	}
 
 	certSecretName := types.NamespacedName{
@@ -839,7 +834,6 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIR
 		return nil, err
 	}
 
-	transport := remote.DefaultTransport.(*http.Transport).Clone()
 	tlsConfig, _, err := tls.KubeTLSClientConfigFromSecret(certSecret, "")
 	if err != nil {
 		return nil, err
@@ -1155,55 +1149,28 @@ func (r *OCIRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *oc
 	}
 }
 
-// craneOptions sets the auth headers, timeout and user agent
-// for all operations against remote container registries.
-func craneOptions(ctx context.Context, insecure bool) []crane.Option {
-	options := []crane.Option{
-		crane.WithContext(ctx),
-		crane.WithUserAgent(oci.UserAgent),
-	}
-
-	if insecure {
-		options = append(options, crane.Insecure)
-	}
-
-	return options
-}
-
 // makeRemoteOptions returns a remoteOptions struct with the authentication and transport options set.
 // The returned struct can be used to interact with a remote registry using go-containerregistry based libraries.
-func makeRemoteOptions(ctxTimeout context.Context, obj *ociv1.OCIRepository, transport http.RoundTripper,
+func makeRemoteOptions(ctxTimeout context.Context, transport http.RoundTripper,
 	keychain authn.Keychain, auth authn.Authenticator) remoteOptions {
-	o := remoteOptions{
-		craneOpts:  craneOptions(ctxTimeout, obj.Spec.Insecure),
-		verifyOpts: []remote.Option{},
-	}
-
-	if transport != nil {
-		o.craneOpts = append(o.craneOpts, crane.WithTransport(transport))
-		o.verifyOpts = append(o.verifyOpts, remote.WithTransport(transport))
-	}
 
+	authOption := remote.WithAuthFromKeychain(keychain)
 	if auth != nil {
 		// auth take precedence over keychain here as we expect the caller to set
 		// the auth only if it is required.
-		o.verifyOpts = append(o.verifyOpts, remote.WithAuth(auth))
+		authOption = remote.WithAuth(auth)
-		o.craneOpts = append(o.craneOpts, crane.WithAuth(auth))
+	}
-		return o
+	return remoteOptions{
+		remote.WithContext(ctxTimeout),
+		remote.WithUserAgent(oci.UserAgent),
+		remote.WithTransport(transport),
+		authOption,
 	}
-
-	o.verifyOpts = append(o.verifyOpts, remote.WithAuthFromKeychain(keychain))
-	o.craneOpts = append(o.craneOpts, crane.WithAuthFromKeychain(keychain))
-
-	return o
 }
 
 // remoteOptions contains the options to interact with a remote registry.
 // It can be used to pass options to go-containerregistry based libraries.
-type remoteOptions struct {
+type remoteOptions []remote.Option
-	craneOpts  []crane.Option
-	verifyOpts []remote.Option
-}
 
 // ociContentConfigChanged evaluates the current spec with the observations
 // of the artifact in the status to determine if artifact content configuration

internal/controller/ocirepository_controller_test.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"crypto/rand"
 	"crypto/tls"
+	cryptotls "crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
@@ -38,6 +39,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/crane"
 	gcrv1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/mutate"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
 	. "github.com/onsi/gomega"
 	coptions "github.com/sigstore/cosign/v2/cmd/cosign/cli/options"
 	"github.com/sigstore/cosign/v2/cmd/cosign/cli/sign"
@@ -793,15 +795,14 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
 				patchOptions:  getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
 			}
 
-			opts := craneOptions(ctx, tt.insecure)
+			opts := makeRemoteOptions(ctx, makeTransport(tt.insecure), authn.DefaultKeychain, nil)
-			opts = append(opts, crane.WithAuthFromKeychain(authn.DefaultKeychain))
+			ref, err := r.getArtifactRef(obj, opts)
-			repoURL, err := r.getArtifactURL(obj, opts)
 			g.Expect(err).To(BeNil())
 
 			assertConditions := tt.assertConditions
 			for k := range assertConditions {
 				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<revision>", fmt.Sprintf("%s@%s", img.tag, img.digest.String()))
-				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", repoURL)
+				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", ref.String())
 			}
 
 			g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
@@ -824,6 +825,15 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
 	}
 }
 
+func makeTransport(insecure bool) http.RoundTripper {
+	transport := remote.DefaultTransport.(*http.Transport).Clone()
+	if insecure {
+		transport.TLSClientConfig = &cryptotls.Config{
+			InsecureSkipVerify: true,
+		}
+	}
+	return transport
+}
 func TestOCIRepository_CertSecret(t *testing.T) {
 	g := NewWithT(t)
 
@@ -1367,9 +1377,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
 				g.Expect(err).ToNot(HaveOccurred())
 			}
 
-			opts := craneOptions(ctx, false)
+			opts := makeRemoteOptions(ctx, makeTransport(true), keychain, nil)
-			opts = append(opts, crane.WithAuthFromKeychain(keychain))
+
-			artifactURL, err := r.getArtifactURL(obj, opts)
+			artifactRef, err := r.getArtifactRef(obj, opts)
 			g.Expect(err).ToNot(HaveOccurred())
 
 			if tt.shouldSign {
@@ -1387,7 +1397,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
 					TlogUpload:       false,
 
 					Registry: coptions.RegistryOptions{Keychain: keychain, AllowInsecure: true, AllowHTTPRegistry: tt.insecure},
-				}, []string{artifactURL})
+				}, []string{artifactRef.String()})
 
 				g.Expect(err).ToNot(HaveOccurred())
 			}
@@ -1396,7 +1406,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
 			assertConditions := tt.assertConditions
 			for k := range assertConditions {
 				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<revision>", fmt.Sprintf("%s@%s", tt.reference.Tag, image.digest.String()))
-				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", artifactURL)
+				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", artifactRef.String())
 				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<provider>", "cosign")
 			}
 
@@ -1414,7 +1424,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
 			artifact := &sourcev1.Artifact{}
 			got, err := r.reconcileSource(ctx, sp, obj, artifact, tmpDir)
 			if tt.wantErr {
-				tt.wantErrMsg = strings.ReplaceAll(tt.wantErrMsg, "<url>", artifactURL)
+				tt.wantErrMsg = strings.ReplaceAll(tt.wantErrMsg, "<url>", artifactRef.String())
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(err.Error()).To(ContainSubstring(tt.wantErrMsg))
 			} else {
@@ -1845,11 +1855,12 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
 	}
 }
 
-func TestOCIRepository_getArtifactURL(t *testing.T) {
+func TestOCIRepository_getArtifactRef(t *testing.T) {
 	g := NewWithT(t)
 
 	tmpDir := t.TempDir()
 	server, err := setupRegistryServer(ctx, tmpDir, registryOptions{})
+	g.Expect(err).ToNot(HaveOccurred())
 	t.Cleanup(func() {
 		server.Close()
 	})
@@ -1867,7 +1878,7 @@ func TestOCIRepository_getArtifactURL(t *testing.T) {
 		{
 			name: "valid url with no reference",
 			url:  "oci://ghcr.io/stefanprodan/charts",
-			want: "ghcr.io/stefanprodan/charts",
+			want: "ghcr.io/stefanprodan/charts:latest",
 		},
 		{
 			name: "valid url with tag reference",
@@ -1929,15 +1940,14 @@ func TestOCIRepository_getArtifactURL(t *testing.T) {
 				obj.Spec.Reference = tt.reference
 			}
 
-			opts := craneOptions(ctx, true)
+			opts := makeRemoteOptions(ctx, makeTransport(true), authn.DefaultKeychain, nil)
-			opts = append(opts, crane.WithAuthFromKeychain(authn.DefaultKeychain))
+			got, err := r.getArtifactRef(obj, opts)
-			got, err := r.getArtifactURL(obj, opts)
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 				return
 			}
 			g.Expect(err).ToNot(HaveOccurred())
-			g.Expect(got).To(Equal(tt.want))
+			g.Expect(got.String()).To(Equal(tt.want))
 		})
 	}
 }