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

SQSCANGHA-51 Make Scanner CLI binaries URL customizable #148

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

antonioaversa
Copy link
Contributor

@antonioaversa antonioaversa commented Nov 18, 2024

Why

CI pipelines in environments that impose strict control to the access to the www fail the wget instruction to download Sonar Scanner CLI from the sonarsource binaries site.

For those scenarios, we need to add additional parameters that users can use to point to a custom repository of binaries.

What

This PR introduces a new optional scannerBinariesUrl action parameter, that allows to set the URI to the CLI. The URI is up to the sonarscanner directory included (https://binaries.sonarsource.com/Distribution/sonar-scanner-cli), and doesn't include the name of the file (e.g. sonar-scanner-cli-6.2.1.4610-linux-x64.zip) since the name of the file depends on operating system (RUNNER_OS) and architecture (RUNNER_ARCH).

    - name: Dart Scan
      uses: sonarsource/sonarqube-scan-action@antonio/binaries-url-config
      env:
        SONAR_HOST_URL: http://localhost:9000
      with:
        scannerBinariesUrl: https://my.custom.binaries.url.com/Distribution/sonar-scanner-cli/

The new scannerBinariesUrl action parameter includes Distribution/sonarscanner so that's easy for a user who need to change it to a custom repo, to point to any directory, without the need to recreate the Distribution/sonarscanner structure.

This means that, the day we will need to download another resource from binaries.sonarsource.com, we will need to add another parameter. Build wrapper, however, won't pose such a problem since it is downloaded from SonarQube.

Builds

Successful build with default value: https://github.com/antonioaversa/dart-tools-test1/actions/runs/11897272492/job/33151305671#step:4:40
Successful build with explicit default value (terminal slash): https://github.com/antonioaversa/dart-tools-test1/actions/runs/11896758678/job/33149767749#step:4:40
Successful build with a different URI, containing the required binary: https://github.com/antonioaversa/dart-tools-test1/actions/runs/11896649263/job/33149029847#step:4:40
Failing build with a different URI, non-containing the required binary: https://github.com/antonioaversa/dart-tools-test1/actions/runs/11896323568/job/33148487407#step:4:43

@antonioaversa antonioaversa force-pushed the antonio/binaries-url-config branch 13 times, most recently from 96e35af to 0bebc68 Compare November 18, 2024 15:21
@@ -68,6 +68,54 @@ jobs:
- name: Assert
run: |
./test/assertFileContains ./output.properties "sonar.projectBaseDir=.*/baseDir"
scannerVersionTest:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was missing. While it is not strictly in scope for the change, but helped as intermediate testing step to come up with a valid test for scannerBinariesUrl.

@@ -30,11 +34,12 @@ runs:
path: ${{ runner.temp }}/sonar-scanner-cli-${{ inputs.scannerVersion }}-${{ runner.os }}-${{ runner.arch }}
key: sonar-scanner-cli-${{ inputs.scannerVersion }}-${{ runner.os }}-${{ runner.arch }}
- name: Install Sonar Scanner CLI
if: steps.sonar-scanner-cli.outputs.cache-hit != 'true'
if: ${{ env.NO_CACHE == 'true' || steps.sonar-scanner-cli.outputs.cache-hit != 'true' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to add this NO_CACHE parameter to ignore previously cached scanners.
Clearing the cache via gh cache delete is possible but requires repo-scope permissions (see here).

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, but out of curiosity, is NO_CACHE env variable something standard?

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