From 022971edb58ee0ceabb45322f61904e46625880f Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Tue, 11 Jun 2024 22:29:47 -0700 Subject: [PATCH 1/2] Refactor server fact loading into a separate class puppet 7.x still uses legacy facts with symbolic fact names, e.g. value(:hostname) (cherry picked from commit 8068bcec0ed0c8386e66a1e202648bbe8c51a52c) --- lib/puppet/indirector/catalog/compiler.rb | 38 ++------------------ lib/puppet/node/server_facts.rb | 43 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 36 deletions(-) create mode 100644 lib/puppet/node/server_facts.rb diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb index f12fc53ca9c..99fdd81a3ff 100644 --- a/lib/puppet/indirector/catalog/compiler.rb +++ b/lib/puppet/indirector/catalog/compiler.rb @@ -1,5 +1,6 @@ require_relative '../../../puppet/environments' require_relative '../../../puppet/node' +require_relative '../../../puppet/node/server_facts' require_relative '../../../puppet/resource/catalog' require_relative '../../../puppet/indirector/code' require_relative '../../../puppet/util/profiler' @@ -425,41 +426,6 @@ def node_from_request(facts, request) # # See also set_server_facts in Puppet::Server::Compiler in puppetserver. def set_server_facts - @server_facts = {} - - # Add our server Puppet Enterprise version, if available. - pe_version_file = '/opt/puppetlabs/server/pe_version' - if File.readable?(pe_version_file) and !File.zero?(pe_version_file) - @server_facts['pe_serverversion'] = File.read(pe_version_file).chomp - end - - # Add our server version to the fact list - @server_facts["serverversion"] = Puppet.version.to_s - - # And then add the server name and IP - {"servername" => "fqdn", - "serverip" => "ipaddress", - "serverip6" => "ipaddress6" - }.each do |var, fact| - value = Puppet.runtime[:facter].value(fact) - if !value.nil? - @server_facts[var] = value - end - end - - if @server_facts["servername"].nil? - host = Puppet.runtime[:facter].value(:hostname) - if host.nil? - Puppet.warning _("Could not retrieve fact servername") - elsif domain = Puppet.runtime[:facter].value(:domain) #rubocop:disable Lint/AssignmentInCondition - @server_facts["servername"] = [host, domain].join(".") - else - @server_facts["servername"] = host - end - end - - if @server_facts["serverip"].nil? && @server_facts["serverip6"].nil? - Puppet.warning _("Could not retrieve either serverip or serverip6 fact") - end + @server_facts = Puppet::Node::ServerFacts.load end end diff --git a/lib/puppet/node/server_facts.rb b/lib/puppet/node/server_facts.rb new file mode 100644 index 00000000000..791888e6870 --- /dev/null +++ b/lib/puppet/node/server_facts.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +class Puppet::Node::ServerFacts + def self.load + server_facts = {} + + # Add our server Puppet Enterprise version, if available. + pe_version_file = '/opt/puppetlabs/server/pe_version' + if File.readable?(pe_version_file) and !File.zero?(pe_version_file) + server_facts['pe_serverversion'] = File.read(pe_version_file).chomp + end + + # Add our server version to the fact list + server_facts["serverversion"] = Puppet.version.to_s + + # And then add the server name and IP + {"servername" => "fqdn", + "serverip" => "ipaddress", + "serverip6" => "ipaddress6"}.each do |var, fact| + value = Puppet.runtime[:facter].value(fact) + if !value.nil? + server_facts[var] = value + end + end + + if server_facts["servername"].nil? + host = Puppet.runtime[:facter].value(:hostname) + if host.nil? + Puppet.warning _("Could not retrieve fact servername") + elsif domain = Puppet.runtime[:facter].value(:domain) #rubocop:disable Lint/AssignmentInCondition + server_facts["servername"] = [host, domain].join(".") + else + server_facts["servername"] = host + end + end + + if server_facts["serverip"].nil? && server_facts["serverip6"].nil? + Puppet.warning _("Could not retrieve either serverip or serverip6 fact") + end + + server_facts + end +end From b8c2736c41c5c155b6c6f4e15b16e26bd21720e0 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Tue, 11 Jun 2024 22:30:23 -0700 Subject: [PATCH 2/2] Add server facts when looking up values During normal catalog compilation, server facts are added by the `compiler` terminus prior to calling `Puppet::Parser::Compiler.compile`[1]. However, the lookup application directly calls `Compiler.compile`, bypassing the `compiler` terminus[2]. Therefore, server facts weren't being added when running the lookup command. Ideally, catalog compilation and the lookup command would compile the catalog in the same way, but changing that is risky. For that to work, we would need to pass the already resolved node and facts to the `compiler` terminus and the terminus would need to add server facts to the node. However, the terminus doesn't add server facts if the node is passed in. It only does that if it resolves the node using the indirector[3]. Rather than mess with the terminus and break compilation, just load server facts in the same way that the `compiler` terminus does. [1] https://github.com/puppetlabs/puppet/blob/8.7.0/lib/puppet/indirector/catalog/compiler.rb#L56 [2] https://github.com/puppetlabs/puppet/blob/8.7.0/lib/puppet/application/lookup.rb#L407 [3] https://github.com/puppetlabs/puppet/blob/8.7.0/lib/puppet/indirector/catalog/compiler.rb#L390 (cherry picked from commit 719efae576a1fa4730ed579db8fbcd79b48a74d5) --- lib/puppet/application/lookup.rb | 2 ++ .../application/environments/production/data/common.yaml | 2 ++ spec/unit/application/lookup_spec.rb | 7 +++++++ 3 files changed, 11 insertions(+) diff --git a/lib/puppet/application/lookup.rb b/lib/puppet/application/lookup.rb index 0746b5cd097..be23e277348 100644 --- a/lib/puppet/application/lookup.rb +++ b/lib/puppet/application/lookup.rb @@ -1,6 +1,7 @@ require_relative '../../puppet/application' require_relative '../../puppet/pops' require_relative '../../puppet/node' +require_relative '../../puppet/node/server_facts' require_relative '../../puppet/parser/compiler' class Puppet::Application::Lookup < Puppet::Application @@ -406,6 +407,7 @@ def generate_scope node.add_extra_facts(given_facts) if given_facts end node.environment = Puppet[:environment] if Puppet.settings.set_by_cli?(:environment) + node.add_server_facts(Puppet::Node::ServerFacts.load) Puppet[:code] = 'undef' unless options[:compile] compiler = Puppet::Parser::Compiler.new(node) if options[:node] diff --git a/spec/fixtures/unit/application/environments/production/data/common.yaml b/spec/fixtures/unit/application/environments/production/data/common.yaml index 88b20ad3a45..453775d84ae 100644 --- a/spec/fixtures/unit/application/environments/production/data/common.yaml +++ b/spec/fixtures/unit/application/environments/production/data/common.yaml @@ -20,5 +20,7 @@ ab: "%{hiera('a')} and %{hiera('b')}" g: "This is%{facts.cx} in facts hash" +h: "server version is %{server_facts.serverversion}" + lookup_options: a: first diff --git a/spec/unit/application/lookup_spec.rb b/spec/unit/application/lookup_spec.rb index 02c0acc6394..4b9afe2fd39 100644 --- a/spec/unit/application/lookup_spec.rb +++ b/spec/unit/application/lookup_spec.rb @@ -543,6 +543,13 @@ def run_lookup(lookup) expect(run_lookup(lookup)).to eql("This is G from facts in facts hash") end + it 'looks up server facts' do + lookup.options[:node] = node + lookup.options[:render_as] = :s + allow(lookup.command_line).to receive(:args).and_return(['h']) + expect(run_lookup(lookup)).to eql("server version is #{Puppet.version}") + end + describe 'when retrieving given facts' do before do lookup.options[:node] = node