From 7d51cac3301fddf6a876b2f75057f432b0841f04 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 13 Sep 2019 12:24:15 -0500 Subject: [PATCH] Don't treat no checksum as a checksum match (#62146) Fixes #61978 * moar tests for get_url fetch behavior with existing file * add changelog fragment --- .../fragments/61978-get-url-no-checksum.yml | 3 ++ .../modules/net_tools/basics/get_url.py | 2 +- .../targets/get_url/tasks/main.yml | 46 ++++++++++++++----- 3 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/61978-get-url-no-checksum.yml diff --git a/changelogs/fragments/61978-get-url-no-checksum.yml b/changelogs/fragments/61978-get-url-no-checksum.yml new file mode 100644 index 0000000000..fd698aa146 --- /dev/null +++ b/changelogs/fragments/61978-get-url-no-checksum.yml @@ -0,0 +1,3 @@ +bugfixes: +- get_url - Don't treat no checksum as a checksum match + (https://github.com/ansible/ansible/issues/61978) diff --git a/lib/ansible/modules/net_tools/basics/get_url.py b/lib/ansible/modules/net_tools/basics/get_url.py index 701df31c82..3c84cb60e9 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -540,7 +540,7 @@ def main(): checksum_mismatch = True # Not forcing redownload, unless checksum does not match - if not force and not checksum_mismatch: + if not force and checksum and not checksum_mismatch: # Not forcing redownload, unless checksum does not match # allow file attribute changes module.params['path'] = dest diff --git a/test/integration/targets/get_url/tasks/main.yml b/test/integration/targets/get_url/tasks/main.yml index 587b8c58f1..82780d84ac 100644 --- a/test/integration/targets/get_url/tasks/main.yml +++ b/test/integration/targets/get_url/tasks/main.yml @@ -257,17 +257,6 @@ - result is changed - "stat_result.stat.mode == '0775'" -- name: Get a file that already exists - get_url: - url: 'https://{{ httpbin_host }}/get' - dest: '{{ remote_tmp_dir }}/test' - register: result - -- name: Assert that we didn't re-download unnecessarily - assert: - that: - - result is not changed - - name: test checksum match in check mode get_url: url: 'https://{{ httpbin_host }}/get' @@ -281,6 +270,41 @@ that: - result is not changed +- name: Get a file that already exists with a checksum + get_url: + url: 'https://{{ httpbin_host }}/cache' + dest: '{{ remote_tmp_dir }}/test' + checksum: 'sha1:{{ stat_result.stat.checksum }}' + register: result + +- name: Assert that the file was not downloaded + assert: + that: + - result.msg == 'file already exists' + +- name: Get a file that already exists + get_url: + url: 'https://{{ httpbin_host }}/cache' + dest: '{{ remote_tmp_dir }}/test' + register: result + +- name: Assert that we didn't re-download unnecessarily + assert: + that: + - result is not changed + - "'304' in result.msg" + +- name: get a file that doesn't respond to If-Modified-Since without checksum + get_url: + url: 'https://{{ httpbin_host }}/get' + dest: '{{ remote_tmp_dir }}/test' + register: result + +- name: Assert that we downloaded the file + assert: + that: + - result is changed + # https://github.com/ansible/ansible/issues/27617 - name: set role facts