Merge remote-tracking branch 'origin/master' into HEAD

* origin/master:
  Update Changelog for 12.2.0
  Revert "Merge pull request #3004 from chef/lcg/deploy-provider-nillable"
  Revert "Merge pull request #2956 from chef/lcg/deploy-fixes"
  Describe Policyfile updates in release notes
  Fixed bug where module_name would return an object instead of string
  Handle cookbook artfact format differences when fetching cookbooks
  Update policyfile URLs and cookbook artifact data format per RFC

Conflicts:
	CHANGELOG.md

21 files changed, 553 insertions, 387 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 74c6746816..4dd283ab18 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,10 +1,7 @@
 ## Unreleased
 
 ## 12.2.0
-
-* make deploy resource attributes nillable (`symlink_before_migrate nil`) works now
-* mixin the LWRP attribute DSL method into Chef::Resource directly
-* make all LWRP attributes nillable
+* Update policyfile API usage to match forthcoming Chef Server release
 * `knife ssh` now has an --exit-on-error option that allows users to
   fail-fast rather than moving on to the next machine.
 * migrate macosx, windows, openbsd, and netbsd resources to dynamic resolution
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 2a59d97736..6c45d2f41e 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,5 +1,15 @@
 # Chef Client Release Notes 12.2.0:
 
+## Policyfile Chef Server 12.0.7 Compatibility
+
+Chef Server 12.0.7 will contain the minimum necessary funtioning
+implementation of Policyfiles to converge a node. Policyfile "native
+mode" is updated to work with the APIs in Chef Server 12.0.7. Note that
+Chef Server 12.0.7 will likely not ship with the necessary code to
+upgrade existing organizations, so you will need to set some special
+configuration to opt-in to enabling the Policyfile APIs in Chef Server.
+That process will be described in the release notes for Chef Server.
+
 ## Desired State Configuration (DSC) Resource
 
 If you are using `Windows Management Framework(WMF) 5`, you can now take advantage of the new `dsc_resource`. 
diff --git a/lib/chef/cookbook/remote_file_vendor.rb b/lib/chef/cookbook/remote_file_vendor.rb
index 2ddce31001..9d895b168e 100644
--- a/lib/chef/cookbook/remote_file_vendor.rb
+++ b/lib/chef/cookbook/remote_file_vendor.rb
@@ -30,7 +30,7 @@ class Chef
 
       def initialize(manifest, rest)
         @manifest = manifest
-        @cookbook_name = @manifest[:cookbook_name]
+        @cookbook_name = @manifest[:cookbook_name] || @manifest[:name]
         @rest = rest
       end
 
diff --git a/lib/chef/cookbook_manifest.rb b/lib/chef/cookbook_manifest.rb
index 0d21e9725c..10654a4945 100644
--- a/lib/chef/cookbook_manifest.rb
+++ b/lib/chef/cookbook_manifest.rb
@@ -36,6 +36,7 @@ class Chef
     def_delegator :@cookbook_version, :root_paths
     def_delegator :@cookbook_version, :segment_filenames
     def_delegator :@cookbook_version, :name
+    def_delegator :@cookbook_version, :identifier
     def_delegator :@cookbook_version, :metadata
     def_delegator :@cookbook_version, :full_name
     def_delegator :@cookbook_version, :version
@@ -141,9 +142,16 @@ class Chef
     # REST api. If there is an existing document on the server and it
     # is marked frozen, a PUT will result in a 409 Conflict.
     def save_url
-      "#{cookbook_url_path}/#{name}/#{version}"
+      if policy_mode?
+        "#{named_cookbook_url}/#{identifier}"
+      else
+        "#{named_cookbook_url}/#{version}"
+      end
     end
 
+    def named_cookbook_url
+      "#{cookbook_url_path}/#{name}"
+    end
     # Adds the `force=true` parameter to the upload URL. This allows
     # the user to overwrite a frozen cookbook (a PUT against the
     # normal #save_url raises a 409 Conflict in this case).
@@ -214,10 +222,16 @@ class Chef
         end
       end
 
-      manifest[:cookbook_name] = name.to_s
       manifest[:metadata] = metadata
       manifest[:version] = metadata.version
-      manifest[:name] = full_name
+
+      if policy_mode?
+        manifest[:name] = name.to_s
+        manifest[:identifier] = identifier
+      else
+        manifest[:name] = full_name
+        manifest[:cookbook_name] = name.to_s
+      end
 
       @manifest_records_by_path = extract_manifest_records_by_path(manifest)
       @manifest = manifest
diff --git a/lib/chef/cookbook_version.rb b/lib/chef/cookbook_version.rb
index b8f32a61bb..8d302eeec2 100644
--- a/lib/chef/cookbook_version.rb
+++ b/lib/chef/cookbook_version.rb
@@ -78,6 +78,16 @@ class Chef
 
     attr_accessor :chef_server_rest
 
+    # The `identifier` field is used for cookbook_artifacts, which are
+    # organized on the chef server according to their content. If the
+    # policy_mode option to CookbookManifest is set to true it will include
+    # this field in the manifest Hash and in the upload URL.
+    #
+    # This field may be removed or have different behavior in the future, don't
+    # use it in 3rd party code.
+    # @api private
+    attr_accessor :identifier
+
     # The first root path is the primary cookbook dir, from which metadata is loaded
     def root_dir
       root_paths[0]
@@ -458,6 +468,15 @@ class Chef
       cookbook_version
     end
 
+    def self.from_cb_artifact_data(o)
+      cookbook_version = new(o["name"])
+      # We want the Chef::Cookbook::Metadata object to always be inflated
+      cookbook_version.metadata = Chef::Cookbook::Metadata.from_hash(o["metadata"])
+      cookbook_version.manifest = o
+      cookbook_version.identifier = o["identifier"]
+      cookbook_version
+    end
+
     # @deprecated This method was used by the Ruby Chef Server and is no longer
     #   needed. There is no replacement.
     def generate_manifest_with_urls(&url_generator)
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb
index baf210bfc5..78d72dc801 100644
--- a/lib/chef/mixin/params_validate.rb
+++ b/lib/chef/mixin/params_validate.rb
@@ -81,58 +81,34 @@ class Chef
         DelayedEvaluator.new(&block)
       end
 
-      NULL_ARG = Object.new
-
-      def nillable_set_or_return(symbol, arg, validation)
+      def set_or_return(symbol, arg, validation)
         iv_symbol = "@#{symbol.to_s}".to_sym
-        if NULL_ARG.equal?(arg)
-          if self.instance_variable_defined?(iv_symbol) == true
-            get_ivar(iv_symbol, symbol, validation)
+        if arg == nil && self.instance_variable_defined?(iv_symbol) == true
+          ivar = self.instance_variable_get(iv_symbol)
+          if(ivar.is_a?(DelayedEvaluator))
+            validate({ symbol => ivar.call }, { symbol => validation })[symbol]
           else
-            # on access we create the iv and set it to nil for back-compat
-            set_ivar(iv_symbol, symbol, nil, validation)
+            ivar
           end
         else
-          set_ivar(iv_symbol, symbol, arg, validation)
-        end
-      end
+          if(arg.is_a?(DelayedEvaluator))
+            val = arg
+          else
+            val = validate({ symbol => arg }, { symbol => validation })[symbol]
 
-      def set_or_return(symbol, arg, validation)
-        iv_symbol = "@#{symbol.to_s}".to_sym
-        if arg == nil && self.instance_variable_defined?(iv_symbol) == true
-          get_ivar(iv_symbol, symbol, validation)
-        else
-          set_ivar(iv_symbol, symbol, arg, validation)
+            # Handle the case where the "default" was a DelayedEvaluator. In
+            # this case, the block yields an optional parameter of +self+,
+            # which is the equivalent of "new_resource"
+            if val.is_a?(DelayedEvaluator)
+              val = val.call(self)
+            end
+          end
+          self.instance_variable_set(iv_symbol, val)
         end
       end
 
       private
 
