diff options
author | Thom May <thom@may.lt> | 2016-11-10 15:43:16 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-10 15:43:16 +0000 |
commit | f0256a4ec1db4c487117912fded9150884894344 (patch) | |
tree | ebd3a4e52409d8df7a8ec030eff9e9df12c8191b | |
parent | b86319aa57ad41b3d8489d7b37f105b0f8701b1d (diff) | |
parent | 65135ccabb85dd4c20982492f1f2cfeed6b12570 (diff) | |
download | chef-f0256a4ec1db4c487117912fded9150884894344.tar.gz |
Merge pull request #5422 from chef/tm/rescue_5115
Mac: ensure launchd's start_calendar_interval gets valid settings
-rw-r--r-- | lib/chef/provider/launchd.rb | 2 | ||||
-rw-r--r-- | lib/chef/resource/launchd.rb | 54 | ||||
-rw-r--r-- | spec/unit/provider/launchd_spec.rb | 84 |
3 files changed, 129 insertions, 11 deletions
diff --git a/lib/chef/provider/launchd.rb b/lib/chef/provider/launchd.rb index c58d4bfa34..0ec8d0cfe1 100644 --- a/lib/chef/provider/launchd.rb +++ b/lib/chef/provider/launchd.rb @@ -115,7 +115,7 @@ class Chef res = Chef::Resource::File.new(@path, run_context) res.name(@path) if @path res.backup(backup) if backup - res.content(content) if content + res.content(content) if content? res.group(group) if group res.mode(mode) if mode res.owner(owner) if owner diff --git a/lib/chef/resource/launchd.rb b/lib/chef/resource/launchd.rb index e584693dee..27937515ec 100644 --- a/lib/chef/resource/launchd.rb +++ b/lib/chef/resource/launchd.rb @@ -29,12 +29,6 @@ class Chef default_action :create allowed_actions :create, :create_if_missing, :delete, :enable, :disable - def initialize(name, run_context = nil) - super - provider = Chef::Provider::Launchd - resource_name = :launchd - end - property :label, String, default: lazy { name }, identity: true property :backup, [Integer, FalseClass] property :cookbook, String @@ -46,6 +40,53 @@ class Chef property :source, String property :session_type, String + # StartCalendarInterval has some gotchas so we coerce it to help sanity + # check. According to `man 5 launchd.plist`: + # StartCalendarInterval <dictionary of integers or array of dictionaries of integers> + # ... Missing arguments are considered to be wildcard. + # What the man page doesn't state, but what was observed (OSX 10.11.5, launchctrl v3.4.0) + # Is that keys that are specified, but invalid, will also be treated as a wildcard + # this means that an entry like: + # { "Hour"=>0, "Weekday"=>"6-7"} + # will not just run on midnight of Sat and Sun, rather it will run _every_ midnight. + property :start_calendar_interval, [Hash, Array], coerce: proc { |type| + # Coerce into an array of hashes to make validation easier + array = if type.is_a?(Array) + type + else + [type] + end + + # Check to make sure that our array only has hashes + unless array.all? { |obj| obj.is_a?(Hash) } + error_msg = "start_calendar_interval must be a single hash or an array of hashes!" + raise Chef::Exceptions::ValidationFailed, error_msg + end + + # Make sure the hashes don't have any incorrect keys/values + array.each do |entry| + allowed_keys = %w{Minute Hour Day Weekday Month} + unless entry.keys.all? { |key| allowed_keys.include?(key) } + failed_keys = entry.keys.reject { |k| allowed_keys.include?(k) }.join(", ") + error_msg = "The following key(s): #{failed_keys} are invalid for start_calendar_interval, must be one of: #{allowed_keys.join(", ")}" + raise Chef::Exceptions::ValidationFailed, error_msg + end + + unless entry.values.all? { |val| val.is_a?(Fixnum) } + failed_values = entry.values.reject { |val| val.is_a?(Fixnum) }.join(", ") + error_msg = "Invalid value(s) (#{failed_values}) for start_calendar_interval item. Values must be integers!" + raise Chef::Exceptions::ValidationFailed, error_msg + end + end + + # Don't return array if we only have one entry + if array.size == 1 + array.first + else + array + end + } + property :type, String, default: "daemon", coerce: proc { |type| type = type ? type.downcase : "daemon" types = %w{daemon agent} @@ -89,7 +130,6 @@ class Chef property :standard_error_path, String property :standard_in_path, String property :standard_out_path, String - property :start_calendar_interval, Hash property :start_interval, Integer property :start_on_mount, [ TrueClass, FalseClass ] property :throttle_interval, Integer diff --git a/spec/unit/provider/launchd_spec.rb b/spec/unit/provider/launchd_spec.rb index 2893044833..3e45433c62 100644 --- a/spec/unit/provider/launchd_spec.rb +++ b/spec/unit/provider/launchd_spec.rb @@ -40,7 +40,7 @@ describe Chef::Provider::Launchd do \t<string>/Library/scripts/call_mom.sh</string> \t<key>StartCalendarInterval</key> \t<dict> -\t\t<key>Hourly</key> +\t\t<key>Hour</key> \t\t<integer>10</integer> \t\t<key>Weekday</key> \t\t<integer>7</integer> @@ -50,13 +50,42 @@ describe Chef::Provider::Launchd do </dict> </plist> XML + let(:test_plist_multiple_intervals) { String.new <<-XML } +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> +<plist version="1.0"> +<dict> +\t<key>Label</key> +\t<string>call.mom.weekly</string> +\t<key>Program</key> +\t<string>/Library/scripts/call_mom.sh</string> +\t<key>StartCalendarInterval</key> +\t<array> +\t\t<dict> +\t\t\t<key>Hour</key> +\t\t\t<integer>11</integer> +\t\t\t<key>Weekday</key> +\t\t\t<integer>1</integer> +\t\t</dict> +\t\t<dict> +\t\t\t<key>Hour</key> +\t\t\t<integer>12</integer> +\t\t\t<key>Weekday</key> +\t\t\t<integer>2</integer> +\t\t</dict> +\t</array> +\t<key>TimeOut</key> +\t<integer>300</integer> +</dict> +</plist> +XML let(:test_hash) do { "Label" => "call.mom.weekly", "Program" => "/Library/scripts/call_mom.sh", "StartCalendarInterval" => { - "Hourly" => 10, + "Hour" => 10, "Weekday" => 7, }, "TimeOut" => 300, @@ -100,12 +129,61 @@ XML it "should produce the test_plist from properties" do new_resource.program "/Library/scripts/call_mom.sh" new_resource.time_out 300 - new_resource.start_calendar_interval "Hourly" => 10, "Weekday" => 7 + new_resource.start_calendar_interval "Hour" => 10, "Weekday" => 7 expect(provider.content?).to be_truthy expect(provider.content).to eql(test_plist) end end + describe "start_calendar_interval is passed" do + it "should allow array of Hashes" do + allowed = (1..2).collect do |num| + { + "Hour" => 10 + num, + "Weekday" => num, + } + end + new_resource.program "/Library/scripts/call_mom.sh" + new_resource.time_out 300 + new_resource.start_calendar_interval allowed + expect(provider.content?).to be_truthy + expect(provider.content).to eql(test_plist_multiple_intervals) + end + + it "should allow all StartCalendarInterval keys" do + allowed = { + "Minute" => 1, + "Hour" => 1, + "Day" => 1, + "Weekday" => 1, + "Month" => 1, + } + new_resource.program "/Library/scripts/call_mom.sh" + new_resource.time_out 300 + new_resource.start_calendar_interval allowed + expect(provider.content?).to be_truthy + %w{Minute Hour Day Weekday Month}.each do |key| + expect(provider.content).to include("<key>#{key}</key>") + end + end + + it "should not allow invalid ShowCalendarInterval keys" do + new_resource.program "/Library/scripts/call_mom.sh" + new_resource.time_out 300 + expect do + new_resource.start_calendar_interval "Hourly" => 1 + end.to raise_error(/Hourly are invalid/) + end + + it "should not allow non-integer values" do + new_resource.program "/Library/scripts/call_mom.sh" + new_resource.time_out 300 + expect do + new_resource.start_calendar_interval "Weekday" => "1-2" + end.to raise_error(/Invalid value.*\(1-2\)/) + end + end + describe "hash is passed" do it "should produce the test_plist from the hash" do new_resource.hash test_hash |