Deprecate setting IPMI credentials

This feature is dangerous, barely maintained and not covered by any CI.
As it was hidden behind a configuration option, we can remove it without
breaking our API contract too much. This change deprecates the option,
and create an API version with this feature already de-activated.

Change-Id: I9e05c36b8c1194f4eeeb80c1f811e808854974c4
Partial-Bug: #1654318

doc/source/http-api.rst

@@ -11,20 +11,14 @@ Start Introspection
 
 ``POST /v1/introspection/<Node ID>`` initiate hardware introspection for node
 ``<Node ID>``. All power management configuration for this node needs to be
-done prior to calling the endpoint (except when :ref:`setting-ipmi-creds`).
+done prior to calling the endpoint.
 
 Requires X-Auth-Token header with Keystone token for authentication.
 
-Optional parameters:
+Deprecated parameters (only available in API before version ``1.9``):
 
-* ``new_ipmi_password`` if set, **ironic-inspector** will try to set IPMI
-  password on the machine to this value. Power credentials validation will be
-  skipped and manual power on will be required. See :ref:`setting-ipmi-creds`
-  for details.
-
-* ``new_ipmi_username`` provides new IPMI user name in addition to password
-  set by ``new_ipmi_password``. Defaults to current ``ipmi_username`` in
-  node ``driver_info`` field.
+* ``new_ipmi_password``
+* ``new_ipmi_username``
 
 Response:
 
@@ -280,8 +274,8 @@ Response:
 * 403 - node is not on introspection
 * 404 - node cannot be found or multiple nodes found
 
-Response body: JSON dictionary. If :ref:`setting-ipmi-creds` is requested,
-body will contain the following keys:
+Response body: JSON dictionary. If setting IPMI credentials (deprecated
+feature) is requested, body will contain the following keys:
 
 * ``ipmi_setup_credentials`` boolean ``True``
 * ``ipmi_username`` new IPMI user name

doc/source/usage.rst

@@ -130,47 +130,6 @@ from introspection, it's using `python string formatting notation
     {"action": "set-attribute", "path": "/driver_info/ipmi_address",
      "value": "{data[inventory][bmc_address]}"}
 
-.. _setting-ipmi-creds:
-
-Setting IPMI Credentials
-~~~~~~~~~~~~~~~~~~~~~~~~
-
-If you have physical access to your nodes, you can use **ironic-inspector** to
-set IPMI credentials for them without knowing the original ones. The workflow
-is as follows:
-
-* Ensure nodes will PXE boot on the right network by default.
-
-* Set ``enable_setting_ipmi_credentials = true`` in the **ironic-inspector**
-  configuration file, restart **ironic-inspector**.
-
-* Enroll nodes in Ironic with setting their ``ipmi_address`` only (or
-  equivalent driver-specific property, as per ``ipmi_address_fields``
-  configuration option).
-
-  Use ironic API version ``1.11`` (introduced in ironic 4.0.0),
-  so that new node gets into ``enroll`` provision state::
-
-    ironic --ironic-api-version 1.11 node-create -d <DRIVER> -i ipmi_address=<ADDRESS>
-
-  Providing ``ipmi_address`` allows **ironic-inspector** to distinguish nodes.
-
-* Start introspection with providing additional parameters:
-
-  * ``new_ipmi_password`` IPMI password to set,
-  * ``new_ipmi_username`` IPMI user name to set, defaults to one in node
-    driver_info.
-
-* Manually power on the nodes and wait.
-
-* After introspection is finished (watch nodes power state or use
-  **ironic-inspector** status API) you can move node to ``manageable`` and
-  then ``available`` states - see `Node States`_.
-
-Note that due to various limitations on password value in different BMC,
-**ironic-inspector** will only accept passwords with length between 1 and 20
-consisting only of letters and numbers.
-
 .. _plugins:
 
 Plugins

example.conf

@@ -794,10 +794,12 @@
 # Deprecated group/name - [discoverd]/overwrite_existing
 #overwrite_existing = true
 
