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 0d1a148f2..61e83a350 100644 --- a/make/migrations/postgresql/0015_1.10.0_schema.up.sql +++ b/make/migrations/postgresql/0015_1.10.0_schema.up.sql @@ -39,10 +39,10 @@ CREATE TABLE scan_report /** Add table for immutable tag **/ CREATE TABLE immutable_tag_rule ( - id SERIAL PRIMARY KEY NOT NULL, - project_id int NOT NULL, - tag_filter text, - enabled boolean default true NOT NULL, + id SERIAL PRIMARY KEY NOT NULL, + project_id int NOT NULL, + tag_filter text, + disabled BOOLEAN NOT NULL DEFAULT FALSE, creation_time timestamp default CURRENT_TIMESTAMP ); diff --git a/src/core/api/immutabletagrule.go b/src/core/api/immutabletagrule.go index 09163d641..91a3b901e 100644 --- a/src/core/api/immutabletagrule.go +++ b/src/core/api/immutabletagrule.go @@ -15,7 +15,7 @@ import ( // ImmutableTagRuleAPI ... type ImmutableTagRuleAPI struct { BaseController - ctr immutabletag.APIController + ctr immutabletag.Controller projectID int64 ID int64 } diff --git a/src/core/middlewares/immutable/handler.go b/src/core/middlewares/immutable/handler.go index e9deb2dbf..e60eb8ab2 100644 --- a/src/core/middlewares/immutable/handler.go +++ b/src/core/middlewares/immutable/handler.go @@ -60,7 +60,7 @@ func (rh *immutableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) log.Warningf("Error occurred when to handle request in immutable handler: %v", err) if _, ok := err.(middlerware_err.ErrImmutable); ok { http.Error(rw, util.MarshalError("DENIED", - fmt.Sprintf("The tag is immutable, cannot be overwrite: %v", err)), http.StatusPreconditionFailed) + fmt.Sprintf("%v", err)), http.StatusPreconditionFailed) return } http.Error(rw, util.MarshalError("InternalError", fmt.Sprintf("Error occurred when to handle request in immutable handler: %v", err)), diff --git a/src/core/middlewares/interceptor/immutable/deletemf.go b/src/core/middlewares/interceptor/immutable/deletemf.go index 6fb2e659a..0b788f367 100644 --- a/src/core/middlewares/interceptor/immutable/deletemf.go +++ b/src/core/middlewares/interceptor/immutable/deletemf.go @@ -55,7 +55,7 @@ func (dmf *delmfInterceptor) HandleRequest(req *http.Request) (err error) { return } if matched { - return middlerware_err.NewErrImmutable(repoName) + return middlerware_err.NewErrImmutable(repoName, af.Tag) } } diff --git a/src/core/middlewares/interceptor/immutable/pushmf.go b/src/core/middlewares/interceptor/immutable/pushmf.go index 307579bd5..990233af8 100644 --- a/src/core/middlewares/interceptor/immutable/pushmf.go +++ b/src/core/middlewares/interceptor/immutable/pushmf.go @@ -57,7 +57,7 @@ func (pmf *pushmfInterceptor) HandleRequest(req *http.Request) (err error) { return } - return middlerware_err.NewErrImmutable(repoName) + return middlerware_err.NewErrImmutable(repoName, pmf.mf.Tag) } // HandleRequest ... diff --git a/src/core/middlewares/util/error/immutable.go b/src/core/middlewares/util/error/immutable.go index a7789300d..0afdbff72 100644 --- a/src/core/middlewares/util/error/immutable.go +++ b/src/core/middlewares/util/error/immutable.go @@ -7,14 +7,18 @@ import ( // ErrImmutable ... type ErrImmutable struct { repo string + tag string } // Error ... func (ei ErrImmutable) Error() string { - return fmt.Sprintf("Failed to process request, due to immutable. '%s'", ei.repo) + return fmt.Sprintf("Failed to process request, due to '%s:%s' is a immutable tag.", ei.repo, ei.tag) } // NewErrImmutable ... -func NewErrImmutable(msg string) ErrImmutable { - return ErrImmutable{repo: msg} +func NewErrImmutable(msg, tag string) ErrImmutable { + return ErrImmutable{ + repo: msg, + tag: tag, + } } diff --git a/src/pkg/immutabletag/controller.go b/src/pkg/immutabletag/controller.go index 482286d92..01ea828a1 100644 --- a/src/pkg/immutabletag/controller.go +++ b/src/pkg/immutabletag/controller.go @@ -9,8 +9,8 @@ var ( ImmuCtr = NewAPIController(NewDefaultRuleManager()) ) -// APIController to handle the requests related with immutabletag -type APIController interface { +// Controller to handle the requests related with immutabletag +type Controller interface { // GetImmutableRule ... GetImmutableRule(id int64) (*model.Metadata, error) @@ -68,7 +68,7 @@ func (r *DefaultAPIController) ListImmutableRules(pid int64) ([]model.Metadata, } // NewAPIController ... -func NewAPIController(immutableMgr Manager) APIController { +func NewAPIController(immutableMgr Manager) Controller { return &DefaultAPIController{ manager: immutableMgr, } diff --git a/src/pkg/immutabletag/controller_test.go b/src/pkg/immutabletag/controller_test.go index 96867ef9f..624c88685 100644 --- a/src/pkg/immutabletag/controller_test.go +++ b/src/pkg/immutabletag/controller_test.go @@ -1 +1,168 @@ package immutabletag + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils/test" + "github.com/goharbor/harbor/src/pkg/immutabletag/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "testing" +) + +type ControllerTestSuite struct { + suite.Suite + ctr Controller + t *testing.T + assert *assert.Assertions + require *require.Assertions + + ruleID int64 +} + +// SetupSuite ... +func (s *ControllerTestSuite) SetupSuite() { + test.InitDatabaseFromEnv() + s.t = s.T() + s.assert = assert.New(s.t) + s.require = require.New(s.t) + s.ctr = ImmuCtr +} + +func (s *ControllerTestSuite) TestImmutableRule() { + + var err error + + projectID, err := dao.AddProject(models.Project{ + Name: "TestImmutableRule", + OwnerID: 1, + }) + + rule := &model.Metadata{ + ProjectID: projectID, + Priority: 1, + Action: "immutable", + Template: "immutable_template", + TagSelectors: []*model.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-**", + }, + }, + ScopeSelectors: map[string][]*model.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "repoMatches", + Pattern: "redis", + }, + }, + }, + } + s.ruleID, err = s.ctr.CreateImmutableRule(rule) + s.require.Nil(err) + + update := &model.Metadata{ + ID: s.ruleID, + ProjectID: projectID, + Priority: 1, + Action: "immutable", + Template: "immutable_template", + TagSelectors: []*model.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-**", + }, + }, + ScopeSelectors: map[string][]*model.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "repoMatches", + Pattern: "postgres", + }, + }, + }, + Disabled: false, + } + err = s.ctr.UpdateImmutableRule(1, update) + s.require.Nil(err) + + getRule, err := s.ctr.GetImmutableRule(s.ruleID) + s.require.Nil(err) + s.require.Equal("postgres", getRule.ScopeSelectors["repository"][0].Pattern) + + update2 := &model.Metadata{ + ID: s.ruleID, + ProjectID: projectID, + Priority: 1, + Action: "immutable", + Template: "immutable_template", + TagSelectors: []*model.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-**", + }, + }, + ScopeSelectors: map[string][]*model.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "repoMatches", + Pattern: "postgres", + }, + }, + }, + Disabled: true, + } + err = s.ctr.UpdateImmutableRule(1, update2) + s.require.Nil(err) + getRule, err = s.ctr.GetImmutableRule(s.ruleID) + s.require.Nil(err) + s.require.True(getRule.Disabled) + + rule2 := &model.Metadata{ + ProjectID: projectID, + Priority: 1, + Action: "immutable", + Template: "immutable_template", + TagSelectors: []*model.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "latest", + }, + }, + ScopeSelectors: map[string][]*model.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "repoMatches", + Pattern: "redis", + }, + }, + }, + } + s.ruleID, err = s.ctr.CreateImmutableRule(rule2) + s.require.Nil(err) + + rules, err := s.ctr.ListImmutableRules(1) + s.require.Nil(err) + s.require.Equal(2, len(rules)) + +} + +// TearDownSuite clears env for test suite +func (s *ControllerTestSuite) TearDownSuite() { + err := s.ctr.DeleteImmutableRule(s.ruleID) + require.NoError(s.T(), err, "delete immutable rule") +} + +// TestController ... +func TestController(t *testing.T) { + suite.Run(t, new(ControllerTestSuite)) +} diff --git a/src/pkg/immutabletag/dao/immutable.go b/src/pkg/immutabletag/dao/immutable.go index 6f07c2b37..7450c5514 100644 --- a/src/pkg/immutabletag/dao/immutable.go +++ b/src/pkg/immutabletag/dao/immutable.go @@ -12,7 +12,7 @@ import ( type ImmutableRuleDao interface { CreateImmutableRule(ir *model.ImmutableRule) (int64, error) UpdateImmutableRule(projectID int64, ir *model.ImmutableRule) (int64, error) - ToggleImmutableRule(id int64, enabled bool) (int64, error) + ToggleImmutableRule(id int64, status bool) (int64, error) GetImmutableRule(id int64) (*model.ImmutableRule, error) QueryImmutableRuleByProjectID(projectID int64) ([]model.ImmutableRule, error) QueryEnabledImmutableRuleByProjectID(projectID int64) ([]model.ImmutableRule, error) @@ -28,7 +28,7 @@ type immutableRuleDao struct{} // CreateImmutableRule creates the Immutable Rule func (i *immutableRuleDao) CreateImmutableRule(ir *model.ImmutableRule) (int64, error) { - ir.Enabled = true + ir.Disabled = false o := dao.GetOrmer() return o.Insert(ir) } @@ -41,10 +41,10 @@ func (i *immutableRuleDao) UpdateImmutableRule(projectID int64, ir *model.Immuta } // ToggleImmutableRule enable/disable immutable rules -func (i *immutableRuleDao) ToggleImmutableRule(id int64, enabled bool) (int64, error) { +func (i *immutableRuleDao) ToggleImmutableRule(id int64, status bool) (int64, error) { o := dao.GetOrmer() - ir := &model.ImmutableRule{ID: id, Enabled: enabled} - return o.Update(ir, "Enabled") + ir := &model.ImmutableRule{ID: id, Disabled: status} + return o.Update(ir, "Disabled") } // GetImmutableRule get immutable rule @@ -76,7 +76,7 @@ func (i *immutableRuleDao) QueryImmutableRuleByProjectID(projectID int64) ([]mod // QueryEnabledImmutableRuleByProjectID get all enabled immutable rule by project func (i *immutableRuleDao) QueryEnabledImmutableRuleByProjectID(projectID int64) ([]model.ImmutableRule, error) { o := dao.GetOrmer() - qs := o.QueryTable(&model.ImmutableRule{}).Filter("ProjectID", projectID).Filter("Enabled", true) + qs := o.QueryTable(&model.ImmutableRule{}).Filter("ProjectID", projectID).Filter("Disabled", false) var r []model.ImmutableRule _, err := qs.All(&r) if err != nil { diff --git a/src/pkg/immutabletag/dao/immutable_test.go b/src/pkg/immutabletag/dao/immutable_test.go index 1fe61faf6..21640b832 100644 --- a/src/pkg/immutabletag/dao/immutable_test.go +++ b/src/pkg/immutabletag/dao/immutable_test.go @@ -58,11 +58,11 @@ func (t *immutableRuleDaoTestSuite) TestEnableImmutableRule() { t.require.Nil(err) t.require.True(id > 0, "Can not create immutable tag rule") - t.dao.ToggleImmutableRule(id, false) + t.dao.ToggleImmutableRule(id, true) newIr, err := t.dao.GetImmutableRule(id) t.require.Nil(err) - t.require.False(newIr.Enabled, "Failed to disable the immutable rule") + t.require.True(newIr.Disabled, "Failed to disable the immutable rule") defer t.dao.DeleteImmutableRule(id) } @@ -95,9 +95,8 @@ func (t *immutableRuleDaoTestSuite) TestGetEnabledImmutableRuleByProject() { for i, ir := range irs { id, _ := t.dao.CreateImmutableRule(ir) if i == 1 { - t.dao.ToggleImmutableRule(id, false) + t.dao.ToggleImmutableRule(id, true) } - } qrs, err := t.dao.QueryEnabledImmutableRuleByProjectID(99) diff --git a/src/pkg/immutabletag/dao/model/rule.go b/src/pkg/immutabletag/dao/model/rule.go index 878cc3838..7bd018dfe 100644 --- a/src/pkg/immutabletag/dao/model/rule.go +++ b/src/pkg/immutabletag/dao/model/rule.go @@ -13,7 +13,7 @@ type ImmutableRule struct { ID int64 `orm:"pk;auto;column(id)" json:"id,omitempty"` ProjectID int64 `orm:"column(project_id)" json:"project_id,omitempty"` TagFilter string `orm:"column(tag_filter)" json:"tag_filter,omitempty"` - Enabled bool `orm:"column(enabled)" json:"enabled,omitempty"` + Disabled bool `orm:"column(disabled)" json:"disabled,omitempty"` } // TableName ... diff --git a/src/pkg/immutabletag/manager.go b/src/pkg/immutabletag/manager.go index 4ef415ae2..3aab69071 100644 --- a/src/pkg/immutabletag/manager.go +++ b/src/pkg/immutabletag/manager.go @@ -36,7 +36,7 @@ type defaultRuleManager struct { func (drm *defaultRuleManager) CreateImmutableRule(ir *model.Metadata) (int64, error) { daoRule := &dao_model.ImmutableRule{} - daoRule.Enabled = !ir.Disabled + daoRule.Disabled = ir.Disabled daoRule.ProjectID = ir.ProjectID data, _ := json.Marshal(ir) daoRule.TagFilter = string(data) @@ -46,6 +46,7 @@ func (drm *defaultRuleManager) CreateImmutableRule(ir *model.Metadata) (int64, e func (drm *defaultRuleManager) UpdateImmutableRule(projectID int64, ir *model.Metadata) (int64, error) { daoRule := &dao_model.ImmutableRule{} data, _ := json.Marshal(ir) + daoRule.ID = ir.ID daoRule.TagFilter = string(data) return drm.dao.UpdateImmutableRule(projectID, daoRule) } @@ -63,6 +64,8 @@ func (drm *defaultRuleManager) GetImmutableRule(id int64) (*model.Metadata, erro if err = json.Unmarshal([]byte(daoRule.TagFilter), rule); err != nil { return nil, err } + rule.ID = daoRule.ID + rule.Disabled = daoRule.Disabled return rule, nil } @@ -77,6 +80,8 @@ func (drm *defaultRuleManager) QueryImmutableRuleByProjectID(projectID int64) ([ if err = json.Unmarshal([]byte(daoRule.TagFilter), &rule); err != nil { return nil, err } + rule.ID = daoRule.ID + rule.Disabled = daoRule.Disabled rules = append(rules, rule) } return rules, nil @@ -93,6 +98,7 @@ func (drm *defaultRuleManager) QueryEnabledImmutableRuleByProjectID(projectID in if err = json.Unmarshal([]byte(daoRule.TagFilter), &rule); err != nil { return nil, err } + rule.ID = daoRule.ID rules = append(rules, rule) } return rules, nil diff --git a/src/pkg/immutabletag/manager_test.go b/src/pkg/immutabletag/manager_test.go index 319685b26..cf14fc960 100644 --- a/src/pkg/immutabletag/manager_test.go +++ b/src/pkg/immutabletag/manager_test.go @@ -110,7 +110,7 @@ func (m *managerTestingSuite) TestQueryImmutableRuleByProjectID() { { ID: 1, ProjectID: 1, - Enabled: true, + Disabled: false, TagFilter: "{\"id\":1, \"projectID\":1,\"priority\":0,\"disabled\":false,\"action\":\"immutable\"," + "\"template\":\"immutable_template\"," + "\"tag_selectors\":[{\"kind\":\"doublestar\",\"decoration\":\"matches\",\"pattern\":\"**\"}]," + @@ -119,7 +119,7 @@ func (m *managerTestingSuite) TestQueryImmutableRuleByProjectID() { { ID: 2, ProjectID: 1, - Enabled: false, + Disabled: false, TagFilter: "{\"id\":2, \"projectID\":1,\"priority\":0,\"disabled\":false,\"action\":\"immutable\"," + "\"template\":\"immutable_template\"," + "\"tag_selectors\":[{\"kind\":\"doublestar\",\"decoration\":\"matches\",\"pattern\":\"**\"}]," + @@ -137,7 +137,7 @@ func (m *managerTestingSuite) TestQueryEnabledImmutableRuleByProjectID() { { ID: 1, ProjectID: 1, - Enabled: true, + Disabled: true, TagFilter: "{\"id\":1, \"projectID\":1,\"priority\":0,\"disabled\":false,\"action\":\"immutable\"," + "\"template\":\"immutable_template\"," + "\"tag_selectors\":[{\"kind\":\"doublestar\",\"decoration\":\"matches\",\"pattern\":\"**\"}]," + @@ -146,7 +146,7 @@ func (m *managerTestingSuite) TestQueryEnabledImmutableRuleByProjectID() { { ID: 2, ProjectID: 1, - Enabled: true, + Disabled: true, TagFilter: "{\"id\":2, \"projectID\":1,\"priority\":0,\"disabled\":false,\"action\":\"immutable\"," + "\"template\":\"immutable_template\"," + "\"tag_selectors\":[{\"kind\":\"doublestar\",\"decoration\":\"matches\",\"pattern\":\"**\"}]," + @@ -163,7 +163,7 @@ func (m *managerTestingSuite) TestGetImmutableRule() { m.mockImmutableDao.On("GetImmutableRule", mock.Anything).Return(&dao_model.ImmutableRule{ ID: 1, ProjectID: 1, - Enabled: true, + Disabled: true, TagFilter: "{\"id\":1, \"projectID\":1,\"priority\":0,\"disabled\":false,\"action\":\"immutable\"," + "\"template\":\"immutable_template\"," + "\"tag_selectors\":[{\"kind\":\"doublestar\",\"decoration\":\"matches\",\"pattern\":\"**\"}]," + diff --git a/src/pkg/immutabletag/match/rule/match_test.go b/src/pkg/immutabletag/match/rule/match_test.go index 7ad4daa10..cad26bdf3 100644 --- a/src/pkg/immutabletag/match/rule/match_test.go +++ b/src/pkg/immutabletag/match/rule/match_test.go @@ -18,7 +18,7 @@ type MatchTestSuite struct { t *testing.T assert *assert.Assertions require *require.Assertions - ctr immutabletag.APIController + ctr immutabletag.Controller ruleID int64 ruleID2 int64 }