Skip to content

Commit f9b945c

Browse files
committed
(PUP-9997) Pass cwd to execute method
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.
1 parent c3de25d commit f9b945c

File tree

2 files changed

+64
-10
lines changed

2 files changed

+64
-10
lines changed

lib/puppet/parser/functions/generate.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
end
3232

3333
begin
34-
Dir.chdir(File.dirname(args[0])) { Puppet::Util::Execution.execute(args).to_str }
34+
dir = File.dirname(args[0])
35+
Puppet::Util::Execution.execute(args, failonfail: true, combine: true, cwd: dir).to_str
3536
rescue Puppet::ExecutionFailure => detail
3637
raise Puppet::ParseError, _("Failed to execute generator %{generator}: %{detail}") % { generator: args[0], detail: detail }, detail.backtrace
3738
end

spec/unit/parser/functions/generate_spec.rb

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,39 @@
11
require 'spec_helper'
22

3+
def with_executor
4+
return yield unless Puppet::Util::Platform.jruby?
5+
6+
Puppet::Util::ExecutionStub.set do |command, options, stdin, stdout, stderr|
7+
require 'open3'
8+
# simulate what puppetserver does
9+
Dir.chdir(options[:cwd]) do
10+
out, err, _status = Open3.capture3(*command)
11+
stdout.write(out)
12+
stderr.write(err)
13+
# execution api expects stdout to be returned
14+
out
15+
end
16+
end
17+
yield
18+
ensure
19+
Puppet::Util::ExecutionStub.reset
20+
end
21+
322
describe "the generate function" do
423
include PuppetSpec::Files
524

625
let :node do Puppet::Node.new('localhost') end
726
let :compiler do Puppet::Parser::Compiler.new(node) end
827
let :scope do Puppet::Parser::Scope.new(compiler) end
28+
let :cwd do tmpdir('generate') end
929

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

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

@@ -35,33 +55,66 @@
3555
end
3656

3757
it "should execute the generate script with the correct working directory" do
38-
command = File.expand_path("/command")
39-
expect(Dir).to receive(:chdir).with(File.dirname(command)).and_return("yay")
40-
expect(scope.function_generate([command])).to eq('yay')
58+
command = File.expand_path("/usr/local/command")
59+
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(cwd: %r{/usr/local})).and_return("yay")
60+
scope.function_generate([command])
61+
end
62+
63+
it "should execute the generate script with failonfail" do
64+
command = File.expand_path("/usr/local/command")
65+
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(failonfail: true)).and_return("yay")
66+
scope.function_generate([command])
67+
end
68+
69+
it "should execute the generate script with combine" do
70+
command = File.expand_path("/usr/local/command")
71+
expect(Puppet::Util::Execution).to receive(:execute).with([command], hash_including(combine: true)).and_return("yay")
72+
scope.function_generate([command])
73+
end
74+
75+
it "executes a command in a working directory" do
76+
if Puppet::Util::Platform.windows?
77+
command = File.join(cwd, 'echo.bat')
78+
File.write(command, <<~END)
79+
@echo off
80+
echo %CD%
81+
END
82+
expect(scope.function_generate([command]).chomp).to match(cwd.gsub('/', '\\'))
83+
else
84+
with_executor do
85+
command = File.join(cwd, 'echo.sh')
86+
File.write(command, <<~END)
87+
#!/bin/sh
88+
echo $PWD
89+
END
90+
Puppet::FileSystem.chmod(0755, command)
91+
expect(scope.function_generate([command]).chomp).to eq(cwd)
92+
end
93+
end
4194
end
4295

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

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

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

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

@@ -81,7 +134,7 @@
81134

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

0 commit comments

Comments
 (0)