diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index 939840c9a..dfac471d0 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -42,6 +42,8 @@ If there are delays in the scan processing (either because the scanned project i the scan results are not available by the time the attest sonar command is executed. In this case you can use the ^--max-wait^ flag to retry the command while waiting for the scan to be processed. This flag takes the maximum number of seconds to wait for the results to be available. The Kosli CLI will then attempt to retrieve the scan results until the maximum wait time is reached, with exponential backoff between retries. Once the results are available they are attested to Kosli as usual. +On SonarQube Cloud, only the latest scan of a short-lived branch records its commit. So this way, or with ^--sonar-ce-task-url^, fails if another scan of the project +has finished since, or another scan of the branch is still being processed. Then use ^--sonar-project-key^ and ^--sonar-revision^ instead. 2. Providing the Sonar project key and either the revision or the pull-request ID of the scan (plus the SonarQube server URL if relevant). For branch scans: if running the Kosli CLI in some CI/CD pipeline, the revision is defaulted to the commit SHA. If you are running the command locally, diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index 6161e9a2f..22bc5d9d4 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -102,6 +102,13 @@ type ActivityResponse struct { Tasks []Task `json:"tasks"` } +// ComponentTasksResponse is the response from the ce/component API +type ComponentTasksResponse struct { + Queue []Task `json:"queue"` + Current Task `json:"current"` + Errors []Error `json:"errors,omitempty"` +} + // These are the structs for the response from the project_analyses/search API type ProjectAnalyses struct { Analyses []Analysis `json:"analyses"` @@ -459,14 +466,7 @@ func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *Sonar if projectAnalysesData.Errors != nil { sonarErr := fmt.Errorf("SonarQube error: %s", projectAnalysesData.Errors[0].Msg) - if sonarResults.Branch == nil { - return "", sonarErr - } - isShort, err := shortLivedBranchAnalysis(httpClient, sonarResults, project, revision) - if !isShort { - return "", sonarErr - } - return "", err + return "", shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarErr, isOfRevision(project, revision)) } for analysis := range projectAnalysesData.Analyses { @@ -496,25 +496,30 @@ func gateIsBranchLatest(sonarResults *SonarResults, analysisID string) bool { return analysisID == "" && sonarResults.Branch != nil } -// shortLivedBranchAnalysis finds the analysis of revision on a SonarCloud short-lived -// branch, which api/project_analyses/search refuses. SonarCloud keeps only the latest -// analysis of such a branch, so that analysis must be of the revision. It reports -// false, so the caller keeps SonarQube's own error, when the branch cannot be looked -// up or is not a listed short-lived branch. -func shortLivedBranchAnalysis(httpClient *http.Client, sonarResults *SonarResults, project *Project, revision string) (bool, error) { +// shortLivedBranchAnalysis takes the revision and date of the analysis being attested +// from a SonarCloud short-lived branch, whose analyses api/project_analyses/search +// refuses. api/project_branches/list gives only the branch's latest analysis, so +// isAttested decides whether that is the one being attested. It returns fallbackErr, +// SonarQube's own error, when the branch cannot be looked up or is not a listed +// short-lived branch. +func shortLivedBranchAnalysis(httpClient *http.Client, sonarResults *SonarResults, project *Project, fallbackErr error, isAttested func(branch BranchInfo) error) error { + if sonarResults.Branch == nil { + return fallbackErr + } branch, err := listedShortLivedBranch(httpClient, sonarResults, project) if err != nil || branch == nil { - return false, nil + return fallbackErr } if branch.Commit.SHA == "" { - return true, fmt.Errorf("analysis for revision %s of project %s not found: branch %s is short-lived on SonarQube Cloud and has no analysis yet.\nThe scan may still be being processed by SonarQube, try again later", revision, project.Key, branch.Name) + return fmt.Errorf("branch %s of project %s is short-lived on SonarQube Cloud and has no analysis yet.\nThe scan may still be being processed by SonarQube, try again later", branch.Name, project.Key) } - if branch.Commit.SHA != revision { - return true, fmt.Errorf("analysis for revision %s of project %s not found: branch %s is short-lived on SonarQube Cloud, which keeps only its latest analysis, and that is of revision %s.\nThe scan for your revision may still be being processed by SonarQube, try again later.\nOtherwise check the revision is correct, or analyse the branch as long-lived: SonarQube Cloud decides this from the branch name when the branch is first analysed", revision, project.Key, branch.Name, branch.Commit.SHA) + if err := isAttested(*branch); err != nil { + return err } + sonarResults.Revision = branch.Commit.SHA sonarResults.AnalysedAt = branch.AnalysisDate sonarResults.Branch.Type = branch.Type - return true, nil + return nil } // listedShortLivedBranch returns the branch from api/project_branches/list when @@ -542,6 +547,62 @@ func listedShortLivedBranch(httpClient *http.Client, sonarResults *SonarResults, return nil, nil } +// isOfRevision accepts the branch's latest analysis when it is of revision. SonarCloud +// keeps only that analysis, so any other revision's is gone or not processed yet. +func isOfRevision(project *Project, revision string) func(BranchInfo) error { + return func(branch BranchInfo) error { + if branch.Commit.SHA == revision { + return nil + } + return fmt.Errorf("analysis for revision %s of project %s not found: branch %s is short-lived on SonarQube Cloud, which keeps only its latest analysis, and that is of revision %s.\nThe scan for your revision may still be being processed by SonarQube, try again later.\nOtherwise check the revision is correct, or analyse the branch as long-lived: SonarQube Cloud decides this from the branch name when the branch is first analysed", revision, project.Key, branch.Name, branch.Commit.SHA) + } +} + +// isOfLatestTask accepts the branch's latest analysis when it is the CE task's, that +// is when no later task of the project has started. It is checked after the branch +// listing, so that a task finishing in between is caught. +func isOfLatestTask(httpClient *http.Client, sonarResults *SonarResults, project *Project) func(BranchInfo) error { + return func(branch BranchInfo) error { + isLatest, err := isLatestTaskOfProject(httpClient, sonarResults, project) + if err != nil || isLatest { + return err + } + return fmt.Errorf("could not find the revision of the scan in task %s: branch %s of project %s is short-lived on SonarQube Cloud, which gives only the revision of its latest analysis, and another task of the project finished after this one, or another scan of the branch is still being processed.\nAttest the scan with --sonar-project-key and --sonar-revision instead, without report-task.txt or --sonar-ce-task-url", sonarResults.TaskID, branch.Name, project.Key) + } +} + +// isLatestTaskOfProject reports whether the CE task is the project's last finished +// task, on any branch or pull request, with none of its branch in progress. SonarQube +// saves a scan's analysis before it records the task as finished, so a task in +// progress may already have replaced the branch's latest analysis. Unlike +// api/ce/activity, api/ce/component needs only Browse permission. +func isLatestTaskOfProject(httpClient *http.Client, sonarResults *SonarResults, project *Project) (bool, error) { + componentURL, err := sonarURL(sonarResults.ServerUrl, "api/ce/component", url.Values{"component": {project.Key}}) + if err != nil { + return false, err + } + componentResponse, err := httpClient.Get(componentURL) + if err != nil { + return false, err + } + defer func() { _ = componentResponse.Body.Close() }() + + componentData := &ComponentTasksResponse{} + if err := json.NewDecoder(componentResponse.Body).Decode(componentData); err != nil { + return false, sonarResponseError(componentResponse.StatusCode) + } + if componentData.Errors != nil { + return false, fmt.Errorf("SonarQube error: %s", componentData.Errors[0].Msg) + } + for _, task := range componentData.Queue { + onOtherWork := task.PullRequest != "" || (task.Branch != "" && task.Branch != sonarResults.Branch.Name) + if task.Status != "PENDING" && !onOtherWork { + return false, nil + } + } + return componentData.Current.TaskID == sonarResults.TaskID, nil +} + func GetProjectAnalysisFromAnalysisID(httpClient *http.Client, sonarResults *SonarResults, project *Project, analysisID string) error { projectAnalysesURL, err := analysesSearchURL(sonarResults, project) if err != nil { @@ -565,7 +626,8 @@ func GetProjectAnalysisFromAnalysisID(httpClient *http.Client, sonarResults *Son } if projectAnalysesData.Errors != nil { - return fmt.Errorf("SonarQube error: %s", projectAnalysesData.Errors[0].Msg) + sonarErr := fmt.Errorf("SonarQube error: %s", projectAnalysesData.Errors[0].Msg) + return shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarErr, isOfLatestTask(httpClient, sonarResults, project)) } for analysis := range projectAnalysesData.Analyses { diff --git a/internal/sonar/sonar_ce_short_branch_test.go b/internal/sonar/sonar_ce_short_branch_test.go new file mode 100644 index 000000000..8d02291e2 --- /dev/null +++ b/internal/sonar/sonar_ce_short_branch_test.go @@ -0,0 +1,292 @@ +package sonar_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "testing" + + "github.com/kosli-dev/cli/internal/sonar" +) + +const ( + shortTaskID = "AaFnjwU6BCVWgkk-qOYR" + shortAnalysisID = "99176d9b-f4f0-4dc4-8dc6-e40d15f3ad97" +) + +// fakeSonarCloudCETask adds the CE endpoints to fakeSonarCloudBranches. The task is +// the scan of the short-lived branch; api/ce/component answers with currentTaskID as +// the project's latest finished task (none when empty), and a queued task with +// queuedStatus when that is set, as SonarCloud does to a token with Browse. +type fakeSonarCloudCETask struct { + branches fakeSonarCloudBranches + currentTaskID string + queuedStatus string + // queuedBranch is the queued task's branch: ours when empty, none when "-", + // pull request 7 when "pr". + queuedBranch string + ceComponentForbidden bool + mu sync.Mutex + paths []string +} + +func (f *fakeSonarCloudCETask) handler() http.HandlerFunc { + branches := f.branches.handler() + return func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + f.paths = append(f.paths, r.URL.Path) + f.mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/ce/task": + _, _ = fmt.Fprintf(w, `{"task":{"organization":"cyber-dojo","id":%q,"type":"REPORT","componentKey":%q,"componentName":"customer project","componentQualifier":"TRK","analysisId":%q,"status":"SUCCESS","submittedAt":"2026-10-20T07:06:20+0000","branch":%q,"branchType":"SHORT"}}`, + shortTaskID, revProjectKey, shortAnalysisID, shortBranch) + case "/api/ce/component": + if f.ceComponentForbidden { + w.WriteHeader(http.StatusForbidden) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Insufficient privileges"}]}`) + return + } + queue := "" + if f.queuedStatus != "" { + branch := fmt.Sprintf(`,"branch":%q,"branchType":"SHORT"`, shortBranch) + switch f.queuedBranch { + case "": + case "-": + branch = "" + case "pr": + branch = `,"pullRequest":"7"` + default: + branch = fmt.Sprintf(`,"branch":%q,"branchType":"SHORT"`, f.queuedBranch) + } + queue = fmt.Sprintf(`{"id":"AaFnz-queued","type":"REPORT","componentKey":%q,"status":%q%s}`, revProjectKey, f.queuedStatus, branch) + } + current := "" + if f.currentTaskID != "" { + current = fmt.Sprintf(`,"current":{"id":%q,"type":"REPORT","componentKey":%q,"analysisId":"another-analysis","status":"SUCCESS","branch":"main","branchType":"LONG"}`, f.currentTaskID, revProjectKey) + } + _, _ = fmt.Fprintf(w, `{"queue":[%s]%s}`, queue, current) + default: + branches(w, r) + } + } +} + +func (f *fakeSonarCloudCETask) calls() []string { + f.mu.Lock() + defer f.mu.Unlock() + return slices.Clone(f.paths) +} + +// newCETaskConfig finds the scan the way the scanner leaves it: through +// report-task.txt, or through --sonar-ce-task-url with no report-task.txt. +func newCETaskConfig(t *testing.T, serverURL string, viaReportTask bool) *sonar.SonarConfig { + t.Helper() + dir := t.TempDir() + ceTaskURL := serverURL + "/api/ce/task?id=" + shortTaskID + if viaReportTask { + report := fmt.Sprintf("serverUrl=%s\ndashboardUrl=%s/dashboard?id=%s\nceTaskUrl=%s\n", serverURL, serverURL, revProjectKey, ceTaskURL) + if err := os.WriteFile(filepath.Join(dir, "report-task.txt"), []byte(report), 0o600); err != nil { + t.Fatal(err) + } + ceTaskURL = "" + } + // The revision is the CI commit the command defaults to, which this path ignores. + return sonar.NewSonarConfig("tok", dir, ceTaskURL, "", "", revRevision, "", "", 5) +} + +// SonarCloud refuses api/project_analyses/search for a short-lived branch, and lists +// only the branch's latest analysis. That is our scan's when no task of the project +// finished after ours, which api/ce/component tells a token with Browse. +func TestGetSonarResults_CETask_ShortLivedBranch(t *testing.T) { + for name, viaReportTask := range map[string]bool{"report-task.txt": true, "--sonar-ce-task-url": false} { + t.Run(name, func(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + results, err := newCETaskConfig(t, srv.URL, viaReportTask).GetSonarResults(discardLogger()) + if err != nil { + t.Fatalf("expected the short-lived branch scan to be found, got error: %v", err) + } + if results.Revision != shortLatestCommit || results.AnalysedAt != shortAnalysisDate { + t.Errorf("expected revision %s analysed at %s, got %s at %s", shortLatestCommit, shortAnalysisDate, results.Revision, results.AnalysedAt) + } + if results.TaskID != shortTaskID || results.Branch == nil || results.Branch.Name != shortBranch || results.Branch.Type != "SHORT" { + t.Errorf("expected task %s on branch %s of type SHORT, got task %s, branch %+v", shortTaskID, shortBranch, results.TaskID, results.Branch) + } + if len(fake.branches.qualityGateQs) != 1 || fake.branches.qualityGateQs[0].Get("analysisId") != shortAnalysisID || fake.branches.qualityGateQs[0].Has("branch") { + t.Errorf("expected one quality gate request for analysis %s, got %v", shortAnalysisID, fake.branches.qualityGateQs) + } + // A task finishing between the two reads must be caught, so the task check + // has to come after the branch listing. + calls := fake.calls() + listed, checked := slices.Index(calls, "/api/project_branches/list"), slices.Index(calls, "/api/ce/component") + if listed < 0 || checked < listed { + t.Errorf("expected api/ce/component after api/project_branches/list, got %v", calls) + } + }) + } +} + +// Another task finished after ours, so the branch's latest analysis may not be ours. +func TestGetSonarResults_CETask_ShortLivedBranch_NotLatestTask(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: "AaFnz-a-later-task"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error: a later task means the branch's latest analysis may be another commit's") + } + for _, want := range []string{"short-lived", shortBranch, shortTaskID, "--sonar-project-key", "--sonar-revision"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("expected %q in the error, got: %v", want, err) + } + } + if len(fake.branches.qualityGateQs) != 0 { + t.Error("expected no quality gate lookup") + } +} + +// SonarQube saves a scan's analysis before it records the task as finished, so a task +// in progress may already have replaced the branch's latest analysis. +func TestGetSonarResults_CETask_ShortLivedBranch_TaskInProgress(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, queuedStatus: "IN_PROGRESS"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil || !strings.Contains(err.Error(), "being processed") { + t.Errorf("expected an error: a task in progress may have replaced the branch's latest analysis, got: %v", err) + } +} + +// SonarQube runs one task of a project at a time, so while a task of another branch +// is in progress, none of ours can be, and our branch's latest analysis stands. +func TestGetSonarResults_CETask_ShortLivedBranch_OtherBranchTaskInProgress(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, queuedStatus: "IN_PROGRESS", queuedBranch: "renovate/other"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + results, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err != nil || results.Revision != shortLatestCommit { + t.Errorf("expected revision %s, got %+v, error: %v", shortLatestCommit, results, err) + } +} + +// A pull request scan never changes a branch's latest analysis. +func TestGetSonarResults_CETask_ShortLivedBranch_PullRequestTaskInProgress(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, queuedStatus: "IN_PROGRESS", queuedBranch: "pr"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + results, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err != nil || results.Revision != shortLatestCommit { + t.Errorf("expected revision %s, got %+v, error: %v", shortLatestCommit, results, err) + } +} + +// A task in progress that names no branch or pull request may be on ours. +func TestGetSonarResults_CETask_ShortLivedBranch_UnknownBranchTaskInProgress(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, queuedStatus: "IN_PROGRESS", queuedBranch: "-"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil || !strings.Contains(err.Error(), "being processed") { + t.Errorf("expected an error: the task in progress may be on our branch, got: %v", err) + } +} + +// A pending task has not started, so it cannot have changed the branch yet. +func TestGetSonarResults_CETask_ShortLivedBranch_TaskPending(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, queuedStatus: "PENDING"} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + results, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err != nil || results.Revision != shortLatestCommit { + t.Errorf("expected revision %s, got %+v, error: %v", shortLatestCommit, results, err) + } +} + +func TestGetSonarResults_CETask_ShortLivedBranch_NoFinishedTask(t *testing.T) { + fake := &fakeSonarCloudCETask{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil || !strings.Contains(err.Error(), shortTaskID) { + t.Errorf("expected an error naming task %s, got: %v", shortTaskID, err) + } +} + +func TestGetSonarResults_CETask_ShortLivedBranch_NoCommit(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, branches: fakeSonarCloudBranches{branchCommits: []string{""}}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil || !strings.Contains(err.Error(), "short-lived") || !strings.Contains(err.Error(), "no analysis yet") { + t.Errorf("expected an error saying the short-lived branch has no analysis yet, got: %v", err) + } +} + +func TestGetSonarResults_CETask_ShortLivedBranch_TaskCheckFails(t *testing.T) { + fake := &fakeSonarCloudCETask{ceComponentForbidden: true} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + if err == nil || !strings.Contains(err.Error(), "Insufficient privileges") { + t.Errorf("expected SonarQube's refusal of api/ce/component, got: %v", err) + } + if len(fake.branches.qualityGateQs) != 0 { + t.Error("expected no quality gate lookup") + } +} + +// When the branch can't be listed, SonarQube's own error stands. +func TestGetSonarResults_CETask_ShortLivedBranch_ListingFails(t *testing.T) { + fake := &fakeSonarCloudCETask{currentTaskID: shortTaskID, branches: fakeSonarCloudBranches{branchCommits: []string{badGateway}}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, true).GetSonarResults(discardLogger()) + want := fmt.Sprintf("SonarQube error: Branch '%s' is not of type LONG", shortBranch) + if err == nil || err.Error() != want { + t.Errorf("expected %q, got: %v", want, err) + } + if slices.Contains(fake.calls(), "/api/ce/component") { + t.Error("expected no task check without a short-lived branch") + } +} + +// A CE task with no branch is the main branch's: there is no short-lived branch to +// look up, so SonarQube's error stands. +func TestGetSonarResults_CETask_NoBranch_SonarErrorUnchanged(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/ce/task": + _, _ = fmt.Fprintf(w, `{"task":{"id":%q,"componentKey":%q,"analysisId":%q,"status":"SUCCESS"}}`, shortTaskID, revProjectKey, shortAnalysisID) + case "/api/project_analyses/search": + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Component key 'x' not found"}]}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + _, err := newCETaskConfig(t, srv.URL, false).GetSonarResults(discardLogger()) + if err == nil || err.Error() != "SonarQube error: Component key 'x' not found" { + t.Errorf("expected the SonarQube error unchanged, got: %v", err) + } +}