Merge pull request #2113 from bowlofeggs/3.2-backports

3.2 backports

bodhi/server/models.py

@@ -1682,12 +1682,16 @@ def contains_critpath_component(builds, release_name):
             bool: ``True`` if the update contains a critical path package, ``False`` otherwise.
         component
         """
+        relname = release_name.lower()
+        components = defaultdict(list)
+        # Get the mess down to a dict of ptype -> [pname]
         for build in builds:
-            # This function call is cached, so there is no need to optimize
-            # it here.
-            critpath_components = get_critpath_components(
-                release_name.lower(), build.package.type.value)
-            if build.package.name in critpath_components:
+            ptype = build.package.type.value
+            pname = build.package.name
+            components[ptype].append(pname)
+
+        for ptype in components:
+            if get_critpath_components(relname, ptype, frozenset(components[ptype])):
                 return True
 
         return False

bodhi/server/scripts/check_policies.py

@@ -24,7 +24,6 @@
 messages in the message bus yet.
 """
 import click
-from sqlalchemy.sql.expression import false
 
 from bodhi.server import config, initialize_db, models, Session
 from bodhi.server.util import greenwave_api_post
@@ -37,9 +36,8 @@ def check():
     initialize_db(config.config)
     session = Session()
 
-    updates = models.Update.query.filter(models.Update.pushed == false())\
-        .filter(models.Update.status.in_(
-                [models.UpdateStatus.pending, models.UpdateStatus.testing]))
+    updates = models.Update.query.filter(models.Update.status.in_(
+        [models.UpdateStatus.pending, models.UpdateStatus.testing]))
     for update in updates:
         # We retrieve updates going to testing (status=pending) and updates
         # (status=testing) going to stable.

bodhi/server/util.py

@@ -29,6 +29,7 @@
 import socket
 import subprocess
 import tempfile
+import time
 import urllib
 
 from kitchen.iterutils import iterate
@@ -243,14 +244,16 @@ def __get__(self, obj, objtype):
 
 
 @memoized
-def get_critpath_components(collection='master', component_type='rpm'):
+def get_critpath_components(collection='master', component_type='rpm', components=None):
     """
-    Return a list of critical path packages for a given collection.
+    Return a list of critical path packages for a given collection, filtered by components.
 
     Args:
-    collection (basestring): The collection/branch to search. Defaults to 'master'.
-    component_type (basestring): The component type to search for. This only affects PDC queries.
-        Defaults to 'rpm'.
+        collection (basestring): The collection/branch to search. Defaults to 'master'.
+        component_type (basestring): The component type to search for. This only affects PDC
+            queries. Defaults to 'rpm'.
+        components (frozenset or None): The list of components we are interested in. If None (the
+            default), all components for the given collection and type are returned.
     Returns:
         list: The critpath components for the given collection and type.
     """
@@ -268,9 +271,15 @@ def get_critpath_components(collection='master', component_type='rpm'):
             critpath_components = results['pkgs'][collection]
     elif critpath_type == 'pdc':
         critpath_components = get_critpath_components_from_pdc(
-            collection, component_type)
+            collection, component_type, components)
     else:
         critpath_components = config.get('critpath_pkgs')
+
+    # Filter the list of components down to what was requested, in case the specific path did
+    # not take our request into account.
+    if components is not None:
+        critpath_components = [c for c in critpath_components if c in components]
+
     return critpath_components
 
 
@@ -1109,43 +1118,63 @@ def sort_severity(value):
     return value_map.get(value, 99)
 
 
