Extract runner-specific Chef configs into their own subclass

This separates the truly basic pieces of Chef (like install url and
log_level) from the runner pieces of Chef (like provisioning_path). This
is necessary because the Chef Apply provisioner does not actually need
most of the Chef configuration options.

plugins/provisioners/chef/config/base.rb

@@ -6,134 +6,59 @@ module VagrantPlugins
       class Base < Vagrant.plugin("2", :config)
         extend Vagrant::Util::Counter
 
-        attr_accessor :arguments
+        # The path to Chef's bin/ directory.
-        attr_accessor :attempts
+        # @return [String]
         attr_accessor :binary_path
+
+        # Arbitrary environment variables to set before running the Chef
+        # provisioner command.
+        # @return [String]
         attr_accessor :binary_env
-        attr_accessor :custom_config_path
+
-        attr_accessor :encrypted_data_bag_secret_key_path
+        # The Chef log level. See the Chef docs for acceptable values.
-        attr_accessor :environment
+        # @return [String, Symbol]
-        attr_accessor :formatter
-        attr_accessor :http_proxy
-        attr_accessor :http_proxy_user
-        attr_accessor :http_proxy_pass
-        attr_accessor :https_proxy
-        attr_accessor :https_proxy_user
-        attr_accessor :https_proxy_pass
-        attr_accessor :json
         attr_accessor :log_level
-        attr_accessor :no_proxy
+
-        attr_accessor :node_name
-        attr_accessor :provisioning_path
-        attr_accessor :run_list
-        attr_accessor :file_cache_path
-        attr_accessor :file_backup_path
-        attr_accessor :verbose_logging
 
         def initialize
           super
 
-          @arguments         = UNSET_VALUE
+          @binary_path      = UNSET_VALUE
-          @attempts          = UNSET_VALUE
+          @binary_env       = UNSET_VALUE
-          @binary_path       = UNSET_VALUE
+          @log_level        = UNSET_VALUE
-          @binary_env        = UNSET_VALUE
-          @custom_config_path = UNSET_VALUE
-          @encrypted_data_bag_secret_key_path = UNSET_VALUE
-          @environment       = UNSET_VALUE
-          @formatter         = UNSET_VALUE
-          @http_proxy        = UNSET_VALUE
-          @http_proxy_user   = UNSET_VALUE
-          @http_proxy_pass   = UNSET_VALUE
-          @https_proxy       = UNSET_VALUE
-          @https_proxy_user  = UNSET_VALUE
-          @https_proxy_pass  = UNSET_VALUE
-          @log_level         = UNSET_VALUE
-          @no_proxy          = UNSET_VALUE
-          @node_name         = UNSET_VALUE
-          @provisioning_path = UNSET_VALUE
-          @file_cache_path   = UNSET_VALUE
-          @file_backup_path  = UNSET_VALUE
-          @verbose_logging   = UNSET_VALUE
-
-          @json              = {}
-          @run_list          = []
-        end
-
-        def encrypted_data_bag_secret=(value)
-          puts "DEPRECATION: Chef encrypted_data_bag_secret has no effect anymore."
-          puts "Remove this from your Vagrantfile since it'll be removed in the next"
-          puts "Vagrant version."
         end
 
         def finalize!
-          @arguments         = nil if @arguments == UNSET_VALUE
+          @binary_path      = nil     if @binary_path == UNSET_VALUE
-          @attempts          = 1 if @attempts == UNSET_VALUE
+          @binary_env       = nil     if @binary_env == UNSET_VALUE
-          @binary_path       = nil if @binary_path == UNSET_VALUE
+          @log_level        = :info   if @log_level == UNSET_VALUE
-          @binary_env        = nil if @binary_env == UNSET_VALUE
-          @custom_config_path = nil if @custom_config_path == UNSET_VALUE
-          @environment       = nil if @environment == UNSET_VALUE
-          @formatter         = nil if @formatter == UNSET_VALUE
-          @http_proxy        = nil if @http_proxy == UNSET_VALUE
-          @http_proxy_user   = nil if @http_proxy_user == UNSET_VALUE
-          @http_proxy_pass   = nil if @http_proxy_pass == UNSET_VALUE
-          @https_proxy       = nil if @https_proxy == UNSET_VALUE
-          @https_proxy_user  = nil if @https_proxy_user == UNSET_VALUE
-          @https_proxy_pass  = nil if @https_proxy_pass == UNSET_VALUE
-          @log_level         = :info if @log_level == UNSET_VALUE
-          @no_proxy          = nil if @no_proxy == UNSET_VALUE
-          @node_name         = nil if @node_name == UNSET_VALUE
-          @provisioning_path = nil if @provisioning_path == UNSET_VALUE
-          @file_backup_path  = "/var/chef/backup" if @file_backup_path == UNSET_VALUE
-          @file_cache_path   = "/var/chef/cache" if @file_cache_path == UNSET_VALUE
-          @verbose_logging   = false if @verbose_logging == UNSET_VALUE
 
