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
6 years ago · f69e3e1cec
parent 361acd3547
commit f69e3e1cec
2 changed files with 75 additions and 67 deletions
--- 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 <dag@wieers.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)

-#Requires -Module Ansible.ModuleUtils.Legacy
-
-$ErrorActionPreference = 'Stop'
-
-$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
+#AnsibleRequires -CSharpUtil Ansible.Basic
+#Requires -Module Ansible.ModuleUtils.AddType
+
+$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
+}

-$webclient_util = @"
+$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec)
+
+$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()
--- 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