Skip to content

Commit

Permalink
(CONT-675) Fix fact detection
Browse files Browse the repository at this point in the history
Prior to this change the legacy facts check would not detect legacy
facts defined as follows:

```
$::facts['osfamily']
````

This changes updates the `check` logic to evaluate tokens that may start
with `::` and extract the fact name as appropriate.
`
  • Loading branch information
chelnak committed Feb 28, 2023
1 parent b029765 commit e3815ce
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
18 changes: 12 additions & 6 deletions lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,25 @@ def check
tokens.select { |x| LEGACY_FACTS_VAR_TYPES.include?(x.type) }.each do |token|
fact_name = ''

# Get rid of the top scope before we do our work. We don't need to
# preserve it because it won't work with the new structured facts.
if token.value.start_with?('::')
fact_name = token.value.sub(%r{^::}, '')
# This matches legacy facts defined in the fact hash that use the top scope
# fact assignment.
if token.value.start_with?('::facts[')
fact_name = token.value.match(%r{::facts\['(.*)'\]})[1]

# This matches legacy facts defined in the fact hash.
elsif token.value.start_with?("facts['")
fact_name = token.value.match(%r{facts\['(.*)'\]})[1]

# This matches using legacy facts in a the new structured fact. For
# example this would match 'uuid' in $facts['uuid'] so it can be converted
# to facts['dmi']['product']['uuid']"
elsif token.value == 'facts'
fact_name = hash_key_for(token)

elsif token.value.start_with?("facts['")
fact_name = token.value.match(%r{facts\['(.*)'\]})[1]
# Now we can get rid of top scopes. We don't need to
# preserve it because it won't work with the new structured facts.
elsif token.value.start_with?('::')
fact_name = token.value.sub(%r{^::}, '')
end

next unless EASY_FACTS.include?(fact_name) || UNCONVERTIBLE_FACTS.include?(fact_name) || fact_name.match(Regexp.union(REGEX_FACTS))
Expand Down
25 changes: 25 additions & 0 deletions spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@
expect(problems).to have(1).problem
end
end

context 'top scoped fact variable using legacy facts hash variable in interpolation' do
let(:code) { "$::facts['osfamily']" }

it 'detects a single problem' do
expect(problems).to have(1).problem
end
end
end

context 'with fix enabled' do
Expand Down Expand Up @@ -165,6 +173,23 @@
end
end

context "fact variable using legacy $::facts['osfamily']" do
let(:code) { "$::facts['osfamily']" }
let(:msg) { "legacy fact 'osfamily'" }

it 'only detects a single problem' do
expect(problems).to have(1).problem
end

it 'fixes the problem' do
expect(problems).to contain_fixed(msg).on_line(1).in_column(1)
end

it 'uses the facts hash' do
expect(manifest).to eq("$facts['os']['family']")
end
end

context 'fact variable using legacy $::osfamily' do
let(:code) { '$::osfamily' }
let(:msg) { "legacy fact 'osfamily'" }
Expand Down

0 comments on commit e3815ce

Please sign in to comment.