From 60281b85fe09e1213ce531c314a898b4dc24da57 Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Wed, 1 Nov 2017 20:31:26 -0700 Subject: [PATCH] Refactored bigip_device_dns (#32483) Module was using old coding standards. This updates the module --- .../modules/network/f5/bigip_device_dns.py | 561 +++++++++--------- .../network/f5/test_bigip_device_dns.py | 143 +++++ 2 files changed, 424 insertions(+), 280 deletions(-) create mode 100644 test/units/modules/network/f5/test_bigip_device_dns.py diff --git a/lib/ansible/modules/network/f5/bigip_device_dns.py b/lib/ansible/modules/network/f5/bigip_device_dns.py index e18ab98ca5..096e39c0e1 100644 --- a/lib/ansible/modules/network/f5/bigip_device_dns.py +++ b/lib/ansible/modules/network/f5/bigip_device_dns.py @@ -4,11 +4,15 @@ # 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 + + ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} -DOCUMENTATION = ''' +DOCUMENTATION = r''' --- module: bigip_device_dns short_description: Manage BIG-IP device DNS settings @@ -22,17 +26,17 @@ options: operation each time a lookup is needed. Please note that this applies only to Access Policy Manager features, such as ACLs, web application rewrites, and authentication. - required: false default: disable choices: - - enable - - disable + - enabled + - disabled name_servers: description: - - A list of name serverz that the system uses to validate DNS lookups + - A list of name servers that the system uses to validate DNS lookups forwarders: description: - A list of BIND servers that the system can use to perform DNS lookups + - Deprecated in 2.4. Use the GUI or edit named.conf. search: description: - A list of domains that the system searches for local domain lookups, @@ -40,7 +44,6 @@ options: ip_version: description: - Specifies whether the DNS specifies IP addresses using IPv4 or IPv6. - required: false choices: - 4 - 6 @@ -48,14 +51,13 @@ options: description: - The state of the variable on the system. When C(present), guarantees that an existing variable is set to C(value). - required: false default: present choices: - absent - present notes: - Requires the f5-sdk Python package on the host. This is as easy as pip - install requests + install f5-sdk. extends_documentation_fragment: f5 requirements: - f5-sdk @@ -63,327 +65,326 @@ author: - Tim Rupp (@caphrim007) ''' -EXAMPLES = ''' +EXAMPLES = r''' - name: Set the DNS settings on the BIG-IP bigip_device_dns: - name_servers: - - 208.67.222.222 - - 208.67.220.220 - search: - - localdomain - - lab.local - state: present - password: "secret" - server: "lb.mydomain.com" - user: "admin" - validate_certs: "no" + name_servers: + - 208.67.222.222 + - 208.67.220.220 + search: + - localdomain + - lab.local + password: secret + server: lb.mydomain.com + user: admin + validate_certs: no delegate_to: localhost ''' -RETURN = ''' +RETURN = r''' cache: - description: The new value of the DNS caching - returned: changed - type: string - sample: "enabled" + description: The new value of the DNS caching + returned: changed + type: string + sample: enabled name_servers: - description: List of name servers that were added or removed - returned: changed - type: list - sample: "['192.0.2.10', '172.17.12.10']" -forwarders: - description: List of forwarders that were added or removed - returned: changed - type: list - sample: "['192.0.2.10', '172.17.12.10']" + description: List of name servers that were set + returned: changed + type: list + sample: ['192.0.2.10', '172.17.12.10'] search: - description: List of search domains that were added or removed - returned: changed - type: list - sample: "['192.0.2.10', '172.17.12.10']" + description: List of search domains that were set + returned: changed + type: list + sample: ['192.0.2.10', '172.17.12.10'] ip_version: - description: IP version that was set that DNS will specify IP addresses in - returned: changed - type: int - sample: 4 + description: IP version that was set that DNS will specify IP addresses in + returned: changed + type: int + sample: 4 +warnings: + description: The list of warnings (if any) generated by module based on arguments + returned: always + type: list + sample: ['...', '...'] ''' +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 F5ModuleError + try: - from f5.bigip.contexts import TransactionContextManager - from f5.bigip import ManagementRoot - HAS_F5SDK = True + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError except ImportError: HAS_F5SDK = False -REQUIRED = ['name_servers', 'search', 'forwarders', 'ip_version', 'cache'] -CACHE = ['disable', 'enable'] -IP = [4, 6] +class Parameters(AnsibleF5Parameters): + api_map = { + 'dhclient.mgmt': 'dhcp', + 'dns.cache': 'cache', + 'nameServers': 'name_servers', + 'include': 'ip_version' + } + api_attributes = [ + 'nameServers', 'search', 'include' + ] -class BigIpDeviceDns(object): - def __init__(self, *args, **kwargs): - if not HAS_F5SDK: - raise F5ModuleError("The python f5-sdk module is required") + updatables = [ + 'cache', 'name_servers', 'search', 'ip_version' + ] - # The params that change in the module - self.cparams = dict() + returnables = [ + 'cache', 'name_servers', 'search', 'ip_version' + ] - # Stores the params that are sent to the module - self.params = kwargs - self.api = ManagementRoot(kwargs['server'], - kwargs['user'], - kwargs['password'], - port=kwargs['server_port']) + absentables = [ + 'name_servers', 'search' + ] - def flush(self): - result = dict() - changed = False - state = self.params['state'] + def to_return(self): + result = {} + for returnable in self.returnables: + result[returnable] = getattr(self, returnable) + result = self._filter_params(result) + return result - if self.dhcp_enabled(): + 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 + + @property + def search(self): + result = [] + if self._values['search'] is None: + return None + for server in self._values['search']: + result.append(str(server)) + return result + + @property + def name_servers(self): + result = [] + if self._values['name_servers'] is None: + return None + for server in self._values['name_servers']: + result.append(str(server)) + return result + + @property + def cache(self): + if str(self._values['cache']) in ['enabled', 'enable']: + return 'enable' + else: + return 'disable' + + @property + def dhcp(self): + valid = ['enable', 'enabled'] + return True if self._values['dhcp'] in valid else False + + @property + def forwarders(self): + if self._values['forwarders'] is None: + return None + else: raise F5ModuleError( - "DHCP on the mgmt interface must be disabled to make use of " + - "this module" + "The modifying of forwarders is not supported." ) - if state == 'absent': - changed = self.absent() + @property + def ip_version(self): + if self._values['ip_version'] in [6, '6', 'options inet6']: + return "options inet6" + elif self._values['ip_version'] in [4, '4', '']: + return "" else: - changed = self.present() + return None - result.update(**self.cparams) + +class ModuleManager(object): + def __init__(self, client): + self.client = client + self.have = None + self.want = Parameters(self.client.module.params) + self.changes = Parameters() + + def _update_changed_options(self): + changed = {} + for key in Parameters.updatables: + if getattr(self.want, key) is not None: + attr1 = getattr(self.want, key) + attr2 = getattr(self.have, key) + if attr1 != attr2: + changed[key] = attr1 + if changed: + self.changes = Parameters(changed) + return True + return False + + def exec_module(self): + changed = False + result = dict() + state = self.want.state + + try: + if state == "present": + changed = self.update() + elif state == "absent": + changed = self.absent() + except iControlUnexpectedHTTPError as e: + raise F5ModuleError(str(e)) + + changes = self.changes.to_return() + result.update(**changes) result.update(dict(changed=changed)) return result - def dhcp_enabled(self): - r = self.api.tm.sys.dbs.db.load(name='dhclient.mgmt') - if r.value == 'enable': - return True - else: - return False - - def read(self): + def read_current_from_device(self): + want_keys = ['dns.cache'] result = dict() + dbs = self.client.api.tm.sys.dbs.get_collection() + for db in dbs: + if db.name in want_keys: + result[db.name] = db.value + dns = self.client.api.tm.sys.dns.load() + attrs = dns.attrs + if 'include' not in attrs: + attrs['include'] = 4 + result.update(attrs) + return Parameters(result) - cache = self.api.tm.sys.dbs.db.load(name='dns.cache') - proxy = self.api.tm.sys.dbs.db.load(name='dns.proxy.__iter__') - dns = self.api.tm.sys.dns.load() - - result['cache'] = str(cache.value) - result['forwarders'] = str(proxy.value).split(' ') - - if hasattr(dns, 'nameServers'): - result['name_servers'] = dns.nameServers - if hasattr(dns, 'search'): - result['search'] = dns.search - if hasattr(dns, 'include') and 'options inet6' in dns.include: - result['ip_version'] = 6 - else: - result['ip_version'] = 4 - return result - - def present(self): - params = dict() - current = self.read() - - # Temporary locations to hold the changed params - update = dict( - dns=None, - forwarders=None, - cache=None - ) - - nameservers = self.params['name_servers'] - search_domains = self.params['search'] - ip_version = self.params['ip_version'] - forwarders = self.params['forwarders'] - cache = self.params['cache'] - check_mode = self.params['check_mode'] - - if nameservers: - if 'name_servers' in current: - if nameservers != current['name_servers']: - params['nameServers'] = nameservers - else: - params['nameServers'] = nameservers - - if search_domains: - if 'search' in current: - if search_domains != current['search']: - params['search'] = search_domains - else: - params['search'] = search_domains - - if ip_version: - if 'ip_version' in current: - if ip_version != int(current['ip_version']): - if ip_version == 6: - params['include'] = 'options inet6' - elif ip_version == 4: - params['include'] = '' - else: - if ip_version == 6: - params['include'] = 'options inet6' - elif ip_version == 4: - params['include'] = '' - - if params: - self.cparams.update(camel_dict_to_snake_dict(params)) - - if 'include' in params: - del self.cparams['include'] - if params['include'] == '': - self.cparams['ip_version'] = 4 - else: - self.cparams['ip_version'] = 6 - - update['dns'] = params.copy() - params = dict() - - if forwarders: - if 'forwarders' in current: - if forwarders != current['forwarders']: - params['forwarders'] = forwarders - else: - params['forwarders'] = forwarders - - if params: - self.cparams.update(camel_dict_to_snake_dict(params)) - update['forwarders'] = ' '.join(params['forwarders']) - params = dict() - - if cache: - if 'cache' in current: - if cache != current['cache']: - params['cache'] = cache - - if params: - self.cparams.update(camel_dict_to_snake_dict(params)) - update['cache'] = params['cache'] - params = dict() - - if self.cparams: - changed = True - if check_mode: - return changed - else: + def update(self): + self.have = self.read_current_from_device() + if not self.should_update(): return False + if self.client.check_mode: + return True + self.update_on_device() + return True - tx = self.api.tm.transactions.transaction - with TransactionContextManager(tx) as api: - cache = api.tm.sys.dbs.db.load(name='dns.cache') - proxy = api.tm.sys.dbs.db.load(name='dns.proxy.__iter__') - dns = api.tm.sys.dns.load() + def should_update(self): + result = self._update_changed_options() + if result: + return True + return False - # Empty values can be supplied, but you cannot supply the - # None value, so we check for that specifically - if update['cache'] is not None: - cache.update(value=update['cache']) - if update['forwarders'] is not None: - proxy.update(value=update['forwarders']) - if update['dns'] is not None: - dns.update(**update['dns']) - return changed + def update_on_device(self): + params = self.want.api_params() + cache = self.client.api.tm.sys.dbs.db.load(name='dns.cache') + dns = self.client.api.tm.sys.dns.load() + + # Empty values can be supplied, but you cannot supply the + # None value, so we check for that specifically + if self.want.cache is not None: + cache.update(value=self.want.cache) + if params: + dns.update(**params) + + def _absent_changed_options(self): + changed = {} + for key in Parameters.absentables: + if getattr(self.want, key) is not None: + set_want = set(getattr(self.want, key)) + set_have = set(getattr(self.have, key)) + set_new = set_have - set_want + if set_new != set_have: + changed[key] = list(set_new) + if changed: + self.changes = Parameters(changed) + return True + return False + + def should_absent(self): + result = self._absent_changed_options() + if result: + return True + return False def absent(self): - params = dict() - current = self.read() - - # Temporary locations to hold the changed params - update = dict( - dns=None, - forwarders=None - ) - - nameservers = self.params['name_servers'] - search_domains = self.params['search'] - forwarders = self.params['forwarders'] - check_mode = self.params['check_mode'] - - if forwarders and 'forwarders' in current: - set_current = set(current['forwarders']) - set_new = set(forwarders) - - forwarders = set_current - set_new - if forwarders != set_current: - forwarders = list(forwarders) - params['forwarders'] = ' '.join(forwarders) - - if params: - changed = True - self.cparams.update(camel_dict_to_snake_dict(params)) - update['forwarders'] = params['forwarders'] - params = dict() - - if nameservers and 'name_servers' in current: - set_current = set(current['name_servers']) - set_new = set(nameservers) - - nameservers = set_current - set_new - if nameservers != set_current: - params['nameServers'] = list(nameservers) - - if search_domains and 'search' in current: - set_current = set(current['search']) - set_new = set(search_domains) - - search_domains = set_current - set_new - if search_domains != set_current: - params['search'] = list(search_domains) - - if params: - changed = True - self.cparams.update(camel_dict_to_snake_dict(params)) - update['dns'] = params.copy() - params = dict() - - if not self.cparams: + self.have = self.read_current_from_device() + if not self.should_absent(): return False + if self.client.check_mode: + return True + self.absent_on_device() + return True - if check_mode: - return changed + def absent_on_device(self): + params = self.changes.api_params() + resource = self.client.api.tm.sys.dns.load() + resource.update(**params) - tx = self.api.tm.transactions.transaction - with TransactionContextManager(tx) as api: - proxy = api.tm.sys.dbs.db.load(name='dns.proxy.__iter__') - dns = api.tm.sys.dns.load() - if update['forwarders'] is not None: - proxy.update(value=update['forwarders']) - if update['dns'] is not None: - dns.update(**update['dns']) - return changed +class ArgumentSpec(object): + def __init__(self): + self.supports_check_mode = True + self.argument_spec = dict( + cache=dict( + required=False, + choices=['disabled', 'enabled', 'disable', 'enable'], + default=None + ), + name_servers=dict( + required=False, + default=None, + type='list' + ), + forwarders=dict( + required=False, + default=None, + type='list' + ), + search=dict( + required=False, + default=None, + type='list' + ), + ip_version=dict( + required=False, + default=None, + choices=[4, 6], + type='int' + ), + state=dict( + required=False, + default='present', + choices=['absent', 'present'] + ) + ) + self.required_one_of = [ + ['name_servers', 'search', 'forwarders', 'ip_version', 'cache'] + ] + self.f5_product_name = 'bigip' def main(): - argument_spec = f5_argument_spec() + if not HAS_F5SDK: + raise F5ModuleError("The python f5-sdk module is required") - meta_args = dict( - cache=dict(required=False, choices=CACHE, default=None), - name_servers=dict(required=False, default=None, type='list'), - forwarders=dict(required=False, default=None, type='list'), - search=dict(required=False, default=None, type='list'), - ip_version=dict(required=False, default=None, choices=IP, type='int') - ) - argument_spec.update(meta_args) - module = AnsibleModule( - argument_spec=argument_spec, - required_one_of=[REQUIRED], - supports_check_mode=True + spec = ArgumentSpec() + + client = AnsibleF5Client( + argument_spec=spec.argument_spec, + supports_check_mode=spec.supports_check_mode, + f5_product_name=spec.f5_product_name, + required_one_of=spec.required_one_of ) try: - obj = BigIpDeviceDns(check_mode=module.check_mode, **module.params) - result = obj.flush() - - module.exit_json(**result) + mm = ModuleManager(client) + results = mm.exec_module() + client.module.exit_json(**results) except F5ModuleError as e: - module.fail_json(msg=str(e)) - -from ansible.module_utils.basic import * -from ansible.module_utils.ec2 import camel_dict_to_snake_dict -from ansible.module_utils.f5_utils import * + client.module.fail_json(msg=str(e)) if __name__ == '__main__': main() diff --git a/test/units/modules/network/f5/test_bigip_device_dns.py b/test/units/modules/network/f5/test_bigip_device_dns.py new file mode 100644 index 0000000000..59679c1675 --- /dev/null +++ b/test/units/modules/network/f5/test_bigip_device_dns.py @@ -0,0 +1,143 @@ +# -*- 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 pytest +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 patch, Mock +from ansible.module_utils import basic +from ansible.module_utils._text import to_bytes +from ansible.module_utils.f5_utils import AnsibleF5Client +from ansible.module_utils.f5_utils import F5ModuleError + +try: + from library.bigip_device_dns import Parameters + from library.bigip_device_dns import ModuleManager + from library.bigip_device_dns import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError +except ImportError: + try: + from ansible.modules.network.f5.bigip_device_dns import Parameters + from ansible.modules.network.f5.bigip_device_dns import ModuleManager + from ansible.modules.network.f5.bigip_device_dns import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError + 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 set_module_args(args): + args = json.dumps({'ANSIBLE_MODULE_ARGS': args}) + basic._ANSIBLE_ARGS = to_bytes(args) + + +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( + cache='disable', + forwarders=['12.12.12.12', '13.13.13.13'], + ip_version=4, + name_servers=['10.10.10.10', '11.11.11.11'], + search=['14.14.14.14', '15.15.15.15'], + server='localhost', + user='admin', + password='password' + ) + p = Parameters(args) + assert p.cache == 'disable' + assert p.name_servers == ['10.10.10.10', '11.11.11.11'] + assert p.search == ['14.14.14.14', '15.15.15.15'] + + # BIG-IP considers "ipv4" to be an empty value + assert p.ip_version == '' + + def test_ipv6_parameter(self): + args = dict( + ip_version=6 + ) + p = Parameters(args) + assert p.ip_version == 'options inet6' + + def test_ensure_forwards_raises_exception(self): + args = dict( + forwarders=['12.12.12.12', '13.13.13.13'], + ) + p = Parameters(args) + with pytest.raises(F5ModuleError) as ex: + foo = p.forwarders + assert 'The modifying of forwarders is not supported' in str(ex) + + +class TestManager(unittest.TestCase): + + def setUp(self): + self.spec = ArgumentSpec() + + @patch('ansible.module_utils.f5_utils.AnsibleF5Client._get_mgmt_root', + return_value=True) + def test_update_settings(self, *args): + set_module_args(dict( + cache='disable', + forwarders=['12.12.12.12', '13.13.13.13'], + ip_version=4, + name_servers=['10.10.10.10', '11.11.11.11'], + search=['14.14.14.14', '15.15.15.15'], + server='localhost', + user='admin', + password='password' + )) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + dict( + cache='enable' + ) + ) + + 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.update_on_device = Mock(return_value=True) + mm.read_current_from_device = Mock(return_value=current) + + results = mm.exec_module() + + assert results['changed'] is True