#211 - Refactoring boolean rules to be more DRY

lib/cfn-nag/custom_rules/EFSFileSystemEncryptedRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class EFSFileSystemEncryptedRule < BaseRule
+class EFSFileSystemEncryptedRule < BooleanBaseRule
   def rule_text
     'EFS FileSystem should have encryption enabled'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F32'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::EFS::FileSystem')
-
-    violating_filesystems = resources.select do |filesystem|
-      filesystem.encrypted.nil? ||
-        filesystem.encrypted.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::EFS::FileSystem'
+  end
 
-    violating_filesystems.map(&:logical_resource_id)
+  def boolean_property
+    :encrypted
   end
 end

lib/cfn-nag/custom_rules/ElastiCacheReplicationGroupAtRestEncryptionRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class ElastiCacheReplicationGroupAtRestEncryptionRule < BaseRule
+class ElastiCacheReplicationGroupAtRestEncryptionRule < BooleanBaseRule
   def rule_text
     'ElastiCache ReplicationGroup should have encryption enabled for at rest'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F25'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::ElastiCache::ReplicationGroup')
-
-    violating_groups = resources.select do |group|
-      group.atRestEncryptionEnabled.nil? ||
-        group.atRestEncryptionEnabled.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::ElastiCache::ReplicationGroup'
+  end
 
-    violating_groups.map(&:logical_resource_id)
+  def boolean_property
+    :atRestEncryptionEnabled
   end
 end

lib/cfn-nag/custom_rules/ElastiCacheReplicationGroupTransitEncryptionRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class ElastiCacheReplicationGroupTransitEncryptionRule < BaseRule
+class ElastiCacheReplicationGroupTransitEncryptionRule < BooleanBaseRule
   def rule_text
     'ElastiCache ReplicationGroup should have encryption enabled for in transit'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F24'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::ElastiCache::ReplicationGroup')
-
-    violating_groups = resources.select do |group|
-      group.transitEncryptionEnabled.nil? ||
-        group.transitEncryptionEnabled.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::ElastiCache::ReplicationGroup'
+  end
 
-    violating_groups.map(&:logical_resource_id)
+  def boolean_property
+    :transitEncryptionEnabled
   end
 end

lib/cfn-nag/custom_rules/NeptuneDBClusterStorageEncryptedRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class NeptuneDBClusterStorageEncryptedRule < BaseRule
+class NeptuneDBClusterStorageEncryptedRule < BooleanBaseRule
   def rule_text
     'Neptune database cluster storage should have encryption enabled'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F30'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::Neptune::DBCluster')
-
-    violating_storage = resources.select do |filesystem|
-      filesystem.storageEncrypted.nil? ||
-        filesystem.storageEncrypted.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::Neptune::DBCluster'
+  end
 
-    violating_storage.map(&:logical_resource_id)
+  def boolean_property
+    :storageEncrypted
   end
 end

lib/cfn-nag/custom_rules/RDSDBClusterStorageEncryptedRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class RDSDBClusterStorageEncryptedRule < BaseRule
+class RDSDBClusterStorageEncryptedRule < BooleanBaseRule
   def rule_text
     'RDS DBCluster should have StorageEncrypted enabled'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F26'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::RDS::DBCluster')
-
-    violating_clusters = resources.select do |cluster|
-      cluster.storageEncrypted.nil? ||
-        cluster.storageEncrypted.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::RDS::DBCluster'
+  end
 
-    violating_clusters.map(&:logical_resource_id)
+  def boolean_property
+    :storageEncrypted
   end
 end

lib/cfn-nag/custom_rules/RDSDBInstanceStorageEncryptedRule.rb

@@ -20,11 +20,7 @@ def audit_impl(cfn_model)
     resources = cfn_model.resources_by_type('AWS::RDS::DBInstance')
 
     violating_instances = resources.select do |instance|
-      instance.dBClusterIdentifier.nil? &&
-        (
-          instance.storageEncrypted.nil? ||
-          instance.storageEncrypted.to_s.casecmp('false').zero?
-        )
+      instance.dBClusterIdentifier.nil? && not_truthy?(instance.storageEncrypted)
     end
 
     violating_instances.map(&:logical_resource_id)