-      def get_ivar(iv_symbol, symbol, validation)
-        ivar = self.instance_variable_get(iv_symbol)
-        if(ivar.is_a?(DelayedEvaluator))
-          validate({ symbol => ivar.call }, { symbol => validation })[symbol]
-        else
-          ivar
-        end
-      end
-
-      def set_ivar(iv_symbol, symbol, arg, validation)
-        if(arg.is_a?(DelayedEvaluator))
-          val = arg
-        else
-          val = validate({ symbol => arg }, { symbol => validation })[symbol]
-
-          # Handle the case where the "default" was a DelayedEvaluator. In
-          # this case, the block yields an optional parameter of +self+,
-          # which is the equivalent of "new_resource"
-          if val.is_a?(DelayedEvaluator)
-            val = val.call(self)
-          end
-        end
-        self.instance_variable_set(iv_symbol, val)
-      end
-
         # Return the value of a parameter, or nil if it doesn't exist.
         def _pv_opts_lookup(opts, key)
           if opts.has_key?(key.to_s)
@@ -263,3 +239,4 @@ class Chef
     end
   end
 end
+
diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb
index d368b055f7..0c7c07f9f3 100644
--- a/lib/chef/policy_builder/policyfile.rb
+++ b/lib/chef/policy_builder/policyfile.rb
@@ -239,7 +239,7 @@ class Chef
       def policyfile_location
         if Chef::Config[:policy_document_native_api]
           validate_policy_config!
-          "policies/#{policy_group}/#{policy_name}"
+          "policy_groups/#{policy_group}/policies/#{policy_name}"
         else
           "data/policyfiles/#{deployment_group}"
         end
@@ -368,16 +368,20 @@ class Chef
       end
 
       def artifact_manifest_for(cookbook_name, lock_data)
-        xyz_version = lock_data["dotted_decimal_identifier"]
-        rel_url = "cookbook_artifacts/#{cookbook_name}/#{xyz_version}"
-        http_api.get(rel_url)
+        identifier = lock_data["identifier"]
+        rel_url = "cookbook_artifacts/#{cookbook_name}/#{identifier}"
+        inflate_cbv_object(http_api.get(rel_url))
       rescue Exception => e
-        message = "Error loading cookbook #{cookbook_name} at version #{xyz_version} from #{rel_url}: #{e.class} - #{e.message}"
+        message = "Error loading cookbook #{cookbook_name} with identifier #{identifier} from #{rel_url}: #{e.class} - #{e.message}"
         err = Chef::Exceptions::CookbookNotFound.new(message)
         err.set_backtrace(e.backtrace)
         raise err
       end
 
+      def inflate_cbv_object(raw_manifest)
+        Chef::CookbookVersion.from_cb_artifact_data(raw_manifest)
+      end
+
     end
   end
 end
diff --git a/lib/chef/provider/deploy.rb b/lib/chef/provider/deploy.rb
index 4449e44689..19e7c01ab1 100644
--- a/lib/chef/provider/deploy.rb
+++ b/lib/chef/provider/deploy.rb
@@ -42,38 +42,18 @@ class Chef
 
         # @configuration is not used by Deploy, it is only for backwards compat with
         # chef-deploy or capistrano hooks that might use it to get environment information
-        @configuration = new_resource.to_hash
+        @configuration = @new_resource.to_hash
         @configuration[:environment] = @configuration[:environment] && @configuration[:environment]["RAILS_ENV"]
       end
 
-      # @return [Array] create_dirs_before_symlink parameter set on the resource
-      def create_dirs_before_symlink
-        new_resource.create_dirs_before_symlink || []
-      end
-
-      # @return [Array] purge_before_symlink paremeter set on the resource
-      def purge_before_symlink
-        new_resource.purge_before_symlink || []
-      end
-
-      # @return [Hash] symlinks parameter set on the resource
-      def symlinks
-        new_resource.symlinks || {}
-      end
-
-      # @return [Hash] symlink_before_migrate parameter set on the resource
-      def symlink_before_migrate
-        new_resource.symlink_before_migrate || {}
-      end
-
       def whyrun_supported?
         true
       end
 
       def load_current_resource
-        scm_provider.load_current_resource
-        @release_path = new_resource.deploy_to + "/releases/#{release_slug}"
-        @shared_path = new_resource.shared_path
+        @scm_provider.load_current_resource
+        @release_path = @new_resource.deploy_to + "/releases/#{release_slug}"
+        @shared_path = @new_resource.shared_path
       end
 
       def sudo(command,&block)
@@ -82,10 +62,10 @@ class Chef
 
       def run(command, &block)
         exec = execute(command, &block)
-        exec.user(new_resource.user) if new_resource.user
-        exec.group(new_resource.group) if new_resource.group
+        exec.user(@new_resource.user) if @new_resource.user
+        exec.group(@new_resource.group) if @new_resource.group
         exec.cwd(release_path) unless exec.cwd
-        exec.environment(new_resource.environment) unless exec.environment
+        exec.environment(@new_resource.environment) unless exec.environment
         converge_by("execute #{command}") do
           exec
         end
@@ -98,8 +78,8 @@ class Chef
           #There is no reason to assume 2 deployments in a single chef run, hence fails in whyrun.
         end
 
-        [ new_resource.before_migrate, new_resource.before_symlink,
-          new_resource.before_restart, new_resource.after_restart ].each do |script|
+        [ @new_resource.before_migrate, @new_resource.before_symlink,
+          @new_resource.before_restart, @new_resource.after_restart ].each do |script|
           requirements.assert(:deploy, :force_deploy) do |a|
             callback_file = "#{release_path}/#{script}"
             a.assertion do
@@ -120,7 +100,7 @@ class Chef
         save_release_state
         if deployed?(release_path )
           if current_release?(release_path )
-            Chef::Log.debug("#{new_resource} is the latest version")
+            Chef::Log.debug("#{@new_resource} is the latest version")
           else
             rollback_to release_path
           end
@@ -137,7 +117,7 @@ class Chef
           converge_by("delete deployed app at #{release_path} prior to force-deploy") do
             Chef::Log.info("Already deployed app at #{release_path}, forcing.")
             FileUtils.rm_rf(release_path)
-            Chef::Log.info("#{new_resource} forcing deploy of already deployed app at #{release_path}")
+            Chef::Log.info("#{@new_resource} forcing deploy of already deployed app at #{release_path}")
           end
         end
 
@@ -163,7 +143,7 @@ class Chef
 
         releases_to_nuke.each do |i|
           converge_by("roll back by removing release #{i}") do
-            Chef::Log.info "#{new_resource} removing release: #{i}"
+            Chef::Log.info "#{@new_resource} removing release: #{i}"
             FileUtils.rm_rf i
           end
           release_deleted(i)
@@ -177,21 +157,21 @@ class Chef
         copy_cached_repo
         install_gems
         enforce_ownership
-        callback(:before_migrate, new_resource.before_migrate)
+        callback(:before_migrate, @new_resource.before_migrate)
         migrate
-        callback(:before_symlink, new_resource.before_symlink)
+        callback(:before_symlink, @new_resource.before_symlink)
         symlink
-        callback(:before_restart, new_resource.before_restart)
+        callback(:before_restart, @new_resource.before_restart)
         restart
-        callback(:after_restart, new_resource.after_restart)
+        callback(:after_restart, @new_resource.after_restart)
         cleanup!
-        Chef::Log.info "#{new_resource} deployed to #{new_resource.deploy_to}"
+        Chef::Log.info "#{@new_resource} deployed to #{@new_resource.deploy_to}"
       end
 
       def rollback
-        Chef::Log.info "#{new_resource} rolling back to previous release #{release_path}"
+        Chef::Log.info "#{@new_resource} rolling back to previous release #{release_path}"
         symlink
-        Chef::Log.info "#{new_resource} restarting with previous release"
+        Chef::Log.info "#{@new_resource} restarting with previous release"
         restart
       end
 
@@ -199,7 +179,7 @@ class Chef
         @collection = Chef::ResourceCollection.new
         case callback_code
         when Proc
-          Chef::Log.info "#{new_resource} running callback #{what}"
+          Chef::Log.info "#{@new_resource} running callback #{what}"
           recipe_eval(&callback_code)
         when String
           run_callback_from_file("#{release_path}/#{callback_code}")
@@ -211,17 +191,17 @@ class Chef
       def migrate
         run_symlinks_before_migrate
 
-        if new_resource.migrate
+        if @new_resource.migrate
           enforce_ownership
 
-          environment = new_resource.environment
+          environment = @new_resource.environment
           env_info = environment && environment.map do |key_and_val|
             "#{key_and_val.first}='#{key_and_val.last}'"
           end.join(" ")
 
