diff --git a/src/controller/scandataexport/execution.go b/src/controller/scandataexport/execution.go index 17227c8d8..2a36f30c0 100644 --- a/src/controller/scandataexport/execution.go +++ b/src/controller/scandataexport/execution.go @@ -185,6 +185,11 @@ func (c *controller) convertToExportExecStatus(ctx context.Context, exec *task.E if userName, ok := exec.ExtraAttrs[export.UserNameAttribute]; ok { execStatus.UserName = userName.(string) } + // Do not define the 'status_message' as const because this is not the final solution, + // should refactor this part. + if statusMessage, ok := exec.ExtraAttrs["status_message"]; ok { + execStatus.StatusMessage = statusMessage.(string) + } artifactExists := c.isCsvArtifactPresent(ctx, exec.ID, execStatus.ExportDataDigest) execStatus.FilePresent = artifactExists return execStatus diff --git a/src/controller/scandataexport/execution_test.go b/src/controller/scandataexport/execution_test.go index b8b597832..6f536393f 100644 --- a/src/controller/scandataexport/execution_test.go +++ b/src/controller/scandataexport/execution_test.go @@ -97,6 +97,7 @@ func (suite *ScanDataExportExecutionTestSuite) TestGetExecution() { attrs := make(map[string]interface{}) attrs[export.JobNameAttribute] = "test-job" attrs[export.UserNameAttribute] = "test-user" + attrs["status_message"] = "test-message" { exec := task.Execution{ ID: 100, @@ -119,6 +120,7 @@ func (suite *ScanDataExportExecutionTestSuite) TestGetExecution() { suite.Equal(exec.ID, exportExec.ID) suite.Equal("test-user", exportExec.UserName) suite.Equal("test-job", exportExec.JobName) + suite.Equal("test-message", exportExec.StatusMessage) suite.Equal(true, exportExec.FilePresent) } diff --git a/src/jobservice/job/impl/scandataexport/scan_data_export.go b/src/jobservice/job/impl/scandataexport/scan_data_export.go index 3f4a23dab..92b65c127 100644 --- a/src/jobservice/job/impl/scandataexport/scan_data_export.go +++ b/src/jobservice/job/impl/scandataexport/scan_data_export.go @@ -114,12 +114,24 @@ func (sde *ScanDataExport) Run(ctx job.Context, params job.Parameters) error { if err != nil { logger.Errorf( "Export Job Id = %v. Error when persisting report file %s to persistent storage: %v", params["JobId"], fileName, err) + // NOTICE: this is a tentative solution to resolve error to push empty blob to S3 storage driver, + // should unify the behaviour for different drivers. + // Temporary set the status message to extra attributes, then the API handler will fetch it and combined to response for better experience. + if stat.Size() == 0 { + extra := map[string]interface{}{ + "status_message": "No vulnerabilities found or matched", + } + updateErr := sde.updateExecAttributes(ctx, params, extra) + if updateErr != nil { + logger.Errorf("Export Job Id = %v. Error when updating the exec extra attributes 'status_message' to 'No vulnerabilities found or matched': %v", params["JobId"], updateErr) + } + } + return err } logger.Infof("Export Job Id = %v. Created system artifact: %v for report file %s to persistent storage: %v", params["JobId"], artID, fileName, err) - err = sde.updateExecAttributes(ctx, params, err, hash) - + err = sde.updateExecAttributes(ctx, params, map[string]interface{}{export.DigestKey: hash.String()}) if err != nil { logger.Errorf("Export Job Id = %v. Error when updating execution record : %v", params["JobId"], err) return err @@ -129,7 +141,7 @@ func (sde *ScanDataExport) Run(ctx job.Context, params job.Parameters) error { return nil } -func (sde *ScanDataExport) updateExecAttributes(ctx job.Context, params job.Parameters, err error, hash digest.Digest) error { +func (sde *ScanDataExport) updateExecAttributes(ctx job.Context, params job.Parameters, attrs map[string]interface{}) error { execID := int64(params["JobId"].(float64)) exec, err := sde.execMgr.Get(ctx.SystemContext(), execID) logger := ctx.GetLogger() @@ -137,11 +149,11 @@ func (sde *ScanDataExport) updateExecAttributes(ctx job.Context, params job.Para logger.Errorf("Export Job Id = %v. Error when fetching execution record for update : %v", params["JobId"], err) return err } - attrsToUpdate := make(map[string]interface{}) - for k, v := range exec.ExtraAttrs { + // copy old extra + attrsToUpdate := exec.ExtraAttrs + for k, v := range attrs { attrsToUpdate[k] = v } - attrsToUpdate[export.DigestKey] = hash.String() return sde.execMgr.UpdateExtraAttrs(ctx.SystemContext(), execID, attrsToUpdate) } diff --git a/src/jobservice/job/impl/scandataexport/scan_data_export_test.go b/src/jobservice/job/impl/scandataexport/scan_data_export_test.go index d0d68632f..654ec97de 100644 --- a/src/jobservice/job/impl/scandataexport/scan_data_export_test.go +++ b/src/jobservice/job/impl/scandataexport/scan_data_export_test.go @@ -106,6 +106,40 @@ func (suite *ScanDataExportJobTestSuite) TestRun() { } +func (suite *ScanDataExportJobTestSuite) TestRunCreateSysArtError() { + oldSysArtMgr := suite.sysArtifactMgr + defer func() { + suite.job.sysArtifactMgr = oldSysArtMgr + }() + + sysArtMgr := &systemartifacttesting.Manager{} + suite.job.sysArtifactMgr = sysArtMgr + sysArtMgr.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(int64(-1), errors.New("create sys artifact error")).Once() + // mock create system artifact error when file is empty + var data []export.Data + mock.OnAnything(suite.exportMgr, "Fetch").Return(data, nil).Once() + mock.OnAnything(suite.exportMgr, "Fetch").Return(make([]export.Data, 0), nil).Once() + mock.OnAnything(suite.digestCalculator, "Calculate").Return(digest.Digest(MockDigest), nil) + + execAttrs := make(map[string]interface{}) + execAttrs[export.JobNameAttribute] = "test-job" + execAttrs[export.UserNameAttribute] = "test-user" + mock.OnAnything(suite.execMgr, "Get").Return(&task.Execution{ID: int64(JobId), ExtraAttrs: execAttrs}, nil) + + params := job.Parameters{} + params[export.JobModeKey] = export.JobModeExport + params["JobId"] = JobId + ctx := &mockjobservice.MockJobContext{} + + err := suite.job.Run(ctx, params) + suite.Error(err) + + extraAttrsMatcher := testifymock.MatchedBy(func(attrsMap map[string]interface{}) bool { + return attrsMap["status_message"] == "No vulnerabilities found or matched" && attrsMap[export.JobNameAttribute] == "test-job" && attrsMap[export.UserNameAttribute] == "test-user" + }) + suite.execMgr.AssertCalled(suite.T(), "UpdateExtraAttrs", mock.Anything, int64(JobId), extraAttrsMatcher) +} + func (suite *ScanDataExportJobTestSuite) TestRunAttributeUpdateError() { data := suite.createDataRecords(3, 1) diff --git a/src/server/v2.0/handler/scanexport.go b/src/server/v2.0/handler/scanexport.go index e4f29192e..6b555f6fc 100644 --- a/src/server/v2.0/handler/scanexport.go +++ b/src/server/v2.0/handler/scanexport.go @@ -132,6 +132,10 @@ func (se *scanDataExportAPI) GetScanDataExportExecution(ctx context.Context, par UserName: execution.UserName, FilePresent: execution.FilePresent, } + // add human friendly message when status is error + if sdeExec.Status == job.ErrorStatus.String() && sdeExec.StatusText == "" { + sdeExec.StatusText = "Please contact the system administrator to check the logs of jobservice." + } return operation.NewGetScanDataExportExecutionOK().WithPayload(&sdeExec) } @@ -226,7 +230,7 @@ func (se *scanDataExportAPI) GetScanDataExportExecutionList(ctx context.Context, FilePresent: execution.FilePresent, } // add human friendly message when status is error - if sdeExec.Status == job.ErrorStatus.String() { + if sdeExec.Status == job.ErrorStatus.String() && sdeExec.StatusText == "" { sdeExec.StatusText = "Please contact the system administrator to check the logs of jobservice." } // store project ids diff --git a/src/server/v2.0/handler/scanexport_test.go b/src/server/v2.0/handler/scanexport_test.go index 01dfff7a7..6130813dc 100644 --- a/src/server/v2.0/handler/scanexport_test.go +++ b/src/server/v2.0/handler/scanexport_test.go @@ -350,16 +350,18 @@ func (suite *ScanExportTestSuite) TestExportScanDataNoPrivileges() { func (suite *ScanExportTestSuite) TestGetScanDataExportExecution() { suite.Security.On("GetUsername").Return("test-user") - suite.Security.On("IsAuthenticated").Return(true).Once() - suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once() + suite.Security.On("IsAuthenticated").Return(true).Twice() + suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Twice() url := "/export/cve/execution/100" endTime := time.Now() startTime := endTime.Add(-10 * time.Minute) + defaultStatusMessage := "Please contact the system administrator to check the logs of jobservice." + customizeStatusMessage := "No vulnerabilities found or matched" execution := &export.Execution{ ID: 100, UserID: 3, - Status: "Success", + Status: "Error", StatusMessage: "", Trigger: "MANUAL", StartTime: startTime, @@ -377,7 +379,17 @@ func (suite *ScanExportTestSuite) TestGetScanDataExportExecution() { json.NewDecoder(res.Body).Decode(&respData) suite.Equal("test-user", respData.UserName) suite.Equal(false, respData.FilePresent) + suite.Equal(defaultStatusMessage, respData.StatusText) + // test customize status message + execution.StatusMessage = customizeStatusMessage + mock.OnAnything(suite.scanExportCtl, "GetExecution").Return(execution, nil).Once() + res, err = suite.DoReq(http.MethodGet, url, nil) + suite.Equal(200, res.StatusCode) + suite.Equal(nil, err) + respData = models.ScanDataExportExecution{} + json.NewDecoder(res.Body).Decode(&respData) + suite.Equal(customizeStatusMessage, respData.StatusText) } func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionUserNotOwnerOfExport() { @@ -567,19 +579,21 @@ func (suite *ScanExportTestSuite) TestDownloadScanDataExecutionError() { func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionList() { suite.Security.On("GetUsername").Return("test-user") - suite.Security.On("IsAuthenticated").Return(true).Once() - suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once() + suite.Security.On("IsAuthenticated").Return(true).Twice() + suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Twice() url, err := url2.Parse("/export/cve/executions") params := url2.Values{} params.Add("user_name", "test-user") url.RawQuery = params.Encode() endTime := time.Now() startTime := endTime.Add(-10 * time.Minute) + defaultStatusMessage := "Please contact the system administrator to check the logs of jobservice." + customizeStatusMessage := "No vulnerabilities found or matched" execution := &export.Execution{ ID: 100, UserID: 3, - Status: "Success", + Status: "Error", StatusMessage: "", Trigger: "MANUAL", StartTime: startTime, @@ -597,6 +611,18 @@ func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionList() { json.NewDecoder(res.Body).Decode(&respData) suite.Equal(1, len(respData.Items)) suite.Equal(int64(100), respData.Items[0].ID) + suite.Equal(defaultStatusMessage, respData.Items[0].StatusText) + // test customize status message + execution.StatusMessage = customizeStatusMessage + mock.OnAnything(suite.scanExportCtl, "ListExecutions").Return([]*export.Execution{execution}, nil).Once() + res, err = suite.DoReq(http.MethodGet, url.String(), nil) + suite.Equal(200, res.StatusCode) + suite.Equal(nil, err) + respData = models.ScanDataExportExecutionList{} + json.NewDecoder(res.Body).Decode(&respData) + suite.Equal(1, len(respData.Items)) + suite.Equal(int64(100), respData.Items[0].ID) + suite.Equal(customizeStatusMessage, respData.Items[0].StatusText) } func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionListFilterNotOwned() {