Repository navigation
[None][infra] Add SonarQube code analysis in github action - #10677
yuanjingx87 wants to merge 3 commits into
Conversation
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughSonarQube code analysis integration is being introduced to the CI pipeline. A new dedicated workflow ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/sonarqube-code-analysis.yml:
- Around line 15-18: The workflow is missing an explicit permissions block; add
a top-level permissions block in the workflow (above or alongside "jobs" /
"build") that grants least-privilege scopes required for analysis—for example
set contents: read and security-events: write (and actions: read if needed by
used actions) so the "build" job and SonarQube/code-scanning actions have
explicit, minimal permissions.
🧹 Nitpick comments (2)
sonar-project.properties (1)
1-1: Consider adding additional SonarQube properties for more precise analysis.The minimal configuration will work, but you may want to consider adding properties such as
sonar.sources,sonar.exclusions, orsonar.python.versionto improve analysis accuracy and exclude irrelevant files (e.g., third-party code, build artifacts).Example enhanced configuration
sonar.projectKey=GPUSW_TensorRT-LLM-Team_TensorRT-LLM_tensorrt-llm +sonar.sources=. +sonar.exclusions=**/third_party/**,**/build/**,**/dist/**.github/workflows/blossom-ci.yml (1)
372-376: SonarQube integration looks good; consider version consistency.The SonarQube scan step is correctly placed after checkout and properly uses secrets via environment variables. However, note that this workflow uses
actions/checkout@v2(line 366) while the newsonarqube-code-analysis.ymlusesactions/checkout@v6. Consider aligning versions for consistency and to benefit from security/performance improvements.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/blossom-ci.yml.github/workflows/sonarqube-code-analysis.ymlsonar-project.properties
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/sonarqube-code-analysis.yml
[warning] 17-37: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Pre-commit Check
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
.github/workflows/sonarqube-code-analysis.yml (2)
26-29: SonarQube scan configuration is correct.The scan action is properly configured with secrets passed via environment variables. The
fetch-depth: 0on checkout ensures full git history is available for accurate blame analysis.
21-24: No action needed —actions/checkout@v6is a valid, released version.The version reference is correct. v6 is officially available with documented release notes.
Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| jobs: | ||
| build: | ||
| name: Build and analyze | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Add explicit permissions block to follow security best practices.
As flagged by CodeQL, the workflow lacks an explicit permissions block. Adding minimal permissions follows the principle of least privilege and improves security posture.
Proposed fix
jobs:
build:
name: Build and analyze
runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ pull-requests: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jobs: | |
| build: | |
| name: Build and analyze | |
| runs-on: ubuntu-latest | |
| jobs: | |
| build: | |
| name: Build and analyze | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| pull-requests: read |
🤖 Prompt for AI Agents
In @.github/workflows/sonarqube-code-analysis.yml around lines 15 - 18, The
workflow is missing an explicit permissions block; add a top-level permissions
block in the workflow (above or alongside "jobs" / "build") that grants
least-privilege scopes required for analysis—for example set contents: read and
security-events: write (and actions: read if needed by used actions) so the
"build" job and SonarQube/code-scanning actions have explicit, minimal
permissions.
c765eac to
364bb00
Compare
364bb00 to
8aac65b
Compare
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
8aac65b to
436edef
Compare
| name: Build and analyze | ||
| runs-on: blossom | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis | ||
| ref: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.ref || github.ref }} | ||
|
|
||
| - uses: SonarSource/sonarqube-scan-action@v4 | ||
| env: | ||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | ||
| SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }} | ||
| # If you wish to fail your job when the Quality Gate is red, uncomment the | ||
| # following lines. This would typically be used to fail a deployment. | ||
| # We do not recommend to use this in a pull request. Prefer using pull request | ||
| # decoration instead. | ||
| # - uses: SonarSource/sonarqube-quality-gate-action@v1 | ||
| # timeout-minutes: 5 | ||
| # env: | ||
| # SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
In general, the fix is to explicitly declare a permissions: block so the GITHUB_TOKEN has only the minimal access required. For this job, it only needs to read repository contents and metadata for analysis, so setting contents: read is appropriate. This can be done either at the workflow root (applies to all jobs) or at the job level (just for build). Since CodeQL flags the job, the clearest fix is to add permissions: under jobs.build.
Concretely, in .github/workflows/sonarqube-code-analysis.yml, add a permissions: section under jobs.build (between runs-on: blossom and steps:), indented two spaces further than build:. Set contents: read inside it. No imports or additional methods are needed; this is purely a YAML configuration change and will not alter existing step behavior, except that it tightens the default token privileges.
| @@ -16,6 +16,8 @@ | ||
| build: | ||
| name: Build and analyze | ||
| runs-on: blossom | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: |
| jobs: | ||
| build: | ||
| name: Build and analyze | ||
| runs-on: blossom |
There was a problem hiding this comment.
We probably won't be able to use the blossom runner for these actions, since they have some special configuration designed to work only with the blossom workflow. I think the standard ubuntu-latest runner should work for this, unless the SonarQube server requires special network access.
There was a problem hiding this comment.
Yeah, this needs to hit internal sonarqube API like: https://sonar.nvidia.com/api/server/version
I closed this PR for now and play with it in my fork, now I set up a new runner with my dev-box in my fork to get it running
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Description
Add SonarQube code analysis in github action
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
Details
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.