(PDK-1615) Add validator for environment.conf

This commit adds a validator for the environment.conf file when being used
inside a control repository.  The validator complies with the rules set out in
https://puppet.com/docs/puppet/latest/config_file_environment.html.

This commit also adds tests for the validator setup, and various example file
content and expected validation results.

lib/pdk/validate.rb

@@ -31,6 +31,7 @@ module Tasks
     end
 
     module Puppet
+      autoload :PuppetEnvironmentConfValidator, 'pdk/validate/puppet/puppet_environment_conf_validator'
       autoload :PuppetEPPValidator, 'pdk/validate/puppet/puppet_epp_validator'
       autoload :PuppetLintValidator, 'pdk/validate/puppet/puppet_lint_validator'
       autoload :PuppetSyntaxValidator, 'pdk/validate/puppet/puppet_syntax_validator'

lib/pdk/validate/puppet/puppet_environment_conf_validator.rb

@@ -0,0 +1,98 @@
+require 'pdk'
+
+module PDK
+  module Validate
+    module Puppet
+      class PuppetEnvironmentConfValidator < InternalRubyValidator
+        ALLOWED_SETTINGS = %w[modulepath manifest config_version environment_timeout].freeze
+
+        def name
+          'puppet-environment-conf'
+        end
+
+        def valid_in_context?
+          context.is_a?(PDK::Context::ControlRepo)
+        end
+
+        def pattern
+          ['environment.conf']
+        end
+
+        def spinner_text
+          _('Checking Puppet Environment settings (%{patterns}).') % {
+            patterns: pattern.join(' '),
+          }
+        end
+
+        def validate_target(report, target)
+          unless PDK::Util::Filesystem.readable?(target)
+            report.add_event(
+              file: target,
+              source: name,
+              state: :failure,
+              severity: 'error',
+              message: _('Could not be read.'),
+            )
+            return 1
+          end
+
+          is_valid = true
+          begin
+            env_conf = PDK::ControlRepo.environment_conf_as_config(target)
+
+            env_conf.resolve.each do |setting_name, setting_value|
+              # Remove the 'environment.' setting_name prefix
+              setting_name = setting_name.slice(12..-1)
+              next if ALLOWED_SETTINGS.include?(setting_name)
+              # A hash indicates that the ini file has a section in it.
+              message = if setting_value.is_a?(Hash)
+                          _("Invalid section '%{name}'") % { name: setting_name }
+                        else
+                          _("Invalid setting '%{name}'") % { name: setting_name }
+                        end
+
+              report.add_event(
+                file:     target,
+                source:   name,
+                state:    :failure,
+                severity: 'error',
+                message:  message,
+              )
+              is_valid = false
+            end
+
+            timeout = env_conf.fetch('environment_timeout', nil)
+            unless timeout.nil? || timeout == '0' || timeout == 'unlimited'
+              report.add_event(
+                file:     target,
+                source:   name,
+                state:    :failure,
+                severity: 'error',
+                message:  _("environment_timeout is set to '%{timeout}' but should be 0, 'unlimited' or not set.") % { timeout: timeout },
+              )
+              is_valid = false
+            end
+
+            return 1 unless is_valid
+            report.add_event(
+              file:     target,
+              source:   name,
+              state:    :passed,
+              severity: 'ok',
+            )
+            return 0
+          rescue StandardError => e
+            report.add_event(
+              file:     target,
+              source:   name,
+              state:    :failure,
+              severity: 'error',
+              message:  e.message,
+            )
+            return 1
+          end
+        end
+      end
+    end
+  end
+end

lib/pdk/validate/puppet/puppet_validator_group.rb

@@ -13,6 +13,7 @@ def validators
             PuppetSyntaxValidator,
             PuppetLintValidator,
             PuppetEPPValidator,
+            PuppetEnvironmentConfValidator,
           ].freeze
         end
       end

spec/unit/pdk/validate/puppet/puppet_environment_conf_validator_spec.rb

