diff --git a/README.md b/README.md index 0f47d444..5210781e 100644 --- a/README.md +++ b/README.md @@ -199,6 +199,13 @@ The custom configuration options for the REST API are listed below: to another dictionary mapping ocp_version label to a binary image pull specification. This is useful in setting up customized binary image for different index image images thus reducing complexity for the end user. This defaults to `{}`. +* `IIB_GRAPH_MODE_INDEX_ALLOW_LIST` - the list of index image pull specs on which using the + `graph_update_mode` parameter while submitting an IIB request is permitted. This defaults to `[]` + . Please check out the [API Documentation](http://release-engineering.github.io/iib) for more + information on how to use "graph_update_mode" for Add requests. +* `IIB_GRAPH_MODE_OPTIONS` - the list of valid options for the `graph_update_mode` parameter in + the Add API endpoint. It defaults to `['replaces', 'semver', 'semver-skippatch']` i.e. the list + of modes supported by OPM. * `IIB_GREENWAVE_CONFIG` - the mapping, `dict(: dict(:))`, of celery task queues to another dictionary of [Greenwave](https://docs.pagure.org/greenwave/) query parameters to their values. This is useful in setting up customized gating for each queue. This defaults to `{}`. Use diff --git a/iib/web/api_v1.py b/iib/web/api_v1.py index aa427ce3..e589f5c9 100644 --- a/iib/web/api_v1.py +++ b/iib/web/api_v1.py @@ -127,6 +127,7 @@ def _get_add_args( flask.current_app.config['IIB_BINARY_IMAGE_CONFIG'], payload.get('deprecation_list', []), payload.get('build_tags', []), + payload.get('graph_update_mode'), ] diff --git a/iib/web/config.py b/iib/web/config.py index 0029578d..5d940989 100644 --- a/iib/web/config.py +++ b/iib/web/config.py @@ -20,6 +20,8 @@ class Config(object): IIB_ADDITIONAL_LOGGERS: List[str] = [] IIB_AWS_S3_BUCKET_NAME: Optional[str] = None IIB_BINARY_IMAGE_CONFIG: Dict[str, Dict[str, str]] = {} + IIB_GRAPH_MODE_INDEX_ALLOW_LIST: List[str] = [] + IIB_GRAPH_MODE_OPTIONS: List[str] = ['replaces', 'semver', 'semver-skippatch'] IIB_GREENWAVE_CONFIG: Dict[str, str] = {} IIB_LOG_FORMAT: str = '%(asctime)s %(name)s %(levelname)s %(module)s.%(funcName)s %(message)s' # This sets the level of the "flask.app" logger, which is accessed from current_app.logger diff --git a/iib/web/iib_static_types.py b/iib/web/iib_static_types.py index 2f96cc24..f3b61cb2 100644 --- a/iib/web/iib_static_types.py +++ b/iib/web/iib_static_types.py @@ -65,6 +65,7 @@ class RelatedBundlesMetadata(TypedDict): 'force_backport', 'from_bundle_image', 'from_index', + 'graph_update_mode', 'labels', 'operators', 'organization', @@ -92,6 +93,7 @@ class AddRequestPayload(TypedDict): distribution_scope: NotRequired[str] force_backport: NotRequired[bool] from_index: NotRequired[str] + graph_update_mode: NotRequired[str] organization: NotRequired[str] overwrite_from_index: NotRequired[bool] overwrite_from_index_token: NotRequired[str] @@ -344,6 +346,7 @@ class AddRmRequestResponseBase(CommonIndexImageResponseBase, APIPartImageBuildRe class AddRequestResponse(AddRmRequestResponseBase): """Datastructure of the response to request from /builds/add API point.""" + graph_update_mode: str omps_operator_version: Dict[str, Any] diff --git a/iib/web/migrations/versions/daf67ddcf4a1_add_support_for_graph_update_mode_in_.py b/iib/web/migrations/versions/daf67ddcf4a1_add_support_for_graph_update_mode_in_.py new file mode 100644 index 00000000..5f15c573 --- /dev/null +++ b/iib/web/migrations/versions/daf67ddcf4a1_add_support_for_graph_update_mode_in_.py @@ -0,0 +1,25 @@ +"""Add support for graph_update_mode in Add endpoint. + +Revision ID: daf67ddcf4a1 +Revises: 8d50f82f0be9 +Create Date: 2023-07-27 15:37:59.568914 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = 'daf67ddcf4a1' +down_revision = '8d50f82f0be9' +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('graph_update_mode', sa.String(), nullable=True)) + + +def downgrade(): + with op.batch_alter_table('request_add', schema=None) as batch_op: + batch_op.drop_column('graph_update_mode') diff --git a/iib/web/models.py b/iib/web/models.py index 530efa24..1486d988 100644 --- a/iib/web/models.py +++ b/iib/web/models.py @@ -1038,6 +1038,7 @@ class RequestAdd(Request, RequestIndexImageMixin): deprecation_list: Mapped[List['Image']] = db.relationship( 'Image', secondary=RequestAddBundleDeprecation.__table__ ) + graph_update_mode: Mapped[Optional[str]] organization: Mapped[Optional[str]] omps_operator_version: Mapped[Optional[str]] @@ -1072,8 +1073,9 @@ 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') - ALLOWED_KEYS_1: Sequence[Literal['cnr_token', 'organization']] = ( + ALLOWED_KEYS_1: Sequence[Literal['cnr_token', 'graph_update_mode', 'organization']] = ( 'cnr_token', + 'graph_update_mode', 'organization', ) for param in ALLOWED_KEYS_1: @@ -1083,6 +1085,21 @@ def from_json( # type: ignore[override] # noqa: F821 if not isinstance(request_kwargs[param], str): 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}' + ) + if not isinstance(request_kwargs.get('force_backport', False), bool): raise ValidationError('"force_backport" must be a boolean') @@ -1099,6 +1116,7 @@ def from_json( # type: ignore[override] # noqa: F821 'bundles', 'distribution_scope', 'deprecation_list', + 'graph_update_mode', 'build_tags', ], batch=batch, @@ -1137,6 +1155,7 @@ def to_json(self, verbose: Optional[bool] = True) -> AddRequestResponse: rv['omps_operator_version'] = {} if self.omps_operator_version: rv['omps_operator_version'] = json.loads(self.omps_operator_version) + rv['graph_update_mode'] = self.graph_update_mode for bundle in self.bundles: if bundle.operator: diff --git a/iib/web/static/api_v1.yaml b/iib/web/static/api_v1.yaml index a5d1e591..d7fcfa54 100644 --- a/iib/web/static/api_v1.yaml +++ b/iib/web/static/api_v1.yaml @@ -1012,6 +1012,14 @@ components: description: > from_index is required when add_arches is not provided. example: 'quay.io/iib-stage/iib:4' + 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 organization: type: string description: > @@ -1055,6 +1063,9 @@ components: example: add omps_operator_version: $ref: '#/components/schemas/OMPSOperatorVersion' + graph_update_mode: + type: string + example: semver RmRequest: type: object properties: diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index 2fa6f11c..2df25f22 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -453,6 +453,7 @@ def _opm_index_add( bundles: List[str], binary_image: str, from_index: Optional[str] = None, + graph_update_mode: Optional[str] = None, overwrite_from_index_token: Optional[str] = None, overwrite_csv: bool = False, container_tool: Optional[str] = None, @@ -469,6 +470,8 @@ def _opm_index_add( gets copied from. This should point to a digest or stable tag. :param str from_index: the pull specification of the container image containing the index that the index image build will be based from. + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. :param str overwrite_from_index_token: the token used for overwriting the input ``from_index`` image. This is required to use ``overwrite_from_index``. The format of the token must be in the format "user:password". @@ -497,6 +500,10 @@ def _opm_index_add( cmd.append('--container-tool') cmd.append(container_tool) + if graph_update_mode: + log.info('Using %s mode to update the channel graph in the index', graph_update_mode) + cmd.extend(['--mode', graph_update_mode]) + log.info('Generating the database file with the following bundle(s): %s', ', '.join(bundles)) if from_index: log.info('Using the existing database from %s', from_index) @@ -789,6 +796,7 @@ def handle_add_request( binary_image_config: Optional[Dict[str, Dict[str, str]]] = None, deprecation_list: Optional[List[str]] = None, build_tags: Optional[List[str]] = None, + graph_update_mode: Optional[str] = None, traceparent: Optional[str] = None, ) -> None: """ @@ -824,6 +832,8 @@ def handle_add_request( :param list deprecation_list: list of deprecated bundles for the target index image. Defaults to ``None``. :param list build_tags: List of tags which will be applied to intermediate index images. + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. :param str traceparent: the traceparent header value to be used for tracing the request. :raises IIBError: if the index image build fails. """ @@ -898,6 +908,7 @@ def handle_add_request( bundles=resolved_bundles, binary_image=prebuild_info['binary_image_resolved'], from_index=from_index_resolved, + graph_update_mode=graph_update_mode, overwrite_from_index_token=overwrite_from_index_token, overwrite_csv=(prebuild_info['distribution_scope'] in ['dev', 'stage']), ) @@ -907,6 +918,7 @@ def handle_add_request( bundles=resolved_bundles, binary_image=prebuild_info['binary_image_resolved'], from_index=from_index_resolved, + graph_update_mode=graph_update_mode, overwrite_from_index_token=overwrite_from_index_token, overwrite_csv=(prebuild_info['distribution_scope'] in ['dev', 'stage']), ) diff --git a/iib/workers/tasks/opm_operations.py b/iib/workers/tasks/opm_operations.py index 295c0084..7625f261 100644 --- a/iib/workers/tasks/opm_operations.py +++ b/iib/workers/tasks/opm_operations.py @@ -524,6 +524,7 @@ def _opm_registry_add( bundles: List[str], overwrite_csv: bool = False, container_tool: Optional[str] = None, + graph_update_mode: Optional[str] = None, ) -> None: """ Add the input bundles to an operator index database. @@ -537,6 +538,8 @@ def _opm_registry_add( :param bool overwrite_csv: a boolean determining if a bundle will be replaced if the CSV already exists. :param str container_tool: the container tool to be used to operate on the index image + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. """ from iib.workers.tasks.utils import run_cmd @@ -561,6 +564,10 @@ def _opm_registry_add( cmd.append('--container-tool') cmd.append(container_tool) + if graph_update_mode: + log.info('Using %s mode to update the channel graph in the index', graph_update_mode) + cmd.extend(['--mode', graph_update_mode]) + log.info('Generating the database file with the following bundle(s): %s', ', '.join(bundles)) if overwrite_csv: @@ -581,6 +588,7 @@ def opm_registry_add_fbc( bundles: List[str], binary_image: str, from_index: Optional[str] = None, + graph_update_mode: Optional[str] = None, overwrite_csv: bool = False, overwrite_from_index_token: Optional[str] = None, container_tool: Optional[str] = None, @@ -597,6 +605,8 @@ def opm_registry_add_fbc( gets copied from. This should point to a digest or stable tag. :param str from_index: the pull specification of the container image containing the index that the index image build will be based from. + :param str graph_update_mode: Graph update mode that defines how channel graphs are updated + in the index. :param bool overwrite_csv: a boolean determining if a bundle will be replaced if the CSV already exists. :param str overwrite_from_index_token: the token used for overwriting the input @@ -617,6 +627,7 @@ def opm_registry_add_fbc( bundles=bundles, overwrite_csv=overwrite_csv, container_tool=container_tool, + graph_update_mode=graph_update_mode, ) fbc_dir, _ = opm_migrate(index_db=index_db_file, base_dir=base_dir) diff --git a/tests/test_web/test_api_v1.py b/tests/test_web/test_api_v1.py index 274c552d..5a556c92 100644 --- a/tests/test_web/test_api_v1.py +++ b/tests/test_web/test_api_v1.py @@ -66,6 +66,7 @@ def test_get_build(app, auth_env, client, db): 'distribution_scope': None, 'from_index': 'quay.io/namespace/repo:latest', 'from_index_resolved': 'quay.io/namespace/from_index@sha256:defghi', + 'graph_update_mode': None, 'id': 1, 'index_image': 'quay.io/namespace/index@sha256:fghijk', 'index_image_resolved': None, @@ -496,11 +497,22 @@ def test_get_build_logs_s3_configured( {'bundles': ['some:thing'], 'binary_image': 'binary:image', 'force_backport': 'spam'}, '"force_backport" must be a boolean', ), + ( + {'bundles': ['some:thing'], 'binary_image': 'binary:image', 'graph_update_mode': 123}, + '"graph_update_mode" must be a string', + ), + ( + {'bundles': ['some:thing'], 'binary_image': 'binary:image', 'graph_update_mode': 'Hi'}, + ( + '"graph_update_mode" must be set to one of these: [\'replaces\', \'semver\'' + ', \'semver-skippatch\']' + ), + ), ), ) @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change') def test_add_bundles_invalid_params_format(mock_smfsc, data, error_msg, db, auth_env, client): - rv = client.post(f'/api/v1/builds/add', json=data, environ_base=auth_env) + rv = client.post('/api/v1/builds/add', json=data, environ_base=auth_env) assert rv.status_code == 400 assert error_msg == rv.json['error'] mock_smfsc.assert_not_called() @@ -521,6 +533,30 @@ def test_add_bundles_overwrite_not_allowed(mock_smfsc, client, db): mock_smfsc.assert_not_called() +@pytest.mark.parametrize('from_index', (None, 'some-common-index')) +@mock.patch('iib.web.api_v1.messaging.send_message_for_state_change') +def test_add_bundles_graph_update_mode_not_allowed( + mock_smfsc, app, client, auth_env, db, from_index +): + app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] = ['some-unique-index'] + data = { + 'binary_image': 'binary:image', + 'bundles': ['some:thing'], + 'from_index': from_index, + 'graph_update_mode': 'semver', + 'overwrite_from_index': True, + 'overwrite_from_index_token': "somettoken", + } + rv = client.post('/api/v1/builds/add', json=data, environ_base=auth_env) + assert rv.status_code == 403 + error_msg = ( + '"graph_update_mode" can only be used on the' + ' following "from_index" pullspecs: [\'some-unique-index\']' + ) + assert error_msg == rv.json['error'] + mock_smfsc.assert_not_called() + + @mock.patch('iib.web.api_v1.db.session') @mock.patch('iib.web.api_v1.flask.jsonify') @mock.patch('iib.web.api_v1.RequestAdd') @@ -732,12 +768,13 @@ def test_add_bundle_from_index_and_add_arches_missing(mock_smfsc, db, auth_env, 'bundles', 'from_index', 'distribution_scope', + 'graph_update_mode', ), ( - (False, None, ['some:thing'], None, None), - (False, None, [], 'some:thing', 'Prod'), - (True, 'username:password', ['some:thing'], 'some:thing', 'StagE'), - (True, 'username:password', [], 'some:thing', 'DeV'), + (False, None, ['some:thing'], None, None, 'semver'), + (False, None, [], 'some:thing', 'Prod', 'semver-skippatch'), + (True, 'username:password', ['some:thing'], 'some:thing', 'StagE', 'replaces'), + (True, 'username:password', [], 'some:thing', 'DeV', 'semver'), ), ) @mock.patch('iib.web.api_v1.handle_add_request') @@ -745,6 +782,7 @@ def test_add_bundle_from_index_and_add_arches_missing(mock_smfsc, db, auth_env, def test_add_bundle_success( mock_smfsc, mock_har, + app, overwrite_from_index, overwrite_from_index_token, db, @@ -753,7 +791,9 @@ def test_add_bundle_success( bundles, from_index, distribution_scope, + graph_update_mode, ): + app.config['IIB_GRAPH_MODE_INDEX_ALLOW_LIST'] = [from_index] data = { 'binary_image': 'binary:image', 'add_arches': ['s390x'], @@ -762,6 +802,7 @@ def test_add_bundle_success( 'overwrite_from_index': overwrite_from_index, 'overwrite_from_index_token': overwrite_from_index_token, 'from_index': from_index, + 'graph_update_mode': graph_update_mode, } expected_distribution_scope = None @@ -786,6 +827,7 @@ def test_add_bundle_success( 'distribution_scope': expected_distribution_scope, 'from_index': from_index, 'from_index_resolved': None, + 'graph_update_mode': graph_update_mode, 'id': 1, 'index_image': None, 'index_image_resolved': None, @@ -860,9 +902,9 @@ def test_add_bundle_overwrite_token_redacted(mock_smfsc, mock_har, app, auth_env 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'][-7] is True + 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'][-6] == token + assert mock_har.apply_async.call_args[1]['args'][-7] == 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'] @@ -1134,6 +1176,7 @@ def test_patch_request_add_success( 'distribution_scope': distribution_scope, 'from_index': None, 'from_index_resolved': None, + 'graph_update_mode': None, 'id': minimal_request_add.id, 'index_image': 'index:image', 'index_image_resolved': 'index:image-resolved', @@ -1753,12 +1796,13 @@ def test_add_rm_batch_success(mock_smfnbor, mock_hrr, mock_har, app, auth_env, c {}, [], [], + None, ], 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, {}, [], []]" + "'hello-operator', None, True, '*****', None, None, {}, [], [], None]" ), link_error=mock.ANY, queue=None, diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index 6e0e86d9..97679f0c 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -334,14 +334,18 @@ def test_get_local_pull_spec(request_id, arch): @pytest.mark.parametrize('bundles', (['bundle:1.2', 'bundle:1.3'], [])) @pytest.mark.parametrize('overwrite_csv', (True, False)) @pytest.mark.parametrize('container_tool', (None, 'podwoman')) +@pytest.mark.parametrize('graph_update_mode', (None, 'semver')) @mock.patch('iib.workers.tasks.build.set_registry_token') @mock.patch('iib.workers.tasks.build.run_cmd') -def test_opm_index_add(mock_run_cmd, mock_srt, from_index, bundles, overwrite_csv, container_tool): +def test_opm_index_add( + mock_run_cmd, mock_srt, from_index, bundles, overwrite_csv, container_tool, graph_update_mode +): build._opm_index_add( '/tmp/somedir', bundles, 'binary-image:latest', from_index, + graph_update_mode, 'user:pass', overwrite_csv, container_tool=container_tool, @@ -368,6 +372,11 @@ def test_opm_index_add(mock_run_cmd, mock_srt, from_index, bundles, overwrite_cs assert container_tool in opm_args else: assert '--container-tool' not in opm_args + if graph_update_mode: + assert '--mode' in opm_args + assert graph_update_mode in opm_args + else: + assert '--mode' not in opm_args assert "--enable-alpha" in opm_args mock_srt.assert_called_once_with('user:pass', from_index, append=True) diff --git a/tests/test_workers/test_tasks/test_opm_operations.py b/tests/test_workers/test_tasks/test_opm_operations.py index db52abe4..58e0a3c6 100644 --- a/tests/test_workers/test_tasks/test_opm_operations.py +++ b/tests/test_workers/test_tasks/test_opm_operations.py @@ -286,6 +286,7 @@ def test_opm_registry_add( @pytest.mark.parametrize('bundles', (['bundle:1.2', 'bundle:1.3'], [])) @pytest.mark.parametrize('overwrite_csv', (True, False)) @pytest.mark.parametrize('container_tool', (None, 'podwoman')) +@pytest.mark.parametrize('graph_update_mode', (None, 'semver-skippatch')) @mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations._opm_registry_add') @@ -303,6 +304,7 @@ def test_opm_registry_add_fbc( bundles, overwrite_csv, container_tool, + graph_update_mode, is_fbc, tmpdir, ): @@ -319,6 +321,7 @@ def test_opm_registry_add_fbc( bundles=bundles, binary_image="some:image", from_index=from_index, + graph_update_mode=graph_update_mode, overwrite_csv=overwrite_csv, container_tool=container_tool, ) @@ -329,6 +332,7 @@ def test_opm_registry_add_fbc( bundles=bundles, overwrite_csv=overwrite_csv, container_tool=container_tool, + graph_update_mode=graph_update_mode, ) mock_om.assert_called_once_with(index_db=index_db_file, base_dir=tmpdir)