-def get_critpath_components_from_pdc(branch, component_type='rpm'):
+# If we need to know about more components than this constant, we will just get the full
+# list, rather than a query per package. This is because at some point, just going through
+# paging becomes more performant than getting the page for every component.
+PDC_CRITPATH_COMPONENTS_GETALL_LIMIT = 10
+
+
+def get_critpath_components_from_pdc(branch, component_type='rpm', components=None):
     """
     Search PDC for critical path packages based on the specified branch.
 
     Args:
         branch (basestring): The branch name to search by.
         component_type (basestring): The component type to search by. Defaults to ``rpm``.
+        components (frozenset or None): The list of components we are interested in. If None (the
+            default), all components for the given branch and type are returned.
     Returns:
         list: Critical path package names.
     """
     pdc_api_url = '{}/rest_api/v1/component-branches/'.format(
         config.get('pdc_url').rstrip('/'))
-    query_args = urllib.urlencode({
+    query_args = {
         'active': 'true',
         'critical_path': 'true',
         'name': branch,
         'page_size': 100,
         'type': component_type
-    })
-    pdc_api_url_with_args = '{0}?{1}'.format(pdc_api_url, query_args)
+    }
 
     critpath_pkgs_set = set()
-    while True:
-        pdc_request_json = pdc_api_get(pdc_api_url_with_args)
+    if components and len(components) < PDC_CRITPATH_COMPONENTS_GETALL_LIMIT:
+        # Do a query for every single component
+        for component in components:
+            query_args['global_component'] = component
+            pdc_api_url_with_args = '{0}?{1}'.format(pdc_api_url, urllib.urlencode(query_args))
+            pdc_request_json = pdc_api_get(pdc_api_url_with_args)
+            for branch_rv in pdc_request_json['results']:
+                critpath_pkgs_set.add(branch_rv['global_component'])
+            if pdc_request_json['next']:
+                raise Exception('We got paging when requesting a single component?!')
+    else:
+        pdc_api_url_with_args = '{0}?{1}'.format(pdc_api_url, urllib.urlencode(query_args))
+        while True:
+            pdc_request_json = pdc_api_get(pdc_api_url_with_args)
 
-        for branch_rv in pdc_request_json['results']:
-            critpath_pkgs_set.add(branch_rv['global_component'])
+            for branch_rv in pdc_request_json['results']:
+                critpath_pkgs_set.add(branch_rv['global_component'])
 
-        if pdc_request_json['next']:
-            pdc_api_url_with_args = pdc_request_json['next']
-        else:
-            # There are no more results to iterate through
-            break
+            if pdc_request_json['next']:
+                pdc_api_url_with_args = pdc_request_json['next']
+            else:
+                # There are no more results to iterate through
+                break
     return list(critpath_pkgs_set)
 
 
-def call_api(api_url, service_name, error_key=None, method='GET', data=None):
+def call_api(api_url, service_name, error_key=None, method='GET', data=None, headers=None,
+             retries=0):
     """
     Perform an HTTP request with response type and error handling.
 
@@ -1157,6 +1186,8 @@ def call_api(api_url, service_name, error_key=None, method='GET', data=None):
             service. If this is set to None, the JSON response will be used as the error message.
         method (basestring): The HTTP method to use for the request. Defaults to ``GET``.
         data (dict): Query string parameters that will be sent along with the request to the server.
+        retries (int): The number of times to retry, each after a 1 second sleep, if we get a
+            non-200 HTTP code. Defaults to 3.
     Returns:
         dict: A dictionary representing the JSON response from the remote service.
     Raises:
@@ -1180,6 +1211,9 @@ def call_api(api_url, service_name, error_key=None, method='GET', data=None):
         rv = http_session.get(api_url, timeout=60)
     if rv.status_code == 200:
         return rv.json()
+    elif retries:
+        time.sleep(1)
+        return call_api(api_url, service_name, error_key, method, data, headers, retries - 1)
     elif rv.status_code == 500:
         # There will be no JSON with an error message here
         error_msg = base_error_msg.format(
@@ -1213,7 +1247,7 @@ def pagure_api_get(pagure_api_url):
     Raises:
         RuntimeError: If the server did not give us a 200 code.
     """
-    return call_api(pagure_api_url, service_name='Pagure', error_key='error')
+    return call_api(pagure_api_url, service_name='Pagure', error_key='error', retries=3)
 
 
 def pdc_api_get(pdc_api_url):
