Merge branch 'master' into no-more-vendor

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

7 files changed, 145 insertions, 19 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 14608f822d..03be083bdb 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -241,6 +241,19 @@ mutable default value, define it inside a `lazy{}` helper like:
 property :x, default: lazy { {} }
 ```
 
+### Rubygems provider sources behavior changed.
+
+The default behavior of the `gem_package` and `chef_gem` resources is now to inherit whatever settings are in the external environment
+that chef is running in.  Chef no longer forces `https://rubygems.org`.  The `Chef::Config[:rubygems_uri]` default has been changed to
+nil.  It can now be set to either a string URI or to an array of string URIs.  The behavior of setting the source on an individual
+resource now overrides the source setting completely and does not inherit the global setting.
+
+Users that previously relied on the source setting always being additive to "https://rubygmes.org" will find that they need to use
+the array form and explicitly add "https://rubygems.org" to their resources.  Users can now more easily remove "https://rubygems.org"
+either globally or on a resource case-by-case basis.
+
+The behavior of the `clear_sources` property is now to only add `--clear-sources` and has no side effects on the source options.
+
 ### `knife cookbook site vendor` has been removed
 
 Please use `knife cookbook site install` instead.
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 9bffb66eca..b4bb6b76ab 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -1050,7 +1050,8 @@ module ChefConfig
     # break Chef community cookbooks and is very highly discouraged.
     default :ruby_encoding, Encoding::UTF_8
 
-    default :rubygems_url, "https://rubygems.org"
+    # can be set to a string or array of strings for URIs to set as rubygems sources
+    default :rubygems_url, nil
 
     # If installed via an omnibus installer, this gives the path to the
     # "embedded" directory which contains all of the software packaged with
diff --git a/lib/chef/cookbook/gem_installer.rb b/lib/chef/cookbook/gem_installer.rb
index 365b185426..5b1426e4e8 100644
--- a/lib/chef/cookbook/gem_installer.rb
+++ b/lib/chef/cookbook/gem_installer.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright (c) 2010-2016 Chef Software, Inc.
+# Copyright:: Copyright (c) 2010-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -50,7 +50,9 @@ class Chef
           begin
             Dir.mktmpdir("chef-gem-bundle") do |dir|
               File.open("#{dir}/Gemfile", "w+") do |tf|
-                tf.puts "source '#{Chef::Config[:rubygems_url]}'"
+                Array(Chef::Config[:rubygems_url] || "https://www.rubygems.org").each do |s|
+                  tf.puts "source '#{s}'"
+                end
                 cookbook_gems.each do |gem_name, args|
                   tf.puts "gem(*#{([gem_name] + args).inspect})"
                 end
diff --git a/lib/chef/provider/package/rubygems.rb b/lib/chef/provider/package/rubygems.rb
index 9bf8fb5fbc..cdb2269c73 100644
--- a/lib/chef/provider/package/rubygems.rb
+++ b/lib/chef/provider/package/rubygems.rb
@@ -423,6 +423,7 @@ class Chef
 
         def source_is_remote?
           return true if new_resource.source.nil?
+          return true if new_resource.source.is_a?(Array)
           scheme = URI.parse(new_resource.source).scheme
           # URI.parse gets confused by MS Windows paths with forward slashes.
           scheme = nil if scheme =~ /^[a-z]$/
@@ -469,7 +470,8 @@ class Chef
         end
 
         def gem_sources
-          new_resource.source ? Array(new_resource.source) : nil
+          srcs = new_resource.source || Chef::Config[:rubygems_url]
+          srcs ? Array(srcs) : nil
         end
 
         def load_current_resource
@@ -533,18 +535,21 @@ class Chef
         end
 
         def install_via_gem_command(name, version)
-          if new_resource.source =~ /\.gem$/i
+          src = []
+          if new_resource.source.is_a?(String) && new_resource.source =~ /\.gem$/i
             name = new_resource.source
-          elsif new_resource.clear_sources
-            src = " --clear-sources"
-            src << (new_resource.source && " --source=#{new_resource.source}" || "")
           else
-            src = new_resource.source && " --source=#{new_resource.source} --source=#{Chef::Config[:rubygems_url]}"
+            src << "--clear-sources" if new_resource.clear_sources
+            srcarry = [ new_resource.source || Chef::Config[:rubygems_url] ].flatten.compact
+            srcarry.each do |s|
+              src << "--source=#{s}"
+            end
           end
