Merge ~andrey-fedoseev/launchpad:uct-import-export-break-fix into launchpad:master

[Andrey Fedoseev]

UCT import/export: handle the `break-fix` entries

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

d62597c... by Andrey Fedoseev on 2022-11-30

UCT import/export: handle the `break-fix` entries

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

Unmerged commits

d62597c... by Andrey Fedoseev on 2022-11-30

UCT import/export: handle the `break-fix` entries

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

	docs:0 (build)
	lint:0 (build)
	mypy:0 (build)

1 → 3 of 3 results

First • Previous • Next • Last

460e925... by Andrey Fedoseev on 2022-10-18

Add `BugPresence` model

It represents a range of versions or git commits in which the bug was present.

	docs:0 (build)
	lint:0 (build)
	mypy:0 (build)

1 → 3 of 3 results

First • Previous • Next • Last

diff --git a/lib/lp/bugs/model/bugpresence.py b/lib/lp/bugs/model/bugpresence.py
index b21371e..4ca8128 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 9813d86..3ce76b7 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,
@@ -661,6 +709,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,
@@ -668,6 +734,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,
@@ -724,6 +804,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]
@@ -799,6 +880,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.