Junos fixes (#22423)

* Fixes for junos_config errors

* Check transport settings for core Junos

* Don't pop from the same list you iterate over

* use of persistent connections are now explicitly enabled in junos

* modules must now explicitly enable persistent connections
* adds rpc support to junos_command

fixes #22166

lib/ansible/module_utils/junos.py

@@ -18,7 +18,7 @@
 #
 from contextlib import contextmanager
 
-from ncclient.xml_ import new_ele, sub_ele, to_xml
+from xml.etree.ElementTree import Element, SubElement, tostring
 
 from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.netconf import send_request
@@ -41,6 +41,7 @@ junos_argument_spec = {
     'ssh_keyfile': dict(fallback=(env_fallback, ['ANSIBLE_NET_SSH_KEYFILE']), type='path'),
     'timeout': dict(type='int', default=10),
     'provider': dict(type='dict'),
+    'transport': dict(choices=['cli', 'netconf'])
 }
 
 def check_args(module, warnings):
@@ -81,17 +82,17 @@ def load_configuration(module, candidate=None, action='merge', rollback=None, fo
     else:
         xattrs = {'action': action, 'format': format}
 
-    obj = new_ele('load-configuration', xattrs)
+    obj = Element('load-configuration', xattrs)
 
     if candidate is not None:
         lookup = {'xml': 'configuration', 'text': 'configuration-text',
                   'set': 'configuration-set', 'json': 'configuration-json'}
 
         if action == 'set':
-            cfg = sub_ele(obj, 'configuration-set')
+            cfg = SubElement(obj, 'configuration-set')
             cfg.text = '\n'.join(candidate)
         else:
-            cfg = sub_ele(obj, lookup[format])
+            cfg = SubElement(obj, lookup[format])
             cfg.append(candidate)
 
     return send_request(module, obj)
@@ -104,22 +105,22 @@ def get_configuration(module, compare=False, format='xml', rollback='0'):
         validate_rollback_id(rollback)
         xattrs['compare'] = 'rollback'
         xattrs['rollback'] = str(rollback)
-    return send_request(module, new_ele('get-configuration', xattrs))
+    return send_request(module, Element('get-configuration', xattrs))
 
 def commit_configuration(module, confirm=False, check=False, comment=None, confirm_timeout=None):
-    obj = new_ele('commit-configuration')
+    obj = Element('commit-configuration')
     if confirm:
-        sub_ele(obj, 'confirmed')
+        SubElement(obj, 'confirmed')
     if check:
-        sub_ele(obj, 'check')
+        SubElement(obj, 'check')
     if comment:
         children(obj, ('log', str(comment)))
     if confirm_timeout:
         children(obj, ('confirm-timeout', int(confirm_timeout)))
     return send_request(module, obj)
 
-lock_configuration = lambda x: send_request(x, new_ele('lock-configuration'))
-unlock_configuration = lambda x: send_request(x, new_ele('unlock-configuration'))
+lock_configuration = lambda x: send_request(x, Element('lock-configuration'))
+unlock_configuration = lambda x: send_request(x, Element('unlock-configuration'))
 
 @contextmanager
 def locked_config(module):
@@ -131,7 +132,7 @@ def locked_config(module):
 
 def get_diff(module):
     reply = get_configuration(module, compare=True, format='text')
-    output = reply.xpath('//configuration-output')
+    output = reply.find('.//configuration-output')
     if output:
         return output[0].text
 

lib/ansible/module_utils/netconf.py

@@ -26,50 +26,50 @@
 # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 from contextlib import contextmanager
-
-from ncclient.xml_ import new_ele, sub_ele, to_xml, to_ele
+from xml.etree.ElementTree import Element, SubElement
+from xml.etree.ElementTree import tostring, fromstring
 
 from ansible.module_utils.connection import exec_command
 
 def send_request(module, obj, check_rc=True):
-    request = to_xml(obj)
+    request = tostring(obj)
     rc, out, err = exec_command(module, request)
     if rc != 0:
         if check_rc:
             module.fail_json(msg=str(err))
-        return to_ele(err)
-    return to_ele(out)
+        return fromstring(out)
+    return fromstring(out)
 
 def children(root, iterable):
     for item in iterable:
         try:
-            ele = sub_ele(ele, item)
+            ele = SubElement(ele, item)
         except NameError:
-            ele = sub_ele(root, item)
+            ele = SubElement(root, item)
 
 def lock(module, target='candidate'):
-    obj = new_ele('lock')
+    obj = Element('lock')
     children(obj, ('target', target))
     return send_request(module, obj)
 
 def unlock(module, target='candidate'):
-    obj = new_ele('unlock')
+    obj = Element('unlock')
     children(obj, ('target', target))
     return send_request(module, obj)
 
 def commit(module):
-    return send_request(module, new_ele('commit'))
+    return send_request(module, Element('commit'))
 
 def discard_changes(module):
-    return send_request(module, new_ele('discard-changes'))
+    return send_request(module, Element('discard-changes'))
 
 def validate(module):
-    obj = new_ele('validate')
+    obj = Element('validate')
     children(obj, ('source', 'candidate'))
     return send_request(module, obj)
 
 def get_config(module, source='running', filter=None):
-    obj = new_ele('get-config')
+    obj = Element('get-config')
     children(obj, ('source', source))
     children(obj, ('filter', filter))
     return send_request(module, obj)

lib/ansible/modules/network/junos/_junos_template.py

@@ -116,8 +116,16 @@ from ansible.module_utils.junos import check_args, junos_argument_spec
 from ansible.module_utils.junos import get_configuration, load
 from ansible.module_utils.six import text_type
 
+USE_PERSISTENT_CONNECTION = True
 DEFAULT_COMMENT = 'configured by junos_template'
 
+def check_transport(module):
+    transport = (module.params['provider'] or {}).get('transport')
+
+    if transport == 'netconf':
+        module.fail_json(msg='junos_template module is only supported over cli transport')
+
+
 def main():
 
     argument_spec = dict(
@@ -127,7 +135,6 @@ def main():
         action=dict(default='merge', choices=['merge', 'overwrite', 'replace']),
         config_format=dict(choices=['text', 'set', 'xml']),
         backup=dict(default=False, type='bool'),
-        transport=dict(default='netconf', choices=['netconf'])
     )
 
     argument_spec.update(junos_argument_spec)
@@ -135,6 +142,8 @@ def main():
     module = AnsibleModule(argument_spec=argument_spec,
                            supports_check_mode=True)
 
+    check_transport(module)
+
     warnings = list()
     check_args(module, warnings)
 

lib/ansible/modules/network/junos/junos_command.py

@@ -127,17 +127,21 @@ failed_conditions:
   sample: ['...', '...']
 """
 import time
+import re
+import shlex
 
 from functools import partial
 from xml.etree import ElementTree as etree
+from xml.etree.ElementTree import Element, SubElement, tostring
+
 
 from ansible.module_utils.junos import run_commands
-from ansible.module_utils.junos import junos_argument_spec
-from ansible.module_utils.junos import check_args as junos_check_args
+from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six import string_types
 from ansible.module_utils.netcli import Conditional, FailedConditionalError
-from ansible.module_utils.network_common import ComplexList
+from ansible.module_utils.netconf import send_request
+from ansible.module_utils.network_common import ComplexList, to_list
+from ansible.module_utils.six import string_types, iteritems
 
 try:
     import jxmlease
@@ -145,12 +149,22 @@ try:
 except ImportError:
     HAS_JXMLEASE = False
 
-def check_args(module, warnings):
-    junos_check_args(module, warnings)
+USE_PERSISTENT_CONNECTION = True
+
+
+VALID_KEYS = {
+    'cli': frozenset(['command', 'output', 'prompt', 'response']),
+    'rpc': frozenset(['command', 'output'])
+}
+
+def check_transport(module):
+    transport = (module.params['provider'] or {}).get('transport')
 
-    if module.params['rpcs']:
-        module.fail_json(msg='argument rpcs has been deprecated, please use '
-                             'junos_rpc instead')
+    if transport == 'netconf' and not module.params['rpcs']:
+        module.fail_json(msg='argument commands is only supported over cli transport')
+
+    elif transport == 'cli' and not module.params['commands']:
+        module.fail_json(msg='argument rpcs is only supported over netconf transport')
 
 def to_lines(stdout):
     lines = list()
@@ -160,7 +174,78 @@ def to_lines(stdout):
         lines.append(item)
     return lines
 
-def parse_commands(module, warnings):
+def run_rpcs(module, items):
+
+    responses = list()
+
+    for item in items:
+        name = item['name']
+        args = item['args']
+
+        name = str(name).replace('_', '-')
+
+        if all((module.check_mode, not name.startswith('get'))):
+            module.fail_json(msg='invalid rpc for running in check_mode')
+
+        xattrs = {'format': item['output']}
+
+        element = Element(name, xattrs)
+
+        for key, value in iteritems(args):
+            key = str(key).replace('_', '-')
+            if isinstance(value, list):
+                for item in value:
+                    child = SubElement(element, key)
+                    if item is not True:
+                        child.text = item
+            else:
+                child = SubElement(element, key)
+                if value is not True:
+                    child.text = value
+
+        reply = send_request(module, element)
+
+        if module.params['display'] == 'text':
+            data = reply.find('.//output')
+            responses.append(data.text.strip())
+        elif module.params['display'] == 'json':
+            responses.append(module.from_json(reply.text.strip()))
+        else:
+            responses.append(tostring(reply))
+
+    return responses
+
+def split(value):
+    lex = shlex.shlex(value)
+    lex.quotes = '"'
+    lex.whitespace_split = True
+    lex.commenters = ''
+    return list(lex)
+
+def parse_rpcs(module):
+    items = list()
+    for rpc in module.params['rpcs']:
+        parts = split(rpc)
+
+        name = parts.pop(0)
+        args = dict()
+
+        for item in parts:
+            key, value = item.split('=')
+            if str(value).upper() in ['TRUE', 'FALSE']:
+                args[key] = bool(value)
+            elif re.match(r'^[0-9]+$', value):
+                args[key] = int(value)
+            else:
+                args[key] = str(value)
+
+        output = module.params['display'] or 'xml'
+        items.append({'name': name, 'args': args, 'output': output})
+
+    return items
+
+
+def parse_commands(module):
     spec = dict(
         command=dict(key=True),
         output=dict(default=module.params['display'], choices=['text', 'json', 'xml']),
@@ -178,6 +263,8 @@ def parse_commands(module, warnings):
                 'executing %s' % item['command']
             )
 
+        if item['command'].startswith('show configuration'):
+            item['output'] = 'text'
         if item['output'] == 'json' and 'display json' not in item['command']:
             item['command'] += '| display json'
         elif item['output'] == 'xml' and 'display xml' not in item['command']:
@@ -195,12 +282,11 @@ def main():
     """entry point for module execution
     """
     argument_spec = dict(
-        commands=dict(type='list', required=True),
-        display=dict(choices=['text', 'json', 'xml'], default='text', aliases=['format', 'output']),
-
-        # deprecated (Ansible 2.3) - use junos_rpc
+        commands=dict(type='list'),
         rpcs=dict(type='list'),
 
+        display=dict(choices=['text', 'json', 'xml'], aliases=['format', 'output']),
+
         wait_for=dict(type='list', aliases=['waitfor']),
         match=dict(default='all', choices=['all', 'any']),
 
@@ -210,14 +296,25 @@ def main():
 
     argument_spec.update(junos_argument_spec)
 
+    mutually_exclusive = [('commands', 'rpcs')]
+
+    required_one_of = [('commands', 'rpcs')]
+
     module = AnsibleModule(argument_spec=argument_spec,
+                           mutually_exclusive=mutually_exclusive,
+                           required_one_of=required_one_of,
                            supports_check_mode=True)
 
+    check_transport(module)
 
     warnings = list()
     check_args(module, warnings)
 
-    commands = parse_commands(module, warnings)
+    if module.params['commands']:
+        items = parse_commands(module)
+    else:
+        items = parse_rpcs(module)
+
 
     wait_for = module.params['wait_for'] or list()
     display = module.params['display']
@@ -228,21 +325,29 @@ def main():
     match = module.params['match']
 
     while retries > 0:
-        responses = run_commands(module, commands)
+        if module.params['commands']:
+            responses = run_commands(module, items)
+        else:
+            responses = run_rpcs(module, items)
 
-        for index, (resp, cmd) in enumerate(zip(responses, commands)):
-            if cmd['output'] == 'xml':
+        transformed = list()
+
+        for item, resp in zip(items, responses):
+            if item['output'] == 'xml':
                 if not HAS_JXMLEASE:
                     module.fail_json(msg='jxmlease is required but does not appear to '
                         'be installed.  It can be installed using `pip install jxmlease`')
+
                 try:
-                    responses[index] = jxmlease.parse(resp)
+                    transformed.append(jxmlease.parse(resp))
                 except:
                     raise ValueError(resp)
+            else:
+                transformed.append(resp)
 
         for item in list(conditionals):
             try:
-                if item(responses):
+                if item(transformed):
                     if match == 'any':
                         conditionals = list()
                         break

lib/ansible/modules/network/junos/junos_config.py

@@ -178,18 +178,24 @@ import re
 import json
 
 from xml.etree import ElementTree
-from ncclient.xml_ import to_xml
 
-from ansible.module_utils.junos import get_diff, load
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.junos import get_config, get_diff, load_config
 from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.junos import check_args as junos_check_args
-from ansible.module_utils.junos import locked_config, load_configuration
-from ansible.module_utils.junos import get_configuration
-from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.netcfg import NetworkConfig
+from ansible.module_utils.six import string_types
 
+USE_PERSISTENT_CONNECTION = True
 DEFAULT_COMMENT = 'configured by junos_config'
 
+def check_transport(module):
+    transport = (module.params['provider'] or {}).get('transport')
+
+    if transport == 'netconf':
+        module.fail_json(msg='junos_config module is only supported over cli transport')
+
+
 def check_args(module, warnings):
     junos_check_args(module, warnings)
     if module.params['zeroize']:
@@ -219,7 +225,7 @@ def guess_format(config):
 
 def config_to_commands(config):
     set_format = config.startswith('set') or config.startswith('delete')
-    candidate = NetworkConfig(indent=4, contents=config, device_os='junos')
+    candidate = NetworkConfig(indent=4, contents=config)
     if not set_format:
         candidate = [c.line for c in candidate.items]
         commands = list()
@@ -237,23 +243,21 @@ def config_to_commands(config):
     return commands
 
 def filter_delete_statements(module, candidate):
-    reply = get_configuration(module, format='set')
-    config = reply.xpath('//configuration-set')[0].text.strip()
+    config = get_config(module)
+
+    modified_candidate = candidate[:]
     for index, line in enumerate(candidate):
         if line.startswith('delete'):
             newline = re.sub('^delete', 'set', line)
             if newline not in config:
-                del candidate[index]
-    return candidate
+                del modified_candidate[index]
+    return modified_candidate
 
-def load_config(module):
-    candidate =  module.params['lines'] or module.params['src']
-    if isinstance(candidate, basestring):
+def load(module):
+    candidate = module.params['lines'] or module.params['src']
+    if isinstance(candidate, string_types):
         candidate = candidate.split('\n')
 
-    confirm = module.params['confirm'] > 0
-    confirm_timeout = module.params['confirm']
-
     kwargs = {
         'confirm': module.params['confirm'] is not None,
         'confirm_timeout': module.params['confirm'],
@@ -269,30 +273,15 @@ def load_config(module):
     # nothing in the config as that will cause an exception to be raised
     if module.params['lines']:
         candidate = filter_delete_statements(module, candidate)
-        kwargs.update({'action': 'set', 'format': 'text'})
 
-    return load(module, candidate, **kwargs)
-
-def rollback_config(module, result):
-    rollback = module.params['rollback']
-    diff = None
-
-    with locked_config:
-        load_configuration(module, rollback=rollback)
-        diff = get_diff(module)
-
-    return diff
-
-def confirm_config(module):
-    with locked_config:
-        commit_configuration(confirm=True)
+    return load_config(module, candidate, **kwargs)
 
 def update_result(module, result, diff=None):
     if diff == '':
         diff = None
     result['changed'] = diff is not None
     if module._diff:
-        result['diff'] =  {'prepared': diff}
+        result['diff'] = {'prepared': diff}
 
 
 def main():
@@ -329,23 +318,22 @@ def main():
                            mutually_exclusive=mutually_exclusive,
                            supports_check_mode=True)
 
+    check_transport(module)
+
     warnings = list()
     check_args(module, warnings)
 
     result = {'changed': False, 'warnings': warnings}
 
     if module.params['backup']:
-        result['__backup__'] = get_configuration()
+        result['__backup__'] = get_config(module)
 
     if module.params['rollback']:
         diff = get_diff(module)
         update_result(module, result, diff)
 
-    elif not any((module.params['src'], module.params['lines'])):
-        confirm_config(module)
-
     else:
-        diff = load_config(module)
+        diff = load(module)
         update_result(module, result, diff)
 
     module.exit_json(**result)

lib/ansible/modules/network/junos/junos_netconf.py

@@ -82,6 +82,14 @@ from ansible.module_utils.junos import junos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.six import iteritems
 
+USE_PERSISTENT_CONNECTION = True
+
+def check_transport(module):
+    transport = (module.params['provider'] or {}).get('transport')
+
+    if transport == 'netconf':
+        module.fail_json(msg='junos_netconf module is only supported over cli transport')
+
 
 def map_obj_to_commands(updates, module):
     want, have = updates
@@ -145,10 +153,13 @@ def main():
     )
 
     argument_spec.update(junos_argument_spec)
+    argument_spec['transport'] = dict(choices=['cli'], default='cli')
 
     module = AnsibleModule(argument_spec=argument_spec,
                            supports_check_mode=True)
 
+    check_transport(module)
+
     warnings = list()
     check_args(module, warnings)
 
@@ -163,7 +174,7 @@ def main():
     if commands:
         commit = not module.check_mode
         diff = load_config(module, commands, commit=commit)
-        if diff and module._diff:
+        if diff:
             if module._diff:
                 result['diff'] = {'prepared': diff}
             result['changed'] = True

lib/ansible/plugins/action/junos.py

@@ -25,7 +25,7 @@ import copy
 
 from ansible.plugins.action.normal import ActionModule as _ActionModule
 from ansible.utils.path import unfrackpath
-from ansible.plugins import connection_loader
+from ansible.plugins import connection_loader, module_loader
 from ansible.compat.six import iteritems
 from ansible.module_utils.junos import junos_argument_spec
 from ansible.module_utils.basic import AnsibleFallbackNotFound
@@ -50,12 +50,18 @@ class ActionModule(_ActionModule):
                     'got %s' % self._play_context.connection
             )
 
+        module = module_loader._load_module_source(self._task.action, module_loader.find_plugin(self._task.action))
+
+        if not getattr(module, 'USE_PERSISTENT_CONNECTION', False):
+            return super(ActionModule, self).run(tmp, task_vars)
+
         provider = self.load_provider()
+        transport = provider['transport'] or 'cli'
 
         pc = copy.deepcopy(self._play_context)
         pc.network_os = 'junos'
 
-        if self._task.action in ('junos_command', 'junos_netconf', 'junos_config', '_junos_template'):
+        if transport == 'cli':
             pc.connection = 'network_cli'
             pc.port = provider['port'] or self._play_context.port or 22
         else:
@@ -82,6 +88,7 @@ class ActionModule(_ActionModule):
 
             if rc != 0:
                 return {'failed': True, 'msg': 'unable to connect to control socket'}
+
         elif pc.connection == 'network_cli':
             # make sure we are in the right cli context which should be
             # enable mode and not config module