Merge pull request #4473 from rtkrruvinskiy/https_metadata

Add HTTPS download options to `box update` and `box outdated`

lib/vagrant/action/builtin/box_check_outdated.rb

@@ -37,13 +37,23 @@ module Vagrant
           end
 
           constraints = machine.config.vm.box_version
+          # Have download options specified in the environment override
+          # options specified for the machine.
+          download_options = {
+            ca_cert: env[:ca_cert] || machine.config.vm.box_download_ca_cert,
+            ca_path: env[:ca_path] || machine.config.vm.box_download_ca_path,
+            client_cert: env[:client_cert] ||
+                           machine.config.vm.box_download_client_cert,
+            insecure: !env[:insecure].nil? ?
+                        env[:insecure] : machine.config.vm.box_download_insecure
+          }
 
           env[:ui].output(I18n.t(
             "vagrant.box_outdated_checking_with_refresh",
             name: box.name))
           update = nil
           begin
-            update = box.has_update?(constraints)
+            update = box.has_update?(constraints, download_options: download_options)
           rescue Errors::BoxMetadataDownloadError => e
             env[:ui].warn(I18n.t(
               "vagrant.box_outdated_metadata_download_error",

lib/vagrant/box.rb

@@ -115,8 +115,9 @@ module Vagrant
     # Loads the metadata URL and returns the latest metadata associated
     # with this box.
     #
+    # @param [Hash] download_options Options to pass to the downloader.
     # @return [BoxMetadata]
-    def load_metadata
+    def load_metadata(**download_options)
       tf = Tempfile.new("vagrant")
       tf.close
 
@@ -127,7 +128,7 @@ module Vagrant
         url = "file:#{url}"
       end
 
-      opts = { headers: ["Accept: application/json"] }
+      opts = { headers: ["Accept: application/json"] }.merge(download_options)
       Util::Downloader.new(url, tf.path, **opts).download!
       BoxMetadata.new(File.open(tf.path, "r"))
     rescue Errors::DownloaderError => e
@@ -148,7 +149,7 @@ module Vagrant
     #   satisfy. If nil, the version constrain defaults to being a
     #   larger version than this box.
     # @return [Array]
-    def has_update?(version=nil)
+    def has_update?(version=nil, download_options: {})
       if !@metadata_url
         raise Errors::BoxUpdateNoMetadata, name: @name
       end
@@ -156,7 +157,7 @@ module Vagrant
       version += ", " if version
       version ||= ""
       version += "> #{@version}"
-      md      = self.load_metadata
+      md      = self.load_metadata(download_options)
       newer   = md.version(version, provider: @provider)
       return nil if !newer
 

plugins/commands/box/command/add.rb

@@ -1,9 +1,13 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Add < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
 
@@ -21,22 +25,7 @@ module VagrantPlugins
               options[:force] = f
             end
 
-            o.on("--insecure", "Do not validate SSL certificates") do |i|
+            build_download_options(o, options)
-              options[:insecure] = i
-            end
-
-            o.on("--cacert FILE", String, "CA certificate for SSL download") do |c|
-              options[:ca_cert] = c
-            end
-
-            o.on("--capath DIR", String, "CA certificate directory for SSL download") do |c|
-              options[:ca_path] = c
-            end
-
-            o.on("--cert FILE", String,
-                 "A client SSL cert, if needed") do |c|
-              options[:client_cert] = c
-            end
 
             o.on("--location-trusted", "Trust 'Location' header from HTTP redirects and use the same credentials for subsequent urls as for the initial one") do |l|
                 options[:location_trusted] = l
@@ -97,7 +86,7 @@ module VagrantPlugins
             box_force: options[:force],
             box_download_ca_cert: options[:ca_cert],
             box_download_ca_path: options[:ca_path],
-            box_download_client_cert: options[:client_cert],
+            box_client_cert: options[:client_cert],
             box_download_insecure: options[:insecure],
             box_download_location_trusted: options[:location_trusted],
             ui: Vagrant::UI::Prefixed.new(@env.ui, "box"),

plugins/commands/box/command/download_mixins.rb

@@ -0,0 +1,29 @@
+module VagrantPlugins
+  module CommandBox
+    module DownloadMixins
+      # This adds common download command line flags to the given
+      # OptionParser, storing the result in the `options` dictionary.
+      #
+      # @param [OptionParser] parser
+      # @param [Hash] options
+      def build_download_options(parser, options)
+        # Add the options
+        parser.on("--insecure", "Do not validate SSL certificates") do |i|
+          options[:insecure] = i
+        end
+
+        parser.on("--cacert FILE", String, "CA certificate for SSL download") do |c|
+          options[:ca_cert] = c
+        end
+
+        parser.on("--capath DIR", String, "CA certificate directory for SSL download") do |c|
+          options[:ca_path] = c
+        end
+
+        parser.on("--cert FILE", String, "A client SSL cert, if needed") do |c|
+          options[:client_cert] = c
+        end
+      end
+    end
+  end
+end

plugins/commands/box/command/outdated.rb

@@ -1,11 +1,16 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Outdated < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
+          download_options = {}
 
           opts = OptionParser.new do |o|
             o.banner = "Usage: vagrant box outdated [options]"
@@ -20,6 +25,8 @@ module VagrantPlugins
             o.on("--global", "Check all boxes installed") do |g|
               options[:global] = g
             end
+
+            build_download_options(o, download_options)
           end
 
           argv = parse_options(opts)
@@ -27,7 +34,7 @@ module VagrantPlugins
 
           # If we're checking the boxes globally, then do that.
           if options[:global]
-            outdated_global
+            outdated_global(download_options)
             return 0
           end
 
@@ -37,11 +44,11 @@ module VagrantPlugins
               box_outdated_refresh: true,
               box_outdated_success_ui: true,
               machine: machine,
-            })
+            }.merge(download_options))
           end
         end
 
