Add IIB_BINARY_IMAGE_CONFIG to IIB and make binary_image param optional

This commit introduces a new config variable called IIB_BINARY_IMAGE_CONFIG
which when specified, doesn't require the user to specify binary_image param
while submitting requests to IIB.

Refers to CLOUDDST-2761

README.md

@@ -140,6 +140,10 @@ The custom configuration options for the REST API are listed below:
 
 * `IIB_ADDITIONAL_LOGGERS` - a list of Python loggers that should have the same log level that is
   set for `IIB_LOG_LEVEL`. This defaults to `[]`.
+* `IIB_BINARY_IMAGE_CONFIG` - the mapping, `dict(<str>: dict(<str>:<str>))`, of distribution scope
+  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_FORCE_OVERWRITE_FROM_INDEX` - a boolean that determines if privileged users should be forced
   to have `overwrite_from_index` set to `True`. This defaults to `False`.
 * `IIB_GREENWAVE_CONFIG` - the mapping, `dict(<str>: dict(<str>:<str>))`, of celery task queues to

iib/web/api_v1.py

@@ -50,13 +50,14 @@ def _get_rm_args(payload, request, overwrite_from_index):
     """
     return [
         payload['operators'],
-        payload['binary_image'],
         request.id,
         payload['from_index'],
+        payload.get('binary_image'),
         payload.get('add_arches'),
         overwrite_from_index,
         payload.get('overwrite_from_index_token'),
         request.distribution_scope,
+        flask.current_app.config['IIB_BINARY_IMAGE_CONFIG'],
     ]
 
 
@@ -71,8 +72,8 @@ def _get_add_args(payload, request, overwrite_from_index, celery_queue):
     """
     return [
         payload.get('bundles', []),
-        payload['binary_image'],
         request.id,
+        payload.get('binary_image'),
         payload.get('from_index'),
         payload.get('add_arches'),
         payload.get('cnr_token'),
@@ -82,6 +83,7 @@ def _get_add_args(payload, request, overwrite_from_index, celery_queue):
         payload.get('overwrite_from_index_token'),
         request.distribution_scope,
         flask.current_app.config['IIB_GREENWAVE_CONFIG'].get(celery_queue),
+        flask.current_app.config['IIB_BINARY_IMAGE_CONFIG'],
     ]
 
 
@@ -371,6 +373,7 @@ def patch_request(request_id):
             )
 
     image_keys = (
+        'binary_image',
         'binary_image_resolved',
         'bundle_image',
         'from_bundle_image_resolved',
@@ -654,14 +657,15 @@ def merge_index_image():
     overwrite_target_index = payload.get('overwrite_target_index', False)
     celery_queue = _get_user_queue(serial=overwrite_target_index)
     args = [
-        payload['binary_image'],
         payload['source_from_index'],
         payload.get('deprecation_list', []),
         request.id,
+        payload.get('binary_image'),
         payload.get('target_index'),
         overwrite_target_index,
         payload.get('overwrite_target_index_token'),
         payload.get('distribution_scope'),
+        flask.current_app.config['IIB_BINARY_IMAGE_CONFIG'],
     ]
     safe_args = _get_safe_args(args, payload)
 

iib/web/app.py

@@ -81,6 +81,31 @@ def validate_api_config(config):
                     f'{queue_name} in "IIB_GREENWAVE_CONFIG"'
                 )
 
+    if config['IIB_BINARY_IMAGE_CONFIG']:
+        if not isinstance(config['IIB_BINARY_IMAGE_CONFIG'], dict):
+            raise ConfigError(
+                'IIB_BINARY_IMAGE_CONFIG must be a dict mapping distribution_scope to '
+                'another dict mapping ocp_version to binary_image'
+            )
+        for distribution_scope, value_dict in config['IIB_BINARY_IMAGE_CONFIG'].items():
+            if not isinstance(distribution_scope, str) or distribution_scope not in (
+                'dev',
+                'stage',
+                'prod',
+            ):
+                raise ConfigError(
+                    'distribution_scope values must be one of the following'
+                    ' "prod", "stage" or "dev" strings.'
+                )
+            if not isinstance(value_dict, dict):
+                raise ConfigError(
+                    'Value for distribution_scope keys must be a dict mapping'
+                    ' ocp_version to binary_image'
+                )
+            for ocp_version, binary_image_value in value_dict.items():
+                if not isinstance(ocp_version, str) or not isinstance(binary_image_value, str):
+                    raise ConfigError('All ocp_version and binary_image values must be strings.')
+
 
 # See app factory pattern:
 #   http://flask.pocoo.org/docs/0.12/patterns/appfactories/

iib/web/config.py

@@ -10,6 +10,7 @@ class Config(object):
 
     # Additional loggers to set to the level defined in IIB_LOG_LEVEL
     IIB_ADDITIONAL_LOGGERS = []
+    IIB_BINARY_IMAGE_CONFIG = {}
     IIB_FORCE_OVERWRITE_FROM_INDEX = False
     IIB_GREENWAVE_CONFIG = {}
     IIB_LOG_FORMAT = '%(asctime)s %(name)s %(levelname)s %(module)s.%(funcName)s %(message)s'

iib/web/migrations/versions/60f89c046096_make_binary_image_optional.py

@@ -0,0 +1,38 @@
+"""Make binary_image optional.
+
+Revision ID: 60f89c046096
+Revises: 983a81fe5e98
+Create Date: 2020-10-12 15:49:24.523019
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '60f89c046096'
+down_revision = '983a81fe5e98'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('request_add', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=True)
+
+    with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=True)
+
+    with op.batch_alter_table('request_rm', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=True)
+
+
+def downgrade():
+    with op.batch_alter_table('request_rm', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=False)
+
+    with op.batch_alter_table('request_merge_index_image', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=False)
+
+    with op.batch_alter_table('request_add', schema=None) as batch_op:
+        batch_op.alter_column('binary_image_id', existing_type=sa.INTEGER(), nullable=False)

iib/web/models.py

@@ -629,7 +629,7 @@ class RequestIndexImageMixin:
     @declared_attr
     def binary_image_id(cls):
         """Return the ID of the image that the opm binary comes from."""
-        return db.Column(db.Integer, db.ForeignKey('image.id'), nullable=False)
+        return db.Column(db.Integer, db.ForeignKey('image.id'))
 
     @declared_attr
     def binary_image_resolved_id(cls):
@@ -696,9 +696,10 @@ def _from_json(
             be created automatically.
         """
         # Validate all required parameters are present
-        required_params = {'binary_image'} | set(additional_required_params or [])
+        required_params = set(additional_required_params or [])
         optional_params = {
             'add_arches',
+            'binary_image',
             'overwrite_from_index',
             'overwrite_from_index_token',
             'distribution_scope',
@@ -752,11 +753,14 @@ def _from_json(
         Architecture.validate_architecture_json(add_arches)
 
         # Validate binary_image is correctly provided
-        binary_image = request_kwargs.pop('binary_image')
-        if not isinstance(binary_image, str):
+        binary_image = request_kwargs.pop('binary_image', None)
+        if binary_image is not None and not isinstance(binary_image, str):
             raise ValidationError('"binary_image" must be a string')
+        elif not binary_image and not current_app.config['IIB_BINARY_IMAGE_CONFIG']:
+            raise ValidationError('The "binary_image" value must be a non-empty string')
 
-        request_kwargs['binary_image'] = Image.get_or_create(pull_specification=binary_image)
+        if binary_image:
+            request_kwargs['binary_image'] = Image.get_or_create(pull_specification=binary_image)
 
         if 'from_index' in request_kwargs:
             if not isinstance(request_kwargs['from_index'], str):
@@ -790,7 +794,7 @@ def get_common_index_image_json(self):
         :rtype: dict
         """
         return {
-            'binary_image': self.binary_image.pull_specification,
+            'binary_image': getattr(self.binary_image, 'pull_specification', None),
             'binary_image_resolved': getattr(
                 self.binary_image_resolved, 'pull_specification', None
             ),
@@ -812,6 +816,7 @@ def get_index_image_mutable_keys(self):
         :rtype: set
         """
         return {
+            'binary_image',
             'binary_image_resolved',
             'from_bundle_image_resolved',
             'from_index_resolved',
@@ -850,16 +855,10 @@ def from_json(cls, kwargs, batch=None):
                 '"bundles" should be either an empty array or an array of non-empty strings'
             )
 
-        # Check if no bundles `from_index and `binary_image` are specified
-        # if no bundles and and no from index then a empty index will be created
-        # if no binary image and just a from_index then we are not updating anything and it would
-        # be a no-op
-        if not request_kwargs.get('bundles') and (
-            not request_kwargs.get('from_index') or not request_kwargs.get('binary_image')
-        ):
-            raise ValidationError(
-                '"from_index" and "binary_image" must be specified if no bundles are specified'
-            )
+        # Check if no bundles and `from_index is specified
+        # if no bundles and no from index then an empty index will be created which is a no-op
+        if not (request_kwargs.get('bundles') or request_kwargs.get('from_index')):
+            raise ValidationError('"from_index" must be specified if no bundles are specified')
 
         for param in ('cnr_token', 'organization'):
             if param not in request_kwargs:
