Merge pull request #12083 from chef/lcg/remote-file-http-options

Add http_options property to remote_file

8 files changed, 106 insertions, 13 deletions

diff --git a/cspell.json b/cspell.json
index 13edadf059..6cb571bbbf 100644
--- a/cspell.json
+++ b/cspell.json
@@ -1046,6 +1046,7 @@
     "netbw",
     "NETCARD",
     "netdom",
+    "nethttp",
     "NETLOGON",
     "netmsg",
     "NETNAME",
diff --git a/lib/chef/http.rb b/lib/chef/http.rb
index 00e1d2bd50..f3d3308206 100644
--- a/lib/chef/http.rb
+++ b/lib/chef/http.rb
@@ -82,6 +82,9 @@ class Chef
     # [Boolean] if we're doing keepalives or not
     attr_reader :keepalives
 
+    # @returns [Hash] options for Net::HTTP to be sent to setters on the object
+    attr_reader :nethttp_opts
+
     # Create a HTTP client object. The supplied +url+ is used as the base for
     # all subsequent requests. For example, when initialized with a base url
     # http://localhost:4000, a call to +get+ with 'nodes' will make an
@@ -94,6 +97,7 @@ class Chef
       @redirect_limit = 10
       @keepalives = options[:keepalives] || false
       @options = options
+      @nethttp_opts = options[:nethttp] || {}
 
       @middlewares = []
       self.class.middlewares.each do |middleware_class|
@@ -311,7 +315,7 @@ class Chef
 
         SocketlessChefZeroClient.new(base_url)
       else
-        BasicClient.new(base_url, ssl_policy: ssl_policy, keepalives: keepalives)
+        BasicClient.new(base_url, ssl_policy: ssl_policy, keepalives: keepalives, nethttp_opts: nethttp_opts)
       end
     end
 
@@ -468,12 +472,12 @@ class Chef
 
     # @api private
     def http_retry_delay
-      config[:http_retry_delay]
+      options[:http_retry_delay] || config[:http_retry_delay]
     end
 
     # @api private
     def http_retry_count
-      config[:http_retry_count]
+      options[:http_retry_count] || config[:http_retry_count]
     end
 
     # @api private
diff --git a/lib/chef/http/basic_client.rb b/lib/chef/http/basic_client.rb
index 2513b750eb..9850d68401 100644
--- a/lib/chef/http/basic_client.rb
+++ b/lib/chef/http/basic_client.rb
@@ -36,16 +36,18 @@ class Chef
       attr_reader :url
       attr_reader :ssl_policy
       attr_reader :keepalives
+      attr_reader :nethttp_opts
 
       # Instantiate a BasicClient.
       # === Arguments:
       # url:: An URI for the remote server.
       # === Options:
       # ssl_policy:: The SSL Policy to use, defaults to DefaultSSLPolicy
-      def initialize(url, opts = {})
+      def initialize(url, ssl_policy: DefaultSSLPolicy, keepalives: false, nethttp_opts: {})
         @url = url
-        @ssl_policy = opts[:ssl_policy] || DefaultSSLPolicy
-        @keepalives = opts[:keepalives] || false
+        @ssl_policy = ssl_policy
+        @keepalives = keepalives
+        @nethttp_opts = ChefUtils::Mash.new(nethttp_opts)
       end
 
       def http_client
@@ -118,8 +120,14 @@ class Chef
           configure_ssl(http_client)
         end
 
-        http_client.read_timeout = config[:rest_timeout]
-        http_client.open_timeout = config[:rest_timeout]
+        opts = nethttp_opts.dup
+        opts["read_timeout"] ||= config[:rest_timeout]
+        opts["open_timeout"] ||= config[:rest_timeout]
+
+        opts.each do |key, value|
+          http_client.send(:"#{key}=", value)
+        end
+
         if keepalives
           http_client.start
         else
@@ -142,11 +150,11 @@ class Chef
       end
 
       def http_proxy_user(proxy_uri)
-        proxy_uri.user || Chef::Config["#{proxy_uri.scheme}_proxy_user"]
+        proxy_uri.user || config["#{proxy_uri.scheme}_proxy_user"]
       end
 
       def http_proxy_pass(proxy_uri)
-        proxy_uri.password || Chef::Config["#{proxy_uri.scheme}_proxy_pass"]
+        proxy_uri.password || config["#{proxy_uri.scheme}_proxy_pass"]
       end
 
       def configure_ssl(http_client)
diff --git a/lib/chef/provider/remote_file/http.rb b/lib/chef/provider/remote_file/http.rb
index ef2461848d..40f64a0e85 100644
--- a/lib/chef/provider/remote_file/http.rb
+++ b/lib/chef/provider/remote_file/http.rb
@@ -137,7 +137,7 @@ class Chef
           if new_resource.ssl_verify_mode
             opts[:ssl_verify_mode] = new_resource.ssl_verify_mode
           end
-          opts
+          opts.merge(new_resource.http_options)
         end
 
       end
diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb
index 18f182f40c..774ca25af4 100644
--- a/lib/chef/resource/remote_file.rb
+++ b/lib/chef/resource/remote_file.rb
@@ -34,6 +34,36 @@ class Chef
 
       description "Use the **remote_file** resource to transfer a file from a remote location using file specificity. This resource is similar to the **file** resource. Note: Fetching files from the `files/` directory in a cookbook should be done with the **cookbook_file** resource."
 
