Check for user in iib_no_ocp_label_allow_list

[MichalZelenak]

Summary by Sourcery

Allow bundle addition when the requesting user is configured to bypass missing OCP version labels.

Enhancements:

• Extend _add_bundles_missing_in_source to treat the requesting user as eligible for no-OCP-label allowances based on worker configuration.

Tests:

• Add tests covering behavior when the requesting user is and is not included in iib_no_ocp_label_allow_list with ignore_bundle_ocp_version enabled.

[sourcery-ai]

Reviewer's Guide

Extends the build_merge_index_image logic so that missing bundle OCP version labels can also be allowed based on the requesting user, and adds tests to cover both allowed and disallowed user scenarios when ignore_bundle_ocp_version is enabled.

Sequence diagram for allow_no_ocp_version evaluation in build_merge_index_image

sequenceDiagram
    actor Requestor
    participant IIBWorker
    participant BuildMergeTask as _add_bundles_missing_in_source
    participant RequestStore as get_request
    participant Config as get_worker_config

    Requestor->>IIBWorker: submit merge index image request
    IIBWorker->>BuildMergeTask: _add_bundles_missing_in_source(request_id, ignore_bundle_ocp_version)

    alt ignore_bundle_ocp_version is True
        BuildMergeTask->>RequestStore: get_request(request_id)
        RequestStore-->>BuildMergeTask: request_data (includes user)
        BuildMergeTask->>Config: get_worker_config()
        Config-->>BuildMergeTask: config (includes iib_no_ocp_label_allow_list)
        loop for each entry in iib_no_ocp_label_allow_list
            BuildMergeTask->>BuildMergeTask: check target_index_tmp startswith entry
            BuildMergeTask->>BuildMergeTask: check source_from_index startswith entry
            BuildMergeTask->>BuildMergeTask: check user equals entry
        end
        BuildMergeTask->>BuildMergeTask: set allow_no_ocp_version based on any match
    else ignore_bundle_ocp_version is False
        BuildMergeTask->>BuildMergeTask: set allow_no_ocp_version to False
    end

Loading

File-Level Changes

Change	Details	Files
Expand allow-list logic for ignoring missing OCP version labels to include matching on the requesting user, not just registry prefixes.	Retrieve the request user via get_request(request_id)['user'] when ignore_bundle_ocp_version is True. Update allow_no_ocp_version computation to check each allow-list entry against target index prefix, source index prefix, or user equality. Keep previous behavior of allowing based on index prefixes while adding user-based conditionally ignoring of missing OCP version labels.	iib/workers/tasks/build_merge_index_image.py
Add unit tests to validate allow-list behavior when the requesting user is and is not in the iib_no_ocp_label_allow_list.	Introduce test_add_bundles_missing_in_source_user_in_allow_list to assert that a missing bundle without an OCP label is not treated as invalid when the user is in the allow list. Introduce test_add_bundles_missing_in_source_user_not_in_allow_list to assert that the same bundle is treated as invalid when the user is not in the allow list. Mock get_worker_config, get_request, and get_image_label to simulate the different configurations, users, and label values while asserting correct missing_bundles and invalid_bundles outputs.	tests/test_workers/test_tasks/test_build_merge_index_image.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[qodo-code-review]

Review Summary by Qodo

Add user check to iib_no_ocp_label_allow_list validation

✨ Enhancement

Walkthroughs

Description

• Add user validation to iib_no_ocp_label_allow_list check
• Allow users in allow list to bypass OCP label validation
• Import get_request function for retrieving user information
• Add comprehensive test coverage for user allow list scenarios

Diagram

flowchart LR
  A["ignore_bundle_ocp_version flag"] --> B["Get request user info"]
  B --> C["Check allow list"]
  C --> D["Match by index OR user"]
  D --> E["Allow or reject bundle"]

Loading

File Changes

1. iib/workers/tasks/build_merge_index_image.py ✨ Enhancement +6/-3

Add user validation to allow list check

• Import get_request function from iib.workers.api_utils
• Retrieve user information from request when checking OCP label allowlist
• Extend allow list matching logic to include user comparison alongside index matching
• Refactor loop variable from index to entry for clarity

