Merge pull request #5856 from chef/lcg/remove-resource-cloning

Chef-13 remove resource cloning and 3694 warnings

5 files changed, 30 insertions, 262 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index ee72e888e0..fe7955653b 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,6 +1,6 @@
 GIT
   remote: https://github.com/chef/chef-server
-  revision: 8944bea554c88a83696576463ca11354c5713170
+  revision: 56811d1726945c85b9d8c09a7954eb7e91bdcb10
   specs:
     oc-chef-pedant (2.2.0)
       activesupport (>= 4.2.7.1, < 6.0)
@@ -37,7 +37,7 @@ GIT
 
 GIT
   remote: https://github.com/chef/ohai.git
-  revision: cd241b6cd73d2d103145c27bda1ca8700a777a4d
+  revision: ca3bdf456c80b68e48e6a380624640f8f85d7811
   specs:
     ohai (13.0.0)
       chef-config (>= 12.5.0.alpha.1, < 14)
@@ -54,7 +54,7 @@ GIT
 
 GIT
   remote: https://github.com/poise/halite.git
-  revision: b414e0995d12f8b563f6e881e2a9e25c4563c22e
+  revision: b0f1372ea7710e47b52c8c843597d21aaed5ebf6
   specs:
     halite (1.4.1.pre)
       bundler
@@ -64,7 +64,7 @@ GIT
 
 GIT
   remote: https://github.com/poise/poise-boiler.git
-  revision: 25dc08432ac88a3f017467f4ca3918503f761c0d
+  revision: 409326e84519cf1a212073aed2f5cd3a6963d347
   specs:
     poise-boiler (1.13.3.pre)
       bundler
@@ -97,7 +97,7 @@ GIT
 
 GIT
   remote: https://github.com/poise/poise.git
-  revision: 3da48ce71b4f7c70013fe455c3bf3643e8f94ead
+  revision: 62056469b3a19144f4bf4810abe14efa1c10463b
   specs:
     poise (2.7.3.pre)
       halite (~> 1.0)
@@ -195,7 +195,7 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    activesupport (5.0.1)
+    activesupport (5.0.2)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (~> 0.7)
       minitest (~> 5.1)
@@ -547,7 +547,7 @@ GEM
       ffi-win32-extensions
     windows-api (0.4.4)
       win32-api (>= 1.4.5)
-    winrm (2.1.2)
+    winrm (2.1.3)
       builder (>= 2.1.2)
       erubis (~> 2.7)
       gssapi (~> 1.2)
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 00af16687a..fb41cfae99 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -1046,12 +1046,6 @@ module ChefConfig
 
     default :rubygems_url, "https://rubygems.org"
 
-    # This controls the behavior of resource cloning (and CHEF-3694 warnings).  For Chef < 12 the behavior
-    # has been that this is 'true', in Chef 13 this will change to false.  Setting this to 'true' in Chef
-    # 13 is not a viable or supported migration strategy since Chef 13 community cookbooks will be expected
-    # to break with this setting set to 'true'.
-    default :resource_cloning, true
-
     # If installed via an omnibus installer, this gives the path to the
     # "embedded" directory which contains all of the software packaged with
     # omnibus. This is used to locate the cacert.pem file on windows.
diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb
index 25eea5b5ec..461f65225b 100644
--- a/lib/chef/deprecated.rb
+++ b/lib/chef/deprecated.rb
@@ -66,6 +66,16 @@ class Chef
       end
     end
 
+    class InternalApi < Base
+      def id
+        0
+      end
+
+      def target
+        "internal_api.html"
+      end
+    end
+
     class JsonAutoInflate < Base
       def id
         1
@@ -148,6 +158,16 @@ class Chef
       end
     end
 
+    class DnfPackageAllowDowngrade < Base
+      def id
+        10
+      end
+
+      def target
+        "dnf_package_allow_downgrade.html"
+      end
+    end
+
     class PropertyNameCollision < Base
       def id
         11
@@ -198,35 +218,7 @@ class Chef
       end
     end
 
-    class ResourceCloning < Base
-      def id
-        3694
-      end
-
-      def target
-        "resource_cloning.html"
-      end
-    end
-
-    class InternalApi < Base
-      def id
-        0
-      end
-
-      def target
-        "internal_api.html"
-      end
-    end
-
-    class DnfPackageAllowDowngrade < Base
-      def id
-        10
-      end
-
-      def target
-        "dnf_package_allow_downgrade.html"
-      end
-    end
+    # id 3694 was deleted
 
     class Generic < Base
       def url
diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb
index 1aee852f5d..275f795362 100644
--- a/lib/chef/resource_builder.rb
+++ b/lib/chef/resource_builder.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Lamont Granquist (<lamont@chef.io>)
-# Copyright:: Copyright 2015-2016, Chef Software, Inc.
+# Copyright:: Copyright 2015-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -52,14 +52,6 @@ class Chef
       resource.source_line = created_at
       resource.declared_type = type
 
-      # If we have a resource like this one, we want to steal its state
-      # 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
-      if prior_resource && Chef::Config[:resource_cloning]
-        resource.load_from(prior_resource)
-      end
-
       resource.cookbook_name = cookbook_name
       resource.recipe_name = recipe_name
       # Determine whether this resource is being created in the context of an enclosing Provider
@@ -79,15 +71,6 @@ class Chef
         end
       end
 
-      # emit a cloned resource warning if it is warranted
-      if prior_resource && Chef::Config[:resource_cloning]
-        if is_trivial_resource?(prior_resource) && identicalish_resources?(prior_resource, resource)
-          emit_harmless_cloning_debug
-        else
-          emit_cloned_resource_warning
-        end
-      end
-
       # Run optional resource hook
       resource.after_created
 
@@ -103,51 +86,6 @@ class Chef
       @resource_class ||= Chef::Resource.resource_for_node(type, run_context.node)
     end
 
-    def is_trivial_resource?(resource)
-      trivial_resource = resource_class.new(name, run_context)
-      # force un-lazy the name property on the created trivial resource
-      name_property = resource_class.properties.find { |sym, p| p.name_property? }
-      trivial_resource.send(name_property[0]) unless name_property.nil?
-      identicalish_resources?(trivial_resource, resource)
-    end
-
-    # this is an equality test specific to checking for 3694 cloning warnings
-    def identicalish_resources?(first, second)
-      skipped_ivars = [ :@source_line, :@cookbook_name, :@recipe_name, :@params, :@elapsed_time, :@declared_type ]
-      checked_ivars = ( first.instance_variables | second.instance_variables ) - skipped_ivars
-      non_matching_ivars = checked_ivars.reject do |iv|
-        if iv == :@action && ( [first.instance_variable_get(iv)].flatten == [:nothing] || [second.instance_variable_get(iv)].flatten == [:nothing] )
-          # :nothing action on either side of the comparison always matches
-          true
-        else
-          first.instance_variable_get(iv) == second.instance_variable_get(iv)
-        end
-      end
-      Chef::Log.debug("ivars which did not match with the prior resource: #{non_matching_ivars}")
-      non_matching_ivars.empty?
-    end
-
-    def emit_cloned_resource_warning
-      message = "Cloning resource attributes for #{resource} from prior resource"
-      message << "\nPrevious #{prior_resource}: #{prior_resource.source_line}" if prior_resource.source_line
-      message << "\nCurrent  #{resource}: #{resource.source_line}" if resource.source_line
-      Chef.deprecated(:resource_cloning, message)
-    end
-
-    def emit_harmless_cloning_debug
-      Chef::Log.debug("Harmless resource cloning from #{prior_resource}:#{prior_resource.source_line} to #{resource}:#{resource.source_line}")
-    end
-
-    def prior_resource
-      @prior_resource ||=
-        begin
-          key = "#{type}[#{name}]"
-          run_context.resource_collection.lookup_local(key)
-        rescue Chef::Exceptions::ResourceNotFound
-          nil
-        end
-    end
-
   end
 end
 
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index e1e3e0ad72..abba1ceb4d 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -3,7 +3,7 @@
 # Author:: Christopher Walters (<cw@chef.io>)
 # Author:: Tim Hinderliter (<tim@chef.io>)
 # Author:: Seth Chisamore (<schisamo@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -225,109 +225,6 @@ describe Chef::Recipe do
       end
     end
 
