Validate Hiera values

* Use compliance_engine gem for data ingest
* Check that all fact combinations return hiera data
* Pass the logger object from the CLI to the Scelint::Lint class
* Dedupe legacy fact warnings
* Add `notes` for info-level log items
* Bump version

Gemfile

@@ -4,6 +4,7 @@ source 'https://rubygems.org'
 gemspec
 
 gem 'rake', '~> 13.2'
+gem 'compliance_engine', git: 'https://github.com/simp/rubygem-simp-compliance_engine.git'
 
 group :tests do
   gem 'rspec', '~> 3.13'
@@ -14,6 +15,7 @@ group :tests do
 end
 
 group :development do
-  gem 'pry'
-  gem 'pry-byebug'
+  gem 'pry', '~> 0.14.1'
+  gem 'pry-byebug', '~> 3.10'
+  gem 'rdoc', '~> 6.4'
 end

lib/scelint.rb

@@ -3,6 +3,8 @@
 require 'yaml'
 require 'json'
 require 'deep_merge'
+require 'logger'
+require 'compliance_engine'
 
 require 'scelint/version'
 
@@ -126,68 +128,25 @@ class Error < StandardError; end
   # @example Look for data in all modules in the current directory
   #    lint = Scelint::Lint.new(Dir.glob('*'))
   class Lint
-    attr_accessor :data, :errors, :warnings
+    attr_accessor :data, :errors, :warnings, :notes, :log
 
-    def initialize(paths = ['.'])
-      @data = {}
+    def initialize(paths = ['.'], logger: Logger.new(STDOUT, level: Logger::INFO))
+      @log = logger
       @errors = []
       @warnings = []
+      @notes = []
 
-      merged_data = {}
-
-      Array(paths).each do |path|
-        if File.directory?(path)
-          [
-            'SIMP/compliance_profiles',
-            'simp/compliance_profiles',
-          ].each do |dir|
-            ['yaml', 'json'].each do |type|
-              Dir.glob("#{path}/#{dir}/**/*.#{type}") do |file|
-                @data[file] = parse(file)
-                merged_data = merged_data.deep_merge!(Marshal.load(Marshal.dump(@data[file])))
-              end
-            end
-          end
-        elsif File.exist?(path)
-          @data[path] = parse(path)
-        else
-          raise "Can't find path '#{path}'"
-        end
-      end
+      @data = ComplianceEngine::Data.new(*Array(paths))
 
-      return nil if @data.empty?
-
-      @data['merged data'] = merged_data
-
-      @data.each do |file, file_data|
-        lint(file, file_data)
-      end
-    end
-
-    def parse(file)
-      return data[file] if data[file]
-
-      type = case file
-             when %r{\.yaml$}
-               'yaml'
-             when %r{\.json$}
-               'json'
-             else
-               errors << "#{file}: Failed to determine file type"
-               nil
-             end
-      begin
-        return YAML.safe_load(File.read(file)) if type == 'yaml'
-        return JSON.parse(File.read(file)) if type == 'json'
-      rescue => e
-        errors << "#{file}: Failed to parse file: #{e.message}"
+      @data.files.each do |file|
+        lint(file, @data.get(file))
       end
 
-      {}
+      validate
     end
 
     def files
-      data.keys - ['merged data']
+      data.files
     end
 
     def check_version(file, file_data)
@@ -265,7 +224,10 @@ def check_confine(file, file_data)
 
       file_data.each_key do |key|
         warnings << "#{file}: unexpected key '#{key}' in confine '#{file_data}'" if not_ok.include?(key)
-        warnings << "#{file}: legacy fact '#{key}' in confine '#{file_data}'" if Scelint::LEGACY_FACTS.any? { |legacy_fact| legacy_fact.is_a?(Regexp) ? legacy_fact.match?(key) : (legacy_fact == key) }
+        if Scelint::LEGACY_FACTS.any? { |legacy_fact| legacy_fact.is_a?(Regexp) ? legacy_fact.match?(key) : (legacy_fact == key) }
+          warning = "#{file}: legacy fact '#{key}' in confine '#{file_data}'"
+          warnings << warning unless warnings.include?(warning)
+        end
       end
     end
 
