Skip to content
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

Replace ScanOverview.__new__ magic with Pydantic Validator #4

Open
pederhan opened this issue Aug 19, 2022 · 0 comments
Open

Replace ScanOverview.__new__ magic with Pydantic Validator #4

pederhan opened this issue Aug 19, 2022 · 0 comments
Assignees

Comments

@pederhan
Copy link
Member

pederhan commented Aug 19, 2022

Frankly, the use of __new__ on models.ScanOverview is both difficult to reason with and difficult to explain in the docs.

https://github.com/pederhan/harborapi/blob/002ff74abc52293e924d6c91c6aaf44cb9a60dd5/harborapi/models/models.py#L260-L271

This should be changed to make it easier to document and maintain get_artifact(..., with_scan_overview=True), as well as the classes models.Artifact, models.ScanOverview and models.NativeReportSummary.

Why is it like this?

Well, ask the Harbor developers. A ScanOverview seems to contain a dict of mime types as keys, and values that conform to the NativeReport spec. This behavior is not documented. We make an assumption that when a user requests a scan overview, they actually want a NativeReportSummary, not a dict of dicts.

This has the unfortunate side effect of discarding all but one report if there are reports for multiple mime types (seems unlikely?) Such as the case below:

{
    "application/vnd.scanner.adapter.vuln.report.harbor+json; version=1.0": {
        # dict that conforms to NativeReportSummary spec
        ...
    },
    "application/vnd.security.vulnerability.report; version=1.1": {
        # dict that conforms to NativeReportSummary spec
        ...
    }
}

As such, it might be beneficial to rethink the whole approach to parsing ScanOverview, and see if we can come up with something that is easier to maintain and use until this part of the API is properly documented.

Proposed Solution

Unsure. Need to investigate if an artifact can ever have NativeReportSummary dicts for more than one MIME type simultaneously. Can the example above ever occur?

@pederhan pederhan self-assigned this Aug 22, 2022
@pederhan pederhan added this to Harbor Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant