Skip to content

elliott: fix close_reconciliation_bugs crash on unbounded JQL when target versions are missing - #3284

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift-eng:mainfrom
kopero2000:fix-close-reconciliation-bugs-none-query
Aug 17, 2026
Merged

elliott: fix close_reconciliation_bugs crash on unbounded JQL when target versions are missing#3284
openshift-merge-bot[bot] merged 1 commit into
openshift-eng:mainfrom
kopero2000:fix-close-reconciliation-bugs-none-query

Conversation

@kopero2000

@kopero2000 kopero2000 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • close_reconciliation_bugs (elliott find-bugs:qe) crashed with JiraError HTTP 400: Unbounded JQL queries are not allowed here whenever a group's configured target-release versions (e.g. 5.1.0, 5.1.z, 5.1 from ocp-build-data/bug.yml) don't yet exist as "Target Version" options in the OCPBUGS JIRA project (e.g. right after a new OCP release is cut).
  • BugTracker._query() already handles this case by returning None when all configured target versions are filtered out, and every other caller (search, blocker_search, cve_tracker_search) checks for None and short-circuits to []. close_reconciliation_bugs was the one caller that skipped this check and passed None straight into _search(), which sent JIRA a query with no real restriction.
  • This fix adds the same if query is None: return []-style guard to close_reconciliation_bugs, matching the existing pattern used elsewhere in bzutil.py.

Test plan

  • Added test_close_reconciliation_bugs_no_valid_target_versions covering the _query() -> None path (asserts _search is never called and no bugs are transitioned/commented on).
  • uv run pytest elliott/tests/test_find_bugs_qe_cli.py — all 5 tests pass.
  • make lint — clean.

Summary by CodeRabbit

  • Bug Fixes

    • Improved reconciliation bug handling when no query is provided.
    • Prevents unnecessary searches, status changes, and comments when no bugs are identified.
  • Tests

    • Added regression coverage for reconciliation workflows with no query.

…ered out

_query() returns None when every configured target-release version is
missing from the JIRA project (e.g. a new OCP release whose Target
Version values haven't been created yet), and every other caller
(search, blocker_search, cve_tracker_search) already checks for that
and short-circuits to []. close_reconciliation_bugs skipped that check
and passed None straight to _search(), which sent JIRA a query with no
real restriction and got rejected with "Unbounded JQL queries are not
allowed here", failing the whole find-bugs:qe step.

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a928fc17-6181-4dc3-a1dd-a5f16f126607

📥 Commits

Reviewing files that changed from the base of the PR and between 0c53010 and cb74399.

📒 Files selected for processing (2)
  • elliott/elliottlib/cli/find_bugs_qe_cli.py
  • elliott/tests/test_find_bugs_qe_cli.py

Walkthrough

close_reconciliation_bugs now treats a missing reconciliation query as no bugs found. A regression test verifies that this path skips bug tracker searches, transitions, and comments.

Changes

Reconciliation query handling

Layer / File(s) Summary
Query guard and regression test
elliott/elliottlib/cli/find_bugs_qe_cli.py, elliott/tests/test_find_bugs_qe_cli.py
None queries produce an empty bug list. The regression test verifies that no search or bug-closing side effects occur.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to cb743

The change prevents the reconciliation command from issuing an unbounded query when target versions are unavailable and safely returns without modifying bugs; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the crash fix and the missing target-version condition addressed by the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The commit only adds a None-query guard and regression test; the changed lines introduce no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparison.
Container-Privileges ✅ Passed The PR changes only Python code and tests; the patch adds no container/Kubernetes manifests or listed privilege settings.
No-Sensitive-Data-In-Logs ✅ Passed The patch adds only a None-query guard and regression test. It adds no logging and introduces no passwords, tokens, keys, PII, hostnames, session IDs, or customer data.
No-Hardcoded-Secrets ✅ Passed The PR adds only a query guard, comments, and test fixtures; scans found no API keys, credentials, private keys, embedded-auth URLs, or long base64 strings.
No-Injection-Vectors ✅ Passed The PR only adds a None-query branch and regression test; the diff introduces no SQL concatenation, shell=True, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInnerHTML.
Ai-Attribution ✅ Passed The supplied PR description and sole PR commit contain no AI-tool mention or attribution trailer; AI Co-Authored-By trailers exist only in the pre-existing parent commit.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kopero2000
kopero2000 requested a review from joepvd August 13, 2026 10:03
@openshift-ci

openshift-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@kopero2000: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security cb74399 link false /test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@joepvd joepvd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2026
@openshift-ci

openshift-ci Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joepvd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 3ccde95 into openshift-eng:main Aug 17, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants