Skip to content

Commit

Permalink
(PUP-9997) Pass cwd to execute method
Browse files Browse the repository at this point in the history
Dir.chdir is problematic because it affects all threads in the current process
and if puppet is started with a current working directory it doesn't have
traverse/execute permission to, then it won't be able to restore the cwd at the
end of the Dir.chdir block.

Puppet supports three execution implementations: posix, windows and stub. The
first two already support the `cwd` option. Puppetserver injects its stub
implementation and it recently added support for `cwd`, see SERVER-3051.
  • Loading branch information
joshcooper committed Jun 12, 2024
1 parent c3de25d commit f9b945c
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 10 deletions.
3 changes: 2 additions & 1 deletion lib/puppet/parser/functions/generate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
end

begin
Dir.chdir(File.dirname(args[0])) { Puppet::Util::Execution.execute(args).to_str }
dir = File.dirname(args[0])
Puppet::Util::Execution.execute(args, failonfail: true, combine: true, cwd: dir).to_str
rescue Puppet::ExecutionFailure => detail
raise Puppet::ParseError, _("Failed to execute generator %{generator}: %{detail}") % { generator: args[0], detail: detail }, detail.backtrace
end
Expand Down
71 changes: 62 additions & 9 deletions spec/unit/parser/functions/generate_spec.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,39 @@
require 'spec_helper'

def with_executor
return yield unless Puppet::Util::Platform.jruby?

Puppet::Util::ExecutionStub.set do |command, options, stdin, stdout, stderr|
require 'open3'
# simulate what puppetserver does
Dir.chdir(options[:cwd]) do
out, err, _status = Open3.capture3(*command)
stdout.write(out)
stderr.write(err)
# execution api expects stdout to be returned
out
end
end
yield
ensure
Puppet::Util::ExecutionStub.reset
end

describe "the generate function" do
include PuppetSpec::Files

let :node do Puppet::Node.new('localhost') end
let :compiler do Puppet::Parser::Compiler.new(node) end
let :scope do Puppet::Parser::Scope.new(compiler) end
let :cwd do tmpdir('generate') end

it "should exist" do
expect(Puppet::Parser::Functions.function("generate")).to eq("function_generate")
end

it "accept a fully-qualified path as a command" do
command = File.expand_path('/command/foo')
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], anything).and_return("yay")
expect(scope.function_generate([command])).to eq("yay")
end

Expand All @@ -35,33 +55,66 @@
end

it "should execute the generate script with the correct working directory" do
command = File.expand_path("/command")
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
command = File.expand_path("/usr/local/command")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: %r{/usr/local})).and_return("yay")
scope.function_generate([command])
end

it "should execute the generate script with failonfail" do
command = File.expand_path("/usr/local/command")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(failonfail: true)).and_return("yay")
scope.function_generate([command])
end

it "should execute the generate script with combine" do
command = File.expand_path("/usr/local/command")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(combine: true)).and_return("yay")
scope.function_generate([command])
end

it "executes a command in a working directory" do
if Puppet::Util::Platform.windows?
command = File.join(cwd, 'echo.bat')
File.write(command, <<~END)
@echo off
echo %CD%
END
expect(scope.function_generate([command]).chomp).to match(cwd.gsub('/', '\\'))
else
with_executor do
command = File.join(cwd, 'echo.sh')
File.write(command, <<~END)
#!/bin/sh
echo $PWD
END
Puppet::FileSystem.chmod(0755, command)
expect(scope.function_generate([command]).chomp).to eq(cwd)
end
end
end

describe "on Windows", :if => Puppet::Util::Platform.windows? do
it "should accept the tilde in the path" do
command = "C:/DOCUME~1/ADMINI~1/foo.bat"
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: "C:/DOCUME~1/ADMINI~1")).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
end

it "should accept lower-case drive letters" do
command = 'd:/command/foo'
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: "d:/command")).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
end

it "should accept upper-case drive letters" do
command = 'D:/command/foo'
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: "D:/command")).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
end

it "should accept forward and backslashes in the path" do
command = 'D:\command/foo\bar'
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: 'D:\command/foo')).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
end

Expand All @@ -81,7 +134,7 @@

it "should accept plus and dash" do
command = "/var/folders/9z/9zXImgchH8CZJh6SgiqS2U+++TM/-Tmp-/foo"
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: '/var/folders/9z/9zXImgchH8CZJh6SgiqS2U+++TM/-Tmp-')).and_return("yay")
expect(scope.function_generate([command])).to eq('yay')
end
end
Expand Down

0 comments on commit f9b945c

Please sign in to comment.