Merge pull request #62 from joshcooper/stringify_process_output

Convert ProcessOutput to String explicitly
puppetlabs · Nov 27, 2023 · 05eb6f5 · 05eb6f5
2 parents 8990735 + be5ae9b
commit 05eb6f5
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 8 deletions.
diff --git a/lib/puppet/provider/cron/filetype.rb b/lib/puppet/provider/cron/filetype.rb
@@ -50,7 +50,7 @@ def read
           return ''
         end
 
-        Puppet::Util::Execution.execute("#{cmdbase} -l", failonfail: true, combine: true)
+        Puppet::Util::Execution.execute("#{cmdbase} -l", failonfail: true, combine: true).to_s
       rescue => detail
         case detail.to_s
         when %r{no crontab for}
@@ -71,7 +71,7 @@ def remove
           cmd = "/bin/echo yes | #{cmd}"
         end
 
-        Puppet::Util::Execution.execute(cmd, failonfail: true, combine: true)
+        Puppet::Util::Execution.execute(cmd, failonfail: true, combine: true).to_s
       end
 
       # Overwrite a specific @path's cron tab; must be passed the @path name
@@ -113,7 +113,7 @@ def read
           return ''
         end
 
-        Puppet::Util::Execution.execute(['crontab', '-l'], cronargs)
+        Puppet::Util::Execution.execute(['crontab', '-l'], cronargs).to_s
       rescue => detail
         case detail.to_s
         when %r{can't open your crontab}
@@ -129,7 +129,7 @@ def read
 
       # Remove a specific @path's cron tab.
       def remove
-        Puppet::Util::Execution.execute(['crontab', '-r'], cronargs)
+        Puppet::Util::Execution.execute(['crontab', '-r'], cronargs).to_s
       rescue => detail
         raise FileReadError, _('Could not remove crontab for %{path}: %{detail}') % { path: @path, detail: detail }, detail.backtrace
       end
@@ -144,7 +144,7 @@ def write(text)
           output_file.close
           # We have to chown the stupid file to the user.
           File.chown(Puppet::Util.uid(@path), nil, output_file.path)
-          Puppet::Util::Execution.execute(['crontab', output_file.path], cronargs)
+          Puppet::Util::Execution.execute(['crontab', output_file.path], cronargs).to_s
         rescue => detail
           raise FileReadError, _('Could not write crontab for %{path}: %{detail}') % { path: @path, detail: detail }, detail.backtrace
         ensure
@@ -164,7 +164,7 @@ def read
           return ''
         end
 
-        Puppet::Util::Execution.execute(['crontab', '-l'], cronargs)
+        Puppet::Util::Execution.execute(['crontab', '-l'], cronargs).to_s
       rescue => detail
         case detail.to_s
         when %r{open.*in.*directory}
@@ -180,7 +180,7 @@ def read
 
       # Remove a specific @path's cron tab.
       def remove
-        Puppet::Util::Execution.execute(['crontab', '-r'], cronargs)
+        Puppet::Util::Execution.execute(['crontab', '-r'], cronargs).to_s
       rescue => detail
         raise FileReadError, _('Could not remove crontab for %{path}: %{detail}') % { path: @path, detail: detail }, detail.backtrace
       end
@@ -196,7 +196,7 @@ def write(text)
           output_file.close
           # We have to chown the stupid file to the user.
           File.chown(Puppet::Util.uid(@path), nil, output_file.path)
-          Puppet::Util::Execution.execute(['crontab', output_file.path], cronargs)
+          Puppet::Util::Execution.execute(['crontab', output_file.path], cronargs).to_s
         rescue => detail
           raise FileReadError, _('Could not write crontab for %{path}: %{detail}') % { path: @path, detail: detail }, detail.backtrace
         ensure

diff --git a/spec/fixtures/unit/provider/cron/filetype/managed_output b/spec/fixtures/unit/provider/cron/filetype/managed_output
@@ -0,0 +1,4 @@
+# HEADER: This file was autogenerated at 2023-11-21 12:32:27 -0700 by puppet.
+# HEADER: While it can still be managed manually, it is definitely not recommended.
+# HEADER: Note particularly that the comments starting with 'Puppet Name' should
+# HEADER: not be deleted, as doing so could cause duplicate cron jobs.
diff --git a/spec/unit/provider/cron/filetype_spec.rb b/spec/unit/provider/cron/filetype_spec.rb
@@ -6,6 +6,7 @@
   shared_examples_for 'crontab provider' do
     let(:cron)         { type.new('no_such_user') }
     let(:crontab)      { File.read(my_fixture(crontab_output)) }
+    let(:managedtab)   { File.read(my_fixture('managed_output')) }
     let(:options)      { { failonfail: true, combine: true } }
     let(:uid)          { 'no_such_user' }
     let(:user_options) { options.merge(uid: uid) }
@@ -30,6 +31,11 @@
         expect(cron.read).to eq(crontab)
       end
 
+      it 'returns a String' do
+        expect(Puppet::Util::Execution).to receive(:execute).with(['crontab', '-l'], user_options).and_return(Puppet::Util::Execution::ProcessOutput.new(managedtab, 0))
+        expect(cron.read).to be_an_instance_of(String)
+      end
+
       it 'does not switch user if current user is the target user' do
         expect(Puppet::Util).to receive(:uid).with(uid).twice.and_return 9000
         expect(Puppet::Util::SUIDManager).to receive(:uid).and_return 9000