diff options
| -rw-r--r-- | chef-config/lib/chef-config/config.rb | 6 | ||||
| -rw-r--r-- | lib/chef/deprecated.rb | 50 | ||||
| -rw-r--r-- | lib/chef/resource_builder.rb | 64 | ||||
| -rw-r--r-- | spec/unit/recipe_spec.rb | 158 |
4 files changed, 23 insertions, 255 deletions
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 |