From e753ad0556e5e6c631b5e9ff6880d677303e22f2 Mon Sep 17 00:00:00 2001 From: Ibone Gonzalez Date: Mon, 20 Jul 2026 15:49:45 +0200 Subject: [PATCH 1/3] Fix the error message Signed-off-by: Ibone Gonzalez --- src/pkg/auditext/event/member/member.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pkg/auditext/event/member/member.go b/src/pkg/auditext/event/member/member.go index 54f3f16bb57..181cba1f2ac 100644 --- a/src/pkg/auditext/event/member/member.go +++ b/src/pkg/auditext/event/member/member.go @@ -217,8 +217,13 @@ func ensureORMContext(ctx context.Context) context.Context { if ctx == nil { return orm.Context() } - if _, err := orm.FromContext(ctx); err == nil { - return ctx + if o, err := orm.FromContext(ctx); err == nil { + if _, ok := o.(beegoorm.TxOrmer); !ok { + return ctx + } } + // Member audit events are resolved asynchronously and may run after the + // request transaction has already been committed/rolled back. Replace + // transaction-bound ORM with a fresh ORM to avoid using a completed tx. return orm.NewContext(ctx, beegoorm.NewOrm()) } From 553ea359d4dedcc6d3d5d570631c9ca0f93d0c53 Mon Sep 17 00:00:00 2001 From: Ibone Gonzalez Date: Wed, 22 Jul 2026 10:11:35 +0200 Subject: [PATCH 2/3] test(audit): add coverage for ensureORMContext non-tx path Signed-off-by: Ibone Gonzalez --- src/pkg/auditext/event/member/member_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/pkg/auditext/event/member/member_test.go b/src/pkg/auditext/event/member/member_test.go index 1b731f511d4..5bbf40dafe8 100644 --- a/src/pkg/auditext/event/member/member_test.go +++ b/src/pkg/auditext/event/member/member_test.go @@ -21,7 +21,9 @@ import ( "github.com/goharbor/harbor/src/controller/event/metadata/commonevent" "github.com/goharbor/harbor/src/controller/event/model" + "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/notifier/event" + ormtesting "github.com/goharbor/harbor/src/testing/lib/orm" ) func stubLookups() func() { @@ -390,6 +392,16 @@ func TestAuditLogMemberEventEnabled_EmptyOperation(t *testing.T) { } } +func TestEnsureORMContext(t *testing.T) { + // context with a regular (non-tx) ORM: returned as-is, covering the new + // TxOrmer type-assertion branch added to ensureORMContext. + regularCtx := orm.NewContext(context.Background(), &ormtesting.FakeOrmer{}) + gotCtx := ensureORMContext(regularCtx) + if gotCtx != regularCtx { + t.Error("ensureORMContext with non-tx ORM should return the same context") + } +} + func TestParsePreResolved(t *testing.T) { tests := []struct { input string From e5daacec7a554ab1de0565bff4fbd891ef948d6c Mon Sep 17 00:00:00 2001 From: Ibone Gonzalez Date: Thu, 23 Jul 2026 12:18:28 +0200 Subject: [PATCH 3/3] cover ensureORMContext tx-replacement Signed-off-by: Ibone Gonzalez --- src/pkg/auditext/event/member/member.go | 7 +++- src/pkg/auditext/event/member/member_test.go | 44 ++++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/pkg/auditext/event/member/member.go b/src/pkg/auditext/event/member/member.go index 181cba1f2ac..61c868eb8e3 100644 --- a/src/pkg/auditext/event/member/member.go +++ b/src/pkg/auditext/event/member/member.go @@ -213,6 +213,11 @@ func parsePreResolved(info string) (string, string) { return info, "" } +// newBeegoOrm builds a fresh, non-transaction ORM. It is a package-level seam +// so tests can exercise the transaction-replacement branch of ensureORMContext +// without a registered default database (beegoorm.NewOrm panics otherwise). +var newBeegoOrm = beegoorm.NewOrm + func ensureORMContext(ctx context.Context) context.Context { if ctx == nil { return orm.Context() @@ -225,5 +230,5 @@ func ensureORMContext(ctx context.Context) context.Context { // Member audit events are resolved asynchronously and may run after the // request transaction has already been committed/rolled back. Replace // transaction-bound ORM with a fresh ORM to avoid using a completed tx. - return orm.NewContext(ctx, beegoorm.NewOrm()) + return orm.NewContext(ctx, newBeegoOrm()) } diff --git a/src/pkg/auditext/event/member/member_test.go b/src/pkg/auditext/event/member/member_test.go index 5bbf40dafe8..b9234cf666a 100644 --- a/src/pkg/auditext/event/member/member_test.go +++ b/src/pkg/auditext/event/member/member_test.go @@ -19,6 +19,8 @@ import ( "net/http" "testing" + beegoorm "github.com/beego/beego/v2/client/orm" + "github.com/goharbor/harbor/src/controller/event/metadata/commonevent" "github.com/goharbor/harbor/src/controller/event/model" "github.com/goharbor/harbor/src/lib/orm" @@ -393,13 +395,41 @@ func TestAuditLogMemberEventEnabled_EmptyOperation(t *testing.T) { } func TestEnsureORMContext(t *testing.T) { - // context with a regular (non-tx) ORM: returned as-is, covering the new - // TxOrmer type-assertion branch added to ensureORMContext. - regularCtx := orm.NewContext(context.Background(), &ormtesting.FakeOrmer{}) - gotCtx := ensureORMContext(regularCtx) - if gotCtx != regularCtx { - t.Error("ensureORMContext with non-tx ORM should return the same context") - } + // Stub the ORM factory so the transaction-replacement branch does not need a + // registered default DB (beegoorm.NewOrm panics otherwise). + origNewOrm := newBeegoOrm + defer func() { newBeegoOrm = origNewOrm }() + fresh := &ormtesting.FakeOrmer{} + newBeegoOrm = func() beegoorm.Ormer { return fresh } + + t.Run("non-tx ORM returned as-is", func(t *testing.T) { + // A regular (non-tx) ORM must be returned unchanged, covering the + // TxOrmer type-assertion branch in ensureORMContext. + ctx := orm.NewContext(context.Background(), &ormtesting.FakeOrmer{}) + if got := ensureORMContext(ctx); got != ctx { + t.Error("ensureORMContext with non-tx ORM should return the same context") + } + }) + + t.Run("tx ORM replaced with fresh non-tx ORM", func(t *testing.T) { + // A transaction ORM must be swapped for a fresh non-tx ORM, guarding the + // regression where member audit events reused a completed transaction. + ctx := orm.NewContext(context.Background(), &ormtesting.FakeTxOrmer{}) + got := ensureORMContext(ctx) + if got == ctx { + t.Fatal("ensureORMContext with tx ORM should return a new context") + } + o, err := orm.FromContext(got) + if err != nil { + t.Fatalf("orm.FromContext returned error: %v", err) + } + if _, isTx := o.(beegoorm.TxOrmer); isTx { + t.Error("ensureORMContext should replace the transaction ORM with a non-tx ORM") + } + if o != fresh { + t.Error("ensureORMContext should install the ORM produced by newBeegoOrm") + } + }) } func TestParsePreResolved(t *testing.T) {