diff --git a/iib/web/api_v1.py b/iib/web/api_v1.py index 41a368522..4293e2604 100644 --- a/iib/web/api_v1.py +++ b/iib/web/api_v1.py @@ -1090,6 +1090,7 @@ def merge_index_image() -> Tuple[flask.Response, int]: request.distribution_scope, flask.current_app.config['IIB_BINARY_IMAGE_CONFIG'], payload.get('build_tags', []), + payload.get('graph_update_mode'), ] safe_args = _get_safe_args(args, payload) diff --git a/iib/web/iib_static_types.py b/iib/web/iib_static_types.py index da5dd4989..67514da74 100644 --- a/iib/web/iib_static_types.py +++ b/iib/web/iib_static_types.py @@ -162,6 +162,7 @@ class MergeIndexImagesPayload(TypedDict): build_tags: NotRequired[List[str]] deprecation_list: NotRequired[List[str]] distribution_scope: NotRequired[str] + graph_update_mode: NotRequired[GRAPH_MODE_LITERAL] overwrite_target_index: NotRequired[bool] overwrite_target_index_token: NotRequired[str] source_from_index: str @@ -389,6 +390,7 @@ class MergeIndexImageRequestResponse(APIPartImageBuildRequestResponse): build_tags: List[str] deprecation_list: List[str] distribution_scope: str + graph_update_mode: GRAPH_MODE_LITERAL index_image: Optional[str] source_from_index: str source_from_index_resolved: Optional[str] diff --git a/iib/web/migrations/versions/9e9d4f9730c8_merge_graph_update.py b/iib/web/migrations/versions/9e9d4f9730c8_merge_graph_update.py new file mode 100644 index 000000000..00b39ad1e --- /dev/null +++ b/iib/web/migrations/versions/9e9d4f9730c8_merge_graph_update.py @@ -0,0 +1,32 @@ +"""Adding graph_update_mode to merge index request. + +Revision ID: 9e9d4f9730c8 +Revises: 7346beaff092 +Create Date: 2023-10-17 11:11:10.558335 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '9e9d4f9730c8' +down_revision = '7346beaff092' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op: + batch_op.add_column(sa.Column('graph_update_mode', sa.String(), nullable=True)) + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op: + batch_op.drop_column('graph_update_mode') + + # ### end Alembic commands ### diff --git a/iib/web/models.py b/iib/web/models.py index f9c3ddcf8..ea172fde5 100644 --- a/iib/web/models.py +++ b/iib/web/models.py @@ -775,6 +775,30 @@ def get_request_query_options(verbose: Optional[bool] = False) -> List[_Abstract return query_options +def validate_graph_mode(graph_update_mode: Optional[str], index_image: Optional[str]): + """ + Validate graph mode and check if index image is allowed to use different graph mode. + + :param str graph_update_mode: one of the graph mode options + :param str index_image: pullspec of index image to which graph mode should be applied to + :raises: ValidationError when incorrect graph_update_mode is set + :raises: Forbidden when graph_mode can't be used for given index image + + """ + if graph_update_mode: + graph_mode_options = current_app.config['IIB_GRAPH_MODE_OPTIONS'] + if graph_update_mode not in graph_mode_options: + raise ValidationError( + f'"graph_update_mode" must be set to one of these: {graph_mode_options}' + ) + allowed_from_indexes: List[str] = current_app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] + if index_image not in allowed_from_indexes: + raise Forbidden( + '"graph_update_mode" can only be used on the' + f' following index image: {allowed_from_indexes}' + ) + + class RequestIndexImageMixin: """ A class for shared functionality between index image requests. @@ -1110,19 +1134,7 @@ def from_json( # type: ignore[override] # noqa: F821 raise ValidationError(f'"{param}" must be a string') if param == 'graph_update_mode': - graph_mode_options = current_app.config['IIB_GRAPH_MODE_OPTIONS'] - if request_kwargs[param] not in graph_mode_options: - raise ValidationError( - f'"{param}" must be set to one of these: {graph_mode_options}' - ) - allowed_from_indexes: List[str] = current_app.config[ - 'IIB_GRAPH_MODE_INDEX_ALLOW_LIST' - ] - if request_kwargs.get('from_index') not in allowed_from_indexes: - raise Forbidden( - '"graph_update_mode" can only be used on the' - f' following "from_index" pullspecs: {allowed_from_indexes}' - ) + validate_graph_mode(request_kwargs[param], request_kwargs.get('from_index')) if not isinstance(request_kwargs.get('force_backport', False), bool): raise ValidationError('"force_backport" must be a boolean') @@ -1498,6 +1510,7 @@ class RequestMergeIndexImage(Request): 'Image', foreign_keys=[target_index_resolved_id], uselist=False ) distribution_scope: Mapped[Optional[str]] + graph_update_mode: Mapped[Optional[str]] __mapper_args__ = { 'polymorphic_identity': RequestTypeMapping.__members__['merge_index_image'].value @@ -1536,6 +1549,9 @@ def from_json( # type: ignore[override] # noqa: F821 pull_specification=source_from_index ) + graph_update_mode = request_kwargs.get('graph_update_mode') + validate_graph_mode(graph_update_mode, request_kwargs.get('target_index')) + target_index = request_kwargs.pop('target_index', None) if target_index: if not isinstance(target_index, str): @@ -1623,6 +1639,7 @@ def to_json(self, verbose: Optional[bool] = True) -> MergeIndexImageRequestRespo self.binary_image_resolved, 'pull_specification', None ) rv['deprecation_list'] = [bundle.pull_specification for bundle in self.deprecation_list] + rv['graph_update_mode'] = self.graph_update_mode rv['index_image'] = getattr(self.index_image, 'pull_specification', None) rv['source_from_index'] = self.source_from_index.pull_specification rv['source_from_index_resolved'] = getattr( diff --git a/iib/web/static/api_v1.yaml b/iib/web/static/api_v1.yaml index 94b02d3b3..5450390dc 100644 --- a/iib/web/static/api_v1.yaml +++ b/iib/web/static/api_v1.yaml @@ -1338,6 +1338,14 @@ components: This determined what level of protection the addition or removal had. type: string example: 'prod' + graph_update_mode: + type: string + description: > + Graph update mode that defines how channel graphs are updated in the index. It must be one + of "replaces", "semver" or "semver-skippatch". This attribute can only be used on index + image pull specs configured in IIB_GRAPH_MODE_INDEX_ALLOW_LIST in the IIB API Config. If not + specified, "--mode" will not be added to OPM commands to add the bundle(s) to the index. + default: None RequestCreateEmptyIndex: type: object properties: diff --git a/iib/workers/tasks/build_merge_index_image.py b/iib/workers/tasks/build_merge_index_image.py index 0970dab29..54e98cfb3 100644 --- a/iib/workers/tasks/build_merge_index_image.py +++ b/iib/workers/tasks/build_merge_index_image.py @@ -60,6 +60,7 @@ def _add_bundles_missing_in_source( arch: str, ocp_version: str, distribution_scope: str, + graph_update_mode: Optional[str] = None, target_index=None, overwrite_target_index_token: Optional[str] = None, ) -> Tuple[List[BundleImage], List[BundleImage]]: @@ -77,6 +78,8 @@ def _add_bundles_missing_in_source( :param int request_id: the ID of the IIB build request. :param str arch: the architecture to build this image for. :param str ocp_version: ocp version which will be added as a label to the image. + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. :param str target_index: the pull specification of the container image :param str overwrite_target_index_token: the token used for overwriting the input ``source_from_index`` image. This is required to use ``overwrite_target_index``. @@ -138,6 +141,7 @@ def _add_bundles_missing_in_source( bundles=missing_bundle_paths, binary_image=binary_image, from_index=source_from_index, + graph_update_mode=graph_update_mode, container_tool='podman', ) else: @@ -146,6 +150,7 @@ def _add_bundles_missing_in_source( bundles=missing_bundle_paths, binary_image=binary_image, from_index=source_from_index, + graph_update_mode=graph_update_mode, # Use podman until opm's default mechanism is more resilient: # https://bugzilla.redhat.com/show_bug.cgi?id=1937097 container_tool='podman', @@ -179,6 +184,7 @@ def handle_merge_request( distribution_scope: Optional[str] = None, binary_image_config: Optional[str] = None, build_tags: Optional[List[str]] = None, + graph_update_mode: Optional[str] = None, ) -> None: """ Coordinate the work needed to merge old (N) index image with new (N+1) index image. @@ -199,6 +205,8 @@ def handle_merge_request( :param str distribution_scope: the scope for distribution of the index image, defaults to ``None``. :param build_tags: list of extra tag to use for intermetdiate index image + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. :raises IIBError: if the index image merge fails. """ _cleanup() @@ -265,6 +273,7 @@ def handle_merge_request( request_id=request_id, arch=arch, ocp_version=prebuild_info['target_ocp_version'], + graph_update_mode=graph_update_mode, target_index=target_index, overwrite_target_index_token=overwrite_target_index_token, distribution_scope=prebuild_info['distribution_scope'], diff --git a/tests/test_web/test_api_v1.py b/tests/test_web/test_api_v1.py index 6202d83cf..2c840be3b 100644 --- a/tests/test_web/test_api_v1.py +++ b/tests/test_web/test_api_v1.py @@ -578,7 +578,7 @@ def test_add_bundles_graph_update_mode_not_allowed( assert rv.status_code == 403 error_msg = ( '"graph_update_mode" can only be used on the' - ' following "from_index" pullspecs: [\'some-unique-index\']' + ' following index image: [\'some-unique-index\']' ) assert error_msg == rv.json['error'] mock_smfsc.assert_not_called() @@ -1937,14 +1937,16 @@ def test_regenerate_add_rm_batch_invalid_input(payload, error_msg, app, auth_env @mock.patch('iib.web.api_v1.handle_merge_request') @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change') def test_merge_index_image_success( - mock_smfsc, mock_merge, db, auth_env, client, distribution_scope + mock_smfsc, mock_merge, app, db, auth_env, client, distribution_scope ): + app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] = 'target_index:image' data = { 'deprecation_list': ['some@sha256:bundle'], 'binary_image': 'binary:image', 'source_from_index': 'source_index:image', 'target_index': 'target_index:image', 'build_tags': [], + 'graph_update_mode': 'semver', } if distribution_scope: @@ -1959,6 +1961,7 @@ def test_merge_index_image_success( 'build_tags': [], 'deprecation_list': ['some@sha256:bundle'], 'distribution_scope': distribution_scope, + 'graph_update_mode': 'semver', 'id': 1, 'index_image': None, 'logs': { @@ -1998,12 +2001,14 @@ def test_merge_index_image_success( def test_merge_index_image_overwrite_token_redacted( mock_smfsc, mock_merge, app, auth_env, client, db ): + app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] = 'target_index:image' token = 'username:password' data = { 'deprecation_list': ['some@sha256:bundle'], 'binary_image': 'binary:image', 'source_from_index': 'source_index:image', 'target_index': 'target_index:image', + 'graph_update_mode': 'replaces', 'overwrite_target_index': True, 'overwrite_target_index_token': token, } @@ -2054,12 +2059,14 @@ def test_merge_index_image_custom_user_queue( overwrite_from_index, expected_queue, ): + app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] = 'target_index:image' app.config['IIB_USER_TO_QUEUE'] = user_to_queue data = { 'deprecation_list': ['some@sha256:bundle'], 'binary_image': 'binary:image', 'source_from_index': 'source_index:image', 'target_index': 'target_index:image', + 'graph_update_mode': 'replaces', } if overwrite_from_index: data['overwrite_target_index'] = True diff --git a/tests/test_workers/test_tasks/test_build_merge_index_image.py b/tests/test_workers/test_tasks/test_build_merge_index_image.py index f75decaf3..124f79721 100644 --- a/tests/test_workers/test_tasks/test_build_merge_index_image.py +++ b/tests/test_workers/test_tasks/test_build_merge_index_image.py @@ -400,6 +400,7 @@ def test_add_bundles_missing_in_source( 'amd64', '4.6', 'dev', + 'replaces', ) assert missing_bundles == [ { @@ -436,6 +437,7 @@ def test_add_bundles_missing_in_source( binary_image='binary-image:4.5', from_index='index-image:4.6', container_tool='podman', + graph_update_mode='replaces', ) assert mock_gil.call_count == 5 assert mock_aolti.call_count == 2 @@ -545,6 +547,7 @@ def test_add_bundles_missing_in_source_error_tag_specified( 'amd64', '4.6', 'dev', + 'semver', ) @@ -611,6 +614,7 @@ def test_add_bundles_missing_in_source_none_missing( 'amd64', '4.6', 'dev', + 'semver', ) assert missing_bundles == [] assert invalid_bundles == [ @@ -634,6 +638,7 @@ def test_add_bundles_missing_in_source_none_missing( binary_image='binary-image:4.5', from_index='index-image:4.6', container_tool='podman', + graph_update_mode='semver', ) assert mock_gil.call_count == 4 assert mock_aolti.call_count == 2