Replace hard-coded paths with path constants

Several run() calls used hard-coded paths rather than pre-defined paths
from ipaplatform.paths. The patch fixes all places that I was able to
find with a simple search.

The fix simplifies Darix's port of freeIPA on openSuSE.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>

client/ipa-client-automount

@@ -92,7 +92,7 @@ def wait_for_sssd():
     time.sleep(1)
     while n < 10 and not found:
         try:
-            ipautil.run(["getent", "passwd", "admin@%s" % api.env.realm])
+            ipautil.run([paths.GETENT, "passwd", "admin@%s" % api.env.realm])
             found = True
         except Exception:
             time.sleep(1)

install/tools/ipa-adtrust-install

@@ -110,7 +110,7 @@ def read_admin_password(admin_name):
 
 def ensure_admin_kinit(admin_name, admin_password):
     try:
-        ipautil.run(['kinit', admin_name], stdin=admin_password+'\n')
+        ipautil.run([paths.KINIT, admin_name], stdin=admin_password+'\n')
     except ipautil.CalledProcessError:
         print("There was error to automatically re-kinit your admin user "
               "ticket.")

install/tools/ipa-ca-install

@@ -341,7 +341,7 @@ def main():
         install(safe_options, options, filename)
 
     # execute ipactl to refresh services status
-    ipautil.run(['ipactl', 'start', '--ignore-service-failures'],
+    ipautil.run([paths.IPACTL, 'start', '--ignore-service-failures'],
                 raiseonerr=False)
 
     api.Backend.ldap2.disconnect()

install/tools/ipa-dns-install

@@ -150,7 +150,7 @@ def main():
     dns_installer.install(True, False, options)
 
     # execute ipactl to refresh services status
-    ipautil.run(['ipactl', 'start', '--ignore-service-failures'],
+    ipautil.run([paths.IPACTL, 'start', '--ignore-service-failures'],
                 raiseonerr=False)
 
     api.Backend.ldap2.disconnect()

ipaclient/install/client.py

@@ -1086,7 +1086,7 @@ def configure_sshd_config(fstore, options):
         )
 
         for candidate in candidates:
-            args = ['sshd', '-t', '-f', os.devnull]
+            args = [paths.SSHD, '-t', '-f', os.devnull]
             for item in candidate.items():
                 args.append('-o')
                 args.append('%s=%s' % item)
@@ -1118,7 +1118,7 @@ def configure_automount(options):
     logger.info('\nConfiguring automount:')
 
     args = [
-        'ipa-client-automount', '--debug', '-U', '--location',
+        paths.IPA_CLIENT_AUTOMOUNT, '--debug', '-U', '--location',
         options.location
     ]
 
@@ -2576,7 +2576,7 @@ def _install(options):
                 subject_base = DN(subject_base)
 
             if options.principal is not None:
-                run(["kdestroy"], raiseonerr=False, env=env)
+                run([paths.KDESTROY], raiseonerr=False, env=env)
 
             # Obtain the TGT. We do it with the temporary krb5.conf, so that
             # only the KDC we're installing under is contacted.
@@ -2911,7 +2911,7 @@ def _install(options):
             # Particulary, SSSD might take longer than 6-8 seconds.
             while n < 10 and not found:
                 try:
-                    ipautil.run(["getent", "passwd", user])
+                    ipautil.run([paths.GETENT, "passwd", user])
                     found = True
                 except Exception as e:
                     time.sleep(1)
@@ -2993,7 +2993,7 @@ def uninstall(options):
     statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE)
 
     try:
-        run(["ipa-client-automount", "--uninstall", "--debug"])
+        run([paths.IPA_CLIENT_AUTOMOUNT, "--uninstall", "--debug"])
     except Exception as e:
         logger.error(
             "Unconfigured automount client failed: %s", str(e))

ipaplatform/base/paths.py

@@ -25,6 +25,8 @@ This base platform module exports default filesystem paths.
 class BasePathNamespace(object):
     BASH = "/bin/bash"
     BIN_HOSTNAMECTL = "/bin/hostnamectl"
