diff --git a/lib/ansible/module_utils/network/f5/common.py b/lib/ansible/module_utils/network/f5/common.py index c89709084d..4356399224 100644 --- a/lib/ansible/module_utils/network/f5/common.py +++ b/lib/ansible/module_utils/network/f5/common.py @@ -84,6 +84,7 @@ f5_top_spec = { ), 'transport': dict( removed_in_version=2.9, + default='rest', choices=['cli', 'rest'] ) } @@ -94,6 +95,14 @@ def get_provider_argspec(): return f5_provider_spec +def load_params(params): + provider = params.get('provider') or dict() + for key, value in iteritems(provider): + if key in f5_argument_spec: + if params.get(key) is None and value is not None: + params[key] = value + + # Fully Qualified name (with the partition) def fqdn_name(partition, value): if value is not None and not value.startswith('/'): @@ -143,7 +152,8 @@ def cleanup_tokens(client): def is_cli(module): transport = module.params['transport'] provider_transport = (module.params['provider'] or {}).get('transport') - return 'cli' in (transport, provider_transport) + result = 'cli' in (transport, provider_transport) + return result class Noop(object): @@ -163,6 +173,7 @@ class Noop(object): class F5BaseClient(object): def __init__(self, *args, **kwargs): self.params = kwargs + load_params(self.params) @property def api(self): diff --git a/lib/ansible/modules/network/f5/bigip_command.py b/lib/ansible/modules/network/f5/bigip_command.py index 4b6c6ba6f0..b86ab2ca86 100644 --- a/lib/ansible/modules/network/f5/bigip_command.py +++ b/lib/ansible/modules/network/f5/bigip_command.py @@ -163,7 +163,6 @@ import re import time from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.basic import env_fallback from ansible.module_utils.six import string_types from ansible.module_utils.network.common.parsing import FailedConditionsError from ansible.module_utils.network.common.parsing import Conditional @@ -259,7 +258,8 @@ class ModuleManager(object): self.module = kwargs.get('module', None) self.client = kwargs.get('client', None) self.want = Parameters(params=self.module.params) - self.changes = Parameters() + self.want.update({'module': self.module}) + self.changes = Parameters(module=self.module) def _to_lines(self, stdout): lines = list() @@ -346,7 +346,7 @@ class ModuleManager(object): 'stdout_lines': self._to_lines(responses), 'warnings': warnings } - self.changes = Parameters(params=changes) + self.changes = Parameters(params=changes, module=self.module) if any(x for x in self.want.user_commands if x.startswith(changed)): return True return False diff --git a/lib/ansible/modules/network/f5/bigip_device_trust.py b/lib/ansible/modules/network/f5/bigip_device_trust.py index 376a97b90d..22bd0bf897 100644 --- a/lib/ansible/modules/network/f5/bigip_device_trust.py +++ b/lib/ansible/modules/network/f5/bigip_device_trust.py @@ -287,10 +287,8 @@ class ModuleManager(object): def remove_from_device(self): result = self.client.api.tm.cm.remove_from_trust.exec_cmd( - 'run', deviceName=self.want.peer_hostname + 'run', deviceName=self.want.peer_hostname, name=self.want.peer_hostname ) - if result: - result.delete() class ArgumentSpec(object): diff --git a/lib/ansible/modules/network/f5/bigip_selfip.py b/lib/ansible/modules/network/f5/bigip_selfip.py index dc57c3114d..3ee54eef01 100644 --- a/lib/ansible/modules/network/f5/bigip_selfip.py +++ b/lib/ansible/modules/network/f5/bigip_selfip.py @@ -24,6 +24,7 @@ options: description: - The IP addresses for the new self IP. This value is ignored upon update as addresses themselves cannot be changed after they are created. + - This value is required when creating new self IPs. allow_service: description: - Configure port lockdown for the Self IP. By default, the Self IP has a @@ -62,6 +63,7 @@ options: description: - The route domain id of the system. When creating a new Self IP, if this value is not specified, a default value of C(0) will be used. + - This value cannot be changed after it is set. version_added: 2.3 partition: description: @@ -249,7 +251,7 @@ except ImportError: HAS_F5SDK = False try: - from netaddr import IPNetwork, AddrFormatError, IPAddress + import netaddr HAS_NETADDR = True except ImportError: HAS_NETADDR = False @@ -262,11 +264,11 @@ class Parameters(AnsibleF5Parameters): } updatables = [ - 'traffic_group', 'allow_service', 'vlan', 'route_domain', 'netmask' + 'traffic_group', 'allow_service', 'vlan', 'netmask', 'address' ] returnables = [ - 'traffic_group', 'allow_service', 'vlan', 'route_domain', 'netmask' + 'traffic_group', 'allow_service', 'vlan', 'route_domain', 'netmask', 'address' ] api_attributes = [ @@ -280,6 +282,19 @@ class Parameters(AnsibleF5Parameters): result = self._filter_params(result) return result + def _fqdn_name(self, value): + if value is not None and not value.startswith('/'): + return '/{0}/{1}'.format(self.partition, value) + return value + + @property + def vlan(self): + if self._values['vlan'] is None: + return None + return self._fqdn_name(self._values['vlan']) + + +class ModuleParameters(Parameters): @property def address(self): address = "{0}%{1}/{2}".format( @@ -287,18 +302,14 @@ class Parameters(AnsibleF5Parameters): ) return address - @address.setter - def address(self, value): - self._values['ip'] = value - @property def ip(self): - if self._values['ip'] is None: + if self._values['address'] is None: return None try: - ip = str(IPAddress(self._values['ip'])) + ip = str(netaddr.IPAddress(self._values['address'])) return ip - except AddrFormatError: + except netaddr.AddrFormatError: raise F5ModuleError( 'The provided address is not a valid IP address' ) @@ -333,13 +344,13 @@ class Parameters(AnsibleF5Parameters): try: # IPv4 netmask address = '0.0.0.0/' + self._values['netmask'] - ip = IPNetwork(address) - except AddrFormatError as ex: + ip = netaddr.IPNetwork(address) + except netaddr.AddrFormatError as ex: try: # IPv6 netmask address = '::/' + self._values['netmask'] - ip = IPNetwork(address) - except AddrFormatError as ex: + ip = netaddr.IPNetwork(address) + except netaddr.AddrFormatError as ex: raise F5ModuleError( 'The provided netmask {0} is neither in IP or CIDR format'.format(self._values['netmask']) ) @@ -405,70 +416,57 @@ class Parameters(AnsibleF5Parameters): result = sorted(list(set(result))) return result - def _fqdn_name(self, value): - if value is not None and not value.startswith('/'): - return '/{0}/{1}'.format(self.partition, value) - return value - - @property - def vlan(self): - if self._values['vlan'] is None: - return None - return self._fqdn_name(self._values['vlan']) - class ApiParameters(Parameters): - api_map = {} - - @property - def address(self): - if self.ip and self.route_domain and self.netmask: - return '{0}%{1}/{2}'.format(self.ip, self.route_domain, self.netmask) - elif self.ip and self.netmask: - return '{0}/{1}'.format(self.ip, self.netmask) - - @address.setter - def address(self, value): - pattern = r'^(?P[0-9A-Fa-f:.]+)%?(?P\d+)?\/(?P\d+)$' - matches = re.match(pattern, value) - if not matches: - raise F5ModuleError( - "The specified address is malformed. Please see documentation." - ) - try: - ip = matches.group('ip') - self._values['ip'] = str(IPAddress(ip)) - except AddrFormatError: - raise F5ModuleError( - 'The provided address is not a valid IP address' - ) - self._values['route_domain'] = matches.group('rd') - self._values['netmask'] = matches.group('nm') - @property def allow_service(self): if self._values['allow_service'] is None: return None + if self._values['allow_service'] == 'all': + self._values['allow_service'] = ['all'] return sorted(self._values['allow_service']) @property - def trafficGroup(self): - return self.traffic_group - - @trafficGroup.setter - def trafficGroup(self, value): - self._values['traffic_group'] = value + def destination_ip(self): + if self._values['address'] is None: + return None + try: + pattern = r'(?P%[0-9]+)' + addr = re.sub(pattern, '', self._values['address']) + ip = netaddr.IPNetwork(addr) + return '{0}/{1}'.format(ip.ip, ip.prefixlen) + except netaddr.AddrFormatError: + raise F5ModuleError( + "The provided destination is not an IP address" + ) @property - def allowService(self): - return self._values['allow_service'] + def netmask(self): + ip = netaddr.IPNetwork(self.destination_ip) + return int(ip.prefixlen) - @allowService.setter - def allowService(self, value): - if value == 'all': - self._values['allow_service'] = ['all'] - else: - self._values['allow_service'] = sorted([str(x) for x in value]) + @property + def ip(self): + result = netaddr.IPNetwork(self.destination_ip) + return str(result.ip) + + +class Changes(Parameters): + pass + + +class UsableChanges(Changes): + @property + def allow_service(self): + if self._values['allow_service'] is None: + return None + if self._values['allow_service'] == ['all']: + return 'all' + return sorted(self._values['allow_service']) + + +class ReportableChanges(Changes): + pass class ModuleManager(object): @@ -476,8 +474,8 @@ class ModuleManager(object): self.module = kwargs.get('module', None) self.client = kwargs.get('client', None) self.have = None - self.want = Parameters(params=self.module.params) - self.changes = ApiParameters() + self.want = ModuleParameters(params=self.module.params) + self.changes = UsableChanges() def _set_changed_options(self): changed = {} @@ -485,7 +483,7 @@ class ModuleManager(object): if getattr(self.want, key) is not None: changed[key] = getattr(self.want, key) if changed: - self.changes = Parameters(params=changed) + self.changes = UsableChanges(params=changed) def _update_changed_options(self): diff = Difference(self.want, self.have) @@ -496,12 +494,12 @@ class ModuleManager(object): if change is None: continue else: - if k in ['netmask', 'route_domain']: + if k in ['netmask']: changed['address'] = change else: changed[k] = change if changed: - self.changes = ApiParameters(params=changed) + self.changes = UsableChanges(params=changed) return True return False @@ -583,11 +581,12 @@ class ModuleManager(object): self.want.update({'route_domain': 0}) if self.want.allow_service: if 'all' in self.want.allow_service: - self.want.update(dict(allow_service='all')) + self.want.update(dict(allow_service=['all'])) elif 'none' in self.want.allow_service: self.want.update(dict(allow_service=[])) elif 'default' in self.want.allow_service: self.want.update(dict(allow_service=['default'])) + self._set_changed_options() if self.want.check_mode: return True self.create_on_device() @@ -597,7 +596,7 @@ class ModuleManager(object): raise F5ModuleError("Failed to create the Self IP") def create_on_device(self): - params = self.want.api_params() + params = self.changes.api_params() self.client.api.tm.net.selfips.selfip.create( name=self.want.name, partition=self.want.partition, @@ -648,6 +647,10 @@ class Difference(object): except AttributeError: return attr1 + @property + def address(self): + pass + @property def allow_service(self): """Returns services formatted for consumption by f5-sdk update @@ -670,7 +673,7 @@ class Difference(object): if result[0] == 'none' and self.have.allow_service is None: return None elif result[0] == 'all' and self.have.allow_service[0] != 'all': - return 'all' + return ['all'] elif result[0] == 'none': return [] elif self.have.allow_service is None: @@ -683,7 +686,7 @@ class Difference(object): if self.want.netmask is None: return None try: - address = IPNetwork(self.have.ip) + address = netaddr.IPNetwork(self.have.ip) if self.want.route_domain is not None: nipnet = "{0}%{1}/{2}".format(address.ip, self.want.route_domain, self.want.netmask) cipnet = "{0}%{1}/{2}".format(address.ip, self.want.route_domain, self.have.netmask) @@ -695,32 +698,11 @@ class Difference(object): cipnet = "{0}/{1}".format(address.ip, self.have.netmask) if nipnet != cipnet: return nipnet - except AddrFormatError: + except netaddr.AddrFormatError: raise F5ModuleError( 'The provided address/netmask value "{0}" was invalid'.format(self.have.ip) ) - @property - def route_domain(self): - if self.want.route_domain is None: - return None - try: - address = IPNetwork(self.have.ip) - - if self.want.netmask is not None: - nipnet = "{0}%{1}/{2}".format(address.ip, self.want.route_domain, self.want.netmask) - cipnet = "{0}%{1}/{2}".format(address.ip, self.have.route_domain, self.want.netmask) - elif self.have.netmask is not None: - nipnet = "{0}%{1}/{2}".format(address.ip, self.want.route_domain, self.have.netmask) - cipnet = "{0}%{1}/{2}".format(address.ip, self.have.route_domain, self.have.netmask) - - if nipnet != cipnet: - return nipnet - except AddrFormatError: - raise F5ModuleError( - 'The provided address/netmask value was invalid' - ) - @property def traffic_group(self): if self.want.traffic_group != self.have.traffic_group: @@ -737,7 +719,7 @@ class ArgumentSpec(object): netmask=dict(), traffic_group=dict(), vlan=dict(), - route_domain=dict(), + route_domain=dict(type='int'), state=dict( default='present', choices=['present', 'absent'] diff --git a/lib/ansible/plugins/action/bigip.py b/lib/ansible/plugins/action/bigip.py index bd299516f5..e11cff45d4 100644 --- a/lib/ansible/plugins/action/bigip.py +++ b/lib/ansible/plugins/action/bigip.py @@ -43,10 +43,6 @@ except ImportError: class ActionModule(_ActionModule): def run(self, tmp=None, task_vars=None): - transport = self._task.args.get('transport', 'rest') - - display.vvvv('connection transport is %s' % transport, self._play_context.remote_addr) - if self._play_context.connection == 'network_cli': provider = self._task.args.get('provider', {}) if any(provider.values()): @@ -67,7 +63,7 @@ class ActionModule(_ActionModule): pc.remote_user = provider.get('user', self._play_context.connection_user) pc.password = provider.get('password', self._play_context.password) pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider.get('timeout', C.PERSISTENT_COMMAND_TIMEOUT)) + pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/test/units/modules/network/f5/test_bigip_selfip.py b/test/units/modules/network/f5/test_bigip_selfip.py index 2188b0f440..d0256ea161 100644 --- a/test/units/modules/network/f5/test_bigip_selfip.py +++ b/test/units/modules/network/f5/test_bigip_selfip.py @@ -21,8 +21,8 @@ from ansible.compat.tests.mock import patch from ansible.module_utils.basic import AnsibleModule try: - from library.bigip_selfip import Parameters from library.bigip_selfip import ApiParameters + from library.bigip_selfip import ModuleParameters from library.bigip_selfip import ModuleManager from library.bigip_selfip import ArgumentSpec from library.module_utils.network.f5.common import F5ModuleError @@ -30,8 +30,8 @@ try: from test.unit.modules.utils import set_module_args except ImportError: try: - from ansible.modules.network.f5.bigip_selfip import Parameters from ansible.modules.network.f5.bigip_selfip import ApiParameters + from ansible.modules.network.f5.bigip_selfip import ModuleParameters from ansible.modules.network.f5.bigip_selfip import ModuleManager from ansible.modules.network.f5.bigip_selfip import ArgumentSpec from ansible.module_utils.network.f5.common import F5ModuleError @@ -79,7 +79,7 @@ class TestParameters(unittest.TestCase): traffic_group='traffic-group-local-only', vlan='net1' ) - p = Parameters(params=args) + p = ModuleParameters(params=args) assert p.address == '10.10.10.10%1/24' assert p.allow_service == ['gre:0', 'tcp:80', 'udp:53'] assert p.name == 'net1' @@ -96,7 +96,7 @@ class TestParameters(unittest.TestCase): 'grp' ] ) - p = Parameters(params=args) + p = ModuleParameters(params=args) with pytest.raises(F5ModuleError) as ex: assert p.allow_service == ['grp', 'tcp:80', 'udp:53'] assert 'The provided protocol' in str(ex) @@ -119,7 +119,6 @@ class TestParameters(unittest.TestCase): assert p.allow_service == ['gre', 'tcp:80', 'udp:53'] assert p.name == 'net1' assert p.netmask == 24 - assert p.route_domain == 1 assert p.traffic_group == '/Common/traffic-group-local-only' assert p.vlan == '/Common/net1'