Harden SonarQube integration

* Do not generate reports when pull request exists for scanned branch.
For more information, see
opendevstack/ods-jenkins-shared-library#663
and cnescatlab/sonar-cnes-report#159.

* Ensure background task on server finishes before generating a
report. For more information, see
opendevstack/ods-jenkins-shared-library#732.

* Unify logging approach: instead of printing to STDOUT directly for
some messages, funnel everything through the logger instance. Other
tasks should adopt this as well.

Closes #227.

cmd/sonar/main.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/opendevstack/pipeline/pkg/logging"
 	"github.com/opendevstack/pipeline/pkg/pipelinectxt"
@@ -18,12 +19,18 @@ type options struct {
 	sonarURL       string
 	sonarEdition   string
 	workingDir     string
+	rootPath       string
 	qualityGate    bool
 	debug          bool
 }
 
 func main() {
-	opts := options{}
+	rootPath, err := filepath.Abs(".")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	opts := options{rootPath: rootPath}
 	flag.StringVar(&opts.sonarAuthToken, "sonar-auth-token", os.Getenv("SONAR_AUTH_TOKEN"), "sonar-auth-token")
 	flag.StringVar(&opts.sonarURL, "sonar-url", os.Getenv("SONAR_URL"), "sonar-url")
 	flag.StringVar(&opts.sonarEdition, "sonar-edition", os.Getenv("SONAR_EDITION"), "sonar-edition")
@@ -38,22 +45,15 @@ func main() {
 	}
 
 	ctxt := &pipelinectxt.ODSContext{}
-	err := ctxt.ReadCache(".")
-	if err != nil {
-		log.Fatal(err)
-	}
-	rootPath, err := filepath.Abs(".")
+	err = ctxt.ReadCache(".")
 	if err != nil {
 		log.Fatal(err)
 	}
+
 	err = os.Chdir(opts.workingDir)
 	if err != nil {
 		log.Fatal(err)
 	}
-	artifactPrefix := ""
-	if opts.workingDir != "." {
-		artifactPrefix = strings.Replace(opts.workingDir, "/", "-", -1) + "-"
-	}
 
 	sonarClient := sonar.NewClient(&sonar.ClientConfig{
 		APIToken:      opts.sonarAuthToken,
@@ -63,61 +63,129 @@ func main() {
 		Logger:        logger,
 	})
 
+	err = sonarScan(logger, opts, ctxt, sonarClient)
+	if err != nil {
+		log.Fatal(err)
+	}
+}
+
+func sonarScan(
+	logger logging.LeveledLoggerInterface,
+	opts options,
+	ctxt *pipelinectxt.ODSContext,
+	sonarClient sonar.ClientInterface) error {
+	artifactPrefix := ""
+	if opts.workingDir != "." {
+		artifactPrefix = strings.Replace(opts.workingDir, "/", "-", -1) + "-"
+	}
+
 	sonarProject := sonar.ProjectKey(ctxt, artifactPrefix)
 
-	fmt.Println("Scanning with sonar-scanner ...")
+	logger.Infof("Scanning with sonar-scanner ...\n")
 	var prInfo *sonar.PullRequest
 	if len(ctxt.PullRequestKey) > 0 && ctxt.PullRequestKey != "0" && len(ctxt.PullRequestBase) > 0 {
+		logger.Infof("Pull request (ID %s) detected.\n", ctxt.PullRequestKey)
 		prInfo = &sonar.PullRequest{
 			Key:    ctxt.PullRequestKey,
 			Branch: ctxt.GitRef,
 			Base:   ctxt.PullRequestBase,
 		}
 	}
-	stdout, err := sonarClient.Scan(
+	scanStdout, err := sonarClient.Scan(
 		sonarProject,
 		ctxt.GitRef,
 		ctxt.GitCommitSHA,
 		prInfo,
 	)
 	if err != nil {
-		fmt.Println(stdout)
-		fmt.Println(err)
-		os.Exit(1)
+		logger.Infof("%s\n", scanStdout)
+		return fmt.Errorf("scan failed: %w", err)
 	}
-	fmt.Println(stdout)
+	logger.Infof("%s\n", scanStdout)
 
-	fmt.Println("Generating reports ...")
-	stdout, err = sonarClient.GenerateReports(
-		sonarProject,
-		"OpenDevStack",
-		ctxt.GitRef,
-		rootPath,
-		artifactPrefix,
-	)
+	logger.Infof("Wait until compute engine task finishes ...")
+	err = waitUntilComputeEngineTaskIsSuccessful(logger, sonarClient)
 	if err != nil {
-		fmt.Println(stdout)
-		fmt.Println(err)
-		os.Exit(1)
+		return fmt.Errorf("background task did not finish successfully: %w", err)
+	}
+
+	if prInfo == nil {
+		logger.Infof("Generating reports ...\n")
+		reportStdout, err := sonarClient.GenerateReports(
+			sonarProject,
+			"OpenDevStack",
+			ctxt.GitRef,
+			opts.rootPath,
+			artifactPrefix,
+		)
+		if err != nil {
+			logger.Infof("%s\n", reportStdout)
+			logger.Infof("%s\n", err)
+			os.Exit(1)
+		}
+		logger.Infof("%s\n", reportStdout)
+	} else {
+		logger.Infof("No reports are generated for pull request scans.\n")
 	}
-	fmt.Println(stdout)
 
 	if opts.qualityGate {
-		fmt.Println("Checking quality gate ...")
+		logger.Infof("Checking quality gate ...\n")
 		qualityGateResult, err := sonarClient.QualityGateGet(
 			sonar.QualityGateGetParams{Project: sonarProject},
 		)
 		if err != nil {
-			log.Fatalln(err)
+			return fmt.Errorf("quality gate could not be retrieved: %w", err)
 		}
 		actualStatus := qualityGateResult.ProjectStatus.Status
 		if actualStatus != sonar.QualityGateStatusOk {
-			log.Fatalf(
-				"Quality gate status is '%s', not '%s'\n",
+			return fmt.Errorf(
+				"quality gate status is '%s', not '%s'",
 				actualStatus, sonar.QualityGateStatusOk,
 			)
 		} else {
-			fmt.Println("Quality gate passed")
+			logger.Infof("Quality gate passed.\n")
+		}
+	}
+
+	return nil
+}
+
+// waitUntilComputeEngineTaskIsSuccessful reads the scanner report file and
+// extracts the task ID. It then waits until the corresponding background task
+// in SonarQube succeeds. If the tasks fails or the timeout is reached, an
+// error is returned.
+func waitUntilComputeEngineTaskIsSuccessful(logger logging.LeveledLoggerInterface, sonarClient sonar.ClientInterface) error {
+	reportTaskID, err := sonarClient.ExtractComputeEngineTaskID(sonar.ReportTaskFile)
+	if err != nil {
+		return fmt.Errorf("cannot read task ID: %w", err)
+	}
+	params := sonar.ComputeEngineTaskGetParams{ID: reportTaskID}
+	attempts := 5
+	sleep := time.Second
+	for i := 0; i < attempts; i++ {
+		if i > 0 {
+			logger.Infof("Waiting %s before checking task status again ...", sleep)
+			time.Sleep(sleep)
+			sleep *= 2
+		}
+		task, err := sonarClient.ComputeEngineTaskGet(params)
+		if err != nil {
+			logger.Infof("cannot get status of task: %w", err)
+			continue
+		}
+		switch task.Status {
+		case sonar.TaskStatusInProgress:
+			logger.Infof("Background task %s has not finished yet", reportTaskID)
+		case sonar.TaskStatusPending:
+			logger.Infof("Background task %s has not started yet", reportTaskID)
+		case sonar.TaskStatusFailed:
+			return fmt.Errorf("background task %s has failed", reportTaskID)
+		case sonar.TaskStatusSuccess:
+			logger.Infof("Background task %s has finished successfully", reportTaskID)
+			return nil
+		default:
+			logger.Infof("Background task %s has unknown status %s", reportTaskID, task.Status)
 		}
 	}
+	return fmt.Errorf("background task %s did not succeed within timeout", reportTaskID)
 }

cmd/sonar/main_test.go

@@ -0,0 +1,152 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/opendevstack/pipeline/pkg/logging"
+	"github.com/opendevstack/pipeline/pkg/pipelinectxt"
+	"github.com/opendevstack/pipeline/pkg/sonar"
+)
+
+type fakeClient struct {
+	scanPerformed        bool
+	passQualityGate      bool
+	qualityGateRetrieved bool
+	reportGenerated      bool
+}
+
+func (c *fakeClient) Scan(sonarProject, branch, commit string, pr *sonar.PullRequest) (string, error) {
+	c.scanPerformed = true
+	return "", nil
+}
+
+func (c *fakeClient) QualityGateGet(p sonar.QualityGateGetParams) (*sonar.QualityGate, error) {
+	c.qualityGateRetrieved = true
+	status := sonar.QualityGateStatusError
+	if c.passQualityGate {
+		status = sonar.QualityGateStatusOk
+	}
+	return &sonar.QualityGate{ProjectStatus: sonar.QualityGateProjectStatus{Status: status}}, nil
+}
+
+func (c *fakeClient) GenerateReports(sonarProject, author, branch, rootPath, artifactPrefix string) (string, error) {
+	c.reportGenerated = true
+	return "", nil
+}
+
+func (c *fakeClient) ExtractComputeEngineTaskID(filename string) (string, error) {
+	return "abc123", nil
+}
+
+func (c *fakeClient) ComputeEngineTaskGet(params sonar.ComputeEngineTaskGetParams) (*sonar.ComputeEngineTask, error) {
+	return &sonar.ComputeEngineTask{Status: sonar.TaskStatusSuccess}, nil
+}
+
+func TestSonarScan(t *testing.T) {
+	logger := &logging.LeveledLogger{Level: logging.LevelDebug}
+
+	tests := map[string]struct {
+		// which SQ edition is in use
+		optSonarEdition string
+		// whether quality gate is required to pass
+		optQualityGate bool
+
+		// PR key
+		ctxtPrKey string
+		// PR base
+		ctxtPrBase string
+
+		// whether the quality gate in SQ passes (faked)
+		passQualityGate bool
+
+		// whether scan should have been performed
+		wantScanPerformed bool
+		// whether report should have been generated
+		wantReportGenerated bool
+		// whether quality gate should have been retrieved
+		wantQualityGateRetrieved bool
+		// whether scanning should fail - if not empty, the actual error message
+		// will be checked to contain wantErr.
+		wantErr string
+	}{
+		"developer edition generates report when no PR is present": {
+			optSonarEdition:          "developer",
+			optQualityGate:           true,
+			ctxtPrKey:                "",
+			ctxtPrBase:               "",
+			passQualityGate:          true,
+			wantScanPerformed:        true,
+			wantReportGenerated:      true,
+			wantQualityGateRetrieved: true,
+		},
+		"developer edition does not generate report when PR is present": {
+			optSonarEdition:          "developer",
+			optQualityGate:           true,
+			ctxtPrKey:                "3",
+			ctxtPrBase:               "master",
+			passQualityGate:          true,
+			wantScanPerformed:        true,
+			wantReportGenerated:      false,
+			wantQualityGateRetrieved: true,
+		},
+		"community edition generates report": {
+			optSonarEdition:          "community",
+			optQualityGate:           true,
+			ctxtPrKey:                "",
+			ctxtPrBase:               "",
+			passQualityGate:          true,
+			wantScanPerformed:        true,
+			wantReportGenerated:      true,
+			wantQualityGateRetrieved: true,
+		},
+		"does not check quality gate if disabled": {
+			optSonarEdition:          "community",
+			optQualityGate:           false,
+			ctxtPrKey:                "",
+			ctxtPrBase:               "",
+			passQualityGate:          true,
+			wantScanPerformed:        true,
+			wantReportGenerated:      true,
+			wantQualityGateRetrieved: false,
+		},
+		"fails if quality gate does not pass": {
+			optSonarEdition:          "community",
+			optQualityGate:           true,
+			ctxtPrKey:                "",
+			ctxtPrBase:               "",
+			passQualityGate:          false,
+			wantScanPerformed:        true,
+			wantReportGenerated:      true,
+			wantQualityGateRetrieved: true,
+			wantErr:                  "quality gate status is 'ERROR', not 'OK'",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			opts := options{
+				sonarEdition: tc.optSonarEdition,
+				qualityGate:  tc.optQualityGate,
+			}
+			ctxt := &pipelinectxt.ODSContext{PullRequestKey: tc.ctxtPrKey, PullRequestBase: tc.ctxtPrBase}
+			sonarClient := &fakeClient{passQualityGate: tc.passQualityGate}
+			err := sonarScan(logger, opts, ctxt, sonarClient)
+			if err != nil {
+				if tc.wantErr == "" || !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("want err to contain: %s, got err: %s", tc.wantErr, err)
+				}
+			}
+			if sonarClient.scanPerformed != tc.wantScanPerformed {
+				t.Fatalf("want scan performed to be %v, got %v", tc.wantScanPerformed, sonarClient.scanPerformed)
+			}
+			if sonarClient.reportGenerated != tc.wantReportGenerated {
+				t.Fatalf("want report generated to be %v, got %v", tc.wantReportGenerated, sonarClient.reportGenerated)
+			}
+			if sonarClient.qualityGateRetrieved != tc.wantQualityGateRetrieved {
+				t.Fatalf("want quality gate retrieved to be %v, got %v", tc.wantQualityGateRetrieved, sonarClient.qualityGateRetrieved)
+			}
+		})
+	}
+
+}

