Make the repo sanity checks much more strict

This makes us use librepo/hawkey to verify that a repo can be correctly read and used
by DNF, which should prevent us from accepting a repo if DNF will then crash on using
it.

Note: per the hawkey docs, it's obsoleted, and one is supposed to use libhif,
but the libhif (redirected to libdnf) repo says it is being "reworked and is unstable".
From my tests, it seems that hawkey calls libdnf underwater, so I think that this
is reasonable to do for now.

Signed-off-by: Patrick Uiterwijk <[email protected]>

bodhi/server/metadata.py

@@ -21,7 +21,6 @@
 import logging
 import os
 import shelve
-import shutil
 import tempfile
 
 from kitchen.text.converters import to_bytes
@@ -57,24 +56,7 @@ def modifyrepo(comp_type, compose_path, filetype, extension, source):
             repodata = os.path.join(repo_path, arch, 'tree', 'repodata')
         else:
             repodata = os.path.join(repo_path, arch, 'os', 'repodata')
-        log.info('Inserting %s.%s into %s', filetype, extension, repodata)
-        target_fname = os.path.join(repodata, '%s.%s' % (filetype, extension))
-        shutil.copyfile(source, target_fname)
-        repomd_xml = os.path.join(repodata, 'repomd.xml')
-        repomd = cr.Repomd(repomd_xml)
-        # create a new record for our repomd.xml
-        rec = cr.RepomdRecord(filetype, target_fname)
-        # compress our metadata file with the comp_type
-        rec_comp = rec.compress_and_fill(cr.SHA256, comp_type)
-        # add hash to the compresed metadata file
-        rec_comp.rename_file()
-        # set type of metadata
-        rec_comp.type = filetype
-        # insert metadata about our metadata in repomd.xml
-        repomd.set_record(rec_comp)
-        with open(repomd_xml, 'w') as repomd_file:
-            repomd_file.write(repomd.xml_dump())
-        os.unlink(target_fname)
+        util.insert_in_repo(comp_type, repodata, filetype, extension, source)
 
 
 class UpdateInfoMetadata(object):

bodhi/server/util.py

@@ -35,11 +35,15 @@
 import arrow
 import bleach
 import colander
+import createrepo_c as cr
+import hawkey
+import libcomps
 import libravatar
 import librepo
 import markdown
 import requests
 import rpm
+import shutil
 from six.moves import map
 from six.moves.urllib.parse import urlencode
 import six
@@ -95,18 +99,85 @@ def get_rpm_header(nvr, tries=0):
     raise ValueError("No rpm headers found in koji for %r" % nvr)
 
 
-def mkmetadatadir(path):
+def insert_in_repo(comp_type, repodata, filetype, extension, source):
+    """
+    Inject a file into the repodata with the help of createrepo_c.
+
+    Args:
+        repodata (basestring): The path to the repo where the metadata will be inserted.
+        filetype (basestring): What type of metadata will be inserted by createrepo_c.
+            This does allow any string to be inserted (custom types). There are some
+            types which are used with dnf repos as primary, updateinfo, comps, filelist etc.
+        extension (basestring): The file extension (xml, sqlite).
+        source (basestring): A file path. File holds the dump of metadata until
+            copied to the repodata folder.
+    """
+    log.info('Inserting %s.%s into %s', filetype, extension, repodata)
+    target_fname = os.path.join(repodata, '%s.%s' % (filetype, extension))
+    shutil.copyfile(source, target_fname)
+    repomd_xml = os.path.join(repodata, 'repomd.xml')
+    repomd = cr.Repomd(repomd_xml)
+    # create a new record for our repomd.xml
+    rec = cr.RepomdRecord(filetype, target_fname)
+    # compress our metadata file with the comp_type
+    rec_comp = rec.compress_and_fill(cr.SHA256, comp_type)
+    # add hash to the compresed metadata file
+    rec_comp.rename_file()
+    # set type of metadata
+    rec_comp.type = filetype
+    # insert metadata about our metadata in repomd.xml
+    repomd.set_record(rec_comp)
+    with open(repomd_xml, 'w') as repomd_file:
+        repomd_file.write(repomd.xml_dump())
+    os.unlink(target_fname)
+
+
+def mkmetadatadir(path, updateinfo=None, comps=None):
     """
     Generate package metadata for a given directory.
 
     If the metadata doesn't exist, then create it.
 
     Args:
         path (basestring): The directory to generate metadata for.
-    """
+        updateinfo (basestring or None or False): The updateinfo to insert instead of example.
+            No updateinfo is inserted if False is passed.
+        comps (basestring or None): The comps to insert instead of example.
+    """
+    compsfile = '''<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE comps PUBLIC "-//Red Hat, Inc.//DTD Comps info//EN" "comps.dtd">
+<comps>
+  <group>
+    <id>testable</id>
+    <_name>Testable</_name>
+    <_description>comps group for testing</_description>
+    <packagelist>
+      <packagereq>testpkg</packagereq>
+    </packagelist>
+  </group>
+</comps>'''
+    updateinfofile = ''
     if not os.path.isdir(path):
         os.makedirs(path)
-    subprocess.check_call(['createrepo_c', '--xz', '--database', '--quiet', path])
+    if not comps:
+        comps = os.path.join(path, 'comps.xml')
+        with open(comps, 'w') as f:
+            f.write(compsfile)
+    if updateinfo is None:
+        updateinfo = os.path.join(path, 'updateinfo.xml')
+        with open(updateinfo, 'w') as f:
+            f.write(updateinfofile)
+
+    subprocess.check_call(['createrepo_c',
+                           '--groupfile', 'comps.xml',
+                           '--deltas',
+                           '--xz',
+                           '--database',
+                           '--quiet',
+                           path])
+    if updateinfo is not False:
+        insert_in_repo(cr.XZ, os.path.join(path, 'repodata'), 'updateinfo', 'xml',
+                       os.path.join(path, 'updateinfo.xml'))
 
 
 def flash_log(msg):
@@ -264,7 +335,7 @@ def sanity_check_repodata(myurl):
     Args:
         myurl (basestring): A path to a repodata directory.
     Raises:
-        bodhi.server.exceptions.RepodataException: If the repodata is not valid or does not exist.
+        Exception: If the repodata is not valid or does not exist.
     """
     h = librepo.Handle()
     h.setopt(librepo.LRO_REPOTYPE, librepo.LR_YUMREPO)
