From f69e3e1cec72df25df3dbb5b1f9a69f19d12113b Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Sun, 18 Nov 2018 23:17:13 +0100 Subject: [PATCH] win_get_url: Rewrite using AnsibleModule (#48390) * win_get_url: Rewrite using AnsibleModule * Fix sanity issue * Implemented review suggestions * Try something else * fix circular dependency issues --- lib/ansible/modules/windows/win_get_url.ps1 | 137 +++++++++++--------- test/sanity/pslint/ignore.txt | 1 - 2 files changed, 73 insertions(+), 65 deletions(-) diff --git a/lib/ansible/modules/windows/win_get_url.ps1 b/lib/ansible/modules/windows/win_get_url.ps1 index 2c061cb62b..ff94c6dd15 100644 --- a/lib/ansible/modules/windows/win_get_url.ps1 +++ b/lib/ansible/modules/windows/win_get_url.ps1 @@ -5,15 +5,45 @@ # Copyright: (c) 2017, Dag Wieers # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -#Requires -Module Ansible.ModuleUtils.Legacy +#AnsibleRequires -CSharpUtil Ansible.Basic +#Requires -Module Ansible.ModuleUtils.AddType -$ErrorActionPreference = 'Stop' +$spec = @{ + options = @{ + url = @{ type='str'; required=$true } + dest = @{ type='path'; required=$true } + timeout = @{ type='int'; default=10 } + headers = @{ type='dict'; default=@{} } + validate_certs = @{ type='bool'; default=$true } + url_username = @{ type='str'; aliases=@( 'username' ) } + url_password = @{ type='str'; aliases=@( 'password' ); no_log=$true } + force_basic_auth = @{ type='bool'; default=$false } + use_proxy = @{ type='bool'; default=$true } + proxy_url = @{ type='str' } + proxy_username = @{ type='str' } + proxy_password = @{ type='str'; no_log=$true } + force = @{ type='bool'; default=$true } + } + supports_check_mode = $true +} -$params = Parse-Args $args -supports_check_mode $true -$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false -$_remote_tmp = Get-AnsibleParam $params "_ansible_remote_tmp" -type "path" -default $env:TMP +$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec) -$webclient_util = @" +$url = $module.Params.url +$dest = $module.Params.dest +$timeout = $module.Params.timeout +$headers = $module.Params.headers +$validate_certs = $module.Params.validate_certs +$url_username = $module.Params.url_username +$url_password = $module.Params.url_password +$force_basic_auth = $module.Params.force_basic_auth +$use_proxy = $module.Params.use_proxy +$proxy_url = $module.Params.proxy_url +$proxy_username = $module.Params.proxy_username +$proxy_password = $module.Params.proxy_password +$force = $module.Params.force + +Add-CSharpType -AnsibleModule $module -References @' using System.Net; public class ExtendedWebClient : WebClient { public int Timeout; @@ -28,14 +58,10 @@ $webclient_util = @" return request; } } -"@ -$original_tmp = $env:TMP -$env:TMP = $_remote_tmp -Add-Type -TypeDefinition $webclient_util -$env:TMP = $original_tmp +'@ -Function CheckModified-File($url, $dest, $headers, $credentials, $timeout, $use_proxy, $proxy) { +Function CheckModified-File($module, $url, $dest, $headers, $credentials, $timeout, $use_proxy, $proxy) { $fileLastMod = ([System.IO.FileInfo]$dest).LastWriteTimeUtc $webLastMod = $null @@ -71,18 +97,18 @@ Function CheckModified-File($url, $dest, $headers, $credentials, $timeout, $use_ $webRequest.Method = [System.Net.WebRequestMethods+Http]::Head } + # FIXME: Split both try-statements and single-out catched exceptions with more specific error messages Try { $webResponse = $webRequest.GetResponse() - $webLastMod = $webResponse.LastModified } Catch [System.Net.WebException] { - $result.status_code = $_.Exception.Response.StatusCode - Fail-Json -obj $result -message "Error requesting '$url'. $($_.Exception.Message)" + $module.Result.status_code = [int] $_.Exception.Response.StatusCode + $module.FailJson("Error requesting '$url'. $($_.Exception.Message)", $_) } Catch { - Fail-Json -obj $result -message "Error when requesting 'Last-Modified' date from '$url'. $($_.Exception.Message)" + $module.FailJson("Error when requesting 'Last-Modified' date from '$url'. $($_.Exception.Message)", $_) } - $result.status_code = [int] $webResponse.StatusCode - $result.msg = $webResponse.StatusDescription + $module.Result.status_code = [int] $webResponse.StatusCode + $module.Result.msg = [string] $webResponse.StatusDescription $webResponse.Close() if ($webLastMod -and ((Get-Date -Date $webLastMod).ToUniversalTime() -lt $fileLastMod)) { @@ -93,14 +119,14 @@ Function CheckModified-File($url, $dest, $headers, $credentials, $timeout, $use_ } -Function Download-File($result, $url, $dest, $headers, $credentials, $timeout, $use_proxy, $proxy, $whatif) { +Function Download-File($module, $url, $dest, $headers, $credentials, $timeout, $use_proxy, $proxy) { $module_start = Get-Date # Check $dest parent folder exists before attempting download, which avoids unhelpful generic error message. $dest_parent = Split-Path -LiteralPath $dest if (-not (Test-Path -LiteralPath $dest_parent -PathType Container)) { - Fail-Json -obj $result -message "The path '$dest_parent' does not exist for destination '$dest', or is not visible to the current user. Ensure download destination folder exists (perhaps using win_file state=directory) before win_get_url runs." + $module.FailJson("The path '$dest_parent' does not exist for destination '$dest', or is not visible to the current user. Ensure download destination folder exists (perhaps using win_file state=directory) before win_get_url runs.") } # TODO: Replace this with WebRequest @@ -129,50 +155,34 @@ Function Download-File($result, $url, $dest, $headers, $credentials, $timeout, $ } } - if (-not $whatif) { + if (-not $module.CheckMode) { + # FIXME: Single-out catched exceptions with more specific error messages Try { $extWebClient.DownloadFile($url, $dest) } Catch [System.Net.WebException] { - $result.status_code = [int] $_.Exception.Response.StatusCode - $result.elapsed = ((Get-Date) - $module_start).TotalSeconds - Fail-Json -obj $result -message "Error downloading '$url' to '$dest': $($_.Exception.Message)" + $module.Result.status_code = [int] $_.Exception.Response.StatusCode + $module.Result.elapsed = ((Get-Date) - $module_start).TotalSeconds + $module.FailJson("Error downloading '$url' to '$dest': $($_.Exception.Message)", $_) } Catch { - $result.elapsed = ((Get-Date) - $module_start).TotalSeconds - Fail-Json -obj $result -message "Unknown error downloading '$url' to '$dest': $($_.Exception.Message)" + $module.Result.elapsed = ((Get-Date) - $module_start).TotalSeconds + $module.FailJson("Unknown error downloading '$url' to '$dest': $($_.Exception.Message)", $_) } } - $result.status_code = 200 - $result.changed = $true - $result.msg = 'OK' - $result.dest = $dest - $result.elapsed = ((Get-Date) - $module_start).TotalSeconds + $module.Result.status_code = 200 + $module.Result.changed = $true + $module.Result.msg = 'OK' + $module.Result.dest = $dest + $module.Result.elapsed = ((Get-Date) - $module_start).TotalSeconds } -$url = Get-AnsibleParam -obj $params -name "url" -type "str" -failifempty $true -$dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true -$timeout = Get-AnsibleParam -obj $params -name "timeout" -type "int" -default 10 -$headers = Get-AnsibleParam -obj $params -name "headers" -type "dict" -default @{} -$validate_certs = Get-AnsibleParam -obj $params -name "validate_certs" -type "bool" -default $true -$url_username = Get-AnsibleParam -obj $params -name "url_username" -type "str" -aliases "username" -$url_password = Get-AnsibleParam -obj $params -name "url_password" -type "str" -aliases "password" -$force_basic_auth = Get-AnsibleParam -obj $params -name "force_basic_auth" -type "bool" -default $false -$use_proxy = Get-AnsibleParam -obj $params -name "use_proxy" -type "bool" -default $true -$proxy_url = Get-AnsibleParam -obj $params -name "proxy_url" -type "str" -$proxy_username = Get-AnsibleParam -obj $params -name "proxy_username" -type "str" -$proxy_password = Get-AnsibleParam -obj $params -name "proxy_password" -type "str" -$force = Get-AnsibleParam -obj $params -name "force" -type "bool" -default $true - -$result = @{ - changed = $false - dest = $dest - elapsed = 0 - url = $url -} +$module.Result.dest = $dest +$module.Result.elapsed = 0 +$module.Result.url = $url if (-not $use_proxy -and ($proxy_url -or $proxy_username -or $proxy_password)) { - Add-Warning -obj $result -msg "Not using a proxy on request, however a 'proxy_url', 'proxy_username' or 'proxy_password' was defined." + $module.Warn("Not using a proxy on request, however a 'proxy_url', 'proxy_username' or 'proxy_password' was defined.") } $proxy = $null @@ -191,7 +201,6 @@ if ($url_username) { } else { $credentials = New-Object System.Net.NetworkCredential($url_username, $url_password) } - } if (-not $validate_certs) { @@ -208,11 +217,14 @@ if (Test-Path -LiteralPath $dest -PathType Container) { } else { $dest = Join-Path -Path $dest -ChildPath $uri.Host } + + # Ensure we have a string instead of a PS object to avoid serialization issues + $dest = $dest.ToString() } elseif (([System.IO.Path]::GetFileName($dest)) -eq '') { # We have a trailing path separator - Fail-Json -obj $result -message "The destination path '$dest' does not exist, or is not visible to the current user. Ensure download destination folder exists (perhaps using win_file state=directory) before win_get_url runs." + $module.FailJson("The destination path '$dest' does not exist, or is not visible to the current user. Ensure download destination folder exists (perhaps using win_file state=directory) before win_get_url runs.") } -$result.dest = $dest +$module.Result.dest = $dest # Enable TLS1.1/TLS1.2 if they're available but disabled (eg. .NET 4.5) $security_protocols = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::SystemDefault @@ -226,23 +238,20 @@ if ([Net.SecurityProtocolType].GetMember("Tls12").Count -gt 0) { if ($force -or -not (Test-Path -LiteralPath $dest)) { - Download-File -result $result -url $url -dest $dest -credentials $credentials ` - -headers $headers -timeout $timeout -use_proxy $use_proxy -proxy $proxy ` - -whatif $check_mode + Download-File -module $module -url $url -dest $dest -credentials $credentials ` + -headers $headers -timeout $timeout -use_proxy $use_proxy -proxy $proxy } else { - $is_modified = CheckModified-File -result $result -url $url -dest $dest -credentials $credentials ` + $is_modified = CheckModified-File -module $module -url $url -dest $dest -credentials $credentials ` -headers $headers -timeout $timeout -use_proxy $use_proxy -proxy $proxy if ($is_modified) { - Download-File -result $result -url $url -dest $dest -credentials $credentials ` - -headers $headers -timeout $timeout -use_proxy $use_proxy -proxy $proxy ` - -whatif $check_mode + Download-File -module $module -url $url -dest $dest -credentials $credentials ` + -headers $headers -timeout $timeout -use_proxy $use_proxy -proxy $proxy } } -Exit-Json -obj $result - +$module.ExitJson() diff --git a/test/sanity/pslint/ignore.txt b/test/sanity/pslint/ignore.txt index e5cbd33ced..120b502f86 100644 --- a/test/sanity/pslint/ignore.txt +++ b/test/sanity/pslint/ignore.txt @@ -37,7 +37,6 @@ lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingWMICmdlet lib/ansible/modules/windows/win_firewall_rule.ps1 PSAvoidUsingCmdletAliases lib/ansible/modules/windows/win_firewall_rule.ps1 PSUseApprovedVerbs lib/ansible/modules/windows/win_get_url.ps1 PSUseApprovedVerbs -lib/ansible/modules/windows/win_get_url.ps1 PSUseSupportsShouldProcess lib/ansible/modules/windows/win_hotfix.ps1 PSUseApprovedVerbs lib/ansible/modules/windows/win_iis_webbinding.ps1 PSUseApprovedVerbs lib/ansible/modules/windows/win_iis_website.ps1 PSAvoidUsingCmdletAliases