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

Jump to bottom

modules: zypper_repository: Do not ignore the 'name' parameter #32376

Closed
wants to merge 1 commit into from
Closed

modules: zypper_repository: Do not ignore the 'name' parameter #32376

wants to merge 1 commit into from

Conversation

hwoarang
Copy link
Contributor

@hwoarang hwoarang commented Oct 31, 2017 •
edited by jctanner

ISSUE TYPE
  • Bugfix Pull Request
SUMMARY

Every zypper repository has the 'repo' parameter set. As such, the
shortname variable was always set to 'repo' even if we had set a
different name for our repository using the 'name' parameter. This
causes problems when trying to determine what to do once the repository
is instelled (for example running 'refresh' to import GPG keys). We
should only set 'shortname' to 'repo' if 'repo' ends up with '.repo'
which means it is a proper repofile.

Fixes #32375

modules: zypper_repository: Do not ignore the 'name' parameter
f2a944c 
Every zypper repository has the 'repo' parameter set. As such, the
shortname variable was always set to 'repo' even if we had set a
different name for our repository using the 'name' parameter. This
causes problems when trying to determine what to do once the repository
is instelled (for example running 'refresh' to import GPG keys). We
should only set 'shortname' to 'repo' if 'repo' ends up with '.repo'
which means it is a proper repofile.

Fixes ansible#32375
@ansibot
Copy link
Contributor

ansibot commented Oct 31, 2017

@hwoarang Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

  • issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Oct 31, 2017

cc @matze @robinro
click here for bot help

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly. needs_triage Needs a first human triage before being processed. 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. bugfix_pull_request labels Oct 31, 2017
@robinro
Copy link
Contributor

robinro commented Oct 31, 2017

@hwoarang Thanks for the bug report and PR!

The general issue is that the resulting name or URL of .repo files is unknown without actually downloading and parsing that file.

The shortname is used for refresh and removerepo. This PR breaks state=absent for repo=http://foo.bar/a.repo name=bla. Also, as far as I see, it doesn't fix the refresh issue for repo=http://foo.bar/a.repo, since zypper does not accept .repo files in -r.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 31, 2017
@robinro
Copy link
Contributor

robinro commented Oct 31, 2017

Please make sure to manually run the integration tests. Currently CI does not work for the zypper repos.

@hwoarang
Copy link
Contributor Author

@robinro

Thank you for the comments. Regarding repo=http://foo.bar/a.repo name=bla this should never happen because the code explicitly forbids using *.repo files with name. Indeed it doesn't seem to fix the case for *.repo file. Let me look at it

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. community_review In order to be merged, this PR must follow the community review workflow. and removed new_contributor This PR is the first contribution by a new community member. needs_info This issue requires further information. Please answer any outstanding questions. needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly. labels Nov 8, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 28, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Mar 10, 2018
@robinro
Copy link
Contributor

robinro commented Jun 11, 2018

This is unchanged for a long time now.
close_me

@ansibot ansibot closed this Jun 11, 2018
@dagwieers dagwieers added the packaging Packaging category label Mar 3, 2019
@ansible ansible locked and limited conversation to collaborators Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
affects_2.5 This issue/PR affects Ansible v2.5 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. packaging Packaging category stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

zypper_repository: Fails to import GPG keys when 'name' parameter is missing
4 participants
@hwoarang @ansibot @robinro @dagwieers