diff --git a/src/controller/securityhub/controller.go b/src/controller/securityhub/controller.go index 2501d2d7d..f53527a5d 100644 --- a/src/controller/securityhub/controller.go +++ b/src/controller/securityhub/controller.go @@ -18,8 +18,6 @@ import ( "context" "github.com/goharbor/harbor/src/lib/q" - "github.com/goharbor/harbor/src/pkg" - "github.com/goharbor/harbor/src/pkg/artifact" "github.com/goharbor/harbor/src/pkg/scan/scanner" "github.com/goharbor/harbor/src/pkg/securityhub" secHubModel "github.com/goharbor/harbor/src/pkg/securityhub/model" @@ -71,19 +69,17 @@ type Controller interface { } type controller struct { - artifactMgr artifact.Manager - scannerMgr scanner.Manager - secHubMgr securityhub.Manager - tagMgr tag.Manager + scannerMgr scanner.Manager + secHubMgr securityhub.Manager + tagMgr tag.Manager } // NewController ... func NewController() Controller { return &controller{ - artifactMgr: pkg.ArtifactMgr, - scannerMgr: scanner.Mgr, - secHubMgr: securityhub.Mgr, - tagMgr: tag.Mgr, + scannerMgr: scanner.Mgr, + secHubMgr: securityhub.Mgr, + tagMgr: tag.Mgr, } } @@ -129,10 +125,7 @@ func (c *controller) scannedArtifactCount(ctx context.Context, projectID int64) } func (c *controller) totalArtifactCount(ctx context.Context, projectID int64) (int64, error) { - if projectID == 0 { - return c.artifactMgr.Count(ctx, nil) - } - return c.artifactMgr.Count(ctx, q.New(q.KeyWords{"project_id": projectID})) + return c.secHubMgr.TotalArtifactsCount(ctx, projectID) } func (c *controller) ListVuls(ctx context.Context, scannerUUID string, projectID int64, withTag bool, query *q.Query) ([]*secHubModel.VulnerabilityItem, error) { diff --git a/src/controller/securityhub/controller_test.go b/src/controller/securityhub/controller_test.go index 773d9b1c5..600c692bf 100644 --- a/src/controller/securityhub/controller_test.go +++ b/src/controller/securityhub/controller_test.go @@ -25,7 +25,6 @@ import ( "github.com/goharbor/harbor/src/pkg/tag/model/tag" htesting "github.com/goharbor/harbor/src/testing" "github.com/goharbor/harbor/src/testing/mock" - artifactMock "github.com/goharbor/harbor/src/testing/pkg/artifact" scannerMock "github.com/goharbor/harbor/src/testing/pkg/scan/scanner" securityMock "github.com/goharbor/harbor/src/testing/pkg/securityhub" tagMock "github.com/goharbor/harbor/src/testing/pkg/tag" @@ -42,11 +41,10 @@ var sum = &model.Summary{ type ControllerTestSuite struct { htesting.Suite - c *controller - artifactMgr *artifactMock.Manager - scannerMgr *scannerMock.Manager - secHubMgr *securityMock.Manager - tagMgr *tagMock.FakeManager + c *controller + scannerMgr *scannerMock.Manager + secHubMgr *securityMock.Manager + tagMgr *tagMock.FakeManager } // TestController is the entry of controller test suite @@ -56,16 +54,14 @@ func TestController(t *testing.T) { // SetupTest prepares env for the controller test suite func (suite *ControllerTestSuite) SetupTest() { - suite.artifactMgr = &artifactMock.Manager{} suite.secHubMgr = &securityMock.Manager{} suite.scannerMgr = &scannerMock.Manager{} suite.tagMgr = &tagMock.FakeManager{} suite.c = &controller{ - artifactMgr: suite.artifactMgr, - secHubMgr: suite.secHubMgr, - scannerMgr: suite.scannerMgr, - tagMgr: suite.tagMgr, + secHubMgr: suite.secHubMgr, + scannerMgr: suite.scannerMgr, + tagMgr: suite.tagMgr, } } @@ -76,7 +72,7 @@ func (suite *ControllerTestSuite) TearDownTest() { func (suite *ControllerTestSuite) TestSecuritySummary() { ctx := suite.Context() - mock.OnAnything(suite.artifactMgr, "Count").Return(int64(1234), nil) + mock.OnAnything(suite.secHubMgr, "TotalArtifactsCount").Return(int64(1234), nil) mock.OnAnything(suite.secHubMgr, "ScannedArtifactsCount").Return(int64(1000), nil) mock.OnAnything(suite.secHubMgr, "Summary").Return(sum, nil).Twice() mock.OnAnything(suite.scannerMgr, "DefaultScannerUUID").Return("ruuid", nil) @@ -125,12 +121,13 @@ func (suite *ControllerTestSuite) TestSecuritySummary() { func (suite *ControllerTestSuite) TestSecuritySummaryError() { ctx := suite.Context() mock.OnAnything(suite.scannerMgr, "DefaultScannerUUID").Return("ruuid", nil) + mock.OnAnything(suite.secHubMgr, "TotalArtifactsCount").Return(int64(0), errors.New("project not found")).Once() mock.OnAnything(suite.secHubMgr, "ScannedArtifactsCount").Return(int64(1000), nil) mock.OnAnything(suite.secHubMgr, "Summary").Return(nil, errors.New("invalid project")).Once() summary, err := suite.c.SecuritySummary(ctx, 0, WithCVE(false), WithArtifact(false)) suite.Error(err) suite.Nil(summary) - mock.OnAnything(suite.artifactMgr, "Count").Return(int64(0), errors.New("failed to connect db")).Once() + mock.OnAnything(suite.secHubMgr, "TotalArtifactsCount").Return(int64(0), errors.New("failed to connect db")).Once() mock.OnAnything(suite.secHubMgr, "Summary").Return(sum, nil).Once() summary, err = suite.c.SecuritySummary(ctx, 0, WithCVE(false), WithArtifact(false)) suite.Error(err) diff --git a/src/pkg/securityhub/dao/security.go b/src/pkg/securityhub/dao/security.go index a9581d030..9ccd0fe95 100644 --- a/src/pkg/securityhub/dao/security.go +++ b/src/pkg/securityhub/dao/security.go @@ -47,10 +47,36 @@ where a.digest = s.digest order by s.critical_cnt desc, s.high_cnt desc, s.medium_cnt desc, s.low_cnt desc limit 5` - // sql to query the scanned artifact count - scannedArtifactCountSQL = `select count(1) -from artifact -where exists (select 1 from scan_report s where artifact.digest = s.digest and s.registration_uuid = ?) ` + // sql to query the total artifact count, exclude the artifact accessory, and child artifact in image index + totalArtifactCountSQL = `SELECT COUNT(1) +FROM artifact A +WHERE NOT EXISTS (select 1 from artifact_accessory acc WHERE acc.artifact_id = a.id) + AND (EXISTS (SELECT 1 FROM tag WHERE tag.artifact_id = a.id) + OR NOT EXISTS (SELECT 1 FROM artifact_reference ref WHERE ref.child_id = a.id))` + + // sql to query the scanned artifact count, exclude the artifact accessory, and child artifact in image index, + // and include the image index artifact which at least one child artifact is scanned + scannedArtifactCountSQL = `SELECT COUNT(1) +FROM artifact a +WHERE EXISTS (SELECT 1 + FROM scan_report s + WHERE a.digest = s.digest + AND s.registration_uuid = ?) + -- exclude artifact accessory + AND NOT EXISTS (SELECT 1 FROM artifact_accessory acc WHERE acc.artifact_id = a.id) + -- exclude artifact without tag and part of the image index + AND EXISTS (SELECT 1 + FROM tag + WHERE tag.artifact_id = id + OR (NOT EXISTS (SELECT 1 FROM artifact_reference ref WHERE ref.child_id = a.id))) + -- include image index which is scanned + OR EXISTS (SELECT 1 + FROM scan_report s, + artifact_reference ref + WHERE s.digest = ref.child_digest + AND ref.parent_id = a.id AND s.registration_uuid = ? AND NOT EXISTS (SELECT 1 + FROM scan_report s + WHERE s.digest = a.digest and s.registration_uuid = ?))` // sql to query the dangerous CVEs dangerousCVESQL = `select vr.* @@ -143,6 +169,8 @@ type SecurityHubDao interface { DangerousCVEs(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) ([]*scan.VulnerabilityRecord, error) // DangerousArtifacts returns top 5 dangerous artifact for the given scanner. return top 5 result DangerousArtifacts(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) ([]*model.DangerousArtifact, error) + // TotalArtifactsCount return the count of total artifacts. + TotalArtifactsCount(ctx context.Context, projectID int64) (int64, error) // ScannedArtifactsCount return the count of scanned artifacts. ScannedArtifactsCount(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) (int64, error) // ListVulnerabilities search vulnerability record by cveID @@ -159,6 +187,19 @@ func New() SecurityHubDao { type dao struct { } +func (d *dao) TotalArtifactsCount(ctx context.Context, projectID int64) (int64, error) { + if projectID != 0 { + return 0, nil + } + o, err := orm.FromContext(ctx) + if err != nil { + return 0, err + } + var count int64 + err = o.Raw(totalArtifactCountSQL).QueryRow(&count) + return count, err +} + func (d *dao) Summary(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) (*model.Summary, error) { if len(scannerUUID) == 0 || projectID != 0 { return nil, nil @@ -199,7 +240,7 @@ func (d *dao) ScannedArtifactsCount(ctx context.Context, scannerUUID string, pro if err != nil { return cnt, err } - err = o.Raw(scannedArtifactCountSQL, scannerUUID).QueryRow(&cnt) + err = o.Raw(scannedArtifactCountSQL, scannerUUID, scannerUUID, scannerUUID).QueryRow(&cnt) return cnt, err } func (d *dao) DangerousCVEs(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) ([]*scan.VulnerabilityRecord, error) { diff --git a/src/pkg/securityhub/dao/security_test.go b/src/pkg/securityhub/dao/security_test.go index d578363bf..c701699cb 100644 --- a/src/pkg/securityhub/dao/security_test.go +++ b/src/pkg/securityhub/dao/security_test.go @@ -44,9 +44,19 @@ func (suite *SecurityDaoTestSuite) SetupSuite() { // SetupTest prepares env for test case. func (suite *SecurityDaoTestSuite) SetupTest() { testDao.ExecuteBatchSQL([]string{ + `delete from tag`, + `delete from artifact_accessory`, + `delete from artifact`, `insert into scan_report(uuid, digest, registration_uuid, mime_type, critical_cnt, high_cnt, medium_cnt, low_cnt, unknown_cnt, fixable_cnt) values('uuid', 'digest1001', 'ruuid', 'application/vnd.scanner.adapter.vuln.report.harbor+json; version=1.0', 50, 50, 50, 0, 0, 20)`, - `insert into artifact (project_id, repository_name, digest, type, pull_time, push_time, repository_id, media_type, manifest_media_type, size, extra_attrs, annotations, icon) -values (1, 'library/hello-world', 'digest1001', 'IMAGE', '2023-06-02 09:16:47.838778', '2023-06-02 01:45:55.050785', 1742, 'application/vnd.docker.container.image.v1+json', 'application/vnd.docker.distribution.manifest.v2+json', 4452, '{"architecture":"amd64","author":"","config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"],"Cmd":["/hello"]},"created":"2023-05-04T17:37:03.872958712Z","os":"linux"}', null, '');`, + `insert into artifact (id, project_id, repository_name, digest, type, pull_time, push_time, repository_id, media_type, manifest_media_type, size, extra_attrs, annotations, icon) +values (1001, 1, 'library/hello-world', 'digest1001', 'IMAGE', '2023-06-02 09:16:47.838778', '2023-06-02 01:45:55.050785', 1742, 'application/vnd.docker.container.image.v1+json', 'application/vnd.docker.distribution.manifest.v2+json', 4452, '{"architecture":"amd64","author":"","config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"],"Cmd":["/hello"]},"created":"2023-05-04T17:37:03.872958712Z","os":"linux"}', null, '');`, + `insert into artifact (id, project_id, repository_name, digest, type, pull_time, push_time, repository_id, media_type, manifest_media_type, size, extra_attrs, annotations, icon) +values (1002, 1, 'library/hello-world', 'digest1002', 'IMAGE', '2023-06-02 09:16:47.838778', '2023-06-02 01:45:55.050785', 1742, 'application/vnd.docker.container.image.v1+json', 'application/vnd.oci.image.config.v1+json', 4452, '{"architecture":"amd64","author":"","config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"],"Cmd":["/hello"]},"created":"2023-05-04T17:37:03.872958712Z","os":"linux"}', null, '');`, + `insert into artifact (id, project_id, repository_name, digest, type, pull_time, push_time, repository_id, media_type, manifest_media_type, size, extra_attrs, annotations, icon) +values (1003, 1, 'library/hello-world', 'digest1003', 'IMAGE', '2023-06-02 09:16:47.838778', '2023-06-02 01:45:55.050785', 1742, 'application/vnd.docker.container.image.v1+json', 'application/vnd.oci.image.config.v1+json', 4452, '{"architecture":"amd64","author":"","config":{"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"],"Cmd":["/hello"]},"created":"2023-05-04T17:37:03.872958712Z","os":"linux"}', null, '');`, + `insert into tag (id, repository_id, artifact_id, name, push_time, pull_time) values (1001, 1742, 1001, 'latest', '2023-06-02 01:45:55.050785', '2023-06-02 09:16:47.838778')`, + `INSERT INTO artifact_accessory (id, artifact_id, subject_artifact_id, type, size, digest, creation_time, subject_artifact_digest, subject_artifact_repo) VALUES (1001, 1002, 1, 'signature.cosign', 2109, 'sha256:08c64c0de2667abcf3974b4b75b82903f294680b81584318adc4826d0dcb7a9c', '2023-08-03 04:54:32.102928', 'sha256:a97a153152fcd6410bdf4fb64f5622ecf97a753f07dcc89dab14509d059736cf', 'library/nuxeo')`, + `INSERT INTO artifact_reference (id, parent_id, child_id, child_digest, platform, urls, annotations) VALUES (1001, 1001, 1003, 'sha256:d2b2f2980e9ccc570e5726b56b54580f23a018b7b7314c9eaff7e5e479c78657', '{"architecture":"amd64","os":"linux"}', '', null)`, `insert into scanner_registration (name, url, uuid, auth) values('trivy', 'https://www.vmware.com', 'ruuid', 'empty')`, `insert into vulnerability_record (id, cve_id, registration_uuid, cvss_score_v3) values (1, '2023-4567-12345', 'ruuid', 9.8)`, `insert into report_vulnerability_record (report_uuid, vuln_record_id) VALUES ('uuid', 1)`, @@ -66,7 +76,10 @@ values (1, 'library/hello-world', 'digest1001', 'IMAGE', '2023-06-02 09:16:47.8 func (suite *SecurityDaoTestSuite) TearDownTest() { testDao.ExecuteBatchSQL([]string{ `delete from scan_report where uuid = 'uuid'`, + `delete from tag where id = 1001`, `delete from artifact where digest = 'digest1001'`, + `delete from artifact_accessory where id = 1001`, + `delete from artifact_reference where id = 1001`, `delete from scanner_registration where uuid='ruuid'`, `delete from scanner_registration where uuid='uuid2'`, `delete from vulnerability_record where cve_id='2023-4567-12345'`, @@ -178,6 +191,11 @@ func (suite *SecurityDaoTestSuite) TestRangeFilter() { } } +func (suite *SecurityDaoTestSuite) TestCountArtifact() { + count, err := suite.dao.TotalArtifactsCount(suite.Context(), 0) + suite.NoError(err) + suite.Equal(int64(1), count) +} func (suite *SecurityDaoTestSuite) TestCountVul() { count, err := suite.dao.CountVulnerabilities(suite.Context(), "ruuid", 0, true, nil) suite.NoError(err) diff --git a/src/pkg/securityhub/manager.go b/src/pkg/securityhub/manager.go index 679367d62..43deab29e 100644 --- a/src/pkg/securityhub/manager.go +++ b/src/pkg/securityhub/manager.go @@ -34,6 +34,8 @@ type Manager interface { Summary(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) (*model.Summary, error) // DangerousArtifacts returns the most dangerous artifact for the given scanner. DangerousArtifacts(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) ([]*model.DangerousArtifact, error) + // TotalArtifactsCount return the count of artifacts. + TotalArtifactsCount(ctx context.Context, projectID int64) (int64, error) // ScannedArtifactsCount return the count of scanned artifacts. ScannedArtifactsCount(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) (int64, error) // DangerousCVEs returns the most dangerous CVEs for the given scanner. @@ -56,6 +58,10 @@ type securityManager struct { dao dao.SecurityHubDao } +func (s *securityManager) TotalArtifactsCount(ctx context.Context, projectID int64) (int64, error) { + return s.dao.TotalArtifactsCount(ctx, projectID) +} + func (s *securityManager) Summary(ctx context.Context, scannerUUID string, projectID int64, query *q.Query) (*model.Summary, error) { return s.dao.Summary(ctx, scannerUUID, projectID, query) } diff --git a/src/testing/pkg/securityhub/manager.go b/src/testing/pkg/securityhub/manager.go index 027f95e0d..c2d1444d9 100644 --- a/src/testing/pkg/securityhub/manager.go +++ b/src/testing/pkg/securityhub/manager.go @@ -146,6 +146,30 @@ func (_m *Manager) Summary(ctx context.Context, scannerUUID string, projectID in return r0, r1 } +// TotalArtifactsCount provides a mock function with given fields: ctx, projectID +func (_m *Manager) TotalArtifactsCount(ctx context.Context, projectID int64) (int64, error) { + ret := _m.Called(ctx, projectID) + + var r0 int64 + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, int64) (int64, error)); ok { + return rf(ctx, projectID) + } + if rf, ok := ret.Get(0).(func(context.Context, int64) int64); ok { + r0 = rf(ctx, projectID) + } else { + r0 = ret.Get(0).(int64) + } + + if rf, ok := ret.Get(1).(func(context.Context, int64) error); ok { + r1 = rf(ctx, projectID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // TotalVuls provides a mock function with given fields: ctx, scannerUUID, projectID, tuneCount, query func (_m *Manager) TotalVuls(ctx context.Context, scannerUUID string, projectID int64, tuneCount bool, query *q.Query) (int64, error) { ret := _m.Called(ctx, scannerUUID, projectID, tuneCount, query)