Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add server facts when looking up values #9392

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

joshcooper
Copy link
Contributor

@joshcooper joshcooper commented Jun 12, 2024

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

Fixes PE-37867

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
@joshcooper joshcooper requested a review from a team as a code owner June 12, 2024 06:20
@joshcooper joshcooper added the bug Something isn't working label Jun 13, 2024
@bastelfreak
Copy link
Contributor

@joshcooper thanks a lot!

@cthorn42 cthorn42 merged commit 52e6707 into puppetlabs:main Jun 17, 2024
10 checks passed
@joshcooper joshcooper deleted the lookup_server_facts branch June 17, 2024 16:52
@joshcooper joshcooper added the backport 7.x Generate a backport PR to 7.x label Jun 18, 2024
Copy link

Backport failed for 7.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 7.x
git worktree add -d .worktree/backport-9392-to-7.x origin/7.x
cd .worktree/backport-9392-to-7.x
git checkout -b backport-9392-to-7.x
ancref=$(git merge-base a423af825d1e87a3158cd68d630e77352e9a5454 719efae576a1fa4730ed579db8fbcd79b48a74d5)
git cherry-pick -x $ancref..719efae576a1fa4730ed579db8fbcd79b48a74d5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 7.x Generate a backport PR to 7.x bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants