Merge pull request #2216 from opscode/tball/bsd_package_name

Notify a resource by the `resource[name]` key it was written as

16 files changed, 617 insertions, 307 deletions

diff --git a/lib/chef/application/client.rb b/lib/chef/application/client.rb
index 7f0a39782a..5463f504bc 100644
--- a/lib/chef/application/client.rb
+++ b/lib/chef/application/client.rb
@@ -238,6 +238,11 @@ class Chef::Application::Client < Chef::Application
       :boolean      => true
   end
 
+  option :audit_mode,
+    :long           => "--[no-]audit-mode",
+    :description    => "If not specified, run converge and audit phase.  If true, run only audit phase.  If false, run only converge phase.",
+    :boolean        => true
+
   IMMEDIATE_RUN_SIGNAL = "1".freeze
 
   attr_reader :chef_client_json
diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index 0f46736823..94b0d2d18b 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -82,21 +82,7 @@ class Chef
 
         resource = build_resource(type, name, created_at, &resource_attrs_block)
 
-        # Some resources (freebsd_package) can be invoked with multiple names
-        # (package || freebsd_package).
-        # https://github.com/opscode/chef/issues/1773
-        # For these resources we want to make sure
-        # their key in resource collection is same as the name they are declared
-        # as. Since this might be a breaking change for resources that define
-        # customer to_s methods, we are working around the issue by letting
-        # resources know of their created_as_type until this issue is fixed in
-        # Chef 12:
-        # https://github.com/opscode/chef/issues/1817
-        if resource.respond_to?(:created_as_type=)
-          resource.created_as_type = type
-        end
-
-        run_context.resource_collection.insert(resource)
+        run_context.resource_collection.insert(resource, resource_type:type, instance_name:name)
         resource
       end
 
@@ -120,7 +106,7 @@ class Chef
         # This behavior is very counter-intuitive and should be removed.
         # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694
         # Moved to this location to resolve CHEF-5052, https://tickets.opscode.com/browse/CHEF-5052
-        resource.load_prior_resource
+        resource.load_prior_resource(type, name)
         resource.cookbook_name = cookbook_name
         resource.recipe_name = recipe_name
         # Determine whether this resource is being created in the context of an enclosing Provider
diff --git a/lib/chef/json_compat.rb b/lib/chef/json_compat.rb
index 3350da0c13..0796984ab2 100644
--- a/lib/chef/json_compat.rb
+++ b/lib/chef/json_compat.rb
@@ -39,6 +39,8 @@ class Chef
     CHEF_SANDBOX            = "Chef::Sandbox".freeze
     CHEF_RESOURCE           = "Chef::Resource".freeze
     CHEF_RESOURCECOLLECTION = "Chef::ResourceCollection".freeze
+    CHEF_RESOURCESET        = "Chef::ResourceCollection::ResourceSet".freeze
+    CHEF_RESOURCELIST       = "Chef::ResourceCollection::ResourceList".freeze
 
     class <<self
 
@@ -145,6 +147,10 @@ class Chef
           Chef::Resource
         when CHEF_RESOURCECOLLECTION
           Chef::ResourceCollection
+        when CHEF_RESOURCESET
+          Chef::ResourceCollection::ResourceSet
+        when CHEF_RESOURCELIST
+          Chef::ResourceCollection::ResourceList
         when /^Chef::Resource/
           Chef::Resource.find_subclass_by_name(json_class)
         else
diff --git a/lib/chef/provider/deploy.rb b/lib/chef/provider/deploy.rb
index db147278c2..b30f7ed17e 100644
--- a/lib/chef/provider/deploy.rb
+++ b/lib/chef/provider/deploy.rb
@@ -375,7 +375,7 @@ class Chef
 
       def gem_resource_collection_runner
         gems_collection = Chef::ResourceCollection.new
-        gem_packages.each { |rbgem| gems_collection << rbgem }
+        gem_packages.each { |rbgem| gems_collection.insert(rbgem) }
         gems_run_context = run_context.dup
         gems_run_context.resource_collection = gems_collection
         Chef::Runner.new(gems_run_context)
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 55de8f059c..0b8fb2cb12 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -305,12 +305,13 @@ F
       end
     end
 
-    def load_prior_resource
+    def load_prior_resource(resource_type, instance_name)
       begin
-        prior_resource = run_context.resource_collection.lookup(self.to_s)
+        key = ::Chef::ResourceCollection::ResourceSet.create_key(resource_type, instance_name)
+        prior_resource = run_context.resource_collection.lookup(key)
         # if we get here, there is a prior resource (otherwise we'd have jumped
         # to the rescue clause).
-        Chef::Log.warn("Cloning resource attributes for #{self.to_s} from prior resource (CHEF-3694)")
+        Chef::Log.warn("Cloning resource attributes for #{key} from prior resource (CHEF-3694)")
         Chef::Log.warn("Previous #{prior_resource}: #{prior_resource.source_line}") if prior_resource.source_line
         Chef::Log.warn("Current  #{self}: #{self.source_line}") if self.source_line
         prior_resource.instance_variables.each do |iv|
diff --git a/lib/chef/resource/freebsd_package.rb b/lib/chef/resource/freebsd_package.rb
index 40cc63fc55..957c25caf1 100644
--- a/lib/chef/resource/freebsd_package.rb
+++ b/lib/chef/resource/freebsd_package.rb
@@ -31,27 +31,15 @@ class Chef
 
       provides :package, :on_platforms => ["freebsd"]
 
