From a9400ba7a3a43230fd4e2c7c9e22469886a38922 Mon Sep 17 00:00:00 2001 From: John Olheiser Date: Tue, 17 Jan 2023 00:50:45 -0600 Subject: [PATCH] Fix container blob mount (#22226) (#22476) Backport #22226 Co-authored-by: KN4CK3R --- models/packages/container/search.go | 10 ++ routers/api/packages/container/blob.go | 135 ++++++++++-------- routers/api/packages/container/container.go | 9 +- templates/package/settings.tmpl | 2 +- .../api_packages_container_test.go | 41 ++++-- 5 files changed, 123 insertions(+), 74 deletions(-) diff --git a/models/packages/container/search.go b/models/packages/container/search.go index e4a5a5384..19661d44e 100644 --- a/models/packages/container/search.go +++ b/models/packages/container/search.go @@ -26,6 +26,7 @@ type BlobSearchOptions struct { Digest string Tag string IsManifest bool + Repository string } func (opts *BlobSearchOptions) toConds() builder.Cond { @@ -54,6 +55,15 @@ func (opts *BlobSearchOptions) toConds() builder.Cond { cond = cond.And(builder.In("package_file.id", builder.Select("package_property.ref_id").Where(propsCond).From("package_property"))) } + if opts.Repository != "" { + var propsCond builder.Cond = builder.Eq{ + "package_property.ref_type": packages.PropertyTypePackage, + "package_property.name": container_module.PropertyRepository, + "package_property.value": opts.Repository, + } + + cond = cond.And(builder.In("package.id", builder.Select("package_property.ref_id").Where(propsCond).From("package_property"))) + } return cond } diff --git a/routers/api/packages/container/blob.go b/routers/api/packages/container/blob.go index 7494fd639..72b6857e7 100644 --- a/routers/api/packages/container/blob.go +++ b/routers/api/packages/container/blob.go @@ -34,6 +34,60 @@ func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_servic contentStore := packages_module.NewContentStore() + uploadVersion, err := getOrCreateUploadVersion(pi) + if err != nil { + return nil, err + } + + err = db.WithTx(func(ctx context.Context) error { + pb, exists, err = packages_model.GetOrInsertBlob(ctx, pb) + if err != nil { + log.Error("Error inserting package blob: %v", err) + return err + } + // FIXME: Workaround to be removed in v1.20 + // https://github.com/go-gitea/gitea/issues/19586 + if exists { + err = contentStore.Has(packages_module.BlobHash256Key(pb.HashSHA256)) + if err != nil && (errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist)) { + log.Debug("Package registry inconsistent: blob %s does not exist on file system", pb.HashSHA256) + exists = false + } + } + if !exists { + if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), hsr, hsr.Size()); err != nil { + log.Error("Error saving package blob in content store: %v", err) + return err + } + } + + return createFileForBlob(ctx, uploadVersion, pb) + }) + if err != nil { + if !exists { + if err := contentStore.Delete(packages_module.BlobHash256Key(pb.HashSHA256)); err != nil { + log.Error("Error deleting package blob from content store: %v", err) + } + } + return nil, err + } + + return pb, nil +} + +// mountBlob mounts the specific blob to a different package +func mountBlob(pi *packages_service.PackageInfo, pb *packages_model.PackageBlob) error { + uploadVersion, err := getOrCreateUploadVersion(pi) + if err != nil { + return err + } + + return db.WithTx(func(ctx context.Context) error { + return createFileForBlob(ctx, uploadVersion, pb) + }) +} + +func getOrCreateUploadVersion(pi *packages_service.PackageInfo) (*packages_model.PackageVersion, error) { var uploadVersion *packages_model.PackageVersion // FIXME: Replace usage of mutex with database transaction @@ -84,66 +138,35 @@ func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_servic return nil }) uploadVersionMutex.Unlock() - if err != nil { - return nil, err + + return uploadVersion, err +} + +func createFileForBlob(ctx context.Context, pv *packages_model.PackageVersion, pb *packages_model.PackageBlob) error { + filename := strings.ToLower(fmt.Sprintf("sha256_%s", pb.HashSHA256)) + + pf := &packages_model.PackageFile{ + VersionID: pv.ID, + BlobID: pb.ID, + Name: filename, + LowerName: filename, + CompositeKey: packages_model.EmptyFileKey, + } + var err error + if pf, err = packages_model.TryInsertFile(ctx, pf); err != nil { + if err == packages_model.ErrDuplicatePackageFile { + return nil + } + log.Error("Error inserting package file: %v", err) + return err } - err = db.WithTx(func(ctx context.Context) error { - pb, exists, err = packages_model.GetOrInsertBlob(ctx, pb) - if err != nil { - log.Error("Error inserting package blob: %v", err) - return err - } - // FIXME: Workaround to be removed in v1.20 - // https://github.com/go-gitea/gitea/issues/19586 - if exists { - err = contentStore.Has(packages_module.BlobHash256Key(pb.HashSHA256)) - if err != nil && (errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist)) { - log.Debug("Package registry inconsistent: blob %s does not exist on file system", pb.HashSHA256) - exists = false - } - } - if !exists { - if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), hsr, hsr.Size()); err != nil { - log.Error("Error saving package blob in content store: %v", err) - return err - } - } - - filename := strings.ToLower(fmt.Sprintf("sha256_%s", pb.HashSHA256)) - - pf := &packages_model.PackageFile{ - VersionID: uploadVersion.ID, - BlobID: pb.ID, - Name: filename, - LowerName: filename, - CompositeKey: packages_model.EmptyFileKey, - } - if pf, err = packages_model.TryInsertFile(ctx, pf); err != nil { - if err == packages_model.ErrDuplicatePackageFile { - return nil - } - log.Error("Error inserting package file: %v", err) - return err - } - - if _, err := packages_model.InsertProperty(ctx, packages_model.PropertyTypeFile, pf.ID, container_module.PropertyDigest, digestFromPackageBlob(pb)); err != nil { - log.Error("Error setting package file property: %v", err) - return err - } - - return nil - }) - if err != nil { - if !exists { - if err := contentStore.Delete(packages_module.BlobHash256Key(pb.HashSHA256)); err != nil { - log.Error("Error deleting package blob from content store: %v", err) - } - } - return nil, err + if _, err := packages_model.InsertProperty(ctx, packages_model.PropertyTypeFile, pf.ID, container_module.PropertyDigest, digestFromPackageBlob(pb)); err != nil { + log.Error("Error setting package file property: %v", err) + return err } - return pb, nil + return nil } func deleteBlob(ownerID int64, image, digest string) error { diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 7af06a917..b1993d727 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -196,10 +196,15 @@ func InitiateUploadBlob(ctx *context.Context) { from := ctx.FormTrim("from") if mount != "" { blob, _ := workaroundGetContainerBlob(ctx, &container_model.BlobSearchOptions{ - Image: from, - Digest: mount, + Repository: from, + Digest: mount, }) if blob != nil { + if err := mountBlob(&packages_service.PackageInfo{Owner: ctx.Package.Owner, Name: image}, blob.Blob); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + setResponseHeaders(ctx.Resp, &containerHeaders{ Location: fmt.Sprintf("/v2/%s/%s/blobs/%s", ctx.Package.Owner.LowerName, image, mount), ContentDigest: mount, diff --git a/templates/package/settings.tmpl b/templates/package/settings.tmpl index 5f045e98b..d1475be59 100644 --- a/templates/package/settings.tmpl +++ b/templates/package/settings.tmpl @@ -51,7 +51,7 @@ {{.locale.Tr "packages.settings.delete"}}
-
+
{{.locale.Tr "packages.settings.delete.notice" .PackageDescriptor.Package.Name .PackageDescriptor.Version.Version}}
diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go index 60cbecd06..de0529f14 100644 --- a/tests/integration/api_packages_container_test.go +++ b/tests/integration/api_packages_container_test.go @@ -257,6 +257,32 @@ func TestPackageContainer(t *testing.T) { }) }) + t.Run("UploadBlob/Mount", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s", url, unknownDigest)) + addTokenAuthHeader(req, userToken) + MakeRequest(t, req, http.StatusAccepted) + + req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s", url, blobDigest)) + addTokenAuthHeader(req, userToken) + resp := MakeRequest(t, req, http.StatusCreated) + + assert.Equal(t, fmt.Sprintf("/v2/%s/%s/blobs/%s", user.Name, image, blobDigest), resp.Header().Get("Location")) + assert.Equal(t, blobDigest, resp.Header().Get("Docker-Content-Digest")) + + req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s&from=%s", url, unknownDigest, "unknown/image")) + addTokenAuthHeader(req, userToken) + MakeRequest(t, req, http.StatusAccepted) + + req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s&from=%s/%s", url, blobDigest, user.Name, image)) + addTokenAuthHeader(req, userToken) + resp = MakeRequest(t, req, http.StatusCreated) + + assert.Equal(t, fmt.Sprintf("/v2/%s/%s/blobs/%s", user.Name, image, blobDigest), resp.Header().Get("Location")) + assert.Equal(t, blobDigest, resp.Header().Get("Docker-Content-Digest")) + }) + for _, tag := range tags { t.Run(fmt.Sprintf("[Tag:%s]", tag), func(t *testing.T) { t.Run("UploadManifest", func(t *testing.T) { @@ -445,21 +471,6 @@ func TestPackageContainer(t *testing.T) { assert.Equal(t, indexManifestDigest, pd.Files[0].Properties.GetByName(container_module.PropertyDigest)) }) - t.Run("UploadBlob/Mount", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - - req := NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s", url, unknownDigest)) - addTokenAuthHeader(req, userToken) - MakeRequest(t, req, http.StatusAccepted) - - req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads?mount=%s", url, blobDigest)) - addTokenAuthHeader(req, userToken) - resp := MakeRequest(t, req, http.StatusCreated) - - assert.Equal(t, fmt.Sprintf("/v2/%s/%s/blobs/%s", user.Name, image, blobDigest), resp.Header().Get("Location")) - assert.Equal(t, blobDigest, resp.Header().Get("Docker-Content-Digest")) - }) - t.Run("HeadBlob", func(t *testing.T) { defer tests.PrintCurrentTest(t)()