mmckinst / puppet-lint-top_scope_facts-check

Find and convert from top scope facts to the trusted facts hash
Apache License 2.0
2 stars 4 forks source link

plugin converts scoped variables into $facts hash #10

Open KeithWard opened 2 years ago

KeithWard commented 2 years ago

We've noticed that the current behaviour of this plugin seems to conflict with other plugins.

There's some history in https://github.com/puppetlabs/puppet-lint/issues/41

Most notably this plugin seems to want to convert all variables with $:: to top level facts, even if they're scoped variables, which results in some weird syntax depending on which plugin gets there first.

Example: It attempts to convert: $::myodule::foo Into: $facts['mymodule']::foo

However In the event that the topscope_variable plugin gets there first, it'll give you a more reasonable: use $mymodule::foo instead of $::mymodule::foo

The shortest solution i can think of for this, is similar to Alex's solution for https://github.com/voxpupuli/puppet-lint-topscope-variable-check/pull/12 where we exclude anything ending in ::

alexjfisher commented 2 years ago

In the event that the topscope_variable plugin gets there first

It comes second alphabetically, so AFAICT, will be registered after this plugin and will be run after too??

natemccurdy commented 1 year ago

This plugin at version 1.0.1 is also erroring on top-scope variables from manifests/site.pp and the ENC.

For example, this:

notice($::some_top_scope_var)
notice($::a_var_from_an_enc)

Throws this error:

foo.pp - WARNING:top_scope_facts - top scope fact instead of facts hash on line 1
foo.pp - WARNING:top_scope_facts - top scope fact instead of facts hash on line 2

And thinking about it... how is this plugin supposed to know what's a fact and what's a real top-scope variable?

natemccurdy commented 1 year ago

Ah, ignore my last question.. it's right in the docs: https://github.com/mmckinst/puppet-lint-top_scope_facts-check#configuring-the-check

The list of top-scope-variables can be configured.