-      attr_accessor :created_as_type
-
       def initialize(name, run_context=nil)
         super
         @resource_name = :freebsd_package
-        @created_as_type = "freebsd_package"
       end
 
       def after_created
         assign_provider
       end
 
-      # This resource can be invoked with multiple names package & freebsd_package.
-      # We override the to_s method to ensure the key in resource collection
-      # matches the type resource is declared as using created_as_type. This
-      # logic can be removed once Chef does this for all resource in Chef 12:
-      # https://github.com/opscode/chef/issues/1817
-      def to_s
-        "#{created_as_type}[#{name}]"
-      end
-
       def supports_pkgng?
         ships_with_pkgng? || !!shell_out!("make -V WITH_PKGNG", :env => nil).stdout.match(/yes/i)
       end
diff --git a/lib/chef/resource_collection.rb b/lib/chef/resource_collection.rb
index cc14a03962..30520cff7e 100644
--- a/lib/chef/resource_collection.rb
+++ b/lib/chef/resource_collection.rb
@@ -17,250 +17,75 @@
 # limitations under the License.
 #
 
-require 'chef/resource'
-require 'chef/resource_collection/stepable_iterator'
-
+require 'chef/resource_collection/resource_set'
+require 'chef/resource_collection/resource_list'
+require 'chef/resource_collection/resource_collection_serialization'
+require 'chef/log'
+require 'forwardable'
+
+##
+# ResourceCollection currently handles two tasks:
+# 1) Keeps an ordered list of resources to use when converging the node
+# 2) Keeps a unique list of resources (keyed as `type[name]`) used for notifications
 class Chef
   class ResourceCollection
-    include Enumerable
-
-    # Matches a multiple resource lookup specification,
-    # e.g., "service[nginx,unicorn]"
-    MULTIPLE_RESOURCE_MATCH = /^(.+)\[(.+?),(.+)\]$/
+    include ResourceCollectionSerialization
+    extend Forwardable
 
-    # Matches a single resource lookup specification,
-    # e.g., "service[nginx]"
-    SINGLE_RESOURCE_MATCH = /^(.+)\[(.+)\]$/
-
-    attr_reader :iterator
+    attr_reader :resource_set, :resource_list
+    private :resource_set, :resource_list
 
     def initialize
-      @resources = Array.new
-      @resources_by_name = Hash.new
-      @insert_after_idx = nil
-    end
-
-    def all_resources
-      @resources
-    end
-
-    def [](index)
-      @resources[index]
-    end
-
-    def []=(index, arg)
-      is_chef_resource(arg)
-      @resources[index] = arg
-      @resources_by_name[arg.to_s] = index
-    end
-
-    def <<(*args)
-      args.flatten.each do |a|
-        is_chef_resource(a)
-        @resources << a
-        @resources_by_name[a.to_s] = @resources.length - 1
-      end
-      self
-    end
-
-    # 'push' is an alias method to <<
-    alias_method :push, :<<
-
-    def insert(resource)
-      if @insert_after_idx
-        # in the middle of executing a run, so any resources inserted now should
-        # be placed after the most recent addition done by the currently executing
-        # resource
-        insert_at(@insert_after_idx + 1, resource)
-        @insert_after_idx += 1
+      @resource_set = ResourceSet.new
+      @resource_list = ResourceList.new
+    end
+
+    # @param resource [Chef::Resource] The resource to insert
+    # @param resource_type [String,Symbol] If known, the resource type used in the recipe, Eg `package`, `execute`
+    # @param instance_name [String] If known, the recource name as used in the recipe, IE `vim` in `package 'vim'`
+    # @param at_location [Integer] If know, a location in the @resource_list to insert resource
+    # If you know the at_location but not the resource_type or instance_name, pass them in as nil
+    # This method is meant to be the 1 insert method necessary in the future.  It should support all known use cases
+    #   for writing into the ResourceCollection.
+    def insert(resource, opts={})
+      resource_type ||= opts[:resource_type] # Would rather use Ruby 2.x syntax, but oh well
+      instance_name ||= opts[:instance_name]
+      resource_list.insert(resource)
+      if !(resource_type.nil? && instance_name.nil?)
+        resource_set.insert_as(resource, resource_type, instance_name)
       else
-        is_chef_resource(resource)
-        @resources << resource
-        @resources_by_name[resource.to_s] = @resources.length - 1
+        resource_set.insert_as(resource)
       end
     end
 
-    def insert_at(insert_at_index, *resources)
-      resources.each do |resource|
-        is_chef_resource(resource)
-      end
-      @resources.insert(insert_at_index, *resources)
-      # update name -> location mappings and register new resource
-      @resources_by_name.each_key do |key|
-        @resources_by_name[key] += resources.size if @resources_by_name[key] >= insert_at_index
-      end
-      resources.each_with_index do |resource, i|
-        @resources_by_name[resource.to_s] = insert_at_index + i
-      end
+    # @deprecated
+    def []=(index, resource)
+      Chef::Log.warn("`[]=` is deprecated, use `insert` with the `at_location` parameter")
+      resource_list[index] = resource
+      resource_set.insert_as(resource)
     end
 
