Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 9
fix(sonar): add --sonar-branch so attest sonar finds scans on non-main branches#1117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9dd1d77a335208c9ba63ebf38a53092bbe220cacfdc50a9c53832c1c201a2a9f44ff09195f34c216a437bbfc33fFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -301,7 +301,8 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file, | ||
| sonarProjectKeyFlag = "[conditional] The project key of the SonarQube project. Only required if you want to use the project key/revision/pull-request to get the scan results rather than using Sonar's metadata file." | ||
| sonarServerURLFlag = "[conditional] The URL of your SonarQube server. Only required if you are using SonarQube Server and not using SonarQube's metadata file to get scan results." | ||
| sonarRevisionFlag = "[conditional] The revision of the SonarQube project. Only required if you want to use the project key/revision to get the scan results rather than using Sonar's metadata file and you have overridden the default revision, or you aren't using a CI. Defaults to the value of the git commit flag. Cannot be used with --pull-request." | ||
| sonarPRFlag = "[conditional] The ID of the pull-request. Only required if you want to use the project key/pull-request to get the scan results rather than using Sonar's metadata file. Cannot be used with --sonar-revision." | ||
| sonarPRFlag = "[conditional] The ID of the pull-request. Only required if you want to use the project key/pull-request to get the scan results rather than using Sonar's metadata file. Cannot be used with --sonar-revision or --sonar-branch." | ||
| sonarBranchFlag = "[conditional] The name of the branch the SonarQube scan ran on. Only required if you are using the project key/revision to get the scan results and the scan ran on a branch other than the project's main branch in SonarQube. Cannot be used with --pull-request." | ||
AlexKantor87 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| sonarMaxWaitFlag = "[optional] Allow the command to wait and retry fetching the scan results from SonarQube, up to the maximum number of seconds provided, with exponential backoff. Useful when using SonarQube's metadata file to retrieve and attest scans that take a long time to process . Defaults to 30 seconds." | ||
| sonarCETaskURLFlag = "[conditional] The URL of the SonarQube CE task. Can be used instead of --sonar-working-dir when the report-task.txt file is not accessible, e.g. due to container isolation in CI/CD pipelines." | ||
| logicalEnvFlag = "[required] The logical environment." | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1177,6 +1177,7 @@ | ||
| "repo-url", | ||
| "repository", | ||
| "sonar-api-token", | ||
| "sonar-branch", | ||
AlexKantor87 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| "sonar-ce-task-url", | ||
| "sonar-project-key", | ||
| "sonar-revision", | ||
| @@ -1211,6 +1212,7 @@ | ||
| "repo-url": "http://example.com", | ||
| "repository": "probe-repository", | ||
| "sonar-api-token": "probe-sonar-api-token", | ||
| "sonar-branch": "probe-sonar-branch", | ||
| "sonar-ce-task-url": "probe-sonar-ce-task-url", | ||
| "sonar-project-key": "probe-sonar-project-key", | ||
| "sonar-revision": "probe-sonar-revision", | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -22,6 +22,7 @@ type SonarConfig struct { | ||||||||||||||||||||||
| projectKey string | ||||||||||||||||||||||
| serverURL string | ||||||||||||||||||||||
| pullRequest string | ||||||||||||||||||||||
| branch string | ||||||||||||||||||||||
| maxWait int | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| @@ -135,7 +136,7 @@ type Error struct { | ||||||||||||||||||||||
| Msg string `json:"msg"` | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| func NewSonarConfig(apiToken, workingDir, ceTaskUrl, projectKey, serverURL, revision, pullRequest string, maxWait int) *SonarConfig { | ||||||||||||||||||||||
| func NewSonarConfig(apiToken, workingDir, ceTaskUrl, projectKey, serverURL, revision, pullRequest, branch string, maxWait int) *SonarConfig { | ||||||||||||||||||||||
| return &SonarConfig{ | ||||||||||||||||||||||
| APIToken: apiToken, | ||||||||||||||||||||||
| WorkingDir: workingDir, | ||||||||||||||||||||||
| @@ -144,6 +145,7 @@ func NewSonarConfig(apiToken, workingDir, ceTaskUrl, projectKey, serverURL, revi | ||||||||||||||||||||||
| projectKey: projectKey, | ||||||||||||||||||||||
| serverURL: serverURL, | ||||||||||||||||||||||
| pullRequest: pullRequest, | ||||||||||||||||||||||
| branch: branch, | ||||||||||||||||||||||
| maxWait: maxWait, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| @@ -158,6 +160,19 @@ func sonarURL(serverURL, apiPath string, params url.Values) (string, error) { | ||||||||||||||||||||||
| return u.String(), nil | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| // analysesSearchURL builds a project_analyses/search URL, scoped to the branch | ||||||||||||||||||||||
| // when there is one: SonarQube otherwise searches only the project's main branch, | ||||||||||||||||||||||
| // so an analysis on any other branch is invisible (#861, #1116). Both lookups | ||||||||||||||||||||||
| // share this so the rule cannot be fixed on one path and left broken on the other, | ||||||||||||||||||||||
| // which is how those two issues came to be six months apart. | ||||||||||||||||||||||
| func analysesSearchURL(sonarResults *SonarResults, project *Project) (string, error) { | ||||||||||||||||||||||
| params := url.Values{"project": {project.Key}} | ||||||||||||||||||||||
| if sonarResults.Branch != nil && sonarResults.Branch.Name != "" { | ||||||||||||||||||||||
| params.Set("branch", sonarResults.Branch.Name) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return sonarURL(sonarResults.ServerUrl, "api/project_analyses/search", params) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error) { | ||||||||||||||||||||||
| var analysisID string | ||||||||||||||||||||||
| var err error | ||||||||||||||||||||||
| @@ -197,6 +212,13 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error | ||||||||||||||||||||||
| project.Key = sc.projectKey | ||||||||||||||||||||||
| sonarResults.ServerUrl = sc.serverURL | ||||||||||||||||||||||
| sonarResults.Revision = sc.revision | ||||||||||||||||||||||
| // On this path there is no CE task to read the branch from, so the branch | ||||||||||||||||||||||
| // can only come from the user (#1116). Set it on the results, which is the | ||||||||||||||||||||||
| // one mechanism both analyses lookups use to scope their search. A pull | ||||||||||||||||||||||
| // request scan is not a branch scan, so the branch is not carried there. | ||||||||||||||||||||||
| if sc.branch != "" && sonarResults.PullRequest == "" { | ||||||||||||||||||||||
| sonarResults.Branch = &Branch{Name: sc.branch} | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
AlexKantor87 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. AlexKantor87 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||||||||||||||||||||||
| project.Url, err = sonarURL(sonarResults.ServerUrl, "dashboard", url.Values{"id": {project.Key}}) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| @@ -212,10 +234,39 @@ func (sc *SonarConfig) GetSonarResults(logger *log.Logger) (*SonarResults, error | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| // The task's own branch wins when it reports one — it is the scan's own | ||||||||||||||||||||||
| // record, reached via an analysis ID the branch-scoped search returned. | ||||||||||||||||||||||
| // But it must not delete the branch the user gave us when it reports none, | ||||||||||||||||||||||
| // which SonarQube does for main-branch tasks and older self-hosted Servers | ||||||||||||||||||||||
| // do more widely (#1116). | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // Unlike the guard above, PullRequest here can also have been set from the | ||||||||||||||||||||||
| // matched task, so this must read the results field rather than the config. | ||||||||||||||||||||||
| if sc.branch != "" && sonarResults.PullRequest == "" && | ||||||||||||||||||||||
| (sonarResults.Branch == nil || sonarResults.Branch.Name == "") { | ||||||||||||||||||||||
| sonarResults.Branch = &Branch{Name: sc.branch} | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if analysisID == "" && sc.CETaskUrl != "" { | ||||||||||||||||||||||
| // Here the scan is identified by report-task.txt or --sonar-ce-task-url, and | ||||||||||||||||||||||
| // the branch is read from the scan task, so a supplied branch reaches nothing. | ||||||||||||||||||||||
| // Say so rather than ignoring it in silence. | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // Reaching this block is what makes that true: readFile puts the file's | ||||||||||||||||||||||
| // ceTaskUrl on sc.CETaskUrl, so both ways of naming a scan by its task arrive | ||||||||||||||||||||||
| // here, and an empty analysisID means the project-key path did not already | ||||||||||||||||||||||
| // resolve one. | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // Unlike the missing-task warning at the end of this function, this one is | ||||||||||||||||||||||
| // deliberately emitted before the lookups that can fail. "The flag you passed | ||||||||||||||||||||||
| // is ignored" is true whether or not the run then succeeds, and is worth | ||||||||||||||||||||||
| // saying either way; the other warning describes a payload, so it would be a | ||||||||||||||||||||||
| // false statement on a run that never publishes one. | ||||||||||||||||||||||
Comment on lines
+262
to
+266
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale after The rule it states is worth keeping — it's the reasoning that decides where this warning goes — it just needs to stand on its own rather than lean on the removed one:
Suggested change
| ||||||||||||||||||||||
| if sc.branch != "" { | ||||||||||||||||||||||
| logger.Warn("--sonar-branch is ignored when the scan is identified by report-task.txt or --sonar-ce-task-url: the branch is read from the scan task") | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
AlexKantor87 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||||||||||||||||||||||
| //Get the analysis ID, status, project name and branch data from the ceTaskURL (ce API) | ||||||||||||||||||||||
| analysisID, err = GetCETaskData(httpClient, project, sonarResults, sc.CETaskUrl, sc.maxWait, logger) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| @@ -380,7 +431,7 @@ func GetCETaskData(httpClient *http.Client, project *Project, sonarResults *Sona | ||||||||||||||||||||||
| func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *SonarResults, project *Project, revision string, logger *log.Logger) (string, error) { | ||||||||||||||||||||||
| var analysisID string | ||||||||||||||||||||||
| projectAnalysesURL, err := sonarURL(sonarResults.ServerUrl, "api/project_analyses/search", url.Values{"project": {project.Key}}) | ||||||||||||||||||||||
| projectAnalysesURL, err := analysesSearchURL(sonarResults, project) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return "", err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| @@ -418,19 +469,22 @@ func GetProjectAnalysisFromRevision(httpClient *http.Client, sonarResults *Sonar | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if sonarResults.AnalysedAt == "" { | ||||||||||||||||||||||
| return "", fmt.Errorf("analysis for revision %s of project %s not found. Check the revision is correct. \nThe scan may still be being processed by SonarQube, try again later.\n Otherwise if you are attesting an older scan, the snapshot may also have been deleted by SonarQube", revision, project.Key) | ||||||||||||||||||||||
| // An empty result reads like a permissions problem, so say which branch was | ||||||||||||||||||||||
| // actually searched: unscoped, SonarQube only searches the main branch (#1116). | ||||||||||||||||||||||
| scope := "only the project's main branch was searched, because no --sonar-branch was given" | ||||||||||||||||||||||
| advice := "Check the revision is correct, and pass --sonar-branch if the scan ran on another branch." | ||||||||||||||||||||||
| if sonarResults.Branch != nil && sonarResults.Branch.Name != "" { | ||||||||||||||||||||||
| scope = fmt.Sprintf("branch %s was searched", sonarResults.Branch.Name) | ||||||||||||||||||||||
| advice = "Check the revision and the branch are correct." | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return "", fmt.Errorf("analysis for revision %s of project %s not found: %s. %s \nThe scan may still be being processed by SonarQube, try again later.\n Otherwise if you are attesting an older scan, the snapshot may also have been deleted by SonarQube", revision, project.Key, scope, advice) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return analysisID, nil | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| func GetProjectAnalysisFromAnalysisID(httpClient *http.Client, sonarResults *SonarResults, project *Project, analysisID string) error { | ||||||||||||||||||||||
| // Forward branch to find analyses on non-default branches (#861). | ||||||||||||||||||||||
| params := url.Values{"project": {project.Key}} | ||||||||||||||||||||||
| if sonarResults.Branch != nil && sonarResults.Branch.Name != "" { | ||||||||||||||||||||||
| params.Set("branch", sonarResults.Branch.Name) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| projectAnalysesURL, err := sonarURL(sonarResults.ServerUrl, "api/project_analyses/search", params) | ||||||||||||||||||||||
| projectAnalysesURL, err := analysesSearchURL(sonarResults, project) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.