+    ECHO = "/bin/echo"
+    GZIP = "/usr/bin/gzip"
     LS = "/bin/ls"
     SH = "/bin/sh"
     SYSTEMCTL = "/bin/systemctl"
@@ -159,8 +161,10 @@ class BasePathNamespace(object):
     GPG = "/usr/bin/gpg"
     GPG_AGENT = "/usr/bin/gpg-agent"
     IPA_GETCERT = "/usr/bin/ipa-getcert"
+    KADMIN_LOCAL = '/usr/sbin/kadmin.local'
     KDESTROY = "/usr/bin/kdestroy"
     KINIT = "/usr/bin/kinit"
+    KLIST = "/usr/bin/klist"
     BIN_KVNO = "/usr/bin/kvno"
     LDAPMODIFY = "/usr/bin/ldapmodify"
     LDAPPASSWD = "/usr/bin/ldappasswd"
@@ -206,6 +210,7 @@ class BasePathNamespace(object):
     GROUPADD = "/usr/sbin/groupadd"
     USERMOD = "/usr/sbin/usermod"
     HTTPD = "/usr/sbin/httpd"
+    IPA_CLIENT_AUTOMOUNT = "/usr/sbin/ipa-client-automount"
     IPA_CLIENT_INSTALL = "/usr/sbin/ipa-client-install"
     IPA_DNS_INSTALL = "/usr/sbin/ipa-dns-install"
     SBIN_IPA_JOIN = "/usr/sbin/ipa-join"
@@ -360,6 +365,9 @@ class BasePathNamespace(object):
     IF_INET6 = '/proc/net/if_inet6'
     AUTHCONFIG = None
     IPA_SERVER_UPGRADE = '/usr/sbin/ipa-server-upgrade'
+    KEYCTL = '/usr/bin/keyctl'
+    GETENT = '/usr/bin/getent'
+    SSHD = '/usr/sbin/sshd'
 
 
 paths = BasePathNamespace()

ipapython/kernel_keyring.py

@@ -21,6 +21,7 @@ import os
 import six
 
 from ipapython.ipautil import run
+from ipaplatform.paths import paths
 
 # NOTE: Absolute path not required for keyctl since we reset the environment
 #       in ipautil.run.
@@ -33,34 +34,38 @@ from ipapython.ipautil import run
 KEYRING = '@s'
 KEYTYPE = 'user'
 
+
 def dump_keys():
     """
     Dump all keys
     """
