From 15898f2069d46a4881894c94f6d8567859b92ba1 Mon Sep 17 00:00:00 2001
From: stonezdj <stonezdj@gmail.com>
Date: Thu, 14 Nov 2019 15:23:21 +0800
Subject: [PATCH] Avoid to create duplicated immutable tag rules in the same
 project

Fix #9681, add constraint on immutable_tag_rule and catch the error

Signed-off-by: stonezdj <stonezdj@gmail.com>
---
 make/migrations/postgresql/0015_1.10.0_schema.up.sql |  3 ++-
 src/core/api/immutabletagrule.go                     |  4 ++++
 src/core/api/immutabletagrule_test.go                |  4 ++--
 src/pkg/immutabletag/controller_test.go              |  2 +-
 src/pkg/immutabletag/dao/immutable_test.go           | 11 ++++++++++-
 5 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/make/migrations/postgresql/0015_1.10.0_schema.up.sql b/make/migrations/postgresql/0015_1.10.0_schema.up.sql
index eb869492d..d8e8666bb 100644
--- a/make/migrations/postgresql/0015_1.10.0_schema.up.sql
+++ b/make/migrations/postgresql/0015_1.10.0_schema.up.sql
@@ -44,7 +44,8 @@ CREATE TABLE immutable_tag_rule
   project_id int NOT NULL,
   tag_filter text,
   disabled BOOLEAN NOT NULL DEFAULT FALSE,
-  creation_time timestamp default CURRENT_TIMESTAMP
+  creation_time timestamp default CURRENT_TIMESTAMP,
+  UNIQUE(project_id, tag_filter)
 );
 
 ALTER TABLE robot ADD COLUMN visible boolean DEFAULT true NOT NULL;
diff --git a/src/core/api/immutabletagrule.go b/src/core/api/immutabletagrule.go
index 91a3b901e..3cc5853bc 100644
--- a/src/core/api/immutabletagrule.go
+++ b/src/core/api/immutabletagrule.go
@@ -92,6 +92,10 @@ func (itr *ImmutableTagRuleAPI) Post() {
 	}
 	ir.ProjectID = itr.projectID
 	id, err := itr.ctr.CreateImmutableRule(ir)
+	if err != nil && strings.Contains(err.Error(), "duplicate key") {
+		itr.RenderError(http.StatusConflict, "immutable tag rule duplicated")
+		return
+	}
 	if err != nil {
 		itr.SendInternalServerError(err)
 		return
diff --git a/src/core/api/immutabletagrule_test.go b/src/core/api/immutabletagrule_test.go
index 8a65e9e1a..dd00b1313 100644
--- a/src/core/api/immutabletagrule_test.go
+++ b/src/core/api/immutabletagrule_test.go
@@ -152,7 +152,7 @@ func TestImmutableTagRuleAPI_Post(t *testing.T) {
 			},
 			code: http.StatusCreated,
 		},
-		// 201
+		// 409
 		{
 			request: &testingRequest{
 				method:     http.MethodPost,
@@ -160,7 +160,7 @@ func TestImmutableTagRuleAPI_Post(t *testing.T) {
 				credential: projAdmin,
 				bodyJSON:   metadata,
 			},
-			code: http.StatusCreated,
+			code: http.StatusConflict,
 		},
 		// 403
 		{
diff --git a/src/pkg/immutabletag/controller_test.go b/src/pkg/immutabletag/controller_test.go
index 624c88685..2886feb3e 100644
--- a/src/pkg/immutabletag/controller_test.go
+++ b/src/pkg/immutabletag/controller_test.go
@@ -150,7 +150,7 @@ func (s *ControllerTestSuite) TestImmutableRule() {
 	s.ruleID, err = s.ctr.CreateImmutableRule(rule2)
 	s.require.Nil(err)
 
-	rules, err := s.ctr.ListImmutableRules(1)
+	rules, err := s.ctr.ListImmutableRules(projectID)
 	s.require.Nil(err)
 	s.require.Equal(2, len(rules))
 
diff --git a/src/pkg/immutabletag/dao/immutable_test.go b/src/pkg/immutabletag/dao/immutable_test.go
index 21640b832..ce71ad8b4 100644
--- a/src/pkg/immutabletag/dao/immutable_test.go
+++ b/src/pkg/immutabletag/dao/immutable_test.go
@@ -1,12 +1,14 @@
 package dao
 
 import (
+	"strings"
+	"testing"
+
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/pkg/immutabletag/dao/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
-	"testing"
 )
 
 type immutableRuleDaoTestSuite struct {
@@ -29,6 +31,13 @@ func (t *immutableRuleDaoTestSuite) TestCreateImmutableRule() {
 	id, err := t.dao.CreateImmutableRule(ir)
 	t.require.Nil(err)
 	t.require.True(id > 0, "Can not create immutable tag rule")
+
+	// insert duplicate rows
+	ir2 := &model.ImmutableRule{TagFilter: "**", ProjectID: 1}
+	id2, err := t.dao.CreateImmutableRule(ir2)
+	t.require.True(strings.Contains(err.Error(), "duplicate key"))
+	t.require.Equal(int64(0), id2)
+
 	_, err = t.dao.DeleteImmutableRule(id)
 	t.require.Nil(err)
 }