From f75b7a9437ad634877921b0727f8af0cb480e71a Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Fri, 4 May 2018 08:39:37 +1000 Subject: [PATCH] win_get_url: Fixed a few issues with using FTP and added tests (#39646) * win_get_url: Fixed a few issues with using FTP and added tests * Fixed typo in docs --- .../fragments/win_get_url-ftp-support.yaml | 2 + lib/ansible/modules/windows/win_get_url.ps1 | 27 ++- lib/ansible/modules/windows/win_get_url.py | 14 +- .../targets/win_get_url/defaults/main.yml | 8 +- .../win_get_url/files/ftp/anon/file.txt | 1 + .../win_get_url/files/ftp/anon/file2.txt | 1 + .../win_get_url/files/ftp/user-pass/file.txt | 1 + .../win_get_url/files/ftp/user/file.txt | 1 + .../targets/win_get_url/tasks/main.yml | 180 +++++------------- .../targets/win_get_url/tasks/tests_ftp.yml | 157 +++++++++++++++ .../targets/win_get_url/tasks/tests_url.yml | 124 ++++++++++++ .../win_get_url/templates/slimftpd.conf.tmpl | 28 +++ 12 files changed, 393 insertions(+), 151 deletions(-) create mode 100644 changelogs/fragments/win_get_url-ftp-support.yaml create mode 100644 test/integration/targets/win_get_url/files/ftp/anon/file.txt create mode 100644 test/integration/targets/win_get_url/files/ftp/anon/file2.txt create mode 100644 test/integration/targets/win_get_url/files/ftp/user-pass/file.txt create mode 100644 test/integration/targets/win_get_url/files/ftp/user/file.txt create mode 100644 test/integration/targets/win_get_url/tasks/tests_ftp.yml create mode 100644 test/integration/targets/win_get_url/tasks/tests_url.yml create mode 100644 test/integration/targets/win_get_url/templates/slimftpd.conf.tmpl diff --git a/changelogs/fragments/win_get_url-ftp-support.yaml b/changelogs/fragments/win_get_url-ftp-support.yaml new file mode 100644 index 0000000000..8c365ccb3e --- /dev/null +++ b/changelogs/fragments/win_get_url-ftp-support.yaml @@ -0,0 +1,2 @@ +bugfixes: +- win_get_url - fixed a few bugs around authentication and force no when using an FTP URL diff --git a/lib/ansible/modules/windows/win_get_url.ps1 b/lib/ansible/modules/windows/win_get_url.ps1 index 1cf4ef5d0f..434371f125 100644 --- a/lib/ansible/modules/windows/win_get_url.ps1 +++ b/lib/ansible/modules/windows/win_get_url.ps1 @@ -5,8 +5,7 @@ # Copyright: (c) 2017, Dag Wieers # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -# WANT_JSON -# POWERSHELL_COMMON +#Requires -Module Ansible.ModuleUtils.Legacy $ErrorActionPreference = 'Stop' @@ -35,8 +34,8 @@ Function CheckModified-File($url, $dest, $headers, $credentials, $timeout, $use_ $fileLastMod = ([System.IO.FileInfo]$dest).LastWriteTimeUtc $webLastMod = $null - $webRequest = [System.Net.HttpWebRequest]::Create($url) - + $webRequest = [System.Net.WebRequest]::Create($url) + foreach ($header in $headers.GetEnumerator()) { $webRequest.Headers.Add($header.Name, $header.Value) } @@ -53,14 +52,23 @@ Function CheckModified-File($url, $dest, $headers, $credentials, $timeout, $use_ } if ($credentials) { - $webRequest.Credentials = $credentials + if ($force_basic_auth) { + $extWebClient.Headers.Add("Authorization","Basic $credentials") + } else { + $extWebClient.Credentials = $credentials + } } - $webRequest.Method = "HEAD" + if ($webRequest -is [System.Net.FtpWebRequest]) { + $webRequest.Method = [System.Net.WebRequestMethods+Ftp]::GetDateTimestamp + } else { + $webRequest.Method = [System.Net.WebRequestMethods+Http]::Head + } + Try { - [System.Net.HttpWebResponse]$webResponse = $webRequest.GetResponse() + $webResponse = $webRequest.GetResponse() - $webLastMod = $webResponse.GetResponseHeader("Last-Modified") + $webLastMod = $webResponse.LastModified } Catch [System.Net.WebException] { $result.status_code = $_.Exception.Response.StatusCode Fail-Json -obj $result -message "Error requesting '$url'. $($_.Exception.Message)" @@ -174,7 +182,7 @@ if ($proxy_url) { } $credentials = $null -if ($url_username -and $url_password) { +if ($url_username) { if ($force_basic_auth) { $credentials = [convert]::ToBase64String([System.Text.Encoding]::ASCII.GetBytes($url_username+":"+$url_password)) } else { @@ -241,3 +249,4 @@ if ($force -or -not (Test-Path -LiteralPath $dest)) { } Exit-Json -obj $result + diff --git a/lib/ansible/modules/windows/win_get_url.py b/lib/ansible/modules/windows/win_get_url.py index 4b1e90c313..630272b8b1 100644 --- a/lib/ansible/modules/windows/win_get_url.py +++ b/lib/ansible/modules/windows/win_get_url.py @@ -15,9 +15,10 @@ DOCUMENTATION = r''' --- module: win_get_url version_added: "1.7" -short_description: Fetches a file from a given URL +short_description: Downloads file from HTTP, HTTPS, or FTP to node description: -- Fetches a file from a URL and saves it locally. +- Downloads files from HTTP, HTTPS, or FTP to the remote server. The remote + server I(must) have direct access to the remote resource. - For non-Windows targets, use the M(get_url) module instead. author: - Paul Durivage (@angstwad) @@ -101,8 +102,6 @@ options: - Timeout in seconds for URL request. default: 10 version_added : '2.4' -notes: - - For non-Windows targets, use the M(get_url) module instead. ''' EXAMPLES = r''' @@ -124,6 +123,13 @@ EXAMPLES = r''' proxy_url: http://10.0.0.1:8080 proxy_username: username proxy_password: password + +- name: Download file from FTP with authentication + win_get_url: + url: ftp://server/file.txt + dest: '%TEMP%\ftp-file.txt' + url_username: ftp-user + url_password: ftp-password ''' RETURN = r''' diff --git a/test/integration/targets/win_get_url/defaults/main.yml b/test/integration/targets/win_get_url/defaults/main.yml index 36f5a76451..4ee191003c 100644 --- a/test/integration/targets/win_get_url/defaults/main.yml +++ b/test/integration/targets/win_get_url/defaults/main.yml @@ -1,8 +1,4 @@ --- - +test_win_get_url_path: '{{win_output_dir}}\win_get_url' test_win_get_url_host: www.redhat.com -test_win_get_url_link: "https://{{ test_win_get_url_host }}" -test_win_get_url_invalid_link: https://www.redhat.com/skynet_module.html -test_win_get_url_invalid_path: 'Q:\Filez\Cyberdyne.html' -test_win_get_url_invalid_path_dir: 'Q:\Filez\' -test_win_get_url_path: '%TEMP%\docs_index.html' +test_win_get_url_env_var: WIN_GET_URL diff --git a/test/integration/targets/win_get_url/files/ftp/anon/file.txt b/test/integration/targets/win_get_url/files/ftp/anon/file.txt new file mode 100644 index 0000000000..7ffe02e39e --- /dev/null +++ b/test/integration/targets/win_get_url/files/ftp/anon/file.txt @@ -0,0 +1 @@ +ftp/anon/file.txt diff --git a/test/integration/targets/win_get_url/files/ftp/anon/file2.txt b/test/integration/targets/win_get_url/files/ftp/anon/file2.txt new file mode 100644 index 0000000000..9dfc1e4aba --- /dev/null +++ b/test/integration/targets/win_get_url/files/ftp/anon/file2.txt @@ -0,0 +1 @@ +ftp/anon/file2.txt diff --git a/test/integration/targets/win_get_url/files/ftp/user-pass/file.txt b/test/integration/targets/win_get_url/files/ftp/user-pass/file.txt new file mode 100644 index 0000000000..b0a23ddabc --- /dev/null +++ b/test/integration/targets/win_get_url/files/ftp/user-pass/file.txt @@ -0,0 +1 @@ +ftp/user-pass/file.txt diff --git a/test/integration/targets/win_get_url/files/ftp/user/file.txt b/test/integration/targets/win_get_url/files/ftp/user/file.txt new file mode 100644 index 0000000000..40438f769f --- /dev/null +++ b/test/integration/targets/win_get_url/files/ftp/user/file.txt @@ -0,0 +1 @@ +ftp/user/file.txt diff --git a/test/integration/targets/win_get_url/tasks/main.yml b/test/integration/targets/win_get_url/tasks/main.yml index 66b2d225ab..8ba5cc4533 100644 --- a/test/integration/targets/win_get_url/tasks/main.yml +++ b/test/integration/targets/win_get_url/tasks/main.yml @@ -1,143 +1,59 @@ -# test code for the win_get_url module -# (c) 2014, Chris Church - -# 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 . - -- setup: - -- name: Remove test file if it exists +--- +- name: ensure testing folder is present win_file: - path: '{{ test_win_get_url_path }}' - state: absent + path: '{{test_win_get_url_path}}' + state: directory -- name: Test win_get_url module +- name: copy across testing files + win_copy: + src: files/ + dest: '{{test_win_get_url_path}}\' + +- name: download SlimFTPd binary win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '{{ test_win_get_url_path }}' - register: win_get_url_result + url: https://s3.amazonaws.com/ansible-ci-files/test/integration/roles/test_win_get_url/SlimFTPd.exe + dest: '{{test_win_get_url_path}}\SlimFTPd.exe' -- name: Check that url was downloaded - assert: - that: - - win_get_url_result is not failed - - win_get_url_result is changed - - win_get_url_result.url - - win_get_url_result.dest +- name: template SlimFTPd configuration file + win_template: + src: slimftpd.conf.tmpl + dest: '{{test_win_get_url_path}}\slimftpd.conf' -- name: Test win_get_url module again (force should be yes by default) - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '{{ test_win_get_url_path }}' - register: win_get_url_result_again +- name: create SlimFTPd service + win_service: + name: SlimFTPd + path: '"{{test_win_get_url_path}}\SlimFTPd.exe" -service' + state: started + dependencies: + - tcpip -- name: Check that url was downloaded again - assert: - that: - - win_get_url_result_again is not failed - - win_get_url_result_again is changed +- name: create env var for win_get_url tests + win_environment: + name: '{{test_win_get_url_env_var}}' + level: machine + value: '{{test_win_get_url_path}}' + state: present -- name: Test win_get_url module again with force=no - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '{{ test_win_get_url_path }}' - force: no - register: win_get_url_result_noforce +- block: + - name: run URL tests + include_tasks: tests_url.yml -- name: Check that url was not downloaded again - assert: - that: - - win_get_url_result_noforce is not failed - - win_get_url_result_noforce is not changed + - name: run FTP tests + include_tasks: tests_ftp.yml -- name: Test win_get_url module with url that returns a 404 - win_get_url: - url: '{{ test_win_get_url_invalid_link }}' - dest: '{{ test_win_get_url_path }}' - register: win_get_url_result_invalid_link - ignore_errors: true + always: + - name: remove SlimFTPd service + win_service: + name: SlimFTPd + state: absent -- name: Check that the download failed for an invalid url - assert: - that: - - win_get_url_result_invalid_link is failed - - win_get_url_result_invalid_link.status_code == 404 + - name: remove test env var for tests + win_environment: + name: '{{test_win_get_url_env_var}}' + level: machine + state: absent -- name: Test win_get_url module with an invalid path - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '{{ test_win_get_url_invalid_path }}' - register: win_get_url_result_invalid_path - ignore_errors: true - -- name: Check that the download failed for an invalid path - assert: - that: - - win_get_url_result_invalid_path is failed - -- name: Test win_get_url module with a valid path that is a directory - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '%TEMP%' - register: win_get_url_result_dir_path - ignore_errors: true - -- name: Check that the download did NOT fail, even though dest was directory - assert: - that: - - win_get_url_result_dir_path is changed - -- name: Test win_get_url with a valid url path and a dest that is a directory (from 2.4 should use url path as filename) - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: '%TEMP%' - register: win_get_url_result_dir_path_urlpath - ignore_errors: true - -- name: Set expected destination path fact - set_fact: - expected_dest_path: '{{ ansible_env.TEMP }}\{{ test_win_get_url_host }}' - -- name: Check that the download succeeded (changed) and dest is as expected - assert: - that: - - win_get_url_result_dir_path_urlpath is changed - - win_get_url_result_dir_path_urlpath.dest == expected_dest_path - -- name: Check you get a helpful message if the parent folder of the dest doesn't exist - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: 'Q:\Filez\' - register: win_get_url_result_invalid_dest - ignore_errors: true - -- name: Check if dest parent dir does not exist, module fails and you get a specific error message - assert: - that: - - win_get_url_result_invalid_dest is failed - - win_get_url_result_invalid_dest.msg is search('does not exist, or is not visible to the current user') - -- name: Check you get a helpful message if the parent folder of the dest doesn't exist - win_get_url: - url: '{{ test_win_get_url_link }}' - dest: 'C:\Filez\' - register: win_get_url_result_invalid_dest2 - ignore_errors: true - -- name: Check if dest parent dir does not exist, module fails and you get a specific error message - assert: - that: - - win_get_url_result_invalid_dest2 is failed - - win_get_url_result_invalid_dest2.msg is search('does not exist') + - name: remove testing folder + win_file: + path: '{{test_win_get_url_path}}' + state: absent diff --git a/test/integration/targets/win_get_url/tasks/tests_ftp.yml b/test/integration/targets/win_get_url/tasks/tests_ftp.yml new file mode 100644 index 0000000000..d7ae4ae8bf --- /dev/null +++ b/test/integration/targets/win_get_url/tasks/tests_ftp.yml @@ -0,0 +1,157 @@ +--- +- name: download file from FTP source (check) + win_get_url: + url: ftp://localhost/anon/file.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + check_mode: yes + register: ftp_anon_check + +- name: get results of download file from FTP source (check) + win_stat: + path: '{{test_win_get_url_path}}\ftp-anon.txt' + register: ftp_anon_result_check + +- name: assert download file from FTP source (check) + assert: + that: + - ftp_anon_check is changed + - not ftp_anon_result_check.stat.exists + +- name: download file from FTP source + win_get_url: + url: ftp://localhost/anon/file.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + register: ftp_anon + +- name: get results of download file from FTP source + win_stat: + path: '{{test_win_get_url_path}}\ftp-anon.txt' + register: ftp_anon_result + +- name: assert download file from FTP source + assert: + that: + - ftp_anon is changed + - ftp_anon_result.stat.exists + - ftp_anon_result.stat.checksum == '67e0de92f29645cc30d8d147b767cceb81756651' + +# TODO: Add check for idempotent with force: yes once tmp download and checksum verify are in + +- name: download file from FTP source with force no (check) + win_get_url: + url: ftp://localhost/anon/file.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + force: no + check_mode: yes + register: ftp_anon_force_no_check + +- name: assert download file from FTP source with force no + assert: + that: + - ftp_anon_force_no_check is not changed + +- name: download file from FTP source with force no + win_get_url: + url: ftp://localhost/anon/file.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + force: no + register: ftp_anon_force_no + +- name: assert download file from FTP source with force no + assert: + that: + - ftp_anon_force_no is not changed + +- name: set last modified time on FTP source to newer datetime + win_shell: (Get-Item -Path '{{test_win_get_url_path}}\ftp\anon\file2.txt').LastWriteTime = (Get-Date).AddHours(24) + +- name: download newer file from FTP source to same dest (check) + win_get_url: + url: ftp://localhost/anon/file2.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + force: no + check_mode: yes + register: ftp_anon_force_no_different_check + +- name: get result of download newer file from FTP source to same dest (check) + win_stat: + path: '{{test_win_get_url_path}}\ftp-anon.txt' + register: ftp_anon_force_no_different_result_check + +- name: assert download newer file from FTP source to same dest (check) + assert: + that: + - ftp_anon_force_no_different_check is changed + - ftp_anon_force_no_different_result_check.stat.checksum == '67e0de92f29645cc30d8d147b767cceb81756651' + +- name: download newer file from FTP source to same dest + win_get_url: + url: ftp://localhost/anon/file2.txt + dest: '{{test_win_get_url_path}}\ftp-anon.txt' + force: no + register: ftp_anon_force_no_different + +- name: get result of download newer file from FTP source to same dest + win_stat: + path: '{{test_win_get_url_path}}\ftp-anon.txt' + register: ftp_anon_force_no_different_result + +- name: assert download newer file from FTP source to same dest (check) + assert: + that: + - ftp_anon_force_no_different is changed + - ftp_anon_force_no_different_result.stat.checksum == 'eac3baccd817f7137c00138559e2e62aca64aab0' + +- name: fail to download file from ftp protected by username + win_get_url: + url: ftp://localhost/user/file.txt + dest: '{{test_win_get_url_path}}\ftp-user.txt' + register: fail_ftp_no_user + ignore_errors: yes + +- name: assert fail to download file from ftp protected by username + assert: + that: + - fail_ftp_no_user is failed + - fail_ftp_no_user is not changed + - fail_ftp_no_user.status_code == 550 + - '"File unavailable (e.g., file not found, no access)." in fail_ftp_no_user.msg' + +- name: download FTP file protected by username + win_get_url: + url: ftp://localhost/user/file.txt + dest: '{{test_win_get_url_path}}\ftp-user.txt' + url_username: username + register: ftp_user_file + +- name: get result of download FTP file protected by username + win_stat: + path: '{{test_win_get_url_path}}\ftp-user.txt' + register: ftp_user_file_result + +- name: assert download FTP file protected by username + assert: + that: + - ftp_user_file is changed + - ftp_user_file_result.stat.exists + - ftp_user_file_result.stat.checksum == '0efc2e97611cf74e25ec17a00d4b2cf65d0c28ba' + +- name: download FTP file protected by username and password + win_get_url: + url: ftp://localhost/user-pass/file.txt + dest: '{{test_win_get_url_path}}\ftp-user-pass.txt' + url_username: userpass + url_password: password + register: ftp_user_pass_file + +- name: get result of download FTP file protected by username and password + win_stat: + path: '{{test_win_get_url_path}}\ftp-user-pass.txt' + register: ftp_user_pass_file_result + +- name: assert download FTP file protected by username and password + assert: + that: + - ftp_user_pass_file is changed + - ftp_user_pass_file_result.stat.exists + - ftp_user_pass_file_result.stat.checksum == '7da5f1124d4a986cba2b4658d38d95eb55afe086' diff --git a/test/integration/targets/win_get_url/tasks/tests_url.yml b/test/integration/targets/win_get_url/tasks/tests_url.yml new file mode 100644 index 0000000000..d8706f633f --- /dev/null +++ b/test/integration/targets/win_get_url/tasks/tests_url.yml @@ -0,0 +1,124 @@ +- name: download single file (check) + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '{{test_win_get_url_path}}\web.html' + check_mode: yes + register: http_download_check + +- name: get result of download single file (check) + win_stat: + path: '{{test_win_get_url_path}}\web.html' + register: http_download_result_check + +- name: assert download single file (check) + assert: + that: + - http_download_check is not failed + - http_download_check is changed + - http_download_check.url + - http_download_check.dest + - not http_download_result_check.stat.exists + +- name: download single file + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '{{test_win_get_url_path}}\web.html' + register: http_download + +- name: get result of download single file + win_stat: + path: '{{test_win_get_url_path}}\web.html' + register: http_download_result + +- name: assert download single file + assert: + that: + - http_download is not failed + - http_download is changed + - http_download.url + - http_download.dest + - http_download_result.stat.exists + +# TODO: add check for idempotent run once it is added with force: yes + +- name: download single file with force no + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '{{test_win_get_url_path}}\web.html' + force: no + register: http_download_no_force + +- name: assert download single file with force no + assert: + that: + - http_download_no_force is not changed + +- name: manually change last modified time on FTP source to older datetime + win_shell: (Get-Item -Path '{{test_win_get_url_path}}\web.html').LastWriteTime = (Get-Date -Date "01/01/1970") + +- name: download newer file with force no + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '{{test_win_get_url_path}}\web.html' + force: no + register: http_download_newer_no_force + +- name: assert download newer file with force no + assert: + that: + - http_download_newer_no_force is changed + +- name: download file to directory + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '{{test_win_get_url_path}}' + register: http_download_to_directory + +- name: get result of download to directory + win_stat: + path: '{{test_win_get_url_path}}\{{test_win_get_url_host}}' + register: http_download_to_directory_result + +- name: assert download file to directory + assert: + that: + - http_download_to_directory is changed + - http_download_to_directory_result.stat.exists + +- name: download to path with env var + win_get_url: + url: https://{{test_win_get_url_host}} + dest: '%{{test_win_get_url_env_var}}%\http-env.html' + register: http_download_with_env + +- name: get result of download to path with env var + win_stat: + path: '{{test_win_get_url_path}}\http-env.html' + register: http_download_with_env_result + +- name: assert download to path with env var + assert: + that: + - http_download_with_env is changed + - http_download_with_env_result.stat.exists + +- name: fail when link returns 404 + win_get_url: + url: https://{{test_win_get_url_host}}/skynet_module.html + dest: '{{test_win_get_url_path}}\skynet_module.html' + ignore_errors: yes + register: fail_download_404 + +- name: assert fail when link returns 404 + assert: + that: + - fail_download_404 is not changed + - fail_download_404 is failed + - fail_download_404.status_code == 404 + +- name: fail when dest is an invalid path + win_get_url: + url: https://{{test_win_get_url_host}} + dest: Q:\Filez\Cyberdyne.html + register: fail_invalid_path + failed_when: '"The path ''Q:\Filez'' does not exist for destination ''Q:\Filez\Cyberdyne.html''" not in fail_invalid_path.msg' diff --git a/test/integration/targets/win_get_url/templates/slimftpd.conf.tmpl b/test/integration/targets/win_get_url/templates/slimftpd.conf.tmpl new file mode 100644 index 0000000000..2b8b2f8744 --- /dev/null +++ b/test/integration/targets/win_get_url/templates/slimftpd.conf.tmpl @@ -0,0 +1,28 @@ +# http://www.wiki.uniformserver.com/index.php/SlimFTPd:_Config_File +BindInterface Local +BindPort 21 +CommandTimeout 300 +ConnectTimeout 15 +MaxConnections 20 +LookupHosts On + + + Password "" + Mount / {{test_win_get_url_path}}\ftp + Allow /anon All + Deny /user All + Deny /user-pass All + + + + Mount / {{test_win_get_url_path}}\ftp + Allow /anon All + Allow /user All + Deny /user-pass All + + + + Password "password" + Mount / {{test_win_get_url_path}}\ftp + Allow / All +