Merge pull request #2426 from opscode/mcquin/controls_group_object

Wait until audit phase to load needed files.

6 files changed, 138 insertions, 97 deletions

diff --git a/lib/chef/audit.rb b/lib/chef/audit.rb
deleted file mode 100644
index ed8db93d96..0000000000
--- a/lib/chef/audit.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-#
-# Author:: Claire McQuin (<claire@getchef.com>)
-# Copyright:: Copyright (c) 2014 Chef Software, Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require 'rspec'
-require 'rspec/its'
-
-require 'serverspec/matcher'
-require 'serverspec/helper'
-require 'serverspec/subject'
-
-require 'specinfra'
-
-require 'chef/dsl/audit'
-require 'chef/audit/runner'
diff --git a/lib/chef/audit/audit_reporter.rb b/lib/chef/audit/audit_reporter.rb
index 5ed1f7bd52..b0eb835c0c 100644
--- a/lib/chef/audit/audit_reporter.rb
+++ b/lib/chef/audit/audit_reporter.rb
@@ -70,7 +70,7 @@ class Chef
 
       def control_group_started(name)
         if ordered_control_groups.has_key?(name)
-          raise AuditControlGroupDuplicate.new(name)
+          raise Chef::Exceptions::AuditControlGroupDuplicate.new(name)
         end
         ordered_control_groups.store(name, ControlGroupData.new(name))
       end
diff --git a/lib/chef/audit/runner.rb b/lib/chef/audit/runner.rb
index 0758dacd6d..2c72e6f11f 100644
--- a/lib/chef/audit/runner.rb
+++ b/lib/chef/audit/runner.rb
@@ -16,11 +16,6 @@
 # limitations under the License.
 #
 
-require 'chef/audit'
-require 'chef/audit/audit_event_proxy'
-require 'chef/audit/rspec_formatter'
-require 'chef/config'
-
 class Chef
   class Audit
     class Runner
@@ -34,80 +29,144 @@ class Chef
 
       def run
         setup
-        register_controls_groups
-
-        # The first parameter passed to RSpec::Core::Runner.new
-        # is an instance of RSpec::Core::ConfigurationOptions, which is
-        # responsible for processing command line options passed through rspec.
-        # This then gets merged with the configuration. We'll just communicate
-        # directly with the Configuration here.
-        audit_runner = RSpec::Core::Runner.new(nil, configuration, world)
-        audit_runner.run_specs(world.ordered_example_groups)
+        register_controls
+        do_run
       end
 
       private
-
-      # RSpec configuration and world objects are heavy, so let's wait until
-      # we actually need them.
-      def configuration
-        RSpec.configuration
+      # Prepare to run audits:
+      #  - Require files
+      #  - Configure RSpec
+      #  - Configure Specinfra/Serverspec
+      def setup
+        require_deps
+        configure_rspec
+        configure_specinfra
       end
 
-      def world
-        RSpec.world
+      # RSpec uses a global configuration object, RSpec.configuration. We found
+      # there was interference between the configuration for audit-mode and
+      # the configuration for our own spec tests in these cases:
+      #   1. Specinfra and Serverspec modify RSpec.configuration when loading.
+      #   2. Setting output/error streams.
+      #   3. Adding formatters.
+      #   4. Defining example group aliases.
+      #
+      # Moreover, Serverspec loads its DSL methods into the global namespace,
+      # which causes conflicts with the Chef namespace for resources and packages.
+      #
+      # We wait until we're in the audit-phase of the chef-client run to load
+      # these files. This helps with the namespacing problems we saw, and
+      # prevents Specinfra and Serverspec from modifying the RSpec configuration
+      # used by our spec tests.
+      def require_deps
+        # TODO: We need to figure out a way to give audits its own configuration
+        # object. This involves finding a way to load these files w/o them adding
+        # to the configuration object used by our spec tests.
+        require 'rspec'
+        require 'rspec/its'
+        require 'specinfra'
+        require 'serverspec/helper'
+        require 'serverspec/matcher'
+        require 'serverspec/subject'
+        require 'chef/audit/audit_event_proxy'
+        require 'chef/audit/rspec_formatter'
       end
 
