Fix: Use unique sub-dir for extracting each FBC fragment

IIB used a config value to name the sub-directory when
extracting the FBC fragment. This worked fine until we allowed
one fragment per request. With the support for multiple fragments
data gets overwritten causing unexpected behavior. This commit
fixes that behavior

Signed-off-by: Yashvardhan Nanavati <[email protected]>

Assisted-by: Cursor
Signed-off-by: Yashvardhan Nanavati <[email protected]>

Author: yashvardhannanavati

iib/workers/tasks/build_fbc_operations.py

@@ -150,6 +150,6 @@ def handle_fbc_operation_request(
     set_request_state(
         request_id,
         'complete',
-        f"The {len(resolved_fbc_fragments)} FBC fragment(s) were successfully added"
+        f"The {len(resolved_fbc_fragments)} FBC fragment(s) were successfully added "
         "in the index image",
     )

iib/workers/tasks/fbc_utils.py

@@ -102,22 +102,27 @@ def merge_catalogs_dirs(src_config: str, dest_config: str):
     opm_validate(conf_dir)
 
 
-def extract_fbc_fragment(temp_dir: str, fbc_fragment: str) -> Tuple[str, List[str]]:
+def extract_fbc_fragment(
+    temp_dir: str, fbc_fragment: str, fragment_index: int = 0
+) -> Tuple[str, List[str]]:
     """
     Extract operator packages from the fbc_fragment image.
 
     :param str temp_dir: base temp directory for IIB request.
     :param str fbc_fragment: pull specification of fbc_fragment in the IIB request.
+    :param int fragment_index: index of the fragment to create unique paths and
+        prevent cross-contamination.
     :return: fbc_fragment path, fbc_operator_packages.
     :rtype: tuple
     """
     from iib.workers.tasks.build import _copy_files_from_image
 
     log.info("Extracting the fbc_fragment's catalog from  %s", fbc_fragment)
-    # store the fbc_fragment at /tmp/iib-**/fbc-fragment
+    # store the fbc_fragment at /tmp/iib-**/fbc-fragment-{index} to prevent
+    # cross-contamination
     conf = get_worker_config()
-    fbc_fragment_path = os.path.join(temp_dir, conf['temp_fbc_fragment_path'])
-    # Copy fbc_fragment's catalog to /tmp/iib-**/fbc-fragment
+    fbc_fragment_path = os.path.join(temp_dir, f"{conf['temp_fbc_fragment_path']}-{fragment_index}")
+    # Copy fbc_fragment's catalog to /tmp/iib-**/fbc-fragment-{index}
     _copy_files_from_image(fbc_fragment, conf['fbc_fragment_catalog_path'], fbc_fragment_path)
 
     log.info("fbc_fragment extracted at %s", fbc_fragment_path)

iib/workers/tasks/opm_operations.py

@@ -1020,10 +1020,10 @@ def opm_registry_add_fbc_fragment(
     fragment_data = []
     all_fragment_operators = []
 
-    for fbc_fragment in fbc_fragments:
+    for i, fbc_fragment in enumerate(fbc_fragments):
         # fragment path will look like /tmp/iib-**/fbc-fragment-{index}
         fragment_path, fragment_operators = extract_fbc_fragment(
-            temp_dir=temp_dir, fbc_fragment=fbc_fragment
+            temp_dir=temp_dir, fbc_fragment=fbc_fragment, fragment_index=i
         )
         fragment_data.append((fragment_path, fragment_operators))
         all_fragment_operators.extend(fragment_operators)
@@ -1066,7 +1066,8 @@ def opm_registry_add_fbc_fragment(
         set_request_state(
             request_id,
             'in_progress',
-            f'Adding fbc fragment {i + 1}/{len(fbc_fragments)} to from_index',
+            f'Adding package(s) {fragment_operators} from fbc fragment '
+            f'{i + 1}/{len(fbc_fragments)} to from_index',
         )
 
         for fragment_operator in fragment_operators:

tests/test_workers/test_tasks/test_fbc_utils.py

@@ -196,8 +196,8 @@ def test_enforce_json_config_dir_multiple_chunks_input(tmpdir):
         '{"one_more": "chunk", "createdAt": "2025-01-21T07:15:29"}'
     )
 
-    input = os.path.join(tmpdir, f"test_file.yaml")
-    output = os.path.join(tmpdir, f"test_file.json")
+    input = os.path.join(tmpdir, "test_file.yaml")
+    output = os.path.join(tmpdir, "test_file.json")
     with open(input, 'w') as w:
         w.write(dedent(multiple_chunks_yaml))
 
@@ -213,15 +213,82 @@ def test_enforce_json_config_dir_multiple_chunks_input(tmpdir):
 def test_extract_fbc_fragment(mock_cffi, mock_osldr, ldr_output, tmpdir):
     test_fbc_fragment = "example.com/test/fbc_fragment:latest"
     mock_osldr.return_value = ldr_output
-    fbc_fragment_path = os.path.join(tmpdir, get_worker_config()['temp_fbc_fragment_path'])
+    # The function now adds -0 suffix by default when fragment_index is not provided
+    fbc_fragment_path = os.path.join(tmpdir, f"{get_worker_config()['temp_fbc_fragment_path']}-0")
 
     if not ldr_output:
         with pytest.raises(IIBError):
             extract_fbc_fragment(tmpdir, test_fbc_fragment)
     else:
         extract_fbc_fragment(tmpdir, test_fbc_fragment)
-    mock_cffi.assert_has_calls([mock.call(test_fbc_fragment, '/configs', fbc_fragment_path)])
-    mock_osldr.assert_has_calls([mock.call(fbc_fragment_path)])
+        mock_cffi.assert_called_once_with(
+            test_fbc_fragment, get_worker_config()['fbc_fragment_catalog_path'], fbc_fragment_path
+        )
+        mock_osldr.assert_called_once_with(fbc_fragment_path)
+
+
+@pytest.mark.parametrize('ldr_output', [['testoperator'], ['test1', 'test2']])
+@mock.patch('os.listdir')
+@mock.patch('iib.workers.tasks.build._copy_files_from_image')
+def test_extract_fbc_fragment_with_index(mock_cffi, mock_osldr, ldr_output, tmpdir):
+    """Test extract_fbc_fragment with non-zero fragment_index values."""
+    test_fbc_fragment = "example.com/test/fbc_fragment:latest"
+    mock_osldr.return_value = ldr_output
+
+    # Test with fragment_index = 2
+    fragment_index = 2
+    fbc_fragment_path = os.path.join(
+        tmpdir, f"{get_worker_config()['temp_fbc_fragment_path']}-{fragment_index}"
+    )
+
+    result_path, result_operators = extract_fbc_fragment(
+        tmpdir, test_fbc_fragment, fragment_index=fragment_index
+    )
+
+    # Verify the path includes the correct index
+    assert result_path == fbc_fragment_path
+    assert result_path.endswith(f"-{fragment_index}")
+    assert result_operators == ldr_output
+
+    # Verify the function was called with the correct path
+    mock_cffi.assert_called_once_with(
+        test_fbc_fragment, get_worker_config()['fbc_fragment_catalog_path'], fbc_fragment_path
+    )
+    mock_osldr.assert_called_once_with(fbc_fragment_path)
+
+
+@mock.patch('os.listdir')
+@mock.patch('iib.workers.tasks.build._copy_files_from_image')
+def test_extract_fbc_fragment_isolation(mock_cffi, mock_osldr, tmpdir):
+    """Test that multiple fragments with different indices create isolated directories."""
+    test_fbc_fragment1 = "example.com/test/fbc_fragment1:latest"
+    test_fbc_fragment2 = "example.com/test/fbc_fragment2:latest"
+
+    # Mock different outputs for each fragment
+    mock_osldr.side_effect = [['operator1'], ['operator2']]
+
+    # Extract first fragment with index 0
+    path1, operators1 = extract_fbc_fragment(tmpdir, test_fbc_fragment1, fragment_index=0)
+
+    # Extract second fragment with index 1
+    path2, operators2 = extract_fbc_fragment(tmpdir, test_fbc_fragment2, fragment_index=1)
+
+    # Verify paths are different and include correct indices
+    assert path1 != path2
+    assert path1.endswith("-0")
+    assert path2.endswith("-1")
+
+    # Verify operators are different (no cross-contamination)
+    assert operators1 == ['operator1']
+    assert operators2 == ['operator2']
+    assert operators1 != operators2
+
+    # Verify _copy_files_from_image was called with different paths
+    expected_calls = [
+        mock.call(test_fbc_fragment1, get_worker_config()['fbc_fragment_catalog_path'], path1),
+        mock.call(test_fbc_fragment2, get_worker_config()['fbc_fragment_catalog_path'], path2),
+    ]
+    mock_cffi.assert_has_calls(expected_calls, any_order=True)
 
 
 def test__serialize_datetime():
@@ -231,5 +298,5 @@ def test__serialize_datetime():
 
 
 def test__serialize_datetime_raise():
-    with pytest.raises(TypeError, match=f"Type <class 'int'> is not serializable."):
+    with pytest.raises(TypeError, match="Type <class 'int'> is not serializable."):
         _serialize_datetime(2025)

tests/test_workers/test_tasks/test_opm_operations.py

@@ -896,7 +896,7 @@ def test_opm_registry_add_fbc_fragment(
         10, tmpdir, from_index, binary_image, [fbc_fragment], None
     )
 
-    mock_eff.assert_called_with(temp_dir=tmpdir, fbc_fragment=fbc_fragment)
+    mock_eff.assert_called_with(temp_dir=tmpdir, fbc_fragment=fbc_fragment, fragment_index=0)
     mock_voe.assert_called_with(
         from_index=from_index,
         base_dir=tmpdir,