feat(scorecard): Implement baseUrl for fetching openssf data#2301
feat(scorecard): Implement baseUrl for fetching openssf data#2301
Conversation
…nd simplied the usage of the openssf client by fetching scorecards only using a baseUrl
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
| async getScorecard(owner: string, repo: string): Promise<OpenSSFResponse> { | ||
| const apiUrl = `${this.baseUrl}/${this.gitServiceHost}/${owner}/${repo}`; | ||
| async getScorecard(entity: Entity): Promise<OpenSSFResponse> { | ||
| const baseUrl = entity.metadata.annotations?.['openssf/baseUrl'] ?? ''; |
There was a problem hiding this comment.
I would rather take baseUrl from app-config of openssf. @christoph-jerolimov WDYT?
There was a problem hiding this comment.
As we spoke earlier (in our wg), the idea of having it inside the component is to be able to have a different URL (per project) to fetch the openssf-scorecard json file.
There was a problem hiding this comment.
In the lastest commit I have renamed the baseUrl to scorecardUrl.
There was a problem hiding this comment.
Also, an important argument to keep the openssf/scorecardUrl in the component: the scorecardUrl is unique per component, therefore moving this to app-config.yaml doesn't make sense.
There was a problem hiding this comment.
The argument about keeping url in the component due to being unique for the component can be resolved by using other annotations to specify parts of url, for example like github does with github.com/project-slug (but it would make code and setup more complex).
Not sure if exposing whole url is alright, but it is true there are some well known annotations that do that, like source location. The other thing that comes to mind is that if company moves to other domain where it publishes scorecard data, all component annotations will need to be updated.
In case auth is needed, we can in future utilize integrations no matter if we use the whole url in annotations or just part, so that is fine.
There was a problem hiding this comment.
As shared in another comment, maybe align this with https://backstage.io/docs/features/software-catalog/well-known-annotations/ and call it [security-]scorecard-location ? Wdyt?
I think its a fair point to specific a base URL somewhere. But we can start simple and support first just "sources" and a base URL in the app-config later?
| name: openssf-scorecard | ||
| annotations: | ||
| openssf/baseUrl: https://api.securityscorecards.dev/projects/github.com/alizard0/rhdh-plugins | ||
| openssf/scorecardUrl: https://api.securityscorecards.dev/projects/github.com/alizard0/rhdh-plugins |
There was a problem hiding this comment.
@dzemanov I think scorecardUrl is a better name for it.
There was a problem hiding this comment.
Maybe you can align it with others annotations, see https://backstage.io/docs/features/software-catalog/well-known-annotations/ and call it openssf.dev/scorecard-location? Wdyt?
| kind: Component | ||
| metadata: | ||
| name: openssf-scorecard-only | ||
| name: openssf-scorecard |
There was a problem hiding this comment.
Is the name change needed for something? The idea was that it is obvous that this entity has just openssf-scorecard scorecard features enabled.
| - The repository is private (OpenSSF only analyzes public repositories) | ||
| - The repository path in the annotation is incorrect | ||
| - The metric score is lower than -1 or higher than 10. | ||
| 18 metrics from [OpenSSF checks](https://github.com/ossf/scorecard/blob/main/docs/checks.md): `openssf.binary_artifacts`, `openssf.branch_protection`, `openssf.cii_best_practices`, `openssf.ci_tests`, `openssf.code_review`, `openssf.contributors`, `openssf.dangerous_workflow`, `openssf.dependency_update_tool`, `openssf.fuzzing`, `openssf.license`, `openssf.maintained`, `openssf.packaging`, `openssf.pinned_dependencies`, `openssf.sast`, `openssf.security_policy`, `openssf.signed_releases`, `openssf.token_permissions`, `openssf.vulnerabilities`. |
There was a problem hiding this comment.
Why did you replaced the table with this metrics? I personally find a list or table more readable then this comma separated list?
| type: service | ||
| lifecycle: production | ||
| owner: my-team | ||
| openssf/baseUrl: https://api.securityscorecards.dev/projects/github.com/owner/repo |
There was a problem hiding this comment.
I think you have changed this annotation
| For the OpenSSF metrics to work, your catalog entities must have the required annotation: | ||
| | Annotation | Required | Description | | ||
| | ----------------- | -------- | --------------------------------------------------------------------------- | | ||
| | `openssf/baseUrl` | Yes | Full scorecard API URL for this component (e.g. public API or self-hosted). | |
There was a problem hiding this comment.
You changed that annotation
| const apiUrl = `${this.baseUrl}/${this.gitServiceHost}/${owner}/${repo}`; | ||
| async getScorecard(entity: Entity): Promise<OpenSSFResponse> { | ||
| const scorecardUrl = | ||
| entity.metadata.annotations?.['openssf/scorecardUrl'] ?? ''; |
There was a problem hiding this comment.
Can you please create a src/annotations.ts with an export for the annotations?
/**
* @public
*/
export enum OpenSSFAnnotation {
URL = 'openssf/scorecardUrl',
}And I've added above. Mayby this should be called "openssf.dev/scorecard-location"? /cc @dzemanov
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
|




Hey, I just made a Pull Request!
✔️ Checklist