-    def each
-      @resources.each do |resource|
-        yield resource
+    # @deprecated
+    def push(*resources)
+      Chef::Log.warn("`push` is deprecated, use `insert`")
+      resources.flatten.each do |res|
+        insert(res)
       end
+      self
     end
 
-    def execute_each_resource(&resource_exec_block)
-      @iterator = StepableIterator.for_collection(@resources)
-      @iterator.each_with_index do |resource, idx|
-        @insert_after_idx = idx
-        yield resource
-      end
-    end
-
-    def each_index
-      @resources.each_index do |i|
-        yield i
-      end
-    end
-
-    def empty?
-      @resources.empty?
-    end
-
-    def lookup(resource)
-      lookup_by = nil
-      if resource.kind_of?(Chef::Resource)
-        lookup_by = resource.to_s
-      elsif resource.kind_of?(String)
-        lookup_by = resource
-      else
-        raise ArgumentError, "Must pass a Chef::Resource or String to lookup"
-      end
-      res = @resources_by_name[lookup_by]
-      unless res
-        raise Chef::Exceptions::ResourceNotFound, "Cannot find a resource matching #{lookup_by} (did you define it first?)"
-      end
-      @resources[res]
-    end
-
-    # Find existing resources by searching the list of existing resources.  Possible
-    # forms are:
-    #
-    # find(:file => "foobar")
-    # find(:file => [ "foobar", "baz" ])
-    # find("file[foobar]", "file[baz]")
-    # find("file[foobar,baz]")
-    #
-    # Returns the matching resource, or an Array of matching resources.
-    #
-    # Raises an ArgumentError if you feed it bad lookup information
-    # Raises a Runtime Error if it can't find the resources you are looking for.
-    def find(*args)
-      results = Array.new
-      args.each do |arg|
-        case arg
-        when Hash
-          results << find_resource_by_hash(arg)
-        when String
-          results << find_resource_by_string(arg)
-        else
-          msg = "arguments to #{self.class.name}#find should be of the form :resource => 'name' or resource[name]"
-          raise Chef::Exceptions::InvalidResourceSpecification, msg
-        end
-      end
-      flat_results = results.flatten
-      flat_results.length == 1 ? flat_results[0] : flat_results
-    end
-
-    # resources is a poorly named, but we have to maintain it for back
-    # compat.
-    alias_method :resources, :find
-
-    # Returns true if +query_object+ is a valid string for looking up a
-    # resource, or raises InvalidResourceSpecification if not.
-    # === Arguments
-    # * query_object should be a string of the form
-    # "resource_type[resource_name]", a single element Hash (e.g., :service =>
-    # "apache2"), or a Chef::Resource (this is the happy path). Other arguments
-    # will raise an exception.
-    # === Returns
-    # * true returns true for all valid input.
-    # === Raises
-    # * Chef::Exceptions::InvalidResourceSpecification for all invalid input.
-    def validate_lookup_spec!(query_object)
-      case query_object
-      when Chef::Resource
-        true
-      when SINGLE_RESOURCE_MATCH, MULTIPLE_RESOURCE_MATCH
-        true
-      when Hash
-        true
-      when String
-        raise Chef::Exceptions::InvalidResourceSpecification,
-          "The string `#{query_object}' is not valid for resource collection lookup. Correct syntax is `resource_type[resource_name]'"
-      else
-        raise Chef::Exceptions::InvalidResourceSpecification,
-          "The object `#{query_object.inspect}' is not valid for resource collection lookup. " +
-          "Use a String like `resource_type[resource_name]' or a Chef::Resource object"
-      end
-    end
-
-    # Serialize this object as a hash
-    def to_hash
-      instance_vars = Hash.new
-      self.instance_variables.each do |iv|
-        instance_vars[iv] = self.instance_variable_get(iv)
-      end
-      {
-        'json_class' => self.class.name,
-        'instance_vars' => instance_vars
-      }
-    end
-
-    def to_json(*a)
-      Chef::JSONCompat.to_json(to_hash, *a)
-    end
-
-    def self.json_create(o)
-      collection = self.new()
-      o["instance_vars"].each do |k,v|
-        collection.instance_variable_set(k.to_sym, v)
-      end
-      collection
-    end
+    # @deprecated
+    alias_method :<<, :insert
 
-    private
+    # Read-only methods are simple to delegate - doing that below
 
-      def find_resource_by_hash(arg)
-        results = Array.new
-        arg.each do |resource_name, name_list|
-          names = name_list.kind_of?(Array) ? name_list : [ name_list ]
-          names.each do |name|
-            res_name = "#{resource_name.to_s}[#{name}]"
-            results << lookup(res_name)
-          end
-        end
-        return results
-      end
+    resource_list_methods = Enumerable.instance_methods +
+        [:iterator, :all_resources, :[], :each, :execute_each_resource, :each_index, :empty?] -
+        [:find] # find needs to run on the set
+    resource_set_methods = [:lookup, :find, :resources, :keys, :validate_lookup_spec!]
 
