From 8c3f833e8eb6ed995f35ea66a7022b324de0e982 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 23 Nov 2015 18:14:32 -0500 Subject: [PATCH] Use the new presence helpers in the Chef provisioner --- lib/vagrant/util/presence.rb | 6 +++++- plugins/provisioners/chef/config/base.rb | 12 +++++------- plugins/provisioners/chef/config/chef_apply.rb | 8 ++++++-- plugins/provisioners/chef/config/chef_client.rb | 7 +++++-- plugins/provisioners/chef/config/chef_solo.rb | 8 ++++++-- plugins/provisioners/chef/config/chef_zero.rb | 8 ++++++-- plugins/provisioners/chef/provisioner/base.rb | 5 ++++- plugins/provisioners/chef/provisioner/chef_client.rb | 5 ++++- test/unit/vagrant/util/presence_test.rb | 4 ++++ 9 files changed, 45 insertions(+), 18 deletions(-) diff --git a/lib/vagrant/util/presence.rb b/lib/vagrant/util/presence.rb index d0788efde..8032bf6c1 100644 --- a/lib/vagrant/util/presence.rb +++ b/lib/vagrant/util/presence.rb @@ -14,7 +14,11 @@ module Vagrant case obj when String !obj.strip.empty? - when Array, Hash + when Symbol + !obj.to_s.strip.empty? + when Array + !obj.compact.empty? + when Hash !obj.empty? when TrueClass, FalseClass obj diff --git a/plugins/provisioners/chef/config/base.rb b/plugins/provisioners/chef/config/base.rb index 2c46198e9..0fa306449 100644 --- a/plugins/provisioners/chef/config/base.rb +++ b/plugins/provisioners/chef/config/base.rb @@ -1,7 +1,11 @@ +require "vagrant/util/presence" + module VagrantPlugins module Chef module Config class Base < Vagrant.plugin("2", :config) + include Vagrant::Util::Presence + # The path to Chef's bin/ directory. # @return [String] attr_accessor :binary_path @@ -114,18 +118,12 @@ EOH def validate_base(machine) errors = _detected_errors - if missing?(log_level) + if !present?(log_level) errors << I18n.t("vagrant.provisioners.chef.log_level_empty") end errors end - - # Determine if the given string is "missing" (blank) - # @return [true, false] - def missing?(obj) - obj.to_s.strip.empty? - end end end end diff --git a/plugins/provisioners/chef/config/chef_apply.rb b/plugins/provisioners/chef/config/chef_apply.rb index 8f7081875..b28f13c0b 100644 --- a/plugins/provisioners/chef/config/chef_apply.rb +++ b/plugins/provisioners/chef/config/chef_apply.rb @@ -1,9 +1,13 @@ +require "vagrant/util/presence" + require_relative "base" module VagrantPlugins module Chef module Config class ChefApply < Base + include Vagrant::Util::Presence + # The raw recipe text (as a string) to execute via chef-apply. # @return [String] attr_accessor :recipe @@ -30,11 +34,11 @@ module VagrantPlugins def validate(machine) errors = validate_base(machine) - if missing?(recipe) + if !present?(recipe) errors << I18n.t("vagrant.provisioners.chef.recipe_empty") end - if missing?(upload_path) + if !present?(upload_path) errors << I18n.t("vagrant.provisioners.chef.upload_path_empty") end diff --git a/plugins/provisioners/chef/config/chef_client.rb b/plugins/provisioners/chef/config/chef_client.rb index b525a3307..343427155 100644 --- a/plugins/provisioners/chef/config/chef_client.rb +++ b/plugins/provisioners/chef/config/chef_client.rb @@ -1,3 +1,4 @@ +require "vagrant/util/presence" require "vagrant/util/which" require_relative "base_runner" @@ -6,6 +7,8 @@ module VagrantPlugins module Chef module Config class ChefClient < BaseRunner + include Vagrant::Util::Presence + # The URL endpoint to the Chef Server. # @return [String] attr_accessor :chef_server_url @@ -55,11 +58,11 @@ module VagrantPlugins def validate(machine) errors = validate_base(machine) - if chef_server_url.to_s.strip.empty? + if !present?(chef_server_url) errors << I18n.t("vagrant.config.chef.server_url_empty") end - if validation_key_path.to_s.strip.empty? + if !present?(validation_key_path) errors << I18n.t("vagrant.config.chef.validation_key_path") end diff --git a/plugins/provisioners/chef/config/chef_solo.rb b/plugins/provisioners/chef/config/chef_solo.rb index f74146f52..79bc7ca58 100644 --- a/plugins/provisioners/chef/config/chef_solo.rb +++ b/plugins/provisioners/chef/config/chef_solo.rb @@ -1,9 +1,13 @@ +require "vagrant/util/presence" + require_relative "base_runner" module VagrantPlugins module Chef module Config class ChefSolo < BaseRunner + include Vagrant::Util::Presence + # The path on disk where Chef cookbooks are stored. # Default is "cookbooks". # @return [String] @@ -82,11 +86,11 @@ module VagrantPlugins def validate(machine) errors = validate_base(machine) - if [cookbooks_path].flatten.compact.empty? + if !present?(Array(cookbooks_path)) errors << I18n.t("vagrant.config.chef.cookbooks_path_empty") end - if environment && environments_path.empty? + if environment && !present?(environments_path) errors << I18n.t("vagrant.config.chef.environment_path_required") end diff --git a/plugins/provisioners/chef/config/chef_zero.rb b/plugins/provisioners/chef/config/chef_zero.rb index 671c2690c..93a036cd7 100644 --- a/plugins/provisioners/chef/config/chef_zero.rb +++ b/plugins/provisioners/chef/config/chef_zero.rb @@ -1,9 +1,13 @@ +require "vagrant/util/presence" + require_relative "chef_solo" module VagrantPlugins module Chef module Config class ChefZero < BaseRunner + include Vagrant::Util::Presence + # The path on disk where Chef cookbooks are stored. # Default is "cookbooks". # @return [String] @@ -69,11 +73,11 @@ module VagrantPlugins def validate(machine) errors = validate_base(machine) - if [cookbooks_path].flatten.compact.empty? + if !present?(Array(cookbooks_path)) errors << I18n.t("vagrant.config.chef.cookbooks_path_empty") end - if [nodes_path].flatten.compact.empty? + if !present?(Array(nodes_path)) errors << I18n.t("vagrant.config.chef.nodes_path_empty") end diff --git a/plugins/provisioners/chef/provisioner/base.rb b/plugins/provisioners/chef/provisioner/base.rb index 4f3636ae2..1ae4a0808 100644 --- a/plugins/provisioners/chef/provisioner/base.rb +++ b/plugins/provisioners/chef/provisioner/base.rb @@ -1,5 +1,6 @@ require 'tempfile' +require "vagrant/util/presence" require "vagrant/util/template_renderer" require_relative "../installer" @@ -11,6 +12,8 @@ module VagrantPlugins # chef-solo and chef-client provisioning are stored. This is **not an actual # provisioner**. Instead, {ChefSolo} or {ChefServer} should be used. class Base < Vagrant.plugin("2", :provisioner) + include Vagrant::Util::Presence + class ChefError < Vagrant::Errors::VagrantError error_namespace("vagrant.provisioners.chef") end @@ -20,7 +23,7 @@ module VagrantPlugins @logger = Log4r::Logger.new("vagrant::provisioners::chef") - if @config.node_name.to_s.empty? + if !present?(@config.node_name) cache = @machine.data_dir.join("chef_node_name") if !cache.exist? diff --git a/plugins/provisioners/chef/provisioner/chef_client.rb b/plugins/provisioners/chef/provisioner/chef_client.rb index e876703d5..5a9ec918e 100644 --- a/plugins/provisioners/chef/provisioner/chef_client.rb +++ b/plugins/provisioners/chef/provisioner/chef_client.rb @@ -1,6 +1,7 @@ require 'pathname' require 'vagrant' +require 'vagrant/util/presence' require 'vagrant/util/subprocess' require_relative "base" @@ -11,6 +12,8 @@ module VagrantPlugins # This class implements provisioning via chef-client, allowing provisioning # with a chef server. class ChefClient < Base + include Vagrant::Util::Presence + def configure(root_config) raise ChefError, :server_validation_key_required if @config.validation_key_path.nil? raise ChefError, :server_validation_key_doesnt_exist if !File.file?(validation_key_path) @@ -132,7 +135,7 @@ module VagrantPlugins def delete_from_chef_server(deletable) node_name = @config.node_name || @machine.config.vm.hostname - if node_name.to_s.empty? + if !present?(node_name) @machine.ui.warn(I18n.t("vagrant.provisioners.chef.missing_node_name", deletable: deletable, )) diff --git a/test/unit/vagrant/util/presence_test.rb b/test/unit/vagrant/util/presence_test.rb index c20cc046e..8be6e87c1 100644 --- a/test/unit/vagrant/util/presence_test.rb +++ b/test/unit/vagrant/util/presence_test.rb @@ -26,6 +26,10 @@ describe Vagrant::Util::Presence do expect(subject.presence([])).to be(false) end + it "returns false for an array with nil values" do + expect(subject.presence([nil, nil])).to be(false) + end + it "returns false for an empty hash" do expect(subject.presence({})).to be(false) end