Skip to content

feat(scorecard): disable annotations customization via config - #4160

Open
dzemanov wants to merge 4 commits into
redhat-developer:mainfrom
dzemanov:scorecard-disable-thresholds-overrides
Open

feat(scorecard): disable annotations customization via config#4160
dzemanov wants to merge 4 commits into
redhat-developer:mainfrom
dzemanov:scorecard-disable-thresholds-overrides

Conversation

@dzemanov

@dzemanov dzemanov commented Aug 4, 2026

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

  • Adds global disable of customization by entity annotations with scorecard.entityAnnotations.enabled in app-config
  • Disable or restrict threshold customization by entity annotations with scorecard.entityAnnotations.thresholds in app-config
  • Follows pattern introduced for disabled metrics in feat(scorecard): Implement exclude metrics annotation  #2393

Example configuration:

scorecard:
  entityAnnotations:
    # Global switch: if false, all scorecard entity annotations are ignored
    # Default: true.
    enabled: true
    thresholds:
      # If false, threshold override annotations have no effect. Default: true.
      enabled: true
      # When thresholds.enabled is true: metric IDs listed here cannot have
      # thresholds customized via entity annotations.
      except:
        - github.openPRs

Fixes

Fixes https://redhat.atlassian.net/browse/RHIDP-11724

How to test

Navigate to github-scorecard-only.yaml to see the result of applied changes.

1. global disable

Uncomment annotations in github-scorecard-only.yaml for disabled metrics and thresholds customization:

annotations:
    scorecard.io/disabled-metrics: github.openPRs
    scorecard.io/github.openPRs.thresholds.rules.warning: '10-25'
    scorecard.io/github.openPRs.thresholds.rules.error: '>25'

Add to app-config:

scorecard:
  entityAnnotations:
    enabled: false

Entity annotations are not applied, github.openPRs metric runs and default number thresholds are used.

2. global enable for disabled metrics annotations
Enable global entity annotations

scorecard:
  entityAnnotations:
    enabled: true

Entity annotations are applied, github.openPRs metric doesn't run.

3. global enable for threshold annotations
Comment out disabling metric:

annotations:
    # scorecard.io/disabled-metrics: github.openPRs

Custom entity threshold annotations are applied.

4. Disable threshold annotations
Disable threshold annotations:

scorecard:
  entityAnnotations:
    thresholds:
      enabled: false

Custom entity threshold annotations are not applied, using default number thresholds.

5. Enable threshold annotations

scorecard:
  entityAnnotations:
    thresholds:
      enabled: true

Custom entity threshold annotations are applied.

6. Threshold annotations override custom app-config annotations

scorecard
  metricProviders:
    github:
      openPRs:
        schedule:
          frequency: { minutes: 5 }
          timeout: { minutes: 10 }
          initialDelay: { seconds: 10 }
        thresholds:
          rules:
            - key: low
              expression: '<10'
              icon: star
              color: '#B4D3B2'
            - key: warning
              expression: '10-1000'
            - key: error
              expression: '>1000'

Comment out threshold annotations to see custom annotations from app-config are applied:

    # scorecard.io/github.openPRs.thresholds.rules.warning: '10-25'
    # scorecard.io/github.openPRs.thresholds.rules.error: '>25'

7. Disable threshold annotations for specific metric

scorecard:
  entityAnnotations:
    thresholds:
      enabled: true
      except:
        - github.openPRs

Custom entity threshold annotations are not applied, using default number thresholds.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Assisted-By: Cursor Desktop
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-scorecard-backend workspaces/scorecard/plugins/scorecard-backend minor v4.1.0

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:08 AM UTC · Completed 10:24 AM UTC
Commit: 16fca54 · View workflow run →

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

feat(scorecard): allow disabling threshold annotation overrides via config

✨ Enhancement 📝 Documentation 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Adds global scorecard.entityAnnotations.enabled switch to disable all entity annotation
 overrides.
