Skip to content

Commit

Permalink
Merge pull request #191 from ksubrama/bundler
Browse files Browse the repository at this point in the history
Escape bundler environment when shelling out to fix #190
  • Loading branch information
tyler-ball committed Sep 3, 2015
2 parents 3d8d9f0 + 202d5fd commit cf89394
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 9 deletions.
2 changes: 2 additions & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--color
-fd
27 changes: 26 additions & 1 deletion lib/kitchen/driver/vagrant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,36 @@ def run(cmd, options = {})
# * `:log_subject` defaults to a String representation of the Driver's
# class name
#
# Since vagrant does not support being run through bundler, we escape
# any bundler environment should we detect one. Otherwise, subcommands
# will inherit our bundled environment.
# @see https://github.com/test-kitchen/kitchen-vagrant/issues/190
# @see Kitchen::ShellOut#run_command
def run_command(cmd, options = {})
merged = {
:use_sudo => config[:use_sudo], :log_subject => name
:use_sudo => config[:use_sudo],
:log_subject => name,
:environment => {}
}.merge(options)

# Attempt to extract bundler and associated GEM related values.
# TODO: To this accurately, we'd need to create a test-kitchen
# launch wrapper that serializes the existing environment before
# bundler touches it so that we can go back to it. Since that is
# "A Hard Problem"(TM), we simply blow away all known bundler
# related changes.
env = merged[:environment]
%w[BUNDLE_BIN_PATH BUNDLE_GEMFILE GEM_HOME GEM_PATH GEM_ROOT RUBY_LIB
RUBY_OPT _ORIGINAL_GEM_PATH].each do |var|
env[var] = nil
end
gem_home = ENV["GEM_HOME"]
if gem_home && (env["PATH"] || ENV["PATH"])
env["PATH"] ||= ENV["PATH"].dup if ENV["PATH"]
gem_bin = File.join(gem_home, "bin") + File::PATH_SEPARATOR
env["PATH"][gem_bin] = "" if env["PATH"].include?(gem_bin)
end

super(cmd, merged)
end

Expand Down
86 changes: 78 additions & 8 deletions spec/kitchen/driver/vagrant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,17 @@
)
end

before { stub_const("ENV", env) }
module RunCommandStub
def run_command(_cmd, options = {})
options
end
end

before(:all) do
Kitchen::Driver::Vagrant.instance_eval { include RunCommandStub }
end

before(:each) { stub_const("ENV", env) }

after do
driver_object.class.send(:winrm_plugin_passed=, nil)
Expand All @@ -77,6 +87,64 @@
Kitchen::Driver::VAGRANT_VERSION)
end

describe "#run_command" do

context "when invoked from a clean environment" do

it "passes through environment variables" do
options = driver.send(
:run_command,
"cmd",
:environment => { "EV1" => "Val1", "EV2" => "Val2" })
expect(options[:environment]["EV1"]).to eq("Val1")
expect(options[:environment]["EV2"]).to eq("Val2")
end

it "leaves path alone" do
path = "/foo/#{File::PATH_SEPARATOR}/bar"
options = driver.send(
:run_command,
"cmd",
:environment => { "PATH" => path })
expect(options[:environment]["PATH"]).to eq(path)
end

end

context "when invoked from a bundler[:environment]" do

let(:bundler_env) do
{
"BUNDLE_BIN_PATH" => "bundle_bin_path",
"BUNDLE_GEMFILE" => "bundle_gem_file",
"GEM_HOME" => "gem_home",
"GEM_PATH" => "gem_path",
"GEM_ROOT" => "gem_root",
"RUBY_LIB" => "ruby_lib",
"RUBY_OPT" => "ruby_opt",
"_ORIGINAL_GEM_PATH" => "original_gem_path"
}
end

it "removes all bundler related variables" do
env.merge!(bundler_env)
options = driver.send(:run_command, "cmd")
bundler_env.each do |k, _v|
expect(options[:environment]).to include(k)
expect(options[:environment][k]).to eq(nil)
end
end

it "fixes path if it notices gem_home in it" do
env.merge!(bundler_env)
env["PATH"] = "gem_home/bin#{File::PATH_SEPARATOR}/something/else"
options = driver.send(:run_command, "cmd")
puts(options)
expect(options[:environment]["PATH"]).to eq("/something/else")
end
end
end

describe "configuration" do

%W[centos debian fedora opensuse ubuntu].each do |name|
Expand Down Expand Up @@ -252,7 +320,7 @@
]

expect(driver[:synced_folders]).to eq([
["/host_path", "/vm_path", "create: true, type: :nfs"]
[File.expand_path("/host_path"), "/vm_path", "create: true, type: :nfs"]
])
end

Expand All @@ -262,7 +330,7 @@
]

expect(driver[:synced_folders]).to eq([
["/root/suitey-fooos-99", "/vm_path", "stuff"]
[File.expand_path("/root/suitey-fooos-99"), "/vm_path", "stuff"]
])
end

Expand All @@ -272,7 +340,7 @@
]

expect(driver[:synced_folders]).to eq([
["/kroot/a", "/vm_path", "stuff"]
[File.expand_path("/kroot/a"), "/vm_path", "stuff"]
])
end

Expand All @@ -282,7 +350,7 @@
]

expect(driver[:synced_folders]).to eq([
["/host_path", "/vm_path", "nil"]
[File.expand_path("/host_path"), "/vm_path", "nil"]
])
end

Expand All @@ -295,13 +363,15 @@
it "sets :vagrantfile_erb to a default value" do
config[:vagrantfile_erb] = "/a/Vagrantfile.erb"

expect(driver[:vagrantfile_erb]).to eq("/a/Vagrantfile.erb")
expect(driver[:vagrantfile_erb]).to eq(
File.expand_path("/a/Vagrantfile.erb"))
end

it "expands path for :vagrantfile_erb" do
config[:vagrantfile_erb] = "Yep.erb"

expect(driver[:vagrantfile_erb]).to eq("/kroot/Yep.erb")
expect(driver[:vagrantfile_erb]).to eq(
File.expand_path("/kroot/Yep.erb"))
end

it "sets :vagrantfiles to an empty array by default" do
Expand All @@ -312,7 +382,7 @@
config[:vagrantfiles] = %W[one two three]

expect(driver[:vagrantfiles]).to eq(
%W[/kroot/one /kroot/two /kroot/three]
%W[/kroot/one /kroot/two /kroot/three].map { |f| File.expand_path(f) }
)
end

Expand Down

0 comments on commit cf89394

Please sign in to comment.