Merge pull request #222 from codylane/bugfix/tempfile_implementation_…

…on_windows_doesnt_always_seem_to_work_github_issue_220

initial commit to address gh-issue: #220

.travis.yml

@@ -5,7 +5,7 @@ cache: bundler
 sudo: false
 
 before_install: ./travis/before_install
-install: bundle install --path .bundle/gems
+install: make
 script: bundle exec rspec
 bundler_args: --without=development
 
@@ -14,15 +14,16 @@ env:
     - NOKOGIRI_USE_SYSTEM_LIBRARIES=true
 
 rvm:
-  - 2.4.4
+  - 2.6.6
 
 matrix:
   include:
+    - env: VAGRANT_VERSION=v2.2.9
+    - env: VAGRANT_VERSION=v2.2.8
+    - env: VAGRANT_VERSION=v2.2.7
+    - env: VAGRANT_VERSION=v2.2.6
+    - env: VAGRANT_VERSION=v2.2.5
     - env: VAGRANT_VERSION=v2.2.4
-    - env: VAGRANT_VERSION=v2.2.3
-    - env: VAGRANT_VERSION=v2.2.2
-    - env: VAGRANT_VERSION=v2.1.5
-    - env: VAGRANT_VERSION=v2.0.4
     - env: VAGRANT_VERSION=master
   allow_failures:
     - env: VAGRANT_VERSION=master

CHANGELOG.md

