Merge 6188e0e into 9f0a89d

omps/api/v1/push.py

@@ -3,16 +3,14 @@
 # see the LICENSE file for license
 #
 
-from distutils import dir_util
 from functools import partial
 import logging
 import os
 from tempfile import NamedTemporaryFile, TemporaryDirectory
 import zipfile
 
 from flask import jsonify, current_app, request
-from operatorcourier.api import flatten
-from operatorcourier.errors import OpCourierError
+from yaml import safe_load
 
 from . import API
 from omps.api.common import extract_auth_token, replace_registries
@@ -24,12 +22,11 @@
     OMPSInvalidVersionFormat,
     OMPSUploadedFileError,
     OMPSExpectedFileError,
+    PackageValidationError,
     QuayPackageNotFound,
-    raise_for_courier_exception,
 )
 from omps.greenwave import GREENWAVE
 from omps.koji_util import KOJI
-from omps.manifests_util import ManifestBundle
 from omps.quay import ReleaseVersion, ORG_MANAGER
 
 logger = logging.getLogger(__name__)
@@ -170,20 +167,22 @@ def get_package_version(quay_org, repo, version=None):
 
 
 def _get_reponame_from_manifests(source_dir):
-    bundle = ManifestBundle.from_dir(source_dir)
-    return bundle.package_name
-
-
-def _flatten_manifest_structure(source_dir, dest_dir):
-    try:
-        flatten(source_dir, dest_dir)
-    except OpCourierError as e:
-        raise_for_courier_exception(e)
-
-    if not os.listdir(dest_dir):
-        # if dest dir is empty, it means that flatten did noop and source dir
-        # has already flat structure
-        dir_util.copy_tree(source_dir, dest_dir)
+    for filename in sorted(os.listdir(source_dir)):
+        filename = os.path.join(source_dir, filename)
+        if filename.endswith('.yaml') or filename.endswith('.yml'):
+            try:
+                with open(filename, 'r') as f:
+                    contents = safe_load(f.read())
+                    if 'packageName' in contents:
+                        name = contents['packageName']
+                        logger.info("Found packageName %s in %s", name, filename)
+                        return name
+            except Exception:
+                message = "Failed to parse yaml file %s" % filename[len(source_dir):]
+                logger.exception(message)
+                raise PackageValidationError(message)
+
+    raise PackageValidationError("Could not find packageName in manifests.")
 
 
 def _dir_files(dir_path):
