openstack/ironic
Code Issues Proposed changes

Move deploy_utils warnings to conductor start

Browse Source 
make these checks once during conductor start after config was parsed.
This will prevent issuing those warnings on module imports.

Without it sphinx-build is broken when warnings are treated as failures.

Change-Id: I455bcc45edae82632aaf4eaebdcd597440d33ad3
Closes-Bug: #1694724
This commit is contained in:
Pavlo Shchelokovskyy 2017-06-01 09:35:28 +03:00
parent 82e8a40b93
commit 892467dc9b
4 changed files with 83 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
ironic/cmd/conductor.py
View File

@ -40,7 +40,7 @@ SECTIONS_WITH_AUTH = (
# TODO(pas-ha) remove this check after deprecation period
def _check_auth_options(conf):
def check_auth_options(conf):
missing = []
for section in SECTIONS_WITH_AUTH:
if not auth.load_auth(conf, section):
@ -59,6 +59,30 @@ def _check_auth_options(conf):
link=link))
def warn_about_unsafe_shred_parameters(conf):
iterations = conf.deploy.shred_random_overwrite_iterations
overwrite_with_zeros = conf.deploy.shred_final_overwrite_with_zeros
if iterations == 0 and overwrite_with_zeros is False:
LOG.warning('With shred_random_overwrite_iterations set to 0 and '
'shred_final_overwrite_with_zeros set to False, disks '
'may NOT be shredded at all, unless they support ATA '
'Secure Erase. This is a possible SECURITY ISSUE!')
def warn_about_missing_default_boot_option(conf):
if not conf.deploy.default_boot_option:
LOG.warning('The default value of default_boot_option '
'configuration will change eventually from '
'"netboot" to "local". It is recommended to set '
'an explicit value for it during the transition period')
def issue_startup_warnings(conf):
check_auth_options(conf)
warn_about_unsafe_shred_parameters(conf)
warn_about_missing_default_boot_option(conf)
def main():
# NOTE(lucasagomes): Safeguard to prevent 'ironic.conductor.manager'
# from being imported prior to the configuration options being loaded.
@ -77,7 +101,7 @@ def main():
'ironic.conductor.manager',
'ConductorManager')
_check_auth_options(CONF)
issue_startup_warnings(CONF)
launcher = service.launch(CONF, mgr)
launcher.wait()

21
ironic/drivers/modules/deploy_utils.py
View File

@ -68,27 +68,6 @@ SUPPORTED_CAPABILITIES = {
DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb')
def warn_about_unsafe_shred_parameters():
iterations = CONF.deploy.shred_random_overwrite_iterations
overwrite_with_zeros = CONF.deploy.shred_final_overwrite_with_zeros
if iterations == 0 and overwrite_with_zeros is False:
LOG.warning('With shred_random_overwrite_iterations set to 0 and '
'shred_final_overwrite_with_zeros set to False, disks '
'may NOT be shredded at all, unless they support ATA '
'Secure Erase. This is a possible SECURITY ISSUE!')
def warn_about_missing_default_boot_option():
if not CONF.deploy.default_boot_option:
LOG.warning('The default value of default_boot_option '
'configuration will change eventually from '
'"netboot" to "local". It is recommended to set '
'an explicit value for it during the transition period')
warn_about_unsafe_shred_parameters()
warn_about_missing_default_boot_option()
# All functions are called from deploy() directly or indirectly.
# They are split for stub-out.

57
ironic/tests/unit/cmd/test_conductor.py Normal file
View File

@ -0,0 +1,57 @@
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import mock
from oslo_config import cfg
from ironic.cmd import conductor
from ironic.tests.unit.db import base
class ConductorStartTestCase(base.DbTestCase):
@mock.patch.object(conductor, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_defaults(self, log_mock):
conductor.warn_about_unsafe_shred_parameters(cfg.CONF)
self.assertFalse(log_mock.warning.called)
@mock.patch.object(conductor, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_zeros(self, log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', True,
'deploy')
conductor.warn_about_unsafe_shred_parameters(cfg.CONF)
self.assertFalse(log_mock.warning.called)
@mock.patch.object(conductor, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_random_no_zeros(self,
log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 1, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', False,
'deploy')
conductor.warn_about_unsafe_shred_parameters(cfg.CONF)
self.assertFalse(log_mock.warning.called)
@mock.patch.object(conductor, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_produces_a_warning(self,
log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', False,
'deploy')
conductor.warn_about_unsafe_shred_parameters(cfg.CONF)
self.assertTrue(log_mock.warning.called)
@mock.patch.object(conductor, 'LOG', autospec=True)
def test_warn_on_missing_default_boot_option(self, log_mock):
conductor.warn_about_missing_default_boot_option(cfg.CONF)
self.assertTrue(log_mock.warning.called)

31
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -1177,37 +1177,6 @@ class OtherFunctionTestCase(db_base.DbTestCase):
mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
[('uuid', 'path')])
@mock.patch.object(utils, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_defaults(self, log_mock):
utils.warn_about_unsafe_shred_parameters()
self.assertFalse(log_mock.warning.called)
@mock.patch.object(utils, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_zeros(self, log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', True,
'deploy')
utils.warn_about_unsafe_shred_parameters()
self.assertFalse(log_mock.warning.called)
@mock.patch.object(utils, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_random_no_zeros(self,
log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 1, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', False,
'deploy')
utils.warn_about_unsafe_shred_parameters()
self.assertFalse(log_mock.warning.called)
@mock.patch.object(utils, 'LOG', autospec=True)
def test_warn_about_unsafe_shred_parameters_produces_a_warning(self,
log_mock):
cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy')
cfg.CONF.set_override('shred_final_overwrite_with_zeros', False,
'deploy')
utils.warn_about_unsafe_shred_parameters()
self.assertTrue(log_mock.warning.called)
@mock.patch.object(utils, '_get_ironic_session')
@mock.patch('ironic.common.keystone.get_service_url')
def test_get_ironic_api_url_from_config(self, mock_get_url, mock_ks):