New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get_url: return no change in check mode when checksum matches #53070
Conversation
Signed-off-by: Tony Finch <dot@dotat.at>
shipit |
if module.check_mode: | ||
if os.path.exists(tmpsrc): | ||
os.remove(tmpsrc) | ||
result['changed'] = ('checksum_dest' not in result or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd better add a test for 'checksum_dest' not in result
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the existing check_mode tests already cover that case, or have I missed something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right.
the following test case covers your code.
name: test HTTP HEAD request for file in check mode
shipit |
@fanf2 Thank you for this fix and for extending the tests @fanf2 If you'd like to see this fixed in the next Ansible 2.7 release please raise a backport PR with a changelog/fragment by following https://docs.ansible.com/ansible/devel/community/development_process.html#making-your-pr-merge-worthy |
SUMMARY
In check_mode,
get_url
does not compare the checksum with the destination file, so it produces spuriouschanged
results which turn out to be green when check_mode is False.This change moves the check_mode logic a little bit later in get_url so that the destination file checksum is available for comparison with the source file checksum.
ISSUE TYPE
COMPONENT NAME
get_url
ADDITIONAL INFORMATION
I have updated the get_url test with a case that illustrates this problem and fix.