Remove resource collision deprecations

Switch to only doing warnings.  Remove the features in the node_map that
we never used.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

5 files changed, 17 insertions, 89 deletions

diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb
index 6516366d49..e3931427ba 100644
--- a/lib/chef/deprecated.rb
+++ b/lib/chef/deprecated.rb
@@ -207,9 +207,7 @@ class Chef
       target 23
     end
 
-    class MapCollision < Base
-      target 25
-    end
+    # id 25 was deleted
 
     # id 3694 was deleted
 
diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb
index e66e249a97..01f9d215cb 100644
--- a/lib/chef/node_map.rb
+++ b/lib/chef/node_map.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Lamont Granquist (<lamont@chef.io>)
-# Copyright:: Copyright 2014-2018, Chef Software Inc.
+# Copyright:: Copyright 2014-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -37,12 +37,7 @@
 #
 class Chef
   class NodeMap
-    COLLISION_WARNING_14 = <<~EOH.gsub(/\s+/, " ").strip
-      %{type_caps} %{key} from a cookbook is overriding the %{type} from the client.  Please upgrade your cookbook
-        or remove the cookbook from your run_list before the next major release of Chef.
-    EOH
-
-    COLLISION_WARNING_15 = <<~EOH.gsub(/\s+/, " ").strip
+    COLLISION_WARNING = <<~EOH.gsub(/\s+/, " ").strip
       %{type_caps} %{key} from the client is overriding the %{type} from a cookbook.  Please upgrade your cookbook
         or remove the cookbook from your run_list.
     EOH
@@ -54,19 +49,13 @@ class Chef
     # @param key [Object] Key to store
     # @param value [Object] Value associated with the key
     # @param filters [Hash] Node filter options to apply to key retrieval
-    # @param allow_cookbook_override [Boolean, String] Allow a cookbook to add
-    #   to this key even in locked mode. If a string is given, it should be a
-    #   Gem::Requirement-compatible value indicating for which Chef versions an
-    #   override from cookbooks is allowed.
-    # @param __core_override__ [Boolean] Advanced-mode override to add to a key
-    #   even in locked mode.
     # @param chef_version [String] version constraint to match against the running Chef::VERSION
     #
     # @yield [node] Arbitrary node filter as a block which takes a node argument
     #
     # @return [NodeMap] Returns self for possible chaining
     #
-    def set(key, klass, platform: nil, platform_version: nil, platform_family: nil, os: nil, canonical: nil, override: nil, allow_cookbook_override: false, __core_override__: false, chef_version: nil, target_mode: nil, &block) # rubocop:disable Lint/UnderscorePrefixedVariableName
+    def set(key, klass, platform: nil, platform_version: nil, platform_family: nil, os: nil, canonical: nil, override: nil, chef_version: nil, target_mode: nil, &block)
       new_matcher = { klass: klass }
       new_matcher[:platform] = platform if platform
       new_matcher[:platform_version] = platform_version if platform_version
@@ -75,8 +64,6 @@ class Chef
       new_matcher[:block] = block if block
       new_matcher[:canonical] = canonical if canonical
       new_matcher[:override] = override if override
-      new_matcher[:cookbook_override] = allow_cookbook_override
-      new_matcher[:core_override] = __core_override__
       new_matcher[:target_mode] = target_mode
 
       if chef_version && Chef::VERSION !~ chef_version
@@ -88,9 +75,7 @@ class Chef
       # 1. Core override mode is not set.
       # 2. The key exists.
       # 3. At least one previous `provides` is now locked.
-      # 4. No previous `provides` had `allow_cookbook_override`, either set to
-      #    true or with a string version matcher that still matches Chef::VERSION
-      if !__core_override__ && map[key] && map[key].any? { |matcher| matcher[:locked] } && !map[key].any? { |matcher| matcher[:cookbook_override].is_a?(String) ? Chef::VERSION =~ matcher[:cookbook_override] : matcher[:cookbook_override] }
+      if map[key] && map[key].any? { |matcher| matcher[:locked] } && !map[key].any? { |matcher| matcher[:cookbook_override].is_a?(String) ? Chef::VERSION =~ matcher[:cookbook_override] : matcher[:cookbook_override] }
         # If we ever use locked mode on things other than the resource and provider handler maps, this probably needs a tweak.
         type_of_thing = if klass < Chef::Resource
                           "resource"
