launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #33079
Re: [Merge] ~ines-almeida/launchpad:uct-export-handler into launchpad:master
Reviewed, left some comments
Diff comments:
> diff --git a/lib/lp/bugs/model/vulnerability.py b/lib/lp/bugs/model/vulnerability.py
> index fd5fba0..1d93b97 100644
> --- a/lib/lp/bugs/model/vulnerability.py
> +++ b/lib/lp/bugs/model/vulnerability.py
> @@ -509,12 +509,19 @@ class VulnerabilitySet:
>
> # Check requester's permissions to handler
> from lp.bugs.scripts.soss.sossexport import SOSSExporter
> + from lp.bugs.scripts.uct.uctexport import UCTExporter
>
> if handler == VulnerabilityHandlerEnum.SOSS:
> distribution = getUtility(IDistributionSet).getByName(
> HANDLER_DISTRIBUTION_MAP[handler]
> )
> exporter = SOSSExporter()
> +
> + elif handler == VulnerabilityHandlerEnum.UCT:
I would ideally remove all `if`/`elif` and only use dicts like the `HANDLER_DISTRIBUTION_MAP` but I think it makes sense to do it in a next iteration.
> + distribution = getUtility(IDistributionSet).getByName(
> + HANDLER_DISTRIBUTION_MAP[handler]
> + )
> + exporter = UCTExporter()
> else:
> raise NotFoundError(f"{handler} not found")
>
> diff --git a/lib/lp/bugs/scripts/uct/models.py b/lib/lp/bugs/scripts/uct/models.py
> index 42331d0..8473ca2 100644
> --- a/lib/lp/bugs/scripts/uct/models.py
> +++ b/lib/lp/bugs/scripts/uct/models.py
> @@ -272,17 +273,11 @@ class UCTRecord(SVTRecord):
>
> return entry
>
> - def save(self, output_dir: Path) -> Path:
> + def to_str(self) -> str:
> """
> - Save UCTRecord to a file in UCT format.
> + Export UCTRecord to a yaml str format.
UCT does not use a valid yaml, it uses a custom format
> """
> - if not output_dir.is_dir():
> - raise ValueError(
> - "{} does not exist or is not a directory", output_dir
> - )
> - output_path = output_dir / self.parent_dir / self.candidate
> - output_path.parent.mkdir(exist_ok=True)
> - output = open(str(output_path), "w")
> + output = StringIO()
> if self.public_date_at_USN:
> self._write_field(
> "PublicDateAtUSN",
> diff --git a/lib/lp/bugs/scripts/uct/tests/test_uct.py b/lib/lp/bugs/scripts/uct/tests/test_uct.py
> index 7b022c0..c5c0ccd 100644
> --- a/lib/lp/bugs/scripts/uct/tests/test_uct.py
> +++ b/lib/lp/bugs/scripts/uct/tests/test_uct.py
> @@ -881,6 +889,127 @@ class TestUCTImporterExporter(TestCaseWithFactory):
> ],
> global_tags={"cisa-kev"},
> )
> +
> + self.uct_record = UCTRecord(
> + assigned_to=assignee.name,
> + bugs=["https://github.com/mm2/Little-CMS/issues/29";],
> + candidate="CVE-2022-23222",
> + crd=datetime(2020, 1, 14, 8, 15, tzinfo=timezone.utc),
> + cvss=[
> + CVSS(
I will need to rebase my wip MP and change this also to python dicts instead of CVSS named tuples. Just to keep in mind it :)
> + authority="nvd",
> + vector_string=(
> + "CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H "
> + "[7.8 HIGH]"
> + ),
> + ),
> + ],
> + description="description",
> + discovered_by="tr3e wang",
> + global_tags={"cisa-kev"},
> + mitigation="mitigation",
> + notes="author> text",
> + packages=[
> + UCTRecord.Package(
> + name=self.ubuntu_package.sourcepackagename.name,
> + statuses=[
> + UCTRecord.SeriesPackageStatus(
> + series="focal",
> + status=UCTRecord.PackageStatus.RELEASED,
> + reason="released",
> + priority=UCTRecord.Priority.HIGH,
> + ),
> + UCTRecord.SeriesPackageStatus(
> + series="jammy",
> + status=UCTRecord.PackageStatus.DOES_NOT_EXIST,
> + reason="does not exist",
> + priority=None,
> + ),
> + UCTRecord.SeriesPackageStatus(
> + series="devel",
> + status=UCTRecord.PackageStatus.NOT_AFFECTED,
> + reason="not affected",
> + priority=None,
> + ),
> + UCTRecord.SeriesPackageStatus(
> + series="upstream",
> + status=UCTRecord.PackageStatus.RELEASED,
> + reason="fix released",
> + priority=UCTRecord.Priority.HIGH,
> + ),
> + ],
> + priority=UCTRecord.Priority.LOW,
> + tags={"review-break-fix"},
> + patches=[
> + UCTRecord.Patch(
> + patch_type="upstream",
> + entry=(
> + "https://github.com/389ds/389-ds-base/commit/123 (1.4.4)" # noqa: E501
> + ),
> + ),
> + UCTRecord.Patch(
> + patch_type="break-fix",
> + entry=(
> + "457f44363a8894135c85b7a9afd2bd8196db24ab "
> + "c25b2ae136039ffa820c26138ed4a5e5f3ab3841|"
> + "local-CVE-2022-23222-fix"
> + ),
> + ),
> + ],
> + ),
> + UCTRecord.Package(
> + name=self.esm_package.sourcepackagename.name,
> + statuses=[
> + UCTRecord.SeriesPackageStatus(
> + series="precise/esm",
> + status=UCTRecord.PackageStatus.IGNORED,
> + reason="ignored",
> + priority=None,
> + ),
> + UCTRecord.SeriesPackageStatus(
> + series="trusty/esm",
> + status=UCTRecord.PackageStatus.NEEDS_TRIAGE,
> + reason="needs triage",
> + priority=None,
> + ),
> + UCTRecord.SeriesPackageStatus(
> + series="upstream",
> + status=UCTRecord.PackageStatus.IGNORED,
> + reason="ignored",
> + priority=UCTRecord.Priority.LOW,
> + ),
> + ],
> + priority=None,
> + tags={"universe-binary"},
> + patches=[
> + UCTRecord.Patch(
> + patch_type="upstream",
> + entry=(
> + "https://github.com/389ds/389-ds-base/commit/456"; # noqa: E501
> + ),
> + ),
> + UCTRecord.Patch(
> + patch_type="break-fix",
> + entry=(
> + "457f44363a8894135c85b7a9afd2bd8196db24ab "
> + "c25b2ae136039ffa820c26138ed4a5e5f3ab3841|"
> + "local-CVE-2022-23222-fix"
> + ),
> + ),
> + ],
> + ),
> + ],
> + parent_dir="active",
> + priority=UCTRecord.Priority.MEDIUM,
> + priority_explanation="",
> + public_date=datetime(2022, 1, 14, 8, 15, tzinfo=timezone.utc),
> + public_date_at_USN=datetime(
> + 2021, 1, 14, 8, 15, tzinfo=timezone.utc
> + ),
> + references=["https://ubuntu.com/security/notices/USN-5368-1";],
> + ubuntu_description="ubuntu-description",
> + )
> +
> self.importer = UCTImporter(self.ubuntu)
> self.exporter = UCTExporter()
>
> @@ -1624,14 +1753,10 @@ class TestUCTImporterExporter(TestCaseWithFactory):
>
> def test_import_cve_from_file(self):
> uct_record = self.cve.to_uct_record()
> - import tempfile
>
> - with tempfile.TemporaryDirectory() as tmpdir:
> - cve_path = uct_record.save(Path(tmpdir))
> - self.importer.import_cve_from_file(cve_path)
> + cve_path = uct_record.save(Path(self.makeTemporaryDirectory()))
Thanks to make this simpler!
> + bug, _ = self.importer.import_cve_from_file(cve_path)
>
> - bug = self.importer._find_existing_bug(self.lp_cve, self.ubuntu)
> - self.importer._find_existing_vulnerability(self.lp_cve, self.ubuntu)
> self.checkBug(bug, self.cve)
> self.checkVulnerabilities(bug, self.cve)
>
> diff --git a/lib/lp/bugs/scripts/uct/uctexport.py b/lib/lp/bugs/scripts/uct/uctexport.py
> index dfad576..5b02917 100644
> --- a/lib/lp/bugs/scripts/uct/uctexport.py
> +++ b/lib/lp/bugs/scripts/uct/uctexport.py
> @@ -43,6 +45,32 @@ class UCTExporter(SVTExporter):
> description: str
> references: List[str]
>
> + def to_record(
> + self,
> + bug: BugModel,
> + vulnerability: Vulnerability,
> + ) -> UCTRecord:
> + """
> + Export the bug and vulnerability related to a cve in a distribution
> + and return a `CVE` instance.
It actually returns a `UCTRecord` instance
> +
> + :param bug: `Bug` model
> + :param vulnerability: `Vulnerability` model
> + :return: `CVE` instance
same ^
> + """
> + if bug is None:
> + raise ValueError("Bug can't be None")
> + if vulnerability is None:
> + raise ValueError("Vulnerability can't be None")
> +
> + cve = self._import_cve(bug, vulnerability)
> + return cve.to_uct_record()
> +
> + def checkUserPermissions(self, user, distribution):
> + return SecurityAdminDistribution(distribution).checkAuthenticated(
> + IPersonRoles(user)
> + )
> +
> def export_bug_to_uct_file(
> self, bug_id: int, output_dir: Path
> ) -> Optional[Path]:
--
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/493617
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:uct-export-handler into launchpad:master.
References