• Adds scorecard.entityAnnotations.thresholds.enabled/except to disable or restrict threshold
 override annotations per metric.
• Applies the same global switch to disabledMetrics annotation handling for consistency.
• Extends docs, example config, and tests to cover the new precedence rules.
Diagram

graph TD
  A["app-config: scorecard.entityAnnotations"] --> B["thresholdAnnotations.ts"] --> C["ThresholdResolver.resolveEntityThresholds"]
  A --> D["metricUtils.isMetricIdDisabled"]
  E["Entity annotations"] --> C
  E --> D
  C --> F["Metric threshold evaluation"]
  D --> G["Metric check execution"]

  subgraph Legend
    direction LR
    _cfg["Config source"] ~~~ _logic(["Decision logic"]) ~~~ _out["Runtime effect"]
  end
Loading
High-Level Assessment

The PR reuses the exact pattern already established for disabledMetrics annotations (a global enabled switch plus a per-feature enabled/except sub-config), which keeps the config schema consistent and predictable for administrators. A bespoke or unified 'annotation policy' abstraction was considered implicitly but would add complexity without clear benefit given only two annotation types exist today; the current approach is appropriate.

Files changed (12) +519 / -73

Enhancement (4) +91 / -1
config.d.tsExtend Config typings with entityAnnotations.enabled and thresholds +27/-1

Extend Config typings with entityAnnotations.enabled and thresholds

• Adds enabled global flag and a new thresholds sub-config (enabled/except) to the entityAnnotations config type, with updated JSDoc.

workspaces/scorecard/plugins/scorecard-backend/config.d.ts

ThresholdResolver.tsGate entity threshold annotation merging behind new config check +4/-0

Gate entity threshold annotation merging behind new config check

• Calls areThresholdAnnotationOverridesAllowed before merging entity annotation thresholds, falling back to resolved metric thresholds when disallowed.

workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.ts

thresholdAnnotations.tsAdd areThresholdAnnotationOverridesAllowed helper +53/-0

Add areThresholdAnnotationOverridesAllowed helper

• New function determining whether threshold entity annotations should be honored based on entityAnnotations.enabled, thresholds.enabled and thresholds.except config.

workspaces/scorecard/plugins/scorecard-backend/src/threshold/thresholdAnnotations.ts

metricUtils.tsHonor global entityAnnotations.enabled switch in isMetricIdDisabled +7/-0

Honor global entityAnnotations.enabled switch in isMetricIdDisabled

• Short-circuits annotation-based metric disabling when scorecard.entityAnnotations.enabled is false, before evaluating disabledMetrics-specific config.

workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts

Tests (3) +344 / -61
ThresholdResolver.test.tsAdd tests for threshold annotation override gating +149/-60

Add tests for threshold annotation override gating

• Refactors duplicated threshold fixtures into shared constants and adds tests covering entityAnnotations.enabled, thresholds.enabled, and except-list scenarios.

workspaces/scorecard/plugins/scorecard-backend/src/threshold/ThresholdResolver.test.ts

thresholdAnnotations.test.tsAdd unit tests for areThresholdAnnotationOverridesAllowed +145/-0

Add unit tests for areThresholdAnnotationOverridesAllowed

• New test file covering global enabled flag, thresholds.enabled flag, and except-list combinations for the new helper function.

workspaces/scorecard/plugins/scorecard-backend/src/threshold/thresholdAnnotations.test.ts

metricUtils.test.tsAdd tests for global entityAnnotations.enabled interaction with disabledMetrics +50/-1

Add tests for global entityAnnotations.enabled interaction with disabledMetrics

• Adds test cases verifying that entityAnnotations.enabled=false disables annotation-based metric disabling regardless of disabledMetrics settings.

workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.test.ts

Documentation (3) +54 / -11
threshold-annotation-controls.mdAdd changeset for threshold annotation controls +5/-0

Add changeset for threshold annotation controls

• Documents the minor version bump for scorecard-backend introducing entityAnnotations.thresholds and the global entityAnnotations.enabled switch.

workspaces/scorecard/.changeset/threshold-annotation-controls.md

disabled-metrics-logic.mdDocument interaction between global entityAnnotations.enabled and disabledMetrics +17/-11

Document interaction between global entityAnnotations.enabled and disabledMetrics

• Updates evaluation order description and truth table to include the new global entityAnnotations.enabled switch.

workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md

thresholds.mdDocument threshold annotation override controls +32/-0

Document threshold annotation override controls

• Adds a new section explaining scorecard.entityAnnotations.thresholds config, its precedence table, and summary of behavior.

workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md

Other (2) +30 / -0
app-config.yamlDocument new entityAnnotations config options as commented examples +27/-0

Document new entityAnnotations config options as commented examples

• Adds commented example configuration showing disabledMetrics, entityAnnotations.enabled, entityAnnotations.thresholds, and entityAnnotations.disabledMetrics usage.

workspaces/scorecard/app-config.yaml

github-scorecard-only.yamlAdd commented example annotations for testing disabled metrics/thresholds +3/-0

Add commented example annotations for testing disabled metrics/thresholds

• Adds commented-out entity annotations demonstrating disabled-metrics and threshold override annotations for manual testing.

workspaces/scorecard/examples/components/github-scorecard-only.yaml

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 11 rules
✅ Cross-repo context
  Explored: repo: redhat-developer/rhdh-local (sha: 00e76453)
  Explored: repo: redhat-developer/rhdh (sha: 3e783f2f)
  Not relevant to this PR: redhat-developer/rhdh-chart
  Not relevant to this PR: redhat-developer/rhdh-operator

Grey Divider


Remediation recommended

1. Wrong docs config key 🐞 Bug ⚙ Maintainability
Description
In disabled-metrics docs, the evaluation-order paragraph refers to
scorecard.entityAnnotations.disabledMetrics as if it were a boolean flag, but the implementation
expects an object and checks scorecard.entityAnnotations.disabledMetrics.enabled. This mismatch
can mislead admins and conflicts with the correct table/summary in the same document.
Code

workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md[5]

+**Evaluation order:** `scorecard.disabledMetrics` is checked first. If the metric ID is in that list, the metric check is always skipped and the rest is ignored. If `scorecard.entityAnnotations.enabled` is `false` (all scorecard entity annotations are ignored) or `scorecard.entityAnnotations.disabledMetrics` is false, users are unable to disable metrics using entity annotations. If enabled, entity annotations for disabled metrics are applied.
Relevance

●●● Strong

Docs correctness fixes in scorecard-backend are typically accepted; wrong config key/type would be
treated as a doc bug.

PR-#4022
PR-#3526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The docs prose uses an incorrect config key/type (disabledMetrics as a boolean), while the code
and config typing clearly treat disabledMetrics as an object with an enabled boolean.

workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md[3-6]
workspaces/scorecard/plugins/scorecard-backend/src/utils/metricUtils.ts[53-67]
workspaces/scorecard/plugins/scorecard-backend/config.d.ts[87-93]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`docs/disabled-metrics-logic.md` says metric disabling via annotations is blocked when `scorecard.entityAnnotations.disabledMetrics` is false, but the actual config toggle is `scorecard.entityAnnotations.disabledMetrics.enabled`.

### Issue Context
The implementation reads `scorecard.entityAnnotations.disabledMetrics` as a config object and checks its `enabled` field.

### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md[3-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request Tests labels Aug 4, 2026
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.12%. Comparing base (3af0fb2) to head (5459674).
⚠️ Report is 17 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4160   +/-   ##
=======================================
  Coverage   58.11%   58.12%           
=======================================
  Files        2422     2423    +1     
  Lines       96490    96509   +19     
  Branches    26892    26891    -1     
=======================================
+ Hits        56079    56098   +19     
  Misses      40168    40168           
  Partials      243      243           
