Skip to content

Commit

Permalink
feat(http): set http options globally
Browse files Browse the repository at this point in the history
New feature allows users to configure their HTTP options with
classic environment variables (e.g. http_proxy, https_proxy,
no_proxy). Additionally, allows users to ignore SSL verification
if uploading certificates for every use case is not practical.

- Fixes #191
- Fixes #192
  • Loading branch information
mefellows committed Feb 25, 2018
1 parent 18816e6 commit cdf36ba
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.3.4
2.4.0
11 changes: 5 additions & 6 deletions lib/pact_broker/badges/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'pact_broker/project_root'
require 'pact_broker/logging'
require 'pact_broker/configuration'
require 'pact_broker/build_http_options'

module PactBroker
module Badges
Expand Down Expand Up @@ -102,12 +103,10 @@ def escape_text text
def do_request(uri)
with_cache uri do
request = Net::HTTP::Get.new(uri)
Net::HTTP.start(uri.hostname, uri.port,
use_ssl: uri.scheme == 'https',
read_timeout: 3,
open_timeout: 1,
ssl_timeout: 1,
continue_timeout: 1) do |http|
options = {read_timeout: 3, open_timeout: 1, ssl_timeout: 1, continue_timeout: 1}
options.merge! PactBroker::BuildHttpOptions.call(uri)

Net::HTTP.start(uri.hostname, uri.port, :ENV, options) do |http|
http.request request
end
end
Expand Down
32 changes: 32 additions & 0 deletions lib/pact_broker/build_http_options.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require 'pact_broker/services'

module PactBroker
class BuildHttpOptions
extend PactBroker::Services

def self.call uri
uri = URI(uri)
options = {}

if uri.scheme == 'https'
options[:use_ssl] = true
options[:cert_store] = cert_store
if disable_ssl_verification?
options[:verify_mode] = OpenSSL::SSL::VERIFY_NONE
else
options[:verify_mode] = OpenSSL::SSL::VERIFY_PEER
end
end
options
end

def self.disable_ssl_verification?
PactBroker.configuration.disable_ssl_verification
end

def self.cert_store
certificate_service.cert_store
end
end
end

5 changes: 4 additions & 1 deletion lib/pact_broker/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class Configuration
:shields_io_base_url,
:check_for_potential_duplicate_pacticipant_names,
:webhook_retry_schedule,
:semver_formats
:semver_formats,
:disable_ssl_verification
]

attr_accessor :log_dir, :database_connection, :auto_migrate_db, :use_hal_browser, :html_pact_renderer
Expand All @@ -28,6 +29,7 @@ class Configuration
attr_accessor :semver_formats
attr_accessor :enable_public_badge_access, :shields_io_base_url
attr_accessor :webhook_retry_schedule
attr_accessor :disable_ssl_verification
attr_writer :logger

def initialize
Expand Down Expand Up @@ -60,6 +62,7 @@ def self.default_configuration
config.semver_formats = ["%M.%m.%p%s%d", "%M.%m", "%M"]
config.webhook_retry_schedule = [10, 60, 120, 300, 600, 1200] #10 sec, 1 min, 2 min, 5 min, 10 min, 20 min => 38 minutes
config.check_for_potential_duplicate_pacticipant_names = true
config.disable_ssl_verification = false
config
end

Expand Down
17 changes: 4 additions & 13 deletions lib/pact_broker/domain/webhook_request.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
require 'pact_broker/build_http_options'
require 'pact_broker/domain/webhook_request_header'
require 'pact_broker/domain/webhook_execution_result'
require 'pact_broker/logging'
require 'pact_broker/messages'
require 'net/http'
require 'pact_broker/webhooks/redact_logs'
require 'pact_broker/api/pact_broker_urls'
require 'pact_broker/services'
require 'pact_broker/build_http_options'

module PactBroker

Expand All @@ -24,7 +25,6 @@ class WebhookRequest

include PactBroker::Logging
include PactBroker::Messages
include PactBroker::Services

attr_accessor :method, :url, :headers, :body, :username, :password, :uuid

Expand Down Expand Up @@ -104,13 +104,8 @@ def build_request uri, pact, execution_logger

def do_request uri, req
logger.info "Making webhook #{uuid} request #{to_s}"
options = {}
if uri.scheme == 'https'
options[:use_ssl] = true
options[:verify_mode] = OpenSSL::SSL::VERIFY_PEER
options[:cert_store] = cert_store
end
Net::HTTP.start(uri.hostname, uri.port, options) do |http|
options = PactBroker::BuildHttpOptions.call(uri)
Net::HTTP.start(uri.hostname, uri.port, :ENV, options) do |http|
http.request req
end
end
Expand Down Expand Up @@ -174,10 +169,6 @@ def gsub_url pact, url
escaped_pact_url = CGI::escape(pact_url)
url.gsub('${pactbroker.pactUrl}', escaped_pact_url)
end

def cert_store
certificate_service.cert_store
end
end
end
end
47 changes: 47 additions & 0 deletions spec/lib/pact_broker/build_http_options_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
require 'spec_helper'
require 'pact_broker/build_http_options'

module PactBroker
describe BuildHttpOptions do

subject { PactBroker::BuildHttpOptions.call(url) }

context "default http options" do
before do
PactBroker.configuration.disable_ssl_verification = false
end

describe "when given an insecure URL" do
let(:url) { 'http://example.org/insecure' }

it "should provide an empty configuration object" do
expect(subject).to eq({})
end

end

describe "when given a secure URL" do
let(:url) { 'https://example.org/secure' }

it "should validate the full certificate chain" do
expect(subject).to include({:use_ssl => true, :verify_mode => 1})
end

end
end

context "disable_ssl_verification is set to true" do
before do
PactBroker.configuration.disable_ssl_verification = true
end

let(:url) { 'https://example.org/secure' }

describe "when given a secure URL" do
it "should not validate certificates" do
expect(subject).to include({:use_ssl => true, :verify_mode => 0})
end
end
end
end
end

0 comments on commit cdf36ba

Please sign in to comment.