Repository: cobbler/cobbler (language: python)

Issue:
Crashing `cobbler reposync` on `main` branch
### Describe the bug

`cobbler reposync` crashes due to the following error message:

```
Exception value: [Errno 2] No such file or directory: '/usr/bin/dnf reposync --newest-only --delete --refresh --remote-time ..'
```

### Steps to reproduce

1. `cobbler reposync`
2. See error

### Expected behavior

`cobbler reposync` finished successfully.

### Cobbler version

````paste below
Not relevant
````

### Operating system

Idenpedent of the OS.

### Cobbler log

````paste below
Not available
````

### Screenshots

None

### Additional information

None



Discussion / hints:

Fix patch:
diff --git a/cobbler/actions/reposync.py b/cobbler/actions/reposync.py
index c09b2f0c61..3e5bd63d3c 100644
--- a/cobbler/actions/reposync.py
+++ b/cobbler/actions/reposync.py
@@ -81,7 +81,7 @@ def __init__(self, api, tries: int = 1, nofail: bool = False):
         self.api = api
         self.settings = api.settings()
         self.repos = api.repos()
-        self.rflags = self.settings.reposync_flags
+        self.rflags = self.settings.reposync_flags.split()
         self.tries = tries
         self.nofail = nofail
         self.logger = logging.getLogger()
@@ -271,9 +271,9 @@ def createrepo_walker(self, repo, dirname: str, fnames):
                             )
 
             blended = utils.blender(self.api, False, repo)
-            flags = blended.get("createrepo_flags", "(ERROR: FLAGS)")
+            flags = blended.get("createrepo_flags", "(ERROR: FLAGS)").split()
             try:
-                cmd = ["createrepo"] + mdoptions + [flags, pipes.quote(dirname)]
+                cmd = ["createrepo"] + mdoptions + flags + [pipes.quote(dirname)]
                 utils.subprocess_call(cmd, shell=False)
             except Exception:
                 utils.log_exc()
@@ -349,27 +349,24 @@ def rsync_sync(self, repo):
 
         dest_path = os.path.join(self.settings.webdir, "repo_mirror", repo.name)
 
-        spacer = ""
+        spacer = []
         if not repo.mirror.startswith("rsync://") and not repo.mirror.startswith("/"):
-            spacer = "-e ssh"
+            spacer = ["-e ssh"]
         if not repo.mirror.endswith("/"):
             repo.mirror = f"{repo.mirror}/"
 
-        flags = ""
+        flags = []
         for repo_option in repo.rsyncopts:
             if repo.rsyncopts[repo_option]:
-                flags += f" {repo_option} {repo.rsyncopts[repo_option]}"
+                flags.append(f"{repo_option} {repo.rsyncopts[repo_option]}")
             else:
-                flags += f" {repo_option}"
+                flags.append(f"{repo_option}")
 
-        if flags == "":
-            flags = self.settings.reposync_rsync_flags
+        if not flags:
+            flags = self.settings.reposync_rsync_flags.split()
 
