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

ci: Added sonar workflow #709

Merged
merged 11 commits into from
Nov 5, 2022
Merged

ci: Added sonar workflow #709

merged 11 commits into from
Nov 5, 2022

Conversation

nikcio
Copy link
Contributor

@nikcio nikcio commented Oct 20, 2022

This adds a sonar cloud workflow for scanning the project when there's pushes or PR's to the master branch.

Prerequisites

https://cwiki.apache.org/confluence/display/INFRA/SonarCloud+for+ASF+projects

  • SonarCloud account with permission to use the Lucenenet repository
  • SONARCLOUD_TOKEN in GitHub secrets for actions

SonarCloud account with permission to use the Lucenenet repository

You can add a project to SonarCloud by registering a user at: https://www.sonarsource.com/products/sonarcloud/
image

image

Then you need give SonarCloud access to use the Lucenenet repository. I don't have any screenshots of that but the guide should lead you straight trough it.

Then You just need to analyze the project:
image

SONAR_TOKEN in GitHub secrets for actions

When prompted with setting up an automated workflow then just press on the "GitHub actions" option and create the Github secret SONAR_TOKEN with the value presented. You don't have to create a workflow like presented that's what is in this PR.

Setting up the project Quality gate

Sonar has the concept of Quality gates this is a description of what a good PR includes and therefore what criteria marks a PR as passing. Because the Workflow in the current state doesn't run any tests we need to disable this parameter so it don't mark everything as not parsing.

Go to your organization (This should be apache I think)
image

Then to Quality gates
image

Just copy the "Sonar way" and name it. Mine is called "Sonar way (no tests)"
image

Then just delete the coverage requirement
image

Navigate to the lucenenet project and select the quality gate by hovering "Administration"
image

image

Setting up the project Quality profiles

Then there's the concept of Quality profiles. This is what decides what rules there's active for the project. Same as "Quality gate" navigate to the organization view and press "Quality profiles" this time.
image

Then scroll down to C# and copy the "Sonar way"
image

Then after it's copied disable the following rules as of decided in #648
image

image

image

Then we need to add the "Quality profile". Navigate back to the project and press the "Quality profile" this time.
image

And set the custom profile here:
image

@NightOwl888
Copy link
Contributor

So, not sure this is going to work as-is. Apache controls what we can set up, and their permissions are sometimes restrictive. But it looks like they have an organization-wide scan set up: https://sonarcloud.io/organizations/apache/projects. Generally, for stuff like this there is a document somewhere that has to be located, and failing that, we can contact INFRA to have them help us.

@NightOwl888
Copy link
Contributor

Okay, I couldn't find an official document on how to set this up, but there is another Apache project that documented the process here. I have already opened an INFRA ticket to set it up for us.

Of course, since we don't use Travis CI, we have to find our own way after that. We use GitHub Actions for PRs, so I guess that is what we need to set up when that is complete.

@NightOwl888
Copy link
Contributor

NightOwl888 commented Oct 20, 2022

When this ran, it produced the following error:

