diff --git a/workspaces/scorecard/.changeset/tired-hoops-happen.md b/workspaces/scorecard/.changeset/tired-hoops-happen.md new file mode 100644 index 0000000000..7ad20d9b72 --- /dev/null +++ b/workspaces/scorecard/.changeset/tired-hoops-happen.md @@ -0,0 +1,16 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-dependabot': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-filecheck': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-sonarqube': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-openssf': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-github': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-jira': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': patch +'@red-hat-developer-hub/backstage-plugin-scorecard-node': patch +--- + +Custom thresholds for filecheck, openssf, and dependabot are now +configurable. Custom threshold handling has been centralized in +`scorecard-backend`, you can define custom thresholds under +`scorecard.plugins..thresholds`. Provider IDs typically +follow the format `.`. diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.test.ts index a7a80d97e1..98cebb3343 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.test.ts @@ -18,7 +18,10 @@ import { ConfigReader } from '@backstage/config'; import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; import type { Entity } from '@backstage/catalog-model'; import { DependabotMetricProvider } from './DependabotMetricProvider'; -import { DEPENDABOT_SEVERITY_METRIC } from './DependabotConfig'; +import { + DEPENDABOT_SEVERITY_METRIC, + DEPENDABOT_THRESHOLDS, +} from './DependabotConfig'; import { mockServices } from '@backstage/backend-test-utils'; jest.mock('@backstage/catalog-model', () => ({ @@ -105,25 +108,13 @@ describe('DependabotMetricProvider', () => { }); describe('getMetricThresholds', () => { - it('returns default thresholds when none provided', () => { + it('returns default thresholds', () => { const provider = new DependabotMetricProvider( mockConfig, mockLogger, 'critical', ); - expect(provider.getMetricThresholds()).toBeDefined(); - expect(provider.getMetricThresholds().rules).toBeDefined(); - }); - - it('returns custom thresholds when provided', () => { - const custom = { rules: [{ key: 'ok', expression: '<1' }] }; - const provider = new DependabotMetricProvider( - mockConfig, - mockLogger, - 'critical', - custom, - ); - expect(provider.getMetricThresholds()).toEqual(custom); + expect(provider.getMetricThresholds()).toEqual(DEPENDABOT_THRESHOLDS); }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.ts index 136e15c51c..a2b2c3d4ae 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.ts @@ -44,18 +44,15 @@ const GITHUB_PROJECT_ANNOTATION = 'github.com/project-slug'; */ export class DependabotMetricProvider implements MetricProvider<'number'> { private readonly dependabotClient: DependabotClient; - private readonly thresholds: ThresholdConfig; private readonly severity: DependabotSeverity; constructor( config: Config, logger: LoggerService, severity: DependabotSeverity, - thresholds?: ThresholdConfig, ) { this.severity = severity; this.dependabotClient = new DependabotClient(config, logger); - this.thresholds = thresholds ?? DEPENDABOT_THRESHOLDS; } getProviderDatasourceId(): string { @@ -82,7 +79,7 @@ export class DependabotMetricProvider implements MetricProvider<'number'> { } getMetricThresholds(): ThresholdConfig { - return this.thresholds; + return DEPENDABOT_THRESHOLDS; } getCatalogFilter(): Record { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.test.ts index 12b73487f9..40e3644b82 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.test.ts @@ -20,6 +20,7 @@ import { createDependabotMetricProviders, } from './DependabotMetricProviderFactory'; import { mockServices } from '@backstage/backend-test-utils'; +import { DEPENDABOT_THRESHOLDS } from './DependabotConfig'; const mockConfig = new ConfigReader({ integrations: { github: [{ host: 'github.com', token: 'test-token' }] }, @@ -36,17 +37,7 @@ describe('createDependabotMetricProvider', () => { expect(provider.getProviderId()).toBe('dependabot.alerts_high'); expect(provider.getProviderDatasourceId()).toBe('dependabot'); expect(provider.getMetricType()).toBe('number'); - }); - - it('accepts optional thresholds', () => { - const thresholds = { rules: [{ key: 'ok', expression: '<1' }] }; - const provider = createDependabotMetricProvider( - mockConfig, - mockLogger, - 'critical', - thresholds, - ); - expect(provider.getMetricThresholds()).toEqual(thresholds); + expect(provider.getMetricThresholds()).toBe(DEPENDABOT_THRESHOLDS); }); }); @@ -61,16 +52,4 @@ describe('createDependabotMetricProviders', () => { 'dependabot.alerts_low', ]); }); - - it('passes optional thresholds to all providers', () => { - const thresholds = { rules: [{ key: 'custom', expression: '>0' }] }; - const providers = createDependabotMetricProviders( - mockConfig, - mockLogger, - thresholds, - ); - providers.forEach(p => { - expect(p.getMetricThresholds()).toEqual(thresholds); - }); - }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.ts b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.ts index 9cab795378..b6c35fca6b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.ts @@ -17,7 +17,6 @@ import { DependabotMetricProvider } from './DependabotMetricProvider'; import { DependabotSeverity, DEPENDABOT_SEVERITIES } from './DependabotConfig'; import { Config } from '@backstage/config'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; /** @@ -27,9 +26,8 @@ export function createDependabotMetricProvider( config: Config, logger: LoggerService, severity: DependabotSeverity, - thresholds?: ThresholdConfig, ): MetricProvider<'number'> { - return new DependabotMetricProvider(config, logger, severity, thresholds); + return new DependabotMetricProvider(config, logger, severity); } /** @@ -38,9 +36,8 @@ export function createDependabotMetricProvider( export function createDependabotMetricProviders( config: Config, logger: LoggerService, - thresholds?: ThresholdConfig, ): MetricProvider<'number'>[] { return DEPENDABOT_SEVERITIES.map(severity => - createDependabotMetricProvider(config, logger, severity, thresholds), + createDependabotMetricProvider(config, logger, severity), ); } diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md index 17e272448f..ebf064af35 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-filecheck/README.md @@ -104,6 +104,25 @@ Each configured file produces one boolean metric. You can override the default thresholds via `app-config.yaml`. Check out the detailed explanation of [threshold configuration](../scorecard-backend/docs/thresholds.md). +Example configuration: + +```yaml +# app-config.yaml +scorecard: + plugins: + filecheck: + thresholds: + rules: + - key: present + expression: '==true' + icon: scorecardSuccessStatusIcon + color: 'success.main' + - key: absent + expression: '==false' + icon: scorecardErrorStatusIcon + color: 'error.main' +``` + ## Schedule Configuration The Scorecard plugin uses Backstage's built-in scheduler service to automatically collect metrics from all registered providers every hour by default. You can change this schedule in the `app-config.yaml` file: 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 8bafab8b79..8a8ba06501 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 @@ -30,16 +30,10 @@ import { export class FilecheckMetricProvider implements MetricProvider<'boolean'> { private readonly client: FilecheckClient; private readonly filesConfig: FilecheckConfig; - private readonly thresholds: ThresholdConfig; - constructor( - client: FilecheckClient, - filesConfig: FilecheckConfig, - thresholds?: ThresholdConfig, - ) { + constructor(client: FilecheckClient, filesConfig: FilecheckConfig) { this.client = client; this.filesConfig = filesConfig; - this.thresholds = thresholds ?? DEFAULT_FILECHECK_THRESHOLDS; } getProviderDatasourceId(): string { @@ -73,7 +67,7 @@ export class FilecheckMetricProvider implements MetricProvider<'boolean'> { } getMetricThresholds(): ThresholdConfig { - return this.thresholds; + return DEFAULT_FILECHECK_THRESHOLDS; } getCatalogFilter(): Record { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.test.ts index 569b32852e..82c20f2cb2 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.test.ts @@ -31,56 +31,11 @@ jest.mock('../github/GithubClient'); describe('GithubOpenPRsProvider', () => { describe('fromConfig', () => { - it('should create provider with default thresholds when no thresholds are configured', () => { + it('should create provider with default thresholds', () => { const provider = GithubOpenPRsProvider.fromConfig(new ConfigReader({})); expect(provider.getMetricThresholds()).toEqual(DEFAULT_NUMBER_THRESHOLDS); }); - - it('should create provider with custom thresholds when configured', () => { - const customThresholds = { - rules: [ - { key: 'error', expression: '>100' }, - { key: 'warning', expression: '50-100' }, - { key: 'success', expression: '<50' }, - ], - }; - - const configWithThresholds = new ConfigReader({ - scorecard: { - plugins: { - github: { - open_prs: { - thresholds: customThresholds, - }, - }, - }, - }, - }); - const provider = GithubOpenPRsProvider.fromConfig(configWithThresholds); - - expect(provider.getMetricThresholds()).toEqual(customThresholds); - }); - - it('should throw error when invalid custom thresholds', () => { - const invalidConfig = new ConfigReader({ - scorecard: { - plugins: { - github: { - open_prs: { - thresholds: { - rules: [{ key: 'error', expression: '>!100' }], - }, - }, - }, - }, - }, - }); - - expect(() => GithubOpenPRsProvider.fromConfig(invalidConfig)).toThrow( - 'Cannot parse "!100" as number from expression: ">!100"', - ); - }); }); describe('calculateMetric', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.ts index e700274940..1263392fac 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.ts @@ -22,20 +22,15 @@ import { Metric, ThresholdConfig, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { - getThresholdsFromConfig, - MetricProvider, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { GithubClient } from '../github/GithubClient'; import { getRepositoryInformationFromEntity } from '../github/utils'; export class GithubOpenPRsProvider implements MetricProvider<'number'> { private readonly githubClient: GithubClient; - private readonly thresholds: ThresholdConfig; - private constructor(config: Config, thresholds?: ThresholdConfig) { + private constructor(config: Config) { this.githubClient = new GithubClient(config); - this.thresholds = thresholds ?? DEFAULT_NUMBER_THRESHOLDS; } getProviderDatasourceId(): string { @@ -62,7 +57,7 @@ export class GithubOpenPRsProvider implements MetricProvider<'number'> { } getMetricThresholds(): ThresholdConfig { - return this.thresholds; + return DEFAULT_NUMBER_THRESHOLDS; } getCatalogFilter(): Record { @@ -72,13 +67,7 @@ export class GithubOpenPRsProvider implements MetricProvider<'number'> { } static fromConfig(config: Config): GithubOpenPRsProvider { - const thresholds = getThresholdsFromConfig( - config, - 'scorecard.plugins.github.open_prs.thresholds', - 'number', - ); - - return new GithubOpenPRsProvider(config, thresholds); + return new GithubOpenPRsProvider(config); } async calculateMetric(entity: Entity): Promise { 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 17d5a68511..792f8dd4af 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 @@ -19,7 +19,6 @@ import type { Entity } from '@backstage/catalog-model'; import { DEFAULT_NUMBER_THRESHOLDS, Metric, - ThresholdConfig, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { JiraOpenIssuesProvider } from './JiraOpenIssuesProvider'; import { JiraClientFactory } from '../clients/JiraClientFactory'; @@ -27,7 +26,6 @@ import { JiraClient } from '../clients/base'; import { mockServices } from '@backstage/backend-test-utils'; import { newEntityComponent, - newThresholdsConfig, newMockRootConfig, } from '../../__fixtures__/testUtils'; import { ScorecardJiraAnnotations } from '../annotations'; @@ -61,8 +59,6 @@ const mockEntity: Entity = newEntityComponent({ [PROJECT_KEY]: 'TEST', }); -const customThresholds: ThresholdConfig = newThresholdsConfig(); - const mockAuthOptions = { discovery: mockServices.discovery(), auth: mockServices.auth(), @@ -142,23 +138,13 @@ describe('JiraOpenIssuesProvider', () => { }); describe('getMetricThresholds', () => { - it('should return default config when no thresholds are configured', () => { + it('should return default provider thresholds', () => { const provider = JiraOpenIssuesProvider.fromConfig( mockConfig, mockAuthOptions, ); expect(provider.getMetricThresholds()).toEqual(DEFAULT_NUMBER_THRESHOLDS); }); - - it('should return custom config when thresholds are configured', () => { - const config = newMockRootConfig({ thresholds: customThresholds }); - - const provider = JiraOpenIssuesProvider.fromConfig( - config, - mockAuthOptions, - ); - expect(provider.getMetricThresholds()).toEqual(customThresholds); - }); }); describe('supportsEntity', () => { @@ -191,27 +177,6 @@ describe('JiraOpenIssuesProvider', () => { expect(provider.getMetricThresholds()).toEqual(DEFAULT_NUMBER_THRESHOLDS); }); - it('should create provider with custom config when thresholds are configured', () => { - const config = newMockRootConfig({ thresholds: customThresholds }); - - const provider = JiraOpenIssuesProvider.fromConfig( - config, - mockAuthOptions, - ); - expect(provider.getMetricThresholds()).toEqual(customThresholds); - }); - - it('should throw an error when invalid thresholds are configured', () => { - const invalidThresholds = { - rules: [{ key: 'invalid', expression: 'bad' }], - }; - const config = newMockRootConfig({ thresholds: invalidThresholds }); - - expect(() => - JiraOpenIssuesProvider.fromConfig(config, mockAuthOptions), - ).toThrow('Invalid thresholds'); - }); - it('should create provider with proxy connection strategy when proxy path is configured', () => { JiraOpenIssuesProvider.fromConfig(mockConfig, mockAuthOptions); expect(mockedProxyConnectionStrategy).toHaveBeenCalledWith( 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 2ab1e176b2..6cdbbbc786 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 @@ -16,16 +16,13 @@ import type { Config } from '@backstage/config'; import type { Entity } from '@backstage/catalog-model'; -import { JIRA_CONFIG_PATH, OPEN_ISSUES_CONFIG_PATH } from '../constants'; +import { JIRA_CONFIG_PATH } from '../constants'; import { DEFAULT_NUMBER_THRESHOLDS, Metric, ThresholdConfig, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { - getThresholdsFromConfig, - MetricProvider, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +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'; @@ -44,20 +41,14 @@ const { PROJECT_KEY } = ScorecardJiraAnnotations; import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; export class JiraOpenIssuesProvider implements MetricProvider<'number'> { - private readonly thresholds: ThresholdConfig; private readonly jiraClient: JiraClient; - private constructor( - config: Config, - connectionStrategy: ConnectionStrategy, - thresholds: ThresholdConfig, - ) { + private constructor(config: Config, connectionStrategy: ConnectionStrategy) { this.jiraClient = JiraClientFactory.create(config, connectionStrategy); - this.thresholds = thresholds; } getMetricThresholds(): ThresholdConfig { - return this.thresholds; + return DEFAULT_NUMBER_THRESHOLDS; } getCatalogFilter(): Record { @@ -119,14 +110,7 @@ export class JiraOpenIssuesProvider implements MetricProvider<'number'> { ); } - const thresholds = - getThresholdsFromConfig( - config, - `${OPEN_ISSUES_CONFIG_PATH}.thresholds`, - 'number', - ) ?? DEFAULT_NUMBER_THRESHOLDS; - - return new JiraOpenIssuesProvider(config, connectionStrategy, thresholds); + return new JiraOpenIssuesProvider(config, connectionStrategy); } async calculateMetric(entity: Entity): Promise { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md index 5a5947f313..cb72ade046 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/README.md @@ -44,7 +44,7 @@ metadata: ### Thresholds -All OpenSSF metrics use fixed thresholds: **Error** <2, **Warning** 2–7, **Success** >7. Not configurable. See [threshold docs](../scorecard-backend/docs/thresholds.md). +All OpenSSF metrics use default thresholds: **Error** <2, **Warning** 2–7, **Success** >7. You can configure custom thresholds, see [threshold docs](../scorecard-backend/docs/thresholds.md). ## Metrics diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFConfig.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFConfig.ts index 3018edcb6f..d1f5fdf004 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/metricProviders/OpenSSFConfig.ts @@ -144,8 +144,8 @@ export const OPENSSF_METRICS: OpenSSFMetricConfig[] = [ export const OPENSSF_THRESHOLDS: ThresholdConfig = { rules: [ - { key: 'error', expression: '<2' }, - { key: 'warning', expression: '2-7' }, { key: 'success', expression: '>7' }, + { key: 'warning', expression: '2-7' }, + { key: 'error', expression: '<2' }, ], }; 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 62acf81c2a..0e67fabb46 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 @@ -60,59 +60,38 @@ describe('OpenSSFMetricProvider', () => { describe('metadata', () => { it('returns metric name from config', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getMetricName()).toBe('Maintained'); }); it('returns display title and description from config', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + 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, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getProviderId()).toBe('openssf.maintained'); }); it('normalizes hyphenated check names for provider id', () => { - const provider = new OpenSSFMetricProvider( - hyphenatedCheckConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(hyphenatedCheckConfig); expect(provider.getProviderId()).toBe('openssf.code_review'); }); it('returns openssf as provider datasource id', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getProviderDatasourceId()).toBe('openssf'); }); it('returns number as metric type', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getMetricType()).toBe('number'); }); it('returns metric descriptor with history enabled', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); const metric = provider.getMetric(); expect(metric.id).toBe('openssf.maintained'); expect(metric.title).toBe('OpenSSF Maintained'); @@ -121,18 +100,12 @@ describe('OpenSSFMetricProvider', () => { }); it('returns configured thresholds', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getMetricThresholds()).toEqual(OPENSSF_THRESHOLDS); }); it('requires openssf/scorecard-location annotation in catalog filter', () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); expect(provider.getCatalogFilter()).toEqual({ 'metadata.annotations.openssf/scorecard-location': CATALOG_FILTER_EXISTS, @@ -161,10 +134,7 @@ describe('OpenSSFMetricProvider', () => { }), }); - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); const result = await provider.calculateMetric(entity); expect(result).toBe(8); @@ -172,10 +142,7 @@ describe('OpenSSFMetricProvider', () => { }); it('propagates errors from the OpenSSF client', async () => { - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); const propagatedError = new Error('OpenSSF client failed'); const getScorecardSpy = jest .spyOn((provider as any).openSSFClient, 'getScorecard') @@ -208,10 +175,7 @@ describe('OpenSSFMetricProvider', () => { }), }); - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); await expect(provider.calculateMetric(entity)).rejects.toThrow( "OpenSSF check 'Maintained' not found in scorecard", @@ -240,10 +204,7 @@ describe('OpenSSFMetricProvider', () => { }), }); - const provider = new OpenSSFMetricProvider( - maintainedConfig, - OPENSSF_THRESHOLDS, - ); + const provider = new OpenSSFMetricProvider(maintainedConfig); await expect(provider.calculateMetric(entity)).rejects.toThrow( `OpenSSF check 'Maintained' has invalid score ${invalidScore}`, 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 6bed221dbb..b60d016acc 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 @@ -32,13 +32,8 @@ import { export class OpenSSFMetricProvider implements MetricProvider<'number'> { protected readonly openSSFClient: OpenSSFClient; - protected readonly thresholds: ThresholdConfig; - constructor( - readonly config: OpenSSFMetricConfig, - thresholds: ThresholdConfig, - ) { - this.thresholds = thresholds; + constructor(readonly config: OpenSSFMetricConfig) { this.config = config; this.openSSFClient = new OpenSSFClient(); } @@ -81,7 +76,7 @@ export class OpenSSFMetricProvider implements MetricProvider<'number'> { } getMetricThresholds(): ThresholdConfig { - return this.thresholds; + return OPENSSF_THRESHOLDS; } getCatalogFilter(): Record { @@ -112,7 +107,5 @@ export class OpenSSFMetricProvider implements MetricProvider<'number'> { * @returns Array of OpenSSF metric providers */ export function createOpenSSFMetricProvider(): MetricProvider<'number'>[] { - return OPENSSF_METRICS.map( - config => new OpenSSFMetricProvider(config, OPENSSF_THRESHOLDS), - ); + return OPENSSF_METRICS.map(config => new OpenSSFMetricProvider(config)); } diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.test.ts index 16349d0d3b..29507a9690 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.test.ts @@ -17,8 +17,6 @@ import { ConfigReader } from '@backstage/config'; import type { Entity } from '@backstage/catalog-model'; -import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; - import { SonarQubeBooleanMetricProvider } from './SonarQubeBooleanMetricProvider'; import { mockServices } from '@backstage/backend-test-utils'; @@ -50,7 +48,7 @@ function entity(projectKey = 'my-project'): Entity { describe('SonarQubeBooleanMetricProvider', () => { describe('getMetricThresholds', () => { - it('should return default thresholds when none provided', () => { + it('should create provider with default thresholds', () => { const provider = SonarQubeBooleanMetricProvider.fromConfig( mockConfig, mockLogger, @@ -59,31 +57,6 @@ describe('SonarQubeBooleanMetricProvider', () => { expect(provider.getMetricThresholds()).toBeDefined(); expect(provider.getMetricThresholds().rules).toHaveLength(2); }); - - it('should return custom thresholds when provided', () => { - const custom: ThresholdConfig = { - rules: [ - { key: 'ok', expression: '==true', color: '#00ff00', icon: 'ok' }, - ], - }; - const mockConfiWithCustomThresholds = new ConfigReader({ - scorecard: { - plugins: { - sonarqube: { - quality_gate: { - thresholds: custom, - }, - }, - }, - }, - }); - const provider = SonarQubeBooleanMetricProvider.fromConfig( - mockConfiWithCustomThresholds, - mockLogger, - 'quality_gate', - ); - expect(provider.getMetricThresholds()).toEqual(custom); - }); }); describe('calculateMetric', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.ts index b6deaf090f..725a388c63 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeBooleanMetricProvider.ts @@ -15,10 +15,7 @@ */ import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { - getThresholdsFromConfig, - MetricProvider, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import type { LoggerService } from '@backstage/backend-plugin-api'; import type { Config } from '@backstage/config'; import { type Entity } from '@backstage/catalog-model'; @@ -61,14 +58,10 @@ export class SonarQubeBooleanMetricProvider metricId: SonarQubeBooleanMetricId, ): SonarQubeBooleanMetricProvider { const client = new SonarQubeClient(config, logger); - - const thresholds = - getThresholdsFromConfig( - config, - `scorecard.plugins.sonarqube.${metricId}.thresholds`, - 'boolean', - ) ?? SONARQUBE_BOOLEAN_THRESHOLDS; - - return new SonarQubeBooleanMetricProvider(client, metricId, thresholds); + return new SonarQubeBooleanMetricProvider( + client, + metricId, + SONARQUBE_BOOLEAN_THRESHOLDS, + ); } } diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts index f276f5ec66..a7d4a98209 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.test.ts @@ -17,8 +17,6 @@ import { ConfigReader } from '@backstage/config'; import type { Entity } from '@backstage/catalog-model'; -import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; - import { SonarQubeNumberMetricProvider } from './SonarQubeNumberMetricProvider'; import { mockServices } from '@backstage/backend-test-utils'; @@ -52,7 +50,7 @@ function entity(projectKey = 'my-project'): Entity { describe('SonarQubeNumberMetricProvider', () => { describe('getMetricThresholds', () => { - it('should return default thresholds when none provided', () => { + it('should create provider with default thresholds', () => { const provider = SonarQubeNumberMetricProvider.fromConfig( mockConfig, mockLogger, @@ -61,32 +59,6 @@ describe('SonarQubeNumberMetricProvider', () => { expect(provider.getMetricThresholds()).toBeDefined(); expect(provider.getMetricThresholds().rules).toBeDefined(); }); - - it('should return custom thresholds when provided', () => { - const custom: ThresholdConfig = { - rules: [ - { key: 'ok', expression: '<5', color: '#00ff00', icon: 'ok' }, - { key: 'rest', expression: '>=5', color: '#ff0000', icon: 'bad' }, - ], - }; - const mockConfiWithCustomThresholds = new ConfigReader({ - scorecard: { - plugins: { - sonarqube: { - open_issues: { - thresholds: custom, - }, - }, - }, - }, - }); - const provider = SonarQubeNumberMetricProvider.fromConfig( - mockConfiWithCustomThresholds, - mockLogger, - 'open_issues', - ); - expect(provider.getMetricThresholds()).toEqual(custom); - }); }); describe('calculateMetric', () => { diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.ts b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.ts index 419ffd2e8d..dfe115dca6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-sonarqube/src/metricProviders/SonarQubeNumberMetricProvider.ts @@ -15,10 +15,7 @@ */ import { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { - getThresholdsFromConfig, - MetricProvider, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import type { LoggerService } from '@backstage/backend-plugin-api'; import type { Config } from '@backstage/config'; import { type Entity } from '@backstage/catalog-model'; @@ -70,14 +67,10 @@ export class SonarQubeNumberMetricProvider metricId: SonarQubeNumberMetricId, ): SonarQubeNumberMetricProvider { const client = new SonarQubeClient(config, logger); - - const thresholds = - getThresholdsFromConfig( - config, - `scorecard.plugins.sonarqube.${metricId}.thresholds`, - 'number', - ) ?? SONARQUBE_NUMBER_THRESHOLDS[metricId]; - - return new SonarQubeNumberMetricProvider(client, metricId, thresholds); + return new SonarQubeNumberMetricProvider( + client, + metricId, + SONARQUBE_NUMBER_THRESHOLDS[metricId], + ); } } diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockEntityBuilder.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockEntityBuilder.ts index 7eb77fe0c3..7e8b63a22b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockEntityBuilder.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockEntityBuilder.ts @@ -35,6 +35,11 @@ export class MockEntityBuilder { return this; } + withAnnotations(annotations: Record): this { + this.metadata = { ...this.metadata, annotations }; + return this; + } + withSpec(spec: Entity['spec']): this { this.spec = spec; return this; diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index c92764c0be..25ca0da578 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -51,7 +51,8 @@ Validation **does not** require full coverage when: ### 1. Provider Default Thresholds -Metric providers can define default thresholds that apply to all entities using that metric. +Metric providers must define 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 in your metric provider. **Example Provider Implementation:** @@ -73,45 +74,10 @@ export class MyMetricProvider implements MetricProvider<'number'> { ### 2. App Configuration Thresholds -You can override provider defaults through app configuration (`app-config.yaml`). Your metric provider needs to support configuration-based thresholds. -Duplicated threshold keys are not allowed (throws `ConfigFormatError`). +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. -**Provider Implementation:** - -```typescript -import { - MetricProvider, - validateThresholdsForMetric, - getThresholdsFromConfig, -} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; - -export class MyMetricProvider implements MetricProvider<'number'> { - private readonly thresholds: ThresholdConfig; - - private constructor(thresholds?: ThresholdConfig) { - // Use configured thresholds or fall back to defaults - this.thresholds = thresholds ?? this.getDefaultThresholds(); - } - - static fromConfig(config: Config): MyMetricProvider { - const configPath = 'scorecard.plugins.myDatasource.myMetric.thresholds'; - // Validates structure, colors/icons, expressions, and number interval coverage (when applicable) - const configuredThresholds = getThresholdsFromConfig( - config, - configPath, - 'number', - ); - - return new MyMetricProvider(configuredThresholds); - } - - getMetricThresholds(): ThresholdConfig { - return this.thresholds; - } -} -``` - -You can also call **`validateThresholdsForMetric(configuredThresholds, 'number')`** directly if you already have a config object (not reading from `Config`). +Threshold configuration is validated in [validateThresholdsForMetric()](../../scorecard-node/src/utils/thresholds/validateThresholds.ts). **Example App Configuration:** @@ -132,6 +98,22 @@ scorecard: myOtherMetric: ... ``` +**Example App Configuration for batch provider:** + +```yaml +scorecard: + plugins: + myDatasource: + thresholds: + rules: + - key: success + expression: '<10' + - key: warning + expression: '<=20' + - key: error + expression: '>20' +``` + ### 3. Entity Annotation Overrides Override thresholds for specific entities using annotations in the entity's metadata: @@ -150,6 +132,8 @@ 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. + #### Annotation Format Reference Entity annotations use this format: @@ -254,12 +238,12 @@ Example: ```yaml rules: - - key: error - expression: '>100' - - key: warning - expression: '80-100' # between 80 and 100 (inclusive) - key: success expression: '<80' + - key: warning + expression: '80-100' # between 80 and 100 (inclusive) + - key: error + expression: '>100' ``` ### Boolean Metric @@ -419,7 +403,7 @@ The `ThresholdEvaluator` service processes threshold rules and determines which 1. **Order-dependent evaluation**: Rules are evaluated in the order they appear. If provider supports overriding defaults through [app configuration](#App-Configuration-Thresholds), you can change the evaluation order by specifying threshold keys in a different order. Entity annotations cannot alter the evaluation order, which is determined by either the [app configuration](#Provider-Default-Thresholds) or, if not specified, the [default provider configuration](#Provider-Default-Thresholds). 2. **First-match wins**: Returns the first threshold rule whose condition the value satisfies 3. **Type-safe**: Validates expressions against metric types -4. **Error handling**: Validate expressions loaded from config in custom providers using **`validateThresholdsForMetric`** or **`getThresholdsFromConfig`** from `@red-hat-developer-hub/backstage-plugin-scorecard-node`. Invalid expressions or gap configurations fail at validation time; unchecked configs may error at evaluation time. +4. **Error handling**: Thresholds from providers and custom thresholds from configuration are validated on startup (using [validateThresholdsForMetric](../../scorecard-node/src/utils/thresholds/validateThresholds.ts) from `@red-hat-developer-hub/backstage-plugin-scorecard-node`). Threshold errors caused by invalid providers or invalid configuration cause startup failures. Annotation-based threshold errors are reported in the UI at evaluation time. ### Best Practices diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts index 1a4e138535..7ed04a12d5 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts @@ -36,6 +36,7 @@ import { DatabaseMetricValues } from './database/DatabaseMetricValues'; import { Scheduler } from './scheduler'; import { validateAggregationConfig } from './validation/validateAggregationConfig'; import { AggregationsService } from './service/aggregations/AggregationService'; +import { ThresholdResolver } from './threshold/ThresholdResolver'; /** * scorecardPlugin backend plugin @@ -94,6 +95,10 @@ export const scorecardPlugin = createBackendPlugin({ const client = await database.getClient(); const dbMetricValues = new DatabaseMetricValues(client); + const thresholdResolver = new ThresholdResolver( + config, + metricProvidersRegistry.listProviders(), + ); const catalogMetricService = new CatalogMetricService({ catalog, @@ -101,6 +106,7 @@ export const scorecardPlugin = createBackendPlugin({ registry: metricProvidersRegistry, database: dbMetricValues, logger: logger, + thresholdResolver, }); const aggregationsService = new AggregationsService({ @@ -123,6 +129,7 @@ export const scorecardPlugin = createBackendPlugin({ database: dbMetricValues, metricProvidersRegistry, thresholdEvaluator: new ThresholdEvaluator(), + thresholdResolver, }).start(); const service = { @@ -138,6 +145,7 @@ export const scorecardPlugin = createBackendPlugin({ httpAuth, permissions, logger, + thresholdResolver, }), ); }, 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 b1442886ae..a616eb76e5 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.test.ts @@ -144,6 +144,25 @@ describe('MetricProvidersRegistry', () => { ); }); + it('should throw error when provider default thresholds are invalid', () => { + class InvalidThresholdFormatProvider extends MockNumberProvider { + getMetricThresholds() { + return { + rules: [{ key: 'error', expression: 'Invalid expression' }], + } as any; + } + } + + const invalidProvider = new InvalidThresholdFormatProvider( + 'github.invalid_threshold_format', + 'github', + ); + + expect(() => registry.register(invalidProvider)).toThrow( + 'Invalid default thresholds for metric provider \'github.invalid_threshold_format\'; caused by ThresholdConfigFormatError: Invalid threshold expression: "Invalid expression"', + ); + }); + describe('batch providers', () => { it('should register batch provider with multiple metric IDs', () => { expect(() => registry.register(filecheckBatchProvider)).not.toThrow(); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts index eeed50306a..7b0c9d3ccd 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/providers/MetricProvidersRegistry.ts @@ -14,13 +14,17 @@ * 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 type { Entity } from '@backstage/catalog-model'; -import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { + MetricProvider, + ThresholdConfigFormatError, + validateThresholdsForMetric, +} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; /** * Registry of all registered metric providers. @@ -32,11 +36,10 @@ export class MetricProvidersRegistry { register(metricProvider: MetricProvider): void { const providerDatasource = metricProvider.getProviderDatasourceId(); const metricType = metricProvider.getMetricType(); + const providerId = metricProvider.getProviderId(); // Support both single and batch providers - const metricIds = metricProvider.getMetricIds?.() ?? [ - metricProvider.getProviderId(), - ]; + const metricIds = metricProvider.getMetricIds?.() ?? [providerId]; const metrics = metricProvider.getMetrics?.() ?? [ metricProvider.getMetric(), ]; @@ -72,7 +75,21 @@ export class MetricProvidersRegistry { `Metric provider with ID '${metricId}' has already been registered`, ); } + } + try { + validateThresholdsForMetric( + metricProvider.getMetricThresholds(), + metricType, + ); + } catch (error) { + throw new ThresholdConfigFormatError( + `Invalid default thresholds for metric provider '${providerId}'`, + error, + ); + } + + for (const metricId of metricIds) { this.metricProviders.set(metricId, metricProvider); // Index by datasource diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts index c742a35020..296aa297b0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts @@ -28,6 +28,7 @@ import { import { mockDatabaseMetricValues } from '../../__fixtures__/mockDatabaseMetricValues'; import { mockMetricProvidersRegistry } from '../../__fixtures__/mockMetricProvidersRegistry'; import { ThresholdEvaluator } from '../threshold/ThresholdEvaluator'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; jest.mock('./tasks/CleanupExpiredMetricsTask'); jest.mock('./tasks/PullMetricsByProviderTask'); @@ -74,6 +75,10 @@ describe('Scheduler', () => { database: mockDatabase, metricProvidersRegistry: mockRegistry, thresholdEvaluator: new ThresholdEvaluator(), + thresholdResolver: new ThresholdResolver( + mockConfig, + mockRegistry.listProviders(), + ), }); mockCleanupTask = { diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts index 823b524ef3..06612ca15f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts @@ -28,6 +28,7 @@ import { PullMetricsByProviderTask } from './tasks/PullMetricsByProviderTask'; import { SchedulerOptions, SchedulerTask } from './types'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { ThresholdEvaluator } from '../threshold/ThresholdEvaluator'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; export class Scheduler { private readonly auth: AuthService; @@ -38,6 +39,7 @@ export class Scheduler { private readonly database: DatabaseMetricValues; private readonly metricProvidersRegistry: MetricProvidersRegistry; private readonly thresholdEvaluator: ThresholdEvaluator; + private readonly thresholdResolver: ThresholdResolver; private tasks: Array<{ name: string; task: SchedulerTask }> = []; @@ -50,6 +52,7 @@ export class Scheduler { this.database = options.database; this.metricProvidersRegistry = options.metricProvidersRegistry; this.thresholdEvaluator = options.thresholdEvaluator; + this.thresholdResolver = options.thresholdResolver; } static create(options: SchedulerOptions): Scheduler { @@ -109,6 +112,7 @@ export class Scheduler { catalog: this.catalog, auth: this.auth, thresholdEvaluator: this.thresholdEvaluator, + thresholdResolver: this.thresholdResolver, }, provider, ), 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 2124ec08de..391f93eff6 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 @@ -17,21 +17,17 @@ import { mockServices } from '@backstage/backend-test-utils'; import { PullMetricsByProviderTask } from './PullMetricsByProviderTask'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; -import { mergeEntityAndProviderThresholds } from '../../utils/mergeEntityAndProviderThresholds'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; import { MockNumberProvider, MockBatchBooleanProvider, } from '../../../__fixtures__/mockProviders'; -import type { Config } from '@backstage/config'; +import { Config } from '@backstage/config'; import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; import { mockDatabaseMetricValues } from '../../../__fixtures__/mockDatabaseMetricValues'; import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; import { mockThresholdRules } from '../../../__fixtures__/mockThresholdRules'; - -jest.mock('../../utils/mergeEntityAndProviderThresholds', () => ({ - mergeEntityAndProviderThresholds: jest.fn(), -})); +import { ThresholdResolver } from '../../threshold/ThresholdResolver'; const scheduleConfig = { frequency: { hours: 2 }, @@ -53,7 +49,7 @@ describe('PullMetricsByProviderTask', () => { let mockProvider: MetricProvider; let mockTaskRunner: { run: jest.Mock }; let mockThresholdEvaluator: jest.Mocked; - let mockMergeEntityAndProviderThresholds: jest.Mock; + let mockThresholdResolver: jest.Mocked; let task: PullMetricsByProviderTask; @@ -87,11 +83,12 @@ describe('PullMetricsByProviderTask', () => { mockTaskRunner as any, ); - mockMergeEntityAndProviderThresholds = - mergeEntityAndProviderThresholds as jest.Mock; - mockMergeEntityAndProviderThresholds.mockReturnValue({ - rules: mockThresholdRules, - }); + mockThresholdResolver = { + resolveEntityThresholds: jest.fn().mockReturnValue({ + rules: mockThresholdRules, + }), + resolveProviderThresholds: jest.fn(), + } as unknown as jest.Mocked; task = new PullMetricsByProviderTask( { @@ -102,6 +99,7 @@ describe('PullMetricsByProviderTask', () => { catalog: mockCatalog, auth: mockAuth, thresholdEvaluator: mockThresholdEvaluator, + thresholdResolver: mockThresholdResolver, }, mockProvider, ); @@ -232,19 +230,15 @@ describe('PullMetricsByProviderTask', () => { expect(getOwnServiceCredentialsSpy).toHaveBeenCalledWith(); }); - it('should merge entity and provider thresholds', async () => { + it('should resolve thresholds for entity/provider', async () => { await (task as any).pullProviderMetrics(mockProvider, mockLogger); - expect(mockMergeEntityAndProviderThresholds).toHaveBeenNthCalledWith( - 1, - mockEntities[0], - mockProvider, - ); - expect(mockMergeEntityAndProviderThresholds).toHaveBeenNthCalledWith( - 2, - mockEntities[1], - mockProvider, - ); + expect( + mockThresholdResolver.resolveEntityThresholds, + ).toHaveBeenNthCalledWith(1, mockEntities[0], mockProvider); + expect( + mockThresholdResolver.resolveEntityThresholds, + ).toHaveBeenNthCalledWith(2, mockEntities[1], mockProvider); }); it('should calculate metric', async () => { @@ -459,6 +453,7 @@ describe('PullMetricsByProviderTask', () => { catalog: mockCatalog, auth: mockAuth, thresholdEvaluator: mockThresholdEvaluator, + thresholdResolver: mockThresholdResolver, }, mockBatchProvider, ); @@ -685,6 +680,7 @@ describe('PullMetricsByProviderTask', () => { catalog: mockCatalog, auth: mockAuth, thresholdEvaluator: mockThresholdEvaluator, + thresholdResolver: mockThresholdResolver, }, mockBatchProvider, ); 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 9ce86acb44..6d64205f2f 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -25,7 +25,6 @@ import { import type { Config } from '@backstage/config'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; -import { mergeEntityAndProviderThresholds } from '../../utils/mergeEntityAndProviderThresholds'; import { isMetricIdDisabled } from '../../utils/metricUtils'; import { normalizeOwnerRef } from '../../utils/normalizeOwnerRef'; import { v4 as uuid } from 'uuid'; @@ -34,6 +33,7 @@ import { DbMetricValueCreate } from '../../database/types'; import { SchedulerOptions, SchedulerTask } from '../types'; import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; import { MetricValue } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { ThresholdResolver } from '../../threshold/ThresholdResolver'; type Options = Pick< SchedulerOptions, @@ -44,6 +44,7 @@ type Options = Pick< | 'catalog' | 'auth' | 'thresholdEvaluator' + | 'thresholdResolver' >; export class PullMetricsByProviderTask implements SchedulerTask { @@ -56,6 +57,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { private readonly scheduler: SchedulerService; private readonly database: DatabaseMetricValues; private readonly thresholdEvaluator: ThresholdEvaluator; + private readonly thresholdResolver: ThresholdResolver; private static readonly CATALOG_BATCH_SIZE = 50; @@ -76,6 +78,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { this.scheduler = options.scheduler; this.database = options.database; this.thresholdEvaluator = options.thresholdEvaluator; + this.thresholdResolver = options.thresholdResolver; } async start(): Promise { @@ -179,10 +182,11 @@ export class PullMetricsByProviderTask implements SchedulerTask { const value = resultsMap.get(metricId) as MetricValue; try { - const thresholds = mergeEntityAndProviderThresholds( - entity, - provider, - ); + const thresholds = + this.thresholdResolver.resolveEntityThresholds( + entity, + provider, + ); const status = this.thresholdEvaluator.getFirstMatchingThreshold( @@ -249,7 +253,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { value = await provider.calculateMetric(entity); - const thresholds = mergeEntityAndProviderThresholds( + const thresholds = this.thresholdResolver.resolveEntityThresholds( entity, provider, ); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/types.ts b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/types.ts index 5ea68db9c7..0ee98337e6 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/types.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/types.ts @@ -24,6 +24,7 @@ import { SchedulerService, } from '@backstage/backend-plugin-api'; import { ThresholdEvaluator } from '../threshold/ThresholdEvaluator'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; export interface SchedulerTask { start(): Promise; @@ -38,4 +39,5 @@ export interface SchedulerOptions { database: DatabaseMetricValues; metricProvidersRegistry: MetricProvidersRegistry; thresholdEvaluator: ThresholdEvaluator; + thresholdResolver: ThresholdResolver; } 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 897227d059..4617a65531 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -40,7 +40,6 @@ import { aggregationTypes, Metric, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import * as thresholdUtils from '../utils/mergeEntityAndProviderThresholds'; import { DbMetricValue, DbAggregatedMetric } from '../database/types'; import { mockThresholdRules } from '../../__fixtures__/mockThresholdRules'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; @@ -51,9 +50,9 @@ import { } from '@backstage/plugin-permission-common'; import { AggregatedMetricMapper } from './mappers'; import { AggregatedMetricLoader } from './aggregations/AggregatedMetricLoader'; -import type { DatabaseMetricValues } from '../database/DatabaseMetricValues'; +import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; -jest.mock('../utils/mergeEntityAndProviderThresholds'); jest.mock('../permissions/permissionUtils'); const provider = new MockNumberProvider('github.important_metric', 'github'); @@ -103,6 +102,7 @@ describe('CatalogMetricService', () => { let mockedRegistry: jest.Mocked; let mockedDatabase: jest.Mocked; let mockedLogger: jest.Mocked; + let mockedThresholdResolver: jest.Mocked; let service: CatalogMetricService; let toAggregatedMetricSpy: jest.SpyInstance; @@ -138,11 +138,12 @@ describe('CatalogMetricService', () => { { id: 'github.important_metric' }, ]); - ( - thresholdUtils.mergeEntityAndProviderThresholds as jest.Mock - ).mockReturnValue({ - rules: mockThresholdRules, - }); + mockedThresholdResolver = { + resolveProviderThresholds: jest.fn(), + resolveEntityThresholds: jest.fn().mockReturnValue({ + rules: mockThresholdRules, + }), + } as unknown as jest.Mocked; toAggregatedMetricSpy = jest.spyOn( AggregatedMetricMapper, @@ -155,6 +156,7 @@ describe('CatalogMetricService', () => { registry: mockedRegistry, database: mockedDatabase, logger: mockedLogger, + thresholdResolver: mockedThresholdResolver, }); jest.useFakeTimers(); @@ -316,25 +318,17 @@ describe('CatalogMetricService', () => { }); it('should merge entity and provider thresholds', async () => { - const mergeEntityAndProviderThresholdsSpy = jest.spyOn( - thresholdUtils, - 'mergeEntityAndProviderThresholds', - ); - await service.getLatestEntityMetrics('component:default/test-component', [ 'github.important_metric', ]); - expect(mergeEntityAndProviderThresholdsSpy).toHaveBeenCalledWith( - mockEntity, - provider, - ); + expect( + mockedThresholdResolver.resolveEntityThresholds, + ).toHaveBeenCalledWith(mockEntity, provider); }); it('should set threshold error when merge thresholds fails', async () => { - ( - thresholdUtils.mergeEntityAndProviderThresholds as jest.Mock - ).mockImplementation(() => { + mockedThresholdResolver.resolveEntityThresholds.mockImplementation(() => { throw new Error('Merge thresholds failed'); }); @@ -508,6 +502,7 @@ describe('CatalogMetricService', () => { registry: mockedRegistry, database: mockedDatabase, logger: mockedLogger, + thresholdResolver: mockedThresholdResolver, }); const results = await service.getLatestEntityMetrics( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index df8e265d00..b824ec1419 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -40,10 +40,10 @@ import { } from '@backstage/plugin-permission-common'; import { CatalogService } from '@backstage/plugin-catalog-node'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; -import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; import { isMetricCalculationError } from '../utils/metricCalculationError'; import { AggregatedMetricMapper } from './mappers'; import { DbMetricValue } from '../database/types'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; type CatalogMetricServiceOptions = { catalog: CatalogService; @@ -51,6 +51,7 @@ type CatalogMetricServiceOptions = { registry: MetricProvidersRegistry; database: DatabaseMetricValues; logger: LoggerService; + thresholdResolver: ThresholdResolver; }; export class CatalogMetricService { @@ -74,6 +75,7 @@ export class CatalogMetricService { private readonly auth: AuthService; private readonly registry: MetricProvidersRegistry; private readonly database: DatabaseMetricValues; + private readonly thresholdResolver: ThresholdResolver; private static readonly MAX_FETCHABLE_ROWS = 10_000; private static readonly BATCH_SIZE = 100; @@ -84,6 +86,7 @@ export class CatalogMetricService { this.registry = options.registry; this.database = options.database; this.logger = options.logger; + this.thresholdResolver = options.thresholdResolver; } /** @@ -129,7 +132,10 @@ export class CatalogMetricService { const metric = this.registry.getMetric(metric_id); try { - thresholds = mergeEntityAndProviderThresholds(entity, provider); + thresholds = this.thresholdResolver.resolveEntityThresholds( + entity, + provider, + ); if (value === null) { thresholdError = 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 d5322370df..3c2750b8e3 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ConfigReader, type Config } from '@backstage/config'; +import { ConfigReader, Config } from '@backstage/config'; import { mockErrorHandler, mockServices, @@ -50,8 +50,8 @@ import { } from '@backstage/backend-plugin-api'; import { mockDatabaseMetricValues } from '../../__fixtures__/mockDatabaseMetricValues'; import { AggregationsService } from './aggregations/AggregationService'; -import type { DatabaseMetricValues } from '../database/DatabaseMetricValues'; -import type { DbAggregatedMetric } from '../database/types'; +import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; +import { DbAggregatedMetric } from '../database/types'; jest.mock('../utils/getEntitiesOwnedByUser', () => ({ getEntitiesOwnedByUser: jest.fn(), @@ -69,6 +69,7 @@ import * as getEntitiesOwnedByUserModule from '../utils/getEntitiesOwnedByUser'; import * as permissionUtilsModule from '../permissions/permissionUtils'; import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; import { AggregatedMetricMapper } from './mappers'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; function createTestAggregationsService( database: DatabaseMetricValues, @@ -103,6 +104,7 @@ describe('createRouter', () => { let metricProvidersRegistry: MetricProvidersRegistry; let catalogMetricService: CatalogMetricService; let aggregationsService: AggregationsService; + let thresholdResolver: ThresholdResolver; let mockLogger: ReturnType; let httpAuthMock: ServiceMock< import('@backstage/backend-plugin-api').HttpAuthService @@ -115,6 +117,10 @@ describe('createRouter', () => { beforeEach(async () => { metricProvidersRegistry = new MetricProvidersRegistry(); + thresholdResolver = new ThresholdResolver( + new ConfigReader({}), + metricProvidersRegistry.listProviders(), + ); const catalog = catalogServiceMock.mock(); mockLogger = mockServices.logger.mock(); catalogMetricService = new CatalogMetricService({ @@ -123,6 +129,7 @@ describe('createRouter', () => { auth: mockServices.auth(), database: mockDatabaseMetricValues, logger: mockLogger, + thresholdResolver, }); aggregationsService = createTestAggregationsService( @@ -153,6 +160,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); app = express(); app.use(router); @@ -620,6 +628,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); aggregationApp = express(); aggregationApp.use(router); @@ -846,6 +855,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const batchApp = express(); batchApp.use(batchAggregationRouter); @@ -966,6 +976,7 @@ describe('createRouter', () => { registry: metricRegistry, database: mockDatabaseMetricValues, logger: mockServices.logger.mock(), + thresholdResolver, }); readAggregatedMetricByEntityRefsSpyAgId = jest @@ -1005,6 +1016,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); aggregationsApp = express(); aggregationsApp.use(router); @@ -1091,6 +1103,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const batchApp = express(); batchApp.use(batchRouter); @@ -1138,6 +1151,7 @@ describe('createRouter', () => { registry: metricRegistry, database: mockDatabaseMetricValues, logger: mockServices.logger.mock(), + thresholdResolver, }); const getSpy = jest @@ -1159,6 +1173,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const kpiApp = express(); kpiApp.use(router); @@ -1202,6 +1217,7 @@ describe('createRouter', () => { registry: metricRegistry, database: mockDatabaseMetricValues, logger: mockServices.logger.mock(), + thresholdResolver, }); const getSpy = jest @@ -1223,6 +1239,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const kpiApp = express(); kpiApp.use(router); @@ -1274,6 +1291,7 @@ describe('createRouter', () => { registry: metaRegistry, database: mockDatabaseMetricValues, logger: mockServices.logger.mock(), + thresholdResolver, }); const aggregationsMetaService = createTestAggregationsService( @@ -1291,6 +1309,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); metaApp = express(); metaApp.use(router); @@ -1336,6 +1355,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const batchMetaApp = express(); batchMetaApp.use(router); @@ -1360,6 +1380,7 @@ describe('createRouter', () => { registry: metaRegistry, database: mockDatabaseMetricValues, logger: mockServices.logger.mock(), + thresholdResolver, }); const aggregationsSvcNoKpi = createTestAggregationsService( @@ -1377,6 +1398,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); const svcApp = express(); @@ -1466,6 +1488,7 @@ describe('createRouter', () => { httpAuth: httpAuthMock, permissions: permissionsMock, logger: mockServices.logger.mock(), + thresholdResolver, }); drillDownApp = express(); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts index f317843bda..7f27b84d4c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts @@ -44,6 +44,7 @@ import { validateAggregationIdParam } from '../middlewares/validateAggregationId import { scorecardMetricReadPermission } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { validateDatasourceQueryParams } from '../middlewares/validateDatasourceQueryParams'; import { AggregationsService } from './aggregations/AggregationService'; +import { ThresholdResolver } from '../threshold/ThresholdResolver'; export type ScorecardRouterOptions = { service: { @@ -55,6 +56,7 @@ export type ScorecardRouterOptions = { httpAuth: HttpAuthService; permissions: PermissionsService; logger: LoggerService; + thresholdResolver: ThresholdResolver; }; export async function createRouter({ @@ -64,6 +66,7 @@ export async function createRouter({ httpAuth, permissions, logger, + thresholdResolver, }: ScorecardRouterOptions): Promise { const router = Router(); router.use(express.json()); @@ -180,7 +183,7 @@ export async function createRouter({ await checkEntityAccess(entityRef, req, permissions, httpAuth); } - const thresholds = provider.getMetricThresholds(); + const thresholds = thresholdResolver.resolveProviderThresholds(provider); 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}).`, @@ -302,7 +305,7 @@ export async function createRouter({ ); } - const thresholds = provider.getMetricThresholds(); + const thresholds = thresholdResolver.resolveProviderThresholds(provider); 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 new file mode 100644 index 0000000000..f2fd498e0a --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts @@ -0,0 +1,250 @@ +/* + * 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 { + MockBooleanProvider, + MockNumberProvider, +} from '../../__fixtures__/mockProviders'; +import { MockEntityBuilder } from '../../__fixtures__/mockEntityBuilder'; +import { ThresholdResolver } from './ThresholdResolver'; + +describe('ThresholdResolver', () => { + const customThresholds = { + scorecard: { + plugins: { + github: { + number_metric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, + }, + }, + }; + + it('uses default provider thresholds when no custom thresholds', () => { + const provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver( + new ConfigReader({ + scorecard: { + plugins: { + github: { + other_metric: { + thresholds: { + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }, + }, + }, + }, + }, + }), + [provider, new MockNumberProvider('github.other_metric', 'github')], + ); + + expect(resolver.resolveProviderThresholds(provider)).toEqual({ + rules: [ + { key: 'error', expression: '>40' }, + { key: 'warning', expression: '>20' }, + { key: 'success', expression: '<=20' }, + ], + }); + }); + + it('uses configured thresholds before provider default thresholds', () => { + const provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver(new ConfigReader(customThresholds), [ + new MockNumberProvider('github.other_metric', 'github'), + provider, + ]); + + expect(resolver.resolveProviderThresholds(provider)).toEqual({ + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>50' }, + { key: 'success', expression: '<=50' }, + ], + }); + }); + + it('uses configured thresholds before provider default thresholds for batch provider', () => { + const provider = new MockBooleanProvider('filecheck', 'filecheck'); + const resolver = new ThresholdResolver( + new ConfigReader({ + scorecard: { + plugins: { + filecheck: { + thresholds: { + rules: [ + { + key: 'present', + expression: '==true', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + { + key: 'absent', + expression: '==false', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + ], + }, + }, + }, + }, + }), + [new MockNumberProvider('github.other_metric', 'github'), provider], + ); + + expect(resolver.resolveProviderThresholds(provider)).toEqual({ + rules: [ + { + key: 'present', + expression: '==true', + color: 'success.main', + icon: 'scorecardSuccessStatusIcon', + }, + { + key: 'absent', + expression: '==false', + color: 'error.main', + icon: 'scorecardErrorStatusIcon', + }, + ], + }); + }); + + it('merges entity annotation overrides on top of default provider thresholds', () => { + const provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver(new ConfigReader({}), [provider]); + const entity = new MockEntityBuilder() + .withAnnotations({ + 'scorecard.io/github.number_metric.thresholds.rules.warning': '>10', + 'scorecard.io/github.number_metric.thresholds.rules.success': '<=10', + }) + .build(); + + expect(resolver.resolveEntityThresholds(entity, provider)).toEqual({ + rules: [ + { key: 'error', expression: '>40' }, + { key: 'warning', expression: '>10' }, + { key: 'success', expression: '<=10' }, + ], + }); + }); + + it('merges entity annotation overrides on top of default provider thresholds when provider is unexpectedly not loaded on startup', () => { + const provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver(new ConfigReader({}), []); + const entity = new MockEntityBuilder() + .withAnnotations({ + 'scorecard.io/github.number_metric.thresholds.rules.warning': '>10', + 'scorecard.io/github.number_metric.thresholds.rules.success': '<=10', + }) + .build(); + + expect(resolver.resolveEntityThresholds(entity, provider)).toEqual({ + rules: [ + { key: 'error', expression: '>40' }, + { key: 'warning', expression: '>10' }, + { key: 'success', expression: '<=10' }, + ], + }); + }); + + it('merges entity annotation overrides on top of custom provider thresholds', () => { + const provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver(new ConfigReader(customThresholds), [ + provider, + ]); + const entity = new MockEntityBuilder() + .withAnnotations({ + 'scorecard.io/github.number_metric.thresholds.rules.warning': '>10', + 'scorecard.io/github.number_metric.thresholds.rules.success': '<=10', + }) + .build(); + + expect(resolver.resolveEntityThresholds(entity, provider)).toEqual({ + rules: [ + { key: 'error', expression: '>100' }, + { key: 'warning', expression: '>10' }, + { key: 'success', expression: '<=10' }, + ], + }); + }); + + 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)).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 provider = new MockNumberProvider('github.number_metric', 'github'); + const resolver = new ThresholdResolver(mockConfig, [provider]); + + resolver.resolveProviderThresholds(provider); + resolver.resolveProviderThresholds(provider); + + expect(mockConfig.getOptional).toHaveBeenCalledTimes(1); + }); + + it('validates configured thresholds at startup', () => { + const mockConfig = { + getOptional: jest.fn().mockReturnValue({ + rules: [{ key: 'error', expression: 'INVALID' }], + }), + } as any; + const provider = new MockNumberProvider('github.number_metric', 'github'); + + expect(() => new ThresholdResolver(mockConfig, [provider])).toThrow( + 'Invalid thresholds configuration at scorecard.plugins.github.number_metric.thresholds', + ); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts new file mode 100644 index 0000000000..aab841cc6c --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts @@ -0,0 +1,65 @@ +/* + * 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 type { Entity } from '@backstage/catalog-model'; +import type { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { + getThresholdsFromConfig, + type MetricProvider, +} from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; +import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; + +export class ThresholdResolver { + private readonly configuredThresholds = new Map(); // providerId: thresholds + + constructor(private readonly config: Config, providers: MetricProvider[]) { + for (const provider of providers) { + this.setConfiguredThresholds(provider); + } + } + + resolveProviderThresholds(provider: MetricProvider): ThresholdConfig { + return ( + this.configuredThresholds.get(provider.getProviderId()) ?? + provider.getMetricThresholds() + ); + } + + resolveEntityThresholds( + entity: Entity, + provider: MetricProvider, + ): ThresholdConfig { + return mergeEntityAndProviderThresholds( + entity, + provider, + this.resolveProviderThresholds(provider), + ); + } + + private setConfiguredThresholds(provider: MetricProvider): void { + const providerId = provider.getProviderId(); + const thresholds = getThresholdsFromConfig( + this.config, + `scorecard.plugins.${providerId}.thresholds`, + provider.getMetricType(), + ); + + if (thresholds) { + this.configuredThresholds.set(providerId, thresholds); + } + } +} diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts index 0c09466851..cfc730507c 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/mergeEntityAndProviderThresholds.ts @@ -54,11 +54,12 @@ function parseEntityAnnotationThresholds( export function mergeEntityAndProviderThresholds( entity: Entity, provider: MetricProvider, + baseThresholds?: ThresholdConfig, ): ThresholdConfig { let isRulesMerged = false; const providerId = provider.getProviderId(); - const providerThresholds = provider.getMetricThresholds(); + const providerThresholds = baseThresholds ?? provider.getMetricThresholds(); const providerMetricType = provider.getMetricType(); const entityAnnotationThresholds = parseEntityAnnotationThresholds( entity, diff --git a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts index 150e133110..c303fc9e24 100644 --- a/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts +++ b/workspaces/scorecard/plugins/scorecard-node/src/utils/thresholds/getThresholdsFromConfig.ts @@ -20,6 +20,7 @@ import { ThresholdConfig, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { validateThresholdsForMetric } from './validateThresholds'; +import { ThresholdConfigFormatError } from '../../errors'; /** * Read and validate threshold configuration from config or return undefined @@ -38,7 +39,7 @@ export function getThresholdsFromConfig( return thresholdsConfig; } } catch (error) { - throw new Error( + throw new ThresholdConfigFormatError( `Invalid thresholds configuration at ${thresholdsPath}: ${error}`, ); }