@@ -1229,7 +1263,7 @@ def pdc_api_get(pdc_api_url):
     """
     # There is no error_key specified because the error key is not consistent
     # based on the error message
-    return call_api(pdc_api_url, service_name='PDC')
+    return call_api(pdc_api_url, service_name='PDC', retries=3)
 
 
 def greenwave_api_post(greenwave_api_url, data):
@@ -1247,4 +1281,4 @@ def greenwave_api_post(greenwave_api_url, data):
     # There is no error_key specified because the error key is not consistent
     # based on the error message
     return call_api(greenwave_api_url, service_name='Greenwave', method='POST',
-                    data=data)
+                    data=data, retries=3)

bodhi/tests/server/consumers/test_masher.py

@@ -574,8 +574,10 @@ def test_mash_no_found_dirs(self, save_state):
                 t.wait_for_mash(fake_popen)
                 assert False, "Mash without generated dirs did not crash"
             except Exception as ex:
-                assert str(ex) == 'We were unable to find a path with prefix ' + \
-                                  'Fedora-17-updates-testing-2017* in mashdir'
+                expected_error = ('We were unable to find a path with prefix '
+                                  'Fedora-17-updates-testing-{}* in mashdir')
+                expected_error = expected_error.format(datetime.datetime.utcnow().year)
+                assert str(ex) == expected_error
             t.db = None
 
     @mock.patch('bodhi.server.consumers.masher.MasherThread.save_state')

bodhi/tests/server/scripts/test_check_policies.py

@@ -17,6 +17,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 """This module contains tests for the bodhi.server.scripts.check_policies module."""
+import datetime
 
 from click import testing
 from mock import patch
@@ -52,9 +53,11 @@ def test_policies_satisfied(self):
 
         expected_query = {
             'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
-            'subject': [{'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
-                        {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
-                        {'item': u'FEDORA-2017-a3bbe1a8f2', 'type': 'bodhi_update'}]}
+            'subject': [
+                {'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
+                {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
+                {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                 'type': 'bodhi_update'}]}
         mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
                                                expected_query)
 
@@ -82,9 +85,11 @@ def test_policies_pending_satisfied(self):
 
         expected_query = {
             'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_testing',
-            'subject': [{'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
-                        {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
-                        {'item': u'FEDORA-2017-a3bbe1a8f2', 'type': 'bodhi_update'}]}
+            'subject': [
+                {'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
+                {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
+                {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                 'type': 'bodhi_update'}]}
         mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
                                                expected_query)
 
@@ -116,9 +121,11 @@ def test_policies_unsatisfied(self):
 
         expected_query = {
             'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
-            'subject': [{'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
-                        {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
-                        {'item': u'FEDORA-2017-a3bbe1a8f2', 'type': 'bodhi_update'}]}
+            'subject': [
+                {'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
+                {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
+                {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                 'type': 'bodhi_update'}]}
         mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
                                                expected_query)
 
@@ -143,11 +150,44 @@ def test_no_policies_enforced(self):
         update = self.db.query(models.Update).filter(models.Update.id == update.id).one()
         # The test_gating_status should still be None.
         self.assertTrue(update.test_gating_status is None)
+        expected_query = {
+            'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
+            'subject': [
+                {'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
+                {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
+                {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                 'type': 'bodhi_update'}]}
+        mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
+                                               expected_query)
+
+    @patch.dict(config, [('greenwave_api_url', 'http://domain.local')])
+    def test_pushed_update(self):
+        """Assert that check() operates on pushed updates."""
+        runner = testing.CliRunner()
+        update = self.db.query(models.Update).all()[0]
+        update.status = models.UpdateStatus.testing
+        update.pushed = True
+        self.db.commit()
+        with patch('bodhi.server.scripts.check_policies.greenwave_api_post') as mock_greenwave:
+            greenwave_response = {
+                'policies_satisfied': False,
+                'summary': 'it broke',
+                'applicable_policies': ['bodhi-unrestricted'],
+            }
+            mock_greenwave.return_value = greenwave_response
+
+            result = runner.invoke(check_policies.check, [])
+
+        self.assertEqual(result.exit_code, 0)
+        update = self.db.query(models.Update).filter(models.Update.id == update.id).one()
+        self.assertEqual(update.test_gating_status, models.TestGatingStatus.failed)
+        self.assertEqual(update.greenwave_summary_string, 'it broke')
         expected_query = {
             'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
             'subject': [{'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
                         {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
-                        {'item': u'FEDORA-2017-a3bbe1a8f2', 'type': 'bodhi_update'}]}
+                        {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                         'type': 'bodhi_update'}]}
         mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
                                                expected_query)
 
@@ -173,8 +213,10 @@ def test_unrestricted_policy(self):
 
         expected_query = {
             'product_version': 'fedora-17', 'decision_context': 'bodhi_update_push_stable',
-            'subject': [{'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
-                        {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
-                        {'item': u'FEDORA-2017-a3bbe1a8f2', 'type': 'bodhi_update'}]}
+            'subject': [
+                {'item': u'bodhi-2.0-1.fc17', 'type': 'koji_build'},
+                {'original_spec_nvr': u'bodhi-2.0-1.fc17'},
+                {'item': u'FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
+                 'type': 'bodhi_update'}]}
         mock_greenwave.assert_called_once_with(config['greenwave_api_url'] + '/decision',
                                                expected_query)

bodhi/tests/server/services/test_updates.py

@@ -594,15 +594,16 @@ def test_edit_with_permission(self):
         """
         Test a logged in User with permissions on the update can see the form
         """
-        resp = self.app.get('/updates/FEDORA-2017-a3bbe1a8f2/edit')
+        resp = self.app.get('/updates/FEDORA-{}-a3bbe1a8f2/edit'.format(datetime.utcnow().year))
         self.assertIn('Editing an update requires JavaScript', resp)
 
     def test_edit_without_permission(self):
         """
         Test a logged in User without permissions on the update can't see the form
         """
         app = TestApp(main({}, testing=u'anonymous', session=self.db, **self.app_settings))
-        resp = app.get('/updates/FEDORA-2017-a3bbe1a8f2/edit', status=400)
+        resp = app.get(
+            '/updates/FEDORA-{}-a3bbe1a8f2/edit'.format(datetime.utcnow().year), status=400)
         self.assertIn(
             'anonymous is not a member of "packager", which is a mandatory packager group', resp)
 
@@ -1245,7 +1246,8 @@ def test_updates_search(self):
         self.assertEquals(len(body['updates']), 0)
 
         # test a search for an alias
-        res = self.app.get('/updates/', {'search': 'FEDORA-2017-a3bbe1a8f2'})
+        res = self.app.get(
+            '/updates/', {'search': 'FEDORA-{}-a3bbe1a8f2'.format(datetime.utcnow().year)})
         body = res.json_body
         self.assertEquals(len(body['updates']), 1)
         up = body['updates'][0]

bodhi/tests/server/test_renderers.py

@@ -1,4 +1,5 @@
-# Copyright 2017 Red Hat, Inc.
+# -*- coding: utf-8 -*-
+# Copyright © 2017 Red Hat, Inc.
 #
 # This file is part of Bodhi.
 #
@@ -17,6 +18,7 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 import copy
+import datetime
 import re
 import StringIO
 
@@ -50,7 +52,7 @@ def test_renderer_jpeg(self):
         })
         app = TestApp(main({}, session=self.db, **settings))
 
-        res = app.get('/updates/FEDORA-2017-a3bbe1a8f2',
+        res = app.get('/updates/FEDORA-{}-a3bbe1a8f2'.format(datetime.datetime.utcnow().year),
                       status=200,
                       headers=dict(accept='text/html'))
         captcha_url = re.search(r'"http://localhost(/captcha/[^"]*)"', str(res)).groups()[0]