Top scope facts not being extracted correctly

[danifr]

I believe the change made in https://github.com/puppetlabs/puppet-lint/commit/a3b7241d6694bda05f4e092c84b2f544ded544d8 introduced a bug related to how top scope facts are extracted:

for example in:

-    ssl_certfile                => "/etc/letsencrypt/live/${::fqdn}/fullchain.pem",
+    ssl_certfile                => "/etc/letsencrypt/live/${facts['facts['networking']['fqdn']']}/fullchain.pem",

${::fqdn} gets replaced by ${facts['facts['networking']['fqdn']']} which does not seem to be correct (check where the ' are)

Environment

• Puppet-lint version [3.2.0]

Thanks, Daniel.

[trondiz]

Just wanted to confirm we see the same thing 3.3.0 e.g.: case $::operatingsystem { becomes: case $facts['facts['os']['name']'] {

[danifr]

The bug was actually introduced in https://github.com/puppetlabs/puppet-lint/commit/f82b9bc3a07593de024e6932ed5dcf678aff280a

I will open a "quick a dirty" fix for this issue

[GSPatton]

hi @danifr. We're having trouble reproducing this issue. Could you give us more details about your environment? It'd be great to get

1. The contents of your Gemfile
2. Your OS
3. Your puppet version

Thanks!

[GSPatton]

i've replicated this issue on Windows. Looking into it

[chelnak]

And can you let us know your Ruby version too?

[danifr]

Thanks!

Operating system: Archlinux Puppet version: I don't have puppet installed on this machine.

Ruby version:

danielfr@latdfr ~> ruby --version
ruby 3.0.5p211 (2022-11-24 revision ba5cf0f7c5) [x86_64-linux]

Gem list

danielfr@latdfr ~> gem list

*** LOCAL GEMS ***

abbrev (default: 0.1.0)
base64 (default: 0.1.1)
benchmark (default: 0.2.0)
bigdecimal (default: 3.1.2)
bundler (default: 2.4.1)
cgi (default: 0.3.6)
csv (default: 3.2.5)
date (default: 3.2.2)
dbm (default: 1.1.0)
debug (default: 0.2.1)
delegate (default: 0.2.0)
did_you_mean (default: 1.6.1)
diff-lcs (1.5.0)
digest (default: 3.1.1)
drb (default: 2.1.0)
english (default: 0.7.1)
erb (default: 4.0.2)
etc (default: 1.3.0)
fcntl (default: 1.0.1)
fiddle (default: 1.1.0)
fileutils (default: 1.6.0)
find (default: 0.1.1)
forwardable (default: 1.3.2)
gdbm (default: 2.1.0)
getoptlong (default: 0.1.1)
io-console (default: 0.5.11)
io-nonblock (default: 0.1.0)
io-wait (default: 0.2.3)
ipaddr (default: 1.2.4)
irb (default: 1.4.2)
json (default: 2.6.3)
logger (default: 1.5.1)
lolcat (100.0.1)
manpages (0.6.1)
matrix (default: 0.3.1)
minitest (5.16.3)
mutex_m (default: 0.1.1)
net-ftp (default: 0.1.2)
net-http (default: 0.2.2)
net-imap (default: 0.1.1)
net-pop (default: 0.1.1)
net-protocol (default: 0.1.1)
net-smtp (default: 0.2.1)
nkf (default: 0.1.0)
observer (default: 0.1.1)
open-uri (default: 0.2.0)
open3 (default: 0.1.1)
openssl (default: 3.0.0)
optimist (3.0.1)
optparse (default: 0.1.1)
ostruct (default: 0.3.1)
paint (2.3.0)
pathname (default: 0.1.0)
power_assert (2.0.2)
pp (default: 0.2.1)
prettyprint (default: 0.1.1)
prime (default: 0.1.2)
pstore (default: 0.1.1)
psych (default: 4.0.6)
puppet-lint (3.3.0)
racc (default: 1.6.0)
rainbow (3.1.1)
rake (13.0.6)
rdoc (default: 6.4.0)
readline (default: 0.0.2)
readline-ext (default: 0.1.1)
reline (default: 0.3.1)
resolv (default: 0.2.1)
resolv-replace (default: 0.1.0)
rexml (3.2.5)
rinda (default: 0.1.1)
rspec (3.12.0)
rspec-core (3.12.0)
rspec-expectations (3.12.0)
rspec-mocks (3.12.0)
rspec-support (3.12.0)
ruby2_keywords (0.0.5)
securerandom (default: 0.1.0)
set (default: 1.0.1)
shellwords (default: 0.1.0)
singleton (default: 0.1.1)
stringio (default: 3.0.2)
strscan (default: 3.0.1)
syslog (default: 0.1.0)
tempfile (default: 0.1.1)
term-ansicolor (1.7.1)
test-unit (3.5.7)
time (default: 0.2.0)
timeout (default: 0.1.1)
tins (1.31.1)
tmpdir (default: 0.1.2)
tracer (default: 0.1.1)
tsort (default: 0.1.0)
un (default: 0.1.0)
uri (default: 0.11.0)
weakref (default: 0.1.1)
yaml (default: 0.1.1)
zlib (default: 2.0.0)

[danifr]

To be clear I don't think this is related to the operating system version, the ruby version or the list of gems installed.

This regression was introduced in https://github.com/puppetlabs/puppet-lint/commit/f82b9bc3a07593de024e6932ed5dcf678aff280a. That code for legacy facts, does not integrate well with the current puppet-lint code and the resulting fix ends up duplicating tokens.

To reproduce the bug: https://github.com/puppetlabs/puppet-lint/pull/114#issuecomment-1490324363

[danifr]

Hello @GSPatton @chelnak, any news on this? Thanks a lot!

[chelnak]

Hey @danifr!

I think this is part of a bigger issue that relates to conflicting plugins.

It's something that I plan on investigating once our Puppet 8 work is clear.

[danifr]

Alright I understand. As a temporarily workaround, can you consider just reverting this commit https://github.com/puppetlabs/puppet-lint/commit/f82b9bc3a07593de024e6932ed5dcf678aff280a and releasing a new version of puppet-lint?

Better not to have that functionality than having something that breaks puppet manifest syntax. Thanks

[danifr]

Just to report that this is still broken with puppet-lint 4.0.0

[chelnak]

Hey yes it wasn't addressed in that release.

We've not forgotten though. It's something I want to look at before the next pdk full release.

[LukasAud]

Hi @danifr, sorry for the long delay in follow up. We have been very busy lately. I have looked into the issue and concluded that the problem was being cause by the interference of the top_scope_fact.rb plugin trying to run its corrections on top of the legacy_fact.rb ones, causing overlapping corrections. Since legacy fact also cleans up top scope namespacing, I have opened a PR (https://github.com/puppetlabs/puppet-lint/pull/138) that simply whitelists all legacy facts for the top_scope corrector.

Hopefully, this should address this issue. If so, please let us know so that we can close this issue and #114.

[danifr]

Ah, nice thank you very much :)

Are you planning on releasing a new version including the patch? or shall I get the files directly and try them on my local installation?

[LukasAud]

Yes, I will cut a small bugfix release as soon as the PR is merged by a member of my team. However, if you want to try it before its out, you can implement the change locally. The fix is very straightforward, simply whitelisting the legacy fact in the appropriate file (lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb) should return the behaviour of the module to our expectations.

[danifr]

Thanks! I confirm that v4.0.1 fixed my issue :) thanks a lot