From 78283bbd7a01524fe2c1e3d51832968d4ec8c439 Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Thu, 19 Dec 2024 17:52:03 +0100 Subject: [PATCH 1/9] chore: close out of desparation --- api/tech/oras/pusher.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/tech/oras/pusher.go b/api/tech/oras/pusher.go index 784b2ae99..3c4d5e74a 100644 --- a/api/tech/oras/pusher.go +++ b/api/tech/oras/pusher.go @@ -23,6 +23,9 @@ func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) ( if err != nil { return err } + defer func() { + reader.Close() + }() repository, err := createRepository(c.ref, c.client, c.plainHTTP) if err != nil { From adab71665eb1ee0c4d32160de7c61ffff517392d Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Thu, 19 Dec 2024 18:11:12 +0100 Subject: [PATCH 2/9] chore: lock out of desparation --- api/tech/oras/pusher.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/tech/oras/pusher.go b/api/tech/oras/pusher.go index 3c4d5e74a..c56de3425 100644 --- a/api/tech/oras/pusher.go +++ b/api/tech/oras/pusher.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/containerd/errdefs" + "github.com/moby/locker" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote/auth" @@ -12,6 +13,8 @@ import ( "ocm.software/ocm/api/oci/ociutils" ) +var _locker = locker.New() + type OrasPusher struct { client *auth.Client ref string @@ -19,6 +22,9 @@ type OrasPusher struct { } func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) (retErr error) { + _locker.Lock(c.ref) + defer _locker.Unlock(c.ref) + reader, err := src.Reader() if err != nil { return err From 57e98bc574d52c748eb41a2a993508a5b537e8cc Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Thu, 19 Dec 2024 18:15:53 +0100 Subject: [PATCH 3/9] chore: log out of desparation --- cmds/ocm/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmds/ocm/main.go b/cmds/ocm/main.go index f092f7a18..61ab463cb 100644 --- a/cmds/ocm/main.go +++ b/cmds/ocm/main.go @@ -9,6 +9,10 @@ import ( ) func main() { + println("start of ocm") + defer func() { + println("end of ocm") + }() c, err := app.NewCliCommandForArgs(clictx.DefaultContext(), os.Args[1:]) if err != nil { fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error()) From 14aa04ffaf5b6def78fd485559cd625d8360e74d Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Thu, 19 Dec 2024 18:21:08 +0100 Subject: [PATCH 4/9] chore: lock out of desparation --- api/tech/oras/pusher.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/tech/oras/pusher.go b/api/tech/oras/pusher.go index c56de3425..73c7a2bbe 100644 --- a/api/tech/oras/pusher.go +++ b/api/tech/oras/pusher.go @@ -3,9 +3,9 @@ package oras import ( "context" "fmt" + "sync" "github.com/containerd/errdefs" - "github.com/moby/locker" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote/auth" @@ -13,17 +13,16 @@ import ( "ocm.software/ocm/api/oci/ociutils" ) -var _locker = locker.New() - type OrasPusher struct { client *auth.Client ref string plainHTTP bool + mu sync.Mutex } func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) (retErr error) { - _locker.Lock(c.ref) - defer _locker.Unlock(c.ref) + c.mu.Lock() + defer c.mu.Unlock() reader, err := src.Reader() if err != nil { From d100ad44a1a7ee407a3da0a1095d0bfce24d8986 Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Thu, 19 Dec 2024 18:35:42 +0100 Subject: [PATCH 5/9] chore: disable blob cache out of desparation --- .../extensions/repositories/ocireg/blobs.go | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/api/oci/extensions/repositories/ocireg/blobs.go b/api/oci/extensions/repositories/ocireg/blobs.go index d8491c266..87dfbd1cf 100644 --- a/api/oci/extensions/repositories/ocireg/blobs.go +++ b/api/oci/extensions/repositories/ocireg/blobs.go @@ -9,7 +9,6 @@ import ( "github.com/sirupsen/logrus" "ocm.software/ocm/api/oci/cpi" - "ocm.software/ocm/api/oci/extensions/attrs/cacheattr" "ocm.software/ocm/api/tech/oras" "ocm.software/ocm/api/utils/accessio" "ocm.software/ocm/api/utils/blobaccess/blobaccess" @@ -38,7 +37,7 @@ type BlobContainers struct { func NewBlobContainers(ctx cpi.Context, fetcher remotes.Fetcher, pusher oras.Pusher) *BlobContainers { return &BlobContainers{ - cache: cacheattr.Get(ctx), + //cache: cacheattr.Get(ctx), fetcher: fetcher, pusher: pusher, mimes: map[string]BlobContainer{}, @@ -81,17 +80,17 @@ func newBlobContainer(mime string, fetcher oras.Fetcher, pusher oras.Pusher) *bl } } -func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher oras.Fetcher, pusher oras.Pusher) (BlobContainer, error) { - c := newBlobContainer(mime, fetcher, pusher) - - if cache == nil { - return c, nil - } - r, err := accessio.CachedAccess(c, c, cache) - if err != nil { - return nil, err - } - return r, nil +func NewBlobContainer(_ accessio.BlobCache, mime string, fetcher oras.Fetcher, pusher oras.Pusher) (BlobContainer, error) { + return newBlobContainer(mime, fetcher, pusher), nil + + //if cache == nil { + // return c, nil + //} + //r, err := accessio.CachedAccess(c, c, cache) + //if err != nil { + // return nil, err + //} + //return r, nil } func (n *blobContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { From 5fc074d6d928fb8ae0702c62d291221fbad96c70 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Thu, 19 Dec 2024 19:56:15 +0100 Subject: [PATCH 6/9] added logging with timestamp and a client based lock Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> --- .../repositories/ocireg/repository.go | 2 ++ api/tech/oras/client.go | 7 +++++-- api/tech/oras/fetcher.go | 20 ++++++++++++++++++ api/tech/oras/pusher.go | 21 +++++++++++++++---- go.mod | 4 ++-- 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/api/oci/extensions/repositories/ocireg/repository.go b/api/oci/extensions/repositories/ocireg/repository.go index 9393cbffa..12fe7276d 100644 --- a/api/oci/extensions/repositories/ocireg/repository.go +++ b/api/oci/extensions/repositories/ocireg/repository.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/errdefs" "github.com/mandelsoft/goutils/errors" "github.com/mandelsoft/logging" + "github.com/moby/locker" "oras.land/oras-go/v2/registry/remote/auth" "oras.land/oras-go/v2/registry/remote/retry" @@ -179,6 +180,7 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) { Client: authClient, PlainHTTP: r.info.Scheme == "http", Logger: logger, + Lock: locker.New(), }), nil } diff --git a/api/tech/oras/client.go b/api/tech/oras/client.go index 60f4fede6..3cde633d6 100644 --- a/api/tech/oras/client.go +++ b/api/tech/oras/client.go @@ -7,6 +7,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/mandelsoft/logging" + "github.com/moby/locker" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" oraserr "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry/remote" @@ -17,18 +18,20 @@ type ClientOptions struct { Client *auth.Client PlainHTTP bool Logger logging.Logger + Lock *locker.Locker } type Client struct { client *auth.Client plainHTTP bool logger logging.Logger + lock *locker.Locker } var _ Resolver = &Client{} func New(opts ClientOptions) *Client { - return &Client{client: opts.Client, plainHTTP: opts.PlainHTTP, logger: opts.Logger} + return &Client{client: opts.Client, plainHTTP: opts.PlainHTTP, logger: opts.Logger, lock: opts.Lock} } func (c *Client) Fetcher(ctx context.Context, ref string) (Fetcher, error) { @@ -36,7 +39,7 @@ func (c *Client) Fetcher(ctx context.Context, ref string) (Fetcher, error) { } func (c *Client) Pusher(ctx context.Context, ref string) (Pusher, error) { - return &OrasPusher{client: c.client, ref: ref, plainHTTP: c.plainHTTP}, nil + return &OrasPusher{client: c.client, ref: ref, plainHTTP: c.plainHTTP, lock: c.lock}, nil } func (c *Client) Lister(ctx context.Context, ref string) (Lister, error) { diff --git a/api/tech/oras/fetcher.go b/api/tech/oras/fetcher.go index 33c1bf191..7cd14dd2d 100644 --- a/api/tech/oras/fetcher.go +++ b/api/tech/oras/fetcher.go @@ -5,7 +5,11 @@ import ( "errors" "fmt" "io" + "log" + "sync" + "time" + "github.com/google/uuid" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -14,9 +18,19 @@ type OrasFetcher struct { client *auth.Client ref string plainHTTP bool + mu sync.Mutex } func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.ReadCloser, error) { + c.mu.Lock() + defer c.mu.Unlock() + + start := time.Now() + id := uuid.New() + + log.Printf("START fetch %s; %s: %s\n", id.String(), c.ref, start) + defer log.Printf("END fetch %s; %s: %s\n", id.String(), c.ref, time.Since(start)) + src, err := createRepository(c.ref, c.client, c.plainHTTP) if err != nil { return nil, fmt.Errorf("failed to resolve ref %q: %w", c.ref, err) @@ -30,8 +44,10 @@ func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.Read // that the mediatype is not set at this point so we don't want ORAS to try to // select the wrong layer to fetch from. if desc.Size < 1 || desc.Digest == "" { + log.Printf("Trying to resolve manifest %s: %s", id.String(), c.ref) rdesc, err := src.Manifests().Resolve(ctx, desc.Digest.String()) if err != nil { + log.Printf("Failed to resolve manifest, trying to resolve blob %s: %s", id.String(), c.ref) var berr error rdesc, berr = src.Blobs().Resolve(ctx, desc.Digest.String()) if berr != nil { @@ -41,10 +57,12 @@ func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.Read return nil, fmt.Errorf("failed to resolve fetch blob %q: %w", desc.Digest.String(), err) } + log.Printf("Blob resolved, fetching reader %s: %s", id.String(), c.ref) reader, err := src.Blobs().Fetch(ctx, rdesc) if err != nil { return nil, fmt.Errorf("failed to fetch blob: %w", err) } + log.Printf("Reader fetched, returning %s: %s", id.String(), c.ref) return reader, nil } @@ -55,10 +73,12 @@ func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.Read // manifest resolve succeeded return the reader directly // mediatype of the descriptor should now be set to the correct type. + log.Printf("Fetching manifest %s: %s", id.String(), c.ref) fetch, err := src.Fetch(ctx, desc) if err != nil { return nil, fmt.Errorf("failed to fetch manifest: %w", err) } + log.Printf("Manifest fetched; returining %s: %s", id.String(), c.ref) return fetch, nil } diff --git a/api/tech/oras/pusher.go b/api/tech/oras/pusher.go index 73c7a2bbe..9a9b51916 100644 --- a/api/tech/oras/pusher.go +++ b/api/tech/oras/pusher.go @@ -3,9 +3,12 @@ package oras import ( "context" "fmt" - "sync" + "log" + "time" "github.com/containerd/errdefs" + "github.com/google/uuid" + "github.com/moby/locker" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote/auth" @@ -17,12 +20,17 @@ type OrasPusher struct { client *auth.Client ref string plainHTTP bool - mu sync.Mutex + lock *locker.Locker } func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) (retErr error) { - c.mu.Lock() - defer c.mu.Unlock() + c.lock.Lock(c.ref) + defer c.lock.Unlock(c.ref) + start := time.Now() + id := uuid.New() + + log.Printf("START push %s; %s: %s\n", id.String(), c.ref, start) + defer log.Printf("END push %s; %s: %s\n", id.String(), c.ref, time.Since(start)) reader, err := src.Reader() if err != nil { @@ -52,10 +60,12 @@ func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) ( // layer with the reference included. PushReference then will tag // that layer resulting in the created tag pointing to the right // blob data. + log.Printf("Pushing Reference %s: %s", ref, id.String()) if err := repository.PushReference(ctx, d, reader, c.ref); err != nil { return fmt.Errorf("failed to push tag: %w", err) } + log.Printf("Pushing Reference done %s: %s", ref, id.String()) return nil } @@ -70,9 +80,12 @@ func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) ( // We have a digest, so we use plain push for the digest. // Push here decides if it's a Manifest or a Blob. + log.Printf("Pushing blob %s: %s", ref, id.String()) if err := repository.Push(ctx, d, reader); err != nil { return fmt.Errorf("failed to push: %w, %s", err, c.ref) } + log.Printf("Pushing blob done %s: %s", ref, id.String()) + return nil } diff --git a/go.mod b/go.mod index 10e02fada..ba477f629 100644 --- a/go.mod +++ b/go.mod @@ -37,6 +37,7 @@ require ( github.com/gobwas/glob v0.2.3 github.com/golang/mock v1.6.0 github.com/google/go-github/v45 v45.2.0 + github.com/google/uuid v1.6.0 github.com/hashicorp/vault-client-go v0.4.3 github.com/klauspost/compress v1.17.11 github.com/klauspost/pgzip v1.2.6 @@ -49,6 +50,7 @@ require ( github.com/mikefarah/yq/v4 v4.44.6 github.com/mitchellh/copystructure v1.2.0 github.com/mittwald/go-helm-client v0.12.15 + github.com/moby/locker v1.0.1 github.com/modern-go/reflect2 v1.0.2 github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 @@ -217,7 +219,6 @@ require ( github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect github.com/google/s2a-go v0.1.8 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.0 // indirect github.com/gorilla/mux v1.8.1 // indirect @@ -258,7 +259,6 @@ require ( github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect - github.com/moby/locker v1.0.1 // indirect github.com/moby/spdystream v0.5.0 // indirect github.com/moby/sys/capability v0.3.0 // indirect github.com/moby/sys/mountinfo v0.7.2 // indirect From 73f0e6289b1979703f1b0300384b52fc190e73e4 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Thu, 19 Dec 2024 20:13:15 +0100 Subject: [PATCH 7/9] resolving blob first THEN the manifest --- api/tech/oras/fetcher.go | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/api/tech/oras/fetcher.go b/api/tech/oras/fetcher.go index 7cd14dd2d..66c9e765c 100644 --- a/api/tech/oras/fetcher.go +++ b/api/tech/oras/fetcher.go @@ -44,41 +44,40 @@ func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.Read // that the mediatype is not set at this point so we don't want ORAS to try to // select the wrong layer to fetch from. if desc.Size < 1 || desc.Digest == "" { - log.Printf("Trying to resolve manifest %s: %s", id.String(), c.ref) - rdesc, err := src.Manifests().Resolve(ctx, desc.Digest.String()) + log.Printf("Trying to resolve blob %s: %s", id.String(), c.ref) + bdesc, err := src.Blobs().Resolve(ctx, desc.Digest.String()) if err != nil { - log.Printf("Failed to resolve manifest, trying to resolve blob %s: %s", id.String(), c.ref) - var berr error - rdesc, berr = src.Blobs().Resolve(ctx, desc.Digest.String()) - if berr != nil { + log.Printf("Failed to resolve blob, trying to resolve manifest %s: %s", id.String(), c.ref) + mdesc, merr := src.Manifests().Resolve(ctx, desc.Digest.String()) + if merr != nil { // also display the first manifest resolve error - err = errors.Join(err, berr) + err = errors.Join(err, merr) - return nil, fmt.Errorf("failed to resolve fetch blob %q: %w", desc.Digest.String(), err) + return nil, fmt.Errorf("failed to resolve manifest %q: %w", desc.Digest.String(), err) } - log.Printf("Blob resolved, fetching reader %s: %s", id.String(), c.ref) - reader, err := src.Blobs().Fetch(ctx, rdesc) + log.Printf("Fetching manifest %s: %s", id.String(), c.ref) + fetch, err := src.Fetch(ctx, mdesc) if err != nil { - return nil, fmt.Errorf("failed to fetch blob: %w", err) + return nil, fmt.Errorf("failed to fetch manifest: %w", err) } - log.Printf("Reader fetched, returning %s: %s", id.String(), c.ref) - return reader, nil + log.Printf("Manifest fetched; returining %s: %s", id.String(), c.ref) + return fetch, nil } // no error - desc = rdesc + desc = bdesc } // manifest resolve succeeded return the reader directly // mediatype of the descriptor should now be set to the correct type. - log.Printf("Fetching manifest %s: %s", id.String(), c.ref) - fetch, err := src.Fetch(ctx, desc) + log.Printf("Blob resolved, fetching reader %s: %s", id.String(), c.ref) + reader, err := src.Blobs().Fetch(ctx, desc) if err != nil { - return nil, fmt.Errorf("failed to fetch manifest: %w", err) + return nil, fmt.Errorf("failed to fetch blob: %w", err) } + log.Printf("Reader fetched, returning %s: %s", id.String(), c.ref) - log.Printf("Manifest fetched; returining %s: %s", id.String(), c.ref) - return fetch, nil + return reader, nil } From 29abde114c37955d3c2c87a5d35bea2a12411d2a Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Fri, 20 Dec 2024 12:04:20 +0100 Subject: [PATCH 8/9] Revert "chore: disable blob cache out of desparation" This reverts commit d100ad44a1a7ee407a3da0a1095d0bfce24d8986. --- .../extensions/repositories/ocireg/blobs.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/api/oci/extensions/repositories/ocireg/blobs.go b/api/oci/extensions/repositories/ocireg/blobs.go index 87dfbd1cf..d8491c266 100644 --- a/api/oci/extensions/repositories/ocireg/blobs.go +++ b/api/oci/extensions/repositories/ocireg/blobs.go @@ -9,6 +9,7 @@ import ( "github.com/sirupsen/logrus" "ocm.software/ocm/api/oci/cpi" + "ocm.software/ocm/api/oci/extensions/attrs/cacheattr" "ocm.software/ocm/api/tech/oras" "ocm.software/ocm/api/utils/accessio" "ocm.software/ocm/api/utils/blobaccess/blobaccess" @@ -37,7 +38,7 @@ type BlobContainers struct { func NewBlobContainers(ctx cpi.Context, fetcher remotes.Fetcher, pusher oras.Pusher) *BlobContainers { return &BlobContainers{ - //cache: cacheattr.Get(ctx), + cache: cacheattr.Get(ctx), fetcher: fetcher, pusher: pusher, mimes: map[string]BlobContainer{}, @@ -80,17 +81,17 @@ func newBlobContainer(mime string, fetcher oras.Fetcher, pusher oras.Pusher) *bl } } -func NewBlobContainer(_ accessio.BlobCache, mime string, fetcher oras.Fetcher, pusher oras.Pusher) (BlobContainer, error) { - return newBlobContainer(mime, fetcher, pusher), nil - - //if cache == nil { - // return c, nil - //} - //r, err := accessio.CachedAccess(c, c, cache) - //if err != nil { - // return nil, err - //} - //return r, nil +func NewBlobContainer(cache accessio.BlobCache, mime string, fetcher oras.Fetcher, pusher oras.Pusher) (BlobContainer, error) { + c := newBlobContainer(mime, fetcher, pusher) + + if cache == nil { + return c, nil + } + r, err := accessio.CachedAccess(c, c, cache) + if err != nil { + return nil, err + } + return r, nil } func (n *blobContainer) GetBlobData(digest digest.Digest) (int64, cpi.DataAccess, error) { From 8174744e2b773bd4368d5f630ec0955cdf4531fc Mon Sep 17 00:00:00 2001 From: jakobmoellerdev Date: Fri, 20 Dec 2024 13:09:16 +0100 Subject: [PATCH 9/9] chore: refactor and remove logs --- api/tech/oras/fetcher.go | 85 +++++++++++++++++++++------------------ api/tech/oras/manifest.go | 27 +++++++++++++ api/tech/oras/pusher.go | 15 ------- cmds/ocm/main.go | 4 -- go.mod | 2 +- 5 files changed, 73 insertions(+), 60 deletions(-) create mode 100644 api/tech/oras/manifest.go diff --git a/api/tech/oras/fetcher.go b/api/tech/oras/fetcher.go index 66c9e765c..a12df685f 100644 --- a/api/tech/oras/fetcher.go +++ b/api/tech/oras/fetcher.go @@ -5,12 +5,10 @@ import ( "errors" "fmt" "io" - "log" "sync" - "time" - "github.com/google/uuid" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" ) @@ -25,59 +23,66 @@ func (c *OrasFetcher) Fetch(ctx context.Context, desc ociv1.Descriptor) (io.Read c.mu.Lock() defer c.mu.Unlock() - start := time.Now() - id := uuid.New() - - log.Printf("START fetch %s; %s: %s\n", id.String(), c.ref, start) - defer log.Printf("END fetch %s; %s: %s\n", id.String(), c.ref, time.Since(start)) - src, err := createRepository(c.ref, c.client, c.plainHTTP) if err != nil { return nil, fmt.Errorf("failed to resolve ref %q: %w", c.ref, err) } - // oras requires a Resolve to happen before a fetch because + // oras requires a Resolve to happen in some cases before a fetch because // -1 or 0 are invalid sizes and result in a content-length mismatch error by design. // This is a security consideration on ORAS' side. // For more information (https://github.com/oras-project/oras-go/issues/822#issuecomment-2325622324) - // We explicitly call resolve on manifest first because it might be - // that the mediatype is not set at this point so we don't want ORAS to try to - // select the wrong layer to fetch from. - if desc.Size < 1 || desc.Digest == "" { - log.Printf("Trying to resolve blob %s: %s", id.String(), c.ref) - bdesc, err := src.Blobs().Resolve(ctx, desc.Digest.String()) - if err != nil { - log.Printf("Failed to resolve blob, trying to resolve manifest %s: %s", id.String(), c.ref) - mdesc, merr := src.Manifests().Resolve(ctx, desc.Digest.String()) - if merr != nil { - // also display the first manifest resolve error - err = errors.Join(err, merr) - - return nil, fmt.Errorf("failed to resolve manifest %q: %w", desc.Digest.String(), err) - } - - log.Printf("Fetching manifest %s: %s", id.String(), c.ref) - fetch, err := src.Fetch(ctx, mdesc) - if err != nil { - return nil, fmt.Errorf("failed to fetch manifest: %w", err) - } - - log.Printf("Manifest fetched; returining %s: %s", id.String(), c.ref) - return fetch, nil + // + // To workaround, we resolve the correct size + if desc.Size < 1 { + if desc, err = c.resolveDescriptor(ctx, desc, src); err != nil { + return nil, err } - - // no error - desc = bdesc } // manifest resolve succeeded return the reader directly // mediatype of the descriptor should now be set to the correct type. - log.Printf("Blob resolved, fetching reader %s: %s", id.String(), c.ref) - reader, err := src.Blobs().Fetch(ctx, desc) + reader, err := src.Fetch(ctx, desc) if err != nil { return nil, fmt.Errorf("failed to fetch blob: %w", err) } - log.Printf("Reader fetched, returning %s: %s", id.String(), c.ref) return reader, nil } + +// resolveDescriptor resolves the descriptor by fetching the blob or manifest based on the digest as a reference. +// If the descriptor has a media type, it will be resolved directly. +// If the descriptor has no media type, it will first try to resolve the blob, then the manifest as a fallback. +func (c *OrasFetcher) resolveDescriptor(ctx context.Context, desc ociv1.Descriptor, src *remote.Repository) (ociv1.Descriptor, error) { + if desc.MediaType != "" { + var err error + // if there is a media type, resolve the descriptor directly + if isManifest(src.ManifestMediaTypes, desc) { + desc, err = src.Manifests().Resolve(ctx, desc.Digest.String()) + } else { + desc, err = src.Blobs().Resolve(ctx, desc.Digest.String()) + } + if err != nil { + return ociv1.Descriptor{}, fmt.Errorf("failed to resolve descriptor %q: %w", desc.Digest.String(), err) + } + return desc, nil + } + + // if there is no media type, first try the blob, then the manifest + // To reader: DO NOT fetch manifest first, this can result in high latency calls + bdesc, err := src.Blobs().Resolve(ctx, desc.Digest.String()) + if err != nil { + mdesc, merr := src.Manifests().Resolve(ctx, desc.Digest.String()) + if merr != nil { + // also display the first manifest resolve error + err = errors.Join(err, merr) + + return ociv1.Descriptor{}, fmt.Errorf("failed to resolve manifest %q: %w", desc.Digest.String(), err) + } + desc = mdesc + } else { + desc = bdesc + } + + return desc, err +} diff --git a/api/tech/oras/manifest.go b/api/tech/oras/manifest.go new file mode 100644 index 000000000..cc473d87f --- /dev/null +++ b/api/tech/oras/manifest.go @@ -0,0 +1,27 @@ +package oras + +import ( + ocispec "github.com/opencontainers/image-spec/specs-go/v1" +) + +// defaultManifestMediaTypes contains the default set of manifests media types. +var defaultManifestMediaTypes = []string{ + "application/vnd.docker.distribution.manifest.v2+json", + "application/vnd.docker.distribution.manifest.list.v2+json", + "application/vnd.oci.artifact.manifest.v1+json", + ocispec.MediaTypeImageManifest, + ocispec.MediaTypeImageIndex, +} + +// isManifest determines if the given descriptor points to a manifest. +func isManifest(manifestMediaTypes []string, desc ocispec.Descriptor) bool { + if len(manifestMediaTypes) == 0 { + manifestMediaTypes = defaultManifestMediaTypes + } + for _, mediaType := range manifestMediaTypes { + if desc.MediaType == mediaType { + return true + } + } + return false +} diff --git a/api/tech/oras/pusher.go b/api/tech/oras/pusher.go index 9a9b51916..0372c13eb 100644 --- a/api/tech/oras/pusher.go +++ b/api/tech/oras/pusher.go @@ -3,11 +3,8 @@ package oras import ( "context" "fmt" - "log" - "time" "github.com/containerd/errdefs" - "github.com/google/uuid" "github.com/moby/locker" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry" @@ -26,11 +23,6 @@ type OrasPusher struct { func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) (retErr error) { c.lock.Lock(c.ref) defer c.lock.Unlock(c.ref) - start := time.Now() - id := uuid.New() - - log.Printf("START push %s; %s: %s\n", id.String(), c.ref, start) - defer log.Printf("END push %s; %s: %s\n", id.String(), c.ref, time.Since(start)) reader, err := src.Reader() if err != nil { @@ -60,12 +52,10 @@ func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) ( // layer with the reference included. PushReference then will tag // that layer resulting in the created tag pointing to the right // blob data. - log.Printf("Pushing Reference %s: %s", ref, id.String()) if err := repository.PushReference(ctx, d, reader, c.ref); err != nil { return fmt.Errorf("failed to push tag: %w", err) } - log.Printf("Pushing Reference done %s: %s", ref, id.String()) return nil } @@ -78,14 +68,9 @@ func (c *OrasPusher) Push(ctx context.Context, d ociv1.Descriptor, src Source) ( return errdefs.ErrAlreadyExists } - // We have a digest, so we use plain push for the digest. - // Push here decides if it's a Manifest or a Blob. - log.Printf("Pushing blob %s: %s", ref, id.String()) if err := repository.Push(ctx, d, reader); err != nil { return fmt.Errorf("failed to push: %w, %s", err, c.ref) } - log.Printf("Pushing blob done %s: %s", ref, id.String()) - return nil } diff --git a/cmds/ocm/main.go b/cmds/ocm/main.go index 61ab463cb..f092f7a18 100644 --- a/cmds/ocm/main.go +++ b/cmds/ocm/main.go @@ -9,10 +9,6 @@ import ( ) func main() { - println("start of ocm") - defer func() { - println("end of ocm") - }() c, err := app.NewCliCommandForArgs(clictx.DefaultContext(), os.Args[1:]) if err != nil { fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error()) diff --git a/go.mod b/go.mod index ba477f629..2eab77b61 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,6 @@ require ( github.com/gobwas/glob v0.2.3 github.com/golang/mock v1.6.0 github.com/google/go-github/v45 v45.2.0 - github.com/google/uuid v1.6.0 github.com/hashicorp/vault-client-go v0.4.3 github.com/klauspost/compress v1.17.11 github.com/klauspost/pgzip v1.2.6 @@ -219,6 +218,7 @@ require ( github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect github.com/google/s2a-go v0.1.8 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect + github.com/google/uuid v1.6.0 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.14.0 // indirect github.com/gorilla/mux v1.8.1 // indirect