Skip to content
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

Add sonarqube integration #2771

Merged
merged 1 commit into from
May 18, 2024
Merged

Add sonarqube integration #2771

merged 1 commit into from
May 18, 2024

Conversation

SorsOps
Copy link
Member

@SorsOps SorsOps commented May 18, 2024

User description

Why does this PR exist?

Integrates Sonarqube scanning for advanced vulnerability and static code analysis

What does this pull request do?

Adds a job after merging into main to perform and update analysis in sonarqube

Testing this change

Test integration post merge to verify working as expected


PR Type

Enhancement


Description

  • Added a new 'analysis' job in the GitHub Actions workflow to perform Sonarqube scans after merges into the main branch.
  • Configured the 'analysis' job to fetch secrets from HashiCorp Vault for Sonarqube authentication.
  • Set up the Sonarqube scan with the necessary environment variables using the Sonarqube scan action.
  • Renamed the existing GitHub Actions workflow from 'Techdocs' to 'Merge'.
  • Added a Sonarqube project key in the sonar-project.properties file to link the repository with the Sonarqube server.

Changes walkthrough 📝

Relevant files
Enhancement
merge.yml
Integrate Sonarqube Analysis in GitHub Actions Workflow   

.github/workflows/merge.yml

  • Renamed workflow from 'Techdocs' to 'Merge'
  • Added a new job 'analysis' to run Sonarqube scans
  • Configured job to use secrets from HashiCorp Vault for Sonarqube
    authentication
  • Set up Sonarqube scan action with necessary environment variables
  • +29/-1   
    Configuration changes
    sonar-project.properties
    Configure Sonarqube Project Key                                                   

    sonar-project.properties

    • Added project key configuration for Sonarqube
    +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    changeset-bot bot commented May 18, 2024

    ⚠️ No Changeset found

    Latest commit: 3922dee

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    @SorsOps SorsOps merged commit 85420dd into main May 18, 2024
    8 checks passed
    @SorsOps SorsOps deleted the feat/sonarqube-integration branch May 18, 2024 16:06
    @github-actions github-actions bot added the enhancement Internal new feature or functionality label May 18, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (3922dee)

    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves integration with external services (Sonarqube and HashiCorp Vault), which requires understanding of both the services and the security implications. The changes in the GitHub Actions workflow are significant and need careful review to ensure they are secure and functional.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Misconfiguration: The use of sonarsource/sonarqube-scan-action@master might lead to unstable builds if breaking changes are introduced in the 'master' branch of the action. It's safer to use a specific version of the action.

    Security Concern: Storing sensitive data like SONAR_TOKEN and SONAR_HOST_URL requires careful handling to ensure they are not exposed. The PR should ensure that these secrets are securely fetched and used without logging or exposing them in the workflow logs.

    🔒 Security concerns

    No

    Code feedback:
    relevant file.github/workflows/merge.yml
    suggestion      

    Consider pinning the version of sonarsource/sonarqube-scan-action to a specific release instead of using @master. This can prevent potential issues from breaking changes in the action. [important]

    relevant line- uses: sonarsource/sonarqube-scan-action@master

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Pin the GitHub action to a specific version for stability

    It's recommended to pin the action version to a specific tag or commit SHA to avoid
    potential issues from changes in the 'master' branch. Using a specific version ensures
    stability and predictability in your CI/CD pipelines.

    .github/workflows/merge.yml [37]

    -- uses: sonarsource/sonarqube-scan-action@master
    +- uses: sonarsource/sonarqube-scan-action@v1.0.0  # Replace 'v1.0.0' with the latest stable version or commit SHA
     
    Suggestion importance[1-10]: 9

    Why: Pinning the GitHub action to a specific version is crucial for maintaining stable and predictable CI/CD pipelines, especially to avoid unexpected changes from the 'master' branch.

    9
    Security
    Review and potentially reduce 'id-token' permission to 'read' for security best practices

    The 'id-token' permission is set to 'write', which is unusual as 'id-token' typically does
    not require write permissions. Review if this permission setting is necessary, or if it
    can be safely changed to 'read' to follow the principle of least privilege.

    .github/workflows/merge.yml [21]

    -id-token: write
    +id-token: read
     
    Suggestion importance[1-10]: 8

    Why: Reducing permissions to the minimum required level is a key security best practice. Changing 'id-token' from 'write' to 'read' could enhance security if 'write' permissions are not necessary.

    8
    Maintainability
    Change secrets configuration to list format for better clarity and maintainability

    The secrets are being fetched in a multi-line string format which might lead to parsing
    errors or accidental misconfigurations. It's safer to use a list format for better clarity
    and maintainability.

    .github/workflows/merge.yml [34-36]

    -secrets: |
    -  secret/data/cicd/sonarqube/global  SONAR_TOKEN |SONAR_TOKEN ;
    -  secret/data/cicd/sonarqube/global  SONAR_HOST_URL | SONAR_HOST_URL;
    +secrets: 
    +  - secret/data/cicd/sonarqube/global  SONAR_TOKEN |SONAR_TOKEN
    +  - secret/data/cicd/sonarqube/global  SONAR_HOST_URL | SONAR_HOST_URL
     
    Suggestion importance[1-10]: 7

    Why: Using a list format for secrets can indeed improve clarity and reduce the risk of misconfigurations, which is important for maintainability and security.

    7
    Enhancement
    Add more properties to the SonarQube project file for detailed analysis configuration

    Consider adding more configuration properties to your SonarQube project file to enhance
    the analysis, such as 'sonar.sources' for specifying the source directories, and
    'sonar.exclusions' to exclude files or directories from analysis.

    sonar-project.properties [1]

     sonar.projectKey=tokens-studio_figma-plugin_0900c296-9153-4298-8372-5e3be33e480e
    +sonar.sources=src
    +sonar.exclusions=**/*Test.*
     
    Suggestion importance[1-10]: 6

    Why: Adding more properties like 'sonar.sources' and 'sonar.exclusions' can provide more detailed and tailored analysis, enhancing the overall utility of SonarQube in the project.

    6

    Copy link
    Contributor

    Commit SHA:faab23c494009b6dea6bdfc1c24fc73b13bd562f
    No changes to code coverage between the base branch and the head branch

    six7 added a commit that referenced this pull request Jun 15, 2024
    * Add sonarqube integration (#2771)
    
    * move providers to modal
    
    * update test case to follow the updates
    
    * update branch
    
    * techdocs?
    
    * remove file
    
    ---------
    
    Co-authored-by: SorsOps <80043879+SorsOps@users.noreply.github.com>
    Co-authored-by: hiroshi <robinhoodie0823@gmail.com>
    Co-authored-by: Jan Six <six.jan@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    enhancement Internal new feature or functionality Review effort [1-5]: 3
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant