From 6e8e601c8d4fcbfd91e278d3210827aef46818d4 Mon Sep 17 00:00:00 2001 From: Steven Zou Date: Sat, 12 Oct 2019 16:29:38 +0800 Subject: [PATCH] make robot account with new robot controller Signed-off-by: Steven Zou --- src/pkg/scan/api/scan/base_controller.go | 106 +++++++++-- src/pkg/scan/api/scan/base_controller_test.go | 160 ++++++++++++----- src/pkg/scan/api/scan/dep_manager.go | 164 ------------------ src/pkg/scan/api/scan/dep_manager_test.go | 56 ------ 4 files changed, 211 insertions(+), 275 deletions(-) delete mode 100644 src/pkg/scan/api/scan/dep_manager.go delete mode 100644 src/pkg/scan/api/scan/dep_manager_test.go diff --git a/src/pkg/scan/api/scan/base_controller.go b/src/pkg/scan/api/scan/base_controller.go index e7aad84f5..514eb9ba5 100644 --- a/src/pkg/scan/api/scan/base_controller.go +++ b/src/pkg/scan/api/scan/base_controller.go @@ -16,31 +16,56 @@ package scan import ( "fmt" + "time" - "github.com/goharbor/harbor/src/jobservice/logger" - + "github.com/goharbor/harbor/src/common" + cj "github.com/goharbor/harbor/src/common/job" jm "github.com/goharbor/harbor/src/common/job/models" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/jobservice/logger" + "github.com/goharbor/harbor/src/pkg/robot" + "github.com/goharbor/harbor/src/pkg/robot/model" sca "github.com/goharbor/harbor/src/pkg/scan" sc "github.com/goharbor/harbor/src/pkg/scan/api/scanner" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" "github.com/goharbor/harbor/src/pkg/scan/report" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" + "github.com/google/uuid" "github.com/pkg/errors" ) // DefaultController is a default singleton scan API controller. var DefaultController = NewController() +const ( + configRegistryEndpoint = "registryEndpoint" + configCoreInternalAddr = "coreInternalAddr" +) + +// uuidGenerator is a func template which is for generating UUID. +type uuidGenerator func() (string, error) + +// configGetter is a func template which is used to wrap the config management +// utility methods. +type configGetter func(cfg string) (string, error) + // basicController is default implementation of api.Controller interface type basicController struct { // Manage the scan report records manager report.Manager // Scanner controller sc sc.Controller - // Dep manager for utility functions - dep DepManager + // Robot account controller + rc robot.Controller + // Job service client + jc cj.Client + // UUID generator + uuid uuidGenerator + // Configuration getter func + config configGetter } // NewController news a scan API controller @@ -48,10 +73,32 @@ func NewController() Controller { return &basicController{ // New report manager manager: report.NewManager(), - // Refer the default scanner controller + // Refer to the default scanner controller sc: sc.DefaultController, - // New dep manager - dep: &basicDepManager{}, + // Refer to the default robot account controller + rc: robot.RobotCtr, + // Refer to the default job service client + jc: cj.GlobalClient, + // Generate UUID with uuid lib + uuid: func() (string, error) { + aUUID, err := uuid.NewUUID() + if err != nil { + return "", err + } + + return aUUID.String(), nil + }, + // Get the required configuration options + config: func(cfg string) (string, error) { + switch cfg { + case configRegistryEndpoint: + return config.ExtEndpoint() + case configCoreInternalAddr: + return config.InternalCoreURL(), nil + default: + return "", errors.Errorf("configuration option %s not defined", cfg) + } + }, } } @@ -75,7 +122,7 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error { // Generate a UUID as track ID which groups the report records generated // by the specified registration for the digest with given mime type. - trackID, err := bc.dep.UUID() + trackID, err := bc.uuid() if err != nil { return errors.Wrap(err, "scan controller: scan") } @@ -229,7 +276,7 @@ func (bc *basicController) GetScanLog(uuid string) ([]byte, error) { } // Job log - return bc.dep.GetJobLog(sr.JobID) + return bc.jc.GetJobLog(sr.JobID) } // HandleJobHooks ... @@ -274,15 +321,48 @@ func (bc *basicController) HandleJobHooks(trackID string, change *job.StatusChan return bc.manager.UpdateStatus(trackID, change.Status, change.Metadata.Revision) } +// makeRobotAccount creates a robot account based on the arguments for scanning. +func (bc *basicController) makeRobotAccount(pid int64, repository string, ttl int64) (string, error) { + // Use uuid as name to avoid duplicated entries. + UUID, err := bc.uuid() + if err != nil { + return "", errors.Wrap(err, "scan controller: make robot account") + } + + expireAt := time.Now().UTC().Add(time.Duration(ttl) * time.Second).Unix() + + logger.Warningf("repository %s and expire time %d are not supported by robot controller", repository, expireAt) + + resource := fmt.Sprintf("/project/%d/repository", pid) + access := []*rbac.Policy{{ + Resource: rbac.Resource(resource), + Action: "pull", + }} + + account := &model.RobotCreate{ + Name: fmt.Sprintf("%s%s", common.RobotPrefix, UUID), + Description: "for scan", + ProjectID: pid, + Access: access, + } + + rb, err := bc.rc.CreateRobotAccount(account) + if err != nil { + return "", errors.Wrap(err, "scan controller: make robot account") + } + + return rb.Token, nil +} + // launchScanJob launches a job to run scan func (bc *basicController) launchScanJob(trackID string, artifact *v1.Artifact, registration *scanner.Registration, mimes []string) (jobID string, err error) { - externalURL, err := bc.dep.GetRegistryEndpoint() + externalURL, err := bc.config(configRegistryEndpoint) if err != nil { return "", errors.Wrap(err, "scan controller: launch scan job") } // Make a robot account with 30 minutes - robotAccount, err := bc.dep.MakeRobotAccount(artifact.NamespaceID, 1800) + robotAccount, err := bc.makeRobotAccount(artifact.NamespaceID, artifact.Repository, 1800) if err != nil { return "", errors.Wrap(err, "scan controller: launch scan job") } @@ -312,7 +392,7 @@ func (bc *basicController) launchScanJob(trackID string, artifact *v1.Artifact, params[sca.JobParameterMimes] = mimes // Launch job - callbackURL, err := bc.dep.GetInternalCoreAddr() + callbackURL, err := bc.config(configCoreInternalAddr) if err != nil { return "", errors.Wrap(err, "launch scan job") } @@ -327,5 +407,5 @@ func (bc *basicController) launchScanJob(trackID string, artifact *v1.Artifact, StatusHook: hookURL, } - return bc.dep.SubmitJob(j) + return bc.jc.SubmitJob(j) } diff --git a/src/pkg/scan/api/scan/base_controller_test.go b/src/pkg/scan/api/scan/base_controller_test.go index 4135dff56..932793f10 100644 --- a/src/pkg/scan/api/scan/base_controller_test.go +++ b/src/pkg/scan/api/scan/base_controller_test.go @@ -20,7 +20,12 @@ import ( "testing" "time" - "github.com/goharbor/harbor/src/common/job/models" + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/rbac" + + "github.com/goharbor/harbor/src/pkg/robot/model" + + cjm "github.com/goharbor/harbor/src/common/job/models" jm "github.com/goharbor/harbor/src/common/job/models" "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/pkg/q" @@ -151,12 +156,28 @@ func (suite *ControllerTestSuite) SetupSuite() { mgr.On("UpdateReportData", "rp-uuid-001", suite.rawReport, (int64)(10000)).Return(nil) mgr.On("UpdateStatus", "the-uuid-123", "Success", (int64)(10000)).Return(nil) - dep := &MockDepManager{} - dep.On("UUID").Return("the-uuid-123", nil) - dep.On("GetRegistryEndpoint").Return("https://core.com", nil) - dep.On("GetInternalCoreAddr").Return("http://core:8080", nil) - dep.On("MakeRobotAccount", suite.artifact.NamespaceID, (int64)(1800)).Return("robot-account", nil) - dep.On("GetJobLog", "the-job-id").Return([]byte("job log"), nil) + rc := &MockRobotController{} + + resource := fmt.Sprintf("/project/%d/repository", suite.artifact.NamespaceID) + access := []*rbac.Policy{{ + Resource: rbac.Resource(resource), + Action: "pull", + }} + + rname := fmt.Sprintf("%s%s", common.RobotPrefix, "the-uuid-123") + account := &model.RobotCreate{ + Name: rname, + Description: "for scan", + ProjectID: suite.artifact.NamespaceID, + Access: access, + } + rc.On("CreateRobotAccount", account).Return(&model.Robot{ + ID: 1, + Name: rname, + Token: "robot-account", + Description: "for scan", + ProjectID: suite.artifact.NamespaceID, + }, nil) // Set job parameters req := &v1.ScanRequest{ @@ -173,6 +194,7 @@ func (suite *ControllerTestSuite) SetupSuite() { regJSON, err := suite.registration.ToJSON() require.NoError(suite.T(), err) + jc := &MockJobServiceClient{} params := make(map[string]interface{}) params[sca.JobParamRegistration] = regJSON params[sca.JobParameterRequest] = rJSON @@ -186,12 +208,27 @@ func (suite *ControllerTestSuite) SetupSuite() { Parameters: params, StatusHook: fmt.Sprintf("%s/service/notifications/jobs/scan/%s", "http://core:8080", "the-uuid-123"), } - dep.On("SubmitJob", j).Return("the-job-id", nil) + jc.On("SubmitJob", j).Return("the-job-id", nil) + jc.On("GetJobLog", "the-job-id").Return([]byte("job log"), nil) suite.c = &basicController{ manager: mgr, sc: sc, - dep: dep, + jc: jc, + rc: rc, + uuid: func() (string, error) { + return "the-uuid-123", nil + }, + config: func(cfg string) (string, error) { + switch cfg { + case configRegistryEndpoint: + return "https://core.com", nil + case configCoreInternalAddr: + return "http://core:8080", nil + } + + return "", nil + }, } } @@ -412,50 +449,89 @@ func (msc *MockScannerController) GetMetadata(registrationUUID string) (*v1.Scan return args.Get(0).(*v1.ScannerAdapterMetadata), args.Error(1) } -// MockDepManager ... -type MockDepManager struct { +// MockJobServiceClient ... +type MockJobServiceClient struct { mock.Mock } -// UUID ... -func (mdm *MockDepManager) UUID() (string, error) { - args := mdm.Called() - - return args.String(0), args.Error(1) -} - -func (mdm *MockDepManager) SubmitJob(jobData *models.JobData) (string, error) { - args := mdm.Called(jobData) - - return args.String(0), args.Error(1) -} - -func (mdm *MockDepManager) GetRegistryEndpoint() (string, error) { - args := mdm.Called() - - return args.String(0), args.Error(1) -} - -func (mdm *MockDepManager) GetInternalCoreAddr() (string, error) { - args := mdm.Called() - - return args.String(0), args.Error(1) -} - -// MakeRobotAccount ... -func (mdm *MockDepManager) MakeRobotAccount(pid int64, ttl int64) (string, error) { - args := mdm.Called(pid, ttl) +// SubmitJob ... +func (mjc *MockJobServiceClient) SubmitJob(jData *cjm.JobData) (string, error) { + args := mjc.Called(jData) return args.String(0), args.Error(1) } // GetJobLog ... -func (mdm *MockDepManager) GetJobLog(uuid string) ([]byte, error) { - args := mdm.Called(uuid) - +func (mjc *MockJobServiceClient) GetJobLog(uuid string) ([]byte, error) { + args := mjc.Called(uuid) if args.Get(0) == nil { return nil, args.Error(1) } return args.Get(0).([]byte), args.Error(1) } + +// PostAction ... +func (mjc *MockJobServiceClient) PostAction(uuid, action string) error { + args := mjc.Called(uuid, action) + + return args.Error(0) +} + +func (mjc *MockJobServiceClient) GetExecutions(uuid string) ([]job.Stats, error) { + args := mjc.Called(uuid) + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).([]job.Stats), args.Error(1) +} + +// MockRobotController ... +type MockRobotController struct { + mock.Mock +} + +// GetRobotAccount ... +func (mrc *MockRobotController) GetRobotAccount(id int64) (*model.Robot, error) { + args := mrc.Called(id) + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*model.Robot), args.Error(1) +} + +// CreateRobotAccount ... +func (mrc *MockRobotController) CreateRobotAccount(robotReq *model.RobotCreate) (*model.Robot, error) { + args := mrc.Called(robotReq) + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*model.Robot), args.Error(1) +} + +// DeleteRobotAccount ... +func (mrc *MockRobotController) DeleteRobotAccount(id int64) error { + args := mrc.Called(id) + + return args.Error(0) +} + +// UpdateRobotAccount ... +func (mrc *MockRobotController) UpdateRobotAccount(r *model.Robot) error { + args := mrc.Called(r) + + return args.Error(0) +} + +// ListRobotAccount ... +func (mrc *MockRobotController) ListRobotAccount(pid int64) ([]*model.Robot, error) { + args := mrc.Called(pid) + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).([]*model.Robot), args.Error(1) +} diff --git a/src/pkg/scan/api/scan/dep_manager.go b/src/pkg/scan/api/scan/dep_manager.go deleted file mode 100644 index 7f836c0a8..000000000 --- a/src/pkg/scan/api/scan/dep_manager.go +++ /dev/null @@ -1,164 +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 scan - -import ( - "encoding/base64" - "fmt" - "time" - - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/job" - "github.com/goharbor/harbor/src/common/job/models" - cmo "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/common/token" - "github.com/goharbor/harbor/src/core/config" - "github.com/google/uuid" - "github.com/pkg/errors" -) - -// DepManager provides dependant functions with an interface. -type DepManager interface { - // Generate a UUID - // - // Returns: - // string : the uuid string - // error : non nil error if any errors occurred - UUID() (string, error) - - // Submit a job - // - // Arguments: - // jobData models.JobData : job data model - // - // Returns: - // string : the uuid of the job - // error : non nil error if any errors occurred - SubmitJob(jobData *models.JobData) (string, error) - - // Get the endpoint of the registry - // - // Returns: - // string : the uuid string - // error : non nil error if any errors occurred - GetRegistryEndpoint() (string, error) - - // Get the internal address of the core - // - // Returns: - // string : the uuid string - // error : non nil error if any errors occurred - GetInternalCoreAddr() (string, error) - - // Make a robot account - // - // Arguments: - // pid int64 : id of the project - // ttl int64 : expire time of the robot account - // - // Returns: - // string : the token encoded string - // error : non nil error if any errors occurred - MakeRobotAccount(pid int64, ttl int64) (string, error) - - // Get the job log - // - // Arguments: - // uuid string : the job uuid - // - // Returns: - // []byte : the log text stream - // error : non nil error if any errors occurred - GetJobLog(uuid string) ([]byte, error) -} - -// basicDepManager is the default implementation of dep manager -type basicDepManager struct{} - -// UUID ... -func (bdm *basicDepManager) UUID() (string, error) { - aUUID, err := uuid.NewUUID() - if err != nil { - return "", err - } - - return aUUID.String(), nil -} - -// SubmitJob ... -func (bdm *basicDepManager) SubmitJob(jobData *models.JobData) (string, error) { - return job.GlobalClient.SubmitJob(jobData) -} - -// GetJobLog ... -func (bdm *basicDepManager) GetJobLog(uuid string) ([]byte, error) { - return job.GlobalClient.GetJobLog(uuid) -} - -// GetRegistryEndpoint ... -func (bdm *basicDepManager) GetRegistryEndpoint() (string, error) { - return config.ExtEndpoint() -} - -// GetInternalCoreAddr ... -func (bdm *basicDepManager) GetInternalCoreAddr() (string, error) { - return config.InternalCoreURL(), nil -} - -// MakeRobotAccount ... -func (bdm *basicDepManager) MakeRobotAccount(pid int64, ttl int64) (string, error) { - // Use uuid as name to avoid duplicated entries. - UUID, err := uuid.NewUUID() - if err != nil { - return "", errors.Wrap(err, "basic dep manager: make robot account") - } - createdName := fmt.Sprintf("%s%s", common.RobotPrefix, UUID.String()) - - expireAt := time.Now().UTC().Add(time.Duration(ttl) * time.Second).Unix() - // First to add a robot account, and get its id. - robot := cmo.Robot{ - Name: createdName, - ProjectID: pid, - ExpiresAt: expireAt, - } - id, err := dao.AddRobot(&robot) - if err != nil { - return "", errors.Wrap(err, "basic dep manager: make robot account") - } - - resource := fmt.Sprintf("/project/%d/repository", pid) - access := []*rbac.Policy{{ - Resource: rbac.Resource(resource), - Action: "pull", - }} - - // Generate the token, token is not stored in the database. - jwtToken, err := token.New(id, pid, expireAt, access) - if err != nil { - return "", errors.Wrap(err, "basic dep manager: make robot account") - } - - rawToken, err := jwtToken.Raw() - if err != nil { - return "", errors.Wrap(err, "basic dep manager: make robot account") - } - - basic := fmt.Sprintf("%s:%s", createdName, rawToken) - encoded := base64.StdEncoding.EncodeToString([]byte(basic)) - - return fmt.Sprintf("Basic %s", encoded), nil -} diff --git a/src/pkg/scan/api/scan/dep_manager_test.go b/src/pkg/scan/api/scan/dep_manager_test.go deleted file mode 100644 index 5e6de3c57..000000000 --- a/src/pkg/scan/api/scan/dep_manager_test.go +++ /dev/null @@ -1,56 +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 scan - -import ( - "testing" - - "github.com/goharbor/harbor/src/common/dao" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" -) - -// DepManagerTestSuite is a test suite for dep manager. -type DepManagerTestSuite struct { - suite.Suite - - m DepManager -} - -// TestDepManager is the entry point of DepManagerTestSuite. -func TestDepManager(t *testing.T) { - suite.Run(t, new(DepManagerTestSuite)) -} - -// SetupSuite ... -func (suite *DepManagerTestSuite) SetupSuite() { - suite.m = &basicDepManager{} - dao.PrepareTestForPostgresSQL() -} - -// TestDepManagerUUID ... -func (suite *DepManagerTestSuite) TestDepManagerUUID() { - theUUID, err := suite.m.UUID() - require.NoError(suite.T(), err) - assert.NotEmpty(suite.T(), theUUID) -} - -// TestDepManagerMakeRobotAccount ... -func (suite *DepManagerTestSuite) TestDepManagerMakeRobotAccount() { - tk, err := suite.m.MakeRobotAccount(1, 1800) - require.NoError(suite.T(), err) - assert.NotEmpty(suite.T(), tk) -}