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

Backport/2.7/47695 - pamd fixes #48307

Merged
merged 3 commits into from
Nov 13, 2018
Merged

Backport/2.7/47695 - pamd fixes #48307

merged 3 commits into from
Nov 13, 2018

Conversation

shepdelacreme
Copy link
Contributor

SUMMARY

Backport of pamd module fixes in #47695

(cherry picked from commit ef690e9)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

pamd

ANSIBLE VERSION
stable-2.7
ADDITIONAL INFORMATION

shepdelacreme and others added 2 commits November 7, 2018 12:20
* Providing fix for #47083 in pamd.py

* Providing fix for #47197

* Fixing pep8 errors

* update regex to account for leading dash and VALID_TYPES with dashes as well

* use a results dictionary and clean up unnecessary items

* remove unnessecary return value. action is already reported in invocation output

* make naming consistent across action returns

* fix comparison so it checks equality instead of identity and indentation in update_rule()

* make sure file always has EOF newline

* updated regex to skip spacing between path and args and add rule arg regex to capture complex args

* new module argument parsing code in function and DRY changes

* remove unused has_rule method on PamdService class

* fix error in parse_module_arguments()

* updated args_present action to make it handle key value args and fail on complex bracketed arguments

* pep8 and other fixes so units still work

* suggested change - make version removed 2.8

Co-Authored-By: shepdelacreme <shepdelacreme@users.noreply.github.com>

* add more error proof test to if statement

(cherry picked from commit ef690e9)
@ansibot
Copy link
Contributor

ansibot commented Nov 7, 2018

Hi @shepdelacreme, thank you for submitting this pull-request!

click here for bot help

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. 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. needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Nov 7, 2018
@juliedavila
Copy link
Contributor

shipit

@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Nov 8, 2018
@abadger
Copy link
Contributor

abadger commented Nov 9, 2018

action needs to be re-added to the return value for 2.7.x. It's a playbook-visible backwards incompatibility so it shouldn't be backported to a released version of ansible.

@shepdelacreme
Copy link
Contributor Author

Ah ok...I was under the impression that since this module is in status: preview it is unstable and the API (parameters, return, etc) may change. Is that an incorrect assumption of what preview means?

@abadger
Copy link
Contributor

abadger commented Nov 9, 2018

API parameters may change between major releases (2.6.x => 2.7.x), even without a deprecation notice if the status is preview (although we discourage that, especially if the module is popular ;-). Within a major release series (2.7.x), the API needs to stay backwards compatible. That way users can anticipate playbook breakage on major release boundaries, not on bugfix upgrades.

@ansibot ansibot removed the needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) label Nov 9, 2018
@shepdelacreme
Copy link
Contributor Author

Ah ok makes sense! I will add that back in and update the changelog fragment shortly!

@abadger abadger merged commit 1abd90e into ansible:stable-2.7 Nov 13, 2018
@abadger
Copy link
Contributor

abadger commented Nov 13, 2018

Merged for the 2.7.2 release

@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. 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. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants