From 3fd53ba896d541fe460b2fd12ea1a5e5392ae7b7 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 24 May 2024 18:04:01 -0700 Subject: [PATCH] (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. --- lib/puppet/parser/functions/generate.rb | 3 +- spec/unit/parser/functions/generate_spec.rb | 73 ++++++++++++++++++--- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/lib/puppet/parser/functions/generate.rb b/lib/puppet/parser/functions/generate.rb index b81b5c04cab..b52eb4a7f36 100644 --- a/lib/puppet/parser/functions/generate.rb +++ b/lib/puppet/parser/functions/generate.rb @@ -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 diff --git a/spec/unit/parser/functions/generate_spec.rb b/spec/unit/parser/functions/generate_spec.rb index b038d606f2f..2d2254510b0 100644 --- a/spec/unit/parser/functions/generate_spec.rb +++ b/spec/unit/parser/functions/generate_spec.rb @@ -1,11 +1,33 @@ require 'spec_helper' +def with_executor + return yield unless Puppet::Util::Platform.jruby? + + begin + 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 +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") @@ -13,7 +35,7 @@ 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 @@ -35,33 +57,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 @@ -81,7 +136,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