Skip to content

Commit

Permalink
Add related_images check in IIB
Browse files Browse the repository at this point in the history
  • Loading branch information
Tulsi Chandwani committed Aug 7, 2023
1 parent 038e18c commit 9d5d90f
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 6 deletions.
1 change: 1 addition & 0 deletions iib/web/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ def _get_add_args(
payload.get('deprecation_list', []),
payload.get('build_tags', []),
payload.get('graph_update_mode'),
payload.get('check_related_images', False),
]


Expand Down
2 changes: 2 additions & 0 deletions iib/web/iib_static_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class AddRequestPayload(TypedDict):
build_tags: NotRequired[List[str]]
bundles: List[str]
cnr_token: NotRequired[str]
check_related_images: NotRequired[bool]
deprecation_list: NotRequired[List[str]]
distribution_scope: NotRequired[str]
force_backport: NotRequired[bool]
Expand Down Expand Up @@ -199,6 +200,7 @@ class RequestPayload(TypedDict):
build_tags: NotRequired[List[str]]
bundles: NotRequired[Optional[List[str]]]
cnr_token: NotRequired[str]
check_related_images: NotRequired[bool]
deprecation_list: NotRequired[List[str]]
distribution_scope: NotRequired[str]
fbc_fragment: NotRequired[bool]
Expand Down
15 changes: 15 additions & 0 deletions iib/web/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,7 @@ def _from_json(
'distribution_scope',
'build_tags',
'output_fbc',
'check_related_images',
} | set(additional_optional_params or [])