-      def find_resource_by_string(arg)
-        results = Array.new
-        case arg
-        when MULTIPLE_RESOURCE_MATCH
-          resource_type = $1
-          arg =~ /^.+\[(.+)\]$/
-          resource_list = $1
-          resource_list.split(",").each do |name|
-            resource_name = "#{resource_type}[#{name}]"
-            results << lookup(resource_name)
-          end
-        when SINGLE_RESOURCE_MATCH
-          resource_type = $1
-          name = $2
-          resource_name = "#{resource_type}[#{name}]"
-          results << lookup(resource_name)
-        else
-          raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!"
-        end
-        return results
-      end
+    def_delegators :resource_list, *resource_list_methods
+    def_delegators :resource_set, *resource_set_methods
 
-      def is_chef_resource(arg)
-        unless arg.kind_of?(Chef::Resource)
-          raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource"
-        end
-        true
-      end
   end
 end
diff --git a/lib/chef/resource_collection/resource_collection_serialization.rb b/lib/chef/resource_collection/resource_collection_serialization.rb
new file mode 100644
index 0000000000..3651fb2a2a
--- /dev/null
+++ b/lib/chef/resource_collection/resource_collection_serialization.rb
@@ -0,0 +1,59 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+class Chef
+  class ResourceCollection
+    module ResourceCollectionSerialization
+      # Serialize this object as a hash
+      def to_hash
+        instance_vars = Hash.new
+        self.instance_variables.each do |iv|
+          instance_vars[iv] = self.instance_variable_get(iv)
+        end
+        {
+            'json_class' => self.class.name,
+            'instance_vars' => instance_vars
+        }
+      end
+
+      def to_json(*a)
+        Chef::JSONCompat.to_json(to_hash, *a)
+      end
+
+      def self.included(base)
+        base.extend(ClassMethods)
+      end
+
+      module ClassMethods
+        def json_create(o)
+          collection = self.new()
+          o["instance_vars"].each do |k,v|
+            collection.instance_variable_set(k.to_sym, v)
+          end
+          collection
+        end
+      end
+
+      def is_chef_resource!(arg)
+        unless arg.kind_of?(Chef::Resource)
+          raise ArgumentError, "Cannot insert a #{arg.class} into a resource collection: must be a subclass of Chef::Resource"
+        end
+        true
+      end
+    end
+  end
+end
diff --git a/lib/chef/resource_collection/resource_list.rb b/lib/chef/resource_collection/resource_list.rb
new file mode 100644
index 0000000000..e083cc0a9f
--- /dev/null
+++ b/lib/chef/resource_collection/resource_list.rb
@@ -0,0 +1,101 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/resource'
+require 'chef/resource_collection/stepable_iterator'
+require 'chef/resource_collection/resource_collection_serialization'
+
+# This class keeps the list of all known Resources in the order they are to be executed in.  It also keeps a pointer
+# to the most recently executed resource so we can add resources-to-execute after this point.
+class Chef
+  class ResourceCollection
+    class ResourceList
+      include ResourceCollection::ResourceCollectionSerialization
+      include Enumerable
+
+      attr_reader :iterator
+
+      def initialize
+        @resources = Array.new
+        @insert_after_idx = nil
+      end
+
+      # @param resource [Chef::Resource] The resource to insert
+      # If @insert_after_idx is nil, we are not currently executing a converge so the Resource is appended to the
+      # end of the list.  If @insert_after_idx is NOT nil, we ARE currently executing a converge so the resource
+      # is inserted into the middle of the list after the last resource that was converged.  If it is called multiple
+      # times (when an LWRP contains multiple resources) it keeps track of that.  See this example ResourceList:
+      # [File1, LWRP1, File2] # The iterator starts and points to File1.  It is executed and @insert_after_idx=0
+      # [File1, LWRP1, File2] # The iterator moves to LWRP1.  It is executed and @insert_after_idx=1
+      # [File1, LWRP1, Service1, File2] # The LWRP execution inserts Service1 and @insert_after_idx=2
+      # [File1, LWRP1, Service1, Service2, File2] # The LWRP inserts Service2 and @insert_after_idx=3.  The LWRP
+      #     finishes executing
+      # [File1, LWRP1, Service1, Service2, File2] # The iterator moves to Service1 since it is the next non-executed
+      #     resource.  The execute_each_resource call below resets @insert_after_idx=2
+      # If Service1 was another LWRP, it would insert its resources between Service1 and Service2.  The iterator keeps
+      # track of executed resources and @insert_after_idx keeps track of where the next resource to insert should be.
+      def insert(resource)
+        is_chef_resource!(resource)
+        if @insert_after_idx
+          @resources.insert(@insert_after_idx += 1, resource)
+        else
+          @resources << resource
+        end
+      end
+
+      # @deprecated - can be removed when it is removed from resource_collection.rb
+      def []=(index, resource)
+        @resources[index] = resource
+      end
+
+      def all_resources
+        @resources
+      end
+
+      def [](index)
+        @resources[index]
+      end
+
+      def each
+        @resources.each do |resource|
+          yield resource
+        end
+      end
+
+      def execute_each_resource(&resource_exec_block)
+        @iterator = ResourceCollection::StepableIterator.for_collection(@resources)
+        @iterator.each_with_index do |resource, idx|
+          @insert_after_idx = idx
+          yield resource
+        end
+      end
+
+      def each_index
+        @resources.each_index do |i|
+          yield i
+        end
+      end
+
+      def empty?
+        @resources.empty?
+      end
+
+    end
+  end
+end
+
diff --git a/lib/chef/resource_collection/resource_set.rb b/lib/chef/resource_collection/resource_set.rb
new file mode 100644
index 0000000000..6425c2ab08
--- /dev/null
+++ b/lib/chef/resource_collection/resource_set.rb
@@ -0,0 +1,170 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/resource'
+require 'chef/resource_collection/resource_collection_serialization'
+
+class Chef
+  class ResourceCollection
+    class ResourceSet
+      include ResourceCollection::ResourceCollectionSerialization
+
+      # Matches a multiple resource lookup specification,
+      # e.g., "service[nginx,unicorn]"
+      MULTIPLE_RESOURCE_MATCH = /^(.+)\[(.+?),(.+)\]$/
+
+      # Matches a single resource lookup specification,
+      # e.g., "service[nginx]"
+      SINGLE_RESOURCE_MATCH = /^(.+)\[(.+)\]$/
+
+      def initialize
+        @resources_by_key = Hash.new
+      end
+
+      def keys
+        @resources_by_key.keys
+      end
+
+      def insert_as(resource, resource_type=nil, instance_name=nil)
+        is_chef_resource!(resource)
+        resource_type ||= resource.resource_name
+        instance_name ||= resource.name
+        key = ResourceSet.create_key(resource_type, instance_name)
+        @resources_by_key[key] = resource
+      end
+
+      def lookup(key)
+        case
+          when key.kind_of?(String)
+            lookup_by = key
+          when key.kind_of?(Chef::Resource)
+            lookup_by = ResourceSet.create_key(key.resource_name, key.name)
+          else
+            raise ArgumentError, "Must pass a Chef::Resource or String to lookup"
+        end
+
+        res = @resources_by_key[lookup_by]
+        unless res
+          raise Chef::Exceptions::ResourceNotFound, "Cannot find a resource matching #{lookup_by} (did you define it first?)"
+        end
+        res
+      end
+
+      # Find existing resources by searching the list of existing resources.  Possible
+      # forms are:
+      #
+      # find(:file => "foobar")
+      # find(:file => [ "foobar", "baz" ])
+      # find("file[foobar]", "file[baz]")
+      # find("file[foobar,baz]")
+      #
+      # Returns the matching resource, or an Array of matching resources.
+      #
+      # Raises an ArgumentError if you feed it bad lookup information
+      # Raises a Runtime Error if it can't find the resources you are looking for.
+      def find(*args)
+        results = Array.new
+        args.each do |arg|
+          case arg
+            when Hash
+              results << find_resource_by_hash(arg)
+            when String
+              results << find_resource_by_string(arg)
+            else
+              msg = "arguments to #{self.class.name}#find should be of the form :resource => 'name' or 'resource[name]'"
+              raise Chef::Exceptions::InvalidResourceSpecification, msg
+          end
+        end
+        flat_results = results.flatten
+        flat_results.length == 1 ? flat_results[0] : flat_results
+      end
+
+      # @deprecated
+      # resources is a poorly named, but we have to maintain it for back
+      # compat.
+      alias_method :resources, :find
+
+      # Returns true if +query_object+ is a valid string for looking up a
+      # resource, or raises InvalidResourceSpecification if not.
+      # === Arguments
+      # * query_object should be a string of the form
+      # "resource_type[resource_name]", a single element Hash (e.g., :service =>
+      # "apache2"), or a Chef::Resource (this is the happy path). Other arguments
+      # will raise an exception.
+      # === Returns
+      # * true returns true for all valid input.
+      # === Raises
+      # * Chef::Exceptions::InvalidResourceSpecification for all invalid input.
+      def validate_lookup_spec!(query_object)
+        case query_object
+          when Chef::Resource
+            true
+          when SINGLE_RESOURCE_MATCH, MULTIPLE_RESOURCE_MATCH
+            true
+          when Hash
+            true
+          when String
+            raise Chef::Exceptions::InvalidResourceSpecification,
+                  "The string `#{query_object}' is not valid for resource collection lookup. Correct syntax is `resource_type[resource_name]'"
+          else
+            raise Chef::Exceptions::InvalidResourceSpecification,
+                  "The object `#{query_object.inspect}' is not valid for resource collection lookup. " +
+                      "Use a String like `resource_type[resource_name]' or a Chef::Resource object"
+        end
+      end
+
+      def self.create_key(resource_type, instance_name)
+        "#{resource_type}[#{instance_name}]"
+      end
+
+      private
+
+      def find_resource_by_hash(arg)
+        results = Array.new
+        arg.each do |resource_type, name_list|
+          instance_names = name_list.kind_of?(Array) ? name_list : [ name_list ]
+          instance_names.each do |instance_name|
+            results << lookup(ResourceSet.create_key(resource_type, instance_name))
+          end
+        end
+        return results
+      end
+
+      def find_resource_by_string(arg)
+        results = Array.new
+        case arg
+          when MULTIPLE_RESOURCE_MATCH
+            resource_type = $1
+            arg =~ /^.+\[(.+)\]$/
+            resource_list = $1
+            resource_list.split(",").each do |instance_name|
+              results << lookup(ResourceSet.create_key(resource_type, instance_name))
+            end
+          when SINGLE_RESOURCE_MATCH
+            resource_type = $1
+            name = $2
+            results << lookup(ResourceSet.create_key(resource_type, name))
+          else
+            raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!"
+        end
+        return results
+      end
+
+    end
+  end
+end
diff --git a/lib/chef/shell/ext.rb b/lib/chef/shell/ext.rb
index bc4e955169..fd785e2f79 100644
--- a/lib/chef/shell/ext.rb
+++ b/lib/chef/shell/ext.rb
@@ -547,7 +547,7 @@ E
       desc "list all the resources on the current recipe"
       def resources(*args)
         if args.empty?