-# Whether to enable setting IPMI credentials during introspection.
-# This is an experimental and not well tested feature, use at your own
-# risk. (boolean value)
+# DEPRECATED: Whether to enable setting IPMI credentials during
+# introspection. This feature will be removed in the Pike release.
+# (boolean value)
 # Deprecated group/name - [discoverd]/enable_setting_ipmi_credentials
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
 #enable_setting_ipmi_credentials = false
 
 # Comma-separated list of default hooks for processing pipeline. Hook

ironic_inspector/conf.py

@@ -76,9 +76,10 @@ PROCESSING_OPTS = [
     cfg.BoolOpt('enable_setting_ipmi_credentials',
                 default=False,
                 help=_('Whether to enable setting IPMI credentials during '
-                       'introspection. This is an experimental and not well '
-                       'tested feature, use at your own risk.'),
-                deprecated_group='discoverd'),
+                       'introspection. This feature will be removed in the '
+                       'Pike release.'),
+                deprecated_group='discoverd',
+                deprecated_for_removal=True),
     cfg.StrOpt('default_processing_hooks',
                default='ramdisk_error,root_disk_selection,scheduler,'
                        'validate_interfaces,capabilities,pci_devices',

ironic_inspector/main.py

@@ -48,7 +48,10 @@ app = flask.Flask(__name__)
 LOG = utils.getProcessingLogger(__name__)
 
 MINIMUM_API_VERSION = (1, 0)
-CURRENT_API_VERSION = (1, 8)
+# TODO(dtantsur): set to the current version as soon we move setting IPMI
+# credentials support completely.
+DEFAULT_API_VERSION = (1, 8)
+CURRENT_API_VERSION = (1, 9)
 _LOGGING_EXCLUDED_KEYS = ('logs',)
 
 
@@ -67,7 +70,7 @@ def _format_version(ver):
     return '%d.%d' % ver
 
 
-_DEFAULT_API_VERSION = _format_version(CURRENT_API_VERSION)
+_DEFAULT_API_VERSION = _format_version(DEFAULT_API_VERSION)
 
 
 def error_response(exc, code=500):
@@ -218,6 +221,10 @@ def api_introspection(node_id):
         else:
             new_ipmi_credentials = None
 
+        if new_ipmi_credentials and _get_version() >= (1, 9):
+            return _('Setting IPMI credentials is deprecated and not allowed '
+                     'starting with API version 1.9'), 400
+
         introspect.introspect(node_id,
                               new_ipmi_credentials=new_ipmi_credentials,
                               token=flask.request.headers.get('X-Auth-Token'))

ironic_inspector/test/unit/test_main.py

@@ -70,6 +70,15 @@ class TestApiIntrospect(BaseAPITest):
             new_ipmi_credentials=('user', 'password'),
             token=None)
 
+    @mock.patch.object(introspect, 'introspect', autospec=True)
+    def test_introspect_set_ipmi_credentials_disabled(self, introspect_mock):
+        headers = {conf.VERSION_HEADER: '1.9'}
+        res = self.app.post('/v1/introspection/%s?new_ipmi_username=user&'
+                            'new_ipmi_password=password' % self.uuid,
+                            headers=headers)
+        self.assertEqual(400, res.status_code)
+        self.assertFalse(introspect_mock.called)
+
     @mock.patch.object(introspect, 'introspect', autospec=True)
     def test_introspect_set_ipmi_credentials_no_user(self, introspect_mock):
         res = self.app.post('/v1/introspection/%s?'

releasenotes/notes/deprecate-setting-ipmi-creds-1581ddc63b273811.yaml

@@ -0,0 +1,12 @@
+---
+deprecations:
+  - |
+    Support for setting IPMI credentials via ironic-inspector is deprecated
+    and will be removed completely in Pike. A new API version 1.9 was
+    introduced with this feature de-activated. For reasoning see
+    https://bugs.launchpad.net/ironic-python-agent/+bug/1654318.
+other:
+  - |
+    Default API version is temporary pinned to 1.8 (before deprecating setting
+    IPMI credentials). It will be reset to the latest version again when
+    support for setting IPMI credentials is removed.