-      # Configure audits before run.
-      # Sets up where output and error streams should stream to, adds formatters
-      # for people-friendly output of audit results and json for reporting. Also
-      # configures expectation frameworks.
-      def setup
-        # We're setting the output stream, but that will only be used for error situations
-        # Our formatter forwards events to the Chef event message bus
-        # TODO so some testing to see if these output to a log file - we probably need
-        # to register these before any formatters are added.
-        configuration.output_stream = Chef::Config[:log_location]
-        configuration.error_stream  = Chef::Config[:log_location]
-        # TODO im pretty sure I only need this because im running locally in rvmsudo
-        configuration.backtrace_exclusion_patterns.push(Regexp.new("/Users".gsub("/", File::SEPARATOR)))
-        configuration.backtrace_exclusion_patterns.push(Regexp.new("(eval)"))
-        configuration.color = Chef::Config[:color]
-        configuration.expose_dsl_globally = false
-
+      # Configure RSpec just the way we like it:
+      #   - Set location of error and output streams
+      #   - Add custom audit-mode formatters
+      #   - Explicitly disable :should syntax
+      #   - Set :color option according to chef config
+      #   - Disable exposure of global DSL
+      def configure_rspec
+        set_streams
         add_formatters
         disable_should_syntax
-        configure_specinfra
+
+        RSpec.configure do |c|
+          c.color = Chef::Config[:color]
+          c.expose_dsl_globally = false
+        end
       end
 
+      # Set the error and output streams which audit-mode will use to report
+      # human-readable audit information.
+      #
+      # This should always be called before #add_formatters. RSpec won't allow
+      # the output stream to be changed for a formatter once the formatter has
+      # been added.
+      def set_streams
+        # TODO: Do some testing to ensure these will output/output properly to
+        # a file.
+        RSpec.configuration.output_stream = Chef::Config[:log_location]
+        RSpec.configuration.error_stream = Chef::Config[:log_location]
+      end
+
+      # Add formatters which we use to
+      #   1. Output human-readable data to the output stream,
+      #   2. Collect JSON data to send back to the analytics server.
       def add_formatters
-        configuration.add_formatter(Chef::Audit::RspecFormatter)
-        configuration.add_formatter(Chef::Audit::AuditEventProxy)
+        RSpec.configuration.add_formatter(Chef::Audit::AuditEventProxy)
+        RSpec.configuration.add_formatter(Chef::Audit::RspecFormatter)
         Chef::Audit::AuditEventProxy.events = run_context.events
       end
 
-      # Explicitly disable :should syntax.
+      # Audit-mode uses RSpec 3. :should syntax is deprecated by default in
+      # RSpec 3, so we explicitly disable it here.
       #
-      # :should is deprecated in RSpec 3 and we have chosen to explicitly disable it
-      # in audits. If :should is used in an audit, the audit will fail with error
-      # message "undefined method `:should`" rather than issue a deprecation warning.
-      #
-      # This can be removed when :should is fully removed from RSpec.
+      # This can be removed once :should is removed from RSpec.
       def disable_should_syntax
-        configuration.expect_with :rspec do |c|
-          c.syntax = :expect
+        RSpec.configure do |config|
+          config.expect_with :rspec do |c|
+            c.syntax = :expect
+          end
         end
       end
 
+      # Set up the backend for Specinfra/Serverspec.
       def configure_specinfra
-        # TODO: We may need to change this based on operating system (there is a
-        # powershell backend) or roll our own.
+        # TODO: We may need to be clever and adjust this based on operating
+        # system, or make it configurable. E.g., there is a PowerShell backend,
+        # as well as an SSH backend.
         Specinfra.configuration.backend = :exec
       end
 