iib/workers/tasks/build_merge_index_image.py

---

2. tests/test_workers/test_tasks/test_build_merge_index_image.py 🧪 Tests +167/-0

Add user allow list validation test coverage

• Add test for user in allow list scenario allowing bundles with missing OCP labels
• Add test for user not in allow list scenario rejecting bundles with missing OCP labels
• Mock get_request to return user information in both test cases
• Verify get_request is called with correct request ID

tests/test_workers/test_tasks/test_build_merge_index_image.py

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Test triggers real HTTP 🐞 Bug ☼ Reliability

Description

_add_bundles_missing_in_source() now calls get_request() whenever
ignore_bundle_ocp_version=True, but the existing test_add_bundles_missing_in_source still
exercises that path without mocking get_request, causing a real HTTP GET during unit tests and
likely failing the test suite.

Code

iib/workers/tasks/build_merge_index_image.py[R169-175]

+        user = get_request(request_id)['user']
        allow_no_ocp_version = any(
-            target_index_tmp.startswith(index) or source_from_index.startswith(index)
-            for index in get_worker_config()['iib_no_ocp_label_allow_list']
+            target_index_tmp.startswith(entry)
+            or source_from_index.startswith(entry)
+            or user == entry
+            for entry in get_worker_config()['iib_no_ocp_label_allow_list']
        )

Evidence

The PR change introduces an unconditional get_request(request_id) call under
ignore_bundle_ocp_version. The existing test test_add_bundles_missing_in_source calls
_add_bundles_missing_in_source(..., ignore_bundle_ocp_version=True) but does not patch
build_merge_index_image.get_request, so the code will execute api_utils.get_request(), which
performs a real requests_session.get() to .../builds/{request_id}.

