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

Launch Sonarqube validation in docker in our CI for each PR validation #4912

Open
romani opened this Issue Aug 8, 2017 · 11 comments

Comments

Projects
None yet
2 participants
@romani
Member

romani commented Aug 8, 2017

Most problems are already resolved in scope of #46.

This tool is also already introduced, but not enforced over the code. I need to make it possible to activate Sonar for PR validation to prevent bad code leak to the project. To do this, I need to make it possible to run Sonar in Docker on CI, put all configs in Sonar by web API and then launch validation of sonar for checkstyle code, as stated here. As validation process will be finished, by means of Sonar API I recheck that no new violations has appeared. Current version of sonar can be found here

Result of work should be:

  • ready to use docker config file to let us build and deploy it to our docker hub.
  • update to codeship CI (or wercker CI) on how to use such docker image and command to run validation of sources. All required sonarqube configs should be stored in "config" folder and provided to sonarqube by its HTTP API.
  • execute sonarqube validation by "mvn sonar:sonar"
  • Sonar gate values should be configurable by our config, to enforce some level of quality ASAP. Each fix for violations should make numbers in gate lower and lower. Final state is any violation(infor, ...blocker) is 0 in code.
  • All violations should be resolved or suppressed (the same as we did with others tools)
  • any new violation in code should break the build of CI.

I highly recommend to provide fixes for our code(resolving violations) separately for docker/CI config.
All questinable violations should be discussed rule by rule. There will be bunch of suppressions and changes to configuration of Sonar's default config.

@romani romani changed the title from Launch Sonarqube validation in docker to Launch Sonarqube validation in docker in our CI for each PR validation Aug 8, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 9, 2017

Member

docker image that could be used - "checkstyle/sonarqube-maven-git" , already used at https://github.com/checkstyle/sonar-checkstyle/blob/master/wercker.yml#L1 , sources of it is at https://github.com/checkstyle/sonar-checkstyle/blob/master/checkstyle-sonar-plugin/config/Dockerfile

Example of validation execution in dockered sonar - https://github.com/checkstyle/sonar-checkstyle/blob/master/wercker.yml#L16
some brief intro on how we tested our sonar plugin by dockered sonar - https://github.com/checkstyle/sonar-checkstyle/wiki/How-to-test-released-binary

Attention: to avoid long/slow interaction with web CIs, you need to prepare SHELL script that do all commands for configuring sonar server and execution of validation and check of results. I will need to approve all that process first and approve merge of it, only after that we could start to make it work in web CI (web CIs are very slow!!! but you can make your custom CI , for you fork to avoid long queue of builds. wercker and shippable CIs are free for open source projects.).

Member

romani commented Aug 9, 2017

docker image that could be used - "checkstyle/sonarqube-maven-git" , already used at https://github.com/checkstyle/sonar-checkstyle/blob/master/wercker.yml#L1 , sources of it is at https://github.com/checkstyle/sonar-checkstyle/blob/master/checkstyle-sonar-plugin/config/Dockerfile

Example of validation execution in dockered sonar - https://github.com/checkstyle/sonar-checkstyle/blob/master/wercker.yml#L16
some brief intro on how we tested our sonar plugin by dockered sonar - https://github.com/checkstyle/sonar-checkstyle/wiki/How-to-test-released-binary

Attention: to avoid long/slow interaction with web CIs, you need to prepare SHELL script that do all commands for configuring sonar server and execution of validation and check of results. I will need to approve all that process first and approve merge of it, only after that we could start to make it work in web CI (web CIs are very slow!!! but you can make your custom CI , for you fork to avoid long queue of builds. wercker and shippable CIs are free for open source projects.).

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Aug 15, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Aug 15, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Aug 18, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 20, 2017

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Aug 21, 2017

Contributor

profile for wercker:

box: checkstyle/sonarqube-maven-git