-        def outdated_global
+        def outdated_global(download_options)
           boxes = {}
           @env.boxes.all.reverse.each do |name, version, provider|
             next if boxes[name]
@@ -58,7 +65,7 @@ module VagrantPlugins
 
             md = nil
             begin
-              md = box.load_metadata
+              md = box.load_metadata(download_options)
             rescue Vagrant::Errors::DownloaderError => e
               @env.ui.error(I18n.t(
                 "vagrant.box_outdated_metadata_error",

plugins/commands/box/command/update.rb

@@ -1,11 +1,16 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Update < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
+          download_options = {}
 
           opts = OptionParser.new do |o|
             o.banner = "Usage: vagrant box update [options]"
@@ -27,21 +32,23 @@ module VagrantPlugins
             o.on("--provider PROVIDER", String, "Update box with specific provider") do |p|
               options[:provider] = p.to_sym
             end
+
+            build_download_options(o, download_options)
           end
 
           argv = parse_options(opts)
           return if !argv
 
           if options[:box]
-            update_specific(options[:box], options[:provider])
+            update_specific(options[:box], options[:provider], download_options)
           else
-            update_vms(argv, options[:provider])
+            update_vms(argv, options[:provider], download_options)
           end
 
           0
         end
 
-        def update_specific(name, provider)
+        def update_specific(name, provider, download_options)
           boxes = {}
           @env.boxes.all.each do |n, v, p|
             boxes[n] ||= {}
@@ -74,11 +81,11 @@ module VagrantPlugins
 
           to_update.each do |n, p, v|
             box = @env.boxes.find(n, p, v)
-            box_update(box, "> #{v}", @env.ui)
+            box_update(box, "> #{v}", @env.ui, download_options)
           end
         end
 
-        def update_vms(argv, provider)
+        def update_vms(argv, provider, download_options)
           with_target_vms(argv, provider: provider) do |machine|
             if !machine.config.vm.box
               machine.ui.output(I18n.t(
@@ -95,17 +102,25 @@ module VagrantPlugins
 
             box = machine.box
             version = machine.config.vm.box_version
-            box_update(box, version, machine.ui)
+            # Get download options from machine configuration if not specified
+            # on the command line.
+            download_options[:ca_cert] ||= machine.config.vm.box_download_ca_cert
+            download_options[:ca_path] ||= machine.config.vm.box_download_ca_path
+            download_options[:client_cert] ||= machine.config.vm.box_download_client_cert
+            if download_options[:insecure].nil?
+              download_options[:insecure] = machine.config.vm.box_download_insecure
+            end
+            box_update(box, version, machine.ui, download_options)
           end
         end
 
-        def box_update(box, version, ui)
+        def box_update(box, version, ui, download_options)
           ui.output(I18n.t("vagrant.box_update_checking", name: box.name))
           ui.detail("Latest installed version: #{box.version}")
           ui.detail("Version constraints: #{version}")
           ui.detail("Provider: #{box.provider}")
 
-          update = box.has_update?(version)
+          update = box.has_update?(version, download_options: download_options)
           if !update
             ui.success(I18n.t(
               "vagrant.box_up_to_date_single",
@@ -124,6 +139,10 @@ module VagrantPlugins
             box_provider: update[2].name,
             box_version: update[1].version,
             ui: ui,
+            box_client_cert: download_options[:client_cert],
+            box_download_ca_cert: download_options[:ca_cert],
+            box_download_ca_path: download_options[:ca_path],
+            box_download_insecure: download_options[:insecure]
           })
         end
       end

test/unit/plugins/commands/box/command/update_test.rb

@@ -19,6 +19,11 @@ describe VagrantPlugins::CommandBox::Command::Update do
   let(:action_runner) { double("action_runner") }
   let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) }
 
+  let(:download_options) { ["--insecure",
+                            "--cacert", "foo",
+                            "--capath", "bar",
+                            "--cert", "baz"] }
+
   subject { described_class.new(argv, iso_env) }
 
   before do
@@ -86,6 +91,10 @@ describe VagrantPlugins::CommandBox::Command::Update do
             expect(opts[:box_url]).to eq(metadata_url.to_s)
             expect(opts[:box_provider]).to eq("virtualbox")
             expect(opts[:box_version]).to eq("1.1")
+            expect(opts[:box_download_ca_path]).to be_nil
+            expect(opts[:box_download_ca_cert]).to be_nil
+            expect(opts[:box_client_cert]).to be_nil
+            expect(opts[:box_download_insecure]).to be_nil
           end
 
           opts
@@ -158,6 +167,50 @@ describe VagrantPlugins::CommandBox::Command::Update do
         end
       end
 
+      context "download options are specified" do
+        let(:argv) { ["--box", "foo" ].concat(download_options) }
+
+        it "passes down download options" do
+          metadata_url.open("w") do |f|
+            f.write(<<-RAW)
+        {
+          "name": "foo",
+          "versions": [
+            {
+              "version": "1.0"
+            },
+            {
+              "version": "1.1",
+              "providers": [
+                {
+                  "name": "virtualbox",
+                  "url": "bar"
+                }
+              ]
+            }
+          ]
+        }
+            RAW
+          end
+
+          action_called = false
+          allow(action_runner).to receive(:run) do |action, opts|
+            if opts[:box_provider]
+              action_called = true
+              expect(opts[:box_download_ca_cert]).to eq("foo")
+              expect(opts[:box_download_ca_path]).to eq("bar")
+              expect(opts[:box_client_cert]).to eq("baz")
+              expect(opts[:box_download_insecure]).to be_true
+            end
+
+            opts
+          end
+
+          subject.execute
+          expect(action_called).to be_true
+        end
+      end
+
       context "with a box that doesn't exist" do
         let(:argv) { ["--box", "nope"] }
 
@@ -192,7 +245,10 @@ describe VagrantPlugins::CommandBox::Command::Update do
       it "doesn't update boxes if they're up-to-date" do
         machine.stub(box: box)
         expect(box).to receive(:has_update?).
-          with(machine.config.vm.box_version).
+          with(machine.config.vm.box_version,
+               {download_options:
+                 {ca_cert: nil, ca_path: nil, client_cert: nil,
+                  insecure: false}}).
           and_return(nil)
 
         expect(action_runner).to receive(:run).never
@@ -200,7 +256,8 @@ describe VagrantPlugins::CommandBox::Command::Update do
         subject.execute
       end
 
-      it "updates boxes if they have an update" do
+      context "boxes have an update" do
+        let(:md) {
           md = Vagrant::BoxMetadata.new(StringIO.new(<<-RAW))
         {
           "name": "foo",
@@ -220,10 +277,16 @@ describe VagrantPlugins::CommandBox::Command::Update do
           ]
         }
           RAW
+        }
 
-        machine.stub(box: box)
+        before { machine.stub(box: box) }
+
+        it "updates boxes" do
           expect(box).to receive(:has_update?).
-          with(machine.config.vm.box_version).
+            with(machine.config.vm.box_version,
+                 {download_options:
+                   {ca_cert: nil, ca_path: nil, client_cert: nil,
+                    insecure: false}}).
             and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
 
           expect(action_runner).to receive(:run).with { |action, opts|
@@ -236,6 +299,59 @@ describe VagrantPlugins::CommandBox::Command::Update do
 
           subject.execute
         end
+
+        context "machine has download options" do
+          before do
+            machine.config.vm.box_download_ca_cert = "oof"
+            machine.config.vm.box_download_ca_path = "rab"
+            machine.config.vm.box_download_client_cert = "zab"
+            machine.config.vm.box_download_insecure = false
+          end
+
+          it "uses download options from machine" do
+            expect(box).to receive(:has_update?).
+              with(machine.config.vm.box_version,
+                   {download_options:
+                     {ca_cert: "oof", ca_path: "rab", client_cert: "zab",
+                      insecure: false}}).
+              and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
+
+            expect(action_runner).to receive(:run).with { |action, opts|
+              expect(opts[:box_download_ca_cert]).to eq("oof")
+              expect(opts[:box_download_ca_path]).to eq("rab")
+              expect(opts[:box_client_cert]).to eq("zab")
+              expect(opts[:box_download_insecure]).to be_false
+              true
+            }
+
+            subject.execute
+          end
+
+          context "download options are specified on the command line" do
+            let(:argv) { download_options }
+
+            it "overrides download options from machine with options from CLI" do
+              expect(box).to receive(:has_update?).
+                with(machine.config.vm.box_version,
+                     {download_options:
+                       {ca_cert: "foo", ca_path: "bar", client_cert: "baz",
+                        insecure: true}}).
+                and_return([md, md.version("1.1"),
+                            md.version("1.1").provider("virtualbox")])
+
+              expect(action_runner).to receive(:run).with { |action, opts|
+                expect(opts[:box_download_ca_cert]).to eq("foo")
+                expect(opts[:box_download_ca_path]).to eq("bar")
+                expect(opts[:box_client_cert]).to eq("baz")
+                expect(opts[:box_download_insecure]).to be_true
+                true
+              }
+
+              subject.execute
+            end
+          end
+        end
+      end
     end
   end
 end

test/unit/vagrant/action/builtin/box_check_outdated_test.rb

@@ -117,7 +117,9 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
       }
       RAW
 
-      expect(box).to receive(:has_update?).with(machine.config.vm.box_version).
+      expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: nil, ca_path: nil, client_cert: nil, insecure: false}}).
         and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
 
       expect(app).to receive(:call).with(env).once
@@ -180,5 +182,38 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
 
       expect { subject.call(env) }.to_not raise_error
     end
+
+    context "when machine download options are specified" do
+      before do
+        machine.config.vm.box_download_ca_cert = "foo"
+        machine.config.vm.box_download_ca_path = "bar"
+        machine.config.vm.box_download_client_cert = "baz"
+        machine.config.vm.box_download_insecure = true
+      end
+
+      it "uses download options from machine" do
+        expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: "foo", ca_path: "bar", client_cert: "baz", insecure: true}})
+
+        expect(app).to receive(:call).with(env).once
+
+        subject.call(env)
+      end
+
+      it "overrides download options from machine with options from env" do
+        expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: "oof", ca_path: "rab", client_cert: "zab", insecure: false}})
+
+        env[:ca_cert] = "oof"
+        env[:ca_path] = "rab"
+        env[:client_cert] = "zab"
+        env[:insecure] = false
+        expect(app).to receive(:call).with(env).once
+
+        subject.call(env)
+      end
+    end
   end
 end