launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+483898@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:cve_scripts into launchpad:master.

Commit message:
Fix cve scripts exporting issues

Fix write Mitigation only when value is not None
Fix logging successfully imported when it does not import the CVE
Fix notes blank line

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/483898
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:cve_scripts into launchpad:master.

diff --git a/lib/lp/bugs/scripts/tests/test_uctimport.py b/lib/lp/bugs/scripts/tests/test_uctimport.py
index 2e36bb5..414e2cb 100644
--- a/lib/lp/bugs/scripts/tests/test_uctimport.py
+++ b/lib/lp/bugs/scripts/tests/test_uctimport.py
@@ -32,7 +32,7 @@ class TestUCTImportScript(TestCase):
         )
         self.assertEqual(0, exit_code)
         self.assertEqual("", out)
-        self.assertIn("CVE-2022-23222 was imported successfully", err)
+        self.assertIn("CVE-2022-23222 was not imported", err)
 
     def test_load_from_directory(self):
         load_from = Path(__file__).parent / "sampledata"
@@ -42,9 +42,9 @@ class TestUCTImportScript(TestCase):
         )
         self.assertEqual(0, exit_code)
         self.assertEqual("", out)
-        self.assertIn("CVE-2007-0255 was imported successfully", err)
-        self.assertIn("CVE-2022-3219 was imported successfully", err)
-        self.assertIn("CVE-2022-23222 was imported successfully", err)
+        self.assertIn("CVE-2007-0255 was not imported", err)
+        self.assertIn("CVE-2022-3219 was not imported", err)
+        self.assertIn("CVE-2022-23222 was not imported", err)
 
     def test_dry_run_does_not_crash(self):
         load_from = Path(__file__).parent / "sampledata" / "CVE-2022-23222"
@@ -64,8 +64,8 @@ class TestUCTImportScript(TestCase):
         )
         self.assertEqual(0, exit_code)
         self.assertEqual("", out)
-        self.assertNotIn("CVE-2022-23222 was imported successfully", err)
-        self.assertIn("CVE-2007-0255 was imported successfully", err)
+        self.assertNotIn("CVE-2022-23222 was not imported", err)
+        self.assertIn("CVE-2007-0255 was not imported", err)
 
         exit_code, out, err = run_script(
             script="scripts/uct-import.py",
@@ -73,9 +73,9 @@ class TestUCTImportScript(TestCase):
         )
         self.assertEqual(0, exit_code)
         self.assertEqual("", out)
-        self.assertIn("CVE-2022-23222 was imported successfully", err)
-        self.assertIn("CVE-2022-3219 was imported successfully", err)
-        self.assertNotIn("CVE-2007-0255 was imported successfully", err)
+        self.assertIn("CVE-2022-23222 was not imported", err)
+        self.assertIn("CVE-2022-3219 was not imported", err)
+        self.assertNotIn("CVE-2007-0255 was not imported", err)
 
         exit_code, out, err = run_script(
             script="scripts/uct-import.py",
@@ -83,6 +83,6 @@ class TestUCTImportScript(TestCase):
         )
         self.assertEqual(0, exit_code)
         self.assertEqual("", out)
-        self.assertIn("CVE-2022-23222 was imported successfully", err)
-        self.assertIn("CVE-2022-3219 was imported successfully", err)
-        self.assertIn("CVE-2007-0255 was imported successfully", err)
+        self.assertIn("CVE-2022-23222 was not imported", err)
+        self.assertIn("CVE-2022-3219 was not imported", err)
+        self.assertIn("CVE-2007-0255 was not imported", err)
diff --git a/lib/lp/bugs/scripts/uct/models.py b/lib/lp/bugs/scripts/uct/models.py
index 418f074..91510e9 100644
--- a/lib/lp/bugs/scripts/uct/models.py
+++ b/lib/lp/bugs/scripts/uct/models.py
@@ -300,11 +300,10 @@ class UCTRecord:
             "Ubuntu-Description", self.ubuntu_description.split("\n"), output
         )
         self._write_field("Notes", self.notes.split("\n"), output)
-        self._write_field(
-            "Mitigation",
-            self.mitigation.split("\n") if self.mitigation else "",
-            output,
-        )
+        if self.mitigation:
+            self._write_field(
+                "Mitigation", self.mitigation.split("\n"), output
+            )
         self._write_field("Bugs", self.bugs, output)
         self._write_field("Priority", self.priority.value, output)
         self._write_field("Discovered-by", self.discovered_by, output)
@@ -382,7 +381,8 @@ class UCTRecord:
         elif isinstance(value, list):
             output.write(f"{name}:\n")
             for line in value:
-                output.write(f" {line}\n")
+                if line != "":
+                    output.write(f" {line}\n")
         else:
             raise AssertionError()
 
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index 845b060..8ee237b 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -83,7 +83,6 @@ class UCTImporter:
         uct_record = UCTRecord.load(cve_path)
         cve = CVE.make_from_uct_record(uct_record)
         self.import_cve(cve)
-        logger.info("%s was imported successfully", cve_path)
 
     def import_cve(self, cve: CVE) -> None:
         """
@@ -94,14 +93,18 @@ class UCTImporter:
         if cve.date_made_public is None:
             logger.warning(
                 "%s does not have a publication date, "
-                "is it embargoed? Aborting.",
+                "is it embargoed? Aborting. "
+                "%s was not imported.",
+                cve.sequence,
                 cve.sequence,
             )
             return
         if not cve.series_packages:
             logger.warning(
-                "%s: could not find any affected packages, aborting.",
+                "%s: could not find any affected packages, aborting."
+                "%s was not imported.",
                 cve.series_packages,
+                cve.sequence,
             )
             return
         lp_cve: CveModel = removeSecurityProxy(
@@ -109,7 +112,10 @@ class UCTImporter:
         )
         if lp_cve is None:
             logger.warning(
-                "%s: could not find the CVE in LP. Aborting.", cve.sequence
+                "%s: could not find the CVE in LP. Aborting. "
+                "%s was not imported.",
+                cve.sequence,
+                cve.sequence,
             )
             return
         bug = self._find_existing_bug(cve, lp_cve)
@@ -143,6 +149,8 @@ class UCTImporter:
         else:
             transaction.commit()
 
+        logger.info("%s was imported successfully", cve.sequence)
+
     def create_bug(self, cve: CVE, lp_cve: CveModel) -> BugModel:
         """
         Create a `Bug` model based on the information contained in a `CVE`.