Merge pull request openshift#336 from jtharris/bugs/BZ924863

http_proxy ENV variable does not have to set protocol.
adellape · Mar 26, 2013 · ba836ba · ba836ba
2 parents f96aefd + 85755f0
commit ba836ba
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/lib/rhc/rest/client.rb b/lib/rhc/rest/client.rb
@@ -199,6 +199,14 @@ class Client < Base
       # See #api_version_negotiated
       CLIENT_API_VERSIONS = [1.1, 1.2, 1.3, 1.4]
 
+      # Set the http_proxy env variable, read by
+      # HTTPClient, being sure to add the http protocol
+      # if not specified already
+      proxy = ENV['http_proxy'] || ENV['HTTP_PROXY']
+      if proxy && proxy !~ /^(\w+):\/\// then
+        ENV['http_proxy'] = "http://#{proxy}"
+      end
+
       def initialize(*args)
         options = args[0].is_a?(Hash) && args[0] || {}
         @end_point, @debug, @preferred_api_versions =
@@ -241,7 +249,7 @@ def url
       end
 
       def api
-        @api ||= RHC::Rest::Api.new(self, @preferred_api_versions).tap do |api| 
+        @api ||= RHC::Rest::Api.new(self, @preferred_api_versions).tap do |api|
           self.current_api_version = api.api_version_negotiated
         end
       end

diff --git a/spec/rhc/rest_client_spec.rb b/spec/rhc/rest_client_spec.rb
@@ -22,13 +22,34 @@ module RHC
   module Rest
     describe Client do
 
+      it 'should set the proxy protocol if it is missing' do
+        ENV['http_proxy'] = 'foo.bar.com:8081'
+        load 'rhc/rest/client.rb'
+
+        ENV['http_proxy'].should == 'http://foo.bar.com:8081'
+      end
+
+      it 'should not alter the proxy protocol if it is present' do
+        ENV['http_proxy'] = 'http://foo.bar.com:8081'
+        load 'rhc/rest/client.rb'
+
+        ENV['http_proxy'].should == 'http://foo.bar.com:8081'
+      end
+
+      it 'should not affect the proxy protocol if nil' do
+        ENV['http_proxy'] = nil
+        load 'rhc/rest/client.rb'
+
+        ENV['http_proxy'].should be_nil
+      end
+
       let(:endpoint){ mock_href }
       let(:username){ nil }# mock_user }
       let(:password){ nil }# mock_pass }
       let(:use_debug){ false }
       #let(:spec_versions){ nil }
-      let(:client) do 
-        respond_to?(:spec_versions) ? 
+      let(:client) do
+        respond_to?(:spec_versions) ?
           RHC::Rest::Client.new(endpoint, username, password, use_debug, spec_versions) :
           RHC::Rest::Client.new(endpoint, username, password, use_debug)
       end
@@ -262,15 +283,15 @@ module Rest
         end
 
         describe RHC::Rest::Cartridge do
-          subject do 
+          subject do
             described_class.new({
               :name => 'foo',
               :links => mock_response_links([
                 ['GET', 'broker/rest/cartridge', 'get']
               ])}, client)
           end
           context "when several messages are present" do
-            before do 
+            before do
               stub_api_request(:get, 'broker/rest/cartridge', true).
                 with(:query => {:include => :status_messages}).
                 to_return(:body => {
@@ -308,7 +329,7 @@ module Rest
                           :status => 200
                         })
           end
-          it "returns a list of existing cartridges" do 
+          it "returns a list of existing cartridges" do
             carts = client.cartridges
             carts.length.should equal(2)
             (0..1).each do |idx|