compilecatalog: Switch to new HTTP client implementation #61
Conversation
bastelfreak
force-pushed
the
httpclient
branch
4 times, most recently
from
e557c96 to
2f55c10
Compare
bastelfreak
force-pushed
the
httpclient
branch
3 times, most recently
from
e56e0f0 to
62bfd85
Compare
| port = server_url.port | ||
| use_ssl = port != 8080 | ||
| connection = Puppet::Network::HttpPool.http_instance(server_url.host, port, use_ssl) |
There was a problem hiding this comment.
Previously we checked the port. PuppetDB uses 8080 for HTTP and 8081 for HTTPS. But the config always returns a full URI: https://puppet.com/docs/puppetdb/7/puppetdb_connection.html#server-urls
Instead of hardcoding the Port/protocol here I think we should use what the config file returns.
| @@ -58,7 +58,7 @@ | |||
| "requirements": [ | |||
| { | |||
| "name": "puppet", | |||
| "version_requirement": ">= 4.10.0 < 7.0.0" | |||
| "version_requirement": ">= 6.11.0 < 7.0.0" | |||
There was a problem hiding this comment.
One of the main use cases of catalog-diff is to help people upgrade from old/unsupported versions of puppet. It may be counterproductive to break it for Puppet 4 and 5.
There was a problem hiding this comment.
I updated the README.md to clarify this. Only the system that runs catalog-diff needs to be Puppet 6.11 or newer. You can still get catalogs from ancient Puppetserver/Puppetdb. I think this is fine. Nobody should run this on a box with Puppet Agent 5 or older.
| rescue PSON::ParserError => e | ||
| raise "Error parsing json output of puppet catalog query for #{node_name}: #{e.message}. Content: #{ret}" | ||
| raise "Error parsing json output of puppet catalog query for #{node_name}: #{e.message}. Content: #{ret.body}" |
There was a problem hiding this comment.
Just of a general comment, unrelated to this PR. Puppet switched from PSON to JSON by default in Puppet 5, we'd like to drop PSON at some point (https://tickets.puppetlabs.com/browse/PUP-9648), and PSON is not suitable to encoding rich data (https://tickets.puppetlabs.com/browse/PUP-10928). Alternatively, you could simplify this code to "get a catalog the same way the agent does without using the indirector". Something like:
http = Puppet.runtime[:http]
session = http.create_session
compiler = session.route_to(:compiler)
catalog = compiler.post_catalog(...)Here is documentation about the arguments to post_catalog
Also this is how the agent calls the same method to get a catalog
There is also a post_catalog4
There was a problem hiding this comment.
Thanks for the note. I will keep that in mind for further refactoring.
|
@alexjfisher any objections on merging this? |
The new HTTP client was introduced in puppetlabs/puppet#7826. First released in Puppet 6.11.0. The old HTTP client is deprecated. The new implementation makes it quite easy to configure additional/custom TLS certificates. I plan to implement that in another PR.
|
I think it's ok. |
The new HTTP client was introduced in puppetlabs/puppet#7826. First released in Puppet 6.11.0. The old HTTP client is deprecated. The new implementation makes it quite easy to configure additional/custom TLS certificates. I
plan to implement that in another PR.