-          converge_by("execute migration command #{new_resource.migration_command}") do
-            Chef::Log.info "#{new_resource} migrating #{new_resource.user} with environment #{env_info}"
-            run_command(run_options(:command => new_resource.migration_command, :cwd=>release_path, :log_level => :info))
+          converge_by("execute migration command #{@new_resource.migration_command}") do
+            Chef::Log.info "#{@new_resource} migrating #{@new_resource.user} with environment #{env_info}"
+            run_command(run_options(:command => @new_resource.migration_command, :cwd=>release_path, :log_level => :info))
           end
         end
       end
@@ -230,18 +210,18 @@ class Chef
         purge_tempfiles_from_current_release
         link_tempfiles_to_current_release
         link_current_release_to_production
-        Chef::Log.info "#{new_resource} updated symlinks"
+        Chef::Log.info "#{@new_resource} updated symlinks"
       end
 
       def restart
-        if restart_cmd = new_resource.restart_command
+        if restart_cmd = @new_resource.restart_command
           if restart_cmd.kind_of?(Proc)
-            Chef::Log.info("#{new_resource} restarting app with embedded recipe")
+            Chef::Log.info("#{@new_resource} restarting app with embedded recipe")
             recipe_eval(&restart_cmd)
           else
-            converge_by("restart app using command #{new_resource.restart_command}") do
-              Chef::Log.info("#{new_resource} restarting app")
-              run_command(run_options(:command => new_resource.restart_command, :cwd => new_resource.current_path))
+            converge_by("restart app using command #{@new_resource.restart_command}") do
+              Chef::Log.info("#{@new_resource} restarting app")
+              run_command(run_options(:command => @new_resource.restart_command, :cwd => @new_resource.current_path))
             end
           end
         end
@@ -252,10 +232,10 @@ class Chef
           release_created(release_path)
         end
 
-        chop = -1 - new_resource.keep_releases
+        chop = -1 - @new_resource.keep_releases
         all_releases[0..chop].each do |old_release|
           converge_by("remove old release #{old_release}") do
-            Chef::Log.info "#{new_resource} removing old release #{old_release}"
+            Chef::Log.info "#{@new_resource} removing old release #{old_release}"
             FileUtils.rm_rf(old_release)
           end
           release_deleted(old_release)
@@ -263,11 +243,11 @@ class Chef
       end
 
       def all_releases
-        Dir.glob(Chef::Util::PathHelper.escape_glob(new_resource.deploy_to) + "/releases/*").sort
+        Dir.glob(Chef::Util::PathHelper.escape_glob(@new_resource.deploy_to) + "/releases/*").sort
       end
 
       def update_cached_repo
-        if new_resource.svn_force_export
+        if @new_resource.svn_force_export
         # TODO assertion, non-recoverable - @scm_provider must be svn if force_export?
           svn_force_export
         else
@@ -276,92 +256,95 @@ class Chef
       end
 
       def run_scm_sync
-        scm_provider.run_action(:sync)
+        @scm_provider.run_action(:sync)
       end
 
       def svn_force_export
-        Chef::Log.info "#{new_resource} exporting source repository"
-        scm_provider.run_action(:force_export)
+        Chef::Log.info "#{@new_resource} exporting source repository"
+        @scm_provider.run_action(:force_export)
       end
 
       def copy_cached_repo
-        target_dir_path = new_resource.deploy_to + "/releases"
+        target_dir_path = @new_resource.deploy_to + "/releases"
         converge_by("deploy from repo to #{target_dir_path} ") do
           FileUtils.rm_rf(release_path) if ::File.exist?(release_path)
           FileUtils.mkdir_p(target_dir_path)
-          FileUtils.cp_r(::File.join(new_resource.destination, "."), release_path, :preserve => true)
-          Chef::Log.info "#{new_resource} copied the cached checkout to #{release_path}"
+          FileUtils.cp_r(::File.join(@new_resource.destination, "."), release_path, :preserve => true)
+          Chef::Log.info "#{@new_resource} copied the cached checkout to #{release_path}"
         end
       end
 
       def enforce_ownership
-        converge_by("force ownership of #{new_resource.deploy_to} to #{new_resource.group}:#{new_resource.user}") do
-          FileUtils.chown_R(new_resource.user, new_resource.group, new_resource.deploy_to)
-          Chef::Log.info("#{new_resource} set user to #{new_resource.user}") if new_resource.user
-          Chef::Log.info("#{new_resource} set group to #{new_resource.group}") if new_resource.group
+        converge_by("force ownership of #{@new_resource.deploy_to} to #{@new_resource.group}:#{@new_resource.user}") do
+          FileUtils.chown_R(@new_resource.user, @new_resource.group, @new_resource.deploy_to)
+          Chef::Log.info("#{@new_resource} set user to #{@new_resource.user}") if @new_resource.user
+          Chef::Log.info("#{@new_resource} set group to #{@new_resource.group}") if @new_resource.group
         end
       end
 
       def verify_directories_exist
-        create_dir_unless_exists(new_resource.deploy_to)
-        create_dir_unless_exists(new_resource.shared_path)
+        create_dir_unless_exists(@new_resource.deploy_to)
+        create_dir_unless_exists(@new_resource.shared_path)
       end
 
       def link_current_release_to_production
-        converge_by(["remove existing link at #{new_resource.current_path}",
-                    "link release #{release_path} into production at #{new_resource.current_path}"]) do
-          FileUtils.rm_f(new_resource.current_path)
+        converge_by(["remove existing link at #{@new_resource.current_path}",
+                    "link release #{release_path} into production at #{@new_resource.current_path}"]) do
+          FileUtils.rm_f(@new_resource.current_path)
           begin
-            FileUtils.ln_sf(release_path, new_resource.current_path)
+            FileUtils.ln_sf(release_path, @new_resource.current_path)
           rescue => e
             raise Chef::Exceptions::FileNotFound.new("Cannot symlink current release to production: #{e.message}")
           end
-          Chef::Log.info "#{new_resource} linked release #{release_path} into production at #{new_resource.current_path}"
+          Chef::Log.info "#{@new_resource} linked release #{release_path} into production at #{@new_resource.current_path}"
         end
         enforce_ownership
       end
 
       def run_symlinks_before_migrate
-        links_info = symlink_before_migrate.map { |src, dst| "#{src} => #{dst}" }.join(", ")
+        links_info = @new_resource.symlink_before_migrate.map { |src, dst| "#{src} => #{dst}" }.join(", ")
         converge_by("make pre-migration symlinks: #{links_info}") do
-          symlink_before_migrate.each do |src, dest|
+          @new_resource.symlink_before_migrate.each do |src, dest|
             begin
-              FileUtils.ln_sf(new_resource.shared_path + "/#{src}", release_path + "/#{dest}")
+              FileUtils.ln_sf(@new_resource.shared_path + "/#{src}", release_path + "/#{dest}")
             rescue => e
-              raise Chef::Exceptions::FileNotFound.new("Cannot symlink #{new_resource.shared_path}/#{src} to #{release_path}/#{dest} before migrate: #{e.message}")
+              raise Chef::Exceptions::FileNotFound.new("Cannot symlink #{@new_resource.shared_path}/#{src} to #{release_path}/#{dest} before migrate: #{e.message}")
             end
           end
-          Chef::Log.info "#{new_resource} made pre-migration symlinks"
+          Chef::Log.info "#{@new_resource} made pre-migration symlinks"
         end
       end
 
       def link_tempfiles_to_current_release
-        dirs_info = create_dirs_before_symlink.join(",")
-        create_dirs_before_symlink.each do |dir|
+        dirs_info = @new_resource.create_dirs_before_symlink.join(",")
+        @new_resource.create_dirs_before_symlink.each do |dir|
           create_dir_unless_exists(release_path + "/#{dir}")
         end
-        Chef::Log.info("#{new_resource} created directories before symlinking: #{dirs_info}")
+        Chef::Log.info("#{@new_resource} created directories before symlinking: #{dirs_info}")
 
-        links_info = symlinks.map { |src, dst| "#{src} => #{dst}" }.join(", ")
+        links_info = @new_resource.symlinks.map { |src, dst| "#{src} => #{dst}" }.join(", ")
         converge_by("link shared paths into current release:  #{links_info}") do