-          pp run_context.resource_collection.instance_variable_get(:@resources_by_name).keys
+          pp run_context.resource_collection.keys
         else
           pp resources = original_resources(*args)
           resources
diff --git a/spec/support/lib/chef/resource/zen_follower.rb b/spec/support/lib/chef/resource/zen_follower.rb
index 0fa0c4af5b..713f122f06 100644
--- a/spec/support/lib/chef/resource/zen_follower.rb
+++ b/spec/support/lib/chef/resource/zen_follower.rb
@@ -21,20 +21,14 @@ require 'chef/json_compat'
 class Chef
   class Resource
     class ZenFollower < Chef::Resource
-      attr_accessor :created_as_type
 
       provides :follower, :on_platforms => ["zen"]
 
       def initialize(name, run_context=nil)
         @resource_name = :zen_follower
-        @created_as_type = "zen_follower"
         super
       end
 
-      def to_s
-        "#{created_as_type}[#{name}]"
-      end
-
       def master(arg=nil)
         if !arg.nil?
           @master = arg
diff --git a/spec/unit/resource_collection/resource_set_spec.rb b/spec/unit/resource_collection/resource_set_spec.rb
new file mode 100644
index 0000000000..29b676f85a
--- /dev/null
+++ b/spec/unit/resource_collection/resource_set_spec.rb
@@ -0,0 +1,199 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+
+describe Chef::ResourceCollection::ResourceSet do
+  let(:collection) { Chef::ResourceCollection::ResourceSet.new }
+
+  let(:zen_master_name) { "Neo" }
+  let(:zen_master2_name) { "Morpheus" }
+  let(:zen_follower_name) { "Squid" }
+  let(:zen_master) { Chef::Resource::ZenMaster.new(zen_master_name) }
+  let(:zen_master2) { Chef::Resource::ZenMaster.new(zen_master2_name) }
+  let(:zen_follower) { Chef::Resource::ZenFollower.new(zen_follower_name) }
+
+  describe "initialize" do
+    it "should return a Chef::ResourceCollection" do
+      expect(collection).to be_instance_of(Chef::ResourceCollection::ResourceSet)
+    end
+  end
+
+  describe "keys" do
+    it "should return an empty list for an empty ResourceSet" do
+      expect(collection.keys).to eq([])
+    end
+
+    it "should return the keys for a non-empty ResourceSet" do
+      collection.instance_variable_get(:@resources_by_key)["key"] = nil
+      expect(collection.keys).to eq(["key"])
+    end
+  end
+
+  describe "insert_as, lookup and find" do
+    # To validate insert_as you need lookup, and vice-versa - putting all tests in 1 context to avoid duplication
+    it "should accept only Chef::Resources" do
+      expect { collection.insert_as(zen_master) }.to_not raise_error
+      expect { collection.insert_as("string") }.to raise_error(ArgumentError)
+    end
+
+    it "should allow you to lookup resources by a default .to_s" do
+      collection.insert_as(zen_master)
+      expect(collection.lookup(zen_master.to_s)).to equal(zen_master)
+    end
+
+    it "should use a custom type and name to insert" do
+      collection.insert_as(zen_master, "OtherResource", "other_resource")
+      expect(collection.lookup("OtherResource[other_resource]")).to equal(zen_master)
+    end
+
+    it "should raise an exception if you send something strange to lookup" do
+      expect { collection.lookup(:symbol) }.to raise_error(ArgumentError)
+    end
+
+    it "should raise an exception if it cannot find a resource with lookup" do
+      expect { collection.lookup(zen_master.to_s) }.to raise_error(Chef::Exceptions::ResourceNotFound)
+    end
+
+    it "should find a resource by type symbol and name" do
+      collection.insert_as(zen_master)
+      expect(collection.find(:zen_master => zen_master_name)).to equal(zen_master)
+    end
+
+    it "should find a resource by type symbol and array of names" do
+      collection.insert_as(zen_master)
+      collection.insert_as(zen_master2)
+      check_by_names(collection.find(:zen_master => [zen_master_name,zen_master2_name]), zen_master_name, zen_master2_name)
+    end
+
+    it "should find a resource by type symbol and array of names with custom names" do
+      collection.insert_as(zen_master, :zzz, "name1")
+      collection.insert_as(zen_master2, :zzz, "name2")
+      check_by_names(collection.find( :zzz => ["name1","name2"]), zen_master_name, zen_master2_name)
+    end
+
+    it "should find resources of multiple kinds (:zen_master => a, :zen_follower => b)" do
+      collection.insert_as(zen_master)
+      collection.insert_as(zen_follower)
+      check_by_names(collection.find(:zen_master => [zen_master_name], :zen_follower => [zen_follower_name]),
+                     zen_master_name, zen_follower_name)
+    end
+
+    it "should find resources of multiple kinds (:zen_master => a, :zen_follower => b with custom names)" do
+      collection.insert_as(zen_master, :zzz, "name1")
+      collection.insert_as(zen_master2, :zzz, "name2")
+      collection.insert_as(zen_follower, :yyy, "name3")
+      check_by_names(collection.find(:zzz => ["name1","name2"], :yyy => ["name3"]),
+                     zen_master_name, zen_follower_name, zen_master2_name)
+    end
+
+    it "should find a resource by string zen_master[a]" do
+      collection.insert_as(zen_master)
+      expect(collection.find("zen_master[#{zen_master_name}]")).to eq(zen_master)
+    end
+
+    it "should find a resource by string zen_master[a] with custom names" do
+      collection.insert_as(zen_master, :zzz, "name1")
+      expect(collection.find("zzz[name1]")).to eq(zen_master)
+    end
+
+    it "should find resources by strings of zen_master[a,b]" do
+      collection.insert_as(zen_master)
+      collection.insert_as(zen_master2)
+      check_by_names(collection.find("zen_master[#{zen_master_name},#{zen_master2_name}]"),
+                     zen_master_name, zen_master2_name)
+    end
+
+    it "should find resources by strings of zen_master[a,b] with custom names" do
+      collection.insert_as(zen_master, :zzz, "name1")
+      collection.insert_as(zen_master2, :zzz, "name2")
+      check_by_names(collection.find("zzz[name1,name2]"),
+                     zen_master_name, zen_master2_name)
+    end
+
+    it "should find resources of multiple types by strings of zen_master[a]" do
+      collection.insert_as(zen_master)
+      collection.insert_as(zen_follower)
+      check_by_names(collection.find("zen_master[#{zen_master_name}]", "zen_follower[#{zen_follower_name}]"),
+                     zen_master_name, zen_follower_name)
+    end
+
+    it "should find resources of multiple types by strings of zen_master[a] with custom names" do
+      collection.insert_as(zen_master, :zzz, "name1")
+      collection.insert_as(zen_master2, :zzz, "name2")
+      collection.insert_as(zen_follower, :yyy, "name3")
+      check_by_names(collection.find("zzz[name1,name2]", "yyy[name3]"),
+                     zen_master_name, zen_follower_name,zen_master2_name)
+    end
+
+    it "should only keep the last copy when multiple instances of a Resource are inserted" do
+      collection.insert_as(zen_master)
+      expect(collection.find("zen_master[#{zen_master_name}]")).to eq(zen_master)
+      new_zm =zen_master.dup
+      new_zm.retries = 10
+      expect(new_zm).to_not eq(zen_master)
+      collection.insert_as(new_zm)
+      expect(collection.find("zen_master[#{zen_master_name}]")).to eq(new_zm)
+    end
+
+    it "should raise an exception if you pass a bad name to resources" do
+      expect { collection.find("michael jackson") }.to raise_error(ArgumentError)
+    end
+
+    it "should raise an exception if you pass something other than a string or hash to resource" do
+      expect { collection.find([Array.new]) }.to raise_error(ArgumentError)
+    end
+
+    it "raises an error when attempting to find a resource that does not exist" do
+      expect { collection.find("script[nonesuch]") }.to raise_error(Chef::Exceptions::ResourceNotFound)
+    end
+  end
+
+  describe "validate_lookup_spec!" do
+    it "accepts a string of the form 'resource_type[resource_name]'" do
+      expect(collection.validate_lookup_spec!("resource_type[resource_name]")).to be_true
+    end
+
+    it "accepts a single-element :resource_type => 'resource_name' Hash" do
+      expect(collection.validate_lookup_spec!(:service => "apache2")).to be_true
+    end
+
+    it "accepts a chef resource object" do
+      expect(collection.validate_lookup_spec!(zen_master)).to be_true
+    end
+
+    it "rejects a malformed query string" do
+      expect { collection.validate_lookup_spec!("resource_type[missing-end-bracket") }.to \
+        raise_error(Chef::Exceptions::InvalidResourceSpecification)
+    end
+
+    it "rejects an argument that is not a String, Hash, or Chef::Resource" do
+      expect { collection.validate_lookup_spec!(Object.new) }.to \
+        raise_error(Chef::Exceptions::InvalidResourceSpecification)
+    end
+
+  end
+
+  def check_by_names(results, *names)
+    expect(results.size).to eq(names.size)
+    names.each do |name|
+      expect(results.detect{|r| r.name == name}).to_not eq(nil)
+    end
+  end
+
+end
diff --git a/spec/unit/resource_collection_spec.rb b/spec/unit/resource_collection_spec.rb
index cf119f1ab0..a575d2996c 100644
--- a/spec/unit/resource_collection_spec.rb
+++ b/spec/unit/resource_collection_spec.rb
@@ -26,6 +26,10 @@ describe Chef::ResourceCollection do
     @resource = Chef::Resource::ZenMaster.new("makoto")
   end
 
