Fixes #39359 - Add shared Faraday transport for Foreman requests#944
Fixes #39359 - Add shared Faraday transport for Foreman requests#944pablomh wants to merge 2 commits into
Conversation
8f6a8ef to
927efcc
Compare
ekohl
left a comment
There was a problem hiding this comment.
What's the advantage of using all of this custom code over plain Faraday? IMHO this makes it complicated to review. Is it an idea to first rewrite it to Faraday and only then add connection pooling?
I see a lot of custom classes and I'd prefer to pass options to Faraday like in https://lostisland.github.io/faraday/#/customization/connection-options.
Then I also know that we have various HTTP "implementations". Perhaps it's also good to do some research on how to unify those. We have those because we didn't include an external dependency, but if you're adding that then it should be considered.
| options[:client_key] = OpenSSL::PKey.read(File.read(ssl_paths[:private_key]), nil) | ||
| end | ||
|
|
||
| apply_future_tls_profile!(options) |
There was a problem hiding this comment.
IMHO this shouldn't be included. We've been steering at using system wide crypto-policies and avoid using application config within Foreman. Smart Proxy isn't quite there today, but application code shouldn't be aware of it if possible.
927efcc to
2ec9012
Compare
Replace the outbound Net::HTTP client path with Faraday while preserving the existing request-building API, CA and client certificate wiring, and configurable read timeout behavior. Add a new foreman_open_timeout setting that defaults to the existing Net::HTTP open timeout when unset. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Quick cleanup/update for review:
The goal here was to keep the diff easier to reason about and make it simpler to split the pooling part later if that is requested. |
2ec9012 to
3340965
Compare
Cache Faraday connections per upstream transport profile so RHSM forwarding can reuse persistent smart-proxy to Foreman connections across request instances and subclasses. Invalidate and rebuild cached connections after transport-level failures, and switch the shared client to the net_http_persistent adapter. Co-authored-by: Cursor <cursoragent@cursor.com>
3340965 to
13b7729
Compare
Summary
Test plan