@@ -222,19 +221,15 @@ def _zip_flow(*, organization, repo, version, extract_manifest_func,
         logger.info("Extracted files: %s", extracted_files)
         data['extracted_files'] = extracted_files
 
-        with TemporaryDirectory() as tmpdir_flatten:
-            # operator-courier supports only flat dir structure
-            _flatten_manifest_structure(tmpdir, tmpdir_flatten)
-
-            if repo is None:
-                repo = _get_reponame_from_manifests(tmpdir_flatten)
+        if repo is None:
+            repo = _get_reponame_from_manifests(tmpdir)
 
-            version = get_package_version(quay_org, repo, version)
-            logger.info("Using release version: %s", version)
+        version = get_package_version(quay_org, repo, version)
+        logger.info("Using release version: %s", version)
 
-            replace_registries(quay_org, tmpdir_flatten)
+        replace_registries(quay_org, tmpdir)
 
-            quay_org.push_operator_manifest(repo, version, tmpdir_flatten)
+        quay_org.push_operator_manifest(repo, version, tmpdir)
 
     data.update({
         'organization': organization,

tests/api/v2/test_api_push.py

@@ -107,7 +107,7 @@ def test_push_zipfile_encrypted(
 def test_push_koji_nvr(
         client, endpoint_push_koji, mocked_quay_io, mocked_op_courier_push,
         auth_header, mocked_koji_archive_download, mocked_greenwave):
-    """Test REST API for pushing operators form koji by NVR"""
+    """Test REST API for pushing operators from koji by NVR"""
     archive = mocked_koji_archive_download
     rv = client.post(
         endpoint_push_koji.url_path,
@@ -125,6 +125,23 @@ def test_push_koji_nvr(
     mocked_greenwave.assert_called_once_with(endpoint_push_koji.nvr)
 
 
+def test_push_invalid_manifests(
+        client, endpoint_push_koji, mocked_quay_io, mocked_op_courier_push,
+        auth_header, mocked_bad_koji_archive_download, mocked_greenwave):
+    """Test REST API for failing to push operators with bad manifests """
+    rv = client.post(
+        endpoint_push_koji.url_path,
+        headers=auth_header
+    )
+    assert rv.status_code == requests.codes.bad_request, rv.get_json()
+    rv_json = rv.get_json()
+    assert rv_json['error'] == 'PackageValidationError'
+    assert rv_json['message'] in (
+        'Could not find packageName in manifests.',
+        'Failed to parse yaml file /not_yaml.yaml',
+    )
+
+
 def test_push_koji_unauthorized(client, endpoint_push_koji):
     """Test if api properly refuses unauthorized requests"""
     rv = client.post(endpoint_push_koji.url_path)

tests/conftest.py

@@ -59,7 +59,7 @@ def datadir(tmpdir):
     ('etcd_op_nested', 'etcd'),
 ])
 def valid_manifest_dir(request, datadir):
-    """Return metadata and path to manifest"""
+    """Return metadata and path to valid manifest"""
     manifest_dir_name, pkg_name = request.param
     path = os.path.join(datadir, manifest_dir_name)
     return ManifestDirMeta(
@@ -69,6 +69,21 @@ def valid_manifest_dir(request, datadir):
     )
 
 
+@pytest.fixture(params=[
+    ('no_package_name', 'no_package_name'),
+    ('not_yaml', 'not_yaml'),
+])
+def invalid_manifest_dir(request, datadir):
+    """Return metadata and path to invalid manifest"""
+    manifest_dir_name, pkg_name = request.param
+    path = os.path.join(datadir, manifest_dir_name)
+    return ManifestDirMeta(
+        path=path,
+        pkg_name=pkg_name,
+        valid=False
+    )
+
+
 @pytest.fixture
 def valid_manifest_flatten_dir(valid_manifest_dir):
     """Most operator-courier operations require flatten dir structure"""
@@ -86,12 +101,11 @@ def valid_manifest_flatten_dir(valid_manifest_dir):
         )
 
 
-@pytest.fixture
-def valid_manifests_archive(datadir, tmpdir, valid_manifest_dir):
+def _manifests_archive(datadir, tmpdir, manifest_dir):
     """Construct valid operator manifest data zip archive"""
     path = os.path.join(tmpdir, 'test_archive.zip')
 
-    start = valid_manifest_dir.path
+    start = manifest_dir.path
     res_files = []
 
     with zipfile.ZipFile(path, 'w') as zip_archive:
@@ -105,8 +119,20 @@ def valid_manifests_archive(datadir, tmpdir, valid_manifest_dir):
     return ArchiveMeta(
         path=path,
         files=sorted(res_files),
-        pkg_name=valid_manifest_dir.pkg_name,
-        valid=valid_manifest_dir.valid)
+        pkg_name=manifest_dir.pkg_name,
+        valid=manifest_dir.valid)
+
+
+@pytest.fixture
+def valid_manifests_archive(datadir, tmpdir, valid_manifest_dir):
+    """Construct valid operator manifest data zip archive"""
+    return _manifests_archive(datadir, tmpdir, valid_manifest_dir)
+
+
+@pytest.fixture
+def invalid_manifests_archive(datadir, tmpdir, invalid_manifest_dir):
+    """Construct invalid operator manifest data zip archive"""
+    return _manifests_archive(datadir, tmpdir, invalid_manifest_dir)
 
 
 @pytest.fixture
@@ -181,22 +207,32 @@ def __exit__(self, *args):
     return CourierPushCM
 
 
-@pytest.fixture
-def mocked_koji_archive_download(valid_manifests_archive):
-    """Mock KojiUtil.koji_download_manifest_archive to return valid archive"""
+def _koji_archive_download(manifests_archive):
     def fake_download(nvr, target_fd):
-        with open(valid_manifests_archive.path, 'rb') as zf:
+        with open(manifests_archive.path, 'rb') as zf:
             target_fd.write(zf.read())
             target_fd.flush()
 
     orig = KOJI.download_manifest_archive
     try:
         KOJI.download_manifest_archive = fake_download
-        yield valid_manifests_archive
+        yield manifests_archive
     finally:
         KOJI.download_manifest_archive = orig
 
 
+@pytest.fixture
+def mocked_koji_archive_download(valid_manifests_archive):
+    """Mock KojiUtil.koji_download_manifest_archive to return valid archive"""
+    yield from _koji_archive_download(valid_manifests_archive)
+
+
+@pytest.fixture
+def mocked_bad_koji_archive_download(invalid_manifests_archive):
+    """Mock KojiUtil.koji_download_manifest_archive to return invalid archive"""
+    yield from _koji_archive_download(invalid_manifests_archive)
+
+
 @pytest.fixture
 def mocked_koji_get_api_version():
     """Mock global KOJI.get_api_version to return valid version"""

tests/data/no_package_name/package.yaml

@@ -0,0 +1,2 @@
+# Note there is no packageName defined here, making this invalid.
+key: value

tests/data/not_yaml/not_yaml.yaml

@@ -0,0 +1,2 @@
+foo: bar
+baz

tests/data/not_yaml/package.yaml

@@ -0,0 +1 @@
+packageName: not_yaml