@@ -1101,7 +1100,7 @@ class RequestMergeIndexImage(Request):
     __tablename__ = 'request_merge_index_image'
 
     id = db.Column(db.Integer, db.ForeignKey('request.id'), autoincrement=False, primary_key=True)
-    binary_image_id = db.Column(db.Integer, db.ForeignKey('image.id'), nullable=False)
+    binary_image_id = db.Column(db.Integer, db.ForeignKey('image.id'))
     binary_image_resolved_id = db.Column(db.Integer, db.ForeignKey('image.id'))
     binary_image = db.relationship('Image', foreign_keys=[binary_image_id], uselist=False)
     binary_image_resolved = db.relationship(
@@ -1190,10 +1189,13 @@ def from_json(cls, kwargs, batch=None):
 
         # Validate binary_image is correctly provided
         binary_image = request_kwargs.pop('binary_image', None)
-        if not isinstance(binary_image, str):
+        if binary_image is not None and not isinstance(binary_image, str):
             raise ValidationError('The "binary_image" value must be a string')
+        elif not binary_image and not current_app.config['IIB_BINARY_IMAGE_CONFIG']:
+            raise ValidationError('The "binary_image" value must be a non-empty string')
 
-        request_kwargs['binary_image'] = Image.get_or_create(pull_specification=binary_image)
+        if binary_image:
+            request_kwargs['binary_image'] = Image.get_or_create(pull_specification=binary_image)
 
         distribution_scope = request_kwargs.pop('distribution_scope', None)
         if distribution_scope:
@@ -1226,7 +1228,7 @@ def to_json(self, verbose=True):
         :rtype: dict
         """
         rv = super().to_json(verbose=verbose)
-        rv['binary_image'] = self.binary_image.pull_specification
+        rv['binary_image'] = getattr(self.binary_image, 'pull_specification', None)
         rv['binary_image_resolved'] = getattr(
             self.binary_image_resolved, 'pull_specification', None
         )
@@ -1254,6 +1256,7 @@ def get_mutable_keys(self):
         rv = super().get_mutable_keys()
         rv.update(
             {
+                'binary_image',
                 'binary_image_resolved',
                 'index_image',
                 'source_from_index_resolved',

iib/web/static/api_v1.yaml

@@ -700,7 +700,6 @@ components:
             type: string
             example: 'prod'
       required:
-        - binary_image
         - bundles
     RequestRemove:
       type: object
@@ -744,7 +743,6 @@ components:
           type: string
           example: 'prod'
       required:
-        - binary_image
         - from_index
         - operators
     RequestRegenerateBundle:
@@ -814,7 +812,6 @@ components:
           type: string
           example: 'prod'
       required:
-        - binary_image
         - source_from_index
     RequestMergeIndexImageOutput:
       allOf: