From 52c681b420b0209384e68069cf403a5f99585617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Arne=20Bj=C3=B8rndal?= Date: Mon, 30 Dec 2024 14:49:49 +0100 Subject: [PATCH 1/3] win_get_url - Make checksum work more like ansible.builtin.get_url --- .../fragments/717-win_get_url-checksum.yml | 7 +++++ plugins/modules/win_get_url.ps1 | 31 ++++++++++++++----- plugins/modules/win_get_url.py | 5 +++ 3 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/717-win_get_url-checksum.yml diff --git a/changelogs/fragments/717-win_get_url-checksum.yml b/changelogs/fragments/717-win_get_url-checksum.yml new file mode 100644 index 00000000..a57d77c6 --- /dev/null +++ b/changelogs/fragments/717-win_get_url-checksum.yml @@ -0,0 +1,7 @@ +minor_changes: + - win_get_url - if checksum is passed and destination file exists with + identical checksum no download is done unless force=yes + (https://github.com/ansible-collections/ansible.windows/issues/717) + - win_get_url - if checksum is passed and destination file exists with + different checksum file is always downloaded + (https://github.com/ansible-collections/ansible.windows/issues/717) diff --git a/plugins/modules/win_get_url.ps1 b/plugins/modules/win_get_url.ps1 index 6e905d40..0315605c 100644 --- a/plugins/modules/win_get_url.ps1 +++ b/plugins/modules/win_get_url.ps1 @@ -151,6 +151,7 @@ Function Invoke-DownloadFile { [Parameter(Mandatory = $true)][Uri]$Uri, [Parameter(Mandatory = $true)][String]$Dest, [String]$Checksum, + [String]$DestChecksum, [String]$ChecksumAlgorithm ) @@ -187,14 +188,12 @@ Function Invoke-DownloadFile { $download = $true if (Test-Path -LiteralPath $Dest) { - # Validate the remote checksum against the existing downloaded file - $dest_checksum = Get-Checksum -Path $Dest -Algorithm $ChecksumAlgorithm - + # Validate the remote checksum against the existing downloaded file. # If we don't need to download anything, save the dest checksum so we don't waste time calculating it # again at the end of the script - if ($dest_checksum -eq $tmp_checksum) { + if ($DestChecksum -eq $tmp_checksum) { $download = $false - $Module.Result.checksum_dest = $dest_checksum + $Module.Result.checksum_dest = $DestChecksum $Module.Result.size = (Get-AnsibleItem -Path $Dest).Length } } @@ -260,17 +259,33 @@ if ($checksum_url) { $checksum = Get-ChecksumFromUri -Module $Module -Uri $checksum_uri -SourceUri $url } -if ($force -or -not (Test-Path -LiteralPath $dest)) { +$dest_checksum = $null +if (Test-Path -LiteralPath $dest) { + $dest_checksum = Get-Checksum -Path $dest -Algorithm $checksum_algorithm + if ($dest_checksum -ne $checksum) { + # Destination does not match checksum, force fetching + $force = $true + } +} + +if ($dest_checksum -and $checksum -and $dest_checksum -eq $checksum -and -not $force) { + # No need to do any requests, the file is already as expected + $module.Result.checksum_dest = $dest_checksum + $module.Result.size = (Get-AnsibleItem -Path $dest).Length +} +elseif ($force -or -not (Test-Path -LiteralPath $dest)) { # force=yes or dest does not exist, download the file # Note: Invoke-DownloadFile will compare the checksums internally if dest exists - Invoke-DownloadFile -Module $module -Uri $url -Dest $dest -Checksum $checksum ` + Invoke-DownloadFile -Module $module -Uri $url -Checksum $checksum ` + -Dest $dest -DestChecksum $dest_checksum ` -ChecksumAlgorithm $checksum_algorithm } else { # force=no, we want to check the last modified dates and only download if they don't match $is_modified = Compare-ModifiedFile -Module $module -Uri $url -Dest $dest if ($is_modified) { - Invoke-DownloadFile -Module $module -Uri $url -Dest $dest -Checksum $checksum ` + Invoke-DownloadFile -Module $module -Uri $url -Checksum $checksum ` + -Dest $dest -DestChecksum $dest_checksum ` -ChecksumAlgorithm $checksum_algorithm } } diff --git a/plugins/modules/win_get_url.py b/plugins/modules/win_get_url.py index fc4daa06..236fe529 100644 --- a/plugins/modules/win_get_url.py +++ b/plugins/modules/win_get_url.py @@ -39,6 +39,11 @@ - If a I(checksum) is passed to this parameter, the digest of the destination file will be calculated after it is downloaded to ensure its integrity and verify that the transfer completed successfully. + - If a checksum is passed in this parameter or via C(checksum_url) and the + file in C(dest) exists then C(checksum_dest) is calculated. + If C(checksum_dest) equals the checksum no download is done unless + C(force) is C(true). If the checksum does not match the file is always + downloaded, as if C(force) was set. - This option cannot be set with I(checksum_url). type: str checksum_algorithm: From e9472cb65c8c8006b521ed29578f3ba4d34d4af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Arne=20Bj=C3=B8rndal?= Date: Wed, 8 Jan 2025 14:33:59 +0100 Subject: [PATCH 2/3] Test for unnecessary downloads --- .../targets/win_get_url/tasks/tests_checksum.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration/targets/win_get_url/tasks/tests_checksum.yml b/tests/integration/targets/win_get_url/tasks/tests_checksum.yml index 80105805..5740223a 100644 --- a/tests/integration/targets/win_get_url/tasks/tests_checksum.yml +++ b/tests/integration/targets/win_get_url/tasks/tests_checksum.yml @@ -59,6 +59,15 @@ checksum_algorithm: sha256 register: download_sha256_value +- name: re-downloading file with matching checksum should not do any downloads if force=no + win_get_url: + url: https://{{ httpbin_host }}/base64/cHR1eA== + dest: '{{ testing_dir }}\sha256.txt' + checksum: b1b6ce5073c8fac263a8fc5edfffdbd5dec1980c784e09c5bc69f8fb6056f006 + checksum_algorithm: sha256 + force: false + register: redownload_sha256 + - name: assert download file with sha256 checksum assert: that: @@ -71,6 +80,7 @@ - not download_sha256_value is changed - download_sha256_value.status_code == 200 - download_sha256_value.checksum_dest == 'b1b6ce5073c8fac263a8fc5edfffdbd5dec1980c784e09c5bc69f8fb6056f006' + - "'status_code' not in redownload_sha256" - name: fail download with invalid checksum and force=no win_get_url: From dcdb7dc14ce28057d1923645bb9773af419110df Mon Sep 17 00:00:00 2001 From: KBjorndal-VizRT <83645484+KBjorndal-VizRT@users.noreply.github.com> Date: Wed, 8 Jan 2025 14:34:30 +0100 Subject: [PATCH 3/3] Document when force/checksum behaviour changed Co-authored-by: Jordan Borean --- plugins/modules/win_get_url.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/win_get_url.py b/plugins/modules/win_get_url.py index 236fe529..c9a7a487 100644 --- a/plugins/modules/win_get_url.py +++ b/plugins/modules/win_get_url.py @@ -43,7 +43,8 @@ file in C(dest) exists then C(checksum_dest) is calculated. If C(checksum_dest) equals the checksum no download is done unless C(force) is C(true). If the checksum does not match the file is always - downloaded, as if C(force) was set. + downloaded, as if C(force) was set. This behaviour was added in the + C(2.7.0) release of this collection. - This option cannot be set with I(checksum_url). type: str checksum_algorithm: