fix(codeqlExecuteScan): added waiting for the SARIF file upload (#4409)

* added waiting for the sarif file uploaded & tests

* increased polling time, added timeout for waiting response from server & tests

* fixed handling error while waiting sarif uploaded

* added params for checking sarif uploaded & refactor

* added test logs

* fixed logs and test

* added returning missed error

* changed params descriptions and server response error processing processing

* fixed retrying logic

* increased polling timeout params & refactored

cmd/codeqlExecuteScan.go

@@ -1,11 +1,13 @@
 package cmd
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/SAP/jenkins-library/pkg/codeql"
 	"github.com/SAP/jenkins-library/pkg/command"
@@ -36,6 +38,9 @@ type codeqlExecuteScanUtilsBundle struct {
 	*piperutils.Files
 }
 
+const sarifUploadComplete = "complete"
+const sarifUploadFailed = "failed"
+
 func newCodeqlExecuteScanUtils() codeqlExecuteScanUtils {
 	utils := codeqlExecuteScanUtilsBundle{
 		Command: &command.Command{},
@@ -160,7 +165,7 @@ func getToken(config *codeqlExecuteScanOptions) (bool, string) {
 	return false, ""
 }
 
-func uploadResults(config *codeqlExecuteScanOptions, repoInfo RepoInfo, token string, utils codeqlExecuteScanUtils) error {
+func uploadResults(config *codeqlExecuteScanOptions, repoInfo RepoInfo, token string, utils codeqlExecuteScanUtils) (string, error) {
 	cmd := []string{"github", "upload-results", "--sarif=" + filepath.Join(config.ModulePath, "target", "codeqlReport.sarif")}
 
 	if config.GithubToken != "" {
@@ -185,13 +190,49 @@ func uploadResults(config *codeqlExecuteScanOptions, repoInfo RepoInfo, token st
 
 	//if no git pramas are passed(commitId, reference, serverUrl, repository), then codeql tries to auto populate it based on git information of the checkout repository.
 	//It also depends on the orchestrator. Some orchestrator keep git information and some not.
+
+	var buffer bytes.Buffer
+	utils.Stdout(&buffer)
 	err := execute(utils, cmd, GeneralConfig.Verbose)
 	if err != nil {
 		log.Entry().Error("failed to upload sarif results")
-		return err
+		return "", err
 	}
+	utils.Stdout(log.Writer())
 
-	return nil
+	url := buffer.String()
+	return strings.TrimSpace(url), nil
+}
+
+func waitSarifUploaded(config *codeqlExecuteScanOptions, codeqlSarifUploader codeql.CodeqlSarifUploader) error {
+	maxRetries := config.SarifCheckMaxRetries
+	retryInterval := time.Duration(config.SarifCheckRetryInterval) * time.Second
+
+	log.Entry().Info("waiting for the SARIF to upload")
+	i := 1
+	for {
+		sarifStatus, err := codeqlSarifUploader.GetSarifStatus()
+		if err != nil {
+			return err
+		}
+		log.Entry().Infof("the SARIF processing status: %s", sarifStatus.ProcessingStatus)
+		if sarifStatus.ProcessingStatus == sarifUploadComplete {
+			return nil
+		}
+		if sarifStatus.ProcessingStatus == sarifUploadFailed {
+			for e := range sarifStatus.Errors {
+				log.Entry().Error(e)
+			}
+			return errors.New("failed to upload sarif file")
+		}
+		if i <= maxRetries {
+			log.Entry().Infof("still waiting for the SARIF to upload: retrying in %d seconds... (retry %d/%d)", config.SarifCheckRetryInterval, i, maxRetries)
+			time.Sleep(retryInterval)
+			i++
+			continue
+		}
+		return errors.New("failed to check sarif uploading status: max retries reached")
+	}
 }
 
 func runCodeqlExecuteScan(config *codeqlExecuteScanOptions, telemetryData *telemetry.CustomData, utils codeqlExecuteScanUtils) ([]piperutils.Path, error) {
@@ -275,11 +316,15 @@ func runCodeqlExecuteScan(config *codeqlExecuteScanOptions, telemetryData *telem
 			return reports, errors.New("failed running upload-results as githubToken was not specified")
 		}
 
-		err = uploadResults(config, repoInfo, token, utils)
+		sarifUrl, err := uploadResults(config, repoInfo, token, utils)
 		if err != nil {
-
 			return reports, err
 		}
+		codeqlSarifUploader := codeql.NewCodeqlSarifUploaderInstance(sarifUrl, token)
+		err = waitSarifUploaded(config, &codeqlSarifUploader)
+		if err != nil {
+			return reports, errors.Wrap(err, "failed to upload sarif")
+		}
 
 		if config.CheckForCompliance {
 			codeqlScanAuditInstance := codeql.NewCodeqlScanAuditInstance(repoInfo.serverUrl, repoInfo.owner, repoInfo.repo, token, []string{})
@@ -294,7 +339,7 @@ func runCodeqlExecuteScan(config *codeqlExecuteScanOptions, telemetryData *telem
 				return reports, errors.Wrap(err, "failed to write json compliance report")
 			}
 
-			unaudited := (scanResults.Total - scanResults.Audited)
+			unaudited := scanResults.Total - scanResults.Audited
 			if unaudited > config.VulnerabilityThresholdTotal {
 				msg := fmt.Sprintf("Your repository %v with ref %v is not compliant. Total unaudited issues are %v which is greater than the VulnerabilityThresholdTotal count %v", repoUrl, repoInfo.ref, unaudited, config.VulnerabilityThresholdTotal)
 				return reports, errors.Errorf(msg)

cmd/codeqlExecuteScan_test.go

@@ -6,9 +6,12 @@ package cmd
 import (
 	"fmt"
 	"testing"
+	"time"
 
+	"github.com/SAP/jenkins-library/pkg/codeql"
 	"github.com/SAP/jenkins-library/pkg/mock"
 	"github.com/SAP/jenkins-library/pkg/orchestrator"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -45,12 +48,6 @@ func TestRunCodeqlExecuteScan(t *testing.T) {
 		assert.Error(t, err)
 	})
 
-	t.Run("Check for compliace fails as repository not specified", func(t *testing.T) {
-		config := codeqlExecuteScanOptions{BuildTool: "maven", ModulePath: "./", UploadResults: true, GithubToken: "test", CheckForCompliance: true}
-		_, err := runCodeqlExecuteScan(&config, nil, newCodeqlExecuteScanTestsUtils())
-		assert.Error(t, err)
-	})
-
 	t.Run("Custom buildtool", func(t *testing.T) {
 		config := codeqlExecuteScanOptions{BuildTool: "custom", Language: "javascript", ModulePath: "./"}
 		_, err := runCodeqlExecuteScan(&config, nil, newCodeqlExecuteScanTestsUtils())
@@ -339,3 +336,87 @@ func TestCreateToolRecordCodeql(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestWaitSarifUploaded(t *testing.T) {
+	t.Parallel()
+	config := codeqlExecuteScanOptions{SarifCheckRetryInterval: 1, SarifCheckMaxRetries: 5}
+	t.Run("Fast complete upload", func(t *testing.T) {
+		codeqlScanAuditMock := CodeqlSarifUploaderMock{counter: 0}
+		timerStart := time.Now()
+		err := waitSarifUploaded(&config, &codeqlScanAuditMock)
+		assert.Less(t, time.Now().Sub(timerStart), time.Second)
+		assert.NoError(t, err)
+	})
+	t.Run("Long completed upload", func(t *testing.T) {
+		codeqlScanAuditMock := CodeqlSarifUploaderMock{counter: 2}
+		timerStart := time.Now()
+		err := waitSarifUploaded(&config, &codeqlScanAuditMock)
+		assert.GreaterOrEqual(t, time.Now().Sub(timerStart), time.Second*2)
+		assert.NoError(t, err)
+	})
+	t.Run("Failed upload", func(t *testing.T) {
+		codeqlScanAuditMock := CodeqlSarifUploaderMock{counter: -1}
+		err := waitSarifUploaded(&config, &codeqlScanAuditMock)
+		assert.Error(t, err)
+		assert.ErrorContains(t, err, "failed to upload sarif file")
+	})
+	t.Run("Error while checking sarif uploading", func(t *testing.T) {
+		codeqlScanAuditErrorMock := CodeqlSarifUploaderErrorMock{counter: -1}
+		err := waitSarifUploaded(&config, &codeqlScanAuditErrorMock)
+		assert.Error(t, err)
+		assert.ErrorContains(t, err, "test error")
+	})
+	t.Run("Completed upload after getting errors from server", func(t *testing.T) {
+		codeqlScanAuditErrorMock := CodeqlSarifUploaderErrorMock{counter: 3}
+		err := waitSarifUploaded(&config, &codeqlScanAuditErrorMock)
+		assert.NoError(t, err)
+	})
+	t.Run("Max retries reached", func(t *testing.T) {
+		codeqlScanAuditErrorMock := CodeqlSarifUploaderErrorMock{counter: 6}
+		err := waitSarifUploaded(&config, &codeqlScanAuditErrorMock)
+		assert.Error(t, err)
+		assert.ErrorContains(t, err, "max retries reached")
+	})
+}
+
+type CodeqlSarifUploaderMock struct {
+	counter int
+}
+
+func (c *CodeqlSarifUploaderMock) GetSarifStatus() (codeql.SarifFileInfo, error) {
+	if c.counter == 0 {
+		return codeql.SarifFileInfo{
+			ProcessingStatus: "complete",
+			Errors:           nil,
+		}, nil
+	}
+	if c.counter == -1 {
+		return codeql.SarifFileInfo{
+			ProcessingStatus: "failed",
+			Errors:           []string{"upload error"},
+		}, nil
+	}
+	c.counter--
+	return codeql.SarifFileInfo{
+		ProcessingStatus: "pending",
+		Errors:           nil,
+	}, nil
+}
+
+type CodeqlSarifUploaderErrorMock struct {
+	counter int
+}
+
+func (c *CodeqlSarifUploaderErrorMock) GetSarifStatus() (codeql.SarifFileInfo, error) {
+	if c.counter == -1 {
+		return codeql.SarifFileInfo{}, errors.New("test error")
+	}
+	if c.counter == 0 {
+		return codeql.SarifFileInfo{
+			ProcessingStatus: "complete",
+			Errors:           nil,
+		}, nil
+	}
+	c.counter--
+	return codeql.SarifFileInfo{ProcessingStatus: "Service unavailable"}, nil
+}

pkg/codeql/codeql_test.go

@@ -56,23 +56,13 @@ func (g *githubCodeqlScanningMock) ListAlertsForRepo(ctx context.Context, owner,
 	return alerts, &response, nil
 }
 
-func (g *githubCodeqlScanningMock) ListAnalysesForRepo(ctx context.Context, owner, repo string, opts *github.AnalysesListOptions) ([]*github.ScanningAnalysis, *github.Response, error) {
-	resultsCount := 3
-	analysis := []*github.ScanningAnalysis{{ResultsCount: &resultsCount}}
-	return analysis, nil, nil
-}
-
 type githubCodeqlScanningErrorMock struct {
 }
 
 func (g *githubCodeqlScanningErrorMock) ListAlertsForRepo(ctx context.Context, owner, repo string, opts *github.AlertListOptions) ([]*github.Alert, *github.Response, error) {
 	return []*github.Alert{}, nil, errors.New("Some error")
 }
 
-func (g *githubCodeqlScanningErrorMock) ListAnalysesForRepo(ctx context.Context, owner, repo string, opts *github.AnalysesListOptions) ([]*github.ScanningAnalysis, *github.Response, error) {
-	return []*github.ScanningAnalysis{}, nil, errors.New("Some error")
-}
-
 func TestGetVulnerabilitiesFromClient(t *testing.T) {
 	ctx := context.Background()
 	t.Parallel()

pkg/codeql/sarif_upload.go

@@ -0,0 +1,68 @@
+package codeql
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+)
+
+type CodeqlSarifUploader interface {
+	GetSarifStatus() (SarifFileInfo, error)
+}
+
+func NewCodeqlSarifUploaderInstance(url, token string) CodeqlSarifUploaderInstance {
+	return CodeqlSarifUploaderInstance{
+		url:   url,
+		token: token,
+	}
+}
+
+type CodeqlSarifUploaderInstance struct {
+	url   string
+	token string
+}
+
+func (codeqlSarifUploader *CodeqlSarifUploaderInstance) GetSarifStatus() (SarifFileInfo, error) {
+	return getSarifUploadingStatus(codeqlSarifUploader.url, codeqlSarifUploader.token)
+}
+
+type SarifFileInfo struct {
+	ProcessingStatus string   `json:"processing_status"`
+	Errors           []string `json:"errors"`
+}
+
+const internalServerError = "Internal server error"
+
+func getSarifUploadingStatus(sarifURL, token string) (SarifFileInfo, error) {
+	client := http.Client{}
+	req, err := http.NewRequest("GET", sarifURL, nil)
+	if err != nil {
+		return SarifFileInfo{}, err
+	}
+	req.Header.Add("Authorization", "Bearer "+token)
+	req.Header.Add("Accept", "application/vnd.github+json")
+	req.Header.Add("X-GitHub-Api-Version", "2022-11-28")
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return SarifFileInfo{}, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode == http.StatusServiceUnavailable || resp.StatusCode == http.StatusBadGateway ||
+		resp.StatusCode == http.StatusGatewayTimeout {
+		return SarifFileInfo{ProcessingStatus: internalServerError}, nil
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return SarifFileInfo{}, err
+	}
+
+	sarifInfo := SarifFileInfo{}
+	err = json.Unmarshal(body, &sarifInfo)
+	if err != nil {
+		return SarifFileInfo{}, err
+	}
+	return sarifInfo, nil
+}

resources/metadata/codeqlExecuteScan.yaml

@@ -104,6 +104,22 @@ spec:
           - STAGES
           - STEPS
         default: false
+      - name: sarifCheckMaxRetries
+        type: int
+        description: "Maximum number of retries when waiting for the server to finish processing the SARIF upload. Only relevant, if checkForCompliance is enabled."
+        scope:
+          - PARAMETERS
+          - STAGES
+          - STEPS
+        default: 10
+      - name: sarifCheckRetryInterval
+        type: int
+        descriptoin: "Interval in seconds between retries when waiting for the server to finish processing the SARIF upload. Only relevant, if checkForCompliance is enabled."
+        scope:
+          - PARAMETERS
+          - STAGES
+          - STEPS
+        default: 30
       - name: threads
         type: string
         description: "Use this many threads for the codeql operations."