Error: .github#L1
highbyte/[email protected] is not allowed to be used in apache/lucenenet. Actions in this workflow must be: within a repository owned by apache, created by GitHub, verified in the GitHub Marketplace, or matching the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+, AdoptOpenJDK/install-jdk@*, JamesIves/github-pages-deploy-action@5dc1d5a192aeb5ab5b7d5a77b7d36aea4a7f5c92, TobKed/label-when-approved-action@*, actions-cool/issues-helper@*, actions-rs/*, al-cheb/configure-pagefile-action@*, amannn/action-semantic-pull-request@*, apache/*, burrunan/gradle-cache-action@*, bytedeco/javacpp-presets/.github/actions/*, chromaui/action@*, codecov/codecov-action@*, conda-incubator/setup-miniconda@*, container-tools/kind-action@*, container-tools/microshift-action@*, dawidd6/action-download-artifact@*, delaguardo/setup-graalvm@*, docker://jekyll/jekyll:*, docker://pandoc/core:2.9, eps1lon/actions-label-merge-conflict@*, gaurav-nelson/github-action-markdown-link-check...

We need approval for this particular plugin. So, if there is one that is already approved that can work for us, it can save us some steps.

@nikcio
Copy link
Contributor Author

nikcio commented Oct 20, 2022

@NightOwl888

It's totally possible to do without the plugin. SonarClouds own example uses a workflow like this one:

name: Build
on:
  push:
    branches:
      - sonarcloud
  pull_request:
    types: [opened, synchronize, reopened]
jobs:
  build:
    name: Build
    runs-on: windows-latest
    steps:
      - name: Set up JDK 11
        uses: actions/setup-java@v1
        with:
          java-version: 1.11
      - uses: actions/checkout@v2
        with:
          fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
      - name: Cache SonarCloud packages
        uses: actions/cache@v1
        with:
          path: ~\sonar\cache
          key: ${{ runner.os }}-sonar
          restore-keys: ${{ runner.os }}-sonar
      - name: Cache SonarCloud scanner
        id: cache-sonar-scanner
        uses: actions/cache@v1
        with:
          path: .\.sonar\scanner
          key: ${{ runner.os }}-sonar-scanner
          restore-keys: ${{ runner.os }}-sonar-scanner
      - name: Install SonarCloud scanner
        if: steps.cache-sonar-scanner.outputs.cache-hit != 'true'
        shell: powershell
        run: |
          New-Item -Path .\.sonar\scanner -ItemType Directory
          dotnet tool update dotnet-sonarscanner --tool-path .\.sonar\scanner
      - name: Build and analyze
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
        shell: powershell
        run: |
          .\.sonar\scanner\dotnet-sonarscanner begin /k:"nikcio_lucenenet" /o:"nikcio" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io"
          <insert_your_build_command>
          .\.sonar\scanner\dotnet-sonarscanner end /d:sonar.login="${{ secrets.SONAR_TOKEN }}"

The reason I use the action instead is because I wanted to use an Ubuntu runner because in my experience they are faster. And when I tried doing it with a template like this one I had some errors and then it was just easier using this action plugin. But using something like this example might also make the runner more stable which might make us able to run coverage tests with something like coverlet.collector.

I also found a repository of the same'ish size from Apache using SonarCloud in the link you found and here it runs un Ubuntu so it's possible: https://github.com/apache/cloudstack/blob/main/.github/workflows/coverage-check.yml

@nikcio
Copy link
Contributor Author

nikcio commented Oct 24, 2022

Okay, I couldn't find an official document on how to set this up, but there is another Apache project that documented the process here. I have already opened an INFRA ticket to set it up for us.

Of course, since we don't use Travis CI, we have to find our own way after that. We use GitHub Actions for PRs, so I guess that is what we need to set up when that is complete.

I've now updated the workflow to use the the SonarCloud template and tested it in my fork to make sure it works. The workflow seems to take about the same amount of time as when running on Ubuntu so for the time being it should be fine to use (a run takes about 20-30 mins).

So it should be ready to add once the INFRA ticket comes through.

I've also tried to run the tests with coverlet.collector installed in the test projects to collect coverage data but this fails in the GitHub action due to: There is not enough space on the disk. : 'D:\a\_temp\_runner_file_commands\add_path_2c3325d9-6997-44fc-9ba2-0c2fe815a6e4'. This run also took 3h 30m 22s so it's properly not ideal to run this on all PR's.

@NightOwl888
Copy link
Contributor

Yikes. That coverage performance is horrible. We cannot run an agent for more than 1 hour on Azure DevOps. And if running on a free Github actions accouunt I believe the limit is 3 hours,

I know lucene's coverage is pretty lousy, so an occasional report would come in handy, even if we had to do it locally on one target framework to make the performance somewhat bearable.

@nikcio
Copy link
Contributor Author

nikcio commented Nov 3, 2022

@NightOwl888 I see you've tested the workflow it seams the SONAR_TOKEN isn't added correctly yet

Sonar login is formated incorrectly 😉

@NightOwl888
Copy link
Contributor

@NightOwl888 I see you've tested the workflow it seams the SONAR_TOKEN isn't added correctly yet

Sonar login is formated incorrectly 😉

@nikcio

Actually, it is supposed to be SONARCLOUD_TOKEN. Infra finally provided a link to the official documentation, which is not SEO optimized: https://cwiki.apache.org/confluence/display/INFRA/SonarCloud+for+ASF+projects.

Side note: I sent you an email a couple of days ago to the address on your GitHub profile. While there is no rush, I just wanted to confirm you saw it and it didn't end up in your SPAM filter. If we need to have it sent to another email address because it bounced, please let me know the alternate address (you can send an email to the address in my profile to provide it).

@NightOwl888
Copy link
Contributor

So, ran into a snag: Apache doesn't allow scans to be run from a PR from a fork. And only Apache committers are allowed to push to apache/lucenenet which is a rare thing, anyway. That basically makes the PR scan useless.

I have looked at several of the projects at: https://sonarcloud.io/organizations/apache/projects, and indeed couldn't find a single one that has a PR count greater than 0.

I took a look into the repositories, and some of them have switched to using CodeQL instead. I suspect this is because there is a github/codeql/action/analyze plugin officially released by GitHub, and therefore it is in Apache's approved list.

Example: https://github.com/apache/commons-compress/blob/910bfc05f7ca693f87166ca376980107d13fa2b5/.github/workflows/codeql-analysis.yml

That being said, I am not sure how much mileage we will get out of this in PRs, anyway.

  1. This is going to make the checks for the PR run even longer.
  2. Diverging from the original Java code is discouraged, so PRs with large amounts of code changes are not likely to be accepted, anyway. A scan for code changes just isn't all that useful until we start working on the upgrade.
  3. We don't want existing problems showing up in scans for minor bug fixes or documentation changes that are contributed.

If we instead set this up as a nightly scan:

  1. We can take as much time as needed to do the scan.
  2. When the results are in, we can have an email sent to the build mailing list with a link to the scan report.
  3. We can set up a code coverage report.

I found a doc that says there is no time limit for running Apache agents. Although, they only have 180 agents across 1200 projects, hanging onto 1 of them for 3-4 hours probably isn't that big of a deal.

https://infra.apache.org/github-actions-secrets.html

NOTE:: We already have a nightly build set up on Azure DevOps, but there is a 1 hour time limit on each agent, so it is less than ideal for doing something like this. It occasionally fails because the original Lucene setup took about 4 hours and we had to cut down the number of iterations to get it under 1 hour. But there are still some slow random choices that it makes that put it over 1 hour - the test framework wasn't designed with a time limit in mind.

@NightOwl888 NightOwl888 marked this pull request as ready for review November 5, 2022 12:17
@NightOwl888 NightOwl888 merged commit 10c24f2 into apache:master Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants