Changes to accomodate checks if cve-bf are cves as well#65
Changes to accomodate checks if cve-bf are cves as well#65roxanan1996 wants to merge 10 commits intomainlinefrom
Conversation
1. Moved run_cve_search from check_kernel_commits.py to ciq_helpers.py 2. Created a wrapper that parses the output of run_cve_search and return the cve number. 3. Used the wrapper instead of doing the same thing twice in check_kernel_commits.py Bonus: This also reduces the level of identation in check_kernel_commits. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
Useful because kt is exposed as package and these helpers can be used in multiple places, not only this repo. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the tooling to better support CVE-related cherry-pick workflows by centralizing CVE lookup logic, adding JIRA automation support, and packaging kt as an installable CLI entrypoint.
Changes:
- Switch various top-level scripts to import helpers from
kt.ktlib.ciq_helpers. - Add CVE lookup + vulns repo setup helpers, and integrate them into
ciq-cherry-pick.pyandcheck_kernel_commits.py. - Add a new
kt kernel_infocommand and introduce aktconsole script entrypoint viapyproject.toml.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| update_lt_spec.py | Updates helper imports to use the packaged kt.ktlib path. |
| rolling-release-update.py | Updates helper import to use the packaged kt.ktlib path. |
| pyproject.toml | Adds a kt console script entrypoint. |
| kt/ktlib/jira.py | Introduces a JIRA wrapper used by automation flows. |
| kt/ktlib/ciq_helpers.py | Adds shared CVE lookup and vulns repo setup helpers. |
| kt/kt.py | Registers the new kernel_info subcommand. |
| kt/KT.md | Updates installation guidance to reflect installing kt. |
| kt/commands/kernel_info/impl.py | Implements kernel metadata output as JSON. |
| kt/commands/kernel_info/command.py | Adds Click wiring and help text for kernel_info. |
| ciq-cherry-pick.py | Adds CVE validation, JIRA ticket mapping, and JIRA updates on success/failure. |
| check_kernel_commits.py | Reuses shared CVE helpers and vulns repo setup logic. |
Comments suppressed due to low confidence (1)
kt/ktlib/ciq_helpers.py:546
- Docstring typos in the newly added
CIQ_setup_vulns_repo()documentation: “Setups” → “Sets up”, and “errros” → “errors”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4701f2d to
cae02b9
Compare
| jira_key = args.jira_key or os.environ.get("JIRA_API_TOKEN") | ||
|
|
||
| if not all([jira_url, jira_user, jira_key]): | ||
| print("WARNING: JIRA credentials not provided. Set via --jira-* args or environment variables.") |
There was a problem hiding this comment.
For external users using this a WARNING might be a little strong. Granted its generally a SINGLE user at the moment.
Perhaps [NOTE]?
There was a problem hiding this comment.
Changed to [NOTE].
| if CIQ_check_if_published_cve(vulns_repo=vulns_repo, cve_id=cve_id): | ||
| return cve_id |
There was a problem hiding this comment.
I'm wondering if we should make note that the CVE is rejected or well dropped from the run_cve_search (i believe thats what happens when it is in the system but no longer published)
its just silent at this point and we've seen Red Hat Publish rejected CVEs due to when it was rejected. So might be a good thing to log
There was a problem hiding this comment.
Yes, it could be useful.
bmastbergen
left a comment
There was a problem hiding this comment.
A couple little things
|
|
||
| # Find the jira ticket corresponding to the CVE only if jira_instance | ||
| kernel = find_lts_kernel(jira_instance=jira_instance, jira_ticket=jira_ticket) | ||
| if matching_cve not in ciq_tags[0]: |
There was a problem hiding this comment.
I think not in here will be a substring check, yes? then we could erroneously match CVE-2026-1234 with CVE-2026-12345. Not likely, but maybe this should be != ?
There was a problem hiding this comment.
Yup, wil fix it
There was a problem hiding this comment.
Fixed, I extract the cve properly now
| "pathlib3x", | ||
| "python3-wget", | ||
| "oyaml", | ||
| "pexpect", |
There was a problem hiding this comment.
Should we add jira and requests here now?
There was a problem hiding this comment.
Added them and removed them from kernel-tools.
| update_jira_failure(jira_instance=jira_instance, ticket_key=updated_jira_ticket, jira_dry_run=jira_dry_run) | ||
| raise e | ||
|
|
||
| update_jira_success(jira_instance=jira_instance, ticket_key=updated_jira_ticket, jira_dry_run=jira_dry_run) |
There was a problem hiding this comment.
update_jira_success runs before cherry_pick_fixes. If cherry_pick_fixes fails, the Jira ticket has already been transitioned to In Progress and had a worklog added, but the cherry-pick is incomplete.
There was a problem hiding this comment.
Good point. For manual work this is fine, because the ciq-cherry-pick is incomplete, manual stuff has to be done.
But, in the context of cve_remediation this is a problem. Will address this.
There was a problem hiding this comment.
update_jira_success is now done after the cve-bf deps are cherry pickes.
I added a TODO there to add extra info in case the deps were not clean cherry picks, but won't do this now.
Thanks again for catching this.
Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
If so, use the proper tag and jira ticket (if it exists). If no jira ticket is found, the original one will be used. In case the cve-bf dependency is a cve and has a corresponding jira ticket, it would be left unassigned. Then the pr_jira_check.py would complain. In order to avoid this, and the "incomplete" logic of updating jira tickets only for cve-bf commits, ciq-cherry-pick.py now updates the jira tickets by default (for the initial CVE and for the dependencies that are CVEs), unless --jira-dry-run is being used. This way, we don't end up with a weird state where just one ticket is updated and the others are not. Extra arguments were needed: - jira credentials because we now do jira queries - vuln repo path to check if a CVE matches a commit - jira-dry-run as described above Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
CIQ_find_matching_cve returned the matching CVE even if it's rejected because the cve_search script from the vuln repo does not check if the CVE is published or rejected. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
It prints a json that matches the kernel information from kt/data/kernels.yml Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
…alled Had to move kt script to kt/kt.py Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
The project is installable. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
cae02b9 to
079de11
Compare
Description
Started from the need to detect if the cve-bf dependencies are cves and if so to adjust the commit message
accordingly. This lead to multiple changes, more or less needed to make it happen, and some that were adjacent fixes and code deduplication.
Example PR where this happened and someone has to intervene to fix it after the PR was created.
Example PR where it was flagged that a commit was a cve, even though that was rejected
ctrliq/kernel-src-tree#1116 (comment)
Main changes and the reason behind it:
In case the cve-bf dependency is a cve and has a corresponding jira ticket,
it would be left unassigned. Then the pr_jira_check.py would complain.
In order to avoid this, and the "incomplete" logic of updating jira tickets
only for cve-bf commits, ciq-cherry-pick.py now updates the jira tickets
by default (for the initial CVE and for the dependencies that are CVEs),
unless --jira-dry-run is being used. This way, we don't end up with a weird
state where just one ticket is updated and the others are not.
Extra arguments were added:
Note
ciq-cherry-pick.py is a bit convoluted, will refactor it before further improvements.
Main problem is the number of arguments that lead to multiple code paths.