-          symlinks.each do |src, dest|
+          @new_resource.symlinks.each do |src, dest|
             begin
-              FileUtils.ln_sf(::File.join(new_resource.shared_path, src), ::File.join(release_path, dest))
+              FileUtils.ln_sf(::File.join(@new_resource.shared_path, src), ::File.join(release_path, dest))
             rescue => e
-              raise Chef::Exceptions::FileNotFound.new("Cannot symlink shared data #{::File.join(new_resource.shared_path, src)} to #{::File.join(release_path, dest)}: #{e.message}")
+              raise Chef::Exceptions::FileNotFound.new("Cannot symlink shared data #{::File.join(@new_resource.shared_path, src)} to #{::File.join(release_path, dest)}: #{e.message}")
             end
           end
-          Chef::Log.info("#{new_resource} linked shared paths into current release: #{links_info}")
+          Chef::Log.info("#{@new_resource} linked shared paths into current release: #{links_info}")
         end
         run_symlinks_before_migrate
         enforce_ownership
       end
 
+      def create_dirs_before_symlink
+      end
+
       def purge_tempfiles_from_current_release
-        log_info = purge_before_symlink.join(", ")
+        log_info = @new_resource.purge_before_symlink.join(", ")
         converge_by("purge directories in checkout #{log_info}") do
-          purge_before_symlink.each { |dir| FileUtils.rm_rf(release_path + "/#{dir}") }
-          Chef::Log.info("#{new_resource} purged directories in checkout #{log_info}")
+          @new_resource.purge_before_symlink.each { |dir| FileUtils.rm_rf(release_path + "/#{dir}") }
+          Chef::Log.info("#{@new_resource} purged directories in checkout #{log_info}")
         end
       end
 
@@ -411,10 +394,10 @@ class Chef
       end
 
       def run_options(run_opts={})
-        run_opts[:user] = new_resource.user if new_resource.user
-        run_opts[:group] = new_resource.group if new_resource.group
-        run_opts[:environment] = new_resource.environment if new_resource.environment
-        run_opts[:log_tag] = new_resource.to_s
+        run_opts[:user] = @new_resource.user if @new_resource.user
+        run_opts[:group] = @new_resource.group if @new_resource.group
+        run_opts[:environment] = @new_resource.environment if @new_resource.environment
+        run_opts[:log_tag] = @new_resource.to_s
         run_opts[:log_level] ||= :debug
         if run_opts[:log_level] == :info
           if STDOUT.tty? && !Chef::Config[:daemon] && Chef::Log.info?
@@ -425,7 +408,7 @@ class Chef
       end
 
       def run_callback_from_file(callback_file)
-        Chef::Log.info "#{new_resource} queueing checkdeploy hook #{callback_file}"
+        Chef::Log.info "#{@new_resource} queueing checkdeploy hook #{callback_file}"
         recipe_eval do
           Dir.chdir(release_path) do
             from_file(callback_file) if ::File.exist?(callback_file)
@@ -435,20 +418,20 @@ class Chef
 
       def create_dir_unless_exists(dir)
         if ::File.directory?(dir)
-          Chef::Log.debug "#{new_resource} not creating #{dir} because it already exists"
+          Chef::Log.debug "#{@new_resource} not creating #{dir} because it already exists"
           return false
         end
         converge_by("create new directory #{dir}") do
           begin
             FileUtils.mkdir_p(dir)
-            Chef::Log.debug "#{new_resource} created directory #{dir}"
-            if new_resource.user
-              FileUtils.chown(new_resource.user, nil, dir)
-              Chef::Log.debug("#{new_resource} set user to #{new_resource.user} for #{dir}")
+            Chef::Log.debug "#{@new_resource} created directory #{dir}"
+            if @new_resource.user
+              FileUtils.chown(@new_resource.user, nil, dir)
+              Chef::Log.debug("#{@new_resource} set user to #{@new_resource.user} for #{dir}")
             end
-            if new_resource.group
-              FileUtils.chown(nil, new_resource.group, dir)
-              Chef::Log.debug("#{new_resource} set group to #{new_resource.group} for #{dir}")
+            if @new_resource.group
+              FileUtils.chown(nil, @new_resource.group, dir)
+              Chef::Log.debug("#{@new_resource} set group to #{@new_resource.group} for #{dir}")
             end
           rescue => e
             raise Chef::Exceptions::FileNotFound.new("Cannot create directory #{dir}: #{e.message}")
@@ -459,7 +442,7 @@ class Chef
       def with_rollback_on_error
         yield
       rescue ::Exception => e
-        if new_resource.rollback_on_error
+        if @new_resource.rollback_on_error
           Chef::Log.warn "Error on deploying #{release_path}: #{e.message}"
           failed_release = release_path
 
@@ -478,8 +461,8 @@ class Chef
       end
 
       def save_release_state
-        if ::File.exists?(new_resource.current_path)
-          release = ::File.readlink(new_resource.current_path)
+        if ::File.exists?(@new_resource.current_path)
+          release = ::File.readlink(@new_resource.current_path)
           @previous_release_path = release if ::File.exists?(release)
         end
       end
diff --git a/lib/chef/provider/dsc_resource.rb b/lib/chef/provider/dsc_resource.rb
index fabb695803..2812c154c6 100644
--- a/lib/chef/provider/dsc_resource.rb
+++ b/lib/chef/provider/dsc_resource.rb
@@ -107,7 +107,7 @@ class Chef
                 if found[0]['Module'].nil?
                   :none
                 else
-                  found[0]['Module']
+                  found[0]['Module']['Name']
                 end
               else
                 raise Chef::Exceptions::MultipleDscResourcesFound, found
diff --git a/lib/chef/provider/git.rb b/lib/chef/provider/git.rb
index 693dd3f2b2..8418f22933 100644
--- a/lib/chef/provider/git.rb
+++ b/lib/chef/provider/git.rb
@@ -39,10 +39,6 @@ class Chef
         end
       end
 
-      def additional_remotes
-        new_resource.additional_remotes || {}
-      end
-
       def define_resource_requirements
         # Parent directory of the target must exist.
         requirements.assert(:checkout, :sync) do |a|
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 2df73a52e8..ea220b6c70 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -997,15 +997,6 @@ class Chef
     end
 
     #
-    # DSL method used to define attribute on a resource wrapping params_validate
-    #
-    def self.attribute(attr_name, validation_opts={})
-      define_method(attr_name) do |arg=NULL_ARG|
-        nillable_set_or_return(attr_name.to_sym, arg, validation_opts)
-      end
-    end
-
-    #
     # The cookbook in which this Resource was defined (if any).
     #
     # @return Chef::CookbookVersion The cookbook in which this Resource was defined.
diff --git a/lib/chef/resource/deploy.rb b/lib/chef/resource/deploy.rb
index fade82a972..4252aa230f 100644
--- a/lib/chef/resource/deploy.rb
+++ b/lib/chef/resource/deploy.rb
@@ -63,7 +63,6 @@ class Chef
         @deploy_to = name
         @environment = nil
         @repository_cache = 'cached-copy'
-        # XXX: if copy_exclude is a kind_of String why is initialized to an array???
         @copy_exclude = []
         @purge_before_symlink = %w{log tmp/pids public/system}
         @create_dirs_before_symlink = %w{tmp public config}
@@ -79,11 +78,10 @@ class Chef
         @scm_provider = Chef::Provider::Git
         @svn_force_export = false
         @allowed_actions.push(:force_deploy, :deploy, :rollback)
-        @additional_remotes = {}
+        @additional_remotes = Hash[]
         @keep_releases = 5
         @enable_checkout = true
         @checkout_branch = "deploy"
-        @timeout = nil
       end
 
       # where the checked out/cloned code goes
@@ -106,18 +104,42 @@ class Chef
       end
 
       # note: deploy_to is your application "meta-root."
