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.

Sign up for GitHub

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

Jump to bottom

Allow checksum file to be local file:// url #71205

Merged
merged 6 commits into from Aug 17, 2020
Merged

Allow checksum file to be local file:// url #71205

merged 6 commits into from Aug 17, 2020

Conversation

madeddie
Copy link
Contributor

SUMMARY

A partial fix for #69364 in that the SHASUMS file can be downloaded and GPG verified but then used from the downloaded location to verify the get_url's file.

or
When using get_url to download a file that has a GPG signed SHASUMS file, this would allow for less filter hackary.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

get_url

@madeddie
Allow checksum file to be local file:// url
f82f9f6 
This would be a partial solution for #69364 in that the SHASUMS file can be downloaded and gpg verified but then used from the downloaded location to verify the get_url's file.
@ansibot ansibot added affects_2.11 bug This issue/PR relates to a bug. module This issue/PR relates to a module. 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. net_tools Net-tools category new_contributor This PR is the first contribution by a new community member. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Aug 11, 2020
@madeddie
Copy link
Contributor Author

Not quite sure why shippable is failing, it looks like a transient error in CI, but if there's a specific error I need to look at, please let me know. Testing succeeds on my local system.

@bmillemathias
Copy link
Contributor

/rebuild_failed

@bmillemathias
Copy link
Contributor

Hi, thanks for the PR

Not quite sure why shippable is failing, it looks like a transient error in CI, but if there's a specific error I need to look at, please let me know. Testing succeeds on my local system.

I've launched again the CI.

To complete the PR, that would be great to add a test with a file:// so we can make sure the code works and won't break, and also add a changelog fragment.

Best

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 11, 2020
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Aug 11, 2020
@samdoran
Copy link
Contributor

Please create a changelog fragment and integration tests. See this fragment as an example.

@madeddie
Make checksum url parsing more explicit
d43b160 
Use urlsplit to test if the checksum string has a (currently tested and) supported url scheme.
@ansibot
Copy link
Contributor

ansibot commented Aug 13, 2020

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/get_url.py:423:0: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 5 errors:

lib/ansible/modules/get_url.py:420:6: E111: indentation is not a multiple of four
lib/ansible/modules/get_url.py:420:6: E117: over-indented
lib/ansible/modules/get_url.py:422:6: E111: indentation is not a multiple of four
lib/ansible/modules/get_url.py:423:1: W293: blank line contains whitespace
lib/ansible/modules/get_url.py:424:6: E111: indentation is not a multiple of four

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. core_review In order to be merged, this PR must follow the core review workflow. and removed 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. labels Aug 13, 2020
@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Aug 13, 2020
@ansibot ansibot removed the core_review In order to be merged, this PR must follow the core review workflow. label Aug 13, 2020
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 13, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 13, 2020
@samdoran samdoran merged commit eb8b3a8 into ansible:devel Aug 17, 2020
@samdoran
Copy link
Contributor

Please create a backport PR against stable-2.9 and stable-2.10 for this to be included in previous versions.

bcoca pushed a commit to bcoca/ansible that referenced this pull request Aug 19, 2020
@madeddie @bcoca
get_url - Allow checksum file to be local file:// url (ansible#71205)
12ca7cd 
This would be a partial solution for ansible#69364 in that the SHASUMS file can be downloaded and gpg verified but then used from the downloaded location to verify the get_url's file.
* Make checksum url parsing more explicit

Use urlsplit to test if the checksum string has a (currently tested and) supported url scheme.

* Fix whitespace
* Changelog fragment
* Added tests
* Fix typo in test setup
@ansible ansible locked and limited conversation to collaborators Sep 14, 2020
@madeddie madeddie deleted the patch-2 branch October 19, 2020 14:42
@Akasurde
Copy link
Member

As of 18 June, 2021 - Stable-2.9 is for critical fixes.
Backport for Stable-2.10 - #75052

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samdoran samdoran samdoran approved these changes

@bcoca bcoca Awaiting requested review from bcoca

Assignees
No one assigned
Labels
affects_2.11 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. net_tools Net-tools category new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@madeddie @bmillemathias @samdoran @ansibot @Akasurde @bcoca