-          if @encrypted_data_bag_secret_key_path == UNSET_VALUE
+          # Make sure the version is a symbol if it's not a boolean
-            @encrypted_data_bag_secret_key_path = nil
+          if @version.respond_to?(:to_sym)
+            @version = @version.to_sym
           end
 
           # Make sure the log level is a symbol
           @log_level = @log_level.to_sym
-
-          # Set the default provisioning path to be a unique path in /tmp
-          if !@provisioning_path
-            counter = self.class.get_and_update_counter(:chef_config)
-            @provisioning_path = "/tmp/vagrant-chef-#{counter}"
-          end
         end
 
-        def merge(other)
+        # Like validate, but returns a list of errors to append.
-          super.tap do |result|
+        #
-            result.instance_variable_set(:@json, @json.merge(other.json))
+        # @return [Array<String>]
-            result.instance_variable_set(:@run_list, (@run_list + other.run_list))
-          end
-        end
-
-        # Just like the normal configuration "validate" method except that
-        # it returns an array of errors that should be merged into some
-        # other error accumulator.
         def validate_base(machine)
           errors = _detected_errors
 
-          if @custom_config_path
+          if missing?(log_level)
-            expanded = File.expand_path(@custom_config_path, machine.env.root_path)
+            errors << I18n.t("vagrant.provisioners.chef.log_level_empty")
-            if !File.file?(expanded)
-              errors << I18n.t("vagrant.config.chef.custom_config_path_missing")
-            end
           end
 
           errors
         end
 
-        # Adds a recipe to the run list
+        # Determine if the given string is "missing" (blank)
-        def add_recipe(name)
+        # @return [true, false]
-          name = "recipe[#{name}]" unless name =~ /^recipe\[(.+?)\]$/
+        def missing?(obj)
-            run_list << name
+          obj.to_s.strip.empty?
-        end
-
-        # Adds a role to the run list
-        def add_role(name)
-          name = "role[#{name}]" unless name =~ /^role\[(.+?)\]$/
-            run_list << name
         end
       end
     end

plugins/provisioners/chef/config/base_runner.rb