Flag Coverage Δ *Carryforward flag
adoption-insights 84.55% <ø> (ø) Carriedforward from 222e303
ai-integrations 69.76% <ø> (ø) Carriedforward from 222e303
app-defaults 69.79% <ø> (ø) Carriedforward from 222e303
augment 46.67% <ø> (ø) Carriedforward from 222e303
boost 76.77% <ø> (ø) Carriedforward from 222e303
bulk-import 72.56% <ø> (ø) Carriedforward from 222e303
cost-management 13.55% <ø> (ø) Carriedforward from 222e303
dcm 60.72% <ø> (ø) Carriedforward from 222e303
extensions 56.59% <ø> (ø) Carriedforward from 222e303
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 222e303
global-header 66.50% <ø> (ø) Carriedforward from 222e303
homepage 47.50% <ø> (ø) Carriedforward from 222e303
install-dynamic-plugins 59.95% <ø> (ø) Carriedforward from 222e303
intelligent-assistant 74.61% <ø> (ø) Carriedforward from 222e303
konflux 91.98% <ø> (ø) Carriedforward from 222e303
lightspeed 69.02% <ø> (ø) Carriedforward from 222e303
mcp-integrations 83.40% <ø> (ø) Carriedforward from 222e303
orchestrator 66.87% <ø> (ø) Carriedforward from 222e303
quickstart 63.74% <ø> (ø) Carriedforward from 222e303
sandbox 79.56% <ø> (ø) Carriedforward from 222e303
scorecard 86.05% <100.00%> (+0.06%) ⬆️
theme 88.52% <ø> (ø) Carriedforward from 222e303
translations 5.12% <ø> (ø) Carriedforward from 222e303
x2a 79.20% <ø> (ø) Carriedforward from 222e303

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3af0fb2...5459674. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [code-organization] workspaces/scorecard/plugins/scorecard-backend/src/threshold/thresholdAnnotations.ts — The global entityAnnotations.enabled check (config.getOptionalBoolean('scorecard.entityAnnotations.enabled')) is duplicated verbatim in both areThresholdAnnotationOverridesAllowed() and isMetricIdDisabled(). Consider extracting a shared areEntityAnnotationsEnabled(config) helper to centralize this guard, especially if more annotation types are added in the future.

  • [test-helper-consistency] workspaces/scorecard/plugins/scorecard-backend/src/threshold/thresholdAnnotations.test.ts — The sibling test file metricUtils.test.ts uses a createConfig() factory to reduce boilerplate. The new test file inlines the full mockServices.rootConfig(...) wrapper 8 times. Consider extracting a similar helper for consistency.

Previous run

Looks good to me

Previous run (2)

Review

Findings

