Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add related_images check in IIB #556

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
4 changes: 4 additions & 0 deletions iib/web/iib_static_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class RelatedBundlesMetadata(TypedDict):
'build_tags',
'bundles',
'cnr_token',
'check_related_images',
'deprecation_list',
'distribution_scope',
'force_backport',
Expand Down Expand Up @@ -89,6 +90,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 +201,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 Expand Up @@ -346,6 +349,7 @@ class AddRmRequestResponseBase(CommonIndexImageResponseBase, APIPartImageBuildRe
class AddRequestResponse(AddRmRequestResponseBase):
"""Datastructure of the response to request from /builds/add API point."""

check_related_images: bool
graph_update_mode: str
omps_operator_version: Dict[str, Any]

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""Add check_related_images flag.

Revision ID: 7346beaff092
Revises: daf67ddcf4a1
Create Date: 2023-08-09 23:48:37.624078

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '7346beaff092'
down_revision = 'daf67ddcf4a1'
branch_labels = None
depends_on = None


def upgrade():
with op.batch_alter_table('request_add', schema=None) as batch_op:
batch_op.add_column(sa.Column('check_related_images', sa.BOOLEAN(), nullable=True))


def downgrade():
with op.batch_alter_table('request_add', schema=None) as batch_op:
batch_op.drop_column('check_related_images')
14 changes: 14 additions & 0 deletions iib/web/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ class RequestAdd(Request, RequestIndexImageMixin):
db.ForeignKey('request.id'), autoincrement=False, primary_key=True
)
bundles: Mapped[List['Image']] = db.relationship('Image', secondary=RequestAddBundle.__table__)
check_related_images: Mapped[Optional[bool]]
deprecation_list: Mapped[List['Image']] = db.relationship(
'Image', secondary=RequestAddBundleDeprecation.__table__
)
Expand Down Expand Up @@ -1073,6 +1074,17 @@ 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')

# Verify that `check_related_images` is specified when bundles are specified
if request_kwargs.get('check_related_images') and not request_kwargs.get('bundles'):
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.get('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 +1130,7 @@ def from_json( # type: ignore[override] # noqa: F821
'deprecation_list',
'graph_update_mode',
'build_tags',
'check_related_images',
],
batch=batch,
)
Expand Down Expand Up @@ -1156,6 +1169,7 @@ def to_json(self, verbose: Optional[bool] = True) -> AddRequestResponse:
if self.omps_operator_version:
rv['omps_operator_version'] = json.loads(self.omps_operator_version)
rv['graph_update_mode'] = self.graph_update_mode
rv['check_related_images'] = self.check_related_images

for bundle in self.bundles:
if bundle.operator:
Expand Down
9 changes: 9 additions & 0 deletions iib/web/static/api_v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,12 @@ components:
items:
type: string
example: ["v4.5-10-08-2021"]
check_related_images:
description: >
Flag to indicate if related and depending images of a bundle should be inspected
type: boolean
example: true
default: false
required:
- bundles
AddResponse:
Expand All @@ -1066,6 +1072,9 @@ components:
graph_update_mode:
type: string
example: semver
check_related_images:
type: boolean
example: true
RmRequest:
type: object
properties:
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 @@ -785,6 +786,44 @@ def _verify_index_image(
)


def inspect_related_images(bundles: List[str], request_id) -> None:
"""
Verify if related_images and other dependancy images in the bundle can be inspected.

: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 @@ -804,6 +843,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 @@ -851,6 +891,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
40 changes: 35 additions & 5 deletions tests/test_web/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def test_get_build(app, auth_env, client, db):
'build_tags': ['build-tag-1'],
'bundle_mapping': {},
'bundles': ['quay.io/namespace/bundle:1.0-3'],
'check_related_images': None,
'deprecation_list': [],
'distribution_scope': None,
'from_index': 'quay.io/namespace/repo:latest',
Expand Down Expand Up @@ -475,6 +476,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 @@ -823,6 +850,7 @@ def test_add_bundle_success(
'build_tags': [],
'bundle_mapping': {},
'bundles': bundles,
'check_related_images': None,
'deprecation_list': [],
'distribution_scope': expected_distribution_scope,
'from_index': from_index,
Expand Down Expand Up @@ -901,10 +929,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 @@ -1172,6 +1200,7 @@ def test_patch_request_add_success(
'build_tags': [],
'bundle_mapping': bundle_mapping,
'bundles': bundles,
'check_related_images': None,
'deprecation_list': [],
'distribution_scope': distribution_scope,
'from_index': None,
Expand Down Expand Up @@ -1797,12 +1826,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
Loading