From 8f769453c339dc978328d720c2079076b1799d97 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Sun, 22 Feb 2026 00:04:20 +0000 Subject: [PATCH 1/4] Remove legacy network HTTP APIs The deprecated Puppet::Network::HTTP::Connection class and associated legacy HTTP pool methods have been removed. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Steven Pritchard --- lib/puppet/http.rb | 7 - lib/puppet/network/http/connection.rb | 288 ---------- lib/puppet/network/http/handler.rb | 8 - lib/puppet/network/http_pool.rb | 68 +-- lib/puppet/reports/http.rb | 1 - lib/puppet/runtime.rb | 3 +- spec/integration/network/http_pool_spec.rb | 275 --------- spec/unit/network/http/connection_spec.rb | 636 --------------------- spec/unit/network/http_pool_spec.rb | 99 ---- 9 files changed, 7 insertions(+), 1378 deletions(-) delete mode 100644 lib/puppet/network/http/connection.rb delete mode 100644 spec/integration/network/http_pool_spec.rb delete mode 100644 spec/unit/network/http/connection_spec.rb diff --git a/lib/puppet/http.rb b/lib/puppet/http.rb index df4d4d4e07..f19c0aabe0 100644 --- a/lib/puppet/http.rb +++ b/lib/puppet/http.rb @@ -38,11 +38,4 @@ module HTTP require_relative 'http/retry_after_handler' require_relative 'http/external_client' end - - # Legacy HTTP API - module Network - module HTTP - require_relative '../puppet/network/http_pool' - end - end end diff --git a/lib/puppet/network/http/connection.rb b/lib/puppet/network/http/connection.rb deleted file mode 100644 index 9840cdb878..0000000000 --- a/lib/puppet/network/http/connection.rb +++ /dev/null @@ -1,288 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../../puppet/http' - -# This will be raised if too many redirects happen for a given HTTP request -class Puppet::Network::HTTP::RedirectionLimitExceededException < Puppet::Error; end - -# This class provides simple methods for issuing various types of HTTP -# requests. It's interface is intended to mirror Ruby's Net::HTTP -# object, but it provides a few important bits of additional -# functionality. Notably: -# -# * Any HTTPS requests made using this class will use Puppet's SSL -# certificate configuration for their authentication, and -# * Provides some useful error handling for any SSL errors that occur -# during a request. -# -# @deprecated Use {Puppet.runtime[:http]} -# @api public -class Puppet::Network::HTTP::Connection - include Puppet::HTTP::ResponseConverter - - OPTION_DEFAULTS = { - :use_ssl => true, - :verifier => nil, - :redirect_limit => 10, - } - - # Creates a new HTTP client connection to `host`:`port`. - # @param host [String] the host to which this client will connect to - # @param port [Integer] the port to which this client will connect to - # @param options [Hash] options influencing the properties of the created - # connection, - # @option options [Boolean] :use_ssl true to connect with SSL, false - # otherwise, defaults to true - # @option options [Puppet::SSL::Verifier] :verifier An object that will configure - # any verification to do on the connection - # @option options [Integer] :redirect_limit the number of allowed - # redirections, defaults to 10 passing any other option in the options - # hash results in a Puppet::Error exception - # - # @note the HTTP connection itself happens lazily only when {#request}, or - # one of the {#get}, {#post}, {#delete}, {#head} or {#put} is called - # @note The correct way to obtain a connection is to use one of the factory - # methods on {Puppet::Network::HttpPool} - # @api private - def initialize(host, port, options = {}) - unknown_options = options.keys - OPTION_DEFAULTS.keys - raise Puppet::Error, _("Unrecognized option(s): %{opts}") % { opts: unknown_options.map(&:inspect).sort.join(', ') } unless unknown_options.empty? - - options = OPTION_DEFAULTS.merge(options) - @use_ssl = options[:use_ssl] - if @use_ssl - unless options[:verifier].is_a?(Puppet::SSL::Verifier) - raise ArgumentError, _("Expected an instance of Puppet::SSL::Verifier but was passed a %{klass}") % { klass: options[:verifier].class } - end - - @verifier = options[:verifier] - end - @redirect_limit = options[:redirect_limit] - @site = Puppet::HTTP::Site.new(@use_ssl ? 'https' : 'http', host, port) - @client = Puppet.runtime[:http] - end - - # The address to connect to. - def address - @site.host - end - - # The port to connect to. - def port - @site.port - end - - # Whether to use ssl - def use_ssl? - @site.use_ssl? - end - - # @api private - def verifier - @verifier - end - - # @!macro [new] common_options - # @param options [Hash] options influencing the request made. Any - # options not recognized by this class will be ignored - no error will - # be thrown. - # @option options [Hash{Symbol => String}] :basic_auth The basic auth - # :username and :password to use for the request, :metric_id Ignored - # by this class - used by Puppet Server only. The metric id by which - # to track metrics on requests. - - # @param path [String] - # @param headers [Hash{String => String}] - # @!macro common_options - # @api public - def get(path, headers = {}, options = {}) - headers ||= {} - options[:ssl_context] ||= resolve_ssl_context - options[:redirect_limit] ||= @redirect_limit - - with_error_handling do - to_ruby_response(@client.get(to_url(path), headers: headers, options: options)) - end - end - - # @param path [String] - # @param data [String] - # @param headers [Hash{String => String}] - # @!macro common_options - # @api public - def post(path, data, headers = nil, options = {}) - headers ||= {} - headers['Content-Type'] ||= "application/x-www-form-urlencoded" - data ||= '' - options[:ssl_context] ||= resolve_ssl_context - options[:redirect_limit] ||= @redirect_limit - - with_error_handling do - to_ruby_response(@client.post(to_url(path), data, headers: headers, options: options)) - end - end - - # @param path [String] - # @param headers [Hash{String => String}] - # @!macro common_options - # @api public - def head(path, headers = {}, options = {}) - headers ||= {} - options[:ssl_context] ||= resolve_ssl_context - options[:redirect_limit] ||= @redirect_limit - - with_error_handling do - to_ruby_response(@client.head(to_url(path), headers: headers, options: options)) - end - end - - # @param path [String] - # @param headers [Hash{String => String}] - # @!macro common_options - # @api public - def delete(path, headers = { 'Depth' => 'Infinity' }, options = {}) - headers ||= {} - options[:ssl_context] ||= resolve_ssl_context - options[:redirect_limit] ||= @redirect_limit - - with_error_handling do - to_ruby_response(@client.delete(to_url(path), headers: headers, options: options)) - end - end - - # @param path [String] - # @param data [String] - # @param headers [Hash{String => String}] - # @!macro common_options - # @api public - def put(path, data, headers = nil, options = {}) - headers ||= {} - headers['Content-Type'] ||= "application/x-www-form-urlencoded" - data ||= '' - options[:ssl_context] ||= resolve_ssl_context - options[:redirect_limit] ||= @redirect_limit - - with_error_handling do - to_ruby_response(@client.put(to_url(path), data, headers: headers, options: options)) - end - end - - def request_get(*args, &block) - path, headers = *args - headers ||= {} - options = { - ssl_context: resolve_ssl_context, - redirect_limit: @redirect_limit - } - - ruby_response = nil - @client.get(to_url(path), headers: headers, options: options) do |response| - ruby_response = to_ruby_response(response) - yield ruby_response if block_given? - end - ruby_response - end - - def request_head(*args, &block) - path, headers = *args - headers ||= {} - options = { - ssl_context: resolve_ssl_context, - redirect_limit: @redirect_limit - } - - response = @client.head(to_url(path), headers: headers, options: options) - ruby_response = to_ruby_response(response) - yield ruby_response if block_given? - ruby_response - end - - def request_post(*args, &block) - path, data, headers = *args - headers ||= {} - headers['Content-Type'] ||= "application/x-www-form-urlencoded" - options = { - ssl_context: resolve_ssl_context, - redirect_limit: @redirect_limit - } - - ruby_response = nil - @client.post(to_url(path), data, headers: headers, options: options) do |response| - ruby_response = to_ruby_response(response) - yield ruby_response if block_given? - end - ruby_response - end - - private - - # Resolve the ssl_context based on the verifier associated with this - # connection or load the available set of certs and key on disk. - # Don't try to bootstrap the agent, as we only want that to be triggered - # when running `puppet ssl` or `puppet agent`. - def resolve_ssl_context - # don't need an ssl context for http connections - return nil unless @site.use_ssl? - - # if our verifier has an ssl_context, use that - ctx = @verifier.ssl_context - return ctx if ctx - - # load available certs - cert = Puppet::X509::CertProvider.new - ssl = Puppet::SSL::SSLProvider.new - begin - password = cert.load_private_key_password - ssl.load_context(certname: Puppet[:certname], password: password) - rescue Puppet::SSL::SSLError => e - Puppet.log_exception(e) - - # if we don't have cacerts, then create a root context that doesn't - # trust anything. The old code used to fallback to VERIFY_NONE, - # which we don't want to emulate. - ssl.create_root_context(cacerts: []) - end - end - - def to_url(path) - if path =~ %r{^https?://} - # The old Connection class accepts a URL as the request path, and sends - # it in "absolute-form" in the request line, e.g. GET https://puppet:8140/. - # See https://httpwg.org/specs/rfc7230.html#absolute-form. It just so happens - # to work because HTTP 1.1 servers are required to accept absolute-form even - # though clients are only supposed to send them to proxies, so the proxy knows - # what upstream server to CONNECT to. This method creates a URL using the - # scheme/host/port that the connection was created with, and appends the path - # and query portions of the absolute-form. The resulting request will use "origin-form" - # as it should have done all along. - abs_form = URI(path) - url = URI("#{@site.addr}/#{normalize_path(abs_form.path)}") - url.query = abs_form.query if abs_form.query - url - else - URI("#{@site.addr}/#{normalize_path(path)}") - end - end - - def normalize_path(path) - if path[0] == '/' - path[1..] - else - path - end - end - - def with_error_handling(&block) - yield - rescue Puppet::HTTP::TooManyRedirects => e - raise Puppet::Network::HTTP::RedirectionLimitExceededException.new(_("Too many HTTP redirections for %{host}:%{port}") % { host: @host, port: @port }, e) - rescue Puppet::HTTP::HTTPError => e - Puppet.log_exception(e, e.message) - case e.cause - when Net::OpenTimeout, Net::ReadTimeout, Net::HTTPError, EOFError - raise e.cause - else - raise e - end - end -end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 1f5cd18019..3ace259f5d 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -37,14 +37,6 @@ def headers(request) raise NotImplementedError end - # The mime type is always passed to the `set_content_type` method, so - # it is no longer necessary to retrieve the Format's mime type. - # - # @deprecated - def format_to_mime(format) - format.is_a?(Puppet::Network::Format) ? format.mime : format - end - # Create a generic puppet request from the implementation-specific request # created by the web server def make_generic_request(request) diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb index 768e11ea07..76d6192fa4 100644 --- a/lib/puppet/network/http_pool.rb +++ b/lib/puppet/network/http_pool.rb @@ -1,16 +1,14 @@ # frozen_string_literal: true -require_relative '../../puppet/network/http/connection' - module Puppet::Network; end -# This module is deprecated. -# -# @api public -# @deprecated Use {Puppet::HTTP::Client} instead. +# Allows external HTTP client implementations (e.g., from Puppet Server) to be +# registered for use at runtime. If no custom class is set, the default +# Puppet::HTTP::Client is used. # +# @api private module Puppet::Network::HttpPool - @http_client_class = Puppet::Network::HTTP::Connection + @http_client_class = nil def self.http_client_class @http_client_class @@ -19,60 +17,4 @@ def self.http_client_class def self.http_client_class=(klass) @http_client_class = klass end - - # Retrieve a connection for the given host and port. - # - # @param host [String] The hostname to connect to - # @param port [Integer] The port on the host to connect to - # @param use_ssl [Boolean] Whether to use an SSL connection - # @param verify_peer [Boolean] Whether to verify the peer credentials, if possible. Verification will not take place if the CA certificate is missing. - # @return [Puppet::Network::HTTP::Connection] - # - # @deprecated Use {Puppet.runtime[:http]} instead. - # @api public - # - def self.http_instance(host, port, use_ssl = true, verify_peer = true) - Puppet.warn_once('deprecations', self, "The method 'Puppet::Network::HttpPool.http_instance' is deprecated. Use Puppet.runtime[:http] instead") - - if verify_peer - verifier = Puppet::SSL::Verifier.new(host, nil) - else - ssl = Puppet::SSL::SSLProvider.new - verifier = Puppet::SSL::Verifier.new(host, ssl.create_insecure_context) - end - http_client_class.new(host, port, use_ssl: use_ssl, verifier: verifier) - end - - # Retrieve a connection for the given host and port. - # - # @param host [String] The host to connect to - # @param port [Integer] The port to connect to - # @param use_ssl [Boolean] Whether to use SSL, defaults to `true`. - # @param ssl_context [Puppet::SSL:SSLContext, nil] The ssl context to use - # when making HTTPS connections. Required when `use_ssl` is `true`. - # @return [Puppet::Network::HTTP::Connection] - # - # @deprecated Use {Puppet.runtime[:http]} instead. - # @api public - # - def self.connection(host, port, use_ssl: true, ssl_context: nil) - Puppet.warn_once('deprecations', self, "The method 'Puppet::Network::HttpPool.connection' is deprecated. Use Puppet.runtime[:http] instead") - - if use_ssl - unless ssl_context - # TRANSLATORS 'ssl_context' is an argument and should not be translated - raise ArgumentError, _("An ssl_context is required when connecting to 'https://%{host}:%{port}'") % { host: host, port: port } - end - - verifier = Puppet::SSL::Verifier.new(host, ssl_context) - http_client_class.new(host, port, use_ssl: true, verifier: verifier) - else - if ssl_context - # TRANSLATORS 'ssl_context' is an argument and should not be translated - Puppet.warning(_("An ssl_context is unnecessary when connecting to 'http://%{host}:%{port}' and will be ignored") % { host: host, port: port }) - end - - http_client_class.new(host, port, use_ssl: false) - end - end end diff --git a/lib/puppet/reports/http.rb b/lib/puppet/reports/http.rb index 27fcc6af75..b59ecad281 100644 --- a/lib/puppet/reports/http.rb +++ b/lib/puppet/reports/http.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require_relative '../../puppet' -require_relative '../../puppet/network/http_pool' require 'uri' Puppet::Reports.register_report(:http) do diff --git a/lib/puppet/runtime.rb b/lib/puppet/runtime.rb index cd03f2692a..a95d0ba88e 100644 --- a/lib/puppet/runtime.rb +++ b/lib/puppet/runtime.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative '../puppet/http' +require_relative '../puppet/network/http_pool' require_relative '../puppet/facter_impl' require 'singleton' @@ -14,7 +15,7 @@ def initialize @runtime_services = { http: proc do klass = Puppet::Network::HttpPool.http_client_class - if klass == Puppet::Network::HTTP::Connection + if klass.nil? Puppet::HTTP::Client.new else Puppet::HTTP::ExternalClient.new(klass) diff --git a/spec/integration/network/http_pool_spec.rb b/spec/integration/network/http_pool_spec.rb deleted file mode 100644 index ee6f8a5de3..0000000000 --- a/spec/integration/network/http_pool_spec.rb +++ /dev/null @@ -1,275 +0,0 @@ -require 'spec_helper' -require 'puppet_spec/https' -require 'puppet_spec/files' -require 'puppet/network/http_pool' - -describe Puppet::Network::HttpPool, unless: Puppet::Util::Platform.jruby? do - include PuppetSpec::Files - - before :all do - WebMock.disable! - end - - after :all do - WebMock.enable! - end - - before :each do - # make sure we don't take too long - Puppet[:http_connect_timeout] = '5s' - end - - let(:hostname) { '127.0.0.1' } - let(:wrong_hostname) { 'localhost' } - let(:server) { PuppetSpec::HTTPSServer.new } - - context "when calling deprecated HttpPool methods" do - before(:each) do - ssldir = tmpdir('http_pool') - Puppet[:ssldir] = ssldir - Puppet.settings.use(:main, :ssl) - - File.write(Puppet[:localcacert], server.ca_cert.to_pem) - File.write(Puppet[:hostcrl], server.ca_crl.to_pem) - File.write(Puppet[:hostcert], server.server_cert.to_pem) - File.write(Puppet[:hostprivkey], server.server_key.to_pem) - end - - def connection(host, port) - Puppet::Network::HttpPool.http_instance(host, port, use_ssl: true) - end - - shared_examples_for 'HTTPS client' do - it "connects over SSL" do - server.start_server do |port| - http = connection(hostname, port) - res = http.get('/') - expect(res.code).to eq('200') - end - end - - it "raises if the server's cert doesn't match the hostname we connected to" do - server.start_server do |port| - http = connection(wrong_hostname, port) - expect { - http.get('/') - }.to raise_error { |err| - expect(err).to be_instance_of(Puppet::SSL::CertMismatchError) - expect(err.message).to match(/\AServer hostname '#{wrong_hostname}' did not match server certificate; expected one of (.+)/) - - md = err.message.match(/expected one of (.+)/) - expect(md[1].split(', ')).to contain_exactly('127.0.0.1', 'DNS:127.0.0.1', 'DNS:127.0.0.2') - } - end - end - - it "raises if the server's CA is unknown" do - # File must exist and by not empty so DefaultValidator doesn't - # downgrade to VERIFY_NONE, so use a different CA that didn't - # issue the server's cert - capath = tmpfile('empty') - File.write(capath, cert_fixture('netlock-arany-utf8.pem')) - Puppet[:localcacert] = capath - Puppet[:certificate_revocation] = false - - server.start_server do |port| - http = connection(hostname, port) - expect { - http.get('/') - }.to raise_error(Puppet::Error, - %r{certificate verify failed.* .self.signed certificate in certificate chain for CN=Test CA.}) - end - end - - it "detects when the server has closed the connection and reconnects" do - server.start_server do |port| - http = connection(hostname, port) - - expect(http.request_get('/')).to be_a(Net::HTTPSuccess) - expect(http.request_get('/')).to be_a(Net::HTTPSuccess) - end - end - end - - context "when using persistent HTTPS connections" do - around :each do |example| - begin - example.run - ensure - Puppet.runtime[:http].close - end - end - - include_examples 'HTTPS client' - end - - shared_examples_for "an HttpPool connection" do |klass, legacy_api| - before :each do - Puppet::Network::HttpPool.http_client_class = klass - end - - it "connects using the scheme, host and port from the http instance preserving the URL path and query" do - request_line = nil - - response_proc = -> (req, res) { - request_line = req.request_line - } - - server.start_server(response_proc: response_proc) do |port| - http = Puppet::Network::HttpPool.http_instance(hostname, port, true) - path = "http://bogus.example.com:443/foo?q=a" - http.get(path) - - if legacy_api - # The old API uses 'absolute-form' and passes the bogus hostname - # which isn't the host we connected to. - expect(request_line).to eq("GET http://bogus.example.com:443/foo?q=a HTTP/1.1\r\n") - else - expect(request_line).to eq("GET /foo?q=a HTTP/1.1\r\n") - end - end - end - - it "requires the caller to URL encode the path and query when using absolute form" do - request_line = nil - - response_proc = -> (req, res) { - request_line = req.request_line - } - - server.start_server(response_proc: response_proc) do |port| - http = Puppet::Network::HttpPool.http_instance(hostname, port, true) - params = { 'key' => 'a value' } - encoded_url = "https://#{hostname}:#{port}/foo%20bar?q=#{Puppet::Util.uri_query_encode(params.to_json)}" - http.get(encoded_url) - - if legacy_api - expect(request_line).to eq("GET #{encoded_url} HTTP/1.1\r\n") - else - expect(request_line).to eq("GET /foo%20bar?q=%7B%22key%22%3A%22a%20value%22%7D HTTP/1.1\r\n") - end - end - end - - it "requires the caller to URL encode the path and query when using a path" do - request_line = nil - - response_proc = -> (req, res) { - request_line = req.request_line - } - - server.start_server(response_proc: response_proc) do |port| - http = Puppet::Network::HttpPool.http_instance(hostname, port, true) - params = { 'key' => 'a value' } - http.get("/foo%20bar?q=#{Puppet::Util.uri_query_encode(params.to_json)}") - - expect(request_line).to eq("GET /foo%20bar?q=%7B%22key%22%3A%22a%20value%22%7D HTTP/1.1\r\n") - end - end - end - - describe Puppet::Network::HTTP::Connection do - it_behaves_like "an HttpPool connection", described_class, false - end - end - - context "when calling HttpPool.connection method" do - let(:ssl) { Puppet::SSL::SSLProvider.new } - let(:ssl_context) { ssl.create_root_context(cacerts: [server.ca_cert], crls: [server.ca_crl]) } - - def connection(host, port, ssl_context:) - Puppet::Network::HttpPool.connection(host, port, ssl_context: ssl_context) - end - - # Configure the server's SSLContext to require a client certificate. The `client_ca` - # setting allows the server to advertise which client CAs it will accept. - def require_client_certs(ctx) - ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER|OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT - ctx.client_ca = [cert_fixture('ca.pem')] - end - - it "connects over SSL" do - server.start_server do |port| - http = connection(hostname, port, ssl_context: ssl_context) - res = http.get('/') - expect(res.code).to eq('200') - end - end - - it "raises if the server's cert doesn't match the hostname we connected to" do - server.start_server do |port| - http = connection(wrong_hostname, port, ssl_context: ssl_context) - expect { - http.get('/') - }.to raise_error { |err| - expect(err).to be_instance_of(Puppet::SSL::CertMismatchError) - expect(err.message).to match(/\AServer hostname '#{wrong_hostname}' did not match server certificate; expected one of (.+)/) - - md = err.message.match(/expected one of (.+)/) - expect(md[1].split(', ')).to contain_exactly('127.0.0.1', 'DNS:127.0.0.1', 'DNS:127.0.0.2') - } - end - end - - it "raises if the server's CA is unknown" do - server.start_server do |port| - ssl_context = ssl.create_root_context(cacerts: [cert_fixture('netlock-arany-utf8.pem')], - crls: [server.ca_crl]) - http = Puppet::Network::HttpPool.connection(hostname, port, ssl_context: ssl_context) - expect { - http.get('/') - }.to raise_error(Puppet::Error, - %r{certificate verify failed.* .self.signed certificate in certificate chain for CN=Test CA.}) - end - end - - it "warns when client has an incomplete client cert chain" do - expect(Puppet).to receive(:warning).with("The issuer 'CN=Test CA Agent Subauthority' of certificate 'CN=pluto' cannot be found locally") - - pluto = cert_fixture('pluto.pem') - - ssl_context = ssl.create_context( - cacerts: [server.ca_cert], crls: [server.ca_crl], - client_cert: pluto, private_key: key_fixture('pluto-key.pem') - ) - - # verify client has incomplete chain - expect(ssl_context.client_chain.map(&:to_der)).to eq([pluto.to_der]) - - # force server to require (not request) client certs - ctx_proc = -> (ctx) { - require_client_certs(ctx) - - # server needs to trust the client's intermediate CA to complete the client's chain - ctx.cert_store.add_cert(cert_fixture('intermediate-agent.pem')) - } - - server.start_server(ctx_proc: ctx_proc) do |port| - http = Puppet::Network::HttpPool.connection(hostname, port, ssl_context: ssl_context) - res = http.get('/') - expect(res.code).to eq('200') - end - end - - it "sends a complete client cert chain" do - pluto = cert_fixture('pluto.pem') - client_ca = cert_fixture('intermediate-agent.pem') - - ssl_context = ssl.create_context( - cacerts: [server.ca_cert, client_ca], - crls: [server.ca_crl, crl_fixture('intermediate-agent-crl.pem')], - client_cert: pluto, - private_key: key_fixture('pluto-key.pem') - ) - - # verify client has complete chain from leaf to root - expect(ssl_context.client_chain.map(&:to_der)).to eq([pluto, client_ca, server.ca_cert].map(&:to_der)) - - server.start_server(ctx_proc: method(:require_client_certs)) do |port| - http = Puppet::Network::HttpPool.connection(hostname, port, ssl_context: ssl_context) - res = http.get('/') - expect(res.code).to eq('200') - end - end - end -end diff --git a/spec/unit/network/http/connection_spec.rb b/spec/unit/network/http/connection_spec.rb deleted file mode 100644 index 2eb6fb9a5c..0000000000 --- a/spec/unit/network/http/connection_spec.rb +++ /dev/null @@ -1,636 +0,0 @@ -require 'spec_helper' -require 'puppet/network/http/connection' -require 'puppet/test_ca' - -describe Puppet::Network::HTTP::Connection do - let(:host) { "me.example.com" } - let(:port) { 8140 } - let(:path) { '/foo' } - let(:url) { "https://#{host}:#{port}#{path}" } - let(:params) { { 'key' => 'a value' } } - let(:encoded_url_with_params) { "#{url}?%7B%22key%22:%22a%20value%22%7D" } - let(:ssl_context) { Puppet::SSL::SSLProvider.new.create_system_context(cacerts: []) } - let(:verifier) { Puppet::SSL::Verifier.new(host, ssl_context) } - - shared_examples_for "an HTTP connection" do |klass| - subject { klass.new(host, port, :verifier => verifier) } - - context "when providing HTTP connections" do - context "when initializing http instances" do - it "should return an http instance created with the passed host and port" do - conn = klass.new(host, port, :verifier => verifier) - - expect(conn.address).to eq(host) - expect(conn.port).to eq(port) - end - - it "should enable ssl on the http instance by default" do - conn = klass.new(host, port, :verifier => verifier) - - expect(conn).to be_use_ssl - end - - it "can disable ssl using an option and ignore the verify" do - conn = klass.new(host, port, :use_ssl => false) - - expect(conn).to_not be_use_ssl - end - - it "can enable ssl using an option" do - conn = klass.new(host, port, :use_ssl => true, :verifier => verifier) - - expect(conn).to be_use_ssl - end - - it "ignores the ':verify' option when ssl is disabled" do - conn = klass.new(host, port, :use_ssl => false, :verifier => verifier) - - expect(conn.verifier).to be_nil - end - - it "wraps the validator in an adapter" do - conn = klass.new(host, port, :verifier => verifier) - - expect(conn.verifier).to be_a(Puppet::SSL::Verifier) - end - - it "should raise Puppet::Error when invalid options are specified" do - expect { klass.new(host, port, :invalid_option => nil) }.to raise_error(Puppet::Error, 'Unrecognized option(s): :invalid_option') - end - - it "accepts a verifier" do - verifier = Puppet::SSL::Verifier.new(host, double('ssl_context')) - conn = klass.new(host, port, :use_ssl => true, :verifier => verifier) - - expect(conn.verifier).to eq(verifier) - end - - it "raises if the wrong verifier class is specified" do - expect { - klass.new(host, port, :verifier => Object.new) - }.to raise_error(ArgumentError, - "Expected an instance of Puppet::SSL::Verifier but was passed a Object") - end - end - end - - context "for streaming GET requests" do - it 'yields the response' do - stub_request(:get, url) - - expect { |b| - subject.request_get('/foo', {}, &b) - }.to yield_with_args(Net::HTTPResponse) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:get, encoded_url_with_params) - - subject.request_get("#{path}?#{params.to_json}") { |_| } - end - - it "merges custom headers with default ones" do - stub_request(:get, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.request_get(path, {'X-Foo' => 'Bar'}) { |_| } - end - - it "returns the response" do - stub_request(:get, url) - - response = subject.request_get(path) { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:get, url_with_query) - - response = subject.request_get(url_with_query) { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for streaming head requests" do - it 'yields the response when request_head is called' do - stub_request(:head, url) - - expect { |b| - subject.request_head('/foo', {}, &b) - }.to yield_with_args(Net::HTTPResponse) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:head, encoded_url_with_params) - - subject.request_head("#{path}?#{params.to_json}") { |_| } - end - - it "merges custom headers with default ones" do - stub_request(:head, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.request_head(path, {'X-Foo' => 'Bar'}) { |_| } - end - - it "returns the response" do - stub_request(:head, url) - - response = subject.request_head(path) { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:head, url_with_query) - - response = subject.request_head(url_with_query) { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for streaming post requests" do - it 'yields the response when request_post is called' do - stub_request(:post, url) - - expect { |b| - subject.request_post('/foo', "param: value", &b) - }.to yield_with_args(Net::HTTPResponse) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:post, encoded_url_with_params) - - subject.request_post("#{path}?#{params.to_json}", "") { |_| } - end - - it "merges custom headers with default ones" do - stub_request(:post, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.request_post(path, "", {'X-Foo' => 'Bar'}) { |_| } - end - - it "returns the response" do - stub_request(:post, url) - - response = subject.request_post(path, "") { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:post, url_with_query) - - response = subject.request_post(url_with_query, "") { |_| } - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for GET requests" do - it "includes default HTTP headers" do - stub_request(:get, url).with(headers: {'User-Agent' => /./}) - - subject.get(path) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:get, encoded_url_with_params) - - subject.get("#{path}?#{params.to_json}") - end - - it "merges custom headers with default ones" do - stub_request(:get, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.get(path, {'X-Foo' => 'Bar'}) - end - - it "returns the response" do - stub_request(:get, url) - - response = subject.get(path) - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "returns the entire response body" do - stub_request(:get, url).to_return(body: "abc") - - response = subject.get(path) - expect(response.body).to eq("abc") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:get, url_with_query) - - response = subject.get(url_with_query) - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for HEAD requests" do - it "includes default HTTP headers" do - stub_request(:head, url).with(headers: {'User-Agent' => /./}) - - subject.head(path) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:head, encoded_url_with_params) - - subject.head("#{path}?#{params.to_json}") - end - - it "merges custom headers with default ones" do - stub_request(:head, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.head(path, {'X-Foo' => 'Bar'}) - end - - it "returns the response" do - stub_request(:head, url) - - response = subject.head(path) - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:head, url_with_query) - - response = subject.head(url_with_query) - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for PUT requests" do - it "includes default HTTP headers" do - stub_request(:put, url).with(headers: {'User-Agent' => /./}) - - subject.put(path, "", {'Content-Type' => 'text/plain'}) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:put, encoded_url_with_params) - - subject.put("#{path}?#{params.to_json}", "") - end - - it "includes custom headers" do - stub_request(:put, url).with(headers: { 'X-Foo' => 'Bar' }) - - subject.put(path, "", {'X-Foo' => 'Bar', 'Content-Type' => 'text/plain'}) - end - - it "returns the response" do - stub_request(:put, url) - - response = subject.put(path, "", {'Content-Type' => 'text/plain'}) - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "sets content-type for the body" do - stub_request(:put, url).with(headers: {"Content-Type" => "text/plain"}) - - subject.put(path, "hello", {'Content-Type' => 'text/plain'}) - end - - it 'sends an empty body' do - stub_request(:put, url).with(body: '') - - subject.put(path, nil) - end - - it 'defaults content-type to application/x-www-form-urlencoded' do - stub_request(:put, url).with(headers: {'Content-Type' => 'application/x-www-form-urlencoded'}) - - subject.put(path, '') - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:put, url_with_query) - - response = subject.put(url_with_query, '') - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for POST requests" do - it "includes default HTTP headers" do - stub_request(:post, url).with(headers: {'User-Agent' => /./}) - - subject.post(path, "", {'Content-Type' => 'text/plain'}) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:post, encoded_url_with_params) - - subject.post("#{path}?#{params.to_json}", "", {'Content-Type' => 'text/plain'}) - end - - it "includes custom headers" do - stub_request(:post, url).with(headers: { 'X-Foo' => 'Bar' }) - - subject.post(path, "", {'X-Foo' => 'Bar', 'Content-Type' => 'text/plain'}) - end - - it "returns the response" do - stub_request(:post, url) - - response = subject.post(path, "", {'Content-Type' => 'text/plain'}) - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "sets content-type for the body" do - stub_request(:post, url).with(headers: {"Content-Type" => "text/plain"}) - - subject.post(path, "hello", {'Content-Type' => 'text/plain'}) - end - - it 'sends an empty body' do - stub_request(:post, url).with(body: '') - - subject.post(path, nil) - end - - it 'defaults content-type to application/x-www-form-urlencoded' do - stub_request(:post, url).with(headers: {'Content-Type' => 'application/x-www-form-urlencoded'}) - - subject.post(path, "") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:post, url_with_query) - - response = subject.post(url_with_query, '') - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "for DELETE requests" do - it "includes default HTTP headers" do - stub_request(:delete, url).with(headers: {'User-Agent' => /./}) - - subject.delete(path) - end - - it "merges custom headers with default ones" do - stub_request(:delete, url).with(headers: { 'X-Foo' => 'Bar', 'User-Agent' => /./ }) - - subject.delete(path, {'X-Foo' => 'Bar'}) - end - - it "stringifies keys and encodes values in the query" do - stub_request(:delete, encoded_url_with_params) - - subject.delete("#{path}?#{params.to_json}") - end - - it "returns the response" do - stub_request(:delete, url) - - response = subject.delete(path) - expect(response).to be_an_instance_of(Net::HTTPOK) - expect(response.code).to eq("200") - end - - it "returns the entire response body" do - stub_request(:delete, url).to_return(body: "abc") - - expect(subject.delete(path).body).to eq("abc") - end - - it "accepts a URL string as the path" do - url_with_query = "#{url}?foo=bar" - stub_request(:delete, url_with_query) - - response = subject.delete(url_with_query) - expect(response).to be_an_instance_of(Net::HTTPOK) - end - end - - context "when response is a redirect" do - subject { klass } - - def create_connection(options = {}) - options[:use_ssl] = false - options[:verifier] = verifier - subject.new(host, port, options) - end - - def redirect_to(url) - { status: 302, headers: { 'Location' => url } } - end - - it "should follow the redirect to the final resource location" do - stub_request(:get, "http://me.example.com:8140/foo").to_return(redirect_to("http://me.example.com:8140/bar")) - stub_request(:get, "http://me.example.com:8140/bar").to_return(status: 200) - - create_connection.get('/foo') - end - - def expects_limit_exceeded(conn) - expect { - conn.get('/') - }.to raise_error(Puppet::Network::HTTP::RedirectionLimitExceededException) - end - - it "should not follow any redirects when the limit is 0" do - stub_request(:get, "http://me.example.com:8140/").to_return(redirect_to("http://me.example.com:8140/foo")) - - conn = create_connection(:redirect_limit => 0) - expects_limit_exceeded(conn) - end - - it "should follow the redirect once" do - stub_request(:get, "http://me.example.com:8140/").to_return(redirect_to("http://me.example.com:8140/foo")) - stub_request(:get, "http://me.example.com:8140/foo").to_return(redirect_to("http://me.example.com:8140/bar")) - - conn = create_connection(:redirect_limit => 1) - expects_limit_exceeded(conn) - end - - it "should raise an exception when the redirect limit is exceeded" do - stub_request(:get, "http://me.example.com:8140/").to_return(redirect_to("http://me.example.com:8140/foo")) - stub_request(:get, "http://me.example.com:8140/foo").to_return(redirect_to("http://me.example.com:8140/bar")) - stub_request(:get, "http://me.example.com:8140/bar").to_return(redirect_to("http://me.example.com:8140/baz")) - stub_request(:get, "http://me.example.com:8140/baz").to_return(redirect_to("http://me.example.com:8140/qux")) - - conn = create_connection(:redirect_limit => 3) - expects_limit_exceeded(conn) - end - - it 'raises an exception when the location header is missing' do - stub_request(:get, "http://me.example.com:8140/").to_return(status: 302) - - expect { - create_connection.get('/') - }.to raise_error(Puppet::HTTP::ProtocolError, /Location response header is missing/) - end - end - - context "when response indicates an overloaded server" do - def retry_after(datetime) - stub_request(:get, url) - .to_return(status: [503, 'Service Unavailable'], headers: {'Retry-After' => datetime}).then - .to_return(status: 200) - end - - it "should return a 503 response if Retry-After is not set" do - stub_request(:get, url).to_return(status: [503, 'Service Unavailable']) - - result = subject.get('/foo') - expect(result.code).to eq("503") - end - - it "should return a 503 response if Retry-After is not convertible to an Integer or RFC 2822 Date" do - retry_after('foo') - - expect { - subject.get('/foo') - }.to raise_error(Puppet::HTTP::ProtocolError, /Failed to parse Retry-After header 'foo'/) - end - - it "should close the connection before sleeping" do - retry_after('42') - - http1 = Net::HTTP.new(host, port) - http1.use_ssl = true - allow(http1).to receive(:started?).and_return(true) - - http2 = Net::HTTP.new(host, port) - http2.use_ssl = true - allow(http1).to receive(:started?).and_return(true) - - # The "with_connection" method is required to yield started connections - pool = Puppet.runtime[:http].pool - - allow(pool).to receive(:with_connection).and_yield(http1).and_yield(http2) - - expect(http1).to receive(:finish).ordered - expect(::Kernel).to receive(:sleep).with(42).ordered - - subject.get('/foo') - end - - it "should sleep and retry if Retry-After is an Integer" do - retry_after('42') - - expect(::Kernel).to receive(:sleep).with(42) - - result = subject.get('/foo') - expect(result.code).to eq("200") - end - - it "should sleep and retry if Retry-After is an RFC 2822 Date" do - retry_after('Wed, 13 Apr 2005 15:18:05 GMT') - - now = DateTime.new(2005, 4, 13, 8, 17, 5, '-07:00') - allow(DateTime).to receive(:now).and_return(now) - - expect(::Kernel).to receive(:sleep).with(60) - - result = subject.get('/foo') - expect(result.code).to eq("200") - end - - it "should sleep for no more than the Puppet runinterval" do - retry_after('60') - - Puppet[:runinterval] = 30 - - expect(::Kernel).to receive(:sleep).with(30) - - subject.get('/foo') - end - - it "should sleep for 0 seconds if the RFC 2822 date has past" do - retry_after('Wed, 13 Apr 2005 15:18:05 GMT') - - expect(::Kernel).to receive(:sleep).with(0) - - subject.get('/foo') - end - end - - context "basic auth" do - let(:auth) { { :user => 'user', :password => 'password' } } - let(:creds) { [ 'user', 'password'] } - - it "is allowed in get requests" do - stub_request(:get, url).with(basic_auth: creds) - - subject.get('/foo', nil, :basic_auth => auth) - end - - it "is allowed in post requests" do - stub_request(:post, url).with(basic_auth: creds) - - subject.post('/foo', 'data', nil, :basic_auth => auth) - end - - it "is allowed in head requests" do - stub_request(:head, url).with(basic_auth: creds) - - subject.head('/foo', nil, :basic_auth => auth) - end - - it "is allowed in delete requests" do - stub_request(:delete, url).with(basic_auth: creds) - - subject.delete('/foo', nil, :basic_auth => auth) - end - - it "is allowed in put requests" do - stub_request(:put, url).with(basic_auth: creds) - - subject.put('/foo', 'data', nil, :basic_auth => auth) - end - end - - it "sets HTTP User-Agent header" do - puppet_ua = "Puppet/#{Puppet.version} Ruby/#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL} (#{RUBY_PLATFORM})" - stub_request(:get, url).with(headers: { 'User-Agent' => puppet_ua }) - - subject.get('/foo') - end - - describe 'connection request errors' do - it "logs and raises generic http errors" do - generic_error = Net::HTTPError.new('generic error', double("response")) - stub_request(:get, url).to_raise(generic_error) - - expect(Puppet).to receive(:log_exception).with(anything, /^.*failed.*: generic error$/) - expect { subject.get('/foo') }.to raise_error(generic_error) - end - - it "logs and raises timeout errors" do - timeout_error = Net::OpenTimeout.new - stub_request(:get, url).to_raise(timeout_error) - - expect(Puppet).to receive(:log_exception).with(anything, /^.*timed out .*after .* seconds/) - expect { subject.get('/foo') }.to raise_error(timeout_error) - end - - it "logs and raises eof errors" do - eof_error = EOFError - stub_request(:get, url).to_raise(eof_error) - - expect(Puppet).to receive(:log_exception).with(anything, /^.*interrupted after .* seconds$/) - expect { subject.get('/foo') }.to raise_error(eof_error) - end - end - end - - describe Puppet::Network::HTTP::Connection do - it_behaves_like "an HTTP connection", described_class - end -end diff --git a/spec/unit/network/http_pool_spec.rb b/spec/unit/network/http_pool_spec.rb index e041baf9da..14957cfd63 100644 --- a/spec/unit/network/http_pool_spec.rb +++ b/spec/unit/network/http_pool_spec.rb @@ -24,14 +24,6 @@ def initialize(host, port, options = {}) end end - it "returns instances of the http client class" do - Puppet::Network::HttpPool.http_client_class = http_impl - http = Puppet::Network::HttpPool.http_instance("me", 54321) - expect(http).to be_an_instance_of(http_impl) - expect(http.host).to eq('me') - expect(http.port).to eq(54321) - end - it "uses the default http client" do expect(Puppet.runtime[:http]).to be_an_instance_of(Puppet::HTTP::Client) end @@ -51,95 +43,4 @@ def initialize(host, port, options = {}) expect(Puppet.runtime[:http]).to eq(new_impl) end end - - describe "when managing http instances" do - it "should return an http instance created with the passed host and port" do - http = Puppet::Network::HttpPool.http_instance("me", 54321) - expect(http).to be_a_kind_of Puppet::Network::HTTP::Connection - expect(http.address).to eq('me') - expect(http.port).to eq(54321) - end - - it "should enable ssl on the http instance by default" do - expect(Puppet::Network::HttpPool.http_instance("me", 54321)).to be_use_ssl - end - - it "can set ssl using an option" do - expect(Puppet::Network::HttpPool.http_instance("me", 54321, false)).not_to be_use_ssl - expect(Puppet::Network::HttpPool.http_instance("me", 54321, true)).to be_use_ssl - end - - context "when calling 'connection'" do - it 'requires an ssl_context' do - expect { - Puppet::Network::HttpPool.connection('me', 8140) - }.to raise_error(ArgumentError, "An ssl_context is required when connecting to 'https://me:8140'") - end - - it 'creates a verifier from the context' do - ssl_context = Puppet::SSL::SSLContext.new - expect( - Puppet::Network::HttpPool.connection('me', 8140, ssl_context: ssl_context).verifier - ).to be_a_kind_of(Puppet::SSL::Verifier) - end - - it 'does not use SSL when specified' do - expect(Puppet::Network::HttpPool.connection('me', 8140, use_ssl: false)).to_not be_use_ssl - end - - it 'defaults to SSL' do - ssl_context = Puppet::SSL::SSLContext.new - conn = Puppet::Network::HttpPool.connection('me', 8140, ssl_context: ssl_context) - expect(conn).to be_use_ssl - end - - it 'warns if an ssl_context is used for an http connection' do - expect(Puppet).to receive(:warning).with("An ssl_context is unnecessary when connecting to 'http://me:8140' and will be ignored") - - ssl_context = Puppet::SSL::SSLContext.new - Puppet::Network::HttpPool.connection('me', 8140, use_ssl: false, ssl_context: ssl_context) - end - end - - describe 'peer verification' do - - before(:each) do - Puppet[:ssldir] = tmpdir('ssl') - Puppet.settings.use(:main) - - Puppet[:certname] = 'signed' - File.write(Puppet[:localcacert], cert_fixture('ca.pem')) - File.write(Puppet[:hostcrl], crl_fixture('crl.pem')) - File.write(Puppet[:hostcert], cert_fixture('signed.pem')) - File.write(Puppet[:hostprivkey], key_fixture('signed-key.pem')) - end - - it 'enables peer verification by default' do - stub_request(:get, "https://me:54321") - - conn = Puppet::Network::HttpPool.http_instance("me", 54321, true) - expect_any_instance_of(Net::HTTP).to receive(:start) do |http| - expect(http.verify_mode).to eq(OpenSSL::SSL::VERIFY_PEER) - end - - conn.get('/') - end - - it 'can disable peer verification' do - stub_request(:get, "https://me:54321") - - conn = Puppet::Network::HttpPool.http_instance("me", 54321, true, false) - expect_any_instance_of(Net::HTTP).to receive(:start) do |http| - expect(http.verify_mode).to eq(OpenSSL::SSL::VERIFY_NONE) - end - - conn.get('/') - end - end - - it "should not cache http instances" do - expect(Puppet::Network::HttpPool.http_instance("me", 54321)). - not_to equal(Puppet::Network::HttpPool.http_instance("me", 54321)) - end - end end From 8e211ee43860fac48423a177b7432fd7b59f6a1f Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Mon, 13 Apr 2026 14:09:40 +0000 Subject: [PATCH 2/4] Workaround for Windows tests `dir_containing` creates the input `.rb` files just before `genface.types` runs and creates the output `.pp` files. On Windows with Ruby 3.2, sub-second or 2-second filesystem timestamp granularity can make test2.rb's mtime appear >= test2.pp's mtime, causing `up_to_date?` to return false incorrectly. The fix backdates both input `.rb` files by 10 seconds before the first `genface.types` run, so the generated `.pp` files will always have unambiguously newer mtimes regardless of filesystem timestamp granularity. The `sleep(1)` before `touch`ing test1.rb is still needed to ensure test1.rb's new mtime is clearly newer than test1.pp's mtime. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Steven Pritchard --- spec/unit/face/generate_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/unit/face/generate_spec.rb b/spec/unit/face/generate_spec.rb index 95b78e459b..6cd7c8d9d7 100644 --- a/spec/unit/face/generate_spec.rb +++ b/spec/unit/face/generate_spec.rb @@ -181,6 +181,11 @@ module Puppet end it 'overwrites if files exists that are not up to date while keeping up to date files' do + # Backdate input files so the generated output will have unambiguously newer mtimes + past = Time.now - 10 + File.utime(past, past, + File.join(m1, 'lib', 'puppet', 'type', 'test1.rb'), + File.join(m2, 'lib', 'puppet', 'type', 'test2.rb')) # create them (first run) genface.types stats_before = [Puppet::FileSystem.stat(File.join(outputdir, 'test1.pp')), Puppet::FileSystem.stat(File.join(outputdir, 'test2.pp'))] From f8b5a5d6e28baaff3627e9a6f502418cf35e6efe Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Mon, 13 Apr 2026 15:04:15 +0000 Subject: [PATCH 3/4] Fix Windows timestamp race in generate_spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace File.utime-based workaround with a sleep(2) before the first genface.types call. File.utime with an explicit Time argument has a DST offset bug on Windows Ruby 3.2 that can shift the stored mtime by ±1 hour, making the input files appear to be in the future and causing spurious regeneration. Sleeping 2 seconds instead ensures output files have unambiguously newer mtimes than inputs on any filesystem, including FAT with its 2-second timestamp granularity. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Steven Pritchard --- spec/unit/face/generate_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/unit/face/generate_spec.rb b/spec/unit/face/generate_spec.rb index 6cd7c8d9d7..2cc4a1e5ee 100644 --- a/spec/unit/face/generate_spec.rb +++ b/spec/unit/face/generate_spec.rb @@ -181,11 +181,10 @@ module Puppet end it 'overwrites if files exists that are not up to date while keeping up to date files' do - # Backdate input files so the generated output will have unambiguously newer mtimes - past = Time.now - 10 - File.utime(past, past, - File.join(m1, 'lib', 'puppet', 'type', 'test1.rb'), - File.join(m2, 'lib', 'puppet', 'type', 'test2.rb')) + # Sleep before the first run so the generated output files will have + # unambiguously newer mtimes than the input files, even on filesystems + # with 2-second timestamp granularity (e.g. Windows/FAT). + sleep(2) # create them (first run) genface.types stats_before = [Puppet::FileSystem.stat(File.join(outputdir, 'test1.pp')), Puppet::FileSystem.stat(File.join(outputdir, 'test2.pp'))] From 18523e2f830e187738ca0c7cd2f7e5b8cc8b8ca1 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Mon, 13 Apr 2026 16:32:35 +0000 Subject: [PATCH 4/4] Additional attempt to fix Windows tests Signed-off-by: Steven Pritchard --- lib/puppet/generate/type.rb | 3 ++- spec/unit/face/generate_spec.rb | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/puppet/generate/type.rb b/lib/puppet/generate/type.rb index 6c06eba3e7..0d41f82d3c 100644 --- a/lib/puppet/generate/type.rb +++ b/lib/puppet/generate/type.rb @@ -49,7 +49,8 @@ def format=(format) # @return [Boolean] Returns true if the output is up-to-date or false if not. def up_to_date?(outputdir) f = effective_output_path(outputdir) - Puppet::FileSystem.exist?(f) && (Puppet::FileSystem.stat(@path) <=> Puppet::FileSystem.stat(f)) <= 0 + Puppet::FileSystem.exist?(f) && + Puppet::FileSystem.stat(@path).mtime <= Puppet::FileSystem.stat(f).mtime end # Gets the filename of the output file. diff --git a/spec/unit/face/generate_spec.rb b/spec/unit/face/generate_spec.rb index 2cc4a1e5ee..2e3a218932 100644 --- a/spec/unit/face/generate_spec.rb +++ b/spec/unit/face/generate_spec.rb @@ -188,9 +188,12 @@ module Puppet # create them (first run) genface.types stats_before = [Puppet::FileSystem.stat(File.join(outputdir, 'test1.pp')), Puppet::FileSystem.stat(File.join(outputdir, 'test2.pp'))] - # fake change in input test1 - sorry about the sleep (which there was a better way to change the modtime + # fake change in input test1 by rewriting its content to update its mtime. + # We use File.write rather than Puppet::FileSystem.touch to avoid potential + # Windows Ruby 3.2 behaviour where touch updates all files in the directory. sleep(1) - Puppet::FileSystem.touch(File.join(m1, 'lib', 'puppet', 'type', 'test1.rb')) + test1_rb_path = File.join(m1, 'lib', 'puppet', 'type', 'test1.rb') + File.write(test1_rb_path, File.read(test1_rb_path)) # generate again genface.types # assert that test1 was overwritten (later) but not test2 (same time)