diff options
author | Chris Doherty <randomcamel@users.noreply.github.com> | 2016-03-18 11:49:58 -0700 |
---|---|---|
committer | Chris Doherty <randomcamel@users.noreply.github.com> | 2016-03-18 11:49:58 -0700 |
commit | 34db0fd2f2d64e093bb17d86198f1fe1483e2187 (patch) | |
tree | 24a848a7cfc447e6237accae57b2b832a68ac8c4 | |
parent | 543eb9b32a6808f220b1e632f54ca5f01847228d (diff) | |
parent | a5d1c7c4fd0682d274a0d0e1fbfb10bbfa9016ae (diff) | |
download | chef-34db0fd2f2d64e093bb17d86198f1fe1483e2187.tar.gz |
Merge pull request #4348 from chef/nls/proxy-cleanup
Make handling of proxies more consistent
-rw-r--r-- | chef-config/lib/chef-config/config.rb | 22 | ||||
-rw-r--r-- | chef-config/spec/unit/config_spec.rb | 109 | ||||
-rw-r--r-- | lib/chef/http/basic_client.rb | 44 | ||||
-rw-r--r-- | lib/chef/provider/remote_file/ftp.rb | 13 | ||||
-rw-r--r-- | spec/unit/http/basic_client_spec.rb | 89 | ||||
-rw-r--r-- | spec/unit/provider/remote_file/ftp_spec.rb | 4 | ||||
-rw-r--r-- | spec/unit/rest/auth_credentials_spec.rb | 128 |
7 files changed, 210 insertions, 199 deletions
diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb index ec41ccdf8c..9da69a1867 100644 --- a/chef-config/lib/chef-config/config.rb +++ b/chef-config/lib/chef-config/config.rb @@ -846,6 +846,28 @@ module ChefConfig ENV["NO_PROXY"] = value unless ENV["NO_PROXY"] end + # Given a scheme, host, and port, return the correct proxy URI based on the + # set environment variables, unless exluded by no_proxy, in which case nil + # is returned + def self.proxy_uri(scheme, host, port) + proxy_env_var = ENV["#{scheme}_proxy"].to_s.strip + + # Check if the proxy string contains a scheme. If not, add the url's scheme to the + # proxy before parsing. The regex /^.*:\/\// matches, for example, http://. Reusing proxy + # here since we are really just trying to get the string built correctly. + proxy = if !proxy_env_var.empty? + if proxy_env_var =~ /^.*:\/\// + URI.parse(proxy_env_var) + else + URI.parse("#{scheme}://#{proxy_env_var}") + end + end + + excludes = ENV["no_proxy"].to_s.split(/\s*,\s*/).compact + excludes = excludes.map { |exclude| exclude =~ /:\d+$/ ? exclude : "#{exclude}:*" } + return proxy unless excludes.any? { |exclude| File.fnmatch(exclude, "#{host}:#{port}") } + end + # Chef requires an English-language UTF-8 locale to function properly. We attempt # to use the 'locale -a' command and search through a list of preferences until we # find one that we can use. On Ubuntu systems we should find 'C.UTF-8' and be diff --git a/chef-config/spec/unit/config_spec.rb b/chef-config/spec/unit/config_spec.rb index 797fe3a233..3bada755de 100644 --- a/chef-config/spec/unit/config_spec.rb +++ b/chef-config/spec/unit/config_spec.rb @@ -802,6 +802,115 @@ RSpec.describe ChefConfig::Config do end end + describe "proxy_uri" do + subject(:proxy_uri) { described_class.proxy_uri(scheme, host, port) } + let(:env) { {} } + let(:scheme) { "http" } + let(:host) { "test.example.com" } + let(:port) { 8080 } + let(:proxy) { "#{proxy_prefix}#{proxy_host}:#{proxy_port}" } + let(:proxy_prefix) { "http://" } + let(:proxy_host) { "proxy.mycorp.com" } + let(:proxy_port) { 8080 } + + before do + stub_const("ENV", env) + end + + shared_examples_for "a proxy uri" do + it "contains the host" do + expect(proxy_uri.host).to eq(proxy_host) + end + + it "contains the port" do + expect(proxy_uri.port).to eq(proxy_port) + end + end + + context "when the config setting is normalized (does not contain the scheme)" do + include_examples "a proxy uri" do + + let(:proxy_prefix) { "" } + + let(:env) do + { + "#{scheme}_proxy" => proxy, + "no_proxy" => nil, + } + end + end + end + + context "when the proxy is set by the environment" do + include_examples "a proxy uri" do + let(:scheme) { "https" } + let(:env) do + { + "https_proxy" => "https://jane_username:opensesame@proxy.mycorp.com:8080", + } + end + end + end + + context "when an empty proxy is set by the environment" do + let(:env) do + { + "https_proxy" => "", + } + end + + it "does not fail with URI parse exception" do + expect { proxy_uri }.to_not raise_error + end + end + + context "when no_proxy is set" do + context "when no_proxy is the exact host" do + let(:env) do + { + "http_proxy" => proxy, + "no_proxy" => host, + } + end + + it { is_expected.to eq nil } + end + + context "when no_proxy includes the same domain with a wildcard" do + let(:env) do + { + "http_proxy" => proxy, + "no_proxy" => "*.example.com", + } + end + + it { is_expected.to eq nil } + end + + context "when no_proxy is included on a list" do + let(:env) do + { + "http_proxy" => proxy, + "no_proxy" => "chef.io,getchef.com,opscode.com,test.example.com", + } + end + + it { is_expected.to eq nil } + end + + context "when no_proxy is included on a list with wildcards" do + let(:env) do + { + "http_proxy" => proxy, + "no_proxy" => "10.*,*.example.com", + } + end + + it { is_expected.to eq nil } + end + end + end + describe "allowing chefdk configuration outside of chefdk" do it "allows arbitrary settings in the chefdk config context" do diff --git a/lib/chef/http/basic_client.rb b/lib/chef/http/basic_client.rb index 58ae496418..190635f798 100644 --- a/lib/chef/http/basic_client.rb +++ b/lib/chef/http/basic_client.rb @@ -95,26 +95,8 @@ class Chef raise end - #adapted from buildr/lib/buildr/core/transports.rb def proxy_uri - proxy = Chef::Config["#{url.scheme}_proxy"] || - env["#{url.scheme.upcase}_PROXY"] || env["#{url.scheme}_proxy"] - - # Check if the proxy string contains a scheme. If not, add the url's scheme to the - # proxy before parsing. The regex /^.*:\/\// matches, for example, http://. Reusing proxy - # here since we are really just trying to get the string built correctly. - if String === proxy && !proxy.strip.empty? - if proxy =~ /^.*:\/\// - proxy = URI.parse(proxy.strip) - else - proxy = URI.parse("#{url.scheme}://#{proxy.strip}") - end - end - - no_proxy = Chef::Config[:no_proxy] || env["NO_PROXY"] || env["no_proxy"] - excludes = no_proxy.to_s.split(/\s*,\s*/).compact - excludes = excludes.map { |exclude| exclude =~ /:\d+$/ ? exclude : "#{exclude}:*" } - return proxy unless excludes.any? { |exclude| File.fnmatch(exclude, "#{host}:#{port}") } + @proxy_uri ||= Chef::Config.proxy_uri(url.scheme, host, port) end def build_http_client @@ -133,30 +115,22 @@ class Chef Chef::Config end - def env - ENV - end - def http_client_builder - http_proxy = proxy_uri - if http_proxy.nil? + if proxy_uri.nil? Net::HTTP else - Chef::Log.debug("Using #{http_proxy.host}:#{http_proxy.port} for proxy") - user = http_proxy_user(http_proxy) - pass = http_proxy_pass(http_proxy) - Net::HTTP.Proxy(http_proxy.host, http_proxy.port, user, pass) + Chef::Log.debug("Using #{proxy_uri.host}:#{proxy_uri.port} for proxy") + Net::HTTP.Proxy(proxy_uri.host, proxy_uri.port, http_proxy_user(proxy_uri), + http_proxy_pass(proxy_uri)) end end - def http_proxy_user(http_proxy) - http_proxy.user || Chef::Config["#{url.scheme}_proxy_user"] || - env["#{url.scheme.upcase}_PROXY_USER"] || env["#{url.scheme}_proxy_user"] + def http_proxy_user(proxy_uri) + proxy_uri.user || Chef::Config["#{proxy_uri.scheme}_proxy_user"] end - def http_proxy_pass(http_proxy) - http_proxy.password || Chef::Config["#{url.scheme}_proxy_pass"] || - env["#{url.scheme.upcase}_PROXY_PASS"] || env["#{url.scheme}_proxy_pass"] + def http_proxy_pass(proxy_uri) + proxy_uri.password || Chef::Config["#{proxy_uri.scheme}_proxy_pass"] end def configure_ssl(http_client) diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb index b49e84fa69..5935e83301 100644 --- a/lib/chef/provider/remote_file/ftp.rb +++ b/lib/chef/provider/remote_file/ftp.rb @@ -146,19 +146,8 @@ class Chef tempfile end - #adapted from buildr/lib/buildr/core/transports.rb via chef/rest/rest_client.rb def proxy_uri(uri) - proxy = Chef::Config["ftp_proxy"] - proxy = URI.parse(proxy) if String === proxy - if Chef::Config["ftp_proxy_user"] - proxy.user = Chef::Config["ftp_proxy_user"] - end - if Chef::Config["ftp_proxy_pass"] - proxy.password = Chef::Config["ftp_proxy_pass"] - end - excludes = Chef::Config[:no_proxy].to_s.split(/\s*,\s*/).compact - excludes = excludes.map { |exclude| exclude =~ /:\d+$/ ? exclude : "#{exclude}:*" } - return proxy unless excludes.any? { |exclude| File.fnmatch(exclude, "#{host}:#{port}") } + Chef::Config.proxy_uri("ftp", hostname, port) end def parse_path diff --git a/spec/unit/http/basic_client_spec.rb b/spec/unit/http/basic_client_spec.rb index 97356b4887..8cf63d4441 100644 --- a/spec/unit/http/basic_client_spec.rb +++ b/spec/unit/http/basic_client_spec.rb @@ -40,90 +40,13 @@ describe "HTTP Connection" do end describe "#proxy_uri" do - shared_examples_for "a proxy uri" do - let(:proxy_host) { "proxy.mycorp.com" } - let(:proxy_port) { 8080 } - let(:proxy) { "#{proxy_prefix}#{proxy_host}:#{proxy_port}" } + subject(:proxy_uri) { basic_client.proxy_uri } - it "should contain the host" do - proxy_uri = subject.proxy_uri - expect(proxy_uri.host).to eq(proxy_host) - end - - it "should contain the port" do - proxy_uri = subject.proxy_uri - expect(proxy_uri.port).to eq(proxy_port) - end - end - - context "when the config setting is normalized (does not contain the scheme)" do - include_examples "a proxy uri" do - - let(:proxy_prefix) { "" } - - before do - Chef::Config["#{uri.scheme}_proxy"] = proxy - Chef::Config[:no_proxy] = nil - end - - end - end - - context "when the config setting is not normalized (contains the scheme)" do - include_examples "a proxy uri" do - let(:proxy_prefix) { "#{uri.scheme}://" } - - before do - Chef::Config["#{uri.scheme}_proxy"] = proxy - Chef::Config[:no_proxy] = nil - end - - end - end - - context "when the proxy is set by the environment" do - - include_examples "a proxy uri" do - - let(:env) do - { - "https_proxy" => "https://proxy.mycorp.com:8080", - "https_proxy_user" => "jane_username", - "https_proxy_pass" => "opensesame", - } - end - - let(:proxy_uri) { URI.parse(env["https_proxy"]) } - - before do - allow(basic_client).to receive(:env).and_return(env) - end - - it "sets the proxy user" do - expect(basic_client.http_proxy_user(proxy_uri)).to eq("jane_username") - end - - it "sets the proxy pass" do - expect(basic_client.http_proxy_pass(proxy_uri)).to eq("opensesame") - end - end - - end - - context "when an empty proxy is set by the environment" do - let(:env) do - { - "https_proxy" => "", - } - end - - before do - allow(subject).to receive(:env).and_return(env) - end - - it "to not fail with URI parse exception" do - expect { subject.proxy_uri }.to_not raise_error - end + it "uses ChefConfig's proxy_uri method" do + expect(ChefConfig::Config).to receive(:proxy_uri).at_least(:once).with( + uri.scheme, uri.host, uri.port + ) + proxy_uri end end end diff --git a/spec/unit/provider/remote_file/ftp_spec.rb b/spec/unit/provider/remote_file/ftp_spec.rb index c044621ed4..9963c401d2 100644 --- a/spec/unit/provider/remote_file/ftp_spec.rb +++ b/spec/unit/provider/remote_file/ftp_spec.rb @@ -200,9 +200,7 @@ describe Chef::Provider::RemoteFile::FTP do context "and proxying is enabled" do before do - Chef::Config[:ftp_proxy] = "socks5://socks.example.com:5000" - Chef::Config[:ftp_proxy_user] = "bill" - Chef::Config[:ftp_proxy_pass] = "ted" + stub_const("ENV", "ftp_proxy" => "socks5://bill:ted@socks.example.com:5000") end it "fetches the file via the proxy" do diff --git a/spec/unit/rest/auth_credentials_spec.rb b/spec/unit/rest/auth_credentials_spec.rb index 164c0eb235..dcc0f923b4 100644 --- a/spec/unit/rest/auth_credentials_spec.rb +++ b/spec/unit/rest/auth_credentials_spec.rb @@ -94,24 +94,25 @@ describe Chef::REST::AuthCredentials do end describe Chef::REST::RESTRequest do + let(:url) { URI.parse("http://chef.example.com:4000/?q=chef_is_awesome") } + def new_request(method = nil) method ||= :POST - Chef::REST::RESTRequest.new(method, @url, @req_body, @headers) + Chef::REST::RESTRequest.new(method, url, @req_body, @headers) end before do @auth_credentials = Chef::REST::AuthCredentials.new("client-name", CHEF_SPEC_DATA + "/ssl/private_key.pem") - @url = URI.parse("http://chef.example.com:4000/?q=chef_is_awesome") @req_body = '{"json_data":"as_a_string"}' @headers = { "Content-type" => "application/json", "Accept" => "application/json", "Accept-Encoding" => Chef::REST::RESTRequest::ENCODING_GZIP_DEFLATE, "Host" => "chef.example.com:4000" } - @request = Chef::REST::RESTRequest.new(:POST, @url, @req_body, @headers) + @request = Chef::REST::RESTRequest.new(:POST, url, @req_body, @headers) end it "stores the url it was created with" do - expect(@request.url).to eq(@url) + expect(@request.url).to eq(url) end it "stores the HTTP method" do @@ -123,6 +124,10 @@ describe Chef::REST::RESTRequest do end describe "configuring the HTTP request" do + let(:url) do + URI.parse("http://homie:theclown@chef.example.com:4000/?q=chef_is_awesome") + end + it "configures GET requests" do @req_body = nil rest_req = new_request(:GET) @@ -152,7 +157,6 @@ describe Chef::REST::RESTRequest do end it "configures HTTP basic auth" do - @url = URI.parse("http://homie:theclown@chef.example.com:4000/?q=chef_is_awesome") rest_req = new_request(:GET) expect(rest_req.http_request.to_hash["authorization"]).to eq(["Basic aG9taWU6dGhlY2xvd24="]) end @@ -172,23 +176,14 @@ describe Chef::REST::RESTRequest do describe "for proxy" do before do - Chef::Config[:http_proxy] = "http://proxy.example.com:3128" - Chef::Config[:https_proxy] = "http://sproxy.example.com:3129" - Chef::Config[:http_proxy_user] = nil - Chef::Config[:http_proxy_pass] = nil - Chef::Config[:https_proxy_user] = nil - Chef::Config[:https_proxy_pass] = nil - Chef::Config[:no_proxy] = nil - end - - after do - Chef::Config[:http_proxy] = nil - Chef::Config[:https_proxy] = nil - Chef::Config[:http_proxy_user] = nil - Chef::Config[:http_proxy_pass] = nil - Chef::Config[:https_proxy_user] = nil - Chef::Config[:https_proxy_pass] = nil - Chef::Config[:no_proxy] = nil + stub_const("ENV", "http_proxy" => "http://proxy.example.com:3128", + "https_proxy" => "http://sproxy.example.com:3129", + "http_proxy_user" => nil, + "http_proxy_pass" => nil, + "https_proxy_user" => nil, + "https_proxy_pass" => nil, + "no_proxy" => nil + ) end describe "with :no_proxy nil" do @@ -201,20 +196,23 @@ describe Chef::REST::RESTRequest do expect(http_client.proxy_pass).to be_nil end - it "configures the proxy address and port when using https scheme" do - @url.scheme = "https" - http_client = new_request.http_client - expect(http_client.proxy?).to eq(true) - expect(http_client.proxy_address).to eq("sproxy.example.com") - expect(http_client.proxy_port).to eq(3129) - expect(http_client.proxy_user).to be_nil - expect(http_client.proxy_pass).to be_nil + context "when the url has an https scheme" do + let(:url) { URI.parse("https://chef.example.com:4000/?q=chef_is_awesome") } + + it "configures the proxy address and port when using https scheme" do + http_client = new_request.http_client + expect(http_client.proxy?).to eq(true) + expect(http_client.proxy_address).to eq("sproxy.example.com") + expect(http_client.proxy_port).to eq(3129) + expect(http_client.proxy_user).to be_nil + expect(http_client.proxy_pass).to be_nil + end end end describe "with :no_proxy set" do before do - Chef::Config[:no_proxy] = "10.*,*.example.com" + stub_const("ENV", "no_proxy" => "10.*,*.example.com") end it "does not configure the proxy address and port when using http scheme" do @@ -226,26 +224,23 @@ describe Chef::REST::RESTRequest do expect(http_client.proxy_pass).to be_nil end - it "does not configure the proxy address and port when using https scheme" do - @url.scheme = "https" - http_client = new_request.http_client - expect(http_client.proxy?).to eq(false) - expect(http_client.proxy_address).to be_nil - expect(http_client.proxy_port).to be_nil - expect(http_client.proxy_user).to be_nil - expect(http_client.proxy_pass).to be_nil + context "when the url has an https scheme" do + let(:url) { URI.parse("https://chef.example.com:4000/?q=chef_is_awesome") } + + it "does not configure the proxy address and port when using https scheme" do + http_client = new_request.http_client + expect(http_client.proxy?).to eq(false) + expect(http_client.proxy_address).to be_nil + expect(http_client.proxy_port).to be_nil + expect(http_client.proxy_user).to be_nil + expect(http_client.proxy_pass).to be_nil + end end end describe "with :http_proxy_user and :http_proxy_pass set" do before do - Chef::Config[:http_proxy_user] = "homie" - Chef::Config[:http_proxy_pass] = "theclown" - end - - after do - Chef::Config[:http_proxy_user] = nil - Chef::Config[:http_proxy_pass] = nil + stub_const("ENV", "http_proxy" => "http://homie:theclown@proxy.example.com:3128") end it "configures the proxy user and pass when using http scheme" do @@ -255,24 +250,23 @@ describe Chef::REST::RESTRequest do expect(http_client.proxy_pass).to eq("theclown") end - it "does not configure the proxy user and pass when using https scheme" do - @url.scheme = "https" - http_client = new_request.http_client - expect(http_client.proxy?).to eq(true) - expect(http_client.proxy_user).to be_nil - expect(http_client.proxy_pass).to be_nil + context "when the url has an https scheme" do + let(:url) { URI.parse("https://chef.example.com:4000/?q=chef_is_awesome") } + + it "does not configure the proxy user and pass when using https scheme" do + http_client = new_request.http_client + expect(http_client.proxy?).to eq(true) + expect(http_client.proxy_user).to be_nil + expect(http_client.proxy_pass).to be_nil + end end end describe "with :https_proxy_user and :https_proxy_pass set" do before do - Chef::Config[:https_proxy_user] = "homie" - Chef::Config[:https_proxy_pass] = "theclown" - end - - after do - Chef::Config[:https_proxy_user] = nil - Chef::Config[:https_proxy_pass] = nil + stub_const("ENV", "http_proxy" => "http://proxy.example.com:3128", + "https_proxy" => "https://homie:theclown@sproxy.example.com:3129" + ) end it "does not configure the proxy user and pass when using http scheme" do @@ -282,15 +276,17 @@ describe Chef::REST::RESTRequest do expect(http_client.proxy_pass).to be_nil end - it "configures the proxy user and pass when using https scheme" do - @url.scheme = "https" - http_client = new_request.http_client - expect(http_client.proxy?).to eq(true) - expect(http_client.proxy_user).to eq("homie") - expect(http_client.proxy_pass).to eq("theclown") + context "when the url has an https scheme" do + let(:url) { URI.parse("https://chef.example.com:4000/?q=chef_is_awesome") } + + it "configures the proxy user and pass when using https scheme" do + http_client = new_request.http_client + expect(http_client.proxy?).to eq(true) + expect(http_client.proxy_user).to eq("homie") + expect(http_client.proxy_pass).to eq("theclown") + end end end end end - end |