@@ -99,11 +84,7 @@ class Chef
                         else
                           klass.superclass.to_s
                         end
-        # For now, only log the warning.
-        Chef.deprecated(:map_collision, COLLISION_WARNING_14 % { type: type_of_thing, key: key, type_caps: type_of_thing.capitalize })
-        # In 15.0, uncomment this and remove the log above.
-        # Chef.deprecated(:map_collision, COLLISION_WARNING_15 % {type: type_of_thing, key: key, type_caps: type_of_thing.capitalize}))
-        # return
+        Chef::Log.warn( COLLISION_WARNING % { type: type_of_thing, key: key, type_caps: type_of_thing.capitalize } )
       end
 
       # The map is sorted in order of preference already; we just need to find
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index e08b2b0bc3..56475af249 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1354,12 +1354,6 @@ class Chef
         remove_canonical_dsl
       end
 
-      # If a resource is in preview mode, set allow_cookbook_override on all its
-      # mappings by default.
-      if preview_resource && !options.include?(:allow_cookbook_override)
-        options[:allow_cookbook_override] = true
-      end
-
       if @chef_version_for_provides && !options.include?(:chef_version)
         options[:chef_version] = @chef_version_for_provides
       end
diff --git a/spec/unit/node_map_spec.rb b/spec/unit/node_map_spec.rb
index 7c867857dc..2b38f8cea5 100644
--- a/spec/unit/node_map_spec.rb
+++ b/spec/unit/node_map_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Lamont Granquist (<lamont@chef.io>)
-# Copyright:: Copyright 2014-2018, Chef Software Inc.
+# Copyright:: Copyright 2014-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -145,14 +145,14 @@ describe Chef::NodeMap do
   describe "deleting classes" do
     it "deletes a class and removes the mapping completely" do
       node_map.set(:thing, Bar)
-      expect( node_map.delete_class(Bar) ).to include({ thing: [{ klass: Bar, cookbook_override: false, core_override: false, target_mode: nil }] })
+      expect( node_map.delete_class(Bar) ).to include({ thing: [{ klass: Bar, target_mode: nil }] })
       expect( node_map.get(node, :thing) ).to eql(nil)
     end
 
     it "deletes a class and leaves the mapping that still has an entry" do
       node_map.set(:thing, Bar)
       node_map.set(:thing, Foo)
-      expect( node_map.delete_class(Bar) ).to eql({ thing: [{ klass: Bar, cookbook_override: false, core_override: false, target_mode: nil }] })
+      expect( node_map.delete_class(Bar) ).to eql({ thing: [{ klass: Bar, target_mode: nil }] })
       expect( node_map.get(node, :thing) ).to eql(Foo)
     end
 
@@ -160,7 +160,7 @@ describe Chef::NodeMap do
       node_map.set(:thing1, Bar)
       node_map.set(:thing2, Bar)
       node_map.set(:thing2, Foo)
-      expect( node_map.delete_class(Bar) ).to eql({ thing1: [{ klass: Bar, cookbook_override: false, core_override: false, target_mode: nil }], thing2: [{ klass: Bar, cookbook_override: false, core_override: false, target_mode: nil }] })
+      expect( node_map.delete_class(Bar) ).to eql({ thing1: [{ klass: Bar, target_mode: nil }], thing2: [{ klass: Bar, target_mode: nil }] })
       expect( node_map.get(node, :thing1) ).to eql(nil)
       expect( node_map.get(node, :thing2) ).to eql(Foo)
     end
@@ -247,7 +247,7 @@ describe Chef::NodeMap do
   describe "locked mode" do
     context "while unlocked" do
       it "allows setting the same key twice" do
-        expect(Chef).to_not receive(:deprecated)
+        expect(Chef::Log).to_not receive(:warn)
         node_map.set(:foo, FooResource)
         node_map.set(:foo, BarResource)
         expect(node_map.get(node, :foo)).to eql(BarResource)
