Merge pull request #16510 from chlins/fix/check-before-update-tag-pulltime

fix: check the existence of the tag before updating pull time

src/controller/artifact/controller.go

@@ -19,10 +19,11 @@ import (
 	"context"
 	stderrors "errors"
 	"fmt"
-	accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
 	"strings"
 	"time"
 
+	accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
+
 	"github.com/goharbor/harbor/src/controller/artifact/processor/chart"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/cnab"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/image"
@@ -531,19 +532,24 @@ func (c *controller) UpdatePullTime(ctx context.Context, artifactID int64, tagID
 	if err := c.artMgr.UpdatePullTime(ctx, artifactID, time); err != nil {
 		return err
 	}
-	tg, err := c.tagCtl.Get(ctx, tagID, nil)
+	// update tag pull time if artifact has tag
-	if err != nil {
+	if tagID != 0 {
-		return err
+		tg, err := c.tagCtl.Get(ctx, tagID, nil)
+		if err != nil {
+			return err
+		}
+		if tg.ArtifactID != artifactID {
+			return fmt.Errorf("tag %d isn't attached to artifact %d", tagID, artifactID)
+		}
+		return c.tagCtl.Update(ctx, &tag.Tag{
+			Tag: model_tag.Tag{
+				ID:       tg.ID,
+				PullTime: time,
+			},
+		}, "PullTime")
 	}
-	if tg.ArtifactID != artifactID {
+
-		return fmt.Errorf("tag %d isn't attached to artifact %d", tagID, artifactID)
+	return nil
-	}
-	return c.tagCtl.Update(ctx, &tag.Tag{
-		Tag: model_tag.Tag{
-			ID:       tg.ID,
-			PullTime: time,
-		},
-	}, "PullTime")
 }
 
 func (c *controller) GetAddition(ctx context.Context, artifactID int64, addition string) (*processor.Addition, error) {

src/controller/artifact/controller_test.go

@@ -598,6 +598,15 @@ func (c *controllerTestSuite) TestUpdatePullTime() {
 	c.Require().NotNil(err)
 	c.tagCtl.AssertExpectations(c.T())
 
+	// if no tag, should not update tag
+	c.SetupTest()
+	c.tagCtl.On("Update").Return(nil)
+	c.artMgr.On("UpdatePullTime", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	err = c.ctl.UpdatePullTime(nil, 1, 0, time.Now())
+	c.Require().Nil(err)
+	c.artMgr.AssertExpectations(c.T())
+	// should not call tag Update
+	c.tagCtl.AssertNotCalled(c.T(), "Update")
 }
 
 func (c *controllerTestSuite) TestGetAddition() {