lib/cfn-nag/custom_rules/RedshiftClusterEncryptedRule.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'cfn-nag/violation'
-require_relative 'base'
+require_relative 'boolean_base_rule'
 
-class RedshiftClusterEncryptedRule < BaseRule
+class RedshiftClusterEncryptedRule < BooleanBaseRule
   def rule_text
     'Redshift Cluster should have encryption enabled'
   end
@@ -16,14 +16,11 @@ def rule_id
     'F28'
   end
 
-  def audit_impl(cfn_model)
-    resources = cfn_model.resources_by_type('AWS::Redshift::Cluster')
-
-    violating_clusters = resources.select do |cluster|
-      cluster.encrypted.nil? ||
-        cluster.encrypted.to_s.casecmp('false').zero?
-    end
+  def resource_type
+    'AWS::Redshift::Cluster'
+  end
 
-    violating_clusters.map(&:logical_resource_id)
+  def boolean_property
+    :encrypted
   end
 end

lib/cfn-nag/custom_rules/boolean_base_rule.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+require 'cfn-nag/util/truthy.rb'
+
+class BooleanBaseRule < BaseRule
+  def resource_type
+    raise 'must implement in subclass'
+  end
+
+  def boolean_property
+    raise 'must implement in subclass'
+  end
+
+  def audit_impl(cfn_model)
+    resources = cfn_model.resources_by_type(resource_type)
+
+    violating_resources = resources.select do |resource|
+      not_truthy?(resource.send(boolean_property))
+    end
+
+    violating_resources.map(&:logical_resource_id)
+  end
+end

lib/cfn-nag/util/truthy.rb

@@ -5,3 +5,7 @@
 def truthy?(string)
   string.to_s.casecmp('true').zero?
 end
+
+def not_truthy?(string)
+  string.nil? || string.to_s.casecmp('false').zero?
+end

spec/custom_rules/boolean_base_rule_spec.rb

@@ -0,0 +1,82 @@
+require 'spec_helper'
+require 'cfn-model'
+require 'cfn-nag/custom_rules/boolean_base_rule'
+
+describe BooleanBaseRule do
+  describe '#audit' do
+    it 'raises an error when properties are not set' do
+      expect do
+        BooleanBaseRule.new.audit_impl nil
+      end.to raise_error 'must implement in subclass'
+    end
+
+    it 'returns violation when boolean value is false' do
+      base_rule = BooleanBaseRule.new
+      base_rule.instance_eval do
+        def rule_id
+          'F3333'
+        end
+
+        def rule_type
+          Violation::FAILING_VIOLATION
+        end
+
+        def rule_text
+          'This is an epic fail!'
+        end
+
+        def resource_type
+          'AWS::EFS::FileSystem'
+        end
+
+        def boolean_property
+          :encrypted
+        end
+      end
+
+      expect(base_rule).to receive(:boolean_property).and_return(:encrypted)
+      expect(base_rule).to receive(:resource_type).and_return('AWS::EFS::FileSystem')
+
+      cfn_model = CfnParser.new.parse read_test_template 'json/efs/filesystem_with_encryption_false.json'
+
+      expected_violation = Violation.new(id: 'F3333',
+                                         type: Violation::FAILING_VIOLATION,
+                                         message: 'This is an epic fail!',
+                                         logical_resource_ids: %w[filesystem])
+
+      expect(base_rule.audit(cfn_model)).to eq expected_violation
+    end
+
+    it 'returns no violation when boolean value is true' do
+      base_rule = BooleanBaseRule.new
+      base_rule.instance_eval do
+        def rule_id
+          'F3333'
+        end
+
+        def rule_type
+          Violation::FAILING_VIOLATION
+        end
+
+        def rule_text
+          'This is an epic fail!'
+        end
+
+        def resource_type
+          'AWS::EFS::FileSystem'
+        end
+
+        def boolean_property
+          :encrypted
+        end
+      end
+
+      expect(base_rule).to receive(:boolean_property).and_return(:encrypted)
+      expect(base_rule).to receive(:resource_type).and_return('AWS::EFS::FileSystem')
+
+      cfn_model = CfnParser.new.parse read_test_template 'json/efs/filesystem_with_encryption.json'
+
+      expect(base_rule.audit(cfn_model)).to be nil
+    end
+  end
+end