Refactor ipautil.run

The ipautil.run function now returns an object with returncode and
output are accessible as attributes.

The stdout and stderr of all commands are logged (unless skip_output is given).

The stdout/stderr contents must be explicitly requested with a keyword
argument, otherwise they are None.
This is because in Python 3, the output needs to be decoded, and that can
fail if it's not decodable (human-readable) text.

The raw (bytes) output is always available from the result object,
as is "leniently" decoded output suitable for logging.

All calls are changed to reflect this.

A use of Popen in cainstance is changed to ipautil.run.

Reviewed-By: Jan Cholasta <jcholast@redhat.com>

1 files changed, 37 insertions, 21 deletions

diff --git a/ipapython/kernel_keyring.py b/ipapython/kernel_keyring.py
index d30531cab..7ba916ccb 100644
--- a/ipapython/kernel_keyring.py
+++ b/ipapython/kernel_keyring.py
@@ -36,24 +36,29 @@ def dump_keys():
     """
     Dump all keys
     """
-    (stdout, stderr, rc) = run(['keyctl', 'list', KEYRING], raiseonerr=False)
-    return stdout
+    result = run(['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.
     """
-    (stdout, stderr, rc) = run(['keyctl', 'search', KEYRING, KEYTYPE, key], raiseonerr=False)
-    if rc:
+    assert isinstance(key, str)
+    result = run(['keyctl', 'search', KEYRING, KEYTYPE, key],
+                 raiseonerr=False, capture_output=True)
+    if result.returncode:
         raise ValueError('key %s not found' % key)
-    return stdout.rstrip()
+    return result.output.rstrip()
 
 def get_persistent_key(key):
-    (stdout, stderr, rc) = run(['keyctl', 'get_persistent', KEYRING, key], raiseonerr=False)
-    if rc:
+    assert isinstance(key, str)
+    result = run(['keyctl', 'get_persistent', KEYRING, key],
+                 raiseonerr=False, capture_output=True)
+    if result.returncode:
         raise ValueError('persistent key %s not found' % key)
-    return stdout.rstrip()
+    return result.output.rstrip()
 
 def is_persistent_keyring_supported():
     uid = os.geteuid()
@@ -68,6 +73,7 @@ def has_key(key):
     """
     Returns True/False whether the key exists in the keyring.
     """
+    assert isinstance(key, str)
     try:
         get_real_key(key)
         return True
@@ -80,22 +86,27 @@ def read_key(key):
 
     Use pipe instead of print here to ensure we always get the raw data.
     """
+    assert isinstance(key, str)
     real_key = get_real_key(key)
-    (stdout, stderr, rc) = run(['keyctl', 'pipe', real_key], raiseonerr=False)
-    if rc:
-        raise ValueError('keyctl pipe failed: %s' % stderr)
+    result = run(['keyctl', 'pipe', real_key], raiseonerr=False,
+                 capture_output=True)
+    if result.returncode:
+        raise ValueError('keyctl pipe failed: %s' % result.error_log)
 
-    return stdout
+    return result.output
 
 def update_key(key, value):
     """
     Update the keyring data. If they key doesn't exist it is created.
     """
+    assert isinstance(key, str)
+    assert isinstance(value, bytes)
     if has_key(key):
         real_key = get_real_key(key)
-        (stdout, stderr, rc) = run(['keyctl', 'pupdate', real_key], stdin=value, raiseonerr=False)
-        if rc:
-            raise ValueError('keyctl pupdate failed: %s' % stderr)
+        result = run(['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)
 
@@ -103,17 +114,22 @@ def add_key(key, value):
     """
     Add a key to the kernel keyring.
     """
+    assert isinstance(key, str)
+    assert isinstance(value, bytes)
     if has_key(key):
         raise ValueError('key %s already exists' % key)
-    (stdout, stderr, rc) = run(['keyctl', 'padd', KEYTYPE, key, KEYRING], stdin=value, raiseonerr=False)
-    if rc:
-        raise ValueError('keyctl padd failed: %s' % stderr)
+    result = run(['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, str)
     real_key = get_real_key(key)
-    (stdout, stderr, rc) = run(['keyctl', 'unlink', real_key, KEYRING], raiseonerr=False)
-    if rc:
-        raise ValueError('keyctl unlink failed: %s' % stderr)
+    result = run(['keyctl', 'unlink', real_key, KEYRING],
+                 raiseonerr=False)
+    if result.returncode:
+        raise ValueError('keyctl unlink failed: %s' % result.error_log)