Skip to content

Commit

Permalink
(CONT-339) Add Top Scope Facts Check
Browse files Browse the repository at this point in the history
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
  • Loading branch information
chelnak committed Feb 21, 2023
1 parent bbcb5c2 commit 5ccece5
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 1 deletion.
35 changes: 35 additions & 0 deletions lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Public: A puppet-lint plugin that will check for the use of top scope facts.
# For example, the fact `$facts['operatingsystem']` should be used over
# `$::operatingsystem`.
#
# The check only finds facts using the top-scope: ie it will find $::operatingsystem
# but not $operatingsystem. It also all top scope variables are facts.
# If you have top scope variables that aren't facts you should configure the
# linter to ignore them.
#
# You can whitelist top scope variables to ignore via the Rake task.
# You should insert the following line to your Rakefile.
# `PuppetLint.configuration.top_scope_variables = ['location', 'role']`
PuppetLint.new_check(:top_scope_facts) do
TOP_SCOPE_FACTS_VAR_TYPES = Set[:VARIABLE, :UNENC_VARIABLE]
def check
whitelist = ['trusted', 'facts'] + (PuppetLint.configuration.top_scope_variables || [])
whitelist = whitelist.join('|')
tokens.select { |x| TOP_SCOPE_FACTS_VAR_TYPES.include?(x.type) }.each do |token|
next unless %r{^::}.match?(token.value)
next if %r{^::(#{whitelist})\[?}.match?(token.value)
next if %r{^::[a-z0-9_][a-zA-Z0-9_]+::}.match?(token.value)

notify :warning, {
message: 'top scope fact instead of facts hash',
line: token.line,
column: token.column,
token: token,
}
end
end

def fix(problem)
problem[:token].value = "facts['" + problem[:token].value.sub(%r{^::}, '') + "']"
end
end
2 changes: 1 addition & 1 deletion puppet-lint.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ Gem::Specification.new do |spec|
]
spec.license = 'MIT'

spec.required_ruby_version = Gem::Requirement.new('>= 2.7'.freeze)
spec.required_ruby_version = Gem::Requirement.new('>= 2.5'.freeze)
end
194 changes: 194 additions & 0 deletions spec/unit/puppet-lint/plugins/top_scope_facts/top_scope_facts_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
require 'spec_helper'

describe 'top_scope_facts' do
let(:msg) { 'top scope fact instead of facts hash' }

context 'with fix disabled' do
context 'fact variable using $facts hash' do
let(:code) { "$facts['operatingsystem']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end
context 'non-fact variable with two colons' do
let(:code) { '$foo::bar' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'top scope $::facts hash' do
let(:code) { "$::facts['os']['family']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'top scope $::trusted hash' do
let(:code) { "$::trusted['certname']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'fact variable using top scope' do
let(:code) { '$::operatingsystem' }

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

it 'creates a warning' do
expect(problems).to contain_warning(msg).on_line(1).in_column(1)
end
end

context 'fact variable using top scope with curly braces in double quote' do
let(:code) { '"${::operatingsystem}"' }

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

it 'creates a warning' do
expect(problems).to contain_warning(msg).on_line(1).in_column(4)
end
end

context 'out of scope namespaced variable with leading ::' do
let(:code) { '$::profile::foo::bar' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end

context 'inside double quotes' do
let(:code) { '"$::profile::foo::bar"' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'with curly braces in double quote' do
let(:code) { '"${::profile::foo::bar}"' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end
end
end

context 'with fix enabled' do
before(:each) do
PuppetLint.configuration.fix = true
end

after(:each) do
PuppetLint.configuration.fix = false
end

context 'fact variable using $facts hash' do
let(:code) { "$facts['operatingsystem']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'non-fact variable with two colons' do
let(:code) { '$foo::bar' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'top scope $::facts hash' do
let(:code) { "$::facts['os']['family']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'top scope $::trusted hash' do
let(:code) { "$::trusted['certname']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'fact variable using top scope' do
let(:code) { '$::operatingsystem' }

it 'onlies detect 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 'shoulds use the facts hash' do
expect(manifest).to eq("$facts['operatingsystem']")
end
end

context 'fact variable using top scope with curly braces in double quote' do
let(:code) { '"${::operatingsystem}"' }

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

it 'shoulds use the facts hash' do
expect(manifest).to eq('"${facts[\'operatingsystem\']}"')
end
end

context 'with custom top scope fact variables' do
before(:each) do
PuppetLint.configuration.top_scope_variables = ['location', 'role']
end

context 'fact variable using $facts hash' do
let(:code) { "$facts['operatingsystem']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'fact variable using $trusted hash' do
let(:code) { "$trusted['certname']" }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'whitelisted top scope variable $::location' do
let(:code) { '$::location' }

it 'does not detect any problems' do
expect(problems).to have(0).problem
end
end

context 'non-whitelisted top scope variable $::application' do
let(:code) { '$::application' }

it 'does not detect any problems' do
expect(problems).to have(1).problem
end
end
end
end
end

0 comments on commit 5ccece5

Please sign in to comment.