Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace httpclient with net/http in bosh-monitor #2557

Merged
merged 3 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ group :production do
end

group :bat do
gem 'httpclient'
gem 'minitar'
gem 'net-ssh'
end
Expand Down
3 changes: 0 additions & 3 deletions src/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ PATH
bosh_common (~> 0.0.0)
bosh_cpi
cf-uaa-lib (~> 3.2.1)
httpclient (~> 2.8.3)
logging
membrane (~> 1.1.0)
nats-pure
Expand Down Expand Up @@ -62,7 +61,6 @@ PATH
async-io
cf-uaa-lib (~> 3.2.1)
dogapi (~> 1.45.0)
httpclient (~> 2.8.3)
logging
nats-pure
net-smtp
Expand Down Expand Up @@ -380,7 +378,6 @@ DEPENDENCIES
bundle-audit
factory_bot
fakefs
httpclient
io-stream (<= 0.4.0)
json (~> 2)
minitar
Expand Down
1 change: 0 additions & 1 deletion src/bosh-director/bosh-director.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ Gem::Specification.new do |spec|
spec.add_dependency 'bcrypt', '~>3.1.16'
spec.add_dependency 'bosh_cpi'
spec.add_dependency 'cf-uaa-lib', '~>3.2.1'
spec.add_dependency 'httpclient', '~>2.8.3'
spec.add_dependency 'logging'
spec.add_dependency 'membrane', '~>1.1.0'
spec.add_dependency 'nats-pure'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'net/http'

module Bosh::Director::ConfigServer
class RetryableHTTPClient
def initialize(http_client)
Expand Down Expand Up @@ -25,8 +27,7 @@ def connection_retryable
Errno::ETIMEDOUT,
Errno::ECONNRESET,
::Timeout::Error,
::HTTPClient::TimeoutError,
::HTTPClient::KeepAliveDisconnected,
Net::HTTPRetriableError,
OpenSSL::SSL::SSLError,
]
Bosh::Retryable.new({sleep: 0, tries: 3, on: handled_exceptions})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require 'uaa'
require 'httpclient'
require 'net/http'

module Bosh::Director::ConfigServer
class UAAAuthProvider
Expand Down Expand Up @@ -55,9 +55,8 @@ def retryable
Errno::ETIMEDOUT,
Errno::ECONNRESET,
::Timeout::Error,
::HTTPClient::TimeoutError,
::HTTPClient::KeepAliveDisconnected,
OpenSSL::SSL::SSLError
Net::HTTPRetriableError,
OpenSSL::SSL::SSLError,
]
Bosh::Retryable.new({sleep: 0, tries: 3, on: handled_exceptions})
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'spec_helper'
require 'rack/test'
require 'httpclient'

describe Bosh::Director::ConfigServer::AuthHTTPClient do
subject { Bosh::Director::ConfigServer::AuthHTTPClient.new }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require 'spec_helper'
require 'httpclient'

module Bosh::Director::ConfigServer

describe 'compatibility of Enabled and Disabled versions of ConfigServer HTTP Clients' do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'spec_helper'
require 'httpclient'

describe Bosh::Director::ConfigServer::RetryableHTTPClient do
subject { Bosh::Director::ConfigServer::RetryableHTTPClient.new(http_client) }
Expand All @@ -14,8 +13,7 @@
Errno::ETIMEDOUT,
Errno::ECONNRESET,
Timeout::Error,
HTTPClient::TimeoutError,
HTTPClient::KeepAliveDisconnected,
Net::HTTPRetriableError,
OpenSSL::SSL::SSLError,
]
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'spec_helper'
require 'rack/test'
require 'httpclient'

describe Bosh::Director::ConfigServer::UAAAuthProvider do
include Support::UaaHelpers
Expand Down Expand Up @@ -145,9 +144,8 @@
Errno::ETIMEDOUT,
Errno::ECONNRESET,
Timeout::Error,
HTTPClient::TimeoutError,
HTTPClient::KeepAliveDisconnected,
OpenSSL::SSL::SSLError
Net::HTTPRetriableError,
OpenSSL::SSL::SSLError,
]

retryable = double("Bosh::Retryable")
Expand Down
1 change: 0 additions & 1 deletion src/bosh-monitor/bosh-monitor.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ Gem::Specification.new do |spec|
spec.add_dependency 'dogapi', '~> 1.45.0'
spec.add_dependency 'riemann-client'
spec.add_dependency 'cf-uaa-lib', '~>3.2.1'
spec.add_dependency 'httpclient', '~>2.8.3'

spec.add_development_dependency 'async-rspec'
spec.add_development_dependency 'timecop'
Expand Down
8 changes: 4 additions & 4 deletions src/bosh-monitor/lib/bosh/monitor/plugins/event_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ def auth_provider(director_info)
def director_info
return @director_info if @director_info

director_info_url = @url.dup
director_info_url = @url.dup
director_info_url.path = '/info'
response = send_http_get_request(director_info_url.to_s)
return nil if response.status_code != 200
body, status = send_http_get_request_synchronous(director_info_url.to_s)
return nil if status != 200

