Added time_out property in cron

- Property accepts hash with its options as keys and value as their values
- Property also accepts Integer or String value
- Minor code drapes
- Maintained chefstyle
- TBD for AIX platforms

Signed-off-by: Nimesh-Msys <nimesh.patni@msystechnologies.com>

5 files changed, 246 insertions, 11 deletions

diff --git a/lib/chef/provider/cron.rb b/lib/chef/provider/cron.rb
index aad59d1eba..dd70497a35 100644
--- a/lib/chef/provider/cron.rb
+++ b/lib/chef/provider/cron.rb
@@ -216,11 +216,13 @@ class Chef
         raise Chef::Exceptions::Cron, "Error updating state of #{new_resource.name}, error: #{e}"
       end
 
-      def get_crontab_entry
-        newcron = ""
-        newcron << "# Chef Name: #{new_resource.name}\n"
+      #
+      # @return [String] The string of Env Variables containing line breaks.
+      #
+      def env_var_str
+        str = []
         %i{mailto path shell home}.each do |v|
-          newcron << "#{v.to_s.upcase}=\"#{new_resource.send(v)}\"\n" if new_resource.send(v)
+          str << "#{v.to_s.upcase}=\"#{new_resource.send(v)}\"" if new_resource.send(v)
         end
         new_resource.environment.each do |name, value|
           if ENVIRONMENT_PROPERTIES.include?(name)
@@ -228,20 +230,63 @@ class Chef
               logger.warn("#{new_resource.name}: the environment property contains the '#{name}' variable, which should be set separately as a property.")
               new_resource.send(name.downcase.to_sym, value.gsub(/^"|"$/, ""))
               new_resource.environment.delete(name)
-              newcron << "#{name.to_s.upcase}=\"#{value}\"\n"
+              str << "#{name.to_s.upcase}=\"#{value}\""
             else
               raise Chef::Exceptions::Cron, "#{new_resource.name}: the '#{name}' property is set and environment property also contains the '#{name}' variable. Remove the variable from the environment property."
             end
           else
-            newcron << "#{name}=#{value}\n"
+            str << "#{name}=#{value}"
           end
         end
+        str.join("\n")
+      end
+
+      #
+      # @return [String] The Cron time string consisting five fields that Cron converts into a time interval.
+      #
+      def duration_str
         if new_resource.time
-          newcron << "@#{new_resource.time} #{new_resource.command}\n"
+          "@#{new_resource.time}"
         else
-          newcron << "#{new_resource.minute} #{new_resource.hour} #{new_resource.day} #{new_resource.month} #{new_resource.weekday} #{new_resource.command}\n"
+          "#{new_resource.minute} #{new_resource.hour} #{new_resource.day} #{new_resource.month} #{new_resource.weekday}"
         end
-        newcron
+      end
+
+      #
+      # @return [String] The timeout command string formed as per time_out property.
+      #
+      def time_out_str
+        return "" if new_resource.time_out.empty?
+
+        str = " timeout"
+        str << " --preserve-status" if new_resource.time_out["preserve-status"].to_s.downcase == "true"
+        str << " --foreground" if new_resource.time_out["foreground"].to_s.downcase == "true"
+        str << " --kill-after #{new_resource.time_out["kill-after"]}" if new_resource.time_out["kill-after"]
+        str << " --signal #{new_resource.time_out["signal"]}" if new_resource.time_out["signal"]
+        str << " #{new_resource.time_out["duration"]};"
+        str
+      end
+
+      #
+      # @return [String] The command to be executed. The new line at the end has been added purposefully.
+      #
+      def cmd_str
+        " #{new_resource.command}\n"
+      end
+
+      # Concatenates various information and formulates a complete string that
+      # could be written in the crontab
+      #
+      # @return [String] A crontab string formed as per the user inputs.
+      #
+      def get_crontab_entry
+        # Initialize
+        newcron = []
+        newcron << "# Chef Name: #{new_resource.name}"
+        newcron << env_var_str unless env_var_str.empty?
+        newcron << duration_str + time_out_str + cmd_str
+
+        newcron.join("\n")
       end
 
       def weekday_in_crontab
diff --git a/lib/chef/provider/cron/aix.rb b/lib/chef/provider/cron/aix.rb
index 5d58e21bca..4b41e5e240 100644
--- a/lib/chef/provider/cron/aix.rb
+++ b/lib/chef/provider/cron/aix.rb
@@ -33,8 +33,11 @@ class Chef
             raise Chef::Exceptions::Cron, "Aix cron entry does not support environment variables. Please set them in script and use script in cron."
           end
 