+          src_str = src.empty? ? "" : " #{src.join(" ")}"
           if !version.nil? && !version.empty?
-            shell_out_with_timeout!("#{gem_binary_path} install #{name} -q --no-rdoc --no-ri -v \"#{version}\"#{src}#{opts}", env: nil)
+            shell_out_with_timeout!("#{gem_binary_path} install #{name} -q --no-rdoc --no-ri -v \"#{version}\"#{src_str}#{opts}", env: nil)
           else
-            shell_out_with_timeout!("#{gem_binary_path} install \"#{name}\" -q --no-rdoc --no-ri #{src}#{opts}", env: nil)
+            shell_out_with_timeout!("#{gem_binary_path} install \"#{name}\" -q --no-rdoc --no-ri #{src_str}#{opts}", env: nil)
           end
         end
 
diff --git a/lib/chef/resource/gem_package.rb b/lib/chef/resource/gem_package.rb
index 5511d3c580..bcbe6d37b3 100644
--- a/lib/chef/resource/gem_package.rb
+++ b/lib/chef/resource/gem_package.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@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");
@@ -23,6 +23,17 @@ class Chef
     class GemPackage < Chef::Resource::Package
       resource_name :gem_package
 
+      # the source can either be a path to a package source like:
+      #   source /var/tmp/mygem-1.2.3.4.gem
+      # or it can be a url rubygems source like:
+      #   https://www.rubygems.org
+      # the default has to be nil in order for the magical wiring up of the name property to
+      # the source pathname to work correctly.
+      #
+      # we don't do coercions here because its all a bit too complicated
+      #
+      # FIXME? the array form of installing paths most likely does not work?
+      #
       property :source, [ String, Array ]
       property :clear_sources, [ true, false ], default: false, desired_state: false
       # Sets a custom gem_binary to run for gem commands.
diff --git a/spec/unit/cookbook/gem_installer_spec.rb b/spec/unit/cookbook/gem_installer_spec.rb
index 69b714d977..91e6959331 100644
--- a/spec/unit/cookbook/gem_installer_spec.rb
+++ b/spec/unit/cookbook/gem_installer_spec.rb
@@ -67,4 +67,19 @@ describe Chef::Cookbook::GemInstaller do
 
     expect(bundler_dsl.dependencies.find { |d| d.name == "httpclient" }.requirements_list.length).to eql(2)
   end
+
+  it "generates a valid Gemfile when Chef::Config[:rubygems_url] is set to a String" do
+    Chef::Config[:rubygems_url] = "https://www.rubygems.org"
+    expect { gem_installer.install }.to_not raise_error
+
+    expect(bundler_dsl.dependencies.find { |d| d.name == "httpclient" }.requirements_list.length).to eql(2)
+  end
+
+  it "generates a valid Gemfile when Chef::Config[:rubygems_url] is set to an Array" do
+    Chef::Config[:rubygems_url] = [ "https://www.rubygems.org" ]
+
+    expect { gem_installer.install }.to_not raise_error
+
+    expect(bundler_dsl.dependencies.find { |d| d.name == "httpclient" }.requirements_list.length).to eql(2)
+  end
 end
diff --git a/spec/unit/provider/package/rubygems_spec.rb b/spec/unit/provider/package/rubygems_spec.rb
index 53c82f2f70..08dd888a2b 100644
--- a/spec/unit/provider/package/rubygems_spec.rb
+++ b/spec/unit/provider/package/rubygems_spec.rb
@@ -534,6 +534,16 @@ describe Chef::Provider::Package::Rubygems do
         end
       end
 
+      context "when the source is from the rubygems_uri" do
+        it "determines the candidate version by querying the remote gem servers" do
+          Chef::Config[:rubygems_url] = "https://mirror1/"
+          expect(provider.gem_env).to receive(:candidate_version_from_remote)
+            .with(gem_dep, "https://mirror1/")
+            .and_return(Gem::Version.new(target_version))
+          expect(provider.candidate_version).to eq(target_version)
+        end
+      end
+
       context "when the requested source is a remote server" do
         let(:source) { "http://mygems.example.com" }
 
@@ -543,6 +553,33 @@ describe Chef::Provider::Package::Rubygems do
             .and_return(Gem::Version.new(target_version))
           expect(provider.candidate_version).to eq(target_version)
         end
