diff --git a/lib/puppet-lint/bin.rb b/lib/puppet-lint/bin.rb index aa697b81..593d06ae 100644 --- a/lib/puppet-lint/bin.rb +++ b/lib/puppet-lint/bin.rb @@ -66,7 +66,10 @@ def run path = path.gsub(File::ALT_SEPARATOR, File::SEPARATOR) if File::ALT_SEPARATOR path = if File.directory?(path) - Dir.glob("#{path}/**/*.pp") + Dir.glob([ + "#{path}/**/*.pp", + "#{path}/**/*.{yaml,yml}", + ]) else @args end diff --git a/lib/puppet-lint/checks.rb b/lib/puppet-lint/checks.rb index 6b3989f3..77b66e8e 100644 --- a/lib/puppet-lint/checks.rb +++ b/lib/puppet-lint/checks.rb @@ -6,6 +6,8 @@ class PuppetLint::Checks # Public: Get an Array of problem Hashes. attr_accessor :problems + YAML_COMPATIBLE_CHECKS = [:legacy_facts].freeze + # Public: Initialise a new PuppetLint::Checks object. def initialize @problems = [] @@ -45,23 +47,31 @@ def load_data(path, content) end end - # Internal: Run the lint checks over the manifest code. + # Internal: Run the lint checks over the manifest or YAML code. # # fileinfo - The path to the file as passed to puppet-lint as a String. - # data - The String manifest code to be checked. + # data - The String manifest or YAML code to be checked. # # Returns an Array of problem Hashes. def run(fileinfo, data) load_data(fileinfo, data) checks_run = [] - enabled_checks.each do |check| - klass = PuppetLint.configuration.check_object[check].new - # FIXME: shadowing #problems - problems = klass.run - checks_run << [klass, problems] + if File.extname(fileinfo).downcase.match?(%r{\.ya?ml$}) + enabled_checks.select { |check| YAML_COMPATIBLE_CHECKS.include?(check) }.each do |check| + klass = PuppetLint.configuration.check_object[check].new + # FIXME: shadowing #problems + problems = klass.run + checks_run << [klass, problems] + end + else + enabled_checks.each do |check| + klass = PuppetLint.configuration.check_object[check].new + # FIXME: shadowing #problems + problems = klass.run + checks_run << [klass, problems] + end end - checks_run.each do |klass, problems| if PuppetLint.configuration.fix @problems.concat(klass.fix_problems) diff --git a/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb b/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb index e435a0af..3c4c52d1 100644 --- a/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb +++ b/lib/puppet-lint/plugins/legacy_facts/legacy_facts.rb @@ -1,3 +1,5 @@ +require 'yaml' + # Public: A puppet-lint custom check to detect legacy facts. # # This check will optionally convert from legacy facts like $::operatingsystem @@ -112,6 +114,51 @@ PuppetLint.new_check(:legacy_facts) do def check + if File.extname(PuppetLint::Data.path).downcase.match?(%r{\.ya?ml$}) + content = PuppetLint::Data.manifest_lines + yaml_content = content.join("\n") + data = YAML.safe_load(yaml_content, aliases: true, permitted_classes: [Symbol]) + search_yaml(data) + else + check_puppet + end + end + + def search_yaml(data, path = []) + case data + when Hash + data.each do |k, v| + search_value(k.to_s, path) + search_yaml(v, path + [k.to_s]) + end + when Array + data.each_with_index { |v, i| search_yaml(v, path + [i]) } + when String + search_value(data, path) + end + end + + def search_value(value, _path) + value.scan(%r{%{(?:(?:::?)?|facts\.)([a-zA-Z0-9_]+)(?!\.[a-zA-Z])}}) do |match| + base_fact = match[0].split('.').first + next unless EASY_FACTS.include?(base_fact) || UNCONVERTIBLE_FACTS.include?(base_fact) || base_fact.match(Regexp.union(REGEX_FACTS)) + + notify :warning, { + message: "legacy fact '#{base_fact}'", + line: find_line_for_content(value), + column: 1 + } + end + end + + def find_line_for_content(content) + PuppetLint::Data.manifest_lines.each_with_index do |line, index| + return index + 1 if line.include?(content) + end + 1 + end + + def check_puppet tokens.select { |x| LEGACY_FACTS_VAR_TYPES.include?(x.type) }.each do |token| fact_name = '' diff --git a/spec/unit/puppet-lint/checks_spec.rb b/spec/unit/puppet-lint/checks_spec.rb index c27a10e2..2ba1fa3c 100644 --- a/spec/unit/puppet-lint/checks_spec.rb +++ b/spec/unit/puppet-lint/checks_spec.rb @@ -196,6 +196,60 @@ end end + describe '#run_yaml' do + let(:fileinfo) { File.join('path', 'to', 'test.yaml') } + let(:data) do + <<~EOS + --- + version: 5 + defaults: + datadir: data + data_hash: yaml_data + hierarchy: + - name: "LEGACY FACT PATH" + paths: + - "os/%{facts.hostname}.yaml" + - name: "osfamily/major release" + paths: + - "os/%{facts.os.name}/%{facts.os.release.major}.yaml" + - name: 'common' + path: 'common.yaml' + EOS + end + let(:enabled_checks) { [:legacy_facts] } + + before(:each) do + allow(instance).to receive(:enabled_checks).and_return(enabled_checks) # rubocop: disable RSpec/SubjectStub + allow(File).to receive(:extname).with(fileinfo).and_return('.yaml') + end + + it 'loads the yaml data' do + expect(instance).to receive(:load_data).with(fileinfo, data).and_call_original # rubocop: disable RSpec/SubjectStub + instance.run(fileinfo, data) + end + + context 'when there are checks enabled' do + let(:enabled_checks) { [:legacy_facts] } + let(:enabled_check_classes) { enabled_checks.map { |r| PuppetLint.configuration.check_object[r] } } + let(:disabled_checks) { PuppetLint.configuration.checks - enabled_checks } + let(:disabled_check_classes) { disabled_checks.map { |r| PuppetLint.configuration.check_object[r] } } + + it 'runs the enabled checks' do + expect(enabled_check_classes).to all(receive(:new).and_call_original) + + instance.run(fileinfo, data) + end + + it 'does not run the disabled checks' do + disabled_check_classes.each do |check_class| + expect(check_class).not_to receive(:new) + end + + instance.run(fileinfo, data) + end + end + end + describe '#enabled_checks' do subject(:enabled_checks) { instance.enabled_checks } diff --git a/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb b/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb index 64311aa6..b1ec4eb0 100644 --- a/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb +++ b/spec/unit/puppet-lint/plugins/legacy_facts/legacy_facts_spec.rb @@ -129,6 +129,65 @@ expect(problems.size).to eq(1) end end + + context 'YAML file processing' do + before(:each) do + allow(File).to receive(:extname).and_return('.yaml') + end + + context 'with YAML string containing legacy fact' do + let(:code) { 'some_key: "%{::osfamily}"' } + + it 'detects a single problem' do + expect(problems.size).to eq(1) + end + end + + context 'with YAML string not containing legacy fact' do + let(:code) { 'some_key: "%{facts.os.name}"' } + + it 'does not detect any problems' do + expect(problems).to be_empty + end + end + + context 'with YAML nested structure containing legacy fact' do + let(:code) { "nested:\n value: \"%{::architecture}\"" } + + it 'detects a single problem' do + expect(problems.size).to eq(1) + end + end + + context 'with YAML array containing legacy facts' do + let(:code) do + [ + 'array:', + ' - "%{::processor0}"', + ' - "%{::ipaddress_eth0}"', + ].join("\n") + end + + it 'detects multiple problems' do + expect(problems.size).to eq(2) + end + end + + context 'with YAML alias containing legacy fact' do + let(:code) do + [ + 'template: &template', + ' fact: "%{::osfamily}"', + 'instance:', + ' <<: *template', + ].join("\n") + end + + it 'detects multiple instances' do + expect(problems.size).to eq(2) + end + end + end end context 'with fix enabled' do