-
Notifications
You must be signed in to change notification settings - Fork 10
Introduce new Findings Report extension #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
- use instances of `(Enriched)ComponentArtefactId` instead of resource nodes to better integrate into existing filter functions and omit custom deduplication logic (use built-in `set` and `__eq__` functions instead) - re-use existing functions to iterate over GitHub issues - consume configuration via extensions-cfg instead of script parameter - also consider the `extraIdentity` in identity checks - factor-out finding type into configuration - create (and optionally auto-merge) a PR containing the updated report instead of a direct push Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
Signed-off-by: Jonas Brand (8R0WNI3) <[email protected]>
| branch: gh-pages | ||
| # @param extensions_cfg.findings_report.mappings[].filename the relativ path in the target | ||
| # repository to a file containing the overview report | ||
| # the report to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment looks truncated 🤔
| if sprint >= due_date: | ||
| return sprint | ||
|
|
||
| logger.warning(f'could not determine target sprint for {due_date=})') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is an extra closing parenthesis
| def sprint_dates( | ||
| delivery_service_client: delivery.client.DeliveryServiceClient, | ||
| date_name: str='release_decision', | ||
| ) -> tuple[datetime.date]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a tuple of dates with variable length this should be tuple[datetime.date, ...]
| ): | ||
| continue | ||
|
|
||
| if component_artefact_id.artefact.artefact_type == ocm.ArtefactType.OCI_IMAGE: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given the guard above, isn't this condition always true? if we pass the
continue, don’t we already know artefact_type == OCI_IMAGE?
| def filter_issues_for_labels( | ||
| issues: collections.abc.Iterable[github3.issues.ShortIssue], | ||
| labels: collections.abc.Iterable[str], | ||
| ) -> tuple[github3.issues.ShortIssue]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tuple[github3.issues.ShortIssue, ...]
|
|
||
| logger.info(f'{rate_limit_remaining=} {rate_limit=}') | ||
|
|
||
| return rate_limit_remaining < relative_gh_quota_minimum * rate_limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if rate_limti().resources.core.limit is missing, we fall back to -1. in that case the comparison becomes remaining < relative_min * -1, which is always false for a non-negative remaining, no? is that intended?
|
|
||
| # Artefact scan overview | ||
|
|
||
| Between ${start_date.isoformat()} and ${end_date.isoformat()}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can start_date / end_date be None here? in generate_report_for_component_and_finding_type
we set them to None if component.time_range is missing.
| # Detailed scan list | ||
|
|
||
| % for component_version in component_versions: | ||
| * [`${component_name}:${component_version}`](${f'./{reports_dirname}/{component_version}'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the links might be missing the .mdsuffix 🤔
we create the per-version reports as <version>.md: with open(os.path.join(reports_dirname, f'{version}.md'), ...)
but the template links to: ./${reports_dirname}/${component_version}
should this be ./${reports_dirname}/${component_version}.md 🤔
What this PR does / why we need it:
This pull request redesigns the existing report generator for findings of type
finding/malwareas a Kubernetes Cronjob based ODG extension. Therefore, several changes were made (including but not limited to):ResourceNodewith(Enriched)ComponentArtefactIdto better integrate into existing filter functions within the ODG context and allow for built-in deduplication means (viasetand__eq__functions). In particular, this also allowed to use stricter checks (i.e. also consider theartefact_extra_id) to create a more detailed report.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
As discussed out-of-bands, due to the stricter checks, there is not always a suitable GitHub issue which can be linked (i.e. if an older rescoring with broader scope had already existed and no GitHub issue has been created). Therefore, the report is enhanced with a reference to the Delivery-Dashboard to be able to retrieve the assessment.
Release note: