From 138690519de13b6790b608fc1f66d6c7536ab05f Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Tue, 4 Dec 2018 11:15:49 -0800 Subject: [PATCH] Various small fixes for f5 ansible modules (#49492) --- .../modules/network/f5/bigip_asm_policy.py | 2 +- .../network/f5/bigip_gtm_topology_record.py | 4 +- lib/ansible/modules/network/f5/bigip_irule.py | 22 +-- .../network/f5/bigip_management_route.py | 9 +- .../modules/network/f5/bigip_monitor_https.py | 19 ++ .../network/f5/bigip_profile_fastl4.py | 4 + lib/ansible/modules/network/f5/bigip_user.py | 24 +-- .../network/f5/bigip_virtual_server.py | 179 ++++++++++-------- lib/ansible/modules/network/f5/bigip_vlan.py | 3 +- .../modules/network/f5/test_bigip_user.py | 66 ------- 10 files changed, 148 insertions(+), 184 deletions(-) diff --git a/lib/ansible/modules/network/f5/bigip_asm_policy.py b/lib/ansible/modules/network/f5/bigip_asm_policy.py index 1d7a4c2d0a6..c8448652b41 100644 --- a/lib/ansible/modules/network/f5/bigip_asm_policy.py +++ b/lib/ansible/modules/network/f5/bigip_asm_policy.py @@ -1003,7 +1003,7 @@ class ArgumentSpec(object): name=dict( required=True, ), - file=dict(), + file=dict(type='path'), template=dict( choices=self.template_map ), diff --git a/lib/ansible/modules/network/f5/bigip_gtm_topology_record.py b/lib/ansible/modules/network/f5/bigip_gtm_topology_record.py index fbcb3cdafd0..f191825f45a 100644 --- a/lib/ansible/modules/network/f5/bigip_gtm_topology_record.py +++ b/lib/ansible/modules/network/f5/bigip_gtm_topology_record.py @@ -44,7 +44,9 @@ options: country: description: - Specifies a country. - - Full continent names and their abbreviated versions are supported. + - In addition to the country full names, you may also specify their abbreviated + form, such as C(US) instead of C(United States). + - Valid country codes can be found here https://countrycode.org/. state: description: - Specifies a state in a given country. diff --git a/lib/ansible/modules/network/f5/bigip_irule.py b/lib/ansible/modules/network/f5/bigip_irule.py index 05e10ac1719..6f31b024f16 100644 --- a/lib/ansible/modules/network/f5/bigip_irule.py +++ b/lib/ansible/modules/network/f5/bigip_irule.py @@ -66,21 +66,23 @@ EXAMPLES = r''' content: "{{ lookup('template', 'irule.tcl') }}" module: ltm name: MyiRule - password: secret - server: lb.mydomain.com state: present - user: admin + provider: + user: admin + password: secret + server: lb.mydomain.com delegate_to: localhost - name: Add the iRule contained in static file irule.tcl to the LTM module bigip_irule: module: ltm name: MyiRule - password: secret - server: lb.mydomain.com src: irule.tcl state: present - user: admin + provider: + user: admin + password: secret + server: lb.mydomain.com delegate_to: localhost ''' @@ -528,13 +530,9 @@ class ArgumentSpec(object): def __init__(self): self.supports_check_mode = True argument_spec = dict( - content=dict( - required=False, - default=None - ), + content=dict(), src=dict( - required=False, - default=None + type='path', ), name=dict(required=True), module=dict( diff --git a/lib/ansible/modules/network/f5/bigip_management_route.py b/lib/ansible/modules/network/f5/bigip_management_route.py index c1ee4aa8120..7ff0da826f4 100644 --- a/lib/ansible/modules/network/f5/bigip_management_route.py +++ b/lib/ansible/modules/network/f5/bigip_management_route.py @@ -64,10 +64,11 @@ EXAMPLES = r''' description: Route to TACACS gateway: 10.10.10.10 network: 11.11.11.0/24 - password: secret - server: lb.mydomain.com state: present - user: admin + provider: + user: admin + password: secret + server: lb.mydomain.com delegate_to: localhost ''' @@ -214,6 +215,8 @@ class Difference(object): def network(self): if self.want.network is None: return None + if self.want.network == '0.0.0.0/0' and self.have.network == 'default': + return None if self.want.network != self.have.network: raise F5ModuleError( "'network' cannot be changed after it is set." diff --git a/lib/ansible/modules/network/f5/bigip_monitor_https.py b/lib/ansible/modules/network/f5/bigip_monitor_https.py index 9c08adef268..0e2e72187da 100644 --- a/lib/ansible/modules/network/f5/bigip_monitor_https.py +++ b/lib/ansible/modules/network/f5/bigip_monitor_https.py @@ -92,6 +92,15 @@ options: for an HTTPS monitor. - This parameter is only supported on BIG-IP versions 13.x and later. version_added: 2.8 + up_interval: + description: + - Specifies the interval for the system to use to perform the health check + when a resource is up. + - When C(0), specifies that the system uses the interval specified in + C(interval) to check the health of the resource. + - When any other number, enables specification of a different interval to + use when checking the health of a resource that is up. + version_added: 2.8 partition: description: - Device partition to manage resources on. @@ -167,6 +176,11 @@ time_until_up: returned: changed type: int sample: 2 +up_interval: + description: Interval for the system to use to perform the health check when a resource is up. + returned: changed + type: int + sample: 0 ''' from ansible.module_utils.basic import AnsibleModule @@ -207,6 +221,7 @@ class Parameters(AnsibleF5Parameters): 'recv': 'receive', 'recvDisable': 'receive_disable', 'sslProfile': 'ssl_profile', + 'upInterval': 'up_interval', } api_attributes = [ @@ -222,6 +237,7 @@ class Parameters(AnsibleF5Parameters): 'recvDisable', 'description', 'sslProfile', + 'upInterval', ] returnables = [ @@ -236,6 +252,7 @@ class Parameters(AnsibleF5Parameters): 'receive_disable', 'description', 'ssl_profile', + 'up_interval', ] updatables = [ @@ -250,6 +267,7 @@ class Parameters(AnsibleF5Parameters): 'receive_disable', 'description', 'ssl_profile', + 'up_interval', ] @property @@ -667,6 +685,7 @@ class ArgumentSpec(object): receive=dict(), receive_disable=dict(required=False), ip=dict(), + up_interval=dict(type='int'), port=dict(), interval=dict(type='int'), timeout=dict(type='int'), diff --git a/lib/ansible/modules/network/f5/bigip_profile_fastl4.py b/lib/ansible/modules/network/f5/bigip_profile_fastl4.py index a0a629b83c0..1a11c66bcd7 100644 --- a/lib/ansible/modules/network/f5/bigip_profile_fastl4.py +++ b/lib/ansible/modules/network/f5/bigip_profile_fastl4.py @@ -483,6 +483,7 @@ except ImportError: class Parameters(AnsibleF5Parameters): api_map = { 'clientTimeout': 'client_timeout', + 'defaultsFrom': 'parent', 'explicitFlowMigration': 'explicit_flow_migration', 'idleTimeout': 'idle_timeout', 'ipDfMode': 'ip_df_mode', @@ -518,6 +519,7 @@ class Parameters(AnsibleF5Parameters): api_attributes = [ 'clientTimeout', + 'defaultsFrom', 'description', 'explicitFlowMigration', 'idleTimeout', @@ -570,6 +572,7 @@ class Parameters(AnsibleF5Parameters): 'loose_close', 'loose_initialization', 'mss_override', + 'parent', 'reassemble_fragments', 'receive_window_size', 'reset_on_timeout', @@ -606,6 +609,7 @@ class Parameters(AnsibleF5Parameters): 'loose_close', 'loose_initialization', 'mss_override', + 'parent', 'reassemble_fragments', 'receive_window_size', 'reset_on_timeout', diff --git a/lib/ansible/modules/network/f5/bigip_user.py b/lib/ansible/modules/network/f5/bigip_user.py index 625b829eabe..b44f31fee9f 100644 --- a/lib/ansible/modules/network/f5/bigip_user.py +++ b/lib/ansible/modules/network/f5/bigip_user.py @@ -50,12 +50,12 @@ options: description: - Specifies the administrative partition to which the user has access. C(partition_access) is required when creating a new account. - Should be in the form "partition:role". Valid roles include - C(acceleration-policy-editor), C(admin), C(application-editor), C(auditor) - C(certificate-manager), C(guest), C(irule-manager), C(manager), C(no-access) + Should be in the form "partition:role". + - Valid roles include C(acceleration-policy-editor), C(admin), C(application-editor), + C(auditor), C(certificate-manager), C(guest), C(irule-manager), C(manager), C(no-access), C(operator), C(resource-admin), C(user-manager), C(web-application-security-administrator), - and C(web-application-security-editor). Partition portion of tuple should - be an existing partition or the value 'all'. + and C(web-application-security-editor). + - Partition portion of tuple should be an existing partition or the value 'all'. state: description: - Whether the account should exist or not, taking action if the state is @@ -67,8 +67,8 @@ options: update_password: description: - C(always) will allow to update passwords if the user chooses to do so. - C(on_create) will only set the password for newly created users. When - C(username_credential) is C(root), this value will be forced to C(always). + C(on_create) will only set the password for newly created users. + - When C(username_credential) is C(root), this value will be forced to C(always). default: always choices: - always @@ -625,16 +625,6 @@ class BaseManager(object): raise F5ModuleError(err) def validate_create_parameters(self): - """Password credentials and partition access are mandatory, - - when creating a user resource. - """ - if self.want.password_credential and \ - self.want.update_password != 'on_create': - err = "The 'update_password' option " \ - "needs to be set to 'on_create' when creating " \ - "a resource with a password." - raise F5ModuleError(err) if self.want.partition_access is None: err = "The 'partition_access' option " \ "is required when creating a resource." diff --git a/lib/ansible/modules/network/f5/bigip_virtual_server.py b/lib/ansible/modules/network/f5/bigip_virtual_server.py index 54ed7e59d6b..aabb89afd14 100644 --- a/lib/ansible/modules/network/f5/bigip_virtual_server.py +++ b/lib/ansible/modules/network/f5/bigip_virtual_server.py @@ -98,6 +98,7 @@ options: - Required when C(state) is C(present) and virtual server does not exist. - When C(type) is C(internal), this parameter is ignored. For all other types, it is required. + - Destination can also be specified as a name for an existing Virtual Address. aliases: - address - ip @@ -296,7 +297,7 @@ options: version_added: 2.8 mask: description: - - Specifies the destination address network mask. This parameter will work with IPv4 and IPv6 tye of addresses. + - Specifies the destination address network mask. This parameter will work with IPv4 and IPv6 type of addresses. - This is an optional parameter which can be specified when creating or updating virtual server. - If C(destination) is set in CIDR notation format and C(mask) is provided the C(mask) parameter takes precedence. - If catchall destination is specified, i.e. C(0.0.0.0) for IPv4 C(::) for IPv6, @@ -305,6 +306,8 @@ options: C(ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff) is set for IPv4 and IPv6 addresses respectively. - When C(destination) is provided in CIDR notation format and C(mask) is not specified the mask parameter is inferred from C(destination). + - When C(destination) is provided as Virtual Address name, and C(mask) is not specified, + the mask will be C(None) allowing device set it with its internal defaults. version_added: 2.8 ip_protocol: description: @@ -403,30 +406,29 @@ author: EXAMPLES = r''' - name: Modify Port of the Virtual Server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret state: present partition: Common name: my-virtual-server port: 8080 + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Delete virtual server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret state: absent partition: Common name: my-virtual-server + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Add virtual server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret state: present partition: Common name: my-virtual-server @@ -449,6 +451,10 @@ EXAMPLES = r''' - ltm-policy-3 enabled_vlans: - /Common/vlan2 + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Add FastL4 virtual server @@ -459,95 +465,108 @@ EXAMPLES = r''' profiles: - fastL4 state: present + provider: + server: lb.mydomain.net + user: admin + password: secret + delegate_to: localhost - name: Add iRules to the Virtual Server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret name: my-virtual-server irules: - irule1 - irule2 + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Remove one iRule from the Virtual Server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret name: my-virtual-server irules: - irule2 + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Remove all iRules from the Virtual Server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret name: my-virtual-server irules: "" + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Remove pool from the Virtual Server bigip_virtual_server: - server: lb.mydomain.net - user: admin - password: secret name: my-virtual-server pool: "" + provider: + server: lb.mydomain.net + user: admin + password: secret delegate_to: localhost - name: Add metadata to virtual bigip_pool: - server: lb.mydomain.com - user: admin - password: secret state: absent name: my-pool partition: Common metadata: ansible: 2.4 updated_at: 2017-12-20T17:50:46Z + provider: + server: lb.mydomain.com + user: admin + password: secret delegate_to: localhost - name: Add virtual with two profiles bigip_pool: - server: lb.mydomain.com - user: admin - password: secret state: absent name: my-pool partition: Common profiles: - http - tcp + provider: + server: lb.mydomain.com + user: admin + password: secret delegate_to: localhost - name: Remove HTTP profile from previous virtual bigip_pool: - server: lb.mydomain.com - user: admin - password: secret state: absent name: my-pool partition: Common profiles: - tcp + provider: + server: lb.mydomain.com + user: admin + password: secret delegate_to: localhost - name: Add the HTTP profile back to the previous virtual bigip_pool: - server: lb.mydomain.com - user: admin - password: secret state: absent name: my-pool partition: Common profiles: - http - tcp + provider: + server: lb.mydomain.com + user: admin + password: secret delegate_to: localhost ''' @@ -678,6 +697,7 @@ ip_intelligence_policy: type: string sample: /Common/ip-intelligence ''' + import os import re @@ -1150,15 +1170,6 @@ class ApiParameters(Parameters): return result destination = re.sub(r'^/[a-zA-Z0-9_.-]+/', '', self._values['destination']) - if is_valid_ip(destination): - result = Destination( - ip=destination, - port=None, - route_domain=None, - mask=self.mask - ) - return result - # Covers the following examples # # /Common/2700:bc00:1f10:101::6%2.80 @@ -1177,11 +1188,6 @@ class ApiParameters(Parameters): port = matches.group('port') if port == 'any': port = 0 - ip = matches.group('ip') - if not is_valid_ip(ip): - raise F5ModuleError( - "The provided destination is not a valid IP address" - ) result = Destination( ip=matches.group('ip'), port=port, @@ -1193,11 +1199,6 @@ class ApiParameters(Parameters): pattern = r'(?P[^%]+)%(?P[0-9]+)' matches = re.search(pattern, destination) if matches: - ip = matches.group('ip') - if not is_valid_ip(ip): - raise F5ModuleError( - "The provided destination is not a valid IP address" - ) result = Destination( ip=matches.group('ip'), port=None, @@ -1206,22 +1207,20 @@ class ApiParameters(Parameters): ) return result - parts = destination.split('.') - if len(parts) == 4: - # IPv4 - ip, port = destination.split(':') - if not is_valid_ip(ip): - raise F5ModuleError( - "The provided destination is not a valid IP address" - ) + pattern = r'(?P^[a-zA-Z0-9_.-]+):(?P[0-9]+)' + matches = re.search(pattern, destination) + if matches: + # this will match any IPv4 address as well as any alphanumeric Virtual Address + # that does not look like an IPv6 address. result = Destination( - ip=ip, - port=int(port), + ip=matches.group('ip'), + port=int(matches.group('port')), route_domain=None, mask=self.mask ) return result - elif len(parts) == 2: + parts = destination.split('.') + if len(parts) == 2: # IPv6 ip, port = destination.split('.') try: @@ -1230,10 +1229,6 @@ class ApiParameters(Parameters): # Can be a port of "any". This only happens with IPv6 if port == 'any': port = 0 - if not is_valid_ip(ip): - raise F5ModuleError( - "The provided destination is not a valid IP address" - ) result = Destination( ip=ip, port=port, @@ -1241,6 +1236,16 @@ class ApiParameters(Parameters): mask=self.mask ) return result + # this check needs to be the last as for some reason IPv6 addr with %2 %2.port were also caught + if is_valid_ip(destination): + result = Destination( + ip=destination, + port=None, + route_domain=None, + mask=self.mask + ) + return result + else: result = Destination(ip=None, port=None, route_domain=None, mask=None) return result @@ -1456,11 +1461,14 @@ class ModuleParameters(Parameters): @property def destination(self): + pattern = r'^[a-zA-Z0-9_.-]+' addr = self._values['destination'].split("%")[0].split('/')[0] if not is_valid_ip(addr): - raise F5ModuleError( - "The provided destination is not a valid IP address" - ) + matches = re.search(pattern, addr) + if not matches: + raise F5ModuleError( + "The provided destination is not a valid IP address or a Virtual Address name" + ) result = self._format_destination(addr, self.port, self.route_domain) return result @@ -1470,6 +1478,11 @@ class ModuleParameters(Parameters): return None result = self._values['destination'].split("%") if len(result) > 1: + pattern = r'^[a-zA-Z0-9_.-]+' + matches = re.search(pattern, result[0]) + if matches and not is_valid_ip(matches.group(0)): + # we need to strip RD because when using Virtual Address names the RD is not needed. + return None return int(result[1]) return None @@ -1479,8 +1492,9 @@ class ModuleParameters(Parameters): if self._values['destination'] is None: result = Destination(ip=None, port=None, route_domain=None, mask=None) return result - sanitized = self._values['destination'].split("%")[0].split('/')[0] - addr = compress_address(u'{0}'.format(sanitized)) + addr = self._values['destination'].split("%")[0].split('/')[0] + if is_valid_ip(addr): + addr = compress_address(u'{0}'.format(addr)) result = Destination(ip=addr, port=self.port, route_domain=self.route_domain, mask=self.mask) return result @@ -1494,8 +1508,11 @@ class ModuleParameters(Parameters): if addr in ['::', '::/0', '::/any6']: return 'any6' if self._values['mask'] is None: - return get_netmask(u'{0}'.format(addr)) - return compress_address(u'{0}'.format(self._values['mask'])) + if is_valid_ip(addr): + return get_netmask(addr) + else: + return None + return compress_address(self._values['mask']) @property def port(self): @@ -2648,12 +2665,6 @@ class Difference(object): def source(self): if self.want.source is None: return None - want = ip_interface(u'{0}'.format(self.want.source)) - have = ip_interface(u'{0}'.format(self.have.destination_tuple.ip)) - if want.version != have.version: - raise F5ModuleError( - "The source and destination addresses for the virtual server must be be the same type (IPv4 or IPv6)." - ) if self.want.source != self.have.source: return self.want.source @@ -3053,7 +3064,7 @@ class ModuleManager(object): except ValueError as ex: raise F5ModuleError(str(ex)) - if 'code' in response and response['code'] == 400: + if 'code' in response and response['code'] in [400, 404]: if 'message' in response: raise F5ModuleError(response['message']) else: @@ -3097,7 +3108,9 @@ class ModuleManager(object): except ValueError as ex: raise F5ModuleError(str(ex)) - if 'code' in response and response['code'] in [400, 403]: + # Code 404 can occur when you specify a fallback profile that does + # not exist + if 'code' in response and response['code'] in [400, 403, 404]: if 'message' in response: raise F5ModuleError(response['message']) else: diff --git a/lib/ansible/modules/network/f5/bigip_vlan.py b/lib/ansible/modules/network/f5/bigip_vlan.py index d268159e703..7e97c74b931 100644 --- a/lib/ansible/modules/network/f5/bigip_vlan.py +++ b/lib/ansible/modules/network/f5/bigip_vlan.py @@ -140,6 +140,7 @@ options: choices: - reboot - restart-all + - failover version_added: 2.8 sflow_poll_interval: description: @@ -914,7 +915,7 @@ class ArgumentSpec(object): fail_safe=dict(type='bool'), fail_safe_timeout=dict(type='int'), fail_safe_action=dict( - choices=['reboot', 'restart-all'] + choices=['reboot', 'restart-all', 'failover'] ), sflow_poll_interval=dict(type='int'), sflow_sampling_rate=dict(type='int'), diff --git a/test/units/modules/network/f5/test_bigip_user.py b/test/units/modules/network/f5/test_bigip_user.py index b96ff84753e..197a088d4f9 100644 --- a/test/units/modules/network/f5/test_bigip_user.py +++ b/test/units/modules/network/f5/test_bigip_user.py @@ -169,39 +169,6 @@ class TestManager(unittest.TestCase): assert results['changed'] is True assert results['partition_access'] == access - def test_create_user_raises(self, *args): - access = [{'name': 'Common', 'role': 'guest'}] - set_module_args(dict( - username_credential='someuser', - password_credential='testpass', - partition_access=access, - password='password', - server='localhost', - user='admin' - )) - - module = AnsibleModule( - argument_spec=self.spec.argument_spec, - supports_check_mode=self.spec.supports_check_mode - ) - - # Override methods to force specific logic in the module to happen - pm = PartitionedManager(module=module, params=module.params) - pm.create_on_device = Mock(return_value=True) - pm.exists = Mock(return_value=False) - - mm = ModuleManager(module=module) - mm.is_version_less_than_13 = Mock(return_value=False) - mm.get_manager = Mock(return_value=pm) - - msg = "The 'update_password' option " \ - "needs to be set to 'on_create' when creating " \ - "a resource with a password." - - with pytest.raises(F5ModuleError) as ex: - mm.exec_module() - assert str(ex.value) == msg - def test_create_user_partition_access_raises(self, *args): set_module_args(dict( username_credential='someuser', @@ -589,39 +556,6 @@ class TestLegacyManager(unittest.TestCase): assert results['changed'] is True assert results['partition_access'] == access - def test_create_user_raises(self, *args): - access = [{'name': 'Common', 'role': 'guest'}] - set_module_args(dict( - username_credential='someuser', - password_credential='testpass', - partition_access=access, - password='password', - server='localhost', - user='admin' - )) - - module = AnsibleModule( - argument_spec=self.spec.argument_spec, - supports_check_mode=self.spec.supports_check_mode - ) - - # Override methods to force specific logic in the module to happen - upm = UnpartitionedManager(module=module, params=module.params) - upm.create_on_device = Mock(return_value=True) - upm.exists = Mock(return_value=False) - - mm = ModuleManager(module=module) - mm.is_version_less_than_13 = Mock(return_value=True) - mm.get_manager = Mock(return_value=upm) - - msg = "The 'update_password' option " \ - "needs to be set to 'on_create' when creating " \ - "a resource with a password." - - with pytest.raises(F5ModuleError) as ex: - mm.exec_module() - assert str(ex.value) == msg - def test_create_user_partition_access_raises(self, *args): set_module_args(dict( username_credential='someuser',