[v14.0/forgejo] fix: cleanup of multi-platform container images (#11254)

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/11246 This change fixes an issue that makes Forgejo clean up too many versions of a container package even though it should keep them according to the rules set for the package. The issue affects multi-platform container images. Forgejo adds a package version for each platform (for example `linux/amd64`, `linux/arm64`) in addition to the actual tag (for example `0.6.0` or `latest`). This results in rows in the table `package_version` similar to this (unimportant columns omitted for brevity): | **lower_version** | **created_unix** | |---|---| | `latest` | `1768742887`| | `0.6.0` | `1768742886` | | `sha256:038e...` | `1768742886` | | `sha256:fc38...` | `1768742886` | | `0.5.0` | `1768742864` | | `sha256:806d...` | `1768742864` | | `sha256:0a19...` | `1768742864` | | `0.4.0` | `1768742848` | | `...` | `...` | The code assumes that the first `<keep count>` entries can be ignored and considers every entry after `<keep count>` as eligible for cleanup. That doesn't work for multi-platform container images because, for `<keep count>=5`, it considers version `0.4.0` as eligible. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.  ## Release notes  - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/11246): cleanup of multi-platform container images  Co-authored-by: wandhydrant <wandhydrant@noreply.codeberg.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11254 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org> Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org> Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
2026-05-12 22:10:25 +00:00 · 2026-02-23 18:49:26 +01:00 · 2026-02-23 18:49:26 +01:00 · dbdbd0f5e6
commit dbdbd0f5e6
parent f4f3a3e0f3
2 changed files with 113 additions and 2 deletions
--- a/services/packages/cleanup/cleanup.go
+++ b/services/packages/cleanup/cleanup.go
@ -136,8 +136,9 @@ func GetCleanupTargets(ctx context.Context, pcr *packages_model.PackageCleanupRu
 		if err != nil {
 			return nil, fmt.Errorf("failure to SearchVersions for package cleanup rule: %w", err)
 		}
-		keep := min(len(pvs), pcr.KeepCount)
-		for _, pv := range pvs[keep:] {
+
+		var keep int
+		for _, pv := range pvs {
 			if pcr.Type == packages_model.TypeContainer {
 				if skip := container_service.ShouldBeSkipped(pv); skip {
 					log.Debug("Rule[%d]: keep '%s/%s' (container)", pcr.ID, p.Name, pv.Version)
@ -145,6 +146,12 @@ func GetCleanupTargets(ctx context.Context, pcr *packages_model.PackageCleanupRu
 				}
 			}

+			keep++
+			if pcr.KeepCount > 0 && keep <= pcr.KeepCount {
+				log.Debug("Rule[%d]: keep '%s/%s' (count)", pcr.ID, p.Name, pv.Version)
+				continue
+			}
+
 			toMatch := pv.LowerVersion
 			if pcr.MatchFullName {
 				toMatch = p.LowerName + "/" + pv.LowerVersion
--- a/services/packages/cleanup/cleanup_test.go
+++ b/services/packages/cleanup/cleanup_test.go
@ -0,0 +1,104 @@
+package container
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"testing"
+	"time"
+
+	"forgejo.org/models/db"
+	"forgejo.org/models/packages"
+	"forgejo.org/models/unittest"
+	"forgejo.org/modules/timeutil"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetCleanupTargets(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	ctx := db.DefaultContext
+
+	createPackageCleanupRule := func(t *testing.T, keepCount, removeDays int) *packages.PackageCleanupRule {
+		t.Helper()
+
+		pcr := packages.PackageCleanupRule{
+			Enabled:    true,
+			OwnerID:    2001,
+			Type:       packages.TypeContainer,
+			KeepCount:  keepCount,
+			RemoveDays: removeDays,
+		}
+		_, err := db.GetEngine(ctx).Insert(&pcr)
+		require.NoError(t, err)
+		return &pcr
+	}
+
+	createContainerVersions := func(t *testing.T, name string, count int) {
+		t.Helper()
+
+		p := packages.Package{
+			OwnerID:   2001,
+			Name:      name,
+			LowerName: name,
+			Type:      packages.TypeContainer,
+		}
+		_, err := db.GetEngine(ctx).Insert(&p)
+		require.NoError(t, err)
+
+		for i := range count {
+			version := fmt.Sprintf("0.%d.0", i+1)
+			created := time.Now().
+				Add(-720 * time.Hour).
+				Add(time.Duration(count-i) * time.Hour * -1).
+				Unix()
+
+			// Create the package version for the amd64 variant of a multi-platform OCI image
+			platformAmd64Hash := sha256.New()
+			platformAmd64Hash.Write([]byte(version + "amd64"))
+			platformAmd64Version := "sha256:" + hex.EncodeToString(platformAmd64Hash.Sum(nil))
+			platformAmd64PackageVersion := packages.PackageVersion{
+				PackageID:    p.ID,
+				Version:      platformAmd64Version,
+				LowerVersion: platformAmd64Version,
+				CreatedUnix:  timeutil.TimeStamp(created),
+			}
+			_, err = db.GetEngine(ctx).NoAutoTime().Insert(&platformAmd64PackageVersion)
+			require.NoError(t, err)
+
+			// Create the package version for the arm64 variant of a multi-platform OCI image
+			platformArm64Hash := sha256.New()
+			platformArm64Hash.Write([]byte(version + "arm64"))
+			platformArm64Version := "sha256:" + hex.EncodeToString(platformArm64Hash.Sum(nil))
+			platformArm64PackageVersion := packages.PackageVersion{
+				PackageID:    p.ID,
+				Version:      platformArm64Version,
+				LowerVersion: platformArm64Version,
+				CreatedUnix:  timeutil.TimeStamp(created),
+			}
+			_, err = db.GetEngine(ctx).NoAutoTime().Insert(&platformArm64PackageVersion)
+			require.NoError(t, err)
+
+			// Create the package version for tagged manifest of a multi-platform OCI image
+			v := packages.PackageVersion{
+				PackageID:    p.ID,
+				Version:      version,
+				LowerVersion: version,
+				CreatedUnix:  timeutil.TimeStamp(created),
+			}
+			_, err = db.GetEngine(ctx).NoAutoTime().Insert(&v)
+			require.NoError(t, err)
+		}
+	}
+
+	t.Run("keeps the last five versions of multi-platform container images", func(t *testing.T) {
+		pcr := createPackageCleanupRule(t, 5, 7)
+		// Create versions 0.1.0 to 0.6.0
+		createContainerVersions(t, "unit/test", 6)
+		targets, err := GetCleanupTargets(ctx, pcr, true)
+		require.NoError(t, err)
+		require.Len(t, targets, 1)
+		require.Equal(t, "0.1.0", targets[0].PackageVersion.LowerVersion)
+	})
+}