launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29357
[Merge] ~andrey-fedoseev/launchpad:uct-import-export-break-fix into launchpad:master
Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad:uct-import-export-break-fix into launchpad:master with ~andrey-fedoseev/launchpad:bug-presense as a prerequisite.
Commit message:
UCT import/export: handle the `break-fix` entries
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/432181
For each `break-fix` entry create a `BugPresence` instance linked to the default git repository of the project (if exists)
If a `break-fix` entry includes multiple git commits in the "fixed" section it means that either of them fixes the issue. So, we create multiple `BugPresence` instances, one per commit listed in the "fixed" section.
Items such as `local-CVE-2022-23222-fix` are currently ignored
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:uct-import-export-break-fix into launchpad:master.
diff --git a/lib/lp/bugs/model/bugpresence.py b/lib/lp/bugs/model/bugpresence.py
index d4f718b..749145a 100644
--- a/lib/lp/bugs/model/bugpresence.py
+++ b/lib/lp/bugs/model/bugpresence.py
@@ -20,6 +20,9 @@ class BugPresence(StormBase):
"""See `IBugPresence`."""
__storm_table__ = "BugPresence"
+ __storm_order__ = [
+ "id",
+ ]
id = Int(primary=True)
diff --git a/lib/lp/bugs/scripts/tests/test_uct.py b/lib/lp/bugs/scripts/tests/test_uct.py
index 20ae931..6bfe91c 100644
--- a/lib/lp/bugs/scripts/tests/test_uct.py
+++ b/lib/lp/bugs/scripts/tests/test_uct.py
@@ -6,10 +6,12 @@ from typing import List
from pytz import UTC
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.enums import VulnerabilityStatus
+from lp.bugs.interfaces.bugpresence import IBugPresenceSet
from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
from lp.bugs.model.bug import Bug
from lp.bugs.model.bugtask import BugTask
@@ -274,6 +276,20 @@ class TestCVE(TestCaseWithFactory):
tags=set(),
patches=[
UCTRecord.Patch(
+ patch_type="break-fix",
+ entry=(
+ "457f44363a8894135c85b7a9afd2bd8196db24ab "
+ "c25b2ae136039ffa820c26138ed4a5e5f3ab3841|"
+ "4969c06a0d83c9c3dc50b8efcdc8eeedfce896f6"
+ ),
+ ),
+ UCTRecord.Patch(
+ patch_type="break-fix",
+ entry=(
+ "- c25b2ae136039ffa820c26138ed4a5e5f3ab3841"
+ ),
+ ),
+ UCTRecord.Patch(
patch_type="upstream",
entry=(
"https://github.com/389ds/389-ds-base/";
@@ -416,6 +432,30 @@ class TestCVE(TestCaseWithFactory):
importance=None,
status=BugTaskStatus.FIXRELEASED,
status_explanation="reason 4",
+ break_fixes=[
+ CVE.BreakFix(
+ broken_git_commit_sha1=(
+ "457f44363a8894135c85b7a9afd2bd8196db24ab"
+ ),
+ fixed_git_commit_sha1=(
+ "c25b2ae136039ffa820c26138ed4a5e5f3ab3841"
+ ),
+ ),
+ CVE.BreakFix(
+ broken_git_commit_sha1=(
+ "457f44363a8894135c85b7a9afd2bd8196db24ab"
+ ),
+ fixed_git_commit_sha1=(
+ "4969c06a0d83c9c3dc50b8efcdc8eeedfce896f6"
+ ),
+ ),
+ CVE.BreakFix(
+ broken_git_commit_sha1=None,
+ fixed_git_commit_sha1=(
+ "c25b2ae136039ffa820c26138ed4a5e5f3ab3841"
+ ),
+ ),
+ ],
),
CVE.UpstreamPackage(
target=product_2,
@@ -423,6 +463,7 @@ class TestCVE(TestCaseWithFactory):
importance=None,
status=BugTaskStatus.FIXRELEASED,
status_explanation="",
+ break_fixes=[],
),
],
importance=BugTaskImportance.CRITICAL,
@@ -564,6 +605,13 @@ class TestUCTImporterExporter(TestCaseWithFactory):
distroseries=self.esm_current_series,
).productseries.product
+ self.product_1_git_repo = self.factory.makeGitRepository(
+ target=self.product_1, target_default=True
+ )
+ self.product_2_git_repo = self.factory.makeGitRepository(
+ target=self.product_2, target_default=True
+ )
+
for series in (
self.ubuntu_supported_series,
self.ubuntu_current_series,
@@ -668,6 +716,24 @@ class TestUCTImporterExporter(TestCaseWithFactory):
importance=BugTaskImportance.HIGH,
status=BugTaskStatus.FIXRELEASED,
status_explanation="fix released",
+ break_fixes=[
+ CVE.BreakFix(
+ broken_git_commit_sha1=(
+ "054623105728b06852f077299e2bf1bf3d5f2b0b"
+ ),
+ fixed_git_commit_sha1=(
+ "db7bee653859ef7179be933e7d1384644f795f26"
+ ),
+ ),
+ CVE.BreakFix(
+ broken_git_commit_sha1=(
+ "054623105728b06852f077299e2bf1bf3d5f2b0b"
+ ),
+ fixed_git_commit_sha1=(
+ "6e61dc9da0b7a0d91d57c2e20b5ea4fd2d4e7e53"
+ ),
+ ),
+ ],
),
CVE.UpstreamPackage(
target=self.product_2,
@@ -675,6 +741,20 @@ class TestUCTImporterExporter(TestCaseWithFactory):
importance=BugTaskImportance.LOW,
status=BugTaskStatus.WONTFIX,
status_explanation="ignored",
+ break_fixes=[
+ CVE.BreakFix(
+ broken_git_commit_sha1=None,
+ fixed_git_commit_sha1=(
+ "6e61dc9da0b7a0d91d57c2e20b5ea4fd2d4e7e53"
+ ),
+ ),
+ CVE.BreakFix(
+ broken_git_commit_sha1=(
+ "4e9b4a6883dd97aff53ae3b08eb900716a5469dc"
+ ),
+ fixed_git_commit_sha1=None,
+ ),
+ ],
),
],
importance=BugTaskImportance.MEDIUM,
@@ -731,6 +811,7 @@ class TestUCTImporterExporter(TestCaseWithFactory):
self.assertEqual(sorted(cve.bug_urls), sorted(w.url for w in watches))
self.checkBugAttachments(bug, cve)
+ self.checkBugPresence(bug, cve)
def checkBugTasks(self, bug: Bug, cve: CVE):
bug_tasks = bug.bugtasks # type: List[BugTask]
@@ -806,6 +887,27 @@ class TestUCTImporterExporter(TestCaseWithFactory):
)
self.assertEqual(expected_title, attachment.title)
+ def checkBugPresence(self, bug: Bug, cve: CVE):
+ expected_break_fixes = set()
+ for upstream_package in cve.upstream_packages:
+ for break_fix in upstream_package.break_fixes:
+ expected_break_fixes.add(
+ (upstream_package.target.name, break_fix)
+ )
+ actual_break_fixes = set()
+ for bp in getUtility(IBugPresenceSet).getByBug(bug):
+ project = removeSecurityProxy(bp).git_repository.project
+ actual_break_fixes.add(
+ (
+ project.name,
+ CVE.BreakFix(
+ broken_git_commit_sha1=bp.broken_git_commit_sha1,
+ fixed_git_commit_sha1=bp.fixed_git_commit_sha1,
+ ),
+ )
+ )
+ self.assertSetEqual(expected_break_fixes, actual_break_fixes)
+
def checkVulnerabilities(self, bug: Bug, cve: CVE):
vulnerabilities = bug.vulnerabilities
diff --git a/lib/lp/bugs/scripts/uct/models.py b/lib/lp/bugs/scripts/uct/models.py
index fd77ea5..2540eda 100644
--- a/lib/lp/bugs/scripts/uct/models.py
+++ b/lib/lp/bugs/scripts/uct/models.py
@@ -450,6 +450,14 @@ class CVE:
),
)
+ BreakFix = NamedTuple(
+ "BreakFix",
+ (
+ ("broken_git_commit_sha1", Optional[str]),
+ ("fixed_git_commit_sha1", Optional[str]),
+ ),
+ )
+
UpstreamPackage = NamedTuple(
"UpstreamPackage",
(
@@ -458,6 +466,7 @@ class CVE:
("importance", Optional[BugTaskImportance]),
("status", BugTaskStatus),
("status_explanation", str),
+ ("break_fixes", List[BreakFix]),
),
)
@@ -475,6 +484,7 @@ class CVE:
# https://github.com/389ds/389-ds-base/commit/123 (1.4.4)
# https://github.com/389ds/389-ds-base/commit/345
PATCH_URL_RE = re.compile(r"^(?P<url>.+?)(\s+\((?P<notes>.+)\))?$")
+ GIT_HASH_RE = re.compile(r"^[a-f0-9]{40}$")
PRIORITY_MAP = {
UCTRecord.Priority.CRITICAL: BugTaskImportance.CRITICAL,
@@ -563,6 +573,7 @@ class CVE:
distro_packages = []
series_packages = []
patch_urls = []
+ break_fixes = {} # type: Dict[SourcePackageName, List[CVE.BreakFix]]
spn_set = getUtility(ISourcePackageNameSet)
@@ -577,6 +588,10 @@ class CVE:
cls.get_patch_urls(source_package_name, uct_package.patches)
)
+ break_fixes[source_package_name] = cls.get_break_fixes(
+ uct_package.patches
+ )
+
package_importance = (
cls.PRIORITY_MAP[uct_package.priority]
if uct_package.priority
@@ -665,6 +680,7 @@ class CVE:
),
status=cls.BUG_TASK_STATUS_MAP[upstream_status.status],
status_explanation=upstream_status.reason,
+ break_fixes=break_fixes[source_package_name],
)
)
@@ -786,15 +802,32 @@ class CVE:
patches=[],
)
+ grouped_break_fixes = OrderedDict()
+ for break_fix in upstream_package.break_fixes:
+ if break_fix.broken_git_commit_sha1 not in grouped_break_fixes:
+ grouped_break_fixes[break_fix.broken_git_commit_sha1] = []
+ if break_fix.fixed_git_commit_sha1:
+ grouped_break_fixes[
+ break_fix.broken_git_commit_sha1
+ ].append(break_fix.fixed_git_commit_sha1)
+
+ for break_commit, fixed_commits in grouped_break_fixes.items():
+ entry = "{} {}".format(
+ break_commit or "-",
+ "|".join(fixed_commits) or "-",
+ )
+ packages_by_name[
+ upstream_package.package_name.name
+ ].patches.append(
+ UCTRecord.Patch(patch_type="break-fix", entry=entry)
+ )
+
for patch_url in self.patch_urls:
entry = patch_url.url
if patch_url.notes:
entry = "{} ({})".format(entry, patch_url.notes)
packages_by_name[patch_url.package_name.name].patches.append(
- UCTRecord.Patch(
- patch_type=patch_url.type,
- entry=entry,
- )
+ UCTRecord.Patch(patch_type=patch_url.type, entry=entry)
)
return UCTRecord(
@@ -909,3 +942,59 @@ class CVE:
url=url,
notes=notes,
)
+
+ @classmethod
+ def get_break_fixes(
+ cls,
+ patches: List[UCTRecord.Patch],
+ ) -> List[BreakFix]:
+
+ break_fixes = []
+
+ for patch in patches:
+ if patch.patch_type != "break-fix":
+ continue
+
+ try:
+ broken_entry, joint_fixed_entries = patch.entry.split()
+ except ValueError:
+ logger.error(
+ "Could not parse break-fix entry: %s", patch.entry
+ )
+ continue
+
+ if broken_entry == "-":
+ broken_entry = None
+ elif not cls.GIT_HASH_RE.match(broken_entry):
+ logger.error(
+ "broken entry doesn't look like git commit SHA1: %s",
+ broken_entry,
+ )
+ continue
+
+ if joint_fixed_entries == "-":
+ if broken_entry:
+ break_fixes.append(
+ cls.BreakFix(
+ broken_git_commit_sha1=broken_entry,
+ fixed_git_commit_sha1=None,
+ )
+ )
+ continue
+
+ for fixed_entry in joint_fixed_entries.split("|"):
+ # TODO: handle the with fixed_entry="local-CVE-2021-20177"
+ if cls.GIT_HASH_RE.match(fixed_entry):
+ break_fixes.append(
+ cls.BreakFix(
+ broken_git_commit_sha1=broken_entry,
+ fixed_git_commit_sha1=fixed_entry,
+ )
+ )
+ else:
+ logger.error(
+ "fixed entry doesn't look like git commit " "SHA1: %s",
+ fixed_entry,
+ )
+
+ return break_fixes
diff --git a/lib/lp/bugs/scripts/uct/uctexport.py b/lib/lp/bugs/scripts/uct/uctexport.py
index 8a1dd27..ff1a9c5 100644
--- a/lib/lp/bugs/scripts/uct/uctexport.py
+++ b/lib/lp/bugs/scripts/uct/uctexport.py
@@ -12,6 +12,7 @@ from zope.security.proxy import removeSecurityProxy
from lp.bugs.interfaces.bug import IBugSet
from lp.bugs.interfaces.bugattachment import BugAttachmentType
+from lp.bugs.interfaces.bugpresence import IBugPresenceSet
from lp.bugs.model.bug import Bug as BugModel
from lp.bugs.model.bugtask import BugTask
from lp.bugs.model.cve import Cve as CveModel
@@ -173,6 +174,13 @@ class UCTExporter:
)
)
+ bug_presence_by_product = defaultdict(list)
+ for bp in getUtility(IBugPresenceSet).getByBug(bug):
+ if bp.git_repository:
+ bug_presence_by_product[
+ removeSecurityProxy(bp).git_repository.project
+ ].append(bp)
+
upstream_packages = []
for bug_task in bug_tasks:
target = removeSecurityProxy(bug_task.target)
@@ -187,6 +195,7 @@ class UCTExporter:
package_name = package_name_by_product[target]
up_importance = bug_task.importance
package_importance = package_importances.get(target.name)
+ bug_presences = bug_presence_by_product.get(target, [])
upstream_packages.append(
CVE.UpstreamPackage(
target=target,
@@ -198,6 +207,13 @@ class UCTExporter:
),
status=bug_task.status,
status_explanation=bug_task.status_explanation,
+ break_fixes=[
+ CVE.BreakFix(
+ broken_git_commit_sha1=bp.broken_git_commit_sha1,
+ fixed_git_commit_sha1=bp.fixed_git_commit_sha1,
+ )
+ for bp in bug_presences
+ ],
)
)
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index 5bc8411..607dd2e 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -25,6 +25,7 @@ Three types of bug tags are created:
status of the package in upstream.
"""
import logging
+from collections import defaultdict
from datetime import timezone
from itertools import chain
from pathlib import Path
@@ -39,6 +40,7 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.interfaces.bug import CreateBugParams, IBugSet
from lp.bugs.interfaces.bugactivity import IBugActivitySet
from lp.bugs.interfaces.bugattachment import BugAttachmentType
+from lp.bugs.interfaces.bugpresence import IBugPresenceSet
from lp.bugs.interfaces.bugtask import BugTaskImportance, IBugTaskSet
from lp.bugs.interfaces.bugwatch import IBugWatchSet
from lp.bugs.interfaces.cve import ICveSet
@@ -48,6 +50,7 @@ from lp.bugs.model.bugtask import BugTask
from lp.bugs.model.cve import Cve as CveModel
from lp.bugs.model.vulnerability import Vulnerability
from lp.bugs.scripts.uct.models import CVE, UCTRecord
+from lp.code.interfaces.gitrepository import IGitRepositorySet
from lp.registry.model.distribution import Distribution
from lp.registry.model.person import Person
from lp.services.database.constants import UTC_NOW
@@ -168,6 +171,7 @@ class UCTImporter:
self._update_external_bug_urls(bug, cve.bug_urls)
self._update_patches(bug, cve.patch_urls)
+ self._update_bug_presence(bug, cve.upstream_packages)
self._create_bug_tasks(
bug,
@@ -225,6 +229,7 @@ class UCTImporter:
self._assign_bug_tasks(bug, cve.assignee)
self._update_external_bug_urls(bug, cve.bug_urls)
self._update_patches(bug, cve.patch_urls)
+ self._update_bug_presence(bug, cve.upstream_packages)
# Update or add new Vulnerabilities
vulnerabilities_by_distro = {
@@ -455,6 +460,47 @@ class UCTImporter:
description=title,
)
+ def _update_bug_presence(
+ self, bug: BugModel, upstream_packages: List[CVE.UpstreamPackage]
+ ):
+ bug_presence_set = getUtility(IBugPresenceSet)
+ bug_presences_by_target = defaultdict(list)
+ for bp in bug_presence_set.getByBug(bug):
+ bug_presences_by_target[bp.target].append(bp)
+ for upstream_package in upstream_packages:
+ if not upstream_package.break_fixes:
+ continue
+ git_repo = getUtility(IGitRepositorySet).getDefaultRepository(
+ upstream_package.target
+ )
+
+ if not git_repo:
+ logger.error(
+ "Could not find a git repository for %s "
+ "to save bug presence information",
+ upstream_package.target.name,
+ )
+ continue
+
+ existing_break_fixes = set()
+ for bp in bug_presences_by_target.get(git_repo, []):
+ existing_break_fixes.add(
+ CVE.BreakFix(
+ broken_git_commit_sha1=bp.broken_git_commit_sha1,
+ fixed_git_commit_sha1=bp.fixed_git_commit_sha1,
+ )
+ )
+
+ for break_fix in upstream_package.break_fixes:
+ if break_fix is existing_break_fixes:
+ continue
+ bug_presence_set.new(
+ bug=bug,
+ target=git_repo,
+ broken_git_commit_sha1=break_fix.broken_git_commit_sha1,
+ fixed_git_commit_sha1=break_fix.fixed_git_commit_sha1,
+ )
+
def _make_bug_description(self, cve: CVE) -> str:
"""
Some `CVE` fields can't be mapped to Launchpad models.
