Skip to content

Commit

Permalink
Added distribution scope for merge_index_image end point
Browse files Browse the repository at this point in the history
  • Loading branch information
midnightercz authored and yashvardhannanavati committed Oct 13, 2020
1 parent c20530f commit 4e326b6
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 42 deletions.
1 change: 1 addition & 0 deletions iib/web/api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ def merge_index_image():
payload.get('target_index'),
overwrite_target_index,
payload.get('overwrite_target_index_token'),
payload.get('distribution_scope'),
]
safe_args = _get_safe_args(args, payload)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"""Added distribution scope attribute for RequestMergeIndexImage.
Revision ID: 983a81fe5e98
Revises: 4c9db41195ec
Create Date: 2020-10-08 13:25:25.662595
"""
from alembic import op
import sqlalchemy as sa


revision = '983a81fe5e98'
down_revision = '4c9db41195ec'
branch_labels = None
depends_on = None


def upgrade():
with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op:
batch_op.add_column(sa.Column('distribution_scope', sa.String(), nullable=True))


def downgrade():
with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op:
batch_op.drop_column('distribution_scope')
29 changes: 18 additions & 11 deletions iib/web/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class BundleDeprecation(db.Model):
primary_key=True,
)
bundle_id = db.Column(
db.Integer, db.ForeignKey('image.id'), autoincrement=False, index=True, primary_key=True,
db.Integer, db.ForeignKey('image.id'), autoincrement=False, index=True, primary_key=True
)

