Merge pull request #377 from petems/MODULES-1582-improve_validate_cmd

(MODULES-1582) File location placeholder

lib/puppet/parser/functions/validate_cmd.rb

@@ -6,9 +6,9 @@ module Puppet::Parser::Functions
     Perform validation of a string with an external command.
     The first argument of this function should be a string to
     test, and the second argument should be a path to a test command
-    taking a file as last argument. If the command, launched against
-    a tempfile containing the passed string, returns a non-null value,
-    compilation will abort with a parse error.
+    taking a % as a placeholder for the file path (will default to the end).
+    If the command, launched against a tempfile containing the passed string,
+    returns a non-null value, compilation will abort with a parse error.
 
     If a third argument is specified, this will be the error message raised and
     seen by the user.
@@ -17,8 +17,12 @@ module Puppet::Parser::Functions
 
     Example:
 
+        # Defaults to end of path
         validate_cmd($sudoerscontent, '/usr/sbin/visudo -c -f', 'Visudo failed to validate sudoers content')
 
+        # % as file location
+        validate_cmd($haproxycontent, '/usr/sbin/haproxy -f % -c', 'Haproxy failed to validate config content')
+
     ENDHEREDOC
     if (args.length < 2) or (args.length > 3) then
       raise Puppet::ParseError, ("validate_cmd(): wrong number of arguments (#{args.length}; must be 2 or 3)")
@@ -34,10 +38,17 @@ module Puppet::Parser::Functions
     begin
       tmpfile.write(content)
       tmpfile.close
+
+      if checkscript =~ /\s%(\s|$)/
+        check_with_correct_location = checkscript.gsub(/%/,tmpfile.path)
+      else
+        check_with_correct_location = "#{checkscript} #{tmpfile.path}"
+      end
+
       if Puppet::Util::Execution.respond_to?('execute')
-        Puppet::Util::Execution.execute("#{checkscript} #{tmpfile.path}")
+        Puppet::Util::Execution.execute(check_with_correct_location)
       else
-        Puppet::Util.execute("#{checkscript} #{tmpfile.path}")
+        Puppet::Util.execute(check_with_correct_location)
       end
     rescue Puppet::ExecutionFailure => detail
       msg += "\n#{detail}"

spec/functions/validate_cmd_spec.rb

@@ -12,37 +12,74 @@
     scope.method(function_name)
   end
 
-  describe "with an explicit failure message" do
-    it "prints the failure message on error" do
-      expect {
-        subject.call ['', '/bin/false', 'failure message!']
-      }.to raise_error Puppet::ParseError, /failure message!/
+  context 'with no % placeholder' do
+    describe "with an explicit failure message" do
+      it "prints the failure message on error" do
+        expect {
+          subject.call ['', '/bin/false', 'failure message!']
+        }.to raise_error Puppet::ParseError, /failure message!/
+      end
     end
-  end
 
-  describe "on validation failure" do
-    it "includes the command error output" do
-      expect {
-        subject.call ['', "#{TOUCHEXE} /cant/touch/this"]
-      }.to raise_error Puppet::ParseError, /(cannot touch|o such file or)/
+    describe "on validation failure" do
+      it "includes the command error output" do
+        expect {
+          subject.call ['', "#{TOUCHEXE} /cant/touch/this"]
+        }.to raise_error Puppet::ParseError, /(cannot touch|o such file or)/
+      end
+
+      it "includes the command return value" do
+        expect {
+          subject.call ['', '/cant/run/this']
+        }.to raise_error Puppet::ParseError, /returned 1\b/
+      end
     end
 
-    it "includes the command return value" do
-      expect {
-        subject.call ['', '/cant/run/this']
-      }.to raise_error Puppet::ParseError, /returned 1\b/
+    describe "when performing actual validation" do
+      it "can positively validate file content" do
+        expect { subject.call ["non-empty", "#{TESTEXE} -s"] }.to_not raise_error
+      end
+
+      it "can negatively validate file content" do
+        expect {
+          subject.call ["", "#{TESTEXE} -s"]
+        }.to raise_error Puppet::ParseError, /failed to validate.*test -s/
+      end
     end
   end
 
-  describe "when performing actual validation" do
-    it "can positively validate file content" do
-      expect { subject.call ["non-empty", "#{TESTEXE} -s"] }.to_not raise_error
+  context 'with % placeholder' do
+    describe "with an explicit failure message" do
+      it "prints the failure message on error" do
+        expect {
+          subject.call ['', '/bin/false % -f', 'failure message!']
+        }.to raise_error Puppet::ParseError, /failure message!/
+      end
     end
+    describe "on validation failure" do
+      it "includes the command error output" do
+        expect {
+          subject.call ['', "#{TOUCHEXE} /cant/touch/this"]
+        }.to raise_error Puppet::ParseError, /(cannot touch|o such file or)/
+      end
+
+      it "includes the command return value" do
+        expect {
+          subject.call ['', '/cant/run/this % -z']
+        }.to raise_error Puppet::ParseError, /Execution of '\/cant\/run\/this .+ -z' returned 1/
+      end
+    end
+
+    describe "when performing actual validation" do
+      it "can positively validate file content" do
+        expect { subject.call ["non-empty", "#{TESTEXE} -s %"] }.to_not raise_error
+      end
 
-    it "can negatively validate file content" do
-      expect {
-        subject.call ["", "#{TESTEXE} -s"]
-      }.to raise_error Puppet::ParseError, /failed to validate.*test -s/
+      it "can negatively validate file content" do
+        expect {
+          subject.call ["", "#{TESTEXE} -s %"]
+        }.to raise_error Puppet::ParseError, /failed to validate.*test -s/
+      end
     end
   end
 end