Merge pull request #7224 from coderanger/map-lock

Implement rfc107: NodeMap locking for resource and provider handlers

2 files changed, 106 insertions, 4 deletions

diff --git a/spec/unit/node_map_spec.rb b/spec/unit/node_map_spec.rb
index 24f3bebe2a..7e4980219a 100644
--- a/spec/unit/node_map_spec.rb
+++ b/spec/unit/node_map_spec.rb
@@ -22,6 +22,12 @@ require "chef/node_map"
 class Foo; end
 class Bar; end
 
+class FooResource < Chef::Resource; end
+class BarResource < Chef::Resource; end
+
+class FooProvider < Chef::Provider; end
+class BarProvider < Chef::Provider; end
+
 describe Chef::NodeMap do
 
   let(:node_map) { Chef::NodeMap.new }
@@ -139,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 eql({ :thing => [{ :klass => Bar }] })
+      expect( node_map.delete_class(Bar) ).to include({ :thing => [{ :klass => Bar, :cookbook_override => false, :core_override => false }] })
       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 }] })
+      expect( node_map.delete_class(Bar) ).to eql({ :thing => [{ :klass => Bar, :cookbook_override => false, :core_override => false }] })
       expect( node_map.get(node, :thing) ).to eql(Foo)
     end
 
@@ -154,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 }], :thing2 => [{ :klass => Bar }] })
+      expect( node_map.delete_class(Bar) ).to eql({ :thing1 => [{ :klass => Bar, :cookbook_override => false, :core_override => false }], :thing2 => [{ :klass => Bar, :cookbook_override => false, :core_override => false }] })
       expect( node_map.get(node, :thing1) ).to eql(nil)
       expect( node_map.get(node, :thing2) ).to eql(Foo)
     end
@@ -204,4 +210,66 @@ describe Chef::NodeMap do
     end
   end
 
+  describe "locked mode" do
+    context "while unlocked" do
+      it "allows setting the same key twice" do
+        expect(Chef).to_not receive(:log_deprecation)
+        node_map.set(:foo, FooResource)
+        node_map.set(:foo, BarResource)
+        expect(node_map.get(node, :foo)).to eql(BarResource)
+      end
+    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(:log_deprecation).with("Trying to register resource foo on top of existing Chef core resource. Check if a new version of the cookbook is available.")
+        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(:log_deprecation)
+        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(:log_deprecation)
+        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(:log_deprecation).with("Trying to register resource foo on top of existing Chef core resource. Check if a new version of the cookbook is available.")
+        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(:log_deprecation)
+        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(:log_deprecation).with("Trying to register provider foo on top of existing Chef core provider. Check if a new version of the cookbook is available.")
+        node_map.set(:foo, FooProvider)
+        node_map.lock!
+        node_map.set(:foo, BarProvider)
+        # expect(node_map.get(node, :foo)).to eql(FooProvider)
+      end
+    end
+  end
+
 end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 2866d5439f..26660c9415 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -867,7 +867,7 @@ end
       snitch_var1 = snitch_var2 = 0
       runner = Chef::Runner.new(run_context)
 
-      Chef::Provider::SnakeOil.provides :cat
+      Chef::Provider::SnakeOil.provides :cat, __core_override__: true
 
       resource1.only_if { snitch_var1 = 1 }
       resource1.not_if { snitch_var2 = 2 }
@@ -1144,4 +1144,38 @@ end
       it { is_expected.to eq [:two, :one] }
     end
   end
+
+  describe ".preview_resource" do
+    let(:klass) { Class.new(Chef::Resource) }
+
+    before do
+      allow(Chef::DSL::Resources).to receive(:add_resource_dsl).with(:test_resource)
+    end
+
+    it "defaults to false" do
+      expect(klass.preview_resource).to eq false
+    end
+
+    it "can be set to true" do
+      klass.preview_resource(true)
+      expect(klass.preview_resource).to eq true
+    end
+
+    it "does not affect provides by default" 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
 end