+
+        it "overwrites the config variable" do
+          Chef::Config[:rubygems_url] = "https://overridden"
+          expect(provider.gem_env).to receive(:candidate_version_from_remote)
+            .with(gem_dep, source)
+            .and_return(Gem::Version.new(target_version))
+          expect(provider.candidate_version).to eq(target_version)
+        end
+      end
+
+      context "when the requested source is an array" do
+        let(:source) { [ "https://mirror1", "https://mirror2" ] }
+
+        it "determines the candidate version by querying the remote gem servers" do
+          expect(provider.gem_env).to receive(:candidate_version_from_remote)
+            .with(gem_dep, *source)
+            .and_return(Gem::Version.new(target_version))
+          expect(provider.candidate_version).to eq(target_version)
+        end
+
+        it "overwrites the config variable" do
+          Chef::Config[:rubygems_url] = "https://overridden"
+          expect(provider.gem_env).to receive(:candidate_version_from_remote)
+            .with(gem_dep, *source)
+            .and_return(Gem::Version.new(target_version))
+          expect(provider.candidate_version).to eq(target_version)
+        end
       end
 
       context "when the requested source is a file" do
@@ -566,13 +603,14 @@ describe Chef::Provider::Package::Rubygems do
         current_resource
       end
 
+      let(:version) { Gem::Version.new(candidate_version) }
+
       before do
-        version = Gem::Version.new(candidate_version)
-        args = [gem_dep]
-        args << source if source
-        allow(provider.gem_env).to receive(:candidate_version_from_remote)
-          .with(*args)
-          .and_return(version)
+        if source
+          allow(provider.gem_env).to receive(:candidate_version_from_remote).with(gem_dep, *source).and_return(version)
+        else
+          allow(provider.gem_env).to receive(:candidate_version_from_remote).with(gem_dep).and_return(version)
+        end
       end
 
       describe "in the current gem environment" do
@@ -633,12 +671,53 @@ describe Chef::Provider::Package::Rubygems do
           end
         end
 
+        context "when the Chef::Config[:rubygems_url] option is provided" do
+          let(:gem_binary) { "/foo/bar" }
+
+          it "installs the gem with rubygems.org as an added source" do
+            Chef::Config[:rubygems_url] = "https://mirror1"
+            expect(provider.gem_env).to receive(:candidate_version_from_remote).with(gem_dep, Chef::Config[:rubygems_url]).and_return(version)
+            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=https://mirror1"
+            expect(provider).to receive(:shell_out!).with(expected, env: nil, timeout: 900)
+            provider.run_action(:install)
+            expect(new_resource).to be_updated_by_last_action
+          end
+        end
+
         context "when another source and binary are provided" do
           let(:source) { "http://mirror.ops.rhcloud.com/mirror/ruby" }
           let(:gem_binary) { "/foo/bar" }
 
           it "installs the gem with rubygems.org as an added source" do
-            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=#{source} --source=https://rubygems.org"
+            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=#{source}"
+            expect(provider).to receive(:shell_out!).with(expected, env: nil, timeout: 900)
+            provider.run_action(:install)
+            expect(new_resource).to be_updated_by_last_action
+          end
+
+          it "ignores the Chef::Config setting" do
+            Chef::Config[:rubygems_url] = "https://ignored"
+            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=#{source}"
+            expect(provider).to receive(:shell_out!).with(expected, env: nil, timeout: 900)
+            provider.run_action(:install)
+            expect(new_resource).to be_updated_by_last_action
+          end
+        end
+
+        context "when the source is an array" do
+          let(:source) { [ "https://mirror1" , "https://mirror2" ] }
+          let(:gem_binary) { "/foo/bar" }
+
+          it "installs the gem with an array as an added source" do
+            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=https://mirror1 --source=https://mirror2"
+            expect(provider).to receive(:shell_out!).with(expected, env: nil, timeout: 900)
+            provider.run_action(:install)
+            expect(new_resource).to be_updated_by_last_action
+          end
+
+          it "ignores the Chef::Config setting" do
+            Chef::Config[:rubygems_url] = "https://ignored"
+            expected = "#{gem_binary} install rspec-core -q --no-rdoc --no-ri -v \"#{target_version}\" --source=https://mirror1 --source=https://mirror2"
             expect(provider).to receive(:shell_out!).with(expected, env: nil, timeout: 900)
             provider.run_action(:install)
             expect(new_resource).to be_updated_by_last_action