Fix container push tag overwriting (#35936)

Fix #35853

Author: wxiaoguang

routers/api/packages/container/manifest.go

@@ -10,7 +10,6 @@ import (
 	"io"
 	"os"
 	"strings"
-	"time"
 
 	"code.gitea.io/gitea/models/db"
 	packages_model "code.gitea.io/gitea/models/packages"
@@ -260,6 +259,13 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
 		return nil, err
 	}
 
+	// "docker buildx imagetools create" multi-arch operations:
+	// {"type":"oci","is_tagged":false,"platform":"unknown/unknown"}
+	// {"type":"oci","is_tagged":false,"platform":"linux/amd64","layer_creation":["ADD file:9233f6f2237d79659a9521f7e390df217cec49f1a8aa3a12147bbca1956acdb9 in /","CMD [\"/bin/sh\"]"]}
+	// {"type":"oci","is_tagged":false,"platform":"unknown/unknown"}
+	// {"type":"oci","is_tagged":false,"platform":"linux/arm64","layer_creation":["ADD file:df53811312284306901fdaaff0a357a4bf40d631e662fe9ce6d342442e494b6c in /","CMD [\"/bin/sh\"]"]}
+	// {"type":"oci","is_tagged":true,"manifests":[{"platform":"linux/amd64","digest":"sha256:72bb73e706c0dec424d00a1febb21deaf1175a70ead009ad8b159729cfcf5769","size":2819478},{"platform":"linux/arm64","digest":"sha256:9e1426dd084a3221663b85ca1ee99d140c50b153917a5c5604c1f9b78229fd24","size":2716499},{"platform":"unknown/unknown","digest":"sha256:b93f03d0ae11b988243e1b2cd8d29accf5b9670547b7bd8c7d96abecc7283e6e","size":1798},{"platform":"unknown/unknown","digest":"sha256:f034b182ba66366c63a5d195c6dfcd3333c027409c0ac98e55ade36aaa3b2963","size":1798}]}
+
 	_pv := &packages_model.PackageVersion{
 		PackageID:    p.ID,
 		CreatorID:    mci.Creator.ID,
@@ -273,25 +279,16 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
 			log.Error("Error inserting package: %v", err)
 			return nil, err
 		}
-
-		if container_module.IsMediaTypeImageIndex(mci.MediaType) {
-			if pv.CreatedUnix.AsTime().Before(time.Now().Add(-24 * time.Hour)) {
-				if err = packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
-					return nil, err
-				}
-				// keep download count on overwriting
-				_pv.DownloadCount = pv.DownloadCount
-				if pv, err = packages_model.GetOrInsertVersion(ctx, _pv); err != nil {
-					if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
-						log.Error("Error inserting package: %v", err)
-						return nil, err
-					}
-				}
-			} else {
-				err = packages_model.UpdateVersion(ctx, &packages_model.PackageVersion{ID: pv.ID, MetadataJSON: _pv.MetadataJSON})
-				if err != nil {
-					return nil, err
-				}
+		if err = packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
+			return nil, err
+		}
+		// keep download count on overwriting
+		_pv.DownloadCount = pv.DownloadCount
+		pv, err = packages_model.GetOrInsertVersion(ctx, _pv)
+		if err != nil {
+			if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
+				log.Error("Error inserting package: %v", err)
+				return nil, err
 			}
 		}
 	}

tests/integration/api_packages_container_test.go