@@ -0,0 +1,137 @@
+require "vagrant/util/counter"
+
+require_relative "base"
+
+module VagrantPlugins
+  module Chef
+    module Config
+      # This is the config base for Chef provisioners that need a full Chef
+      # Runner object, like chef-solo or chef-client. For provisioners like
+      # chef-apply, these options are not valid
+      class BaseRunner < Base
+        attr_accessor :arguments
+        attr_accessor :attempts
+        attr_accessor :custom_config_path
+        attr_accessor :encrypted_data_bag_secret_key_path
+        attr_accessor :environment
+        attr_accessor :formatter
+        attr_accessor :http_proxy
+        attr_accessor :http_proxy_user
+        attr_accessor :http_proxy_pass
+        attr_accessor :https_proxy
+        attr_accessor :https_proxy_user
+        attr_accessor :https_proxy_pass
+        attr_accessor :json
+        attr_accessor :no_proxy
+        attr_accessor :node_name
+        attr_accessor :provisioning_path
+        attr_accessor :run_list
+        attr_accessor :file_cache_path
+        attr_accessor :file_backup_path
+        attr_accessor :verbose_logging
+
+        def initialize
+          super
+
+          @arguments          = UNSET_VALUE
+          @attempts           = UNSET_VALUE
+          @custom_config_path = UNSET_VALUE
+
+          # /etc/chef/client.rb config options
+          @encrypted_data_bag_secret_key_path = UNSET_VALUE
+          @environment        = UNSET_VALUE
+          @formatter          = UNSET_VALUE
+          @http_proxy         = UNSET_VALUE
+          @http_proxy_user    = UNSET_VALUE
+          @http_proxy_pass    = UNSET_VALUE
+          @https_proxy        = UNSET_VALUE
+          @https_proxy_user   = UNSET_VALUE
+          @https_proxy_pass   = UNSET_VALUE
+          @no_proxy           = UNSET_VALUE
+          @node_name          = UNSET_VALUE
+          @provisioning_path  = UNSET_VALUE
+          @file_cache_path    = UNSET_VALUE
+          @file_backup_path   = UNSET_VALUE
+          @verbose_logging    = UNSET_VALUE
+
+          # Runner options
+          @json     = {}
+          @run_list = []
+        end
+
+        def encrypted_data_bag_secret=(value)
+          puts "DEPRECATION: Chef encrypted_data_bag_secret has no effect anymore."
+          puts "Remove this from your Vagrantfile since it'll be removed in the next"
+          puts "Vagrant version."
+        end
+
+        def finalize!
+          super
+
+          @arguments          = nil if @arguments == UNSET_VALUE
+          @attempts           = 1   if @attempts == UNSET_VALUE
+          @custom_config_path = nil if @custom_config_path == UNSET_VALUE
+          @environment        = nil if @environment == UNSET_VALUE
+          @formatter          = nil if @formatter == UNSET_VALUE
+          @http_proxy         = nil if @http_proxy == UNSET_VALUE
+          @http_proxy_user    = nil if @http_proxy_user == UNSET_VALUE
+          @http_proxy_pass    = nil if @http_proxy_pass == UNSET_VALUE
+          @https_proxy        = nil if @https_proxy == UNSET_VALUE
+          @https_proxy_user   = nil if @https_proxy_user == UNSET_VALUE
+          @https_proxy_pass   = nil if @https_proxy_pass == UNSET_VALUE
+          @no_proxy           = nil if @no_proxy == UNSET_VALUE
+          @node_name          = nil if @node_name == UNSET_VALUE
+          @provisioning_path  = nil if @provisioning_path == UNSET_VALUE
+          @file_backup_path   = "/var/chef/backup" if @file_backup_path == UNSET_VALUE
+          @file_cache_path    = "/var/chef/cache"  if @file_cache_path == UNSET_VALUE
+          @verbose_logging    = false if @verbose_logging == UNSET_VALUE
+
+          if @encrypted_data_bag_secret_key_path == UNSET_VALUE
+            @encrypted_data_bag_secret_key_path = nil
+          end
+
+          # Set the default provisioning path to be a unique path in /tmp
+          if !@provisioning_path
+            counter = self.class.get_and_update_counter(:chef_config)
+            @provisioning_path = "/tmp/vagrant-chef-#{counter}"
+          end
+        end
+
+        def merge(other)
+          super.tap do |result|
+            result.instance_variable_set(:@json, @json.merge(other.json))
+            result.instance_variable_set(:@run_list, (@run_list + other.run_list))
+          end
+        end
+
+        # Just like the normal configuration "validate" method except that
+        # it returns an array of errors that should be merged into some
+        # other error accumulator.
+        def validate_base(machine)
+          errors = super
+
+          if @custom_config_path
+            expanded = File.expand_path(@custom_config_path, machine.env.root_path)
+            if !File.file?(expanded)
+              errors << I18n.t("vagrant.config.chef.custom_config_path_missing")
+            end
+          end
+
+          errors
+        end
+
+        # Adds a recipe to the run list
+        def add_recipe(name)
+          name = "recipe[#{name}]" unless name =~ /^recipe\[(.+?)\]$/
+            run_list << name
+        end
+
+        # Adds a role to the run list
+        def add_role(name)
+          name = "role[#{name}]" unless name =~ /^role\[(.+?)\]$/
+            run_list << name
+        end
+      end
+    end
+  end
+end

plugins/provisioners/chef/config/chef_apply.rb

@@ -1,9 +1,7 @@
 module VagrantPlugins
   module Chef
     module Config
-      class ChefApply < Vagrant.plugin("2", :config)
+      class ChefApply < Base
-        extend Vagrant::Util::Counter
-
         # The raw recipe text (as a string) to execute via chef-apply.
         # @return [String]
         attr_accessor :recipe
@@ -13,25 +11,17 @@ module VagrantPlugins
         # @return [String]
         attr_accessor :upload_path
 
-        # The Chef log level.
-        # @return [String]
-        attr_accessor :log_level
-
         def initialize
-          @recipe  = UNSET_VALUE
+          super
 
-          @log_level   = UNSET_VALUE
+          @recipe      = UNSET_VALUE
           @upload_path = UNSET_VALUE
         end
 
         def finalize!
-          @recipe = nil if @recipe == UNSET_VALUE
+          super
 
-          if @log_level == UNSET_VALUE
+          @recipe = nil if @recipe == UNSET_VALUE
-            @log_level = :info
-          else
-            @log_level = @log_level.to_sym
-          end
 
           if @upload_path == UNSET_VALUE
             counter = self.class.get_and_update_counter(:chef_apply)
