mirror of
https://github.com/goharbor/harbor.git
synced 2024-09-29 13:57:33 +02:00
fix: add human friendly message when export cve job failure (#17726)
Add human friendly when export CVE in the condition of empty CSV file, because this file will be stored as system artifact and pushed to distribution, but it will leads to error when push empty blob to S3 storage driver. Signed-off-by: chlins <chenyuzh@vmware.com>
This commit is contained in:
parent
0edc01a395
commit
bf4cfe9e1e
@ -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
|
||||
|
@ -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)
|
||||
}
|
||||
|
||||
|
@ -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)
|
||||
}
|
||||
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
|
@ -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() {
|
||||
|
Loading…
Reference in New Issue
Block a user