diff options
| -rw-r--r-- | cspell.json | 1 | ||||
| -rw-r--r-- | lib/chef/compliance/default_attributes.rb | 3 | ||||
| -rw-r--r-- | lib/chef/compliance/fetcher/automate.rb | 7 | ||||
| -rw-r--r-- | lib/chef/compliance/reporter/automate.rb | 24 | ||||
| -rw-r--r-- | lib/chef/compliance/reporter/chef_server_automate.rb | 17 | ||||
| -rw-r--r-- | lib/chef/compliance/reporter/cli.rb | 4 | ||||
| -rw-r--r-- | lib/chef/compliance/reporter/compliance_enforcer.rb | 4 | ||||
| -rw-r--r-- | lib/chef/compliance/reporter/json_file.rb | 6 | ||||
| -rw-r--r-- | lib/chef/compliance/runner.rb | 72 | ||||
| -rw-r--r-- | spec/unit/compliance/fetcher/automate_spec.rb | 16 | ||||
| -rw-r--r-- | spec/unit/compliance/reporter/automate_spec.rb | 28 | ||||
| -rw-r--r-- | spec/unit/compliance/reporter/chef_server_automate_spec.rb | 20 | ||||
| -rw-r--r-- | spec/unit/compliance/reporter/compliance_enforcer_spec.rb | 1 | ||||
| -rw-r--r-- | spec/unit/compliance/runner_spec.rb | 28 |
14 files changed, 165 insertions, 66 deletions
diff --git a/cspell.json b/cspell.json index a54071a0db..0ee63efc67 100644 --- a/cspell.json +++ b/cspell.json @@ -235,6 +235,7 @@ "cmds", "CMDS", "Cmnd", + "CMPL", "cname", "codepage", "coderanger", diff --git a/lib/chef/compliance/default_attributes.rb b/lib/chef/compliance/default_attributes.rb index a1ffff440f..f214135d59 100644 --- a/lib/chef/compliance/default_attributes.rb +++ b/lib/chef/compliance/default_attributes.rb @@ -38,11 +38,12 @@ class Chef # Allow for connections to HTTPS endpoints using self-signed ssl certificates. "insecure" => nil, - # Controls verbosity of Chef InSpec runner. + # Controls verbosity of Chef InSpec runner. See less output when true. "quiet" => true, # Chef Inspec Compliance profiles to be used for scan of node. # See README.md for details + # MPTD - ^there is no README.md anymore^ "profiles" => {}, # Extra inputs passed to Chef InSpec to allow finer-grained control over behavior. diff --git a/lib/chef/compliance/fetcher/automate.rb b/lib/chef/compliance/fetcher/automate.rb index 64aff6833a..47dc7f2e6a 100644 --- a/lib/chef/compliance/fetcher/automate.rb +++ b/lib/chef/compliance/fetcher/automate.rb @@ -46,13 +46,6 @@ class Chef config["token"] = Chef::Config[:data_collector][:token] - if config["token"].nil? - raise Inspec::FetcherFailure, - "No data-collector token set, which is required by the chef-automate fetcher. " \ - "Set the `data_collector.token` configuration parameter in your client.rb " \ - 'or use the "chef-server-automate" reporter which does not require any ' \ - "data-collector settings and uses #{ChefUtils::Dist::Server::PRODUCT} to fetch profiles." - end end new(profile_fetch_url, config) diff --git a/lib/chef/compliance/reporter/automate.rb b/lib/chef/compliance/reporter/automate.rb index cae0256085..23514c857d 100644 --- a/lib/chef/compliance/reporter/automate.rb +++ b/lib/chef/compliance/reporter/automate.rb @@ -28,18 +28,28 @@ class Chef @token = Chef::Config[:data_collector][:token] end - # Method used in order to send the inspec report to the data_collector server - def send_report(report) - unless @entity_uuid && @run_id - Chef::Log.error "entity_uuid(#{@entity_uuid}) or run_id(#{@run_id}) can't be nil, not sending report to #{ChefUtils::Dist::Automate::PRODUCT}" - return false + def validate_config! + unless @entity_uuid + # TODO - this is a weird leakage of naming from the parent class + # but entity_uuid is never an attribute that the user can see; + # it is sourced from chef_guid, which we don't technically know about in this class - + # but telling the operator about a missing chef_guid is more helpful than telling + # them about a missing field they've never heard of. Better would be a dock link + # that described how to fix this situation. + raise "CMPL004: automate_reporter: chef_guid is not available and must be provided. Aborting because we cannot report the scan." + end + + unless @run_id + raise "CMPL005: automate_reporter: run_id is not available, aborting because we cannot report the scan." end unless @url && @token - Chef::Log.warn "data_collector.token and data_collector.server_url must be defined in client.rb! Further information: https://docs.chef.io/chef_compliance_phase/#direct-reporting-to-chef-automate" - return false + raise "CMPL006: data_collector.token and data_collector.server_url must be configured in client.rb! Further information: https://docs.chef.io/chef_compliance_phase/#direct-reporting-to-chef-automate" end + end + # Method used in order to send the inspec report to the data_collector server + def send_report(report) headers = { "Content-Type" => "application/json", "x-data-collector-auth" => "version=1.0", diff --git a/lib/chef/compliance/reporter/chef_server_automate.rb b/lib/chef/compliance/reporter/chef_server_automate.rb index 46ca7dc7ba..569fb22426 100644 --- a/lib/chef/compliance/reporter/chef_server_automate.rb +++ b/lib/chef/compliance/reporter/chef_server_automate.rb @@ -30,11 +30,6 @@ class Chef end def send_report(report) - unless @entity_uuid && @run_id - Chef::Log.error "entity_uuid(#{@entity_uuid}) or run_id(#{@run_id}) can't be nil, not sending report to #{ChefUtils::Dist::Automate::PRODUCT}" - return false - end - automate_report = truncate_controls_results(enriched_report(report), @control_results_limit) report_size = Chef::JSONCompat.to_json(automate_report, validate_utf8: false).bytesize @@ -51,6 +46,16 @@ class Chef false end + def validate_config! + unless @entity_uuid + raise "CMPL007: chef_server_automate reporter: chef_guid is not available and must be provided. Aborting because we cannot report the scan" + end + + unless @run_id + raise "CMPL008: chef_server_automate reporter: run_id is not available, aborting because we cannot report the scan." + end + end + def http_client config = if @insecure Chef::Config.merge(ssl_verify_mode: :verify_none) @@ -80,7 +85,7 @@ class Chef when /404/ Chef::Log.error "Object does not exist on remote server." when /413/ - Chef::Log.error "You most likely hit the erchef request size in #{ChefUtils::Dist::Server::PRODUCT} that defaults to ~2MB. To increase this limit see the Compliance Phase troubleshooting documentation (http://docs.chef.io/chef_compliance_phase/#troubleshooting) or the Chef Infra Server configuration documentation (https://docs.chef.io/server/config_rb_server/)" + Chef::Log.error "You most likely hit the request size limit in #{ChefUtils::Dist::Server::PRODUCT} that defaults to ~2MB. To increase this limit see the Compliance Phase troubleshooting documentation (http://docs.chef.io/chef_compliance_phase/#troubleshooting) or the Chef Infra Server configuration documentation (https://docs.chef.io/server/config_rb_server/)" when /429/ Chef::Log.error "This error typically means the data sent was larger than #{ChefUtils::Dist::Automate::PRODUCT}'s limit (4 MB). Run InSpec locally to identify any controls producing large diffs." end diff --git a/lib/chef/compliance/reporter/cli.rb b/lib/chef/compliance/reporter/cli.rb index 4905c1f653..7061e5751c 100644 --- a/lib/chef/compliance/reporter/cli.rb +++ b/lib/chef/compliance/reporter/cli.rb @@ -22,6 +22,10 @@ class Chef puts output.join("\n") end + def validate_config! + true + end + private # pastel.decorate is a lightweight replacement for highline.color diff --git a/lib/chef/compliance/reporter/compliance_enforcer.rb b/lib/chef/compliance/reporter/compliance_enforcer.rb index 1c63e43b28..47b3a4d2df 100644 --- a/lib/chef/compliance/reporter/compliance_enforcer.rb +++ b/lib/chef/compliance/reporter/compliance_enforcer.rb @@ -14,6 +14,10 @@ class Chef end true end + + def validate_config! + true + end end end end diff --git a/lib/chef/compliance/reporter/json_file.rb b/lib/chef/compliance/reporter/json_file.rb index 471d9f64b1..4d074242ca 100644 --- a/lib/chef/compliance/reporter/json_file.rb +++ b/lib/chef/compliance/reporter/json_file.rb @@ -13,6 +13,12 @@ class Chef File.write(@path, Chef::JSONCompat.to_json(report)) end + + def validate_config! + if @path.nil? || @path.class != String || @path.empty? + raise "CMPL007: json_file reporter: node['audit']['json_file']['location'] must contain a file path" + end + end end end end diff --git a/lib/chef/compliance/runner.rb b/lib/chef/compliance/runner.rb index c083109875..7aebeeab36 100644 --- a/lib/chef/compliance/runner.rb +++ b/lib/chef/compliance/runner.rb @@ -1,17 +1,15 @@ autoload :Inspec, "inspec" require_relative "default_attributes" -require_relative "reporter/automate" -require_relative "reporter/chef_server_automate" -require_relative "reporter/compliance_enforcer" -require_relative "reporter/cli" -require_relative "reporter/json_file" class Chef module Compliance class Runner < EventDispatch::Base extend Forwardable + SUPPORTED_REPORTERS = %w{chef-automate chef-server-automate json-file audit-enforcer cli}.freeze + SUPPORTED_FETCHERS = %w{chef-automate chef-server}.freeze + attr_accessor :run_id attr_reader :node def_delegators :node, :logger @@ -47,6 +45,15 @@ class Chef self.run_id = run_status.run_id end + def converge_start(run_context) + # With all attributes - including cookook - loaded, we now have enough data to validate + # configuration. Because the converge is best coupled with the associated compliance run, these validations + # will raise (and abort the converge) if the compliance phase configuration is incorrect/will + # prevent compliance phase from completing and submitting its report to all configured reporters. + # can abort the converge if the compliance phase configuration (node attributes and client config) + load_and_validate! + end + def run_completed(_node, _run_status) return unless enabled? @@ -85,6 +92,8 @@ class Chef end def report(report = generate_report) + # This is invoked at report-time instead of with the normal validations at node loaded, + # because we want to ensure that it is visible in the output - and not lost in back-scroll. warn_for_deprecated_config_values! if report.empty? @@ -92,8 +101,9 @@ class Chef return end - Array(node["audit"]["reporter"]).each do |reporter| - send_report(reporter, report) + Array(node["audit"]["reporter"]).each do |reporter_type| + logger.info "Reporting to #{reporter_type}" + @reporters[reporter_type].send_report(report) end end @@ -119,10 +129,8 @@ class Chef def inspec_profiles profiles = node["audit"]["profiles"] - - # TODO: Custom exception class here? unless profiles.respond_to?(:map) && profiles.all? { |_, p| p.respond_to?(:transform_keys) && p.respond_to?(:update) } - raise "#{Inspec::Dist::PRODUCT_NAME} profiles specified in an unrecognized format, expected a hash of hashes." + raise "CMPL010: #{Inspec::Dist::PRODUCT_NAME} profiles specified in an unrecognized format, expected a hash of hashes." end profiles.map do |name, profile| @@ -138,8 +146,6 @@ class Chef require_relative "fetcher/chef_server" when nil # intentionally blank - else - raise "Invalid value specified for Compliance Phase's fetcher: '#{node["audit"]["fetcher"]}'. Valid values are 'chef-automate', 'chef-server', or nil." end end @@ -212,17 +218,10 @@ class Chef } end - def send_report(reporter_type, report) - logger.info "Reporting to #{reporter_type}" - - reporter = reporter(reporter_type) - - reporter.send_report(report) if reporter - end - def reporter(reporter_type) - case reporter_type + case reporter_type.downcase when "chef-automate" + require_relative "reporter/automate" opts = { control_results_limit: node["audit"]["control_results_limit"], entity_uuid: node["chef_guid"], @@ -233,6 +232,7 @@ class Chef } Chef::Compliance::Reporter::Automate.new(opts) when "chef-server-automate" + require_relative "reporter/chef_server_automate" opts = { control_results_limit: node["audit"]["control_results_limit"], entity_uuid: node["chef_guid"], @@ -244,15 +244,16 @@ class Chef } Chef::Compliance::Reporter::ChefServerAutomate.new(opts) when "json-file" + require_relative "reporter/json_file" path = node["audit"]["json_file"]["location"] logger.info "Writing compliance report to #{path}" Chef::Compliance::Reporter::JsonFile.new(file: path) when "audit-enforcer" + require_relative "reporter/compliance_enforcer" Chef::Compliance::Reporter::ComplianceEnforcer.new when "cli" + require_relative "reporter/cli" Chef::Compliance::Reporter::Cli.new - else - raise "'#{reporter_type}' is not a supported reporter for Compliance Phase." end end @@ -269,6 +270,31 @@ class Chef url.path = File.join(url.path, "organizations/#{org}/data-collector") url end + + # Load the resources required for this runner, and validate configuration + # is correct to proceed. Requires node state to be loaded. + # Will raise exception if fetcher is not valid, if a reporter is not valid, + # or the configuration required by a reporter is not provided. + def load_and_validate! + return unless enabled? + + @reporters = {} + Array(node["audit"]["reporter"]).each do |reporter_type| + type = reporter_type.downcase + unless SUPPORTED_REPORTERS.include? type + raise "CMPL003: '#{reporter_type}' found in node['audit']['reporter'] is not a supported reporter for Compliance Phase. Supported reporters are: #{SUPPORTED_REPORTERS.join(",")}. For more information, see the documentation at https://docs.chef.io/chef_compliance_phase/chef_compliance_runners/#reporters" + end + + @reporters[type] = reporter(type) + @reporters[type].validate_config! + end + + unless (fetcher = node["audit"]["fetcher"]).nil? + unless SUPPORTED_FETCHERS.include? fetcher.downcase + raise "CMPL002: Unrecognized Compliance Phase fetcher (node['audit']['fetcher'] is #{fetcher}. Supported fetchers are: or #{SUPPORTED_FETCHERS.join(",")}, or nil. For more information, see the documentation at https://docs.chef.io/chef_compliance_phase/chef_compliance_runners/#fetchers" + end + end + end end end end diff --git a/spec/unit/compliance/fetcher/automate_spec.rb b/spec/unit/compliance/fetcher/automate_spec.rb index f3554b8b0f..a4cd0c76c3 100644 --- a/spec/unit/compliance/fetcher/automate_spec.rb +++ b/spec/unit/compliance/fetcher/automate_spec.rb @@ -29,14 +29,6 @@ describe Chef::Compliance::Fetcher::Automate do expect(res.target).to eq(expected) end - it "raises an exception with no data collector token" do - Chef::Config[:data_collector].delete(:token) - - expect { - Chef::Compliance::Fetcher::Automate.resolve("compliance://namespace/profile_name") - }.to raise_error(/No data-collector token set/) - end - it "includes the data collector token" do expect(Chef::Compliance::Fetcher::Automate).to receive(:new).with( "https://automate.test/compliance/profiles/namespace/profile_name/tar", @@ -108,14 +100,6 @@ describe Chef::Compliance::Fetcher::Automate do expect(res.target).to eq(expected) end - it "raises an exception with no data collector token" do - Chef::Config[:data_collector].delete(:token) - - expect { - Chef::Compliance::Fetcher::Automate.resolve(compliance: "namespace/profile_name") - }.to raise_error(Inspec::FetcherFailure, /No data-collector token set/) - end - it "includes the data collector token" do expect(Chef::Compliance::Fetcher::Automate).to receive(:new).with( "https://automate.test/compliance/profiles/namespace/profile_name/tar", diff --git a/spec/unit/compliance/reporter/automate_spec.rb b/spec/unit/compliance/reporter/automate_spec.rb index e0a33892b0..60d630d32b 100644 --- a/spec/unit/compliance/reporter/automate_spec.rb +++ b/spec/unit/compliance/reporter/automate_spec.rb @@ -1,6 +1,7 @@ require "spec_helper" require "json" # For .to_json +require "chef/compliance/reporter/automate" describe Chef::Compliance::Reporter::Automate do let(:reporter) { Chef::Compliance::Reporter::Automate.new(opts) } @@ -264,11 +265,34 @@ describe Chef::Compliance::Reporter::Automate do expect(metasearch_stub).to have_been_requested expect(report_stub).to have_been_requested end + end - it "does not send report when entity_uuid is missing" do + describe "#validate_config!" do + it "raises CMPL004 when entity_uuid is not present" do opts.delete(:entity_uuid) + expect { reporter.validate_config! }.to raise_error(/^CMPL004/) + end + + it "raises CMPL005 when run_id is not present" do + opts.delete(:run_id) + expect { reporter.validate_config! }.to raise_error(/^CMPL005/) + end + + it "raises CMPL006 when data collector URL is missing" do + Chef::Config[:data_collector] = { token: "not_nil", server_url: nil } reporter = Chef::Compliance::Reporter::Automate.new(opts) - expect(reporter.send_report(inspec_report)).to eq(false) + expect { reporter.validate_config! }.to raise_error(/^CMPL006/) + end + + it "raises CMPL006 when data collector token is missing" do + Chef::Config[:data_collector] = { token: nil, server_url: "not_nil" } + reporter = Chef::Compliance::Reporter::Automate.new(opts) + expect { reporter.validate_config! }.to raise_error(/^CMPL006/) + end + + it "otherwise passes" do + Chef::Config[:data_collector] = { token: "not_nil", server_url: "not_nil" } + reporter.validate_config! end end diff --git a/spec/unit/compliance/reporter/chef_server_automate_spec.rb b/spec/unit/compliance/reporter/chef_server_automate_spec.rb index e45a7157ee..33642dea31 100644 --- a/spec/unit/compliance/reporter/chef_server_automate_spec.rb +++ b/spec/unit/compliance/reporter/chef_server_automate_spec.rb @@ -1,7 +1,9 @@ require "spec_helper" +require "chef/compliance/reporter/chef_server_automate" describe Chef::Compliance::Reporter::ChefServerAutomate do before do + # Isn't this already done globally in WebMock.disable_net_connect! Chef::Config[:client_key] = File.expand_path("../../../data/ssl/private_key.pem", __dir__) @@ -174,4 +176,22 @@ describe Chef::Compliance::Reporter::ChefServerAutomate do expect(report_stub).to have_been_requested end + + describe "#validate_config!" do + it "raises CMPL007 when entity_uuid is not present" do + opts.delete(:entity_uuid) + expect { reporter.validate_config! }.to raise_error(/^CMPL007/) + end + + it "raises CMPL008 when run_id is not present" do + opts.delete(:run_id) + expect { reporter.validate_config! }.to raise_error(/^CMPL008/) + end + + it "otherwise passes" do + reporter.validate_config! + end + + end + end diff --git a/spec/unit/compliance/reporter/compliance_enforcer_spec.rb b/spec/unit/compliance/reporter/compliance_enforcer_spec.rb index ae63cf0853..3f3ce6286b 100644 --- a/spec/unit/compliance/reporter/compliance_enforcer_spec.rb +++ b/spec/unit/compliance/reporter/compliance_enforcer_spec.rb @@ -1,4 +1,5 @@ require "spec_helper" +require "chef/compliance/reporter/compliance_enforcer" describe Chef::Compliance::Reporter::AuditEnforcer do let(:reporter) { Chef::Compliance::Reporter::AuditEnforcer.new } diff --git a/spec/unit/compliance/runner_spec.rb b/spec/unit/compliance/runner_spec.rb index c100029a2c..3948970137 100644 --- a/spec/unit/compliance/runner_spec.rb +++ b/spec/unit/compliance/runner_spec.rb @@ -130,7 +130,7 @@ describe Chef::Compliance::Runner do expect(runner.inspec_profiles).to eq(expected) end - it "raises an error when the profiles are in the old audit-cookbook format" do + it "raises a CMPL010 message when the profiles are in the old audit-cookbook format" do node.normal["audit"]["profiles"] = [ { name: "Windows 2019 Baseline", @@ -138,7 +138,7 @@ describe Chef::Compliance::Runner do }, ] - expect { runner.inspec_profiles }.to raise_error(/profiles specified in an unrecognized format, expected a hash of hashes./) + expect { runner.inspec_profiles }.to raise_error(/CMPL010:/) end end @@ -186,9 +186,29 @@ describe Chef::Compliance::Runner do end end - it "fails with unexpected reporter value" do - expect { runner.reporter("tacos") }.to raise_error(/'tacos' is not a supported reporter for Compliance Phase/) + end + + describe "#load_and_validate! when compliance is enabled" do + before do + allow(runner).to receive(:enabled?).and_return(true) + end + + it "raises CMPL003 when the reporter is not a supported reporter type" do + node.normal["audit"]["reporter"] = [ "invalid" ] + expect { runner.load_and_validate! }.to raise_error(/^CMPL003:/) end + it "raises CMPL002 if the configured fetcher is not supported" do + node.normal["audit"]["fetcher"] = "invalid" + expect { runner.load_and_validate! }.to raise_error(/^CMPL002:/) + end + + it "validates configured reporters" do + node.normal["audit"]["reporter"] = [ "chef-automate" ] + reporter_double = double("reporter", validate_config!: nil) + expect(runner).to receive(:reporter).with("chef-automate").and_return(reporter_double) + runner.load_and_validate! + end + end describe "#inspec_opts" do |