feat(scorecard): Add SonarQube metric providers#2576
Open
christoph-jerolimov wants to merge 8 commits intoredhat-developer:mainfrom
Open
feat(scorecard): Add SonarQube metric providers#2576christoph-jerolimov wants to merge 8 commits intoredhat-developer:mainfrom
christoph-jerolimov wants to merge 8 commits intoredhat-developer:mainfrom
Conversation
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
…ues, and security Add four metric providers to the scorecard-backend-module-sonarqube plugin: - Quality gate status (boolean) - Open issues count (number) - Security rating (number, A=1 to E=5) - Security issues/vulnerabilities count (number) Includes SonarQubeClient, config, factory, example catalog entity, and unit tests. SonarQube baseUrl defaults to https://sonarcloud.io; token is optional for public projects. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
…config schema - Add config.d.ts with typed config schema supporting default + named instances - Refactor SonarQubeClient to resolve instance by name from sonarqube.instances[] - Parse sonarqube.org/project-key annotation for optional instance prefix (instance/project-key) - Use apiKey + authType (Basic/Bearer) from config.d.ts instead of token - Falls back to default instance when no instance prefix in annotation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
SonarQube expects Basic auth as base64(apiKey:) with an appended colon. Bearer auth passes the apiKey directly without encoding. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
christoph-jerolimov
requested review from
a team,
Eswaraiahsapram and
its-mitesh-kumar
as code owners
Review Summary by QodoAdd SonarQube metric providers for scorecard with multi-instance support
WalkthroughsDescription• Add SonarQube backend module with four metric providers - Quality gate status (boolean) - Open issues count (number) - Security rating (number, A=1 to E=5) - Security issues/vulnerabilities count (number) • Support multiple SonarQube instances with named configuration • Implement SonarQubeClient with Basic/Bearer auth and base64 encoding • Include comprehensive unit tests and configuration schema • Add README documentation and example catalog entity Diagramflowchart LR
Config["Config Schema<br/>config.d.ts"]
Client["SonarQubeClient<br/>API Integration"]
BoolProvider["BooleanMetricProvider<br/>Quality Gate"]
NumProvider["NumberMetricProvider<br/>Issues/Rating"]
Factory["MetricProviderFactory<br/>Provider Creation"]
Module["Backend Module<br/>Registration"]
Config -->|"Instance Config"| Client
Client -->|"API Calls"| BoolProvider
Client -->|"API Calls"| NumProvider
Factory -->|"Creates"| BoolProvider
Factory -->|"Creates"| NumProvider
Module -->|"Registers"| Factory
File Changes1. workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/config.d.ts
|
Code Review by Qodo
1. Instance apiKey wrongly required
|
Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/package.json
Outdated
Show resolved
Hide resolved
Comment on lines
+63
to
+68
| /** | ||
| * The api key to access the sonarqube instance. | ||
| * @visibility secret | ||
| */ | ||
| apiKey: string; | ||
|
|
There was a problem hiding this comment.
2. Instance apikey wrongly required 🐞 Bug ✓ Correctness
The config schema requires sonarqube.instances[].apiKey, but the README/tests/implementation support instances without an apiKey for public projects. This mismatch will cause config schema validation to reject documented configurations.
Agent Prompt
### Issue description
`config.d.ts` requires `sonarqube.instances[].apiKey`, but the implementation and documentation allow instances without an apiKey (for public projects). This will cause Backstage config schema validation to fail for valid configs.
### Issue Context
README and tests demonstrate an instance without `apiKey`.
### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/config.d.ts[39-75]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/README.md[65-79]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.test.ts[206-223]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.ts[46-55]
### Proposed fix
- Change `instances[].apiKey: string` to `instances[].apiKey?: string`.
- Update the JSDoc comment to state apiKey is optional for public projects (matching top-level `apiKey?: string`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+37
to
+69
| private resolveInstance(instanceName?: string): SonarQubeInstance { | ||
| const sonarqubeConfig = this.config.getOptionalConfig('sonarqube'); | ||
|
|
||
| if (instanceName && sonarqubeConfig) { | ||
| const instances = | ||
| sonarqubeConfig.getOptionalConfigArray('instances') ?? []; | ||
| const instance = instances.find( | ||
| i => i.getString('name') === instanceName, | ||
| ); | ||
| if (instance) { | ||
| return { | ||
| baseUrl: instance.getString('baseUrl').replace(/\/$/, ''), | ||
| apiKey: instance.getOptionalString('apiKey'), | ||
| authType: | ||
| (instance.getOptionalString('authType') as | ||
| | 'Bearer' | ||
| | 'Basic' | ||
| | undefined) ?? 'Basic', | ||
| }; | ||
| } | ||
| throw new Error( | ||
| `SonarQube instance '${instanceName}' not found in configuration`, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| baseUrl: ( | ||
| sonarqubeConfig?.getOptionalString('baseUrl') ?? DEFAULT_BASE_URL | ||
| ).replace(/\/$/, ''), | ||
| apiKey: sonarqubeConfig?.getOptionalString('apiKey'), | ||
| authType: 'Basic', | ||
| }; | ||
| } |
There was a problem hiding this comment.
3. Instance prefix ignored silently 🐞 Bug ✓ Correctness
If an entity uses an instance-prefixed project key (e.g. internal/my-project) but the sonarqube config block is missing, resolveInstance ignores the instanceName and falls back to the default instance. This can query the wrong SonarQube/SonarCloud instance without any explicit error about the missing instance configuration.
Agent Prompt
### Issue description
`SonarQubeClient.resolveInstance(instanceName)` ignores a provided `instanceName` when the `sonarqube` config block is missing, silently falling back to the default instance. This can cause requests to go to the wrong SonarQube/SonarCloud instance.
### Issue Context
Instance names come from entity annotations (`sonarqube.org/project-key`) via `parseProjectKeyAnnotation`, and metric providers pass `instanceName` through to the client.
### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.ts[37-69]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.ts[84-96]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeConfig.ts[32-43]
### Proposed fix
- If `instanceName` is provided but `sonarqubeConfig` is missing, throw a clear error like:
`SonarQube instance '<name>' requested but no 'sonarqube' configuration found`.
- (Optional) Add a unit test covering `instanceName` + empty root config to prevent regressions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+114
to
+129
| case 'security_rating': { | ||
| const measures = await this.client.getMeasures( | ||
| projectKey, | ||
| SONARQUBE_API_METRIC_KEYS.security_rating, | ||
| instanceName, | ||
| ); | ||
| return measures.security_rating; | ||
| } | ||
| case 'security_issues': { | ||
| const measures = await this.client.getMeasures( | ||
| projectKey, | ||
| SONARQUBE_API_METRIC_KEYS.security_issues, | ||
| instanceName, | ||
| ); | ||
| return measures.vulnerabilities; | ||
| } |
There was a problem hiding this comment.
4. Missing measures return undefined 🐞 Bug ⛯ Reliability
SonarQubeNumberMetricProvider returns measures.security_rating/measures.vulnerabilities without verifying they exist in the response. If SonarQube omits a measure, calculateMetric returns undefined, violating the MetricProvider contract and resulting in null/undefined metric values stored without an error.
Agent Prompt
### Issue description
`SonarQubeNumberMetricProvider.calculateMetric` can return `undefined` when a requested measure is missing from the SonarQube API response (e.g., `security_rating` or `vulnerabilities` not present). This violates the `MetricProvider` contract (must return a number) and leads to silent null/undefined values being stored.
### Issue Context
`SonarQubeClient.getMeasures` only adds metrics that appear in `data.component.measures`, and the provider accesses keys directly.
### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.ts[120-140]
- workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.ts[114-129]
- workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts[25-65]
### Proposed fix
Choose one consistent behavior:
- **Preferred (fail fast):** After fetching measures, check for the requested key(s); if missing, throw an Error like `Missing SonarQube measure 'security_rating' for project '<key>'` so the scheduler stores `error_message`.
- **Alternative (default value):** Return `0` when a measure is absent, if that better matches scorecard semantics.
Also consider guarding against `data.component?.measures` being absent and throwing a clearer error message.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…blic Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
christoph-jerolimov
changed the title
Add metrics for code coverage, code duplications, security review rating, security hotspots, reliability rating/issues, and maintainability rating/issues. Refactors calculateMetric to use a data-driven API key mapping table instead of a switch statement, and deduplicates rating thresholds into a shared constant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Christoph Jerolimov <jerolimov+git@redhat.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey, I just made a Pull Request!
New module with new metrics from sonarqube/sonarcloud; agentic-eng. by @imykhno, myself and Claude. 😀
TODOs✔️ Checklist