summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLamont Granquist <lamont@scriptkiddie.org>2014-12-08 13:44:59 -0800
committerLamont Granquist <lamont@scriptkiddie.org>2014-12-08 13:45:29 -0800
commit1b3705ca383b64092c9ef54d7e46201fab6271c7 (patch)
treee14588c68939b80e9cd8c5bf0b0eab4bfddbc96c
parent2bdc79409711e836a7698f7b44d171fa7973d4f1 (diff)
downloadchef-1b3705ca383b64092c9ef54d7e46201fab6271c7.tar.gz
Execute and Script Resource improvements
- Warning on incorrect usage of the command resource in any script resource - Warning on code in script resource being nil - Specs added to force deprecation of incorrect usage in Chef-13 - Specs added around the (supported) incorrect usage in Chef-12 - Cleanup+Modernization of providers and specs - Fixed some global state bugs around the Chef::Log.level in the spec tests
-rw-r--r--lib/chef/exceptions.rb1
-rw-r--r--lib/chef/provider/execute.rb91
-rw-r--r--lib/chef/provider/mount/solaris.rb2
-rw-r--r--lib/chef/provider/script.rb38
-rw-r--r--lib/chef/resource/execute.rb14
-rw-r--r--lib/chef/resource/script.rb11
-rw-r--r--spec/functional/resource/bash_spec.rb88
-rw-r--r--spec/functional/resource/execute_spec.rb133
-rw-r--r--spec/spec_helper.rb4
-rw-r--r--spec/support/platform_helpers.rb8
-rw-r--r--spec/support/shared/unit/script_resource.rb56
-rw-r--r--spec/unit/cookbook/syntax_check_spec.rb5
-rw-r--r--spec/unit/http/validate_content_length_spec.rb9
-rw-r--r--spec/unit/provider/execute_spec.rb206
-rw-r--r--spec/unit/provider/git_spec.rb5
-rw-r--r--spec/unit/provider/script_spec.rb104
-rw-r--r--spec/unit/resource/script_spec.rb8
-rw-r--r--spec/unit/run_context_spec.rb7
18 files changed, 523 insertions, 267 deletions
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index c8d26dbed2..bf33cb9b52 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -45,6 +45,7 @@ class Chef
class FileNotFound < RuntimeError; end
class Package < RuntimeError; end
class Service < RuntimeError; end
+ class Script < RuntimeError; end
class Route < RuntimeError; end
class SearchIndex < RuntimeError; end
class Override < RuntimeError; end
diff --git a/lib/chef/provider/execute.rb b/lib/chef/provider/execute.rb
index 48b2a344d1..b44112c19e 100644
--- a/lib/chef/provider/execute.rb
+++ b/lib/chef/provider/execute.rb
@@ -18,67 +18,86 @@
require 'chef/log'
require 'chef/provider'
+require 'forwardable'
class Chef
class Provider
class Execute < Chef::Provider
+ extend Forwardable
provides :execute
+ def_delegators :@new_resource, :command, :returns, :environment, :user, :group, :cwd, :umask, :creates
+
def load_current_resource
- true
+ current_resource = Chef::Resource::Execute.new(new_resource.name)
+ current_resource
end
def whyrun_supported?
true
end
- def action_run
- opts = {}
-
- if sentinel_file = sentinel_file_if_exists
- Chef::Log.debug("#{@new_resource} sentinel file #{sentinel_file} exists - nothing to do")
- return false
- end
+ def define_resource_requirements
+ # @todo: this should change to raise in some appropriate major version bump.
+ if creates && creates_relative? && !cwd
+ Chef::Log.warn "Providing a relative path for the creates attribute without the cwd is deprecated and will be changed to fail (CHEF-3819)"
+ end
+ end
+ def timeout
# original implementation did not specify a timeout, but ShellOut
# *always* times out. So, set a very long default timeout
- opts[:timeout] = @new_resource.timeout || 3600
- opts[:returns] = @new_resource.returns if @new_resource.returns
- opts[:environment] = @new_resource.environment if @new_resource.environment
- opts[:user] = @new_resource.user if @new_resource.user
- opts[:group] = @new_resource.group if @new_resource.group
- opts[:cwd] = @new_resource.cwd if @new_resource.cwd
- opts[:umask] = @new_resource.umask if @new_resource.umask
- opts[:log_level] = :info
- opts[:log_tag] = @new_resource.to_s
- if STDOUT.tty? && !Chef::Config[:daemon] && Chef::Log.info? && !@new_resource.sensitive
- opts[:live_stream] = STDOUT
+ new_resource.timeout || 3600
+ end
+
+ def action_run
+ if creates && sentinel_file.exist?
+ Chef::Log.debug("#{new_resource} sentinel file #{sentinel_file} exists - nothing to do")
+ return false
end
- description = @new_resource.sensitive ? "sensitive resource" : @new_resource.command
+
converge_by("execute #{description}") do
- result = shell_out!(@new_resource.command, opts)
- Chef::Log.info("#{@new_resource} ran successfully")
+ result = shell_out!(command, opts)
+ Chef::Log.info("#{new_resource} ran successfully")
end
end
private
- def sentinel_file_if_exists
- if sentinel_file = @new_resource.creates
- relative = Pathname(sentinel_file).relative?
- cwd = @new_resource.cwd
- if relative && !cwd
- Chef::Log.warn "You have provided relative path for execute#creates (#{sentinel_file}) without execute#cwd (see CHEF-3819)"
- end
-
- if ::File.exists?(sentinel_file)
- sentinel_file
- elsif cwd && relative
- sentinel_file = ::File.join(cwd, sentinel_file)
- sentinel_file if ::File.exists?(sentinel_file)
- end
+ def sensitive?
+ !!new_resource.sensitive
+ end
+
+ def opts
+ opts = {}
+ opts[:timeout] = timeout
+ opts[:returns] = returns if returns
+ opts[:environment] = environment if environment
+ opts[:user] = user if user
+ opts[:group] = group if group
+ opts[:cwd] = cwd if cwd
+ opts[:umask] = umask if umask
+ opts[:log_level] = :info
+ opts[:log_tag] = new_resource.to_s
+ if STDOUT.tty? && !Chef::Config[:daemon] && Chef::Log.info? && !sensitive?
+ opts[:live_stream] = STDOUT
end
+ opts
+ end
+
+ def description
+ sensitive? ? "sensitive resource" : command
+ end
+
+ def creates_relative?
+ Pathname(creates).relative?
+ end
+
+ def sentinel_file
+ Pathname.new(Chef::Util::PathHelper.cleanpath(
+ ( cwd && creates_relative? ) ? ::File.join(cwd, creates) : creates
+ ))
end
end
end
diff --git a/lib/chef/provider/mount/solaris.rb b/lib/chef/provider/mount/solaris.rb
index cf04150322..d8cec24138 100644
--- a/lib/chef/provider/mount/solaris.rb
+++ b/lib/chef/provider/mount/solaris.rb
@@ -88,7 +88,7 @@ class Chef
# FIXME: Should remount always do the remount or only if the options change?
actual_options = options || []
actual_options.delete('noauto')
- mount_options = actual_options.empty? ? '' : ",#{actual_options.join(',')}"
+ mount_options = actual_options.empty? ? '' : ",#{actual_options.join(',')}"
shell_out!("mount -o remount#{mount_options} #{mount_point}")
end
diff --git a/lib/chef/provider/script.rb b/lib/chef/provider/script.rb
index 1615517553..ea286cb0e4 100644
--- a/lib/chef/provider/script.rb
+++ b/lib/chef/provider/script.rb
@@ -18,10 +18,13 @@
require 'tempfile'
require 'chef/provider/execute'
+require 'forwardable'
class Chef
class Provider
class Script < Chef::Provider::Execute
+ extend Forwardable
+
provides :bash
provides :csh
provides :perl
@@ -29,30 +32,40 @@ class Chef
provides :ruby
provides :script
+ def_delegators :@new_resource, :code, :interpreter, :flags
+
def initialize(new_resource, run_context)
super
- @code = @new_resource.code
+ end
+
+ def command
+ "\"#{interpreter}\" #{flags} \"#{script_file.path}\""
+ end
+
+ def load_current_resource
+ super
+ # @todo Chef-13: change this to an exception
+ if code.nil?
+ Chef::Log.warn "#{@new_resource}: No code attribute was given, resource does nothing, this behavior is deprecated and will be removed in Chef-13"
+ end
end
def action_run
- script_file.puts(@code)
+ script_file.puts(code)
script_file.close
set_owner_and_group
- @new_resource.command("\"#{interpreter}\" #{flags} \"#{script_file.path}\"")
super
- converge_by(nil) do
- # ensure script is unlinked at end of converge!
- unlink_script_file
- end
+
+ unlink_script_file
end
def set_owner_and_group
# FileUtils itself implements a no-op if +user+ or +group+ are nil
# You can prove this by running FileUtils.chown(nil,nil,'/tmp/file')
# as an unprivileged user.
- FileUtils.chown(@new_resource.user, @new_resource.group, script_file.path)
+ FileUtils.chown(new_resource.user, new_resource.group, script_file.path)
end
def script_file
@@ -60,16 +73,9 @@ class Chef
end
def unlink_script_file
- @script_file && @script_file.close!
+ script_file && script_file.close!
end
- def interpreter
- @new_resource.interpreter
- end
-
- def flags
- @new_resource.flags
- end
end
end
end
diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index 980035b079..dfae88b24e 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -106,7 +106,7 @@ class Chef
set_or_return(
:timeout,
arg,
- :kind_of => [ Integer ]
+ :kind_of => [ Integer, Float ]
)
end
@@ -135,12 +135,12 @@ class Chef
end
set_guard_inherited_attributes(
- :cwd,
- :environment,
- :group,
- :user,
- :umask
- )
+ :cwd,
+ :environment,
+ :group,
+ :user,
+ :umask
+ )
end
end
diff --git a/lib/chef/resource/script.rb b/lib/chef/resource/script.rb
index 479295922c..fd0fd5a7fd 100644
--- a/lib/chef/resource/script.rb
+++ b/lib/chef/resource/script.rb
@@ -24,11 +24,13 @@ class Chef
class Resource
class Script < Chef::Resource::Execute
+ # Chef-13: go back to using :name as the identity attr
identity_attr :command
def initialize(name, run_context=nil)
super
@resource_name = :script
+ # Chef-13: the command variable should be initialized to nil
@command = name
@code = nil
@interpreter = nil
@@ -36,6 +38,15 @@ class Chef
@default_guard_interpreter = :default
end
+ def command(arg=nil)
+ unless arg.nil?
+ # Chef-13: change this to raise if the user is trying to set a value here
+ Chef::Log.warn "Specifying command attribute on a script resource is a coding error, use the 'code' attribute, or the execute resource"
+ Chef::Log.warn "This attribute is deprecated and must be fixed or this code will fail on Chef-13"
+ end
+ super
+ end
+
def code(arg=nil)
set_or_return(
:code,
diff --git a/spec/functional/resource/bash_spec.rb b/spec/functional/resource/bash_spec.rb
new file mode 100644
index 0000000000..209ec4a12f
--- /dev/null
+++ b/spec/functional/resource/bash_spec.rb
@@ -0,0 +1,88 @@
+#
+# Author:: Serdar Sutay (<serdar@opscode.com>)
+# Copyright:: Copyright (c) 2014 Opscode, 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 'spec_helper'
+require 'functional/resource/base'
+
+describe Chef::Resource::Bash, :unix_only do
+ let(:code) { "echo hello" }
+ let(:resource) {
+ resource = Chef::Resource::Bash.new("foo_resource", run_context)
+ resource.code(code)
+ resource
+ }
+
+ describe "when setting the command attribute" do
+ let (:command) { 'wizard racket' }
+
+ # in Chef-12 the `command` attribute is largely useless, but does set the identity attribute
+ # so that notifications need to target the value of the command. it will not run the `command`
+ # and if it is given without a code block then it does nothing and always succeeds.
+ describe "in Chef-12", :chef_lt_13_only do
+ it "gets the commmand attribute from the name" do
+ expect(resource.command).to eql("foo_resource")
+ end
+
+ it "sets the resource identity to the command name" do
+ resource.command command
+ expect(resource.identity).to eql(command)
+ end
+
+ it "warns when the code is not present and a useless `command` is present" do
+ expect(Chef::Log).to receive(:warn).with(/coding error/)
+ expect(Chef::Log).to receive(:warn).with(/deprecated/)
+ resource.code nil
+ resource.command command
+ expect { resource.run_action(:run) }.not_to raise_error
+ end
+
+ describe "when the code is not present" do
+ let(:code) { nil }
+ it "warns" do
+ expect(Chef::Log).to receive(:warn)
+ expect { resource.run_action(:run) }.not_to raise_error
+ end
+ end
+ end
+
+ # in Chef-13 the `command` attribute needs to be for internal use only
+ describe "in Chef-13", :chef_gte_13_only do
+ it "should raise an exception when trying to set the command" do
+ expect { resource.command command }.to raise_error # FIXME: add a real error in Chef-13
+ end
+
+ it "should initialize the command to nil" do
+ expect(resource.command).to be_nil
+ end
+
+ describe "when the code is not present" do
+ let(:code) { nil }
+ it "raises an exception" do
+ expect { resource.run_action(:run) }.to raise_error # FIXME: add a real error in Chef-13
+ expect { resource.run_action(:run) }.not_to raise_error
+ end
+ end
+ end
+ end
+
+ it "times out when a timeout is set on the resource" do
+ resource.code 'sleep 600'
+ resource.timeout 0.1
+ expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+ end
+end
diff --git a/spec/functional/resource/execute_spec.rb b/spec/functional/resource/execute_spec.rb
index 39fef76ab0..cebcc52fcf 100644
--- a/spec/functional/resource/execute_spec.rb
+++ b/spec/functional/resource/execute_spec.rb
@@ -20,94 +20,99 @@ require 'spec_helper'
require 'functional/resource/base'
describe Chef::Resource::Execute do
- let(:execute_resource) {
- exec_resource = Chef::Resource::Execute.new("foo_resource", run_context)
-
- exec_resource.environment(resource_environment) if resource_environment
- exec_resource.cwd(resource_cwd) if resource_cwd
- exec_resource.command("echo hello")
- if guard
- if guard_options
- exec_resource.only_if(guard, guard_options)
- else
- exec_resource.only_if(guard)
- end
- end
- exec_resource
+ let(:resource) {
+ resource = Chef::Resource::Execute.new("foo_resource", run_context)
+ resource.command("echo hello")
+ resource
}
- let(:resource_environment) { nil }
- let(:resource_cwd) { nil }
- let(:guard) { nil }
- let(:guard_options) { nil }
-
describe "when guard is ruby block" do
it "guard can still run" do
- execute_resource.only_if do
- true
- end
- execute_resource.run_action(:run)
- expect(execute_resource).to be_updated_by_last_action
+ resource.only_if { true }
+ resource.run_action(:run)
+ expect(resource).to be_updated_by_last_action
end
end
describe "when parent resource sets :cwd" do
- let(:resource_cwd) { CHEF_SPEC_DATA }
-
let(:guard) { %{ruby -e 'exit 1 unless File.exists?("./big_json_plus_one.json")'} }
- it "guard inherits :cwd from resource" do
- execute_resource.run_action(:run)
- expect(execute_resource).to be_updated_by_last_action
+ it "guard inherits :cwd from resource and runs" do
+ resource.cwd CHEF_SPEC_DATA
+ resource.only_if guard
+ resource.run_action(:run)
+ expect(resource).to be_updated_by_last_action
+ end
+
+ it "guard inherits :cwd from resource and does not run" do
+ resource.cwd CHEF_SPEC_DATA
+ resource.not_if guard
+ resource.run_action(:run)
+ expect(resource).not_to be_updated_by_last_action
end
end
+ # We use ruby command so that we don't need to deal with platform specific
+ # commands while testing execute resource. We set it so that the resource
+ # will be updated if the ENV variable is set to what we are intending
+ #
+ # FIXME: yeah, but invoking ruby is slow...
describe "when parent resource sets :environment" do
- let(:resource_environment) do
- {
+ before do
+ resource.environment({
"SAWS_SECRET" => "supersecret",
- "SAWS_KEY" => "qwerty"
- }
+ "SAWS_KEY" => "qwerty",
+ })
end
- # We use ruby command so that we don't need to deal with platform specific
- # commands while testing execute resource. We set it so that the resource
- # will be updated if the ENV variable is set to what we are intending
- let(:guard) { %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "supersecret"'} }
-
- it "guard inherits :environment value from resource" do
- execute_resource.run_action(:run)
- expect(execute_resource).to be_updated_by_last_action
+ it "guard inherits :environment value from resource and runs" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "supersecret"'}
+ resource.run_action(:run)
+ expect(resource).to be_updated_by_last_action
end
- describe "when guard sets additional values in the :environment" do
- let(:guard) { %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] != "regularsecret"'} }
-
- let(:guard_options) do
- {
- :environment => { 'SGCE_SECRET' => "regularsecret" }
- }
- end
+ it "guard inherits :environment value from resource and does not run" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] == "supersecret"'}
+ resource.run_action(:run)
+ expect(resource).not_to be_updated_by_last_action
+ end
- it "guard sees merged value for in its ENV" do
- execute_resource.run_action(:run)
- expect(execute_resource).to be_updated_by_last_action
- end
+ it "guard adds additional values in its :environment and runs" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] != "regularsecret"'}, {
+ :environment => { 'SGCE_SECRET' => "regularsecret" }
+ }
+ resource.run_action(:run)
+ expect(resource).to be_updated_by_last_action
end
- describe "when guard sets same value in the :environment" do
- let(:guard) { %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "regularsecret"'} }
+ it "guard adds additional values in its :environment and does not run" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] == "regularsecret"'}, {
+ :environment => { 'SGCE_SECRET' => "regularsecret" }
+ }
+ resource.run_action(:run)
+ expect(resource).not_to be_updated_by_last_action
+ end
- let(:guard_options) do
- {
- :environment => { 'SAWS_SECRET' => "regularsecret" }
- }
- end
+ it "guard overwrites value with its :environment and runs" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "regularsecret"'}, {
+ :environment => { 'SAWS_SECRET' => "regularsecret" }
+ }
+ resource.run_action(:run)
+ expect(resource).to be_updated_by_last_action
+ end
- it "guard sees value from guard options in its ENV" do
- execute_resource.run_action(:run)
- expect(execute_resource).to be_updated_by_last_action
- end
+ it "guard overwrites value with its :environment and does not runs" do
+ resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] == "regularsecret"'}, {
+ :environment => { 'SAWS_SECRET' => "regularsecret" }
+ }
+ resource.run_action(:run)
+ expect(resource).not_to be_updated_by_last_action
end
end
+
+ it "times out when a timeout is set on the resource" do
+ resource.command %{ruby -e 'sleep 600'}
+ resource.timeout 0.1
+ expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+ end
end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index e3de80f3f1..b155e6da24 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -129,6 +129,10 @@ RSpec.configure do |config|
config.filter_run_excluding :ruby_gte_19_only => true unless ruby_gte_19?
config.filter_run_excluding :ruby_20_only => true unless ruby_20?
config.filter_run_excluding :ruby_gte_20_only => true unless ruby_gte_20?
+ # chef_gte_XX_only and chef_lt_XX_only pair up correctly with the same XX
+ # number. please conserve this pattern & resist filling out all the operators
+ config.filter_run_excluding :chef_gte_13_only => true unless chef_gte_13?
+ config.filter_run_excluding :chef_lt_13_only => true unless chef_lt_13?
config.filter_run_excluding :requires_root => true unless root?
config.filter_run_excluding :requires_root_or_running_windows => true unless (root? || windows?)
config.filter_run_excluding :requires_unprivileged_user => true if root?
diff --git a/spec/support/platform_helpers.rb b/spec/support/platform_helpers.rb
index 8d17af3993..db0f9ccee4 100644
--- a/spec/support/platform_helpers.rb
+++ b/spec/support/platform_helpers.rb
@@ -11,6 +11,14 @@ def ruby_lt_20?
!ruby_gte_20?
end
+def chef_gte_13?
+ Chef::VERSION.split('.').first.to_i >= 13
+end
+
+def chef_lt_13?
+ Chef::VERSION.split('.').first.to_i < 13
+end
+
def ruby_gte_19?
RUBY_VERSION.to_f >= 1.9
end
diff --git a/spec/support/shared/unit/script_resource.rb b/spec/support/shared/unit/script_resource.rb
index 58f7f98f19..18ee94606e 100644
--- a/spec/support/shared/unit/script_resource.rb
+++ b/spec/support/shared/unit/script_resource.rb
@@ -21,47 +21,56 @@ require 'spec_helper'
shared_examples_for "a script resource" do
- before(:each) do
- @resource = script_resource
- end
-
it "should create a new Chef::Resource::Script" do
- expect(@resource).to be_a_kind_of(Chef::Resource)
- expect(@resource).to be_a_kind_of(Chef::Resource::Script)
+ expect(script_resource).to be_a_kind_of(Chef::Resource)
+ expect(script_resource).to be_a_kind_of(Chef::Resource::Script)
end
it "should have a resource name of :script" do
- expect(@resource.resource_name).to eql(resource_name)
+ expect(script_resource.resource_name).to eql(resource_name)
end
- it "should set command to the argument provided to new" do
- expect(@resource.command).to eql(resource_instance_name)
+ it "should set command to nil on the resource", :chef_gte_13_only do
+ expect(script_resource.command).to be nil
+ end
+
+ it "should set command to the name on the resource", :chef_lt_13_only do
+ expect(script_resource.command).to eql script_resource.name
end
it "should accept a string for the code" do
- @resource.code "hey jude"
- expect(@resource.code).to eql("hey jude")
+ script_resource.code "hey jude"
+ expect(script_resource.code).to eql("hey jude")
end
it "should accept a string for the flags" do
- @resource.flags "-f"
- expect(@resource.flags).to eql("-f")
+ script_resource.flags "-f"
+ expect(script_resource.flags).to eql("-f")
end
- describe "when executing guards" do
- let(:resource) { @resource }
-
- before(:each) do
- node = Chef::Node.new
+ it "should raise an exception if users set command on the resource", :chef_gte_13_only do
+ expect { script_resource.command('foo') }.to raise_error(Chef::Exceptions::Script)
+ end
- node.automatic[:platform] = "debian"
- node.automatic[:platform_version] = "6.0"
+ it "should not raise an exception if users set command on the resource", :chef_lt_13_only do
+ expect { script_resource.command('foo') }.not_to raise_error
+ end
- events = Chef::EventDispatch::Dispatcher.new
- run_context = Chef::RunContext.new(node, {}, events)
+ describe "when executing guards" do
+ let(:resource) {
+ resource = script_resource
resource.run_context = run_context
resource.code 'echo hi'
- end
+ resource
+ }
+ let(:node) {
+ node = Chef::Node.new
+ node.automatic[:platform] = "debian"
+ node.automatic[:platform_version] = "6.0"
+ node
+ }
+ let(:events) { Chef::EventDispatch::Dispatcher.new }
+ let(:run_context) { Chef::RunContext.new(node, {}, events) }
it "inherits exactly the :cwd, :environment, :group, :path, :user, and :umask attributes from a parent resource class" do
inherited_difference = Chef::Resource::Script.guard_inherited_attributes -
@@ -87,4 +96,3 @@ shared_examples_for "a script resource" do
end
end
end
-
diff --git a/spec/unit/cookbook/syntax_check_spec.rb b/spec/unit/cookbook/syntax_check_spec.rb
index 67d31cdca5..471fc01831 100644
--- a/spec/unit/cookbook/syntax_check_spec.rb
+++ b/spec/unit/cookbook/syntax_check_spec.rb
@@ -43,6 +43,7 @@ describe Chef::Cookbook::SyntaxCheck do
before do
Chef::Log.logger = Logger.new(StringIO.new)
+ @original_log_level = Chef::Log.level
Chef::Log.level = :warn # suppress "Syntax OK" messages
@attr_files = %w{default.rb smokey.rb}.map { |f| File.join(cookbook_path, 'attributes', f) }
@@ -61,6 +62,10 @@ describe Chef::Cookbook::SyntaxCheck do
@template_files = basenames.map { |f| File.join(cookbook_path, 'templates', 'default', f)}
end
+ after do
+ Chef::Log.level = @original_log_level
+ end
+
it "creates a syntax checker given the cookbook name when Chef::Config.cookbook_path is set" do
Chef::Config[:cookbook_path] = File.dirname(cookbook_path)
syntax_check = Chef::Cookbook::SyntaxCheck.for_cookbook(:openldap)
diff --git a/spec/unit/http/validate_content_length_spec.rb b/spec/unit/http/validate_content_length_spec.rb
index 34b6a61a3a..79bd539af3 100644
--- a/spec/unit/http/validate_content_length_spec.rb
+++ b/spec/unit/http/validate_content_length_spec.rb
@@ -83,12 +83,17 @@ describe Chef::HTTP::ValidateContentLength do
let(:debug_stream) { StringIO.new }
let(:debug_output) { debug_stream.string }
- before(:each) {
+ before(:each) do
+ @original_log_level = Chef::Log.level
Chef::Log.level = :debug
allow(Chef::Log).to receive(:debug) do |message|
debug_stream.puts message
end
- }
+ end
+
+ after(:each) do
+ Chef::Log.level = @original_log_level
+ end
describe "without response body" do
let(:request_type) { :direct }
diff --git a/spec/unit/provider/execute_spec.rb b/spec/unit/provider/execute_spec.rb
index ea16d8cf54..794994533e 100644
--- a/spec/unit/provider/execute_spec.rb
+++ b/spec/unit/provider/execute_spec.rb
@@ -15,91 +15,161 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-require File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "spec_helper"))
-#require 'spec_helper'
+
+require 'spec_helper'
describe Chef::Provider::Execute do
- before do
- @node = Chef::Node.new
- @cookbook_collection = Chef::CookbookCollection.new([])
- @events = Chef::EventDispatch::Dispatcher.new
- @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
- @new_resource = Chef::Resource::Execute.new("foo_resource", @run_context)
- @new_resource.timeout 3600
- @new_resource.returns 0
- @new_resource.creates "/foo_resource"
- @provider = Chef::Provider::Execute.new(@new_resource, @run_context)
- @current_resource = Chef::Resource::Ifconfig.new("foo_resource", @run_context)
- @provider.current_resource = @current_resource
- Chef::Log.level = :info
- # FIXME: There should be a test for how STDOUT.tty? changes the live_stream option being passed
- allow(STDOUT).to receive(:tty?).and_return(true)
- end
+
+ let(:node) { Chef::Node.new }
+ let(:events) { Chef::EventDispatch::Dispatcher.new }
+ let(:run_context) { Chef::RunContext.new(node, {}, events) }
+ let(:provider) { Chef::Provider::Execute.new(new_resource, run_context) }
+ let(:current_resource) { Chef::Resource::Ifconfig.new("foo_resource", run_context) }
let(:opts) do
{
- timeout: @new_resource.timeout,
- returns: @new_resource.returns,
- log_level: :info,
- log_tag: @new_resource.to_s,
- live_stream: STDOUT
+ timeout: 3600,
+ returns: 0,
+ log_level: :info,
+ log_tag: new_resource.to_s,
+ live_stream: STDOUT,
}
end
- it "should execute foo_resource" do
- allow(@provider).to receive(:load_current_resource)
- expect(@provider).to receive(:shell_out!).with(@new_resource.command, opts)
- expect(@provider).to receive(:converge_by).with("execute foo_resource").and_call_original
- expect(Chef::Log).not_to receive(:warn)
+ let(:new_resource) { Chef::Resource::Execute.new("foo_resource", run_context) }
- @provider.run_action(:run)
- expect(@new_resource).to be_updated
+ before do
+ @original_log_level = Chef::Log.level
+ Chef::Log.level = :info
+ allow(STDOUT).to receive(:tty?).and_return(true)
end
- it "should honor sensitive attribute" do
- @new_resource.sensitive true
- @provider = Chef::Provider::Execute.new(@new_resource, @run_context)
- allow(@provider).to receive(:load_current_resource)
- # Since the resource is sensitive, it should not have :live_stream set
- expect(@provider).to receive(:shell_out!).with(@new_resource.command, opts.reject { |k| k == :live_stream })
- expect(Chef::Log).not_to receive(:warn)
- expect(@provider).to receive(:converge_by).with("execute sensitive resource").and_call_original
- @provider.run_action(:run)
- expect(@new_resource).to be_updated
+ after do
+ Chef::Log.level = @original_log_level
end
- it "should do nothing if the sentinel file exists" do
- allow(@provider).to receive(:load_current_resource)
- expect(File).to receive(:exists?).with(@new_resource.creates).and_return(true)
- expect(@provider).not_to receive(:shell_out!)
- expect(Chef::Log).not_to receive(:warn)
-
- @provider.run_action(:run)
- expect(@new_resource).not_to be_updated
+ describe "#initialize" do
+ it "should return a Chef::Provider::Execute provider" do
+ expect(provider.class).to eql(Chef::Provider::Execute)
+ end
end
- it "should respect cwd options for 'creates'" do
- @new_resource.cwd "/tmp"
- @new_resource.creates "foo_resource"
- allow(@provider).to receive(:load_current_resource)
- expect(File).to receive(:exists?).with(@new_resource.creates).and_return(false)
- expect(File).to receive(:exists?).with(File.join("/tmp", @new_resource.creates)).and_return(true)
- expect(Chef::Log).not_to receive(:warn)
- expect(@provider).not_to receive(:shell_out!)
-
- @provider.run_action(:run)
- expect(@new_resource).not_to be_updated
+ describe "#load_current_resource" do
+ before do
+ expect(Chef::Resource::Execute).to receive(:new).with(new_resource.name).and_return(current_resource)
+ end
+
+ it "should return the current resource" do
+ expect(provider.load_current_resource).to eql(current_resource)
+ end
+
+ it "our timeout should default to 3600" do
+ provider.load_current_resource
+ expect(provider.timeout).to eql(3600)
+ end
end
- it "should warn if user specified relative path without cwd" do
- @new_resource.creates "foo_resource"
- allow(@provider).to receive(:load_current_resource)
- expect(Chef::Log).to receive(:warn).with(/relative path/)
- expect(File).to receive(:exists?).with(@new_resource.creates).and_return(true)
- expect(@provider).not_to receive(:shell_out!)
+ describe "#action_run" do
+ it "runs shell_out with the default options" do
+ expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+ expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
- @provider.run_action(:run)
- expect(@new_resource).not_to be_updated
+ it "if you pass a command attribute, it runs the command" do
+ new_resource.command "/usr/argelbargle/bin/oogachacka 12345"
+ expect(provider).to receive(:shell_out!).with(new_resource.command, opts)
+ expect(provider).to receive(:converge_by).with("execute #{new_resource.command}").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
+
+ it "should honor sensitive attribute" do
+ new_resource.sensitive true
+ # Since the resource is sensitive, it should not have :live_stream set
+ opts.delete(:live_stream)
+ expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+ expect(provider).to receive(:converge_by).with("execute sensitive resource").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
+
+ it "should do nothing if the sentinel file exists" do
+ new_resource.creates "/foo_resource"
+ expect(FileTest).to receive(:exist?).with(new_resource.creates).and_return(true)
+ expect(provider).not_to receive(:shell_out!)
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).not_to be_updated
+ end
+
+ describe "when the user specifies a relative path without cwd" do
+ before do
+ expect(new_resource.cwd).to be_falsey
+ new_resource.creates "foo_resource"
+ end
+
+ it "should warn in Chef-12", :chef_lt_13_only do
+ expect(Chef::Log).to receive(:warn).with(/relative path/)
+ expect(FileTest).to receive(:exist?).with(new_resource.creates).and_return(true)
+ expect(provider).not_to receive(:shell_out!)
+ provider.run_action(:run)
+ expect(new_resource).not_to be_updated
+ end
+
+ it "should raise if user specified relative path without cwd for Chef-13", :chef_gte_13_only do
+ expect(Chef::Log).to receive(:warn).with(/relative path/)
+ expect(FileTest).to receive(:exist?).with(new_resource.creates).and_return(true)
+ expect(provider).not_to receive(:shell_out!)
+ expect { provider.run_action(:run) }.to raise_error # @todo: add a real error for Chef-13
+ end
+ end
+
+ it "should respect cwd options for 'creates'" do
+ new_resource.cwd "/tmp"
+ new_resource.creates "foo_resource"
+ expect(FileTest).not_to receive(:exist?).with(new_resource.creates)
+ expect(FileTest).to receive(:exist?).with(File.join("/tmp", new_resource.creates)).and_return(true)
+ expect(Chef::Log).not_to receive(:warn)
+ expect(provider).not_to receive(:shell_out!)
+
+ provider.run_action(:run)
+ expect(new_resource).not_to be_updated
+ end
+
+ it "should unset the live_stream if STDOUT is not a tty" do
+ expect(STDOUT).to receive(:tty?).and_return(false)
+ opts.delete(:live_stream)
+ expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+ expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
+
+ it "should unset the live_stream if chef is running as a daemon" do
+ allow(Chef::Config).to receive(:[]).and_call_original
+ expect(Chef::Config).to receive(:[]).with(:daemon).and_return(true)
+ opts.delete(:live_stream)
+ expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+ expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
+
+ it "should unset the live_stream if we are not running with a log level of at least :info" do
+ expect(Chef::Log).to receive(:info?).and_return(false)
+ opts.delete(:live_stream)
+ expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+ expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+ expect(Chef::Log).not_to receive(:warn)
+ provider.run_action(:run)
+ expect(new_resource).to be_updated
+ end
end
end
-
diff --git a/spec/unit/provider/git_spec.rb b/spec/unit/provider/git_spec.rb
index 1e282c098d..0106244665 100644
--- a/spec/unit/provider/git_spec.rb
+++ b/spec/unit/provider/git_spec.rb
@@ -22,6 +22,7 @@ describe Chef::Provider::Git do
before(:each) do
allow(STDOUT).to receive(:tty?).and_return(true)
+ @original_log_level = Chef::Log.level
Chef::Log.level = :info
@current_resource = Chef::Resource::Git.new("web2.0 app")
@@ -38,6 +39,10 @@ describe Chef::Provider::Git do
@provider.current_resource = @current_resource
end
+ after(:each) do
+ Chef::Log.level = @original_log_level
+ end
+
context "determining the revision of the currently deployed checkout" do
before do
diff --git a/spec/unit/provider/script_spec.rb b/spec/unit/provider/script_spec.rb
index 4979475b79..d1759981aa 100644
--- a/spec/unit/provider/script_spec.rb
+++ b/spec/unit/provider/script_spec.rb
@@ -19,77 +19,95 @@
require 'spec_helper'
describe Chef::Provider::Script, "action_run" do
- before(:each) do
- @node = Chef::Node.new
- @events = Chef::EventDispatch::Dispatcher.new
- @run_context = Chef::RunContext.new(@node, {}, @events)
- @new_resource = Chef::Resource::Script.new('run some perl code')
- @new_resource.code "$| = 1; print 'i like beans'"
- @new_resource.interpreter 'perl'
+ let(:node) { Chef::Node.new }
+
+ let(:events) { Chef::EventDispatch::Dispatcher.new }
+
+ let(:run_context) { Chef::RunContext.new(node, {}, events) }
+
+ let(:new_resource) {
+ new_resource = Chef::Resource::Script.new('run some perl code')
+ new_resource.code "$| = 1; print 'i like beans'"
+ new_resource.interpreter 'perl'
+ new_resource
+ }
- @provider = Chef::Provider::Script.new(@new_resource, @run_context)
+ let(:provider) { Chef::Provider::Script.new(new_resource, run_context) }
- @script_file = StringIO.new
- allow(@script_file).to receive(:path).and_return('/tmp/the_script_file')
+ let(:tempfile) { Tempfile.open("rspec-provider-script") }
- allow(@provider).to receive(:shell_out!).and_return(true)
+ before(:each) do
+ allow(provider).to receive(:shell_out!).and_return(true)
+ allow(provider).to receive(:script_file).and_return(tempfile)
end
- it "creates a temporary file to store the script" do
- expect(@provider.script_file).to be_an_instance_of(Tempfile)
+ context "#script_file" do
+ it "creates a temporary file to store the script" do
+ allow(provider).to receive(:script_file).and_call_original
+ expect(provider.script_file).to be_an_instance_of(Tempfile)
+ end
end
- it "unlinks the tempfile when finished" do
- tempfile_path = @provider.script_file.path
- @provider.unlink_script_file
- expect(File.exist?(tempfile_path)).to be_falsey
+ context "#unlink_script_file" do
+ it "unlinks the tempfile" do
+ tempfile_path = tempfile.path
+ provider.unlink_script_file
+ expect(File.exist?(tempfile_path)).to be false
+ end
end
- it "sets the owner and group for the script file" do
- @new_resource.user 'toor'
- @new_resource.group 'wheel'
- allow(@provider).to receive(:script_file).and_return(@script_file)
- expect(FileUtils).to receive(:chown).with('toor', 'wheel', "/tmp/the_script_file")
- @provider.set_owner_and_group
+ context "#set_owner_and_group" do
+ it "sets the owner and group for the script file" do
+ new_resource.user 'toor'
+ new_resource.group 'wheel'
+ expect(FileUtils).to receive(:chown).with('toor', 'wheel', tempfile.path)
+ provider.set_owner_and_group
+ end
end
context "with the script file set to the correct owner and group" do
before do
- allow(@provider).to receive(:set_owner_and_group)
- allow(@provider).to receive(:script_file).and_return(@script_file)
+ allow(provider).to receive(:set_owner_and_group)
end
+
describe "when writing the script to the file" do
it "should put the contents of the script in the temp file" do
- @provider.action_run
- @script_file.rewind
- expect(@script_file.string).to eq("$| = 1; print 'i like beans'\n")
+ allow(provider).to receive(:unlink_script_file) # stub to avoid remove
+ provider.action_run
+ expect(IO.read(tempfile.path)).to eq("$| = 1; print 'i like beans'\n")
+ provider.unlink_script_file
end
it "closes before executing the script and unlinks it when finished" do
- @provider.action_run
- expect(@script_file).to be_closed
+ tempfile_path = tempfile.path
+ provider.action_run
+ expect(tempfile).to be_closed
+ expect(File.exist?(tempfile_path)).to be false
end
-
end
describe "when running the script" do
- it 'should set the command to "interpreter" "tempfile"' do
- @provider.action_run
- expect(@new_resource.command).to eq('"perl" "/tmp/the_script_file"')
- end
+ let (:default_opts) {
+ {timeout: 3600, returns: 0, log_level: :info, log_tag: "script[run some perl code]", live_stream: STDOUT}
+ }
- describe "with flags set on the resource" do
- before do
- @new_resource.flags '-f'
- end
+ before do
+ allow(STDOUT).to receive(:tty?).and_return(true)
+ end
- it "should set the command to 'interpreter flags tempfile'" do
- @provider.action_run
- expect(@new_resource.command).to eq('"perl" -f "/tmp/the_script_file"')
- end
+ it 'should set the command to "interpreter" "tempfile"' do
+ expect(provider.command).to eq(%Q{"perl" "#{tempfile.path}"})
+ end
+ it 'should call shell_out! with the command' do
+ expect(provider).to receive(:shell_out!).with(provider.command, default_opts).and_return(true)
+ provider.action_run
end
+ it "should set the command to 'interpreter flags tempfile'" do
+ new_resource.flags '-f'
+ expect(provider.command).to eq(%Q{"perl" -f "#{tempfile.path}"})
+ end
end
end
diff --git a/spec/unit/resource/script_spec.rb b/spec/unit/resource/script_spec.rb
index 9d744baaa5..4affee8e8c 100644
--- a/spec/unit/resource/script_spec.rb
+++ b/spec/unit/resource/script_spec.rb
@@ -29,18 +29,16 @@ describe Chef::Resource::Script do
expect(script_resource.interpreter).to eql("naaaaNaNaNaaNaaNaaNaa")
end
- describe "when it has interpreter and flags" do
+ context "when it has interpreter and flags" do
before do
- script_resource.command("grep")
script_resource.interpreter("gcc")
script_resource.flags("-al")
end
- it "returns the command as its identity" do
- expect(script_resource.identity).to eq("grep")
+ it "returns the name as its identity" do
+ expect(script_resource.identity).to eq(resource_instance_name)
end
end
it_behaves_like "a script resource"
end
-
diff --git a/spec/unit/run_context_spec.rb b/spec/unit/run_context_spec.rb
index bb251c9cf4..cc112f332a 100644
--- a/spec/unit/run_context_spec.rb
+++ b/spec/unit/run_context_spec.rb
@@ -21,10 +21,11 @@
require 'spec_helper'
require 'support/lib/library_load_order'
-Chef::Log.level = :debug
describe Chef::RunContext do
before(:each) do
+ @original_log_level = Chef::Log.level
+ Chef::Log.level = :debug
@chef_repo_path = File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "cookbooks"))
cl = Chef::CookbookLoader.new(@chef_repo_path)
cl.load_cookbooks
@@ -35,6 +36,10 @@ describe Chef::RunContext do
@run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
end
+ after(:each) do
+ Chef::Log.level = @original_log_level
+ end
+
it "has a cookbook collection" do
expect(@run_context.cookbook_collection).to eq(@cookbook_collection)
end