From dbdbd0f5e6aa95d305f84fb37e455a49c02f37c9 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Mon, 23 Feb 2026 18:49:26 +0100 Subject: [PATCH] [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 `` entries can be ignored and considers every entry after `` as eligible for cleanup. That doesn't work for multi-platform container images because, for `=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/.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 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11254 Reviewed-by: Mathieu Fenniak Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- services/packages/cleanup/cleanup.go | 11 ++- services/packages/cleanup/cleanup_test.go | 104 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 services/packages/cleanup/cleanup_test.go diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index fac2e62540..3c177abca4 100644 --- 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 diff --git a/services/packages/cleanup/cleanup_test.go b/services/packages/cleanup/cleanup_test.go new file mode 100644 index 0000000000..d6c0f00400 --- /dev/null +++ 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) + }) +}