@@ -255,53 +255,20 @@ describe Chef::NodeMap do
     end
 
     context "while locked" do
-      # Uncomment the commented `expect`s in 15.0.
-      it "rejects setting the same key twice" do
-        expect(Chef).to receive(:deprecated).with(:map_collision, /Resource foo/)
+      it "warns on setting the same key twice" do
+        expect(Chef::Log).to receive(:warn).with(/Resource foo/)
         node_map.set(:foo, FooResource)
         node_map.lock!
         node_map.set(:foo, BarResource)
-        # expect(node_map.get(node, :foo)).to eql(FooResource)
-      end
-
-      it "allows setting the same key twice when the first has allow_cookbook_override" do
-        expect(Chef).to_not receive(:deprecated)
-        node_map.set(:foo, FooResource, allow_cookbook_override: true)
-        node_map.lock!
-        node_map.set(:foo, BarResource)
-        expect(node_map.get(node, :foo)).to eql(BarResource)
-      end
-
-      it "allows setting the same key twice when the first has allow_cookbook_override with a future version" do
-        expect(Chef).to_not receive(:deprecated)
-        node_map.set(:foo, FooResource, allow_cookbook_override: "< 100")
-        node_map.lock!
-        node_map.set(:foo, BarResource)
-        expect(node_map.get(node, :foo)).to eql(BarResource)
-      end
-
-      it "rejects setting the same key twice when the first has allow_cookbook_override with a past version" do
-        expect(Chef).to receive(:deprecated).with(:map_collision, /Resource foo/)
-        node_map.set(:foo, FooResource, allow_cookbook_override: "< 1")
-        node_map.lock!
-        node_map.set(:foo, BarResource)
-        # expect(node_map.get(node, :foo)).to eql(FooResource)
-      end
-
-      it "allows setting the same key twice when the second has __core_override__" do
-        expect(Chef).to_not receive(:deprecated)
-        node_map.set(:foo, FooResource)
-        node_map.lock!
-        node_map.set(:foo, BarResource, __core_override__: true)
         expect(node_map.get(node, :foo)).to eql(BarResource)
       end
 
-      it "rejects setting the same key twice for a provider" do
-        expect(Chef).to receive(:deprecated).with(:map_collision, /Provider foo/)
+      it "warns on setting the same key twice for a provider" do
+        expect(Chef::Log).to receive(:warn).with(/Provider foo/)
         node_map.set(:foo, FooProvider)
         node_map.lock!
         node_map.set(:foo, BarProvider)
-        # expect(node_map.get(node, :foo)).to eql(FooProvider)
+        expect(node_map.get(node, :foo)).to eql(BarProvider)
       end
     end
   end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 3bd13b4c97..1c8e991566 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -867,7 +867,7 @@ describe Chef::Resource do
       snitch_var1 = snitch_var2 = 0
       runner = Chef::Runner.new(run_context)
 
-      Chef::Provider::SnakeOil.provides :cat, __core_override__: true
+      Chef::Provider::SnakeOil.provides :cat
 
       resource1.only_if { snitch_var1 = 1 }
       resource1.not_if { snitch_var2 = 2 }
@@ -1191,18 +1191,6 @@ describe Chef::Resource do
       expect(Chef.resource_handler_map).to receive(:set).with(:test_resource, klass, { canonical: true })
       klass.resource_name(:test_resource)
     end
-
-    it "adds allow_cookbook_override when true" do
-      expect(Chef.resource_handler_map).to receive(:set).with(:test_resource, klass, { canonical: true, allow_cookbook_override: true })
-      klass.preview_resource(true)
-      klass.resource_name(:test_resource)
-    end
-
-    it "allows manually overriding back to false" do
-      expect(Chef.resource_handler_map).to receive(:set).with(:test_resource, klass, { allow_cookbook_override: false })
-      klass.preview_resource(true)
-      klass.provides(:test_resource, allow_cookbook_override: false)
-    end
   end
 
   describe "tagged" do