From 1ca7f86f766681ee936eeca8edc3edaf7a000a37 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 23 Jan 2014 15:46:30 -0800 Subject: [PATCH] core: verify providers with direct box adding --- lib/vagrant/action/builtin/box_add.rb | 244 +++++------------- .../vagrant/action/builtin/box_add_test.rb | 36 +++ 2 files changed, 104 insertions(+), 176 deletions(-) diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index d96b25d9a..7168347cd 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -38,27 +38,15 @@ module Vagrant # TODO: what if we have no name name = env[:box_name] url = env[:box_url] + provider = env[:box_provider] + provider = Array(provider) if provider - # Now we have a URL, we have to download this URL. - box = nil - begin - box_url = download(url, env) - - # Add the box! - box = env[:box_collection].add(box_url, name, "0") - ensure - # Make sure we delete the temporary file after we add it, - # unless we were interrupted, in which case we keep it around - # so we can resume the download later. - if !@download_interrupted - @logger.debug("Deleting temporary box: #{box_url}") - box_url.delete - end - end - - env[:ui].success(I18n.t( - "vagrant.box_added", - name: box.name, provider: box.provider)) + box_add( + url, + name, + "0", + provider, + env) end # Adds a box given that the URL is a metadata document. @@ -108,77 +96,14 @@ module Vagrant metadata_version.providers.first) end - env[:ui].output(I18n.t( - "vagrant.box_add_with_version", - name: metadata.name, - version: metadata_version.version, - provider: metadata_provider.name)) - - # Verify the box we're adding doesn't already exist - if !env[:box_force] - box = env[:box_collection].find( - metadata.name, metadata_provider.name, metadata_version.version) - if box - raise Errors::BoxAlreadyExists, - name: metadata.name, - provider: metadata_provider.name, - version: metadata_version.version - end - end - - # Now we have a URL, we have to download this URL. - box = nil - begin - box_url = download(metadata_provider.url, env) - - # Add the box! - box = env[:box_collection].add( - box_url, metadata.name, metadata_version.version, - force: env[:box_force]) - ensure - # Make sure we delete the temporary file after we add it, - # unless we were interrupted, in which case we keep it around - # so we can resume the download later. - if !@download_interrupted - @logger.debug("Deleting temporary box: #{box_url}") - box_url.delete - end - end - - env[:ui].success(I18n.t( - "vagrant.box_added", - name: box.name, provider: box.provider)) + box_add( + metadata_provider.url, + metadata.name, + metadata_version.version, + metadata_provider.name, env) end =begin - box_name = env[:box_name] - box_formats = env[:box_provider] - if box_formats - # Determine the formats a box can support and allow the box to - # be any of those formats. - provider_plugin = Vagrant.plugin("2").manager.providers[env[:box_provider]] - if provider_plugin - box_formats = provider_plugin[1][:box_format] - box_formats ||= env[:box_provider] - end - end - - # Determine if we already have the box before downloading - # it again. We can only do this if we specify a format - if box_formats && !env[:box_force] - begin - if env[:box_collection].find(box_name, box_formats) - raise Errors::BoxAlreadyExists, - :name => box_name, - :formats => [box_formats].flatten.join(", ") - end - rescue Vagrant::Errors::BoxUpgradeRequired - # If the box needs to be upgraded, do it. - env[:box_collection].upgrade(box_name) - retry - end - end - # Determine the checksum type to use checksum = (env[:box_checksum] || "").to_s checksum_klass = nil @@ -237,28 +162,6 @@ module Vagrant end end - # Add the box - env[:ui].info I18n.t("vagrant.actions.box.add.adding", :name => box_name) - box_added = nil - begin - box_added = env[:box_collection].add( - @temp_path, box_name, box_formats, env[:box_force]) - rescue Vagrant::Errors::BoxUpgradeRequired - # Upgrade the box - env[:box_collection].upgrade(box_name) - - # Try adding it again - retry - end - - # Call the 'recover' method in all cases to clean up the - # downloaded temporary file. - recover(env) - - # Success, we added a box! - env[:ui].success( - I18n.t("vagrant.actions.box.add.added", name: box_added.name, provider: box_added.provider)) - # Persists URL used on download and the time it was added write_extra_info(box_added, download_url) @@ -269,72 +172,6 @@ module Vagrant @app.call(env) end - def recover(env) - if @temp_path && File.exist?(@temp_path) && !@download_interrupted - File.unlink(@temp_path) - end - end - - def download_box_url(url, env) - temp_path = env[:tmp_path].join("box" + Digest::SHA1.hexdigest(url)) - @logger.info("Downloading box: #{url} => #{temp_path}") - - if File.file?(url) || url !~ /^[a-z0-9]+:.*$/i - @logger.info("URL is a file or protocol not found and assuming file.") - file_path = File.expand_path(url) - file_path = Util::Platform.cygwin_windows_path(file_path) - url = "file:#{file_path}" - end - - downloader_options = {} - downloader_options[:ca_cert] = env[:box_download_ca_cert] - downloader_options[:continue] = true - downloader_options[:insecure] = env[:box_download_insecure] - downloader_options[:ui] = env[:ui] - downloader_options[:client_cert] = env[:box_client_cert] - - # If the temporary path exists, verify it is not too old. If its - # too old, delete it first because the data may have changed. - if temp_path.file? - delete = false - if env[:box_clean] - @logger.info("Cleaning existing temp box file.") - delete = true - elsif temp_path.mtime.to_i < (Time.now.to_i - 6 * 60 * 60) - @logger.info("Existing temp file is too old. Removing.") - delete = true - end - - temp_path.unlink if delete - end - - # Download the box to a temporary path. We store the temporary - # path as an instance variable so that the `#recover` method can - # access it. - env[:ui].info(I18n.t( - "vagrant.actions.box.download.downloading", - url: url)) - if temp_path.file? - env[:ui].info(I18n.t("vagrant.actions.box.download.resuming")) - end - - begin - downloader = Util::Downloader.new(url, temp_path, downloader_options) - downloader.download! - rescue Errors::DownloaderInterrupted - # The downloader was interrupted, so just return, because that - # means we were interrupted as well. - @download_interrupted = true - env[:ui].info(I18n.t("vagrant.actions.box.download.interrupted")) - rescue Errors::DownloaderError - # The download failed for some reason, clean out the temp path - temp_path.unlink if temp_path.file? - raise - end - - temp_path - end - def write_extra_info(box_added, url) info = {'url' => url, 'downloaded_at' => Time.now.utc} box_added.directory.join('info.json').open("w+") do |f| @@ -345,6 +182,61 @@ module Vagrant protected + # Shared helper to add a box once you know various details + # about it. Shared between adding via metadata or by direct. + # + # @param [String] url + # @param [String] name + # @param [String] version + # @param [String] provider + # @param [Hash] env + # @return [Box] + def box_add(url, name, version, provider, env, **opts) + env[:ui].output(I18n.t( + "vagrant.box_add_with_version", + name: name, + version: version, + provider: provider)) + + # Verify the box we're adding doesn't already exist + if provider && !env[:box_force] + box = env[:box_collection].find( + name, provider, version) + if box + raise Errors::BoxAlreadyExists, + name: name, + provider: provider, + version: version + end + end + + # Now we have a URL, we have to download this URL. + box = nil + begin + box_url = download(url, env) + + # Add the box! + box = env[:box_collection].add( + box_url, name, version, + force: env[:box_force], + providers: provider) + ensure + # Make sure we delete the temporary file after we add it, + # unless we were interrupted, in which case we keep it around + # so we can resume the download later. + if !@download_interrupted + @logger.debug("Deleting temporary box: #{box_url}") + box_url.delete + end + end + + env[:ui].success(I18n.t( + "vagrant.box_added", + name: box.name, provider: box.provider)) + + box + end + # Returns the download options for the download. # # @return [Hash] diff --git a/test/unit/vagrant/action/builtin/box_add_test.rb b/test/unit/vagrant/action/builtin/box_add_test.rb index 3439b159a..eead8ff83 100644 --- a/test/unit/vagrant/action/builtin/box_add_test.rb +++ b/test/unit/vagrant/action/builtin/box_add_test.rb @@ -54,6 +54,42 @@ describe Vagrant::Action::Builtin::BoxAdd do subject.call(env) end + + it "raises an error if the box already exists" do + box_path = iso_env.box2_file(:virtualbox) + + env[:box_name] = "foo" + env[:box_url] = box_path.to_s + env[:box_provider] = "virtualbox" + + box_collection.should_receive(:find).with( + "foo", ["virtualbox"], "0").and_return(box) + box_collection.should_receive(:add).never + app.should_receive(:call).never + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::BoxAlreadyExists) + end + + it "force adds if exists and specified" do + box_path = iso_env.box2_file(:virtualbox) + + env[:box_force] = true + env[:box_name] = "foo" + env[:box_url] = box_path.to_s + env[:box_provider] = "virtualbox" + + box_collection.stub(find: box) + box_collection.should_receive(:add).with do |path, name, version| + expect(checksum(path)).to eq(checksum(box_path)) + expect(name).to eq("foo") + expect(version).to eq("0") + true + end.and_return(box) + app.should_receive(:call).with(env).once + + subject.call(env) + end end context "with box metadata" do