SonarQube 8.1 Support #58
Conversation
|
See sonarqube-community-branch-plugin-1.2.1-SNAPSHOT.zip for a ZIP file containing the plugin/module JAR for anyone who wants to try it out. Please report any initial testing results here. |
mc1arke
force-pushed
the
sq-8_1-support
branch
from
a1a9ddd
to
fad4c32
Compare
|
Hi. I also tryied the sonarqube-community-branch-plugin-1.2.1-SNAPSHOT.zip with SonarQube 8.1.0.31237. I did it under Azure DevOps build. For me integration with SonarQube 8.1 works correctly. I did't had issues as @Sotam reported. I tried analysis of different branches and pull requests. |
|
Hi, please ignore my previous post (I've also deleted it) as it's working now. I'm not quite sure how, but I've setup a new SonarQube (8.1.0.31237) docker container and it's working now. Thanks! |
|
Running 8.1 [ERROR] Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:3.7.0.1746:sonar (default-cli) on project Varial-Fibu-Pojos: Unable to load component class org.sonar.scanner.scan.filesystem.InputComponentStore:
Unable to load component interface org.sonar.scanner.scan.branch.BranchConfiguration: SHORT -> [Help 1]``` |
|
Hi, i tried it on sonarqube version 8.1.0.31237 and I get the following error, when running In the server log I see the following error: |
|
@stefanwendelmann are you sure you're running the right version of the plugin in both the @stefan01 It sounds like you've not installed the plugin to one of these locations. Please check you've followed the installation instructions fully. |
|
@mc1arke thx for the fast response! |
|
@mc1arke running 1.2.0 in both folders |
|
@mc1arke could you rebase this onto latest master features such as pr decorations? |
|
Hello, I have tested the new Sonarqube v8.1 with the sonarqube-community-branch-plugin-1.2.1-SNAPSHOT I can see a difference between the Sonarqube v8.0 with the sonarqube-community-branch-plugin-1.2.0-SNAPSHOT and this new version : From Sonarqube v8.0 + 1.2.0-SNAPSHOT : From Sonarqube v8.1 + 1.2.1-SNAPSHOT : Am I the only one with this result, or is it same in other testers ? Thanks |
mc1arke
force-pushed
the
sq-8_1-support
branch
from
fad4c32
to
fdb382a
Compare
|
I've pushed an updated version of my 8.1 modifications that provides a few changes. See the commit message for full details but the highlights are:
@4n4n4s I'm interested in your opinions on this change given it removed the delete functionality from the Bitbucket decorator you'd written. @tisoft I'm also interested in your opinion on the fact I've removed the delete functionality for the Gitlab decorator, but you may also want to take a look at the Could I ask for people to run this and report any feedback (both positive and negative) please? |
|
Hi @mc1arke I think for Bitbucket the delete functionality can still be kept. The challenge is, that you need to find the userSlug for the user, that is commenting on the PR. If you don't want to add those additional feature toggles (delete comments, comment on file level, comment overall) to the UI anymore, then I would suggest that the default for delete should also be enabled, as otherwise the PR will be cluttered with tons and tons of comments if it's open for some time. |
|
I'd just like to agree with @4n4n4s, deleting old comments is really useful because MR (GitLab) / PR (everything else) will be filled with old, irrelevant comments. :) |
|
We are just using the plugin with standard git I get this error from a gradle build using the snapshot on 8.1 |
|
@jrepko You need to provide a bit more information if you want help diagnosing your issue: what command are you using to run your scan, what configuration is in your |
|
@4n4n4s I agree with you on the cluttering with comments. In Github this is handled by creating CheckRuns per commit and only the last CheckRun being kept for each app on each commit. Gitlab has the concept of resolving a discussion, which is what I think we should be using for this case, as deleting a comment still leaves the discussion marker in place, but without any relevant content. We'd need to add some logic for identifying where an issue was no longer present in a re-scan so we scan ensure we're not closing discussions just to create an identical issue, especially if a real discussion has then stemmed from the Sonarqube comment For the Bitbucket use-case, I have no issue with deleting comments if we can identify the correct ones to delete. However, we probably need to check if someone has responded to a comment as we'd then end up with orphaned responses, or Sonarqube creating a comment on a response that may reference the original Sonarqube comment. Does the endpoint you've suggested exist in all versions of Bitbucket server (can we reliably depend on it)? Is it worth introducing a pattern into the decorators where they get told if an issue is new/updated/resolved so they can then check for a comment relating to that issue and either create a new comment or delete/resolve/comment-on an outdated comment? |
|
I agree, that resolving discussions instead of deleting them is the way to go for gitlab. I think, that discussions on obsolete commits (e.g. in case of rebases) are autoresolved. But I think if there is an issue on a commit and a new commit fixes this, the discussion might stay there. I will test the behaviour, and see what needs to be done for gitlab. |
|
The auto-resolution of out-dated discussions on Gitlab is a configurable feature. I know that the main instance I regularly use has the auto-resolution disabled on all their projects to ensure that any discussions are still reviewed to ensure that a subsequent change not actually associated with the discussion on that line doesn't incorrectly resolve the discussion (e.g. changing formatting, but not fixing a potential |
|
@mc1arke for Bitbucket you cannot delete comments that has responses so the discussion will remain. Actually, Bitbucket has a similar concept as CheckRun at GitHub, but it only came in new versions. Looks like that's how the paid sonar works https://docs.sonarqube.org/latest/analysis/pr-decoration/ Endpoint "/plugins/servlet/applinks/whoami" was added for testing but it's on by default and was added a long time ago. I think it can be used by default, and if an error occurs, require a login in the configuration. |
|
@artemy-osipov Are you sure you can't delete comments that have responses? The UI certainly allows me to do it on bitbucket.com, I've not checked through the API on a BitBucket server instance though. Whilst possibly less useful for some people who aren't quick to upgrade their self-hosted software, is it worth switching to use the new BitBucket features so we don't have to worry about using an undocumented/unsupported API, and can get the benefit of any future enhancements those features provide? |
|
@mc1arke I checked the master branch for this case and the sonar comments with the replies were not deleted. This corresponds to documentation:
It's most likely because Bitbucket Server and Bitbucket Cloud are different products with different code bases and the rules for working with comments may differ. I think the Code Insight reports is more promising and preferable, but until it is implemented, I hope the current way through comments will remain so that we can start using this plugin. |
@mc1arke and the config from the build.gradle has The exception is below: |
I have the same problem. I already ensured the plugin is in both locations but the problem persists. |
|
HI guys, We have same issue, can you provide the last 1.2.1-SNAPSHOT for test on sonar 8.1 ? |
|
Test latest release 1.3.0 with sonarqube 8.2 and github integration After save configuration, I got next error: |
|
Just asking, I see that #85 is fixed here. Is that fix compatible with LTS (7.9) and could be backported to that? |
|
I've update the branch with a refactor of my previous changes plus the missing unit tests for the new functionality. However, the submission of a new branch to Sonarqube does not give the results I'd expect: it treats all new issues as an existing issue, alongside the existing issues. Subsequent scans then show some of these new issues as existing issues, which seems incorrect. I've not manage to pinpoint what Sonarqube is failing with here, but suspect the reference branch being passed in the scanner result is being set incorrectly. I'll dig into the further, but would welcome any other input here. |
|
Hello, I built a snapshot version from this branch, plugin is working and I got PR analyzed. I there any way how to debug it? |
|
@mc1arke Thanks a lot for all the good work. Just so you know, I've got it working with Github. Besides the big issue of mixing old with existing issues, I found two small things:
Cheers! |
mc1arke
force-pushed
the
sq-8_1-support
branch
2 times, most recently
from
11eac55
to
9b3aedf
Compare
Sonarqube 8.1 removed the concept of short and long lived branches, instead simplifying the setup to either `BRANCH` or `PULL_REQUEST`. This release of Sonarqube also moved the management of Pull Request decoration into a standard User Interface calling a set of 'ALM' services that provide the management of decorators, and the binding of each project to these decorators. However the implementation of these services is not included in the Community Edition of Sonarqube, although the UI is made visible based on the presence of the branch management components provided by this plugin. This change therefore introduces the services required to support UI components for the management of Pull Request decoration, as well as updating the handling of the configuration and loading of branches to support the removal of the SHORT/LONG branch constructs. As these changes require the reference of classes that were not present in older version of Sonarqube (namely `AlmSettingsDao` and `ProjectAlmSettingsDao`), the compatibility interfaces for these versions have been removed, as well as any methods and classes that existed purely to allow these versions to be supported by this plugin, meaning this version of the plugin is not backwards compatible with previous Sonarqube versions. Given the standard UI provides a restricted set of fields for creating and binding each Pull Request decorator, some of the patterns that were previously used by this plugin for configuring the decoration of Pull Requests do not fit into this UI, and were awkward to find/manage when moved to another screen in the UI. This results in the configuration for disabling the addition and removing of comments on Merge Requests being removed from the plugin, with the Gitlab decorator removing the deleting of old comments since this was leading to discussion boxes being shown in Gitlab with no content, and the BitBucket decorator removing since it can't work out which user posted comments based purely on the configuration provided by Sonarqube. To ensure the UI works for all configuration options, the services for configuring and binding Azure DevOps configuration have been included in this change-set, although no decorator currently exists for this ALM, so any project attempting to use this configuration will get a warning appear in the CE logs when attempting to use this decorator. Similarly, as the UI does not provide any options for configuring the URL for the Gitlab API, or the Slug/name of the project on the binding, a `Sensor` has been added into the scanner to detect these properties from injection by the Gitlab CI Runner, as well as injection directly from scanner arguments of `com.github.mc1arke.sonarqube.plugin.branch.pullrequest.gitlab.url` and `com.github.mc1arke.sonarqube.plugin.branch.pullrequest.gitlab.repositorySlug` for the repository URL and repository-name configuration entries.
The Github ALM Binding Web Service uses the `AlmRepo` field to store the repository name, but the Github decorator was using `AlmSlug` to try and retrieve the repository name, so was getting a `null` value back and failing to find a matching repository. Switching to using `AlmRepo` in the decorator overcomes this issues.
|
Kudos, SonarCloud Quality Gate passed!
|
Adds support for Sonarqube 8.1. As this change is not completely backwards compatible, the approach taken here may be introducing more complexity than required, so may be subject to change. In the meantime, this PR should be backwards compatible with Sonarqube 7.8 to 8.0 (inclusive)
Please see the latest comments on this PR: the recent changes mean that this PR will no longer support SonarQube 8.0 and any older versions