diff options
author | Shuang Wang <ooocamel@icloud.com> | 2018-08-14 10:14:53 +0900 |
---|---|---|
committer | Matt Davis <nitzmahone@users.noreply.github.com> | 2018-08-13 18:14:53 -0700 |
commit | 3f7ca8daf13246416037a3e60ae110d4b6354068 (patch) | |
tree | dad0317199d7e1eea3947454525d1faae55230bb | |
parent | 8a0fa4a3e39881eedc9fb2e2995684ab85446e37 (diff) | |
download | ansible-3f7ca8daf13246416037a3e60ae110d4b6354068.tar.gz |
fix issue [ get_url does not change mode when checksum matches ] (#43342) (#43623)
* fix #29614
* add change log for #43342
* Cleanup tests and add tests for this scenario
Co-authored-by: ptux
(cherry picked from commit 68683b4c73c08fa6b70c740c3c4deb863eddef5b)
-rw-r--r-- | changelogs/fragments/get_url_bug_fix.yaml | 2 | ||||
-rw-r--r-- | lib/ansible/modules/net_tools/basics/get_url.py | 22 | ||||
-rw-r--r-- | test/integration/targets/get_url/tasks/main.yml | 71 |
3 files changed, 61 insertions, 34 deletions
diff --git a/changelogs/fragments/get_url_bug_fix.yaml b/changelogs/fragments/get_url_bug_fix.yaml new file mode 100644 index 0000000000..a0a3edacd1 --- /dev/null +++ b/changelogs/fragments/get_url_bug_fix.yaml @@ -0,0 +1,2 @@ +bugfixes: +- get_url - fix the bug that get_url does not change mode when checksum matches (https://github.com/ansible/ansible/issues/29614) diff --git a/lib/ansible/modules/net_tools/basics/get_url.py b/lib/ansible/modules/net_tools/basics/get_url.py index b9642debd9..2185ac8dde 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -446,22 +446,18 @@ def main(): destination_checksum = module.digest_from_file(dest, algorithm) if checksum == destination_checksum: - module.exit_json(msg="file already exists", dest=dest, url=url, changed=False) + # Not forcing redownload, unless checksum does not match + # allow file attribute changes + module.params['path'] = dest + file_args = module.load_file_common_arguments(module.params) + file_args['path'] = dest + changed = module.set_fs_attributes_if_different(file_args, False) + if changed: + module.exit_json(msg="file already exists but file attributes changed", dest=dest, url=url, changed=changed) + module.exit_json(msg="file already exists", dest=dest, url=url, changed=changed) checksum_mismatch = True - # Not forcing redownload, unless checksum does not match - if not force and not checksum_mismatch: - # allow file attribute changes - module.params['path'] = dest - file_args = module.load_file_common_arguments(module.params) - file_args['path'] = dest - changed = module.set_fs_attributes_if_different(file_args, False) - - if changed: - module.exit_json(msg="file already exists but file attributes changed", dest=dest, url=url, changed=changed) - module.exit_json(msg="file already exists", dest=dest, url=url, changed=changed) - # If the file already exists, prepare the last modified time for the # request. mtime = os.path.getmtime(dest) diff --git a/test/integration/targets/get_url/tasks/main.yml b/test/integration/targets/get_url/tasks/main.yml index 9751a688b5..3d6a4f0fc2 100644 --- a/test/integration/targets/get_url/tasks/main.yml +++ b/test/integration/targets/get_url/tasks/main.yml @@ -1,4 +1,4 @@ -# Test code for the file module. +# Test code for the get_url module # (c) 2014, Richard Isaacson <richard.c.isaacson@gmail.com> # This file is part of Ansible @@ -51,8 +51,8 @@ - name: assert success and change assert: that: - - result.changed - - '"OK" in result.msg' + - result is changed + - '"OK" in result.msg' - name: test nonexisting file fetch get_url: @@ -64,21 +64,27 @@ - name: assert success and change assert: that: - - result.failed + - result is failed - name: test HTTP HEAD request for file in check mode - get_url: url="https://{{ httpbin_host }}/get" dest={{ output_dir }}/get_url_check.txt force=yes + get_url: + url: "https://{{ httpbin_host }}/get" + dest: "{{ output_dir }}/get_url_check.txt" + force: yes check_mode: True register: result - name: assert that the HEAD request was successful in check mode assert: that: - - result.changed + - result is changed - '"OK" in result.msg' - name: test HTTP HEAD for nonexistent URL in check mode - get_url: url="https://{{ httpbin_host }}/DOESNOTEXIST" dest={{ output_dir }}/shouldnotexist.html force=yes + get_url: + url: "https://{{ httpbin_host }}/DOESNOTEXIST" + dest: "{{ output_dir }}/shouldnotexist.html" + force: yes check_mode: True register: result ignore_errors: True @@ -86,7 +92,7 @@ - name: assert that HEAD request for nonexistent URL failed assert: that: - - result.failed + - result is failed - name: test https fetch get_url: url="https://{{ httpbin_host }}/get" dest={{output_dir}}/get_url.txt force=yes @@ -95,8 +101,8 @@ - name: assert the get_url call was successful assert: that: - - result.changed - - '"OK" in result.msg' + - result is changed + - '"OK" in result.msg' - name: test https fetch to a site with mismatched hostname and certificate get_url: @@ -130,7 +136,7 @@ - name: Assert that the file was downloaded assert: that: - - "result.changed == true" + - result is changed - "stat_result.stat.exists == true" # SNI Tests @@ -144,21 +150,23 @@ - command: "grep '{{ sni_host }}' {{ output_dir}}/sni.html" register: data_result - when: "{{ python_has_ssl_context }}" + when: python_has_ssl_context + +- debug: + var: get_url_result -- debug: var=get_url_result - name: Assert that SNI works with this python version assert: that: - 'data_result.rc == 0' - when: "{{ python_has_ssl_context }}" + when: python_has_ssl_context # If the client doesn't support SNI then get_url should have failed with a certificate mismatch - name: Assert that hostname verification failed because SNI is not supported on this version of python assert: that: - 'get_url_result is failed' - when: "{{ not python_has_ssl_context }}" + when: not python_has_ssl_context # These tests are just side effects of how the site is hosted. It's not # specifically a test site. So the tests may break due to the hosting changing @@ -171,22 +179,24 @@ - command: "grep '{{ sni_host }}' {{ output_dir}}/sni.html" register: data_result - when: "{{ python_has_ssl_context }}" + when: python_has_ssl_context + +- debug: + var: get_url_result -- debug: var=get_url_result - name: Assert that SNI works with this python version assert: that: - 'data_result.rc == 0' - 'get_url_result is not failed' - when: "{{ python_has_ssl_context }}" + when: python_has_ssl_context # If the client doesn't support SNI then get_url should have failed with a certificate mismatch - name: Assert that hostname verification failed because SNI is not supported on this version of python assert: that: - 'get_url_result is failed' - when: "{{ not python_has_ssl_context }}" + when: not python_has_ssl_context # End hacky SNI test section - name: Test get_url with redirect @@ -208,7 +218,7 @@ - name: Assert that the file has the right permissions assert: that: - - "result.changed == true" + - result is changed - "stat_result.stat.mode == '0707'" - name: Test that setting file modes on an already downlaoded file work @@ -225,9 +235,28 @@ - name: Assert that the file has the right permissions assert: that: - - "result.changed == true" + - result is changed - "stat_result.stat.mode == '0070'" +# https://github.com/ansible/ansible/issues/29614 +- name: Change mode on an already downloaded file and specify checksum + get_url: + url: 'https://{{ httpbin_host }}/' + dest: '{{ output_dir }}/test' + checksum: 'sha256:7036ede810fad2b5d2e7547ec703cae8da61edbba43c23f9d7203a0239b765c4.' + mode: '0775' + register: result + +- stat: + path: "{{ output_dir }}/test" + register: stat_result + +- name: Assert that file permissions on already downloaded file were changed + assert: + that: + - result is changed + - "stat_result.stat.mode == '0775'" + #https://github.com/ansible/ansible/issues/16191 - name: Test url split with no filename get_url: |