build:
  steps:
  - script:
      name: setup mvn local repo
      code: |-
        export MAVEN_OPTS="-Dmaven.repo.local=${WERCKER_CACHE_DIR}"
        mvn -version
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}/* | sort -h

  # Build Checkstyle
  - script:
      name: Package plugin, install to sonar
      code: |-
        cd /opt/sonarqube/
        echo "starting sonar"
        ./bin/run.sh &

        echo "sleeping 60 to let sonar start up"
        sleep 60

        cd -
        source ./.ci/sonar.sh

  # Cleanup
  - script:
      name: Cleanup maven local repo
      code:  |-
        find ${WERCKER_CACHE_DIR} -type d -name "*SNAPSHOT" -ls -exec rm -rf {} +
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}/* | sort -h
        echo "------"
        du -hs * | sort -h

Contributor

Nimfadora commented Aug 21, 2017

profile for wercker:

box: checkstyle/sonarqube-maven-git

build:
  steps:
  - script:
      name: setup mvn local repo
      code: |-
        export MAVEN_OPTS="-Dmaven.repo.local=${WERCKER_CACHE_DIR}"
        mvn -version
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}/* | sort -h

  # Build Checkstyle
  - script:
      name: Package plugin, install to sonar
      code: |-
        cd /opt/sonarqube/
        echo "starting sonar"
        ./bin/run.sh &

        echo "sleeping 60 to let sonar start up"
        sleep 60

        cd -
        source ./.ci/sonar.sh

  # Cleanup
  - script:
      name: Cleanup maven local repo
      code:  |-
        find ${WERCKER_CACHE_DIR} -type d -name "*SNAPSHOT" -ls -exec rm -rf {} +
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}
        echo "------"
        du -hs ${WERCKER_CACHE_DIR}/* | sort -h
        echo "------"
        du -hs * | sort -h

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Aug 21, 2017

@romani romani moved this from To Do to In Progress in Practice What You Preach Aug 22, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 24, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 24, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 27, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 31, 2017

romani added a commit that referenced this issue Aug 31, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 31, 2017

Member

@Nimfadora ,

  1. Without suprpessions, this tool is not usable. Please create simple config that suppres as "Wont fix" or "False positive" some certain issues on certain code.

  2. please prepare PR that will cause non zero exit code from shell script when sonar detect new issue.

  3. https://www.versioneye.com/user/projects/5504ca834a1064774400049a
    please upgrade sonar-maven-plugin from 2.7.1 to 3.3.0.603

Member

romani commented Aug 31, 2017

@Nimfadora ,

  1. Without suprpessions, this tool is not usable. Please create simple config that suppres as "Wont fix" or "False positive" some certain issues on certain code.

  2. please prepare PR that will cause non zero exit code from shell script when sonar detect new issue.

  3. https://www.versioneye.com/user/projects/5504ca834a1064774400049a
    please upgrade sonar-maven-plugin from 2.7.1 to 3.3.0.603

@romani romani added this to the 8.3 milestone Aug 31, 2017

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Sep 2, 2017

Contributor

@romani

Without suprpessions, this tool is not usable. Please create simple config that suppres as "Wont fix" or "False positive" some certain issues on certain code.

I found this functionality in SonarQube UI and was able to perform it via REST calls.
However, this will not work for us, cause request contains issueId.
E.g.

curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/do_transition?issue=ac539576-df80-4a1f-9631-991b31ad27ef&transition= falsepositive'

Issue ID changes each time Sonar restarted (I've checked this)...

Contributor

Nimfadora commented Sep 2, 2017

@romani

Without suprpessions, this tool is not usable. Please create simple config that suppres as "Wont fix" or "False positive" some certain issues on certain code.

I found this functionality in SonarQube UI and was able to perform it via REST calls.
However, this will not work for us, cause request contains issueId.
E.g.

curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/do_transition?issue=ac539576-df80-4a1f-9631-991b31ad27ef&transition= falsepositive'

Issue ID changes each time Sonar restarted (I've checked this)...

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 2, 2017

Member

@Nimfadora ,

their suppressions are persistent, GUID could be different , yes, but they store suppression in their DB and all of them are persistent during all validations and server restarts. There should be some other way. Please ask question on their mail list on how to do this.

Member

romani commented Sep 2, 2017

@Nimfadora ,

their suppressions are persistent, GUID could be different , yes, but they store suppression in their DB and all of them are persistent during all validations and server restarts. There should be some other way. Please ask question on their mail list on how to do this.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Sep 2, 2017

Contributor

@romani by restarts I meant docker container remove/create. There nothing will be persistent... Unless we will create a custom sonar docker image, with predefined DB.

I have an idea, how we still can use it - Sonar by default uses H2 database. We can do following:
-Start sonar locally
-run validation of checkstyle code.
-suppress all issues in UI
-dump DB to the SQL script
-add execution of SQL script to the our shell script before execution of analysis

Does it make sense?

Contributor

Nimfadora commented Sep 2, 2017

@romani by restarts I meant docker container remove/create. There nothing will be persistent... Unless we will create a custom sonar docker image, with predefined DB.

I have an idea, how we still can use it - Sonar by default uses H2 database. We can do following:
-Start sonar locally
-run validation of checkstyle code.
-suppress all issues in UI
-dump DB to the SQL script
-add execution of SQL script to the our shell script before execution of analysis

Does it make sense?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 3, 2017

Member

yes, it make sense.
But if Sonar can take that data from DB , it mean that it could/should take the same from API.
It mean that Sonar use not only guid to match violation to code. There should be smth else - line, col, xpath to node in AST, .... .
Look at H2 content to see how violation is matched with suppression.

Member

romani commented Sep 3, 2017

yes, it make sense.
But if Sonar can take that data from DB , it mean that it could/should take the same from API.
It mean that Sonar use not only guid to match violation to code. There should be smth else - line, col, xpath to node in AST, .... .
Look at H2 content to see how violation is matched with suppression.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Sep 3, 2017

Contributor

@romani Looked in the DB structure, basing on 2 rules, which were giving violations -

  1. code duplicates - this has no code markers. Only the file name. Also, I've noticed, that on Sonar UI this issue is always displayed on top of the file with comment like - "There is some code duplications somewhere in this file"
  2. usage of deprecated - this rule has code marker - line. This is present in ISSUES table.
Contributor

Nimfadora commented Sep 3, 2017

@romani Looked in the DB structure, basing on 2 rules, which were giving violations -

  1. code duplicates - this has no code markers. Only the file name. Also, I've noticed, that on Sonar UI this issue is always displayed on top of the file with comment like - "There is some code duplications somewhere in this file"
  2. usage of deprecated - this rule has code marker - line. This is present in ISSUES table.
@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Sep 3, 2017

Contributor

Only the file name

Actually file ID. To get actual file name, in both cases, we need to go to the inner Elastic Search. And query the components collection, to match file ID from the DB to the file name.

By the way, this looks like, that the approach with SQL dump will not be correct. ES index is also required.

But, all sonar data is located in one folder. instead of dumping/executing SQL we can archive this folder in some canonical state (with all suppression), compress it, upload somewhere. And in script just get it, and put into appropriate folder.

The size of data - ±100MB. Compressed - 76 MB

Maybe, we can pretend it is a Maven dependency and download it in repository folder, just as we did with Eclipse jar

Contributor

Nimfadora commented Sep 3, 2017

Only the file name

Actually file ID. To get actual file name, in both cases, we need to go to the inner Elastic Search. And query the components collection, to match file ID from the DB to the file name.

By the way, this looks like, that the approach with SQL dump will not be correct. ES index is also required.

But, all sonar data is located in one folder. instead of dumping/executing SQL we can archive this folder in some canonical state (with all suppression), compress it, upload somewhere. And in script just get it, and put into appropriate folder.

The size of data - ±100MB. Compressed - 76 MB

Maybe, we can pretend it is a Maven dependency and download it in repository folder, just as we did with Eclipse jar

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Sep 3, 2017

Contributor

Also, I've checked, if we can create(predefine) issues with Web Api.
Looks like - no.
I've checked the doc - https://docs.sonarqube.org/pages/viewpage.action?pageId=2392181 (Create a Manual Issue), and there is a limitation of rule key, only some manual rules are available.

Contributor

Nimfadora commented Sep 3, 2017

Also, I've checked, if we can create(predefine) issues with Web Api.
Looks like - no.
I've checked the doc - https://docs.sonarqube.org/pages/viewpage.action?pageId=2392181 (Create a Manual Issue), and there is a limitation of rule key, only some manual rules are available.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 3, 2017

Member

if suppression is not easy to change, nobody will do this. (Re)Creation of some SQL dump is not an option.

You already know suppressions are very frequent event, and should be done in the same PR (plain text form).

Please contact Sonar support to ask them a question.

Member

romani commented Sep 3, 2017

if suppression is not easy to change, nobody will do this. (Re)Creation of some SQL dump is not an option.

You already know suppressions are very frequent event, and should be done in the same PR (plain text form).

Please contact Sonar support to ask them a question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment