launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #33199
[Merge] ~enriqueesanchz/launchpad:add-stats-vulnerability-jobs into launchpad:master
Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-stats-vulnerability-jobs into launchpad:master.
Commit message:
Add create/update stats to importvulnerabilityjob
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/495114
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-stats-vulnerability-jobs into launchpad:master.
diff --git a/lib/lp/bugs/model/importvulnerabilityjob.py b/lib/lp/bugs/model/importvulnerabilityjob.py
index 14635d5..41cf351 100644
--- a/lib/lp/bugs/model/importvulnerabilityjob.py
+++ b/lib/lp/bugs/model/importvulnerabilityjob.py
@@ -138,7 +138,8 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
},
"result": {
"error_description": [],
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
},
}
@@ -201,36 +202,30 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
distribution, information_type=information_type
)
else:
- exception = VulnerabilityJobException("Handler not found")
- self.notifyUserError(exception)
- raise exception
+ self._notify_and_raise("Handler not found")
return parser, importer
- def run(self):
- """See `IRunnableJob`."""
- # InformationType is passed as a value as DBItem is not serializable
- information_type = InformationType.items[self.information_type]
- parser, importer = self._get_parser_importer(
- self.context.handler, information_type
- )
+ def _notify_and_raise(self, message: str):
+ """Creates, notifies, and raises a VulnerabilityJobException."""
+ exception = VulnerabilityJobException(message)
+ self.notifyUserError(exception)
+ raise exception
- # Get git repository
+ def _get_git_repository(self, id):
git_lookup = getUtility(IGitLookup)
- repository = removeSecurityProxy(git_lookup.get(self.git_repository))
+ repository = removeSecurityProxy(git_lookup.get(id))
if not repository:
- exception = VulnerabilityJobException("Git repository not found")
- self.notifyUserError(exception)
- raise exception
+ self._notify_and_raise("Git repository not found")
+ return repository
- # Get git reference
+ def _get_git_reference(self, repository, ref_name):
ref = removeSecurityProxy(repository.getRefByPath(self.git_ref))
if not ref:
- exception = VulnerabilityJobException("Git ref not found")
- self.notifyUserError(exception)
- raise exception
+ self._notify_and_raise("Git ref not found")
+ return ref
- # turnip API call to get added/modified files
+ def _get_changed_files(self, repository, ref):
try:
stats = getUtility(IGitHostingClient).getDiffStats(
path=repository.getInternalPath(),
@@ -239,34 +234,49 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
logger=logger,
)
except GitRepositoryScanFault:
- exception = VulnerabilityJobException(
+ message = (
f"Git diff between {self.import_since_commit_sha1} and "
f"{ref.commit_sha1} for {self.git_ref} not found"
)
- self.notifyUserError(exception)
- raise exception
+ self._notify_and_raise(message)
files = [*stats.get("added", ()), *stats.get("modified", ())]
- for file in files:
- # Check if files that changed are in the desired path
- found_path = False
- for path in self.git_paths:
- if file.startswith(path):
- found_path = True
- break
+ return files
- if not found_path:
- logger.debug(
- f"[ImportVulnerabilityJob] {file} is not in git_paths"
- )
- continue
+ def _is_valid(self, file, cve_sequence, valid_paths):
+ if not file.startswith(valid_paths):
+ return False
+ if not CVE_PATTERN.match(cve_sequence):
+ logger.debug(
+ f"[ImportVulnerabilityJob] {cve_sequence} is not a CVE "
+ "sequence"
+ )
+ return False
+ return True
+
+ def run(self):
+ """See `IRunnableJob`."""
+ # InformationType is passed as a value as DBItem is not serializable
+ information_type = InformationType.items[self.information_type]
+ parser, importer = self._get_parser_importer(
+ self.context.handler, information_type
+ )
+
+ # Get git repository
+ repository = self._get_git_repository(self.git_repository)
+
+ # Get git reference
+ ref = self._get_git_reference(repository, self.git_ref)
+
+ # turnip API call to get added/modified files
+ files = self._get_changed_files(repository, ref)
+ valid_paths = tuple(self.git_paths)
+
+ for file in files:
+ # Check if files that changed are in the desired path
cve_sequence = file.rsplit("/", maxsplit=1)[-1]
- if not CVE_PATTERN.match(cve_sequence):
- logger.debug(
- f"[ImportVulnerabilityJob] {cve_sequence} is not a CVE "
- "sequence"
- )
+ if not self._is_valid(file, cve_sequence, valid_paths):
continue
success = False
@@ -281,11 +291,16 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
)
record = parser.from_str(blob)
- bug, vulnerability = importer.from_record(record, cve_sequence)
+ bug, vulnerability, created = importer.from_record(
+ record, cve_sequence
+ )
if bug and vulnerability:
success = True
- self.metadata["result"]["succeeded"].append(cve_sequence)
+ if created:
+ self.metadata["result"]["created"].append(cve_sequence)
+ else:
+ self.metadata["result"]["updated"].append(cve_sequence)
except (
GitRepositoryScanFault,
GitRepositoryBlobNotFound,
@@ -293,11 +308,11 @@ class ImportVulnerabilityJob(VulnerabilityJobDerived):
) as e:
# If there is a problem with git repository API call, we stop
# as we won't be able to fetch more files
- self.notifyUserError(f"{cve_sequence}: {e}")
- raise VulnerabilityJobException(
+ message = (
f"Failed to get file from Git repository: {file}."
f"Aborting. {e}"
)
+ self._notify_and_raise(message)
except Exception as e:
self.notifyUserError(f"{cve_sequence}: {e}")
finally:
diff --git a/lib/lp/bugs/scripts/soss/sossimport.py b/lib/lp/bugs/scripts/soss/sossimport.py
index d7eef1f..15fd5ff 100644
--- a/lib/lp/bugs/scripts/soss/sossimport.py
+++ b/lib/lp/bugs/scripts/soss/sossimport.py
@@ -110,8 +110,10 @@ class SOSSImporter(SVTImporter):
with open(cve_path, encoding="utf-8") as file:
soss_record = SOSSRecord.from_yaml(file)
- bug, vulnerability = self.from_record(soss_record, cve_sequence)
- return bug, vulnerability
+ bug, vulnerability, created = self.from_record(
+ soss_record, cve_sequence
+ )
+ return bug, vulnerability, created
def from_record(
self, soss_record: SOSSRecord, cve_sequence: str
@@ -119,12 +121,14 @@ class SOSSImporter(SVTImporter):
"""Import CVE from SOSS record."""
self._validate_soss_record(soss_record, cve_sequence)
lp_cve = self._get_launchpad_cve(cve_sequence)
+ created = False
vulnerability = self._find_existing_vulnerability(lp_cve, self.soss)
if not vulnerability:
vulnerability = self._create_vulnerability(
soss_record, lp_cve, self.soss
)
+ created = True
else:
vulnerability = self._update_vulnerability(
vulnerability, soss_record
@@ -148,7 +152,7 @@ class SOSSImporter(SVTImporter):
f"{cve_sequence}"
)
- return bug, vulnerability
+ return bug, vulnerability, created
def _create_bug(
self, soss_record: SOSSRecord, lp_cve: CveModel
diff --git a/lib/lp/bugs/scripts/soss/tests/test_sossexport.py b/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
index 1752d8e..dc34a13 100644
--- a/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
+++ b/lib/lp/bugs/scripts/soss/tests/test_sossexport.py
@@ -46,7 +46,7 @@ class TestSOSSExporter(TestCaseWithFactory):
with open(file) as f:
soss_record = SOSSRecord.from_yaml(f)
- bug, _ = self.soss_importer.import_cve_from_file(file)
+ bug, _, _ = self.soss_importer.import_cve_from_file(file)
naked_bug = removeSecurityProxy(bug)
packages = self.soss_exporter._get_packages(naked_bug.bugtasks)
@@ -62,7 +62,7 @@ class TestSOSSExporter(TestCaseWithFactory):
with open(file) as f:
soss_record = SOSSRecord.from_yaml(f)
- _, vulnerability = self.soss_importer.import_cve_from_file(file)
+ _, vulnerability, _ = self.soss_importer.import_cve_from_file(file)
naked_vulnerability = removeSecurityProxy(vulnerability)
cvss = self.soss_exporter._get_cvss(naked_vulnerability.cvss)
@@ -78,7 +78,7 @@ class TestSOSSExporter(TestCaseWithFactory):
with open(file) as f:
soss_record = SOSSRecord.from_yaml(f)
- bug, vulnerability = soss_importer.import_cve_from_file(file)
+ bug, vulnerability, _ = soss_importer.import_cve_from_file(file)
exported = self.soss_exporter.to_record(bug, vulnerability)
self.assertEqual(soss_record, exported)
@@ -92,7 +92,7 @@ class TestSOSSExporter(TestCaseWithFactory):
for file in self.sampledata.iterdir():
- bug, vulnerability = soss_importer.import_cve_from_file(file)
+ bug, vulnerability, _ = soss_importer.import_cve_from_file(file)
exported = self.soss_exporter.to_record(bug, vulnerability)
with open(file) as f:
diff --git a/lib/lp/bugs/scripts/soss/tests/test_sossimport.py b/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
index 8eb3e97..af73352 100644
--- a/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
+++ b/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
@@ -241,13 +241,15 @@ class TestSOSSImporter(TestCaseWithFactory):
self.assertEqual(vulnerability.bugs[0], bug)
def test_import_cve_from_file(self):
- """Test import a SOSS cve from file"""
+ """Test import a SOSS cve from file. It also imports again to check
+ that we don't create duplicates."""
file = self.sampledata / "CVE-2025-1979"
soss_importer = SOSSImporter(
self.soss, information_type=InformationType.PROPRIETARY
)
- bug, vulnerability = soss_importer.import_cve_from_file(file)
+ bug, vulnerability, created = soss_importer.import_cve_from_file(file)
+ self.assertEqual(created, True)
# Check bug fields
self._check_bug_fields(bug, self.bugtask_reference)
@@ -256,9 +258,12 @@ class TestSOSSImporter(TestCaseWithFactory):
self._check_vulnerability_fields(vulnerability, bug)
# Import again to check that it doesn't create new objects
- bug_copy, vulnerability_copy = soss_importer.import_cve_from_file(file)
+ bug_copy, vulnerability_copy, created = (
+ soss_importer.import_cve_from_file(file)
+ )
self.assertEqual(bug, bug_copy)
self.assertEqual(vulnerability, vulnerability_copy)
+ self.assertEqual(created, False)
def test_create_update_bug(self):
"""Test create and update a bug from a SOSS cve file"""
diff --git a/lib/lp/bugs/scripts/uct/tests/test_uct.py b/lib/lp/bugs/scripts/uct/tests/test_uct.py
index 4e7780a..d2885c9 100644
--- a/lib/lp/bugs/scripts/uct/tests/test_uct.py
+++ b/lib/lp/bugs/scripts/uct/tests/test_uct.py
@@ -1720,7 +1720,7 @@ class TestUCTImporterExporter(TestCaseWithFactory):
uct_record = self.cve.to_uct_record()
cve_path = uct_record.save(Path(self.makeTemporaryDirectory()))
- bug, _ = self.importer.import_cve_from_file(cve_path)
+ bug, _, _ = self.importer.import_cve_from_file(cve_path)
self.checkBug(bug, self.cve)
self.checkVulnerabilities(bug, self.cve)
@@ -1759,7 +1759,7 @@ class TestUCTImporterExporter(TestCaseWithFactory):
def test_exporter_to_record(self):
"""Test to_record returns expected UCTRecord"""
- bug, vulnerability = self.importer.import_cve(self.cve)
+ bug, vulnerability, _ = self.importer.import_cve(self.cve)
uct_record = self.exporter.to_record(bug, vulnerability)
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index d39a22e..744e49c 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -112,7 +112,7 @@ class UCTImporter(SVTImporter):
cve.sequence,
cve_sequence,
)
- return None, None
+ return None, None, None
return self.import_cve(cve)
@@ -122,6 +122,7 @@ class UCTImporter(SVTImporter):
:param cve: `CVE` with information from UCT
"""
+ created = False
if cve.date_made_public is None:
logger.warning(
"%s does not have a publication date, "
@@ -157,6 +158,7 @@ class UCTImporter(SVTImporter):
logger.info(
"%s: created bug with ID: %s", cve.sequence, bug.id
)
+ created = True
else:
logging.info(
"%s: found existing bug with ID: %s",
@@ -181,7 +183,7 @@ class UCTImporter(SVTImporter):
transaction.commit()
logger.info("%s was imported successfully", cve.sequence)
- return bug, vulnerability
+ return bug, vulnerability, created
def create_bug(self, cve: CVE, lp_cve: CveModel) -> BugModel:
"""
diff --git a/lib/lp/bugs/tests/test_exportvulnerabilityjob.py b/lib/lp/bugs/tests/test_exportvulnerabilityjob.py
index 8711c02..dadb4b0 100644
--- a/lib/lp/bugs/tests/test_exportvulnerabilityjob.py
+++ b/lib/lp/bugs/tests/test_exportvulnerabilityjob.py
@@ -285,7 +285,7 @@ class ExportVulnerabilityJobTests(WithScenarios, TestCaseWithFactory):
if not self.cve_set[cve_sequence]:
self.factory.makeCVE(sequence=cve_sequence)
- bug, vulnerability = importer.import_cve_from_file(file)
+ bug, vulnerability, _ = importer.import_cve_from_file(file)
imported_list.append((cve_sequence, bug, vulnerability))
return imported_list
@@ -472,7 +472,7 @@ class ExportVulnerabilityTestViaCelery(WithScenarios, TestCaseWithFactory):
if not self.cve_set[cve_sequence]:
self.factory.makeCVE(sequence=cve_sequence)
- bug, vulnerability = importer.import_cve_from_file(file)
+ bug, vulnerability, _ = importer.import_cve_from_file(file)
imported_list.append((cve_sequence, bug, vulnerability))
return imported_list
diff --git a/lib/lp/bugs/tests/test_importvulnerabilityjob.py b/lib/lp/bugs/tests/test_importvulnerabilityjob.py
index 9c0c1c9..d93eb9b 100644
--- a/lib/lp/bugs/tests/test_importvulnerabilityjob.py
+++ b/lib/lp/bugs/tests/test_importvulnerabilityjob.py
@@ -136,7 +136,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
},
"result": {
"error_description": [],
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
},
}
@@ -346,7 +347,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
},
"result": {
"error_description": [],
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
},
}
@@ -405,7 +407,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": ["CVE-2025-1979"],
+ "created": ["CVE-2025-1979"],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -475,7 +478,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": ["CVE-2022-3219"],
+ "created": ["CVE-2022-3219"],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -529,7 +533,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": ["CVE-2025-1979"],
+ "created": ["CVE-2025-1979"],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -588,7 +593,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": ["CVE-2025-1979"],
+ "created": ["CVE-2025-1979"],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -628,7 +634,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -653,7 +660,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
"error_description": ["Git repository not found"],
},
@@ -687,7 +695,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
"error_description": ["Git ref not found"],
},
@@ -729,7 +738,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": ["CVE-2025-1979"],
"error_description": [
"CVE-2025-1979: None is not a valid PriorityEnum"
@@ -771,7 +781,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
self.assertEqual(
job.metadata.get("result"),
{
- "succeeded": ["CVE-2025-1979"],
+ "created": ["CVE-2025-1979"],
+ "updated": [],
"failed": [],
"error_description": [],
},
@@ -820,7 +831,8 @@ class ImportVulnerabilityJobTests(TestCaseWithFactory):
},
"result": {
"error_description": [error_msg],
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
},
},
@@ -935,7 +947,8 @@ class TestViaCelery(TestCaseWithFactory):
},
"result": {
"error_description": [],
- "succeeded": [],
+ "created": [],
+ "updated": [],
"failed": [],
},
}