-          newcron = ""
-          newcron << "# Chef Name: #{new_resource.name}\n"
+          if time_out_set?
+            raise Chef::Exceptions::Cron, "Aix cron entry does not support timeout."
+          end
+
+          newcron = "# Chef Name: #{new_resource.name}\n"
           newcron << "#{@new_resource.minute} #{@new_resource.hour} #{@new_resource.day} #{@new_resource.month} #{@new_resource.weekday}"
 
           newcron << " #{@new_resource.command}\n"
@@ -44,6 +47,10 @@ class Chef
         def env_vars_are_set?
           @new_resource.environment.length > 0 || !@new_resource.mailto.nil? || !@new_resource.path.nil? || !@new_resource.shell.nil? || !@new_resource.home.nil?
         end
+
+        def time_out_set?
+          !@new_resource.time_out.empty?
+        end
       end
     end
   end
diff --git a/lib/chef/resource/cron.rb b/lib/chef/resource/cron.rb
index dbc6a998cc..cb3f7f4dcd 100644
--- a/lib/chef/resource/cron.rb
+++ b/lib/chef/resource/cron.rb
@@ -162,6 +162,34 @@ class Chef
         description: "A Hash of environment variables in the form of ({'ENV_VARIABLE' => 'VALUE'}).",
         default: lazy { {} }
 
+      TIMEOUT_OPTS = %w{duration preserve-status foreground kill-after signal}.freeze
+      TIMEOUT_REGEX = /\A\S+/.freeze
+
+      property :time_out, Hash,
+        description: "A Hash of timeouts in the form of ({'OPTION' => 'VALUE'}).
+        Accepted valid options are:
+        preserve-status (BOOL, default: 'false'),
+        foreground (BOOL, default: 'false'),
+        kill-after (in seconds),
+        signal (a name like 'HUP' or a number)",
+        default: lazy { {} },
+        coerce: proc { |h|
+          if h.is_a?(Hash)
+            invalid_keys = h.keys - TIMEOUT_OPTS
+            unless invalid_keys.empty?
+              error_msg = "Key of option time_out must be equal to one of: \"#{TIMEOUT_OPTS.join('", "')}\"!  You passed \"#{invalid_keys.join(", ")}\"."
+              raise Chef::Exceptions::ValidationFailed, error_msg
+            end
+            unless h.values.all? { |x| x =~ TIMEOUT_REGEX }
+              error_msg = "Values of option time_out should be non-empty string without any leading whitespaces."
+              raise Chef::Exceptions::ValidationFailed, error_msg
+            end
+            h
+          elsif h.is_a?(Integer) || h.is_a?(String)
+            { "duration" => h }
+          end
+        }
+
       private
 
       def integerize(integerish)
diff --git a/lib/chef/resource/cron_d.rb b/lib/chef/resource/cron_d.rb
index ca3b91a4b2..6cc2ea77b3 100644
--- a/lib/chef/resource/cron_d.rb
+++ b/lib/chef/resource/cron_d.rb
@@ -206,6 +206,34 @@ class Chef
         description: "A Hash containing additional arbitrary environment variables under which the cron job will be run in the form of ``({'ENV_VARIABLE' => 'VALUE'})``.",
         default: lazy { {} }
 
+      TIMEOUT_OPTS = %w{duration preserve-status foreground kill-after signal}.freeze
+      TIMEOUT_REGEX = /\A\S+/.freeze
+
+      property :time_out, Hash,
+        description: "A Hash of timeouts in the form of ({'OPTION' => 'VALUE'}).
+        Accepted valid options are:
+        preserve-status (BOOL, default: 'false'),
+        foreground (BOOL, default: 'false'),
+        kill-after (in seconds),
+        signal (a name like 'HUP' or a number)",
+        default: lazy { {} },
+        coerce: proc { |h|
+          if h.is_a?(Hash)
+            invalid_keys = h.keys - TIMEOUT_OPTS
+            unless invalid_keys.empty?
+              error_msg = "Key of option time_out must be equal to one of: \"#{TIMEOUT_OPTS.join('", "')}\"!  You passed \"#{invalid_keys.join(", ")}\"."
+              raise Chef::Exceptions::ValidationFailed, error_msg
+            end
+            unless h.values.all? { |x| x =~ TIMEOUT_REGEX }
+              error_msg = "Values of option time_out should be non-empty string without any leading whitespaces."
+              raise Chef::Exceptions::ValidationFailed, error_msg
+            end
+            h
+          elsif h.is_a?(Integer) || h.is_a?(String)
+            { "duration" => h }
+          end
+        }
+
       property :mode, [String, Integer],
         description: "The octal mode of the generated crontab file.",
         default: "0600"
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index 4cd8a140af..06628b631b 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -1081,4 +1081,131 @@ describe Chef::Provider::Cron do
       end
     end
   end
