fix: restore missing blob file when re-publishing a package

modules/packages/content_store.go

@@ -4,11 +4,14 @@
 package packages
 
 import (
+	"errors"
 	"io"
+	"io/fs"
 	"net/url"
 	"path"
 	"strings"
 
+	"gitea.dev/modules/optional"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/storage"
 	"gitea.dev/modules/util"
@@ -40,11 +43,15 @@ func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename, method st
 	return s.store.ServeDirectURL(KeyToRelativePath(key), filename, method, reqParams)
 }
 
-// FIXME: Workaround to be removed in v1.20
-// https://github.com/go-gitea/gitea/issues/19586
-func (s *ContentStore) Has(key BlobHash256Key) error {
-	_, err := s.store.Stat(KeyToRelativePath(key))
-	return err
+func (s *ContentStore) OptionalSize(key BlobHash256Key) (sz optional.Option[int64], _ error) {
+	st, err := s.store.Stat(KeyToRelativePath(key))
+	if errors.Is(err, fs.ErrNotExist) {
+		return sz, nil
+	}
+	if err != nil {
+		return sz, err
+	}
+	return optional.Some(st.Size()), nil
 }
 
 // Save stores a package blob

modules/storage/azureblob.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"net/url"
 	"os"
@@ -108,7 +109,7 @@ func convertAzureBlobErr(err error) error {
 	}
 
 	if bloberror.HasCode(err, bloberror.BlobNotFound) {
-		return os.ErrNotExist
+		return fs.ErrNotExist
 	}
 	var respErr *azcore.ResponseError
 	if !errors.As(err, &respErr) {

modules/storage/local.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -138,7 +139,7 @@ func (l *LocalStorage) ServeDirectURL(path, name, _ string, reqParams *ServeDire
 }
 
 func (l *LocalStorage) normalizeWalkError(err error) error {
-	if errors.Is(err, os.ErrNotExist) {
+	if errors.Is(err, fs.ErrNotExist) {
 		// ignore it because the file may be deleted during the walk, and we don't care about it
 		return nil
 	}

modules/storage/local_test.go

@@ -4,6 +4,7 @@
 package storage
 
 import (
+	"io/fs"
 	"os"
 	"strings"
 	"testing"
@@ -72,7 +73,7 @@ func TestLocalStorageDelete(t *testing.T) {
 		if exists {
 			require.NoError(t, err)
 		} else {
-			require.ErrorIs(t, err, os.ErrNotExist)
+			require.ErrorIs(t, err, fs.ErrNotExist)
 		}
 	}
 

modules/storage/minio.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"net/url"
 	"os"
@@ -87,9 +88,9 @@ func convertMinioErr(err error, optMsg ...string) error {
 	// Convert two responses to standard analogues
 	switch errResp.Code {
 	case "NoSuchKey":
-		return wrapErr(os.ErrNotExist)
+		return wrapErr(fs.ErrNotExist)
 	case "AccessDenied":
-		return wrapErr(os.ErrPermission)
+		return wrapErr(fs.ErrPermission)
 	}
 
 	return wrapErr(err)

routers/api/packages/container/blob.go

@@ -8,7 +8,6 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"os"
 	"strings"
 
 	"gitea.dev/models/db"
@@ -18,7 +17,6 @@ import (
 	"gitea.dev/modules/log"
 	packages_module "gitea.dev/modules/packages"
 	container_module "gitea.dev/modules/packages/container"
-	"gitea.dev/modules/util"
 	packages_service "gitea.dev/services/packages"
 
 	"github.com/opencontainers/go-digest"
@@ -51,28 +49,10 @@ func saveAsPackageBlobInternal(ctx context.Context, hsr packages_module.HashedSi
 		if err := packages_service.CheckSizeQuotaExceeded(ctx, pci.Creator, pci.Owner, packages_model.TypeContainer, hsr.Size()); err != nil {
 			return err
 		}
-
-		pb, exists, err = packages_model.GetOrInsertBlob(ctx, pb)
+		pb, exists, err = packages_service.GetOrSavePackageBlob(ctx, contentStore, pb, hsr)
 		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 {

routers/api/packages/container/container.go

@@ -9,7 +9,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -28,7 +27,6 @@ import (
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/storage"
 	"gitea.dev/modules/structs"
-	"gitea.dev/modules/util"
 	"gitea.dev/routers/api/packages/helper"
 	auth_service "gitea.dev/services/auth"
 	"gitea.dev/services/context"
@@ -250,7 +248,7 @@ func PostBlobsUploads(ctx *context.Context) {
 	mount := ctx.FormTrim("mount")
 	from := ctx.FormTrim("from")
 	if mount != "" {
-		blob, _ := workaroundGetContainerBlob(ctx, &container_model.BlobSearchOptions{
+		blob, _ := getExistingContainerBlob(ctx, &container_model.BlobSearchOptions{
 			Repository: from,
 			Digest:     mount,
 		})
@@ -506,7 +504,7 @@ func getBlobFromContext(ctx *context.Context) (*packages_model.PackageFileDescri
 		return nil, container_model.ErrContainerBlobNotExist
 	}
 
-	return workaroundGetContainerBlob(ctx, &container_model.BlobSearchOptions{
+	return getExistingContainerBlob(ctx, &container_model.BlobSearchOptions{
 		OwnerID: ctx.Package.Owner.ID,
 		Image:   ctx.PathParam("image"),
 		Digest:  string(d),
@@ -643,7 +641,7 @@ func getManifestFromContext(ctx *context.Context) (*packages_model.PackageFileDe
 		return nil, err
 	}
 
-	return workaroundGetContainerBlob(ctx, opts)
+	return getExistingContainerBlob(ctx, opts)
 }
 
 // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#checking-if-content-exists-in-the-registry
@@ -789,23 +787,20 @@ func GetTagsList(ctx *context.Context) {
 	})
 }
 
-// FIXME: Workaround to be removed in v1.20.
-// Update maybe we should never really remote it, as long as there is legacy data?
-// https://github.com/go-gitea/gitea/issues/19586
-func workaroundGetContainerBlob(ctx *context.Context, opts *container_model.BlobSearchOptions) (*packages_model.PackageFileDescriptor, error) {
+// getExistingContainerBlob checks if the blob exists in the database and content store, and returns the package file descriptor if it does.
+func getExistingContainerBlob(ctx *context.Context, opts *container_model.BlobSearchOptions) (*packages_model.PackageFileDescriptor, error) {
 	blob, err := container_model.GetContainerBlob(ctx, opts)
 	if err != nil {
 		return nil, err
 	}
 
-	err = packages_module.NewContentStore().Has(packages_module.BlobHash256Key(blob.Blob.HashSHA256))
+	storeKey := packages_module.BlobHash256Key(blob.Blob.HashSHA256)
+	sz, err := packages_module.NewContentStore().OptionalSize(storeKey)
 	if err != nil {
-		if errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist) {
-			log.Debug("Package registry inconsistent: blob %s does not exist on file system", blob.Blob.HashSHA256)
-			return nil, container_model.ErrContainerBlobNotExist
-		}
-		return nil, err
+		return nil, fmt.Errorf("unable to check object size in content store: %w", err)
+	}
+	if sz.ValueOrDefault(-1) != blob.Blob.Size {
+		return nil, container_model.ErrContainerBlobNotExist
 	}
-
 	return blob, nil
 }

routers/api/packages/container/manifest.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 
 	"gitea.dev/models/db"
@@ -20,7 +19,6 @@ import (
 	"gitea.dev/modules/log"
 	packages_module "gitea.dev/modules/packages"
 	container_module "gitea.dev/modules/packages/container"
-	"gitea.dev/modules/util"
 	notify_service "gitea.dev/services/notify"
 	packages_service "gitea.dev/services/packages"
 	container_service "gitea.dev/services/packages/container"
@@ -379,26 +377,10 @@ func createFileFromBlobReference(ctx context.Context, pv, uploadVersion *package
 }
 
 func createManifestBlob(ctx context.Context, contentStore *packages_module.ContentStore, mci *manifestCreationInfo, pv *packages_model.PackageVersion, buf *packages_module.HashedBuffer) (_ *packages_model.PackageBlob, created bool, manifestDigest string, _ error) {
-	pb, exists, err := packages_model.GetOrInsertBlob(ctx, packages_service.NewPackageBlob(buf))
+	pb, exists, err := packages_service.GetOrSavePackageBlob(ctx, contentStore, packages_service.NewPackageBlob(buf), buf)
 	if err != nil {
-		log.Error("Error inserting package blob: %v", err)
 		return nil, false, "", 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), buf, buf.Size()); err != nil {
-			log.Error("Error saving package blob in content store: %v", err)
-			return nil, false, "", err
-		}
-	}
 
 	manifestDigest = digestFromHashSummer(buf)
 	pf, err := createFileFromBlobReference(ctx, pv, nil, &blobReference{

services/packages/packages.go

@@ -264,6 +264,31 @@ func NewPackageBlob(hsr packages_module.HashedSizeReader) *packages_model.Packag
 	}
 }
 
+func GetOrSavePackageBlob(ctx context.Context, contentStore *packages_module.ContentStore, blob *packages_model.PackageBlob, data packages_module.HashedSizeReader) (*packages_model.PackageBlob, bool, error) {
+	if blob.Size != data.Size() {
+		return nil, false, fmt.Errorf("size mismatch: blob size %d, data size %d", blob.Size, data.Size())
+	}
+	pb, exists, err := packages_model.GetOrInsertBlob(ctx, blob)
+	if err != nil {
+		return nil, false, fmt.Errorf("unable to get or insert blob: %w", err)
+	}
+	storeKey := packages_module.BlobHash256Key(pb.HashSHA256)
+	if exists {
+		// check if the blob file actually is valid in the content store
+		sz, err := contentStore.OptionalSize(storeKey)
+		if err != nil {
+			return nil, false, fmt.Errorf("unable to check object size in content store: %w", err)
+		}
+		exists = sz.ValueOrDefault(-1) == blob.Size
+	}
+	if !exists {
+		if err := contentStore.Save(storeKey, data, data.Size()); err != nil {
+			return nil, false, fmt.Errorf("unable to save object in content store: %w", err)
+		}
+	}
+	return pb, exists, nil
+}
+
 func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pvi *PackageInfo, pfci *PackageFileCreationInfo) (*packages_model.PackageFile, *packages_model.PackageBlob, bool, error) {
 	if err := CheckSizeQuotaExceeded(ctx, pfci.Creator, pvi.Owner, pvi.PackageType, pfci.Data.Size()); err != nil {
 		return nil, nil, false, err
@@ -275,17 +300,9 @@ func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVers
 func addFileToPackageVersionUnchecked(ctx context.Context, pv *packages_model.PackageVersion, pfci *PackageFileCreationInfo) (*packages_model.PackageFile, *packages_model.PackageBlob, bool, error) {
 	log.Trace("Adding package file: %v, %s", pv.ID, pfci.Filename)
 
-	pb, exists, err := packages_model.GetOrInsertBlob(ctx, NewPackageBlob(pfci.Data))
+	pb, exists, err := GetOrSavePackageBlob(ctx, packages_module.NewContentStore(), NewPackageBlob(pfci.Data), pfci.Data)
 	if err != nil {
-		log.Error("Error inserting package blob: %v", err)
-		return nil, nil, false, err
-	}
-	if !exists {
-		contentStore := packages_module.NewContentStore()
-		if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), pfci.Data, pfci.Data.Size()); err != nil {
-			log.Error("Error saving package blob in content store: %v", err)
-			return nil, nil, false, err
-		}
+		return nil, nil, !exists, err
 	}
 
 	if pfci.OverwriteExisting {