@director_info = JSON.parse(response.body)
@director_info = JSON.parse(body)
end
end
end
Expand Down
48 changes: 31 additions & 17 deletions src/bosh-monitor/lib/bosh/monitor/plugins/http_request_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'httpclient'
require 'async/http'
require 'async/http/proxy'
require 'net/http'

module Bosh::Monitor::Plugins
module HttpRequestHelper
Expand All @@ -14,31 +14,45 @@ def send_http_post_request(uri, request)
process_async_http_request(method: :post, uri: uri, headers: request.fetch(:head, {}), body: request.fetch(:body, nil), proxy: request.fetch(:proxy, nil))
end

def send_http_get_request(uri, headers = nil)
def send_http_get_request_synchronous(uri, headers = nil)
parsed_uri = URI.parse(uri.to_s)

# we are interested in response, so send sync request
logger.debug("Sending GET request to #{uri}")
cli = sync_client(OpenSSL::SSL::VERIFY_NONE)
env_proxy = URI.parse(uri.to_s).find_proxy
cli.proxy = env_proxy unless env_proxy.nil?
logger.debug("Sending GET request to #{parsed_uri}")

net_http = sync_client(parsed_uri, OpenSSL::SSL::VERIFY_NONE)

return cli.get(uri) if headers.nil?
response = net_http.get(parsed_uri.request_uri, headers)

cli.get(uri, nil, headers)
[response.body, response.code.to_i]
end

def send_http_post_sync_request(uri, request)
cli = sync_client
env_proxy = URI.parse(uri.to_s).find_proxy
cli.proxy = env_proxy unless env_proxy.nil?
cli.post(uri, request[:body])
def send_http_post_request_synchronous_with_tls_verify_peer(uri, request)
parsed_uri = URI.parse(uri.to_s)

net_http = sync_client(parsed_uri, OpenSSL::SSL::VERIFY_PEER)

response = net_http.post(parsed_uri.request_uri, request[:body])

[response.body, response.code.to_i]
end

private

def sync_client(ssl_verify_mode = OpenSSL::SSL::VERIFY_PEER)
client = HTTPClient.new
client.ssl_config.verify_mode = ssl_verify_mode
client
def sync_client(parsed_uri, ssl_verify_mode)
net_http = Net::HTTP.new(parsed_uri.host, parsed_uri.port)
net_http.use_ssl = (parsed_uri.scheme == 'https')
net_http.verify_mode = ssl_verify_mode

env_proxy = parsed_uri.find_proxy
unless env_proxy.nil?
net_http.proxy_address = env_proxy.host
net_http.proxy_port = env_proxy.port
net_http.proxy_user = env_proxy.user
net_http.proxy_pass = env_proxy.password
end

net_http
end

def process_async_http_request(method:, uri:, headers: {}, body: nil, proxy: nil)
Expand Down
7 changes: 6 additions & 1 deletion src/bosh-monitor/lib/bosh/monitor/plugins/pagerduty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,13 @@ def process(event)

request[:proxy] = options['http_proxy'] if options['http_proxy']

# Note this appears to be the only use of `#send_http_post_request_synchronous_with_tls_verify_peer` from git
# history it appears that the previous asynchronous implementation (EventMachine) was not able to use
# TLS-proxies, so a synchronous call was made from asynchronously.
# This should probably be re-implemented to use the asynchronous `HttpRequestHelper#send_http_post_request`
# method _however_ that method should be updated to use `OpenSSL::SSL::VERIFY_PEER`.
Async do
send_http_post_sync_request(API_URI, request)
send_http_post_request_synchronous_with_tls_verify_peer(API_URI, request)
rescue StandardError => e
logger.error("Error sending pagerduty event: #{e}")
end
Expand Down
14 changes: 7 additions & 7 deletions src/bosh-monitor/lib/bosh/monitor/plugins/resurrector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,15 @@ def scan_and_fix_already_queued_or_processing?(deployment_name)
'Content-Type' => 'application/json',
}
url.query = URI.encode_www_form({ deployment: deployment_name, state: 'queued,processing', verbose: 2 })
tasks_response = send_http_get_request(url.to_s, headers)
body, status = send_http_get_request_synchronous(url.to_s, headers)

# Getting the current tasks may fail. In a situation where the director is already dealing with lots of scan and fix tasks,
# we may want to postpone adding another one to the queue to give the director time to deal with the currently scheduled tasks.
# In the case of the tasks endpoint misbehaving ( status != 200 ) we can safely skip scheduling the the scan and fix in the current iteration.
# The alerts about missing healthchecks will trigger again some time later (when the director is under less pressure).
return true if tasks_response.status != 200
# The alerts about missing health-checks will trigger again some time later (when the director is under less pressure).
return true if status != 200

queued_scan_and_fix = JSON.parse(tasks_response.body).select do |task|
queued_scan_and_fix = JSON.parse(body).select do |task|
task['description'] == 'scan and fix'
end

Expand All @@ -142,10 +142,10 @@ def director_info

url = @uri.dup
url.path = '/info'
response = send_http_get_request(url.to_s)
return nil if response.status_code != 200
body, status = send_http_get_request_synchronous(url.to_s)
return nil if status != 200

@director_info = JSON.parse(response.body)
@director_info = JSON.parse(body)
end

def pretty_str(jobs_to_instances)
Expand Down
Loading