feat(scan): add rule directory recursive (#596)

Author: dclayton-godaddy

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

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
 

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,

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)

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,23 +36,32 @@ 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$/ }
     Logging.logger['log'].debug "rule_filenames: #{rule_filenames}"
     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')

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