Merge ~litios/ubuntu-cve-tracker:customer_ppas/cve_triage_no_cve into ubuntu-cve-tracker:master

Proposed by David Fernandez Gonzalez
Status: Merged
Merged at revision: a90ed317cfd91ffb661cb102521da0e4ca796bce
Proposed branch: ~litios/ubuntu-cve-tracker:customer_ppas/cve_triage_no_cve
Merge into: ubuntu-cve-tracker:master
Diff against target: 53 lines (+20/-0)
3 files modified
scripts/active_edit (+4/-0)
scripts/check-syntax (+5/-0)
scripts/cve_lib.py (+11/-0)
Reviewer Review Type Date Requested Status
Eduardo Barretto Approve
Review via email: mp+455554@code.launchpad.net

Description of the change

If the subproject doesn't require CVE triage, don't create the CVE file.

Testing: https://pastebin.canonical.com/p/MKb3v9823V/

To post a comment you must log in.
Revision history for this message
Simon Quigley (tsimonq2) wrote :

I realize this is a customer-driven diff, but if possible, since this is a public code base, could you provide some (redacted), public-facing testing instructions?

This has no impact on whether it is merged or not, but I'm just curious.

Thanks.

Revision history for this message
David Fernandez Gonzalez (litios) wrote :

> I realize this is a customer-driven diff, but if possible, since this is a
> public code base, could you provide some (redacted), public-facing testing
> instructions?

Sure! To test this change you need to:

1. Create a 'subprojects' folder in the root of ubuntu-cve-tracker and, inside this new one, another folder with the name of your subproject.
2. Populate config.yml inside the subproject as needed by `load_external_subprojects` function in cve_lib. You need to provide all the mandatory fields.
3. Run scripts/check-syntax to verify that the tool alerts about a missing CVE when cve_triage is set to true and that it doesn't if false.

> This has no impact on whether it is merged or not, but I'm just curious.
>
> Thanks.

Revision history for this message
Alex Murray (alexmurray) wrote :

I don't think the name `cve_triage` accurately reflects the purpose - also this is a change to check_syntax which doesn't create any CVEs, so I am not sure the commit description is accurate either.

Should this instead be called `required` or `requires_cves` or something else?

Revision history for this message
Emilia Torino (emitorino) wrote :

> I don't think the name `cve_triage` accurately reflects the purpose - also
> this is a change to check_syntax which doesn't create any CVEs, so I am not
> sure the commit description is accurate either.
>
> Should this instead be called `required` or `requires_cves` or something else?

Maybe requires_cves_triage? Since there is a related cve_patching config

26bff98... by David Fernandez Gonzalez

Don't add CVE entries if cve_triage is false

For those subprojects where the customer doesn't require CVE triage,
(that means cve_triage: false in config.yml), we shouldn't add any
entries. Those CVE files shouldn't exist in the subproject directory.

Signed-off-by: David Fernandez Gonzalez <email address hidden>

Revision history for this message
David Fernandez Gonzalez (litios) wrote :

Thanks for the feedback! I updated active_edit and created an appropriate function.

Regarding the naming, I don't have a strong preference either way, but I feel like changing that key name should be a different PR.

Let me know if you don't agree with something :)

Revision history for this message
Eduardo Barretto (ebarretto) wrote :

lgtm, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/scripts/active_edit b/scripts/active_edit
index d9068d6..2b46b07 100755
--- a/scripts/active_edit
+++ b/scripts/active_edit
@@ -138,6 +138,10 @@ def _add_pkg(p, fp, fixed, parent, embargoed):
138 elif pkgs_from_generic:138 elif pkgs_from_generic:
139 pkgs_to_add = pkgs_from_generic139 pkgs_to_add = pkgs_from_generic
140140
141 # If the subproject doesn't require cve triage,
142 # we shouldn't add any entries to the CVE
143 if not cve_lib.is_cve_triage_required(rel): continue
144
141 for pkg in pkgs_to_add:145 for pkg in pkgs_to_add:
142 rel_pkgname = rel + '/' + pkg146 rel_pkgname = rel + '/' + pkg
143 if not rel_pkgname in added_rel_pkg:147 if not rel_pkgname in added_rel_pkg:
diff --git a/scripts/check-syntax b/scripts/check-syntax
index 6a27d5e..734aa04 100755
--- a/scripts/check-syntax
+++ b/scripts/check-syntax
@@ -540,6 +540,11 @@ def check_cve(cve):
540 # only warn on active CVEs540 # only warn on active CVEs
541 if is_active(cve) and \541 if is_active(cve) and \
542 rel in source and pkg in source[rel]:542 rel in source and pkg in source[rel]:
543
544 # If the subproject doesn't require cve triage,
545 # we shouldn't add any entries to the CVE
546 if not cve_lib.is_cve_triage_required(rel): continue
547
543 filename = srcmap["pkgs"][pkg][nearby_rel][0]548 filename = srcmap["pkgs"][pkg][nearby_rel][0]
544 linenum = srcmap["pkgs"][pkg][nearby_rel][1]549 linenum = srcmap["pkgs"][pkg][nearby_rel][1]
545 print(550 print(
diff --git a/scripts/cve_lib.py b/scripts/cve_lib.py
index 01ffd2e..0ff90e3 100755
--- a/scripts/cve_lib.py
+++ b/scripts/cve_lib.py
@@ -898,6 +898,17 @@ def get_subproject_description(rel):
898898
899 return description899 return description
900900
901def is_cve_triage_required(rel):
902 """Check if CVE triage is required for a given release"""
903 if rel not in external_releases:
904 return True
905
906 try:
907 _,_,_,details = get_subproject_details(rel)
908 return details['support_metadata']['cve_triage']
909 except KeyError:
910 # Taking the safe option here
911 return True
901912
902def get_external_subproject_cve_dir(subproject):913def get_external_subproject_cve_dir(subproject):
903 """Get the directory where CVE files are stored for the subproject.914 """Get the directory where CVE files are stored for the subproject.

Subscribers

People subscribed via source and target branches