__table_args__ = (
Expand Down Expand Up @@ -705,7 +705,7 @@ def _from_json(
} | set(additional_optional_params or [])

validate_request_params(
request_kwargs, required_params=required_params, optional_params=optional_params,
request_kwargs, required_params=required_params, optional_params=optional_params
)

# Check if both `from_index` and `add_arches` are not specified
Expand Down Expand Up @@ -830,9 +830,7 @@ class RequestAdd(Request, RequestIndexImageMixin):

omps_operator_version = db.Column(db.String, nullable=True)

__mapper_args__ = {
'polymorphic_identity': RequestTypeMapping.__members__['add'].value,
}
__mapper_args__ = {'polymorphic_identity': RequestTypeMapping.__members__['add'].value}

@classmethod
def from_json(cls, kwargs, batch=None):
Expand Down Expand Up @@ -944,9 +942,7 @@ class RequestRm(Request, RequestIndexImageMixin):
from_index_id = db.Column(db.Integer, db.ForeignKey('image.id'), nullable=False)
operators = db.relationship('Operator', secondary=RequestRmOperator.__table__)

__mapper_args__ = {
'polymorphic_identity': RequestTypeMapping.__members__['rm'].value,
}
__mapper_args__ = {'polymorphic_identity': RequestTypeMapping.__members__['rm'].value}

@classmethod
def from_json(cls, kwargs, batch=None):
Expand Down Expand Up @@ -1023,7 +1019,7 @@ class RequestRegenerateBundle(Request):
organization = db.Column(db.String, nullable=True)

__mapper_args__ = {
'polymorphic_identity': RequestTypeMapping.__members__['regenerate_bundle'].value,
'polymorphic_identity': RequestTypeMapping.__members__['regenerate_bundle'].value
}

@classmethod
Expand All @@ -1039,7 +1035,7 @@ def from_json(cls, kwargs, batch=None):
request_kwargs = deepcopy(kwargs)

validate_request_params(
request_kwargs, required_params={'from_bundle_image'}, optional_params={'organization'},
request_kwargs, required_params={'from_bundle_image'}, optional_params={'organization'}
)

# Validate organization is correctly provided
Expand Down Expand Up @@ -1130,9 +1126,10 @@ class RequestMergeIndexImage(Request):
target_index_resolved = db.relationship(
'Image', foreign_keys=[target_index_resolved_id], uselist=False
)
distribution_scope = db.Column(db.String, nullable=True)

__mapper_args__ = {
'polymorphic_identity': RequestTypeMapping.__members__['merge_index_image'].value,
'polymorphic_identity': RequestTypeMapping.__members__['merge_index_image'].value
}

@classmethod
Expand Down Expand Up @@ -1198,6 +1195,15 @@ def from_json(cls, kwargs, batch=None):

request_kwargs['binary_image'] = Image.get_or_create(pull_specification=binary_image)

distribution_scope = request_kwargs.pop('distribution_scope', None)
if distribution_scope:
distribution_scope = distribution_scope.lower()
if distribution_scope not in ['prod', 'stage', 'dev']:
raise ValidationError(
'The "distribution_scope" value must be one of "dev", "stage", or "prod"'
)
request_kwargs['distribution_scope'] = distribution_scope

# current_user.is_authenticated is only ever False when auth is disabled
if current_user.is_authenticated:
request_kwargs['user'] = current_user
Expand Down Expand Up @@ -1234,6 +1240,7 @@ def to_json(self, verbose=True):
rv['target_index_resolved'] = getattr(
self.target_index_resolved, 'pull_specification', None
)
rv['distribution_scope'] = getattr(self.distribution_scope, 'distribution_scope', None)

return rv

Expand Down
12 changes: 12 additions & 0 deletions iib/web/static/api_v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,12 @@ components:
in the format "user:password". If token is required for accessing the index images,
then both of them have to be in the same repository.
example: token
distribution_scope:
description: >
The scope of distribution for the index created by the request.
This will determine what level of protection the addition will have.
type: string
example: 'prod'
required:
- binary_image
- source_from_index
Expand Down Expand Up @@ -851,6 +857,12 @@ components:
index_image:
type: string
example: 'quay.io/iib-stage/iib:5'
distribution_scope:
description: >
The scope of distribution for the index created by the request.
This determined what level of protection the addition or removal had.
type: string
example: 'prod'

RequestRegenerateBundleOutput:
allOf:
Expand Down
24 changes: 17 additions & 7 deletions iib/workers/tasks/build_merge_index_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def _add_bundles_missing_in_source(
arch,
ocp_version,
overwrite_target_index_token=None,
distribution_scope=None,
):
"""
Rebuild index image with bundles missing from source image but present in target image.
Expand Down Expand Up @@ -101,7 +102,13 @@ def _add_bundles_missing_in_source(
overwrite_target_index_token,
)
_add_label_to_index(
'com.redhat.index.delivery.version', ocp_version, base_dir, 'index.Dockerfile',
'com.redhat.index.delivery.version', ocp_version, base_dir, 'index.Dockerfile'
)
_add_label_to_index(
'com.redhat.index.delivery.distribution_scope',
distribution_scope,
base_dir,
'index.Dockerfile',
)
_build_image(base_dir, 'index.Dockerfile', request_id, arch)
_push_image(request_id, arch)
Expand Down Expand Up @@ -134,7 +141,7 @@ def _get_bundles_from_deprecation_list(bundles, deprecation_list):


def _deprecate_bundles(
bundles, base_dir, binary_image, from_index, overwrite_target_index_token=None,
bundles, base_dir, binary_image, from_index, overwrite_target_index_token=None
):
"""
Deprecate the specified bundles from the index image.
Expand Down Expand Up @@ -162,9 +169,7 @@ def _deprecate_bundles(
','.join(bundles),
]
with set_registry_token(overwrite_target_index_token, from_index):
run_cmd(
cmd, {'cwd': base_dir}, exc_msg='Failed to deprecate the bundles',
)
run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to deprecate the bundles')


@app.task
Expand All @@ -177,6 +182,7 @@ def handle_merge_request(
target_index=None,
overwrite_target_index=False,
overwrite_target_index_token=None,
distribution_scope=None,
):
"""
Coordinate the work needed to merge old (N) index image with new (N+1) index image.
Expand All @@ -194,6 +200,8 @@ def handle_merge_request(
:param str overwrite_target_index_token: the token used for overwriting the input
``target_index`` image. This is required for non-privileged users to use
``overwrite_target_index``. The format of the token must be in the format "user:password".
:param str distribution_scope: the scope for distribution of the index image, defaults to
``None``.
:raises IIBError: if the index image merge fails.
"""
_cleanup()
Expand All @@ -203,6 +211,7 @@ def handle_merge_request(
overwrite_from_index_token=overwrite_target_index_token,
source_from_index=source_from_index,
target_index=target_index,
distribution_scope=distribution_scope,
)
_update_index_image_build_state(request_id, prebuild_info)

Expand All @@ -229,13 +238,14 @@ def handle_merge_request(
arch,
prebuild_info['target_ocp_version'],
overwrite_target_index_token,
distribution_scope=prebuild_info['distribution_scope'],
)

set_request_state(request_id, 'in_progress', 'Deprecating bundles in the deprecation list')
log.info('Deprecating bundles in the deprecation list')
intermediate_bundles = source_index_bundles + missing_bundles
deprecate_bundles = _get_bundles_from_deprecation_list(
intermediate_bundles, deprecation_list,
intermediate_bundles, deprecation_list
)
intermediate_image_name = _get_external_arch_pull_spec(
request_id, arch, include_transport=False
Expand Down Expand Up @@ -273,5 +283,5 @@ def handle_merge_request(
overwrite_target_index_token,
)
set_request_state(
request_id, 'complete', 'The index image was successfully cleaned and updated.',
request_id, 'complete', 'The index image was successfully cleaned and updated.'
)
28 changes: 10 additions & 18 deletions tests/test_web/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,11 +678,7 @@ def test_add_bundle_custom_user_queue(
mock_smfsc, mock_har, app, auth_env, client, user_to_queue, overwrite_from_index, expected_queue
):
app.config['IIB_USER_TO_QUEUE'] = user_to_queue
data = {
'bundles': ['some:thing'],
'binary_image': 'binary:image',
'add_arches': ['s390x'],
}
data = {'bundles': ['some:thing'], 'binary_image': 'binary:image', 'add_arches': ['s390x']}
if overwrite_from_index:
data['from_index'] = 'index:image'
data['overwrite_from_index'] = True
Expand Down Expand Up @@ -872,10 +868,7 @@ def test_patch_request_add_success(mock_smfsc, db, minimal_request_add, worker_a
'quay.io/some-operator2:v2.1.0',
]

bundle_mapping = {
'some-operator': bundles[0:2],
'some-operator2': bundles[2:],
}
bundle_mapping = {'some-operator': bundles[0:2], 'some-operator2': bundles[2:]}

data = {
'arches': ['arches'],
Expand Down Expand Up @@ -927,7 +920,7 @@ def test_patch_request_add_success(mock_smfsc, db, minimal_request_add, worker_a
db.session.commit()

rv = client.patch(
f'/api/v1/builds/{minimal_request_add.id}', json=data, environ_base=worker_auth_env,
f'/api/v1/builds/{minimal_request_add.id}', json=data, environ_base=worker_auth_env
)
rv_json = rv.json
assert rv.status_code == 200, rv_json
Expand Down Expand Up @@ -987,7 +980,7 @@ def test_patch_request_rm_success(mock_smfsc, db, minimal_request_rm, worker_aut
db.session.commit()

rv = client.patch(
f'/api/v1/builds/{minimal_request_rm.id}', json=data, environ_base=worker_auth_env,
f'/api/v1/builds/{minimal_request_rm.id}', json=data, environ_base=worker_auth_env
)
rv_json = rv.json
assert rv.status_code == 200, rv_json
Expand Down Expand Up @@ -1092,7 +1085,7 @@ def test_remove_operator_success(mock_smfsc, mock_rm, db, auth_env, client):
'state': 'in_progress',
'state_reason': 'The request was initiated',
'updated': '2020-02-12T17:03:00Z',
},
}
],
'state_reason': 'The request was initiated',
'updated': '2020-02-12T17:03:00Z',
Expand Down Expand Up @@ -1212,9 +1205,7 @@ def test_not_found(client):
@mock.patch('iib.web.api_v1.handle_regenerate_bundle_request')
@mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
def test_regenerate_bundle_success(mock_smfsc, mock_hrbr, db, auth_env, client):
data = {
'from_bundle_image': 'registry.example.com/bundle-image:latest',
}
data = {'from_bundle_image': 'registry.example.com/bundle-image:latest'}

# Assume a timestamp to simplify tests
_timestamp = '2020-02-12T17:03:00Z'
Expand Down Expand Up @@ -1588,6 +1579,7 @@ def test_merge_index_image_success(mock_smfsc, mock_merge, db, auth_env, client)
'binary_image': 'binary:image',
'binary_image_resolved': None,
'deprecation_list': ['some@sha256:bundle'],
'distribution_scope': None,
'id': 1,
'index_image': None,
'logs': {
Expand Down Expand Up @@ -1642,8 +1634,8 @@ def test_merge_index_image_overwrite_token_redacted(
assert rv.status_code == 201
mock_merge.apply_async.assert_called_once()
# Second to last element in args is the overwrite_from_index parameter
assert mock_merge.apply_async.call_args[1]['args'][-2] is True
assert mock_merge.apply_async.call_args[1]['args'][-1] == token
assert mock_merge.apply_async.call_args[1]['args'][5] is True
assert mock_merge.apply_async.call_args[1]['args'][6] == token
assert 'overwrite_target_index_token' not in rv_json
assert token not in json.dumps(rv_json)
assert token not in mock_merge.apply_async.call_args[1]['argsrepr']
Expand Down Expand Up @@ -1707,7 +1699,7 @@ def test_merge_index_image_custom_user_queue(
@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_fail_on_missing_overwrite_params(
mock_smfsc, mock_merge, app, auth_env, client, overwrite_from_index,
mock_smfsc, mock_merge, app, auth_env, client, overwrite_from_index
):
data = {
'deprecation_list': ['some@sha256:bundle'],
Expand Down
Loading

0 comments on commit 4e326b6

Please sign in to comment.