Chef-13: remove method_missing access to node object.

So again the reasons why we break this:

- node.class vs. node['class'] kinds of problems
- adding node#foo to Chef::Node as an extension to the API and
  breaking node.foo meaning node["foo"] (we can't make exensions
  to that class without possibly breaking someone)
- crazy things like the old CHEF-3799 issue in the old ticketing
  system where IO#puts in ruby blindly calls #to_ary on stuff and
  expects it to raise -- whereas we would potentially autovivify
  a 'to_ary' hash key and return nil which breaks the world.

This also has caused issues with the hashie gem and they've gone to
spamming warnings by default to try to deal with it:

https://github.com/berkshelf/berkshelf/issues/1665

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

5 files changed, 13 insertions, 128 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 4febd47b44..761694e010 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -1,7 +1,7 @@
 #--
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: AJ Christensen (<aj@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");
@@ -476,25 +476,6 @@ class Chef
 
       alias :each_attribute :each
 
-      def method_missing(symbol, *args)
-        if symbol == :to_ary
-          merged_attributes.send(symbol, *args)
-        elsif args.empty?
-          Chef.deprecated(:attributes, %q{method access to node attributes (node.foo.bar) is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]["bar"])})
-          if key?(symbol)
-            self[symbol]
-          else
-            raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'"
-          end
-        elsif symbol.to_s =~ /=$/
-          Chef.deprecated(:attributes, %q{method setting of node attributes (node.foo="bar") is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]="bar")})
-          key_to_set = symbol.to_s[/^(.+)=$/, 1]
-          self[key_to_set] = (args.length == 1 ? args[0] : args)
-        else
-          raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'"
-        end
-      end
-
       def to_s
         merged_attributes.to_s
       end
diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb
index 694b5fbc3a..a31b2d2b9b 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -1,6 +1,6 @@
 #--
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2012-2016, Chef Software, Inc.
+# Copyright:: Copyright 2012-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -180,24 +180,6 @@ class Chef
 
       alias :attribute? :has_key?
 
-      def method_missing(symbol, *args)
-        # Calling `puts arg` implicitly calls #to_ary on `arg`. If `arg` does
-        # not implement #to_ary, ruby recognizes it as a single argument, and
-        # if it returns an Array, then ruby prints each element. If we don't
-        # account for that here, we'll auto-vivify a VividMash for the key
-        # :to_ary which creates an unwanted key and raises a TypeError.
-        if symbol == :to_ary
-          super
-        elsif args.empty?
-          self[symbol]
-        elsif symbol.to_s =~ /=$/
-          key_to_set = symbol.to_s[/^(.+)=$/, 1]
-          self[key_to_set] = (args.length == 1 ? args[0] : args)
-        else
-          raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'. To set an attribute, use `#{symbol}=value' instead."
-        end
-      end
-
       def convert_key(key)
         super
       end
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index dad712e078..12ee2e5dd8 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2012-2016, Chef Software, Inc.
+# Copyright:: Copyright 2012-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -125,24 +125,6 @@ class Chef
 
       alias :attribute? :has_key?
 
