Refactors the bigip_sys_global module (#34438)

Changes include the current f5 coding standards and unit tests
for the module

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'
+    }
+
+    api_attributes = [
+        'guiSecurityBanner', 'guiSecurityBannerText', 'guiSetup', 'lcdDisplay',
+        'mgmtDhcp', 'netReboot', 'quietBoot', 'consoleInactivityTimeout'
+    ]
+
+    returnables = [
+        'security_banner', 'banner_text', 'gui_setup', 'lcd_display',
+        'mgmt_dhcp', 'net_reboot', 'quiet_boot', 'console_timeout'
+    ]
+
+    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_changes(self):
-        result = dict()
 
-        changed = self.apply_to_running_config()
+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])
 
-        result.update(**self.changed_params)
-        result.update(dict(changed=changed))
+    @property
+    def security_banner(self):
+        result = self._get_boolean_like_return_value('security_banner')
         return result
 
-    def apply_to_running_config(self):
-        try:
-            self.api = self.connect_to_bigip(**self.params)
-            return self.update_sys_global_settings()
-        except iControlUnexpectedHTTPError as e:
-            raise F5ModuleError(str(e))
+    @property
+    def gui_setup(self):
+        result = self._get_boolean_like_return_value('gui_setup')
+        return result
 
-    def connect_to_bigip(self, **kwargs):
-        return ManagementRoot(kwargs['server'],
-                              kwargs['user'],
-                              kwargs['password'],
-                              port=kwargs['server_port'])
+    @property
+    def banner_text(self):
+        result = self._get_boolean_like_return_value('banner_text')
+        return result
 
-    def read_sys_global_information(self):
-        settings = self.load_sys_global()
-        return self.format_sys_global_information(settings)
+    @property
+    def lcd_display(self):
+        result = self._get_boolean_like_return_value('lcd_display')
+        return result
 
-    def load_sys_global(self):
-        return self.api.tm.sys.global_settings.load()
+    @property
+    def mgmt_dhcp(self):
+        result = self._get_boolean_like_return_value('mgmt_dhcp')
+        return result
 
-    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']
+    @property
+    def net_reboot(self):
+        result = self._get_boolean_like_return_value('net_reboot')
         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
+    @property
+    def quiet_boot(self):
+        result = self._get_boolean_like_return_value('quiet_boot')
+        return result
 
-    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 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
+class Changes(Parameters):
+    def to_return(self):
+        result = {}
+        try:
+            for returnable in self.returnables:
+                result[returnable] = getattr(self, returnable)
+            result = self._filter_params(result)
+        except Exception:
+            pass
+        return result
 
-    def lcd_display_is_changed(self, current):
-        if self.params['lcd_display'] is None:
-            return False
-        if 'lcd_display' not in current:
-            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
+class UsableChanges(Changes):
+    pass
 
-    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
+class ReportableChanges(Changes):
+    pass
 
-    def console_timeout_is_changed(self, current):
-        if self.params['console_timeout'] is None:
-            return False
-        if 'console_timeout' not in current:
+
+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
-        if self.params['console_timeout'] == current['console_timeout']:
-            return False
-        else:
+        return False
+
+    def should_update(self):
+        result = self._update_changed_options()
+        if result:
             return True
+        return False
 
-    def format_sys_global_information(self, settings):
+    def exec_module(self):
         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)
+
+        try:
+            changed = self.present()
+        except iControlUnexpectedHTTPError as e:
+            raise F5ModuleError(str(e))
+
+        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 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:
+    def _announce_deprecations(self, result):
+        warnings = result.pop('__warnings', [])
+        for warning in warnings:
+            self.client.module.deprecate(
+                msg=warning['msg'],
+                version=warning['version']
+            )
+
+    def present(self):
+        return self.update()
+
+    def read_current_from_device(self):
+        resource = self.client.api.tm.sys.global_settings.load()
+        result = resource.attrs
+        return ApiParameters(result)
+
+    def update(self):
+        self.have = self.read_current_from_device()
+        if not self.should_update():
             return False
-        self.update_sys_global_settings_on_device(params)
+        if self.client.check_mode:
+            return True
+        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()
 
-    try:
-        obj = BigIpSysGlobalManager(
-            check_mode=module.check_mode, **module.params
-        )
-        result = obj.apply_changes()
+    client = AnsibleF5Client(
+        argument_spec=spec.argument_spec,
+        supports_check_mode=spec.supports_check_mode,
+        f5_product_name=spec.f5_product_name
+    )
 
-        module.exit_json(**result)
+    try:
+        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__':

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"
+}

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