deploy/central/tasks-chart/templates/task-ods-build-go-with-sidecar.yaml

@@ -25,9 +25,9 @@ spec:
     When `sonar-quality-gate` is set to `true`, the task will fail if the quality gate
     is not passed. If SonarQube is not desired, it can be disabled via `sonar-skip`.
     The SonarQube scan will include parameters to perform a pull request analysis if
-    there is an open pull request for the branch being built. Pull request decoration
-    in Bitbucket is done automatically by SonarQube provided the ALM integration is setup
-    properly in SonarQube.
+    there is an open pull request for the branch being built. If the
+    link:https://docs.sonarqube.org/latest/analysis/bitbucket-integration/[ALM integration]
+    is setup properly, pull request decoration in Bitbucket is done automatically.
 
     **Sidecar variant!** Use this task if you need to run a container next to the build task.
     For example, this could be used to run a database to allow for integration tests.

deploy/central/tasks-chart/templates/task-ods-build-go.yaml

@@ -23,9 +23,9 @@ spec:
     When `sonar-quality-gate` is set to `true`, the task will fail if the quality gate
     is not passed. If SonarQube is not desired, it can be disabled via `sonar-skip`.
     The SonarQube scan will include parameters to perform a pull request analysis if
-    there is an open pull request for the branch being built. Pull request decoration
-    in Bitbucket is done automatically by SonarQube provided the ALM integration is setup
-    properly in SonarQube.
+    there is an open pull request for the branch being built. If the
+    link:https://docs.sonarqube.org/latest/analysis/bitbucket-integration/[ALM integration]
+    is setup properly, pull request decoration in Bitbucket is done automatically.
   params:
     - name: working-dir
       description: |