+  it "should throw an error when calling a non-delegated method" do
+    expect { @rc.not_a_method }.to raise_error(NoMethodError)
+  end
+
   describe "initialize" do
     it "should return a Chef::ResourceCollection" do
       @rc.should be_kind_of(Chef::ResourceCollection)
@@ -35,7 +39,7 @@ describe Chef::ResourceCollection do
   describe "[]" do
     it "should accept Chef::Resources through [index]" do
       lambda { @rc[0] = @resource }.should_not raise_error
-      lambda { @rc[0] = "string" }.should raise_error
+      lambda { @rc[0] = "string" }.should raise_error(ArgumentError)
     end
 
     it "should allow you to fetch Chef::Resources by position" do
@@ -47,7 +51,7 @@ describe Chef::ResourceCollection do
   describe "push" do
     it "should accept Chef::Resources through pushing" do
       lambda { @rc.push(@resource) }.should_not raise_error
-      lambda { @rc.push("string") }.should raise_error
+      lambda { @rc.push("string") }.should raise_error(ArgumentError)
     end
   end
 
@@ -60,7 +64,12 @@ describe Chef::ResourceCollection do
   describe "insert" do
     it "should accept only Chef::Resources" do
       lambda { @rc.insert(@resource) }.should_not raise_error
-      lambda { @rc.insert("string") }.should raise_error
+      lambda { @rc.insert("string") }.should raise_error(ArgumentError)
+    end
+
+    it "should accept named arguments in any order" do
+      @rc.insert(@resource, :instance_name => 'foo', :resource_type =>'bar')
+      expect(@rc[0]).to eq(@resource)
     end
 
     it "should append resources to the end of the collection when not executing a run" do
