fix: prune RPM metadata when a local file is evicted (#100)
ci/woodpecker/tag/docker Pipeline was successful
ci/woodpecker/tag/docker Pipeline was successful
Follow-up to #99. ## Why Evicting or deleting a local RPM removed the \`local_files\` row but left its \`rpm_metadata\` behind. Since generated repodata is built from \`rpm_metadata\`, \`primary.xml\` kept advertising a package that no longer exists, producing 404s for clients that tried to fetch it. ## Changes - Add \`PostDeleteHook\` and \`MetadataDeleter\` provider interfaces (symmetric to the existing \`PostUploadHook\`/\`MetadataStore\`), plus a \`DeleteRPMMetadata\` DB method. - Implement \`AfterDelete\` in the RPM provider to drop the metadata row for the deleted file. - Route both local delete paths — the new \`evictLocal\` and the existing files handler's \`remove\` — through a shared \`deleteLocalFile\` helper that removes the file then runs the provider's post-delete hook. Non-RPM providers have no hook, so nothing changes for them. - Cover the cleanup with a dockerised test. Reviewed-on: #100 Co-authored-by: Ben Vincent <ben@unkin.net> Co-committed-by: Ben Vincent <ben@unkin.net>
This commit was merged in pull request #100.
This commit is contained in:
@@ -185,13 +185,35 @@ func (h *LocalHandler) remove(w http.ResponseWriter, r *http.Request) {
|
|||||||
repoName := chi.URLParam(r, "name")
|
repoName := chi.URLParam(r, "name")
|
||||||
filePath := chi.URLParam(r, "*")
|
filePath := chi.URLParam(r, "*")
|
||||||
|
|
||||||
if err := h.db.DeleteLocalFile(r.Context(), repoName, filePath); err != nil {
|
if err := deleteLocalFile(r.Context(), h.db, repoName, filePath); err != nil {
|
||||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
w.WriteHeader(http.StatusNoContent)
|
w.WriteHeader(http.StatusNoContent)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// deleteLocalFile removes a local file and runs the provider's post-delete hook,
|
||||||
|
// so provider-derived state (e.g. RPM metadata that feeds generated repodata)
|
||||||
|
// stops referencing a package that no longer exists.
|
||||||
|
func deleteLocalFile(ctx context.Context, db *database.DB, repoName, filePath string) error {
|
||||||
|
if err := db.DeleteLocalFile(ctx, repoName, filePath); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
remote, err := db.GetRemote(ctx, repoName)
|
||||||
|
if err != nil {
|
||||||
|
return nil // file is gone; no repo left to resolve a cleanup hook from
|
||||||
|
}
|
||||||
|
prov, err := provider.Get(remote.PackageType)
|
||||||
|
if err != nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
if hook, ok := prov.(provider.PostDeleteHook); ok {
|
||||||
|
return hook.AfterDelete(ctx, repoName, filePath, db)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func (h *LocalHandler) DB() *database.DB {
|
func (h *LocalHandler) DB() *database.DB {
|
||||||
return h.db
|
return h.db
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,75 @@
|
|||||||
|
package v2
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
|
||||||
|
"git.unkin.net/unkin/artifactapi/internal/database"
|
||||||
|
"git.unkin.net/unkin/artifactapi/internal/provider"
|
||||||
|
_ "git.unkin.net/unkin/artifactapi/internal/provider/rpm" // register the rpm provider so its PostDeleteHook runs
|
||||||
|
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestLocalEvictCleansRPMMetadata verifies that evicting an RPM from a local
|
||||||
|
// repo also removes the derived rpm_metadata row, so generated repodata stops
|
||||||
|
// listing the deleted package.
|
||||||
|
func TestLocalEvictCleansRPMMetadata(t *testing.T) {
|
||||||
|
if testDSN == "" {
|
||||||
|
t.Skip("Docker unavailable")
|
||||||
|
}
|
||||||
|
ctx := context.Background()
|
||||||
|
db, err := database.New(testDSN)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
defer db.Close()
|
||||||
|
|
||||||
|
const repo = "rpm-evict-cleanup"
|
||||||
|
if err := db.CreateRemote(ctx, &models.Remote{Name: repo, PackageType: models.PackageRPM, RepoType: models.RepoTypeLocal}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
const hash = "sha256:bb22"
|
||||||
|
const path = "Packages/example-0.1.0-1.x86_64.rpm"
|
||||||
|
if err := db.UpsertBlob(ctx, hash, "blobs/bb/22", 2048, "application/x-rpm"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := db.CreateLocalFile(ctx, repo, path, hash); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := db.InsertRPMMetadata(ctx, &provider.RPMMetadata{
|
||||||
|
RepoName: repo, FilePath: path, ContentHash: hash,
|
||||||
|
Name: "example", Version: "0.1.0", Release: "1", Arch: "x86_64",
|
||||||
|
Requires: []provider.RPMDep{}, Provides: []provider.RPMDep{},
|
||||||
|
Files: []provider.RPMFile{}, Changelogs: []provider.RPMChangelog{},
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
h := NewObjectsHandler(db)
|
||||||
|
router := chi.NewRouter()
|
||||||
|
router.Route("/locals/{name}/objects", func(r chi.Router) {
|
||||||
|
r.Delete("/*", h.LocalRoutes().ServeHTTP)
|
||||||
|
})
|
||||||
|
|
||||||
|
del := httptest.NewRequest("DELETE", "/locals/"+repo+"/objects/"+path, nil)
|
||||||
|
dw := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(dw, del)
|
||||||
|
if dw.Code != 204 {
|
||||||
|
t.Fatalf("evict = %d, want 204", dw.Code)
|
||||||
|
}
|
||||||
|
|
||||||
|
if f, _ := db.GetLocalFile(ctx, repo, path); f != nil {
|
||||||
|
t.Fatalf("local file still present after evict: %+v", f)
|
||||||
|
}
|
||||||
|
entries, err := db.ListRPMMetadataEntries(ctx, repo)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if len(entries) != 0 {
|
||||||
|
t.Fatalf("rpm_metadata still present after evict: %+v", entries)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -75,7 +75,7 @@ func (h *ObjectsHandler) evictLocal(w http.ResponseWriter, r *http.Request) {
|
|||||||
repoName := chi.URLParam(r, "name")
|
repoName := chi.URLParam(r, "name")
|
||||||
path := chi.URLParam(r, "*")
|
path := chi.URLParam(r, "*")
|
||||||
|
|
||||||
if err := h.db.DeleteLocalFile(r.Context(), repoName, path); err != nil {
|
if err := deleteLocalFile(r.Context(), h.db, repoName, path); err != nil {
|
||||||
http.Error(w, fmt.Sprintf("evict failed: %v", err), http.StatusInternalServerError)
|
http.Error(w, fmt.Sprintf("evict failed: %v", err), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -32,6 +32,11 @@ func (db *DB) InsertRPMMetadata(ctx context.Context, meta *provider.RPMMetadata)
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (db *DB) DeleteRPMMetadata(ctx context.Context, repoName, filePath string) error {
|
||||||
|
_, err := db.Pool.Exec(ctx, `DELETE FROM rpm_metadata WHERE repo_name = $1 AND file_path = $2`, repoName, filePath)
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
type RPMMetadataRow struct {
|
type RPMMetadataRow struct {
|
||||||
RepoName string
|
RepoName string
|
||||||
FilePath string
|
FilePath string
|
||||||
|
|||||||
@@ -53,10 +53,20 @@ type PostUploadHook interface {
|
|||||||
AfterUpload(ctx context.Context, repoName, storagePath, contentHash string, blobs BlobReader, db MetadataStore)
|
AfterUpload(ctx context.Context, repoName, storagePath, contentHash string, blobs BlobReader, db MetadataStore)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// PostDeleteHook lets a provider clean up derived state (e.g. RPM metadata that
|
||||||
|
// feeds generated repodata) after a local file is removed.
|
||||||
|
type PostDeleteHook interface {
|
||||||
|
AfterDelete(ctx context.Context, repoName, storagePath string, db MetadataDeleter) error
|
||||||
|
}
|
||||||
|
|
||||||
type MetadataStore interface {
|
type MetadataStore interface {
|
||||||
InsertRPMMetadata(ctx context.Context, meta *RPMMetadata) error
|
InsertRPMMetadata(ctx context.Context, meta *RPMMetadata) error
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type MetadataDeleter interface {
|
||||||
|
DeleteRPMMetadata(ctx context.Context, repoName, filePath string) error
|
||||||
|
}
|
||||||
|
|
||||||
type RPMMetadataReader interface {
|
type RPMMetadataReader interface {
|
||||||
ListRPMMetadataEntries(ctx context.Context, repoName string) ([]RPMMetadata, error)
|
ListRPMMetadataEntries(ctx context.Context, repoName string) ([]RPMMetadata, error)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -151,6 +151,15 @@ func (p *Provider) AfterUpload(ctx context.Context, repoName, storagePath, conte
|
|||||||
slog.Info("rpm metadata: parsed", "repo", repoName, "name", meta.Name, "version", meta.Version, "arch", meta.Arch)
|
slog.Info("rpm metadata: parsed", "repo", repoName, "name", meta.Name, "version", meta.Version, "arch", meta.Arch)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (p *Provider) AfterDelete(ctx context.Context, repoName, storagePath string, db provider.MetadataDeleter) error {
|
||||||
|
if err := db.DeleteRPMMetadata(ctx, repoName, storagePath); err != nil {
|
||||||
|
slog.Error("rpm metadata: delete failed", "repo", repoName, "path", storagePath, "error", err)
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
slog.Info("rpm metadata: deleted", "repo", repoName, "path", storagePath)
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func rpmDepFromEntry(e rpmlib.Dependency) provider.RPMDep {
|
func rpmDepFromEntry(e rpmlib.Dependency) provider.RPMDep {
|
||||||
dep := provider.RPMDep{Name: e.Name()}
|
dep := provider.RPMDep{Name: e.Name()}
|
||||||
if e.Flags() != 0 {
|
if e.Flags() != 0 {
|
||||||
|
|||||||
Reference in New Issue
Block a user