Skip to content

Commit

Permalink
(PUP-9194) Use the cwd param. for check commands (e.g. unless, onlyif)
Browse files Browse the repository at this point in the history
PUP-6919 introduced a regression where check commands like unless
and onlyif no longer respected the cwd parameter. Instead, they would
always execute in the pwd. The regression came about due to a
misinterpretation of the code in the original base Exec provider.
Specifically, the base Exec provider was cleaned up to utilize the
newly added :cwd option in Puppet::Util::Execution.execute. The cleanup
tried to preserve the original semantics of the code, but ended up
mistaking an unless statement for an if statement, which made it seem like
a check command did not use the cwd parameter (when, in fact, it did).

This commit fixes the base Exec provider to now use the cwd parameter for
check commands. It also updates the acceptance tests with this
change.
  • Loading branch information
ekinanp committed Oct 3, 2018
1 parent 3acaa21 commit c127bcb
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 23 deletions.
29 changes: 10 additions & 19 deletions acceptance/tests/resource/exec/should_run_command_in_cwd.rb
Expand Up @@ -98,29 +98,20 @@ def to_regex(str)
end
end

step 'Runs a "check" command (:onlyif or :unless) in the PWD instead of the passed-in CWD' do
# Idea here is to create a directory (subdir) inside our tmpdir.
# This will be our CWD. We run our print_cwd command unless
# subdir exists, which can only happen if our CWD is our tmpdir.
# Thus, if print_cwd runs successfully, that means our unless
# check indicated that subdir did not exist, meaning it ran in our
# pwd instead of our CWD. Note that this is an approximation, but
# I think it is a good (and simple enough) test to ensure that we
# enforce this property.
subdir = 'FOO'
step 'Runs a "check" command (:onlyif or :unless) in the CWD' do
cwd = "#{tmpdir}"
onlyif = print_cwd
if agent.platform =~ /windows/
cwd = "#{tmpdir}\\#{subdir}"
on(agent, "cmd.exe /c mkdir '#{cwd}'")
unless_ = "cmd.exe /c dir #{subdir}"
command = "cmd.exe /c echo Hello"
else
cwd = "#{tmpdir}/#{subdir}"
on(agent, "mkdir '#{cwd}'")
unless_ = "ls #{subdir}"
command = "echo Hello"
end

manifest = exec_resource_manifest.call(cwd: cwd, unless: unless_)
apply_manifest_on(agent, manifest) do |result|
assert_match(to_regex(cwd), result.stdout, 'The Exec resource runs a "check" command in the CWD when it should run it in the PWD instead, only using the CWD for the actual command.')
manifest = exec_resource_manifest.call(command: command, cwd: cwd, onlyif: onlyif)

# debug: true prints the output of the onlyif command
apply_manifest_on(agent, manifest, debug: true) do |result|
assert_match(to_regex(cwd), result.stdout, 'The Exec resource runs a "check" command in the PWD instead of the CWD.')
end
end
end
Expand Down
5 changes: 1 addition & 4 deletions lib/puppet/provider/exec.rb
Expand Up @@ -46,10 +46,7 @@ def run(command, check = false)
#
# This is backwards compatible all the way to Ruby 1.8.7.
Timeout::timeout(resource[:timeout], Timeout::Error) do
# If we're running a command that's meant to be a check on whether we should run
# our actual command (e.g. like a command passed to the :onlyif or :unless properties),
# then we should not set the cwd when executing the check's corresponding command.
cwd = check ? nil : resource[:cwd]
cwd = resource[:cwd]
cwd ||= Dir.pwd

# note that we are passing "false" for the "override_locale" parameter, which ensures that the user's
Expand Down

0 comments on commit c127bcb

Please sign in to comment.