@@ -28,6 +28,7 @@ import (
 
 	oci "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPackageContainer(t *testing.T) {
@@ -70,13 +71,12 @@ func TestPackageContainer(t *testing.T) {
 
 	manifestDigest := "sha256:4f10484d1c1bb13e3956b4de1cd42db8e0f14a75be1617b60f2de3cd59c803c6"
 	manifestContent := `{"schemaVersion":2,"mediaType":"` + container_module.ContentTypeDockerDistributionManifestV2 + `","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":"sha256:4607e093bec406eaadb6f3a340f63400c9d3a7038680744c406903766b938f0d","size":1069},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","digest":"sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4","size":32}]}`
-	manifestContentType := container_module.ContentTypeDockerDistributionManifestV2
 
 	untaggedManifestDigest := "sha256:4305f5f5572b9a426b88909b036e52ee3cf3d7b9c1b01fac840e90747f56623d"
 	untaggedManifestContent := `{"schemaVersion":2,"mediaType":"` + oci.MediaTypeImageManifest + `","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":"sha256:4607e093bec406eaadb6f3a340f63400c9d3a7038680744c406903766b938f0d","size":1069},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","digest":"sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4","size":32}]}`
 
-	indexManifestDigest := "sha256:bab112d6efb9e7f221995caaaa880352feb5bd8b1faf52fae8d12c113aa123ec"
-	indexManifestContent := `{"schemaVersion":2,"mediaType":"` + oci.MediaTypeImageIndex + `","manifests":[{"mediaType":"application/vnd.docker.distribution.manifest.v2+json","digest":"` + manifestDigest + `","platform":{"os":"linux","architecture":"arm","variant":"v7"}},{"mediaType":"` + oci.MediaTypeImageManifest + `","digest":"` + untaggedManifestDigest + `","platform":{"os":"linux","architecture":"arm64","variant":"v8"}}]}`
+	indexManifestDigest := "sha256:2c6b5afb967d5de02795ee1d177c3746d005df4b4c2b829385b0d186b3414b6b"
+	indexManifestContent := `{"schemaVersion":2,"mediaType":"` + oci.MediaTypeImageIndex + `","is_tagged":true,"manifests":[{"mediaType":"application/vnd.docker.distribution.manifest.v2+json","digest":"` + manifestDigest + `","platform":{"os":"linux","architecture":"arm","variant":"v7"}},{"mediaType":"` + oci.MediaTypeImageManifest + `","digest":"` + untaggedManifestDigest + `","platform":{"os":"linux","architecture":"arm64","variant":"v8"}}]}`
 
 	anonymousToken := ""
 	userToken := ""
@@ -467,15 +467,16 @@ func TestPackageContainer(t *testing.T) {
 						assert.NoError(t, err)
 						assert.EqualValues(t, 1, pv.DownloadCount)
 
-						// Overwrite existing tag should keep the download count
-						req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/%s", url, tag), strings.NewReader(manifestContent)).
-							AddTokenAuth(userToken).
-							SetHeader("Content-Type", oci.MediaTypeImageManifest)
-						MakeRequest(t, req, http.StatusCreated)
+						t.Run("OverwriteTagKeepDownloadCount", func(t *testing.T) {
+							req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/%s", url, tag), strings.NewReader(manifestContent)).
+								AddTokenAuth(userToken).
+								SetHeader("Content-Type", oci.MediaTypeImageManifest)
+							MakeRequest(t, req, http.StatusCreated)
 
-						pv, err = packages_model.GetVersionByNameAndVersion(t.Context(), user.ID, packages_model.TypeContainer, image, tag)
-						assert.NoError(t, err)
-						assert.EqualValues(t, 1, pv.DownloadCount)
+							pv, err = packages_model.GetVersionByNameAndVersion(t.Context(), user.ID, packages_model.TypeContainer, image, tag)
+							assert.NoError(t, err)
+							assert.EqualValues(t, 1, pv.DownloadCount)
+						})
 					})
 
 					t.Run("HeadManifest", func(t *testing.T) {
@@ -505,7 +506,7 @@ func TestPackageContainer(t *testing.T) {
 						resp := MakeRequest(t, req, http.StatusOK)
 
 						assert.Equal(t, strconv.Itoa(len(manifestContent)), resp.Header().Get("Content-Length"))
-						assert.Equal(t, manifestContentType, resp.Header().Get("Content-Type"))
+						assert.Equal(t, oci.MediaTypeImageManifest, resp.Header().Get("Content-Type")) // the manifest is overwritten by above OverwriteTagKeepDownloadCount
 						assert.Equal(t, manifestDigest, resp.Header().Get("Docker-Content-Digest"))
 						assert.Equal(t, manifestContent, resp.Body.String())
 					})
@@ -599,6 +600,17 @@ func TestPackageContainer(t *testing.T) {
 				assert.True(t, pd.Files[0].File.IsLead)
 				assert.Equal(t, oci.MediaTypeImageIndex, pd.Files[0].Properties.GetByName(container_module.PropertyMediaType))
 				assert.Equal(t, indexManifestDigest, pd.Files[0].Properties.GetByName(container_module.PropertyDigest))
+
+				lastPackageVersionID := pv.ID
+				t.Run("UploadAgain", func(t *testing.T) {
+					req := NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/%s", url, multiTag), strings.NewReader(indexManifestContent)).
+						AddTokenAuth(userToken).
+						SetHeader("Content-Type", oci.MediaTypeImageIndex)
+					MakeRequest(t, req, http.StatusCreated)
+					pv, err := packages_model.GetVersionByNameAndVersion(t.Context(), user.ID, packages_model.TypeContainer, image, multiTag)
+					require.NoError(t, err)
+					assert.NotEqual(t, lastPackageVersionID, pv.ID)
+				})
 			})
 
 			t.Run("HeadBlob", func(t *testing.T) {