From f797cf61fb04f5263686c184e851b5c018c08e92 Mon Sep 17 00:00:00 2001
From: mprahl <mprahl@redhat.com>
Date: Mar 21 2018 13:05:06 +0000
Subject: [PATCH 1/2] Show errors when a module build fails


Signed-off-by: mprahl <mprahl@redhat.com>

---

diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 74cdbfc..6df03f0 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -3082,11 +3082,12 @@ class Commands(object):
             'POST', url, body, auth_method, oidc_id_provider, oidc_client_id,
             oidc_client_secret, oidc_scopes, timeout=120)
 
-        data = {}
-        try:
-            data = resp.json()
-            return data['id']
-        except (KeyError, ValueError):
+        if not resp.ok:
+            try:
+                data = resp.json()
+                return data['id']
+            except (KeyError, ValueError):
+                raise rpkgError('The build failed with an unexpected error')
             if 'message' in data:
                 error_msg = data['message']
             else:

From a5048e5f810900f0b004271e1c9284b10338c331 Mon Sep 17 00:00:00 2001
From: mprahl <mprahl@redhat.com>
Date: Mar 22 2018 12:58:26 +0000
Subject: [PATCH 2/2] Add support for Module Stream Expansion (MBS API v2)


Signed-off-by: mprahl <mprahl@redhat.com>

---

diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 6df03f0..34b19e7 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -3063,7 +3063,8 @@ class Commands(object):
         be None.
         :kwarg oidc_scopes: a list of OIDC scopes when MBS is using OIDC for
         authentication
-        :return: None
+        :return: a list of module build IDs that are being built from this
+        request
         """
         body = {'scmurl': scm_url, 'branch': branch}
         optional = optional if optional else []
@@ -3085,7 +3086,6 @@ class Commands(object):
         if not resp.ok:
             try:
                 data = resp.json()
-                return data['id']
             except (KeyError, ValueError):
                 raise rpkgError('The build failed with an unexpected error')
             if 'message' in data:
@@ -3093,15 +3093,27 @@ class Commands(object):
             else:
                 error_msg = resp.text
             raise rpkgError('The build failed with:\n{0}'.format(error_msg))
+        data = resp.json()
+        builds = data if isinstance(data, list) else [data]
+        return [build['id'] for build in builds]
 
-    def module_watch_build(self, api_url, build_id):
+    def module_watch_build(self, api_url, build_ids):
         """
-        Watches the MBS build in a loop that updates every 15 seconds.
-        The loop ends when the build state is 'failed', 'done', or 'ready'.
+        Watches the first MBS build in the list in a loop that updates every
+        15 seconds. The loop ends when the build state is 'failed', 'done', or
+        'ready'.
         :param api_url: a string of the URL of the MBS API
-        :param build_id: an integer of the module build to watch.
+        :param build_ids: a list of module build IDs
         :return: None
         """
+        build_id = build_ids[0]
+        warning = None
+        if len(build_ids) > 1:
+            warning = (
+                'WARNING: Module builds {0} and {1} were generated using '
+                'stream expansion. Only module build {2} is being watched.'
+                .format(', '.join([str(b_id) for b_id in build_ids[0:-1]]),
+                        str(build_ids[-1]), str(build_id)))
         # Load the Koji session anonymously so we get access to the Koji web
         # URL
         self.load_kojisession(anon=True)
@@ -3128,6 +3140,9 @@ class Commands(object):
                 # clearing the screen using print
                 print(chr(27) + "[2J")
 
+            if warning:
+                print(warning)
+
             # Display all RPMs that have built or have failed
             build_state = 0
             failed_state = 3
@@ -3167,3 +3182,29 @@ class Commands(object):
             print(template.format(**build))
             if not done:
                 time.sleep(15)
+
+    def module_get_api_version(self, api_url):
+        """
+        Queries the /about/ API to determine what the latest API version that
+        MBS supports is and returns the latest API that both rpkg and MBS
+        support.
+        :param api_url: a string of the URL of the MBS API
+        :return: an int of the API version
+        """
+        url = '{0}/about/'.format(api_url.rstrip('/'))
+        response = requests.get(url, timeout=60)
+        if response.ok:
+            api_version = response.json().get('api_version', 1)
+            if api_version >= 2:
+                # Max out at API v2
+                return 2
+            else:
+                return 1
+        else:
+            try:
+                error_msg = response.json()['message']
+            except (ValueError, KeyError):
+                error_msg = response.text
+            raise rpkgError(
+                'The following error occurred while trying to determine the '
+                'API versions supported by MBS: {0}'.format(error_msg))
diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py
index b165e64..d7e998e 100644
--- a/pyrpkg/cli.py
+++ b/pyrpkg/cli.py
@@ -21,6 +21,7 @@ import random
 import string
 import sys
 import time
+import re
 
 import koji_cli.lib
 import pyrpkg.utils as utils
@@ -53,6 +54,10 @@ class cliClient(object):
         # Property holders, set to none
         self._cmd = None
         self._module = None
+        # Set some MBS properties to be None so it can be determined and stored
+        # when a module command gets executed
+        self._module_api_version = None
+        self._module_api_url = None
         # Setup the base argparser
         self.setup_argparser()
         # Add a subparser
@@ -1430,32 +1435,35 @@ see API KEY section of copr-cli(1) man page.
         Builds a module using MBS
         :return: None
         """
