Fixes, and updates, bigip action plugin and module utils (#34947)

These fixes make provider work across more things. Adds a timeout
value, and makes the action plugin look similar to other network
action plugins

lib/ansible/module_utils/network/f5/common.py

@@ -21,13 +21,34 @@ except ImportError:
 
 
 f5_provider_spec = {
-    'server': dict(fallback=(env_fallback, ['F5_SERVER'])),
+    'server': dict(
-    'server_port': dict(type='int', default=443, fallback=(env_fallback, ['F5_SERVER_PORT'])),
+        fallback=(env_fallback, ['F5_SERVER'])
-    'user': dict(fallback=(env_fallback, ['F5_USER', 'ANSIBLE_NET_USERNAME'])),
+    ),
-    'password': dict(no_log=True, fallback=(env_fallback, ['F5_PASSWORD', 'ANSIBLE_NET_PASSWORD'])),
+    'server_port': dict(
-    'ssh_keyfile': dict(fallback=(env_fallback, ['ANSIBLE_NET_SSH_KEYFILE']), type='path'),
+        type='int',
-    'validate_certs': dict(type='bool', fallback=(env_fallback, ['F5_VALIDATE_CERTS'])),
+        default=443,
-    'transport': dict(default='rest', choices=['cli', 'rest'])
+        fallback=(env_fallback, ['F5_SERVER_PORT'])
+    ),
+    'user': dict(
+        fallback=(env_fallback, ['F5_USER', 'ANSIBLE_NET_USERNAME'])
+    ),
+    'password': dict(
+        no_log=True,
+        fallback=(env_fallback, ['F5_PASSWORD', 'ANSIBLE_NET_PASSWORD'])
+    ),
+    'ssh_keyfile': dict(
+        fallback=(env_fallback, ['ANSIBLE_NET_SSH_KEYFILE']),
+        type='path'
+    ),
+    'validate_certs': dict(
+        type='bool',
+        fallback=(env_fallback, ['F5_VALIDATE_CERTS'])
+    ),
+    'transport': dict(
+        default='rest',
+        choices=['cli', 'rest']
+    ),
+    'timeout': dict(type='int'),
 }
 
 f5_argument_spec = {
@@ -35,12 +56,34 @@ f5_argument_spec = {
 }
 
 f5_top_spec = {
-    'server': dict(removed_in_version=2.9, fallback=(env_fallback, ['F5_SERVER'])),
+    'server': dict(
-    'user': dict(removed_in_version=2.9, fallback=(env_fallback, ['F5_USER', 'ANSIBLE_NET_USERNAME'])),
+        removed_in_version=2.9,
-    'password': dict(removed_in_version=2.9, no_log=True, fallback=(env_fallback, ['F5_PASSWORD'])),
+        fallback=(env_fallback, ['F5_SERVER'])
-    'validate_certs': dict(removed_in_version=2.9, type='bool', fallback=(env_fallback, ['F5_VALIDATE_CERTS'])),
+    ),
-    'server_port': dict(removed_in_version=2.9, type='int', default=443, fallback=(env_fallback, ['F5_SERVER_PORT'])),
+    'user': dict(
-    'transport': dict(removed_in_version=2.9, choices=['cli', 'rest'])
+        removed_in_version=2.9,
+        fallback=(env_fallback, ['F5_USER', 'ANSIBLE_NET_USERNAME'])
+    ),
+    'password': dict(
+        removed_in_version=2.9,
+        no_log=True,
+        fallback=(env_fallback, ['F5_PASSWORD', 'ANSIBLE_NET_PASSWORD'])
+    ),
+    'validate_certs': dict(
+        removed_in_version=2.9,
+        type='bool',
+        fallback=(env_fallback, ['F5_VALIDATE_CERTS'])
+    ),
+    'server_port': dict(
+        removed_in_version=2.9,
+        type='int',
+        default=443,
+        fallback=(env_fallback, ['F5_SERVER_PORT'])
+    ),
+    'transport': dict(
+        removed_in_version=2.9,
+        choices=['cli', 'rest']
+    )
 }
 f5_argument_spec.update(f5_top_spec)
 
@@ -80,7 +123,7 @@ def run_commands(module, commands, check_rc=True):
         cmd = module.jsonify(cmd)
         rc, out, err = exec_command(module, cmd)
         if check_rc and rc != 0:
-            module.fail_json(msg=to_text(err, errors='surrogate_then_replace'), rc=rc)
+            raise F5ModuleError(to_text(err, errors='surrogate_then_replace'))
         responses.append(to_text(out, errors='surrogate_then_replace'))
     return responses
 
@@ -95,6 +138,12 @@ def cleanup_tokens(client):
         pass
 
 
+def is_cli(module):
+    transport = module.params['transport']
+    provider_transport = (module.params['provider'] or {}).get('transport')
+    return 'cli' in (transport, provider_transport)
+
+
 class Noop(object):
     """Represent no-operation required
 

lib/ansible/plugins/action/bigip.py

@@ -25,10 +25,14 @@ import copy
 from ansible import constants as C
 from ansible.module_utils._text import to_text
 from ansible.module_utils.connection import Connection
-from ansible.module_utils.f5_utils import F5_COMMON_ARGS
 from ansible.module_utils.network.common.utils import load_provider
 from ansible.plugins.action.normal import ActionModule as _ActionModule
 
+try:
+    from library.module_utils.network.f5.common import f5_provider_spec
+except:
+    from ansible.module_utils.network.f5.common import f5_provider_spec
+
 try:
     from __main__ import display
 except ImportError:
@@ -43,30 +47,49 @@ class ActionModule(_ActionModule):
 
         display.vvvv('connection transport is %s' % transport, self._play_context.remote_addr)
 
-        if transport == 'cli':
+        if self._play_context.connection == 'network_cli':
-            provider = load_provider(F5_COMMON_ARGS, self._task.args)
+            provider = self._task.args.get('provider', {})
-            self._task.args.pop('provider', None)
+            if any(provider.values()):
-            pc = copy.deepcopy(self._play_context)
+                display.warning('provider is unnecessary when using network_cli and will be ignored')
-            pc.connection = 'network_cli'
+        elif self._play_context.connection == 'local':
-            pc.network_os = 'bigip'
+            provider = load_provider(f5_provider_spec, self._task.args)
-            pc.remote_addr = provider.get('server', self._play_context.remote_addr)
-            pc.port = int(provider['server_port'] or self._play_context.port or 22)
-            pc.remote_user = provider.get('user', self._play_context.connection_user)
-            pc.password = provider.get('password', self._play_context.password)
-            pc.timeout = int(provider.get('timeout', C.PERSISTENT_COMMAND_TIMEOUT))
 
-            display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr)
+            transport = provider['transport'] or 'rest'
-            connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin)
 
-            socket_path = connection.run()
+            display.vvvv('connection transport is %s' % transport, self._play_context.remote_addr)
-            display.vvvv('socket_path: %s' % socket_path, pc.remote_addr)
-            if not socket_path:
-                return {'failed': True,
-                        'msg': 'unable to open shell. Please see: ' +
-                               'https://docs.ansible.com/ansible/network_debug_troubleshooting.html#unable-to-open-shell'}
 
+            if transport == 'cli':
+                pc = copy.deepcopy(self._play_context)
+                pc.connection = 'network_cli'
+                pc.network_os = 'bigip'
+                pc.remote_addr = provider.get('server', self._play_context.remote_addr)
+                pc.port = int(provider['server_port'] or self._play_context.port or 22)
+                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))
+
+                display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr)
+                connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin)
+
+                socket_path = connection.run()
+                display.vvvv('socket_path: %s' % socket_path, pc.remote_addr)
+                if not socket_path:
+                    return {'failed': True,
+                            'msg': 'unable to open shell. Please see: ' +
+                                   'https://docs.ansible.com/ansible/network_debug_troubleshooting.html#unable-to-open-shell'}
+
+                task_vars['ansible_socket'] = socket_path
+            else:
+                self._task.args['provider'] = ActionModule.rest_implementation(provider, self._play_context)
+        else:
+            return {'failed': True, 'msg': 'Connection type %s is not valid for this module' % self._play_context.connection}
+
+        if (self._play_context.connection == 'local' and transport == 'cli') or self._play_context.connection == 'network_cli':
             # make sure we are in the right cli context which should be
             # enable mode and not config module
+            if socket_path is None:
+                socket_path = self._connection.socket_path
             conn = Connection(socket_path)
             out = conn.get_prompt()
             while '(config' in to_text(out, errors='surrogate_then_replace').strip():
@@ -74,7 +97,61 @@ class ActionModule(_ActionModule):
                 conn.send_command('exit')
                 out = conn.get_prompt()
 
-            task_vars['ansible_socket'] = socket_path
-
         result = super(ActionModule, self).run(tmp, task_vars)
         return result
+
+    @staticmethod
+    def rest_implementation(provider, play_context):
+        """Provides a generic argument spec using Play context vars
+
+        This method will return a set of default values to use for connecting
+        to a remote BIG-IP in the event that you do not use either
+
+        * The environment fallback variables F5_USER, F5_PASSWORD, etc
+        * The "provider" spec
+
+        With this "spec" (for lack of a better name) Ansible will attempt
+        to fill in the provider arguments itself using the play context variables.
+        These variables are contained in the list of MAGIC_VARIABLE_MAPPING
+        found in the constants file
+
+        * https://github.com/ansible/ansible/blob/devel/lib/ansible/constants.py
+
+        Therefore, if you do not use the provider nor that environment args, this
+        method here will be populate the "provider" dict with with the necessary
+        F5 connection params, from the following host vars,
+
+        * remote_addr=('ansible_ssh_host', 'ansible_host'),
+        * remote_user=('ansible_ssh_user', 'ansible_user'),
+        * password=('ansible_ssh_pass', 'ansible_password'),
+        * port=('ansible_ssh_port', 'ansible_port'),
+        * timeout=('ansible_ssh_timeout', 'ansible_timeout'),
+        * private_key_file=('ansible_ssh_private_key_file', 'ansible_private_key_file'),
+
+        For example, this may leave your inventory looking like this
+
+          bigip2 ansible_host=1.2.3.4 ansible_port=10443 ansible_user=admin ansible_password=admin
+
+        :param provider:
+        :param play_context:
+        :return:
+        """
+        provider['transport'] = 'rest'
+
+        if provider.get('server') is None:
+            provider['server'] = play_context.remote_addr
+
+        if provider.get('server_port') is None:
+            default_port = provider['server_port'] if provider['server_port'] else 443
+            provider['server_port'] = int(play_context.port or default_port)
+
+        if provider.get('timeout') is None:
+            provider['timeout'] = C.PERSISTENT_COMMAND_TIMEOUT
+
+        if provider.get('user') is None:
+            provider['user'] = play_context.connection_user
+
+        if provider.get('password') is None:
+            provider['password'] = play_context.password
+
+        return provider