Skip to content

Commit 6b1cc3f

Browse files
(PUP-7362) Fail fast if there are no facts for a node
Previously, puppet lookup would return incorrect results for any target node that had no facts cached. For example when using the `Per-node data` hierarchy, with the path to the data fail being the value of `$trusted[certname]`, a node that had the said fact cached would return the correct data from its data file, but a node which didn't have any facts cached, would default to a common configuration file, outputting the data from there. This is misleading since it doesn't error out, giving the impression that the run went well, when in actuality it failed to fetch the correct data. This commit fixes this issue by failing the run the moment we are able to see that the target node has no cached facts.
1 parent 0694a45 commit 6b1cc3f

File tree

4 files changed

+37
-4
lines changed

4 files changed

+37
-4
lines changed

acceptance/tests/parser_functions/puppet_lookup_cmd.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,15 @@
318318
',
319319
}
320320

321+
## facts file
322+
file { '#{@coderoot}/facts.yaml':
323+
ensure => file,
324+
mode => "0644",
325+
content => '---
326+
my_data_key: "my_data_value"
327+
',
328+
}
329+
321330
file { '#{@coderoot}/hieradata/global.yaml':
322331
ensure => file,
323332
mode => "0644",
@@ -2553,7 +2562,7 @@ def data()
25532562
apply_manifest_on(master, @encmanifest, :catch_failures => true)
25542563

25552564
step "--compile uses environment specified in ENC"
2556-
r = on(master, puppet('lookup', '--compile', "--node #{@node1}", "--confdir #{@confdir}", 'environment_key'))
2565+
r = on(master, puppet('lookup', '--compile', "--node #{@node1}", "--confdir #{@confdir}", "--facts #{@coderoot}/facts.yaml", 'environment_key'))
25572566
result = r.stdout
25582567
assert_match(
25592568
/env-env2-ruby-function/,
@@ -2562,12 +2571,20 @@ def data()
25622571
)
25632572

25642573
step "without --compile does not use environment specified in ENC"
2565-
r = on(master, puppet('lookup', "--node #{@node1}", "--confdir #{@confdir}", 'environment_key'))
2574+
r = on(master, puppet('lookup', "--node #{@node1}", "--confdir #{@confdir}", "--facts #{@coderoot}/facts.yaml", 'environment_key'))
25662575
result = r.stdout
25672576
assert_match(
25682577
/env-production hiera provided value/,
25692578
result,
25702579
"lookup in production environment failed"
25712580
)
25722581

2582+
step "lookup fails when there are no facts available"
2583+
r = on(master, puppet('lookup', '--compile', "--node #{@node1}", "--confdir #{@confdir}", 'environment_key'), :acceptable_exit_codes => [1])
2584+
result = r.stderr
2585+
assert_match(
2586+
/No facts available/,
2587+
result,
2588+
"Expected to raise when there were no facts available."
2589+
)
25732590
end

lib/puppet/application/lookup.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,10 @@ def generate_scope
360360
facts = Puppet::Node::Facts.new(node, {}) if facts.nil?
361361
facts.add_extra_values(given_facts) if given_facts
362362

363+
if facts.values.empty?
364+
raise _("No facts available for target node: %{node}") % { node: node}
365+
end
366+
363367
ni = Puppet::Node.indirection
364368
tc = ni.terminus_class
365369

spec/integration/application/lookup_spec.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
include PuppetSpec::Files
88

99
context 'with an environment' do
10-
let(:facts) { Puppet::Node::Facts.new("facts", {}) }
1110
let(:env_name) { 'spec' }
1211
let(:env_dir) { tmpdir('environments') }
1312
let(:environment_files) do
@@ -45,6 +44,11 @@
4544
let(:app) { Puppet::Application[:lookup] }
4645
let(:env) { Puppet::Node::Environment.create(env_name.to_sym, [File.join(populated_env_dir, env_name, 'modules')]) }
4746
let(:environments) { Puppet::Environments::Directories.new(populated_env_dir, []) }
47+
let(:facts) { Puppet::Node::Facts.new("facts", {'my_fact' => 'my_fact_value'}) }
48+
49+
before do
50+
allow(Puppet::Node::Facts.indirection).to receive(:find).and_return(facts)
51+
end
4852

4953
let(:populated_env_dir) do
5054
dir_contained_in(env_dir, environment_files)
@@ -110,7 +114,7 @@ def explain(key, options = {})
110114
require 'puppet/indirector/node/exec'
111115
require 'puppet/indirector/node/plain'
112116

113-
let(:node) { Puppet::Node.new('testnode', :environment => env) }
117+
let(:node) { Puppet::Node.new('testnode', :facts => facts, :environment => env) }
114118

115119
it ':plain without --compile' do
116120
Puppet.settings[:node_terminus] = 'exec'

spec/unit/application/lookup_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,14 @@ def run_lookup(lookup)
631631
lookup.run_command
632632
}.to raise_error(/Incorrectly formatted data in .+ given via the --facts flag \(only accepts yaml and json files\)/)
633633
end
634+
635+
it "fails when a node doesn't have facts" do
636+
lookup.options[:node] = "bad.node"
637+
allow(lookup.command_line).to receive(:args).and_return(['c'])
638+
639+
expected_error = "No facts available for target node: #{lookup.options[:node]}"
640+
expect { lookup.run_command }.to raise_error(RuntimeError, expected_error)
641+
end
634642
end
635643
end
636644

0 commit comments

Comments
 (0)