Fix DNF related issues with source selection

DNF Repo.dump() function cannot be used as a .repo file for dnf due to
it writing baseurl and gpgkey as a list instead of a string. Add a new
function to write this in the correct format, and limited to the fields
we use.

Add a test for the new function.

Fix /projects/source/info to return an error 400 if a nonexistant TOML
source is requested. If JSON is used the error is part of the standard
response.

Update test_server.py to check for the correct error code.

src/pylorax/api/projects.py

@@ -322,6 +322,39 @@ def modules_info(dbo, module_names):
 
     return modules
 
+def dnf_repo_to_file_repo(repo):
+    """Return a string representation of a DNF Repo object suitable for writing to a .repo file
+
+    :param repo: DNF Repository
+    :type repo: dnf.RepoDict
+    :returns: A string
+    :rtype: str
+
+    The DNF Repo.dump() function does not produce a string that can be used as a dnf .repo file,
+    it ouputs baseurl and gpgkey as python lists which DNF cannot read. So do this manually with
+    only the attributes we care about.
+    """
+    repo_str = "[%s]\n" % repo.id
+    if repo.metalink:
+        repo_str += "metalink = %s\n" % repo.metalink
+    elif repo.mirrorlist:
+        repo_str += "mirrorlist = %s\n" % repo.mirrorlist
+    elif repo.baseurl:
+        repo_str += "baseurl = %s\n" % repo.baseurl[0]
+    else:
+        raise RuntimeError("Repo has no baseurl, metalink, or mirrorlist")
+
+    # proxy is optional
+    if repo.proxy:
+        repo_str += "proxy = %s\n" % repo.proxy
+
+    repo_str += "sslverify = %s\n" % repo.sslverify
+    repo_str += "gpgcheck = %s\n" % repo.gpgcheck
+    if repo.gpgkey:
+        repo_str += "gpgkey = %s\n" % ",".join(repo.gpgkey)
+
+    return repo_str
+
 def repo_to_source(repo, system_source):
     """Return a Weldr Source dict created from the DNF Repository
 
@@ -410,7 +443,7 @@ def source_to_repo(source, dnf_conf):
     repo.skip_if_unavailable = False
 
     if source["type"] == "yum-baseurl":
-        repo.baseurl = [source["url"]]
+        repo.baseurl = source["url"]
     elif source["type"] == "yum-metalink":
         repo.metalink = source["url"]
     elif source["type"] == "yum-mirrorlist":
@@ -430,7 +463,7 @@ def source_to_repo(source, dnf_conf):
         repo.gpgcheck = False
 
     if "gpgkey_urls" in source:
-        repo.gpgkey = source["gpgkey_urls"]
+        repo.gpgkey = ",".join(source["gpgkey_urls"])
 
     repo.enable()
 

src/pylorax/api/v0.py

@@ -973,7 +973,7 @@ from pylorax.api.compose import start_build, compose_types
 from pylorax.api.crossdomain import crossdomain
 from pylorax.api.projects import projects_list, projects_info, projects_depsolve
 from pylorax.api.projects import modules_list, modules_info, ProjectsError, repo_to_source
-from pylorax.api.projects import get_repo_sources, delete_repo_source, source_to_repo
+from pylorax.api.projects import get_repo_sources, delete_repo_source, source_to_repo, dnf_repo_to_file_repo
 from pylorax.api.queue import queue_status, build_status, uuid_delete, uuid_status, uuid_info
 from pylorax.api.queue import uuid_tar, uuid_image, uuid_cancel, uuid_log
 from pylorax.api.recipes import list_branch_files, read_recipe_commit, recipe_filename, list_commits
@@ -1423,9 +1423,12 @@ def v0_api(api):
                 continue
             sources[repo.id] = repo_to_source(repo, repo.id in system_sources)
 
-        if out_fmt == "toml":
+        if out_fmt == "toml" and not errors:
             # With TOML output we just want to dump the raw sources, skipping the errors
             return toml.dumps(sources)
+        elif out_fmt == "toml" and errors:
+            # TOML requested, but there was an error
+            return jsonify(status=False, errors=errors), 400
         else:
             return jsonify(sources=sources, errors=errors)
 
@@ -1470,7 +1473,7 @@ def v0_api(api):
             # Make sure the source name can't contain a path traversal by taking the basename
             source_path = joinpaths(repo_dir, os.path.basename("%s.repo" % source["name"]))
             with open(source_path, "w") as f:
-                f.write(str(repo))
+                f.write(dnf_repo_to_file_repo(repo))
         except Exception as e:
             log.error("(v0_projects_source_add) adding %s failed: %s", source["name"], str(e))
 

tests/pylorax/source/bad-repo.toml

@@ -0,0 +1,6 @@
+name = "bad-repo-1"
+url = "file:///tmp/not-a-repo/"
+type = "yum-baseurl"
+check_ssl = true
+check_gpg = true
+gpgkey_urls = ["file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-$releasever-$basearch"]

tests/pylorax/test_projects.py

@@ -28,6 +28,7 @@ from pylorax.api.projects import api_time, api_changelog, pkg_to_project, pkg_to
 from pylorax.api.projects import proj_to_module, projects_list, projects_info, projects_depsolve
 from pylorax.api.projects import modules_list, modules_info, ProjectsError, dep_evra, dep_nevra
 from pylorax.api.projects import repo_to_source, get_repo_sources, delete_repo_source, source_to_repo
+from pylorax.api.projects import dnf_repo_to_file_repo
 from pylorax.api.dnfbase import get_base_object
 
 class Package(object):
@@ -234,6 +235,13 @@ def fakerepo_baseurl():
         "url": "https://fake-repo.base.url"
     }
 
+def fakerepo_baseurl_str():
+    return """[fake-repo-baseurl]
+baseurl = https://fake-repo.base.url
+sslverify = True
+gpgcheck = True
+"""
+
 class FakeSystemRepo():
     id = "fake-system-repo"
     baseurl = ["https://fake-repo.base.url"]
@@ -273,6 +281,13 @@ def fakerepo_metalink():
         "url": "https://fake-repo.metalink"
     }
 
+def fakerepo_metalink_str():
+    return """[fake-repo-metalink]
+metalink = https://fake-repo.metalink
+sslverify = True
+gpgcheck = True
+"""
+
 class FakeRepoMirrorlist():
     id = "fake-repo-mirrorlist"
     baseurl = []
@@ -293,6 +308,13 @@ def fakerepo_mirrorlist():
         "url": "https://fake-repo.mirrorlist"
     }
 
+def fakerepo_mirrorlist_str():
+    return """[fake-repo-mirrorlist]
+mirrorlist = https://fake-repo.mirrorlist
+sslverify = True
+gpgcheck = True
+"""
+
 class FakeRepoProxy():
     id = "fake-repo-proxy"
     baseurl = ["https://fake-repo.base.url"]
@@ -314,6 +336,14 @@ def fakerepo_proxy():
         "url": "https://fake-repo.base.url"
     }
 
+def fakerepo_proxy_str():
+    return """[fake-repo-proxy]
+baseurl = https://fake-repo.base.url
+proxy = https://fake-repo.proxy
+sslverify = True
+gpgcheck = True
+"""
+
 class FakeRepoGPGKey():
     id = "fake-repo-gpgkey"
     baseurl = ["https://fake-repo.base.url"]
@@ -337,6 +367,14 @@ def fakerepo_gpgkey():
         "url": "https://fake-repo.base.url"
     }
 
+def fakerepo_gpgkey_str():
+    return """[fake-repo-gpgkey]
+baseurl = https://fake-repo.base.url
+sslverify = True
+gpgcheck = True
+gpgkey = https://fake-repo.gpgkey
+"""
+
 class SourceTest(unittest.TestCase):
     @classmethod
     def setUpClass(self):
@@ -360,19 +398,19 @@ class SourceTest(unittest.TestCase):
 
     def test_repo_to_source_metalink(self):
         """Test a repo with a metalink"""
-        self.assertEqual(repo_to_source(FakeRepoMetalink, False), fakerepo_metalink())
+        self.assertEqual(repo_to_source(FakeRepoMetalink(), False), fakerepo_metalink())
 
     def test_repo_to_source_mirrorlist(self):
         """Test a repo with a mirrorlist"""
-        self.assertEqual(repo_to_source(FakeRepoMirrorlist, False), fakerepo_mirrorlist())
+        self.assertEqual(repo_to_source(FakeRepoMirrorlist(), False), fakerepo_mirrorlist())
 
     def test_repo_to_source_proxy(self):
         """Test a repo with a proxy"""
-        self.assertEqual(repo_to_source(FakeRepoProxy, False), fakerepo_proxy())
+        self.assertEqual(repo_to_source(FakeRepoProxy(), False), fakerepo_proxy())
 
     def test_repo_to_source_gpgkey(self):
         """Test a repo with a GPG key"""
-        self.assertEqual(repo_to_source(FakeRepoGPGKey, False), fakerepo_gpgkey())
+        self.assertEqual(repo_to_source(FakeRepoGPGKey(), False), fakerepo_gpgkey())
 
     def test_get_repo_sources(self):
         """Test getting a list of sources from a repo directory"""
@@ -427,3 +465,23 @@ class SourceTest(unittest.TestCase):
         """Test creating a dnf.Repo with a proxy"""
         repo = source_to_repo(fakerepo_gpgkey(), self.dbo.conf)
         self.assertEqual(repo.gpgkey, fakerepo_gpgkey()["gpgkey_urls"])
+
+    def test_drtfr_baseurl(self):
+        """Test creating a dnf .repo file from a baseurl Repo object"""
+        self.assertEqual(dnf_repo_to_file_repo(FakeRepoBaseUrl()), fakerepo_baseurl_str())
+
+    def test_drtfr_metalink(self):
+        """Test creating a dnf .repo file from a metalink Repo object"""
+        self.assertEqual(dnf_repo_to_file_repo(FakeRepoMetalink()), fakerepo_metalink_str())
+
+    def test_drtfr_mirrorlist(self):
+        """Test creating a dnf .repo file from a mirrorlist Repo object"""
+        self.assertEqual(dnf_repo_to_file_repo(FakeRepoMirrorlist()), fakerepo_mirrorlist_str())
+
+    def test_drtfr_proxy(self):
+        """Test creating a dnf .repo file from a baseurl Repo object with proxy"""
+        self.assertEqual(dnf_repo_to_file_repo(FakeRepoProxy()), fakerepo_proxy_str())
+
+    def test_drtfr_gpgkey(self):
+        """Test creating a dnf .repo file from a baseurl Repo object with gpgkey"""
+        self.assertEqual(dnf_repo_to_file_repo(FakeRepoGPGKey()), fakerepo_gpgkey_str())

tests/pylorax/test_server.py

@@ -565,6 +565,7 @@ class ServerTestCase(unittest.TestCase):
         resp = self.server.post("/api/v0/projects/source/new",
                                 data=toml_source,
                                 content_type="text/x-toml")
+        self.assertEqual(resp.status_code, 400)
         data = json.loads(resp.data)
         self.assertEqual(data["status"], False)
 
@@ -574,6 +575,7 @@ class ServerTestCase(unittest.TestCase):
             resp = self.server.delete("/api/v0/projects/source/delete/rawhide")
         else:
             resp = self.server.delete("/api/v0/projects/source/delete/fedora")
+        self.assertEqual(resp.status_code, 400)
         data = json.loads(resp.data)
         self.assertNotEqual(data, None)
         self.assertEqual(data["status"], False)
@@ -603,6 +605,7 @@ class ServerTestCase(unittest.TestCase):
     def test_projects_source_03_delete_unknown(self):
         """Test /api/v0/projects/source/delete an unknown source"""
         resp = self.server.delete("/api/v0/projects/source/delete/unknown-repo")
+        self.assertEqual(resp.status_code, 400)
         data = json.loads(resp.data)
         self.assertNotEqual(data, None)
         self.assertEqual(data["status"], False)