Merge pull request #154 from jay7x/unrest

refactor: replace rest-client with net/http

Author: bastelfreak

lib/puppet_blacksmith/forge.rb

@@ -1,7 +1,8 @@
-require 'rest-client'
+require 'net/http'
 require 'json'
 require 'yaml'
 require 'base64'
+require 'uri'
 
 module Blacksmith
   class Forge
@@ -21,7 +22,6 @@ def initialize(username = nil, password = nil, url = nil, forge_type = nil, toke
       self.password = password
       self.token = token
       self.api_key = api_key
-      RestClient.proxy = ENV.fetch('http_proxy', nil)
       load_credentials
       self.url = url unless url.nil?
       if %r{http(s)?://forge.puppetlabs.com}.match?(self.url)
@@ -51,15 +51,32 @@ def push!(name, package = nil, author = nil, version = nil)
     private
 
     def upload(author, name, file)
-      url = http_url(author, name, file)
-      case forge_type
-      when FORGE_TYPE_ARTIFACTORY
-        RestClient::Request.execute(method: :put, url: url, payload: File.new(file, 'rb'), headers: http_headers)
-      else
-        RestClient::Request.execute(method: :post, url: url, payload: { file: File.new(file, 'rb') }, headers: http_headers)
+      target_url = http_url(author, name, file)
+      uri = URI(target_url)
+
+      http = Net::HTTP.new(uri.host, uri.port)
+      http.use_ssl = (uri.scheme == 'https')
+
+      response = File.open(file, 'rb') do |f|
+        if forge_type == FORGE_TYPE_ARTIFACTORY
+          request = Net::HTTP::Put.new(uri.request_uri, http_headers)
+          request.body_stream = f
+        else
+          request = Net::HTTP::Post.new(uri.request_uri, http_headers)
+          request.set_form([
+                             ['file', f, { filename: File.basename(file) }],
+                           ], 'multipart/form-data')
+        end
+        http.request(request)
       end
-    rescue RestClient::Exception => e
-      raise Blacksmith::Error, "Error uploading #{name} to the forge #{url} [#{e.message}]: #{e.response}"
+
+      raise Blacksmith::Error, "Error uploading #{name} to the forge #{target_url} [#{response.code} #{response.message}]: #{response.body}" unless response.is_a?(Net::HTTPSuccess)
+    rescue Net::HTTPClientException => e
+      raise Blacksmith::Error, "Error uploading #{name} to the forge #{target_url} [#{e.response.code} #{e.response.message}]: #{e.response.body}"
+    rescue Net::HTTPFatalError => e
+      raise Blacksmith::Error, "HTTP error during upload: #{e.message}"
+    rescue StandardError => e
+      raise Blacksmith::Error, "Error uploading #{name} to the forge #{target_url} [#{e.class} #{e.message}]"
     end
 
     def http_url(author, name, file)

puppet-blacksmith.gemspec

@@ -18,7 +18,6 @@ Gem::Specification.new do |s|
 
   s.add_dependency 'base64', '>= 0.2', '< 0.4'
   s.add_dependency 'puppet-modulebuilder', '~> 2.0', '>= 2.0.2'
-  s.add_dependency 'rest-client', '~>2.0'
   s.add_development_dependency 'aruba', '~> 2.1'
   s.add_development_dependency 'cucumber', '>= 9', '< 11'
   s.add_development_dependency 'rake', '~> 13.0', '>= 13.0.6'

spec/puppet_blacksmith/forge_shared.rb

@@ -21,8 +21,6 @@
   let(:target) { File.expand_path(File.join(__dir__, '..', '..', 'pkg', module_name)) }
   let(:package) { "#{target}.tar.gz" }
 
-  let(:headers) { { 'User-Agent' => %r{^Blacksmith/#{Blacksmith::VERSION} Ruby/.* \(.*\)$}o } }
-
   def create_tarball
     FileUtils.mkdir_p(target)
 

spec/puppet_blacksmith/forge_spec.rb

@@ -69,21 +69,43 @@
     before { create_tarball }
 
     context 'when using a Forge API key' do
-      before do
-        stub_request(:post, "#{forge}/v3/releases").with(
-          headers: headers.merge({ 'Accept' => '*/*', 'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
-                                   'Authorization' => 'Bearer e52f78b62e97cb8d8db6659a73aa522cca0f5c74d4714e0ed0bdd10000000000', 'Content-Type' => %r{\Amultipart/form-data;}, }),
-        ) do |request|
-          request.body =~ %r{Content-Disposition: form-data; name="file"; filename="maestrodev-test.tar.gz"\r\nContent-Type: application/gzip}
-        end.to_return(status: 200, body: File.read(File.join(spec_data, 'response.json')), headers: {})
+      # WebMock doesn't support `set_form()` yet: https://github.com/bblimke/webmock/issues/959
+      # Disable it and use RSpec mocks instead
+      before { WebMock.disable! }
+      after { WebMock.enable! }
+
+      let(:http) do
+        http = instance_double(Net::HTTP)
+        allow(http).to receive(:use_ssl=)
+        http
+      end
+      let(:response_ok) do
+        resp = instance_double(Net::HTTPOK, body: '{}', code: 200, message: 'OK')
+        allow(resp).to receive(:is_a?).with(Net::HTTPSuccess).and_return(true)
+        resp
       end
 
       it 'push the module' do
         subject.api_key = api_key
         subject.url = forge
 
-        resp = subject.push!(module_name, package)
-        expect(resp.code).to eq(200)
+        allow(Net::HTTP).to receive(:new).and_return(http)
+        # Allow any POST and return OK always
+        allow(http).to receive(:request).with(instance_of(Net::HTTP::Post)).and_return(response_ok)
+
+        subject.push!(module_name, package)
+        # Ensure the connection is to the right host & port
+        expect(Net::HTTP).to have_received(:new).with('forgestagingapi.puppetlabs.com', 443)
+        # Ensure the expected request actually happens
+        expect(http).to have_received(:request).with(
+          satisfy do |req|
+            expect(req).to be_a(Net::HTTP::Post)
+            expect(req.path).to eq('/v3/releases')
+            expect(req['Content-Type']).to match(%r{multipart/form-data})
+            expect(req['Authorization']).to eq("Bearer #{api_key}")
+            expect(req['User-Agent']).to match(%r{^Blacksmith/#{Blacksmith::VERSION} Ruby/.* \(.*\)$}o)
+          end,
+        )
       end
     end
   end