Merge pull request #6778 from chef/lcg/immutable-node-apis

update immutable API blacklist and whitelist

3 files changed, 232 insertions, 9 deletions

diff --git a/lib/chef/node/mixin/immutablize_array.rb b/lib/chef/node/mixin/immutablize_array.rb
index bd330cf8a9..88c7e6ffa9 100644
--- a/lib/chef/node/mixin/immutablize_array.rb
+++ b/lib/chef/node/mixin/immutablize_array.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,15 +19,119 @@ class Chef
   class Node
     module Mixin
       module ImmutablizeArray
+        # Allowed methods that MUST NOT mutate the object
+        # (if any of these methods mutate the underlying object that is a bug that needs to be fixed)
+        ALLOWED_METHODS = [
+          :&,
+          :*,
+          :+,
+          :-,
+          :[],
+          :abbrev,
+          :all?,
+          :any?,
+          :assoc,
+          :at,
+          :bsearch,
+          :bsearch_index,
+          :chunk,
+          :chunk_while,
+          :collect,
+          :collect_concat,
+          :combination,
+          :compact,
+          :count,
+          :cycle,
+          :detect,
+          :dig,
+          :drop,
+          :drop_while,
+          :each,
+          :each_cons,
+          :each_entry,
+          :each_index,
+          :each_slice,
+          :each_with_index,
+          :each_with_object,
+          :empty?,
+          :entries,
+          :fetch,
+          :find,
+          :find_all,
+          :find_index,
+          :first,
+          :flat_map,
+          :flatten,
+          :grep,
+          :grep_v,
+          :group_by,
+          :include?,
+          :index,
+          :inject,
+          :join,
+          :last,
+          :lazy,
+          :length,
+          :map,
+          :max,
+          :max_by,
+          :member?,
+          :min,
+          :min_by,
+          :minmax,
+          :minmax_by,
+          :none?,
+          :one?,
+          :pack,
+          :partition,
+          :permutation,
+          :product,
+          :rassoc,
+          :reduce,
+          :reject,
+          :repeated_combination,
+          :repeated_permutation,
+          :reverse,
+          :reverse_each,
+          :rindex,
+          :rotate,
+          :sample,
+          :save_plist,
+          :select,
+          :shelljoin,
+          :shuffle,
+          :size,
+          :slice,
+          :slice_after,
+          :slice_before,
+          :slice_when,
+          :sort,
+          :sort_by,
+          :sum,
+          :take,
+          :take_while,
+          :to_a,
+          :to_ary,
+          :to_h,
+          :to_plist,
+          :to_set,
+          :transpose,
+          :uniq,
+          :values_at,
+          :zip,
+          :|,
+        ]
         # A list of methods that mutate Array. Each of these is overridden to
         # raise an error, making this instances of this class more or less
         # immutable.
         DISALLOWED_MUTATOR_METHODS = [
           :<<,
           :[]=,
+          :append,
           :clear,
           :collect!,
           :compact!,
+          :concat,
           :default=,
           :default_proc=,
           :delete,
@@ -40,18 +144,21 @@ class Chef
           :map!,
           :merge!,
           :pop,
+          :prepend,
           :push,
-          :update,
           :reject!,
-          :reverse!,
           :replace,
+          :reverse!,
+          :rotate!,
           :select!,
           :shift,
+          :shuffle!,
           :slice!,
           :sort!,
           :sort_by!,
           :uniq!,
           :unshift,
+          :update,
         ]
 
         # Redefine all of the methods that mutate a Hash to raise an error when called.
diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb
index f6b22ed7d7..cd2bbf7bf6 100644
--- a/lib/chef/node/mixin/immutablize_hash.rb
+++ b/lib/chef/node/mixin/immutablize_hash.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,10 +19,112 @@ class Chef
   class Node
     module Mixin
       module ImmutablizeHash
