From 7e125969dd78fc19018a834d1c83d730f25dc703 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 28 Apr 2020 15:24:01 -0400 Subject: [PATCH 1/2] Add option `box_download_options` Allow users to specify a map of extra options to pass to the downloader. These options will be passed to curl, with a `--` appended to the key --- lib/vagrant/action/builtin/box_add.rb | 3 +- lib/vagrant/action/builtin/handle_box.rb | 2 ++ lib/vagrant/util/downloader.rb | 5 +++ lib/vagrant/util/map_command_options.rb | 33 +++++++++++++++++++ plugins/kernel_v2/config/vm.rb | 7 ++++ templates/locales/en.yml | 2 ++ test/unit/vagrant/util/downloader_test.rb | 16 +++++++++ .../vagrant/util/map_command_options_test.rb | 21 ++++++++++++ .../docs/vagrantfile/machine_settings.html.md | 6 ++++ 9 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 lib/vagrant/util/map_command_options.rb create mode 100644 test/unit/vagrant/util/map_command_options_test.rb diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index dcd3a8ce4..2f6056149 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -433,7 +433,8 @@ module Vagrant downloader_options[:headers] = ["Accept: application/json"] if opts[:json] downloader_options[:ui] = env[:ui] if opts[:ui] downloader_options[:location_trusted] = env[:box_download_location_trusted] - + downloader_options[:box_download_options] = env[:box_download_options] + Util::Downloader.new(url, temp_path, downloader_options) end diff --git a/lib/vagrant/action/builtin/handle_box.rb b/lib/vagrant/action/builtin/handle_box.rb index 96d7f9f0c..ccbb47c00 100644 --- a/lib/vagrant/action/builtin/handle_box.rb +++ b/lib/vagrant/action/builtin/handle_box.rb @@ -67,6 +67,7 @@ module Vagrant box_download_checksum_type = machine.config.vm.box_download_checksum_type box_download_checksum = machine.config.vm.box_download_checksum box_download_location_trusted = machine.config.vm.box_download_location_trusted + box_download_options = machine.config.vm.box_download_options box_formats = machine.provider_options[:box_format] || machine.provider_name @@ -92,6 +93,7 @@ module Vagrant box_checksum_type: box_download_checksum_type, box_checksum: box_download_checksum, box_download_location_trusted: box_download_location_trusted, + box_download_options: box_download_options, })) rescue Errors::BoxAlreadyExists # Just ignore this, since it means the next part will succeed! diff --git a/lib/vagrant/util/downloader.rb b/lib/vagrant/util/downloader.rb index d13a508be..6a2838222 100644 --- a/lib/vagrant/util/downloader.rb +++ b/lib/vagrant/util/downloader.rb @@ -9,6 +9,7 @@ require "vagrant/util/platform" require "vagrant/util/subprocess" require "vagrant/util/curl_helper" require "vagrant/util/file_checksum" +require "vagrant/util/map_command_options" module Vagrant module Util @@ -69,6 +70,7 @@ module Vagrant :sha384 => options[:sha384], :sha512 => options[:sha512] }.compact + @extra_download_options = options[:box_download_options] end # This executes the actual download, downloading the source file @@ -245,6 +247,9 @@ module Vagrant options << "-u" << @auth if @auth options << "--location-trusted" if @location_trusted + options.concat( + Vagrant::Util::MapCommandOptions.map_to_command_options(@extra_download_options)) + if @headers Array(@headers).each do |header| options << "-H" << header diff --git a/lib/vagrant/util/map_command_options.rb b/lib/vagrant/util/map_command_options.rb new file mode 100644 index 000000000..5accdffac --- /dev/null +++ b/lib/vagrant/util/map_command_options.rb @@ -0,0 +1,33 @@ +module Vagrant + module Util + class MapCommandOptions + # Given a hash map of user specified argments, will generate + # a list. Set the key to the command flag, and the value to + # it's value. If the value is boolean (true), only the flag is + # added. eg. + # {a: "opt-a", b: true} -> ["--a", "opt-a", "--b"] + # + # @param [Hash] map of commands + # @param [String] string prepended to cmd line flags (keys) + # + # @return[Array] commands in list form + def self.map_to_command_options(map, cmd_flag="--") + opt_list = [] + if map == nil + return opt_list + end + map.each do |k, v| + # If the value is true (bool) add the key as the cmd flag + if v.is_a?(TrueClass) + opt_list.push("#{cmd_flag}#{k}") + # If the value is a string, add the key as the flag, and value as the flags argument + elsif v.is_a?(String) + opt_list.push("#{cmd_flag}#{k}") + opt_list.push(v) + end + end + return opt_list + end + end + end +end diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 744649a12..9154e03b3 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -37,6 +37,7 @@ module VagrantPlugins attr_accessor :box_download_client_cert attr_accessor :box_download_insecure attr_accessor :box_download_location_trusted + attr_accessor :box_download_options attr_accessor :communicator attr_accessor :graceful_halt_timeout attr_accessor :guest @@ -66,6 +67,7 @@ module VagrantPlugins @box_download_client_cert = UNSET_VALUE @box_download_insecure = UNSET_VALUE @box_download_location_trusted = UNSET_VALUE + @box_download_options = UNSET_VALUE @box_url = UNSET_VALUE @box_version = UNSET_VALUE @clone = UNSET_VALUE @@ -467,6 +469,7 @@ module VagrantPlugins @box_download_location_trusted = false if @box_download_location_trusted == UNSET_VALUE @box_url = nil if @box_url == UNSET_VALUE @box_version = nil if @box_version == UNSET_VALUE + @box_download_options = {} if @box_download_options == UNSET_VALUE @clone = nil if @clone == UNSET_VALUE @communicator = nil if @communicator == UNSET_VALUE @graceful_halt_timeout = 60 if @graceful_halt_timeout == UNSET_VALUE @@ -714,6 +717,10 @@ module VagrantPlugins end end + if !box_download_options.is_a?(Hash) + errors << I18n.t("vagrant.config.vm.box_download_options_type", type: box_download_options.class.to_s) + end + used_guest_paths = Set.new @__synced_folders.each do |id, options| # If the shared folder is disabled then don't worry about validating it diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 291a8c41a..22e280794 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1938,6 +1938,8 @@ en: box_download_checksum_notblank: |- Checksum specified but must also specify "box_download_checksum_type" box_missing: "A box must be specified." + box_download_options_type: |- + Found "box_download_options" specified as type '%{type}', should be a Hash clone_and_box: "Only one of clone or box can be specified." hostname_invalid_characters: |- The hostname set for the VM '%{name}' should only contain letters, numbers, diff --git a/test/unit/vagrant/util/downloader_test.rb b/test/unit/vagrant/util/downloader_test.rb index 973510259..382f94efa 100644 --- a/test/unit/vagrant/util/downloader_test.rb +++ b/test/unit/vagrant/util/downloader_test.rb @@ -257,6 +257,22 @@ describe Vagrant::Util::Downloader do end end end + + context "when extra download options specified" do + let(:options) { {:box_download_options => {test: "arbitrary"}} } + subject { described_class.new(source, destination, options) } + + it "inserts the extra download options" do + i = curl_options.index("--output") + curl_options.insert(i, "arbitrary") + curl_options.insert(i, "--test") + expect(Vagrant::Util::Subprocess).to receive(:execute). + with("curl", *curl_options). + and_return(subprocess_result) + + expect(subject.download!).to be(true) + end + end end end diff --git a/test/unit/vagrant/util/map_command_options_test.rb b/test/unit/vagrant/util/map_command_options_test.rb new file mode 100644 index 000000000..1f8471133 --- /dev/null +++ b/test/unit/vagrant/util/map_command_options_test.rb @@ -0,0 +1,21 @@ +require File.expand_path("../../../base", __FILE__) + +require 'vagrant/util/map_command_options' + +describe Vagrant::Util::MapCommandOptions do + + subject { described_class } + + it "should convert a map to a list of command options" do + [ + [{a: "opt1", b: true, c: "opt3"}, "--", ["--a", "opt1", "--b", "--c", "opt3"]], + [{a: "opt1", b: false, c: "opt3"}, "-", ["-a", "opt1", "-c", "opt3"]], + [{a: "opt1", b: 1}, "--", ["--a", "opt1"]], + [{a: 1, b: 1}, "--", []], + [{}, "--", []], + [nil, nil, []] + ].each do |map, cmd_flag, expected_output| + expect(subject.map_to_command_options(map, cmd_flag)).to eq(expected_output) + end + end +end diff --git a/website/source/docs/vagrantfile/machine_settings.html.md b/website/source/docs/vagrantfile/machine_settings.html.md index f41e93a0b..cdc56817f 100644 --- a/website/source/docs/vagrantfile/machine_settings.html.md +++ b/website/source/docs/vagrantfile/machine_settings.html.md @@ -60,6 +60,12 @@ bundle. CA certificates for downloading a box directly. By default, Vagrant will use the Mozilla CA cert bundle. +* `config.vm.box_download_options` (map) - A map of extra download options +to pass to the downloader. For example, a path to a key that the downloader +should use could be specified as `{key: ""}`. The keys should +be options supported by `curl` using the unshortened form of the flag. For +example, use `append` instead of `a`. + * `config.vm.box_download_insecure` (boolean) - If true, then SSL certificates from the server will not be verified. By default, if the URL is an HTTPS URL, then SSL certs will be verified. From 646de433a9f278b4b59f16ada9de4484f0562ccc Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 29 Apr 2020 14:44:45 -0400 Subject: [PATCH 2/2] Validate conversion of map to cmd options --- lib/vagrant/action/builtin/box_add.rb | 2 +- lib/vagrant/action/builtin/handle_box.rb | 4 ++-- lib/vagrant/util/downloader.rb | 5 ++--- plugins/kernel_v2/config/vm.rb | 12 ++++++++++++ templates/locales/en.yml | 2 ++ test/unit/vagrant/util/downloader_test.rb | 2 +- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index 2f6056149..dac1fefa5 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -433,7 +433,7 @@ module Vagrant downloader_options[:headers] = ["Accept: application/json"] if opts[:json] downloader_options[:ui] = env[:ui] if opts[:ui] downloader_options[:location_trusted] = env[:box_download_location_trusted] - downloader_options[:box_download_options] = env[:box_download_options] + downloader_options[:box_extra_download_options] = env[:box_extra_download_options] Util::Downloader.new(url, temp_path, downloader_options) end diff --git a/lib/vagrant/action/builtin/handle_box.rb b/lib/vagrant/action/builtin/handle_box.rb index ccbb47c00..c9c5d130d 100644 --- a/lib/vagrant/action/builtin/handle_box.rb +++ b/lib/vagrant/action/builtin/handle_box.rb @@ -67,7 +67,7 @@ module Vagrant box_download_checksum_type = machine.config.vm.box_download_checksum_type box_download_checksum = machine.config.vm.box_download_checksum box_download_location_trusted = machine.config.vm.box_download_location_trusted - box_download_options = machine.config.vm.box_download_options + box_extra_download_options = machine.config.vm.box_extra_download_options box_formats = machine.provider_options[:box_format] || machine.provider_name @@ -93,7 +93,7 @@ module Vagrant box_checksum_type: box_download_checksum_type, box_checksum: box_download_checksum, box_download_location_trusted: box_download_location_trusted, - box_download_options: box_download_options, + box_extra_download_options: box_extra_download_options, })) rescue Errors::BoxAlreadyExists # Just ignore this, since it means the next part will succeed! diff --git a/lib/vagrant/util/downloader.rb b/lib/vagrant/util/downloader.rb index 6a2838222..dd45933f6 100644 --- a/lib/vagrant/util/downloader.rb +++ b/lib/vagrant/util/downloader.rb @@ -70,7 +70,7 @@ module Vagrant :sha384 => options[:sha384], :sha512 => options[:sha512] }.compact - @extra_download_options = options[:box_download_options] + @extra_download_options = options[:box_extra_download_options] || [] end # This executes the actual download, downloading the source file @@ -247,8 +247,7 @@ module Vagrant options << "-u" << @auth if @auth options << "--location-trusted" if @location_trusted - options.concat( - Vagrant::Util::MapCommandOptions.map_to_command_options(@extra_download_options)) + options.concat(@extra_download_options) if @headers Array(@headers).each do |header| diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 9154e03b3..a096bbfdc 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -46,6 +46,7 @@ module VagrantPlugins attr_accessor :usable_port_range attr_reader :provisioners attr_reader :disks + attr_reader :box_extra_download_options # This is an experimental feature that isn't public yet. attr_accessor :clone @@ -68,6 +69,7 @@ module VagrantPlugins @box_download_insecure = UNSET_VALUE @box_download_location_trusted = UNSET_VALUE @box_download_options = UNSET_VALUE + @box_extra_download_options = UNSET_VALUE @box_url = UNSET_VALUE @box_version = UNSET_VALUE @clone = UNSET_VALUE @@ -470,6 +472,7 @@ module VagrantPlugins @box_url = nil if @box_url == UNSET_VALUE @box_version = nil if @box_version == UNSET_VALUE @box_download_options = {} if @box_download_options == UNSET_VALUE + @box_extra_download_options = Vagrant::Util::MapCommandOptions.map_to_command_options(@box_download_options) @clone = nil if @clone == UNSET_VALUE @communicator = nil if @communicator == UNSET_VALUE @graceful_halt_timeout = 60 if @graceful_halt_timeout == UNSET_VALUE @@ -721,6 +724,15 @@ module VagrantPlugins errors << I18n.t("vagrant.config.vm.box_download_options_type", type: box_download_options.class.to_s) end + box_download_options.each do |k, v| + # If the value is truthy and + # if `box_extra_download_options` does not include the key + # then the conversion to extra download options produced an error + if v && !box_extra_download_options.include?("--#{k}") + errors << I18n.t("vagrant.config.vm.box_download_options_not_converted", missing_key: k) + end + end + used_guest_paths = Set.new @__synced_folders.each do |id, options| # If the shared folder is disabled then don't worry about validating it diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 22e280794..7ddadd519 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1940,6 +1940,8 @@ en: box_missing: "A box must be specified." box_download_options_type: |- Found "box_download_options" specified as type '%{type}', should be a Hash + box_download_options_not_converted: |- + Something went wrong converting VM config `box_download_options`. Value for provided key '%{missing_key}' is invalid type. Should be String or Bool clone_and_box: "Only one of clone or box can be specified." hostname_invalid_characters: |- The hostname set for the VM '%{name}' should only contain letters, numbers, diff --git a/test/unit/vagrant/util/downloader_test.rb b/test/unit/vagrant/util/downloader_test.rb index 382f94efa..2ff8932ef 100644 --- a/test/unit/vagrant/util/downloader_test.rb +++ b/test/unit/vagrant/util/downloader_test.rb @@ -259,7 +259,7 @@ describe Vagrant::Util::Downloader do end context "when extra download options specified" do - let(:options) { {:box_download_options => {test: "arbitrary"}} } + let(:options) { {:box_extra_download_options => ["--test", "arbitrary"]} } subject { described_class.new(source, destination, options) } it "inserts the extra download options" do