From dcd431b0d417b8c9a7650da021bd65370150d624 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 1 Oct 2025 03:11:03 +0200 Subject: [PATCH] fix: ensure deleted Debian package does not remain referenced in the apt repository files (#9386) Introduces a new `Notifier` which listens for `PackageDelete` events and triggers a rebuild of the `Packages` & `Release` files which are stored in the debian package repository. Fixes #9369. ## 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... - [ ] in their respective `*_test.go` for unit tests. - [x] 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 - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9386 Reviewed-by: Gusted Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- routers/api/packages/debian/debian.go | 12 ++-- services/packages/debian/notifier.go | 55 +++++++++++++++++++ tests/integration/api_packages_debian_test.go | 31 +++++++++++ 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 services/packages/debian/notifier.go diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go index fd64e35657..49b2153724 100644 --- a/routers/api/packages/debian/debian.go +++ b/routers/api/packages/debian/debian.go @@ -298,11 +298,13 @@ func DeletePackageFile(ctx *context.Context) { if pd != nil { notify_service.PackageDelete(ctx, ctx.Doer, pd) - } - - if err := debian_service.BuildSpecificRepositoryFiles(ctx, ctx.Package.Owner.ID, distribution, component, architecture); err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return + } else { + // If the entire package version wasn't deleted and some other distribution/cmoponent/architecture remains for + // this package, we need to explicitly rebuild this part of the package index. + if err := debian_service.BuildSpecificRepositoryFiles(ctx, ctx.Package.Owner.ID, distribution, component, architecture); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } } ctx.Status(http.StatusNoContent) diff --git a/services/packages/debian/notifier.go b/services/packages/debian/notifier.go new file mode 100644 index 0000000000..96a4ede7da --- /dev/null +++ b/services/packages/debian/notifier.go @@ -0,0 +1,55 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package debian + +import ( + "context" + + packages_model "forgejo.org/models/packages" + user_model "forgejo.org/models/user" + "forgejo.org/modules/log" + debian_module "forgejo.org/modules/packages/debian" + "forgejo.org/services/notify" +) + +func init() { + notify.RegisterNotifier(&debianPackageNotifier{}) +} + +type debianPackageNotifier struct { + notify.NullNotifier +} + +func (m *debianPackageNotifier) PackageDelete(ctx context.Context, doer *user_model.User, pd *packages_model.PackageDescriptor) { + rebuildFromPackageEvent(ctx, pd) +} + +func rebuildFromPackageEvent(ctx context.Context, pd *packages_model.PackageDescriptor) { + if pd.Package == nil || pd.Package.Type != packages_model.TypeDebian { + return + } + + pv, err := GetOrCreateRepositoryVersion(ctx, pd.Owner.ID) + if err != nil { + log.Error("GetOrCreateRepositoryVersion failed with error: %v", err) + return + } + + for _, file := range pd.Files { + distribution := file.Properties.GetByName(debian_module.PropertyDistribution) + component := file.Properties.GetByName(debian_module.PropertyComponent) + architecture := file.Properties.GetByName(debian_module.PropertyArchitecture) + + if distribution == "" || component == "" || architecture == "" { + log.Warn("Debian package had file missing all expected properties; distribution = %q, component = %q, architecture = %q", distribution, component, architecture) + return + } + + log.Trace("Debian package change triggered rebuild of repository index for distribution = %q, component = %q, architecture = %q", distribution, component, architecture) + err = buildRepositoryFiles(ctx, pd.Owner.ID, pv, distribution, component, architecture) + if err != nil { + log.Error("buildRepositoryFiles failed with error: %v", err) + } + } +} diff --git a/tests/integration/api_packages_debian_test.go b/tests/integration/api_packages_debian_test.go index 67498ec043..b965de9e2c 100644 --- a/tests/integration/api_packages_debian_test.go +++ b/tests/integration/api_packages_debian_test.go @@ -264,4 +264,35 @@ func TestPackageDebian(t *testing.T) { assert.Contains(t, body, "Components: "+strings.Join(components, " ")+"\n") assert.Contains(t, body, "Architectures: "+architectures[1]+"\n") }) + + t.Run("Delete via UI", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Test precondition -- ensure that the packageVersion & packageVersion2 are listed in the index + indexURL := fmt.Sprintf("%s/dists/%s/%s/binary-%s/Packages", rootURL, distributions[1], components[0], architectures[0]) + req := NewRequest(t, "GET", indexURL) + resp := MakeRequest(t, req, http.StatusOK) + body := resp.Body.String() + require.Contains(t, body, fmt.Sprintf("Version: %s", packageVersion)) + require.Contains(t, body, fmt.Sprintf("Version: %s", packageVersion2)) + + // Perform backend request that simulates the "Delete package" UI option, which is a generic codepath without + // debian-specific package management awareness... + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + settingsURL := fmt.Sprintf("/user2/-/packages/debian/%s/%s/settings", packageName, packageVersion) + uiURL := fmt.Sprintf("/user2/-/packages/debian/%s/%s", packageName, packageVersion) + req = NewRequestWithValues(t, "POST", settingsURL, map[string]string{ + "_csrf": GetCSRF(t, session, uiURL), + "action": "delete", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // Ensure that the package index has been rebuilt without the deleted package, handled by debianPackageNotifier + req = NewRequest(t, "GET", indexURL) + resp = MakeRequest(t, req, http.StatusOK) + body = resp.Body.String() + assert.NotContains(t, body, fmt.Sprintf("Version: %s", packageVersion)) + require.Contains(t, body, fmt.Sprintf("Version: %s", packageVersion2)) + }) }