From c2f7f36fc5a476a8f16a8d9c47afbc4e24f696c6 Mon Sep 17 00:00:00 2001 From: Trishna Guha Date: Thu, 24 May 2018 20:21:18 +0530 Subject: [PATCH] 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 * update integration test Signed-off-by: Trishna Guha * make sure local test uses provider Signed-off-by: Trishna Guha * update tests Signed-off-by: Trishna Guha * update doc Signed-off-by: Trishna Guha * clarify action plugin comment Signed-off-by: Trishna Guha * 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 * blacklist nxapi Signed-off-by: Trishna Guha * Address review comment Signed-off-by: Trishna Guha --- .../modules/network/nxos/nxos_file_copy.py | 15 ++-- lib/ansible/plugins/action/nxos.py | 11 ++- .../targets/nxos_file_copy/tasks/cli.yaml | 10 +-- .../targets/nxos_file_copy/tasks/nxapi.yaml | 6 ++ .../nxos_file_copy/tests/cli/sanity.yaml | 12 +-- .../tests/nxapi/badtransport.yaml | 17 ++++ .../nxos_file_copy/tests/nxapi/sanity.yaml | 87 ------------------- 7 files changed, 46 insertions(+), 112 deletions(-) create mode 100644 test/integration/targets/nxos_file_copy/tests/nxapi/badtransport.yaml delete mode 100644 test/integration/targets/nxos_file_copy/tests/nxapi/sanity.yaml diff --git a/lib/ansible/modules/network/nxos/nxos_file_copy.py b/lib/ansible/modules/network/nxos/nxos_file_copy.py index 435968e46f2..dd55757a379 100644 --- a/lib/ansible/modules/network/nxos/nxos_file_copy.py +++ b/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.get('host') or provider.get('host') - username = module.params.get('username') or provider.get('username') - password = module.params.get('password') or provider.get('password') - port = module.params.get('connect_ssh_port') + hostname = module.params['host'] + username = module.params['username'] + password = module.params['password'] + port = module.params['connect_ssh_port'] ssh = paramiko.SSHClient() ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) diff --git a/lib/ansible/plugins/action/nxos.py b/lib/ansible/plugins/action/nxos.py index d8c40638b68..db190355ae9 100644 --- a/lib/ansible/plugins/action/nxos.py +++ b/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' diff --git a/test/integration/targets/nxos_file_copy/tasks/cli.yaml b/test/integration/targets/nxos_file_copy/tasks/cli.yaml index edbff7dfafb..ac06cd7ea72 100644 --- a/test/integration/targets/nxos_file_copy/tasks/cli.yaml +++ b/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) - include: "{{ test_case_to_run }} ansible_connection=local connection={{ cli }}" - with_first_found: "{{ test_items }}" +- 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 diff --git a/test/integration/targets/nxos_file_copy/tasks/nxapi.yaml b/test/integration/targets/nxos_file_copy/tasks/nxapi.yaml index 653bcfefe96..c85edec402a 100644 --- a/test/integration/targets/nxos_file_copy/tasks/nxapi.yaml +++ b/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 diff --git a/test/integration/targets/nxos_file_copy/tests/cli/sanity.yaml b/test/integration/targets/nxos_file_copy/tests/cli/sanity.yaml index c6ea9bd64d0..806128d0c55 100644 --- a/test/integration/targets/nxos_file_copy/tests/cli/sanity.yaml +++ b/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="Using provider={{ connection.transport }}" - when: ansible_connection == "local" +- debug: msg="START connection={{ ansible_connection }} nxos_file_copy sanity test" - name: "Setup - Remove existing file" nxos_command: &remove_file @@ -20,9 +18,6 @@ nxos_file_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" diff --git a/test/integration/targets/nxos_file_copy/tests/nxapi/badtransport.yaml b/test/integration/targets/nxos_file_copy/tests/nxapi/badtransport.yaml new file mode 100644 index 00000000000..de0693d51aa --- /dev/null +++ b/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" diff --git a/test/integration/targets/nxos_file_copy/tests/nxapi/sanity.yaml b/test/integration/targets/nxos_file_copy/tests/nxapi/sanity.yaml deleted file mode 100644 index 91ea22cb6a8..00000000000 --- a/test/integration/targets/nxos_file_copy/tests/nxapi/sanity.yaml +++ /dev/null @@ -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: ©_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: ©_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"