From a4fc84ef49a0d3f32bdbefc4561460527b65320b Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Tue, 28 Jul 2026 10:08:51 +0200 Subject: [PATCH 1/5] Introduce new app config Assisted-By: Cursor Desktop Signed-off-by: Dominika Zemanovicova --- .../metric-level-thresholds-config.md | 44 +++ workspaces/scorecard/AGENTS.md | 162 ++++---- .../scorecard/app-config.local.EXAMPLE.yaml | 46 +-- .../scorecard/app-config.production.yaml | 11 +- workspaces/scorecard/app-config.yaml | 22 +- .../README.md | 19 +- .../README.md | 14 +- .../config.d.ts | 7 +- .../scorecard-backend-module-github/README.md | 41 +- .../config.d.ts | 16 +- .../scorecard-backend-module-jira/README.md | 51 +-- .../__fixtures__/testUtils.ts | 8 +- .../scorecard-backend-module-jira/config.d.ts | 23 +- .../src/constants/jiraOpenIssues.ts | 2 +- .../README.md | 61 +-- .../README.md | 165 +++++---- .../plugins/scorecard-backend/config.d.ts | 56 ++- .../scorecard-backend/docs/thresholds.md | 126 ++++--- .../providers/MetricProvidersRegistry.test.ts | 143 ++++--- .../src/providers/MetricProvidersRegistry.ts | 39 +- .../tasks/PullMetricsByProviderTask.test.ts | 112 +++--- .../tasks/PullMetricsByProviderTask.ts | 21 +- .../src/service/CatalogMetricService.test.ts | 13 +- .../src/service/CatalogMetricService.ts | 2 - .../src/service/router.test.ts | 6 +- .../scorecard-backend/src/service/router.ts | 14 +- .../src/threshold/ThresholdResolver.test.ts | 350 ++++++++++++------ .../src/threshold/ThresholdResolver.ts | 48 +-- ...=> mergeEntityAndMetricThresholds.test.ts} | 140 ++----- ...s.ts => mergeEntityAndMetricThresholds.ts} | 24 +- .../utils/metricProviderConfigKeys.test.ts | 259 +++++++++++++ .../src/utils/metricProviderConfigKeys.ts | 136 +++++++ .../validateMetricProviderIds.test.ts | 67 ++++ .../validation/validateMetricProviderIds.ts | 48 +++ .../scorecard-common/src/types/threshold.ts | 2 +- 35 files changed, 1493 insertions(+), 805 deletions(-) create mode 100644 workspaces/scorecard/.changeset/metric-level-thresholds-config.md rename workspaces/scorecard/plugins/scorecard-backend/src/utils/{mergeEntityAndProviderThresholds.test.ts => mergeEntityAndMetricThresholds.test.ts} (73%) rename workspaces/scorecard/plugins/scorecard-backend/src/utils/{mergeEntityAndProviderThresholds.ts => mergeEntityAndMetricThresholds.ts} (83%) create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts diff --git a/workspaces/scorecard/.changeset/metric-level-thresholds-config.md b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md new file mode 100644 index 00000000000..f2c96c52f0f --- /dev/null +++ b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md @@ -0,0 +1,44 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-github': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-jira': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-filecheck': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-sonarqube': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-dependabot': major +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-openssf': major +--- + +**BREAKING**: Thresholds moved from provider level to metric level. Configuration is restructured to enable thresholds to be defined directly for each metric with fallback options at the datasource and provider levels. The new top-level key `metricProviders` now houses all metric-specific configurations. Schedule options are added to datasource level. Threshold annotation overrides now require full metric ID instead of provider ID. + +Thresholds from configuration are determined by the most specific setting. Priority order is: Metric > MetricProvider > Datasource: + +1. `plugins..metricProviders..metrics..thresholds` +2. `plugins..metricProviders..thresholds` +3. `plugins..thresholds` + +Config keys are local names (no datasource prefix). Single-metric plugins can set thresholds directly under the datasource. Schedules use `plugins..schedule` with optional override at `metricProviders..schedule`. + +Entity annotations must use the full metric ID: +`scorecard.io/.thresholds.rules.` (for example +`scorecard.io/filecheck.readme.thresholds.rules.success`). + +If you customized thresholds or schedule under a metric provider, nest that provider under `metricProviders`: + +```diff + scorecard: + plugins: + github: +- openPRs: ++ metricProviders: ++ openPRs: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '10-50' + - key: error + expression: '>50' +``` + +Apply the same nesting for other datasources (`jira`, `sonarqube`, `dependabot`, `openssf`, etc.): move `plugins..` to `plugins..metricProviders.`. diff --git a/workspaces/scorecard/AGENTS.md b/workspaces/scorecard/AGENTS.md index 374b2589db8..f4fd9a4d282 100644 --- a/workspaces/scorecard/AGENTS.md +++ b/workspaces/scorecard/AGENTS.md @@ -25,41 +25,41 @@ snake_case in the API layer only -- they are not metric IDs. ### GitHub (1 metric) -| Metric ID | Type | Source | -|---|---|---| +| Metric ID | Type | Source | +| ---------------- | ------ | -------------------------- | | `github.openPRs` | number | `GithubOpenPRsProvider.ts` | ### Jira (1 metric) -| Metric ID | Type | Source | -|---|---|---| +| Metric ID | Type | Source | +| ----------------- | ------ | --------------------------- | | `jira.openIssues` | number | `JiraOpenIssuesProvider.ts` | ### Dependabot (4 metrics) -| Metric ID | Type | Source | -|---|---|---| +| Metric ID | Type | Source | +| --------------------------- | ------ | --------------------- | | `dependabot.alertsCritical` | number | `DependabotConfig.ts` | -| `dependabot.alertsHigh` | number | `DependabotConfig.ts` | -| `dependabot.alertsMedium` | number | `DependabotConfig.ts` | -| `dependabot.alertsLow` | number | `DependabotConfig.ts` | +| `dependabot.alertsHigh` | number | `DependabotConfig.ts` | +| `dependabot.alertsMedium` | number | `DependabotConfig.ts` | +| `dependabot.alertsLow` | number | `DependabotConfig.ts` | ### SonarQube (12 metrics) -| Metric ID | Type | SonarQube API Key | Source | -|---|---|---|---| -| `sonarqube.qualityGate` | boolean | _(quality gate API)_ | `SonarQubeConfig.ts` | -| `sonarqube.openIssues` | number | _(open issues API)_ | `SonarQubeConfig.ts` | -| `sonarqube.securityRating` | number | `security_rating` | `SonarQubeConfig.ts` | -| `sonarqube.securityIssues` | number | `vulnerabilities` | `SonarQubeConfig.ts` | -| `sonarqube.securityReviewRating` | number | `security_review_rating` | `SonarQubeConfig.ts` | -| `sonarqube.securityHotspots` | number | `security_hotspots` | `SonarQubeConfig.ts` | -| `sonarqube.reliabilityRating` | number | `reliability_rating` | `SonarQubeConfig.ts` | -| `sonarqube.reliabilityIssues` | number | `bugs` | `SonarQubeConfig.ts` | -| `sonarqube.maintainabilityRating` | number | `sqale_rating` | `SonarQubeConfig.ts` | -| `sonarqube.maintainabilityIssues` | number | `code_smells` | `SonarQubeConfig.ts` | -| `sonarqube.codeCoverage` | number | `coverage` | `SonarQubeConfig.ts` | -| `sonarqube.codeDuplications` | number | `duplicated_lines_density` | `SonarQubeConfig.ts` | +| Metric ID | Type | SonarQube API Key | Source | +| --------------------------------- | ------- | -------------------------- | -------------------- | +| `sonarqube.qualityGate` | boolean | _(quality gate API)_ | `SonarQubeConfig.ts` | +| `sonarqube.openIssues` | number | _(open issues API)_ | `SonarQubeConfig.ts` | +| `sonarqube.securityRating` | number | `security_rating` | `SonarQubeConfig.ts` | +| `sonarqube.securityIssues` | number | `vulnerabilities` | `SonarQubeConfig.ts` | +| `sonarqube.securityReviewRating` | number | `security_review_rating` | `SonarQubeConfig.ts` | +| `sonarqube.securityHotspots` | number | `security_hotspots` | `SonarQubeConfig.ts` | +| `sonarqube.reliabilityRating` | number | `reliability_rating` | `SonarQubeConfig.ts` | +| `sonarqube.reliabilityIssues` | number | `bugs` | `SonarQubeConfig.ts` | +| `sonarqube.maintainabilityRating` | number | `sqale_rating` | `SonarQubeConfig.ts` | +| `sonarqube.maintainabilityIssues` | number | `code_smells` | `SonarQubeConfig.ts` | +| `sonarqube.codeCoverage` | number | `coverage` | `SonarQubeConfig.ts` | +| `sonarqube.codeDuplications` | number | `duplicated_lines_density` | `SonarQubeConfig.ts` | ### OpenSSF (18 metrics) @@ -74,26 +74,26 @@ conversion lowercases first, these become `cii`, `ci`, and `sast` -- not `cII`, `cI`, or `SAST`. Any change to the ID generation regex must be verified against this full table. -| Check Name | Metric ID | Edge Case? | -|---|---|---| -| `Binary-Artifacts` | `openssf.binaryArtifacts` | | -| `Branch-Protection` | `openssf.branchProtection` | | -| `CII-Best-Practices` | `openssf.ciiBestPractices` | Yes -- `CII` becomes `cii` | -| `CI-Tests` | `openssf.ciTests` | Yes -- `CI` becomes `ci` | -| `Code-Review` | `openssf.codeReview` | | -| `Contributors` | `openssf.contributors` | | -| `Dangerous-Workflow` | `openssf.dangerousWorkflow` | | -| `Dependency-Update-Tool` | `openssf.dependencyUpdateTool` | | -| `Fuzzing` | `openssf.fuzzing` | | -| `License` | `openssf.license` | | -| `Maintained` | `openssf.maintained` | | -| `Packaging` | `openssf.packaging` | | -| `Pinned-Dependencies` | `openssf.pinnedDependencies` | | -| `SAST` | `openssf.sast` | Yes -- all-caps becomes all-lowercase | -| `Security-Policy` | `openssf.securityPolicy` | | -| `Signed-Releases` | `openssf.signedReleases` | | -| `Token-Permissions` | `openssf.tokenPermissions` | | -| `Vulnerabilities` | `openssf.vulnerabilities` | | +| Check Name | Metric ID | Edge Case? | +| ------------------------ | ------------------------------ | ------------------------------------- | +| `Binary-Artifacts` | `openssf.binaryArtifacts` | | +| `Branch-Protection` | `openssf.branchProtection` | | +| `CII-Best-Practices` | `openssf.ciiBestPractices` | Yes -- `CII` becomes `cii` | +| `CI-Tests` | `openssf.ciTests` | Yes -- `CI` becomes `ci` | +| `Code-Review` | `openssf.codeReview` | | +| `Contributors` | `openssf.contributors` | | +| `Dangerous-Workflow` | `openssf.dangerousWorkflow` | | +| `Dependency-Update-Tool` | `openssf.dependencyUpdateTool` | | +| `Fuzzing` | `openssf.fuzzing` | | +| `License` | `openssf.license` | | +| `Maintained` | `openssf.maintained` | | +| `Packaging` | `openssf.packaging` | | +| `Pinned-Dependencies` | `openssf.pinnedDependencies` | | +| `SAST` | `openssf.sast` | Yes -- all-caps becomes all-lowercase | +| `Security-Policy` | `openssf.securityPolicy` | | +| `Signed-Releases` | `openssf.signedReleases` | | +| `Token-Permissions` | `openssf.tokenPermissions` | | +| `Vulnerabilities` | `openssf.vulnerabilities` | | ### Filecheck (dynamic) @@ -147,64 +147,56 @@ or all `"boolean"`). ### Threshold resolution priority `ThresholdResolver` (in `scorecard-backend`) resolves thresholds for -each metric using a three-tier override chain. Higher tiers override -lower tiers: +each metric, stored per **metric ID** in `ThresholdResolver.configuredThresholds`. +Higher tiers override lower tiers: 1. **Entity annotations** (highest priority) — per-entity overrides set via catalog annotations with the prefix - `scorecard.io/.thresholds.rules.`. Merged on top of - the provider-level thresholds by `mergeEntityAndProviderThresholds()`. -2. **`app-config.yaml` overrides** (middle priority) — cluster-wide - overrides configured at `scorecard.plugins..thresholds` - in `app-config.yaml`. Read by `getThresholdsFromConfig()` and stored - per provider ID in `ThresholdResolver.configuredThresholds`. + `scorecard.io/.thresholds.rules.` (full metric ID). + Merged on top of resolved thresholds by + `mergeEntityAndMetricThresholds()`. +2. **`app-config.yaml` overrides** (middle priority) — administrator + overrides by configuration, most specific level wins + (datasource < provider < metric), complete replace of metric defaults. + - metric: + `plugins..metricProviders..metrics..thresholds` + - provider: `plugins..metricProviders..thresholds` + - datasource: `plugins..thresholds` + Config keys are local names (no datasource prefix). 3. **Metric defaults** (lowest priority) — the `thresholds` field on each `Metric` object returned by `getMetrics()`. Used when no config or entity override exists. -**Current limitation:** config overrides (tier 2) are keyed by -`providerId`, not by individual `metricId`. This means all metrics -within a multi-metric provider (e.g., SonarQube with 12 metrics, or -Dependabot with 4 metrics) share a single `app-config.yaml` threshold -override. Cluster administrators cannot customize thresholds for -individual metrics within the same provider via config alone — they -must use entity-level annotations (tier 1) for per-metric granularity. - -In `ThresholdResolver.setConfiguredThresholds()`, the config is -validated against `metrics[0].type`, which is safe only because the -generic constraint guarantees type homogeneity within a provider. -However, this still applies one threshold config to all metrics in -that provider. +Provider IDs are either `datasource` (batch providers such as filecheck) +or `datasource.`. Metric IDs must always be +`datasource.`. ### Review guidance for threshold changes When reviewing changes to `ThresholdResolver`, the `Metric` type, -`mergeEntityAndProviderThresholds`, or `getThresholdsFromConfig`: +`mergeEntityAndMetricThresholds`, or `getThresholdsFromConfig`: - **Flag changes that affect threshold granularity as high severity.** - Any change to how thresholds are keyed (per-provider vs per-metric) - or how overrides are resolved directly impacts how cluster - administrators configure the scorecard in `app-config.yaml`. This is - not a theoretical concern — it determines whether operators can tune - thresholds for individual metrics. + Any change to how thresholds are keyed or how overrides are resolved + directly impacts how cluster administrators configure the scorecard + in `app-config.yaml`. - **Verify all three resolution tiers.** A change that fixes one tier (e.g., improving config overrides) can break another (e.g., entity annotation merging) if the data structures change. -- **Check config path strings.** The config path - `scorecard.plugins.${providerId}.thresholds` is constructed as a - string. Changes to provider IDs or config schema must update both the - path construction and `config.d.ts`. +- **Check config path strings.** Threshold paths are built via + `getMetricThresholdsConfigPath()`. Changes to provider/metric IDs or + config schema must update both path helpers and `config.d.ts`. ### Key files -| File | Package | Role | -|---|---|---| -| `ThresholdResolver.ts` | `scorecard-backend` | Resolves thresholds using the three-tier chain | -| `Metric.ts` | `scorecard-common` | Defines `Metric`, `MetricType`, and `MetricValue` types | -| `MetricProvider.ts` | `scorecard-node` | Defines the `MetricProvider` interface | -| `mergeEntityAndProviderThresholds.ts` | `scorecard-backend` | Merges entity annotation overrides with provider thresholds | -| `getThresholdsFromConfig.ts` | `scorecard-node` | Reads and validates threshold config from `app-config.yaml` | -| `DependabotConfig.ts` | `scorecard-backend-module-dependabot` | Dependabot provider metric and threshold definitions | -| `SonarQubeConfig.ts` | `scorecard-backend-module-sonarqube` | SonarQube provider metric and threshold definitions | -| `OpenSSFConfig.ts` | `scorecard-backend-module-openssf` | OpenSSF provider metric and threshold definitions | -| `FilecheckConfig.ts` | `scorecard-backend-module-filecheck` | Filecheck provider metric and threshold definitions | +| File | Package | Role | +| ----------------------------------- | ------------------------------------- | ----------------------------------------------------------- | +| `ThresholdResolver.ts` | `scorecard-backend` | Resolves thresholds using the three-tier chain | +| `Metric.ts` | `scorecard-common` | Defines `Metric`, `MetricType`, and `MetricValue` types | +| `MetricProvider.ts` | `scorecard-node` | Defines the `MetricProvider` interface | +| `mergeEntityAndMetricThresholds.ts` | `scorecard-backend` | Merges entity annotation overrides with metric thresholds | +| `getThresholdsFromConfig.ts` | `scorecard-node` | Reads and validates threshold config from `app-config.yaml` | +| `DependabotConfig.ts` | `scorecard-backend-module-dependabot` | Dependabot provider metric and threshold definitions | +| `SonarQubeConfig.ts` | `scorecard-backend-module-sonarqube` | SonarQube provider metric and threshold definitions | +| `OpenSSFConfig.ts` | `scorecard-backend-module-openssf` | OpenSSF provider metric and threshold definitions | +| `FilecheckConfig.ts` | `scorecard-backend-module-filecheck` | Filecheck provider metric and threshold definitions | diff --git a/workspaces/scorecard/app-config.local.EXAMPLE.yaml b/workspaces/scorecard/app-config.local.EXAMPLE.yaml index 53604a2542e..185e6c37c26 100644 --- a/workspaces/scorecard/app-config.local.EXAMPLE.yaml +++ b/workspaces/scorecard/app-config.local.EXAMPLE.yaml @@ -79,26 +79,28 @@ scorecard: color: '#be1ec7' # purple plugins: jira: - openIssues: - schedule: - frequency: { days: 1 } - timeout: { minutes: 2 } - initialDelay: { minutes: 1 } - # Optional — uncomment to narrow which Jira issues are counted (otherwise all open issues match the provider rules). - # options: - # mandatoryFilter: type = Story - # customFilter: priority = High + metricProviders: + openIssues: + schedule: + frequency: { days: 1 } + timeout: { minutes: 2 } + initialDelay: { minutes: 1 } + # Optional — uncomment to narrow which Jira issues are counted (otherwise all open issues match the provider rules). + # options: + # mandatoryFilter: type = Story + # customFilter: priority = High github: - openPRs: - # Example threshold overrides; remove to use provider defaults. - thresholds: - rules: - - key: success - expression: '<=20' - color: 'success.main' - - key: warning - expression: '20-50' - color: '#FFFF00' - - key: error - expression: '>50' - color: 'rgb(255, 0, 0)' + # Provider-level thresholds, could be also set under `metricProviders.openPRs.metrics.openPRs.thresholds`. + metricProviders: + openPRs: + thresholds: + rules: + - key: success + expression: '<=20' + color: 'success.main' + - key: warning + expression: '20-50' + color: '#FFFF00' + - key: error + expression: '>50' + color: 'rgb(255, 0, 0)' diff --git a/workspaces/scorecard/app-config.production.yaml b/workspaces/scorecard/app-config.production.yaml index 065887f8d55..fbe41d10df3 100644 --- a/workspaces/scorecard/app-config.production.yaml +++ b/workspaces/scorecard/app-config.production.yaml @@ -63,8 +63,9 @@ catalog: # metricId: github.openPRs # plugins: # github: -# openPRs: -# schedule: -# frequency: { hours: 1 } -# timeout: { minutes: 15 } -# initialDelay: { minutes: 1 } +# metricProviders: +# openPRs: +# schedule: +# frequency: { hours: 1 } +# timeout: { minutes: 15 } +# initialDelay: { minutes: 1 } diff --git a/workspaces/scorecard/app-config.yaml b/workspaces/scorecard/app-config.yaml index ac9ba4f569f..49ea48f2896 100644 --- a/workspaces/scorecard/app-config.yaml +++ b/workspaces/scorecard/app-config.yaml @@ -276,17 +276,19 @@ scorecard: metricId: filecheck.license plugins: jira: - openIssues: - schedule: - frequency: { minutes: 5 } - timeout: { minutes: 10 } - initialDelay: { seconds: 10 } + metricProviders: + openIssues: + schedule: + frequency: { minutes: 5 } + timeout: { minutes: 10 } + initialDelay: { seconds: 10 } github: - openPRs: - schedule: - frequency: { minutes: 5 } - timeout: { minutes: 10 } - initialDelay: { seconds: 10 } + metricProviders: + openPRs: + schedule: + frequency: { minutes: 5 } + timeout: { minutes: 10 } + initialDelay: { seconds: 10 } filecheck: files: license: 'LICENSE' diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/README.md index 870eede273b..eb51911664a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/README.md @@ -17,15 +17,16 @@ All four severity metrics share the same default thresholds. Default thresholds scorecard: plugins: dependabot: - alertsCritical: - thresholds: - rules: - - key: success - expression: '<1' - - key: warning - expression: '1-7' - - key: error - expression: '>7' + metricProviders: + alertsCritical: + thresholds: + rules: + - key: success + expression: '<1' + - key: warning + expression: '1-7' + - key: error + expression: '>7' ``` Replace `alertsCritical` with `alertsHigh`, `alertsMedium`, or `alertsLow` for the other severity metrics. See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom configuration. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md index 5a96e220409..8bdfbacb082 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md @@ -59,12 +59,12 @@ scorecard: This produces the following metrics: -| Metric ID | Checked file path | -| ------------------------ | ----------------- | -| `filecheck.readme` | `README.md` | -| `filecheck.license` | `LICENSE` | -| `filecheck.codeowners` | `CODEOWNERS` | -| `filecheck.contributing` | `CONTRIBUTING.md` | +| Metric ID | Provider ID | Checked file path | +| ------------------------ | ----------- | ----------------- | +| `filecheck.readme` | `filecheck` | `README.md` | +| `filecheck.license` | `filecheck` | `LICENSE` | +| `filecheck.codeowners` | `filecheck` | `CODEOWNERS` | +| `filecheck.contributing` | `filecheck` | `CONTRIBUTING.md` | If no files are configured, no metrics are registered and the module has no effect. @@ -136,6 +136,6 @@ scorecard: seconds: 5 ``` -The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). +The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). For more details on how to configure schedule, see [Metric Collection Scheduling](../scorecard-backend/docs/providers.md#metric-collection-scheduling). Note: all configured file checks share a single schedule — the module fetches each entity's repository tree once per run and checks all configured paths in that single request. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts index b90465df547..21818bafa19 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts @@ -13,7 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { SchedulerServiceTaskScheduleDefinitionConfig } from '@backstage/backend-plugin-api'; +import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; export interface Config { /** Configuration for scorecard plugin */ @@ -24,9 +26,12 @@ export interface Config { filecheck?: { /** File existence checks configuration. Keys are metric identifier suffixes, values are relative file paths. */ files?: { - [metricId: string]: string; + [metricName: string]: string; }; + /** How often filecheck metrics will be calculated. */ schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How filecheck metric values are categorized. */ + thresholds?: ThresholdConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-github/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-github/README.md index 49670da9db7..99ebafc4552 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-github/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-github/README.md @@ -65,6 +65,7 @@ spec: This metric counts all pull requests that are currently in an "open" state for the repository specified in the entity's `github.com/project-slug` annotation. - **Metric ID**: `github.openPRs` +- **Metric Provider ID**: `github.openPRs` - **Type**: Number - **Datasource**: `github` @@ -77,18 +78,19 @@ Default thresholds for `github.openPRs`: scorecard: plugins: github: - openPRs: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '10-50' - - key: error - expression: '>50' + metricProviders: + openPRs: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '10-50' + - key: error + expression: '>50' ``` -See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom configuration. +See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom thresholds configuration. ## Configuration @@ -100,14 +102,15 @@ The Scorecard plugin uses Backstage's built-in scheduler service to automaticall scorecard: plugins: github: - openPRs: - schedule: - frequency: - cron: '0 6 * * *' - timeout: - minutes: 5 - initialDelay: - seconds: 5 + metricProviders: + openPRs: + schedule: + frequency: + cron: '0 6 * * *' + timeout: + minutes: 5 + initialDelay: + seconds: 5 ``` -The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). +The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). See [schedule configuration](../scorecard-backend/docs/providers.md#metric-collection-schedulin) for custom schedule configuration. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts index 3f06df48538..8fcfdf0b172 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts @@ -13,7 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { SchedulerServiceTaskScheduleDefinitionConfig } from '@backstage/backend-plugin-api'; +import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; export interface Config { /** Configuration for scorecard plugin */ @@ -22,15 +24,13 @@ export interface Config { plugins?: { /** Github datasource configuration */ github?: { - openPRs?: { - thresholds?: { - rules?: Array<{ - key: 'error' | 'warning' | 'success'; - /** Threshold expression - supports: >=, <=, >, <, ==, !=, - (range) */ - expression: string; - }>; + metricProviders?: { + openPRs?: { + /** How often github.openPRs metrics will be calculated */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How github.openPRs metric values are categorized */ + thresholds?: ThresholdConfig; }; - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-jira/README.md index 2a3c7185a7b..caaaf30cefe 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/README.md @@ -86,12 +86,13 @@ Options define configuration that affect fetch jira issues global configuration, scorecard: plugins: jira: - openIssues: - options: - # Optional: use mandatoryFilter filter if need to replaces default which is "type = Bug AND resolution = Unresolved" - mandatoryFilter: Type = Task AND Resolution = Resolved - # Optional: use to specify global customFilter, however the annotation `jira/custom-filter` will replaces them - customFilter: priority in ("Critical", "Blocker") + metricProviders: + openIssues: + options: + # Optional: use mandatoryFilter filter if need to replaces default which is "type = Bug AND resolution = Unresolved" + mandatoryFilter: Type = Task AND Resolution = Resolved + # Optional: use to specify global customFilter, however the annotation `jira/custom-filter` will replaces them + customFilter: priority in ("Critical", "Blocker") ``` ## Schedule Configuration @@ -102,17 +103,18 @@ The Scorecard plugin uses Backstage's built-in scheduler service to automaticall scorecard: plugins: jira: - openIssues: - schedule: - frequency: - cron: '0 6 * * *' - timeout: - minutes: 5 - initialDelay: - seconds: 5 + metricProviders: + openIssues: + schedule: + frequency: + cron: '0 6 * * *' + timeout: + minutes: 5 + initialDelay: + seconds: 5 ``` -The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). +The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). For more details on how to configure schedule, see [Metric Collection Scheduling](../scorecard-backend/docs/providers.md#metric-collection-scheduling). ## Installation @@ -193,15 +195,16 @@ Default thresholds for `jira.openIssues`: scorecard: plugins: jira: - openIssues: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '10-50' - - key: error - expression: '>50' + metricProviders: + openIssues: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '10-50' + - key: error + expression: '>50' ``` See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom configuration. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts index 4d89f965449..7da1e46cf8f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts @@ -78,9 +78,11 @@ export function newMockRootConfig({ scorecard: { plugins: { jira: { - openIssues: { - options, - thresholds, + metricProviders: { + openIssues: { + options, + thresholds, + }, }, }, }, diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts index 8f6f4c3e4d1..737896fb593 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts @@ -15,6 +15,7 @@ */ import { SchedulerServiceTaskScheduleDefinitionConfig } from '@backstage/backend-plugin-api'; +import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; export interface Config { /** Configuration for jira plugin */ @@ -38,19 +39,17 @@ export interface Config { plugins?: { /** JIRA datasource configuration */ jira?: { - openIssues?: { - options?: { - mandatoryFilter?: string; - customFilter?: string; + metricProviders?: { + openIssues?: { + options?: { + mandatoryFilter?: string; + customFilter?: string; + }; + /** How often jira.openIssues metrics will be calculated */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How jira.openIssues metric values are categorized */ + thresholds?: ThresholdConfig; }; - thresholds?: { - rules?: Array<{ - key: 'error' | 'warning' | 'success'; - /** Threshold expression - supports: >=, <=, >, <, ==, !=, - (range) */ - expression: string; - }>; - }; - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts index d75e231ad58..f2aa77a1cbf 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts @@ -15,7 +15,7 @@ */ export const OPEN_ISSUES_CONFIG_PATH = - 'scorecard.plugins.jira.openIssues' as const; + 'scorecard.plugins.jira.metricProviders.openIssues' as const; /** * Jira integration configuration path diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md index cfbcc4372ad..d79aba2375a 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md @@ -46,26 +46,26 @@ metadata: 18 metrics from [OpenSSF checks](https://github.com/ossf/scorecard/blob/main/docs/checks.md): -| Metric | Description | -| ------------------------------ | ------------------------------------------------------------------------------------------- | -| `openssf.binaryArtifacts` | No executable (binary) artifacts in the source repository. | -| `openssf.branchProtection` | Default and release branches protected (e.g. require review, status checks, no force push). | -| `openssf.ciiBestPractices` | Project has an OpenSSF Best Practices badge (passing, silver, or gold). | -| `openssf.ciTests` | Tests run before pull requests are merged. | -| `openssf.codeReview` | Human code review required before PRs are merged. | -| `openssf.contributors` | Recent contributors from multiple organizations. | -| `openssf.dangerousWorkflow` | GitHub Actions workflows avoid dangerous patterns (untrusted checkout, script injection). | -| `openssf.dependencyUpdateTool` | Dependency update tool in use (e.g. Dependabot, Renovate). | -| `openssf.fuzzing` | Fuzzing in use (e.g. OSS-Fuzz, ClusterFuzzLite, or language fuzz tests). | -| `openssf.license` | Project has a published license. | -| `openssf.maintained` | Project is actively maintained (not archived, recent activity). | -| `openssf.packaging` | Project is published as a package. | -| `openssf.pinnedDependencies` | Dependencies pinned (hash or fixed version) in build/release. | -| `openssf.sast` | Static application security testing (SAST) in use. | -| `openssf.securityPolicy` | Security policy present (e.g. SECURITY.md). | -| `openssf.signedReleases` | Releases are cryptographically signed. | -| `openssf.tokenPermissions` | GitHub Actions use minimal token permissions. | -| `openssf.vulnerabilities` | Known vulnerabilities in dependencies (lower score = more issues). | +| Metric ID | Provider ID | Description | +| ------------------------------ | ------------------------------ | ------------------------------------------------------------------------------------------- | +| `openssf.binaryArtifacts` | `openssf.binaryArtifacts` | No executable (binary) artifacts in the source repository. | +| `openssf.branchProtection` | `openssf.branchProtection` | Default and release branches protected (e.g. require review, status checks, no force push). | +| `openssf.ciiBestPractices` | `openssf.ciiBestPractices` | Project has an OpenSSF Best Practices badge (passing, silver, or gold). | +| `openssf.ciTests` | `openssf.ciTests` | Tests run before pull requests are merged. | +| `openssf.codeReview` | `openssf.codeReview` | Human code review required before PRs are merged. | +| `openssf.contributors` | `openssf.contributors` | Recent contributors from multiple organizations. | +| `openssf.dangerousWorkflow` | `openssf.dangerousWorkflow` | GitHub Actions workflows avoid dangerous patterns (untrusted checkout, script injection). | +| `openssf.dependencyUpdateTool` | `openssf.dependencyUpdateTool` | Dependency update tool in use (e.g. Dependabot, Renovate). | +| `openssf.fuzzing` | `openssf.fuzzing` | Fuzzing in use (e.g. OSS-Fuzz, ClusterFuzzLite, or language fuzz tests). | +| `openssf.license` | `openssf.license` | Project has a published license. | +| `openssf.maintained` | `openssf.maintained` | Project is actively maintained (not archived, recent activity). | +| `openssf.packaging` | `openssf.packaging` | Project is published as a package. | +| `openssf.pinnedDependencies` | `openssf.pinnedDependencies` | Dependencies pinned (hash or fixed version) in build/release. | +| `openssf.sast` | `openssf.sast` | Static application security testing (SAST) in use. | +| `openssf.securityPolicy` | `openssf.securityPolicy` | Security policy present (e.g. SECURITY.md). | +| `openssf.signedReleases` | `openssf.signedReleases` | Releases are cryptographically signed. | +| `openssf.tokenPermissions` | `openssf.tokenPermissions` | GitHub Actions use minimal token permissions. | +| `openssf.vulnerabilities` | `openssf.vulnerabilities` | Known vulnerabilities in dependencies (lower score = more issues). | ## Default thresholds @@ -76,18 +76,19 @@ All OpenSSF metrics share the same default thresholds. Default thresholds for `o scorecard: plugins: openssf: - maintained: - thresholds: - rules: - - key: success - expression: '>7' - - key: warning - expression: '2-7' - - key: error - expression: '<2' + metricProviders: + maintained: + thresholds: + rules: + - key: success + expression: '>7' + - key: warning + expression: '2-7' + - key: error + expression: '<2' ``` -Higher scores are better (OpenSSF check scores are 0–10). Replace `maintained` with any OpenSSF metric name (e.g. `branchProtection`, `license`). See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom configuration. +Higher scores are better (OpenSSF check scores are 0–10). Replace `maintained` with any OpenSSF metric provider name (e.g. `branchProtection`, `license`). See [threshold configuration](../scorecard-backend/docs/thresholds.md) for custom configuration. ## Troubleshooting diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/README.md index ddf6ae50566..5696346dd28 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/README.md @@ -4,20 +4,20 @@ Adds SonarQube / SonarCloud metrics to the scorecard plugin. ## Metrics -| Metric ID | Type | Description | -| --------------------------------- | ------- | ------------------------------------------------ | -| `sonarqube.qualityGate` | boolean | Whether the project passes its quality gate | -| `sonarqube.openIssues` | number | Count of open issues (OPEN, CONFIRMED, REOPENED) | -| `sonarqube.securityRating` | number | Security rating | -| `sonarqube.securityIssues` | number | Count of open security vulnerabilities | -| `sonarqube.securityReviewRating` | number | Security review rating | -| `sonarqube.securityHotspots` | number | Count of security hotspots to review | -| `sonarqube.reliabilityRating` | number | Reliability rating | -| `sonarqube.reliabilityIssues` | number | Count of open bugs | -| `sonarqube.maintainabilityRating` | number | Maintainability rating | -| `sonarqube.maintainabilityIssues` | number | Count of open code smells | -| `sonarqube.codeCoverage` | number | Overall code coverage percentage | -| `sonarqube.codeDuplications` | number | Percentage of duplicated lines | +| Metric ID | Provider ID | Type | Description | +| --------------------------------- | --------------------------------- | ------- | ------------------------------------------------ | +| `sonarqube.qualityGate` | `sonarqube.qualityGate` | boolean | Whether the project passes its quality gate | +| `sonarqube.openIssues` | `sonarqube.openIssues` | number | Count of open issues (OPEN, CONFIRMED, REOPENED) | +| `sonarqube.securityRating` | `sonarqube.securityRating` | number | Security rating | +| `sonarqube.securityIssues` | `sonarqube.securityIssues` | number | Count of open security vulnerabilities | +| `sonarqube.securityReviewRating` | `sonarqube.securityReviewRating` | number | Security review rating | +| `sonarqube.securityHotspots` | `sonarqube.securityHotspots` | number | Count of security hotspots to review | +| `sonarqube.reliabilityRating` | `sonarqube.reliabilityRating` | number | Reliability rating | +| `sonarqube.reliabilityIssues` | `sonarqube.reliabilityIssues` | number | Count of open bugs | +| `sonarqube.maintainabilityRating` | `sonarqube.maintainabilityRating` | number | Maintainability rating | +| `sonarqube.maintainabilityIssues` | `sonarqube.maintainabilityIssues` | number | Count of open code smells | +| `sonarqube.codeCoverage` | `sonarqube.codeCoverage` | number | Overall code coverage percentage | +| `sonarqube.codeDuplications` | `sonarqube.codeDuplications` | number | Percentage of duplicated lines | ## Installation @@ -70,13 +70,14 @@ Default thresholds for `sonarqube.qualityGate`: scorecard: plugins: sonarqube: - qualityGate: - thresholds: - rules: - - key: success - expression: '==true' - - key: error - expression: '==false' + metricProviders: + qualityGate: + thresholds: + rules: + - key: success + expression: '==true' + - key: error + expression: '==false' ``` ### Count metrics (lower is better) @@ -88,26 +89,27 @@ Default thresholds for `sonarqube.openIssues`: scorecard: plugins: sonarqube: - openIssues: - thresholds: - rules: - - key: success - expression: '<1' - - key: warning - expression: '1-10' - - key: error - expression: '>10' + metricProviders: + openIssues: + thresholds: + rules: + - key: success + expression: '<1' + - key: warning + expression: '1-10' + - key: error + expression: '>10' ``` -| Metric | Success | Warning | Error | -| --------------------------------- | ------- | ------- | ----- | -| `sonarqube.openIssues` | `<1` | `1-10` | `>10` | -| `sonarqube.securityIssues` | `<1` | `1-5` | `>5` | -| `sonarqube.securityHotspots` | `<1` | `1-5` | `>5` | -| `sonarqube.reliabilityIssues` | `<1` | `1-5` | `>5` | -| `sonarqube.maintainabilityIssues` | `<10` | `10-50` | `>50` | +| Metric ID | Provider ID | Success | Warning | Error | +| --------------------------------- | --------------------------------- | ------- | ------- | ----- | +| `sonarqube.openIssues` | `sonarqube.openIssues` | `<1` | `1-10` | `>10` | +| `sonarqube.securityIssues` | `sonarqube.securityIssues` | `<1` | `1-5` | `>5` | +| `sonarqube.securityHotspots` | `sonarqube.securityHotspots` | `<1` | `1-5` | `>5` | +| `sonarqube.reliabilityIssues` | `sonarqube.reliabilityIssues` | `<1` | `1-5` | `>5` | +| `sonarqube.maintainabilityIssues` | `sonarqube.maintainabilityIssues` | `<10` | `10-50` | `>50` | -Replace the metric name in the path above for the metrics in this table (e.g. `securityIssues`, `securityHotspots`). Use the same `scorecard.plugins.sonarqube..thresholds` structure as `openIssues`. +Replace the provider key in the path above for the other providers in this table (e.g. `securityIssues`, `securityHotspots`). Use the same `scorecard.plugins.sonarqube.metricProviders..thresholds` structure as `openIssues`. ### Rating metrics (`securityRating`, `securityReviewRating`, `reliabilityRating`, `maintainabilityRating`) @@ -118,32 +120,33 @@ All four rating metrics share the same default thresholds. Default thresholds fo scorecard: plugins: sonarqube: - securityRating: - thresholds: - rules: - - key: A - expression: '==1' - color: 'success.main' - icon: scorecardSuccessStatusIcon - - key: B - expression: '==2' - color: '#bdcb28' - icon: scorecardSuccessStatusIcon - - key: C - expression: '==3' - color: 'warning.main' - icon: scorecardWarningStatusIcon - - key: D - expression: '==4' - color: '#cf5813' - icon: scorecardErrorStatusIcon - - key: E - expression: '==5' - color: 'error.main' - icon: scorecardErrorStatusIcon + metricProviders: + securityRating: + thresholds: + rules: + - key: A + expression: '==1' + color: 'success.main' + icon: scorecardSuccessStatusIcon + - key: B + expression: '==2' + color: '#bdcb28' + icon: scorecardSuccessStatusIcon + - key: C + expression: '==3' + color: 'warning.main' + icon: scorecardWarningStatusIcon + - key: D + expression: '==4' + color: '#cf5813' + icon: scorecardErrorStatusIcon + - key: E + expression: '==5' + color: 'error.main' + icon: scorecardErrorStatusIcon ``` -Replace `securityRating` with `securityReviewRating`, `reliabilityRating`, or `maintainabilityRating` for the other rating metrics. +Replace the `securityRating` provider key with `securityReviewRating`, `reliabilityRating`, or `maintainabilityRating` for the other rating providers. ### Percentage metrics @@ -154,15 +157,16 @@ Default thresholds for `sonarqube.codeCoverage` (higher is better): scorecard: plugins: sonarqube: - codeCoverage: - thresholds: - rules: - - key: success - expression: '>80' - - key: warning - expression: '50-80' - - key: error - expression: '<50' + metricProviders: + codeCoverage: + thresholds: + rules: + - key: success + expression: '>80' + - key: warning + expression: '50-80' + - key: error + expression: '<50' ``` Default thresholds for `sonarqube.codeDuplications` (lower is better): @@ -172,15 +176,16 @@ Default thresholds for `sonarqube.codeDuplications` (lower is better): scorecard: plugins: sonarqube: - codeDuplications: - thresholds: - rules: - - key: success - expression: '<3' - - key: warning - expression: '3-10' - - key: error - expression: '>10' + metricProviders: + codeDuplications: + thresholds: + rules: + - key: success + expression: '<3' + - key: warning + expression: '3-10' + - key: error + expression: '>10' ``` ## Configuration diff --git a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts index 723486a3039..f1d2db4e695 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts @@ -66,29 +66,45 @@ export interface Config { except?: string[]; }; }; - /** Configuration for scorecard metric providers */ + /** Configuration for scorecard datasources plugins */ plugins?: { - /** Configuration for datasource */ + /** Configuration for datasource (e.g. jira, github, filecheck) */ [datasource: string]: { - /** Configuration for metric providers within the datasource. - * Each key corresponds to the metric name part of the provider ID (datasource.metricName). + /** + * How often metrics will be calculated for all metric providers in this + * datasource. Overridden by `metricProviders..schedule` when set. */ - [metricName: string]: { - /** Threshold configuration for the metric */ - thresholds?: ThresholdConfig; - /** - * Schedule for collecting this metric. If not set, the default hourly schedule is used. - * - * Default schedule: - * ```ts - * { - * frequency: { hours: 1 }, - * timeout: { minutes: 15 }, - * initialDelay: { minutes: 1 }, - * } - * ``` - */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** + * How metric values are categorized for all metrics in this datasource. + * Overridden by provider- or metric-level thresholds when set. + */ + thresholds?: ThresholdConfig; + /** Configuration for metric providers within the datasource. Keys are local provider names (no datasource prefix). */ + metricProviders?: { + [providerName: string]: { + /** + * How often metrics will be calculated for this provider. + * Overrides the datasource-level schedule when set. + */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** + * How metric values are categorized for all metrics of this provider. + * Overridden by metric-level thresholds when set. + * Overrides datasource-level thresholds. + */ + thresholds?: ThresholdConfig; + /** Per-metric configuration. Keys are local metric names (no datasource prefix). */ + metrics?: { + [metricName: string]: { + /** + * How metric values are categorized for this metric. + * Overrides datasource- and provider-level thresholds. + */ + thresholds?: ThresholdConfig; + }; + }; + }; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index 17d719d8cb3..f5852df7bd6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -49,10 +49,10 @@ Validation **does not** require full coverage when: ## Threshold Configuration Options -### 1. Provider Default Thresholds +### 1. Metric Default Thresholds -Metric providers must define default thresholds that apply to all entities using that metric in `getMetricThresholds`. -Plugin `@red-hat-developer-hub/backstage-plugin-scorecard-common` provides pre-defined `DEFAULT_NUMBER_THRESHOLDS` which you can import and use in your metric provider. +Each metric returned by `getMetrics()` must include default `thresholds` that apply to all entities using that metric. +Plugin `@red-hat-developer-hub/backstage-plugin-scorecard-common` provides pre-defined `DEFAULT_NUMBER_THRESHOLDS` which you can import and use on your metrics. **Example Provider Implementation:** @@ -60,45 +60,81 @@ Plugin `@red-hat-developer-hub/backstage-plugin-scorecard-common` provides pre-d export class MyMetricProvider implements MetricProvider<'number'> { ... - getMetricThresholds(): ThresholdConfig { - return { - rules: [ - { key: 'success', expression: '<10' }, - { key: 'warning', expression: '10-50' }, - { key: 'error', expression: '>50' }, - ], - }; + getMetrics(): Metric<'number'>[] { + return [ + { + id: 'myDatasource.myMetric', + title: 'My Metric', + description: 'Example metric.', + type: 'number', + thresholds: { + rules: [ + { key: 'success', expression: '<10' }, + { key: 'warning', expression: '10-50' }, + { key: 'error', expression: '>50' }, + ], + }, + }, + ]; } } ``` ### 2. App Configuration Thresholds -You can override provider defaults with your custom thresholds through app configuration (`app-config.yaml`) under `scorecard.plugins..thresholds`. Provider IDs typically -follow the format `.`. Batch providers that specify `` as the `providerId` currently only allow to override global provider thresholds that apply to all metrics the provider defines. +You can override metric defaults through app configuration (`app-config.yaml`). Thresholds may be set at three levels; the **most specific** level winds and completely replaces metric defaults: + +1. **Metric** (highest app-config priority): `scorecard.plugins..metricProviders..metrics..thresholds` +2. **Provider**: `scorecard.plugins..metricProviders..thresholds` +3. **Datasource** (lowest app-config priority): `scorecard.plugins..thresholds` + +Keys under `plugins.` are **local names only** (no datasource prefix for both metric provider IDs and metric IDs). The full runtime provider ID is `.` or `` and metric ID is `.`. + +Datasource- or provider-level thresholds must be valid for every metric they apply to (same metric type). You can use provider-level or datasource-level for single-metric plugins. Threshold configuration is validated in [validateThresholdsForMetric()](../../scorecard-node/src/utils/thresholds/validateThresholds.ts). **Example App Configuration:** +**_Metric level:_** + ```yaml scorecard: plugins: myDatasource: - myMetric: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '<=20' - - key: error - expression: '>20' - myOtherDatasource: - myOtherMetric: ... + metricProviders: + myProvider: + metrics: + myMetric: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '<=20' + - key: error + expression: '>20' +``` + +**_Provider level:_** + +```yaml +scorecard: + plugins: + myDatasource: + metricProviders: + myProvider: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '<=20' + - key: error + expression: '>20' ``` -**Example App Configuration for batch provider:** +**_Datasource level:_** ```yaml scorecard: @@ -107,16 +143,14 @@ scorecard: thresholds: rules: - key: success - expression: '<10' - - key: warning - expression: '<=20' + expression: '==true' - key: error - expression: '>20' + expression: '==false' ``` ### 3. Entity Annotation Overrides -Override thresholds for specific entities using annotations in the entity's metadata: +Override thresholds for specific entities using annotations in the entity's metadata. Annotations use the **full metric ID**: ```yaml apiVersion: backstage.io/v1alpha1 @@ -125,32 +159,32 @@ metadata: name: my-service annotations: # Override specific threshold rules for this entity - scorecard.io/myDatasource.myMetric.thresholds.rules.warning: '10-15' - scorecard.io/myDatasource.myMetric.thresholds.rules.error: '>15' + scorecard.io/myMetricId.thresholds.rules.warning: '10-15' + scorecard.io/myMetricId.thresholds.rules.error: '>15' # success threshold will use the default config value spec: type: service ``` -You can only override existing threshold severity keys for provider. This means you can not specify new custom severity keys in entity annotations, they must be first configured for provider in app configuration. +You can only override existing threshold severity keys for the metric. This means you can not specify new custom severity keys in entity annotations, they must be first configured for the metric in app configuration or defined in the metric code. #### Annotation Format Reference Entity annotations use this format: ```yaml -scorecard.io/{providerId}.thresholds.rules.{thresholdKey}: '{expression}' +scorecard.io/{metricId}.thresholds.rules.{thresholdKey}: '{expression}' ``` Where: -- `{providerId}`: The metric provider ID (e.g., `github.openPRs`) +- `{metricId}`: The full metric ID (e.g., `github.openPRs`, `filecheck.readme`) - `{thresholdKey}`: The threshold category (e.g., `success`, `warning`, `error`) - `{expression}`: The threshold expression (e.g., `>10`, `==true`, `5-15`) -For **number** metrics, each overridden expression is validated in isolation first. If **any** rule was replaced from an annotation, the backend then validates the **merged** rule list for the same **joint full-line coverage** as app-config and provider defaults (see [Joint coverage (number metrics)](#joint-coverage-number-metrics)). If the union of all merged expressions leaves a gap, startup or merge-time evaluation throws **`ThresholdConfigFormatError`** with the usual message starting with `Number threshold rules do not cover the entire real line…` (this is **not** wrapped in the `Invalid threshold annotation '…'` prefix used for single-rule parse errors). +For **number** metrics, each overridden expression is validated in isolation first. If **any** rule was replaced from an annotation, the backend then validates the **merged** rule list for the same **joint full-line coverage** as app-config and metric defaults (see [Joint coverage (number metrics)](#joint-coverage-number-metrics)). If the union of all merged expressions leaves a gap, startup or merge-time evaluation throws **`ThresholdConfigFormatError`** with the usual message starting with `Number threshold rules do not cover the entire real line…` (this is **not** wrapped in the `Invalid threshold annotation '…'` prefix used for single-rule parse errors). -**Counterexample:** Provider rules partition the line (`'<10'`, `'10-20'`, `'>20'`). Overriding only warning to `'11-20'` leaves **`10`** and **`(10, 11)`** uncovered on the merged set—fix the override or adjacent rules so the union again covers **(-∞, +∞)**. +**Counterexample:** Metric default rules partition the line (`'<10'`, `'10-20'`, `'>20'`). Overriding only warning to `'11-20'` leaves **`10`** and **`(10, 11)`** uncovered on the merged set—fix the override or adjacent rules so the union again covers **(-∞, +∞)**. ### 4. Aggregation KPI result thresholds (`weightedStatusScore` type) @@ -171,22 +205,24 @@ These thresholds are **not** per-entity metric rules. They apply only to homepag Thresholds are applied with the following priority (highest to lowest): 1. **Entity Annotations** (highest priority) - _merged_ with existing rules -2. **App Configuration** - _completely replaces_ provider defaults -3. **Provider Defaults** (lowest priority) +2. **App Configuration (metric)** - _completely replaces_ metric code defaults +3. **App Configuration (provider)** - _completely replaces_ metric code defaults +4. **App Configuration (datasource)** - _completely replaces_ metric code defaults +5. **Metric code defaults** (lowest priority) **Merging Behavior:** -- **App Configuration**: Completely replaces provider defaults (no merging), missing rules are not applied +- **App Configuration**: The most specific of metric / provider / datasource completely replaces lower levels (no rule merging between those levels) - **Entity Annotations**: Merged with existing rules from `app-config` or defaults: - Rules with the same `key` are **replaced** by annotation values - - Missing rules fall back to app-config or provider defaults + - Missing rules fall back to app-config or metric defaults - For **number** metrics, if at least one rule came from an annotation, the **merged** rules must still satisfy joint full-line coverage together (see [§3 Entity Annotation Overrides](#3-entity-annotation-overrides)) **Example Priority Application:** ```typescript -// 1. Provider defaults -providerDefaults: [ +// 1. Metric defaults +metricDefaults: [ { key: 'success', expression: '<10' }, { key: 'warning', expression: '10-50' }, { key: 'error', expression: '>50' } @@ -201,8 +237,8 @@ appConfig: [ // 3. Entity annotation overrides (merged with app-config) annotations: { - 'scorecard.io/myProviderId.thresholds.rules.warning': '10-25', - 'scorecard.io/myProviderId.thresholds.rules.error': '>25', + 'scorecard.io/myDatasource.myMetric.thresholds.rules.warning': '10-25', + 'scorecard.io/myDatasource.myMetric.thresholds.rules.error': '>25', } // Final result (annotations merged with app-config) diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts index e2e5ec75501..67afae16483 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts @@ -23,11 +23,23 @@ import { jiraBooleanProvider, MockNumberProvider, MockBooleanProvider, - MockBatchBooleanProvider, filecheckBatchProvider, filecheckBatchMetrics, } from '../../__fixtures__/mockProviders'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; +import { + validateMetricId, + validateProviderId, +} from '../validation/validateMetricProviderIds'; + +jest.mock('../validation/validateMetricProviderIds', () => { + const actual = jest.requireActual('../validation/validateMetricProviderIds'); + return { + ...actual, + validateProviderId: jest.fn(actual.validateProviderId), + validateMetricId: jest.fn(actual.validateMetricId), + }; +}); describe('MetricProvidersRegistry', () => { let registry: MetricProvidersRegistry; @@ -36,6 +48,8 @@ describe('MetricProvidersRegistry', () => { beforeEach(() => { registry = new MetricProvidersRegistry(); + jest.mocked(validateProviderId).mockClear(); + jest.mocked(validateMetricId).mockClear(); }); describe('register', () => { @@ -54,46 +68,94 @@ describe('MetricProvidersRegistry', () => { ]); }); - it('should throw ConflictError when registering provider with duplicate ID', () => { - const provider1 = new MockNumberProvider('jira.duplicateId', 'jira'); - const provider2 = new MockBooleanProvider('jira.duplicateId', 'jira'); - registry.register(provider1); + it('should throw ConflictError when registering duplicate metric IDs', () => { + class ProviderWithSharedMetric extends MockNumberProvider { + getMetrics() { + return [ + { + id: 'jira.sharedMetric', + title: 'Shared', + description: 'Shared', + type: 'number' as const, + thresholds: this.getDefaultThresholds(), + }, + ]; + } + } - expect(() => registry.register(provider2)).toThrow( + registry.register(new ProviderWithSharedMetric('jira.providerA', 'jira')); + + expect(() => + registry.register( + new ProviderWithSharedMetric('jira.providerB', 'jira'), + ), + ).toThrow( new ConflictError( - "Metric provider with ID 'jira.duplicateId' has already been registered", + "Metric with ID 'jira.sharedMetric' has already been registered", ), ); }); - it('should throw error when provider ID does not start with datasource ID', () => { - const invalidProvider = new MockNumberProvider('invalidFormat', 'github'); + it('should throw ConflictError when registering providers with duplicate provider ID', () => { + class ProviderWithMetricA extends MockNumberProvider { + getMetrics() { + return [ + { + id: 'github.metricA', + title: 'A', + description: 'A', + type: 'number' as const, + thresholds: this.getDefaultThresholds(), + }, + ]; + } + } + class ProviderWithMetricB extends MockNumberProvider { + getMetrics() { + return [ + { + id: 'github.metricB', + title: 'B', + description: 'B', + type: 'number' as const, + thresholds: this.getDefaultThresholds(), + }, + ]; + } + } - expect(() => registry.register(invalidProvider)).toThrow( - new Error( - "Invalid metric provider with ID invalidFormat, must have format 'github.' where metric name is not empty", + registry.register(new ProviderWithMetricA('github.shared', 'github')); + + expect(() => + registry.register(new ProviderWithMetricB('github.shared', 'github')), + ).toThrow( + new ConflictError( + "Metric provider with ID 'github.shared' has already been registered", ), ); }); - it('should throw error when provider ID has no metric name after datasource', () => { - const invalidProvider = new MockNumberProvider('github.', 'github'); + it('should call validateProviderId when registering', () => { + registry.register(githubNumberProvider); - expect(() => registry.register(invalidProvider)).toThrow( - new Error( - "Invalid metric provider with ID github., must have format 'github.' where metric name is not empty", - ), + expect(validateProviderId).toHaveBeenCalledWith( + githubNumberProvider.getProviderId(), + githubNumberProvider.getProviderDatasourceId(), ); }); - it('should throw error for provider ID missing dot separator', () => { - const invalidProvider = new MockNumberProvider('githubopenPrs', 'github'); + it('should call validateMetricId for each metric when registering', () => { + registry.register(filecheckBatchProvider); - expect(() => registry.register(invalidProvider)).toThrow( - new Error( - "Invalid metric provider with ID githubopenPrs, must have format 'github.' where metric name is not empty", - ), + expect(validateMetricId).toHaveBeenCalledTimes( + filecheckBatchMetrics.length, ); + for (const metric of filecheckBatchMetrics) { + expect(validateMetricId).toHaveBeenCalledWith( + metric.id, + filecheckBatchProvider.getProviderDatasourceId(), + ); + } }); it('should throw error when provider default thresholds are invalid', () => { @@ -152,41 +214,10 @@ describe('MetricProvidersRegistry', () => { expect(() => registry.register(filecheckBatchProvider)).toThrow( new ConflictError( - "Metric provider with ID 'filecheck.readme' has already been registered", + "Metric with ID 'filecheck.readme' has already been registered", ), ); }); - - it('should throw error when batch provider metric ID has wrong format', () => { - class InvalidBatchProvider extends MockBatchBooleanProvider { - getMetrics() { - return [ - { - id: 'invalidFormat', - title: 'Invalid', - description: 'Invalid', - type: 'boolean' as const, - thresholds: { - rules: [ - { key: 'success', expression: '==true' }, - { key: 'error', expression: '==false' }, - ], - }, - }, - ]; - } - } - - const invalidProvider = new InvalidBatchProvider( - 'github', - 'filecheck', - [], - ); - - expect(() => registry.register(invalidProvider)).toThrow( - "Invalid metric provider with ID invalidFormat, must have format 'github.' where metric name is not empty", - ); - }); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts index 36e8523aee2..426c1dbd4be 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts @@ -25,36 +25,45 @@ import { ThresholdConfigFormatError, validateThresholdsForMetric, } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { + validateMetricId, + validateProviderId, +} from '../validation/validateMetricProviderIds'; /** * Registry of all registered metric providers. */ export class MetricProvidersRegistry { + /** metricId → provider (a multi-metric provider is stored under each of its metric IDs) */ private readonly metricProviders = new Map(); + /** datasourceId → set of metricIds for that datasource */ private readonly datasourceIndex = new Map>(); + /** Registered provider IDs (unique; used for scheduler task / config keys) */ + private readonly registeredProviderIds = new Set(); register(metricProvider: MetricProvider): void { const providerDatasource = metricProvider.getProviderDatasourceId(); const providerId = metricProvider.getProviderId(); + validateProviderId(providerId, providerDatasource); + + if (this.registeredProviderIds.has(providerId)) { + throw new ConflictError( + `Metric provider with ID '${providerId}' has already been registered`, + ); + } + const metrics = metricProvider.getMetrics(); const metricIds = metrics.map(m => m.id); for (const metric of metrics) { const metricId = metric.id; - // Validate: Provider ID format (datasource.metricName) - const expectedPrefix = `${providerDatasource}.`; - if (!metricId.startsWith(expectedPrefix) || metricId === expectedPrefix) { - throw new Error( - `Invalid metric provider with ID ${metricId}, must have format ` + - `'${providerDatasource}.' where metric name is not empty`, - ); - } + validateMetricId(metricId, providerDatasource); if (this.metricProviders.has(metricId)) { throw new ConflictError( - `Metric provider with ID '${metricId}' has already been registered`, + `Metric with ID '${metricId}' has already been registered`, ); } @@ -68,16 +77,18 @@ export class MetricProvidersRegistry { } } + this.registeredProviderIds.add(providerId); + for (const metricId of metricIds) { this.metricProviders.set(metricId, metricProvider); // Index by datasource - let datasourceProviders = this.datasourceIndex.get(providerDatasource); - if (!datasourceProviders) { - datasourceProviders = new Set(); - this.datasourceIndex.set(providerDatasource, datasourceProviders); + let datasourceMetricIds = this.datasourceIndex.get(providerDatasource); + if (!datasourceMetricIds) { + datasourceMetricIds = new Set(); + this.datasourceIndex.set(providerDatasource, datasourceMetricIds); } - datasourceProviders.add(metricId); + datasourceMetricIds.add(metricId); } } diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts index f0d889b7a31..5467277081e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts @@ -62,7 +62,11 @@ describe('PullMetricsByProviderTask', () => { mockConfig = mockServices.rootConfig({ data: { scorecard: { - schedule: scheduleConfig, + plugins: { + github: { + schedule: scheduleConfig, + }, + }, }, }, }); @@ -126,25 +130,44 @@ describe('PullMetricsByProviderTask', () => { describe('start', () => { beforeEach(async () => { - (task as any).getScheduleFromConfig = jest - .fn() - .mockReturnValue({ frequency: { hours: 1 } }); (task as any).pullProviderMetrics = jest .fn() .mockResolvedValue(undefined); await (task as any).start(); }); - it('should get scheduled from config', () => { - expect((task as any).getScheduleFromConfig).toHaveBeenCalledWith( - 'scorecard.plugins.github.testMetric.schedule', + it('should create a scheduled task runner with schedule from config', () => { + expect(mockScheduler.createScheduledTaskRunner).toHaveBeenCalledTimes(1); + expect(mockScheduler.createScheduledTaskRunner).toHaveBeenCalledWith( + scheduleConfig, ); }); - it('should create a scheduled task runner with correct schedule', () => { - expect(mockScheduler.createScheduledTaskRunner).toHaveBeenCalledTimes(1); + it('should use the default schedule when none is configured', async () => { + mockScheduler.createScheduledTaskRunner.mockClear(); + const taskWithoutSchedule = new PullMetricsByProviderTask( + { + scheduler: mockScheduler, + logger: mockLogger, + database: mockDatabaseMetricValues, + config: mockServices.rootConfig({ data: {} }), + catalog: mockCatalog, + auth: mockAuth, + thresholdEvaluator: mockThresholdEvaluator, + thresholdResolver: mockThresholdResolver, + }, + mockProvider, + ); + (taskWithoutSchedule as any).pullProviderMetrics = jest + .fn() + .mockResolvedValue(undefined); + + await (taskWithoutSchedule as any).start(); + expect(mockScheduler.createScheduledTaskRunner).toHaveBeenCalledWith({ frequency: { hours: 1 }, + timeout: { minutes: 15 }, + initialDelay: { minutes: 1 }, }); }); @@ -157,24 +180,6 @@ describe('PullMetricsByProviderTask', () => { }); }); - describe('getScheduleFromConfig', () => { - it('should return the default schedule if not configured', () => { - const config = (task as any).getScheduleFromConfig( - 'scorecard.schedule.notExists', - ); - expect(config).toEqual({ - frequency: { hours: 1 }, - timeout: { minutes: 15 }, - initialDelay: { minutes: 1 }, - }); - }); - - it('should return the schedule from config if configured', () => { - const config = (task as any).getScheduleFromConfig('scorecard.schedule'); - expect(config).toEqual(scheduleConfig); - }); - }); - describe('pullProviderMetrics', () => { beforeEach(async () => { mockCatalog.queryEntities.mockResolvedValueOnce({ @@ -223,26 +228,16 @@ describe('PullMetricsByProviderTask', () => { expect(getOwnServiceCredentialsSpy).toHaveBeenCalledWith(); }); - it('should resolve thresholds for entity/metric/provider', async () => { + it('should resolve thresholds for entity/metric', async () => { await (task as any).pullProviderMetrics(mockProvider, mockLogger); const metric = mockProvider.getMetrics()[0]; expect( mockThresholdResolver.resolveEntityThresholds, - ).toHaveBeenNthCalledWith( - 1, - mockEntities[0], - metric, - mockProvider.getProviderId(), - ); + ).toHaveBeenNthCalledWith(1, mockEntities[0], metric); expect( mockThresholdResolver.resolveEntityThresholds, - ).toHaveBeenNthCalledWith( - 2, - mockEntities[1], - metric, - mockProvider.getProviderId(), - ); + ).toHaveBeenNthCalledWith(2, mockEntities[1], metric); }); it('should calculate metrics', async () => { @@ -732,18 +727,39 @@ describe('PullMetricsByProviderTask', () => { expect(savedRecords).toHaveLength(1); }); - it('should get schedule from correct config path for batch provider', async () => { - (task as any).getScheduleFromConfig = jest - .fn() - .mockReturnValue({ frequency: { hours: 1 } }); - (task as any).pullProviderMetrics = jest + it('should use datasource schedule when no provider schedule set', async () => { + mockScheduler.createScheduledTaskRunner.mockClear(); + const batchTask = new PullMetricsByProviderTask( + { + scheduler: mockScheduler, + logger: mockLogger, + database: mockDatabaseMetricValues, + config: mockServices.rootConfig({ + data: { + scorecard: { + plugins: { + filecheck: { + schedule: scheduleConfig, + }, + }, + }, + }, + }), + catalog: mockCatalog, + auth: mockAuth, + thresholdEvaluator: mockThresholdEvaluator, + thresholdResolver: mockThresholdResolver, + }, + mockBatchProvider, + ); + (batchTask as any).pullProviderMetrics = jest .fn() .mockResolvedValue(undefined); - await (task as any).start(); + await (batchTask as any).start(); - expect((task as any).getScheduleFromConfig).toHaveBeenCalledWith( - 'scorecard.plugins.filecheck.schedule', + expect(mockScheduler.createScheduledTaskRunner).toHaveBeenCalledWith( + scheduleConfig, ); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts index ce43ad4b7b1..bb122ad8bf7 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -17,7 +17,6 @@ import { DatabaseMetricValues } from '../../database/DatabaseMetricValues'; import { AuthService, - readSchedulerServiceTaskScheduleDefinitionFromConfig, SchedulerService, SchedulerServiceTaskScheduleDefinition, LoggerService, @@ -28,6 +27,7 @@ import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecar import { isMetricIdDisabled } from '../../utils/metricUtils'; import { randomUUID } from 'node:crypto'; import { normalizeOwnerRef } from '../../utils/normalizeOwnerRef'; +import { resolveScheduleFromConfig } from '../../utils/metricProviderConfigKeys'; import { stringifyEntityRef } from '@backstage/catalog-model'; import { DbMetricValueCreate } from '../../database/types'; import { SchedulerOptions, SchedulerTask } from '../types'; @@ -85,8 +85,12 @@ export class PullMetricsByProviderTask implements SchedulerTask { } async start(): Promise { - const scheduleConfigPath = `scorecard.plugins.${this.providerId}.schedule`; - const schedule = this.getScheduleFromConfig(scheduleConfigPath); + const schedule = + resolveScheduleFromConfig( + this.config, + this.provider.getProviderDatasourceId(), + this.providerId, + ) ?? PullMetricsByProviderTask.DEFAULT_SCHEDULE; const taskRunner = this.scheduler.createScheduledTaskRunner(schedule); @@ -111,16 +115,6 @@ export class PullMetricsByProviderTask implements SchedulerTask { }); } - private getScheduleFromConfig( - schedulePath: string, - ): SchedulerServiceTaskScheduleDefinition { - return this.config.has(schedulePath) - ? readSchedulerServiceTaskScheduleDefinitionFromConfig( - this.config.getConfig(schedulePath), - ) - : PullMetricsByProviderTask.DEFAULT_SCHEDULE; - } - private async pullProviderMetrics( provider: MetricProvider, logger: LoggerService, @@ -188,7 +182,6 @@ export class PullMetricsByProviderTask implements SchedulerTask { this.thresholdResolver.resolveEntityThresholds( entity, metric, - provider.getProviderId(), ); const status = diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts index bebcde5769d..d1106ddd82b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -297,16 +297,6 @@ describe('CatalogMetricService', () => { expect(result).toEqual([]); }); - it('should get provider by metric ID', async () => { - await service.getLatestEntityMetrics('component:default/test-component', [ - 'github.importantMetric', - ]); - - expect(mockedRegistry.getProvider).toHaveBeenCalledWith( - 'github.importantMetric', - ); - }); - it('should get metric from registry', async () => { await service.getLatestEntityMetrics('component:default/test-component', [ 'github.importantMetric', @@ -317,7 +307,7 @@ describe('CatalogMetricService', () => { ); }); - it('should merge entity and provider thresholds', async () => { + it('should merge entity and metric thresholds', async () => { await service.getLatestEntityMetrics('component:default/test-component', [ 'github.importantMetric', ]); @@ -327,7 +317,6 @@ describe('CatalogMetricService', () => { ).toHaveBeenCalledWith( mockEntity, expect.objectContaining({ id: 'github.importantMetric' }), - provider.getProviderId(), ); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index 8d044a716f8..eb20bfaaea4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -128,14 +128,12 @@ export class CatalogMetricService { let thresholds: ThresholdConfig | undefined; let thresholdError: string | undefined; - const provider = this.registry.getProvider(metric_id); const metric = this.registry.getMetric(metric_id); try { thresholds = this.thresholdResolver.resolveEntityThresholds( entity, metric, - provider.getProviderId(), ); if (value === null) { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts index 768b4a3685c..2830a9b31bc 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -737,12 +737,8 @@ describe('createRouter', () => { const emptyAggregatedMetric = AggregatedMetricMapper.toAggregatedMetric(); const { AggregatedMetricMapper: ActualAggregatedMetricMapper } = jest.requireActual('./mappers'); - const provider = metricProvidersRegistry.getProvider('github.openPRs'); const metric = metricProvidersRegistry.getMetric('github.openPRs'); - const thresholds = thresholdResolver.resolveMetricThresholds( - metric, - provider.getProviderId(), - ); + const thresholds = thresholdResolver.resolveMetricThresholds(metric); const emptyAggregatedMetricResult = ActualAggregatedMetricMapper.toAggregatedMetricResult( metric, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts index 491e9e1ffcd..ec1a834b309 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts @@ -157,7 +157,6 @@ export async function createRouter({ scorecardMetricReadPermission, ); - const provider = metricProvidersRegistry.getProvider(metricId); const metric = metricProvidersRegistry.getMetric(metricId); const authorizedMetrics = filterAuthorizedMetrics([metric], conditions); @@ -183,10 +182,7 @@ export async function createRouter({ await checkEntityAccess(entityRef, req, permissions, httpAuth); } - const thresholds = thresholdResolver.resolveMetricThresholds( - metric, - provider.getProviderId(), - ); + const thresholds = thresholdResolver.resolveMetricThresholds(metric); logger.warn( `Deprecated Scorecard API: GET /metrics/${metricId}/catalog/aggregations is deprecated; use GET /aggregations/:aggregationId (e.g. when the aggregation id matches the metric id, GET /aggregations/${metricId}).`, @@ -285,9 +281,6 @@ export async function createRouter({ const aggregationConfig = aggregationsService.getAggregationConfig(aggregationId); - const provider = metricProvidersRegistry.getProvider( - aggregationConfig.metricId, - ); const metric = metricProvidersRegistry.getMetric( aggregationConfig?.metricId ?? aggregationId, ); @@ -308,10 +301,7 @@ export async function createRouter({ ); } - const thresholds = thresholdResolver.resolveMetricThresholds( - metric, - provider.getProviderId(), - ); + const thresholds = thresholdResolver.resolveMetricThresholds(metric); res.json( await aggregationsService.getAggregatedMetricByEntityRefs({ diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts index a1e10cdb50e..b685913198f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts @@ -16,8 +16,8 @@ import { ConfigReader } from '@backstage/config'; import { - MockBooleanProvider, MockNumberProvider, + MockBatchBooleanProvider, } from '../../__fixtures__/mockProviders'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; import { ThresholdResolver } from './ThresholdResolver'; @@ -27,13 +27,19 @@ describe('ThresholdResolver', () => { scorecard: { plugins: { github: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], + metricProviders: { + numberMetric: { + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, }, }, }, @@ -48,13 +54,19 @@ describe('ThresholdResolver', () => { scorecard: { plugins: { github: { - otherMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], + metricProviders: { + otherMetric: { + metrics: { + otherMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, }, }, }, @@ -64,12 +76,7 @@ describe('ThresholdResolver', () => { [provider, new MockNumberProvider('github.otherMetric', 'github')], ); - expect( - resolver.resolveMetricThresholds( - provider.getMetrics()[0], - provider.getProviderId(), - ), - ).toEqual({ + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ rules: [ { key: 'error', expression: '>40' }, { key: 'warning', expression: '>20' }, @@ -85,12 +92,7 @@ describe('ThresholdResolver', () => { provider, ]); - expect( - resolver.resolveMetricThresholds( - provider.getMetrics()[0], - provider.getProviderId(), - ), - ).toEqual({ + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ rules: [ { key: 'error', expression: '>100' }, { key: 'warning', expression: '>50' }, @@ -99,42 +101,171 @@ describe('ThresholdResolver', () => { }); }); - it('uses configured thresholds before provider default thresholds for batch provider', () => { - const provider = new MockBooleanProvider('filecheck', 'filecheck'); + it('uses datasource-level thresholds when no provider or metric thresholds', () => { + const provider = new MockNumberProvider('github.numberMetric', 'github'); const resolver = new ThresholdResolver( new ConfigReader({ scorecard: { plugins: { - filecheck: { + github: { thresholds: { rules: [ - { - key: 'present', - expression: '==true', - color: 'success.main', - icon: 'scorecardSuccessStatusIcon', - }, - { - key: 'absent', - expression: '==false', - color: 'error.main', - icon: 'scorecardErrorStatusIcon', + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, + }, + }), + [provider], + ); + + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }); + }); + + it('uses provider-level thresholds over datasource-level thresholds', () => { + const provider = new MockNumberProvider('github.numberMetric', 'github'); + const resolver = new ThresholdResolver( + new ConfigReader({ + scorecard: { + plugins: { + github: { + thresholds: { + rules: [ + { key: 'error', expression: '>200' }, + { key: 'warning', expression: '>100' }, + { key: 'success', expression: '<=100' }, + ], + }, + metricProviders: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, + }, + }, + }, + }, + }, + }), + [provider], + ); + + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }); + }); + + it('uses metric-level thresholds over provider- and datasource-level thresholds', () => { + const provider = new MockNumberProvider('github.numberMetric', 'github'); + const resolver = new ThresholdResolver( + new ConfigReader({ + scorecard: { + plugins: { + github: { + thresholds: { + rules: [ + { key: 'error', expression: '>200' }, + { key: 'warning', expression: '>100' }, + { key: 'success', expression: '<=100' }, ], }, + metricProviders: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>150' }, + { key: 'warning', expression: '>75' }, + { key: 'success', expression: '<=75' }, + ], + }, + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, + }, + }, }, }, }, }), - [new MockNumberProvider('github.otherMetric', 'github'), provider], + [provider], ); - expect( - resolver.resolveMetricThresholds( - provider.getMetrics()[0], - provider.getProviderId(), - ), - ).toEqual({ + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }); + }); + + it('uses configured thresholds per metric when provider ID equals datasource', () => { + const provider = new MockBatchBooleanProvider('filecheck', 'filecheck', [ + { id: 'readme', path: 'README.md' }, + { id: 'license', path: 'LICENSE' }, + ]); + const resolver = new ThresholdResolver( + new ConfigReader({ + scorecard: { + plugins: { + filecheck: { + metricProviders: { + filecheck: { + metrics: { + readme: { + thresholds: { + rules: [ + { + key: 'present', + expression: '==true', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + { + key: 'absent', + expression: '==false', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }), + [provider], + ); + + expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ rules: [ { key: 'present', @@ -150,6 +281,14 @@ describe('ThresholdResolver', () => { }, ], }); + + // license has no config override — falls back to metric defaults + expect(resolver.resolveMetricThresholds(provider.getMetrics()[1])).toEqual({ + rules: [ + { key: 'success', expression: '==true' }, + { key: 'error', expression: '==false' }, + ], + }); }); it('merges entity annotation overrides on top of default provider thresholds', () => { @@ -163,11 +302,7 @@ describe('ThresholdResolver', () => { .build(); expect( - resolver.resolveEntityThresholds( - entity, - provider.getMetrics()[0], - provider.getProviderId(), - ), + resolver.resolveEntityThresholds(entity, provider.getMetrics()[0]), ).toEqual({ rules: [ { key: 'error', expression: '>40' }, @@ -188,11 +323,7 @@ describe('ThresholdResolver', () => { .build(); expect( - resolver.resolveEntityThresholds( - entity, - provider.getMetrics()[0], - provider.getProviderId(), - ), + resolver.resolveEntityThresholds(entity, provider.getMetrics()[0]), ).toEqual({ rules: [ { key: 'error', expression: '>40' }, @@ -215,11 +346,7 @@ describe('ThresholdResolver', () => { .build(); expect( - resolver.resolveEntityThresholds( - entity, - provider.getMetrics()[0], - provider.getProviderId(), - ), + resolver.resolveEntityThresholds(entity, provider.getMetrics()[0]), ).toEqual({ rules: [ { key: 'error', expression: '>100' }, @@ -229,67 +356,60 @@ describe('ThresholdResolver', () => { }); }); - it('merges entity annotation overrides on top of custom provider thresholds for batch provider', () => { - const provider = new MockBooleanProvider('filecheck', 'filecheck'); - const resolver = new ThresholdResolver(new ConfigReader(customThresholds), [ - provider, - ]); - const entity = new MockEntityBuilder() - .withAnnotations({ - 'scorecard.io/filecheck.thresholds.rules.success': '==false', - 'scorecard.io/filecheck.thresholds.rules.error': '==true', - }) - .build(); - - expect( - resolver.resolveEntityThresholds( - entity, - provider.getMetrics()[0], - provider.getProviderId(), - ), - ).toEqual({ - rules: [ - { key: 'success', expression: '==false' }, - { key: 'error', expression: '==true' }, - ], - }); - }); - it('loads configured thresholds once at startup', () => { - const mockConfig = { - getOptional: jest.fn().mockReturnValue({ - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }), - } as any; + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + metricProviders: { + numberMetric: { + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, + }, + }, + }, + }, + }, + }); + const getOptionalSpy = jest.spyOn(config, 'getOptional'); const provider = new MockNumberProvider('github.numberMetric', 'github'); - const resolver = new ThresholdResolver(mockConfig, [provider]); + const resolver = new ThresholdResolver(config, [provider]); - resolver.resolveMetricThresholds( - provider.getMetrics()[0], - provider.getProviderId(), - ); - resolver.resolveMetricThresholds( - provider.getMetrics()[0], - provider.getProviderId(), - ); + resolver.resolveMetricThresholds(provider.getMetrics()[0]); + resolver.resolveMetricThresholds(provider.getMetrics()[0]); - expect(mockConfig.getOptional).toHaveBeenCalledTimes(1); + expect(getOptionalSpy).toHaveBeenCalledTimes(1); }); it('validates configured thresholds at startup', () => { - const mockConfig = { - getOptional: jest.fn().mockReturnValue({ - rules: [{ key: 'error', expression: 'INVALID' }], - }), - } as any; + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + metricProviders: { + numberMetric: { + thresholds: { + rules: [{ key: 'error', expression: 'INVALID' }], + }, + }, + }, + }, + }, + }, + }); const provider = new MockNumberProvider('github.numberMetric', 'github'); - expect(() => new ThresholdResolver(mockConfig, [provider])).toThrow( - 'Invalid thresholds configuration at scorecard.plugins.github.numberMetric.thresholds', + expect(() => new ThresholdResolver(config, [provider])).toThrow( + 'Invalid thresholds configuration at scorecard.plugins.github.metricProviders.numberMetric.thresholds', ); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts index 387ceb2c528..e207dd2e8d1 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts @@ -24,10 +24,11 @@ import { getThresholdsFromConfig, type MetricProvider, } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; -import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; +import { mergeEntityAndMetricThresholds } from '../utils/mergeEntityAndMetricThresholds'; +import { resolveThresholdsConfigPath } from '../utils/metricProviderConfigKeys'; export class ThresholdResolver { - private readonly configuredThresholds = new Map(); // providerId: thresholds + private readonly configuredThresholds = new Map(); // metricId: thresholds constructor(private readonly config: Config, providers: MetricProvider[]) { for (const provider of providers) { @@ -35,36 +36,41 @@ export class ThresholdResolver { } } - resolveMetricThresholds(metric: Metric, providerId: string): ThresholdConfig { - return this.configuredThresholds.get(providerId) ?? metric.thresholds; + resolveMetricThresholds(metric: Metric): ThresholdConfig { + return this.configuredThresholds.get(metric.id) ?? metric.thresholds; } - resolveEntityThresholds( - entity: Entity, - metric: Metric, - providerId: string, - ): ThresholdConfig { - return mergeEntityAndProviderThresholds( + resolveEntityThresholds(entity: Entity, metric: Metric): ThresholdConfig { + return mergeEntityAndMetricThresholds( entity, metric, - providerId, - this.resolveMetricThresholds(metric, providerId), + this.resolveMetricThresholds(metric), ); } private setConfiguredThresholds(provider: MetricProvider): void { + const datasourceId = provider.getProviderDatasourceId(); const providerId = provider.getProviderId(); - const metrics = provider.getMetrics(); - if (metrics.length === 0) return; - const thresholds = getThresholdsFromConfig( - this.config, - `scorecard.plugins.${providerId}.thresholds`, - metrics[0].type, - ); + for (const metric of provider.getMetrics()) { + const thresholdsPath = resolveThresholdsConfigPath( + this.config, + datasourceId, + providerId, + metric.id, + ); + if (!thresholdsPath) { + continue; + } - if (thresholds) { - this.configuredThresholds.set(providerId, thresholds); + const thresholds = getThresholdsFromConfig( + this.config, + thresholdsPath, + metric.type, + ); + if (thresholds) { + this.configuredThresholds.set(metric.id, thresholds); + } } } } diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.test.ts similarity index 73% rename from workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.test.ts rename to workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.test.ts index 8eb89060ee0..373f2311062 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.test.ts @@ -19,7 +19,7 @@ import { MockNumberProvider, MockBooleanProvider, } from '../../__fixtures__/mockProviders'; -import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; +import { mergeEntityAndMetricThresholds } from './mergeEntityAndMetricThresholds'; import { ThresholdConfigFormatError } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; @@ -38,7 +38,7 @@ const booleanMetricThresholds = { ], }; -describe('mergeEntityAndProviderThresholds', () => { +describe('mergeEntityAndMetricThresholds', () => { let entity: Entity; const numberMetricProvider = new MockNumberProvider( @@ -70,41 +70,25 @@ describe('mergeEntityAndProviderThresholds', () => { describe('when entity has no threshold overrides', () => { it('should return metric thresholds unchanged for number metric', () => { - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual(numberMetricThresholds); }); it('should return metric thresholds unchanged for boolean metric', () => { - const result = mergeEntityAndProviderThresholds( - entity, - booleanMetric, - booleanMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, booleanMetric); expect(result).toEqual(booleanMetricThresholds); }); it('should handle entity with empty annotations', () => { entity.metadata.annotations = {}; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual(numberMetricThresholds); }); it('should handle entity with no metadata', () => { entity.metadata = { name: 'test-component' }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual(numberMetricThresholds); }); }); @@ -114,11 +98,7 @@ describe('mergeEntityAndProviderThresholds', () => { entity.metadata.annotations = { 'scorecard.io/github.importantMetric.thresholds.rules.error': '>50', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual({ rules: [ @@ -135,11 +115,7 @@ describe('mergeEntityAndProviderThresholds', () => { 'scorecard.io/github.importantMetric.thresholds.rules.warning': '>30', 'scorecard.io/github.importantMetric.thresholds.rules.success': '<=30', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual({ rules: [ @@ -154,11 +130,7 @@ describe('mergeEntityAndProviderThresholds', () => { entity.metadata.annotations = { 'scorecard.io/jira.booleanMetric.thresholds.rules.success': '!=false', }; - const result = mergeEntityAndProviderThresholds( - entity, - booleanMetric, - booleanMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, booleanMetric); expect(result).toEqual({ rules: [ @@ -172,26 +144,18 @@ describe('mergeEntityAndProviderThresholds', () => { entity.metadata.annotations = { 'scorecard.io/github.importantMetric.thresholds.rules.error': '', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual(numberMetricThresholds); }); - it('should ignore annotations that do not match the provider prefix', () => { + it('should ignore annotations that do not match the metric prefix', () => { entity.metadata.annotations = { 'scorecard.io/other.provider.thresholds.rules.error': '>50', 'scorecard.io/github.importantMetric.thresholds.rules.warning': '>30', 'scorecard.io/github.importantMetric.thresholds.rules.success': '<=30', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual({ rules: [ @@ -208,11 +172,7 @@ describe('mergeEntityAndProviderThresholds', () => { 'scorecard.io/github.importantMetric.thresholds.rules.warning': '0-100', 'scorecard.io/github.importantMetric.thresholds.rules.success': '<0', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result).toEqual({ rules: [ @@ -248,11 +208,7 @@ describe('mergeEntityAndProviderThresholds', () => { 'scorecard.io/github.customMetric.thresholds.rules.error': '>60', }; - const result = mergeEntityAndProviderThresholds( - entity, - customMetric, - 'github.customMetric', - ); + const result = mergeEntityAndMetricThresholds(entity, customMetric); expect(result).toEqual({ rules: [ @@ -288,18 +244,10 @@ describe('mergeEntityAndProviderThresholds', () => { }; expect(() => - mergeEntityAndProviderThresholds( - entity, - partitionMetric, - 'github.partitionMetric', - ), + mergeEntityAndMetricThresholds(entity, partitionMetric), ).toThrow(ThresholdConfigFormatError); expect(() => - mergeEntityAndProviderThresholds( - entity, - partitionMetric, - 'github.partitionMetric', - ), + mergeEntityAndMetricThresholds(entity, partitionMetric), ).toThrow(/do not cover the entire real line/); }); }); @@ -308,20 +256,10 @@ describe('mergeEntityAndProviderThresholds', () => { entity.metadata.annotations = { 'scorecard.io/github.importantMetric.thresholds.rules.error': 'invalid', }; - expect(() => - mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ), - ).toThrow(ThresholdConfigFormatError); - expect(() => - mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ), - ).toThrow( + expect(() => mergeEntityAndMetricThresholds(entity, numberMetric)).toThrow( + ThresholdConfigFormatError, + ); + expect(() => mergeEntityAndMetricThresholds(entity, numberMetric)).toThrow( "Invalid threshold annotation 'scorecard.io/github.importantMetric.thresholds.rules.error: invalid' in entity 'component:default/test-component'", ); }); @@ -330,20 +268,10 @@ describe('mergeEntityAndProviderThresholds', () => { entity.metadata.annotations = { 'scorecard.io/jira.booleanMetric.thresholds.rules.success': '>40', }; - expect(() => - mergeEntityAndProviderThresholds( - entity, - booleanMetric, - booleanMetricProvider.getProviderId(), - ), - ).toThrow(ThresholdConfigFormatError); - expect(() => - mergeEntityAndProviderThresholds( - entity, - booleanMetric, - booleanMetricProvider.getProviderId(), - ), - ).toThrow( + expect(() => mergeEntityAndMetricThresholds(entity, booleanMetric)).toThrow( + ThresholdConfigFormatError, + ); + expect(() => mergeEntityAndMetricThresholds(entity, booleanMetric)).toThrow( "Invalid threshold annotation 'scorecard.io/jira.booleanMetric.thresholds.rules.success: >40' in entity 'component:default/test-component'", ); }); @@ -353,11 +281,7 @@ describe('mergeEntityAndProviderThresholds', () => { 'scorecard.io/github.importantMetric.thresholds.rules.success': '<=20', 'scorecard.io/github.importantMetric.thresholds.rules.error': '>50', }; - const result = mergeEntityAndProviderThresholds( - entity, - numberMetric, - numberMetricProvider.getProviderId(), - ); + const result = mergeEntityAndMetricThresholds(entity, numberMetric); expect(result.rules[0]).toEqual({ key: 'error', expression: '>50' }); expect(result.rules[1]).toEqual({ key: 'warning', expression: '>20' }); @@ -379,20 +303,12 @@ describe('mergeEntityAndProviderThresholds', () => { 'scorecard.io/github.importantMetric.thresholds.rules.success': '<=10', }; expect(() => - mergeEntityAndProviderThresholds( - entity, - singleRuleMetric, - 'github.importantMetric', - ), + mergeEntityAndMetricThresholds(entity, singleRuleMetric), ).toThrow(ThresholdConfigFormatError); expect(() => - mergeEntityAndProviderThresholds( - entity, - singleRuleMetric, - 'github.importantMetric', - ), + mergeEntityAndMetricThresholds(entity, singleRuleMetric), ).toThrow( - 'Unable to override component:default/test-component thresholds by {"key":"success","expression":"<=10"}, metric provider github.importantMetric does not support key success', + 'Unable to override component:default/test-component thresholds by {"key":"success","expression":"<=10"}, metric github.importantMetric does not support key success', ); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.ts similarity index 83% rename from workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts rename to workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.ts index 24c3525d0af..2932ea531d5 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndMetricThresholds.ts @@ -27,18 +27,18 @@ import { } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { isError } from '@backstage/errors'; -const thresholdRulesAnnotationPrefix = (providerId: string) => - `scorecard.io/${providerId}.thresholds.rules.`; +const thresholdRulesAnnotationPrefix = (metricId: string) => + `scorecard.io/${metricId}.thresholds.rules.`; /** - * Extract threshold override rules from entity annotations for a given provider, doesn't validate rules. + * Extract threshold override rules from entity annotations for a given metric, doesn't validate rules. */ function parseEntityAnnotationThresholds( entity: Entity, - providerId: string, + metricId: string, ): ThresholdRule[] { const annotations = entity.metadata?.annotations || {}; - const prefix = thresholdRulesAnnotationPrefix(providerId); + const prefix = thresholdRulesAnnotationPrefix(metricId); const overrides: ThresholdRule[] = []; for (const [annotationKey, expression] of Object.entries(annotations)) { @@ -51,22 +51,22 @@ function parseEntityAnnotationThresholds( return overrides; } -export function mergeEntityAndProviderThresholds( +export function mergeEntityAndMetricThresholds( entity: Entity, metric: Metric, - providerId: string, baseThresholds?: ThresholdConfig, ): ThresholdConfig { let isRulesMerged = false; - const providerThresholds = baseThresholds ?? metric.thresholds; + const metricThresholds = baseThresholds ?? metric.thresholds; const metricType = metric.type; + const metricId = metric.id; const entityAnnotationThresholds = parseEntityAnnotationThresholds( entity, - providerId, + metricId, ); - const mergedRules = [...providerThresholds.rules]; + const mergedRules = [...metricThresholds.rules]; for (const override of entityAnnotationThresholds) { const foundKey = mergedRules.findIndex(rule => rule.key === override.key); if (foundKey === -1) { @@ -75,7 +75,7 @@ export function mergeEntityAndProviderThresholds( entity, )} thresholds by ${JSON.stringify( override, - )}, metric provider ${providerId} does not support key ${override.key}`, + )}, metric ${metricId} does not support key ${override.key}`, ); } @@ -88,7 +88,7 @@ export function mergeEntityAndProviderThresholds( if (isError(e)) { throw new ThresholdConfigFormatError( `Invalid threshold annotation '${thresholdRulesAnnotationPrefix( - providerId, + metricId, )}${override.key}: ${ override.expression }' in entity '${stringifyEntityRef(entity)}': ${e.message}`, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts new file mode 100644 index 00000000000..664be3de6c4 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts @@ -0,0 +1,259 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConfigReader } from '@backstage/config'; +import { + getDatasourceScheduleConfigPath, + getDatasourceThresholdsConfigPath, + getMetricLocalConfigKey, + getMetricThresholdsConfigPath, + getProviderLocalConfigKey, + getProviderScheduleConfigPath, + getProviderThresholdsConfigPath, + resolveScheduleFromConfig, + resolveThresholdsConfigPath, +} from './metricProviderConfigKeys'; + +describe('metricProviderConfigKeys', () => { + describe('getProviderLocalConfigKey', () => { + it('returns datasource when provider ID equals datasource', () => { + expect(getProviderLocalConfigKey('filecheck', 'filecheck')).toBe( + 'filecheck', + ); + }); + + it('returns local name for prefixed provider ID', () => { + expect(getProviderLocalConfigKey('jira.openIssues', 'jira')).toBe( + 'openIssues', + ); + }); + }); + + describe('getMetricLocalConfigKey', () => { + it('returns local name for prefixed metric ID', () => { + expect(getMetricLocalConfigKey('filecheck.readme', 'filecheck')).toBe( + 'readme', + ); + }); + }); + + describe('threshold config paths', () => { + it('builds datasource-level path', () => { + expect(getDatasourceThresholdsConfigPath('github')).toBe( + 'scorecard.plugins.github.thresholds', + ); + }); + + it('builds provider-level path', () => { + expect(getProviderThresholdsConfigPath('github', 'github.openPRs')).toBe( + 'scorecard.plugins.github.metricProviders.openPRs.thresholds', + ); + expect(getProviderThresholdsConfigPath('filecheck', 'filecheck')).toBe( + 'scorecard.plugins.filecheck.metricProviders.filecheck.thresholds', + ); + }); + + it('builds metric-level path for single-metric provider', () => { + expect( + getMetricThresholdsConfigPath( + 'github', + 'github.openPRs', + 'github.openPRs', + ), + ).toBe( + 'scorecard.plugins.github.metricProviders.openPRs.metrics.openPRs.thresholds', + ); + }); + + it('builds metric-level path for batch provider', () => { + expect( + getMetricThresholdsConfigPath( + 'filecheck', + 'filecheck', + 'filecheck.readme', + ), + ).toBe( + 'scorecard.plugins.filecheck.metricProviders.filecheck.metrics.readme.thresholds', + ); + }); + }); + + describe('schedule config paths', () => { + it('builds datasource- and provider-level schedule paths', () => { + expect(getDatasourceScheduleConfigPath('github')).toBe( + 'scorecard.plugins.github.schedule', + ); + expect(getProviderScheduleConfigPath('github', 'github.openPRs')).toBe( + 'scorecard.plugins.github.metricProviders.openPRs.schedule', + ); + }); + + it('prefers provider schedule when present', () => { + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + schedule: { + frequency: { hours: 1 }, + timeout: { minutes: 15 }, + }, + metricProviders: { + openPRs: { + schedule: { + frequency: { hours: 2 }, + timeout: { minutes: 20 }, + }, + }, + }, + }, + }, + }, + }); + + expect( + resolveScheduleFromConfig(config, 'github', 'github.openPRs'), + ).toEqual({ + frequency: { hours: 2 }, + timeout: { minutes: 20 }, + }); + }); + + it('falls back to datasource schedule when provider schedule is absent', () => { + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + schedule: { + frequency: { hours: 1 }, + timeout: { minutes: 15 }, + }, + }, + }, + }, + }); + + expect( + resolveScheduleFromConfig(config, 'github', 'github.openPRs'), + ).toEqual({ + frequency: { hours: 1 }, + timeout: { minutes: 15 }, + }); + }); + + it('returns undefined when no schedule is configured', () => { + expect( + resolveScheduleFromConfig( + new ConfigReader({}), + 'github', + 'github.openPRs', + ), + ).toBeUndefined(); + }); + }); + + describe('resolveThresholdsConfigPath', () => { + it('prefers metric over provider and datasource', () => { + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + thresholds: { rules: [] }, + metricProviders: { + openPRs: { + thresholds: { rules: [] }, + metrics: { + openPRs: { + thresholds: { rules: [] }, + }, + }, + }, + }, + }, + }, + }, + }); + + expect( + resolveThresholdsConfigPath( + config, + 'github', + 'github.openPRs', + 'github.openPRs', + ), + ).toBe( + 'scorecard.plugins.github.metricProviders.openPRs.metrics.openPRs.thresholds', + ); + }); + + it('prefers provider over datasource when metric is absent', () => { + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + thresholds: { rules: [] }, + metricProviders: { + openPRs: { + thresholds: { rules: [] }, + }, + }, + }, + }, + }, + }); + + expect( + resolveThresholdsConfigPath( + config, + 'github', + 'github.openPRs', + 'github.openPRs', + ), + ).toBe('scorecard.plugins.github.metricProviders.openPRs.thresholds'); + }); + + it('falls back to datasource when provider and metric are absent', () => { + const config = new ConfigReader({ + scorecard: { + plugins: { + github: { + thresholds: { rules: [] }, + }, + }, + }, + }); + + expect( + resolveThresholdsConfigPath( + config, + 'github', + 'github.openPRs', + 'github.openPRs', + ), + ).toBe('scorecard.plugins.github.thresholds'); + }); + + it('returns undefined when no thresholds are configured', () => { + expect( + resolveThresholdsConfigPath( + new ConfigReader({}), + 'github', + 'github.openPRs', + 'github.openPRs', + ), + ).toBeUndefined(); + }); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts new file mode 100644 index 00000000000..1272487f324 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts @@ -0,0 +1,136 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Config } from '@backstage/config'; +import { + readSchedulerServiceTaskScheduleDefinitionFromConfig, + type SchedulerServiceTaskScheduleDefinition, +} from '@backstage/backend-plugin-api'; + +/** + * Local config key for a provider under `scorecard.plugins..metricProviders.`. + * Provider/metric IDs are validated at registry startup. + */ +export function getProviderLocalConfigKey( + providerId: string, + datasourceId: string, +): string { + return providerId === datasourceId + ? providerId + : providerId.slice(datasourceId.length + 1); +} + +/** + * Local config key for a metric under + * `scorecard.plugins..metricProviders..metrics.`. + * Provider/metric IDs are validated at registry startup. + */ +export function getMetricLocalConfigKey( + metricId: string, + datasourceId: string, +): string { + return metricId.slice(datasourceId.length + 1); +} + +/** Get datasource thresholds config key under `scorecard.plugins..thresholds` */ +export function getDatasourceThresholdsConfigPath( + datasourceId: string, +): string { + return `scorecard.plugins.${datasourceId}.thresholds`; +} + +/** Get provider thresholds config key under + * `scorecard.plugins..metricProviders..thresholds` + */ +export function getProviderThresholdsConfigPath( + datasourceId: string, + providerId: string, +): string { + const providerKey = getProviderLocalConfigKey(providerId, datasourceId); + return `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}.thresholds`; +} + +/** Get metric thresholds config key under + * `scorecard.plugins..metricProviders..metrics..thresholds` + */ +export function getMetricThresholdsConfigPath( + datasourceId: string, + providerId: string, + metricId: string, +): string { + const providerKey = getProviderLocalConfigKey(providerId, datasourceId); + const metricKey = getMetricLocalConfigKey(metricId, datasourceId); + return ( + `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}` + + `.metrics.${metricKey}.thresholds` + ); +} + +/** Get datasource schedule config key under `scorecard.plugins..schedule` */ +export function getDatasourceScheduleConfigPath(datasourceId: string): string { + return `scorecard.plugins.${datasourceId}.schedule`; +} + +/** Get provider schedule config key under `scorecard.plugins..metricProviders..schedule` */ +export function getProviderScheduleConfigPath( + datasourceId: string, + providerId: string, +): string { + const providerKey = getProviderLocalConfigKey(providerId, datasourceId); + return `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}.schedule`; +} + +/** + * Resolves the schedule from config, preferring provider-level over datasource-level. + * Returns undefined when neither is set. + */ +export function resolveScheduleFromConfig( + config: Config, + datasourceId: string, + providerId: string, +): SchedulerServiceTaskScheduleDefinition | undefined { + const schedulePath = [ + getProviderScheduleConfigPath(datasourceId, providerId), + getDatasourceScheduleConfigPath(datasourceId), + ].find(path => config.has(path)); + + if (!schedulePath) { + return undefined; + } + + return readSchedulerServiceTaskScheduleDefinitionFromConfig( + config.getConfig(schedulePath), + ); +} + +/** + * Resolves the thresholds config path for a metric. + * Most specific wins: metric > provider > datasource. + * Returns undefined when no thresholds are set. + */ +export function resolveThresholdsConfigPath( + config: Config, + datasourceId: string, + providerId: string, + metricId: string, +): string | undefined { + const paths = [ + getMetricThresholdsConfigPath(datasourceId, providerId, metricId), + getProviderThresholdsConfigPath(datasourceId, providerId), + getDatasourceThresholdsConfigPath(datasourceId), + ]; + return paths.find(path => config.has(path)); +} diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts new file mode 100644 index 00000000000..e074917657d --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts @@ -0,0 +1,67 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + validateMetricId, + validateProviderId, +} from './validateMetricProviderIds'; + +describe('validateMetricProviderIds', () => { + describe('validateProviderId', () => { + it('allows provider ID equal to datasource', () => { + expect(() => validateProviderId('filecheck', 'filecheck')).not.toThrow(); + }); + + it('allows provider ID with datasource prefix', () => { + expect(() => + validateProviderId('github.openPRs', 'github'), + ).not.toThrow(); + }); + + it.each([ + ['wrong datasource prefix', 'other.openPRs', 'github'], + ['missing dot separator', 'githubopenPrs', 'github'], + ['empty ID', '', 'github'], + ['empty provider name', 'github.', 'github'], + ['too many segments', 'github.foo.bar', 'github'], + ] as const)('rejects %s (%s)', (_desc, providerId, datasourceId) => { + expect(() => validateProviderId(providerId, datasourceId)).toThrow( + /Invalid provider ID/, + ); + }); + }); + + describe('validateMetricId', () => { + it('allows metric ID with datasource prefix', () => { + expect(() => validateMetricId('github.openPRs', 'github')).not.toThrow(); + expect(() => + validateMetricId('filecheck.readme', 'filecheck'), + ).not.toThrow(); + }); + + it.each([ + ['equal to datasource', 'filecheck', 'filecheck'], + ['wrong datasource prefix', 'other.metric', 'github'], + ['empty metric name', 'github.', 'github'], + ['empty ID', '', 'github'], + ['too many segments', 'github.openPRs.other', 'github'], + ] as const)('rejects %s (%s)', (_desc, metricId, datasourceId) => { + expect(() => validateMetricId(metricId, datasourceId)).toThrow( + /Invalid metric ID/, + ); + }); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts new file mode 100644 index 00000000000..e76066d4891 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts @@ -0,0 +1,48 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Validates provider ID format: `` or `.`. + */ +export function validateProviderId( + providerId: string, + datasourceId: string, +): void { + if (providerId === datasourceId) { + return; + } + + const [datasource, providerName, ...rest] = providerId.split('.'); + if (datasource !== datasourceId || !providerName || rest.length > 0) { + throw new Error( + `Invalid provider ID '${providerId}', must be equal to datasource '${datasourceId}' ` + + `or have format '${datasourceId}.' where provider name is not empty`, + ); + } +} + +/** + * Validates metric ID format: must be `.`. + */ +export function validateMetricId(metricId: string, datasourceId: string): void { + const [datasource, metricName, ...rest] = metricId.split('.'); + if (datasource !== datasourceId || !metricName || rest.length > 0) { + throw new Error( + `Invalid metric ID '${metricId}', must have format ` + + `'${datasourceId}.' where metric name is not empty`, + ); + } +} diff --git a/workspaces/scorecard/plugins/scorecard-common/src/types/threshold.ts b/workspaces/scorecard/plugins/scorecard-common/src/types/threshold.ts index f71fab1968b..d06dde275c3 100644 --- a/workspaces/scorecard/plugins/scorecard-common/src/types/threshold.ts +++ b/workspaces/scorecard/plugins/scorecard-common/src/types/threshold.ts @@ -55,7 +55,7 @@ export type ThresholdRule = { }; /** - * Threshold configuration + * Threshold configuration that categorizes metric values. * @public */ export type ThresholdConfig = { From 477650e69c38e27b59feed26658e150818776bfc Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Tue, 28 Jul 2026 10:34:50 +0200 Subject: [PATCH 2/5] add commin Signed-off-by: Dominika Zemanovicova --- .../scorecard/.changeset/metric-level-thresholds-config.md | 1 + 1 file changed, 1 insertion(+) diff --git a/workspaces/scorecard/.changeset/metric-level-thresholds-config.md b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md index f2c96c52f0f..28c1f5fbe9e 100644 --- a/workspaces/scorecard/.changeset/metric-level-thresholds-config.md +++ b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md @@ -6,6 +6,7 @@ '@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-sonarqube': major '@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-dependabot': major '@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-openssf': major +'@red-hat-developer-hub/backstage-plugin-scorecard-common': major --- **BREAKING**: Thresholds moved from provider level to metric level. Configuration is restructured to enable thresholds to be defined directly for each metric with fallback options at the datasource and provider levels. The new top-level key `metricProviders` now houses all metric-specific configurations. Schedule options are added to datasource level. Threshold annotation overrides now require full metric ID instead of provider ID. From 8ea31890bad0ce2d7e093a2587d0477b0c33859f Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Tue, 28 Jul 2026 11:53:15 +0200 Subject: [PATCH 3/5] Remove unused code Signed-off-by: Dominika Zemanovicova --- .../mockMetricProvidersRegistry.ts | 2 - .../providers/MetricProvidersRegistry.test.ts | 105 ------------------ .../src/providers/MetricProvidersRegistry.ts | 38 +------ .../tasks/PullMetricsByProviderTask.ts | 6 +- 4 files changed, 2 insertions(+), 149 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts index 6d8fad1bd89..96a20e0d717 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockMetricProvidersRegistry.ts @@ -22,8 +22,6 @@ export const mockMetricProvidersRegistry = { register: jest.fn(), getProvider: jest.fn(), getMetric: jest.fn(), - calculateMetric: jest.fn(), - calculateMetrics: jest.fn(), listProviders: jest.fn().mockReturnValue([]), listMetrics: jest.fn().mockReturnValue([]), listMetricsByDatasource: jest.fn().mockReturnValue([]), diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts index 67afae16483..ac507427480 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts @@ -26,7 +26,6 @@ import { filecheckBatchProvider, filecheckBatchMetrics, } from '../../__fixtures__/mockProviders'; -import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; import { validateMetricId, validateProviderId, @@ -44,8 +43,6 @@ jest.mock('../validation/validateMetricProviderIds', () => { describe('MetricProvidersRegistry', () => { let registry: MetricProvidersRegistry; - const mockEntity = new MockEntityBuilder().build(); - beforeEach(() => { registry = new MetricProvidersRegistry(); jest.mocked(validateProviderId).mockClear(); @@ -272,108 +269,6 @@ describe('MetricProvidersRegistry', () => { }); }); - describe('calculateMetric', () => { - it('should calculate metric for registered provider', async () => { - registry.register(githubNumberProvider); - - const result = await registry.calculateMetric( - 'github.numberMetric', - mockEntity, - ); - - expect(result).toBe(42); - }); - - it('should throw NotFoundError for unregistered provider', async () => { - await expect( - registry.calculateMetric('nonExistent', mockEntity), - ).rejects.toThrow( - new NotFoundError( - "No metric provider registered for metric ID 'nonExistent'.", - ), - ); - }); - }); - - describe('calculateMetrics', () => { - it('should handle empty provider IDs array', async () => { - const results = await registry.calculateMetrics([], mockEntity); - - expect(results).toEqual([]); - }); - - it('should calculate metrics for multiple registered providers', async () => { - registry.register(githubNumberProvider); - registry.register(jiraBooleanProvider); - - const results = await registry.calculateMetrics( - ['github.numberMetric', 'jira.booleanMetric'], - mockEntity, - ); - - expect(results).toHaveLength(2); - expect(results[0]).toEqual({ - metricId: 'github.numberMetric', - value: 42, - }); - expect(results[1]).toEqual({ - metricId: 'jira.booleanMetric', - value: false, - }); - }); - - it('should calculate metrics for only specified providers', async () => { - registry.register(githubNumberProvider); - registry.register( - new MockNumberProvider( - 'github.openIssues', - 'github', - 'GitHub Open Issues', - 'Github Open Issues description', - 10, - ), - ); - registry.register(jiraBooleanProvider); - - const results = await registry.calculateMetrics( - ['github.numberMetric', 'github.openIssues'], - mockEntity, - ); - - expect(results).toHaveLength(2); - expect(results[0]).toEqual({ - metricId: 'github.numberMetric', - value: 42, - }); - expect(results[1]).toEqual({ - metricId: 'github.openIssues', - value: 10, - }); - }); - - it('should handle mix of successful and failed metric calculations', async () => { - registry.register(githubNumberProvider); - - const results = await registry.calculateMetrics( - ['github.numberMetric', 'nonExistent'], - mockEntity, - ); - - expect(results).toHaveLength(2); - expect(results[0]).toEqual({ - metricId: 'github.numberMetric', - value: 42, - }); - expect(results[1]).toEqual({ - metricId: 'nonExistent', - error: expect.any(NotFoundError), - }); - expect(results[1].error?.message).toBe( - "No metric provider registered for metric ID 'nonExistent'.", - ); - }); - }); - describe('listProviders', () => { it('should return empty array when no providers registered', () => { const providers = registry.listProviders(); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts index 426c1dbd4be..175427a8232 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts @@ -14,12 +14,8 @@ * limitations under the License. */ -import type { Entity } from '@backstage/catalog-model'; import { ConflictError, NotFoundError } from '@backstage/errors'; -import { - Metric, - MetricValue, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { MetricProvider, ThresholdConfigFormatError, @@ -119,38 +115,6 @@ export class MetricProvidersRegistry { ); } - async calculateMetric( - metricId: string, - entity: Entity, - ): Promise { - const provider = this.getProvider(metricId); - const results = await provider.calculateMetrics(entity); - const value = results.get(metricId); - if (value === undefined) { - throw new Error( - `Provider '${provider.getProviderId()}' did not return a value for metric '${metricId}'`, - ); - } - return value; - } - - async calculateMetrics( - metricIds: string[], - entity: Entity, - ): Promise<{ metricId: string; value?: MetricValue; error?: Error }[]> { - const results = await Promise.allSettled( - metricIds.map(metricId => this.calculateMetric(metricId, entity)), - ); - - return results.map((result, index) => { - const metricId = metricIds[index]; - if (result.status === 'fulfilled') { - return { metricId, value: result.value }; - } - return { metricId, error: result.reason as Error }; - }); - } - listProviders(): MetricProvider[] { // Deduplicate providers since batch providers are stored under multiple metric IDs return [...new Set(this.metricProviders.values())]; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts index bb122ad8bf7..1ba0434b024 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -235,11 +235,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { ).then(promises => promises.reduce((acc, curr) => { if (curr.status === 'fulfilled' && curr.value !== undefined) { - const result = curr.value; - if (Array.isArray(result)) { - return [...acc, ...result]; - } - return [...acc, result]; + return [...acc, ...curr.value]; } return acc; }, [] as DbMetricValueCreate[]), From 2183cbb45a3c8e41b17a6fa6d76d4e6af5cefcac Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Tue, 28 Jul 2026 17:27:26 +0200 Subject: [PATCH 4/5] Rename plugins key to metricProviders and update filecheck provider id Assisted-By: Cursor Desktop Signed-off-by: Dominika Zemanovicova --- .../metric-level-thresholds-config.md | 57 ++-- .../scorecard/app-config.local.EXAMPLE.yaml | 47 ++-- .../scorecard/app-config.production.yaml | 13 +- workspaces/scorecard/app-config.yaml | 40 +-- .../README.md | 71 ++--- .../config.d.ts | 29 +- .../src/metricProviders/FilecheckConfig.ts | 2 +- .../FilecheckMetricProvider.test.ts | 170 ++++-------- .../FilecheckMetricProvider.ts | 2 +- .../FilecheckMetricProviderFactory.ts | 2 +- .../config.d.ts | 16 +- .../__fixtures__/testUtils.ts | 10 +- .../scorecard-backend-module-jira/config.d.ts | 22 +- .../src/constants/jiraOpenIssues.ts | 2 +- .../__fixtures__/mockProviders.ts | 10 +- .../plugins/scorecard-backend/config.d.ts | 59 ++--- .../scorecard-backend/docs/providers.md | 8 +- .../scorecard-backend/docs/thresholds.md | 71 ++--- .../tasks/PullMetricsByProviderTask.test.ts | 18 +- .../src/service/router.test.ts | 6 +- .../src/threshold/ThresholdResolver.test.ts | 250 +++++++----------- .../utils/metricProviderConfigKeys.test.ts | 139 ++-------- .../src/utils/metricProviderConfigKeys.ts | 50 ++-- .../validateMetricProviderIds.test.ts | 8 +- .../validation/validateMetricProviderIds.ts | 10 +- 25 files changed, 438 insertions(+), 674 deletions(-) diff --git a/workspaces/scorecard/.changeset/metric-level-thresholds-config.md b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md index 28c1f5fbe9e..afe73ab983d 100644 --- a/workspaces/scorecard/.changeset/metric-level-thresholds-config.md +++ b/workspaces/scorecard/.changeset/metric-level-thresholds-config.md @@ -9,37 +9,46 @@ '@red-hat-developer-hub/backstage-plugin-scorecard-common': major --- -**BREAKING**: Thresholds moved from provider level to metric level. Configuration is restructured to enable thresholds to be defined directly for each metric with fallback options at the datasource and provider levels. The new top-level key `metricProviders` now houses all metric-specific configurations. Schedule options are added to datasource level. Threshold annotation overrides now require full metric ID instead of provider ID. +**BREAKING**: Scorecard provider configuration now lives under top-level `scorecard.metricProviders` instead of `scorecard.plugins`. Provider IDs must be `.` (no longer equal to the datasource alone). Entity annotations for thresholds use now the full metric ID instead of provider ID. -Thresholds from configuration are determined by the most specific setting. Priority order is: Metric > MetricProvider > Datasource: +Thresholds from configuration are determined by the most specific setting (**metric > provider**): -1. `plugins..metricProviders..metrics..thresholds` -2. `plugins..metricProviders..thresholds` -3. `plugins..thresholds` +1. `metricProviders...metrics..thresholds` +2. `metricProviders...thresholds` -Config keys are local names (no datasource prefix). Single-metric plugins can set thresholds directly under the datasource. Schedules use `plugins..schedule` with optional override at `metricProviders..schedule`. +Config keys are local names (no datasource prefix). Entity annotations use the full metric ID: +`scorecard.io/.thresholds.rules.`. -Entity annotations must use the full metric ID: -`scorecard.io/.thresholds.rules.` (for example -`scorecard.io/filecheck.readme.thresholds.rules.success`). +Filecheck provider ID is now `filecheck.fileExistence`; files move under `options`: -If you customized thresholds or schedule under a metric provider, nest that provider under `metricProviders`: +```diff + scorecard: +- plugins: +- filecheck: +- files: +- license: LICENSE +- codeowners: CODEOWNERS +- thresholds: ... +- schedule: ... ++ metricProviders: ++ filecheck: ++ fileExistence: ++ options: ++ files: ++ license: LICENSE ++ codeowners: CODEOWNERS ++ thresholds: ... ++ schedule: ... +``` + +Migration from the previous `scorecard.plugins` layout: ```diff scorecard: - plugins: +- plugins: ++ metricProviders: github: -- openPRs: -+ metricProviders: -+ openPRs: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '10-50' - - key: error - expression: '>50' + openPRs: + schedule: ... + thresholds: ... ``` - -Apply the same nesting for other datasources (`jira`, `sonarqube`, `dependabot`, `openssf`, etc.): move `plugins..` to `plugins..metricProviders.`. diff --git a/workspaces/scorecard/app-config.local.EXAMPLE.yaml b/workspaces/scorecard/app-config.local.EXAMPLE.yaml index 185e6c37c26..bb7be12a66a 100644 --- a/workspaces/scorecard/app-config.local.EXAMPLE.yaml +++ b/workspaces/scorecard/app-config.local.EXAMPLE.yaml @@ -77,30 +77,27 @@ scorecard: - key: error expression: '<51' color: '#be1ec7' # purple - plugins: + metricProviders: jira: - metricProviders: - openIssues: - schedule: - frequency: { days: 1 } - timeout: { minutes: 2 } - initialDelay: { minutes: 1 } - # Optional — uncomment to narrow which Jira issues are counted (otherwise all open issues match the provider rules). - # options: - # mandatoryFilter: type = Story - # customFilter: priority = High + openIssues: + schedule: + frequency: { days: 1 } + timeout: { minutes: 2 } + initialDelay: { minutes: 1 } + # Optional — uncomment to narrow which Jira issues are counted (otherwise all open issues match the provider rules). + # options: + # mandatoryFilter: type = Story + # customFilter: priority = High github: - # Provider-level thresholds, could be also set under `metricProviders.openPRs.metrics.openPRs.thresholds`. - metricProviders: - openPRs: - thresholds: - rules: - - key: success - expression: '<=20' - color: 'success.main' - - key: warning - expression: '20-50' - color: '#FFFF00' - - key: error - expression: '>50' - color: 'rgb(255, 0, 0)' + openPRs: + thresholds: + rules: + - key: success + expression: '<=20' + color: 'success.main' + - key: warning + expression: '20-50' + color: '#FFFF00' + - key: error + expression: '>50' + color: 'rgb(255, 0, 0)' diff --git a/workspaces/scorecard/app-config.production.yaml b/workspaces/scorecard/app-config.production.yaml index fbe41d10df3..bf77286ed7d 100644 --- a/workspaces/scorecard/app-config.production.yaml +++ b/workspaces/scorecard/app-config.production.yaml @@ -61,11 +61,10 @@ catalog: # description: Open PRs across owned entities, grouped by status. # type: statusGrouped # metricId: github.openPRs -# plugins: +# metricProviders: # github: -# metricProviders: -# openPRs: -# schedule: -# frequency: { hours: 1 } -# timeout: { minutes: 15 } -# initialDelay: { minutes: 1 } +# openPRs: +# schedule: +# frequency: { hours: 1 } +# timeout: { minutes: 15 } +# initialDelay: { minutes: 1 } diff --git a/workspaces/scorecard/app-config.yaml b/workspaces/scorecard/app-config.yaml index 49ea48f2896..503d39261e8 100644 --- a/workspaces/scorecard/app-config.yaml +++ b/workspaces/scorecard/app-config.yaml @@ -274,26 +274,26 @@ scorecard: type: statusGrouped description: This KPI is provide information about whether the license file exists in the repository. metricId: filecheck.license - plugins: + metricProviders: jira: - metricProviders: - openIssues: - schedule: - frequency: { minutes: 5 } - timeout: { minutes: 10 } - initialDelay: { seconds: 10 } + openIssues: + schedule: + frequency: { minutes: 5 } + timeout: { minutes: 10 } + initialDelay: { seconds: 10 } github: - metricProviders: - openPRs: - schedule: - frequency: { minutes: 5 } - timeout: { minutes: 10 } - initialDelay: { seconds: 10 } + openPRs: + schedule: + frequency: { minutes: 5 } + timeout: { minutes: 10 } + initialDelay: { seconds: 10 } filecheck: - files: - license: 'LICENSE' - codeowners: 'CODEOWNERS' - schedule: - frequency: { minutes: 5 } - timeout: { minutes: 10 } - initialDelay: { seconds: 10 } + fileExistence: + options: + files: + license: 'LICENSE' + codeowners: 'CODEOWNERS' + schedule: + frequency: { minutes: 5 } + timeout: { minutes: 10 } + initialDelay: { seconds: 10 } diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md index 8bdfbacb082..76735ce587b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md @@ -43,28 +43,30 @@ backend.start(); ### Files Configuration -Define which files to check under `scorecard.plugins.filecheck.files` in your `app-config.yaml`. Keys become the metric identifier suffix and values are relative file paths inside the repository: +Define which files to check under `scorecard.metricProviders.filecheck.fileExistence.options.files` in your `app-config.yaml`. Keys become the metric identifier suffix and values are relative file paths inside the repository: ```yaml # app-config.yaml scorecard: - plugins: + metricProviders: filecheck: - files: - readme: README.md - license: LICENSE - codeowners: CODEOWNERS - contributing: CONTRIBUTING.md + fileExistence: + options: + files: + readme: README.md + license: LICENSE + codeowners: CODEOWNERS + contributing: CONTRIBUTING.md ``` This produces the following metrics: -| Metric ID | Provider ID | Checked file path | -| ------------------------ | ----------- | ----------------- | -| `filecheck.readme` | `filecheck` | `README.md` | -| `filecheck.license` | `filecheck` | `LICENSE` | -| `filecheck.codeowners` | `filecheck` | `CODEOWNERS` | -| `filecheck.contributing` | `filecheck` | `CONTRIBUTING.md` | +| Metric ID | Provider ID | Checked file path | +| ------------------------ | ------------------------- | ----------------- | +| `filecheck.readme` | `filecheck.fileExistence` | `README.md` | +| `filecheck.license` | `filecheck.fileExistence` | `LICENSE` | +| `filecheck.codeowners` | `filecheck.fileExistence` | `CODEOWNERS` | +| `filecheck.contributing` | `filecheck.fileExistence` | `CONTRIBUTING.md` | If no files are configured, no metrics are registered and the module has no effect. @@ -91,28 +93,30 @@ metadata: Each configured file produces one boolean metric. - **Metric ID**: `filecheck.` (where `` is the key from the `files` config) +- **Provider ID**: `filecheck.fileExistence` - **Type**: Boolean - **Datasource**: `filecheck` ## Default thresholds -All configured file checks share the same default thresholds. Default thresholds for `filecheck` (applies to every `filecheck.` metric): +All configured file checks share the same default thresholds. Provider-level thresholds for `filecheck.fileExistence` (applies to every `filecheck.` metric): ```yaml # app-config.yaml scorecard: - plugins: + metricProviders: filecheck: - thresholds: - rules: - - key: exist - expression: '==true' - icon: scorecardSuccessStatusIcon - color: 'success.main' - - key: missing - expression: '==false' - icon: scorecardErrorStatusIcon - color: 'error.main' + fileExistence: + thresholds: + rules: + - key: exist + expression: '==true' + icon: scorecardSuccessStatusIcon + color: 'success.main' + - key: missing + expression: '==false' + icon: scorecardErrorStatusIcon + color: 'error.main' ``` Custom threshold keys other than `success`, `warning`, or `error` must include `color` and `icon` in app-config. @@ -125,15 +129,16 @@ The Scorecard plugin uses Backstage's built-in scheduler service to automaticall ```yaml scorecard: - plugins: + metricProviders: filecheck: - schedule: - frequency: - cron: '0 6 * * *' - timeout: - minutes: 5 - initialDelay: - seconds: 5 + fileExistence: + schedule: + frequency: + cron: '0 6 * * *' + timeout: + minutes: 5 + initialDelay: + seconds: 5 ``` The schedule configuration follows Backstage's `SchedulerServiceTaskScheduleDefinitionConfig` [schema](https://github.com/backstage/backstage/blob/master/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts#L157). For more details on how to configure schedule, see [Metric Collection Scheduling](../scorecard-backend/docs/providers.md#metric-collection-scheduling). diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts index 21818bafa19..37ed64177e6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/config.d.ts @@ -20,18 +20,29 @@ import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scoreca export interface Config { /** Configuration for scorecard plugin */ scorecard?: { - /** Configuration for scorecard plugins/datasources */ - plugins?: { + /** Metric providers calculate one or more metrics on a schedule. */ + metricProviders?: { /** File existence check configuration */ filecheck?: { - /** File existence checks configuration. Keys are metric identifier suffixes, values are relative file paths. */ - files?: { - [metricName: string]: string; + fileExistence?: { + /** How often filecheck.fileExistence metrics will be calculated */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How filecheck metric values are categorized */ + thresholds?: ThresholdConfig; + /** Provider-specific options */ + options?: { + /** Keys are metric identifier suffixes, values are relative file paths. */ + files?: { + [metricName: string]: string; + }; + }; + /** Per-metric configuration. Keys are local metric names (no datasource prefix). */ + metrics?: { + [metricName: string]: { + thresholds?: ThresholdConfig; + }; + }; }; - /** How often filecheck metrics will be calculated. */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; - /** How filecheck metric values are categorized. */ - thresholds?: ThresholdConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckConfig.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckConfig.ts index 12ef0339ef0..0aa1129691c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckConfig.ts @@ -69,7 +69,7 @@ export function parseFilecheckConfig( config: Config, ): FilecheckConfig | undefined { const filesConfig = config.getOptionalConfig( - 'scorecard.plugins.filecheck.files', + 'scorecard.metricProviders.filecheck.fileExistence.options.files', ); if (!filesConfig) { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.test.ts index 3e73caaa0e6..b22dbc05e1f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.test.ts @@ -24,6 +24,22 @@ import { NotModifiedError } from '@backstage/errors'; import { DEFAULT_FILECHECK_THRESHOLDS } from './FilecheckConfig'; import { createFilecheckMetricProvider } from './FilecheckMetricProviderFactory'; +function filecheckConfig(files: Record) { + return { + scorecard: { + metricProviders: { + filecheck: { + fileExistence: { + options: { + files, + }, + }, + }, + }, + }, + }; +} + function createMockCacheService(): jest.Mocked { const store = new Map(); return { @@ -108,9 +124,7 @@ describe('FilecheckMetricProvider', () => { it('should return undefined when files object is empty', () => { const provider = createFilecheckMetricProvider( - new ConfigReader({ - scorecard: { plugins: { filecheck: { files: {} } } }, - }), + new ConfigReader(filecheckConfig({})), mockUrlReader, mockCacheService, ); @@ -118,15 +132,9 @@ describe('FilecheckMetricProvider', () => { }); it('should create provider with files configuration', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { readme: 'README.md', license: 'LICENSE' }, - }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ readme: 'README.md', license: 'LICENSE' }), + ); const provider = createFilecheckMetricProvider( config, @@ -142,13 +150,9 @@ describe('FilecheckMetricProvider', () => { }); it('should throw error when file path contains a double quote', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { files: { bad: 'path/with"quote.txt' } }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ bad: 'path/with"quote.txt' }), + ); expect(() => createFilecheckMetricProvider(config, mockUrlReader, mockCacheService), @@ -158,13 +162,9 @@ describe('FilecheckMetricProvider', () => { }); it('should throw error when file path contains a newline', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { files: { bad: 'path/with\nnewline' } }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ bad: 'path/with\nnewline' }), + ); expect(() => createFilecheckMetricProvider(config, mockUrlReader, mockCacheService), @@ -174,15 +174,9 @@ describe('FilecheckMetricProvider', () => { }); it('should throw error when file path contains a backslash', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { bad: String.raw`path\file.txt` }, - }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ bad: String.raw`path\file.txt` }), + ); expect(() => createFilecheckMetricProvider(config, mockUrlReader, mockCacheService), @@ -192,13 +186,9 @@ describe('FilecheckMetricProvider', () => { }); it('should throw error when file path starts with /', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { files: { bad: '/absolute/path.txt' } }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ bad: '/absolute/path.txt' }), + ); expect(() => createFilecheckMetricProvider(config, mockUrlReader, mockCacheService), @@ -208,13 +198,9 @@ describe('FilecheckMetricProvider', () => { }); it('should throw error when file path starts with ./', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { files: { bad: './relative/path.txt' } }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ bad: './relative/path.txt' }), + ); expect(() => createFilecheckMetricProvider(config, mockUrlReader, mockCacheService), @@ -229,25 +215,19 @@ describe('FilecheckMetricProvider', () => { const mockCacheService = createMockCacheService(); const provider = createFilecheckMetricProvider( - new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { - readme: 'README.md', - codeowners: 'CODEOWNERS', - dockerfile: 'Dockerfile', - }, - }, - }, - }, - }), + new ConfigReader( + filecheckConfig({ + readme: 'README.md', + codeowners: 'CODEOWNERS', + dockerfile: 'Dockerfile', + }), + ), mockUrlReader, mockCacheService, ); it('should return correct provider ID', () => { - expect(provider?.getProviderId()).toBe('filecheck'); + expect(provider?.getProviderId()).toBe('filecheck.fileExistence'); }); it('should return correct datasource ID', () => { @@ -302,15 +282,9 @@ describe('FilecheckMetricProvider', () => { const existingFiles = new Set(['README.md']); const mockUrlReader = createMockUrlReader(existingFiles); - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { readme: 'README.md', license: 'LICENSE' }, - }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ readme: 'README.md', license: 'LICENSE' }), + ); const provider = createFilecheckMetricProvider( config, mockUrlReader, @@ -327,20 +301,14 @@ describe('FilecheckMetricProvider', () => { const existingFiles = new Set(['README.md', 'LICENSE', 'Dockerfile']); const mockUrlReader = createMockUrlReader(existingFiles); - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { - readme: 'README.md', - license: 'LICENSE', - codeowners: 'CODEOWNERS', - dockerfile: 'Dockerfile', - }, - }, - }, - }, - }); + const config = new ConfigReader( + filecheckConfig({ + readme: 'README.md', + license: 'LICENSE', + codeowners: 'CODEOWNERS', + dockerfile: 'Dockerfile', + }), + ); const provider = createFilecheckMetricProvider( config, mockUrlReader, @@ -363,15 +331,7 @@ describe('FilecheckMetricProvider', () => { search: jest.fn(), } as unknown as jest.Mocked; - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { readme: 'README.md' }, - }, - }, - }, - }); + const config = new ConfigReader(filecheckConfig({ readme: 'README.md' })); const provider = createFilecheckMetricProvider( config, mockUrlReader, @@ -395,15 +355,7 @@ describe('FilecheckMetricProvider', () => { const existingFiles = new Set(['README.md']); const mockUrlReader = createMockUrlReader(existingFiles); - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { readme: 'README.md' }, - }, - }, - }, - }); + const config = new ConfigReader(filecheckConfig({ readme: 'README.md' })); const provider = createFilecheckMetricProvider( config, mockUrlReader, @@ -423,15 +375,7 @@ describe('FilecheckMetricProvider', () => { const existingFiles = new Set(['README.md']); const mockUrlReader = createMockUrlReader(existingFiles); - const config = new ConfigReader({ - scorecard: { - plugins: { - filecheck: { - files: { readme: 'README.md' }, - }, - }, - }, - }); + const config = new ConfigReader(filecheckConfig({ readme: 'README.md' })); const sharedCache = createMockCacheService(); const provider = createFilecheckMetricProvider( config, diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.ts index 05887ba1550..64947a4ce4b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.ts @@ -38,7 +38,7 @@ export class FilecheckMetricProvider implements MetricProvider<'boolean'> { } getProviderId(): string { - return 'filecheck'; + return 'filecheck.fileExistence'; } getMetrics(): Metric<'boolean'>[] { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProviderFactory.ts b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProviderFactory.ts index b66d9439cf4..af563370d37 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProviderFactory.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProviderFactory.ts @@ -25,7 +25,7 @@ import { FilecheckMetricProvider } from './FilecheckMetricProvider'; /** * Creates a FilecheckMetricProvider from root Backstage config and services. - * Returns undefined if no files are configured under `scorecard.plugins.filecheck.files`. + * Returns undefined if no files are configured under `scorecard.metricProviders.filecheck.fileExistence.options.files`. */ export function createFilecheckMetricProvider( config: Config, diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts index 8fcfdf0b172..ae0c7e1b17f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-github/config.d.ts @@ -20,17 +20,15 @@ import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scoreca export interface Config { /** Configuration for scorecard plugin */ scorecard?: { - /** Configuration for scorecard plugins/datasources */ - plugins?: { + /** Metric providers calculate one or more metrics on a schedule. */ + metricProviders?: { /** Github datasource configuration */ github?: { - metricProviders?: { - openPRs?: { - /** How often github.openPRs metrics will be calculated */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; - /** How github.openPRs metric values are categorized */ - thresholds?: ThresholdConfig; - }; + openPRs?: { + /** How often github.openPRs metrics will be calculated */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How github.openPRs metric values are categorized */ + thresholds?: ThresholdConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts index 7da1e46cf8f..90e9ab244e9 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/__fixtures__/testUtils.ts @@ -76,13 +76,11 @@ export function newMockRootConfig({ data: { jira, scorecard: { - plugins: { + metricProviders: { jira: { - metricProviders: { - openIssues: { - options, - thresholds, - }, + openIssues: { + options, + thresholds, }, }, }, diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts index 737896fb593..f7334e8590d 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/config.d.ts @@ -35,21 +35,19 @@ export interface Config { }; /** Configuration for scorecard plugin */ scorecard?: { - /** Configuration for scorecard plugins/datasources */ - plugins?: { + /** Metric providers calculate one or more metrics on a schedule. */ + metricProviders?: { /** JIRA datasource configuration */ jira?: { - metricProviders?: { - openIssues?: { - options?: { - mandatoryFilter?: string; - customFilter?: string; - }; - /** How often jira.openIssues metrics will be calculated */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; - /** How jira.openIssues metric values are categorized */ - thresholds?: ThresholdConfig; + openIssues?: { + options?: { + mandatoryFilter?: string; + customFilter?: string; }; + /** How often jira.openIssues metrics will be calculated */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** How jira.openIssues metric values are categorized */ + thresholds?: ThresholdConfig; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts index f2aa77a1cbf..11acc0ab035 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/constants/jiraOpenIssues.ts @@ -15,7 +15,7 @@ */ export const OPEN_ISSUES_CONFIG_PATH = - 'scorecard.plugins.jira.metricProviders.openIssues' as const; + 'scorecard.metricProviders.jira.openIssues' as const; /** * Jira integration configuration path diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts index 3df0e7a4c4f..4d54b1a8fb2 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts @@ -169,7 +169,7 @@ export class MockBatchBooleanProvider implements MetricProvider<'boolean'> { constructor( private readonly datasourceId: string, - private readonly providerIdPrefix: string, + private readonly providerId: string, metricConfigs: Array<{ id: string; path: string }>, ) { this.metricConfigs = metricConfigs; @@ -180,12 +180,12 @@ export class MockBatchBooleanProvider implements MetricProvider<'boolean'> { } getProviderId(): string { - return this.providerIdPrefix; + return this.providerId; } getMetrics(): Metric<'boolean'>[] { return this.metricConfigs.map(c => ({ - id: `${this.providerIdPrefix}.${c.id}`, + id: `${this.datasourceId}.${c.id}`, title: `File: ${c.path}`, description: `Checks if ${c.path} exists.`, type: 'boolean' as const, @@ -200,7 +200,7 @@ export class MockBatchBooleanProvider implements MetricProvider<'boolean'> { async calculateMetrics(_entity: Entity): Promise> { const results = new Map(); for (const config of this.metricConfigs) { - results.set(`${this.providerIdPrefix}.${config.id}`, true); + results.set(`${this.datasourceId}.${config.id}`, true); } return results; } @@ -208,7 +208,7 @@ export class MockBatchBooleanProvider implements MetricProvider<'boolean'> { export const filecheckBatchProvider = new MockBatchBooleanProvider( 'filecheck', - 'filecheck', + 'filecheck.fileExistence', [ { id: 'readme', path: 'README.md' }, { id: 'license', path: 'LICENSE' }, diff --git a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts index f1d2db4e695..c67df5bb402 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts @@ -66,45 +66,36 @@ export interface Config { except?: string[]; }; }; - /** Configuration for scorecard datasources plugins */ - plugins?: { - /** Configuration for datasource (e.g. jira, github, filecheck) */ + /** Metric providers calculate one or more metrics on a schedule. */ + metricProviders?: { + /** Datasource ID, matches `getProviderDatasourceId()` of a provider (e.g., `jira`, `github`, `filecheck`). */ [datasource: string]: { - /** - * How often metrics will be calculated for all metric providers in this - * datasource. Overridden by `metricProviders..schedule` when set. + /** Configuration for a specific metric provider. + * Use the local name without datasource prefix (e.g., `openPRs` instead of `github.openPRs`). */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; - /** - * How metric values are categorized for all metrics in this datasource. - * Overridden by provider- or metric-level thresholds when set. - */ - thresholds?: ThresholdConfig; - /** Configuration for metric providers within the datasource. Keys are local provider names (no datasource prefix). */ - metricProviders?: { - [providerName: string]: { - /** - * How often metrics will be calculated for this provider. - * Overrides the datasource-level schedule when set. - */ - schedule?: SchedulerServiceTaskScheduleDefinitionConfig; - /** - * How metric values are categorized for all metrics of this provider. - * Overridden by metric-level thresholds when set. - * Overrides datasource-level thresholds. + [providerName: string]: { + /** How often metrics will be calculated for this provider. */ + schedule?: SchedulerServiceTaskScheduleDefinitionConfig; + /** + * How metric values are categorized for all metrics of this provider. + * Overridden by metric-level thresholds when set. + */ + thresholds?: ThresholdConfig; + /** Per-metric configuration. */ + metrics?: { + /** Configuration for a specific metric. + * Use the local name without datasource prefix (e.g., 'openPRs' instead of 'github.openPRs'). */ - thresholds?: ThresholdConfig; - /** Per-metric configuration. Keys are local metric names (no datasource prefix). */ - metrics?: { - [metricName: string]: { - /** - * How metric values are categorized for this metric. - * Overrides datasource- and provider-level thresholds. - */ - thresholds?: ThresholdConfig; - }; + [metricName: string]: { + /** + * How metric values are categorized for this metric. + * Overrides provider-level thresholds. + */ + thresholds?: ThresholdConfig; }; }; + /** Provider-specific options (shape defined by each module). */ + options?: unknown; }; }; }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md b/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md index f517a70af5c..cb17141c130 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/providers.md @@ -163,13 +163,13 @@ curl -X GET "{{url}}/api/scorecard/metrics?datasource=my_datasource" -H "Content The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers. Each metric provider runs on its own schedule to collect and store metric values in the database. -You can customize the schedule for any metric provider by adding a `schedule` configuration in your `app-config.yaml`, under path `scorecard.plugins.{datasourceId}.{metricName}`: +You can customize the schedule for any metric provider by adding a `schedule` configuration in your `app-config.yaml`, under path `scorecard.metricProviders..`: ```yaml scorecard: - plugins: - my_datasource: - example_metric: + metricProviders: + myDatasource: + exampleProvider: schedule: frequency: cron: '0 6 * * *' diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index f5852df7bd6..dcf9967cdfd 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -82,15 +82,14 @@ export class MyMetricProvider implements MetricProvider<'number'> { ### 2. App Configuration Thresholds -You can override metric defaults through app configuration (`app-config.yaml`). Thresholds may be set at three levels; the **most specific** level winds and completely replaces metric defaults: +You can override metric defaults through app configuration (`app-config.yaml`). Thresholds may be set at two levels; the **most specific** level wins and completely replaces metric defaults: -1. **Metric** (highest app-config priority): `scorecard.plugins..metricProviders..metrics..thresholds` -2. **Provider**: `scorecard.plugins..metricProviders..thresholds` -3. **Datasource** (lowest app-config priority): `scorecard.plugins..thresholds` +1. **Metric** (highest app-config priority): `scorecard.metricProviders...metrics..thresholds` +2. **Provider**: `scorecard.metricProviders...thresholds` -Keys under `plugins.` are **local names only** (no datasource prefix for both metric provider IDs and metric IDs). The full runtime provider ID is `.` or `` and metric ID is `.`. +Keys under `metricProviders.` are **local provider names** (no datasource prefix). Keys under `metrics` are **local metric names** (no datasource prefix). The full provider ID is `.` and metric ID is `.`. -Datasource- or provider-level thresholds must be valid for every metric they apply to (same metric type). You can use provider-level or datasource-level for single-metric plugins. +Provider-level thresholds must be valid for every metric they apply to. Prefer provider-level thresholds for multi-metric providers (for example OpenSSF or Filecheck) when all metrics share the same rules. Threshold configuration is validated in [validateThresholdsForMetric()](../../scorecard-node/src/utils/thresholds/validateThresholds.ts). @@ -100,52 +99,36 @@ Threshold configuration is validated in [validateThresholdsForMetric()](../../sc ```yaml scorecard: - plugins: + metricProviders: myDatasource: - metricProviders: - myProvider: - metrics: - myMetric: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '<=20' - - key: error - expression: '>20' + myProvider: + metrics: + myMetric: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '<=20' + - key: error + expression: '>20' ``` **_Provider level:_** ```yaml scorecard: - plugins: + metricProviders: myDatasource: - metricProviders: - myProvider: - thresholds: - rules: - - key: success - expression: '<10' - - key: warning - expression: '<=20' - - key: error - expression: '>20' -``` - -**_Datasource level:_** - -```yaml -scorecard: - plugins: - myDatasource: - thresholds: - rules: - - key: success - expression: '==true' - - key: error - expression: '==false' + myProvider: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '<=20' + - key: error + expression: '>20' ``` ### 3. Entity Annotation Overrides diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts index 5467277081e..1496ff8e0d0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts @@ -62,9 +62,11 @@ describe('PullMetricsByProviderTask', () => { mockConfig = mockServices.rootConfig({ data: { scorecard: { - plugins: { + metricProviders: { github: { - schedule: scheduleConfig, + testMetric: { + schedule: scheduleConfig, + }, }, }, }, @@ -430,13 +432,13 @@ describe('PullMetricsByProviderTask', () => { ).rejects.toThrow('test error'); }); - describe('batch providers', () => { + describe('batch providers (return multiple metrics)', () => { let mockBatchProvider: MockBatchBooleanProvider; beforeEach(() => { mockBatchProvider = new MockBatchBooleanProvider( 'filecheck', - 'filecheck', + 'filecheck.fileExistence', [ { id: 'readme', path: 'README.md' }, { id: 'license', path: 'LICENSE' }, @@ -727,7 +729,7 @@ describe('PullMetricsByProviderTask', () => { expect(savedRecords).toHaveLength(1); }); - it('should use datasource schedule when no provider schedule set', async () => { + it('should use provider schedule from config', async () => { mockScheduler.createScheduledTaskRunner.mockClear(); const batchTask = new PullMetricsByProviderTask( { @@ -737,9 +739,11 @@ describe('PullMetricsByProviderTask', () => { config: mockServices.rootConfig({ data: { scorecard: { - plugins: { + metricProviders: { filecheck: { - schedule: scheduleConfig, + fileExistence: { + schedule: scheduleConfig, + }, }, }, }, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts index 2830a9b31bc..6e5705b2b28 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -833,7 +833,7 @@ describe('createRouter', () => { it('should use registry.getMetric to resolve the correct metric for batch providers', async () => { const batchProvider = new MockBatchBooleanProvider( 'filecheck', - 'filecheck', + 'filecheck.fileExistence', [ { id: 'readme', path: 'README.md' }, { id: 'license', path: 'LICENSE' }, @@ -1076,7 +1076,7 @@ describe('createRouter', () => { it('should resolve the correct metric for batch providers', async () => { const batchProvider = new MockBatchBooleanProvider( 'filecheck', - 'filecheck', + 'filecheck.fileExistence', [ { id: 'readme', path: 'README.md' }, { id: 'license', path: 'LICENSE' }, @@ -1334,7 +1334,7 @@ describe('createRouter', () => { it('should resolve the correct metric metadata for batch providers', async () => { const batchProvider = new MockBatchBooleanProvider( 'filecheck', - 'filecheck', + 'filecheck.fileExistence', [ { id: 'readme', path: 'README.md' }, { id: 'license', path: 'LICENSE' }, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts index b685913198f..64bdbd421aa 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts @@ -25,19 +25,17 @@ import { ThresholdResolver } from './ThresholdResolver'; describe('ThresholdResolver', () => { const customThresholds = { scorecard: { - plugins: { + metricProviders: { github: { - metricProviders: { - numberMetric: { - metrics: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, + numberMetric: { + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, }, }, @@ -52,19 +50,17 @@ describe('ThresholdResolver', () => { const resolver = new ThresholdResolver( new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - metricProviders: { - otherMetric: { - metrics: { - otherMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, + otherMetric: { + metrics: { + otherMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, }, }, @@ -85,75 +81,20 @@ describe('ThresholdResolver', () => { }); }); - it('uses configured thresholds before provider default thresholds', () => { - const provider = new MockNumberProvider('github.numberMetric', 'github'); - const resolver = new ThresholdResolver(new ConfigReader(customThresholds), [ - new MockNumberProvider('github.otherMetric', 'github'), - provider, - ]); - - expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }); - }); - - it('uses datasource-level thresholds when no provider or metric thresholds', () => { - const provider = new MockNumberProvider('github.numberMetric', 'github'); - const resolver = new ThresholdResolver( - new ConfigReader({ - scorecard: { - plugins: { - github: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, - }, - }, - }, - }), - [provider], - ); - - expect(resolver.resolveMetricThresholds(provider.getMetrics()[0])).toEqual({ - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }); - }); - - it('uses provider-level thresholds over datasource-level thresholds', () => { + it('uses configured provider-level thresholds when configured metric thresholds are absent', () => { const provider = new MockNumberProvider('github.numberMetric', 'github'); const resolver = new ThresholdResolver( new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - thresholds: { - rules: [ - { key: 'error', expression: '>200' }, - { key: 'warning', expression: '>100' }, - { key: 'success', expression: '<=100' }, - ], - }, - metricProviders: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, }, }, @@ -172,38 +113,29 @@ describe('ThresholdResolver', () => { }); }); - it('uses metric-level thresholds over provider- and datasource-level thresholds', () => { + it('uses configured metric-level thresholds over configured provider-level thresholds', () => { const provider = new MockNumberProvider('github.numberMetric', 'github'); const resolver = new ThresholdResolver( new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - thresholds: { - rules: [ - { key: 'error', expression: '>200' }, - { key: 'warning', expression: '>100' }, - { key: 'success', expression: '<=100' }, - ], - }, - metricProviders: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>150' }, - { key: 'warning', expression: '>75' }, - { key: 'success', expression: '<=75' }, - ], - }, - metrics: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>150' }, + { key: 'warning', expression: '>75' }, + { key: 'success', expression: '<=75' }, + ], + }, + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, }, }, @@ -224,36 +156,38 @@ describe('ThresholdResolver', () => { }); }); - it('uses configured thresholds per metric when provider ID equals datasource', () => { - const provider = new MockBatchBooleanProvider('filecheck', 'filecheck', [ - { id: 'readme', path: 'README.md' }, - { id: 'license', path: 'LICENSE' }, - ]); + it('uses configured thresholds per metric for batch providers', () => { + const provider = new MockBatchBooleanProvider( + 'filecheck', + 'filecheck.fileExistence', + [ + { id: 'readme', path: 'README.md' }, + { id: 'license', path: 'LICENSE' }, + ], + ); const resolver = new ThresholdResolver( new ConfigReader({ scorecard: { - plugins: { + metricProviders: { filecheck: { - metricProviders: { - filecheck: { - metrics: { - readme: { - thresholds: { - rules: [ - { - key: 'present', - expression: '==true', - color: 'success.main', - icon: 'scorecardSuccessStatusIcon', - }, - { - key: 'absent', - expression: '==false', - color: 'error.main', - icon: 'scorecardErrorStatusIcon', - }, - ], - }, + fileExistence: { + metrics: { + readme: { + thresholds: { + rules: [ + { + key: 'present', + expression: '==true', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + { + key: 'absent', + expression: '==false', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + ], }, }, }, @@ -359,19 +293,17 @@ describe('ThresholdResolver', () => { it('loads configured thresholds once at startup', () => { const config = new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - metricProviders: { - numberMetric: { - metrics: { - numberMetric: { - thresholds: { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '>50' }, - { key: 'success', expression: '<=50' }, - ], - }, + numberMetric: { + metrics: { + numberMetric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], }, }, }, @@ -393,13 +325,11 @@ describe('ThresholdResolver', () => { it('validates configured thresholds at startup', () => { const config = new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - metricProviders: { - numberMetric: { - thresholds: { - rules: [{ key: 'error', expression: 'INVALID' }], - }, + numberMetric: { + thresholds: { + rules: [{ key: 'error', expression: 'INVALID' }], }, }, }, @@ -409,7 +339,7 @@ describe('ThresholdResolver', () => { const provider = new MockNumberProvider('github.numberMetric', 'github'); expect(() => new ThresholdResolver(config, [provider])).toThrow( - 'Invalid thresholds configuration at scorecard.plugins.github.metricProviders.numberMetric.thresholds', + 'Invalid thresholds configuration at scorecard.metricProviders.github.numberMetric.thresholds', ); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts index 664be3de6c4..12aa616ede6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.test.ts @@ -16,8 +16,6 @@ import { ConfigReader } from '@backstage/config'; import { - getDatasourceScheduleConfigPath, - getDatasourceThresholdsConfigPath, getMetricLocalConfigKey, getMetricThresholdsConfigPath, getProviderLocalConfigKey, @@ -29,16 +27,13 @@ import { describe('metricProviderConfigKeys', () => { describe('getProviderLocalConfigKey', () => { - it('returns datasource when provider ID equals datasource', () => { - expect(getProviderLocalConfigKey('filecheck', 'filecheck')).toBe( - 'filecheck', - ); - }); - it('returns local name for prefixed provider ID', () => { expect(getProviderLocalConfigKey('jira.openIssues', 'jira')).toBe( 'openIssues', ); + expect( + getProviderLocalConfigKey('filecheck.fileExistence', 'filecheck'), + ).toBe('fileExistence'); }); }); @@ -51,19 +46,13 @@ describe('metricProviderConfigKeys', () => { }); describe('threshold config paths', () => { - it('builds datasource-level path', () => { - expect(getDatasourceThresholdsConfigPath('github')).toBe( - 'scorecard.plugins.github.thresholds', - ); - }); - it('builds provider-level path', () => { expect(getProviderThresholdsConfigPath('github', 'github.openPRs')).toBe( - 'scorecard.plugins.github.metricProviders.openPRs.thresholds', - ); - expect(getProviderThresholdsConfigPath('filecheck', 'filecheck')).toBe( - 'scorecard.plugins.filecheck.metricProviders.filecheck.thresholds', + 'scorecard.metricProviders.github.openPRs.thresholds', ); + expect( + getProviderThresholdsConfigPath('filecheck', 'filecheck.fileExistence'), + ).toBe('scorecard.metricProviders.filecheck.fileExistence.thresholds'); }); it('builds metric-level path for single-metric provider', () => { @@ -74,48 +63,27 @@ describe('metricProviderConfigKeys', () => { 'github.openPRs', ), ).toBe( - 'scorecard.plugins.github.metricProviders.openPRs.metrics.openPRs.thresholds', - ); - }); - - it('builds metric-level path for batch provider', () => { - expect( - getMetricThresholdsConfigPath( - 'filecheck', - 'filecheck', - 'filecheck.readme', - ), - ).toBe( - 'scorecard.plugins.filecheck.metricProviders.filecheck.metrics.readme.thresholds', + 'scorecard.metricProviders.github.openPRs.metrics.openPRs.thresholds', ); }); }); describe('schedule config paths', () => { - it('builds datasource- and provider-level schedule paths', () => { - expect(getDatasourceScheduleConfigPath('github')).toBe( - 'scorecard.plugins.github.schedule', - ); + it('builds provider-level schedule path', () => { expect(getProviderScheduleConfigPath('github', 'github.openPRs')).toBe( - 'scorecard.plugins.github.metricProviders.openPRs.schedule', + 'scorecard.metricProviders.github.openPRs.schedule', ); }); - it('prefers provider schedule when present', () => { + it('resolves provider schedule when present', () => { const config = new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - schedule: { - frequency: { hours: 1 }, - timeout: { minutes: 15 }, - }, - metricProviders: { - openPRs: { - schedule: { - frequency: { hours: 2 }, - timeout: { minutes: 20 }, - }, + openPRs: { + schedule: { + frequency: { hours: 2 }, + timeout: { minutes: 20 }, }, }, }, @@ -131,28 +99,6 @@ describe('metricProviderConfigKeys', () => { }); }); - it('falls back to datasource schedule when provider schedule is absent', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - github: { - schedule: { - frequency: { hours: 1 }, - timeout: { minutes: 15 }, - }, - }, - }, - }, - }); - - expect( - resolveScheduleFromConfig(config, 'github', 'github.openPRs'), - ).toEqual({ - frequency: { hours: 1 }, - timeout: { minutes: 15 }, - }); - }); - it('returns undefined when no schedule is configured', () => { expect( resolveScheduleFromConfig( @@ -165,19 +111,16 @@ describe('metricProviderConfigKeys', () => { }); describe('resolveThresholdsConfigPath', () => { - it('prefers metric over provider and datasource', () => { + it('prefers metric over provider', () => { const config = new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - thresholds: { rules: [] }, - metricProviders: { - openPRs: { - thresholds: { rules: [] }, - metrics: { - openPRs: { - thresholds: { rules: [] }, - }, + openPRs: { + thresholds: { rules: [] }, + metrics: { + openPRs: { + thresholds: { rules: [] }, }, }, }, @@ -194,20 +137,17 @@ describe('metricProviderConfigKeys', () => { 'github.openPRs', ), ).toBe( - 'scorecard.plugins.github.metricProviders.openPRs.metrics.openPRs.thresholds', + 'scorecard.metricProviders.github.openPRs.metrics.openPRs.thresholds', ); }); - it('prefers provider over datasource when metric is absent', () => { + it('falls back to provider when metric is absent', () => { const config = new ConfigReader({ scorecard: { - plugins: { + metricProviders: { github: { - thresholds: { rules: [] }, - metricProviders: { - openPRs: { - thresholds: { rules: [] }, - }, + openPRs: { + thresholds: { rules: [] }, }, }, }, @@ -221,28 +161,7 @@ describe('metricProviderConfigKeys', () => { 'github.openPRs', 'github.openPRs', ), - ).toBe('scorecard.plugins.github.metricProviders.openPRs.thresholds'); - }); - - it('falls back to datasource when provider and metric are absent', () => { - const config = new ConfigReader({ - scorecard: { - plugins: { - github: { - thresholds: { rules: [] }, - }, - }, - }, - }); - - expect( - resolveThresholdsConfigPath( - config, - 'github', - 'github.openPRs', - 'github.openPRs', - ), - ).toBe('scorecard.plugins.github.thresholds'); + ).toBe('scorecard.metricProviders.github.openPRs.thresholds'); }); it('returns undefined when no thresholds are configured', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts index 1272487f324..d2ad7edf3d4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/metricProviderConfigKeys.ts @@ -21,22 +21,20 @@ import { } from '@backstage/backend-plugin-api'; /** - * Local config key for a provider under `scorecard.plugins..metricProviders.`. - * Provider/metric IDs are validated at registry startup. + * Local config key for a provider under `scorecard.metricProviders..`. + * Provider IDs are validated at registry startup as `.`. */ export function getProviderLocalConfigKey( providerId: string, datasourceId: string, ): string { - return providerId === datasourceId - ? providerId - : providerId.slice(datasourceId.length + 1); + return providerId.slice(datasourceId.length + 1); } /** * Local config key for a metric under - * `scorecard.plugins..metricProviders..metrics.`. - * Provider/metric IDs are validated at registry startup. + * `scorecard.metricProviders...metrics.`. + * Metric IDs are validated at registry startup as `.`. */ export function getMetricLocalConfigKey( metricId: string, @@ -45,26 +43,19 @@ export function getMetricLocalConfigKey( return metricId.slice(datasourceId.length + 1); } -/** Get datasource thresholds config key under `scorecard.plugins..thresholds` */ -export function getDatasourceThresholdsConfigPath( - datasourceId: string, -): string { - return `scorecard.plugins.${datasourceId}.thresholds`; -} - /** Get provider thresholds config key under - * `scorecard.plugins..metricProviders..thresholds` + * `scorecard.metricProviders...thresholds` */ export function getProviderThresholdsConfigPath( datasourceId: string, providerId: string, ): string { const providerKey = getProviderLocalConfigKey(providerId, datasourceId); - return `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}.thresholds`; + return `scorecard.metricProviders.${datasourceId}.${providerKey}.thresholds`; } /** Get metric thresholds config key under - * `scorecard.plugins..metricProviders..metrics..thresholds` + * `scorecard.metricProviders...metrics..thresholds` */ export function getMetricThresholdsConfigPath( datasourceId: string, @@ -74,40 +65,32 @@ export function getMetricThresholdsConfigPath( const providerKey = getProviderLocalConfigKey(providerId, datasourceId); const metricKey = getMetricLocalConfigKey(metricId, datasourceId); return ( - `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}` + + `scorecard.metricProviders.${datasourceId}.${providerKey}` + `.metrics.${metricKey}.thresholds` ); } -/** Get datasource schedule config key under `scorecard.plugins..schedule` */ -export function getDatasourceScheduleConfigPath(datasourceId: string): string { - return `scorecard.plugins.${datasourceId}.schedule`; -} - -/** Get provider schedule config key under `scorecard.plugins..metricProviders..schedule` */ +/** Get provider schedule config key under `scorecard.metricProviders...schedule` */ export function getProviderScheduleConfigPath( datasourceId: string, providerId: string, ): string { const providerKey = getProviderLocalConfigKey(providerId, datasourceId); - return `scorecard.plugins.${datasourceId}.metricProviders.${providerKey}.schedule`; + return `scorecard.metricProviders.${datasourceId}.${providerKey}.schedule`; } /** - * Resolves the schedule from config, preferring provider-level over datasource-level. - * Returns undefined when neither is set. + * Resolves the provider schedule from config. + * Returns undefined when not set (caller uses the default schedule). */ export function resolveScheduleFromConfig( config: Config, datasourceId: string, providerId: string, ): SchedulerServiceTaskScheduleDefinition | undefined { - const schedulePath = [ - getProviderScheduleConfigPath(datasourceId, providerId), - getDatasourceScheduleConfigPath(datasourceId), - ].find(path => config.has(path)); + const schedulePath = getProviderScheduleConfigPath(datasourceId, providerId); - if (!schedulePath) { + if (!config.has(schedulePath)) { return undefined; } @@ -118,7 +101,7 @@ export function resolveScheduleFromConfig( /** * Resolves the thresholds config path for a metric. - * Most specific wins: metric > provider > datasource. + * Most specific wins: metric > provider. * Returns undefined when no thresholds are set. */ export function resolveThresholdsConfigPath( @@ -130,7 +113,6 @@ export function resolveThresholdsConfigPath( const paths = [ getMetricThresholdsConfigPath(datasourceId, providerId, metricId), getProviderThresholdsConfigPath(datasourceId, providerId), - getDatasourceThresholdsConfigPath(datasourceId), ]; return paths.find(path => config.has(path)); } diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts index e074917657d..07a1fd59e46 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.test.ts @@ -21,17 +21,17 @@ import { describe('validateMetricProviderIds', () => { describe('validateProviderId', () => { - it('allows provider ID equal to datasource', () => { - expect(() => validateProviderId('filecheck', 'filecheck')).not.toThrow(); - }); - it('allows provider ID with datasource prefix', () => { expect(() => validateProviderId('github.openPRs', 'github'), ).not.toThrow(); + expect(() => + validateProviderId('filecheck.fileExistence', 'filecheck'), + ).not.toThrow(); }); it.each([ + ['equal to datasource', 'filecheck', 'filecheck'], ['wrong datasource prefix', 'other.openPRs', 'github'], ['missing dot separator', 'githubopenPrs', 'github'], ['empty ID', '', 'github'], diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts index e76066d4891..a3e764dcf2e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/validation/validateMetricProviderIds.ts @@ -15,21 +15,17 @@ */ /** - * Validates provider ID format: `` or `.`. + * Validates provider ID format: `.`. */ export function validateProviderId( providerId: string, datasourceId: string, ): void { - if (providerId === datasourceId) { - return; - } - const [datasource, providerName, ...rest] = providerId.split('.'); if (datasource !== datasourceId || !providerName || rest.length > 0) { throw new Error( - `Invalid provider ID '${providerId}', must be equal to datasource '${datasourceId}' ` + - `or have format '${datasourceId}.' where provider name is not empty`, + `Invalid provider ID '${providerId}', must have format ` + + `'${datasourceId}.' where provider name is not empty`, ); } } From e5079b2b5cad1155742f4d0f9f931a1c904192c2 Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Tue, 28 Jul 2026 17:38:55 +0200 Subject: [PATCH 5/5] Remove unused provider properties Signed-off-by: Dominika Zemanovicova --- .../JiraOpenIssuesProvider.test.ts | 20 ------------------- .../metricProviders/JiraOpenIssuesProvider.ts | 6 ------ .../OpenSSFMetricProvider.test.ts | 12 +---------- .../metricProviders/OpenSSFMetricProvider.ts | 20 ++++--------------- .../__fixtures__/mockProviders.ts | 4 ---- 5 files changed, 5 insertions(+), 57 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.test.ts index 7c410a1f1d0..f79f85d6ff0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.test.ts @@ -111,26 +111,6 @@ describe('JiraOpenIssuesProvider', () => { }); }); - describe('supportsEntity', () => { - it('should return true when entity has project key annotation', () => { - const provider = JiraOpenIssuesProvider.fromConfig( - mockConfig, - mockAuthOptions, - ); - expect(provider.supportsEntity(mockEntity)).toBe(true); - }); - - it('should return false when entity does not have project key annotation', () => { - const mockEmptyEntity: Entity = newEntityComponent({}); - - const provider = JiraOpenIssuesProvider.fromConfig( - mockConfig, - mockAuthOptions, - ); - expect(provider.supportsEntity(mockEmptyEntity)).toBe(false); - }); - }); - describe('fromConfig', () => { it('should create provider with default thresholds on metric', () => { const provider = JiraOpenIssuesProvider.fromConfig( diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.ts index 527f75ab2d4..e4116e91060 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-jira/src/metricProviders/JiraOpenIssuesProvider.ts @@ -24,7 +24,6 @@ import { import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { JiraClient } from '../clients/base'; import { JiraClientFactory } from '../clients/JiraClientFactory'; -import { ScorecardJiraAnnotations } from '../annotations'; import { type AuthService, type DiscoveryService, @@ -36,7 +35,6 @@ import { } from '../strategies/ConnectionStrategy'; import { Product } from '../clients/types'; -const { PROJECT_KEY } = ScorecardJiraAnnotations; import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; export class JiraOpenIssuesProvider implements MetricProvider<'number'> { @@ -74,10 +72,6 @@ export class JiraOpenIssuesProvider implements MetricProvider<'number'> { ]; } - supportsEntity(entity: Entity): boolean { - return entity.metadata.annotations?.[PROJECT_KEY] !== undefined; - } - static fromConfig( config: Config, options: { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.test.ts index 821eed0e8b4..a0ac5ffb215 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.test.ts @@ -59,17 +59,6 @@ describe('OpenSSFMetricProvider', () => { }); describe('metadata', () => { - it('returns metric name from config', () => { - const provider = new OpenSSFMetricProvider(maintainedConfig); - expect(provider.getMetricName()).toBe('Maintained'); - }); - - it('returns display title and description from config', () => { - const provider = new OpenSSFMetricProvider(maintainedConfig); - expect(provider.getMetricDisplayTitle()).toBe('OpenSSF Maintained'); - expect(provider.getMetricDescription()).toContain('actively maintained'); - }); - it('returns provider id as openssf.', () => { const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getProviderId()).toBe('openssf.maintained'); @@ -92,6 +81,7 @@ describe('OpenSSFMetricProvider', () => { const metric = metrics[0]; expect(metric.id).toBe('openssf.maintained'); expect(metric.title).toBe('OpenSSF Maintained'); + expect(metric.description).toContain('actively maintained'); expect(metric.type).toBe('number'); expect(metric.thresholds).toEqual(OPENSSF_THRESHOLDS); expect(metric.history).toBe(true); diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.ts index cbae3563761..0723826cb17 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFMetricProvider.ts @@ -35,24 +35,12 @@ export class OpenSSFMetricProvider implements MetricProvider<'number'> { this.openSSFClient = new OpenSSFClient(); } - getMetricName(): string { - return this.config.name; - } - - getMetricDisplayTitle(): string { - return this.config.displayTitle; - } - - getMetricDescription(): string { - return this.config.description; - } - getProviderDatasourceId(): string { return 'openssf'; } getProviderId(): string { - const normalizedName = this.getMetricName() + const normalizedName = this.config.name .toLowerCase() .replace(/-([a-z])/g, (_, c) => c.toUpperCase()); return `openssf.${normalizedName}`; @@ -62,8 +50,8 @@ export class OpenSSFMetricProvider implements MetricProvider<'number'> { return [ { id: this.getProviderId(), - title: this.getMetricDisplayTitle(), - description: this.getMetricDescription(), + title: this.config.displayTitle, + description: this.config.description, type: 'number', thresholds: OPENSSF_THRESHOLDS, history: true, @@ -80,7 +68,7 @@ export class OpenSSFMetricProvider implements MetricProvider<'number'> { async calculateMetrics(entity: Entity): Promise> { const scorecard = await this.openSSFClient.getScorecard(entity); - const metricName = this.getMetricName(); + const metricName = this.config.name; const metric = scorecard.checks.find(c => c.name === metricName); if (!metric) { diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts index 4d54b1a8fb2..589220d069e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockProviders.ts @@ -63,10 +63,6 @@ abstract class MockMetricProvider return this.providerId; } - supportsEntity(_: Entity): boolean { - return true; - } - getMetrics(): Metric[] { return [ {