diff --git a/workspaces/scorecard/examples/openssf-scorecard-only.yaml b/workspaces/scorecard/examples/openssf-scorecard-only.yaml index b16a5cd821..01825a999d 100644 --- a/workspaces/scorecard/examples/openssf-scorecard-only.yaml +++ b/workspaces/scorecard/examples/openssf-scorecard-only.yaml @@ -5,6 +5,7 @@ metadata: name: openssf-scorecard-only annotations: openssf/scorecard-location: https://api.securityscorecards.dev/projects/github.com/alizard0/rhdh-plugins + scorecard.io/disabled-metrics: openssf.maintained spec: type: service owner: group:development/guests diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.test.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.test.ts index 7602b1ec0d..7c5cb641f4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.test.ts @@ -49,6 +49,13 @@ const mockOpenSSFResponse: OpenSSFResponse = { details: null, documentation: { short: '', url: '' }, }, + { + name: 'Code-Review', + score: 9, + reason: null, + details: null, + documentation: { short: '', url: '' }, + }, ], }; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.ts index bc8d1e0a2d..f17c1b618d 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/clients/OpenSSFClient.ts @@ -43,8 +43,6 @@ export class OpenSSFClient { ); } - const data: OpenSSFResponse = await response.json(); - - return data; + return await response.json(); } } diff --git a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts index 20ceedbd9d..110276e4e8 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/config.d.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/config.d.ts @@ -21,6 +21,23 @@ export interface Config { scorecard?: { /** Number of days to retain metric data in the database. Older data will be automatically cleaned up. Default: 365 days */ dataRetentionDays?: number; + /** List of metric IDs (e.g. openssf.packaging) that are disabled globally. Entity annotations cannot override this. */ + disabledMetrics?: string[]; + /** + * Control whether users can override behavior via entity annotations. + * This only affects entity annotations (e.g. scorecard.io/disabled-metrics); it does not affect scorecard.disabledMetrics or other app-config. + */ + entityOverrides?: { + /** Whether entity scorecard.io/disabled-metrics annotation can override. Only affects annotations; global disabledMetrics is unchanged. */ + disabledMetrics?: { + /** If true (default), entity can disable metrics via annotation; except list can force some to run. If false, entity list is still applied (union with disabledMetrics) but entity cannot override to re-enable anything. */ + enabled?: boolean; + /** When enabled is true: metric IDs that entity cannot disable (must run). When enabled is false: not used. */ + except?: string[]; + }; + }; + /** @deprecated Use entityOverrides.disabledMetrics.except (with enabled: true) instead. List of metric IDs that must run even when disabled via entity annotation. */ + includeMetrics?: string[]; /** Configuration for scorecard metric providers */ plugins?: { /** Configuration for datasource */ 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 64169b046d..5e7c089e97 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 @@ -156,6 +156,267 @@ describe('PullMetricsByProviderTask', () => { }); }); + describe('isMetricIdExcluded', () => { + const providerId = 'openssf.maintained'; + + function createTaskWithScorecardConfig( + scorecardOverrides: { + includeMetrics?: string[]; + disabledMetrics?: string[]; + entityOverrides?: { + disabledMetrics?: { enabled?: boolean; except?: string[] }; + }; + } = {}, + ) { + const config = mockServices.rootConfig({ + data: { + scorecard: { + schedule: scheduleConfig, + ...scorecardOverrides, + }, + }, + }); + return new PullMetricsByProviderTask( + { + scheduler: mockScheduler, + logger: mockLogger, + database: mockDatabaseMetricValues, + config, + catalog: mockCatalog, + auth: mockAuth, + thresholdEvaluator: mockThresholdEvaluator, + }, + mockProvider, + ); + } + + function createEntity(annotationValue?: string) { + return { + apiVersion: '1.0.0' as const, + kind: 'Component' as const, + metadata: { + name: 'test-entity', + ...(annotationValue !== undefined && { + annotations: { + 'scorecard.io/disabled-metrics': annotationValue, + }, + }), + }, + }; + } + + it('returns true when metric is in app-config disabledMetrics', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + disabledMetrics: [providerId], + }); + const entity = createEntity(); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by app-config: ${providerId}`, + ); + }); + + it('returns true when metric is in app-config disabledMetrics even if also in includeMetrics (disabled wins)', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + includeMetrics: [providerId], + disabledMetrics: [providerId], + }); + const entity = createEntity(); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by app-config: ${providerId}`, + ); + }); + + it('returns false when disabled by annotation but included by app-config (include overrides annotation)', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + includeMetrics: [providerId], + }); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(false); + expect(mockLogger.info).toHaveBeenCalledWith( + `Entity override: metric disabled by annotation but in entityOverrides.disabledMetrics.except (must run): ${providerId}`, + ); + }); + + it('returns true when disabled by annotation and not in app-config includeMetrics', () => { + const taskWithConfig = createTaskWithScorecardConfig(); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by annotation: ${providerId}`, + ); + }); + + it('returns true when disabled by annotation and app-config includeMetrics exists but does not contain this metric', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + includeMetrics: ['other_metric'], + }); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by annotation: ${providerId}`, + ); + }); + + it('parses comma-separated annotation and excludes when providerId is in the list', () => { + const taskWithConfig = createTaskWithScorecardConfig(); + const entity = createEntity('github.test_metric,openssf.maintained'); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by annotation: ${providerId}`, + ); + }); + + it('returns false when not disabled by app-config or annotation', () => { + const taskWithConfig = createTaskWithScorecardConfig(); + const entity = createEntity(); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(false); + }); + + it('returns true when entityOverrides.disabledMetrics.enabled is false and metric is in entity annotation (union of app-config and entity list)', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + entityOverrides: { + disabledMetrics: { enabled: false }, + }, + }); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by annotation (entity overrides disabled): ${providerId}`, + ); + }); + + it('returns false when entityOverrides.disabledMetrics.enabled is false and metric is not in entity annotation', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + entityOverrides: { + disabledMetrics: { enabled: false }, + }, + }); + const entity = createEntity(); // no annotation + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(false); + }); + + it('returns true when enabled is false with nested config and entity annotation (getOptionalConfig path)', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + entityOverrides: { + disabledMetrics: { + enabled: false, + except: [providerId], + }, + }, + }); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + `Disabled metric by annotation (entity overrides disabled): ${providerId}`, + ); + }); + + it('returns false when disabled by annotation but metric is in entityOverrides.disabledMetrics.except', () => { + const taskWithConfig = createTaskWithScorecardConfig({ + entityOverrides: { + disabledMetrics: { enabled: true, except: [providerId] }, + }, + }); + const entity = createEntity(providerId); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(false); + expect(mockLogger.info).toHaveBeenCalledWith( + `Entity override: metric disabled by annotation but in entityOverrides.disabledMetrics.except (must run): ${providerId}`, + ); + }); + + it('returns false when no config and no annotation', () => { + const taskWithConfig = createTaskWithScorecardConfig(); + const entity = createEntity(); + + const result = (taskWithConfig as any).isMetricIdExcluded( + providerId, + entity, + mockLogger, + ); + + expect(result).toBe(false); + }); + }); + describe('getScheduleFromConfig', () => { it('should return the default schedule if not configured', () => { const config = (task as any).getScheduleFromConfig( @@ -304,6 +565,48 @@ describe('PullMetricsByProviderTask', () => { ); }); + it('should skip entities when scorecard.io/disabled-metrics annotation contains the provider id', async () => { + const entityExcluded = { + apiVersion: '1.0.0', + kind: 'Component', + metadata: { + name: 'excluded-entity', + annotations: { + 'scorecard.io/disabled-metrics': 'github.test_metric', + }, + }, + }; + const entityIncluded = { + apiVersion: '1.0.0', + kind: 'Component', + metadata: { name: 'included-entity' }, + }; + mockCatalog.queryEntities.mockReset().mockResolvedValueOnce({ + items: [entityExcluded, entityIncluded], + pageInfo: { nextCursor: undefined }, + totalItems: 2, + }); + + const calculateMetricSpy = jest.spyOn(mockProvider, 'calculateMetric'); + const createMetricValuesSpy = jest.spyOn( + mockDatabaseMetricValues, + 'createMetricValues', + ); + await (task as any).pullProviderMetrics(mockProvider, mockLogger); + + expect(calculateMetricSpy).toHaveBeenCalledTimes(1); + expect(calculateMetricSpy).toHaveBeenCalledWith(entityIncluded); + expect(createMetricValuesSpy).toHaveBeenCalledTimes(1); + expect(createMetricValuesSpy).toHaveBeenCalledWith([ + expect.objectContaining({ + catalog_entity_ref: 'component:default/included-entity', + metric_id: 'github.test_metric', + value: 42, + status: 'success', + }), + ]); + }); + it('should throw error if pullProviderMetrics fails', async () => { (task as any).pullProviderMetrics = jest .fn() 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 c1844a3141..1a65a4d186 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts @@ -27,7 +27,7 @@ import { CatalogService } from '@backstage/plugin-catalog-node'; import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node'; import { mergeEntityAndProviderThresholds } from '../../utils/mergeEntityAndProviderThresholds'; import { v4 as uuid } from 'uuid'; -import { stringifyEntityRef } from '@backstage/catalog-model'; +import { stringifyEntityRef, Entity } from '@backstage/catalog-model'; import { DbMetricValueCreate } from '../../database/types'; import { SchedulerOptions, SchedulerTask } from '../types'; import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; @@ -113,6 +113,100 @@ export class PullMetricsByProviderTask implements SchedulerTask { : PullMetricsByProviderTask.DEFAULT_SCHEDULE; } + /** + * Check if the metric is disabled by app-config or entity annotation. + * 1. Top-level disabledMetrics: always disabled, entity cannot override. + * 2. entityOverrides.disabledMetrics.enabled: if false, entity annotation is ignored. + * 3. entityOverrides.disabledMetrics.except (or includeMetrics): when entity override is allowed, these metrics cannot be disabled by entity. + * @param providerId - The ID of the provider + * @param entity - The entity to check + * @param logger - The logger to use + * @returns true if the metric is disabled, false otherwise + */ + private isMetricIdExcluded( + providerId: string, + entity: Entity, + logger: LoggerService, + ): boolean { + const disabledMetricsFromAppConfig = this.config.getOptionalStringArray( + 'scorecard.disabledMetrics', + ); + logger.debug( + `Loaded scorecard.disabledMetrics from app-config: ${JSON.stringify( + disabledMetricsFromAppConfig, + )}`, + ); + + const isDisabledByAppConfig = + disabledMetricsFromAppConfig?.includes(providerId) ?? false; + + // if the metric is disabled by app-config, always disabled (entity cannot override) + if (isDisabledByAppConfig) { + logger.info(`Disabled metric by app-config: ${providerId}`); + return true; + } + + // entityOverrides.disabledMetrics: when false, entity list still applied (union) but entity cannot override to re-enable + const entityOverridesDisabledMetricsConfig = this.config.getOptionalConfig( + 'scorecard.entityOverrides.disabledMetrics', + ); + const entityOverrideEnabled = + entityOverridesDisabledMetricsConfig?.getOptionalBoolean('enabled'); + const entityOverrideExcept = + entityOverridesDisabledMetricsConfig?.getOptionalStringArray('except'); + const includeMetricsFromAppConfig = this.config.getOptionalStringArray( + 'scorecard.includeMetrics', + ); + const exceptList = entityOverrideExcept ?? includeMetricsFromAppConfig; + logger.debug( + `Loaded entityOverrides.disabledMetrics (enabled=${entityOverrideEnabled}, except=${JSON.stringify( + exceptList, + )}) and includeMetrics (deprecated): ${JSON.stringify( + includeMetricsFromAppConfig, + )}`, + ); + + const disabledMetricsFromComponentAnnotation = + entity.metadata.annotations?.['scorecard.io/disabled-metrics'] + ?.split(',') + .map((s: string) => s.trim()); + logger.debug( + `Loaded scorecard.io/disabled-metrics annotation: ${JSON.stringify( + disabledMetricsFromComponentAnnotation, + )}`, + ); + + const isInExceptList = exceptList?.includes(providerId) ?? false; + const isDisabledByAnnotation = + disabledMetricsFromComponentAnnotation?.includes(providerId) ?? false; + + // when entity overrides are disabled (enabled === false): apply both app-config and entity list (union) — entity cannot override to re-enable + if (entityOverrideEnabled === false) { + if (isDisabledByAnnotation) { + logger.info( + `Disabled metric by annotation (entity overrides disabled): ${providerId}`, + ); + return true; + } + return false; + } + + // when entity overrides are allowed (enabled !== false): except list = metrics entity cannot disable + if (isDisabledByAnnotation && isInExceptList) { + logger.info( + `Entity override: metric disabled by annotation but in entityOverrides.disabledMetrics.except (must run): ${providerId}`, + ); + return false; + } + + if (isDisabledByAnnotation && !isInExceptList) { + logger.info(`Disabled metric by annotation: ${providerId}`); + return true; + } + + return false; + } + private async pullProviderMetrics( provider: MetricProvider, logger: LoggerService, @@ -142,6 +236,16 @@ export class PullMetricsByProviderTask implements SchedulerTask { let value: MetricValue | undefined; try { + if ( + this.isMetricIdExcluded( + provider.getProviderId(), + entity, + logger, + ) + ) { + return undefined; + } + value = await provider.calculateMetric(entity); const thresholds = mergeEntityAndProviderThresholds( @@ -181,7 +285,7 @@ export class PullMetricsByProviderTask implements SchedulerTask { }), ).then(promises => promises.reduce((acc, curr) => { - if (curr.status === 'fulfilled') { + if (curr.status === 'fulfilled' && curr.value !== undefined) { return [...acc, curr.value]; } return acc;