@@ -508,6 +470,100 @@ def check_checks(file, file_data)
       end
     end
 
+    def normalize_confinement(confine)
+      normalized = []
+
+      # Step 1, sort the hash keys
+      sorted = confine.sort.to_h
+
+      # Step 2, expand all possible combinations of Array values
+      index = 0
+      max_count = 1
+      sorted.each_value { |value| max_count *= Array(value).size }
+
+      sorted.each do |key, value|
+        (index..(max_count - 1)).each do |i|
+          normalized[i] ||= {}
+          normalized[i][key] = Array(value)[i % Array(value).size]
+        end
+      end
+
+      # Step 3, convert dotted fact names into a facts hash
+      normalized.map do |c|
+        c.each_with_object({}) do |(key, value), result|
+          current = result
+          parts = key.split('.')
+          parts.each_with_index do |part, i|
+            if i == parts.length - 1
+              current[part] = value
+            else
+              current[part] ||= {}
+              current = current[part]
+            end
+          end
+        end
+      end
+    end
+
+    def confines
+      return @confines unless @confines.nil?
+
+      @confines = []
+
+      [:profiles, :ces, :checks, :controls].each do |type|
+        data.public_send(type).each_value do |value|
+          # FIXME: This is calling a private method
+          value.send(:fragments).each_value do |v|
+            next unless v.is_a?(Hash)
+            next unless v.key?('confine')
+            normalize_confinement(v['confine']).each do |confine|
+              @confines << confine unless @confines.include?(confine)
+            end
+          end
+        end
+      end
+
+      @confines
+    end
+
+    def validate
+      if data.profiles.keys.empty?
+        notes << 'No profiles found, unable to validate Hiera data'
+        return nil
+      end
+
+      # Unconfined, verify that hiera data exists
+      data.profiles.each_key do |profile|
+        hiera = data.hiera([profile])
+        if hiera.nil?
+          errors << "Profile '#{profile}': Invalid Hiera data (returned nil)"
+          next
+        end
+        if hiera.empty?
+          warnings << "Profile '#{profile}': No Hiera data found"
+          next
+        end
+        log.debug "Profile '#{profile}': Hiera data found (#{hiera.keys.count} keys)"
+      end
+
+      # Again, this time confined
+      confines.each do |confine|
+        data.facts = confine
+        data.profiles.select { |_, value| value.ces&.count&.positive? || value.controls&.count&.positive? }.each_key do |profile|
+          hiera = data.hiera([profile])
+          if hiera.nil?
+            errors << "Profile '#{profile}': Invalid Hiera data (returned nil) with facts #{confine}"
+            next
+          end
+          if hiera.empty?
+            warnings << "Profile '#{profile}': No Hiera data found with facts #{confine}"
+            next
+          end
+          log.debug "Profile '#{profile}': Hiera data found (#{hiera.keys.count} keys) with facts #{confine}"
+        end
+      end
+    end
+
     def lint(file, file_data)
       check_version(file, file_data)
       check_keys(file, file_data)
@@ -516,6 +572,8 @@ def lint(file, file_data)
       check_ce(file, file_data['ce']) if file_data['ce']
       check_checks(file, file_data['checks']) if file_data['checks']
       check_controls(file, file_data['controls']) if file_data['controls']
+    rescue => e
+      errors << "#{file}: #{e.message} (not a hash?)"
     end
   end
 end

lib/scelint/cli.rb

@@ -14,7 +14,7 @@ class Scelint::CLI < Thor
   option :strict, type: :boolean, aliases: '-s', default: false
   def lint(*paths)
     paths = ['.'] if paths.nil? || paths.empty?
-    lint = Scelint::Lint.new(paths)
+    lint = Scelint::Lint.new(paths, logger: logger)
 
     count = lint.files.count
 
@@ -31,6 +31,10 @@ def lint(*paths)
       logger.warn warning
     end
 
+    lint.notes.each do |note|
+      logger.info note
+    end
+
     message = "Checked #{count} files."
     if lint.errors.count == 0
       message += '  No errors.'
@@ -45,6 +49,8 @@ def lint(*paths)
       exit_code = 1 if options[:strict]
     end
 