-        self.module_validate_config()
+        api_url = self.module_api_url
         scm_url, branch = self.cmd.module_get_scm_info(
             self.args.scm_url, self.args.branch)
-        api_url = self.config.get(self.config_section, 'api_url')
         auth_method, oidc_id_provider, oidc_client_id, oidc_client_secret, \
             oidc_scopes = self.module_get_auth_config()
 
         if not self.args.q:
             print('Submitting the module build...')
-        build_id = self._cmd.module_submit_build(
+        build_ids = self._cmd.module_submit_build(
             api_url, scm_url, branch, auth_method, self.args.optional,
             oidc_id_provider, oidc_client_id, oidc_client_secret, oidc_scopes)
         if self.args.watch:
-            self.module_watch_build(build_id)
+            self.module_watch_build(build_ids)
         elif not self.args.q:
-            print('The build #{0} was submitted to the MBS'
-                  .format(build_id))
+            if len(build_ids) > 1:
+                ids_to_print = 'builds {0} and {1} were'.format(
+                    ', '.join([str(b_id) for b_id in build_ids[0:-1]]),
+                    str(build_ids[-1]))
+            else:
+                ids_to_print = 'build {0} was'.format(str(build_ids[0]))
+            print('The {0} submitted to the MBS' .format(ids_to_print))
 
     def module_build_cancel(self):
         """
         Cancel an MBS build
         :return: None
         """
-        self.module_validate_config()
+        api_url = self.module_api_url
         build_id = self.args.build_id
-        api_url = self.config.get(self.config_section, 'api_url')
         auth_method, oidc_id_provider, oidc_client_id, oidc_client_secret, \
             oidc_scopes = self.module_get_auth_config()
 
@@ -1472,9 +1480,7 @@ see API KEY section of copr-cli(1) man page.
         Show information about an MBS build
         :return: None
         """
-        self.module_validate_config()
-        api_url = self.config.get(self.config_section, 'api_url')
-        self.cmd.module_build_info(api_url, self.args.build_id)
+        self.cmd.module_build_info(self.module_api_url, self.args.build_id)
 
     def module_build_local(self):
         """
@@ -1529,23 +1535,67 @@ see API KEY section of copr-cli(1) man page.
         return (auth_method, oidc_id_provider, oidc_client_id,
                 oidc_client_secret, oidc_scopes)
 
+    @property
+    def module_api_version(self):
+        """
+        A property that returns that maximum API version supported by both
+        rpkg and MBS
+        :return: an int of the API version
+        """
+        if self._module_api_version is None:
+            self.module_validate_config()
+            api_url = self.config.get(self.config_section, 'api_url')
+            # Eventually, tools built with rpkg should have their configuration
+            # updated to not hardcode the MBS API version. This checks if it is
+            # hardcoded.
+            if not re.match(r'^.+/\d+/$', api_url):
+                # The API version is not hardcoded, so let's query using the v1
+                # API to find out the API versions that MBS supports.
+                api_url = '{0}/1/'.format(api_url.rstrip('/'))
+            self._module_api_version = self.cmd.module_get_api_version(api_url)
+        return self._module_api_version
+
+    @property
+    def module_api_url(self):
+        """
+        A property that returns the MBS base API URL based on the maximum API
+        version supported by both rpkg and MBS
+        :return: a string of the MBS API URL
+        """
+        if self._module_api_url:
+            return self._module_api_url
+        # Calling this now will ensure that self.module_validate_config()
+        # has been run
+        api_version = self.module_api_version
+        api_url = self.config.get(self.config_section, 'api_url')
+        # Eventually, tools built with rpkg should have their configuration
+        # updated to not hardcode the MBS API version. This checks if it is
+        # hardcoded.
+        if re.match(r'.+/\d+/$', api_url):
+            self._module_api_url = re.sub(
+                r'/\d+/$', '/{0}/'.format(str(api_version)), api_url)
+        else:
+            # The API version is not hardcoded, so we can simply add on the
+            # API version we want to use
+            self._module_api_url = '{0}/{1}/'.format(
+                api_url.rstrip('/'), api_version)
+        return self._module_api_url
+
     def module_build_watch(self):
         """
         Watch an MBS build from the command-line
         :return: None
         """
-        self.module_validate_config()
-        self.module_watch_build(self.args.build_id)
+        self.module_watch_build([self.args.build_id])
 
     def module_overview(self):
         """
         Show the overview of the latest builds in the MBS
         :return: None
         """
-        self.module_validate_config()
-        api_url = self.config.get(self.config_section, 'api_url')
         self.cmd.module_overview(
-            api_url, self.args.limit, finished=(not self.args.unfinished))
+            self.module_api_url, self.args.limit,
+            finished=(not self.args.unfinished))
 
     def module_validate_config(self):
         """
@@ -1590,16 +1640,15 @@ see API KEY section of copr-cli(1) man page.
                 raise rpkgError(config_error.format(
                     required_config, self.config_section))
 
-    def module_watch_build(self, build_id):
+    def module_watch_build(self, build_ids):
         """
