Skip to content

ci: enable sonar scanning and report results to sonacloud.io #788

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

arjun-rajappa
Copy link

No description provided.

@arjun-rajappa arjun-rajappa requested a review from a team as a code owner August 21, 2025 13:03
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Copy link
Member

@pvital pvital left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the image of the final_job to use python:3.13, please?

@@ -296,7 +303,7 @@ jobs:
- pip-install-deps
- pip-install-tests-deps
- store-pytest-results
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove those three steps and only install coverage by changing line 96 to something like:

            python -m venv venv
            . venv/bin/activate
            pip install --upgrade pip coverage

Copy link

@arjun-rajappa the signed-off-by was not found in the following 1 commits:

  • 2c81ec1: Merge branch 'main' into sonar-scan

📝 What should I do to fix it?

All proposed commits should include a sign-off in their messages, ideally at the end.

❔ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO, reformatted for readability:

By making a contribution to this project, I certify that:

a. The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

b. The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

c. The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

d. I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <[email protected]>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

Copy link

Copy link
Member

@pvital pvital left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions and requests....

Comment on lines +298 to +306
- image: public.ecr.aws/docker/library/python:3.13
working_directory: ~/repo
steps:
- checkout
- check-if-tests-needed
- pip-install-deps
- pip-install-tests-deps
- store-pytest-results
# - run_sonarqube
- run_sonarqube
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are still executing the pip install commands that are not necessary here:

Suggested change
- image: public.ecr.aws/docker/library/python:3.13
working_directory: ~/repo
steps:
- checkout
- check-if-tests-needed
- pip-install-deps
- pip-install-tests-deps
- store-pytest-results
# - run_sonarqube
- run_sonarqube
- image: public.ecr.aws/docker/library/python:3.13
working_directory: ~/repo
steps:
- checkout
- check-if-tests-needed
- run_sonarqube

Comment on lines +96 to +99
python -m venv venv
. venv/bin/activate
coverage combine ./coverage_results
coverage xml -i
wget -O /tmp/sonar-scanner-cli.zip https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-4.8.1.3023.zip
unzip -d /tmp /tmp/sonar-scanner-cli.zip
if [[ -n "${CIRCLE_PR_NUMBER}" ]]; then
/tmp/sonar-scanner-4.8.1.3023/bin/sonar-scanner \
-Dsonar.host.url=${SONARQUBE_URL} \
-Dsonar.login="${SONARQUBE_LOGIN}" \
-Dsonar.pullrequest.key="${CIRCLE_PR_NUMBER}" \
pip install --upgrade pip coverage

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the files used by the coverage command you removed necessary for the sonar-scanner execution?

If not, you can then continue to remove those two lines, but nothing else is necessary to install, then:

Suggested change
python -m venv venv
. venv/bin/activate
coverage combine ./coverage_results
coverage xml -i
wget -O /tmp/sonar-scanner-cli.zip https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-4.8.1.3023.zip
unzip -d /tmp /tmp/sonar-scanner-cli.zip
if [[ -n "${CIRCLE_PR_NUMBER}" ]]; then
/tmp/sonar-scanner-4.8.1.3023/bin/sonar-scanner \
-Dsonar.host.url=${SONARQUBE_URL} \
-Dsonar.login="${SONARQUBE_LOGIN}" \
-Dsonar.pullrequest.key="${CIRCLE_PR_NUMBER}" \
pip install --upgrade pip coverage

If they are necessary, you should keep those lines and change the order of them:

Suggested change
python -m venv venv
. venv/bin/activate
coverage combine ./coverage_results
coverage xml -i
wget -O /tmp/sonar-scanner-cli.zip https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-4.8.1.3023.zip
unzip -d /tmp /tmp/sonar-scanner-cli.zip
if [[ -n "${CIRCLE_PR_NUMBER}" ]]; then
/tmp/sonar-scanner-4.8.1.3023/bin/sonar-scanner \
-Dsonar.host.url=${SONARQUBE_URL} \
-Dsonar.login="${SONARQUBE_LOGIN}" \
-Dsonar.pullrequest.key="${CIRCLE_PR_NUMBER}" \
pip install --upgrade pip coverage
python -m venv venv
. venv/bin/activate
pip install --upgrade pip coverage
coverage combine ./coverage_results
coverage xml -i

Comment on lines +111 to +113
-Dsonar.organization=instana \
-Dsonar.projectKey=instana_python-sensor \
-Dsonar.sources=. \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't these lines overwrite the configuration of the sonar-project.properties file?

In addition, I guess it should be the following to only check the library code, not everything in the repository:

Suggested change
-Dsonar.organization=instana \
-Dsonar.projectKey=instana_python-sensor \
-Dsonar.sources=. \
-Dsonar.organization=instana \
-Dsonar.projectKey=instana_python-sensor \
-Dsonar.sources=.src/instana/ \

Comment on lines +119 to +121
-Dsonar.organization=instana \
-Dsonar.projectKey=instana_python-sensor \
-Dsonar.sources=. \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the previous comment.

Comment on lines 6 to +7
sonar.tests=tests/
sonar.test.inclusions=test/**/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the documentation mentions, if the sonar.tests "property is not defined, no code will be analyzed as test code as there is no default value."

I guess we can remove those two lines:

Suggested change
sonar.tests=tests/
sonar.test.inclusions=test/**/*

@pvital
Copy link
Member

pvital commented Aug 22, 2025

@arjun-rajappa, perhaps we can use the pysonar CLI command with the configuration in the pyproject.toml file to run the checking as described in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants