From 8068bcec0ed0c8386e66a1e202648bbe8c51a52c 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 --- lib/puppet/indirector/catalog/compiler.rb | 37 ++----------------- lib/puppet/node/server_facts.rb | 43 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 35 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 7694949ecf1..dfac7554647 100644 --- a/lib/puppet/indirector/catalog/compiler.rb +++ b/lib/puppet/indirector/catalog/compiler.rb @@ -2,6 +2,7 @@ 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' @@ -426,40 +427,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" => "networking.fqdn", - "serverip" => "networking.ip", - "serverip6" => "networking.ip6" }.each do |var, fact| - value = Puppet.runtime[:facter].value(fact) - unless value.nil? - @server_facts[var] = value - end - end - - if @server_facts["servername"].nil? - host = Puppet.runtime[:facter].value('networking.hostname') - if host.nil? - Puppet.warning _("Could not retrieve fact servername") - elsif domain = Puppet.runtime[:facter].value('networking.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..9e064339488 --- /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" => "networking.fqdn", + "serverip" => "networking.ip", + "serverip6" => "networking.ip6" }.each do |var, fact| + value = Puppet.runtime[:facter].value(fact) + unless value.nil? + server_facts[var] = value + end + end + + if server_facts["servername"].nil? + host = Puppet.runtime[:facter].value('networking.hostname') + if host.nil? + Puppet.warning _("Could not retrieve fact servername") + elsif domain = Puppet.runtime[:facter].value('networking.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 719efae576a1fa4730ed579db8fbcd79b48a74d5 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 --- 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 384c99f2c5b..e250725ed18 100644 --- a/lib/puppet/application/lookup.rb +++ b/lib/puppet/application/lookup.rb @@ -3,6 +3,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 @@ -403,6 +404,7 @@ def generate_scope end 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 00842837176..1a1cd54169e 100644 --- a/spec/unit/application/lookup_spec.rb +++ b/spec/unit/application/lookup_spec.rb @@ -546,6 +546,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