@@ -278,17 +349,44 @@ def sanity_check_repodata(myurl):
     h.setopt(librepo.LRO_URLS, [myurl])
     h.setopt(librepo.LRO_LOCAL, True)
     h.setopt(librepo.LRO_CHECKSUM, True)
+    h.setopt(librepo.LRO_IGNOREMISSING, False)
+    r = librepo.Result()
     try:
-        h.perform()
+        h.perform(r)
     except librepo.LibrepoException as e:
         rc, msg, general_msg = e.args
         raise RepodataException(msg)
 
-    updateinfo = os.path.join(myurl, 'updateinfo.xml.gz')
-    if os.path.exists(updateinfo):
-        ret = subprocess.call(['zgrep', '<id/>', updateinfo])
-        if not ret:
-            raise RepodataException('updateinfo.xml.gz contains empty ID tags')
+    repo_info = r.getinfo(librepo.LRR_YUM_REPO)
+    primary_sack = hawkey.Sack()
+    hk_repo = hawkey.Repo(myurl)
+    try:
+        hk_repo.filelists_fn = repo_info['filelists']
+        hk_repo.presto_fn = repo_info['prestodelta']
+        hk_repo.primary_fn = repo_info['primary']
+        hk_repo.repomd_fn = repo_info['repomd']
+        hk_repo.updateinfo_fn = repo_info['updateinfo']
+    except KeyError:
+        raise RepodataException('Required part not in repomd.xml')
+    primary_sack.load_repo(hk_repo,
+                           build_cache=False,
+                           load_filelists=True,
+                           load_presto=True,
+                           load_updateinfo=True)
+
+    # Test comps
+    comps = libcomps.Comps()
+    try:
+        ret = comps.fromxml_f(repo_info['group'])
+    except Exception:
+        raise RepodataException('Comps file unable to be parsed')
+    if len(comps.groups) < 1:
+        raise RepodataException('Comps file empty')
+
+    # Test updateinfo
+    ret = subprocess.call(['zgrep', '<id/>', repo_info['updateinfo']])
+    if not ret:
+        raise RepodataException('updateinfo.xml.gz contains empty ID tags')
 
 
 def age(context, date, nuke_ago=False):

bodhi/tests/server/test_metadata.py

@@ -47,7 +47,7 @@ def setUp(self):
         self.tempdir = tempfile.mkdtemp('bodhi')
         self.tempcompdir = join(self.tempdir, 'f17-updates-testing')
         self.temprepo = join(self.tempcompdir, 'compose', 'Everything', 'i386', 'os')
-        mkmetadatadir(join(self.temprepo, 'f17-updates-testing', 'i386'))
+        mkmetadatadir(join(self.temprepo, 'f17-updates-testing', 'i386'), updateinfo=False)
         config['cache_dir'] = os.path.join(self.tempdir, 'cache')
         os.makedirs(config['cache_dir'])
 
@@ -241,8 +241,9 @@ def setUp(self):
         os.makedirs(os.path.join(config['mash_dir'], 'f17-updates-testing'))
 
         # Initialize our temporary repo
-        mkmetadatadir(self.temprepo)
-        mkmetadatadir(join(self.tempcompdir, 'compose', 'Everything', 'source', 'tree'))
+        mkmetadatadir(self.temprepo, updateinfo=False)
+        mkmetadatadir(join(self.tempcompdir, 'compose', 'Everything', 'source', 'tree'),
+                      updateinfo=False)
         self.repodata = join(self.temprepo, 'repodata')
         assert exists(join(self.repodata, 'repomd.xml'))
 