+
+  describe "#env_var_str" do
+    context "when no env vars are set" do
+      it "returns an empty string" do
+        expect(@provider.send(:env_var_str)).to be_empty
+      end
+    end
+    let(:mailto) { "foo@example.com" }
+    context "When set directly" do
+      it "returns string with value" do
+        @new_resource.mailto mailto
+        expect(@provider.send(:env_var_str)).to include(mailto)
+      end
+    end
+    context "When set within the hash" do
+      context "env properties" do
+        it "returns string with a warning" do
+          @new_resource.environment "MAILTO" => mailto
+          expect(logger).to receive(:warn).with("cronhole some stuff: the environment property contains the 'MAILTO' variable, which should be set separately as a property.")
+          expect(@provider.send(:env_var_str)).to include(mailto)
+        end
+      end
+      context "other properties" do
+        it "returns string with no warning" do
+          @new_resource.environment "FOOMAILTO" => mailto
+          expect(logger).not_to receive(:warn).with("cronhole some stuff: the environment property contains the 'MAILTO' variable, which should be set separately as a property.")
+          expect(@provider.send(:env_var_str)).to include(mailto)
+        end
+        it "and a line break within properties" do
+          @new_resource.environment "FOOMAILTO" => mailto, "BARMAILTO" => mailto
+          expect(@provider.send(:env_var_str)).to eq("FOOMAILTO=foo@example.com\nBARMAILTO=foo@example.com")
+        end
+      end
+      context "both env and other properties" do
+        it "returns string with line break within the properties" do
+          @new_resource.mailto mailto
+          @new_resource.environment "FOOMAILTO" => mailto
+          expect(@provider.send(:env_var_str)).to eq("MAILTO=\"foo@example.com\"\nFOOMAILTO=foo@example.com")
+        end
+      end
+    end
+  end
+
+  describe "#duration_str" do
+    context "time as a frequency" do
+      it "returns string" do
+        @new_resource.time :yearly
+        expect(@provider.send(:duration_str)).to eq("@yearly")
+      end
+    end
+    context "time as a duration" do
+      it "defaults to * (No Specific Value)" do
+        @new_resource.minute "1"
+        expect(@provider.send(:duration_str)).to eq("1 * * * *")
+      end
+      it "returns cron format string" do
+        @new_resource.minute "1"
+        @new_resource.hour "2"
+        @new_resource.day "3"
+        @new_resource.month "4"
+        @new_resource.weekday "5"
+        expect(@provider.send(:duration_str)).to eq("1 2 3 4 5")
+      end
+    end
+  end
+
+  describe "#time_out_str" do
+    context "When not given" do
+      it "Returns an empty string" do
+        expect(@provider.send(:time_out_str)).to be_empty
+      end
+    end
+    context "When given" do
+      let(:time_out_str_val) { " timeout 10;" }
+      context "as String" do
+        it "returns string" do
+          @new_resource.time_out "10"
+          expect(@provider.send(:time_out_str)).to eq time_out_str_val
+        end
+      end
+      context "as Integer" do
+        it "returns string" do
+          @new_resource.time_out "10"
+          expect(@provider.send(:time_out_str)).to eq time_out_str_val
+        end
+      end
+      context "as Hash" do
+        it "returns string" do
+          @new_resource.time_out "duration" => "10"
+          expect(@provider.send(:time_out_str)).to eq time_out_str_val
+        end
+        it "also contains properties" do
+          @new_resource.time_out "duration" => "10", "foreground" => "true", "signal" => "FOO"
+          expect(@provider.send(:time_out_str)).to eq " timeout --foreground --signal FOO 10;"
+        end
+      end
+    end
+  end
+
+  describe "#cmd_str" do
+    context "With command" do
+      let(:cmd) { "FOOBAR" }
+      before {
+        @new_resource.command cmd
+      }
+      it "returns a string with command" do
+        expect(@provider.send(:cmd_str)).to include(cmd)
+      end
+      it "string ends with a next line" do
+        expect(@provider.send(:cmd_str)[-1]).to eq("\n")
+      end
+    end
+    context "Without command, passed" do
+      context "as nil" do
+        it "returns an empty string with a next line" do
+          @new_resource.command nil
+          expect(@provider.send(:cmd_str)).to eq(" \n")
+        end
+      end
+      context "as an empty string" do
+        it "returns an empty string with a next line" do
+          @new_resource.command ""
+          expect(@provider.send(:cmd_str)).to eq(" \n")
+        end
+      end
+    end
+  end
 end