nxos_file_copy network_cli and httpapi fix (#40592)

* Leverage action plugin to pass credentials to nxos_file_copy for network_cli, httpapi

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* update integration test

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* make sure local test uses provider

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* update tests

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* update doc

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* clarify action plugin comment

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Add connection=local back to nxos_file_copy because that module is weird

Also blacklist it running on nxapi, because that is meaningless

* remove provider

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* blacklist nxapi

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* Address review comment

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

lib/ansible/modules/network/nxos/nxos_file_copy.py

@@ -28,7 +28,8 @@ version_added: "2.2"
 short_description: Copy a file to a remote NXOS device over SCP.
 description:
   - Copy a file to the flash (or bootflash) remote network device
-    on NXOS devices.
+    on NXOS devices. This module only supports the use of connection
+    C(network_cli) or C(Cli) transport with connection C(local).
 author:
   - Jason Edelman (@jedelman8)
   - Gabriele Gerbino (@GGabriele)
@@ -65,8 +66,6 @@ EXAMPLES = '''
 - nxos_file_copy:
     local_file: "./test_file.txt"
     remote_file: "test_file.txt"
-    provider: "{{ cli }}"
-    connect_ssh_port: "{{ ansible_ssh_port }}"
 '''
 
 RETURN = '''
@@ -153,12 +152,10 @@ def transfer_file(module, dest):
     if not enough_space(module):
         module.fail_json(msg='Could not transfer file. Not enough space on device.')
 
-    provider = module.params['provider']
+    hostname = module.params['host']
-
+    username = module.params['username']
-    hostname = module.params.get('host') or provider.get('host')
+    password = module.params['password']
-    username = module.params.get('username') or provider.get('username')
+    port = module.params['connect_ssh_port']
-    password = module.params.get('password') or provider.get('password')
-    port = module.params.get('connect_ssh_port')
 
     ssh = paramiko.SSHClient()
     ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())

lib/ansible/plugins/action/nxos.py

@@ -44,9 +44,17 @@ class ActionModule(_ActionModule):
         socket_path = None
 
         if (self._play_context.connection == 'httpapi' or self._task.args.get('provider', {}).get('transport') == 'nxapi') \
-                and self._task.action == 'nxos_nxapi':
+                and self._task.action in ('nxos_file_copy', 'nxos_nxapi'):
             return {'failed': True, 'msg': "Transport type 'nxapi' is not valid for '%s' module." % (self._task.action)}
 
+        if self._task.action == 'nxos_file_copy':
+            self._task.args['host'] = self._play_context.remote_addr
+            self._task.args['password'] = self._play_context.password
+            if self._play_context.connection == 'network_cli':
+                self._task.args['username'] = self._play_context.remote_user
+            elif self._play_context.connection == 'local':
+                self._task.args['username'] = self._play_context.connection_user
+
         if self._play_context.connection in ('network_cli', 'httpapi'):
             provider = self._task.args.get('provider', {})
             if any(provider.values()):
@@ -55,6 +63,7 @@ class ActionModule(_ActionModule):
             if self._task.args.get('transport'):
                 display.warning('transport is unnecessary when using %s and will be ignored' % self._play_context.connection)
                 del self._task.args['transport']
+
         elif self._play_context.connection == 'local':
             provider = load_provider(nxos_provider_spec, self._task.args)
             transport = provider['transport'] or 'cli'

test/integration/targets/nxos_file_copy/tasks/cli.yaml

@@ -1,5 +1,5 @@
 ---
-- name: collect common cli test cases
+- name: collect common test cases
   find:
     paths: "{{ role_path }}/tests/common"
     patterns: "{{ testcase }}.yaml"
@@ -21,13 +21,13 @@
   set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
 
 - name: run test cases (connection=network_cli)
-  include: "{{ test_case_to_run }} ansible_connection=network_cli connection={}"
+  include: "{{ test_case_to_run }} ansible_connection=network_cli"
   with_items: "{{ test_items }}"
   loop_control:
     loop_var: test_case_to_run
 
-- name: run test case (connection=local)
+- name: run test cases (connection=local)
-  include: "{{ test_case_to_run }} ansible_connection=local connection={{ cli }}"
+  include: "{{ test_case_to_run }} ansible_connection=local"
-  with_first_found: "{{ test_items }}"
+  with_items: "{{ test_items }}"
   loop_control:
     loop_var: test_case_to_run

test/integration/targets/nxos_file_copy/tasks/nxapi.yaml

@@ -25,3 +25,9 @@
   with_items: "{{ test_items }}"
   loop_control:
     loop_var: test_case_to_run
+
+- name: run test cases (connection=local)
+  include: "{{ test_case_to_run }} ansible_connection=local"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run

test/integration/targets/nxos_file_copy/tests/cli/sanity.yaml

@@ -1,7 +1,5 @@
 ---
-- debug: msg="START TRANSPORT:CLI nxos_file_copy sanity test"
+- debug: msg="START connection={{ ansible_connection }} nxos_file_copy sanity test"
-- debug: msg="Using provider={{ connection.transport }}"
-  when: ansible_connection == "local"
 
 - name: "Setup - Remove existing file"
   nxos_command: &remove_file
@@ -20,9 +18,6 @@
     nxos_file_copy: &copy_file_same_name
       local_file: "./network-integration.cfg"
       file_system: "bootflash:"
-      username: "{{ ansible_ssh_user }}"
-      password: "{{ ansible_ssh_pass }}"
-      host: "{{ ansible_host }}"
       connect_ssh_port: "{{ ansible_ssh_port }}"
     register: result
 
@@ -47,9 +42,6 @@
       local_file: "./inventory.networking.template"
       remote_file: "network-integration.cfg"
       file_system: "bootflash:"
-      username: "{{ ansible_ssh_user }}"
-      password: "{{ ansible_ssh_pass }}"
-      host: "{{ ansible_host }}"
       connect_ssh_port: "{{ ansible_ssh_port }}"
     register: result
 
@@ -80,4 +72,4 @@
       feature: scp-server
       state: disabled
 
-  - debug: msg="END TRANSPORT:CLI nxos_file_copy sanity test"  
+  - debug: msg="END connection={{ ansible_connection }} nxos_file_copy sanity test"

test/integration/targets/nxos_file_copy/tests/nxapi/badtransport.yaml

@@ -0,0 +1,17 @@
+---
+- debug: msg="START nxapi/badtransport.yaml"
+
+- name: Sending transport other than cli should fail
+  nxos_file_copy:
+    local_file: "./network-integration.cfg"
+    file_system: "bootflash:"
+    connect_ssh_port: "{{ ansible_ssh_port }}"
+    provider: "{{ nxapi }}"
+  register: result
+  ignore_errors: yes
+
+- assert:
+     that:
+        - result.failed and result.msg is search('Transport')
+
+- debug: msg="END nxapi/badtransport.yaml"

test/integration/targets/nxos_file_copy/tests/nxapi/sanity.yaml

@@ -1,87 +0,0 @@
----
-- debug: msg="START TRANSPORT:NXAPI nxos_file_copy sanity test"
-- debug: msg="Using provider={{ connection.transport }}"
-  when: ansible_connection == "local"
-
-- name: "Setup - Remove existing file"
-  nxos_command: &remove_file
-    commands:
-      - command: terminal dont-ask
-        output: text
-      - command: delete network-integration.cfg
-        output: text
-  ignore_errors: yes
-
-- name: "Setup - Turn on feature scp-server"
-  nxos_feature: 
-    feature: scp-server
-    state: enabled
-
-- block:
-  - name: "Copy network-integration.cfg to bootflash"
-    nxos_file_copy: &copy_file_same_name
-      local_file: "./network-integration.cfg"
-      file_system: "bootflash:"
-      username: "{{ ansible_ssh_user }}"
-      password: "{{ ansible_ssh_pass }}"
-      host: "{{ ansible_host }}"
-      connect_ssh_port: "{{ ansible_ssh_port }}"
-    register: result
-
-  - assert: &true
-      that:
-        - "result.changed == true"
-
-  - name: "Check Idempotence - Copy network-integration.cfg to bootflash"
-    nxos_file_copy: *copy_file_same_name
-    register: result
-
-  - assert: &false
-      that:
-        - "result.changed == false"
-
-  - name: "Setup - Remove existing file"
-    nxos_command: *remove_file
-    register: result
-    ignore_errors: yes
-
-  - name: "Copy inventory.networking.template to bootflash as another name"
-    nxos_file_copy: &copy_file_different_name
-      local_file: "./inventory.networking.template"
-      remote_file: "network-integration.cfg"
-      file_system: "bootflash:"
-      username: "{{ ansible_ssh_user }}"
-      password: "{{ ansible_ssh_pass }}"
-      host: "{{ ansible_host }}"
-      connect_ssh_port: "{{ ansible_ssh_port }}"
-    register: result
-
-  - assert: *true
-
-  - name: "Check Idempotence - Copy inventory.networking.template to bootflash as another name"
-    nxos_file_copy: *copy_file_different_name
-    register: result
-
-  - assert: *false
-
-  - name: "Setup - Remove existing file"
-    nxos_command: *remove_file
-    register: result
-    ignore_errors: yes
-
-  rescue:
-
-  - debug: msg="TRANSPORT:NXAPI nxos_file_copy failure detected"
-
-  always:
-
-  - name: "Remove file"
-    nxos_command: *remove_file
-    ignore_errors: yes
-
-  - name: "Turn off feature scp-server"
-    nxos_feature:
-      feature: scp-server
-      state: disabled
-
-  - debug: msg="END TRANSPORT:NXAPI nxos_file_copy sanity test"