Use package_suffix instead of repository_suffix

* OSBS-8412

Signed-off-by: Luiz Carvalho <[email protected]>

Dockerfile

@@ -18,9 +18,9 @@ RUN dnf -y install \
     python3-flask \
     python3-jsonschema \
     python3-koji \
-    python3-pyyaml \
     python3-requests \
     python3-operator-courier \
+    python3-ruamel-yaml \
     && dnf -y clean all \
     && rm -rf /tmp/*
 

README.md

@@ -101,13 +101,14 @@ Here's an example:
 ```
 Replacements occur when pushing manifests into the application registry.
 
-#### Altering repository names
+#### Altering package names
 
-Organizations can be configured so a suffix is appended to the repository name.
-The suffix is only applied if the repository name does not already end with the suffix.
+Organizations can be configured so a suffix is appended to the package and the repository names.
+The suffix is only applied to the package if it does not already end with the suffix.
+The repository is only modified if it's being taken from the packageName as in the v2 API.
 Example configuration:
 ```
-"repository_suffix": "-suffix"
+"package_name_suffix": "-suffix"
 ```
 
 ### Greenwave integration

omps/api/v1/push.py

@@ -10,7 +10,7 @@
 import zipfile
 
 from flask import jsonify, current_app, request
-from yaml import safe_load
+from ruamel.yaml import YAML
 
 from . import API
 from omps.api.common import extract_auth_token, replace_registries
@@ -166,19 +166,36 @@ def get_package_version(quay_org, repo, version=None):
     return version
 
 
-def _get_reponame_from_manifests(source_dir):
-    for filename in sorted(os.listdir(source_dir)):
-        filename = os.path.join(source_dir, filename)
+def _process_package_name(quay_org, dir_path):
+    """Returns package name from metadata
+    Retrieve the package name from the packageName attribute
+    of the package yaml file.
+    If not found, or malformed, raise PackageValidationError.
+    If package_name_suffix is configured, modify the packageName.
+    """
+    yaml = YAML()
+    for filename in sorted(os.listdir(dir_path)):
+        filename = os.path.join(dir_path, filename)
         if filename.endswith('.yaml') or filename.endswith('.yml'):
             try:
-                with open(filename, 'r') as f:
-                    contents = safe_load(f.read())
+                with open(filename, 'r+') as f:
+                    contents = yaml.load(f.read())
                     if 'packageName' in contents:
                         name = contents['packageName']
                         logger.info("Found packageName %s in %s", name, filename)
+                        if quay_org.package_name_suffix:
+                            if not name.endswith(quay_org.package_name_suffix):
+                                new_name = name + quay_org.package_name_suffix
+                                contents['packageName'] = new_name
+                                f.seek(0)
+                                yaml.dump(contents, f)
+                                f.truncate()
+                                logger.info("Modified packageName %s in %s to %s",
+                                            name, filename, new_name)
+                                name = new_name
                         return name
             except Exception:
-                message = "Failed to parse yaml file %s" % filename[len(source_dir):]
+                message = "Failed to process yaml file %s" % filename[len(dir_path):]
                 logger.exception(message)
                 raise PackageValidationError(message)
 
@@ -221,10 +238,9 @@ def _zip_flow(*, organization, repo, version, extract_manifest_func,
         logger.info("Extracted files: %s", extracted_files)
         data['extracted_files'] = extracted_files
 
+        package_name = _process_package_name(quay_org, tmpdir)
         if repo is None:
-            repo = _get_reponame_from_manifests(tmpdir)
-
-        repo = quay_org.adjust_repository_name(repo)
+            repo = package_name
 
         version = get_package_version(quay_org, repo, version)
         logger.info("Using release version: %s", version)

omps/quay.py

@@ -157,8 +157,8 @@ class OrgManager:
                             "required": ["old", "new"],
                         }
                     },
-                    "repository_suffix": {
-                        "description": "suffix to append to repository name",
+                    "package_name_suffix": {
+                        "description": "suffix to append to package name",
                         "type": "string"
                     }
                 },
@@ -204,7 +204,7 @@ def get_org(self, organization, cnr_token):
             public=org_config.get('public', False),
             timeout=self._timeout,
             replace_registry_conf=org_config.get('replace_registry'),
-            repository_suffix=org_config.get('repository_suffix'),
+            package_name_suffix=org_config.get('package_name_suffix'),
         )
 
 
@@ -213,7 +213,7 @@ class QuayOrganization:
 
     def __init__(
         self, organization, cnr_token, oauth_token=None, public=False,
-        timeout=None, replace_registry_conf=None, repository_suffix=None
+        timeout=None, replace_registry_conf=None, package_name_suffix=None
     ):
         """
         :param organization: organization name
@@ -231,7 +231,7 @@ def __init__(
         self._public = public
         self._timeout = timeout
         self._replace_registry_conf = replace_registry_conf
-        self._repository_suffix = repository_suffix
+        self._package_name_suffix = package_name_suffix
 
     @property
     def public(self):
@@ -245,6 +245,10 @@ def oauth_access(self):
     def organization(self):
         return self._organization
 
+    @property
+    def package_name_suffix(self):
+        return self._package_name_suffix
+
     @property
     def registry_replacing_enabled(self):
         return bool(self._replace_registry_conf)
@@ -277,24 +281,6 @@ def replace_registries(self, text):
 
         return text
 
-    def adjust_repository_name(self, repo):
-        """Returns the modified repository name
-
-        The repository name is modified if there's a repository_suffix
-        configured for the Quay organization *and* the repository name
-        does not already end in the repository_suffix
-
-        :param repo: name of repository
-        :rtype: str
-        :return: text with replaced registries
-        """
-        if self._repository_suffix:
-            if not repo.endswith(self._repository_suffix):
-                self.logger.info("Applying repository suffix, %s, to %s",
-                                 self._repository_suffix, repo)
-                repo = repo + self._repository_suffix
-        return repo
-
     def push_operator_manifest(self, repo, version, source_dir):
         """Build, verify and push operators artifact to quay.io registry
 

setup.py

@@ -60,9 +60,9 @@ def get_project_version(version_file='omps/__init__.py'):
         'Flask==1.0.*',
         'jsonschema',
         'koji',
-        'PyYAML',
         'requests',
         'operator-courier>=2.1.1',
+        'ruamel.yaml',
     ],
     extras_require={
         'test': [

tests/api/v1/test_api_push.py

@@ -4,11 +4,16 @@
 #
 
 from io import BytesIO
+from textwrap import dedent
+from unittest import mock
+import os.path
 
+import flexmock
 import requests
 import pytest
 
 from omps import constants
+from omps.quay import QuayOrganization
 
 
 def test_push_zipfile(
@@ -36,6 +41,78 @@ def test_push_zipfile(
     assert rv.get_json() == expected
 
 
+@pytest.mark.parametrize(('suffix_func', 'expected_pkg_name_func'), (
+    (lambda _: '-suffix', lambda x: x + '-suffix'),
+    (lambda x: x, lambda x: x),
+    (lambda x: '', lambda x: x),
+))
+@mock.patch('omps.api.v1.push.ORG_MANAGER')
+def test_push_zipfile_with_package_name_suffix(
+        mocked_org_manager, client, valid_manifests_archive,
+        endpoint_push_zipfile, mocked_quay_io, mocked_op_courier_push,
+        auth_header, suffix_func, expected_pkg_name_func):
+    """Test REST API for pushing operators form zipfile with package_name_suffix"""
+    original_pkg_name = valid_manifests_archive.pkg_name
+    expected_pkg_name = expected_pkg_name_func(original_pkg_name)
+
+    mocked_org_manager.get_org.return_value = QuayOrganization(
+        endpoint_push_zipfile.org, 'cnr_token',
+        package_name_suffix=suffix_func(original_pkg_name))
+
+    def verify_modified_package_name(repo, version, source_dir):
+        # The modified YAML file should retain comments and defined order. The
+        # only modification should be the package name.
+        if valid_manifests_archive.pkg_name == 'marketplace':
+            pkg_manifest = (
+                'deploy/chart/catalog_resources/rh-operators/'
+                'marketplace.v0.0.1.clusterserviceversion.yaml')
+            expected_packages_yaml = dedent("""\
+                #! package-manifest: {pkg_manifest}
+                packageName: {pkg_name}
+                channels:
+                - name: alpha
+                  currentCSV: marketplace-operator.v0.0.1
+                """)
+            packages_yaml_path = os.path.join(source_dir, 'packages.yaml')
+        elif valid_manifests_archive.pkg_name == 'etcd':
+            pkg_manifest = (
+                './deploy/chart/catalog_resources/rh-operators/'
+                'etcdoperator.v0.9.2.clusterserviceversion.yaml')
+            expected_packages_yaml = dedent("""\
+                #! package-manifest: {pkg_manifest}
+                packageName: {pkg_name}
+                channels:
+                - name: alpha
+                  currentCSV: etcdoperator.v0.9.2
+                """)
+            packages_yaml_path = os.path.join(source_dir, 'etcd.package.yaml')
+        else:
+            raise ValueError(
+                'Unsupported manifests archive, {}'.format(valid_manifests_archive))
+        expected_packages_yaml = expected_packages_yaml.format(
+            pkg_manifest=pkg_manifest,
+            pkg_name=expected_pkg_name)
+        with open(packages_yaml_path) as f:
+            assert f.read() == expected_packages_yaml
+
+    flexmock(QuayOrganization)\
+        .should_receive('push_operator_manifest')\
+        .replace_with(verify_modified_package_name)\
+        .once()
+
+    with open(valid_manifests_archive.path, 'rb') as f:
+        rv = client.post(
+            endpoint_push_zipfile.url_path,
+            headers=auth_header,
+            data={'file': (f, f.name)},
+            content_type='multipart/form-data',
+        )
+
+    assert rv.status_code == 200, rv.get_json()
+    # In V1, package_name_suffix does not change the repository name.
+    assert rv.get_json()['repo'] == endpoint_push_zipfile.repo
+
+
 @pytest.mark.parametrize('filename', (
     'test.json',  # test invalid extension
     'test.zip',  # test invalid content

tests/api/v2/test_api_push.py

@@ -4,11 +4,16 @@
 #
 
 from io import BytesIO
+from textwrap import dedent
+from unittest import mock
+import os.path
 
+import flexmock
 import requests
 import pytest
 
 from omps import constants
+from omps.quay import QuayOrganization
 
 
 def test_push_zipfile(
@@ -36,6 +41,78 @@ def test_push_zipfile(
     assert rv.get_json() == expected
 
 
+@pytest.mark.parametrize(('suffix_func', 'expected_pkg_name_func'), (
+    (lambda _: '-suffix', lambda x: x + '-suffix'),
+    (lambda x: x, lambda x: x),
+    (lambda x: '', lambda x: x),
+))
+@mock.patch('omps.api.v1.push.ORG_MANAGER')
+def test_push_zipfile_with_package_name_suffix(
+        mocked_org_manager, client, valid_manifests_archive,
+        endpoint_push_zipfile, mocked_quay_io, mocked_op_courier_push,
+        auth_header, suffix_func, expected_pkg_name_func):
+    """Test REST API for pushing operators form zipfile with package_name_suffix"""
+    original_pkg_name = valid_manifests_archive.pkg_name
+    expected_pkg_name = expected_pkg_name_func(original_pkg_name)
+
+    mocked_org_manager.get_org.return_value = QuayOrganization(
+        endpoint_push_zipfile.org, 'cnr_token',
+        package_name_suffix=suffix_func(original_pkg_name))
+
+    def verify_modified_package_name(repo, version, source_dir):
+        # The modified YAML file should retain comments and defined order. The
+        # only modification should be the package name.
+        if valid_manifests_archive.pkg_name == 'marketplace':
+            pkg_manifest = (
+                'deploy/chart/catalog_resources/rh-operators/'
+                'marketplace.v0.0.1.clusterserviceversion.yaml')
+            expected_packages_yaml = dedent("""\
+                #! package-manifest: {pkg_manifest}
+                packageName: {pkg_name}
+                channels:
+                - name: alpha
+                  currentCSV: marketplace-operator.v0.0.1
+                """)
+            packages_yaml_path = os.path.join(source_dir, 'packages.yaml')
+        elif valid_manifests_archive.pkg_name == 'etcd':
+            pkg_manifest = (
+                './deploy/chart/catalog_resources/rh-operators/'
+                'etcdoperator.v0.9.2.clusterserviceversion.yaml')
+            expected_packages_yaml = dedent("""\
+                #! package-manifest: {pkg_manifest}
+                packageName: {pkg_name}
+                channels:
+                - name: alpha
+                  currentCSV: etcdoperator.v0.9.2
+                """)
+            packages_yaml_path = os.path.join(source_dir, 'etcd.package.yaml')
+        else:
+            raise ValueError(
+                'Unsupported manifests archive, {}'.format(valid_manifests_archive))
+        expected_packages_yaml = expected_packages_yaml.format(
+            pkg_manifest=pkg_manifest,
+            pkg_name=expected_pkg_name)
+        with open(packages_yaml_path) as f:
+            assert f.read() == expected_packages_yaml
+
+    flexmock(QuayOrganization)\
+        .should_receive('push_operator_manifest')\
+        .replace_with(verify_modified_package_name)\
+        .once()
+
+    with open(valid_manifests_archive.path, 'rb') as f:
+        rv = client.post(
+            endpoint_push_zipfile.url_path,
+            headers=auth_header,
+            data={'file': (f, f.name)},
+            content_type='multipart/form-data',
+        )
+
+    assert rv.status_code == 200, rv.get_json()
+    # In V2, package_name_suffix also modifies the repository
+    assert rv.get_json()['repo'] == expected_pkg_name
+
+
 @pytest.mark.parametrize('filename', (
     'test.json',  # test invalid extension
     'test.zip',  # test invalid content
@@ -138,7 +215,7 @@ def test_push_invalid_manifests(
     assert rv_json['error'] == 'PackageValidationError'
     assert rv_json['message'] in (
         'Could not find packageName in manifests.',
-        'Failed to parse yaml file /not_yaml.yaml',
+        'Failed to process yaml file /not_yaml.yaml',
     )