+      examples <<~DOC
+      **Download a file from an http server**:
+
+      ```ruby
+        remote_file '/tmp/remote.txt' do
+          source 'https://example.org/remote.txt'
+        end
+      ```
+
+      **Set Chef::HTTP options and configure the Net::HTTP object**
+
+      ```ruby
+        remote_file '/tmp/remote.txt' do
+          source 'https://example.org/remote.txt'
+          http_options({
+            http_retry_delay: 0,
+            http_retry_count: 0,
+            keepalives: false,
+            nethttp: {
+              continue_timeout: 5,
+              max_retries: 5,
+              read_timeout: 5,
+              write_timeout: 5,
+              ssl_timeout: 5,
+            },
+          })
+        end
+      ```
+      DOC
+
       def initialize(name, run_context = nil)
         super
         @source = []
@@ -118,6 +148,10 @@ class Chef
 
       property :authentication, Symbol, equal_to: %i{remote local}, default: :remote
 
+      property :http_options, Hash, default: {},
+        introduced: "17.5",
+        description: "A Hash of custom HTTP options. For example: `http_options({ http_retry_count: 0, http_retry_delay: 2 })`"
+
       def after_created
         validate_identity_platform(remote_user, remote_password, remote_domain)
         identity = qualify_user(remote_user, remote_password, remote_domain)
diff --git a/spec/unit/http/basic_client_spec.rb b/spec/unit/http/basic_client_spec.rb
index 0def00a220..57177ceac1 100644
--- a/spec/unit/http/basic_client_spec.rb
+++ b/spec/unit/http/basic_client_spec.rb
@@ -47,6 +47,36 @@ describe "HTTP Connection" do
       expect(Net::HTTP).to receive(:new).and_return(net_http_mock)
       expect(basic_client.http_client).to eql(net_http_mock)
     end
+
+    it "allows setting net-http accessor options" do
+      basic_client = Chef::HTTP::BasicClient.new(uri, nethttp_opts: {
+        "continue_timeout" => 5,
+        "max_retries" => 5,
+        "read_timeout" => 5,
+        "write_timeout" => 5,
+        "ssl_timeout" => 5,
+      })
+      expect(basic_client.http_client.continue_timeout).to eql(5)
+      expect(basic_client.http_client.max_retries).to eql(5)
+      expect(basic_client.http_client.read_timeout).to eql(5)
+      expect(basic_client.http_client.write_timeout).to eql(5)
+      expect(basic_client.http_client.ssl_timeout).to eql(5)
+    end
+
+    it "allows setting net-http accssor options as symbols" do
+      basic_client = Chef::HTTP::BasicClient.new(uri, nethttp_opts: {
+        continue_timeout: 5,
+        max_retries: 5,
+        read_timeout: 5,
+        write_timeout: 5,
+        ssl_timeout: 5,
+      })
+      expect(basic_client.http_client.continue_timeout).to eql(5)
+      expect(basic_client.http_client.max_retries).to eql(5)
+      expect(basic_client.http_client.read_timeout).to eql(5)
+      expect(basic_client.http_client.write_timeout).to eql(5)
+      expect(basic_client.http_client.ssl_timeout).to eql(5)
+    end
   end
 
   describe "#build_http_client" do
diff --git a/spec/unit/http_spec.rb b/spec/unit/http_spec.rb
index bb52db60fd..d7e2488440 100644
--- a/spec/unit/http_spec.rb
+++ b/spec/unit/http_spec.rb
@@ -46,13 +46,19 @@ describe Chef::HTTP do
   describe "#initialize" do
     it "accepts a keepalive option and passes it to the http_client" do
       http = Chef::HTTP.new(uri, keepalives: true)
-      expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, keepalives: true).and_call_original
+      expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, nethttp_opts: {}, keepalives: true).and_call_original
       expect(http.http_client).to be_a_kind_of(Chef::HTTP::BasicClient)
     end
 
     it "the default is not to use keepalives" do
       http = Chef::HTTP.new(uri)
-      expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, keepalives: false).and_call_original
+      expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, nethttp_opts: {}, keepalives: false).and_call_original
+      expect(http.http_client).to be_a_kind_of(Chef::HTTP::BasicClient)
+    end
+
+    it "allows setting the nethttp options hash" do
+      http = Chef::HTTP.new(uri, { nethttp: { "continue_timeout" => 5 } })
+      expect(Chef::HTTP::BasicClient).to receive(:new).with(uri, ssl_policy: Chef::HTTP::APISSLPolicy, nethttp_opts: { "continue_timeout" => 5 }, keepalives: false).and_call_original
       expect(http.http_client).to be_a_kind_of(Chef::HTTP::BasicClient)
     end
   end
diff --git a/spec/unit/provider/remote_file/http_spec.rb b/spec/unit/provider/remote_file/http_spec.rb
index 032cf474ef..8eca721cd0 100644
--- a/spec/unit/provider/remote_file/http_spec.rb
+++ b/spec/unit/provider/remote_file/http_spec.rb
@@ -321,4 +321,14 @@ describe Chef::Provider::RemoteFile::HTTP do
 
   end
 
+  describe "#http_client_opts" do
+    before do
+      new_resource.http_options({ retries: 2, retry_delay: 3 })
+    end
+
+    it "should set http client options" do
+      expect(fetcher.send(:http_client_opts)).to eq({ retries: 2, retry_delay: 3 })
+    end
+  end
+
 end