Merge "Flexible IPMI credential persistence method configuration"

doc/source/admin/drivers/ipmitool.rst

@@ -82,6 +82,28 @@ with an IPMItool-based driver. For example::
         --driver-info ipmi_username=<username> \
         --driver-info ipmi_password=<password>
 
+
+Changing The Default IPMI Credential Persistence Method
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- ``store_cred_in_env``: :oslo.config:option:`ipmi.store_cred_in_env`.
+
+The `store_cred_in_env` configuration option allow users to switch
+between file-based and environment variable persistence methods for
+IPMI password.
+
+For the temporary file option, long lived IPMI sessions, such as those for
+console support, leave files with credentials on the conductor disk for the
+duration of the session.
+
+To switch to environment variable persistence, set the
+``store_cred_in_env`` parameter to ``True`` in the configuration file:
+
+.. code-block:: ini
+
+  [ipmi]
+  store_cred_in_env = True
+
 Advanced configuration
 ======================
 

ironic/conf/ipmi.py

@@ -75,6 +75,11 @@ opts = [
                        'additional debugging output. This is a separate '
                        'option as ipmitool can log a substantial amount '
                        'of misleading text when in this mode.')),
+    cfg.BoolOpt('store_cred_in_env',
+                default=False,
+                help=_('Boolean flag to determine IPMI password persistence '
+                       'method. Defaults to False (file-based persistence). '
+                       )),
     cfg.ListOpt('cipher_suite_versions',
                 default=[],
                 help=_('List of possible cipher suites versions that can '

ironic/drivers/modules/ipmitool.py

@@ -71,9 +71,9 @@ REQUIRED_PROPERTIES = {
 }
 OPTIONAL_PROPERTIES = {
     'ipmi_password': _("password. Optional."),
-    'ipmi_hex_kg_key': _('Kg key for IPMIv2 authentication. '
+    'ipmi_hex_kg_key': _("Kg key for IPMIv2 authentication. "
-                         'The key is expected in hexadecimal format. '
+                         "The key is expected in hexadecimal format. "
-                         'Optional.'),
+                         "Optional."),
     'ipmi_port': _("remote IPMI RMCP port. Optional."),
     'ipmi_priv_level': _("privilege level; default is ADMINISTRATOR. One of "
                          "%s. Optional.") % ', '.join(VALID_PRIV_LEVELS),
@@ -280,18 +280,23 @@ def _console_pwfile_path(uuid):
 
 
 @contextlib.contextmanager
-def _make_password_file(password):
+def _prepare_ipmi_password(driver_info):
-    """Makes a temporary file that contains the password.
+    """Prepares the IPMI password by either setting it in the environment
 
-    :param password: the password
+    or creating a temporary file.
-    :returns: the absolute pathname of the temporary file
+
+    :param driver_info: the ipmitool parameters for accessing a node.
+    :returns: the absolute pathname of the password
     :raises: PasswordFileFailedToCreate from creating or writing to the
              temporary file
     """
+    if CONF.ipmi.store_cred_in_env:
+        yield _persist_ipmi_password(driver_info)
+    else:
         f = None
         try:
             f = tempfile.NamedTemporaryFile(mode='w', dir=CONF.tempdir)
-        f.write(str(password))
+            f.write(str(driver_info['password'] or '\0'))
             f.flush()
         except (IOError, OSError) as exc:
             if f is not None:
@@ -303,11 +308,11 @@ def _make_password_file(password):
                     f.close()
 
         try:
-        # NOTE(jlvillal): This yield can not be in the try/except block above
+            # NOTE(jlvillal): This yield can not be in the try/except block
-        # because an exception by the caller of this function would then get
+            # above because an exception by the caller of this function would
-        # changed to a PasswordFileFailedToCreate exception which would mislead
+            # then get changed to a PasswordFileFailedToCreate exception which
-        # about the problem and its cause.
+            # would mislead about the problem and its cause.
-        yield f.name
+            yield ('-f', f.name)
         finally:
             if f is not None:
                 f.close()
@@ -495,6 +500,25 @@ def _get_ipmitool_args(driver_info, pw_file=None):
     return args
 
 
+def _persist_ipmi_password(driver_info):
+    """Persists IPMI password for passing to the ipmitool
+
+    :param driver_info: driver info with the ipmitool parameters
+    """
+    env = {}
+    if CONF.ipmi.store_cred_in_env:
+        password = driver_info.get('password')
+        if password:
+            env = {'IPMI_PASSWORD': password}
+        return '-E', env
+
+    path = _console_pwfile_path(driver_info['uuid'])
+    pw_file = console_utils.make_persistent_password_file(
+        path, driver_info['password'] or '\0')
+
+    return '-f', pw_file
+
+
 def _ipmitool_timing_args():
     if not _is_option_supported('timing'):
         return []
@@ -644,10 +668,15 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
         # 'ipmitool' command will prompt password if there is no '-f'
         # option, we set it to '\0' to write a password file to support
         # empty password
-        with _make_password_file(driver_info['password'] or '\0') as pw_file:
+
-            cmd_args.append('-f')
+        with _prepare_ipmi_password(driver_info) as (flag, env_path):
-            cmd_args.append(pw_file)
+            cmd_args.append(flag)
+            if CONF.ipmi.store_cred_in_env:
+                extra_args['env_variables'] = env_path
+            else:
+                cmd_args.append(env_path)
             cmd_args.extend(command.split(" "))
+
             try:
                 out, err = utils.execute(*cmd_args, **extra_args)
                 return out, err
@@ -679,6 +708,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
                                     'Error: %(error)s',
                                     {'node': driver_info['uuid'],
                                      'cmd': e.cmd, 'error': e})
+
             finally:
                 LAST_CMD_TIME[driver_info['address']] = time.time()
 
@@ -1543,17 +1573,17 @@ class IPMIConsole(base.ConsoleInterface):
                  created
         :raises: ConsoleSubprocessFailed when invoking the subprocess failed
         """
-        path = _console_pwfile_path(driver_info['uuid'])
-        pw_file = console_utils.make_persistent_password_file(
-            path, driver_info['password'] or '\0')
-        ipmi_cmd = self._get_ipmi_cmd(driver_info, pw_file)
-        ipmi_cmd += ' sol activate'
-
         try:
-            start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd)
+            cmd = self._get_ipmi_cmd(driver_info, pw_file=None)
-        except (exception.ConsoleError, exception.ConsoleSubprocessFailed):
+            cmd += ' sol activate'
-            with excutils.save_and_reraise_exception():
+            start_method(driver_info['uuid'], driver_info['port'], cmd)
-                ironic_utils.unlink_without_raise(path)
+        except (exception.ConsoleError,
+                exception.ConsoleSubprocessFailed) as e:
+            LOG.exception('IPMI Error while attempting "%(cmd)s" '
+                          'for node %(node)s. Error: %(error)s',
+                          {'node': driver_info['uuid'],
+                           'cmd': cmd, 'error': e})
+            raise
 
 
 class IPMIShellinaboxConsole(IPMIConsole):

releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Adds a new configuration option ``store_cred_in_env`` to allow
+    switching between file-based and environment variable persistence for
+    IPMI credentials. Defaults to ``False``.