-      # Register each controls group with the world, which will handle
-      # the ordering of the audits that will be run.
-      def register_controls_groups
-        run_context.controls_groups.each { |ctls_grp| world.register(ctls_grp) }
+      # Iterates through the controls registered to this run_context, builds an
+      # example group (RSpec::Core::ExampleGroup) object per controls, and
+      # registers the group with the RSpec.world.
+      #
+      # We could just store an array of example groups and not use RSpec.world,
+      # but it may be useful later if we decide to apply our own ordering scheme
+      # or use example group filters.
+      def register_controls
+        add_example_group_methods
+        run_context.controls.each do |name, group|
+          ctl_grp = RSpec::Core::ExampleGroup.__controls__(*group[:args], &group[:block])
+          RSpec.world.register(ctl_grp)
+        end
+      end
+
+      # Add example group method aliases to RSpec.
+      #
+      # __controls__: Used internally to create example groups from the controls
+      #               saved in the run_context.
+      #      control: Used within the context of a controls block, like RSpec's
+      #               describe or context.
+      def add_example_group_methods
+        RSpec::Core::ExampleGroup.define_example_group_method :__controls__
+        RSpec::Core::ExampleGroup.define_example_group_method :control
+      end
+
+      # Run the audits!
+      def do_run
+        # RSpec::Core::Runner wants to be initialized with an
+        # RSpec::Core::ConfigurationOptions object, which is used to process
+        # command line configuration arguments. We directly fiddle with the
+        # internal RSpec configuration object, so we give nil here and let
+        # RSpec pick up its own configuration and world.
+        runner = RSpec::Core::Runner.new(nil)
+        runner.run_specs(RSpec.world.ordered_example_groups)
       end
 
     end
diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index b27a2b693d..9e1d2dc207 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -25,7 +25,7 @@ require 'chef/log'
 require 'chef/rest'
 require 'chef/api_client'
 require 'chef/api_client/registration'
-require 'chef/audit'
+require 'chef/audit/runner'
 require 'chef/node'
 require 'chef/role'
 require 'chef/file_cache'
@@ -441,8 +441,17 @@ class Chef
 
         run_context = setup_run_context
 
-        converge_error = converge_and_save(run_context) unless (Chef::Config[:audit_mode] == true)
-        audit_error = run_audits(run_context) unless (Chef::Config[:audit_mode] == false)
+        unless Chef::Config[:audit_mode] == true
+          converge_error = converge_and_save(run_context)
+        else
+          Chef::Log.debug("Skipping converge. Chef is configured to run audits only.")
+        end
+
+        unless Chef::Config[:audit_mode] == false
+          audit_error = run_audits(run_context)
+        else
+          Chef::Log.debug("Skipping audits. Chef is configured to converge the node only.")
+        end
 
         if converge_error || audit_error
           e = Chef::Exceptions::RunFailedWrappingError.new(converge_error, audit_error)
diff --git a/lib/chef/dsl/audit.rb b/lib/chef/dsl/audit.rb
index 1849b65633..e22c38f587 100644
--- a/lib/chef/dsl/audit.rb
+++ b/lib/chef/dsl/audit.rb
@@ -16,23 +16,25 @@
 # limitations under the License.
 #
 
-require 'rspec/core'
+require 'chef/exceptions'
 
 class Chef
   module DSL
     module Audit
 
       # Can encompass tests in a `control` block or `describe` block
-      ::RSpec::Core::ExampleGroup.define_example_group_method :control
-      ::RSpec::Core::ExampleGroup.define_example_group_method :__controls__
-
       # Adds the controls group and block (containing controls to execute) to the runner's list of pending examples
       def controls(*args, &block)
-        raise ::Chef::Exceptions::NoAuditsProvided unless block
+        raise Chef::Exceptions::NoAuditsProvided unless block
+
         name = args[0]
-        raise AuditNameMissing if name.nil? || name.empty?
+        if name.nil? || name.empty?
+          raise Chef::Exceptions::AuditNameMissing
+        elsif run_context.controls.has_key?(name)
+          raise Chef::Exceptions::AuditControlGroupDuplicate.new(name)
+        end
 
-        run_context.controls_groups << ::RSpec::Core::ExampleGroup.__controls__(*args, &block)
+        run_context.controls[name] = { :args => args, :block => block }
       end
 
     end
diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index 8f7296822c..a724789d3c 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -51,7 +51,7 @@ class Chef
     attr_accessor :resource_collection
 
     # The list of control groups to execute during the audit phase
-    attr_accessor :controls_groups
+    attr_accessor :controls
 
     # A Hash containing the immediate notifications triggered by resources
     # during the converge phase of the chef run.
@@ -76,7 +76,7 @@ class Chef
       @node = node
       @cookbook_collection = cookbook_collection
       @resource_collection = Chef::ResourceCollection.new
-      @controls_groups = []
+      @controls = {}
       @immediate_notification_collection = Hash.new {|h,k| h[k] = []}
       @delayed_notification_collection = Hash.new {|h,k| h[k] = []}
       @definitions = Hash.new