From d416062a39ba5835477d8fda866126431c778f7a Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Mon, 20 Feb 2023 16:38:08 +0000 Subject: [PATCH] (CONT-339) Add Top Scope Facts Check 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 and @seanmil. Permission has been given to migrate this check to puppet-lints default plugin set in this PR: https://github.com/mmckinst/puppet-lint-top_scope_facts-check/pull/11 --- .../top_scope_facts/top_scope_facts.rb | 38 ++++ .../top_scope_facts/top_scope_facts_spec.rb | 194 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb create mode 100644 spec/unit/puppet-lint/plugins/top_scope_facts/top_scope_facts_spec.rb diff --git a/lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb b/lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb new file mode 100644 index 00000000..bd5060e2 --- /dev/null +++ b/lib/puppet-lint/plugins/top_scope_facts/top_scope_facts.rb @@ -0,0 +1,38 @@ +# Public: A puppet-lint plugin that will check for the use of top scope facts. +# For example, the fact `$facts['kernel']` should be used over +# `$::kernel`. +# +# 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']` +# +# This plugin was adoped in to puppet-lint from https://github.com/mmckinst/puppet-lint-top_scope_facts-check +# Thanks to @mmckinst and @seanmil for the original work. +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 diff --git a/spec/unit/puppet-lint/plugins/top_scope_facts/top_scope_facts_spec.rb b/spec/unit/puppet-lint/plugins/top_scope_facts/top_scope_facts_spec.rb new file mode 100644 index 00000000..c09f8bae --- /dev/null +++ b/spec/unit/puppet-lint/plugins/top_scope_facts/top_scope_facts_spec.rb @@ -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