From bdafb42156d95804b39e461cd5b8b85d57b2813b Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Wed, 3 Jan 2018 20:39:08 -0800 Subject: [PATCH] Refactors the bigip_sys_global module (#34438) Changes include the current f5 coding standards and unit tests for the module --- .../modules/network/f5/bigip_sys_global.py | 476 ++++++++++-------- .../f5/fixtures/load_sys_global_settings.json | 24 + .../network/f5/test_bigip_sys_global.py | 131 +++++ 3 files changed, 425 insertions(+), 206 deletions(-) create mode 100644 test/units/modules/network/f5/fixtures/load_sys_global_settings.json create mode 100644 test/units/modules/network/f5/test_bigip_sys_global.py diff --git a/lib/ansible/modules/network/f5/bigip_sys_global.py b/lib/ansible/modules/network/f5/bigip_sys_global.py index 6604b87532..d5fcae780d 100644 --- a/lib/ansible/modules/network/f5/bigip_sys_global.py +++ b/lib/ansible/modules/network/f5/bigip_sys_global.py @@ -31,45 +31,36 @@ options: description: - C(enable) or C(disabled) the Setup utility in the browser-based Configuration utility - choices: - - enabled - - disabled + choices: ['yes', 'no'] lcd_display: description: - Specifies, when C(enabled), that the system menu displays on the LCD screen on the front of the unit. This setting has no effect when used on the VE platform. - choices: - - enabled - - disabled + choices: ['yes', 'no'] mgmt_dhcp: description: - Specifies whether or not to enable DHCP client on the management interface - choices: - - enabled - - disabled + choices: ['yes', 'no'] net_reboot: description: - Specifies, when C(enabled), that the next time you reboot the system, the system boots to an ISO image on the network, rather than an internal media drive. - choices: - - enabled - - disabled + choices: ['yes', 'no'] quiet_boot: description: - Specifies, when C(enabled), that the system suppresses informational text on the console during the boot cycle. When C(disabled), the system presents messages and informational text on the console during the boot cycle. + choices: ['yes', 'no'] security_banner: description: - Specifies whether the system displays an advisory message on the login screen. - choices: - - enabled - - disabled + choices: ['yes', 'no'] state: description: - The state of the variable on the system. When C(present), guarantees @@ -148,17 +139,15 @@ security_banner: sample: enabled ''' -try: - from f5.bigip.contexts import TransactionContextManager - from f5.bigip import ManagementRoot -except ImportError: - pass # Handled via f5_utils.HAS_F5SDK - -from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.ec2 import camel_dict_to_snake_dict -from ansible.module_utils.f5_utils import F5ModuleError +from ansible.module_utils.f5_utils import AnsibleF5Client +from ansible.module_utils.f5_utils import AnsibleF5Parameters from ansible.module_utils.f5_utils import HAS_F5SDK -from ansible.module_utils.f5_utils import f5_argument_spec +from ansible.module_utils.f5_utils import F5ModuleError +from ansible.module_utils.parsing.convert_bool import BOOLEANS +from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE +from ansible.module_utils.parsing.convert_bool import BOOLEANS_FALSE +from ansible.module_utils.six import iteritems +from collections import defaultdict try: from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError @@ -166,193 +155,264 @@ except ImportError: HAS_F5SDK = False -class BigIpSysGlobalManager(object): - def __init__(self, *args, **kwargs): - self.changed_params = dict() - self.params = kwargs - self.api = None +class Parameters(AnsibleF5Parameters): + api_map = { + 'guiSecurityBanner': 'security_banner', + 'guiSecurityBannerText': 'banner_text', + 'guiSetup': 'gui_setup', + 'lcdDisplay': 'lcd_display', + 'mgmtDhcp': 'mgmt_dhcp', + 'netReboot': 'net_reboot', + 'quietBoot': 'quiet_boot', + 'consoleInactivityTimeout': 'console_timeout' + } - def apply_changes(self): - result = dict() + api_attributes = [ + 'guiSecurityBanner', 'guiSecurityBannerText', 'guiSetup', 'lcdDisplay', + 'mgmtDhcp', 'netReboot', 'quietBoot', 'consoleInactivityTimeout' + ] - changed = self.apply_to_running_config() + returnables = [ + 'security_banner', 'banner_text', 'gui_setup', 'lcd_display', + 'mgmt_dhcp', 'net_reboot', 'quiet_boot', 'console_timeout' + ] - result.update(**self.changed_params) - result.update(dict(changed=changed)) + updatables = [ + 'security_banner', 'banner_text', 'gui_setup', 'lcd_display', + 'mgmt_dhcp', 'net_reboot', 'quiet_boot', 'console_timeout' + ] + + def __init__(self, params=None): + self._values = defaultdict(lambda: None) + self._values['__warnings'] = [] + if params: + self.update(params=params) + + def update(self, params=None): + if params: + for k, v in iteritems(params): + if self.api_map is not None and k in self.api_map: + map_key = self.api_map[k] + else: + map_key = k + + # Handle weird API parameters like `dns.proxy.__iter__` by + # using a map provided by the module developer + class_attr = getattr(type(self), map_key, None) + if isinstance(class_attr, property): + # There is a mapped value for the api_map key + if class_attr.fset is None: + # If the mapped value does not have + # an associated setter + self._values[map_key] = v + else: + # The mapped value has a setter + setattr(self, map_key, v) + else: + # If the mapped value is not a @property + self._values[map_key] = v + + def api_params(self): + result = {} + for api_attribute in self.api_attributes: + if self.api_map is not None and api_attribute in self.api_map: + result[api_attribute] = getattr(self, self.api_map[api_attribute]) + else: + result[api_attribute] = getattr(self, api_attribute) + result = self._filter_params(result) return result - def apply_to_running_config(self): + +class ApiParameters(Parameters): + pass + + +class ModuleParameters(Parameters): + def _get_boolean_like_return_value(self, parameter): + if self._values[parameter] is None: + return None + elif self._values[parameter] in ['enabled', 'disabled']: + self._values['__warnings'].append( + dict(version='2.5', msg='enabled/disabled are deprecated. Use boolean values (true, yes, no, 1, 0) instead.') + ) + true = list(BOOLEANS_TRUE) + ['True'] + false = list(BOOLEANS_FALSE) + ['False'] + if self._values[parameter] in true: + return 'enabled' + if self._values[parameter] in false: + return 'disabled' + else: + return str(self._values[parameter]) + + @property + def security_banner(self): + result = self._get_boolean_like_return_value('security_banner') + return result + + @property + def gui_setup(self): + result = self._get_boolean_like_return_value('gui_setup') + return result + + @property + def banner_text(self): + result = self._get_boolean_like_return_value('banner_text') + return result + + @property + def lcd_display(self): + result = self._get_boolean_like_return_value('lcd_display') + return result + + @property + def mgmt_dhcp(self): + result = self._get_boolean_like_return_value('mgmt_dhcp') + return result + + @property + def net_reboot(self): + result = self._get_boolean_like_return_value('net_reboot') + return result + + @property + def quiet_boot(self): + result = self._get_boolean_like_return_value('quiet_boot') + return result + + +class Changes(Parameters): + def to_return(self): + result = {} try: - self.api = self.connect_to_bigip(**self.params) - return self.update_sys_global_settings() + for returnable in self.returnables: + result[returnable] = getattr(self, returnable) + result = self._filter_params(result) + except Exception: + pass + return result + + +class UsableChanges(Changes): + pass + + +class ReportableChanges(Changes): + pass + + +class Difference(object): + def __init__(self, want, have=None): + self.want = want + self.have = have + + def compare(self, param): + try: + result = getattr(self, param) + return result + except AttributeError: + return self.__default(param) + + def __default(self, param): + attr1 = getattr(self.want, param) + try: + attr2 = getattr(self.have, param) + if attr1 != attr2: + return attr1 + except AttributeError: + return attr1 + + +class ModuleManager(object): + def __init__(self, client): + self.client = client + self.want = ModuleParameters(params=self.client.module.params) + self.have = ApiParameters() + self.changes = UsableChanges() + + def _set_changed_options(self): + changed = {} + for key in Parameters.returnables: + if getattr(self.want, key) is not None: + changed[key] = getattr(self.want, key) + if changed: + self.changes = UsableChanges(changed) + + def _update_changed_options(self): + diff = Difference(self.want, self.have) + updatables = Parameters.updatables + changed = dict() + for k in updatables: + change = diff.compare(k) + if change is None: + continue + else: + if isinstance(change, dict): + changed.update(change) + else: + changed[k] = change + if changed: + self.changes = UsableChanges(changed) + return True + return False + + def should_update(self): + result = self._update_changed_options() + if result: + return True + return False + + def exec_module(self): + result = dict() + + try: + changed = self.present() except iControlUnexpectedHTTPError as e: raise F5ModuleError(str(e)) - def connect_to_bigip(self, **kwargs): - return ManagementRoot(kwargs['server'], - kwargs['user'], - kwargs['password'], - port=kwargs['server_port']) - - def read_sys_global_information(self): - settings = self.load_sys_global() - return self.format_sys_global_information(settings) - - def load_sys_global(self): - return self.api.tm.sys.global_settings.load() - - def get_changed_parameters(self): - result = dict() - current = self.read_sys_global_information() - if self.security_banner_is_changed(current): - result['guiSecurityBanner'] = self.params['security_banner'] - if self.banner_text_is_changed(current): - result['guiSecurityBannerText'] = self.params['banner_text'] - if self.gui_setup_is_changed(current): - result['guiSetup'] = self.params['gui_setup'] - if self.lcd_display_is_changed(current): - result['lcdDisplay'] = self.params['lcd_display'] - if self.mgmt_dhcp_is_changed(current): - result['mgmtDhcp'] = self.params['mgmt_dhcp'] - if self.net_reboot_is_changed(current): - result['netReboot'] = self.params['net_reboot'] - if self.quiet_boot_is_changed(current): - result['quietBoot'] = self.params['quiet_boot'] - if self.console_timeout_is_changed(current): - result['consoleInactivityTimeout'] = self.params['console_timeout'] + reportable = ReportableChanges(self.changes.to_return()) + changes = reportable.to_return() + result.update(**changes) + result.update(dict(changed=changed)) + self._announce_deprecations(result) return result - def security_banner_is_changed(self, current): - if self.params['security_banner'] is None: - return False - if 'security_banner' not in current: - return True - if self.params['security_banner'] == current['security_banner']: - return False - else: - return True + def _announce_deprecations(self, result): + warnings = result.pop('__warnings', []) + for warning in warnings: + self.client.module.deprecate( + msg=warning['msg'], + version=warning['version'] + ) - def banner_text_is_changed(self, current): - if self.params['banner_text'] is None: - return False - if 'banner_text' not in current: - return True - if self.params['banner_text'] == current['banner_text']: - return False - else: - return True + def present(self): + return self.update() - def gui_setup_is_changed(self, current): - if self.params['gui_setup'] is None: - return False - if 'gui_setup' not in current: - return True - if self.params['gui_setup'] == current['gui_setup']: - return False - else: - return True + def read_current_from_device(self): + resource = self.client.api.tm.sys.global_settings.load() + result = resource.attrs + return ApiParameters(result) - def lcd_display_is_changed(self, current): - if self.params['lcd_display'] is None: + def update(self): + self.have = self.read_current_from_device() + if not self.should_update(): return False - if 'lcd_display' not in current: + if self.client.check_mode: return True - if self.params['lcd_display'] == current['lcd_display']: - return False - else: - return True - - def mgmt_dhcp_is_changed(self, current): - if self.params['mgmt_dhcp'] is None: - return False - if 'mgmt_dhcp' not in current: - return True - if self.params['mgmt_dhcp'] == current['mgmt_dhcp']: - return False - else: - return True - - def net_reboot_is_changed(self, current): - if self.params['net_reboot'] is None: - return False - if 'net_reboot' not in current: - return True - if self.params['net_reboot'] == current['net_reboot']: - return False - else: - return True - - def quiet_boot_is_changed(self, current): - if self.params['quiet_boot'] is None: - return False - if 'quiet_boot' not in current: - return True - if self.params['quiet_boot'] == current['quiet_boot']: - return False - else: - return True - - def console_timeout_is_changed(self, current): - if self.params['console_timeout'] is None: - return False - if 'console_timeout' not in current: - return True - if self.params['console_timeout'] == current['console_timeout']: - return False - else: - return True - - def format_sys_global_information(self, settings): - result = dict() - if hasattr(settings, 'guiSecurityBanner'): - result['security_banner'] = str(settings.guiSecurityBanner) - if hasattr(settings, 'guiSecurityBannerText'): - result['banner_text'] = str(settings.guiSecurityBannerText) - if hasattr(settings, 'guiSetup'): - result['gui_setup'] = str(settings.guiSetup) - if hasattr(settings, 'lcdDisplay'): - result['lcd_display'] = str(settings.lcdDisplay) - if hasattr(settings, 'mgmtDhcp'): - result['mgmt_dhcp'] = str(settings.mgmtDhcp) - if hasattr(settings, 'netReboot'): - result['net_reboot'] = str(settings.netReboot) - if hasattr(settings, 'quietBoot'): - result['quiet_boot'] = str(settings.quietBoot) - if hasattr(settings, 'consoleInactivityTimeout'): - result['console_timeout'] = int(settings.consoleInactivityTimeout) - return result - - def update_sys_global_settings(self): - params = self.get_changed_parameters() - if params: - self.changed_params = camel_dict_to_snake_dict(params) - if self.params['check_mode']: - return True - else: - return False - self.update_sys_global_settings_on_device(params) + self.update_on_device() return True - def update_sys_global_settings_on_device(self, params): - tx = self.api.tm.transactions.transaction - with TransactionContextManager(tx) as api: - r = api.tm.sys.global_settings.load() - r.update(**params) + def update_on_device(self): + params = self.want.api_params() + resource = self.client.api.tm.sys.global_settings.load() + resource.modify(**params) -class BigIpSysGlobalModuleConfig(object): +class ArgumentSpec(object): def __init__(self): - self.argument_spec = dict() - self.meta_args = dict() self.supports_check_mode = True self.states = ['present'] - self.on_off_choices = ['enabled', 'disabled'] - - self.initialize_meta_args() - self.initialize_argument_spec() - - def initialize_meta_args(self): - args = dict( + self.on_off_choices = ['enabled', 'disabled', 'True', 'False'] + list(BOOLEANS) + self.argument_spec = dict( security_banner=dict( choices=self.on_off_choices ), @@ -375,35 +435,39 @@ class BigIpSysGlobalModuleConfig(object): console_timeout=dict(required=False, type='int', default=None), state=dict(default='present', choices=['present']) ) - self.meta_args = args + self.f5_product_name = 'bigip' - def initialize_argument_spec(self): - self.argument_spec = f5_argument_spec() - self.argument_spec.update(self.meta_args) - def create(self): - return AnsibleModule( - argument_spec=self.argument_spec, - supports_check_mode=self.supports_check_mode +def cleanup_tokens(client): + try: + resource = client.api.shared.authz.tokens_s.token.load( + name=client.api.icrs.token ) + resource.delete() + except Exception: + pass def main(): if not HAS_F5SDK: raise F5ModuleError("The python f5-sdk module is required") - config = BigIpSysGlobalModuleConfig() - module = config.create() + spec = ArgumentSpec() + + client = AnsibleF5Client( + argument_spec=spec.argument_spec, + supports_check_mode=spec.supports_check_mode, + f5_product_name=spec.f5_product_name + ) try: - obj = BigIpSysGlobalManager( - check_mode=module.check_mode, **module.params - ) - result = obj.apply_changes() - - module.exit_json(**result) + mm = ModuleManager(client) + results = mm.exec_module() + cleanup_tokens(client) + client.module.exit_json(**results) except F5ModuleError as e: - module.fail_json(msg=str(e)) + cleanup_tokens(client) + client.module.fail_json(msg=str(e)) if __name__ == '__main__': diff --git a/test/units/modules/network/f5/fixtures/load_sys_global_settings.json b/test/units/modules/network/f5/fixtures/load_sys_global_settings.json new file mode 100644 index 0000000000..191d8178a4 --- /dev/null +++ b/test/units/modules/network/f5/fixtures/load_sys_global_settings.json @@ -0,0 +1,24 @@ +{ + "kind": "tm:sys:global-settings:global-settingsstate", + "selfLink": "https://localhost/mgmt/tm/sys/global-settings?ver=13.0.0", + "awsApiMaxConcurrency": 1, + "consoleInactivityTimeout": 0, + "customAddr": "none", + "failsafeAction": "go-offline-restart-tm", + "fileBlacklistPathPrefix": "{/shared/3dns/} {/shared/bin/} {/shared/core/} {/shared/datasync/} {/shared/em/} {/shared/GeoIP/} {/shared/images/} {/shared/lib/} {/shared/lib64/} {/shared/log/} {/shared/lost+found/} {/shared/mgmt/} {/shared/nfb/} {/shared/ssh/} {/shared/statsd/} {/shared/tmstat/} {/shared/vadc/} {/config/aaa/} {/config/big3d/} {/config/bigip/} {/config/filestore/} {/config/gtm/} {/config/httpd/} {/config/ntp.conf} {/config/rndc.key} {/config/ssh/} {/config/ssl/}", + "fileBlacklistReadOnlyPathPrefix": "{/etc/shadow}", + "fileLocalPathPrefix": "{/shared/} {/tmp/}", + "fileWhitelistPathPrefix": "{/var/local/scf} {/tmp/} {/shared/} {/config/} {/usr/share/aws/}", + "guiSecurityBanner": "enabled", + "guiSecurityBannerText": "Welcome to the BIG-IP Configuration Utility.\n\nLog in with your username and password using the fields on the left.", + "guiSetup": "disabled", + "hostAddrMode": "management", + "hostname": "bigip1", + "lcdDisplay": "enabled", + "ledLocator": "disabled", + "mgmtDhcp": "enabled", + "netReboot": "disabled", + "passwordPrompt": "Password", + "quietBoot": "enabled", + "usernamePrompt": "Username" +} diff --git a/test/units/modules/network/f5/test_bigip_sys_global.py b/test/units/modules/network/f5/test_bigip_sys_global.py new file mode 100644 index 0000000000..18a71c1c51 --- /dev/null +++ b/test/units/modules/network/f5/test_bigip_sys_global.py @@ -0,0 +1,131 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2017 F5 Networks Inc. +# GNU General Public License v3.0 (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import os +import json +import sys + +from nose.plugins.skip import SkipTest +if sys.version_info < (2, 7): + raise SkipTest("F5 Ansible modules require Python >= 2.7") + +from ansible.compat.tests import unittest +from ansible.compat.tests.mock import Mock +from ansible.compat.tests.mock import patch +from ansible.module_utils.f5_utils import AnsibleF5Client + +try: + from library.bigip_sys_global import ApiParameters + from library.bigip_sys_global import ModuleParameters + from library.bigip_sys_global import ModuleManager + from library.bigip_sys_global import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError + from test.unit.modules.utils import set_module_args +except ImportError: + try: + from ansible.modules.network.f5.bigip_sys_global import ApiParameters + from ansible.modules.network.f5.bigip_sys_global import ModuleParameters + from ansible.modules.network.f5.bigip_sys_global import ModuleManager + from ansible.modules.network.f5.bigip_sys_global import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError + from units.modules.utils import set_module_args + except ImportError: + raise SkipTest("F5 Ansible modules require the f5-sdk Python library") + +fixture_path = os.path.join(os.path.dirname(__file__), 'fixtures') +fixture_data = {} + + +def load_fixture(name): + path = os.path.join(fixture_path, name) + + if path in fixture_data: + return fixture_data[path] + + with open(path) as f: + data = f.read() + + try: + data = json.loads(data) + except Exception: + pass + + fixture_data[path] = data + return data + + +class TestParameters(unittest.TestCase): + def test_module_parameters(self): + args = dict( + banner_text='this is a banner', + console_timeout=100, + gui_setup='yes', + lcd_display='yes', + mgmt_dhcp='yes', + net_reboot='yes', + quiet_boot='yes', + security_banner='yes', + ) + p = ModuleParameters(args) + assert p.banner_text == 'this is a banner' + assert p.console_timeout == 100 + assert p.gui_setup == 'enabled' + assert p.lcd_display == 'enabled' + assert p.mgmt_dhcp == 'enabled' + assert p.net_reboot == 'enabled' + assert p.quiet_boot == 'enabled' + assert p.security_banner == 'enabled' + + def test_api_parameters(self): + args = load_fixture('load_sys_global_settings.json') + p = ApiParameters(args) + assert 'Welcome to the BIG-IP Configuration Utility' in p.banner_text + assert p.console_timeout == 0 + assert p.gui_setup == 'disabled' + assert p.lcd_display == 'enabled' + assert p.mgmt_dhcp == 'enabled' + assert p.net_reboot == 'disabled' + assert p.quiet_boot == 'enabled' + assert p.security_banner == 'enabled' + + +@patch('ansible.module_utils.f5_utils.AnsibleF5Client._get_mgmt_root', + return_value=True) +class TestManager(unittest.TestCase): + + def setUp(self): + self.spec = ArgumentSpec() + + def test_update(self, *args): + set_module_args(dict( + banner_text='this is a banner', + console_timeout=100, + password='admin', + server='localhost', + user='admin', + state='present' + )) + + # Configure the parameters that would be returned by querying the + # remote device + current = ApiParameters(load_fixture('load_sys_global_settings.json')) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + mm.exists = Mock(return_value=False) + mm.read_current_from_device = Mock(return_value=current) + mm.update_on_device = Mock(return_value=True) + + results = mm.exec_module() + assert results['changed'] is True