-        Watches the MBS build in a loop that updates every 15 seconds.
-        The loop ends when the build state is 'failed', 'done', or 'ready'.
-        :param build_id: an integer of the module build to watch
+        Watches the first MBS build in the list in a loop that updates every
+        15 seconds. The loop ends when the build state is 'failed', 'done', or
+        'ready'.
+        :param build_ids: a list of module build IDs
         :return: None
         """
-        self.module_validate_config()
-        api_url = self.config.get(self.config_section, 'api_url')
-        self.cmd.module_watch_build(api_url, build_id)
+        self.cmd.module_watch_build(self.module_api_url, build_ids)
 
     def new(self):
         new_diff = self.cmd.new()
diff --git a/tests/fixtures/rpkg.conf b/tests/fixtures/rpkg.conf
index ae8e953..90bfd74 100644
--- a/tests/fixtures/rpkg.conf
+++ b/tests/fixtures/rpkg.conf
@@ -12,7 +12,7 @@ clone_config =
 
 [rpkg.mbs]
 auth_method = oidc
-api_url = https://mbs.fedoraproject.org/module-build-service/1/
+api_url = https://mbs.fedoraproject.org/module-build-service/
 oidc_id_provider = https://id.fedoraproject.org/openidc/
 oidc_client_id = mbs-authorizer
 oidc_client_secret = notsecret
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 135b25e..d141b27 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1773,8 +1773,9 @@ class TestModulesCli(CliTestCase):
     }
 
     @patch('sys.stdout', new=StringIO())
+    @patch('requests.get')
     @patch('openidc_client.OpenIDCClient.send_request')
-    def test_module_build(self, mock_oidc_req):
+    def test_module_build_v1(self, mock_oidc_req, mock_get):
         """
         Test a module build with an SCM URL and branch supplied
         """
@@ -1786,14 +1787,21 @@ class TestModulesCli(CliTestCase):
             'git://pkgs.fedoraproject.org/modules/testmodule?#79d87a5a',
             'master'
         ]
-        mock_rv = Mock()
-        mock_rv.json.return_value = {'id': 1094}
-        mock_oidc_req.return_value = mock_rv
+        mock_get.return_value.ok = True
+        mock_get.return_value.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 1
+        }
+        mock_oidc_req.return_value.json.return_value = {'id': 1094}
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.module_build()
 
+        mock_get.assert_called_once_with(
+            'https://mbs.fedoraproject.org/module-build-service/1/about/',
+            timeout=60
+        )
         exp_url = ('https://mbs.fedoraproject.org/module-build-service/1/'
                    'module-builds/')
         exp_json = {
@@ -1807,13 +1815,62 @@ class TestModulesCli(CliTestCase):
             scopes=self.scopes,
             timeout=120)
         output = sys.stdout.getvalue().strip()
-        expected_output = ('Submitting the module build...\nThe build #1094 '
+        expected_output = ('Submitting the module build...\nThe build 1094 '
                            'was submitted to the MBS')
         self.assertEqual(output, expected_output)
 
     @patch('sys.stdout', new=StringIO())
+    @patch('requests.get')
+    @patch('openidc_client.OpenIDCClient.send_request')
+    def test_module_build_v2(self, mock_oidc_req, mock_get):
+        """
+        Test a module build with an SCM URL and branch supplied on the v2 API
+        """
+        cli_cmd = [
+            'rpkg',
+            '--path',
+            self.cloned_repo_path,
+            'module-build',
+            'git://pkgs.fedoraproject.org/modules/testmodule?#79d87a5a',
+            'master'
+        ]
+        mock_get.return_value.ok = True
+        mock_get.return_value.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_oidc_req.return_value.json.return_value = [
+            {'id': 1094}, {'id': 1095}, {'id': 1096}]
+
+        with patch('sys.argv', new=cli_cmd):
+            cli = self.new_cli()
+            cli.module_build()
+
+        mock_get.assert_called_once_with(
+            'https://mbs.fedoraproject.org/module-build-service/1/about/',
+            timeout=60
+        )
+        exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/'
+                   'module-builds/')
+        exp_json = {
+            'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?'
+                       '#79d87a5a'),
+            'branch': 'master'}
+        mock_oidc_req.assert_called_once_with(
+            exp_url,
+            http_method='POST',
+            json=exp_json,
+            scopes=self.scopes,
+            timeout=120)
+        output = sys.stdout.getvalue().strip()
+        expected_output = ('Submitting the module build...\nThe builds 1094, '
+                           '1095 and 1096 were submitted to the MBS')
+        self.assertEqual(output, expected_output)
+
+    @patch('sys.stdout', new=StringIO())
+    @patch('requests.get')
     @patch('openidc_client.OpenIDCClient.send_request')
-    def test_module_build_input(self, mock_oidc_req):
+    def test_module_build_input(self, mock_oidc_req, mock_get):
         """
         Test a module build with default parameters
         """
@@ -1823,18 +1880,25 @@ class TestModulesCli(CliTestCase):
             self.cloned_repo_path,
             'module-build'
         ]
-        mock_rv = Mock()
-        mock_rv.json.return_value = {'id': 1094}
-        mock_oidc_req.return_value = mock_rv
+        mock_get.return_value.ok = True
+        mock_get.return_value.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_oidc_req.return_value.json.return_value = [{'id': 1094}]
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.module_build()
 
         output = sys.stdout.getvalue().strip()
-        expected_output = ('Submitting the module build...\nThe build #1094 '
+        expected_output = ('Submitting the module build...\nThe build 1094 '
                            'was submitted to the MBS')
         self.assertEqual(output, expected_output)
+        mock_get.assert_called_once_with(
+            'https://mbs.fedoraproject.org/module-build-service/1/about/',
+            timeout=60
+        )
         # Can't verify the calls since the SCM commit hash always changes
         mock_oidc_req.assert_called_once()
 
@@ -1853,22 +1917,32 @@ class TestModulesCli(CliTestCase):
             '1125'
         ]
         mock_rv = Mock()
-        mock_rv.json.return_value = {'id': 1094}
-        mock_get.return_value = mock_rv
+        mock_rv.ok = True
+        mock_rv.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
         mock_rv_two = Mock()
-        mock_rv_two.json.ok = True
-        mock_oidc_req.return_value = mock_rv_two
+        mock_rv_two.json.return_value = [{'id': 1094}]
+        mock_get.side_effect = [mock_rv, mock_rv_two]
+        mock_oidc_req.return_value.ok = True
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.module_build_cancel()
+
         exp_url = ('https://mbs.fedoraproject.org/module-build-service/1/'
-                   'module-builds/1125?verbose=true')
-        mock_get.assert_called_once_with(exp_url, timeout=60)
-        exp_url_two = ('https://mbs.fedoraproject.org/module-build-service/1/'
-                       'module-builds/1125')
+                   'about/')
+        exp_url_two = ('https://mbs.fedoraproject.org/module-build-service/2/'
+                       'module-builds/1125?verbose=true')
+        self.assertEqual(mock_get.call_args_list, [
+            call(exp_url, timeout=60),
+            call(exp_url_two, timeout=60)
+        ])
+        exp_url_three = ('https://mbs.fedoraproject.org/module-build-service/'
+                         '2/module-builds/1125')
         mock_oidc_req.assert_called_once_with(
-            exp_url_two,
+            exp_url_three,
             http_method='PATCH',
             json={'state': 'failed'},
             scopes=self.scopes,
@@ -1892,13 +1966,19 @@ class TestModulesCli(CliTestCase):
             '1125'
         ]
         mock_rv = Mock()
-        mock_rv.ok = False
+        mock_rv.ok = True
         mock_rv.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_rv_two = Mock()
+        mock_rv_two.ok = False
+        mock_rv_two.json.return_value = {
             'status': 404,
             'message': 'No such module found.',
             'error': 'Not Found'
         }
-        mock_get.return_value = mock_rv
+        mock_get.side_effect = [mock_rv, mock_rv_two]
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
@@ -1911,8 +1991,13 @@ class TestModulesCli(CliTestCase):
                                   'such module found.')
                 self.assertEqual(str(error), expected_error)
         exp_url = ('https://mbs.fedoraproject.org/module-build-service/1/'
-                   'module-builds/1125?verbose=true')
-        mock_get.assert_called_once_with(exp_url, timeout=60)
+                   'about/')
+        exp_url_two = ('https://mbs.fedoraproject.org/module-build-service/2/'
+                       'module-builds/1125?verbose=true')
+        self.assertEqual(mock_get.call_args_list, [
+            call(exp_url, timeout=60),
+            call(exp_url_two, timeout=60)
+        ])
         mock_oidc_req.assert_not_called()
 
     @patch('sys.stdout', new=StringIO())
@@ -1933,15 +2018,26 @@ class TestModulesCli(CliTestCase):
         ]
         mock_rv = Mock()
         mock_rv.ok = True
-        mock_rv.json.return_value = self.module_build_json
-        mock_get.return_value = mock_rv
+        mock_rv.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_rv_two = Mock()
+        mock_rv_two.ok = True
+        mock_rv_two.json.return_value = self.module_build_json
+        mock_get.side_effect = [mock_rv, mock_rv_two]
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.module_build_info()
         exp_url = ('https://mbs.fedoraproject.org/module-build-service/1/'
-                   'module-builds/2150?verbose=true')
-        mock_get.assert_called_once_with(exp_url, timeout=60)
+                   'about/')
+        exp_url_two = ('https://mbs.fedoraproject.org/module-build-service/2/'
+                       'module-builds/2150?verbose=true')
+        self.assertEqual(mock_get.call_args_list, [
+            call(exp_url, timeout=60),
+            call(exp_url_two, timeout=60)
+        ])
         output = sys.stdout.getvalue().strip()
         expected_output = """\
 Name:           python3-ecosystem