@@ -1,3 +1,21 @@
+# 2.0.8 / 2020-06-10
+
+This is a bug fix release for windows users which fixes a bug with the
+tempfile creation when uploading and downloading files from the vagrant
+machine.
+
+#### Closed Issues
+
+- (tempfile implementation on windows doesn't always seem to work
+  #220)[https://github.com/tmatilai/vagrant-proxyconf/issues/220]
+
+#### Credits
+
+Big thanks to @chucknelson for debugging and troubleshooting and getting
+to the bottom of this for windows users. Thank you!
+
+- @chucknelson
+
 # 2.0.7 / 2019-11-14
 
 This is a bug fix release.

Gemfile

@@ -1,25 +1,8 @@
 source 'https://rubygems.org'
 
-#### Added due to https://groups.google.com/forum/#!topic/vagrant-up/J8J6LmhzBqM/discussion
-embedded_locations = %w(/Applications/Vagrant/embedded /opt/vagrant/embedded)
-
-embedded_locations.each do |p|
-    ENV['VAGRANT_INSTALLER_EMBEDDED_DIR'] = p if File.directory?(p)
-end
-
-unless ENV.key?('VAGRANT_INSTALLER_EMBEDDED_DIR')
-    $stderr.puts "Couldn't find a packaged install of vagrant, and we need this"
-    $stderr.puts 'in order to make use of the RubyEncoder libraries.'
-    $stderr.puts 'I looked in:'
-    embedded_locations.each do |p|
-        $stderr.puts "  #{p}"
-    end
-end
-#### End Added due to https://groups.google.com/forum/#!topic/vagrant-up/J8J6LmhzBqM/discussion
-
 gem 'vagrant',
     git: 'https://github.com/hashicorp/vagrant.git',
-    tag: ENV.fetch('VAGRANT_VERSION', 'v2.2.4')
+    tag: ENV.fetch('VAGRANT_VERSION', 'v2.2.9')
 
 gem 'rake'
 gem 'rspec', '~> 3.1'

Makefile

@@ -0,0 +1,33 @@
+.PHONY: clean
+.PHONY: help
+.PHONY: init
+.PHONY: patch_bundler
+
+
+all: clean init patch_bundler
+
+
+clean:
+	rm -f Gemfile.lock
+	rm -rf .bundle/
+
+
+help:
+	@echo "[Commands]"
+	@echo ""
+	@echo "  all           -> The default which runs all tasks"
+	@echo "  clean         -> Deletes Gemfile.lock and .bundle/"
+	@echo "  init          -> Creates the bundle for developing the next release"
+	@echo "  patch_bundler -> Attemps to patch vagrant bundler with known issues"
+	@echo ""
+
+init: clean
+	bundle config set path '.bundle/gems'
+	bundle install
+
+
+patch_bundler:
+	[ "$(VAGRANT_VERSION)" == "v2.2.5" ] && (patch -p0 --batch --backup -d "`bundle info --path vagrant`" < deps/patches/lib/vagrant/bundler.rb.patch) || true
+	[ "$(VAGRANT_VERSION)" == "v2.2.6" ] && (patch -p0 --batch --backup -d "`bundle info --path vagrant`" < deps/patches/lib/vagrant/bundler.rb.patch) || true
+	[ "$(VAGRANT_VERSION)" == "v2.2.7" ] && (patch -p0 --batch --backup -d "`bundle info --path vagrant`" < deps/patches/lib/vagrant/bundler.rb.patch) || true
+	[ "$(VAGRANT_VERSION)" == "v2.2.8" ] && (egrep -q 'cap/redhat' `bundle info --path vagrant`/plugins/provisioners/docker/plugin.rb && sed -i.bak -e 's/redhat/centos/g' `bundle info --path vagrant`/plugins/provisioners/docker/plugin.rb) || true

README.md

@@ -350,4 +350,5 @@ end
 * @hexmode
 * @craigmunro
 * @greut
+* @chucknelson
 * @codylane

deps/patches/lib/vagrant/bundler.rb.patch

@@ -0,0 +1,14 @@
+--- lib/vagrant/bundler.rb	2020-04-27 14:19:39.000000000 -0600
++++ lib/vagrant/bundler.rb	2020-04-27 14:20:01.000000000 -0600
+@@ -421,8 +421,9 @@
+     def vagrant_internal_specs
+       # activate any dependencies up front so we can always
+       # pin them when resolving
+-      Gem::Specification.find { |s| s.name == "vagrant" && s.activated? }.
+-        runtime_dependencies.each { |d| gem d.name, *d.requirement.as_list }
++      if (vs = Gem::Specification.find { |s| s.name == "vagrant" && s.activated? })
++        vs.runtime_dependencies.each { |d| gem d.name, *d.requirement.as_list }
++      end
+       # discover all the gems we have available
+       list = {}
+       directories = [Gem::Specification.default_specifications_dir]

lib/vagrant-proxyconf/action/base.rb

@@ -108,11 +108,18 @@ def escape(value)
 
         # @return [Tempfile] a temporary file with the specified content
         def tempfile(content)
-          Tempfile.new("vagrant").tap do |temp|
-            temp.binmode
-            temp.write(content)
-            temp.close
+          tempfile = Tempfile.new("vagrant-proxyconf")
+
+          begin
+            tempfile.tap do |tmp|
+              tmp.binmode
+              tmp.write(content)
+            end
+          ensure
+            tempfile.close
           end
+
+          tempfile
         end
 
         def cap_name

lib/vagrant-proxyconf/action/configure_docker_proxy.rb

@@ -39,28 +39,28 @@ def unconfigure_machine
           true
         end
 
-        def docker_client_config_path
-          return @docker_client_config_path if @docker_client_config_path
+        def docker_client_config
+          return @docker_client_config if @docker_client_config
           return if !supports_config_json?
 
-          @docker_client_config_path = tempfile(Hash.new)
+          @docker_client_config = tempfile(Hash.new)
 
           @machine.communicate.tap do |comm|
             if comm.test("[ -f /etc/docker/config.json ]")
               logger.info('Downloading file /etc/docker/config.json')
               comm.sudo("chmod 0644 /etc/docker/config.json")
-              comm.download("/etc/docker/config.json", @docker_client_config_path.path)
-              logger.info("Downloaded /etc/docker/config.json to #{@docker_client_config_path.path}")
+              comm.download("/etc/docker/config.json", @docker_client_config.path)
+              logger.info("Downloaded /etc/docker/config.json to #{@docker_client_config.path}")
             end
           end
 
-          @docker_client_config_path = @docker_client_config_path.path
+          @docker_client_config
         end
 
         def update_docker_client_config
-          return if !supports_config_json? || !docker_client_config_path
+          return if !supports_config_json? || !docker_client_config
 
-          content = File.read(@docker_client_config_path)
+          content = File.read(@docker_client_config)
           data  = JSON.load(content)
 
           if disabled?
@@ -92,10 +92,10 @@ def update_docker_client_config
 
           config_json = JSON.pretty_generate(data)
 
-          @docker_client_config_path = tempfile(config_json)
+          @docker_client_config = tempfile(config_json)
 
           @machine.communicate.tap do |comm|
-            comm.upload(@docker_client_config_path.path, "/tmp/vagrant-proxyconf-docker-config.json")
+            comm.upload(@docker_client_config.path, "/tmp/vagrant-proxyconf-docker-config.json")
             comm.sudo("mkdir -p /etc/docker")
             comm.sudo("chown root:docker /etc/docker")
             comm.sudo("mv /tmp/vagrant-proxyconf-docker-config.json /etc/docker/config.json")
@@ -104,9 +104,6 @@ def update_docker_client_config
             comm.sudo("rm -f /tmp/vagrant-proxyconf-docker-config.json")
 
             comm.sudo("sed -i.bak -e '/^DOCKER_CONFIG/d' /etc/environment")
-            if !disabled?
-              comm.sudo("echo DOCKER_CONFIG=/etc/docker >> /etc/environment")
-            end
           end
 
           config_json
@@ -136,12 +133,12 @@ def update_docker_systemd_config
           end
 
           systemd_config = docker_systemd_config
-          @docker_systemd_config = tempfile(systemd_config).path
+          @docker_systemd_config = tempfile(systemd_config)
 
           @machine.communicate.tap do |comm|
 
             comm.sudo("mkdir -p /etc/systemd/system/docker.service.d")
-            comm.upload(@docker_systemd_config, "/tmp/vagrant-proxyconf-docker-systemd-config")
+            comm.upload(@docker_systemd_config.path, "/tmp/vagrant-proxyconf-docker-systemd-config")
 
             if comm.test("diff -Naur /etc/systemd/system/docker.service.d/http-proxy.conf /tmp/vagrant-proxyconf-docker-systemd-config")
               # system config file is the same as the current config

lib/vagrant-proxyconf/version.rb

@@ -1,5 +1,5 @@
 module VagrantPlugins
   module ProxyConf
-    VERSION = '2.0.7'
+    VERSION = '2.0.8'
   end
 end

spec/unit/vagrant-proxyconf/action/configure_docker_proxy_spec.rb

@@ -72,9 +72,9 @@ def configure_docker_proxy(fixture)
         docker_proxy = described_class.new(app, env)
         docker_proxy.instance_variable_set(:@machine, machine)
 
-        # #docker_client_config_path mock
+        # #docker_client_config mock
         fixture = docker_proxy.send(:tempfile, load_fixture(fixture)).path
-        docker_proxy.instance_variable_set(:@docker_client_config_path, fixture)
+        docker_proxy.instance_variable_set(:@docker_client_config, fixture)
 
         # #supported? mock
         allow(machine).to receive_message_chain(:guest, :capability?).with(:docker_proxy_conf).and_return(true)
@@ -124,9 +124,9 @@ def configure_docker_proxy(fixture)
           docker_proxy = described_class.new(app, env)
           docker_proxy.instance_variable_set(:@machine, machine)
 
-          # #docker_client_config_path mock
+          # #docker_client_config mock
           fixture = docker_proxy.send(:tempfile, load_fixture(fixture)).path
-          docker_proxy.instance_variable_set(:@docker_client_config_path, fixture)
+          docker_proxy.instance_variable_set(:@docker_client_config, fixture)
 
           # #supported? mock
           allow(machine).to receive_message_chain(:guest, :capability?).with(:docker_proxy_conf).and_return(true)
@@ -183,7 +183,7 @@ def configure_docker_proxy(fixture)
     end
   end
 
-  describe "#docker_client_config_path" do
+  describe "#docker_client_config" do
     let(:machine) { double('machine') }
 
     context "when not supported" do
@@ -197,7 +197,7 @@ def configure_docker_proxy(fixture)
         allow(machine).to receive_message_chain(:guest, :capability?).with(:docker_proxy_conf).and_return(true)
         allow(machine).to receive_message_chain(:guest, :capability).with(:docker_proxy_conf).and_return('/etc/default/docker')
 
-        docker_proxy.send(:docker_client_config_path)
+        docker_proxy.send(:docker_client_config)
       end
 
       it { is_expected.to eq nil }
@@ -208,35 +208,35 @@ def configure_docker_proxy(fixture)
         subject do
           docker_proxy = described_class.new(nil, nil)
           docker_proxy.instance_variable_set(:@machine, machine)
-          docker_proxy.instance_variable_set(:@docker_client_config_path, nil)
+          docker_proxy.instance_variable_set(:@docker_client_config, nil)
 
           allow(docker_proxy).to receive(:supports_config_json?).and_return(true)
 
           allow(machine).to receive_message_chain(:communicate, :test).with("[ -f /etc/docker/config.json ]").and_return(true)
           allow(machine).to receive_message_chain(:communicate, :sudo).with("chmod 0644 /etc/docker/config.json")
           allow(machine).to receive_message_chain(:communicate, :download)
 
-          docker_proxy.send(:docker_client_config_path)
+          docker_proxy.send(:docker_client_config)
         end
 
-        it { expect(File.exists?(subject)).to eq true }
+        it { expect(File.exists?(subject.path)).to eq true }
       end
 
       context "when /etc/docker/config.json does not exist" do
         subject do
           docker_proxy = described_class.new(nil, nil)
           docker_proxy.instance_variable_set(:@machine, machine)
-          docker_proxy.instance_variable_set(:@docker_client_config_path, nil)
+          docker_proxy.instance_variable_set(:@docker_client_config, nil)
 
           allow(docker_proxy).to receive(:supports_config_json?).and_return(true)
 
           allow(machine).to receive_message_chain(:communicate, :test).with("[ -f /etc/docker/config.json ]").and_return(false)
 
-          docker_proxy.send(:docker_client_config_path)
+          docker_proxy.send(:docker_client_config)
         end
 
         it do
-          expect(File.exists?(subject)).to eq true
+          expect(File.exists?(subject.path)).to eq true
           expect(File.read(subject)).to eq "{}"
         end
       end
@@ -265,14 +265,14 @@ def configure_docker_proxy(fixture)
 
     end
 
-    context "when #docker_client_config_path returns nil" do
+    context "when #docker_client_config returns nil" do
      it 'return nil' do
         docker_proxy = described_class.new(app, env)
         docker_proxy.instance_variable_set(:@machine, machine)
 
         # mock a result that looks like no proxy is configured for the config.json
         allow(docker_proxy).to receive(:supports_config_json?).and_return(true)
-        allow(docker_proxy).to receive(:docker_client_config_path).and_return(nil)
+        allow(docker_proxy).to receive(:docker_client_config).and_return(nil)
 
         # #supported? mock
         allow(machine).to receive_message_chain(:guest, :capability?).with(:docker_proxy_conf).and_return(true)
@@ -294,7 +294,7 @@ def configure_docker_proxy(fixture)
           fixture_content = load_fixture(fixture)
           config_path = docker_proxy.send(:tempfile, fixture_content).path
 
-          docker_proxy.instance_variable_set(:@docker_client_config_path, config_path)
+          docker_proxy.instance_variable_set(:@docker_client_config, config_path)
 
           allow(docker_proxy).to receive(:supports_config_json?).and_return(true)
           allow(docker_proxy).to receive(:disabled?).and_return(true)
@@ -333,7 +333,7 @@ def configure_docker_proxy(fixture)
           fixture_content = load_fixture(fixture)
           config_path = docker_proxy.send(:tempfile, fixture_content).path
 
-          docker_proxy.instance_variable_set(:@docker_client_config_path, config_path)
+          docker_proxy.instance_variable_set(:@docker_client_config, config_path)
 
           allow(docker_proxy).to receive(:supports_config_json?).and_return(true)
           allow(docker_proxy).to receive(:disabled?).and_return(false)
@@ -392,7 +392,7 @@ def configure_docker_proxy(fixture)
 
           fixture = fixture_file("docker_client_config_json_enabled_proxy")
           config_path = docker_proxy.send(:tempfile, load_fixture(fixture)).path
-          docker_proxy.instance_variable_set(:@docker_client_config_path, config_path)
+          docker_proxy.instance_variable_set(:@docker_client_config, config_path)
 
           # to isolate this test, we turn of support for systemd
           allow(docker_proxy).to receive(:supports_systemd?).and_return(false)
@@ -441,7 +441,7 @@ def configure_docker_proxy(fixture)
 
           fixture = fixture_file("docker_client_config_json_enabled_proxy")
           config_path = docker_proxy.send(:tempfile, load_fixture(fixture)).path
-          docker_proxy.instance_variable_set(:@docker_client_config_path, config_path)
+          docker_proxy.instance_variable_set(:@docker_client_config, config_path)
 
           allow(machine).to receive_message_chain(:guest, :capability?).with(:docker_proxy_conf).and_return(true)
           allow(machine).to receive_message_chain(:guest, :capability).with(:docker_proxy_conf).and_return('/etc/default/docker')

test/issues/172/spec/docker_host/redhat_spec.rb

@@ -10,9 +10,9 @@
 describe file('/etc/docker/config.json') do
   it { should be_file }
   it { should exist }
-  it { should be_mode 600 }
+  it { should be_mode 644 }
   it { should be_owned_by "root" }
-  it { should be_grouped_into "root" }
+  it { should be_grouped_into "docker" }
 end
 
 context 'when proxy is enabled' do