From 6f6f113f0fdc88b53b05b492716833916fa690c4 Mon Sep 17 00:00:00 2001 From: wang yan Date: Thu, 10 Oct 2019 19:15:02 +0800 Subject: [PATCH] refactor robot api 1, add API controller for robot account, make it callable internally 2, add Manager to handler dao releate operation Signed-off-by: wang yan --- src/common/dao/robot.go | 106 ------------ src/common/dao/robot_test.go | 159 ------------------ src/common/models/base.go | 1 - src/common/security/robot/context.go | 5 +- src/common/security/robot/context_test.go | 13 +- src/core/api/immutabletagrule_test.go | 5 +- src/core/api/robot.go | 110 ++++-------- src/core/api/robot_test.go | 20 +-- src/core/filter/security.go | 4 +- src/pkg/robot/controller.go | 115 +++++++++++++ src/pkg/robot/controller_test.go | 103 ++++++++++++ src/pkg/robot/dao/robot.go | 121 +++++++++++++ src/pkg/robot/dao/robot_test.go | 140 +++++++++++++++ src/pkg/robot/manager.go | 71 ++++++++ src/pkg/robot/manager_test.go | 142 ++++++++++++++++ .../models => pkg/robot/model}/robot.go | 45 +++-- 16 files changed, 773 insertions(+), 387 deletions(-) delete mode 100644 src/common/dao/robot.go delete mode 100644 src/common/dao/robot_test.go create mode 100644 src/pkg/robot/controller.go create mode 100644 src/pkg/robot/controller_test.go create mode 100644 src/pkg/robot/dao/robot.go create mode 100644 src/pkg/robot/dao/robot_test.go create mode 100644 src/pkg/robot/manager.go create mode 100644 src/pkg/robot/manager_test.go rename src/{common/models => pkg/robot/model}/robot.go (70%) diff --git a/src/common/dao/robot.go b/src/common/dao/robot.go deleted file mode 100644 index 0d8b5c7f1..000000000 --- a/src/common/dao/robot.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dao - -import ( - "github.com/astaxie/beego/orm" - "github.com/goharbor/harbor/src/common/models" - "strings" - "time" -) - -// AddRobot ... -func AddRobot(robot *models.Robot) (int64, error) { - now := time.Now() - robot.CreationTime = now - robot.UpdateTime = now - id, err := GetOrmer().Insert(robot) - if err != nil { - if strings.Contains(err.Error(), "duplicate key value violates unique constraint") { - return 0, ErrDupRows - } - return 0, err - } - return id, nil -} - -// GetRobotByID ... -func GetRobotByID(id int64) (*models.Robot, error) { - robot := &models.Robot{ - ID: id, - } - if err := GetOrmer().Read(robot); err != nil { - if err == orm.ErrNoRows { - return nil, nil - } - return nil, err - } - - return robot, nil -} - -// ListRobots list robots according to the query conditions -func ListRobots(query *models.RobotQuery) ([]*models.Robot, error) { - qs := getRobotQuerySetter(query).OrderBy("Name") - if query != nil { - if query.Size > 0 { - qs = qs.Limit(query.Size) - if query.Page > 0 { - qs = qs.Offset((query.Page - 1) * query.Size) - } - } - } - robots := []*models.Robot{} - _, err := qs.All(&robots) - return robots, err -} - -func getRobotQuerySetter(query *models.RobotQuery) orm.QuerySeter { - qs := GetOrmer().QueryTable(&models.Robot{}) - - if query == nil { - return qs - } - - if len(query.Name) > 0 { - if query.FuzzyMatchName { - qs = qs.Filter("Name__icontains", query.Name) - } else { - qs = qs.Filter("Name", query.Name) - } - } - if query.ProjectID != 0 { - qs = qs.Filter("ProjectID", query.ProjectID) - } - return qs -} - -// CountRobot ... -func CountRobot(query *models.RobotQuery) (int64, error) { - return getRobotQuerySetter(query).Count() -} - -// UpdateRobot ... -func UpdateRobot(robot *models.Robot) error { - robot.UpdateTime = time.Now() - _, err := GetOrmer().Update(robot) - return err -} - -// DeleteRobot ... -func DeleteRobot(id int64) error { - _, err := GetOrmer().QueryTable(&models.Robot{}).Filter("ID", id).Delete() - return err -} diff --git a/src/common/dao/robot_test.go b/src/common/dao/robot_test.go deleted file mode 100644 index 0ffbcf081..000000000 --- a/src/common/dao/robot_test.go +++ /dev/null @@ -1,159 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dao - -import ( - "testing" - - "github.com/goharbor/harbor/src/common/models" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestAddRobot(t *testing.T) { - robotName := "test1" - robot := &models.Robot{ - Name: robotName, - Description: "test1 description", - ProjectID: 1, - } - - // add - id, err := AddRobot(robot) - require.Nil(t, err) - robot.ID = id - - require.Nil(t, err) - assert.NotNil(t, id) - -} - -func TestGetRobot(t *testing.T) { - robotName := "test2" - robot := &models.Robot{ - Name: robotName, - Description: "test2 description", - ProjectID: 1, - } - - // add - id, err := AddRobot(robot) - require.Nil(t, err) - robot.ID = id - - robot, err = GetRobotByID(id) - require.Nil(t, err) - assert.Equal(t, robotName, robot.Name) - -} - -func TestListRobots(t *testing.T) { - robotName := "test3" - robot := &models.Robot{ - Name: robotName, - Description: "test3 description", - ProjectID: 1, - } - - _, err := AddRobot(robot) - require.Nil(t, err) - - robots, err := ListRobots(&models.RobotQuery{ - ProjectID: 1, - }) - require.Nil(t, err) - assert.Equal(t, 3, len(robots)) - -} - -func TestDisableRobot(t *testing.T) { - robotName := "test4" - robot := &models.Robot{ - Name: robotName, - Description: "test4 description", - ProjectID: 1, - } - - // add - id, err := AddRobot(robot) - require.Nil(t, err) - - // Disable - robot.Disabled = true - err = UpdateRobot(robot) - require.Nil(t, err) - - // Get - robot, err = GetRobotByID(id) - require.Nil(t, err) - assert.Equal(t, true, robot.Disabled) - -} - -func TestEnableRobot(t *testing.T) { - robotName := "test5" - robot := &models.Robot{ - Name: robotName, - Description: "test5 description", - Disabled: true, - ProjectID: 1, - } - - // add - id, err := AddRobot(robot) - require.Nil(t, err) - - // Disable - robot.Disabled = false - err = UpdateRobot(robot) - require.Nil(t, err) - - // Get - robot, err = GetRobotByID(id) - require.Nil(t, err) - assert.Equal(t, false, robot.Disabled) - -} - -func TestDeleteRobot(t *testing.T) { - robotName := "test6" - robot := &models.Robot{ - Name: robotName, - Description: "test6 description", - ProjectID: 1, - } - - // add - id, err := AddRobot(robot) - require.Nil(t, err) - - // Disable - err = DeleteRobot(id) - require.Nil(t, err) - - // Get - robot, err = GetRobotByID(id) - assert.Nil(t, robot) - -} - -func TestListAllRobot(t *testing.T) { - - robots, err := ListRobots(nil) - require.Nil(t, err) - assert.Equal(t, 5, len(robots)) - -} diff --git a/src/common/models/base.go b/src/common/models/base.go index de04d0285..c08b0f37c 100644 --- a/src/common/models/base.go +++ b/src/common/models/base.go @@ -35,7 +35,6 @@ func init() { new(UserGroup), new(AdminJob), new(JobLog), - new(Robot), new(OIDCUser), new(NotificationPolicy), new(NotificationJob), diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index 8fc622fe0..43ebf5921 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -18,17 +18,18 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/pkg/robot/model" ) // SecurityContext implements security.Context interface based on database type SecurityContext struct { - robot *models.Robot + robot *model.Robot pm promgr.ProjectManager policy []*rbac.Policy } // NewSecurityContext ... -func NewSecurityContext(robot *models.Robot, pm promgr.ProjectManager, policy []*rbac.Policy) *SecurityContext { +func NewSecurityContext(robot *model.Robot, pm promgr.ProjectManager, policy []*rbac.Policy) *SecurityContext { return &SecurityContext{ robot: robot, pm: pm, diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go index 36a8a5316..a16cd40fe 100644 --- a/src/common/security/robot/context_test.go +++ b/src/common/security/robot/context_test.go @@ -26,6 +26,7 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" + "github.com/goharbor/harbor/src/pkg/robot/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -96,7 +97,7 @@ func TestIsAuthenticated(t *testing.T) { assert.False(t, ctx.IsAuthenticated()) // authenticated - ctx = NewSecurityContext(&models.Robot{ + ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, }, nil, nil) @@ -109,7 +110,7 @@ func TestGetUsername(t *testing.T) { assert.Equal(t, "", ctx.GetUsername()) // authenticated - ctx = NewSecurityContext(&models.Robot{ + ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, }, nil, nil) @@ -122,7 +123,7 @@ func TestIsSysAdmin(t *testing.T) { assert.False(t, ctx.IsSysAdmin()) // authenticated, non admin - ctx = NewSecurityContext(&models.Robot{ + ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, }, nil, nil) @@ -141,7 +142,7 @@ func TestHasPullPerm(t *testing.T) { Action: rbac.ActionPull, }, } - robot := &models.Robot{ + robot := &model.Robot{ Name: "test_robot_1", Description: "desc", } @@ -158,7 +159,7 @@ func TestHasPushPerm(t *testing.T) { Action: rbac.ActionPush, }, } - robot := &models.Robot{ + robot := &model.Robot{ Name: "test_robot_2", Description: "desc", } @@ -179,7 +180,7 @@ func TestHasPushPullPerm(t *testing.T) { Action: rbac.ActionPull, }, } - robot := &models.Robot{ + robot := &model.Robot{ Name: "test_robot_3", Description: "desc", } diff --git a/src/core/api/immutabletagrule_test.go b/src/core/api/immutabletagrule_test.go index 0877b442b..8a65e9e1a 100644 --- a/src/core/api/immutabletagrule_test.go +++ b/src/core/api/immutabletagrule_test.go @@ -97,9 +97,10 @@ func TestImmutableTagRuleAPI_List(t *testing.T) { func TestImmutableTagRuleAPI_Post(t *testing.T) { // body := `{ - // "id":0, - // "projectID":1, + // "projectID":1, // "priority":0, + // "template": "immutable_template", + // "action": "immutable", // "disabled":false, // "action":"immutable", // "template":"immutable_template", diff --git a/src/core/api/robot.go b/src/core/api/robot.go index 2d4c91012..5c42629a8 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -15,30 +15,29 @@ package api import ( - "errors" "fmt" - "net/http" - "strconv" - "time" - - "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/rbac/project" - "github.com/goharbor/harbor/src/common/token" - "github.com/goharbor/harbor/src/core/config" + "github.com/goharbor/harbor/src/pkg/robot" + "github.com/goharbor/harbor/src/pkg/robot/model" + "github.com/pkg/errors" + "net/http" + "strconv" ) // RobotAPI ... type RobotAPI struct { BaseController project *models.Project - robot *models.Robot + ctr robot.Controller + robot *model.Robot } // Prepare ... func (r *RobotAPI) Prepare() { + r.BaseController.Prepare() method := r.Ctx.Request.Method @@ -68,6 +67,7 @@ func (r *RobotAPI) Prepare() { return } r.project = project + r.ctr = robot.RobotCtr if method == http.MethodPut || method == http.MethodDelete { id, err := r.GetInt64FromPath(":id") @@ -75,8 +75,7 @@ func (r *RobotAPI) Prepare() { r.SendBadRequestError(errors.New("invalid robot ID")) return } - - robot, err := dao.GetRobotByID(id) + robot, err := r.ctr.GetRobotAccount(id) if err != nil { r.SendInternalServerError(fmt.Errorf("failed to get robot %d: %v", id, err)) return @@ -101,7 +100,7 @@ func (r *RobotAPI) Post() { return } - var robotReq models.RobotReq + var robotReq model.RobotCreate isValid, err := r.DecodeJSONReqAndValidate(&robotReq) if !isValid { r.SendBadRequestError(err) @@ -113,59 +112,25 @@ func (r *RobotAPI) Post() { return } - // Token duration in minutes - tokenDuration := time.Duration(config.RobotTokenDuration()) * time.Minute - expiresAt := time.Now().UTC().Add(tokenDuration).Unix() - createdName := common.RobotPrefix + robotReq.Name - - // first to add a robot account, and get its id. - robot := models.Robot{ - Name: createdName, - Description: robotReq.Description, - ProjectID: r.project.ProjectID, - ExpiresAt: expiresAt, - } - id, err := dao.AddRobot(&robot) + robot, err := r.ctr.CreateRobotAccount(&robotReq) if err != nil { if err == dao.ErrDupRows { r.SendConflictError(errors.New("conflict robot account")) return } - r.SendInternalServerError(fmt.Errorf("failed to create robot account: %v", err)) + r.SendInternalServerError(errors.Wrap(err, "robot API: post")) return } - // generate the token, and return it with response data. - // token is not stored in the database. - jwtToken, err := token.New(id, r.project.ProjectID, expiresAt, robotReq.Access) - if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to valid parameters to generate token for robot account, %v", err)) - err := dao.DeleteRobot(id) - if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to delete the robot account: %d, %v", id, err)) - } - return - } - - rawTk, err := jwtToken.Raw() - if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to sign token for robot account, %v", err)) - err := dao.DeleteRobot(id) - if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to delete the robot account: %d, %v", id, err)) - } - return - } - - robotRep := models.RobotRep{ - Name: robot.Name, - Token: rawTk, - } - w := r.Ctx.ResponseWriter w.Header().Set("Content-Type", "application/json") - r.Redirect(http.StatusCreated, strconv.FormatInt(id, 10)) + robotRep := model.RobotRep{ + Name: robot.Name, + Token: robot.Token, + } + + r.Redirect(http.StatusCreated, strconv.FormatInt(robot.ID, 10)) r.Data["json"] = robotRep r.ServeJSON() } @@ -176,28 +141,19 @@ func (r *RobotAPI) List() { return } - query := models.RobotQuery{ - ProjectID: r.project.ProjectID, - } - - count, err := dao.CountRobot(&query) + robots, err := r.ctr.ListRobotAccount(r.project.ProjectID) if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to list robots on project: %d, %v", r.project.ProjectID, err)) + r.SendInternalServerError(errors.Wrap(err, "robot API: list")) return } - query.Page, query.Size, err = r.GetPaginationParams() + count := len(robots) + page, size, err := r.GetPaginationParams() if err != nil { r.SendBadRequestError(err) return } - robots, err := dao.ListRobots(&query) - if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to get robots %v", err)) - return - } - - r.SetPaginationHeader(count, query.Page, query.Size) + r.SetPaginationHeader(int64(count), page, size) r.Data["json"] = robots r.ServeJSON() } @@ -214,13 +170,13 @@ func (r *RobotAPI) Get() { return } - robot, err := dao.GetRobotByID(id) + robot, err := r.ctr.GetRobotAccount(id) if err != nil { - r.SendInternalServerError(fmt.Errorf("failed to get robot %d: %v", id, err)) + r.SendInternalServerError(errors.Wrap(err, "robot API: get robot")) return } if robot == nil { - r.SendNotFoundError(fmt.Errorf("robot %d not found", id)) + r.SendNotFoundError(fmt.Errorf("robot API: robot %d not found", id)) return } @@ -234,7 +190,7 @@ func (r *RobotAPI) Put() { return } - var robotReq models.RobotReq + var robotReq model.RobotCreate if err := r.DecodeJSONReq(&robotReq); err != nil { r.SendBadRequestError(err) return @@ -242,8 +198,8 @@ func (r *RobotAPI) Put() { r.robot.Disabled = robotReq.Disabled - if err := dao.UpdateRobot(r.robot); err != nil { - r.SendInternalServerError(fmt.Errorf("failed to update robot %d: %v", r.robot.ID, err)) + if err := r.ctr.UpdateRobotAccount(r.robot); err != nil { + r.SendInternalServerError(errors.Wrap(err, "robot API: update")) return } @@ -255,13 +211,13 @@ func (r *RobotAPI) Delete() { return } - if err := dao.DeleteRobot(r.robot.ID); err != nil { - r.SendInternalServerError(fmt.Errorf("failed to delete robot %d: %v", r.robot.ID, err)) + if err := r.ctr.DeleteRobotAccount(r.robot.ID); err != nil { + r.SendInternalServerError(errors.Wrap(err, "robot API: delete")) return } } -func validateRobotReq(p *models.Project, robotReq *models.RobotReq) error { +func validateRobotReq(p *models.Project, robotReq *model.RobotCreate) error { if len(robotReq.Access) == 0 { return errors.New("access required") } diff --git a/src/core/api/robot_test.go b/src/core/api/robot_test.go index 6316e2602..c6644ca13 100644 --- a/src/core/api/robot_test.go +++ b/src/core/api/robot_test.go @@ -19,8 +19,8 @@ import ( "net/http" "testing" - "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/pkg/robot/model" ) var ( @@ -53,7 +53,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{}, + bodyJSON: &model.RobotCreate{}, credential: nonSysAdmin, }, code: http.StatusForbidden, @@ -63,7 +63,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test", Description: "test desc", Access: policies, @@ -77,7 +77,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "testIllgel#", Description: "test desc", }, @@ -89,7 +89,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test", Description: "resource not exist", Access: []*rbac.Policy{ @@ -104,7 +104,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test", Description: "action not exist", Access: []*rbac.Policy{ @@ -119,7 +119,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test", Description: "policy not exit", Access: []*rbac.Policy{ @@ -135,7 +135,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test2", Description: "test2 desc", }, @@ -149,7 +149,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.RobotReq{ + bodyJSON: &model.RobotCreate{ Name: "test", Description: "test desc", Access: policies, @@ -306,7 +306,7 @@ func TestRobotAPIPut(t *testing.T) { request: &testingRequest{ method: http.MethodPut, url: fmt.Sprintf("%s/%d", robotPath, 1), - bodyJSON: &models.Robot{ + bodyJSON: &model.Robot{ Disabled: true, }, credential: projAdmin4Robot, diff --git a/src/core/filter/security.go b/src/core/filter/security.go index 785fbba72..b56429252 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -43,6 +43,7 @@ import ( "strings" "github.com/goharbor/harbor/src/pkg/authproxy" + "github.com/goharbor/harbor/src/pkg/robot" ) // ContextValueKey for content value @@ -194,7 +195,8 @@ func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { return false } // Do authn for robot account, as Harbor only stores the token ID, just validate the ID and disable. - robot, err := dao.GetRobotByID(htk.Claims.(*token.RobotClaims).TokenID) + ctr := robot.RobotCtr + robot, err := ctr.GetRobotAccount(htk.Claims.(*token.RobotClaims).TokenID) if err != nil { log.Errorf("failed to get robot %s: %v", robotName, err) return false diff --git a/src/pkg/robot/controller.go b/src/pkg/robot/controller.go new file mode 100644 index 000000000..7f793fdbf --- /dev/null +++ b/src/pkg/robot/controller.go @@ -0,0 +1,115 @@ +package robot + +import ( + "fmt" + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/token" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/core/config" + "github.com/goharbor/harbor/src/pkg/robot/model" + "github.com/pkg/errors" + "time" +) + +var ( + // RobotCtr is a global variable for the default robot account controller implementation + RobotCtr = NewController(NewDefaultRobotAccountManager()) +) + +// Controller to handle the requests related with robot account +type Controller interface { + // GetRobotAccount ... + GetRobotAccount(id int64) (*model.Robot, error) + + // CreateRobotAccount ... + CreateRobotAccount(robotReq *model.RobotCreate) (*model.Robot, error) + + // DeleteRobotAccount ... + DeleteRobotAccount(id int64) error + + // UpdateRobotAccount ... + UpdateRobotAccount(r *model.Robot) error + + // ListRobotAccount ... + ListRobotAccount(pid int64) ([]*model.Robot, error) +} + +// DefaultAPIController ... +type DefaultAPIController struct { + manager Manager +} + +// NewController ... +func NewController(robotMgr Manager) Controller { + return &DefaultAPIController{ + manager: robotMgr, + } +} + +// GetRobotAccount ... +func (d *DefaultAPIController) GetRobotAccount(id int64) (*model.Robot, error) { + return d.manager.GetRobotAccount(id) +} + +// CreateRobotAccount ... +func (d *DefaultAPIController) CreateRobotAccount(robotReq *model.RobotCreate) (*model.Robot, error) { + + var deferDel error + // Token duration in minutes + tokenDuration := time.Duration(config.RobotTokenDuration()) * time.Minute + expiresAt := time.Now().UTC().Add(tokenDuration).Unix() + createdName := common.RobotPrefix + robotReq.Name + + // first to add a robot account, and get its id. + robot := &model.Robot{ + Name: createdName, + Description: robotReq.Description, + ProjectID: robotReq.ProjectID, + ExpiresAt: expiresAt, + } + id, err := d.manager.CreateRobotAccount(robot) + if err != nil { + return nil, err + } + + // generate the token, and return it with response data. + // token is not stored in the database. + jwtToken, err := token.New(id, robotReq.ProjectID, expiresAt, robotReq.Access) + if err != nil { + deferDel = err + return nil, fmt.Errorf("failed to valid parameters to generate token for robot account, %v", err) + } + + rawTk, err := jwtToken.Raw() + if err != nil { + deferDel = err + return nil, fmt.Errorf("failed to sign token for robot account, %v", err) + } + + defer func(deferDel error) { + if deferDel != nil { + if err := d.manager.DeleteRobotAccount(id); err != nil { + log.Error(errors.Wrap(err, fmt.Sprintf("failed to delete the robot account: %d", id))) + } + } + }(deferDel) + + robot.Token = rawTk + robot.ID = id + return robot, nil +} + +// DeleteRobotAccount ... +func (d *DefaultAPIController) DeleteRobotAccount(id int64) error { + return d.manager.DeleteRobotAccount(id) +} + +// UpdateRobotAccount ... +func (d *DefaultAPIController) UpdateRobotAccount(r *model.Robot) error { + return d.manager.UpdateRobotAccount(r) +} + +// ListRobotAccount ... +func (d *DefaultAPIController) ListRobotAccount(pid int64) ([]*model.Robot, error) { + return d.manager.ListRobotAccount(pid) +} diff --git a/src/pkg/robot/controller_test.go b/src/pkg/robot/controller_test.go new file mode 100644 index 000000000..6557942bd --- /dev/null +++ b/src/pkg/robot/controller_test.go @@ -0,0 +1,103 @@ +package robot + +import ( + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/utils/test" + core_cfg "github.com/goharbor/harbor/src/core/config" + "github.com/goharbor/harbor/src/pkg/robot/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 + + robotID int64 +} + +// SetupSuite ... +func (s *ControllerTestSuite) SetupSuite() { + test.InitDatabaseFromEnv() + conf := map[string]interface{}{ + common.RobotTokenDuration: "30", + } + core_cfg.InitWithSettings(conf) + s.t = s.T() + s.assert = assert.New(s.t) + s.require = require.New(s.t) + s.ctr = RobotCtr +} + +func (s *ControllerTestSuite) TestRobotAccount() { + + res := rbac.Resource("/project/1") + + rbacPolicy := &rbac.Policy{ + Resource: res.Subresource(rbac.ResourceRepository), + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + robot1 := &model.RobotCreate{ + Name: "robot1", + Description: "TestCreateRobotAccount", + ProjectID: int64(1), + Access: policies, + } + + robot, err := s.ctr.CreateRobotAccount(robot1) + s.require.Nil(err) + s.require.Equal(robot.ProjectID, int64(1)) + s.require.Equal(robot.Description, "TestCreateRobotAccount") + s.require.NotEmpty(robot.Token) + s.require.Equal(robot.Name, common.RobotPrefix+"robot1") + + robotGet, err := s.ctr.GetRobotAccount(robot.ID) + s.require.Nil(err) + s.require.Equal(robotGet.ProjectID, int64(1)) + s.require.Equal(robotGet.Description, "TestCreateRobotAccount") + + robot.Disabled = true + err = s.ctr.UpdateRobotAccount(robot) + s.require.Nil(err) + s.require.Equal(robot.Disabled, true) + + robot2 := &model.RobotCreate{ + Name: "robot2", + Description: "TestCreateRobotAccount", + ProjectID: int64(1), + Access: policies, + } + r2, _ := s.ctr.CreateRobotAccount(robot2) + s.robotID = r2.ID + + robots, err := s.ctr.ListRobotAccount(int64(1)) + s.require.Nil(err) + s.require.Equal(len(robots), 2) + s.require.Equal(robots[1].Name, common.RobotPrefix+"robot2") + + err = s.ctr.DeleteRobotAccount(robot.ID) + s.require.Nil(err) + + robots, err = s.ctr.ListRobotAccount(int64(1)) + s.require.Equal(len(robots), 1) +} + +// TearDownSuite clears env for test suite +func (s *ControllerTestSuite) TearDownSuite() { + err := s.ctr.DeleteRobotAccount(s.robotID) + require.NoError(s.T(), err, "delete robot") +} + +// TestController ... +func TestController(t *testing.T) { + suite.Run(t, new(ControllerTestSuite)) +} diff --git a/src/pkg/robot/dao/robot.go b/src/pkg/robot/dao/robot.go new file mode 100644 index 000000000..fbfe1509d --- /dev/null +++ b/src/pkg/robot/dao/robot.go @@ -0,0 +1,121 @@ +package dao + +import ( + "fmt" + "github.com/astaxie/beego/orm" + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/robot/model" + "strings" + "time" +) + +// RobotAccountDao defines the interface to access the ImmutableRule data model +type RobotAccountDao interface { + // CreateRobotAccount ... + CreateRobotAccount(robot *model.Robot) (int64, error) + + // UpdateRobotAccount ... + UpdateRobotAccount(robot *model.Robot) error + + // GetRobotAccount ... + GetRobotAccount(id int64) (*model.Robot, error) + + // ListRobotAccounts ... + ListRobotAccounts(query *q.Query) ([]*model.Robot, error) + + // DeleteRobotAccount ... + DeleteRobotAccount(id int64) error +} + +// New creates a default implementation for RobotAccountDao +func New() RobotAccountDao { + return &robotAccountDao{} +} + +type robotAccountDao struct{} + +// CreateRobotAccount ... +func (r *robotAccountDao) CreateRobotAccount(robot *model.Robot) (int64, error) { + now := time.Now() + robot.CreationTime = now + robot.UpdateTime = now + id, err := dao.GetOrmer().Insert(robot) + if err != nil { + if strings.Contains(err.Error(), "duplicate key value violates unique constraint") { + return 0, dao.ErrDupRows + } + return 0, err + } + return id, nil +} + +// GetRobotAccount ... +func (r *robotAccountDao) GetRobotAccount(id int64) (*model.Robot, error) { + robot := &model.Robot{ + ID: id, + } + if err := dao.GetOrmer().Read(robot); err != nil { + if err == orm.ErrNoRows { + return nil, nil + } + return nil, err + } + + return robot, nil +} + +// ListRobotAccounts ... +func (r *robotAccountDao) ListRobotAccounts(query *q.Query) ([]*model.Robot, error) { + o := dao.GetOrmer() + qt := o.QueryTable(new(model.Robot)) + + if query != nil { + if len(query.Keywords) > 0 { + for k, v := range query.Keywords { + qt = qt.Filter(fmt.Sprintf("%s__icontains", k), v) + } + } + + if query.PageNumber > 0 && query.PageSize > 0 { + qt = qt.Limit(query.PageSize, (query.PageNumber-1)*query.PageSize) + } + } + + robots := make([]*model.Robot, 0) + _, err := qt.All(&robots) + return robots, err +} + +// UpdateRobotAccount ... +func (r *robotAccountDao) UpdateRobotAccount(robot *model.Robot) error { + robot.UpdateTime = time.Now() + _, err := dao.GetOrmer().Update(robot) + return err +} + +// DeleteRobotAccount ... +func (r *robotAccountDao) DeleteRobotAccount(id int64) error { + _, err := dao.GetOrmer().QueryTable(&model.Robot{}).Filter("ID", id).Delete() + return err +} + +func getRobotQuerySetter(query *model.RobotQuery) orm.QuerySeter { + qs := dao.GetOrmer().QueryTable(&model.Robot{}) + + if query == nil { + return qs + } + + if len(query.Name) > 0 { + if query.FuzzyMatchName { + qs = qs.Filter("Name__icontains", query.Name) + } else { + qs = qs.Filter("Name", query.Name) + } + } + if query.ProjectID != 0 { + qs = qs.Filter("ProjectID", query.ProjectID) + } + return qs +} diff --git a/src/pkg/robot/dao/robot_test.go b/src/pkg/robot/dao/robot_test.go new file mode 100644 index 000000000..b55d722d4 --- /dev/null +++ b/src/pkg/robot/dao/robot_test.go @@ -0,0 +1,140 @@ +package dao + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/robot/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "testing" +) + +type robotAccountDaoTestSuite struct { + suite.Suite + require *require.Assertions + assert *assert.Assertions + dao RobotAccountDao + id1 int64 + id2 int64 + id3 int64 + id4 int64 +} + +func (t *robotAccountDaoTestSuite) SetupSuite() { + t.require = require.New(t.T()) + t.assert = assert.New(t.T()) + dao.PrepareTestForPostgresSQL() + t.dao = New() +} + +func (t *robotAccountDaoTestSuite) TestCreateRobotAccount() { + robotName := "test1" + robot := &model.Robot{ + Name: robotName, + Description: "test1 description", + ProjectID: 1, + } + id, err := t.dao.CreateRobotAccount(robot) + t.require.Nil(err) + t.id1 = id + t.require.Nil(err) + t.require.NotNil(id) +} + +func (t *robotAccountDaoTestSuite) TestGetRobotAccount() { + robotName := "test2" + robot := &model.Robot{ + Name: robotName, + Description: "test2 description", + ProjectID: 1, + } + + // add + id, err := t.dao.CreateRobotAccount(robot) + t.require.Nil(err) + t.id2 = id + + robot, err = t.dao.GetRobotAccount(id) + t.require.Nil(err) + t.require.Equal(robotName, robot.Name) +} + +func (t *robotAccountDaoTestSuite) TestListRobotAccounts() { + robotName := "test3" + robot := &model.Robot{ + Name: robotName, + Description: "test3 description", + ProjectID: 1, + } + + id, err := t.dao.CreateRobotAccount(robot) + t.require.Nil(err) + t.id3 = id + + keywords := make(map[string]interface{}) + keywords["ProjectID"] = 1 + robots, err := t.dao.ListRobotAccounts(&q.Query{ + Keywords: keywords, + }) + t.require.Nil(err) + t.require.Equal(3, len(robots)) +} + +func (t *robotAccountDaoTestSuite) TestUpdateRobotAccount() { + robotName := "test4" + robot := &model.Robot{ + Name: robotName, + Description: "test4 description", + ProjectID: 1, + } + // add + id, err := t.dao.CreateRobotAccount(robot) + t.require.Nil(err) + t.id4 = id + // Disable + robot.Disabled = true + err = t.dao.UpdateRobotAccount(robot) + t.require.Nil(err) + // Get + robot, err = t.dao.GetRobotAccount(id) + t.require.Nil(err) + t.require.Equal(true, robot.Disabled) +} + +func (t *robotAccountDaoTestSuite) TestDeleteRobotAccount() { + robotName := "test5" + robot := &model.Robot{ + Name: robotName, + Description: "test5 description", + ProjectID: 1, + } + // add + id, err := t.dao.CreateRobotAccount(robot) + t.require.Nil(err) + // Disable + err = t.dao.DeleteRobotAccount(id) + t.require.Nil(err) + // Get + robot, err = t.dao.GetRobotAccount(id) + t.require.Nil(err) +} + +// TearDownSuite clears env for test suite +func (t *robotAccountDaoTestSuite) TearDownSuite() { + err := t.dao.DeleteRobotAccount(t.id1) + require.NoError(t.T(), err, "delete robot 1") + + err = t.dao.DeleteRobotAccount(t.id2) + require.NoError(t.T(), err, "delete robot 2") + + err = t.dao.DeleteRobotAccount(t.id3) + require.NoError(t.T(), err, "delete robot 3") + + err = t.dao.DeleteRobotAccount(t.id4) + require.NoError(t.T(), err, "delete robot 4") +} + +func TestRobotAccountDaoTestSuite(t *testing.T) { + suite.Run(t, &robotAccountDaoTestSuite{}) +} diff --git a/src/pkg/robot/manager.go b/src/pkg/robot/manager.go new file mode 100644 index 000000000..57a86b7e1 --- /dev/null +++ b/src/pkg/robot/manager.go @@ -0,0 +1,71 @@ +package robot + +import ( + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/robot/dao" + "github.com/goharbor/harbor/src/pkg/robot/model" +) + +var ( + // Mgr is a global variable for the default robot account manager implementation + Mgr = NewDefaultRobotAccountManager() +) + +// Manager ... +type Manager interface { + // GetRobotAccount ... + GetRobotAccount(id int64) (*model.Robot, error) + + // CreateRobotAccount ... + CreateRobotAccount(m *model.Robot) (int64, error) + + // DeleteRobotAccount ... + DeleteRobotAccount(id int64) error + + // UpdateRobotAccount ... + UpdateRobotAccount(m *model.Robot) error + + // ListRobotAccount ... + ListRobotAccount(pid int64) ([]*model.Robot, error) +} + +type defaultRobotManager struct { + dao dao.RobotAccountDao +} + +// NewDefaultRobotAccountManager return a new instance of defaultRobotManager +func NewDefaultRobotAccountManager() Manager { + return &defaultRobotManager{ + dao: dao.New(), + } +} + +// GetRobotAccount ... +func (drm *defaultRobotManager) GetRobotAccount(id int64) (*model.Robot, error) { + return drm.dao.GetRobotAccount(id) +} + +// CreateRobotAccount ... +func (drm *defaultRobotManager) CreateRobotAccount(r *model.Robot) (int64, error) { + return drm.dao.CreateRobotAccount(r) +} + +// DeleteRobotAccount ... +func (drm *defaultRobotManager) DeleteRobotAccount(id int64) error { + return drm.dao.DeleteRobotAccount(id) +} + +// UpdateRobotAccount ... +func (drm *defaultRobotManager) UpdateRobotAccount(r *model.Robot) error { + return drm.dao.UpdateRobotAccount(r) +} + +// ListRobotAccount ... +func (drm *defaultRobotManager) ListRobotAccount(pid int64) ([]*model.Robot, error) { + keywords := make(map[string]interface{}) + keywords["ProjectID"] = pid + query := q.Query{ + Keywords: keywords, + } + return drm.dao.ListRobotAccounts(&query) +} diff --git a/src/pkg/robot/manager_test.go b/src/pkg/robot/manager_test.go new file mode 100644 index 000000000..fad225969 --- /dev/null +++ b/src/pkg/robot/manager_test.go @@ -0,0 +1,142 @@ +package robot + +import ( + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/robot/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "os" + "testing" +) + +type mockRobotDao struct { + mock.Mock +} + +func (m *mockRobotDao) CreateRobotAccount(r *model.Robot) (int64, error) { + args := m.Called(r) + return int64(args.Int(0)), args.Error(1) +} + +func (m *mockRobotDao) UpdateRobotAccount(r *model.Robot) error { + args := m.Called(r) + return args.Error(1) +} + +func (m *mockRobotDao) DeleteRobotAccount(id int64) error { + args := m.Called(id) + return args.Error(1) +} + +func (m *mockRobotDao) GetRobotAccount(id int64) (*model.Robot, error) { + args := m.Called(id) + var r *model.Robot + if args.Get(0) != nil { + r = args.Get(0).(*model.Robot) + } + return r, args.Error(1) +} + +func (m *mockRobotDao) ListRobotAccounts(query *q.Query) ([]*model.Robot, error) { + args := m.Called() + var rs []*model.Robot + if args.Get(0) != nil { + rs = args.Get(0).([]*model.Robot) + } + return rs, args.Error(1) +} + +type managerTestingSuite struct { + suite.Suite + t *testing.T + assert *assert.Assertions + require *require.Assertions + mockRobotDao *mockRobotDao +} + +func (m *managerTestingSuite) SetupSuite() { + m.t = m.T() + m.assert = assert.New(m.t) + m.require = require.New(m.t) + + err := os.Setenv("RUN_MODE", "TEST") + m.require.Nil(err) +} + +func (m *managerTestingSuite) TearDownSuite() { + err := os.Unsetenv("RUN_MODE") + m.require.Nil(err) +} + +func (m *managerTestingSuite) SetupTest() { + m.mockRobotDao = &mockRobotDao{} + Mgr = &defaultRobotManager{ + dao: m.mockRobotDao, + } +} + +func TestManagerTestingSuite(t *testing.T) { + suite.Run(t, &managerTestingSuite{}) +} + +func (m *managerTestingSuite) TestCreateRobotAccount() { + m.mockRobotDao.On("CreateRobotAccount", mock.Anything).Return(1, nil) + id, err := Mgr.CreateRobotAccount(&model.Robot{}) + m.mockRobotDao.AssertCalled(m.t, "CreateRobotAccount", mock.Anything) + m.require.Nil(err) + m.assert.Equal(int64(1), id) +} + +func (m *managerTestingSuite) TestUpdateRobotAccount() { + m.mockRobotDao.On("UpdateRobotAccount", mock.Anything).Return(1, nil) + err := Mgr.UpdateRobotAccount(&model.Robot{}) + m.mockRobotDao.AssertCalled(m.t, "UpdateRobotAccount", mock.Anything) + m.require.Nil(err) +} + +func (m *managerTestingSuite) TestDeleteRobotAccount() { + m.mockRobotDao.On("DeleteRobotAccount", mock.Anything).Return(1, nil) + err := Mgr.DeleteRobotAccount(int64(1)) + m.mockRobotDao.AssertCalled(m.t, "DeleteRobotAccount", mock.Anything) + m.require.Nil(err) +} + +func (m *managerTestingSuite) TestGetRobotAccount() { + m.mockRobotDao.On("GetRobotAccount", mock.Anything).Return(&model.Robot{ + ID: 1, + ProjectID: 1, + Disabled: true, + ExpiresAt: 150000, + }, nil) + ir, err := Mgr.GetRobotAccount(1) + m.mockRobotDao.AssertCalled(m.t, "GetRobotAccount", mock.Anything) + m.require.Nil(err) + m.require.NotNil(ir) + m.assert.Equal(int64(1), ir.ID) +} + +func (m *managerTestingSuite) ListRobotAccount() { + m.mockRobotDao.On("ListRobotAccount", mock.Anything).Return([]model.Robot{ + { + ID: 1, + ProjectID: 1, + Disabled: false, + ExpiresAt: 12345, + }, + { + ID: 2, + ProjectID: 1, + Disabled: false, + ExpiresAt: 54321, + }}, nil) + + rs, err := Mgr.ListRobotAccount(int64(1)) + m.mockRobotDao.AssertCalled(m.t, "ListRobotAccount", mock.Anything) + m.require.Nil(err) + m.assert.Equal(len(rs), 2) + m.assert.Equal(rs[0].Disabled, false) + m.assert.Equal(rs[1].ExpiresAt, 54321) + +} diff --git a/src/common/models/robot.go b/src/pkg/robot/model/robot.go similarity index 70% rename from src/common/models/robot.go rename to src/pkg/robot/model/robot.go index 2e64ca8d2..a83a6f13a 100644 --- a/src/common/models/robot.go +++ b/src/pkg/robot/model/robot.go @@ -1,20 +1,7 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package models +package model import ( + "github.com/astaxie/beego/orm" "github.com/astaxie/beego/validation" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils" @@ -24,10 +11,15 @@ import ( // RobotTable is the name of table in DB that holds the robot object const RobotTable = "robot" +func init() { + orm.RegisterModel(&Robot{}) +} + // Robot holds the details of a robot. type Robot struct { ID int64 `orm:"pk;auto;column(id)" json:"id"` Name string `orm:"column(name)" json:"name"` + Token string `orm:"-" json:"token"` Description string `orm:"column(description)" json:"description"` ProjectID int64 `orm:"column(project_id)" json:"project_id"` ExpiresAt int64 `orm:"column(expiresat)" json:"expires_at"` @@ -36,6 +28,11 @@ type Robot struct { UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"` } +// TableName ... +func (r *Robot) TableName() string { + return RobotTable +} + // RobotQuery ... type RobotQuery struct { Name string @@ -45,16 +42,23 @@ type RobotQuery struct { Pagination } -// RobotReq ... -type RobotReq struct { +// RobotCreate ... +type RobotCreate struct { Name string `json:"name"` + ProjectID int64 `json:"pid"` Description string `json:"description"` Disabled bool `json:"disabled"` Access []*rbac.Policy `json:"access"` } +// Pagination ... +type Pagination struct { + Page int64 + Size int64 +} + // Valid ... -func (rq *RobotReq) Valid(v *validation.Validation) { +func (rq *RobotCreate) Valid(v *validation.Validation) { if utils.IsIllegalLength(rq.Name, 1, 255) { v.SetError("name", "robot name with illegal length") } @@ -68,8 +72,3 @@ type RobotRep struct { Name string `json:"name"` Token string `json:"token"` } - -// TableName ... -func (r *Robot) TableName() string { - return RobotTable -}