From ebc5d2482b275631b47161af3dbc6fffba82a1e8 Mon Sep 17 00:00:00 2001 From: Steven Zou Date: Mon, 4 Nov 2019 17:34:42 +0800 Subject: [PATCH] do improvements to the scan all job - update scan all job to avoid sending too many HTTP requets - update scan controller to support scan options - update the db schema of the scan report to introduce requester - introduce scan all metrics to report the overall progress of scan all job - fix the status updating bug in scan report - enhance the admin job status updats - add duplicate checking before triggering generic admin job - update the db scheme of admin job fix #9705 fix #9722 fix #9670 Signed-off-by: Steven Zou --- .../postgresql/0015_1.10.0_schema.up.sql | 5 + src/common/dao/admin_job.go | 36 +++- src/common/dao/admin_job_test.go | 118 ++++++++++--- src/common/dao/dao_test.go | 4 +- src/common/job/models/models.go | 1 + src/common/models/adminjob.go | 2 + src/core/api/admin_job.go | 93 ++++++++++ src/core/api/models/admin_job.go | 8 + src/core/api/scan_all.go | 41 ++++- src/core/api/scan_test.go | 14 +- src/core/api/scanners.go | 2 +- .../notification/scan_image_handler_test.go | 13 +- src/core/router.go | 5 + .../service/notifications/admin/handler.go | 20 ++- src/jobservice/job/impl/scan/all.go | 141 --------------- src/jobservice/job/status.go | 6 + src/jobservice/runtime/bootstrap.go | 4 +- src/pkg/scan/all/checkin.go | 47 +++++ src/pkg/scan/all/job.go | 163 ++++++++++++++++++ src/pkg/scan/all/stats.go | 34 ++++ src/pkg/scan/api/scan/all_handler.go | 42 +++++ src/pkg/scan/api/scan/base_controller.go | 44 ++++- src/pkg/scan/api/scan/base_controller_test.go | 12 ++ src/pkg/scan/api/scan/controller.go | 14 +- src/pkg/scan/api/scan/options.go | 38 ++++ src/pkg/scan/dao/scan/model.go | 1 + src/pkg/scan/dao/scan/report.go | 41 ++++- src/pkg/scan/dao/scan/report_test.go | 114 +++++++++++- src/pkg/scan/dao/scanner/registration.go | 3 + src/pkg/scan/report/base_manager.go | 48 ++++++ src/pkg/scan/report/base_manager_test.go | 35 ++++ src/pkg/scan/report/manager.go | 15 +- 32 files changed, 964 insertions(+), 200 deletions(-) delete mode 100644 src/jobservice/job/impl/scan/all.go create mode 100644 src/pkg/scan/all/checkin.go create mode 100644 src/pkg/scan/all/job.go create mode 100644 src/pkg/scan/all/stats.go create mode 100644 src/pkg/scan/api/scan/all_handler.go create mode 100644 src/pkg/scan/api/scan/options.go 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 61e83a350..eb869492d 100644 --- a/make/migrations/postgresql/0015_1.10.0_schema.up.sql +++ b/make/migrations/postgresql/0015_1.10.0_schema.up.sql @@ -27,6 +27,7 @@ CREATE TABLE scan_report mime_type VARCHAR(256) NOT NULL, job_id VARCHAR(64), track_id VARCHAR(64), + requester VARCHAR(64), status VARCHAR(1024) NOT NULL, status_code INTEGER DEFAULT 0, status_rev BIGINT DEFAULT 0, @@ -63,3 +64,7 @@ DROP TABLE IF EXISTS clair_vuln_timestamp; /* Add limited guest role */ INSERT INTO role (role_code, name) VALUES ('LRS', 'limitedGuest'); + +/* Add revision and status code columns for admin job table */ +ALTER TABLE admin_job ADD COLUMN revision BIGINT DEFAULT 0; +ALTER TABLE admin_job ADD COLUMN status_code INTEGER DEFAULT 0; \ No newline at end of file diff --git a/src/common/dao/admin_job.go b/src/common/dao/admin_job.go index 04b1d1994..34efeec36 100644 --- a/src/common/dao/admin_job.go +++ b/src/common/dao/admin_job.go @@ -59,17 +59,35 @@ func DeleteAdminJob(id int64) error { } // UpdateAdminJobStatus ... -func UpdateAdminJobStatus(id int64, status string) error { +func UpdateAdminJobStatus(id int64, status string, statusCode uint16, revision int64) error { o := GetOrmer() - j := models.AdminJob{ - ID: id, - Status: status, - UpdateTime: time.Now(), - } - n, err := o.Update(&j, "Status", "UpdateTime") + qt := o.QueryTable(&models.AdminJob{}) + + // The generated sql statement example:{ + // + // UPDATE "admin_job" SET "update_time" = $1, "status" = $2, "status_code" = $3, "revision" = $4 + // WHERE "id" IN ( SELECT T0."id" FROM "admin_job" T0 WHERE + // ( T0."revision" = $5 AND T0."status_code" < $6 ) OR ( T0."revision" < $7 ) + // AND T0."id" = $8 ) + // + // } + cond := orm.NewCondition() + c1 := cond.And("revision", revision).And("status_code__lt", statusCode) + c2 := cond.And("revision__lt", revision) + c := cond.AndCond(c1).OrCond(c2) + + data := make(orm.Params) + data["status"] = status + data["status_code"] = statusCode + data["revision"] = revision + data["update_time"] = time.Now() + + n, err := qt.SetCond(c).Filter("id", id).Update(data) + if n == 0 { log.Warningf("no records are updated when updating admin job %d", id) } + return err } @@ -92,7 +110,7 @@ func GetTop10AdminJobsOfName(name string) ([]*models.AdminJob, error) { o := GetOrmer() jobs := []*models.AdminJob{} n, err := o.Raw(`select * from admin_job - where deleted = false and job_name = ? order by update_time desc limit 10`, name).QueryRows(&jobs) + where deleted = false and job_name = ? order by id desc limit 10`, name).QueryRows(&jobs) if err != nil { return nil, err } @@ -137,6 +155,6 @@ func adminQueryConditions(query *models.AdminJobQuery) orm.QuerySeter { qs = qs.Filter("UUID", query.UUID) } qs = qs.Filter("Deleted", false) - return qs + return qs.OrderBy("-ID") } diff --git a/src/common/dao/admin_job_test.go b/src/common/dao/admin_job_test.go index 97ccefb1b..196122843 100644 --- a/src/common/dao/admin_job_test.go +++ b/src/common/dao/admin_job_test.go @@ -15,14 +15,29 @@ package dao import ( + "fmt" "testing" "github.com/goharbor/harbor/src/common/models" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) -func TestAddAdminJob(t *testing.T) { +// AdminJobSuite is a test suite for testing admin job +type AdminJobSuite struct { + suite.Suite + + job0 *models.AdminJob + ids []int64 +} + +// TestAdminJob is the entry point of AdminJobSuite +func TestAdminJob(t *testing.T) { + suite.Run(t, &AdminJobSuite{}) +} + +// SetupSuite prepares testing env for the suite +func (suite *AdminJobSuite) SetupSuite() { job := &models.AdminJob{ Name: "job", Kind: "jobKind", @@ -33,43 +48,96 @@ func TestAddAdminJob(t *testing.T) { Kind: "testKind", } + suite.ids = make([]int64, 0) + // add id, err := AddAdminJob(job0) - require.Nil(t, err) + require.NoError(suite.T(), err) job0.ID = id + suite.job0 = job0 + suite.ids = append(suite.ids, id) + id1, err := AddAdminJob(job) + require.NoError(suite.T(), err) + suite.ids = append(suite.ids, id1) +} + +// TearDownSuite cleans testing env +func (suite *AdminJobSuite) TearDownSuite() { + for _, id := range suite.ids { + err := DeleteAdminJob(id) + suite.NoError(err, fmt.Sprintf("clear admin job: %d", id)) + } +} + +// TestAdminJobBase ... +func (suite *AdminJobSuite) TestAdminJobBase() { // get - job1, err := GetAdminJob(id) - require.Nil(t, err) - assert.Equal(t, job1.ID, job0.ID) - assert.Equal(t, job1.Name, job0.Name) - - // update status - err = UpdateAdminJobStatus(id, "testStatus") - require.Nil(t, err) - job2, err := GetAdminJob(id) - assert.Equal(t, job2.Status, "testStatus") + job1, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + suite.Equal(job1.ID, suite.job0.ID) + suite.Equal(job1.Name, suite.job0.Name) // set uuid - err = SetAdminJobUUID(id, "f5ef34f4cb3588d663176132") - require.Nil(t, err) - job3, err := GetAdminJob(id) - require.Nil(t, err) - assert.Equal(t, job3.UUID, "f5ef34f4cb3588d663176132") + err = SetAdminJobUUID(suite.job0.ID, "f5ef34f4cb3588d663176132") + require.Nil(suite.T(), err) + job3, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + suite.Equal(job3.UUID, "f5ef34f4cb3588d663176132") // get admin jobs - _, err = AddAdminJob(job) - require.Nil(t, err) query := &models.AdminJobQuery{ Name: "job", } jobs, err := GetAdminJobs(query) - assert.Equal(t, len(jobs), 1) + suite.Equal(len(jobs), 1) // get top 10 - _, err = AddAdminJob(job) - require.Nil(t, err) - jobs, _ = GetTop10AdminJobsOfName("job") - assert.Equal(t, len(jobs), 2) + suite.Equal(len(jobs), 1) +} + +// TestAdminJobUpdateStatus ... +func (suite *AdminJobSuite) TestAdminJobUpdateStatus() { + // update status + err := UpdateAdminJobStatus(suite.job0.ID, "testStatus", 1, 10000) + require.Nil(suite.T(), err) + + job2, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + suite.Equal(job2.Status, "testStatus") + + // Update status with same rev + err = UpdateAdminJobStatus(suite.job0.ID, "testStatus3", 3, 10000) + require.Nil(suite.T(), err) + + job3, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + suite.Equal(job3.Status, "testStatus3") + + // Update status with same rev, previous status + err = UpdateAdminJobStatus(suite.job0.ID, "testStatus2", 2, 10000) + require.Nil(suite.T(), err) + + job4, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + // No status change + suite.Equal(job4.Status, "testStatus3") + + // Update status with previous rev + err = UpdateAdminJobStatus(suite.job0.ID, "testStatus4", 4, 9999) + require.Nil(suite.T(), err) + + job5, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + // No status change + suite.Equal(job5.Status, "testStatus3") + + // Update status with latest rev + err = UpdateAdminJobStatus(suite.job0.ID, "testStatus", 1, 10001) + require.Nil(suite.T(), err) + + job6, err := GetAdminJob(suite.job0.ID) + require.Nil(suite.T(), err) + suite.Equal(job6.Status, "testStatus") } diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index 42ff990ca..b0f216059 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -159,8 +159,8 @@ func testForAll(m *testing.M) int { func clearAll() { tables := []string{"project_member", "project_metadata", "access_log", "repository", "replication_policy", - "registry", "replication_execution", "replication_task", "img_scan_job", - "replication_schedule_job", "img_scan_overview", "clair_vuln_timestamp", "project", "harbor_user"} + "registry", "replication_execution", "replication_task", + "replication_schedule_job", "project", "harbor_user"} for _, t := range tables { if err := ClearTable(t); err != nil { log.Errorf("Failed to clear table: %s,error: %v", t, err) diff --git a/src/common/job/models/models.go b/src/common/job/models/models.go index 4cf21c716..8f92cdd21 100644 --- a/src/common/job/models/models.go +++ b/src/common/job/models/models.go @@ -50,6 +50,7 @@ type StatsInfo struct { UpstreamJobID string `json:"upstream_job_id,omitempty"` // Ref the upstream job if existing NumericPID int64 `json:"numeric_policy_id,omitempty"` // The numeric policy ID of the periodic job Parameters Parameters `json:"parameters,omitempty"` + Revision int64 `json:"revision,omitempty"` // For differentiating the each retry of the same job } // JobPoolStats represents the healthy and status of all the running worker pools. diff --git a/src/common/models/adminjob.go b/src/common/models/adminjob.go index ca31a9c18..104f871ac 100644 --- a/src/common/models/adminjob.go +++ b/src/common/models/adminjob.go @@ -32,6 +32,8 @@ type AdminJob struct { Cron string `orm:"column(cron_str)" json:"cron_str"` Status string `orm:"column(status)" json:"job_status"` UUID string `orm:"column(job_uuid)" json:"-"` + Revision int64 `orm:"column(revision)" json:"-"` + StatusCode uint16 `orm:"column(status_code)" json:"-"` Deleted bool `orm:"column(deleted)" json:"deleted"` CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"` diff --git a/src/core/api/admin_job.go b/src/core/api/admin_job.go index 71a486c50..8856566ae 100644 --- a/src/core/api/admin_job.go +++ b/src/core/api/admin_job.go @@ -16,8 +16,10 @@ package api import ( "fmt" + "math" "net/http" "strconv" + "time" "github.com/goharbor/harbor/src/common/dao" common_http "github.com/goharbor/harbor/src/common/http" @@ -26,6 +28,7 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/api/models" utils_core "github.com/goharbor/harbor/src/core/utils" + "github.com/goharbor/harbor/src/pkg/scan/api/scan" "github.com/pkg/errors" ) @@ -241,6 +244,54 @@ func (aj *AJAPI) submit(ajr *models.AdminJobReq) { aj.SendPreconditionFailedError(errors.New("fail to set schedule for admin job as always had one, please delete it firstly then to re-schedule")) return } + } else { + // So far, it should be a generic job for the manually trigger case. + // Only needs to care the 1st generic job. + // Check if there are still ongoing scan jobs triggered by the previous admin job. + // TODO: REPLACE WITH TASK MANAGER METHODS IN FUTURE + query := &common_models.AdminJobQuery{ + Name: ajr.Name, + Kind: common_job.JobKindGeneric, + } + query.Size = 1 + query.Page = 1 + + ajbs, err := dao.GetAdminJobs(query) + if err != nil { + aj.SendInternalServerError(errors.Wrap(err, "AJAPI")) + return + } + + if len(ajbs) > 0 { + jb := ajbs[0] + + // With a reasonable timeout duration + if jb.UpdateTime.Add(2 * time.Hour).After(time.Now()) { + if isOnGoing(jb.Status) { + err := errors.Errorf("reject job submitting: job %s with ID %d is %s", jb.Name, jb.ID, jb.Status) + aj.SendInternalServerError(errors.Wrap(err, "submit : AJAPI")) + return + } + + // For scan all job, check more + if jb.Name == common_job.ImageScanAllJob { + // Get the overall stats with the ID of the previous job + stats, err := scan.DefaultController.GetStats(fmt.Sprintf("%d", jb.ID)) + if err != nil { + aj.SendInternalServerError(errors.Wrap(err, "submit : AJAPI")) + return + } + + if stats.Total != stats.Completed { + // Not all scan processes are completed + // In case status is hang, add outdated timeout + err := errors.Errorf("scan processes started by %s job with ID %d is in progress: %s", jb.Name, jb.ID, progress(stats.Completed, stats.Total)) + aj.SendPreconditionFailedError(errors.Wrap(err, "submit : AJAPI")) + return + } + } + } + } } id, err := dao.AddAdminJob(&common_models.AdminJob{ @@ -271,6 +322,29 @@ func (aj *AJAPI) submit(ajr *models.AdminJobReq) { } } +func (aj *AJAPI) getLatestScanAllJobIDByKind(kind string) (int64, error) { + query := &common_models.AdminJobQuery{ + Name: common_job.ImageScanAllJob, + Kind: kind, + } + query.Size = 1 + query.Page = 1 + + jbs, err := dao.GetAdminJobs(query) + + if err != nil { + return 0, err + } + + if len(jbs) == 0 { + // Not exist + return 0, nil + } + + // Return the latest one (with biggest ID) + return jbs[0].ID, nil +} + func convertToAdminJobRep(job *common_models.AdminJob) (models.AdminJobRep, error) { if job == nil { return models.AdminJobRep{}, nil @@ -294,3 +368,22 @@ func convertToAdminJobRep(job *common_models.AdminJob) (models.AdminJobRep, erro } return AdminJobRep, nil } + +func progress(completed, total uint) string { + if total == 0 { + return fmt.Sprintf("0%s", "%") + } + + v := float64(completed) + vv := float64(total) + + p := (int)(math.Round((v / vv) * 100)) + + return fmt.Sprintf("%d%s", p, "%") +} + +func isOnGoing(status string) bool { + return status == common_models.JobRunning || + status == common_models.JobScheduled || + status == common_models.JobPending +} diff --git a/src/core/api/models/admin_job.go b/src/core/api/models/admin_job.go index 630a0598b..a055494c1 100644 --- a/src/core/api/models/admin_job.go +++ b/src/core/api/models/admin_job.go @@ -113,6 +113,14 @@ func (ar *AdminJobReq) ToJob() *models.JobData { StatusHook: fmt.Sprintf("%s/service/notifications/jobs/adminjob/%d", config.InternalCoreURL(), ar.ID), } + + // Append admin job ID as job parameter + if jobData.Parameters == nil { + jobData.Parameters = make(models.Parameters) + } + // As string + jobData.Parameters["admin_job_id"] = fmt.Sprintf("%d", ar.ID) + return jobData } diff --git a/src/core/api/scan_all.go b/src/core/api/scan_all.go index ef95463c9..01fc84b12 100644 --- a/src/core/api/scan_all.go +++ b/src/core/api/scan_all.go @@ -1,13 +1,15 @@ package api import ( + "fmt" "net/http" "strconv" - "github.com/goharbor/harbor/src/pkg/q" - common_job "github.com/goharbor/harbor/src/common/job" "github.com/goharbor/harbor/src/core/api/models" + "github.com/goharbor/harbor/src/pkg/q" + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/goharbor/harbor/src/pkg/scan/api/scan" "github.com/goharbor/harbor/src/pkg/scan/api/scanner" "github.com/pkg/errors" ) @@ -97,6 +99,41 @@ func (sc *ScanAllAPI) List() { sc.list(common_job.ImageScanAllJob) } +// GetScheduleMetrics returns the progress metrics for the latest scheduled scan all job +func (sc *ScanAllAPI) GetScheduleMetrics() { + sc.getMetrics(common_job.JobKindPeriodic) +} + +// GetScanAllMetrics returns the progress metrics for the latest manually triggered scan all job +func (sc *ScanAllAPI) GetScanAllMetrics() { + sc.getMetrics(common_job.JobKindGeneric) +} + +func (sc *ScanAllAPI) getMetrics(kind string) { + id, err := sc.getLatestScanAllJobIDByKind(kind) + if err != nil { + sc.SendInternalServerError(errors.Wrap(err, "get metrics: scan all API")) + return + } + + var sts *all.Stats + if id > 0 { + sts, err = scan.DefaultController.GetStats(fmt.Sprintf("%d", id)) + if err != nil { + sc.SendInternalServerError(errors.Wrap(err, "get metrics: scan all API")) + return + } + } + + // Return empty + if sts == nil { + sts = &all.Stats{} + } + + sc.Data["json"] = sts + sc.ServeJSON() +} + func isScanEnabled() (bool, error) { kws := make(map[string]interface{}) kws["is_default"] = true diff --git a/src/core/api/scan_test.go b/src/core/api/scan_test.go index 8e08d8f36..0c40a5698 100644 --- a/src/core/api/scan_test.go +++ b/src/core/api/scan_test.go @@ -19,6 +19,8 @@ import ( "net/http" "testing" + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/pkg/scan/api/scan" dscan "github.com/goharbor/harbor/src/pkg/scan/dao/scan" @@ -170,7 +172,7 @@ type MockScanAPIController struct { } // Scan ... -func (msc *MockScanAPIController) Scan(artifact *v1.Artifact) error { +func (msc *MockScanAPIController) Scan(artifact *v1.Artifact, option ...scan.Option) error { args := msc.Called(artifact) return args.Error(0) @@ -215,3 +217,13 @@ func (msc *MockScanAPIController) HandleJobHooks(trackID string, change *job.Sta func (msc *MockScanAPIController) DeleteReports(digests ...string) error { return nil } + +func (msc *MockScanAPIController) GetStats(requester string) (*all.Stats, error) { + args := msc.Called(requester) + + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*all.Stats), args.Error(1) +} diff --git a/src/core/api/scanners.go b/src/core/api/scanners.go index 237e40f8b..ce88567a5 100644 --- a/src/core/api/scanners.go +++ b/src/core/api/scanners.go @@ -221,7 +221,7 @@ func (sa *ScannerAPI) Delete() { // Immutable registration is not allowed if r.Immutable { - sa.SendForbiddenError(errors.Errorf("registration %s is not allowed to delete as it is immutable: scanner API: update", r.Name)) + sa.SendForbiddenError(errors.Errorf("registration %s is not allowed to delete as it is immutable: scanner API: delete", r.Name)) return } diff --git a/src/core/notifier/handler/notification/scan_image_handler_test.go b/src/core/notifier/handler/notification/scan_image_handler_test.go index aefb03a59..bd6f9a099 100644 --- a/src/core/notifier/handler/notification/scan_image_handler_test.go +++ b/src/core/notifier/handler/notification/scan_image_handler_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/notifier" @@ -105,7 +107,7 @@ type MockScanAPIController struct { } // Scan ... -func (msc *MockScanAPIController) Scan(artifact *v1.Artifact) error { +func (msc *MockScanAPIController) Scan(artifact *v1.Artifact, option ...sc.Option) error { args := msc.Called(artifact) return args.Error(0) @@ -157,6 +159,15 @@ func (msc *MockScanAPIController) DeleteReports(digests ...string) error { return args.Error(0) } +func (msc *MockScanAPIController) GetStats(requester string) (*all.Stats, error) { + args := msc.Called(requester) + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*all.Stats), args.Error(1) +} + // MockHTTPHandler ... type MockHTTPHandler struct{} diff --git a/src/core/router.go b/src/core/router.go index 7005e3bd8..97e2186a8 100755 --- a/src/core/router.go +++ b/src/core/router.go @@ -214,6 +214,11 @@ func initRouters() { // Handle scan hook beego.Router("/service/notifications/jobs/scan/:uuid", &jobs.Handler{}, "post:HandleScan") + // Add routes for scan all metrics + scanAllAPI := &api.ScanAllAPI{} + beego.Router("/api/scans/all/metrics", scanAllAPI, "get:GetScanAllMetrics") + beego.Router("/api/scans/schedule/metrics", scanAllAPI, "get:GetScheduleMetrics") + // Error pages beego.ErrorController(&controllers.ErrorController{}) diff --git a/src/core/service/notifications/admin/handler.go b/src/core/service/notifications/admin/handler.go index cd85d3e46..e2fd6a84b 100644 --- a/src/core/service/notifications/admin/handler.go +++ b/src/core/service/notifications/admin/handler.go @@ -23,6 +23,8 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/api" + j "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/pkg/scan/api/scan" ) var statusMap = map[string]string{ @@ -42,6 +44,10 @@ type Handler struct { UUID string status string UpstreamJobID string + revision int64 + jobName string + checkIn string + statusCode uint16 } // Prepare ... @@ -74,20 +80,30 @@ func (h *Handler) Prepare() { h.Abort("200") return } + h.statusCode = (uint16)(j.Status(data.Status).Code()) h.status = status + h.revision = data.Metadata.Revision + h.jobName = data.Metadata.JobName + h.checkIn = data.CheckIn } // HandleAdminJob handles the webhook of admin jobs func (h *Handler) HandleAdminJob() { - log.Infof("received admin job status update event: job-%d, status-%s", h.id, h.status) + log.Infof("received admin job status update event: job-%d, job_uuid-%s, status-%s, revision-%d", h.id, h.UUID, h.status, h.revision) + // create the mapping relationship between the jobs in database and jobservice if err := dao.SetAdminJobUUID(h.id, h.UUID); err != nil { h.SendInternalServerError(err) return } - if err := dao.UpdateAdminJobStatus(h.id, h.status); err != nil { + if err := dao.UpdateAdminJobStatus(h.id, h.status, h.statusCode, h.revision); err != nil { log.Errorf("Failed to update job status, id: %d, status: %s", h.id, h.status) h.SendInternalServerError(err) return } + + // For scan all job + if h.jobName == job.ImageScanAllJob { + scan.HandleCheckIn(h.checkIn) + } } diff --git a/src/jobservice/job/impl/scan/all.go b/src/jobservice/job/impl/scan/all.go deleted file mode 100644 index 812ea0ac9..000000000 --- a/src/jobservice/job/impl/scan/all.go +++ /dev/null @@ -1,141 +0,0 @@ -// Copyright 2018 The 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 ( - "bytes" - "fmt" - "io/ioutil" - - "net/http" - "os" - "strings" - - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/jobservice/job" - "github.com/goharbor/harbor/src/jobservice/job/impl/utils" -) - -// All query the DB and Registry for all image and tags, -// then call Harbor's API to scan each of them. -type All struct { - registryURL string - secret string - tokenServiceEndpoint string - harborAPIEndpoint string - coreClient *http.Client -} - -// MaxFails implements the interface in job/Interface -func (sa *All) MaxFails() uint { - return 1 -} - -// ShouldRetry implements the interface in job/Interface -func (sa *All) ShouldRetry() bool { - return false -} - -// Validate implements the interface in job/Interface -func (sa *All) Validate(params job.Parameters) error { - if len(params) > 0 { - return fmt.Errorf("the parms should be empty for scan all job") - } - return nil -} - -// Run implements the interface in job/Interface -func (sa *All) Run(ctx job.Context, params job.Parameters) error { - logger := ctx.GetLogger() - logger.Info("Scanning all the images in the registry") - err := sa.init(ctx) - if err != nil { - logger.Errorf("Failed to initialize the job handler, error: %v", err) - return err - } - - repos, err := dao.GetRepositories() - if err != nil { - logger.Errorf("Failed to get the list of repositories, error: %v", err) - return err - } - - for _, r := range repos { - repoClient, err := utils.NewRepositoryClientForJobservice(r.Name, sa.registryURL, sa.secret, sa.tokenServiceEndpoint) - if err != nil { - logger.Errorf("Failed to get repo client for repo: %s, error: %v", r.Name, err) - continue - } - tags, err := repoClient.ListTag() - if err != nil { - logger.Errorf("Failed to get tags for repo: %s, error: %v", r.Name, err) - continue - } - for _, t := range tags { - logger.Infof("Calling harbor-core API to scan image, %s:%s", r.Name, t) - resp, err := sa.coreClient.Post(fmt.Sprintf("%s/repositories/%s/tags/%s/scan", sa.harborAPIEndpoint, r.Name, t), - "application/json", - bytes.NewReader([]byte("{}"))) - if err != nil { - logger.Errorf("Failed to trigger image scan, error: %v", err) - } else { - data, err := ioutil.ReadAll(resp.Body) - if err != nil { - logger.Errorf("Failed to read response, error: %v", err) - } else if resp.StatusCode != http.StatusAccepted { - logger.Errorf("Unexpected response code: %d, data: %v", resp.StatusCode, data) - } - resp.Body.Close() - } - } - - } - - return nil -} - -func (sa *All) init(ctx job.Context) error { - if v, err := getAttrFromCtx(ctx, common.RegistryURL); err == nil { - sa.registryURL = v - } else { - return err - } - if v := os.Getenv("JOBSERVICE_SECRET"); len(v) > 0 { - sa.secret = v - } else { - return fmt.Errorf("failed to read evnironment variable JOBSERVICE_SECRET") - } - sa.coreClient, _ = utils.GetClient() - if v, err := getAttrFromCtx(ctx, common.TokenServiceURL); err == nil { - sa.tokenServiceEndpoint = v - } else { - return err - } - if v, err := getAttrFromCtx(ctx, common.CoreURL); err == nil { - v = strings.TrimSuffix(v, "/") - sa.harborAPIEndpoint = v + "/api" - } else { - return err - } - return nil -} - -func getAttrFromCtx(ctx job.Context, key string) (string, error) { - if v, ok := ctx.Get(key); ok && len(v.(string)) > 0 { - return v.(string), nil - } - return "", fmt.Errorf("failed to get required property: %s", key) -} diff --git a/src/jobservice/job/status.go b/src/jobservice/job/status.go index 2c89f9ac2..6b2d85218 100644 --- a/src/jobservice/job/status.go +++ b/src/jobservice/job/status.go @@ -80,3 +80,9 @@ func (s Status) Compare(another Status) int { func (s Status) String() string { return string(s) } + +// Final returns if the status is final status +// e.g: "Stopped", "Error" or "Success" +func (s Status) Final() bool { + return s.Code() == 3 +} diff --git a/src/jobservice/runtime/bootstrap.go b/src/jobservice/runtime/bootstrap.go index 78c657605..4e737de05 100644 --- a/src/jobservice/runtime/bootstrap.go +++ b/src/jobservice/runtime/bootstrap.go @@ -34,7 +34,6 @@ import ( "github.com/goharbor/harbor/src/jobservice/job/impl/notification" "github.com/goharbor/harbor/src/jobservice/job/impl/replication" "github.com/goharbor/harbor/src/jobservice/job/impl/sample" - "github.com/goharbor/harbor/src/jobservice/job/impl/scan" "github.com/goharbor/harbor/src/jobservice/lcm" "github.com/goharbor/harbor/src/jobservice/logger" "github.com/goharbor/harbor/src/jobservice/mgt" @@ -43,6 +42,7 @@ import ( "github.com/goharbor/harbor/src/jobservice/worker/cworker" "github.com/goharbor/harbor/src/pkg/retention" sc "github.com/goharbor/harbor/src/pkg/scan" + "github.com/goharbor/harbor/src/pkg/scan/all" "github.com/goharbor/harbor/src/pkg/scheduler" "github.com/gomodule/redigo/redis" "github.com/pkg/errors" @@ -243,7 +243,7 @@ func (bs *Bootstrap) loadAndRunRedisWorkerPool( job.SampleJob: (*sample.Job)(nil), // Functional jobs job.ImageScanJob: (*sc.Job)(nil), - job.ImageScanAllJob: (*scan.All)(nil), + job.ImageScanAllJob: (*all.Job)(nil), job.ImageGC: (*gc.GarbageCollector)(nil), job.Replication: (*replication.Replication)(nil), job.ReplicationScheduler: (*replication.Scheduler)(nil), diff --git a/src/pkg/scan/all/checkin.go b/src/pkg/scan/all/checkin.go new file mode 100644 index 000000000..23041da8e --- /dev/null +++ b/src/pkg/scan/all/checkin.go @@ -0,0 +1,47 @@ +// 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 all + +import ( + "encoding/json" + + v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" + "github.com/pkg/errors" +) + +// CheckInData is designed for checking the data generated by the scan all job. +type CheckInData struct { + Artifacts []*v1.Artifact `json:"artifacts"` + Requester string `json:"requester"` +} + +// ToJSON marshals `CheckInData` to JSON str +func (c *CheckInData) ToJSON() (string, error) { + data, err := json.Marshal(c) + if err != nil { + return "", errors.Wrap(err, "ToJSON : CheckInData") + } + + return string(data), nil +} + +// FromJSON unmarshal give bytes to `CheckInData` +func (c *CheckInData) FromJSON(data []byte) error { + if err := json.Unmarshal(data, c); err != nil { + return errors.Wrap(err, "FromJSON : CheckInData") + } + + return nil +} diff --git a/src/pkg/scan/all/job.go b/src/pkg/scan/all/job.go new file mode 100644 index 000000000..fb4bff423 --- /dev/null +++ b/src/pkg/scan/all/job.go @@ -0,0 +1,163 @@ +// 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 all + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/pkg/art" + "github.com/goharbor/harbor/src/pkg/q" + v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" + "github.com/pkg/errors" +) + +const ( + // The max number of the goroutines to retrieve the tags + maxProcessors = 25 + // Job parameter key for the admin job ID + jobParamAJID = "admin_job_id" +) + +// Job query the DB and Registry for all image and tags, +// then call Harbor's API to scan each of them. +type Job struct{} + +// MaxFails implements the interface in job/Interface +func (sa *Job) MaxFails() uint { + return 1 +} + +// ShouldRetry implements the interface in job/Interface +func (sa *Job) ShouldRetry() bool { + return false +} + +// Validate implements the interface in job/Interface +func (sa *Job) Validate(params job.Parameters) error { + _, err := parseAJID(params) + if err != nil { + return errors.Wrap(err, "job validation: scan all job") + } + + return nil +} + +// Run implements the interface in job/Interface +func (sa *Job) Run(ctx job.Context, params job.Parameters) error { + logger := ctx.GetLogger() + logger.Info("Scanning all the images in the registry") + + // No need to check error any more as it has been checked in job validation. + requester, _ := parseAJID(params) + + // List all the repositories of registry + // TODO: REPLACE DAO WITH CORRESPONDING MANAGER OR CTL + repos, err := dao.GetRepositories() + if err != nil { + err = errors.Wrap(err, "list repositories : scan all job") + logger.Error(err) + return err + } + logger.Infof("Found %d repositories", len(repos)) + + // Initialize tokens + tokens := make(chan bool, maxProcessors) + for i := 0; i < maxProcessors; i++ { + // Assign tokens at first + tokens <- true + } + + // Get the tags under the repository + for _, r := range repos { + // Get token first + <-tokens + + go func(repo *models.RepoRecord) { + defer func() { + // Return the token when process ending + tokens <- true + }() + + logger.Infof("Scan artifacts under repository: %s", repo.Name) + + // Query artifacts under the repository + query := &q.Query{ + Keywords: make(map[string]interface{}), + } + query.Keywords["repo"] = repo.Name + + al, err := art.DefaultController.List(query) + if err != nil { + logger.Errorf("Failed to get tags for repo: %s, error: %v", repo.Name, err) + return + } + + if len(al) > 0 { + // Check in the data + arts := make([]*v1.Artifact, 0) + + for _, a := range al { + artf := &v1.Artifact{ + NamespaceID: repo.ProjectID, + Repository: repo.Name, + Tag: a.Tag, + Digest: a.Digest, + MimeType: v1.MimeTypeDockerArtifact, // default + } + + arts = append(arts, artf) + } + + logger.Infof("Found %d artifacts under repository %s", len(arts), repo.Name) + + ck := &CheckInData{ + Artifacts: arts, + Requester: requester, + } + + jsn, err := ck.ToJSON() + if err != nil { + logger.Error(errors.Wrap(err, "scan all job")) + return + } + + if err := ctx.Checkin(jsn); err != nil { + logger.Error(errors.Wrap(err, "check in data: scan all job")) + } + + logger.Infof("Check in scanning artifacts for repository: %s", repo.Name) + // Debug more + logger.Debugf("Check in: %s\n", jsn) + } else { + logger.Infof("No scanning artifacts found under repository: %s", repo.Name) + } + }(r) + } + + return nil +} + +func parseAJID(params job.Parameters) (string, error) { + if len(params) > 0 { + if v, ok := params[jobParamAJID]; ok { + if id, y := v.(string); y { + return id, nil + } + } + } + + return "", errors.Errorf("missing required job parameter: %s", jobParamAJID) +} diff --git a/src/pkg/scan/all/stats.go b/src/pkg/scan/all/stats.go new file mode 100644 index 000000000..d678b9738 --- /dev/null +++ b/src/pkg/scan/all/stats.go @@ -0,0 +1,34 @@ +// 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 all + +// Stats provides the overall progress of the scan all process. +type Stats struct { + Total uint `json:"total"` + // Status including `Success`, `Error` or `Stopped` will be counted as completed. + // This data may be influenced by job retrying + Completed uint `json:"completed"` + Metrics StatusMetrics `json:"metrics"` + Requester string `json:"requester"` +} + +// StatusMetrics contains the metrics of each status. +// The key should be the following valid status texts: +// - "pending" +// - "running" +// - "success" +// - "error" +// - "stopped" +type StatusMetrics map[string]uint diff --git a/src/pkg/scan/api/scan/all_handler.go b/src/pkg/scan/api/scan/all_handler.go new file mode 100644 index 000000000..c10c17dd6 --- /dev/null +++ b/src/pkg/scan/api/scan/all_handler.go @@ -0,0 +1,42 @@ +// 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 ( + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/pkg/errors" +) + +// HandleCheckIn handles the check in data of the scan all job +func HandleCheckIn(checkIn string) { + if len(checkIn) == 0 { + // Nothing to handle, directly return + return + } + + ck := &all.CheckInData{} + if err := ck.FromJSON([]byte(checkIn)); err != nil { + log.Error(errors.Wrap(err, "handle check in")) + } + + // Start to scan the artifacts + for _, art := range ck.Artifacts { + if err := DefaultController.Scan(art, WithRequester(ck.Requester)); err != nil { + // Just logged + log.Error(errors.Wrap(err, "handle check in")) + } + } +} diff --git a/src/pkg/scan/api/scan/base_controller.go b/src/pkg/scan/api/scan/base_controller.go index d40ce31f9..7548d2638 100644 --- a/src/pkg/scan/api/scan/base_controller.go +++ b/src/pkg/scan/api/scan/base_controller.go @@ -31,6 +31,7 @@ import ( "github.com/goharbor/harbor/src/pkg/robot" "github.com/goharbor/harbor/src/pkg/robot/model" sca "github.com/goharbor/harbor/src/pkg/scan" + "github.com/goharbor/harbor/src/pkg/scan/all" 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" @@ -115,11 +116,17 @@ func NewController() Controller { } // Scan ... -func (bc *basicController) Scan(artifact *v1.Artifact) error { +func (bc *basicController) Scan(artifact *v1.Artifact, options ...Option) error { if artifact == nil { return errors.New("nil artifact to scan") } + // Parse options + ops, err := parseOptions(options...) + if err != nil { + return errors.Wrap(err, "scan controller: scan") + } + r, err := bc.sc.GetRegistrationByProject(artifact.NamespaceID) if err != nil { return errors.Wrap(err, "scan controller: scan") @@ -171,6 +178,14 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error { TrackID: trackID, MimeType: pm, } + // Set requester if it is specified + if len(ops.Requester) > 0 { + reportPlaceholder.Requester = ops.Requester + } else { + // Use the trackID as the requester + reportPlaceholder.Requester = trackID + } + _, e := bc.manager.Create(reportPlaceholder) if e != nil { // Check if it is a status conflict error with common error format. @@ -377,7 +392,21 @@ func (bc *basicController) HandleJobHooks(trackID string, change *job.StatusChan // DeleteReports ... func (bc *basicController) DeleteReports(digests ...string) error { - return bc.manager.DeleteByDigests(digests...) + if err := bc.manager.DeleteByDigests(digests...); err != nil { + return errors.Wrap(err, "scan controller: delete reports") + } + + return nil +} + +// GetStats ... +func (bc *basicController) GetStats(requester string) (*all.Stats, error) { + sts, err := bc.manager.GetStats(requester) + if err != nil { + return nil, errors.Wrap(err, "scan controller: delete reports") + } + + return sts, nil } // makeBasicAuthorization creates authorization from a robot account based on the arguments for scanning. @@ -509,3 +538,14 @@ func makeBearerAuthorization(repository string, username string) (string, error) return fmt.Sprintf("Bearer %s", accessToken.Token), nil } + +func parseOptions(options ...Option) (*Options, error) { + ops := &Options{} + for _, op := range options { + if err := op(ops); err != nil { + return nil, err + } + } + + return ops, nil +} diff --git a/src/pkg/scan/api/scan/base_controller_test.go b/src/pkg/scan/api/scan/base_controller_test.go index acd901f39..37fb01177 100644 --- a/src/pkg/scan/api/scan/base_controller_test.go +++ b/src/pkg/scan/api/scan/base_controller_test.go @@ -30,6 +30,7 @@ import ( "github.com/goharbor/harbor/src/pkg/q" "github.com/goharbor/harbor/src/pkg/robot/model" sca "github.com/goharbor/harbor/src/pkg/scan" + "github.com/goharbor/harbor/src/pkg/scan/all" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" @@ -105,6 +106,7 @@ func (suite *ControllerTestSuite) SetupSuite() { Status: "Pending", StatusCode: 0, TrackID: "the-uuid-123", + Requester: "the-uuid-123", }).Return("r-uuid", nil) mgr.On("UpdateScanJobID", "the-uuid-123", "the-job-id").Return(nil) @@ -352,6 +354,16 @@ func (mrm *MockReportManager) DeleteByDigests(digests ...string) error { return args.Error(0) } +func (mrm *MockReportManager) GetStats(requester string) (*all.Stats, error) { + args := mrm.Called(requester) + + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*all.Stats), args.Error(1) +} + // MockScannerController ... type MockScannerController struct { mock.Mock diff --git a/src/pkg/scan/api/scan/controller.go b/src/pkg/scan/api/scan/controller.go index 5ca0a28c4..38c3fff5a 100644 --- a/src/pkg/scan/api/scan/controller.go +++ b/src/pkg/scan/api/scan/controller.go @@ -16,6 +16,7 @@ package scan import ( "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/pkg/scan/all" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/scan/report" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" @@ -29,10 +30,11 @@ type Controller interface { // // Arguments: // artifact *v1.Artifact : artifact to be scanned + // options ...Option : options for triggering a scan // // Returns: // error : non nil error if any errors occurred - Scan(artifact *v1.Artifact) error + Scan(artifact *v1.Artifact, options ...Option) error // GetReport gets the reports for the given artifact identified by the digest // @@ -86,4 +88,14 @@ type Controller interface { // Returns: // error : non nil error if any errors occurred DeleteReports(digests ...string) error + + // Get the stats of the scan reports requested by the given requester. + // + // Arguments: + // requester string : requester identity + // + // Returns: + // *all.AllStats: stats object including the related metric data + // error : non nil error if any errors occurred + GetStats(requester string) (*all.Stats, error) } diff --git a/src/pkg/scan/api/scan/options.go b/src/pkg/scan/api/scan/options.go new file mode 100644 index 000000000..77015844e --- /dev/null +++ b/src/pkg/scan/api/scan/options.go @@ -0,0 +1,38 @@ +// 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 + +// Options keep the settings/configurations for scanning. +type Options struct { + // Mark the scan triggered by who. + // Identified by the UUID. + Requester string +} + +// Option represents an option item by func template. +// The validation result of the options are marked by nil/non-nil error. +// e.g: +// If the option is required and the input arg is empty, +// then a non nil error should be returned at then. +type Option func(options *Options) error + +// WithRequester sets the requester option. +func WithRequester(Requester string) Option { + return func(options *Options) error { + options.Requester = Requester + + return nil + } +} diff --git a/src/pkg/scan/dao/scan/model.go b/src/pkg/scan/dao/scan/model.go index 9d7f3ff4a..9cc2dd919 100644 --- a/src/pkg/scan/dao/scan/model.go +++ b/src/pkg/scan/dao/scan/model.go @@ -26,6 +26,7 @@ type Report struct { MimeType string `orm:"column(mime_type)"` JobID string `orm:"column(job_id)"` TrackID string `orm:"column(track_id)"` + Requester string `orm:"column(requester)"` Status string `orm:"column(status)"` StatusCode int `orm:"column(status_code)"` StatusRevision int64 `orm:"column(status_rev)"` diff --git a/src/pkg/scan/dao/scan/report.go b/src/pkg/scan/dao/scan/report.go index f640f9091..c68f9ff65 100644 --- a/src/pkg/scan/dao/scan/report.go +++ b/src/pkg/scan/dao/scan/report.go @@ -16,6 +16,7 @@ package scan import ( "fmt" + "strconv" "time" "github.com/astaxie/beego/orm" @@ -123,9 +124,18 @@ func UpdateReportStatus(trackID string, status string, statusCode int, statusRev data["end_time"] = time.Now().UTC() } - count, err := qt.Filter("track_id", trackID). - Filter("status_rev__lte", statusRev). - Filter("status_code__lte", statusCode).Update(data) + // qt generates sql statements: + // UPDATE "scan_report" SET "end_time" = $1, "status" = $2, "status_code" = $3, "status_rev" = $4 + // WHERE "id" IN ( SELECT T0."id" FROM "scan_report" T0 WHERE ( T0."status_rev" = $5 AND T0."status_code" < $6 ) + // OR ( T0."status_rev" < $7 ) AND T0."track_id" = $8 ) + cond := orm.NewCondition() + c1 := cond.And("status_rev", statusRev).And("status_code__lt", statusCode) + c2 := cond.And("status_rev__lt", statusRev) + c := cond.AndCond(c1).OrCond(c2) + + count, err := qt.SetCond(c). + Filter("track_id", trackID). + Update(data) if err != nil { return err @@ -151,3 +161,28 @@ func UpdateJobID(trackID string, jobID string) error { return err } + +// GetScanStats gets the scan stats organized by status +func GetScanStats(requester string) (map[string]uint, error) { + res := make(orm.Params) + + o := dao.GetOrmer() + if _, err := o.Raw("select status, count(status) from (select status from scan_report where requester=? group by track_id, status) as scan_status group by status"). + SetArgs(requester). + RowsToMap(&res, "status", "count"); err != nil { + return nil, err + } + + m := make(map[string]uint) + for k, v := range res { + vl, err := strconv.ParseInt(v.(string), 10, 32) + if err != nil { + log.Error(errors.Wrap(err, "get scan stats")) + continue + } + + m[k] = uint(vl) + } + + return m, nil +} diff --git a/src/pkg/scan/dao/scan/report_test.go b/src/pkg/scan/dao/scan/report_test.go index daf5159d2..1830daddf 100644 --- a/src/pkg/scan/dao/scan/report_test.go +++ b/src/pkg/scan/dao/scan/report_test.go @@ -21,6 +21,7 @@ import ( "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/pkg/q" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -48,17 +49,13 @@ func (suite *ReportTestSuite) SetupTest() { TrackID: "track-uuid", Digest: "digest1001", RegistrationUUID: "ruuid", + Requester: "requester", MimeType: v1.MimeTypeNativeReport, Status: job.PendingStatus.String(), StatusCode: job.PendingStatus.Code(), } - id, err := CreateReport(r) - require.NoError(suite.T(), err) - require.Condition(suite.T(), func() (success bool) { - success = id > 0 - return - }) + suite.create(r) } // TearDownTest clears enf for test case. @@ -124,9 +121,112 @@ func (suite *ReportTestSuite) TestReportUpdateStatus() { err := UpdateReportStatus("track-uuid", job.RunningStatus.String(), job.RunningStatus.Code(), 1000) require.NoError(suite.T(), err) - err = UpdateReportStatus("track-uuid", job.RunningStatus.String(), job.RunningStatus.Code(), 900) + err = checkStatus("track-uuid", job.RunningStatus.String()) + suite.NoError(err, "regular status update") + + err = UpdateReportStatus("track-uuid", job.SuccessStatus.String(), job.SuccessStatus.Code(), 900) require.NoError(suite.T(), err) + err = checkStatus("track-uuid", job.RunningStatus.String()) + suite.NoError(err, "update with outdated revision") + err = UpdateReportStatus("track-uuid", job.PendingStatus.String(), job.PendingStatus.Code(), 1000) require.NoError(suite.T(), err) + + err = checkStatus("track-uuid", job.RunningStatus.String()) + suite.NoError(err, "update with same revision and previous status") + + err = UpdateReportStatus("track-uuid", job.PendingStatus.String(), job.PendingStatus.Code(), 1001) + require.NoError(suite.T(), err) + + err = checkStatus("track-uuid", job.PendingStatus.String()) + suite.NoError(err, "update latest revision and previous status") +} + +// TestReportGetStats ... +func (suite *ReportTestSuite) TestReportGetStats() { + // Two more for getting stats + r2 := &Report{ + UUID: "uuid2", + TrackID: "track-uuid2", + Digest: "digest1003", + RegistrationUUID: "ruuid", + Requester: "requester", + MimeType: v1.MimeTypeNativeReport, + Status: job.RunningStatus.String(), + StatusCode: job.RunningStatus.Code(), + } + suite.create(r2) + + r3 := &Report{ + UUID: "uuid3", + TrackID: "track-uuid2", + Digest: "digest1003", + RegistrationUUID: "ruuid", + Requester: "requester", + MimeType: v1.MimeTypeRawReport, + Status: job.RunningStatus.String(), + StatusCode: job.RunningStatus.Code(), + } + suite.create(r3) + + defer func() { + err := DeleteReport("uuid2") + suite.NoError(err) + + err = DeleteReport("uuid3") + suite.NoError(err) + }() + + m, err := GetScanStats("requester") + require.NoError(suite.T(), err) + suite.Equal(2, len(m)) + suite.Condition(func() (success bool) { + v, ok := m[job.RunningStatus.String()] + vv, ook := m[job.PendingStatus.String()] + + success = ok && ook && v == 1 && vv == 1 + + return + }) + +} + +func (suite *ReportTestSuite) create(r *Report) { + id, err := CreateReport(r) + require.NoError(suite.T(), err) + require.Condition(suite.T(), func() (success bool) { + success = id > 0 + return + }) +} + +func list(trackID string) ([]*Report, error) { + kws := make(map[string]interface{}) + kws["track_id"] = trackID + query := &q.Query{ + Keywords: kws, + } + + l, err := ListReports(query) + if err != nil { + return nil, err + } + + return l, nil +} + +func checkStatus(trackID string, status string) error { + l, err := list(trackID) + if err != nil { + return err + } + + for _, r := range l { + if r.Status != status { + return errors.Errorf("status is not matched: current %s : expected %s", r.Status, status) + } + } + + return nil } diff --git a/src/pkg/scan/dao/scanner/registration.go b/src/pkg/scan/dao/scanner/registration.go index db489fe17..8bcdc13ae 100644 --- a/src/pkg/scan/dao/scanner/registration.go +++ b/src/pkg/scan/dao/scanner/registration.go @@ -109,6 +109,9 @@ func ListRegistrations(query *q.Query) ([]*Registration, error) { } } + // Order the list + qt = qt.OrderBy("-is_default", "-create_time") + l := make([]*Registration, 0) _, err := qt.All(&l) diff --git a/src/pkg/scan/report/base_manager.go b/src/pkg/scan/report/base_manager.go index 471f6435c..850fdd9b6 100644 --- a/src/pkg/scan/report/base_manager.go +++ b/src/pkg/scan/report/base_manager.go @@ -17,6 +17,8 @@ package report import ( "time" + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/pkg/q" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" @@ -237,3 +239,49 @@ func (bm *basicManager) DeleteByDigests(digests ...string) error { return err } + +// GetStats ... +func (bm *basicManager) GetStats(requester string) (*all.Stats, error) { + if len(requester) == 0 { + return nil, errors.New("empty requester") + } + + m, err := scan.GetScanStats(requester) + if err != nil { + return nil, errors.Wrap(err, "report manager: get stats") + } + + sts := &all.Stats{ + Metrics: make(all.StatusMetrics), + } + + for k, v := range m { + // Increase the total metrics + sts.Total += v + + s := job.Status(k) + // Increase the completed metrics if the status is not predefined ones or + // the status is the final status. + if s.Validate() != nil || s.Final() { + sts.Completed += v + } + + // Not standard error status. + // Convert it to standard error status. + if s.Validate() != nil { + tv := v + + if val, ok := sts.Metrics[job.ErrorStatus.String()]; ok { + tv = val + v + } + sts.Metrics[job.ErrorStatus.String()] = tv + + continue + } + + sts.Metrics[k] = v + } + sts.Requester = requester + + return sts, nil +} diff --git a/src/pkg/scan/report/base_manager_test.go b/src/pkg/scan/report/base_manager_test.go index f04782d1d..1b2eb0f05 100644 --- a/src/pkg/scan/report/base_manager_test.go +++ b/src/pkg/scan/report/base_manager_test.go @@ -53,6 +53,7 @@ func (suite *TestManagerSuite) SetupTest() { RegistrationUUID: "ruuid", MimeType: v1.MimeTypeNativeReport, TrackID: "tid001", + Requester: "requester", } uuid, err := suite.m.Create(rp) @@ -188,3 +189,37 @@ func (suite *TestManagerSuite) TestManagerDeleteByDigests() { suite.NoError(err) suite.Nil(r) } + +// TestManagerGetStats ... +func (suite *TestManagerSuite) TestManagerGetStats() { + // Mock new data + rp := &scan.Report{ + Digest: "d1001", + RegistrationUUID: "ruuid", + MimeType: v1.MimeTypeNativeReport, + TrackID: "tid002", + Requester: "requester", + } + + uuid, err := suite.m.Create(rp) + require.NoError(suite.T(), err) + require.NotEmpty(suite.T(), uuid) + + defer func() { + err := scan.DeleteReport(uuid) + suite.NoError(err, "clear test data") + }() + + err = suite.m.UpdateStatus("tid002", job.SuccessStatus.String(), 1000) + require.NoError(suite.T(), err) + + st, err := suite.m.GetStats("requester") + require.NoError(suite.T(), err) + require.NotNil(suite.T(), st) + + suite.Equal(uint(2), st.Total) + suite.Equal(uint(1), st.Completed) + suite.Equal(2, len(st.Metrics)) + suite.Equal(uint(1), st.Metrics[job.SuccessStatus.String()]) + suite.Equal(uint(1), st.Metrics[job.PendingStatus.String()]) +} diff --git a/src/pkg/scan/report/manager.go b/src/pkg/scan/report/manager.go index 604781bf8..15125641b 100644 --- a/src/pkg/scan/report/manager.go +++ b/src/pkg/scan/report/manager.go @@ -14,7 +14,10 @@ package report -import "github.com/goharbor/harbor/src/pkg/scan/dao/scan" +import ( + "github.com/goharbor/harbor/src/pkg/scan/all" + "github.com/goharbor/harbor/src/pkg/scan/dao/scan" +) // Manager is used to manage the scan reports. type Manager interface { @@ -96,4 +99,14 @@ type Manager interface { // Returns: // error : non nil error if any errors occurred DeleteByDigests(digests ...string) error + + // GetStats retrieves and calculates the overall report stats organized by status targeting the + // given requester. + // Arguments: + // requester string : the requester of the scan (all) + // + // Returns: + // *all.AllStats: stats object including the related metric data + // error : non nil error if any errors occurred + GetStats(requester string) (*all.Stats, error) }