Merge pull request #2428 from opscode/sersut/basic-audit-test

Fix "log writing failed. closed stream" errors after audit phase

15 files changed, 308 insertions, 36 deletions

diff --git a/.gitignore b/.gitignore
index a9e4338e2a..ecba9f4030 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,3 +38,6 @@ Berksfile.lock
 # Vagrant
 Vagrantfile
 .vagrant/
+
+# Kitchen Tests Local Mode Data
+kitchen-tests/nodes/*
diff --git a/.kitchen.yml b/.kitchen.yml
new file mode 100644
index 0000000000..c9be1b56e7
--- /dev/null
+++ b/.kitchen.yml
@@ -0,0 +1,84 @@
+driver:
+  name: vagrant
+  forward_agent: yes
+  customize:
+    cpus: 4
+    memory: 4096
+  synced_folders:
+    - ['.', '/home/vagrant/chef']
+    - ['../ohai', '/home/vagrant/ohai']
+    - ['../triager', '/home/vagrant/triager']
+
+provisioner:
+  name: chef_zero
+  require_chef_omnibus: 12.0.0.rc.1
+
+platforms:
+  - name: centos-5.10
+    run_list:
+  - name: centos-6.5
+    run_list:
+  - name: debian-7.2.0
+    run_list:
+  - name: debian-7.4
+    run_list:
+  - name: debian-6.0.8
+    run_list:
+  - name: freebsd-9.2
+    run_list:
+  - name: freebsd-10.0
+    run_list:
+  - name: ubuntu-10.04
+    run_list:
+  - name: ubuntu-12.04
+    run_list:
+  - name: ubuntu-12.10
+    run_list:
+  - name: ubuntu-13.04
+    run_list:
+  - name: ubuntu-13.10
+    run_list:
+  - name: ubuntu-14.04
+    run_list:
+  # The following boxes are shared via VagrantCloud. Until kitchen-vagrant
+  # is updated you'll need to add the box manually:
+  #
+  #   vagrant box add chef/windows-8.1-professional
+  #
+  # Please note this may require a `vagrant login` if the box is private.
+  #
+  # The following boxes are VMware only also. You can enable VMware Fusion
+  # as the default provider by copying `.kitchen.local.yml.vmware.example`
+  # over to `.kitchen.local.yml`.
+  #
+  - name: macosx-10.8
+    driver:
+      box: chef/macosx-10.8 # private
+  - name: macosx-10.9
+    driver:
+      box: chef/macosx-10.9 # private
+  - name: macosx-10.10
+    driver:
+      box: chef/macosx-10.10 # private
+  # - name: windows-7-professional
+  #   provisioner:
+  #     name: windows_chef_zero
+  #     require_chef_omnibus: 11.12.4
+  #   driver:
+  #     box: chef/windows-7-professional # private
+  # - name: windows-8.1-professional
+  #   provisioner:
+  #     name: windows_chef_zero
+  #     require_chef_omnibus: 11.12.4
+  #   driver:
+  #     box: chef/windows-8.1-professional # private
+  # - name: windows-2008r2-standard
+  #   provisioner:
+  #     name: windows_chef_zero
+  #     require_chef_omnibus: 11.12.4
+  #   driver:
+  #     box: chef/windows-server-2008r2-standard # private
+
+suites:
+  - name: chef
+    run_list:
diff --git a/kitchen-tests/.chef/client.rb b/kitchen-tests/.chef/client.rb
index 5eb200a939..98f773d691 100644
--- a/kitchen-tests/.chef/client.rb
+++ b/kitchen-tests/.chef/client.rb
@@ -1,7 +1,8 @@
 
-chef_dir = File.expand_path(File.dirame(__FILE__))
-repo_dir = File.expand_path(Fild.join(chef_dir, '..'))
+chef_dir = File.expand_path(File.dirname(__FILE__))
+repo_dir = File.expand_path(File.join(chef_dir, '..'))
 
-log_level  :info
+log_level       :info
 chef_repo_path  repo_dir
-local_mode  true
+local_mode      true
+cache_path      "#{ENV['HOME']}/.cache/chef"
diff --git a/kitchen-tests/cookbooks/audit_test/.gitignore b/kitchen-tests/cookbooks/audit_test/.gitignore
new file mode 100644
index 0000000000..ec2a890bd3
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/.gitignore
@@ -0,0 +1,16 @@
+.vagrant
+Berksfile.lock
+*~
+*#
+.#*
+\#*#
+.*.sw[a-z]
+*.un~
+
+# Bundler
+Gemfile.lock
+bin/*
+.bundle/*
+
+.kitchen/
+.kitchen.local.yml
diff --git a/kitchen-tests/cookbooks/audit_test/.kitchen.yml b/kitchen-tests/cookbooks/audit_test/.kitchen.yml
new file mode 100644
index 0000000000..3775752da2
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/.kitchen.yml
@@ -0,0 +1,16 @@
+---
+driver:
+  name: vagrant
+
+provisioner:
+  name: chef_zero
+
+platforms:
+  - name: ubuntu-12.04
+  - name: centos-6.4
+
+suites:
+  - name: default
+    run_list:
+      - recipe[audit_test::default]
+    attributes:
diff --git a/kitchen-tests/cookbooks/audit_test/Berksfile b/kitchen-tests/cookbooks/audit_test/Berksfile
new file mode 100644
index 0000000000..0ac9b78cf7
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/Berksfile
@@ -0,0 +1,3 @@
+source "https://supermarket.getchef.com"
+
+metadata
diff --git a/kitchen-tests/cookbooks/audit_test/README.md b/kitchen-tests/cookbooks/audit_test/README.md
new file mode 100644
index 0000000000..31fb97a12d
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/README.md
@@ -0,0 +1,4 @@
+# audit_test
+
+TODO: Enter the cookbook description here.
+
diff --git a/kitchen-tests/cookbooks/audit_test/chefignore b/kitchen-tests/cookbooks/audit_test/chefignore
new file mode 100644
index 0000000000..80dc2d20ef
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/chefignore
@@ -0,0 +1,95 @@
+# Put files/directories that should be ignored in this file when uploading
+# or sharing to the community site.
+# Lines that start with '# ' are comments.
+
+# OS generated files #
+######################
+.DS_Store
+Icon?
+nohup.out
+ehthumbs.db
+Thumbs.db
+
+# SASS #
+########
+.sass-cache
+
+# EDITORS #
+###########
+\#*
+.#*
+*~
+*.sw[a-z]
+*.bak
+REVISION
+TAGS*
+tmtags
+*_flymake.*
+*_flymake
+*.tmproj
+.project
+.settings
+mkmf.log
+
+## COMPILED ##
+##############
+a.out
+*.o
+*.pyc
+*.so
+*.com
+*.class
+*.dll
+*.exe
+*/rdoc/
+
+# Testing #
+###########
+.watchr
+.rspec
+spec/*
+spec/fixtures/*
+test/*
+features/*
+Guardfile
+Procfile
+
+# SCM #
+#######
+.git
+*/.git
+.gitignore
+.gitmodules
+.gitconfig
+.gitattributes
+.svn
+*/.bzr/*
+*/.hg/*
+*/.svn/*
+
+# Berkshelf #
+#############
+Berksfile
+Berksfile.lock
+cookbooks/*
+tmp
+
+# Cookbooks #
+#############
+CONTRIBUTING
+
+# Strainer #
+############
+Colanderfile
+Strainerfile
+.colander
+.strainer
+
+# Vagrant #
+###########
+.vagrant
+Vagrantfile
+
+# Travis #
+##########
+.travis.yml
diff --git a/kitchen-tests/cookbooks/audit_test/metadata.rb b/kitchen-tests/cookbooks/audit_test/metadata.rb
new file mode 100644
index 0000000000..4a60104e92
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/metadata.rb
@@ -0,0 +1,8 @@
+name             'audit_test'
+maintainer       'The Authors'
+maintainer_email 'you@example.com'
+license          'all_rights'
+description      'Installs/Configures audit_test'
+long_description 'Installs/Configures audit_test'
+version          '0.1.0'
+
diff --git a/kitchen-tests/cookbooks/audit_test/recipes/default.rb b/kitchen-tests/cookbooks/audit_test/recipes/default.rb
new file mode 100644
index 0000000000..f02f24c2c9
--- /dev/null
+++ b/kitchen-tests/cookbooks/audit_test/recipes/default.rb
@@ -0,0 +1,11 @@
+#
+# Cookbook Name:: audit_test
+# Recipe:: default
+#
+# Copyright (c) 2014 The Authors, All Rights Reserved.
+
+controls "basic control" do
+  it "should pass" do
+    expect(2 - 2).to eq(0)
+  end
+end
diff --git a/lib/chef/audit/audit_reporter.rb b/lib/chef/audit/audit_reporter.rb
index b1c9d30bfc..40776ebefb 100644
--- a/lib/chef/audit/audit_reporter.rb
+++ b/lib/chef/audit/audit_reporter.rb
@@ -30,11 +30,7 @@ class Chef
       PROTOCOL_VERSION = '0.1.0'
 
       def initialize(rest_client)
-        if Chef::Config[:audit_mode] == false
-          @audit_enabled = false
-        else
-          @audit_enabled = true
-        end
+        @audit_enabled = Chef::Config[:audit_mode]
         @rest_client = rest_client
         # Ruby 1.9.3 and above "enumerate their values in the order that the corresponding keys were inserted."
         @ordered_control_groups = Hash.new
@@ -46,7 +42,7 @@ class Chef
       end
 
       def audit_phase_complete
-        Chef::Log.debug("Audit Reporter completed successfully without errors")
+        Chef::Log.debug("Audit Reporter completed successfully without errors.")
         ordered_control_groups.each do |name, control_group|
           audit_data.add_control_group(control_group)
         end
@@ -57,7 +53,7 @@ class Chef
       # that runs tests - normal errors are interpreted as EXAMPLE failures and captured.
       def audit_phase_failed(error)
         # The stacktrace information has already been logged elsewhere
-        Chef::Log.error("Audit Reporter failed - sending error to server with available example information")
+        Chef::Log.debug("Audit Reporter failed.")
         ordered_control_groups.each do |name, control_group|
           audit_data.add_control_group(control_group)
         end
@@ -88,34 +84,50 @@ class Chef
       private
 
       def post_auditing_data(error = nil)
-        if auditing_enabled?
-          audit_history_url = "controls"
-          Chef::Log.info("Sending audit report (run-id: #{audit_data.run_id})")
-          run_data = audit_data.to_hash
+        unless auditing_enabled?
+          Chef::Log.debug("Audit Reports are disabled. Skipping sending reports.")
+          return
+        end
 
-          if error
-            run_data[:error] = "#{error.class.to_s}: #{error.message}\n#{error.backtrace.join("\n")}"
-          end
+        audit_history_url = "controls"
+        Chef::Log.info("Sending audit report (run-id: #{audit_data.run_id})")
+        run_data = audit_data.to_hash
+
+        if error
+          # TODO: Rather than a single string we might want to format the exception here similar to
+          # lib/chef/resource_reporter.rb#83
+          run_data[:error] = "#{error.class.to_s}: #{error.message}\n#{error.backtrace.join("\n")}"
+        end
 
-          Chef::Log.debug run_data.inspect
-          compressed_data = encode_gzip(Chef::JSONCompat.to_json(run_data))
-          Chef::Log.debug("Sending compressed audit data...")
-          # Since we're posting compressed data we can not directly call post_rest which expects JSON
-          audit_url = rest_client.create_url(audit_history_url)
-          begin
-            puts Chef::JSONCompat.to_json_pretty(run_data)
-            rest_client.raw_http_request(:POST, audit_url, headers({'Content-Encoding' => 'gzip'}), compressed_data)
-          rescue StandardError => e
-            if e.respond_to? :response
+        Chef::Log.debug "Audit Report:\n#{Chef::JSONCompat.to_json_pretty(run_data)}"
+        compressed_data = encode_gzip(Chef::JSONCompat.to_json(run_data))
+        # Since we're posting compressed data we can not directly call post_rest which expects JSON
+        audit_url = rest_client.create_url(audit_history_url)
+        begin
+          rest_client.raw_http_request(:POST, audit_url, headers({'Content-Encoding' => 'gzip'}), compressed_data)
+        rescue StandardError => e
+          if e.respond_to? :response
+            code = e.response.code.nil? ? "Exception Code Empty" : e.response.code
+
+            # 404 error code is OK. This means the version of server we're running against doesn't support
+            # audit reporting. Don't alarm failure in this case.
+            if code == "404"
+              Chef::Log.debug("Server doesn't support audit reporting. Skipping report.")
+              return
+            else
+              # Save the audit report to local disk
               error_file = "failed-audit-data.json"
               Chef::FileCache.store(error_file, Chef::JSONCompat.to_json_pretty(run_data), 0640)
-              Chef::Log.error("Failed to post audit report to server (HTTP #{e.response.code}), saving to #{Chef::FileCache.load(error_file, false)}")
-            else
-              Chef::Log.error("Failed to post audit report to server (#{e})")
+              Chef::Log.error("Failed to post audit report to server. Saving report to #{Chef::FileCache.load(error_file, false)}")
             end
+          else
+            Chef::Log.error("Failed to post audit report to server (#{e})")
+          end
+
+          if Chef::Config[:enable_reporting_url_fatals]
+            Chef::Log.error("Reporting fatals enabled. Aborting run.")
+            raise
           end
-        else
-          Chef::Log.debug("Server doesn't support audit report, skipping.")
         end
       end
 
diff --git a/lib/chef/audit/rspec_formatter.rb b/lib/chef/audit/rspec_formatter.rb
new file mode 100644
index 0000000000..990c1cd780
--- /dev/null
+++ b/lib/chef/audit/rspec_formatter.rb
@@ -0,0 +1,19 @@
+require 'rspec/core'
+
+class Chef
+  class Audit
+    class RspecFormatter < RSpec::Core::Formatters::DocumentationFormatter
+      RSpec::Core::Formatters.register self, :close
+
+      # @api public
+      #
+      # Invoked at the very end, `close` allows the formatter to clean
+      # up resources, e.g. open streams, etc.
+      #
+      # @param _notification [NullNotification] (Ignored)
+      def close(_notification)
+        # Normally Rspec closes the streams it's given. We don't want it for Chef.
+      end
+    end
+  end
+end
diff --git a/lib/chef/audit/runner.rb b/lib/chef/audit/runner.rb
index 4059741359..0758dacd6d 100644
--- a/lib/chef/audit/runner.rb
+++ b/lib/chef/audit/runner.rb
@@ -18,6 +18,7 @@
 
 require 'chef/audit'
 require 'chef/audit/audit_event_proxy'
+require 'chef/audit/rspec_formatter'
 require 'chef/config'
 
 class Chef
@@ -79,7 +80,7 @@ class Chef
       end
 
       def add_formatters
-        configuration.add_formatter(RSpec::Core::Formatters::DocumentationFormatter)
+        configuration.add_formatter(Chef::Audit::RspecFormatter)
         configuration.add_formatter(Chef::Audit::AuditEventProxy)
         Chef::Audit::AuditEventProxy.events = run_context.events
       end
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index d3871c38e8..7613a4fd4a 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -319,6 +319,7 @@ class Chef
     default :client_fork, true
     default :enable_reporting, true
     default :enable_reporting_url_fatals, false
+    default :audit_mode, nil
 
     # Policyfile is an experimental feature where a node gets its run list and
     # cookbook version set from a single document on the server instead of
diff --git a/lib/chef/monologger.rb b/lib/chef/monologger.rb
index 464b21bdd3..f7d226f82e 100644
--- a/lib/chef/monologger.rb
+++ b/lib/chef/monologger.rb
@@ -1,5 +1,4 @@
 require 'logger'
-
 require 'pp'
 
 #== MonoLogger
@@ -89,4 +88,3 @@ class MonoLogger < Logger
 
 
 end
-