@@ -313,8 +314,9 @@ def test_extended_metadata_cache(self):
         """
         self._test_extended_metadata(True)
         shutil.rmtree(self.temprepo)
-        mkmetadatadir(self.temprepo)
-        mkmetadatadir(join(self.tempcompdir, 'compose', 'Everything', 'source', 'tree'))
+        mkmetadatadir(self.temprepo, updateinfo=False)
+        mkmetadatadir(join(self.tempcompdir, 'compose', 'Everything', 'source', 'tree'),
+                      updateinfo=False)
         DevBuildsys.__rpms__ = []
         self._test_extended_metadata(True)
 

bodhi/tests/server/test_util.py

@@ -16,7 +16,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-import gzip
 import json
 import os
 import shutil
@@ -530,27 +529,48 @@ def setUp(self):
     def tearDown(self):
         shutil.rmtree(self.tempdir)
 
+    def test_correct_repo(self):
+        """No Exception should be raised if the repo is normal."""
+        util.mkmetadatadir(self.tempdir)
+
+        # No exception should be raised here.
+        util.sanity_check_repodata(self.tempdir)
+
     def test_updateinfo_empty_tags(self):
         """RepodataException should be raised if <id/> is found in updateinfo."""
-        util.mkmetadatadir(self.tempdir)
-        updateinfo = os.path.join(self.tempdir, 'updateinfo.xml.gz')
-        with gzip.GzipFile(updateinfo, 'w') as uinfo:
-            uinfo.write('<id/>'.encode('utf-8'))
+        updateinfo = os.path.join(self.tempdir, 'updateinfo.xml')
+        with open(updateinfo, 'w') as uinfo:
+            uinfo.write('<id/>')
+        util.mkmetadatadir(self.tempdir, updateinfo=updateinfo)
 
         with self.assertRaises(util.RepodataException) as exc:
             util.sanity_check_repodata(self.tempdir)
 
         self.assertEqual(str(exc.exception), 'updateinfo.xml.gz contains empty ID tags')
 
-    def test_updateinfo_nonempty_tags(self):
-        """No Exception should be raised if <id/> is found in updateinfo."""
-        util.mkmetadatadir(self.tempdir)
-        updateinfo = os.path.join(self.tempdir, 'updateinfo.xml.gz')
-        with gzip.GzipFile(updateinfo, 'w') as uinfo:
-            uinfo.write('<id>some id</id>'.encode('utf-8'))
+    def test_comps_invalid_notxml(self):
+        """RepodataException should be raised if comps is invalid."""
+        comps = os.path.join(self.tempdir, 'comps.xml')
+        with open(comps, 'w') as uinfo:
+            uinfo.write('this is not even xml')
+        util.mkmetadatadir(self.tempdir, comps=comps)
 
-        # No exception should be raised here.
-        util.sanity_check_repodata(self.tempdir)
+        with self.assertRaises(util.RepodataException) as exc:
+            util.sanity_check_repodata(self.tempdir)
+
+        self.assertEqual(str(exc.exception), 'Comps file unable to be parsed')
+
+    def test_comps_invalid_nonsense(self):
+        """RepodataException should be raised if comps is invalid."""
+        comps = os.path.join(self.tempdir, 'comps.xml')
+        with open(comps, 'w') as uinfo:
+            uinfo.write('<whatever />')
+        util.mkmetadatadir(self.tempdir, comps=comps)
+
+        with self.assertRaises(util.RepodataException) as exc:
+            util.sanity_check_repodata(self.tempdir)
+
+        self.assertEqual(str(exc.exception), 'Comps file empty')
 
 
 class TestUpdate2HTML(base.BaseTestCase):

devel/ci/f26-packages

@@ -6,7 +6,9 @@
     python-pyramid-fas-openid \
     python-pytest \
     python-simplemediawiki \
-    python-webtest
+    python-webtest \
+    python2-hawkey \
+    python2-libcomps
 
 RUN pip-2 install cornice\<3.2.0
 RUN pip-2 install -e git+https://github.com/Cornices/cornice.ext.sphinx.git@master#egg=cornice_sphinx

devel/ci/f27-packages

@@ -6,7 +6,9 @@
     python-pyramid-fas-openid \
     python-pytest \
     python-simplemediawiki \
-    python-webtest
+    python-webtest \
+    python2-hawkey \
+    python2-libcomps
 
 RUN pip-2 install cornice\<3.2.0
 RUN pip-2 install -e git+https://github.com/Cornices/cornice.ext.sphinx.git@master#egg=cornice_sphinx

devel/ci/pip-packages

@@ -5,7 +5,9 @@
     python2-devel \
     python3-devel \
     python3-simplemediawiki \
-    redhat-rpm-config
+    redhat-rpm-config \
+    python2-hawkey \
+    python2-libcomps
 
 COPY requirements.txt /bodhi/requirements.txt
 

devel/ci/rawhide-packages

@@ -10,4 +10,6 @@
     python3-cornice \
     python3-cornice-sphinx \
     python2-webtest \
-    python3-pyramid-fas-openid
+    python3-pyramid-fas-openid \
+    python2-hawkey \
+    python2-libcomps