aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJosh Cooper <joshcooper@users.noreply.github.com>2023-11-22 10:54:32 -0800
committerJosh Cooper <joshcooper@users.noreply.github.com>2023-11-22 15:13:28 -0800
commitbe5ae9bc9e198a00f2a790992b663dda374006ba (patch)
tree60eaae0ad250b7e6efd7329873be74dd77813649
parent8990735b40f5aa0e95b444aaff8af08feab35968 (diff)
downloadpuppet-cron_core-be5ae9bc9e198a00f2a790992b663dda374006ba.tar.gz
puppet-cron_core-be5ae9bc9e198a00f2a790992b663dda374006ba.tar.bz2
Convert ProcessOutput to String explicitly
The `filetype` provider executes `crontab` using Puppet's execution API, which returns ProcessOutput objects that inherit from String. See puppetlabs/puppet@732d450 The provider later uses String#gsub to strip off the HEADER. In Ruby 2.7, the gsub method returns a new instance of ProcessOutput: irb(main):002:0> Puppet::Util::Execution::ProcessOutput.new("# HEADER\n0 4 * * * /etc/init.d/script.sh\n", 0).gsub(/# HEADER/, '').class => Puppet::Util::Execution::ProcessOutput If you later serialize the crontab entries to YAML using `puppet resource`, then puppet warns about serializing unknown data types: # puppet resource cron --to_yaml Warning: Cron[unmanaged:/etc/init.d/script.sh-1]['command'] contains a Puppet::Util::Execution::ProcessOutput value. It will be converted to the String '/etc/init.d/script.sh' This wasn't an issue with Ruby 3.2, because String#gsub always returns a String: irb(main):002:0> Puppet::Util::Execution::ProcessOutput.new("# HEADER\n0 4 * * * /etc/init.d/script.sh\n", 0).gsub(/# HEADER/, '').class => String This commit explicitly converts the ProcessOutput to a String so the provider behaves consistently on all Ruby versions. Fixes #61
-rw-r--r--lib/puppet/provider/cron/filetype.rb16
-rw-r--r--spec/fixtures/unit/provider/cron/filetype/managed_output4
-rw-r--r--spec/unit/provider/cron/filetype_spec.rb6
3 files changed, 18 insertions, 8 deletions
diff --git a/lib/puppet/provider/cron/filetype.rb b/lib/puppet/provider/cron/filetype.rb
index 455ec07..3badeff 100644
--- a/lib/puppet/provider/cron/filetype.rb
+++ b/lib/puppet/provider/cron/filetype.rb
@@ -50,7 +50,7 @@ class Puppet::Provider::Cron
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 @@ class Puppet::Provider::Cron
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 @@ class Puppet::Provider::Cron
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 @@ class Puppet::Provider::Cron
# 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 @@ class Puppet::Provider::Cron
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 @@ class Puppet::Provider::Cron
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 @@ class Puppet::Provider::Cron
# 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 @@ class Puppet::Provider::Cron
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
new file mode 100644
index 0000000..684eb5f
--- /dev/null
+++ 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
index bd579c1..8dc0c32 100644
--- a/spec/unit/provider/cron/filetype_spec.rb
+++ b/spec/unit/provider/cron/filetype_spec.rb
@@ -6,6 +6,7 @@ describe Puppet::Provider::Cron::FileType do
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 @@ describe Puppet::Provider::Cron::FileType do
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