Merge pull request #5816 from chef/lcg/yum-coerce-array-attributes

coerce immutable arrays to normal arrays in the yum_package resource

3 files changed, 55 insertions, 5 deletions

diff --git a/lib/chef/resource/package.rb b/lib/chef/resource/package.rb
index baaa5be2c8..a1f174a6f3 100644
--- a/lib/chef/resource/package.rb
+++ b/lib/chef/resource/package.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Tyler Cloke (<tyler@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");
diff --git a/lib/chef/resource/yum_package.rb b/lib/chef/resource/yum_package.rb
index 1e0ad197ba..2fc5db5cc3 100644
--- a/lib/chef/resource/yum_package.rb
+++ b/lib/chef/resource/yum_package.rb
@@ -1,6 +1,6 @@
 #
 # 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");
@@ -24,8 +24,17 @@ class Chef
       resource_name :yum_package
       provides :package, os: "linux", platform_family: %w{rhel fedora}
 
-      # Install a specific arch
-      property :arch, [ String, Array ]
+      # XXX: the coercions here are due to the provider promiscuously updating the properties on the
+      # new_resource which causes immutable modification exceptions when passed an immutable node array.
+      #
+      # <lecture>
+      # THIS is why updating the new_resource in a provider is so terrible, and is equivalent to methods scribbling over
+      # its own arguments as unintended side-effects (and why functional languages that don't allow modifcations
+      # of variables eliminate entire classes of bugs).
+      # </lecture>
+      property :package_name, [ String, Array ], identity: true, coerce: proc { |x| x.is_a?(Array) ? x.to_a : x }
+      property :version, [ String, Array ], coerce: proc { |x| x.is_a?(Array) ? x.to_a : x }
+      property :arch, [ String, Array ], coerce: proc { |x| x.is_a?(Array) ? x.to_a : x }
 
       property :flush_cache,
         Hash,
diff --git a/spec/unit/resource/yum_package_spec.rb b/spec/unit/resource/yum_package_spec.rb
index bc2d19d50e..a1e8417e0e 100644
--- a/spec/unit/resource/yum_package_spec.rb
+++ b/spec/unit/resource/yum_package_spec.rb
@@ -1,6 +1,6 @@
 #
 # 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");
@@ -32,6 +32,47 @@ describe Chef::Resource::YumPackage, "initialize" do
 
 end
 
+describe Chef::Resource::YumPackage do
+  before(:each) do
+    @resource = Chef::Resource::YumPackage.new("foo")
+  end
+
+  # this set of tests is somewhat terrible.  the yum provider promiscuously writes over
+  # the new_resource.package_named/version/arch properties.  until that is fixed properly
+  # we need to coerce and dup those properties into normal arrays.  this does not affect
+  # strings because those are not mutated in place and they are not (currently) frozen
+  # in immutable attributes (even though they really, really should be).
+  context "when passed immutable node attribute arrays" do
+    let(:node) { Chef::Node.new }
+
+    before do
+      node.default["foo"] = %w{one two three}
+    end
+
+    it "allows mutation of the package_name array" do
+      @resource.package_name node["foo"]
+      expect(@resource.package_name).not_to be_a_kind_of(Chef::Node::ImmutableArray)
+      expect { @resource.package_name[0] = "four" }.not_to raise_error
+      expect(@resource.package_name).to eql(%w{four two three})
+    end
+
+    it "allows mutation of the version array" do
+      @resource.version node["foo"]
+      expect(@resource.version).not_to be_a_kind_of(Chef::Node::ImmutableArray)
+      expect { @resource.version[0] = "four" }.not_to raise_error
+      expect(@resource.version).to eql(%w{four two three})
+    end
+
+    it "allows mutation of the arch array" do
+      @resource.arch node["foo"]
+      expect(@resource.arch).not_to be_a_kind_of(Chef::Node::ImmutableArray)
+      expect { @resource.arch[0] = "four" }.not_to raise_error
+      expect(@resource.arch).to eql(%w{four two three})
+    end
+
+  end
+end
+
 describe Chef::Resource::YumPackage, "arch" do
   before(:each) do
     @resource = Chef::Resource::YumPackage.new("foo")