-
Notifications
You must be signed in to change notification settings - Fork 129
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
base: dev
Are you sure you want to change the base?
Conversation
96e35af
to
0bebc68
Compare
0bebc68
to
ab7fbab
Compare
@@ -68,6 +68,54 @@ jobs: | |||
- name: Assert | |||
run: | | |||
./test/assertFileContains ./output.properties "sonar.projectBaseDir=.*/baseDir" | |||
scannerVersionTest: |
There was a problem hiding this comment.
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' }} |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
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 thesonarscanner
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
).The new
scannerBinariesUrl
action parameter includesDistribution/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 theDistribution/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