validate_request_params(
Expand Down Expand Up @@ -1073,6 +1074,19 @@ def from_json( # type: ignore[override] # noqa: F821
if not (request_kwargs.get('bundles') or request_kwargs.get('from_index')):
raise ValidationError('"from_index" must be specified if no bundles are specified')

if (
request_kwargs.get('check_related_images') is not None
and request_kwargs.get('bundles') is None
):
raise ValidationError(
'"check_related_images" must be specified only when bundles are specified'
)

# Verify that `check_related_images` is the correct type
check_related_images = request_kwargs.pop('check_related_images', False)
if not isinstance(check_related_images, bool):
raise ValidationError('The "check_related_images" parameter must be a boolean')

ALLOWED_KEYS_1: Sequence[Literal['cnr_token', 'graph_update_mode', 'organization']] = (
'cnr_token',
'graph_update_mode',
Expand Down Expand Up @@ -1118,6 +1132,7 @@ def from_json( # type: ignore[override] # noqa: F821
'deprecation_list',
'graph_update_mode',
'build_tags',
'check_related_images',
],
batch=batch,
)
Expand Down
44 changes: 43 additions & 1 deletion iib/workers/tasks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
import shutil
import stat
import tempfile
import ruamel.yaml
from typing import Dict, List, Optional, Set, Tuple

from operator_manifest.operator import ImageName
from operator_manifest.operator import ImageName, OperatorManifest
from tenacity import (
before_sleep_log,
retry,
Expand Down Expand Up @@ -778,6 +779,44 @@ def _verify_index_image(
)


def inspect_related_images(bundles: List[str], request_id) -> None:
"""
Verify if related_images in the bundle are pullable.
:param list bundles: a list of strings representing the pull specifications of the bundles to
add to the index image being built.
:param int request_id: the ID of the request this index image is for.
:raises IIBError: if one of the bundles does not have the pullable related_image.
"""
from iib.workers.tasks.build_regenerate_bundle import _get_bundle_metadata

invalid_related_images = []
for bundle in bundles:
manifest_location = get_image_label(
bundle, "operators.operatorframework.io.bundle.manifests.v1"
)
with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir:
_copy_files_from_image(bundle, manifest_location, temp_dir)
manifest_path = os.path.join(temp_dir, manifest_location)
try:
operator_manifest = OperatorManifest.from_directory(manifest_path)
except (ruamel.yaml.YAMLError, ruamel.yaml.constructor.DuplicateKeyError) as e:
error = f'The Operator Manifest is not in a valid YAML format: {e}'
log.exception(error)
raise IIBError(error)
bundle_metadata = _get_bundle_metadata(operator_manifest, False)
for related_image in bundle_metadata['found_pullspecs']:
related_image_pull_spec = related_image.to_str()
try:
skopeo_inspect(f"docker://{related_image_pull_spec}")
except IIBError as e:
log.error(e)
invalid_related_images.append(related_image_pull_spec)

if invalid_related_images:
raise IIBError(f"IIB cannot access the following related images {invalid_related_images}")


@app.task
@request_logger
def handle_add_request(
Expand All @@ -797,6 +836,7 @@ def handle_add_request(
deprecation_list: Optional[List[str]] = None,
build_tags: Optional[List[str]] = None,
graph_update_mode: Optional[str] = None,
check_related_images: bool = False,
traceparent: Optional[str] = None,
) -> None:
"""
Expand Down Expand Up @@ -844,6 +884,8 @@ def handle_add_request(
with set_registry_token(overwrite_from_index_token, from_index, append=True):
resolved_bundles = get_resolved_bundles(bundles)
verify_labels(resolved_bundles)
if check_related_images:
inspect_related_images(resolved_bundles, request_id)

# Check if Gating passes for all the bundles
if greenwave_config:
Expand Down
37 changes: 32 additions & 5 deletions tests/test_web/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,32 @@ def test_get_build_logs_s3_configured(
},
'The "overwrite_from_index" parameter must be a boolean',
),
(
{
'bundles': ['some:thing'],
'from_index': 'pull:spec',
'binary_image': 'binary:image',
'check_related_images': 123,
},
'The "check_related_images" parameter must be a boolean',
),
(
{
'bundles': ['some:thing'],
'from_index': 'pull:spec',
'binary_image': 'binary:image',
'check_related_images': "123",
},
'The "check_related_images" parameter must be a boolean',
),
(
{
'from_index': 'pull:spec',
'binary_image': 'binary:image',
'check_related_images': "123",
},
'"check_related_images" must be specified only when bundles are specified',
),
(
{
'bundles': ['some:thing'],
Expand Down Expand Up @@ -901,10 +927,10 @@ def test_add_bundle_overwrite_token_redacted(mock_smfsc, mock_har, app, auth_env
rv_json = rv.json
assert rv.status_code == 201
mock_har.apply_async.assert_called_once()
# Fourth to last element in args is the overwrite_from_index parameter
assert mock_har.apply_async.call_args[1]['args'][-8] is True
# Third to last element in args is the overwrite_from_index_token parameter
assert mock_har.apply_async.call_args[1]['args'][-7] == token
# Ninth to last element in args is the overwrite_from_index parameter
assert mock_har.apply_async.call_args[1]['args'][-9] is True
# Eighth to last element in args is the overwrite_from_index_token parameter
assert mock_har.apply_async.call_args[1]['args'][-8] == token
assert 'overwrite_from_index_token' not in rv_json
assert token not in json.dumps(rv_json)
assert token not in mock_har.apply_async.call_args[1]['argsrepr']
Expand Down Expand Up @@ -1797,12 +1823,13 @@ def test_add_rm_batch_success(mock_smfnbor, mock_hrr, mock_har, app, auth_env, c
[],
[],
None,
False,
],
argsrepr=(
"[['registry-proxy/rh-osbs/lgallett-bundle:v1.0-9'], "
"1, 'registry-proxy/rh-osbs/openshift-ose-operator-registry:v4.5', "
"'registry-proxy/rh-osbs-stage/iib:v4.5', ['amd64'], '*****', "
"'hello-operator', None, True, '*****', None, None, {}, [], [], None]"
"'hello-operator', None, True, '*****', None, None, {}, [], [], None, False]"
),
link_error=mock.ANY,
queue=None,
Expand Down
118 changes: 118 additions & 0 deletions tests/test_workers/test_tasks/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from iib.workers.tasks import build
from iib.workers.tasks.utils import RequestConfigAddRm
from iib.workers.config import get_worker_config
from operator_manifest.operator import ImageName

worker_config = get_worker_config()

Expand Down Expand Up @@ -1335,3 +1336,120 @@ def test_get_no_present_bundles(
assert bundle == []
assert bundle_pull_spec == []
mock_run_cmd.assert_called_once()


@mock.patch('iib.workers.tasks.build.skopeo_inspect')
@mock.patch('iib.workers.tasks.build_regenerate_bundle._get_bundle_metadata')
@mock.patch('iib.workers.tasks.build.OperatorManifest.from_directory')
@mock.patch('iib.workers.tasks.build._copy_files_from_image')
@mock.patch('iib.workers.tasks.build.get_image_label')
def test_inspect_related_images(mock_gil, mock_cffi, mock_fd, mock_gbd, mock_si, tmpdir):
bundles = ['quay.io/repo/image@sha256:123', 'quay.io/repo2/image2@sha256:456']
request_id = 5
mock_gil.return_value = '/manifests'
mock_fd.return_value = mock.ANY

mock_gbd.side_effect = [
{
'found_pullspecs': set(
[
ImageName.parse('quay.io/related/image@sha256:1'),
ImageName.parse('quay.io/related/image@sha256:2'),
ImageName.parse('quay.io/related/image@sha256:3'),
]
)
},
{
'found_pullspecs': set(
[
ImageName.parse('quay.io/related/image@sha256:4'),
ImageName.parse('quay.io/related/image@sha256:5'),
]
)
},
]
build.inspect_related_images(bundles=bundles, request_id=request_id)

assert mock_si.call_count == 5
assert mock_gbd.call_count == 2
assert mock_gil.call_args_list == [
mock.call(
'quay.io/repo/image@sha256:123', 'operators.operatorframework.io.bundle.manifests.v1'
),
mock.call(
'quay.io/repo2/image2@sha256:456', 'operators.operatorframework.io.bundle.manifests.v1'
),
]


@mock.patch('iib.workers.tasks.utils.run_cmd')
@mock.patch('iib.workers.tasks.build_regenerate_bundle._get_bundle_metadata')
@mock.patch('iib.workers.tasks.build.OperatorManifest.from_directory')
@mock.patch('iib.workers.tasks.build._copy_files_from_image')
@mock.patch('iib.workers.tasks.build.get_image_label')
def test_inspect_related_images_fail(mock_gil, mock_cffi, mock_fd, mock_gbd, mock_rc, tmpdir):
bundles = ['quay.io/repo/image@sha256:123']
request_id = 5
mock_gil.return_value = '/manifests'
mock_fd.return_value = mock.ANY
related_images = ['quay.io/related/image@sha256:1']

mock_gbd.return_value = {
'found_pullspecs': set(
[
ImageName.parse(related_images[0]),
]
)
}

error_msg = f'IIB cannot access the following related images {related_images}'
with pytest.raises(IIBError, match=re.escape(error_msg)):
mock_rc.side_effect = IIBError('Image not accessible')
build.inspect_related_images(bundles=bundles, request_id=request_id)

assert mock_rc.call_count == 5
assert mock_gbd.call_count == 1
assert mock_gil.call_args_list == [
mock.call(
'quay.io/repo/image@sha256:123', 'operators.operatorframework.io.bundle.manifests.v1'
)
]


@mock.patch('iib.workers.tasks.build._cleanup')
@mock.patch('iib.workers.tasks.build.set_request_state')
@mock.patch('iib.workers.tasks.build.get_resolved_bundles')
@mock.patch('iib.workers.tasks.build.verify_labels')
@mock.patch('iib.workers.tasks.build.inspect_related_images')
def test_handle_add_request_check_related_images_fail(
mock_iri, mock_vl, mock_grb, mock_srs, mock_cleanup
):
bundles = ['some-bundle:2.3-1']
error_msg = 'IIB cannot access the following related images [quay.io/related/image@sha256:1]'
mock_grb.return_value = ['some-bundle@sha256:123']
mock_iri.side_effect = IIBError(error_msg)
with pytest.raises(IIBError, match=re.escape(error_msg)):
build.handle_add_request(
bundles=bundles,
request_id=3,
binary_image='binary-image:latest',
from_index='from-index:latest',
add_arches=['s390x'],
cnr_token='token',
organization='org',
force_backport=False,
overwrite_from_index=False,
overwrite_from_index_token=None,
distribution_scope=None,
greenwave_config=None,
binary_image_config={'prod': {'v4.5': 'some_image'}},
deprecation_list=[],
build_tags=None,
graph_update_mode=None,
check_related_images=True,
)
assert mock_cleanup.call_count == 1
mock_srs.assert_called_once()
mock_grb.assert_called_once_with(bundles)
mock_vl.assert_called_once()
mock_iri.assert_called_once_with(['some-bundle@sha256:123'], 3)

0 comments on commit 9d5d90f

Please sign in to comment.