@@ -88,39 +97,6 @@ describe Chef::ResourceCollection do
     end
   end
 
-  describe "insert_at" do
-    it "should accept only Chef::Resources" do
-      lambda { @rc.insert_at(0, @resource, @resource) }.should_not raise_error
-      lambda { @rc.insert_at(0, "string") }.should raise_error
-      lambda { @rc.insert_at(0, @resource, "string") }.should raise_error
-    end
-
-    it "should toss an error if it receives a bad index" do
-      @rc.insert_at(10, @resource)
-    end
-
-    it "should insert resources at the beginning when asked" do
-      @rc.insert(Chef::Resource::ZenMaster.new('1'))
-      @rc.insert(Chef::Resource::ZenMaster.new('2'))
-      @rc.insert_at(0, Chef::Resource::ZenMaster.new('X'))
-      @rc.all_resources.map { |r| r.name }.should == [ 'X', '1', '2' ]
-    end
-
-    it "should insert resources in the middle when asked" do
-      @rc.insert(Chef::Resource::ZenMaster.new('1'))
-      @rc.insert(Chef::Resource::ZenMaster.new('2'))
-      @rc.insert_at(1, Chef::Resource::ZenMaster.new('X'))
-      @rc.all_resources.map { |r| r.name }.should == [ '1', 'X', '2' ]
-    end
-
-    it "should insert resources at the end when asked" do
-      @rc.insert(Chef::Resource::ZenMaster.new('1'))
-      @rc.insert(Chef::Resource::ZenMaster.new('2'))
-      @rc.insert_at(2, Chef::Resource::ZenMaster.new('X'))
-      @rc.all_resources.map { |r| r.name }.should == [ '1', '2', 'X' ]
-    end
-  end
-
   describe "each" do
     it "should allow you to iterate over every resource in the collection" do
       load_up_resources
