From 635db52b4dd8672009ef031974896ee6a93fb913 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 5 Jan 2017 17:30:07 +0100 Subject: [PATCH] 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 | 18 +++----- doc/source/usage.rst | 41 ------------------- example.conf | 8 ++-- ironic_inspector/conf.py | 7 ++-- ironic_inspector/main.py | 11 ++++- ironic_inspector/test/unit/test_main.py | 9 ++++ ...e-setting-ipmi-creds-1581ddc63b273811.yaml | 12 ++++++ 7 files changed, 45 insertions(+), 61 deletions(-) create mode 100644 releasenotes/notes/deprecate-setting-ipmi-creds-1581ddc63b273811.yaml diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index c95ce7c5f..6759ef2d8 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -11,20 +11,14 @@ Start Introspection ``POST /v1/introspection/`` initiate hardware introspection for node ````. 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 diff --git a/doc/source/usage.rst b/doc/source/usage.rst index 2cb38fb93..f64560c7b 100644 --- a/doc/source/usage.rst +++ b/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 -i ipmi_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 diff --git a/example.conf b/example.conf index 3de199078..f764a8fa7 100644 --- a/example.conf +++ b/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 diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index 30e15765e..d15b1b624 100644 --- a/ironic_inspector/conf.py +++ b/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', diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index fe3906458..750a5286a 100644 --- a/ironic_inspector/main.py +++ b/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')) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index 21df05b67..79e870b2b 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/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?' diff --git a/releasenotes/notes/deprecate-setting-ipmi-creds-1581ddc63b273811.yaml b/releasenotes/notes/deprecate-setting-ipmi-creds-1581ddc63b273811.yaml new file mode 100644 index 000000000..ab7e658ba --- /dev/null +++ b/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.