-      attribute :deploy_to, :kind_of => [ String ]
+      def deploy_to(arg=nil)
+        set_or_return(
+          :deploy_to,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :repo, :kind_of => [ String ]
+      def repo(arg=nil)
+        set_or_return(
+          :repo,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
       alias :repository :repo
 
-      attribute :remote, :kind_of => [ String ]
+      def remote(arg=nil)
+        set_or_return(
+          :remote,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :role, :kind_of => [ String ]
+      def role(arg=nil)
+        set_or_return(
+          :role,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      def restart_command(arg=NULL_ARG, &block)
-        arg = block if block_given?
-        nillable_set_or_return(
+      def restart_command(arg=nil, &block)
+        arg ||= block
+        set_or_return(
           :restart_command,
           arg,
           :kind_of => [ String, Proc ]
@@ -125,60 +147,155 @@ class Chef
       end
       alias :restart :restart_command
 
-      attribute :migrate, :kind_of => [ TrueClass, FalseClass ]
+      def migrate(arg=nil)
+        set_or_return(
+          :migrate,
+          arg,
+          :kind_of => [ TrueClass, FalseClass ]
+        )
+      end
 
-      attribute :migration_command, kind_of: String
+      def migration_command(arg=nil)
+        set_or_return(
+          :migration_command,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :rollback_on_error, :kind_of => [ TrueClass, FalseClass ]
+      def rollback_on_error(arg=nil)
+        set_or_return(
+          :rollback_on_error,
+          arg,
+          :kind_of => [ TrueClass, FalseClass ]
+        )
+      end
 
-      attribute :user, kind_of: String
+      def user(arg=nil)
+        set_or_return(
+          :user,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :group, kind_of: [ String ]
+      def group(arg=nil)
+        set_or_return(
+          :group,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :enable_submodules, kind_of: [ TrueClass, FalseClass ]
+      def enable_submodules(arg=nil)
+        set_or_return(
+          :enable_submodules,
+          arg,
+          :kind_of => [ TrueClass, FalseClass ]
+        )
+      end
 
-      attribute :shallow_clone, kind_of: [ TrueClass, FalseClass ]
+      def shallow_clone(arg=nil)
+        set_or_return(
+          :shallow_clone,
+          arg,
+          :kind_of => [ TrueClass, FalseClass ]
+        )
+      end
 
-      attribute :repository_cache, kind_of: String
+      def repository_cache(arg=nil)
+        set_or_return(
+          :repository_cache,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :copy_exclude, kind_of: String
+      def copy_exclude(arg=nil)
+        set_or_return(
+          :copy_exclude,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :revision, kind_of: String
+      def revision(arg=nil)
+        set_or_return(
+          :revision,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
       alias :branch :revision
 
-      attribute :git_ssh_wrapper, kind_of: String
+      def git_ssh_wrapper(arg=nil)
+        set_or_return(
+          :git_ssh_wrapper,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
       alias :ssh_wrapper :git_ssh_wrapper
 
-      attribute :svn_username, kind_of: String
+      def svn_username(arg=nil)
+        set_or_return(
+          :svn_username,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :svn_password, kind_of: String
+      def svn_password(arg=nil)
+        set_or_return(
+          :svn_password,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :svn_arguments, kind_of: String
+      def svn_arguments(arg=nil)
+        set_or_return(
+          :svn_arguments,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
 
-      attribute :svn_info_args, kind_of: String
+      def svn_info_args(arg=nil)
+        set_or_return(
+          :svn_arguments,
+          arg,
+          :kind_of => [ String ])
+      end
 
-      def scm_provider(arg=NULL_ARG)
+      def scm_provider(arg=nil)
         klass = if arg.kind_of?(String) || arg.kind_of?(Symbol)
                   lookup_provider_constant(arg)
                 else
                   arg
                 end
-        nillable_set_or_return(
+        set_or_return(
           :scm_provider,
           klass,
           :kind_of => [ Class ]
         )
       end
 
-      attribute :svn_force_export, kind_of: [ TrueClass, FalseClass ]
+      def svn_force_export(arg=nil)
+        set_or_return(
+          :svn_force_export,
+          arg,
+          :kind_of => [ TrueClass, FalseClass ]
+        )
+      end
 
-      def environment(arg=NULL_ARG)
+      def environment(arg=nil)
         if arg.is_a?(String)
           Chef::Log.debug "Setting RAILS_ENV, RACK_ENV, and MERB_ENV to `#{arg}'"
           Chef::Log.warn "[DEPRECATED] please modify your deploy recipe or attributes to set the environment using a hash"
           arg = {"RAILS_ENV"=>arg,"MERB_ENV"=>arg,"RACK_ENV"=>arg}
         end
-        nillable_set_or_return(
+        set_or_return(
           :environment,
           arg,
           :kind_of => [ Hash ]
@@ -186,8 +303,8 @@ class Chef
       end
 
        # The number of old release directories to keep around after cleanup
-      def keep_releases(arg=NULL_ARG)
-        [nillable_set_or_return(
+      def keep_releases(arg=nil)
+        [set_or_return(
           :keep_releases,
           arg,
           :kind_of => [ Integer ]), 1].max
@@ -197,7 +314,13 @@ class Chef
       # SCM clone/checkout before symlinking. Use this to get rid of files and
       # directories you want to be shared between releases.
       # Default: ["log", "tmp/pids", "public/system"]
-      attribute :purge_before_symlink, kind_of: Array
+      def purge_before_symlink(arg=nil)
+        set_or_return(
+          :purge_before_symlink,
+          arg,
+          :kind_of => Array
+        )
+      end
 
       # An array of paths, relative to your app's root, where you expect dirs to
       # exist before symlinking. This runs after #purge_before_symlink, so you
@@ -207,7 +330,13 @@ class Chef
       # then specify tmp here so that the tmp directory will exist when you
       # symlink the pids directory in to the current release.
       # Default: ["tmp", "public", "config"]
-      attribute :create_dirs_before_symlink, kind_of: Array
+      def create_dirs_before_symlink(arg=nil)
+        set_or_return(
+          :create_dirs_before_symlink,
+          arg,
+          :kind_of => Array
+        )
+      end
 
       # A Hash of shared/dir/path => release/dir/path. This attribute determines
       # which files and dirs in the shared directory get symlinked to the current
@@ -215,7 +344,13 @@ class Chef
       # $shared/pids that you would like to symlink as $current_release/tmp/pids
       # you specify it as "pids" => "tmp/pids"
       # Default {"system" => "public/system", "pids" => "tmp/pids", "log" => "log"}
-      attribute :symlinks, kind_of: Hash
+      def symlinks(arg=nil)
+        set_or_return(
+          :symlinks,
+          arg,
+          :kind_of => Hash
+        )
+      end
 
       # A Hash of shared/dir/path => release/dir/path. This attribute determines
       # which files in the shared directory get symlinked to the current release
@@ -224,44 +359,74 @@ class Chef
       # For a rails/merb app, this is used to link in a known good database.yml
       # (with the production db password) before running migrate.
       # Default {"config/database.yml" => "config/database.yml"}
-      attribute :symlink_before_migrate, kind_of: Hash
+      def symlink_before_migrate(arg=nil)
+        set_or_return(
+          :symlink_before_migrate,
+          arg,
+          :kind_of => Hash
+        )
+      end
 
       # Callback fires before migration is run.
-      def before_migrate(arg=NULL_ARG, &block)
-        arg = block if block_given?
-        nillable_set_or_return(:before_migrate, arg, kind_of: [Proc, String])
+      def before_migrate(arg=nil, &block)
+        arg ||= block
+        set_or_return(:before_migrate, arg, :kind_of => [Proc, String])
       end
 
       # Callback fires before symlinking
-      def before_symlink(arg=NULL_ARG, &block)
-        arg = block if block_given?
-        nillable_set_or_return(:before_symlink, arg, kind_of: [Proc, String])
+      def before_symlink(arg=nil, &block)
+        arg ||= block
+        set_or_return(:before_symlink, arg, :kind_of => [Proc, String])
       end
 
       # Callback fires before restart
-      def before_restart(arg=NULL_ARG, &block)
-        arg = block if block_given?
-        nillable_set_or_return(:before_restart, arg, kind_of: [Proc, String])
+      def before_restart(arg=nil, &block)
+        arg ||= block
+        set_or_return(:before_restart, arg, :kind_of => [Proc, String])
       end
 
       # Callback fires after restart
-      def after_restart(arg=NULL_ARG, &block)
-        arg = block if block_given?
-        nillable_set_or_return(:after_restart, arg, kind_of: [Proc, String])
+      def after_restart(arg=nil, &block)
+        arg ||= block
+        set_or_return(:after_restart, arg, :kind_of => [Proc, String])
       end
 
-      attribute :additional_remotes, kind_of: Hash
+      def additional_remotes(arg=nil)
+        set_or_return(
+          :additional_remotes,
+          arg,
+          :kind_of => Hash
+        )
+      end
 
-      attribute :enable_checkout, kind_of: [ TrueClass, FalseClass ]
+      def enable_checkout(arg=nil)
+        set_or_return(
+          :enable_checkout,
+          arg,
+          :kind_of => [TrueClass, FalseClass]
+        )
+      end
 
-      attribute :checkout_branch, kind_of: String
+      def checkout_branch(arg=nil)
+        set_or_return(
+          :checkout_branch,
+          arg,
+          :kind_of => String
+        )
+      end
 
       # FIXME The Deploy resource may be passed to an SCM provider as its
       # resource.  The SCM provider knows that SCM resources can specify a
       # timeout for SCM operations. The deploy resource must therefore support
       # a timeout method, but the timeout it describes is for SCM operations,
       # not the overall deployment. This is potentially confusing.
-      attribute :timeout, kind_of: Integer
+      def timeout(arg=nil)
+        set_or_return(
+          :timeout,
+          arg,
+          :kind_of => Integer
+        )
+      end
 
     end
   end
diff --git a/lib/chef/resource/git.rb b/lib/chef/resource/git.rb
index 306efe633d..7156873315 100644
--- a/lib/chef/resource/git.rb
+++ b/lib/chef/resource/git.rb
@@ -27,7 +27,7 @@ class Chef
       def initialize(name, run_context=nil)
         super
         @resource_name = :git
-        @additional_remotes = {}
+        @additional_remotes = Hash[]
       end
 
       def additional_remotes(arg=nil)
diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb
index a777c511f0..ce72e98028 100644
--- a/lib/chef/resource/lwrp_base.rb
+++ b/lib/chef/resource/lwrp_base.rb
@@ -70,6 +70,14 @@ class Chef
         alias_method :resource_name=, :resource_name
       end
 
+      # Define an attribute on this resource, including optional validation
+      # parameters.
+      def self.attribute(attr_name, validation_opts={})
+        define_method(attr_name) do |arg=nil|
+          set_or_return(attr_name.to_sym, arg, validation_opts)
+        end
+      end
+
       # Sets the default action
       def self.default_action(action_name=NULL_ARG)
         unless action_name.equal?(NULL_ARG)
diff --git a/spec/functional/resource/deploy_revision_spec.rb b/spec/functional/resource/deploy_revision_spec.rb
index bd45e32771..e5f5341fcd 100644
--- a/spec/functional/resource/deploy_revision_spec.rb
+++ b/spec/functional/resource/deploy_revision_spec.rb
@@ -201,41 +201,6 @@ describe Chef::Resource::DeployRevision, :unix_only => true do
       end
     end
 
-    describe "setting default parameters to nil" do
-      before do
-        FileUtils.mkdir_p(rel_path("releases"))
-        FileUtils.mkdir_p(rel_path("shared"))
-      end
-
-      it "supports setting symlink_before_migrate to nil" do
-        deploy_to_latest_rev.symlink_before_migrate(nil)
-        expect(deploy_to_latest_rev.symlink_before_migrate).to eql(nil)
-        deploy_to_latest_rev.run_action(:deploy)
-        expect(deploy_to_latest_rev).to be_updated_by_last_action
-      end
-
-      it "supports setting symlinks to nil" do
-        deploy_to_latest_rev.symlinks(nil)
-        expect(deploy_to_latest_rev.symlinks).to eql(nil)
-        deploy_to_latest_rev.run_action(:deploy)
-        expect(deploy_to_latest_rev).to be_updated_by_last_action
-      end
-
-      it "supports setting purge_before_symlink to nil" do
-        deploy_to_latest_rev.purge_before_symlink(nil)
-        expect(deploy_to_latest_rev.purge_before_symlink).to eql(nil)
-        deploy_to_latest_rev.run_action(:deploy)
-        expect(deploy_to_latest_rev).to be_updated_by_last_action
-      end
-
-      it "supports setting create_dirs_before_symlink to nil" do
-        deploy_to_latest_rev.create_dirs_before_symlink(nil)
-        expect(deploy_to_latest_rev.create_dirs_before_symlink).to eql(nil)
-        deploy_to_latest_rev.run_action(:deploy)
-        expect(deploy_to_latest_rev).to be_updated_by_last_action
-      end
-    end
-
     describe "back to a previously deployed revision, with the directory structure precreated" do
       before do
         FileUtils.mkdir_p(rel_path("releases"))
diff --git a/spec/unit/cookbook/file_vendor_spec.rb b/spec/unit/cookbook/file_vendor_spec.rb
index 4fad7d5808..145541a63f 100644
--- a/spec/unit/cookbook/file_vendor_spec.rb
+++ b/spec/unit/cookbook/file_vendor_spec.rb
@@ -21,9 +21,6 @@ describe Chef::Cookbook::FileVendor do
 
   let(:file_vendor_class) { Class.new(described_class) }
 
-  # A manifest is a Hash of the format defined by Chef::CookbookVersion#manifest
-  let(:manifest) { {:cookbook_name => "bob"} }
-
   context "when configured to fetch files over http" do
 
     let(:http) { double("Chef::REST") }
@@ -40,19 +37,42 @@ describe Chef::Cookbook::FileVendor do
       expect(file_vendor_class.initialization_options).to eq(http)
     end
 
-    it "creates a RemoteFileVendor for a given manifest" do
-      file_vendor = file_vendor_class.create_from_manifest(manifest)
-      expect(file_vendor).to be_a_kind_of(Chef::Cookbook::RemoteFileVendor)
-      expect(file_vendor.rest).to eq(http)
-      expect(file_vendor.cookbook_name).to eq("bob")
+    context "with a manifest from a cookbook version" do
+
+      # A manifest is a Hash of the format defined by Chef::CookbookVersion#manifest
+      let(:manifest) { {:cookbook_name => "bob", :name => "bob-1.2.3"} }
+
+      it "creates a RemoteFileVendor for a given manifest" do
+        file_vendor = file_vendor_class.create_from_manifest(manifest)
+        expect(file_vendor).to be_a_kind_of(Chef::Cookbook::RemoteFileVendor)
+        expect(file_vendor.rest).to eq(http)
+        expect(file_vendor.cookbook_name).to eq("bob")
+      end
+
     end
 
+    context "with a manifest from a cookbook artifact" do
+
+      # A manifest is a Hash of the format defined by Chef::CookbookVersion#manifest
+      let(:manifest) { {:name => "bob"} }
+
+      it "creates a RemoteFileVendor for a given manifest" do
+        file_vendor = file_vendor_class.create_from_manifest(manifest)
+        expect(file_vendor).to be_a_kind_of(Chef::Cookbook::RemoteFileVendor)
+        expect(file_vendor.rest).to eq(http)
+        expect(file_vendor.cookbook_name).to eq("bob")
+      end
+
+    end
   end
 
   context "when configured to load files from disk" do
 
     let(:cookbook_path) { %w[/var/chef/cookbooks /var/chef/other_cookbooks] }
 
+    # A manifest is a Hash of the format defined by Chef::CookbookVersion#manifest
+    let(:manifest) { {:cookbook_name => "bob"} }
+
     before do
       file_vendor_class.fetch_from_disk(cookbook_path)
     end
diff --git a/spec/unit/cookbook_manifest_spec.rb b/spec/unit/cookbook_manifest_spec.rb
index 938f72c743..f985942e09 100644
--- a/spec/unit/cookbook_manifest_spec.rb
+++ b/spec/unit/cookbook_manifest_spec.rb
@@ -24,6 +24,8 @@ describe Chef::CookbookManifest do
 
   let(:version) { "1.2.3" }
 
+  let(:identifier) { "9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b" }
+
   let(:metadata) do
     Chef::Cookbook::Metadata.new.tap do |m|
       m.version(version)
@@ -35,6 +37,7 @@ describe Chef::CookbookManifest do
   let(:cookbook_version) do
     Chef::CookbookVersion.new("tatft", cookbook_root).tap do |c|
       c.metadata = metadata
+      c.identifier = identifier
     end
   end
 
@@ -212,12 +215,26 @@ describe Chef::CookbookManifest do
 
       let(:policy_mode) { true }
 
+      let(:cookbook_manifest_hash) { cookbook_manifest.to_hash }
+
+      it "sets the identifier in the manifest data" do
+        expect(cookbook_manifest_hash["identifier"]).to eq("9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b")
+      end
+
+      it "sets the name to just the name" do
+        expect(cookbook_manifest_hash["name"]).to eq("tatft")
+      end
+
+      it "does not set a 'cookbook_name' field" do
+        expect(cookbook_manifest_hash).to_not have_key("cookbook_name")
+      end
+
       it "gives the save URL" do
-        expect(cookbook_manifest.save_url).to eq("cookbook_artifacts/tatft/1.2.3")
+        expect(cookbook_manifest.save_url).to eq("cookbook_artifacts/tatft/9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b")
       end
 
       it "gives the force save URL" do
-        expect(cookbook_manifest.force_save_url).to eq("cookbook_artifacts/tatft/1.2.3?force=true")
+        expect(cookbook_manifest.force_save_url).to eq("cookbook_artifacts/tatft/9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b?force=true")
       end
 
     end
diff --git a/spec/unit/cookbook_uploader_spec.rb b/spec/unit/cookbook_uploader_spec.rb
index 152e5373f0..76727c18e2 100644
--- a/spec/unit/cookbook_uploader_spec.rb
+++ b/spec/unit/cookbook_uploader_spec.rb
@@ -25,11 +25,17 @@ describe Chef::CookbookUploader do
   let(:cookbook_loader) do
     loader = Chef::CookbookLoader.new(File.join(CHEF_SPEC_DATA, "cookbooks"))
     loader.load_cookbooks
+    loader.cookbooks_by_name["apache2"].identifier = apache2_identifier
+    loader.cookbooks_by_name["java"].identifier = java_identifier
     loader
   end
 
+  let(:apache2_identifier) { "6644e6cb2ade90b8aff2ebb44728958fbc939ebf" }
+
   let(:apache2_cookbook) { cookbook_loader.cookbooks_by_name["apache2"] }
 
+  let(:java_identifier) { "edd40c30c4e0ebb3658abde4620597597d2e9c17" }
+
   let(:java_cookbook) { cookbook_loader.cookbooks_by_name["java"] }
 
   let(:cookbooks_to_upload) { [apache2_cookbook, java_cookbook] }
@@ -175,7 +181,7 @@ describe Chef::CookbookUploader do
       let(:policy_mode) { true }
 
       def expected_save_url(cookbook)
-        "cookbook_artifacts/#{cookbook.name}/#{cookbook.version}"
+        "cookbook_artifacts/#{cookbook.name}/#{cookbook.identifier}"
       end
 
       it "uploads all files in a sandbox transaction, then creates cookbooks on the server using cookbook_artifacts API" do
diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb
index 1b61f9b238..85e1c1abab 100644
--- a/spec/unit/mixin/params_validate_spec.rb
+++ b/spec/unit/mixin/params_validate_spec.rb
@@ -339,83 +339,69 @@ describe Chef::Mixin::ParamsValidate do
     end.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
-  def self.test_set_or_return_method(method)
-    # caller is responsible for passing in the right arg to get 'return' behavior
-    return_arg = method == :nillable_set_or_return ? TinyClass::NULL_ARG : nil
-
-    it "#{method} should set and return a value, then return the same value" do
-      value = "meow"
-      expect(@vo.send(method,:test, value, {}).object_id).to eq(value.object_id)
-      expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id)
-    end
-
-    it "#{method} should set and return a default value when the argument is nil, then return the same value" do
-      value = "meow"
-      expect(@vo.send(method,:test, return_arg, { :default => value }).object_id).to eq(value.object_id)
-      expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id)
-    end
-
-    it "#{method} should raise an ArgumentError when argument is nil and required is true" do
-      expect {
-        @vo.send(method,:test, return_arg, { :required => true })
-      }.to raise_error(ArgumentError)
-    end
-
-    it "#{method} should not raise an error when argument is nil and required is false" do
-      expect {
-        @vo.send(method,:test, return_arg, { :required => false })
-      }.not_to raise_error
-    end
-
-    it "#{method} should set and return @name, then return @name for foo when argument is nil" do
-      value = "meow"
-      expect(@vo.send(method,:name, value, { }).object_id).to eq(value.object_id)
-      expect(@vo.send(method,:foo, return_arg, { :name_attribute => true }).object_id).to eq(value.object_id)
-    end
-
-    it "#{method} should allow DelayedEvaluator instance to be set for value regardless of restriction" do
-      value = Chef::DelayedEvaluator.new{ 'test' }
-      @vo.send(method,:test, value, {:kind_of => Numeric})
-    end
-
-    it "#{method} should raise an error when delayed evaluated attribute is not valid" do
-      value = Chef::DelayedEvaluator.new{ 'test' }
-      @vo.send(method,:test, value, {:kind_of => Numeric})
-      expect do
-        @vo.send(method,:test, return_arg, {:kind_of => Numeric})
-      end.to raise_error(Chef::Exceptions::ValidationFailed)
-    end
-
-    it "#{method} should create DelayedEvaluator instance when #lazy is used" do
-      @vo.send(method,:delayed, @vo.lazy{ 'test' }, {})
-      expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator)
-    end
-
-    it "#{method} should execute block on each call when DelayedEvaluator" do
-      value = 'fubar'
-      @vo.send(method,:test, @vo.lazy{ value }, {})
-      expect(@vo.send(method,:test, return_arg, {})).to eq('fubar')
-      value = 'foobar'
-      expect(@vo.send(method,:test, return_arg, {})).to eq('foobar')
-      value = 'fauxbar'
-      expect(@vo.send(method,:test, return_arg, {})).to eq('fauxbar')
-    end
-
-    it "#{method} should not evaluate non DelayedEvaluator instances" do
-      value = lambda{ 'test' }
-      @vo.send(method,:test, value, {})
-      expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id)
-      expect(@vo.send(method,:test, return_arg, {})).to be_a(Proc)
-    end
+  it "should set and return a value, then return the same value" do
+    value = "meow"
+    expect(@vo.set_or_return(:test, value, {}).object_id).to eq(value.object_id)
+    expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id)
   end
 
-  test_set_or_return_method(:set_or_return)
-  test_set_or_return_method(:nillable_set_or_return)
+  it "should set and return a default value when the argument is nil, then return the same value" do
+    value = "meow"
+    expect(@vo.set_or_return(:test, nil, { :default => value }).object_id).to eq(value.object_id)
+    expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id)
+  end
+
+  it "should raise an ArgumentError when argument is nil and required is true" do
+    expect {
+      @vo.set_or_return(:test, nil, { :required => true })
+    }.to raise_error(ArgumentError)
+  end
+
+  it "should not raise an error when argument is nil and required is false" do
+    expect {
+      @vo.set_or_return(:test, nil, { :required => false })
+    }.not_to raise_error
+  end
+
+  it "should set and return @name, then return @name for foo when argument is nil" do
+    value = "meow"
+    expect(@vo.set_or_return(:name, value, { }).object_id).to eq(value.object_id)
+    expect(@vo.set_or_return(:foo, nil, { :name_attribute => true }).object_id).to eq(value.object_id)
+  end
 
-  it "nillable_set_or_return supports nilling values" do
-    expect(@vo.nillable_set_or_return(:test, "meow", {})).to eq("meow")
-    expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to eq("meow")
-    expect(@vo.nillable_set_or_return(:test, nil, {})).to be_nil
-    expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to be_nil
+  it "should allow DelayedEvaluator instance to be set for value regardless of restriction" do
+    value = Chef::DelayedEvaluator.new{ 'test' }
+    @vo.set_or_return(:test, value, {:kind_of => Numeric})
   end
+
+  it "should raise an error when delayed evaluated attribute is not valid" do
+    value = Chef::DelayedEvaluator.new{ 'test' }
+    @vo.set_or_return(:test, value, {:kind_of => Numeric})
+    expect do
+      @vo.set_or_return(:test, nil, {:kind_of => Numeric})
+    end.to raise_error(Chef::Exceptions::ValidationFailed)
+  end
+
+  it "should create DelayedEvaluator instance when #lazy is used" do
+    @vo.set_or_return(:delayed, @vo.lazy{ 'test' }, {})
+    expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator)
+  end
+
+  it "should execute block on each call when DelayedEvaluator" do
+    value = 'fubar'
+    @vo.set_or_return(:test, @vo.lazy{ value }, {})
+    expect(@vo.set_or_return(:test, nil, {})).to eq('fubar')
+    value = 'foobar'
+    expect(@vo.set_or_return(:test, nil, {})).to eq('foobar')
+    value = 'fauxbar'
+    expect(@vo.set_or_return(:test, nil, {})).to eq('fauxbar')
+  end
+
+  it "should not evaluate non DelayedEvaluator instances" do
+    value = lambda{ 'test' }
+    @vo.set_or_return(:test, value, {})
+    expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id)
+    expect(@vo.set_or_return(:test, nil, {})).to be_a(Proc)
+  end
+
 end
diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb
index 8b6e928a46..e4f7388a1c 100644
--- a/spec/unit/policy_builder/policyfile_spec.rb
+++ b/spec/unit/policy_builder/policyfile_spec.rb
@@ -256,7 +256,7 @@ describe Chef::PolicyBuilder::Policyfile do
 
       context "and policy_name and policy_group are configured" do
 
-        let(:policy_relative_url) { "policies/policy-stage/example" }
+        let(:policy_relative_url) { "policy_groups/policy-stage/policies/example" }
 
         before do
           expect(http_api).to receive(:get).with(policy_relative_url).and_return(parsed_policyfile_json)
@@ -386,6 +386,9 @@ describe Chef::PolicyBuilder::Policyfile do
 
       describe "fetching the desired cookbook set" do
 
+        let(:example1_cookbook_data) { double("CookbookVersion Hash for example1 cookbook") }
+        let(:example2_cookbook_data) { double("CookbookVersion Hash for example2 cookbook") }
+
         let(:example1_cookbook_object) { double("Chef::CookbookVersion for example1 cookbook") }
         let(:example2_cookbook_object) { double("Chef::CookbookVersion for example2 cookbook") }
 
@@ -396,9 +399,12 @@ describe Chef::PolicyBuilder::Policyfile do
         let(:example1_xyz_version) { example1_lock_data["dotted_decimal_identifier"] }
         let(:example2_xyz_version) { example2_lock_data["dotted_decimal_identifier"] }
 
+        let(:example1_identifier) { example1_lock_data["identifier"] }
+        let(:example2_identifier) { example2_lock_data["identifier"] }
+
         let(:cookbook_synchronizer) { double("Chef::CookbookSynchronizer") }
 
-        shared_examples_for "fetching cookbooks" do
+        shared_examples "fetching cookbooks when they don't exist" do
           context "and a cookbook is missing" do
 
             let(:error404) { Net::HTTPServerException.new("404 message", :body) }
@@ -418,7 +424,9 @@ describe Chef::PolicyBuilder::Policyfile do
             end
 
           end
+        end
 
+        shared_examples_for "fetching cookbooks when they exist" do
           context "and the cookbooks can be fetched" do
             before do
               expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
@@ -426,11 +434,6 @@ describe Chef::PolicyBuilder::Policyfile do
               policy_builder.load_node
               policy_builder.build_node
 
-              expect(http_api).to receive(:get).with(cookbook1_url).
-                and_return(example1_cookbook_object)
-              expect(http_api).to receive(:get).with(cookbook2_url).
-                and_return(example2_cookbook_object)
-
               allow(Chef::CookbookSynchronizer).to receive(:new).
                 with(expected_cookbook_hash, events).
                 and_return(cookbook_synchronizer)
@@ -457,11 +460,23 @@ describe Chef::PolicyBuilder::Policyfile do
         end # shared_examples_for "fetching cookbooks"
 
         context "when using compatibility mode (policy_document_native_api == false)" do
-          include_examples "fetching cookbooks" do
+          let(:cookbook1_url) { "cookbooks/example1/#{example1_xyz_version}" }
+          let(:cookbook2_url) { "cookbooks/example2/#{example2_xyz_version}" }
 
-            let(:cookbook1_url) { "cookbooks/example1/#{example1_xyz_version}" }
-            let(:cookbook2_url) { "cookbooks/example2/#{example2_xyz_version}" }
+          context "when the cookbooks don't exist on the server" do
+            include_examples "fetching cookbooks when they don't exist"
+          end
+
+          context "when the cookbooks exist on the server" do
+
+            before do
+              expect(http_api).to receive(:get).with(cookbook1_url).
+                and_return(example1_cookbook_object)
+              expect(http_api).to receive(:get).with(cookbook2_url).
+                and_return(example2_cookbook_object)
+            end
 
+            include_examples "fetching cookbooks when they exist"
           end
 
         end
@@ -474,13 +489,33 @@ describe Chef::PolicyBuilder::Policyfile do
             Chef::Config[:policy_name] = "example"
           end
 
-          include_examples "fetching cookbooks" do
+          let(:cookbook1_url) { "cookbook_artifacts/example1/#{example1_identifier}" }
+          let(:cookbook2_url) { "cookbook_artifacts/example2/#{example2_identifier}" }
+
+          context "when the cookbooks don't exist on the server" do
+            include_examples "fetching cookbooks when they don't exist"
+          end
+
 
-            let(:cookbook1_url) { "cookbook_artifacts/example1/#{example1_xyz_version}" }
-            let(:cookbook2_url) { "cookbook_artifacts/example2/#{example2_xyz_version}" }
+          context "when the cookbooks exist on the server" do
+
+            before do
+              expect(http_api).to receive(:get).with(cookbook1_url).
+                and_return(example1_cookbook_data)
+              expect(http_api).to receive(:get).with(cookbook2_url).
+                and_return(example2_cookbook_data)
+
+              expect(Chef::CookbookVersion).to receive(:from_cb_artifact_data).with(example1_cookbook_data).
+                and_return(example1_cookbook_object)
+              expect(Chef::CookbookVersion).to receive(:from_cb_artifact_data).with(example2_cookbook_data).
+                and_return(example2_cookbook_object)
+            end
+
+            include_examples "fetching cookbooks when they exist"
 
           end
 
+
         end
 
       end
diff --git a/spec/unit/resource/deploy_spec.rb b/spec/unit/resource/deploy_spec.rb
index 07f5f973c0..0403a7ba6b 100644
--- a/spec/unit/resource/deploy_spec.rb
+++ b/spec/unit/resource/deploy_spec.rb
@@ -30,35 +30,12 @@ describe Chef::Resource::Deploy do
 
 
   class << self
-
-    def resource_has_a_hash_attribute(attr_name)
-      it "has a Hash attribute for #{attr_name.to_s}" do
-        @resource.send(attr_name, {foo: "bar"})
-        expect(@resource.send(attr_name)).to eql({foo: "bar"})
-        expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError)
-      end
-
-      it "the Hash attribute for #{attr_name.to_s} is nillable" do
-        @resource.send(attr_name, {foo: "bar"})
-        expect(@resource.send(attr_name)).to eql({foo: "bar"})
-        @resource.send(attr_name, nil)
-        expect(@resource.send(attr_name)).to eql(nil)
-      end
-    end
-
     def resource_has_a_string_attribute(attr_name)
       it "has a String attribute for #{attr_name.to_s}" do
         @resource.send(attr_name, "this is a string")
         expect(@resource.send(attr_name)).to eql("this is a string")
         expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError)
       end
-
-      it "the String attribute for #{attr_name.to_s} is nillable" do
-        @resource.send(attr_name, "this is a string")
-        expect(@resource.send(attr_name)).to eql("this is a string")
-        @resource.send(attr_name, nil)
-        expect(@resource.send(attr_name)).to eql(nil)
-      end
     end
 
     def resource_has_a_boolean_attribute(attr_name, opts={:defaults_to=>false})
@@ -212,10 +189,6 @@ describe Chef::Resource::Deploy do
     expect(@resource.symlink_before_migrate).to eq({"wtf?" => "wtf is going on"})
   end
 
-  resource_has_a_hash_attribute :symlink_before_migrate
-  resource_has_a_hash_attribute :symlinks
-  resource_has_a_hash_attribute :additional_remotes
-
   resource_has_a_callback_attribute :before_migrate
   resource_has_a_callback_attribute :before_symlink
   resource_has_a_callback_attribute :before_restart