@@ -40,28 +30,18 @@ module VagrantPlugins
         end
 
         def validate(machine)
-          errors = _detected_errors
+          errors = validate_base(machine)
 
-          if missing(recipe)
+          if missing?(recipe)
             errors << I18n.t("vagrant.provisioners.chef.recipe_empty")
           end
 
-          if missing(log_level)
+          if missing?(upload_path)
-            errors << I18n.t("vagrant.provisioners.chef.log_level_empty")
-          end
-
-          if missing(upload_path)
             errors << I18n.t("vagrant.provisioners.chef.upload_path_empty")
           end
 
           { "chef apply provisioner" => 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

plugins/provisioners/chef/config/chef_client.rb

@@ -1,16 +1,33 @@
 require "vagrant/util/which"
 
-require_relative "base"
+require_relative "base_runner"
 
 module VagrantPlugins
   module Chef
     module Config
-      class ChefClient < Base
+      class ChefClient < BaseRunner
+        # The URL endpoint to the Chef Server.
+        # @return [String]
         attr_accessor :chef_server_url
+
+        # The path on disk to the Chef client key,
+        # @return [String]
         attr_accessor :client_key_path
+
+        # Delete the client key when the VM is destroyed. Default is false.
+        # @return [true, false]
         attr_accessor :delete_client
+
+        # Delete the node when the VM is destroyed. Default is false.
+        # @return [true, false]
         attr_accessor :delete_node
+
+        # The path to the validation key on disk.
+        # @return [String]
         attr_accessor :validation_key_path
+
+        # The name of the validation client.
+        # @return [String]
         attr_accessor :validation_client_name
 
         def initialize
@@ -36,8 +53,7 @@ module VagrantPlugins
         end
 
         def validate(machine)
-          errors = _detected_errors
+          errors = validate_base(machine)
-          errors.concat(validate_base(machine))
 
           if chef_server_url.to_s.strip.empty?
             errors << I18n.t("vagrant.config.chef.server_url_empty")

plugins/provisioners/chef/config/chef_solo.rb

@@ -1,25 +1,60 @@
-require_relative "base"
+require_relative "base_runner"
 
 module VagrantPlugins
   module Chef
     module Config
-      class ChefSolo < Base
+      class ChefSolo < BaseRunner
+        # The path on disk where Chef cookbooks are stored.
+        # Default is "cookbooks".
+        # @return [String]
         attr_accessor :cookbooks_path
+
+        # The path where data bags are stored on disk.
+        # @return [String]
         attr_accessor :data_bags_path
+
+        # The path where environments are stored on disk.
+        # @return [String]
         attr_accessor :environments_path
+
+        # A URL download a remote recipe from. Note: you should use chef-apply
+        # instead.
+        #
+        # @deprecated
+        #
+        # @return [String]
         attr_accessor :recipe_url
+
+        # The path where roles are stored on disk.
+        # @return [String]
         attr_accessor :roles_path
+
+        # The type of synced folders to use.
+        # @return [String]
         attr_accessor :synced_folder_type
 
         def initialize
           super
 
-          @cookbooks_path            = UNSET_VALUE
+          @cookbooks_path      = UNSET_VALUE
-          @data_bags_path            = UNSET_VALUE
+          @data_bags_path      = UNSET_VALUE
-          @environments_path         = UNSET_VALUE
+          @environments_path   = UNSET_VALUE
-          @recipe_url                = UNSET_VALUE
+          @recipe_url          = UNSET_VALUE
-          @roles_path                = UNSET_VALUE
+          @roles_path          = UNSET_VALUE
-          @synced_folder_type        = UNSET_VALUE
+          @synced_folder_type  = UNSET_VALUE
+        end
+
+        # @deprecated This is deprecated in Chef and will be removed in Chef 12.
+        def recipe_url=(value)
+          puts "DEPRECATION: The 'recipe_url' setting for the Chef Solo"
+          puts "provisioner is deprecated. This value will be removed in"
+          puts "Chef 12. It is recommended you use the Chef Apply provisioner"
+          puts "instead. The 'recipe_url' setting will be removed in the next"
+          puts "version of Vagrant."
+
+          if value
+            @recipe_url = value
+          end
         end
 
         def nfs=(value)
@@ -63,8 +98,7 @@ module VagrantPlugins
         end
 
         def validate(machine)
-          errors = _detected_errors
+          errors = validate_base(machine)
-          errors.concat(validate_base(machine))
 
           if [cookbooks_path].flatten.compact.empty?
             errors << I18n.t("vagrant.config.chef.cookbooks_path_empty")