From dfaed0f38378974ab0138832048342b3aa146f5d Mon Sep 17 00:00:00 2001 From: Byron Lagrone Date: Fri, 31 May 2019 14:55:59 -0500 Subject: [PATCH] Refactor CLI Interface, Add Scan fail-on-warnings (#197) * Refactor CLI Interface, Add Scan fail-on-warnings [Finishes #165899506]; [Finishes #165899494]; factors out command line arguments reading; adds fail-on-warnings support to cfn_nag_scan; removes repeated code across cfn_nag_scan and cfn_nag by refactoring into an executor. Post-refactor, rubocop and testing required the following included supplmental changes: CfnNagExecutor is too large if read_cli_options is included, so factors that function out to its own file; CfnNag class is too large with the added fail_on_warnings option, refactors initialize parameters as a separate CfnNagConfig class; refactors the cfn_nag integration tests to use the new signatures * Fix Missing validate_options Call Fixes a missing call to validate_options in the executor; refactors to stay within the 15 line per function bounds of rubocop * Executor Tests, Name Fixes Adds naming corrections per @erickascic 's review; adds tests for CfnNagExecutor class at 88% * Test Coverage Bump Adds additional tests to get cfn_nag_executor and other introduced files to 100% coverage * Keep cli_options Backwards Compatible Updates cli_options to be backwards compatible -- removes alphabetical ordering, splits into two options categories for cfn_nag and cfn_nag_scan * Factory Pattern, cleanup CfnNagExecutor arguments Refactors cli_options and argf_supplier into a factory-pattern from lambdas; changes the CfnNagExecutor to have no initializer arguments and scan to properly have a cfn_nag_scan argument instead, for clarity * Test Coverage Raises Argf and Options test coverage to 100% * Resolve Test Issues, Incorporate Eric's Changeset Incorporates Eric's changeset from Slack; removes a test that is both no longer necessary and not working under the new changes; resolves rubocop issues * No @ Allowed In Trollop.options Removes @ items used in Trollop.options block -- those are unsupported and cause nil class exceptions * Resolve Failing e2e Tests - Extraneous puts An extraneous puts options call in cfn_nag_executor was causing failures in the e2e tests; removes * Add output format option to cfn_nag Complies with Jesse's PR #215, adding output format option to cfn_nag * Rename execute_single_scan, options type, deprecated argf class Per Eric Kascic's review, removes the Argf class (which was unused after prior changes); renames execute_single_scan execute_file_or_piped_scan since the argument can be an explicit file or piped input; changes the options type value for file/piped scans to 'file' from 'cli' --- bin/cfn_nag | 103 +---------- bin/cfn_nag_scan | 106 +---------- lib/cfn-nag.rb | 1 + lib/cfn-nag/cfn_nag.rb | 35 ++-- lib/cfn-nag/cfn_nag_config.rb | 30 ++++ lib/cfn-nag/cfn_nag_executor.rb | 102 +++++++++++ lib/cfn-nag/cli_options.rb | 165 ++++++++++++++++++ .../cfn_nag_cloudfront_distribution_spec.rb | 3 +- .../cfn_nag_ec2_instance_spec.rb | 3 +- .../cfn_nag_ec2_volume_spec.rb | 3 +- ..._elasticloadbalancing_loadbalancer_spec.rb | 3 +- .../cfn_nag_executor_spec.rb | 151 ++++++++++++++++ .../cfn_nag_iam_user_spec.rb | 3 +- .../cfn_nag_lambda_permission_spec.rb | 3 +- .../cfn_nag_rds_instance_spec.rb | 3 +- .../cfn_nag_s3_bucket_policy_spec.rb | 3 +- .../cfn_nag_s3_bucket_spec.rb | 3 +- .../cfn_nag_security_group_spec.rb | 3 +- .../cfn_nag_sns_policy_spec.rb | 3 +- .../cfn_nag_sqs_policy_spec.rb | 3 +- spec/cfn_nag_integration/test_path.txt | 1 + spec/cfn_nag_spec.rb | 9 +- 22 files changed, 496 insertions(+), 243 deletions(-) create mode 100644 lib/cfn-nag/cfn_nag_config.rb create mode 100644 lib/cfn-nag/cfn_nag_executor.rb create mode 100644 lib/cfn-nag/cli_options.rb create mode 100644 spec/cfn_nag_integration/cfn_nag_executor_spec.rb create mode 100644 spec/cfn_nag_integration/test_path.txt diff --git a/bin/cfn_nag b/bin/cfn_nag index b1abc42f..87b59683 100755 --- a/bin/cfn_nag +++ b/bin/cfn_nag @@ -1,110 +1,11 @@ #!/usr/bin/env ruby # frozen_string_literal: true -require 'trollop' require 'cfn-nag' require 'logging' require 'json' require 'rubygems/specification' -# rubocop:disable Metrics/BlockLength -opts = Trollop.options do - options_message = '[options] |' \ - '' - custom_rule_exceptions_message = 'Isolate custom rule exceptions - ' \ - 'just emit the exception without stack ' \ - ' trace and keep chugging' - usage options_message - version Gem::Specification.find_by_name('cfn-nag').version +exec = CfnNagExecutor.new - opt :debug, - 'Enable debug output', - type: :boolean, - required: false, - default: false - opt :allow_suppression, - 'Allow using Metadata to suppress violations', - type: :boolean, - required: false, - default: true - opt :print_suppression, - 'Emit suppressions to stderr', - type: :boolean, - required: false, - default: false - opt :rule_directory, - 'Extra rule directory', - type: :io, - required: false, - default: nil - opt :profile_path, - 'Path to a profile file', - type: :io, - required: false, - default: nil - opt :blacklist_path, - 'Path to a blacklist file', - type: :io, - required: false, - default: nil - opt :parameter_values_path, - 'Path to a JSON file to pull Parameter values from', - type: :io, - required: false, - default: nil - opt :isolate_custom_rule_exceptions, - custom_rule_exceptions_message, - type: :boolean, - required: false, - default: false - opt :fail_on_warnings, - 'Treat warnings as failing violations', - type: :boolean, - required: false, - default: false -end -# rubocop:enable Metrics/BlockLength - -CfnNagLogging.configure_logging(opts) - -profile_definition = nil -unless opts[:profile_path].nil? - profile_definition = IO.read(opts[:profile_path]) -end - -blacklist_definition = nil -unless opts[:blacklist_path].nil? - blacklist_definition = IO.read(opts[:blacklist_path]) -end - -parameter_values_string = nil -unless opts[:parameter_values_path].nil? - parameter_values_string = IO.read(opts[:parameter_values_path]) -end - -cfn_nag = CfnNag.new( - profile_definition: profile_definition, - blacklist_definition: blacklist_definition, - rule_directory: opts[:rule_directory], - allow_suppression: opts[:allow_suppression], - print_suppression: opts[:print_suppression], - isolate_custom_rule_exceptions: opts[:isolate_custom_rule_exceptions] -) - -total_failure_count = 0 -until ARGF.closed? || ARGF.eof? - results = cfn_nag.audit(cloudformation_string: ARGF.file.read, - parameter_values_string: parameter_values_string) - ARGF.close - - total_failure_count += if opts[:fail_on_warnings] - results[:violations].length - else - results[:failure_count] - end - - results[:violations] = results[:violations].map(&:to_h) - puts JSON.pretty_generate(results) -end - -exit total_failure_count +exit exec.scan(options_type: 'file') diff --git a/bin/cfn_nag_scan b/bin/cfn_nag_scan index 60ec5af8..f4378951 100755 --- a/bin/cfn_nag_scan +++ b/bin/cfn_nag_scan @@ -1,113 +1,13 @@ #!/usr/bin/env ruby # frozen_string_literal: true -require 'trollop' require 'cfn-nag' require 'logging' require 'json' require 'rubygems/specification' -# rubocop:disable Metrics/BlockLength -opts = Trollop.options do - version Gem::Specification.find_by_name('cfn-nag').version +exec = CfnNagExecutor.new - input_path_message = 'CloudFormation template to nag on or directory of ' \ - 'templates. Default is all *.json, *.yaml, *.yml ' \ - 'and *.template recursively, but can be constrained ' \ - 'by --template-pattern' - - custom_rule_exceptions_message = 'Isolate custom rule exceptions - just ' \ - 'emit the exception without stack trace ' \ - 'and keep chugging' - - template_pattern_message = 'Within the --input-path, match files to scan ' \ - 'against this regular expression' - - opt :input_path, - input_path_message, - type: :io, - required: true - opt :output_format, - 'Format of results: [txt, json]', - type: :string, - default: 'txt' - opt :debug, - 'Enable debug output', - type: :boolean, - required: false, - default: false - opt :rule_directory, - 'Extra rule directory', - type: :io, - required: false, - default: nil - opt :profile_path, - 'Path to a profile file', - type: :io, - required: false, - default: nil - opt :blacklist_path, - 'Path to a blacklist file', - type: :io, - required: false, - default: nil - opt :parameter_values_path, - 'Path to a JSON file to pull Parameter values from', - type: :io, - required: false, - default: nil - opt :allow_suppression, - 'Allow using Metadata to suppress violations', - type: :boolean, - required: false, - default: true - opt :print_suppression, - 'Emit suppressions to stderr', - type: :boolean, - required: false, - default: false - opt :isolate_custom_rule_exceptions, - custom_rule_exceptions_message, - type: :boolean, - required: false, - default: false - opt :template_pattern, - template_pattern_message, - type: :string, - required: false, - default: '..*\.json|..*\.yaml|..*\.yml|..*\.template' -end -# rubocop:enable Metrics/BlockLength - -unless %w[txt json].include?(opts[:output_format]) - Trollop.die(:output_format, - 'Must be txt or json') -end - -CfnNagLogging.configure_logging(opts) - -profile_definition = nil -unless opts[:profile_path].nil? - profile_definition = IO.read(opts[:profile_path]) -end - -blacklist_definition = nil -unless opts[:blacklist_path].nil? - blacklist_definition = IO.read(opts[:blacklist_path]) -end - -cfn_nag = CfnNag.new( - profile_definition: profile_definition, - blacklist_definition: blacklist_definition, - rule_directory: opts[:rule_directory], - allow_suppression: opts[:allow_suppression], - print_suppression: opts[:print_suppression], - isolate_custom_rule_exceptions: opts[:isolate_custom_rule_exceptions] -) - -exit cfn_nag.audit_aggregate_across_files_and_render_results( - input_path: opts[:input_path], - output_format: opts[:output_format], - parameter_values_path: opts[:parameter_values_path], - template_pattern: opts[:template_pattern] +exit exec.scan( + options_type: 'scan' ) diff --git a/lib/cfn-nag.rb b/lib/cfn-nag.rb index 4e3f7eb6..bcfccf11 100644 --- a/lib/cfn-nag.rb +++ b/lib/cfn-nag.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'cfn-nag/cfn_nag' +require 'cfn-nag/cfn_nag_executor' require 'cfn-nag/cfn_nag_logging' require 'cfn-nag/violation' require 'cfn-nag/rule_dumper' diff --git a/lib/cfn-nag/cfn_nag.rb b/lib/cfn-nag/cfn_nag.rb index 0729b8f7..1610d691 100644 --- a/lib/cfn-nag/cfn_nag.rb +++ b/lib/cfn-nag/cfn_nag.rb @@ -12,24 +12,9 @@ class CfnNag include ViolationFiltering - # rubocop:disable Metrics/ParameterLists - def initialize(profile_definition: nil, - blacklist_definition: nil, - rule_directory: nil, - allow_suppression: true, - print_suppression: false, - isolate_custom_rule_exceptions: 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 - ) - @profile_definition = profile_definition - @blacklist_definition = blacklist_definition + def initialize(config:) + @config = config end - # rubocop:enable Metrics/ParameterLists ## # Given a file or directory path, emit aggregate results to stdout @@ -48,7 +33,11 @@ def audit_aggregate_across_files_and_render_results(input_path:, output_format: output_format) aggregate_results.inject(0) do |total_failure_count, results| - total_failure_count + results[:file_results][:failure_count] + if @config.fail_on_warnings + total_failure_count + results[:file_results][:violations].length + else + total_failure_count + results[:file_results][:failure_count] + end end end @@ -87,7 +76,7 @@ def audit(cloudformation_string:, parameter_values_string: nil) cfn_model = CfnParser.new.parse cloudformation_string, parameter_values_string, true - violations += @custom_rule_loader.execute_custom_rules(cfn_model) + violations += @config.custom_rule_loader.execute_custom_rules(cfn_model) violations = filter_violations_by_blacklist_and_profile(violations) violations = mark_line_numbers(violations, cfn_model) @@ -115,15 +104,15 @@ def mark_line_numbers(violations, cfn_model) def filter_violations_by_blacklist_and_profile(violations) violations = filter_violations_by_profile( - profile_definition: @profile_definition, - rule_definitions: @custom_rule_loader.rule_definitions, + profile_definition: @config.profile_definition, + rule_definitions: @config.custom_rule_loader.rule_definitions, violations: violations ) # this must come after - blacklist should always win violations = filter_violations_by_blacklist( - blacklist_definition: @blacklist_definition, - rule_definitions: @custom_rule_loader.rule_definitions, + blacklist_definition: @config.blacklist_definition, + rule_definitions: @config.custom_rule_loader.rule_definitions, violations: violations ) violations diff --git a/lib/cfn-nag/cfn_nag_config.rb b/lib/cfn-nag/cfn_nag_config.rb new file mode 100644 index 00000000..bdb85c81 --- /dev/null +++ b/lib/cfn-nag/cfn_nag_config.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class CfnNagConfig + # rubocop:disable Metrics/ParameterLists + def initialize(profile_definition: nil, + blacklist_definition: nil, + rule_directory: nil, + allow_suppression: true, + print_suppression: false, + isolate_custom_rule_exceptions: false, + fail_on_warnings: 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 + ) + @profile_definition = profile_definition + @blacklist_definition = blacklist_definition + @fail_on_warnings = fail_on_warnings + end + # rubocop:enable Metrics/ParameterLists + + attr_reader :rule_directory + attr_reader :custom_rule_loader + attr_reader :profile_definition + attr_reader :blacklist_definition + attr_reader :fail_on_warnings +end diff --git a/lib/cfn-nag/cfn_nag_executor.rb b/lib/cfn-nag/cfn_nag_executor.rb new file mode 100644 index 00000000..e3f64868 --- /dev/null +++ b/lib/cfn-nag/cfn_nag_executor.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'trollop' +require 'cfn-nag/cli_options' +require 'cfn-nag/cfn_nag_config' + +class CfnNagExecutor + def initialize + @profile_definition = nil + @blacklist_definition = nil + @parameter_values_string = nil + end + + def scan(options_type:) + options = Options.for(options_type) + validate_options(options) + execute_io_options(options) + + CfnNagLogging.configure_logging(options) + + cfn_nag = CfnNag.new( + config: cfn_nag_config(options) + ) + + options_type == 'scan' ? execute_aggregate_scan(cfn_nag, options) : execute_file_or_piped_scan(cfn_nag, options) + end + + private + + def execute_file_or_piped_scan(cfn_nag, opts) + total_failure_count = 0 + until argf_finished? + results = cfn_nag.audit(cloudformation_string: argf_read, + parameter_values_string: @parameter_values_string) + argf_close + + total_failure_count += if opts[:fail_on_warnings] + results[:violations].length + else + results[:failure_count] + end + + results[:violations] = results[:violations].map(&:to_h) + puts JSON.pretty_generate(results) + end + total_failure_count + end + + def execute_aggregate_scan(cfn_nag, opts) + cfn_nag.audit_aggregate_across_files_and_render_results( + input_path: opts[:input_path], + output_format: opts[:output_format], + parameter_values_path: opts[:parameter_values_path], + template_pattern: opts[:template_pattern] + ) + end + + def validate_options(opts) + unless opts[:output_format].nil? || %w[txt json].include?(opts[:output_format]) + Trollop.die(:output_format, + 'Must be txt or json') + end + end + + def execute_io_options(opts) + unless opts[:profile_path].nil? + @profile_definition = IO.read(opts[:profile_path]) + end + + unless opts[:blacklist_path].nil? + @blacklist_definition = IO.read(opts[:blacklist_path]) + end + + unless opts[:parameter_values_path].nil? + @parameter_values_string = IO.read(opts[:parameter_values_path]) + end + end + + def cfn_nag_config(opts) + CfnNagConfig.new( + profile_definition: @profile_definition, + blacklist_definition: @blacklist_definition, + rule_directory: opts[:rule_directory], + allow_suppression: opts[:allow_suppression], + print_suppression: opts[:print_suppression], + isolate_custom_rule_exceptions: opts[:isolate_custom_rule_exceptions], + fail_on_warnings: opts[:fail_on_warnings] + ) + end + + def argf_finished? + ARGF.closed? || ARGF.eof? + end + + def argf_close + ARGF.close + end + + def argf_read + ARGF.file.read + end +end diff --git a/lib/cfn-nag/cli_options.rb b/lib/cfn-nag/cli_options.rb new file mode 100644 index 00000000..3ef41964 --- /dev/null +++ b/lib/cfn-nag/cli_options.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +require 'trollop' + +# rubocop:disable Metrics/ClassLength +class Options + @custom_rule_exceptions_message = 'Isolate custom rule exceptions - just ' \ + 'emit the exception without stack trace ' \ + 'and keep chugging' + + @version = Gem::Specification.find_by_name('cfn-nag').version + + def self.for(type) + case type + when 'file' + file_options + when 'scan' + scan_options + else + raise "Unsupported Options type #{type}; use 'file' or 'scan'" + end + end + + # rubocop:disable Metrics/BlockLength + # rubocop:disable Metrics/MethodLength + def self.file_options + options_message = '[options] |' \ + '' + custom_rule_exceptions_message = @custom_rule_exceptions_message + version = @version + + Trollop.options do + usage options_message + version version + + opt :debug, + 'Enable debug output', + type: :boolean, + required: false, + default: false + opt :allow_suppression, + 'Allow using Metadata to suppress violations', + type: :boolean, + required: false, + default: true + opt :print_suppression, + 'Emit suppressions to stderr', + type: :boolean, + required: false, + default: false + opt :rule_directory, + 'Extra rule directory', + type: :io, + required: false, + default: nil + opt :profile_path, + 'Path to a profile file', + type: :io, + required: false, + default: nil + opt :blacklist_path, + 'Path to a blacklist file', + type: :io, + required: false, + default: nil + opt :parameter_values_path, + 'Path to a JSON file to pull Parameter values from', + type: :io, + required: false, + default: nil + opt :isolate_custom_rule_exceptions, + custom_rule_exceptions_message, + type: :boolean, + required: false, + default: false + opt :fail_on_warnings, + 'Treat warnings as failing violations', + type: :boolean, + required: false, + default: false + opt :output_format, + 'Format of results: [txt, json]', + type: :string, + default: 'txt' + end + end + + def self.scan_options + input_path_message = 'CloudFormation template to nag on or directory of ' \ + 'templates. Default is all *.json, *.yaml, *.yml ' \ + 'and *.template recursively, but can be constrained ' \ + 'by --template-pattern' + + template_pattern_message = 'Within the --input-path, match files to scan ' \ + 'against this regular expression' + + custom_rule_exceptions_message = @custom_rule_exceptions_message + version = @version + + Trollop.options do + version version + opt :input_path, + input_path_message, + type: :io, + required: true + opt :output_format, + 'Format of results: [txt, json]', + type: :string, + default: 'txt' + opt :debug, + 'Enable debug output', + type: :boolean, + required: false, + default: false + opt :rule_directory, + 'Extra rule directory', + type: :io, + required: false, + default: nil + opt :profile_path, + 'Path to a profile file', + type: :io, + required: false, + default: nil + opt :blacklist_path, + 'Path to a blacklist file', + type: :io, + required: false, + default: nil + opt :parameter_values_path, + 'Path to a JSON file to pull Parameter values from', + type: :io, + required: false, + default: nil + opt :allow_suppression, + 'Allow using Metadata to suppress violations', + type: :boolean, + required: false, + default: true + opt :print_suppression, + 'Emit suppressions to stderr', + type: :boolean, + required: false, + default: false + opt :isolate_custom_rule_exceptions, + custom_rule_exceptions_message, + type: :boolean, + required: false, + default: false + opt :template_pattern, + template_pattern_message, + type: :string, + required: false, + default: '..*\.json|..*\.yaml|..*\.yml|..*\.template' + opt :fail_on_warnings, + 'Treat warnings as failing violations', + type: :boolean, + required: false, + default: false + end + end + # rubocop:enable Metrics/BlockLength + # rubocop:enable Metrics/MethodLength +end +# rubocop:enable Metrics/ClassLength diff --git a/spec/cfn_nag_integration/cfn_nag_cloudfront_distribution_spec.rb b/spec/cfn_nag_integration/cfn_nag_cloudfront_distribution_spec.rb index 3ac1bc10..1de11e2b 100644 --- a/spec/cfn_nag_integration/cfn_nag_cloudfront_distribution_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_cloudfront_distribution_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'cloudfront distro without logging', :cf do diff --git a/spec/cfn_nag_integration/cfn_nag_ec2_instance_spec.rb b/spec/cfn_nag_integration/cfn_nag_ec2_instance_spec.rb index 96b62476..61ee7992 100644 --- a/spec/cfn_nag_integration/cfn_nag_ec2_instance_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_ec2_instance_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'when authentication metadata is specified' do diff --git a/spec/cfn_nag_integration/cfn_nag_ec2_volume_spec.rb b/spec/cfn_nag_integration/cfn_nag_ec2_volume_spec.rb index ffe98715..2b9871fb 100644 --- a/spec/cfn_nag_integration/cfn_nag_ec2_volume_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_ec2_volume_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'EBS volumes without encryption', :ebs do diff --git a/spec/cfn_nag_integration/cfn_nag_elasticloadbalancing_loadbalancer_spec.rb b/spec/cfn_nag_integration/cfn_nag_elasticloadbalancing_loadbalancer_spec.rb index 5abd7dc9..fe7a6aa9 100644 --- a/spec/cfn_nag_integration/cfn_nag_elasticloadbalancing_loadbalancer_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_elasticloadbalancing_loadbalancer_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'two load balancers without access logging enabled' do diff --git a/spec/cfn_nag_integration/cfn_nag_executor_spec.rb b/spec/cfn_nag_integration/cfn_nag_executor_spec.rb new file mode 100644 index 00000000..880560e3 --- /dev/null +++ b/spec/cfn_nag_integration/cfn_nag_executor_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true +require 'spec_helper' +require 'cfn-nag/cfn_nag_logging' +require 'cfn-nag/cfn_nag_executor' + +describe CfnNagExecutor do + + before(:all) do + CfnNagLogging.configure_logging(debug: false) + @default_cli_options = { + allow_suppression: true, + debug: false, + isolate_custom_rule_exceptions: false, + print_suppression: false, + rule_directory: nil, + template_pattern: '..*\.json|..*\.yaml|..*\.yml|..*\.template', + output_format: 'json' + } + end + + context 'single file cfn_nag with fail on warnings' do + it 'returns a nonzero exit code' do + test_file = 'spec/test_templates/yaml/ec2_subnet/ec2_subnet_map_public_ip_on_launch_true_boolean.yml' + + cli_options = @default_cli_options.clone + cli_options[:fail_on_warnings] = true + expect(Options).to receive(:file_options).and_return(cli_options) + + cfn_nag_executor = CfnNagExecutor.new + expect(cfn_nag_executor).to receive(:argf_read).and_return(IO.read(test_file)) + expect(cfn_nag_executor).to receive(:argf_close).and_return(nil) + expect(cfn_nag_executor).to receive(:argf_finished?).and_return(false, true) + + result = cfn_nag_executor.scan(options_type: 'file') + expect(result).to eq 1 + end + end + + context 'multi file cfn_nag with fail on warnings' do + it 'returns a nonzero exit code' do + test_file1 = 'spec/test_templates/yaml/ec2_subnet/ec2_subnet_map_public_ip_on_launch_true_boolean.yml' + test_file2 = 'spec/test_templates/yaml/ec2_subnet/ec2_subnet_map_public_ip_on_launch_true_boolean.yml' + + cli_options = @default_cli_options.clone + cli_options[:fail_on_warnings] = true + expect(Options).to receive(:file_options).and_return(cli_options) + + cfn_nag_executor = CfnNagExecutor.new + expect(cfn_nag_executor).to receive(:argf_read).and_return(IO.read(test_file1), IO.read(test_file2)) + expect(cfn_nag_executor).to receive(:argf_close).and_return(nil, nil) + expect(cfn_nag_executor).to receive(:argf_finished?).and_return(false, false, true) + + result = cfn_nag_executor.scan(options_type: 'file') + expect(result).to eq 2 + end + end + + context 'single file cfn_nag' do + it 'returns a successful zero exit code' do + test_file = 'spec/test_templates/yaml/ec2_subnet/ec2_subnet_map_public_ip_on_launch_true_boolean.yml' + + expect(Options).to receive(:file_options).and_return(@default_cli_options) + + cfn_nag_executor = CfnNagExecutor.new + expect(cfn_nag_executor).to receive(:argf_read).and_return(IO.read(test_file)) + expect(cfn_nag_executor).to receive(:argf_close).and_return(nil) + expect(cfn_nag_executor).to receive(:argf_finished?).and_return(false, true) + + result = cfn_nag_executor.scan(options_type: 'file') + + expect(result).to eq 0 + end + end + + # this one triggers a trollop 'system exit' mid-flow :( + context 'no input path specified' do + it 'throws error on nil input_path' do + expect { + cfn_nag_executor = CfnNagExecutor.new + + _ = cfn_nag_executor.scan(options_type: 'scan') + }.to raise_error(SystemExit) + end + end + + context 'input path specified with neptune directory', :poo do + it 'records three failures' do + cli_options = @default_cli_options.clone + cli_options[:input_path] = 'spec/test_templates/json/neptune' + expect(Options).to receive(:scan_options).and_return(cli_options) + puts cli_options + + cfn_nag_executor = CfnNagExecutor.new + + result = cfn_nag_executor.scan(options_type: 'scan') + + expect(result).to eq 3 + end + end + + context 'input path specified with fail on warnings' do + it 'records four failures' do + cli_options = @default_cli_options.clone + cli_options[:input_path] = 'spec/test_templates/yaml/ec2_subnet' + cli_options[:fail_on_warnings] = true + expect(Options).to receive(:scan_options).and_return(cli_options) + + cfn_nag_executor = CfnNagExecutor.new + + result = cfn_nag_executor.scan(options_type: 'scan') + + expect(result).to eq 4 + end + end + + context 'invalid value provided for output type' do + it 'dies at validate_opts' do + cli_options = @default_cli_options.clone + cli_options[:output_format] = 'invalid' + expect(Options).to receive(:scan_options).and_return(cli_options) + + expect { + cfn_nag_executor = CfnNagExecutor.new + + _ = cfn_nag_executor.scan(options_type: 'scan') + }.to raise_error(SystemExit) + end + end + + context 'use profile, blacklist, and parameter path options' do + it 'raises a TypeError once it tries to read the invalid files' do + cli_options = @default_cli_options.clone + cli_options[:blacklist_definition] = 'spec/cfn_nag_integration/test_path.txt' + cli_options[:parameter_values_path] = 'spec/cfn_nag_integration/test_path.txt' + cli_options[:profile_path] = 'spec/cfn_nag_integration/test_path.txt' + expect(Options).to receive(:scan_options).and_return(cli_options) + + expect { + cfn_nag_executor = CfnNagExecutor.new + + _ = cfn_nag_executor.scan(options_type: 'scan') + }.to raise_error(TypeError) + end + end + + context 'invalid Options types' do + it 'raises errors' do + expect {Options.for('invalid')}.to raise_error(RuntimeError) + end + end +end diff --git a/spec/cfn_nag_integration/cfn_nag_iam_user_spec.rb b/spec/cfn_nag_integration/cfn_nag_iam_user_spec.rb index 2a24fd40..022da7f4 100644 --- a/spec/cfn_nag_integration/cfn_nag_iam_user_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_iam_user_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'iam user has no group membership', :sns do diff --git a/spec/cfn_nag_integration/cfn_nag_lambda_permission_spec.rb b/spec/cfn_nag_integration/cfn_nag_lambda_permission_spec.rb index 170d2574..74bf4643 100644 --- a/spec/cfn_nag_integration/cfn_nag_lambda_permission_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_lambda_permission_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'lambda permission with some out of the ordinary items', :lambda do diff --git a/spec/cfn_nag_integration/cfn_nag_rds_instance_spec.rb b/spec/cfn_nag_integration/cfn_nag_rds_instance_spec.rb index 0592e2b1..c506f91d 100644 --- a/spec/cfn_nag_integration/cfn_nag_rds_instance_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_rds_instance_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'one RDS instance with public access' do diff --git a/spec/cfn_nag_integration/cfn_nag_s3_bucket_policy_spec.rb b/spec/cfn_nag_integration/cfn_nag_s3_bucket_policy_spec.rb index d8c57a08..49d9795d 100644 --- a/spec/cfn_nag_integration/cfn_nag_s3_bucket_policy_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_s3_bucket_policy_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 's3 with wildcards', :s3 do diff --git a/spec/cfn_nag_integration/cfn_nag_s3_bucket_spec.rb b/spec/cfn_nag_integration/cfn_nag_s3_bucket_spec.rb index d0623607..c87b0521 100644 --- a/spec/cfn_nag_integration/cfn_nag_s3_bucket_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_s3_bucket_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'two buckets with insecure ACL - PublicRead and PublicReadWrite' do diff --git a/spec/cfn_nag_integration/cfn_nag_security_group_spec.rb b/spec/cfn_nag_integration/cfn_nag_security_group_spec.rb index bd12d135..ae694fa0 100644 --- a/spec/cfn_nag_integration/cfn_nag_security_group_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_security_group_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'when sg properties are missing', :foo do diff --git a/spec/cfn_nag_integration/cfn_nag_sns_policy_spec.rb b/spec/cfn_nag_integration/cfn_nag_sns_policy_spec.rb index c18938d1..5f85b4c1 100644 --- a/spec/cfn_nag_integration/cfn_nag_sns_policy_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_sns_policy_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'sns with wildcard principal', :sns do diff --git a/spec/cfn_nag_integration/cfn_nag_sqs_policy_spec.rb b/spec/cfn_nag_integration/cfn_nag_sqs_policy_spec.rb index fc7d2ed6..1c77f95a 100644 --- a/spec/cfn_nag_integration/cfn_nag_sqs_policy_spec.rb +++ b/spec/cfn_nag_integration/cfn_nag_sqs_policy_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' describe CfnNag do before(:all) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end context 'SQS Queue Policy with NotAction' do diff --git a/spec/cfn_nag_integration/test_path.txt b/spec/cfn_nag_integration/test_path.txt new file mode 100644 index 00000000..2cb609de --- /dev/null +++ b/spec/cfn_nag_integration/test_path.txt @@ -0,0 +1 @@ +Test Data diff --git a/spec/cfn_nag_spec.rb b/spec/cfn_nag_spec.rb index 1ae5996f..f937bbec 100644 --- a/spec/cfn_nag_spec.rb +++ b/spec/cfn_nag_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'cfn-nag/cfn_nag_config' require 'cfn-nag/cfn_nag' require 'cfn-nag/cfn_nag_logging' require 'cfn-nag/profile_loader' @@ -6,7 +7,7 @@ describe CfnNag do before(:each) do CfnNagLogging.configure_logging(debug: false) - @cfn_nag = CfnNag.new + @cfn_nag = CfnNag.new(config: CfnNagConfig.new) end describe '#audit_aggregate_across_files_and_render_results' do @@ -141,7 +142,7 @@ message: "fakeo#{num}") end - @cfn_nag = CfnNag.new(profile_definition: "F16\n") + @cfn_nag = CfnNag.new(config: CfnNagConfig.new(profile_definition: "F16\n")) actual_aggregate_results = @cfn_nag.audit_aggregate_across_files( @@ -155,7 +156,7 @@ context 'when template has suppression metadata' do it 'ignores rules on suppressed resources' do template_name = 'yaml/security_group/sg_with_suppression.yml' - @cfn_nag = CfnNag.new print_suppression: true + @cfn_nag = CfnNag.new(config: CfnNagConfig.new(print_suppression: true)) expected_aggregate_results = [ { @@ -205,7 +206,7 @@ context 'when template has suppression metadata and suppression is disallowed' do it 'ignores rules on suppressed resources' do - @cfn_nag = CfnNag.new(allow_suppression: false) + @cfn_nag = CfnNag.new(config: CfnNagConfig.new(allow_suppression: false)) template_name = 'yaml/security_group/sg_with_suppression.yml'