Skip to content

Commit

Permalink
Adding graph update mode for merge index image API endpoint
Browse files Browse the repository at this point in the history
[CLOUDDST-20163]
  • Loading branch information
lipoja committed Oct 19, 2023
1 parent f28f96c commit 7227ea8
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 15 deletions.
2 changes: 2 additions & 0 deletions iib/web/iib_static_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
32 changes: 32 additions & 0 deletions iib/web/migrations/versions/9e9d4f9730c8_merge_graph_update.py
Original file line number Diff line number Diff line change
@@ -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 ###
43 changes: 30 additions & 13 deletions iib/web/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions iib/web/static/api_v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions iib/workers/tasks/build_merge_index_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]:
Expand All @@ -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``.
Expand Down Expand Up @@ -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:
Expand All @@ -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',
Expand Down Expand Up @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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'],
Expand Down
11 changes: 9 additions & 2 deletions tests/test_web/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand All @@ -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': {
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions tests/test_workers/test_tasks/test_build_merge_index_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ def test_add_bundles_missing_in_source(
'amd64',
'4.6',
'dev',
'replaces',
)
assert missing_bundles == [
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -545,6 +547,7 @@ def test_add_bundles_missing_in_source_error_tag_specified(
'amd64',
'4.6',
'dev',
'semver',
)


Expand Down Expand Up @@ -611,6 +614,7 @@ def test_add_bundles_missing_in_source_none_missing(
'amd64',
'4.6',
'dev',
'semver',
)
assert missing_bundles == []
assert invalid_bundles == [
Expand All @@ -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
Expand Down

0 comments on commit 7227ea8

Please sign in to comment.