+    message += " #{lint.notes.count} notes." if lint.notes.count > 0
+
     logger.info message
     exit exit_code
   rescue => e

lib/scelint/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Scelint
-  VERSION = '0.2.0'
+  VERSION = '0.3.0'
 end

scelint.gemspec

@@ -24,6 +24,8 @@ Gem::Specification.new do |spec|
   spec.executables   = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
   spec.require_paths = ['lib']
 
-  spec.add_runtime_dependency 'deep_merge', '~> 1.2'
-  spec.add_runtime_dependency 'thor', '~> 1.3'
+  spec.add_dependency 'deep_merge', '~> 1.2'
+  spec.add_dependency 'thor', '~> 1.3'
+  # FIXME: Add dependency on compliance_engine once it has been released
+  # spec.add_dependency 'compliance_engine', '~> 0.1.0'
 end

spec/fixtures/modules/test_module_11/SIMP/compliance_profiles/ces.yaml

@@ -0,0 +1,6 @@
+---
+version: 2.0.0
+ce:
+  11_ce1:
+    controls:
+      11_control1: true

spec/fixtures/modules/test_module_11/SIMP/compliance_profiles/checks.yaml

@@ -0,0 +1,17 @@
+---
+version: 2.0.0
+checks:
+  11_check1:
+    type: puppet-class-parameter
+    settings:
+      parameter: test_module_11::test_param
+      value: a string
+    ces:
+    - 11_ce1
+  11_check2:
+    type: puppet-class-parameter
+    settings:
+      parameter: test_module_11::test_param2
+      value: another string
+    ces:
+    - 11_ce1

spec/unit/scelint/lint_spec.rb

@@ -3,11 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Scelint::Lint do
-  # Each test assumes 3 files, no errors, no warnings.
+  # Each test assumes 3 files, no errors, no warnings, no notes.
   # Exceptions are listed below.
-  let(:lint_files) { { '04' => 37 } }
+  let(:lint_files) { { '04' => 37, '11' => 2 } }
   let(:lint_errors) { {} }
   let(:lint_warnings) { { '04' => 17 } }
+  let(:lint_notes) { { '11' => 1 } }
 
   test_modules = Dir.glob(File.join(File.expand_path('../../fixtures', __dir__), 'modules', 'test_module_*'))
   test_modules.each do |test_module|
@@ -26,13 +27,6 @@
         expect(lint.files.count).to eq(lint_files[index] || 3)
       end
 
-      it 'has the expected data' do
-        lint.files.each do |file|
-          require 'yaml'
-          expect(lint.data[file]).to eq(YAML.safe_load(File.read(file)))
-        end
-      end
-
       it 'has expected errors' do
         expect(lint.errors).to be_instance_of(Array)
         pp lint.errors if lint.errors.count != (lint_errors[index] || 0)
@@ -44,6 +38,12 @@
         pp lint.warnings if lint.warnings.count != (lint_warnings[index] || 0)
         expect(lint.warnings.count).to eq(lint_warnings[index] || 0)
       end
+
+      it 'has expected notes' do
+        expect(lint.notes).to be_instance_of(Array)
+        pp lint.notes if lint.notes.count != (lint_notes[index] || 0)
+        expect(lint.notes.count).to eq(lint_notes[index] || 0)
+      end
     end
   end
 
@@ -68,6 +68,12 @@
         lint_warnings[index] || 0
       end
     end
+    let(:total_notes) do
+      test_modules.sum do |test_module|
+        index = File.basename(test_module).delete_prefix('test_module_')
+        lint_notes[index] || 0
+      end
+    end
 
     it 'initializes' do
       expect(lint).to be_instance_of(described_class)
@@ -90,5 +96,12 @@
       pp lint.warnings if lint.warnings.count != total_warnings
       expect(lint.warnings.count).to eq(total_warnings)
     end
+
+    it 'has expected notes' do
+      pending "The number of notes isn't a simple addition."
+      expect(lint.notes).to be_instance_of(Array)
+      pp lint.notes if lint.notes.count != total_notes
+      expect(lint.notes.count).to eq(total_notes)
+    end
   end
 end