From 976320ec0685b996cdce8953d5aa837194cd1a0b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 1 Nov 2018 09:43:59 -0700 Subject: [PATCH] Limit automatic box outdated checks to once per hour --- .../action/builtin/box_check_outdated.rb | 1 + lib/vagrant/box.rb | 27 +++++++++++++++++++ .../action/builtin/box_check_outdated_test.rb | 6 ++--- test/unit/vagrant/box_test.rb | 22 +++++++++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/vagrant/action/builtin/box_check_outdated.rb b/lib/vagrant/action/builtin/box_check_outdated.rb index 8212c669a..336a0e4a4 100644 --- a/lib/vagrant/action/builtin/box_check_outdated.rb +++ b/lib/vagrant/action/builtin/box_check_outdated.rb @@ -40,6 +40,7 @@ module Vagrant # Have download options specified in the environment override # options specified for the machine. download_options = { + automatic_check: true, 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] || diff --git a/lib/vagrant/box.rb b/lib/vagrant/box.rb index cd839ee91..2f12775f5 100644 --- a/lib/vagrant/box.rb +++ b/lib/vagrant/box.rb @@ -16,6 +16,9 @@ module Vagrant class Box include Comparable + # Number of seconds to wait between checks for box updates + BOX_UPDATE_CHECK_INTERVAL = 3600 + # The box name. This is the logical name used when adding the box. # # @return [String] @@ -154,6 +157,11 @@ module Vagrant raise Errors::BoxUpdateNoMetadata, name: @name end + if download_options.delete(:automatic_check) && !automatic_update_check_allowed? + @logger.info("Skipping box update check") + return + end + version += ", " if version version ||= "" version += "> #{@version}" @@ -164,6 +172,25 @@ module Vagrant [md, newer, newer.provider(@provider)] end + # Check if a box update check is allowed. Uses a file + # in the box data directory to track when the last auto + # update check was performed and returns true if the + # BOX_UPDATE_CHECK_INTERVAL has passed. + # + # @return [Boolean] + def automatic_update_check_allowed? + check_path = directory.join("box_update_check") + if check_path.exist? + last_check_span = Time.now.to_i - check_path.mtime.to_i + if last_check_span < BOX_UPDATE_CHECK_INTERVAL + @logger.info("box update check is under the interval threshold") + return false + end + end + FileUtils.touch(check_path) + true + end + # This repackages this box and outputs it to the given path. # # @param [Pathname] path The full path (filename included) of where diff --git a/test/unit/vagrant/action/builtin/box_check_outdated_test.rb b/test/unit/vagrant/action/builtin/box_check_outdated_test.rb index 058684467..6ec412bc8 100644 --- a/test/unit/vagrant/action/builtin/box_check_outdated_test.rb +++ b/test/unit/vagrant/action/builtin/box_check_outdated_test.rb @@ -119,7 +119,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do 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}}). + {automatic_check: true, 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 @@ -205,7 +205,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do 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}}) + {automatic_check: true, ca_cert: "foo", ca_path: "bar", client_cert: "baz", insecure: true}}) expect(app).to receive(:call).with(env).once @@ -215,7 +215,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do 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}}) + {automatic_check: true, ca_cert: "oof", ca_path: "rab", client_cert: "zab", insecure: false}}) env[:ca_cert] = "oof" env[:ca_path] = "rab" diff --git a/test/unit/vagrant/box_test.rb b/test/unit/vagrant/box_test.rb index 35c3d913a..7d2675a53 100644 --- a/test/unit/vagrant/box_test.rb +++ b/test/unit/vagrant/box_test.rb @@ -195,6 +195,28 @@ describe Vagrant::Box, :skip_windows do end end + context "#automatic_update_check_allowed?" do + it "should return true on intial check" do + expect(subject.automatic_update_check_allowed?).to be_truthy + end + + it "should return false on second check" do + expect(subject.automatic_update_check_allowed?).to be_truthy + expect(subject.automatic_update_check_allowed?).to be_falsey + end + + it "should use a file to mark last check time" do + expect(FileUtils).to receive(:touch) + subject.automatic_update_check_allowed? + end + + it "should return true when time since last check is greater than check interval" do + subject.automatic_update_check_allowed? + stub_const("Vagrant::Box::BOX_UPDATE_CHECK_INTERVAL", -1) + expect(subject.automatic_update_check_allowed?).to be_truthy + end + end + context "#in_use?" do let(:index) { [] }