-        cmd = [
-            "rsync",
-            flags,
-            "--delete-after",
-            spacer,
+        cmd = ["rsync"] + flags + ["--delete-after"]
+        cmd += spacer + [
             "--delete",
             "--exclude-from=/etc/cobbler/rsync.exclude",
             pipes.quote(repo.mirror),
@@ -385,9 +382,8 @@ def rsync_sync(self, repo):
         if "--archive" in flags:
             archive = True
         else:
-            # split flags and skip all --{options} as we need to look for combined flags like -vaH
-            flag = flags.split()
-            for option in flag:
+            # skip all flags --{options} as we need to look for combined flags like -vaH
+            for option in flags:
                 if option.startswith("--"):
                     pass
                 else:
@@ -401,7 +397,8 @@ def rsync_sync(self, repo):
 
     # ====================================================================================
 
-    def reposync_cmd(self) -> str:
+    @staticmethod
+    def reposync_cmd() -> list:
 
         """
         Determine reposync command
@@ -413,9 +410,9 @@ def reposync_cmd(self) -> str:
             raise CX("no librepo found, please install python3-librepo")
 
         if os.path.exists("/usr/bin/dnf"):
-            cmd = "/usr/bin/dnf reposync"
+            cmd = ["/usr/bin/dnf", "reposync"]
         elif os.path.exists("/usr/bin/reposync"):
-            cmd = "/usr/bin/reposync"
+            cmd = ["/usr/bin/reposync"]
         else:
             # Warn about not having yum-utils.  We don't want to require it in the package because Fedora 22+ has moved
             # to dnf.
@@ -470,11 +467,10 @@ def rhn_sync(self, repo):
             # Counter-intuitive, but we want the newish kernels too
             arch = "i686"
 
-        cmd = [
-            self.reposync_cmd(),
-            self.rflags,
+        cmd = self.reposync_cmd()
+        cmd += self.rflags + [
             f"--repo={pipes.quote(rest)}",
-            f"-p={pipes.quote(repos_path)}",
+            f"--download-path={pipes.quote(repos_path)}",
         ]
         if arch != "none":
             cmd.append(f'--arch="{arch}"')
@@ -568,12 +564,13 @@ def yum_sync(self, repo):
 
         if not has_rpm_list:
             # If we have not requested only certain RPMs, use reposync
-            cmd = (
-                f"{cmd} {self.rflags} --config={temp_file} --repoid={pipes.quote(repo.name)}"
-                f" -p {pipes.quote(repos_path)}"
-            )
+            cmd += self.rflags + [
+                f"--config={temp_file}",
+                f"--repoid={pipes.quote(repo.name)}",
+                f"--download-path={pipes.quote(repos_path)}",
+            ]
             if arch != "none":
-                cmd = f"{cmd} -a {arch}"
+                cmd.append(f"--arch={arch}")
 
         else:
             # Create the output directory if it doesn't exist
@@ -581,12 +578,11 @@ def yum_sync(self, repo):
                 os.makedirs(dest_path)
 
             # Older yumdownloader sometimes explodes on --resolvedeps if this happens to you, upgrade yum & yum-utils
-            extra_flags = self.settings.yumdownloader_flags
+            extra_flags = self.settings.yumdownloader_flags.split()
             cmd = [
                 "/usr/bin/dnf",
                 "download",
-                extra_flags,
-            ]
+            ] + extra_flags
             if arch == "src":
                 cmd.append("--source")
             cmd += [
@@ -594,8 +590,8 @@ def yum_sync(self, repo):
                 f"--enablerepo={pipes.quote(repo.name)}",
                 f"-c={temp_file}",
                 f"--destdir={pipes.quote(dest_path)}",
-                " ".join(repo.rpm_list),
             ]
+            cmd += repo.rpm_list
 
         # Now regardless of whether we're doing yumdownloader or reposync or whether the repo was http://, ftp://, or
         # rhn://, execute all queued commands here.  Any failure at any point stops the operation.
diff --git a/config/cobbler/settings.yaml b/config/cobbler/settings.yaml
index 2816e3f1a8..a3eecd3b25 100644
--- a/config/cobbler/settings.yaml
+++ b/config/cobbler/settings.yaml
@@ -159,9 +159,8 @@ cheetah_import_whitelist:
  - "netaddr"
 
 # Default "createrepo_flags" to use for new repositories. If you have createrepo >= 0.4.10, consider
-# "-c cache --update -C", which can dramatically improve your "cobbler reposync" time. "-s sha" enables working with
-# Fedora repos from F11/F12 from EL-4 or EL-5 without python-hashlib installed (which is not available on EL-4)
-createrepo_flags: "-c cache -s sha"
+# "--cachedir=cache --update", which can dramatically improve your "cobbler reposync" time.
+createrepo_flags: "--cachedir=cache --update"
 
 # if no autoinstall template is specified to profile add, use this template (path is relative to template root)
 autoinstall: "default.ks"
@@ -421,7 +420,7 @@ redhat_management_key: ""
 register_new_installs: false
 
 # Flags to use for dnf's reposync. You can exclude some packages by adding --exclude.
-# For example exclude source packages: --exclude=\\*.src
+# For example exclude source packages: --exclude=*.src
 reposync_flags: "--newest-only --delete --refresh --remote-time"
 
 # Flags to use for rysync's reposync. If flag 'a' is used then createrepo
diff --git a/docs/cobbler-conf/settings-yaml.rst b/docs/cobbler-conf/settings-yaml.rst
index 256e6e2cbe..c2c1a60051 100644
--- a/docs/cobbler-conf/settings-yaml.rst
+++ b/docs/cobbler-conf/settings-yaml.rst
@@ -281,7 +281,7 @@ createrepo_flags
 
 Default ``createrepo_flags`` to use for new repositories.
 
-default: ``"-c cache -s sha"``
+default: ``"--cachedir=cache --update"``
 
 default_name_*
 ##############
@@ -767,10 +767,10 @@ default: ``"-avzH"``
 reposync_flags
 ##############
 
-Flags to use for yum's reposync. If your version of yum reposync does not support ``-l``, you may need to remove that
-option.
+Flags to use for yum's reposync. If your version of yum reposync does not support some options, you may need to remove
+that options.
 
-default: ``"-l -n -d"``
+default: ``"--newest-only --delete --refresh --remote-time"``
 
 reposync_rsync_flags
 ####################


Test patch:
diff --git a/system-tests/tests/settings-cli-migrate b/system-tests/tests/settings-cli-migrate
index 1b52e5d81e..56b6e840b0 100755
--- a/system-tests/tests/settings-cli-migrate
+++ b/system-tests/tests/settings-cli-migrate
@@ -124,7 +124,7 @@ cheetah_import_whitelist:
  - "re"
  - "time"
  - "netaddr"
-createrepo_flags: "-c cache -s sha"
+createrepo_flags: "--cachedir=cache --update"
 autoinstall: "default.ks"
 default_name_servers: []
 default_name_servers_search: []
@@ -251,6 +251,7 @@ cheetah_import_whitelist:
 - re
 - time
 - netaddr
+createrepo_flags: --cachedir=cache --update
 default_password_crypted: /UHC.
 grub2_mod_dir: '@@grub_mod_folder@@'
 ldap_base_dn: DC=example,DC=com
diff --git a/system-tests/tests/svc-settings b/system-tests/tests/svc-settings
index fa14c1be0c..a382b49aa5 100755
--- a/system-tests/tests/svc-settings
+++ b/system-tests/tests/svc-settings
@@ -51,7 +51,7 @@ cat >${tmp}/a <<-EOF
     "client_use_localhost": false,
     "cobbler_master": "",
     "convert_server_to_ip": false,
-    "createrepo_flags": "-c cache -s sha",
+    "createrepo_flags": "--cachedir=cache --update",
     "autoinstall": "default.ks",
     "default_name_servers": [],
     "default_name_servers_search": [],
diff --git a/tests/actions/reposync_test.py b/tests/actions/reposync_test.py
index 1591957c35..f3a00a6e38 100644
--- a/tests/actions/reposync_test.py
+++ b/tests/actions/reposync_test.py
@@ -80,8 +80,8 @@ def test_fun(arg, top, names):
 @pytest.mark.parametrize(
     "input_has_librepo,input_path_exists_side_effect,expected_exception,expected_result",
     [
-        (True, [True, False], does_not_raise(), "/usr/bin/dnf reposync"),
-        (True, [False, True], does_not_raise(), "/usr/bin/reposync"),
+        (True, [True, False], does_not_raise(), ["/usr/bin/dnf", "reposync"]),
+        (True, [False, True], does_not_raise(), ["/usr/bin/reposync"]),
         (True, [False, False], pytest.raises(cexceptions.CX), ""),
         (False, [False, True], pytest.raises(cexceptions.CX), ""),
     ],
@@ -191,7 +191,7 @@ def test_reposync_yum(
         reposync_object, "create_local_file", return_value="/create/local/file"
     )
     mocker.patch.object(
-        reposync_object, "reposync_cmd", return_value="/my/fake/dnf reposync"
+        reposync_object, "reposync_cmd", return_value=["/my/fake/dnf", "reposync"]
     )
     mocker.patch.object(reposync_object, "rflags", return_value="--fake-r-flakg")
     mocker.patch.object(
@@ -396,7 +396,7 @@ def test_reposync_rhn(mocker, reposync_object, repo):
     mocker.patch("cobbler.actions.reposync.repo_walker")
     mocker.patch.object(reposync_object, "create_local_file")
     mocker.patch.object(
-        reposync_object, "reposync_cmd", return_value="/my/fake/reposync"
+        reposync_object, "reposync_cmd", return_value=["/my/fake/reposync"]
     )
 
     # Act
@@ -409,7 +409,7 @@ def test_reposync_rhn(mocker, reposync_object, repo):
             "/my/fake/reposync",
             "--testflag",
             "--repo=testrepo0",
-            "-p=/srv/www/cobbler/repo_mirror",
+            "--download-path=/srv/www/cobbler/repo_mirror",
         ],
         shell=False,
     )
diff --git a/tests/test_data/V3_4_0/settings.yaml b/tests/test_data/V3_4_0/settings.yaml
index 105d8748aa..6a5673b40f 100644
--- a/tests/test_data/V3_4_0/settings.yaml
+++ b/tests/test_data/V3_4_0/settings.yaml
@@ -159,9 +159,8 @@ cheetah_import_whitelist:
  - "netaddr"
 
 # Default "createrepo_flags" to use for new repositories. If you have createrepo >= 0.4.10, consider
-# "-c cache --update -C", which can dramatically improve your "cobbler reposync" time. "-s sha" enables working with
-# Fedora repos from F11/F12 from EL-4 or EL-5 without python-hashlib installed (which is not available on EL-4)
-createrepo_flags: "-c cache -s sha"
+# "--cachedir=cache --update", which can dramatically improve your "cobbler reposync" time.
+createrepo_flags: "--cachedir=cache --update"
 
 # if no autoinstall template is specified to profile add, use this template (path is relative to template root)
 autoinstall: "default.ks"
@@ -421,7 +420,7 @@ redhat_management_key: ""
 register_new_installs: false
 
 # Flags to use for dnf's reposync. You can exclude some packages by adding --exclude.
-# For example exclude source packages: --exclude=\\*.src
+# For example exclude source packages: --exclude=*.src
 reposync_flags: "--newest-only --delete --refresh --remote-time"
 
 # Flags to use for rysync's reposync. If flag 'a' is used then createrepo