Low

  • [naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/threshold/thresholdAnnotations.ts:32 — Variable thresholdsAnnotationConfig uses a different naming pattern than the analogous entityAnnotationsDisabledMetricsConfig in metricUtils.ts. The name starts with thresholds instead of entityAnnotations and uses singular Annotation instead of plural. Consider renaming to entityAnnotationsThresholdsConfig for cross-file consistency.

  • [stale-doc] workspaces/scorecard/plugins/scorecard-backend/README.md:115 — The "Disabling Metrics" summary section doesn't mention the new scorecard.entityAnnotations.enabled global switch. The detailed disabled-metrics-logic.md is updated in this PR, but the README summary could note the configuration control.

  • [stale-doc] workspaces/scorecard/plugins/scorecard-backend/README.md:123 — The Thresholds section lists entity annotation overrides without noting they can now be disabled via scorecard.entityAnnotations configuration. The detailed thresholds.md is updated in this PR, but the README summary could add a qualifier.

  • [stale-doc] workspaces/scorecard/AGENTS.md:153 — The "Threshold resolution priority" section doesn't mention the new areThresholdAnnotationOverridesAllowed gating check that can prevent entity annotation overrides from being applied.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 4, 2026
Assisted-By: Cursor Desktop
Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 11:24 AM UTC · Ended 11:27 AM UTC
Commit: 96e7594 · View workflow run →

@dzemanov
dzemanov requested a review from imykhno August 4, 2026 11:27
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:28 AM UTC · Completed 11:42 AM UTC
Commit: 222e303 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@imykhno imykhno left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to ask are we planning to implement rule-level enforced? It was one of AC "Enforced rules ignore overrides".
Flag on each threshold rule may be following:

scorecard:
  metricProviders:
    dependabot:
      alertsCritical:
        thresholds:
          rules:
            - key: success
              expression: '<1'
              # enforced omitted → customizable (backward compatible)
            - key: warning
              expression: '1-7'
            - key: error
              expression: '>7'
              enforced: true

Comment thread workspaces/scorecard/examples/components/github-scorecard-only.yaml
Comment thread workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md
Comment thread workspaces/scorecard/plugins/scorecard-backend/docs/disabled-metrics-logic.md Outdated
@imykhno

imykhno commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Quick follow-up: did we discuss showing locked thresholds in the UI? It would help users understand why attribute customization isn't working.

@dzemanov

dzemanov commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

I would like to ask are we planning to implement rule-level enforced? It was one of AC "Enforced rules ignore overrides".

Thank you for raising this question, I will ask PM if it is required also on individual rule level and if not remove it from AC, otherwise create a separate task for it.

The original Feature request was about forcing metric checks to always run (was done as part of OpenSSF Scorecard https://redhat.atlassian.net/browse/RHIDP-12183, so this is following the same syntax) and to prevent users from overriding thresholds for checks via entity annotations.

Personally I think having the option to set each individual rule as enforced is unnecessary and enforcing all rules per check as a whole is enough, since in most cases, you would need to anyway override all rules via entity annotations to cover the whole real line (e.g moving low from <1 to <4 will force you to also override middle rule from 1-7 to e.g. 4-7). This makes locking per rule less useful and potentially confusing. However, thank you for catching this, we should double-check the granularity level.

@dzemanov

dzemanov commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Quick follow-up: did we discuss showing locked thresholds in the UI? It would help users understand why attribute customization isn't working.

We did not. Let me get back on this with answer about the granularity

Signed-off-by: Dominika Zemanovicova <dzemanov@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 6, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 9:41 AM UTC · Ended 9:57 AM UTC
Commit: 5459674 · View workflow run →

@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

metricId: string,
): boolean {
const entityAnnotationsGlobalEnabled = config.getOptionalBoolean(
'scorecard.entityAnnotations.enabled',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] code-organization

The global entityAnnotations.enabled check is duplicated verbatim in both areThresholdAnnotationOverridesAllowed() (thresholdAnnotations.ts) and isMetricIdDisabled() (metricUtils.ts). Consider extracting a shared areEntityAnnotationsEnabled(config) helper to centralize this guard.

Suggested fix: Extract a shared helper (e.g., areEntityAnnotationsEnabled(config: Config): boolean) into a utility file and call it from both areThresholdAnnotationOverridesAllowed and isMetricIdDisabled.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:41 AM UTC · Completed 9:57 AM UTC
Commit: 5459674 · View workflow run →

@imykhno

imykhno commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

I was testing changes locally:

  • Global disable: works as expected ✅
  • Global enable for disabled metrics annotations: works as expected ✅
  • Disable threshold annotations: works as expected ✅
  • Enable threshold annotations: works as expected ✅
  • Threshold annotations override custom app-config annotations: works as expected ✅
  • Disable threshold annotations for specific metric: works as expected ✅
  • Enable threshold annotations for specific metric: doesn't work as expected ❌
    • If I set thresholds.enabled: false with thresholds.except: - github.openPRs, customization should be off globally, except for github.openPRs. Is that right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request ready-for-merge All reviewers approved — ready to merge Tests workspace/scorecard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants