Feature/macos user #14
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Create new testing cookbook that contains tests for both plistbuddy and macos_user resources. The hope here is to have a single testing cookbook that contains multiple suites and suite testing variations - Create new macos_user resource - contains single boolean 'true' settings for autologin and admin user. Still work to be done for admin user and all steps that need to be taken to acheive the task.
- Modify Kcpassword helper module to use a method for magic bits. This isn't ideal, but it works for now - Update smoke tests so that john_jr's group ID is 20. This may not be correct, but setting as 20 until it is further investigated
- Consolodate testing suites into single default suite - Add tests for keep_awake recipe - Use more common examples to test PlistBuddy resource
- Previously, the plistbuddy resource did not allow any keys with spaces unless they were used in the value itself. Now, you can use keys with spaces in the value property without needing to double-quote it - Also, fix the PlistBuddy executable string to be correct
- Also, skip Xcode test temporarily
- Remove systemsetup and pmset resource calls in keep_awake until they are made idempotent
- Changes to binary plists via PlistBuddy must be converted back to binary since the change implicitly converts the file to xml. This change contains logic to convert back to binary if the file is detected to be binary in the first place.
A wild FAILING TEST appeared! ... HANKO used TDD! It's super effective!
jazaval
requested changes
Nov 28, 2017
resources/systemsetup.rb
Outdated
| @@ -3,7 +3,7 @@ | |||
| resource_name :systemsetup | |||
|
|
|||
| property :setting, desired_state: false | |||
| property :value, desired_state: true, coerce: proc { |m| m.to_s.capitalize } | |||
| property :value, desired_state: true, coerce: proc { |m| m.to_s.capitalize unless m.to_s.include?('/') } | |||
There was a problem hiding this comment.
Is this to fix the set networktimeserver to Time.windows.com behavior? I'm still seeing the capitalization in the Chef run, but the keep_awake smoke test doesn't validate it - is this because they're node attributes?
recipes/keep_awake.rb
Outdated
| value 0 | ||
| end | ||
|
|
||
| plistbuddy 'disable screensaver' do |
There was a problem hiding this comment.
If you're not using defaults for this preference, it needs to be written to "/Users/#{node['macos']['admin_user']}/Library/Preferences/com.apple.screensaver.#{Hardware_UUID}.plist" Currently the screensaver settings are not changing in a converge on all platforms.
There was a problem hiding this comment.
If that were true, wouldn't this test fail?
describe command("/usr/libexec/PlistBuddy -c 'Print :idleTime' /Users/vagrant/Library/Preferences/com.apple.screensaver.plist") do
its('stdout') { should match(/0/) }
endThere was a problem hiding this comment.
I don't think macOS listens to the idleTime entry in plist being tested.
brownmike
reviewed
Nov 28, 2017
resources/plistbuddy.rb
Outdated
|
|
||
| action :delete do | ||
| execute format_plistbuddy_command(:delete, new_resource.entry, new_resource.value) + ' ' + new_resource.path | ||
| unless new_resource.binary == false |
There was a problem hiding this comment.
Comparing to false is not necessary unless nil is a valid value.
resources/plistbuddy.rb
Outdated
| @@ -1,34 +1,38 @@ | |||
| resource_name :plistbuddy | |||
There was a problem hiding this comment.
Can we call this resource plist? plistbuddy hints at implementation details that are irrelevant.
There was a problem hiding this comment.
I thought about this, and I like the idea a lot. It would be nice to incorporate the three major tools that help users manipulate property lists programmatically (/usr/libexec/PlistBuddy, /usr/bin/defaults, and /usr/bin/plutil) into a single resource (depending on the type of Plist and/or what needs to be accomplished) This would be in contrast to a resource for each one, which is a significantly better experience IMO.
I'd like @ehanlon and @jazaval to weigh in before moving forward with this change. I think it would mean replacing defaults and making sure the plist resource provides the same capabilities that defaults does. Right now, that isn't totally clear.
There was a problem hiding this comment.
My pipe dream is that a preference resource uses /usr/libexec/PlistBuddy, /usr/bin/defaults, or /usr/bin/plutil depending on what's required, rather than requiring the user to know about ByHost preferences or actual plist paths. This is very lofty, but I think changing the name to plist is a step in the right direction, rather than trying to equate cookbook resources with specific command-line utilities.
There was a problem hiding this comment.
@jazaval I think that's a good lofty goal. From a UX perspective, we don't care what tool is used to get the job done, we just know what behavior we want. Generically naming this resource gives us reasonable freedom to use the best tool for the job.
libraries/kcpassword.rb
Outdated
| @@ -0,0 +1,36 @@ | |||
| module KcpasswordHelpers | |||
There was a problem hiding this comment.
What's the purpose of this top level namespace? It seems we're already mixing these in to Chef::Recipe and Chef::Resource, so the extra layer of scoping doesn't seem necessary.
| end | ||
| end | ||
| load_current_value do |desired| | ||
| command_output = shell_out('/usr/sbin/systemsetup', "-get#{desired.setting}").stdout.split(': ').last.strip |
There was a problem hiding this comment.
This is the dangerous kind of method chaining that often results in NoMethodErrors on nil objects at runtime. As a general rule, we should try to adhere to the Law of Demeter by only chaining methods on objects of the same type i.e. some_string.downcase.strip.squeeze.split('//') is okay because every method in the chain until the last one returns another string. And strictly speaking, this is not the command output; the command output ends at .stdout.
DanielOlewski
approved these changes
Nov 29, 2017
- Fix bug where screensaver was not properly being disabled, update tests accordingly - Rename plistbuddy resource to plist, including all related files and callers - Rename kcpassword helper to macos_user to match helper -> resource convention; fix top level namespace to be MacOS - Correctly name recipe unit tests - Implement plist resource into disable_software_updates.rb recipe - Add disable_software_updates.rb recipe to default kitchen suite and appropriate smoke tests - Remove relatively useless logic in systemsetup value property coercion
jazaval
approved these changes
Nov 30, 2017
americanhanko
pushed a commit
that referenced
this pull request
Dec 14, 2017
…kbook and documentation. (#24) * first commit plistbuddy * all recipe/plist in macos * extract plistbuddy to resource * Straighten out unit tests - Correctly include modules - Correctly setup spec_helper - Correct unit test / respec setup * first passing test * add more data type tests * Most data types passing - Boolean values are working - Array and data type values are not - PlistBuddy "commands" (add, print, etc.) are passing * More testing - Skip Array and Dict types as their implementation will be more complicated - Add test for float type - Put the correct type in the test description - Cookstyle changes * PlistBuddy and testing update - Add separate test cookbook for PlistBuddy resource - Add kitchen.yml for the above purpose - Modify and update plistbuddy resource/helpers * remove extra newline * add metadata file * edit metadata; add Berksfile * move Berksfile to correct location * add testing requirements for berks vendor * move kitchen file * Converge passing in test cookbook - Opportunistic cleanup of xcode test - some temp debugging info in resource definition - Rename name_property to command so as to not override action * Itempotence - Removed 'bool' prefix from boolean types, as that was somehow causing issues - Changed name_property to the path and removed that property - Use actual actions in the resource for the PlistBuddy commands * Fix false positive? - Not sure how the previous test was passing previously, but I adjusted the boolean value for the BazEntry test so that the signature input matches the output * Add logic for adding a value - The data type should only be used when adding a value to a plist. Otherwise, we don't need to use the data type * remove debugging; fix smoke tests * remove .kitchen.yml * bump version to 0.8.5 * unskip ChefSpec tests - they're passing * mark args_formatter as private method * Feature/macos user (#14) * first commit add_user recipe * Testing Refactor & macos_user resource - Create new testing cookbook that contains tests for both plistbuddy and macos_user resources. The hope here is to have a single testing cookbook that contains multiple suites and suite testing variations - Create new macos_user resource - contains single boolean 'true' settings for autologin and admin user. Still work to be done for admin user and all steps that need to be taken to acheive the task. * fix method call * fix another method call * user native ruby File class and not overriden * fix some minor issues with resource; add kcpassword file * Add OG Ruby kcpassword file helper module * slightly different approach for Kcpassword module * passing test * Fix up kcpassword library module, smoke tests - Modify Kcpassword helper module to use a method for magic bits. This isn't ideal, but it works for now - Update smoke tests so that john_jr's group ID is 20. This may not be correct, but setting as 20 until it is further investigated * opportunistic refactor of keep_awake recipe * Refactor testing cookbook layout - Consolodate testing suites into single default suite - Add tests for keep_awake recipe - Use more common examples to test PlistBuddy resource * move xcode test to test cookbook * Opportunistic bug fix for plistbuddy resource - Previously, the plistbuddy resource did not allow any keys with spaces unless they were used in the value itself. Now, you can use keys with spaces in the value property without needing to double-quote it - Also, fix the PlistBuddy executable string to be correct * revert kcpassword.rb helper to a671164 * fix incorrect PlistBuddy data type name * Fix kcpassword.rb and update plistbuddy UTs * Extract disabling screen saver to plistbuddy - Also, skip Xcode test temporarily * PlistBuddy idempotence - Remove systemsetup and pmset resource calls in keep_awake until they are made idempotent * update test and plist name * Implement binary plist conversion - Changes to binary plists via PlistBuddy must be converted back to binary since the change implicitly converts the file to xml. This change contains logic to convert back to binary if the file is detected to be binary in the first place. * attempts at idempotence using chef helper methods * early commit to get it on the remote * implement logic for plist return type to ruby data type * Add floating type value support A wild FAILING TEST appeared! ... HANKO used TDD! It's super effective! * move function * real idempotence! * passing all unit tests * update test cookbook name * update new_users integration tests * systemsetup idempotence * all smoke and unit tests passing * quick fix for timezone bug * remove kcpassword file as it is no longer needed * update path for test cookbook to be default repo name * Rename keep_awake_spec.rb to keep_awake_test.rb * Rename preferences_spec.rb to preferences_test.rb * Rename new_users_spec.rb to new_users_test.rb * fix typo * Use relative path for macos cookbook source * Improve readability of relative path for macos cookbook source * Bump version * Addressing changes in Pull Request #14 - Fix bug where screensaver was not properly being disabled, update tests accordingly - Rename plistbuddy resource to plist, including all related files and callers - Rename kcpassword helper to macos_user to match helper -> resource convention; fix top level namespace to be MacOS - Correctly name recipe unit tests - Implement plist resource into disable_software_updates.rb recipe - Add disable_software_updates.rb recipe to default kitchen suite and appropriate smoke tests - Remove relatively useless logic in systemsetup value property coercion * Implement binary plist conversion * Fix unit test and apply the Law of Demeter * update helper pattern to match current standard; include everywhere * remove helper reference in Xcode resource * bump version * update Xcode attribute to latest * extract xcode smoke tests to a different suite * update xcversion path for simulators test * update namespace again * refactor XcodeHelper modules in XCVersion, Xcode, and XCVersion modules updated methods to simplify site calls in resource * updated method calls for new modules * bump version * extract xcversion module to separate library file * add missing macos module * include parent module macos instead of child for xcode and xcversion * update available_simulator_versions to new method * bump version * add platform-specific smoke test for xcode * add xcode test recipe * update kitchen with xcode suite * refactor simulator method to call it's own available list of simulators * use new simulator method to find highest available version * move xcode prep work into the setup action * clear kitchen customizations * lazily evalutate ios simulator property * move lazy evaluation to simulator action in xcode resource * revert lazy execution * warn and retry if simulator list is empty * restore spacing * more whitespace * refactor highest_semantic_simulator method into class, smaller methods * implement simulator class * add more xcversion command methods * implement xcversion command methods * implement command methods correctly * add list methods for xcversion * implement xcversion list methods * implement install xcode correctly * reorder methods * add method to list installed xcodes * use more descriptive name for 'command' method * move where the space lives * implement installed xcodes command * implement install xcodes correctly * disable downloads prior to disabling software update check * Renamed name to device_name * Change name.rb to machine_name.rb to be similar with the apex automation cookbook. Instances of name are also all changed to machine_name. * Add new test recipe and smoke test for machine_name. * add disable-swu test suite * Bump up cookbook version numbers. * update kitchen test suite order, remove standalone swu testing * add sleep for El Capitan platform to avoid race condition this occurs when integrated with the macos-user test recipe * Improve README/Documentation (#23) * implement build definition badge and minor tweaks * Add descriptions and attribute lists for recipes * Remove 'description' label from recipe names * Bump version, add documentation dir * Fix up the last commit * Separate 10.12 and 10.13 build status * Add table for build status * Add plist resource documentation * Add Xcode documentation * bump version
americanhanko
pushed a commit
that referenced
this pull request
Dec 21, 2017
…it tests (#28) * first commit plistbuddy * all recipe/plist in macos * extract plistbuddy to resource * Straighten out unit tests - Correctly include modules - Correctly setup spec_helper - Correct unit test / respec setup * first passing test * add more data type tests * Most data types passing - Boolean values are working - Array and data type values are not - PlistBuddy "commands" (add, print, etc.) are passing * More testing - Skip Array and Dict types as their implementation will be more complicated - Add test for float type - Put the correct type in the test description - Cookstyle changes * PlistBuddy and testing update - Add separate test cookbook for PlistBuddy resource - Add kitchen.yml for the above purpose - Modify and update plistbuddy resource/helpers * remove extra newline * add metadata file * edit metadata; add Berksfile * move Berksfile to correct location * add testing requirements for berks vendor * move kitchen file * Converge passing in test cookbook - Opportunistic cleanup of xcode test - some temp debugging info in resource definition - Rename name_property to command so as to not override action * Itempotence - Removed 'bool' prefix from boolean types, as that was somehow causing issues - Changed name_property to the path and removed that property - Use actual actions in the resource for the PlistBuddy commands * Fix false positive? - Not sure how the previous test was passing previously, but I adjusted the boolean value for the BazEntry test so that the signature input matches the output * Add logic for adding a value - The data type should only be used when adding a value to a plist. Otherwise, we don't need to use the data type * remove debugging; fix smoke tests * remove .kitchen.yml * bump version to 0.8.5 * unskip ChefSpec tests - they're passing * mark args_formatter as private method * Feature/macos user (#14) * first commit add_user recipe * Testing Refactor & macos_user resource - Create new testing cookbook that contains tests for both plistbuddy and macos_user resources. The hope here is to have a single testing cookbook that contains multiple suites and suite testing variations - Create new macos_user resource - contains single boolean 'true' settings for autologin and admin user. Still work to be done for admin user and all steps that need to be taken to acheive the task. * fix method call * fix another method call * user native ruby File class and not overriden * fix some minor issues with resource; add kcpassword file * Add OG Ruby kcpassword file helper module * slightly different approach for Kcpassword module * passing test * Fix up kcpassword library module, smoke tests - Modify Kcpassword helper module to use a method for magic bits. This isn't ideal, but it works for now - Update smoke tests so that john_jr's group ID is 20. This may not be correct, but setting as 20 until it is further investigated * opportunistic refactor of keep_awake recipe * Refactor testing cookbook layout - Consolodate testing suites into single default suite - Add tests for keep_awake recipe - Use more common examples to test PlistBuddy resource * move xcode test to test cookbook * Opportunistic bug fix for plistbuddy resource - Previously, the plistbuddy resource did not allow any keys with spaces unless they were used in the value itself. Now, you can use keys with spaces in the value property without needing to double-quote it - Also, fix the PlistBuddy executable string to be correct * revert kcpassword.rb helper to a671164 * fix incorrect PlistBuddy data type name * Fix kcpassword.rb and update plistbuddy UTs * Extract disabling screen saver to plistbuddy - Also, skip Xcode test temporarily * PlistBuddy idempotence - Remove systemsetup and pmset resource calls in keep_awake until they are made idempotent * update test and plist name * Implement binary plist conversion - Changes to binary plists via PlistBuddy must be converted back to binary since the change implicitly converts the file to xml. This change contains logic to convert back to binary if the file is detected to be binary in the first place. * attempts at idempotence using chef helper methods * early commit to get it on the remote * implement logic for plist return type to ruby data type * Add floating type value support A wild FAILING TEST appeared! ... HANKO used TDD! It's super effective! * move function * real idempotence! * passing all unit tests * update test cookbook name * update new_users integration tests * systemsetup idempotence * all smoke and unit tests passing * quick fix for timezone bug * remove kcpassword file as it is no longer needed * update path for test cookbook to be default repo name * Rename keep_awake_spec.rb to keep_awake_test.rb * Rename preferences_spec.rb to preferences_test.rb * Rename new_users_spec.rb to new_users_test.rb * fix typo * Use relative path for macos cookbook source * Improve readability of relative path for macos cookbook source * Bump version * Addressing changes in Pull Request #14 - Fix bug where screensaver was not properly being disabled, update tests accordingly - Rename plistbuddy resource to plist, including all related files and callers - Rename kcpassword helper to macos_user to match helper -> resource convention; fix top level namespace to be MacOS - Correctly name recipe unit tests - Implement plist resource into disable_software_updates.rb recipe - Add disable_software_updates.rb recipe to default kitchen suite and appropriate smoke tests - Remove relatively useless logic in systemsetup value property coercion * Implement binary plist conversion * Fix unit test and apply the Law of Demeter * update helper pattern to match current standard; include everywhere * remove helper reference in Xcode resource * bump version * update Xcode attribute to latest * extract xcode smoke tests to a different suite * update xcversion path for simulators test * update namespace again * refactor XcodeHelper modules in XCVersion, Xcode, and XCVersion modules updated methods to simplify site calls in resource * updated method calls for new modules * bump version * extract xcversion module to separate library file * add missing macos module * include parent module macos instead of child for xcode and xcversion * update available_simulator_versions to new method * bump version * add platform-specific smoke test for xcode * add xcode test recipe * update kitchen with xcode suite * refactor simulator method to call it's own available list of simulators * use new simulator method to find highest available version * move xcode prep work into the setup action * clear kitchen customizations * lazily evalutate ios simulator property * move lazy evaluation to simulator action in xcode resource * revert lazy execution * warn and retry if simulator list is empty * restore spacing * more whitespace * refactor highest_semantic_simulator method into class, smaller methods * implement simulator class * add more xcversion command methods * implement xcversion command methods * implement command methods correctly * add list methods for xcversion * implement xcversion list methods * implement install xcode correctly * reorder methods * add method to list installed xcodes * use more descriptive name for 'command' method * move where the space lives * implement installed xcodes command * implement install xcodes correctly * disable downloads prior to disabling software update check * Renamed name to device_name * Change name.rb to machine_name.rb to be similar with the apex automation cookbook. Instances of name are also all changed to machine_name. * Add new test recipe and smoke test for machine_name. * add disable-swu test suite * Bump up cookbook version numbers. * update kitchen test suite order, remove standalone swu testing * add sleep for El Capitan platform to avoid race condition this occurs when integrated with the macos-user test recipe * Improve README/Documentation (#23) * implement build definition badge and minor tweaks * Add descriptions and attribute lists for recipes * Remove 'description' label from recipe names * Bump version, add documentation dir * Fix up the last commit * Separate 10.12 and 10.13 build status * Add table for build status * Add plist resource documentation * Add Xcode documentation * bump version * Fix for enhancement macos_user needs fullName property #25 1. Added fullname property to macos_user resource 2. Modified test recipe for user johnny to have full name 3. Added smoke test to test for RealName using dscl command Ideally we should have a custom inspec resource. * Fix extra new lines * add spec tests for xcversion module * add spec test for Simulator * update Simulator spec tests * Fix code based on suggestions in PR #26 1. Hide usage of escape characters in multi-word fullnames in macos_user resource 2. Pass an array of strings to command method for auto escape * Remove escape character * bump version * Incorporate Eric's changes in PR 28 regarding macos_user resource and fullname * Make sure it passes cookstyle
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
macos_userrepresents a user on the system. Properties include:autologin- set totrueso that this user will log in automatically after restartadmin- set totrueto make the user an administratormacos_testcookbook, used for testing resources or recipes.systemsetupresource for idempotence.plistbuddyusing more of the custom resource DSLplistbuddyresource also includes more unit tests and binary conversionkeep_awake.rbrecipe - now fully idempotent now usingplistbuddyandsystemsetuponly.Note: We may want to think about deprecating
pmsetgoing forward as it is simply a wrapper for editing/Library/Preferences/com.apple.PowerManagement.plistkeep_awake.rbmacos_testcookbook pass on the following macOS versions: