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

Fix osx_defaults idempotency when value contains accent #53216

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@Maastiff
Copy link

Maastiff commented Mar 3, 2019

SUMMARY

When providing value containing accent as oyé, osx_defaults idempotency would be broken due to encoding problem.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

osx_defaults

ADDITIONAL INFORMATION

If you call ansible playbook executing the task below two times successively, osx_defaults will return changed status also at the second execution that is unexpected.

- name: Change value of TestDefaultsKey to "oyé"
  osx_defaults:
    domain: NSGlobalDomain
    key: TestDefaultsKey
    type: string
    value: "oyé"
    state: present

If we manipulate defaults from command line, it seems that text is not utf-8 encoded.

$ defaults write /Library/Preferences/com.apple.loginwindow LoginwindowText -string "oyé"
$ defaults read /Library/Preferences/com.apple.loginwindow LoginwindowText
oy\351

Thus when module try to compare new value oyé with fetched value from defaults command oy\351 it is considered different.

Converting defaults read command output in python unicode-escape encoding solve the issue.

@Maastiff Maastiff force-pushed the Maastiff:osx_defaults_fix_accented_value branch from d2ab4f4 to e9d96ca Mar 3, 2019

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 3, 2019

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

lib/ansible/modules/system/osx_defaults.py:256:112: bad-whitespace No space allowed around keyword argument assignment         rc, out, err = self.module.run_command(self._base_command() + ["read", self.domain, self.key], encoding = 'unicode-escape')                                                                                                                 ^

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

lib/ansible/modules/system/osx_defaults.py:256:112: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/system/osx_defaults.py:256:114: E251 unexpected spaces around keyword / parameter equals

click here for bot help

@ansibot ansibot removed the small_patch label Mar 3, 2019

Maastiff added some commits Mar 3, 2019

Refactor completely osx_defaults integration test
- prevent modifying MacOS System defaults (use only com.ansible.fake domain)
- clean defined OSX defaults properties at the end
- try to be consistent in naming to ease test understandability
- add missing tests for array 'list' state
@bbrabant-octo

This comment has been minimized.

Copy link

bbrabant-octo commented Mar 3, 2019

Hi!
Since the first failure of pipeline, I completely reworked integration test to prevent side effect. I also try to write them in a more consistent way to ease understanding. Finally I also add few missing tests that should be useful to ensure non regression for #24808.

I always have a problem with the python 2 compatibility. This fix works well with python 3 but due to the way python 2 handle strings, encoding configuration does not apply correctly in this case.
Any help to finish the work would be appreciated 😗

Thanks

@Akasurde Akasurde requested review from Akasurde and dagwieers Mar 4, 2019

@mattclay mattclay added the ci_verified label Mar 6, 2019

@ansibot ansibot removed the needs_triage label Mar 6, 2019

@ansibot ansibot added the stale_ci label Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.