diff --git a/lib/cfn-nag/cfn_nag_config.rb b/lib/cfn-nag/cfn_nag_config.rb index c806d449..39977e1f 100644 --- a/lib/cfn-nag/cfn_nag_config.rb +++ b/lib/cfn-nag/cfn_nag_config.rb @@ -11,14 +11,16 @@ def initialize(profile_definition: nil, fail_on_warnings: false, ignore_fatal: false, rule_repository_definitions: [], - rule_arguments: {}) + rule_arguments: {}, + rule_directory_recursive: false) @rule_directory = rule_directory @custom_rule_loader = CustomRuleLoader.new( rule_directory: rule_directory, allow_suppression: allow_suppression, print_suppression: print_suppression, isolate_custom_rule_exceptions: isolate_custom_rule_exceptions, - rule_repository_definitions: rule_repository_definitions + rule_repository_definitions: rule_repository_definitions, + rule_directory_recursive: rule_directory_recursive ) @profile_definition = profile_definition @deny_list_definition = deny_list_definition diff --git a/lib/cfn-nag/cfn_nag_executor.rb b/lib/cfn-nag/cfn_nag_executor.rb index ba30c800..58a3420c 100644 --- a/lib/cfn-nag/cfn_nag_executor.rb +++ b/lib/cfn-nag/cfn_nag_executor.rb @@ -130,7 +130,8 @@ def cfn_nag_config(opts) fail_on_warnings: opts[:fail_on_warnings], rule_repository_definitions: @rule_repository_definitions, ignore_fatal: opts[:ignore_fatal], - rule_arguments: merge_rule_arguments(opts) + rule_arguments: merge_rule_arguments(opts), + rule_directory_recursive: opts[:rule_directory_recursive] ) end diff --git a/lib/cfn-nag/cli_options.rb b/lib/cfn-nag/cli_options.rb index 0881c351..31382b96 100644 --- a/lib/cfn-nag/cli_options.rb +++ b/lib/cfn-nag/cli_options.rb @@ -54,6 +54,11 @@ def self.file_options type: :string, required: false, default: nil + opt :rule_directory_recursive, + 'Recursively search extra rule directory', + type: :boolean, + required: false, + default: false opt :profile_path, 'Path to a profile file', type: :string, diff --git a/lib/cfn-nag/custom_rule_loader.rb b/lib/cfn-nag/custom_rule_loader.rb index e82e22a6..04b8a7e7 100644 --- a/lib/cfn-nag/custom_rule_loader.rb +++ b/lib/cfn-nag/custom_rule_loader.rb @@ -27,12 +27,14 @@ def initialize(rule_directory: nil, allow_suppression: true, print_suppression: false, isolate_custom_rule_exceptions: false, - rule_repository_definitions: []) + rule_repository_definitions: [], + rule_directory_recursive: false) @rule_directory = rule_directory @allow_suppression = allow_suppression @print_suppression = print_suppression @isolate_custom_rule_exceptions = isolate_custom_rule_exceptions @rule_repository_definitions = rule_repository_definitions + @rule_directory_recursive = rule_directory_recursive @registry = nil end @@ -43,7 +45,8 @@ def initialize(rule_directory: nil, # def rule_definitions(force_refresh: false) if @registry.nil? || force_refresh - @registry = FileBasedRuleRepo.new(@rule_directory).discover_rules + @registry = FileBasedRuleRepo.new(@rule_directory, + rule_directory_recursive: @rule_directory_recursive).discover_rules @registry.merge! GemBasedRuleRepo.new.discover_rules @registry = RuleRepositoryLoader.new.merge(@registry, @rule_repository_definitions) diff --git a/lib/cfn-nag/rule_repos/file_based_rule_repo.rb b/lib/cfn-nag/rule_repos/file_based_rule_repo.rb index 1f704668..65e8fe34 100644 --- a/lib/cfn-nag/rule_repos/file_based_rule_repo.rb +++ b/lib/cfn-nag/rule_repos/file_based_rule_repo.rb @@ -8,8 +8,9 @@ # client's choosing # class FileBasedRuleRepo - def initialize(rule_directory) + def initialize(rule_directory, rule_directory_recursive: false) @rule_directory = rule_directory + @rule_directory_recursive = rule_directory_recursive validate_extra_rule_directory rule_directory end @@ -19,7 +20,8 @@ def discover_rules # we look on the file system, and we load from the file system into a Class # that the runtime can refer back to later from the registry which is effectively # just a set of rule definitons - discover_rule_classes(@rule_directory).each do |rule_class| + discover_rule_classes(@rule_directory, + rule_directory_recursive: @rule_directory_recursive).each do |rule_class| rule_registry.definition(rule_class) end @@ -34,12 +36,18 @@ def validate_extra_rule_directory(rule_directory) raise "Not a real directory #{rule_directory}" end - def discover_rule_filenames(rule_directory) + def locate_rule_files(rule_directory, rule_directory_recursive) + return Dir.glob(File.join(rule_directory, '**/*Rule.rb')).sort if rule_directory_recursive + + Dir[File.join(rule_directory, '*Rule.rb')].sort + end + + def discover_rule_filenames(rule_directory, rule_directory_recursive: false) rule_filenames = [] unless rule_directory.nil? - rule_filenames += Dir[File.join(rule_directory, '*Rule.rb')].sort + rule_filenames += locate_rule_files(rule_directory, rule_directory_recursive) end - rule_filenames += Dir[File.join(__dir__, '..', 'custom_rules', '*Rule.rb')].sort + rule_filenames += locate_rule_files(File.join(__dir__, '..', 'custom_rules'), rule_directory_recursive) # Windows fix when running ruby from Command Prompt and not bash rule_filenames.reject! { |filename| filename =~ /_rule.rb$/ } @@ -47,10 +55,13 @@ def discover_rule_filenames(rule_directory) rule_filenames end - def discover_rule_classes(rule_directory) + def discover_rule_classes(rule_directory, rule_directory_recursive: false) rule_classes = [] - rule_filenames = discover_rule_filenames(rule_directory) + rule_filenames = discover_rule_filenames( + rule_directory, + rule_directory_recursive: rule_directory_recursive + ) rule_filenames.each do |rule_filename| require(File.absolute_path(rule_filename)) rule_classname = File.basename(rule_filename, '.rb') diff --git a/spec/rules_repos/file_base_rule_repo_spec.rb b/spec/rules_repos/file_base_rule_repo_spec.rb index 525a756e..a9c53a08 100644 --- a/spec/rules_repos/file_base_rule_repo_spec.rb +++ b/spec/rules_repos/file_base_rule_repo_spec.rb @@ -31,6 +31,30 @@ def audit_impl(cfn_model) RULE end + let(:valid_rule_text2) do + <<~RULE + require 'cfn-nag/custom_rules/base' + require 'cfn-nag/violation' + class ValidCustom2Rule < BaseRule + def rule_text + 'this is fake rule text 2' + end + + def rule_type + Violation::WARNING + end + + def rule_id + 'W9934' + end + + def audit_impl(cfn_model) + %w(hardwired1 hardwired2) + end + end + RULE + end + it 'includes external rule definition from absolute directories' do Dir.mktmpdir(%w[custom_rule loader]) do |custom_rule_directory| # Write out a valid rule @@ -88,6 +112,50 @@ def audit_impl(cfn_model) expect(actual_rule_classes.include?(expected_rule_classes)).to be true end end + + it 'includes external rule definitions from subdirectories' do + Dir.mktmpdir(%w[custom_rule loader], Dir.getwd) do |custom_rule_directory| + # Write out a valid rule + File.write(File.join(custom_rule_directory, 'ValidCustomRule.rb'), valid_rule_text) + # Create a subdirectory for rules + rule_subdir = File.join(custom_rule_directory, 'subdir') + Dir.mkdir(rule_subdir) + # Write a rule in a subdirectory + File.write(File.join(rule_subdir, 'ValidCustom2Rule.rb'), valid_rule_text2) + # Write out a invalid rule + File.write(File.join(custom_rule_directory, 'InvalidRuleNotMatching.rb'), 'fake_rule') + + core_rules_registry = FileBasedRuleRepo.new(nil).discover_rules + actual_rule_registry = FileBasedRuleRepo.new(File.basename(custom_rule_directory), + rule_directory_recursive: true).discover_rules + + # Expect one additional rule loaded + expect(actual_rule_registry.rules.size).to eq(core_rules_registry.rules.size+2) + + # Validate that the rule was loaded by id + expected_rule_definition = RuleDefinition.new id: 'W9933', + name: 'ValidCustomRule', + message: 'this is fake rule text', + type: RuleDefinition::WARNING + + actual_rule_definition = actual_rule_registry.by_id 'W9933' + expect(actual_rule_definition).to eq expected_rule_definition + + # Validate that the rule was loaded by id + expected_rule_definition = RuleDefinition.new id: 'W9934', + name: 'ValidCustom2Rule', + message: 'this is fake rule text 2', + type: RuleDefinition::WARNING + + actual_rule_definition = actual_rule_registry.by_id 'W9934' + expect(actual_rule_definition).to eq expected_rule_definition + + # Validate that the rule class was mapped correctly + actual_rule_classes = actual_rule_registry.rule_classes.map { |rule_class| rule_class.name } + expect(actual_rule_classes.include?('ValidCustomRule')).to be true + expect(actual_rule_classes.include?('ValidCustom2Rule')).to be true + end + end end end end