From 3ede6720f8266a143b79cd1183ebe1ab483abda7 Mon Sep 17 00:00:00 2001 From: Rasmus Kock Thygesen Date: Tue, 21 Jul 2026 11:29:59 +0200 Subject: [PATCH 1/2] fix(retention): return 404 for missing policies Signed-off-by: Rasmus Kock Thygesen --- api/v2.0/swagger.yaml | 2 ++ src/pkg/retention/dao/retention.go | 2 +- src/pkg/retention/dao/retention_test.go | 8 +++++--- src/pkg/retention/manager.go | 5 ----- src/pkg/retention/manager_test.go | 7 ++++--- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 0489f222fab..f825b0551df 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -5144,6 +5144,8 @@ paths: $ref: '#/responses/401' '403': $ref: '#/responses/403' + '404': + $ref: '#/responses/404' '500': $ref: '#/responses/500' put: diff --git a/src/pkg/retention/dao/retention.go b/src/pkg/retention/dao/retention.go index 1df7290343b..9bfd814cbf4 100644 --- a/src/pkg/retention/dao/retention.go +++ b/src/pkg/retention/dao/retention.go @@ -64,7 +64,7 @@ func GetPolicy(ctx context.Context, id int64) (*models.RetentionPolicy, error) { ID: id, } if err := o.Read(p); err != nil { - return nil, err + return nil, orm.WrapNotFoundError(err, "retention policy %d not found", id) } return p, nil } diff --git a/src/pkg/retention/dao/retention_test.go b/src/pkg/retention/dao/retention_test.go index 950eeb42289..d3928e5663b 100644 --- a/src/pkg/retention/dao/retention_test.go +++ b/src/pkg/retention/dao/retention_test.go @@ -2,14 +2,15 @@ package dao import ( "encoding/json" + "fmt" "os" - "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/retention/dao/models" @@ -98,6 +99,7 @@ func TestPolicy(t *testing.T) { assert.Nil(t, err) p1, err = GetPolicy(ctx, id) - assert.NotNil(t, err) - assert.True(t, strings.Contains(err.Error(), "no row found")) + assert.True(t, errors.IsNotFoundErr(err)) + assert.EqualError(t, err, fmt.Sprintf("retention policy %d not found", id)) + assert.Nil(t, p1) } diff --git a/src/pkg/retention/manager.go b/src/pkg/retention/manager.go index beaad20fe92..494584e14c2 100644 --- a/src/pkg/retention/manager.go +++ b/src/pkg/retention/manager.go @@ -17,10 +17,8 @@ package retention import ( "context" "encoding/json" - "fmt" "time" - "github.com/beego/beego/v2/client/orm" "github.com/go-openapi/strfmt" "github.com/goharbor/harbor/src/common/utils" @@ -87,9 +85,6 @@ func (d *DefaultManager) DeletePolicy(ctx context.Context, id int64) error { func (d *DefaultManager) GetPolicy(ctx context.Context, id int64) (*policy.Metadata, error) { p1, err := dao.GetPolicy(ctx, id) if err != nil { - if err == orm.ErrNoRows { - return nil, fmt.Errorf("no such Retention policy with id %v", id) - } return nil, err } p := &policy.Metadata{} diff --git a/src/pkg/retention/manager_test.go b/src/pkg/retention/manager_test.go index e4e36c8946b..bba4e4cd5cd 100644 --- a/src/pkg/retention/manager_test.go +++ b/src/pkg/retention/manager_test.go @@ -1,13 +1,14 @@ package retention import ( + "fmt" "os" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/retention/policy" "github.com/goharbor/harbor/src/pkg/retention/policy/rule" @@ -81,8 +82,8 @@ func TestPolicy(t *testing.T) { assert.Nil(t, err) p1, err = m.GetPolicy(ctx, id) - assert.NotNil(t, err) - assert.True(t, strings.Contains(err.Error(), "no such Retention policy")) + assert.True(t, errors.IsNotFoundErr(err)) + assert.EqualError(t, err, fmt.Sprintf("retention policy %d not found", id)) assert.Nil(t, p1) } From 8faa4683df9acf76e79b73ad2b6b31bf21f71d0f Mon Sep 17 00:00:00 2001 From: Rasmus Kock Thygesen Date: Tue, 21 Jul 2026 12:30:22 +0200 Subject: [PATCH 2/2] test(retention): update controller not-found assertion Signed-off-by: Rasmus Kock Thygesen --- src/controller/retention/controller_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/controller/retention/controller_test.go b/src/controller/retention/controller_test.go index ccb30fb3db0..9f992a2c331 100644 --- a/src/controller/retention/controller_test.go +++ b/src/controller/retention/controller_test.go @@ -16,8 +16,8 @@ package retention import ( "context" + "fmt" "os" - "strings" "testing" "github.com/stretchr/testify/mock" @@ -26,6 +26,7 @@ import ( "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/retention" @@ -191,8 +192,8 @@ func (s *ControllerTestSuite) TestPolicy() { s.Require().Nil(err) p1, err = c.GetRetention(ctx, id) - s.Require().NotNil(err) - s.Require().True(strings.Contains(err.Error(), "no such Retention policy")) + s.Require().True(errors.IsNotFoundErr(err)) + s.Require().EqualError(err, fmt.Sprintf("retention policy %d not found", id)) s.Require().Nil(p1) }