-    result = run(['keyctl', 'list', KEYRING], raiseonerr=False,
+    result = run([paths.KEYCTL, 'list', KEYRING], raiseonerr=False,
                  capture_output=True)
     return result.output
 
+
 def get_real_key(key):
     """
     One cannot request a key based on the description it was created with
     so find the one we're looking for.
     """
     assert isinstance(key, six.string_types)
-    result = run(['keyctl', 'search', KEYRING, KEYTYPE, key],
+    result = run([paths.KEYCTL, 'search', KEYRING, KEYTYPE, key],
                  raiseonerr=False, capture_output=True)
     if result.returncode:
         raise ValueError('key %s not found' % key)
     return result.raw_output.rstrip()
 
+
 def get_persistent_key(key):
     assert isinstance(key, six.string_types)
-    result = run(['keyctl', 'get_persistent', KEYRING, key],
+    result = run([paths.KEYCTL, 'get_persistent', KEYRING, key],
                  raiseonerr=False, capture_output=True)
     if result.returncode:
         raise ValueError('persistent key %s not found' % key)
     return result.raw_output.rstrip()
 
+
 def is_persistent_keyring_supported():
     uid = os.geteuid()
     try:
@@ -70,6 +75,7 @@ def is_persistent_keyring_supported():
 
     return True
 
+
 def has_key(key):
     """
     Returns True/False whether the key exists in the keyring.
@@ -81,6 +87,7 @@ def has_key(key):
     except ValueError:
         return False
 
+
 def read_key(key):
     """
     Read the keyring and return the value for key.
@@ -89,13 +96,14 @@ def read_key(key):
     """
     assert isinstance(key, six.string_types)
     real_key = get_real_key(key)
-    result = run(['keyctl', 'pipe', real_key], raiseonerr=False,
+    result = run([paths.KEYCTL, 'pipe', real_key], raiseonerr=False,
                  capture_output=True)
     if result.returncode:
         raise ValueError('keyctl pipe failed: %s' % result.error_log)
 
     return result.raw_output
 
+
 def update_key(key, value):
     """
     Update the keyring data. If they key doesn't exist it is created.
@@ -104,13 +112,14 @@ def update_key(key, value):
     assert isinstance(value, bytes)
     if has_key(key):
         real_key = get_real_key(key)
-        result = run(['keyctl', 'pupdate', real_key], stdin=value,
+        result = run([paths.KEYCTL, 'pupdate', real_key], stdin=value,
                      raiseonerr=False)
         if result.returncode:
             raise ValueError('keyctl pupdate failed: %s' % result.error_log)
     else:
         add_key(key, value)
 
+
 def add_key(key, value):
     """
     Add a key to the kernel keyring.
@@ -119,18 +128,19 @@ def add_key(key, value):
     assert isinstance(value, bytes)
     if has_key(key):
         raise ValueError('key %s already exists' % key)
-    result = run(['keyctl', 'padd', KEYTYPE, key, KEYRING],
+    result = run([paths.KEYCTL, 'padd', KEYTYPE, key, KEYRING],
                  stdin=value, raiseonerr=False)
     if result.returncode:
         raise ValueError('keyctl padd failed: %s' % result.error_log)
 
+
 def del_key(key):
     """
     Remove a key from the keyring
     """
     assert isinstance(key, six.string_types)
     real_key = get_real_key(key)
-    result = run(['keyctl', 'unlink', real_key, KEYRING],
+    result = run([paths.KEYCTL, 'unlink', real_key, KEYRING],
                  raiseonerr=False)
     if result.returncode:
         raise ValueError('keyctl unlink failed: %s' % result.error_log)

ipaserver/install/adtrustinstance.py

@@ -547,8 +547,10 @@ class ADTRUSTInstance(service.Service):
     def clean_samba_keytab(self):
         if os.path.exists(self.keytab):
             try:
-                ipautil.run(["ipa-rmkeytab", "--principal", self.principal,
+                ipautil.run([
-                             "-k", self.keytab])
+                    paths.IPA_RMKEYTAB, "--principal", self.principal,
+                    "-k", self.keytab
+                ])
             except ipautil.CalledProcessError as e:
                 if e.returncode != 5:
                     logger.critical("Failed to remove old key for %s",

ipaserver/install/installutils.py

@@ -599,19 +599,26 @@ def get_directive(filename, directive, separator=' '):
     fd.close()
     return None
 
+
 def kadmin(command):
-    return ipautil.run(["kadmin.local", "-q", command,
+    return ipautil.run(
-                        "-x", "ipa-setup-override-restrictions"],
+        [
+            paths.KADMIN_LOCAL, "-q", command,
+            "-x", "ipa-setup-override-restrictions"
+        ],
         capture_output=True,
-                       capture_error=True)
+        capture_error=True
+    )
 
 
 def kadmin_addprinc(principal):
     return kadmin("addprinc -randkey " + principal)
 
+
 def kadmin_modprinc(principal, options):
     return kadmin("modprinc " + options + " " + principal)
 
+
 def create_keytab(path, principal):
     try:
         if os.path.isfile(path):
@@ -832,7 +839,7 @@ def expand_replica_info(filename, password):
     tarfile = top_dir+"/files.tar"
     dir_path = top_dir + "/realm_info"
     decrypt_file(filename, tarfile, password, top_dir)
-    ipautil.run(["tar", "xf", tarfile, "-C", top_dir])
+    ipautil.run([paths.TAR, "xf", tarfile, "-C", top_dir])
     os.remove(tarfile)
 
     return top_dir, dir_path

ipaserver/install/ipa_backup.py

@@ -310,7 +310,7 @@ class Backup(admintool.AdminTool):
                     dirsrv.stop(capture_output=False)
             else:
                 logger.info('Stopping IPA services')
-                run(['ipactl', 'stop'])
+                run([paths.IPACTL, 'stop'])
 
             instance = installutils.realm_to_serverid(api.env.realm)
             if os.path.exists(paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE %
@@ -333,7 +333,7 @@ class Backup(admintool.AdminTool):
                     dirsrv.start(capture_output=False)
             else:
                 logger.info('Starting IPA service')
-                run(['ipactl', 'start'])
+                run([paths.IPACTL, 'start'])
 
         finally:
             try:
@@ -535,7 +535,7 @@ class Backup(admintool.AdminTool):
 
         # Compress the archive. This is done separately, since 'tar' cannot
         # append to a compressed archive.
-        result = run(['gzip', tarfile], raiseonerr=False)
+        result = run([paths.GZIP, tarfile], raiseonerr=False)
         if result.returncode != 0:
             raise admintool.ScriptError(
                 'gzip returned non-zero code %d '

ipaserver/install/ipa_restore.py

@@ -379,7 +379,7 @@ class Restore(admintool.AdminTool):
                     dirsrv.start(capture_output=False)
             else:
                 logger.info('Stopping IPA services')
-                result = run(['ipactl', 'stop'], raiseonerr=False)
+                result = run([paths.IPACTL, 'stop'], raiseonerr=False)
                 if result.returncode not in [0, 6]:
                     logger.warning('Stopping IPA failed: %s', result.error_log)
 
@@ -419,7 +419,7 @@ class Restore(admintool.AdminTool):
                 gssproxy = services.service('gssproxy', api)
                 gssproxy.reload_or_restart()
                 logger.info('Starting IPA services')
-                run(['ipactl', 'start'])
+                run([paths.IPACTL, 'start'])
                 logger.info('Restarting SSSD')
                 sssd = services.service('sssd', api)
                 sssd.restart()

ipaserver/install/krbinstance.py

@@ -345,7 +345,7 @@ class KrbInstance(service.Service):
         MIN_KRB5KDC_WITH_WORKERS = "1.9"
         cpus = os.sysconf('SC_NPROCESSORS_ONLN')
         workers = False
-        result = ipautil.run(['klist', '-V'],
+        result = ipautil.run([paths.KLIST, '-V'],
                              raiseonerr=False, capture_output=True)
         if result.returncode == 0:
             verstr = result.output.split()[-1]

ipatests/pytest_plugins/integration/__init__.py

@@ -31,6 +31,7 @@ import pytest
 from pytest_multihost import make_multihost_fixture
 
 from ipapython import ipautil
+from ipaplatform.paths import paths
 from ipatests.test_util import yield_fixture
 from .config import Config
 from .env_config import get_global_config
@@ -150,7 +151,7 @@ def collect_logs(name, logs_dict, logfile_dir=None, beakerlib_plugin=None):
             # delete from remote
             host.run_command(['rm', '-f', tmpname])
             # Unpack on the local side
-            ipautil.run(['tar', 'xJvf', 'logs.tar.xz'], cwd=dirname,
+            ipautil.run([paths.TAR, 'xJvf', 'logs.tar.xz'], cwd=dirname,
                         raiseonerr=False)
             os.unlink(tarname)
 

ipatests/test_integration/test_caless.py

@@ -335,7 +335,7 @@ class CALessBase(IntegrationTest):
                 with open(cert_fname) as cert:
                     chain.write(cert.read())
 
-        ipautil.run(["openssl", "pkcs12", "-export", "-out", filename,
+        ipautil.run([paths.OPENSSL, "pkcs12", "-export", "-out", filename,
                      "-inkey", key_fname, "-in", certchain_fname, "-passin",
                      "pass:" + cls.cert_password, "-passout", "pass:" +
                      password, "-name", nickname], cwd=cls.cert_dir)

ipatests/test_ipapython/test_ipautil.py

@@ -26,6 +26,7 @@ import pytest
 import six
 import tempfile
 
+from ipaplatform.paths import paths
 from ipapython import ipautil
 
 pytestmark = pytest.mark.tier0
@@ -419,7 +420,7 @@ class TestTimeParser(object):
 
 
 def test_run():
-    result = ipautil.run(['echo', 'foo\x02bar'],
+    result = ipautil.run([paths.ECHO, 'foo\x02bar'],
                          capture_output=True,
                          capture_error=True)
     assert result.returncode == 0
@@ -430,7 +431,7 @@ def test_run():
 
 
 def test_run_no_capture_output():
-    result = ipautil.run(['echo', 'foo\x02bar'])
+    result = ipautil.run([paths.ECHO, 'foo\x02bar'])
     assert result.returncode == 0
     assert result.output is None
     assert result.raw_output == b'foo\x02bar\n'
@@ -439,13 +440,13 @@ def test_run_no_capture_output():
 
 
 def test_run_bytes():
-    result = ipautil.run(['echo', b'\x01\x02'], capture_output=True)
+    result = ipautil.run([paths.ECHO, b'\x01\x02'], capture_output=True)
     assert result.returncode == 0
     assert result.raw_output == b'\x01\x02\n'
 
 
 def test_run_decode():
-    result = ipautil.run(['echo', u'á'.encode('utf-8')],
+    result = ipautil.run([paths.ECHO, u'á'.encode('utf-8')],
                          encoding='utf-8', capture_output=True)
     assert result.returncode == 0
     if six.PY3:
@@ -457,11 +458,11 @@ def test_run_decode():
 def test_run_decode_bad():
     if six.PY3:
         with pytest.raises(UnicodeDecodeError):
-            ipautil.run(['echo', b'\xa0\xa1'],
+            ipautil.run([paths.ECHO, b'\xa0\xa1'],
                         capture_output=True,
                         encoding='utf-8')
     else:
-        result = ipautil.run(['echo', '\xa0\xa1'],
+        result = ipautil.run([paths.ECHO, '\xa0\xa1'],
                              capture_output=True,
                              encoding='utf-8')
         assert result.returncode == 0
@@ -469,7 +470,7 @@ def test_run_decode_bad():
 
 
 def test_backcompat():
-    result = out, err, rc = ipautil.run(['echo', 'foo\x02bar'],
+    result = out, err, rc = ipautil.run([paths.ECHO, 'foo\x02bar'],
                                         capture_output=True,
                                         capture_error=True)
     assert rc is result.returncode

ipatests/test_xmlrpc/test_caacl_profile_enforcement.py

@@ -16,6 +16,7 @@ from cryptography.hazmat.primitives import hashes, serialization
 from cryptography.hazmat.primitives.asymmetric import rsa
 
 from ipalib import api, errors
+from ipaplatform.paths import paths
 from ipatests.util import (
     prepare_config, unlock_principal_password, change_principal,
     host_keytab)
@@ -48,7 +49,7 @@ def generate_user_csr(username, domain=None):
         username=username)
 
     with tempfile.NamedTemporaryFile(mode='w') as csr_file:
-        run(['openssl', 'req', '-new', '-key', CERT_RSA_PRIVATE_KEY_PATH,
+        run([paths.OPENSSL, 'req', '-new', '-key', CERT_RSA_PRIVATE_KEY_PATH,
              '-out', csr_file.name,
              '-config', prepare_config(
                  CERT_OPENSSL_CONFIG_TEMPLATE, csr_values)])

ipatests/test_xmlrpc/test_cert_plugin.py

@@ -176,7 +176,7 @@ class test_cert(BaseCert):
         result = api.Command.cert_show(sn, out=unicode(self.certfile))
         with open(self.certfile, "rb") as f:
             pem_cert = f.read().decode('ascii')
-        result = run(['openssl', 'x509', '-text'],
+        result = run([paths.OPENSSL, 'x509', '-text'],
                      stdin=pem_cert, capture_output=True)
         assert _EXP_CRL_URI in result.output
         assert _EXP_OCSP_URI in result.output