@@ -0,0 +1,182 @@
+require 'spec_helper'
+require 'pdk/validate/puppet/puppet_environment_conf_validator'
+
+describe PDK::Validate::Puppet::PuppetEnvironmentConfValidator do
+  let(:validator) { described_class.new(validator_context, validator_options) }
+  let(:validator_context) { PDK::Context::ControlRepo.new(Dir.pwd, Dir.pwd) }
+  let(:validator_options) { {} }
+
+  describe '.name' do
+    subject { validator.name }
+
+    it { is_expected.to eq('puppet-environment-conf') }
+  end
+
+  describe '.spinner_text' do
+    subject(:spinner_text) { validator.spinner_text }
+
+    it { is_expected.to match(%r{Checking Puppet Environment settings}i) }
+  end
+
+  describe '.valid_in_context?' do
+    subject(:valid_context) { validator.valid_in_context? }
+
+    context 'in a Control Repo' do
+      let(:validator_context) { PDK::Context::ControlRepo.new(nil, nil) }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'in a Module' do
+      let(:validator_context) { PDK::Context::Module.new(nil, nil) }
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'in an unknown context' do
+      let(:validator_context) { PDK::Context::None.new(nil) }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '.validate_target' do
+    subject(:return_value) { described_class.new.validate_target(report, target[:name]) }
+
+    let(:report) { PDK::Report.new }
+
+    before(:each) do
+      [
+        target[:name],
+        File.join(validator_context.root_path, target[:name]),
+      ].each do |filename|
+        allow(PDK::Util::Filesystem).to receive(:directory?).with(filename).and_return(target.fetch(:directory, false))
+        allow(PDK::Util::Filesystem).to receive(:file?).with(filename).and_return(target.fetch(:file, true))
+        allow(PDK::Util::Filesystem).to receive(:readable?).with(filename).and_return(target.fetch(:readable, true))
+        allow(PDK::Util::Filesystem).to receive(:read_file).with(filename).and_return(target.fetch(:content, ''))
+      end
+    end
+
+    context 'when a target is provided that is an unreadable file' do
+      let(:target) { { name: 'environment.conf', readable: false } }
+
+      it 'adds a failure event to the report' do
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :failure,
+          severity: 'error',
+          message:  'Could not be read.',
+        )
+        expect(return_value).to eq(1)
+      end
+    end
+
+    context 'when a target is provided that is not a file' do
+      let(:target) { { name: 'environment.conf', file: false } }
+
+      it 'skips the target' do
+        expect(report).not_to receive(:add_event)
+      end
+    end
+
+    context 'when a target is provided that is empty' do
+      let(:target) { { name: 'environment.conf', content: '' } }
+
+      it 'adds a passing event to the report' do
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :passed,
+          severity: 'ok',
+        )
+        expect(return_value).to eq(0)
+      end
+    end
+
+    context 'when a target is provided that has all allowed settings and is valid' do
+      let(:target) do
+        {
+          name: 'environment.conf',
+          content: <<-EOT
+                   modulepath=foo
+                   manifest=foo
+                   config_version=foo
+                   environment_timeout=0
+                   EOT
+        }
+      end
+
+      it 'adds a passing event to the report' do
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :passed,
+          severity: 'ok',
+        )
+        expect(return_value).to eq(0)
+      end
+    end
+
+    context 'when a target is provided that has invalid settings and sections' do
+      let(:target) do
+        {
+          name: 'environment.conf',
+          content: <<-EOT
+                   modulepath=foo
+                   manifest=foo
+                   config_version=foo
+                   environment_timeout=foo
+                   invalid=true
+
+                   [invalid_section]
+                   invalid=true
+                   EOT
+        }
+      end
+
+      it 'does not add a passing event to the report' do
+        expect(report).not_to receive(:add_event).with(
+          state:    :passed,
+        )
+        expect(return_value).to eq(1)
+      end
+
+      it 'adds a invalid setting failures event to the report' do
+        allow(report).to receive(:add_event).and_call_original
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :failure,
+          severity: 'error',
+          message:  a_string_matching(%r{Invalid setting \'invalid\'}),
+        )
+        expect(return_value).to eq(1)
+      end
+
+      it 'adds a invalid section failures event to the report' do
+        allow(report).to receive(:add_event).and_call_original
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :failure,
+          severity: 'error',
+          message:  a_string_matching(%r{Invalid section \'invalid_section\'}),
+        )
+        expect(return_value).to eq(1)
+      end
+
+      it 'adds a invalid environment_timeout failures event to the report' do
+        allow(report).to receive(:add_event).and_call_original
+        expect(report).to receive(:add_event).with(
+          file:     target[:name],
+          source:   validator.name,
+          state:    :failure,
+          severity: 'error',
+          message:  a_string_matching(%r{environment_timeout is set to \'foo\' but should be}),
+        )
+        expect(return_value).to eq(1)
+      end
+    end
+  end
+end