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

user: compare Mac user properties using same type #62973

Merged
merged 5 commits into from
Nov 22, 2019
Merged

user: compare Mac user properties using same type #62973

merged 5 commits into from
Nov 22, 2019

Conversation

kryptonite303
Copy link
Contributor

@kryptonite303 kryptonite303 commented Sep 30, 2019 •
edited by samdoran

SUMMARY

self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly.

On Darwin systems, _get_user_property returns a string, however, self.__dict__[field[0]] sometimes returns an int, such as in the case when comparing the uid. In order for the comparison

current != self.__dict__[field[0]]

to work correctly, both variables must be of the same type. This change casts self.__dict__[field[0]] to a string so that the comparison works properly.

Fixes #62969

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

user module

ADDITIONAL INFORMATION
Before:
failed: [127.0.0.1] (item={u'uid': u'500', u'name': u'test'}) => {"ansible_loop_var": "item", "changed": false, "err": "", "item": {"name": "test", "uid": "500"}, "msg": "Cannot update property \"uid\" for user \"test\".", "out": "", "rc": 40}

After:
ok: [127.0.0.1] => (item={u'uid': 500, u'name': u'test'})

@ansibot
Copy link
Contributor

ansibot commented Sep 30, 2019

cc @samdoran @sfromm
click here for bot help

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 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. needs_triage Needs a first human triage before being processed. 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. system System category labels Sep 30, 2019
@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 Oct 9, 2019
bcoca
bcoca previously requested changes Oct 10, 2019
View reviewed changes
lib/ansible/modules/system/user.py Outdated Show resolved Hide resolved
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Oct 10, 2019
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. 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 Oct 10, 2019
@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 Oct 19, 2019
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Nov 4, 2019
@samdoran samdoran dismissed bcoca’s stale review November 22, 2019 21:35

Requested changes were made.

kryptonite303 and others added 4 commits November 22, 2019 16:38
@kryptonite303 @samdoran
user: compare Mac user properties using same type
97e605b 
self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly
@kryptonite303 @samdoran
user: using to_text() instead of str()
88782df
@samdoran
Add changelog
2025878
@samdoran
Add to_text import
bac6b66
@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. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Nov 22, 2019
@ansibot ansibot removed small_patch 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 Nov 22, 2019
@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Nov 22, 2019
@samdoran samdoran merged commit c73288a into ansible:devel Nov 22, 2019
samdoran pushed a commit to samdoran/ansible that referenced this pull request Nov 22, 2019
@kryptonite303 @samdoran
[stable-2.9] user - compare macOS user properties using same type (an…
5c5513c 
…sible#62973)

self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly
* Add changelog
* Add to_text import
* Add integration test.
(cherry picked from commit c73288a)

Co-authored-by: John Chen <kryptonite303@users.noreply.github.com>
@samdoran samdoran mentioned this pull request Nov 22, 2019
Merged
samdoran pushed a commit to samdoran/ansible that referenced this pull request Dec 3, 2019
@kryptonite303 @samdoran
[stable-2.9] user - compare macOS user properties using same type (an…
7c14ece 
…sible#62973)

self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly
* Add changelog
* Add to_text import
* Add integration test.
(cherry picked from commit c73288a)

Co-authored-by: John Chen <kryptonite303@users.noreply.github.com>
D3DeFi pushed a commit to D3DeFi/ansible that referenced this pull request Dec 8, 2019
@kryptonite303 @D3DeFi
user - compare macOS user properties using same type (ansible#62973)
50d7169 
self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly
* Add changelog
* Add to_text import
* Add integration test
anshulbehl pushed a commit to anshulbehl/ansible that referenced this pull request Dec 10, 2019
@kryptonite303 @anshulbehl
user - compare macOS user properties using same type (ansible#62973)
249fa41 
self._get_user_property returns a string, so when doing a comparison
using this value, cast the second variable to a string so that the
comparison behaves correctly
* Add changelog
* Add to_text import
* Add integration test
@ansible ansible locked and limited conversation to collaborators Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bcoca bcoca bcoca left review comments

Assignees

@samdoran samdoran

Labels
affects_2.10 This issue/PR affects Ansible v2.10 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. 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. system System category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

User module on Darwin is not idempotent
4 participants
@kryptonite303 @ansibot @bcoca @samdoran