Removes bigip_command from the skip file (#32407)

Includes fixes and enhancements to make it unnecessary to include this
module in the skip file

lib/ansible/modules/network/f5/bigip_command.py

@@ -4,14 +4,18 @@
 # 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_command
-short_description: Run arbitrary command on F5 devices.
+short_description: Run arbitrary command on F5 devices
 description:
   - Sends an arbitrary command to an BIG-IP node and returns the results
     read from the device. This module includes an argument that will cause
@@ -85,24 +89,24 @@ author:
   - Tim Rupp (@caphrim007)
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
 - name: run show version on remote devices
   bigip_command:
     commands: show sys version
-    server: "lb.mydomain.com"
+    server: lb.mydomain.com
-    password: "secret"
+    password: secret
-    user: "admin"
+    user: admin
-    validate_certs: "no"
+    validate_certs: no
   delegate_to: localhost
 
 - name: run show version and check to see if output contains BIG-IP
   bigip_command:
     commands: show sys version
     wait_for: result[0] contains BIG-IP
-    server: "lb.mydomain.com"
+    server: lb.mydomain.com
-    password: "secret"
+    password: secret
-    user: "admin"
+    user: admin
-    validate_certs: "no"
+    validate_certs: no
   delegate_to: localhost
 
 - name: run multiple commands on remote nodes
@@ -110,10 +114,10 @@ EXAMPLES = '''
     commands:
       - show sys version
       - list ltm virtual
-    server: "lb.mydomain.com"
+    server: lb.mydomain.com
-    password: "secret"
+    password: secret
-    user: "admin"
+    user: admin
-    validate_certs: "no"
+    validate_certs: no
   delegate_to: localhost
 
 - name: run multiple commands and evaluate the output
@@ -124,10 +128,10 @@ EXAMPLES = '''
     wait_for:
       - result[0] contains BIG-IP
       - result[1] contains my-vs
-    server: "lb.mydomain.com"
+    server: lb.mydomain.com
-    password: "secret"
+    password: secret
-    user: "admin"
+    user: admin
-    validate_certs: "no"
+    validate_certs: no
   delegate_to: localhost
 
 - name: tmsh prefixes will automatically be handled
@@ -135,41 +139,45 @@ EXAMPLES = '''
     commands:
       - show sys version
       - tmsh list ltm virtual
-    server: "lb.mydomain.com"
+    server: lb.mydomain.com
-    password: "secret"
+    password: secret
-    user: "admin"
+    user: admin
-    validate_certs: "no"
+    validate_certs: no
   delegate_to: localhost
 '''
 
-RETURN = '''
+RETURN = r'''
 stdout:
-    description: The set of responses from the commands
+  description: The set of responses from the commands
-    returned: always
+  returned: always
-    type: list
+  type: list
-    sample: ['...', '...']
+  sample: ['...', '...']
-
 stdout_lines:
-    description: The value of stdout split into a list
+  description: The value of stdout split into a list
-    returned: always
+  returned: always
-    type: list
+  type: list
-    sample: [['...', '...'], ['...'], ['...']]
+  sample: [['...', '...'], ['...'], ['...']]
-
 failed_conditions:
-    description: The list of conditionals that have failed
+  description: The list of conditionals that have failed
-    returned: failed
+  returned: failed
-    type: list
+  type: list
-    sample: ['...', '...']
+  sample: ['...', '...']
 '''
 
+import re
 import time
 
 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
-from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
+
-from ansible.module_utils.f5_utils import run_commands
+try:
+    from ansible.module_utils.f5_utils import run_commands
+    HAS_CLI_TRANSPORT = True
+except ImportError:
+    HAS_CLI_TRANSPORT = False
+
 from ansible.module_utils.netcli import FailedConditionsError
 from ansible.module_utils.six import string_types
 from ansible.module_utils.netcli import Conditional
@@ -177,6 +185,11 @@ from ansible.module_utils.network_common import ComplexList
 from ansible.module_utils.network_common import to_list
 from collections import deque
 
+try:
+    from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
+except ImportError:
+    HAS_F5SDK = False
+
 
 class Parameters(AnsibleF5Parameters):
     returnables = ['stdout', 'stdout_lines', 'warnings']
@@ -198,6 +211,10 @@ class Parameters(AnsibleF5Parameters):
             commands = map(self._ensure_tmsh_prefix, list(commands))
         return list(commands)
 
+    @property
+    def user_commands(self):
+        return map(self._ensure_tmsh_prefix, list(self._values['commands']))
+
     def _ensure_tmsh_prefix(self, cmd):
         cmd = cmd.strip()
         if cmd[0:5] != 'tmsh ':
@@ -234,16 +251,17 @@ class ModuleManager(object):
         result = dict()
 
         try:
-            self.execute()
+            changed = self.execute()
         except iControlUnexpectedHTTPError as e:
             raise F5ModuleError(str(e))
 
         result.update(**self.changes.to_return())
-        result.update(dict(changed=True))
+        result.update(dict(changed=changed))
         return result
 
     def execute(self):
         warnings = list()
+        changed = ('tmsh modify', 'tmsh create', 'tmsh delete')
 
         commands = self.parse_commands(warnings)
 
@@ -256,7 +274,7 @@ class ModuleManager(object):
             return
 
         while retries > 0:
-            if self.client.module.params['transport'] == 'cli':
+            if self.client.module.params['transport'] == 'cli' and HAS_CLI_TRANSPORT:
                 responses = run_commands(self.client.module, self.want.commands)
             else:
                 responses = self.execute_on_device(commands)
@@ -283,6 +301,9 @@ class ModuleManager(object):
             'stdout_lines': self._to_lines(responses),
             'warnings': warnings
         })
+        if any(x for x in self.want.user_commands if x.startswith(changed)):
+            return True
+        return False
 
     def parse_commands(self, warnings):
         results = []
@@ -315,10 +336,12 @@ class ModuleManager(object):
 
     def execute_on_device(self, commands):
         responses = []
+        escape_patterns = r'([$' + "'])"
         for item in to_list(commands):
+            command = re.sub(escape_patterns, r'\\\1', item['command'])
             output = self.client.api.tm.util.bash.exec_cmd(
                 'run',
-                utilCmdArgs='-c "{0}"'.format(item['command'])
+                utilCmdArgs='-c "{0}"'.format(command)
             )
             if hasattr(output, 'commandResult'):
                 responses.append(str(output.commandResult))

test/sanity/import/skip.txt

@@ -14,7 +14,6 @@ lib/ansible/modules/cloud/webfaction/webfaction_mailbox.py
 lib/ansible/modules/cloud/webfaction/webfaction_site.py
 lib/ansible/modules/clustering/consul_acl.py
 lib/ansible/modules/network/cloudengine/ce_file_copy.py
-lib/ansible/modules/network/f5/bigip_command.py
 lib/ansible/modules/network/f5/bigip_config.py
 lib/ansible/modules/network/f5/bigip_configsync_actions.py
 lib/ansible/modules/network/f5/bigip_gtm_pool.py

test/units/modules/network/f5/test_bigip_command.py

@@ -1,21 +1,7 @@
 # -*- coding: utf-8 -*-
 #
-# Copyright 2017 F5 Networks Inc.
+# Copyright (c) 2017 F5 Networks Inc.
-#
+# GNU General Public License v3.0 (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
@@ -38,11 +24,13 @@ try:
     from library.bigip_command import Parameters
     from library.bigip_command import ModuleManager
     from library.bigip_command import ArgumentSpec
+    from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
 except ImportError:
     try:
         from ansible.modules.network.f5.bigip_command import Parameters
         from ansible.modules.network.f5.bigip_command import ModuleManager
         from ansible.modules.network.f5.bigip_command import ArgumentSpec
+        from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
     except ImportError:
         raise SkipTest("F5 Ansible modules require the f5-sdk Python library")
 
@@ -111,6 +99,29 @@ class TestManager(unittest.TestCase):
 
         results = mm.exec_module()
 
+        assert results['changed'] is False
+        self.assertEqual(self.run_commands.call_count, 0)
+        self.assertEqual(self.execute_on_device.call_count, 1)
+
+    def test_run_single_modification_command(self, *args):
+        set_module_args(dict(
+            commands=[
+                "tmsh create ltm virtual foo"
+            ],
+            server='localhost',
+            user='admin',
+            password='password'
+        ))
+
+        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)
+
+        results = mm.exec_module()
+
         assert results['changed'] is True
         self.assertEqual(self.run_commands.call_count, 0)
         self.assertEqual(self.execute_on_device.call_count, 1)
@@ -131,6 +142,6 @@ class TestManager(unittest.TestCase):
             f5_product_name=self.spec.f5_product_name
         )
         mm = ModuleManager(client)
-        results = mm.exec_module()
+        mm.exec_module()
         self.assertEqual(self.run_commands.call_count, 1)
         self.assertEqual(self.execute_on_device.call_count, 0)