+        # allowed methods that MUST NOT mutate the object
+        # (if any of these methods mutate the underlying object that is a bug that needs to be fixed)
+        ALLOWED_METHODS = [
+          :<,
+          :<=,
+          :>,
+          :>=,
+          :[],
+          :all?,
+          :any?,
+          :assoc,
+          :chunk,
+          :chunk_while,
+          :collect,
+          :collect_concat,
+          :compact,
+          :compare_by_identity,
+          :compare_by_identity?,
+          :count,
+          :cycle,
+          :default,
+          :default_proc,
+          :detect,
+          :dig,
+          :drop,
+          :drop_while,
+          :each,
+          :each_cons,
+          :each_entry,
+          :each_key,
+          :each_pair,
+          :each_slice,
+          :each_value,
+          :each_with_index,
+          :each_with_object,
+          :empty?,
+          :entries,
+          :fetch,
+          :fetch_values,
+          :find,
+          :find_all,
+          :find_index,
+          :first,
+          :flat_map,
+          :flatten,
+          :grep,
+          :grep_v,
+          :group_by,
+          :has_key?,
+          :has_value?,
+          :include?,
+          :index,
+          :inject,
+          :invert,
+          :key,
+          :key?,
+          :keys,
+          :lazy,
+          :length,
+          :map,
+          :max,
+          :max_by,
+          :member?,
+          :merge,
+          :min,
+          :min_by,
+          :minmax,
+          :minmax_by,
+          :none?,
+          :one?,
+          :partition,
+          :rassoc,
+          :reduce,
+          :reject,
+          :reverse_each,
+          :save_plist,
+          :select,
+          :size,
+          :slice,
+          :slice_after,
+          :slice_before,
+          :slice_when,
+          :sort,
+          :sort_by,
+          :sum,
+          :take,
+          :take_while,
+          :to_a,
+          :to_h,
+          :to_hash,
+          :to_plist,
+          :to_proc,
+          :to_set,
+          :transform_keys,
+          :transform_values,
+          :uniq,
+          :value?,
+          :values,
+          :values_at,
+          :zip,
+        ]
         DISALLOWED_MUTATOR_METHODS = [
           :[]=,
           :clear,
           :collect!,
+          :compact!,
           :default=,
           :default_proc=,
           :delete,
@@ -30,15 +132,19 @@ class Chef
           :keep_if,
           :map!,
           :merge!,
-          :update,
+          :rehash,
           :reject!,
           :replace,
           :select!,
           :shift,
-          :write,
-          :write!,
-          :unlink,
+          :store,
+          :transform_keys!,
+          :transform_values!,
           :unlink!,
+          :unlink,
+          :update,
+          :write!,
+          :write,
         ]
 
         # Redefine all of the methods that mutate a Hash to raise an error when called.
diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb
index 96adedddcb..29ae8799e9 100644
--- a/spec/unit/node/immutable_collections_spec.rb
+++ b/spec/unit/node/immutable_collections_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2012-2017, Chef Software Inc.
+# Copyright:: Copyright 2012-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -34,6 +34,11 @@ describe Chef::Node::ImmutableMash do
     @immutable_mash = @node["key"]
   end
 
+  it "does not have any unaudited methods" do
+    unaudited_methods = Hash.instance_methods - Object.instance_methods - Chef::Node::Mixin::ImmutablizeHash::DISALLOWED_MUTATOR_METHODS - Chef::Node::Mixin::ImmutablizeHash::ALLOWED_METHODS
+    expect(unaudited_methods).to be_empty
+  end
+
   it "element references like regular hash" do
     expect(@immutable_mash[:top][:second_level]).to eq("some value")
   end
@@ -228,6 +233,11 @@ describe Chef::Node::ImmutableArray do
     end
   end
 
+  it "does not have any unaudited methods" do
+    unaudited_methods = Array.instance_methods - Object.instance_methods - Chef::Node::Mixin::ImmutablizeArray::DISALLOWED_MUTATOR_METHODS - Chef::Node::Mixin::ImmutablizeArray::ALLOWED_METHODS
+    expect(unaudited_methods).to be_empty
+  end
+
   it "can be duped even if some elements can't" do
     @immutable_array.dup
   end