From 92f876945825408e176b043f0d471bcd81cd1373 Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 23 Oct 2026 08:26:03 +0100 Subject: [PATCH 1/3] fix(sonar): attest a SonarQube Cloud short-lived branch by project key SonarQube Cloud serves api/project_analyses/search only for long-lived branches, so --sonar-branch on a feature branch failed with "Branch '...' is not of type LONG". When the search fails and the branch is listed as short-lived, use its latest analysis from api/project_branches/list and its quality gate by project key and branch. The revision must be that latest commit, checked again after reading the gate in case a newer push was processed in between. Fixes #1294 Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/attestSonar.go | 1 + internal/sonar/sonar.go | 89 ++++++- internal/sonar/sonar_short_branch_test.go | 283 ++++++++++++++++++++++ 3 files changed, 360 insertions(+), 13 deletions(-) create mode 100644 internal/sonar/sonar_short_branch_test.go diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index dbf4bb63a..939840c9a 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -48,6 +48,7 @@ For branch scans: if running the Kosli CLI in some CI/CD pipeline, the revision or have overridden the revision in SonarQube via parameters to the Sonar scanner, you can provide the correct revision using the ^--sonar-revision^ flag. If the scan ran on a branch other than the project's main branch in SonarQube, also provide the branch name using the ^--sonar-branch^ flag. SonarQube only searches the project's main branch unless told otherwise, so without this flag the scan cannot be found. +On SonarQube Cloud, a short-lived branch keeps only its latest analysis, so the revision must be the latest commit analysed on that branch. For pull request scans: provide the pull-request ID using the ^--pull-request^ flag instead of the revision. SonarQube keeps only the latest analysis of a pull request, so if the scan for the current push has not finished, that is the previous push's analysis. To make sure the attested analysis is of the commit you expect, also pass ^--sonar-revision^: the command then fails if the pull request's analysis is of another commit. diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index a300e831f..213988211 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -131,6 +131,18 @@ type PRCommit struct { SHA string `json:"sha"` } +// These are the structs for the response from the project_branches/list API +type BranchesResponse struct { + Branches []BranchInfo `json:"branches"` +} + +type BranchInfo struct { + Name string `json:"name"` + Type string `json:"type"` + AnalysisDate string `json:"analysisDate"` + Commit PRCommit `json:"commit"` +} + // Struct for error messages from sonar APIs type Error struct { Msg string `json:"msg"` @@ -271,6 +283,13 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error if err != nil { return nil, err } + // A short-lived branch's gate is its latest, so a push processed since the + // revision was matched would have replaced it with the newer commit's. + if analysisID == "" && sonarResults.Branch != nil && sonarResults.Branch.Type == "SHORT" { + if _, err := shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarResults.Revision); err != nil { + return nil, err + } + } sonarResults.Project = *project sonarResults.QualityGate = qualityGate @@ -432,7 +451,15 @@ func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *Sonar } if projectAnalysesData.Errors != nil { - return "", fmt.Errorf("SonarQube error: %s", projectAnalysesData.Errors[0].Msg) + 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 } for analysis := range projectAnalysesData.Analyses { @@ -456,6 +483,44 @@ func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *Sonar return analysisID, 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 is not a listed +// short-lived branch. +func shortLivedBranchAnalysis(httpClient *http.Client, sonarResults *SonarResults, project *Project, revision string) (bool, error) { + branchesURL, err := sonarURL(sonarResults.ServerUrl, "api/project_branches/list", url.Values{"project": {project.Key}}) + if err != nil { + return false, err + } + branchesResponse, err := httpClient.Get(branchesURL) + if err != nil { + return false, err + } + defer func() { _ = branchesResponse.Body.Close() }() + + branchesData := &BranchesResponse{} + if err := json.NewDecoder(branchesResponse.Body).Decode(branchesData); err != nil { + return false, nil + } + + for _, branch := range branchesData.Branches { + if branch.Name != sonarResults.Branch.Name || branch.Type != "SHORT" { + continue + } + 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) + } + 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) + } + sonarResults.AnalysedAt = branch.AnalysisDate + sonarResults.Branch.Type = branch.Type + return true, nil + } + return false, nil +} + func GetProjectAnalysisFromAnalysisID(httpClient *http.Client, sonarResults *SonarResults, project *Project, analysisID string) error { projectAnalysesURL, err := analysesSearchURL(sonarResults, project) if err != nil { @@ -538,18 +603,16 @@ func GetPRAnalysisData(httpClient *http.Client, sonarResults *SonarResults, proj } func GetQualityGate(httpClient *http.Client, sonarResults *SonarResults, qualityGate *QualityGate, analysisID, projectKey, pullRequest string) (*QualityGate, error) { - var qualityGateURL string - var err error - if pullRequest == "" { - qualityGateURL, err = sonarURL(sonarResults.ServerUrl, "api/qualitygates/project_status", url.Values{"analysisId": {analysisID}}) - if err != nil { - return nil, err - } - } else { - qualityGateURL, err = sonarURL(sonarResults.ServerUrl, "api/qualitygates/project_status", url.Values{"projectKey": {projectKey}, "pullRequest": {pullRequest}}) - if err != nil { - return nil, err - } + params := url.Values{"analysisId": {analysisID}} + if pullRequest != "" { + params = url.Values{"projectKey": {projectKey}, "pullRequest": {pullRequest}} + } else if analysisID == "" && sonarResults.Branch != nil { + // A short-lived branch has no analysis ID we can find, only a latest gate. + params = url.Values{"projectKey": {projectKey}, "branch": {sonarResults.Branch.Name}} + } + qualityGateURL, err := sonarURL(sonarResults.ServerUrl, "api/qualitygates/project_status", params) + if err != nil { + return nil, err } qualityGateRequest, err := http.NewRequest("GET", qualityGateURL, nil) if err != nil { diff --git a/internal/sonar/sonar_short_branch_test.go b/internal/sonar/sonar_short_branch_test.go new file mode 100644 index 000000000..e9d9b50c0 --- /dev/null +++ b/internal/sonar/sonar_short_branch_test.go @@ -0,0 +1,283 @@ +package sonar_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + + "github.com/kosli-dev/cli/internal/sonar" +) + +// The responses below are trimmed copies of what sonarcloud.io returned on 23 Oct 2026 +// for the public project cyber-dojo_differ, whose branch run-workflow-196 is short-lived. +// The branch name here has a slash, as the customer's feature branches do. +const ( + shortBranch = "feature/kosli-integration" + shortLatestCommit = "6860082827803e7c2dabef20d9941de77d4eda23" + shortAnalysisDate = "2026-10-20T07:06:05+0000" + longBranch = "release/uat" + unlistedBranch = "no-such-branch" +) + +type fakeSonarCloudBranches struct { + mu sync.Mutex + paths []string + qualityGateQs []url.Values + // branchCommits is the short branch's latest commit at each branch listing in + // turn, to simulate a newer push being processed between two calls. + branchCommits []string +} + +func (f *fakeSonarCloudBranches) nextBranchCommit() string { + f.mu.Lock() + defer f.mu.Unlock() + if len(f.branchCommits) == 0 { + return shortLatestCommit + } + commit := f.branchCommits[0] + if len(f.branchCommits) > 1 { + f.branchCommits = f.branchCommits[1:] + } + return commit +} + +func (f *fakeSonarCloudBranches) handler() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + f.mu.Lock() + f.paths = append(f.paths, r.URL.Path) + if r.URL.Path == "/api/qualitygates/project_status" { + f.qualityGateQs = append(f.qualityGateQs, q) + } + f.mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/project_analyses/search": + switch q.Get("branch") { + case shortBranch: + w.WriteHeader(http.StatusBadRequest) + _, _ = fmt.Fprintf(w, `{"errors":[{"msg":"Branch '%s' is not of type LONG"}]}`, shortBranch) + return + case unlistedBranch: + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprintf(w, `{"errors":[{"msg":"Component '%s' on branch '%s' not found"}]}`, revProjectKey, unlistedBranch) + return + } + _, _ = fmt.Fprintf(w, `{"paging":{"pageIndex":1,"pageSize":500,"total":0},"analyses":[]}`) + case "/api/project_branches/list": + commit := fmt.Sprintf(`,"commit":{"sha":%q}`, f.nextBranchCommit()) + if strings.HasSuffix(commit, `""}`) { + commit = "" + } + _, _ = fmt.Fprintf(w, `{"branches":[ + {"name":"main","isMain":true,"type":"LONG","analysisDate":"2026-10-23T06:59:29+0000","commit":{"sha":"ce9bf5fdfb6e10b5d2de1b324fd03483e8b67173"}}, + {"name":%q,"isMain":false,"type":"LONG","analysisDate":"2026-10-08T06:59:29+0000","commit":{"sha":"aaaa"}}, + {"name":%q,"isMain":false,"type":"SHORT","mergeBranch":"main","analysisDate":%q%s}]}`, + longBranch, shortBranch, shortAnalysisDate, commit) + case "/api/ce/activity": + // SonarCloud refuses this to a token without project administration. + w.WriteHeader(http.StatusForbidden) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Insufficient privileges"}]}`) + case "/api/qualitygates/project_status": + if q.Get("analysisId") == "" && q.Get("branch") == "" && q.Get("pullRequest") == "" { + w.WriteHeader(http.StatusBadRequest) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Either 'analysisId', 'projectId' or 'projectKey' must be provided"}]}`) + return + } + _, _ = fmt.Fprint(w, `{"projectStatus":{"status":"OK","conditions":[{"status":"OK","metricKey":"new_reliability_rating","comparator":"GT","periodIndex":1,"errorThreshold":"1","actualValue":"1"}],"periods":[],"ignoredConditions":false}}`) + default: + http.NotFound(w, r) + } + } +} + +func (f *fakeSonarCloudBranches) called(path string) bool { + f.mu.Lock() + defer f.mu.Unlock() + for _, p := range f.paths { + if p == path { + return true + } + } + return false +} + +// SonarCloud serves api/project_analyses/search only for long-lived branches, so a +// scan on a short-lived branch is found through the branch's latest analysis. +func TestGetSonarResults_ShortLivedBranch_LatestCommit(t *testing.T) { + fake := &fakeSonarCloudBranches{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) + results, err := sc.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.Branch == nil || results.Branch.Name != shortBranch || results.Branch.Type != "SHORT" { + t.Errorf("expected branch %s of type SHORT, got %+v", shortBranch, results.Branch) + } + if results.QualityGate == nil || results.QualityGate.Status != "OK" || len(results.QualityGate.Conditions) != 1 { + t.Fatalf("expected the branch's quality gate, got %+v", results.QualityGate) + } + if len(fake.qualityGateQs) != 1 { + t.Fatalf("expected one quality gate request, got %d", len(fake.qualityGateQs)) + } + q := fake.qualityGateQs[0] + if q.Get("projectKey") != revProjectKey || q.Get("branch") != shortBranch || q.Has("analysisId") { + t.Errorf("expected the quality gate by project key and branch, got %v", q) + } +} + +// SonarCloud keeps only the latest analysis of a short-lived branch, so any other +// revision cannot be attested: its results are gone, or not processed yet. +func TestGetSonarResults_ShortLivedBranch_OtherRevision(t *testing.T) { + const otherRevision = "0000000000000000000000000000000000000000" + fake := &fakeSonarCloudBranches{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, otherRevision, "", shortBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error: the branch's latest analysis is of another revision") + } + for _, want := range []string{"short-lived", shortBranch, shortLatestCommit, otherRevision} { + if !strings.Contains(err.Error(), want) { + t.Errorf("expected %q in the error, got: %v", want, err) + } + } + if fake.called("/api/qualitygates/project_status") { + t.Error("expected no quality gate lookup for a revision that was not found") + } +} + +// A SonarQube error on a branch that is not short-lived is not ours to explain, +// so it reaches the user unchanged. +func TestGetSonarResults_SonarErrorOnOtherBranch_Unchanged(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/project_analyses/search": + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Component key 'x' not found"}]}`) + case "/api/project_branches/list": + // SonarQube Server has no short-lived branches: every branch is of type BRANCH. + _, _ = fmt.Fprintf(w, `{"branches":[{"name":%q,"type":"BRANCH","commit":{"sha":%q}}]}`, longBranch, revRevision) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, revRevision, "", longBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil || err.Error() != "SonarQube error: Component key 'x' not found" { + t.Errorf("expected the SonarQube error unchanged, got: %v", err) + } +} + +// The branch lookup is only a fallback: a search that succeeds costs no extra call. +func TestGetSonarResults_LongBranch_NoBranchLookup(t *testing.T) { + fake := &fakeSonarProject{} + branchLookups := 0 + var qualityGateQ url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/project_branches/list": + branchLookups++ + case "/api/qualitygates/project_status": + qualityGateQ = r.URL.Query() + } + fake.handler()(w, r) + })) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, revRevision, "", revFeatureBranch, 5) + if _, err := sc.GetSonarResults(discardLogger()); err != nil { + t.Fatalf("expected the long-lived branch scan to be found, got error: %v", err) + } + if branchLookups != 0 { + t.Errorf("expected no api/project_branches/list call, got %d", branchLookups) + } + if qualityGateQ.Get("analysisId") != revAnalysisKey || qualityGateQ.Has("branch") { + t.Errorf("expected the quality gate of analysis %s, got %v", revAnalysisKey, qualityGateQ) + } +} + +// The quality gate of a short-lived branch is its latest one and cannot be asked +// for by commit. If a newer push is processed between finding the revision and +// reading the gate, the gate is the newer commit's, so the attestation must fail. +func TestGetSonarResults_ShortLivedBranch_NewerAnalysisDuringLookup(t *testing.T) { + const newerCommit = "1111111111111111111111111111111111111111" + fake := &fakeSonarCloudBranches{branchCommits: []string{shortLatestCommit, newerCommit}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error: the quality gate read may be of the newer commit") + } + for _, want := range []string{newerCommit, shortLatestCommit} { + if !strings.Contains(err.Error(), want) { + t.Errorf("expected %q in the error, got: %v", want, err) + } + } +} + +// A listed short-lived branch without a commit has no analysis we could attest. +func TestGetSonarResults_ShortLivedBranch_NoCommit(t *testing.T) { + fake := &fakeSonarCloudBranches{branchCommits: []string{""}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error for a branch with no analysed commit") + } + if !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) + } +} + +// A branch SonarCloud does not list keeps SonarQube's own error. +func TestGetSonarResults_UnlistedBranch_Unchanged(t *testing.T) { + fake := &fakeSonarCloudBranches{} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", unlistedBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + want := fmt.Sprintf("SonarQube error: Component '%s' on branch '%s' not found", revProjectKey, unlistedBranch) + if err == nil || err.Error() != want { + t.Errorf("expected %q, got: %v", want, err) + } +} + +// Without --sonar-branch there is no branch to look up, so SonarQube's error stands. +func TestGetSonarResults_SonarErrorWithoutBranch_Unchanged(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/project_branches/list" { + _, _ = fmt.Fprintf(w, `{"branches":[{"name":%q,"type":"SHORT","commit":{"sha":%q}}]}`, shortBranch, revRevision) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Component key 'x' not found"}]}`) + })) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, revRevision, "", "", 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil || err.Error() != "SonarQube error: Component key 'x' not found" { + t.Errorf("expected the SonarQube error unchanged, got: %v", err) + } +} From 23748012c4112835e4d8538341b27e023ebb3f9c Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 23 Oct 2026 08:32:44 +0100 Subject: [PATCH 2/3] fix(sonar): fail when a short-lived branch's commit can't be re-confirmed The check after reading a short-lived branch's quality gate let the attestation through when the second branch listing failed or no longer listed the branch, so a gate of another commit could be attested. Fail instead, and decide "gate is the branch's latest" in one place. Co-Authored-By: Claude Opus 5.5 --- internal/sonar/sonar.go | 21 ++++++++++++----- internal/sonar/sonar_short_branch_test.go | 28 ++++++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index 213988211..6b569af3e 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -283,12 +283,16 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error if err != nil { return nil, err } - // A short-lived branch's gate is its latest, so a push processed since the - // revision was matched would have replaced it with the newer commit's. - if analysisID == "" && sonarResults.Branch != nil && sonarResults.Branch.Type == "SHORT" { - if _, err := shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarResults.Revision); err != nil { + // A push processed since the revision was matched would have replaced the + // branch's latest gate with the newer commit's. + if gateIsBranchLatest(sonarResults, analysisID) { + isShort, err := shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarResults.Revision) + if err != nil { return nil, err } + if !isShort { + return nil, fmt.Errorf("could not confirm that the latest analysis of short-lived branch %s of project %s is still of revision %s, so its quality gate may be of another commit. Try again later", sonarResults.Branch.Name, project.Key, sonarResults.Revision) + } } sonarResults.Project = *project @@ -483,6 +487,12 @@ func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *Sonar return analysisID, nil } +// gateIsBranchLatest reports whether the quality gate can only be read as the +// branch's latest. Only a short-lived branch is found without an analysis ID. +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 @@ -606,8 +616,7 @@ func GetQualityGate(httpClient *http.Client, sonarResults *SonarResults, quality params := url.Values{"analysisId": {analysisID}} if pullRequest != "" { params = url.Values{"projectKey": {projectKey}, "pullRequest": {pullRequest}} - } else if analysisID == "" && sonarResults.Branch != nil { - // A short-lived branch has no analysis ID we can find, only a latest gate. + } else if gateIsBranchLatest(sonarResults, analysisID) { params = url.Values{"projectKey": {projectKey}, "branch": {sonarResults.Branch.Name}} } qualityGateURL, err := sonarURL(sonarResults.ServerUrl, "api/qualitygates/project_status", params) diff --git a/internal/sonar/sonar_short_branch_test.go b/internal/sonar/sonar_short_branch_test.go index e9d9b50c0..71dbcc28f 100644 --- a/internal/sonar/sonar_short_branch_test.go +++ b/internal/sonar/sonar_short_branch_test.go @@ -21,6 +21,8 @@ const ( shortAnalysisDate = "2026-10-20T07:06:05+0000" longBranch = "release/uat" unlistedBranch = "no-such-branch" + // badGateway in branchCommits makes that branch listing fail. + badGateway = "502" ) type fakeSonarCloudBranches struct { @@ -70,7 +72,14 @@ func (f *fakeSonarCloudBranches) handler() http.HandlerFunc { } _, _ = fmt.Fprintf(w, `{"paging":{"pageIndex":1,"pageSize":500,"total":0},"analyses":[]}`) case "/api/project_branches/list": - commit := fmt.Sprintf(`,"commit":{"sha":%q}`, f.nextBranchCommit()) + next := f.nextBranchCommit() + if next == badGateway { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusBadGateway) + _, _ = fmt.Fprint(w, "502 Bad Gateway") + return + } + commit := fmt.Sprintf(`,"commit":{"sha":%q}`, next) if strings.HasSuffix(commit, `""}`) { commit = "" } @@ -281,3 +290,20 @@ func TestGetSonarResults_SonarErrorWithoutBranch_Unchanged(t *testing.T) { t.Errorf("expected the SonarQube error unchanged, got: %v", err) } } + +// If the second listing cannot confirm the branch's latest commit, the gate just +// read may be another commit's, so nothing is attested. +func TestGetSonarResults_ShortLivedBranch_RecheckFails(t *testing.T) { + fake := &fakeSonarCloudBranches{branchCommits: []string{shortLatestCommit, badGateway}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error: the branch's latest commit could not be confirmed after reading the gate") + } + if !strings.Contains(err.Error(), shortBranch) { + t.Errorf("expected the branch in the error, got: %v", err) + } +} From c33ffba88f0d7bc640bff8a9b957c8ad852aa0fa Mon Sep 17 00:00:00 2001 From: Alex Kantor Date: Fri, 23 Oct 2026 08:55:41 +0100 Subject: [PATCH 3/3] fix(sonar): keep the cause when a short-lived branch can't be re-confirmed The branch lookup now reports a non-2xx response as an error, so the re-check after reading the gate keeps the HTTP status, and a branch no longer listed is named as the cause. When a newer analysis has replaced the attested revision, the error says so rather than suggesting a retry that cannot help. search: grep -nE 'isShort|shortLivedBranchAnalysis' internal/sonar/sonar.go 465-466 the first lookup in GetProjectAnalysisFromRevision, still the only caller; 499/504 its definition. The re-check no longer calls it. Co-Authored-By: Claude Opus 5.5 --- internal/sonar/sonar.go | 61 ++++++++++++--------- internal/sonar/sonar_short_branch_test.go | 65 +++++++++++++++++++---- 2 files changed, 92 insertions(+), 34 deletions(-) diff --git a/internal/sonar/sonar.go b/internal/sonar/sonar.go index 6b569af3e..6161e9a2f 100644 --- a/internal/sonar/sonar.go +++ b/internal/sonar/sonar.go @@ -283,15 +283,18 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error if err != nil { return nil, err } - // A push processed since the revision was matched would have replaced the - // branch's latest gate with the newer commit's. + // A short-lived branch's gate is its latest, so a push processed since the + // revision was matched would have replaced it with the newer commit's. if gateIsBranchLatest(sonarResults, analysisID) { - isShort, err := shortLivedBranchAnalysis(httpClient, sonarResults, project, sonarResults.Revision) + branch, err := listedShortLivedBranch(httpClient, sonarResults, project) + if err == nil && branch == nil { + err = fmt.Errorf("the branch is no longer listed") + } if err != nil { - return nil, err + return nil, fmt.Errorf("could not confirm that the latest analysis of short-lived branch %s of project %s is still of revision %s, so its quality gate may be of another commit: %w", sonarResults.Branch.Name, project.Key, sonarResults.Revision, err) } - if !isShort { - return nil, fmt.Errorf("could not confirm that the latest analysis of short-lived branch %s of project %s is still of revision %s, so its quality gate may be of another commit. Try again later", sonarResults.Branch.Name, project.Key, sonarResults.Revision) + if branch.Commit.SHA != sonarResults.Revision { + return nil, fmt.Errorf("a newer analysis, of revision %s, replaced the analysis of revision %s on short-lived branch %s of project %s while it was being attested.\nSonarQube Cloud keeps only the latest analysis of a short-lived branch, so revision %s can no longer be attested", branch.Commit.SHA, sonarResults.Revision, branch.Name, project.Key, sonarResults.Revision) } } @@ -496,39 +499,47 @@ func gateIsBranchLatest(sonarResults *SonarResults, analysisID string) bool { // 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 is not a listed -// short-lived branch. +// 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) { + branch, err := listedShortLivedBranch(httpClient, sonarResults, project) + if err != nil || branch == nil { + return false, nil + } + 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) + } + 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) + } + sonarResults.AnalysedAt = branch.AnalysisDate + sonarResults.Branch.Type = branch.Type + return true, nil +} + +// listedShortLivedBranch returns the branch from api/project_branches/list when +// SonarCloud lists it as short-lived, and nil when it does not. +func listedShortLivedBranch(httpClient *http.Client, sonarResults *SonarResults, project *Project) (*BranchInfo, error) { branchesURL, err := sonarURL(sonarResults.ServerUrl, "api/project_branches/list", url.Values{"project": {project.Key}}) if err != nil { - return false, err + return nil, err } branchesResponse, err := httpClient.Get(branchesURL) if err != nil { - return false, err + return nil, err } defer func() { _ = branchesResponse.Body.Close() }() branchesData := &BranchesResponse{} - if err := json.NewDecoder(branchesResponse.Body).Decode(branchesData); err != nil { - return false, nil + if branchesResponse.StatusCode/100 != 2 || json.NewDecoder(branchesResponse.Body).Decode(branchesData) != nil { + return nil, sonarResponseError(branchesResponse.StatusCode) } - for _, branch := range branchesData.Branches { - if branch.Name != sonarResults.Branch.Name || branch.Type != "SHORT" { - continue - } - 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) - } - 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 branch.Name == sonarResults.Branch.Name && branch.Type == "SHORT" { + return &branch, nil } - sonarResults.AnalysedAt = branch.AnalysisDate - sonarResults.Branch.Type = branch.Type - return true, nil } - return false, nil + return nil, nil } func GetProjectAnalysisFromAnalysisID(httpClient *http.Client, sonarResults *SonarResults, project *Project, analysisID string) error { diff --git a/internal/sonar/sonar_short_branch_test.go b/internal/sonar/sonar_short_branch_test.go index 71dbcc28f..b50107472 100644 --- a/internal/sonar/sonar_short_branch_test.go +++ b/internal/sonar/sonar_short_branch_test.go @@ -21,8 +21,11 @@ const ( shortAnalysisDate = "2026-10-20T07:06:05+0000" longBranch = "release/uat" unlistedBranch = "no-such-branch" - // badGateway in branchCommits makes that branch listing fail. + // In branchCommits, badGateway and forbidden make that branch listing fail, + // and deleted lists the project without the short-lived branch. badGateway = "502" + forbidden = "403" + deleted = "deleted" ) type fakeSonarCloudBranches struct { @@ -79,6 +82,16 @@ func (f *fakeSonarCloudBranches) handler() http.HandlerFunc { _, _ = fmt.Fprint(w, "502 Bad Gateway") return } + if next == forbidden { + // The JSON error body decodes into an empty branch list. + w.WriteHeader(http.StatusForbidden) + _, _ = fmt.Fprint(w, `{"errors":[{"msg":"Insufficient privileges"}]}`) + return + } + if next == deleted { + _, _ = fmt.Fprint(w, `{"branches":[{"name":"main","isMain":true,"type":"LONG","commit":{"sha":"ce9bf5fdfb6e10b5d2de1b324fd03483e8b67173"}}]}`) + return + } commit := fmt.Sprintf(`,"commit":{"sha":%q}`, next) if strings.HasSuffix(commit, `""}`) { commit = "" @@ -235,11 +248,15 @@ func TestGetSonarResults_ShortLivedBranch_NewerAnalysisDuringLookup(t *testing.T if err == nil { t.Fatal("expected an error: the quality gate read may be of the newer commit") } - for _, want := range []string{newerCommit, shortLatestCommit} { + for _, want := range []string{newerCommit, shortLatestCommit, "newer analysis"} { if !strings.Contains(err.Error(), want) { t.Errorf("expected %q in the error, got: %v", want, err) } } + // Retrying cannot bring the older analysis back. + if strings.Contains(err.Error(), "try again later") { + t.Errorf("expected no advice to retry, got: %v", err) + } } // A listed short-lived branch without a commit has no analysis we could attest. @@ -292,18 +309,48 @@ func TestGetSonarResults_SonarErrorWithoutBranch_Unchanged(t *testing.T) { } // If the second listing cannot confirm the branch's latest commit, the gate just -// read may be another commit's, so nothing is attested. +// read may be another commit's, so nothing is attested, and the cause is kept. func TestGetSonarResults_ShortLivedBranch_RecheckFails(t *testing.T) { - fake := &fakeSonarCloudBranches{branchCommits: []string{shortLatestCommit, badGateway}} + cases := []struct { + name string + listing string + want string + }{ + {name: "server error", listing: badGateway, want: "HTTP 502"}, + {name: "refused with a JSON body", listing: forbidden, want: "HTTP 403"}, + {name: "branch deleted", listing: deleted, want: "no longer listed"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + fake := &fakeSonarCloudBranches{branchCommits: []string{shortLatestCommit, c.listing}} + srv := httptest.NewServer(fake.handler()) + defer srv.Close() + + sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) + _, err := sc.GetSonarResults(discardLogger()) + if err == nil { + t.Fatal("expected an error: the branch's latest commit could not be confirmed after reading the gate") + } + for _, want := range []string{shortBranch, c.want} { + if !strings.Contains(err.Error(), want) { + t.Errorf("expected %q in the error, got: %v", want, err) + } + } + }) + } +} + +// If the branch listing fails on the first lookup, the user sees the search's own +// error, which says what SonarQube refused. +func TestGetSonarResults_ShortLivedBranch_FirstListingFails(t *testing.T) { + fake := &fakeSonarCloudBranches{branchCommits: []string{badGateway}} srv := httptest.NewServer(fake.handler()) defer srv.Close() sc := sonar.NewSonarConfig("tok", t.TempDir(), "", revProjectKey, srv.URL, shortLatestCommit, "", shortBranch, 5) _, err := sc.GetSonarResults(discardLogger()) - if err == nil { - t.Fatal("expected an error: the branch's latest commit could not be confirmed after reading the gate") - } - if !strings.Contains(err.Error(), shortBranch) { - t.Errorf("expected the branch in the error, got: %v", err) + 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) } }