-      def method_missing(symbol, *args)
-        if symbol == :to_ary
-          super
-        elsif args.empty?
-          if key?(symbol)
-            self[symbol]
-          else
-            raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'"
-          end
-          # This will raise a ImmutableAttributeModification error:
-        elsif symbol.to_s =~ /=$/
-          key_to_set = symbol.to_s[/^(.+)=$/, 1]
-          self[key_to_set] = (args.length == 1 ? args[0] : args)
-        else
-          raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'"
-        end
-      end
-
       # Mash uses #convert_value to mashify values on input.
       # Since we're handling this ourselves, override it to be a no-op
       def convert_value(value)
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index a3e62ff939..e9515ff3b4 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: AJ Christensen (<aj@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");
@@ -490,11 +490,6 @@ describe Chef::Node::Attribute do
       expect(@attributes.has_key?("does_not_exist_at_all")).to eq(false)
     end
 
-    it "should return true if an attribute exists but is set to nil using dot notation" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      expect(@attributes.music.deeper.has_key?("gates_of_ishtar")).to eq(true)
-    end
-
     it "should return true if an attribute exists but is set to false" do
       @attributes.has_key?("music")
       expect(@attributes["music"].has_key?("apophis")).to eq(true)
@@ -530,19 +525,6 @@ describe Chef::Node::Attribute do
 
   end
 
-  describe "method_missing" do
-    it "should behave like a [] lookup" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      expect(@attributes.music.mastodon).to eq("rocks")
-    end
-
-    it "should allow the last method to set a value if it has an = sign on the end" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      @attributes.normal.music.mastodon = %w{dream still shining}
-      expect(@attributes.normal.music.mastodon).to eq(%w{dream still shining})
-    end
-  end
-
   describe "keys" do
     before(:each) do
       @attributes = Chef::Node::Attribute.new(
@@ -1109,25 +1091,25 @@ describe Chef::Node::Attribute do
   describe "when setting a component attribute to a new value" do
     it "converts the input in to a VividMash tree (default)" do
       @attributes.default = {}
-      @attributes.default.foo = "bar"
+      @attributes.default["foo"] = "bar"
       expect(@attributes.merged_attributes[:foo]).to eq("bar")
     end
 
     it "converts the input in to a VividMash tree (normal)" do
       @attributes.normal = {}
-      @attributes.normal.foo = "bar"
+      @attributes.normal["foo"] = "bar"
       expect(@attributes.merged_attributes[:foo]).to eq("bar")
     end
 
     it "converts the input in to a VividMash tree (override)" do
       @attributes.override = {}
-      @attributes.override.foo = "bar"
+      @attributes.override["foo"] = "bar"
       expect(@attributes.merged_attributes[:foo]).to eq("bar")
     end
 
     it "converts the input in to a VividMash tree (automatic)" do
       @attributes.automatic = {}
-      @attributes.automatic.foo = "bar"
+      @attributes.automatic["foo"] = "bar"
       expect(@attributes.merged_attributes[:foo]).to eq("bar")
     end
   end
@@ -1170,11 +1152,6 @@ describe Chef::Node::Attribute do
     it "raises an error when using []=" do
       expect { @attributes[:new_key] = "new value" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
     end
-
-    it "raises an error when using `attr=value`" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      expect { @attributes.new_key = "new value" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
-    end
   end
 
   describe "deeply converting values" do
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index a8f50de046..40780e523b 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -238,43 +238,25 @@ describe Chef::Node do
       expect(node["battles"]["people"].attribute?("snozzberry")).to eq(false)
     end
 
-    it "does not allow you to set an attribute via method_missing" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      expect { node.sunshine = "is bright" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
-    end
-
     it "does not allow modification of node attributes via hash methods" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
       node.default["h4sh"] = { foo: "bar" }
       expect { node["h4sh"].delete("foo") }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
-      expect { node.h4sh.delete("foo") }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
     end
 
     it "does not allow modification of node attributes via array methods" do
       Chef::Config[:treat_deprecation_warnings_as_errors] = false
       node.default["array"] = []
       expect { node["array"] << "boom" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
-      expect { node.array << "boom" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification)
     end
 
     it "returns merged immutable attributes for arrays" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
       node.default["array"] = []
       expect( node["array"].class ).to eql(Chef::Node::ImmutableArray)
-      expect( node.array.class ).to eql(Chef::Node::ImmutableArray)
     end
 
     it "returns merged immutable attributes for hashes" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
       node.default["h4sh"] = {}
       expect( node["h4sh"].class ).to eql(Chef::Node::ImmutableMash)
-      expect( node.h4sh.class ).to eql(Chef::Node::ImmutableMash)
-    end
-
-    it "should allow you get get an attribute via method_missing" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      node.default.sunshine = "is bright"
-      expect(node.sunshine).to eql("is bright")
     end
 
     describe "normal attributes" do
@@ -321,12 +303,6 @@ describe Chef::Node do
         expect(node[:snoopy][:is_a_puppy]).to eq(true)
       end
 
-      it "auto-vivifies attributes created via method syntax" do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-        node.normal.fuu.bahrr.baz = "qux"
-        expect(node.fuu.bahrr.baz).to eq("qux")
-      end
-
       it "should let you use tag as a convience method for the tags attribute" do
         node.normal["tags"] = %w{one two}
         node.tag("three", "four")
@@ -407,12 +383,6 @@ describe Chef::Node do
         expect(node["a"]["r1"]["g"]["u"]).to eql("u1")
       end
 
-      it "auto-vivifies attributes created via method syntax" do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-        node.default.fuu.bahrr.baz = "qux"
-        expect(node.fuu.bahrr.baz).to eq("qux")
-      end
-
       it "default_unless correctly resets the deep merge cache" do
         node.normal["tags"] = []  # this sets our top-level breadcrumb
         node.default_unless["foo"]["bar"] = "NK-19V"
@@ -469,12 +439,6 @@ describe Chef::Node do
         node.override[:snoopy][:is_a_puppy] = true
         expect(node[:snoopy][:is_a_puppy]).to eq(true)
       end
-
-      it "auto-vivifies attributes created via method syntax" do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-        node.override.fuu.bahrr.baz = "qux"
-        expect(node.fuu.bahrr.baz).to eq("qux")
-      end
     end
 
     describe "globally deleting attributes" do
@@ -811,10 +775,9 @@ describe Chef::Node do
     #
     describe "deep merge attribute cache edge conditions" do
       it "does not error with complicated attribute substitution" do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
         node.default["chef_attribute_hell"]["attr1"] = "attribute1"
-        node.default["chef_attribute_hell"]["attr2"] = "#{node.chef_attribute_hell.attr1}/attr2"
-        expect { node.default["chef_attribute_hell"]["attr3"] = "#{node.chef_attribute_hell.attr2}/attr3" }.not_to raise_error
+        node.default["chef_attribute_hell"]["attr2"] = "#{node[:chef_attribute_hell][:attr1]}/attr2"
+        expect { node.default["chef_attribute_hell"]["attr3"] = "#{node[:chef_attribute_hell][:attr2]}/attr3" }.not_to raise_error
       end
 
       it "caches both strings and symbols correctly" do
@@ -829,7 +792,7 @@ describe Chef::Node do
         Chef::Config[:treat_deprecation_warnings_as_errors] = false
         node.default["passenger"]["version"]     = "4.0.57"
         node.default["passenger"]["root_path"]   = "passenger-#{node['passenger']['version']}"
-        node.default["passenger"]["root_path_2"] = "passenger-#{node.passenger['version']}"
+        node.default["passenger"]["root_path_2"] = "passenger-#{node[:passenger]['version']}"
         expect(node["passenger"]["root_path_2"]).to eql("passenger-4.0.57")
         expect(node[:passenger]["root_path_2"]).to eql("passenger-4.0.57")
       end
@@ -841,8 +804,8 @@ describe Chef::Node do
     end
 
     it "should allow you to iterate over attributes with each_attribute" do
-      node.default.sunshine = "is bright"
-      node.default.canada = "is a nice place"
+      node.default["sunshine"] = "is bright"
+      node.default["canada"] = "is a nice place"
       seen_attributes = Hash.new
       node.each_attribute do |a, v|
         seen_attributes[a] = v