Merge pull request #6852 from aspectcapital/PUP-8773

(PUP-8773) Fix logging to a JSON file target
puppetlabs · May 30, 2018 · 05ebba8 · 05ebba8
2 parents f4f375c + 35e8f8e
commit 05ebba8
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 13 deletions.
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
@@ -109,7 +109,7 @@ def initialize(path)
   def handle(msg)
     if @json > 0
       @json > 1 ? @file.puts(',') : @json = 2
-      Puppet::Util::Json.dump(msg.to_structured_hash, @file)
+      @file.puts(Puppet::Util::Json.dump(msg.to_structured_hash))
     else
       @file.puts("#{msg.time} #{msg.source} (#{msg.level}): #{msg}")
     end

diff --git a/spec/unit/util/log/destinations_spec.rb b/spec/unit/util/log/destinations_spec.rb
@@ -47,24 +47,40 @@
     end
 
     describe "on POSIX systems", :if => Puppet.features.posix? do
-      let (:abspath) { '/tmp/log' }
-      let (:relpath) { 'log' }
+      describe "with a normal file" do
+        let (:abspath) { '/tmp/log' }
+        let (:relpath) { 'log' }
 
-      it_behaves_like "file destination"
+        it_behaves_like "file destination"
+
+        it "logs an error if it can't chown the file owner & group" do
+          FileUtils.expects(:chown).with(Puppet[:user], Puppet[:group], abspath).raises(Errno::EPERM)
+          Puppet.features.expects(:root?).returns(true)
+          Puppet.expects(:err).with("Unable to set ownership to #{Puppet[:user]}:#{Puppet[:group]} for log file: #{abspath}")
 
-      it "logs an error if it can't chown the file owner & group" do
-        FileUtils.expects(:chown).with(Puppet[:user], Puppet[:group], abspath).raises(Errno::EPERM)
-        Puppet.features.expects(:root?).returns(true)
-        Puppet.expects(:err).with("Unable to set ownership to #{Puppet[:user]}:#{Puppet[:group]} for log file: #{abspath}")
+          @class.new(abspath)
+        end
 
-        @class.new(abspath)
+        it "doesn't attempt to chown when running as non-root" do
+          FileUtils.expects(:chown).with(Puppet[:user], Puppet[:group], abspath).never
+          Puppet.features.expects(:root?).returns(false)
+
+          @class.new(abspath)
+        end
       end
 
-      it "doesn't attempt to chown when running as non-root" do
-        FileUtils.expects(:chown).with(Puppet[:user], Puppet[:group], abspath).never
-        Puppet.features.expects(:root?).returns(false)
+      describe "with a JSON file" do
+        let (:abspath) { '/tmp/log.json' }
+        let (:relpath) { 'log.json' }
+
+        it_behaves_like "file destination"
 
-        @class.new(abspath)
+        it "should log messages as JSON" do
+          msg = Puppet::Util::Log.new(:level => :info, :message => "don't panic")
+          dest = @class.new(abspath)
+          dest.handle(msg)
+          expect(JSON.parse(File.read(abspath) + ']')).to include(a_hash_including({"message" => "don't panic"}))
+        end
       end
     end