-    describe "when cloning resources" do
-      def expect_warning
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-      end
-
-      it "should emit a 3694 warning when attributes change" do
-        recipe.zen_master "klopp" do
-          something "bvb"
-        end
-        expect_warning
-        recipe.zen_master "klopp" do
-          something "vbv"
-        end
-      end
-
-      it "should emit a 3694 warning when attributes change" do
-        recipe.zen_master "klopp" do
-          something "bvb"
-        end
-        expect_warning
-        recipe.zen_master "klopp" do
-          something "bvb"
-          peace true
-        end
-      end
-
-      it "should emit a 3694 warning when attributes change" do
-        recipe.zen_master "klopp" do
-          something "bvb"
-          peace true
-        end
-        expect_warning
-        recipe.zen_master "klopp" do
-          something "bvb"
-        end
-      end
-
-      it "should emit a 3694 warning for non-trivial attributes (unfortunately)" do
-        recipe.zen_master "klopp" do
-          something "bvb"
-        end
-        expect_warning
-        recipe.zen_master "klopp" do
-          something "bvb"
-        end
-      end
-
-      it "should not emit a 3694 warning for completely trivial resource cloning" do
-        recipe.zen_master "klopp"
-        expect(Chef).to_not receive(:deprecated)
-        recipe.zen_master "klopp"
-      end
-
-      it "should not emit a 3694 warning when attributes do not change and the first action is :nothing" do
-        recipe.zen_master "klopp" do
-          action :nothing
-        end
-        expect(Chef).to_not receive(:deprecated)
-        recipe.zen_master "klopp" do
-          action :score
-        end
-      end
-
-      it "should not emit a 3694 warning when attributes do not change and the second action is :nothing" do
-        recipe.zen_master "klopp" do
-          action :score
-        end
-        expect(Chef).to_not receive(:deprecated)
-        recipe.zen_master "klopp" do
-          action :nothing
-        end
-      end
-
-      class Coerced < Chef::Resource
-        resource_name :coerced
-        provides :coerced
-        default_action :whatever
-        property :package_name, [String, Array], coerce: proc { |x| [x].flatten }, name_property: true
-        def after_created
-          Array(action).each do |action|
-            run_action(action)
-          end
-        end
-        action :whatever do
-          package_name # unlazy the package_name
-        end
-      end
-
-      it "does not emit 3694 when the name_property is unlazied by running it at compile_time" do
-        recipe.coerced "string"
-        expect(Chef).to_not receive(:deprecated)
-        recipe.coerced "string"
-      end
-
-      it "validating resources via build_resource" do
-        expect do
-          recipe.build_resource(:remote_file, "klopp") do
-            source Chef::DelayedEvaluator.new { "http://chef.io" }
-          end end.to_not raise_error
-      end
-
-    end
-
     describe "creating resources via declare_resource" do
       let(:zm_resource) do
         recipe.declare_resource(:zen_master, "klopp") do
@@ -351,7 +248,6 @@ describe Chef::Recipe do
       end
 
       it "will insert another resource if create_if_missing is not set (cloned resource as of Chef-12)" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
         zm_resource
         recipe.declare_resource(:zen_master, "klopp")
         expect(run_context.resource_collection.count).to eql(2)
@@ -453,58 +349,6 @@ describe Chef::Recipe do
 
     end
 
-    describe "resource cloning" do
-
-      let(:second_recipe) do
-        Chef::Recipe.new("second_cb", "second_recipe", run_context)
-      end
-
-      let(:original_resource) do
-        recipe.zen_master("klopp") do
-          something "bvb09"
-          action :score
-        end
-      end
-
-      let(:duplicated_resource) do
-        original_resource
-        second_recipe.zen_master("klopp") do
-          # attrs should be cloned
-        end
-      end
-
-      it "copies attributes from the first resource" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-        expect(duplicated_resource.something).to eq("bvb09")
-      end
-
-      it "does not copy the action from the first resource" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-        expect(original_resource.action).to eq([:score])
-        expect(duplicated_resource.action).to eq([:nothing])
-      end
-
-      it "does not copy the source location of the first resource" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-        # sanity check source location:
-        expect(original_resource.source_line).to include(__FILE__)
-        expect(duplicated_resource.source_line).to include(__FILE__)
-        # actual test:
-        expect(original_resource.source_line).not_to eq(duplicated_resource.source_line)
-      end
-
-      it "sets the cookbook name on the cloned resource to that resource's cookbook" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-        expect(duplicated_resource.cookbook_name).to eq("second_cb")
-      end
-
-      it "sets the recipe name on the cloned resource to that resoure's recipe" do
-        expect(Chef).to receive(:deprecated).with(:resource_cloning, /^Cloning resource attributes for zen_master\[klopp\]/)
-        expect(duplicated_resource.recipe_name).to eq("second_recipe")
-      end
-
-    end
-
     describe "resource definitions" do
       it "should execute defined resources" do
         crow_define = Chef::ResourceDefinition.new