(PDK-1615) Add validator for environment.conf

lib/pdk/validate.rb

@@ -8,15 +8,22 @@ module Validate
     autoload :Validator, 'pdk/validate/validator'
     autoload :ValidatorGroup, 'pdk/validate/validator_group'
 
+    module ControlRepo
+      autoload :ControlRepoValidatorGroup, 'pdk/validate/control_repo/control_repo_validator_group'
+      autoload :EnvironmentConfValidator, 'pdk/validate/control_repo/environment_conf_validator'
+    end
+
     module Metadata
       autoload :MetadataJSONLintValidator, 'pdk/validate/metadata/metadata_json_lint_validator'
       autoload :MetadataSyntaxValidator, 'pdk/validate/metadata/metadata_syntax_validator'
       autoload :MetadataValidatorGroup, 'pdk/validate/metadata/metadata_validator_group'
     end
 
-    module YAML
-      autoload :YAMLSyntaxValidator, 'pdk/validate/yaml/yaml_syntax_validator'
-      autoload :YAMLValidatorGroup, 'pdk/validate/yaml/yaml_validator_group'
+    module Puppet
+      autoload :PuppetEPPValidator, 'pdk/validate/puppet/puppet_epp_validator'
+      autoload :PuppetLintValidator, 'pdk/validate/puppet/puppet_lint_validator'
+      autoload :PuppetSyntaxValidator, 'pdk/validate/puppet/puppet_syntax_validator'
+      autoload :PuppetValidatorGroup, 'pdk/validate/puppet/puppet_validator_group'
     end
 
     module Ruby
@@ -25,16 +32,14 @@ module Ruby
     end
 
     module Tasks
-      autoload :TasksNameValidator, 'pdk/validate/tasks/tasks_name_validator'
       autoload :TasksMetadataLintValidator, 'pdk/validate/tasks/tasks_metadata_lint_validator'
+      autoload :TasksNameValidator, 'pdk/validate/tasks/tasks_name_validator'
       autoload :TasksValidatorGroup, 'pdk/validate/tasks/tasks_validator_group'
     end
 
-    module Puppet
-      autoload :PuppetEPPValidator, 'pdk/validate/puppet/puppet_epp_validator'
-      autoload :PuppetLintValidator, 'pdk/validate/puppet/puppet_lint_validator'
-      autoload :PuppetSyntaxValidator, 'pdk/validate/puppet/puppet_syntax_validator'
-      autoload :PuppetValidatorGroup, 'pdk/validate/puppet/puppet_validator_group'
+    module YAML
+      autoload :YAMLSyntaxValidator, 'pdk/validate/yaml/yaml_syntax_validator'
+      autoload :YAMLValidatorGroup, 'pdk/validate/yaml/yaml_validator_group'
     end
 
     def self.validators
@@ -47,8 +52,9 @@ def self.validator_names
 
     # @api private
     def self.validator_hash
-      # TODO: This isn't the most performant... But with only 5 items, it's fine
+      # TODO: This isn't the most performant... But with only 6 items, it's fine
       @validator_hash ||= [
+        ControlRepo::ControlRepoValidatorGroup,
         Metadata::MetadataValidatorGroup,
         Puppet::PuppetValidatorGroup,
         Ruby::RubyValidatorGroup,

lib/pdk/validate/control_repo/control_repo_validator_group.rb

@@ -0,0 +1,23 @@
+require 'pdk'
+
+module PDK
+  module Validate
+    module ControlRepo
+      class ControlRepoValidatorGroup < ValidatorGroup
+        def name
+          'control-repo'
+        end
+
+        def valid_in_context?
+          context.is_a?(PDK::Context::ControlRepo)
+        end
+
+        def validators
+          [
+            EnvironmentConfValidator,
+          ].freeze
+        end
+      end
+    end
+  end
+end

lib/pdk/validate/control_repo/environment_conf_validator.rb

@@ -0,0 +1,98 @@
+require 'pdk'
+
+module PDK
+  module Validate
+    module ControlRepo
+      class EnvironmentConfValidator < InternalRubyValidator
+        ALLOWED_SETTINGS = %w[modulepath manifest config_version environment_timeout].freeze
+
+        def name
+          '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

spec/support/valid_in_context.rb

@@ -0,0 +1,19 @@
+RSpec.shared_examples_for 'only valid in specified PDK contexts' do |*context_class|
+  describe '.valid_in_context?' do
+    [
+      PDK::Context::None.new(nil),
+      PDK::Context::Module.new(nil, nil),
+      PDK::Context::ControlRepo.new(nil, nil),
+    ].each do |pdk_context|
+      context "in #{pdk_context.display_name}" do
+        subject { described_class.new(pdk_context, {}).valid_in_context? }
+
+        if context_class.include?(pdk_context.class)
+          it { is_expected.to eq(true) }
+        else
+          it { is_expected.to eq(false) }
+        end
+      end
+    end
+  end
+end

spec/unit/pdk/validate/control_repo/control_repo_validator_group_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+require 'pdk/validate/control_repo/control_repo_validator_group'
+
+describe PDK::Validate::ControlRepo::ControlRepoValidatorGroup do
+  subject(:validator) { described_class.new(validator_context, validator_options) }
+
+  let(:validator_context) { nil }
+  let(:validator_options) { {} }
+
+  describe '.name' do
+    subject { validator.name }
+
+    it { is_expected.to eq('control-repo') }
+  end
+
+  it_behaves_like 'only valid in specified PDK contexts', PDK::Context::ControlRepo
+
+  describe '.validators' do
+    subject { validator.validators }
+
+    it { is_expected.not_to be_empty }
+  end
+end

spec/unit/pdk/validate/control_repo/environment_conf_validator_spec.rb

@@ -0,0 +1,162 @@
+require 'spec_helper'
+require 'pdk/validate/control_repo/environment_conf_validator'
+
+describe PDK::Validate::ControlRepo::EnvironmentConfValidator 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('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
+
+  it_behaves_like 'only valid in specified PDK contexts', PDK::Context::ControlRepo
+
+  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