iib/workers/tasks/build_merge_index_image.py[167-175]
tests/test_workers/test_tasks/test_build_merge_index_image.py[556-639]
iib/workers/api_utils.py[49-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_add_bundles_missing_in_source()` now calls `get_request()` when `ignore_bundle_ocp_version=True`, which triggers a real HTTP request in `test_add_bundles_missing_in_source` because that test does not mock `get_request`.

### Issue Context
Unit tests should not perform network I/O; this will make CI flaky/fail.

### Fix
Patch `iib.workers.tasks.build_merge_index_image.get_request` in `test_add_bundles_missing_in_source` and set a deterministic return value (e.g., `{'user': 'some_user'}`), or set `ignore_bundle_ocp_version=False` if that path isn’t intended to be tested there.

### Fix Focus Areas
- tests/test_workers/test_tasks/test_build_merge_index_image.py[556-639]
- iib/workers/tasks/build_merge_index_image.py[167-175]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Wrong config patch target 🐞 Bug ≡ Correctness

Description

The newly added tests patch iib.workers.config.get_worker_config, but build_merge_index_image
imports get_worker_config into its module namespace; the patch won’t affect
build_merge_index_image.get_worker_config, so the tests won’t actually control the allow list
value as intended.

Code

tests/test_workers/test_tasks/test_build_merge_index_image.py[R700-710]

+@mock.patch('iib.workers.config.get_worker_config')
+@mock.patch('iib.workers.tasks.build_merge_index_image.is_image_fbc')
+@mock.patch('iib.workers.tasks.build_merge_index_image.get_image_label')
+@mock.patch('iib.workers.tasks.build_merge_index_image._create_and_push_manifest_list')
+@mock.patch('iib.workers.tasks.build_merge_index_image._push_image')
+@mock.patch('iib.workers.tasks.build_merge_index_image._build_image')
+@mock.patch('iib.workers.tasks.build_merge_index_image._add_label_to_index')
+@mock.patch('iib.workers.tasks.build_merge_index_image.opm_index_add')
+@mock.patch('iib.workers.tasks.build_merge_index_image.set_request_state')
+@mock.patch('iib.workers.tasks.build_merge_index_image.get_request')
+def test_add_bundles_missing_in_source_user_in_allow_list(

Evidence

build_merge_index_image.py binds get_worker_config via `from iib.workers.config import
get_worker_config, so patching the symbol in iib.workers.config` does not replace the
already-imported reference. The new tests patch the wrong target and then rely on
mock_gwc.return_value to set iib_no_ocp_label_allow_list, which will not be used by the code
under test.

iib/workers/tasks/build_merge_index_image.py[12-16]
tests/test_workers/test_tasks/test_build_merge_index_image.py[700-748]
iib/workers/tasks/build_merge_index_image.py[170-175]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Tests are patching `iib.workers.config.get_worker_config`, but the code under test uses `build_merge_index_image.get_worker_config` (imported directly). This makes the mock ineffective.

### Issue Context
In Python, `from module import name` creates a local binding; patching `module.name` later does not update already-imported bindings.

### Fix
Update the new tests to patch `iib.workers.tasks.build_merge_index_image.get_worker_config` (the lookup site), not `iib.workers.config.get_worker_config`.

### Fix Focus Areas
- tests/test_workers/test_tasks/test_build_merge_index_image.py[700-866]
- iib/workers/tasks/build_merge_index_image.py[12-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Allow list docs inconsistent 🐞 Bug ⚙ Maintainability

Description

The code now treats iib_no_ocp_label_allow_list entries as either index-image prefixes or
usernames, but README/API schema/config comments still describe it only as a list of index images,
creating a high risk of misconfiguration and operational confusion.

Code

iib/workers/tasks/build_merge_index_image.py[R169-175]

+        user = get_request(request_id)['user']
        allow_no_ocp_version = any(
-            target_index_tmp.startswith(index) or source_from_index.startswith(index)
-            for index in get_worker_config()['iib_no_ocp_label_allow_list']
+            target_index_tmp.startswith(entry)
+            or source_from_index.startswith(entry)
+            or user == entry
+            for entry in get_worker_config()['iib_no_ocp_label_allow_list']
        )

Evidence

The PR adds or user == entry to the allow-list check, expanding the meaning of
iib_no_ocp_label_allow_list. Multiple docs still state it is a list of index images (and the API
spec describes only target_index-based allow listing), which is now incomplete/inaccurate.

iib/workers/tasks/build_merge_index_image.py[169-175]
README.md[345-349]
iib/workers/config.py[49-52]
iib/web/static/api_v1.yaml[1363-1369]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`iib_no_ocp_label_allow_list` now supports usernames in addition to index-image prefixes, but documentation/comments/API schema still describe only index-image entries.

### Issue Context
This can lead to operators not realizing username entries are supported (or mistakenly assuming they are not), and makes config intent ambiguous.

### Fix
Update README, worker config comments, and the OpenAPI schema text for `ignore_bundle_ocp_version` to explicitly state that allow-list entries may be either:
- index image prefix strings (existing behavior), or
- usernames (new behavior)

### Fix Focus Areas
- README.md[345-349]
- iib/workers/config.py[49-52]
- iib/web/static/api_v1.yaml[1363-1369]
- iib/workers/tasks/build_merge_index_image.py[169-175]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[sourcery-ai]

Hey - I've left some high level feedback:

• Mixing index prefixes and usernames in the same iib_no_ocp_label_allow_list entry type makes the semantics of the allow list unclear; consider splitting this into separate config keys (e.g. index prefixes vs allowed users) or clearly distinguishing types in code.
• Accessing get_request(request_id)['user'] assumes the user key is always present; consider using .get('user') with a safe default or handling the missing-key case explicitly to avoid a KeyError.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Mixing index prefixes and usernames in the same `iib_no_ocp_label_allow_list` entry type makes the semantics of the allow list unclear; consider splitting this into separate config keys (e.g. index prefixes vs allowed users) or clearly distinguishing types in code.
- Accessing `get_request(request_id)['user']` assumes the `user` key is always present; consider using `.get('user')` with a safe default or handling the missing-key case explicitly to avoid a `KeyError`.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[chandwanitulsi]

Overall looks good, one question on unrelated change

[lipoja]

I agree with the comments from Yash and Tulsi. Let's address it and then we can do final approval.

[lipoja]

Important

Please propagate this change to main branch as well.

@MichalZelenak