diff --git a/Gemfile b/Gemfile index 66a69c37..94d8d3e8 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,8 @@ else end gem 'puppet-strings', '>= 1.2.1' +# https://github.com/theforeman/kafo_parsers/pull/49 +gem 'kafo_parsers', github: 'ekohl/kafo_parsers', branch: 'bulk-strings' group :puppet_module do gem 'metadata-json-lint' diff --git a/lib/kafo/configuration.rb b/lib/kafo/configuration.rb index d873b7bb..8a1d9baf 100644 --- a/lib/kafo/configuration.rb +++ b/lib/kafo/configuration.rb @@ -1,6 +1,8 @@ # encoding: UTF-8 require 'yaml' require 'tmpdir' +require 'kafo/parser_cache_reader' +require 'kafo_parsers/parsers' require 'kafo/puppet_module' require 'kafo/color_scheme' require 'kafo/data_type_parser' @@ -159,6 +161,7 @@ def kafo_modules_dir end def add_module(name) + # TOOD: invalidate parser? mod = PuppetModule.new(name, configuration: self).parse unless modules.map(&:name).include?(mod.name) mod.enable @@ -343,6 +346,14 @@ def parser_cache end end + # Parsed data for a PuppetModule + # @param [Kafo::PuppetModule] mod + # The module to parse. It's retrieved from the cache if available + # @return [Hash] The raw data returned from KafoParsers + def parsed(mod) + parser_cache&.get(mod.identifier, mod.manifest_path) || parser.by_name(mod.class_name) + end + private def custom_storage @@ -399,5 +410,32 @@ def register_data_types end end end + + def paths_to_parse + types = module_dirs.map { |module_dir| File.join(module_dir, '*', 'types', '**', '*.pp') } + # TODO: limit to @data.keys after mapping it + manifests = module_dirs.map { |module_dir| File.join(module_dir, '*', 'manifests', '**', '*.pp') } + + manifests + types + end + + def parser + @parser ||= begin + parser = KafoParsers::Parsers.find_available(:logger => @logger) + if parser + @logger.debug "Using Puppet module parser #{parser}" + parser.new(paths_to_parse) + else + @logger.debug "No available Puppet module parser found" + :none # prevent continually re-checking + end + end + + if @parser == :none + raise ParserError.new("No Puppet module parser is installed and no cache of the file #{manifest_path} is available. Please check debug logs and install optional dependencies for the parser.") + end + + @parser + end end end diff --git a/lib/kafo/puppet_module.rb b/lib/kafo/puppet_module.rb index c51d15b1..cd2c9c42 100644 --- a/lib/kafo/puppet_module.rb +++ b/lib/kafo/puppet_module.rb @@ -1,8 +1,6 @@ # encoding: UTF-8 require 'kafo/param' require 'kafo/param_builder' -require 'kafo/parser_cache_reader' -require 'kafo_parsers/parsers' module Kafo class PuppetModule @@ -11,21 +9,7 @@ class PuppetModule attr_reader :name, :identifier, :params, :dir_name, :class_name, :manifest_name, :manifest_path, :groups, :params_path, :params_class_name, :configuration, :raw_data - def self.find_parser - @parser ||= begin - logger = KafoConfigure.logger - parser = KafoParsers::Parsers.find_available(:logger => logger) - if parser - logger.debug "Using Puppet module parser #{parser}" - parser - else - logger.debug "No available Puppet module parser found" - :none # prevent continually re-checking - end - end - end - - def initialize(identifier, parser: nil, configuration: KafoConfigure.config) + def initialize(identifier, configuration: KafoConfigure.config) @identifier = identifier @configuration = configuration @name = get_name @@ -40,8 +24,6 @@ def initialize(identifier, parser: nil, configuration: KafoConfigure.config) warn("Manifest #{module_manifest_path} was not found in #{@configuration.module_dirs.join(', ')}") end @manifest_path = File.join(module_dir, module_manifest_path) - @parser = parser - @parser_cache = @configuration.parser_cache @logger = KafoConfigure.logger @groups = {} @params_path = get_params_path @@ -63,15 +45,7 @@ def enable end def parse(builder_klass = ParamBuilder) - @raw_data = @parser_cache.get(identifier, manifest_path) if @parser_cache - if @raw_data.nil? - @parser = self.class.find_parser if @parser.nil? - if @parser.nil? || @parser == :none - raise ParserError.new("No Puppet module parser is installed and no cache of the file #{manifest_path} is available. Please check debug logs and install optional dependencies for the parser.") - else - @raw_data = @parser.parse(manifest_path) - end - end + @raw_data = @configuration.parsed(self) builder = builder_klass.new(self, @raw_data) @@ -81,7 +55,7 @@ def parse(builder_klass = ParamBuilder) self rescue ConfigurationException => e - @logger.fatal "Unable to continue because of: #{e.message}" + @logger.fatal "Unable to parse #{manifest_path} because of: #{e.message}" KafoConfigure.exit(:manifest_error) end @@ -128,12 +102,12 @@ def mapping # custom module directory name def get_dir_name - mapping[identifier].nil? ? default_dir_name : (mapping[identifier][:dir_name] || default_dir_name) + mapping.dig(identifier, :dir_name) || default_dir_name end # custom manifest filename without .pp extension def get_manifest_name - mapping[identifier].nil? ? default_manifest_name : (mapping[identifier][:manifest_name] || default_manifest_name) + mapping.dig(identifier, :manifest_name) || default_manifest_name end def get_class_name @@ -141,11 +115,11 @@ def get_class_name end def get_params_path - mapping[identifier].nil? ? default_params_path : (mapping[identifier][:params_path] || default_params_path) + mapping.dig(identifier, :params_path) || default_params_path end def get_params_name - mapping[identifier].nil? ? default_params_name : (mapping[identifier][:params_name] || default_params_name) + mapping.dig(identifier, :params_name) || default_params_name end def get_params_class_name diff --git a/test/fixtures/basic_answers.yaml b/test/fixtures/basic_answers.yaml index accae097..df2db51a 100644 --- a/test/fixtures/basic_answers.yaml +++ b/test/fixtures/basic_answers.yaml @@ -1 +1 @@ -puppet: true +testing: true diff --git a/test/kafo/configuration_test.rb b/test/kafo/configuration_test.rb index 85576c1c..aa771ae5 100644 --- a/test/kafo/configuration_test.rb +++ b/test/kafo/configuration_test.rb @@ -212,6 +212,46 @@ module Kafo end end end + + describe '#parse' do + describe "with parser cache" do + let(:module_name) { 'testing' } + let(:parser_cache_factory) do + ParserCacheFactory.build({:files => {module_name => {:data => {:parameters => [], :groups => []}}}}) + end + before do + config.app[:parser_cache_path] = parser_cache_factory.path + end + subject { config.parse(module_name, '/path/to/manifest' } + + specify { _(subject.raw_data[:parameters]).must_equal [] } + specify { _(subject.raw_data[:groups]).must_equal [] } + end + end + + describe '#parser' do + subject { config.send(:parser) } + + describe "with nil parser and no cache" do + before { config.instance_variable_set(:@parser, nil) } + + it do + KafoParsers::Parsers.stub(:find_available, nil) do + _(subject).must_raise ParserError + end + end + end + + describe "with :none parser cached" do + before { config.instance_variable_set(:@parser, :none) } + + specify do + KafoParsers::Parsers.stub(:find_available, 'something') do + _(config.parser).must_raise ParserError + end + end + end + end end end end diff --git a/test/kafo/parser_cache_reader_test.rb b/test/kafo/parser_cache_reader_test.rb index 3cacef28..7bd4e649 100644 --- a/test/kafo/parser_cache_reader_test.rb +++ b/test/kafo/parser_cache_reader_test.rb @@ -59,11 +59,13 @@ module Kafo describe "compatibility with writer" do before do - KafoConfigure.config = Configuration.new(ConfigFileFactory.build('basic', BASIC_CONFIGURATION).path) + KafoConfigure.config = config + config.stub(:parser, parser) end + let(:config) { Configuration.new(ConfigFileFactory.build('basic', BASIC_CONFIGURATION).path) } let(:parser) { TestParser.new(BASIC_MANIFEST) } - let(:mod) { PuppetModule.new('puppet', parser: parser) } + let(:mod) { PuppetModule.new('puppet') } let(:writer) { ParserCacheWriter.write([mod]) } let(:cache) { ParserCacheFactory.build(writer) } diff --git a/test/kafo/puppet_module_test.rb b/test/kafo/puppet_module_test.rb index 122c634f..8de25c5f 100644 --- a/test/kafo/puppet_module_test.rb +++ b/test/kafo/puppet_module_test.rb @@ -2,13 +2,16 @@ module Kafo describe PuppetModule do + let(:config) { Configuration.new(ConfigFileFactory.build('basic', BASIC_CONFIGURATION).path) } + let(:module_name) { 'testing' } + let(:manifest) { BASIC_MANIFEST } + let(:parser) { TestParser.new(manifest) } + let(:mod) { PuppetModule.new(module_name, configuration: config) } + before do - KafoConfigure.config = Configuration.new(ConfigFileFactory.build('basic', BASIC_CONFIGURATION).path) + config.instance_variable_set(:@parser, parser) end - let(:parser) { TestParser.new(BASIC_MANIFEST) } - let(:mod) { PuppetModule.new 'puppet', parser: parser } - describe "#enabled?" do specify { _(mod.enabled?).must_equal true } end @@ -24,21 +27,21 @@ module Kafo end # Uses default Puppet autoloader locations - let(:plugin1_mod) { PuppetModule.new 'foreman::plugin::default_hostgroup', parser: parser } + let(:plugin1_mod) { PuppetModule.new('foreman::plugin::default_hostgroup', configuration: config) } # BASIC_CONFIGURATION has mapping configured for this module - let(:plugin2_mod) { PuppetModule.new 'foreman::plugin::chef', parser: parser } + let(:plugin2_mod) { PuppetModule.new('foreman::plugin::chef', configuration: config) } # BASIC_CONFIGURATION has mapping configured for this module - let(:certs_mod) { PuppetModule.new 'certs', parser: parser } + let(:certs_mod) { PuppetModule.new('certs', configuration: config) } describe "#name" do - specify { _(mod.name).must_equal 'puppet' } + specify { _(mod.name).must_equal 'testing' } specify { _(plugin1_mod.name).must_equal 'foreman_plugin_default_hostgroup' } specify { _(plugin2_mod.name).must_equal 'foreman_plugin_chef' } specify { _(certs_mod.name).must_equal 'certs' } end describe "#dir_name" do - specify { _(mod.dir_name).must_equal 'puppet' } + specify { _(mod.dir_name).must_equal 'testing' } specify { _(plugin1_mod.dir_name).must_equal 'foreman' } specify { _(plugin2_mod.dir_name).must_equal 'custom' } specify { _(certs_mod.dir_name).must_equal 'certificates' } @@ -52,21 +55,21 @@ module Kafo end describe "#class_name" do - specify { _(mod.class_name).must_equal 'puppet' } + specify { _(mod.class_name).must_equal 'testing' } specify { _(plugin1_mod.class_name).must_equal 'foreman::plugin::default_hostgroup' } specify { _(plugin2_mod.class_name).must_equal 'custom::plugin::custom_chef' } specify { _(certs_mod.class_name).must_equal 'certificates::foreman' } end describe "#manifest_path" do - specify { _(mod.manifest_path).must_match %r"test/fixtures/modules/puppet/manifests/init\.pp$" } + specify { _(mod.manifest_path).must_match %r"test/fixtures/modules/testing/manifests/init\.pp$" } specify { _(plugin1_mod.manifest_path).must_match %r"test/fixtures/modules/foreman/manifests/plugin/default_hostgroup\.pp$" } specify { _(plugin2_mod.manifest_path).must_match %r"test/fixtures/modules/custom/manifests/plugin/custom_chef\.pp$" } specify { _(certs_mod.manifest_path).must_match %r"test/fixtures/modules/certificates/manifests/foreman\.pp$" } end describe "#params_path" do - specify { _(mod.params_path).must_equal 'puppet/manifests/params.pp' } + specify { _(mod.params_path).must_equal 'testing/manifests/params.pp' } specify { _(plugin1_mod.params_path).must_equal 'foreman/manifests/plugin/default_hostgroup/params.pp' } specify { _(plugin2_mod.params_path).must_equal 'custom/plugin/chef/params.pp' } specify { _(certs_mod.params_path).must_equal 'certificates/manifests/foreman/params.pp' } @@ -80,166 +83,137 @@ module Kafo end describe "#raw_data" do + before { mod.parse } + subject { mod.raw_data } + it "returns data from parser" do - mod.parse - _(mod.raw_data).must_equal parser.parse(mod.manifest_path) + _(subject).must_equal parser.parse(mod.manifest_path) end end - let(:parsed) { mod.parse } + describe "#parse" do + subject { mod.parse } - describe "#parse(builder)" do describe "without documentation" do + let(:manifest) { NO_DOC_MANIFEST } before do - KafoConfigure.config.app[:ignore_undocumented] = true + config.app[:ignore_undocumented] = true end - let(:mod) { PuppetModule.new 'puppet', parser: TestParser.new(NO_DOC_MANIFEST) } - let(:docs) { parsed.params.map(&:doc) } - specify { docs.each { |doc| _(doc).must_be_nil } } + specify { subject.params.map(&:doc).each { |doc| _(doc).must_be_nil } } end describe "with not ignoring docs inconsitency" do before do - KafoConfigure.config.app[:ignore_undocumented] = false + config.app[:ignore_undocumented] = false end describe "undocumented params" do it "does throw an error" do KafoConfigure.stub(:exit, 'expected to exit') do - _(mod.parse).must_equal 'expected to exit' + _(subject).must_equal 'expected to exit' end end end end + end - describe "with parser cache" do - before do - KafoConfigure.config.app[:parser_cache_path] = ParserCacheFactory.build( - {:files => {"puppet" => {:data => {:parameters => [], :groups => []}}}} - ).path - @@parsed_cache_with_cache ||= parsed - end + describe '#groups' do + before { mod.parse } + subject { mod.groups } - specify { _(@@parsed_cache_with_cache.raw_data[:parameters]).must_equal [] } - specify { _(@@parsed_cache_with_cache.raw_data[:groups]).must_equal [] } + specify 'names' do + _(subject.map(&:name)).must_equal(['Parameters', 'Advanced parameters', 'Extra parameters']) end + end - describe "with nil parser and no cache" do - let(:parser) { nil } + describe "#primary_parameter_group" do + before { mod.parse } + subject { mod.primary_parameter_group } - it do - PuppetModule.stub(:find_parser, nil) do - _(Proc.new { parsed }).must_raise ParserError - end - end + specify 'params' do + _(subject.params.map(&:name)).must_equal(['version', 'undef', 'multiline', 'typed', 'multivalue']) end - describe "without :none parser and no cache" do - let(:parser) { :none } - specify { _(Proc.new { parsed }).must_raise ParserError } + specify 'children' do + _(subject.children).must_be_empty end - describe "with groups" do - before { @@parsed_cache_with_groups ||= parsed } - let(:groups) { @@parsed_cache_with_groups.groups.map(&:name) } - specify { _(groups).must_include('Parameters') } - specify { _(groups).must_include('Advanced parameters') } - specify { _(groups).must_include('Extra parameters') } - specify { _(groups).wont_include('MySQL') } - specify { _(groups).wont_include('Sqlite') } + describe "manifest without primary group" do + let(:module_name) { 'testing2' } + let(:manifest) { MANIFEST_WITHOUT_PRIMARY_GROUP } + + specify 'params' do + _(subject.params).must_be_empty + end + + specify 'children' do + _(subject.children.map(&:name)).must_equal(['Basic parameters:', 'Advanced parameters:']) + end end - describe "parses parameter names" do - before { @@parsed_cache_parameter_names ||= parsed } - let(:param_names) { @@parsed_cache_parameter_names.params.map(&:name) } - specify { _(param_names).must_include('version') } - specify { _(param_names).must_include('debug') } - specify { _(param_names).must_include('remote') } - specify { _(param_names).must_include('file') } - specify { _(param_names).must_include('m_i_a') } + describe "manifest without any group" do + let(:module_name) { 'testing3' } + let(:manifest) { MANIFEST_WITHOUT_ANY_GROUP } + + specify 'params' do + _(subject.params.map(&:name)).must_equal(['version', 'documented']) + end + + specify 'children' do + _(subject.children).must_be_empty + end end end - describe "#primary_parameter_group" do - before { @@parsed_cache_primary ||= parsed } - let(:primary_params) { @@parsed_cache_primary.primary_parameter_group.params.map(&:name) } - specify { _(primary_params).must_include('version') } - specify { _(primary_params).must_include('undef') } - specify { _(primary_params).must_include('multiline') } - specify { _(primary_params).must_include('typed') } - specify { _(primary_params).wont_include('documented') } - specify { _(primary_params).wont_include('debug') } - specify { _(primary_params).wont_include('remote') } - - let(:other_groups) { parsed.other_parameter_groups } - let(:other_groups_names) { other_groups.map(&:name) } - specify { _(other_groups_names).must_include('Advanced parameters') } - specify { _(other_groups_names).must_include('Extra parameters') } - - let(:advanced_group) { other_groups.detect { |g| g.name == 'Advanced parameters' } } - specify { _(advanced_group.children).must_be_empty } - let(:advanced_params) { advanced_group.params.map(&:name) } - specify { _(advanced_params).must_include('debug') } - specify { _(advanced_params).must_include('db_type') } - specify { _(advanced_params).must_include('remote') } - specify { _(advanced_params).must_include('file') } - specify { _(advanced_params).wont_include('log_level') } + describe '#other_parameter_groups' do + before { mod.parse } + subject { mod.other_parameter_groups } - describe "manifest without primary group" do - let(:mod_wo_prim) { @@mod_wo_prim ||= PuppetModule.new('puppet', parser: TestParser.new(MANIFEST_WITHOUT_PRIMARY_GROUP)).parse } - let(:primary_group) { mod_wo_prim.primary_parameter_group } - specify { _(primary_group.params).must_be_empty } - let(:children_group_names) { primary_group.children.map(&:name) } - specify { _(children_group_names).must_include 'Basic parameters:' } - specify { _(children_group_names).must_include 'Advanced parameters:' } + specify 'names' do + _(subject.map(&:name)).must_equal(['Advanced parameters', 'Extra parameters']) end - describe "manifest without any group" do - let(:mod_wo_any) { @@mod_wo_any ||= PuppetModule.new('puppet', parser: TestParser.new(MANIFEST_WITHOUT_ANY_GROUP)).parse } - let(:primary_group) { mod_wo_any.primary_parameter_group } - let(:primary_params) { primary_group.params } - specify { _(primary_params).wont_be_empty } - let(:primary_param_names) { primary_params.map(&:name) } - specify { _(primary_param_names).must_include 'version' } - specify { _(primary_param_names).must_include 'documented' } - specify { _(primary_group.children).must_be_empty } + describe 'advanced group' do + subject { mod.other_parameter_groups.detect { |g| g.name == 'Advanced parameters' } } + + specify { _(subject.children).must_be_empty } + it 'param names' do + _(subject.params.map(&:name)).must_equal(['debug', 'db_type', 'remote', 'server', 'username', 'pool_size', 'file']) + end + end + end + + describe "#params" do + before { mod.parse } + subject { mod.params } + + specify 'names' do + _(subject.map(&:name)).must_equal(['version', 'undocumented', 'undef', 'multiline', 'typed', 'multivalue', 'debug', 'db_type', 'remote', 'server', 'username', 'pool_size', 'file', 'm_i_a']) end end describe "#params_hash" do - before { @@parsed_cache_params_hash ||= parsed } - let(:params_hash) { @@parsed_cache_params_hash.params_hash } - let(:keys) { params_hash.keys } - specify { _(keys).must_include 'version' } - specify { _(keys).must_include 'undocumented' } - specify { _(keys).must_include 'undef' } - specify { _(keys).must_include 'multiline' } - specify { _(keys).must_include 'typed' } - specify { _(keys).must_include 'debug' } - specify { _(keys).must_include 'db_type' } - specify { _(keys).must_include 'remote' } - specify { _(keys).must_include 'file' } - specify { _(keys).must_include 'm_i_a' } - specify { _(keys).wont_include 'documented' } - - specify { _(params_hash['version']).must_equal '1.0' } - specify { _(params_hash['undef']).must_be_nil } + before { mod.parse } + subject { mod.params_hash } + + specify 'keys' do + _(subject.keys).must_equal(['version', 'undocumented', 'undef', 'multiline', 'typed', 'multivalue', 'debug', 'db_type', 'remote', 'server', 'username', 'pool_size', 'file', 'm_i_a']) + end + specify { _(subject['version']).must_equal '1.0' } + specify { _(subject['undef']).must_be_nil } end describe "#<=>" do - let(:a) { PuppetModule.new('a') } - let(:b) { PuppetModule.new('b') } - let(:c) { PuppetModule.new('c') } - let(:d) { PuppetModule.new('d') } + let(:a) { PuppetModule.new('a', configuration: config) } + let(:b) { PuppetModule.new('b', configuration: config) } + let(:c) { PuppetModule.new('c', configuration: config) } + let(:d) { PuppetModule.new('d', configuration: config) } let(:sorted) { [a, b, c, d] } - let(:unsorted_1) { [a, c, b, d] } - let(:unsorted_2) { [d, b, c, a] } - let(:unsorted_3) { [a, b, d, c] } - specify { _(unsorted_1.sort).must_equal sorted } - specify { _(unsorted_2.sort).must_equal sorted } - specify { _(unsorted_3.sort).must_equal sorted } + specify { _([a, c, b, d].sort).must_equal sorted } + specify { _([d, b, c, a].sort).must_equal sorted } + specify { _([a, b, d, c].sort).must_equal sorted } end end