@@ -1992,16 +2088,27 @@ Components:
         ]
         mock_rv = Mock()
         mock_rv.ok = True
-        mock_rv.json.return_value = self.module_build_json
-        mock_get.return_value = mock_rv
+        mock_rv.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_rv_two = Mock()
+        mock_rv_two.ok = True
+        mock_rv_two.json.return_value = self.module_build_json
+        mock_get.side_effect = [mock_rv, mock_rv_two]
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.module_build_watch()
 
         exp_url = ('https://mbs.fedoraproject.org/module-build-service/1/'
-                   'module-builds/1500?verbose=true')
-        mock_get.assert_called_once_with(exp_url, timeout=60)
+                   'about/')
+        exp_url_two = ('https://mbs.fedoraproject.org/module-build-service/2/'
+                       'module-builds/1500?verbose=true')
+        self.assertEqual(mock_get.call_args_list, [
+            call(exp_url, timeout=60),
+            call(exp_url_two, timeout=60)
+        ])
         mock_system.assert_called_once_with('clear')
         output = sys.stdout.getvalue().strip()
         expected_output = """\
@@ -2098,8 +2205,14 @@ torsava's build #2150 of python3-ecosystem-master is in the "failed" state (reas
 
         mock_rv = Mock()
         mock_rv.ok = True
-        mock_rv.json.side_effect = [json_one, json_two, json_three]
-        mock_get.return_value = mock_rv
+        mock_rv.json.return_value = {
+            'auth_method': 'oidc',
+            'api_version': 2
+        }
+        mock_rv_two = Mock()
+        mock_rv_two.ok = True
+        mock_rv_two.json.side_effect = [json_one, json_two, json_three]
+        mock_get.side_effect = [mock_rv, mock_rv_two, mock_rv_two, mock_rv_two]
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
@@ -2107,7 +2220,7 @@ torsava's build #2150 of python3-ecosystem-master is in the "failed" state (reas
 
         # Can't confirm the call parameters because multithreading makes the
         # order random
-        self.assertEqual(mock_get.call_count, 3)
+        self.assertEqual(mock_get.call_count, 4)
         output = sys.stdout.getvalue().strip()
         expected_output = """
 ID:       1100