@@ -300,13 +276,13 @@ describe Chef::ResourceCollection do
 
   describe "provides access to the raw resources array" do
     it "returns the resources via the all_resources method" do
-      @rc.all_resources.should equal(@rc.instance_variable_get(:@resources))
+      @rc.all_resources.should equal(@rc.instance_variable_get(:@resource_list).instance_variable_get(:@resources))
     end
   end
 
   describe "provides access to stepable iterator" do
     it "returns the iterator object" do
-      @rc.instance_variable_set(:@iterator, :fooboar)
+      @rc.instance_variable_get(:@resource_list).instance_variable_set(:@iterator, :fooboar)
       @rc.iterator.should == :fooboar
     end
   end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 5dfc17f333..bcc91d52bc 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -174,12 +174,12 @@ describe Chef::Resource do
     end
 
     it "should load the attributes of a prior resource" do
-      @resource.load_prior_resource
+      @resource.load_prior_resource(@resource.resource_name, @resource.name)
       @resource.supports.should == { :funky => true }
     end
 
     it "should not inherit the action from the prior resource" do
-      @resource.load_prior_resource
+      @resource.load_prior_resource(@resource.resource_name, @resource.name)
       @resource.action.should_not == @prior_resource.action
     end
   end
diff --git a/spec/unit/shell/shell_ext_spec.rb b/spec/unit/shell/shell_ext_spec.rb
index c24acbca3e..8485b66d23 100644
--- a/spec/unit/shell/shell_ext_spec.rb
+++ b/spec/unit/shell/shell_ext_spec.rb
@@ -121,7 +121,7 @@ describe Shell::Extensions do
       Shell.session.stub(:rebuild_context)
       events = Chef::EventDispatch::Dispatcher.new
       run_context = Chef::RunContext.new(Chef::Node.new, {}, events)
-      run_context.resource_collection.instance_variable_set(:@iterator, :the_iterator)
+      run_context.resource_collection.instance_variable_get(:@resource_list).instance_variable_set(:@iterator, :the_iterator)
       Shell.session.run_context = run_context
       @root_context.chef_run.should == :the_iterator
     end