Skip to content
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

[2.10] get_url: Fix checksum binary validation #74674

Merged
merged 2 commits into from Jul 12, 2021
Merged

[2.10] get_url: Fix checksum binary validation #74674

merged 2 commits into from Jul 12, 2021

Conversation

resmo
Copy link
Contributor

@resmo resmo commented May 12, 2021

SUMMARY

backport of #74502 and #71435 ( /cc @Akasurde )

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

get_url

ADDITIONAL INFORMATION

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels May 12, 2021
@resmo resmo changed the title [2.10]: Fix checksum binary validation [2.10]: get_url: Fix checksum binary validation May 12, 2021
@resmo resmo changed the title [2.10]: get_url: Fix checksum binary validation [2.10] get_url: Fix checksum binary validation May 12, 2021
@ansibot
Copy link
Contributor

ansibot commented May 12, 2021

The test ansible-test sanity --test docs-build [explain] failed with 3 errors:

docs/docsite/rst/user_guide/playbooks_filters.rst:1605:0: reference-target-not-found: py:func reference target not found: jinja2:map
docs/docsite/rst/user_guide/playbooks_python_version.rst:22:0: reference-target-not-found: py:func reference target not found: jinja2:list
docs/docsite/rst/user_guide/playbooks_python_version.rst:62:0: reference-target-not-found: py:func reference target not found: jinja2:list

click here for bot help

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels May 12, 2021
@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label May 14, 2021
@relrod
Copy link
Member

relrod commented May 17, 2021

needs_info (failing CI)

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label May 17, 2021
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 25, 2021
@ansibot
Copy link
Contributor

ansibot commented Jun 18, 2021

@resmo This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jun 18, 2021
@resmo
Copy link
Contributor Author

resmo commented Jun 18, 2021

rebased

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_info This issue requires further information. Please answer any outstanding questions. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Jun 18, 2021
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 18, 2021
Comment on lines 438 to 447
- name: download src with sha256 checksum url with file scheme
get_url:
url: 'http://localhost:{{ http_port }}/27617.txt'
dest: '{{ remote_tmp_dir }}/27617sha256_with_file_scheme.txt'
checksum: 'sha256:file://{{ files_dir }}/sha256sum.txt'
register: result_sha256_with_file_scheme

- stat:
path: "{{ remote_tmp_dir }}/27617sha256_with_dot.txt"
register: stat_result_sha256_with_file_scheme
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_url does not have support for file:// in Stable-2.10 which was added via #71205

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added backport via #75052

Comment on lines +493 to +503
- name: download 71420.txt with sha256 checksum url with file scheme
get_url:
url: 'http://localhost:{{ http_port }}/71420.txt'
dest: '{{ remote_tmp_dir }}/71420sha256_with_file_scheme.txt'
checksum: 'sha256:file://{{ files_dir }}/sha256sum.txt'
register: result_sha256_with_file_scheme_71420

- stat:
path: "{{ remote_tmp_dir }}/71420sha256_with_dot.txt"
register: stat_result_sha256_with_file_scheme_71420

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Jun 21, 2021
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 29, 2021
@relrod
Copy link
Member

relrod commented Jul 12, 2021

This needs another rebase and to see if it passes tests now that #75052 was merged.

needs_info

@ansibot ansibot added needs_info This issue requires further information. Please answer any outstanding questions. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jul 12, 2021
Akasurde and others added 2 commits July 12, 2021 12:48
Fixes: #71420

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
(cherry picked from commit 1595446)
From the sha512sum man page:

... The default mode is to print a line with checksum, a character indicating type ('*' for binary, ' ' for text), and name for each FILE.

(cherry picked from commit 403a5d1)
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 12, 2021
@resmo
Copy link
Contributor Author

resmo commented Jul 12, 2021

rebased but zuul is not happy.

I don't see why and how to make zuul happy.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_info This issue requires further information. Please answer any outstanding questions. labels Jul 12, 2021
@samdoran
Copy link
Contributor

The Zuul check was enabled by mistake and can be ignored.

@relrod relrod merged commit 3ee1694 into ansible:stable-2.10 Jul 12, 2021
@resmo resmo deleted the fix/2.10-74502 branch July 12, 2021 19:46
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Jul 15, 2021
@ansible ansible locked and limited conversation to collaborators Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants