diff options
| -rw-r--r-- | .travis.yml | 1 | ||||
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | lib/chef/dsl/recipe.rb | 42 | ||||
| -rw-r--r-- | lib/chef/exceptions.rb | 8 | ||||
| -rw-r--r-- | lib/chef/knife/cookbook_site_install.rb | 2 | ||||
| -rw-r--r-- | lib/chef/mixin/shell_out.rb | 8 | ||||
| -rw-r--r-- | lib/chef/platform/provider_priority_map.rb | 1 | ||||
| -rw-r--r-- | lib/chef/provider/package/macports.rb | 3 | ||||
| -rw-r--r-- | lib/chef/provider/user/dscl.rb | 21 | ||||
| -rw-r--r-- | lib/chef/resource.rb | 20 | ||||
| -rw-r--r-- | lib/chef/resource/chef_gem.rb | 23 | ||||
| -rw-r--r-- | lib/chef/resource/macports_package.rb | 3 | ||||
| -rw-r--r-- | lib/chef/resource_builder.rb | 137 | ||||
| -rw-r--r-- | spec/functional/resource/execute_spec.rb | 9 | ||||
| -rw-r--r-- | spec/unit/knife/cookbook_site_install_spec.rb | 6 | ||||
| -rw-r--r-- | spec/unit/mixin/shell_out_spec.rb | 71 | ||||
| -rw-r--r-- | spec/unit/provider/user/dscl_spec.rb | 16 | ||||
| -rw-r--r-- | spec/unit/provider_resolver_spec.rb | 14 | ||||
| -rw-r--r-- | spec/unit/recipe_spec.rb | 75 | ||||
| -rw-r--r-- | spec/unit/resource/chef_gem_spec.rb | 60 | ||||
| -rw-r--r-- | spec/unit/resource_builder_spec.rb | 1 | ||||
| -rw-r--r-- | spec/unit/resource_spec.rb | 6 |
22 files changed, 429 insertions, 101 deletions
diff --git a/.travis.yml b/.travis.yml index 37418ab621..c083a31ba9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,7 @@ matrix: include: - rvm: 2.0.0 - rvm: 2.1.5 + - rvm: 2.2.0 - rvm: 2.1.5 gemfile: pedant.gemfile script: bundle exec rake pedant diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ea4b68264..d39d0b7820 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ * Cleaned up script and execute provider + specs * Added deprecation warnings around the use of command attribute in script resources * Audit mode feature added - see the RELEASE_NOTES for details +* shell_out now sets `LANGUAGE` and `LANG` to the `Chef::Config[:internal_locale]` in addition to `LC_ALL` forcing +* chef_gem supports a compile_time flag and will warn if it is not set (behavior will change in the future) +* suppress 3694 warnings on the most trivial resource cloning ## 12.0.3 * [**Phil Dibowitz**](https://github.com/jaymzh): diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb index d70266c14c..edd7e9aed1 100644 --- a/lib/chef/dsl/recipe.rb +++ b/lib/chef/dsl/recipe.rb @@ -19,6 +19,7 @@ require 'chef/mixin/convert_to_class_name' require 'chef/exceptions' +require 'chef/resource_builder' class Chef module DSL @@ -92,37 +93,16 @@ class Chef def build_resource(type, name, created_at=nil, &resource_attrs_block) created_at ||= caller[0] - # Checks the new platform => short_name => resource mapping initially - # then fall back to the older approach (Chef::Resource.const_get) for - # backward compatibility - resource_class = resource_class_for(type) - - raise ArgumentError, "You must supply a name when declaring a #{type} resource" if name.nil? - - resource = resource_class.new(name, run_context) - resource.source_line = created_at - resource.declared_type = type - # If we have a resource like this one, we want to steal its state - # This behavior is very counter-intuitive and should be removed. - # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694 - # Moved to this location to resolve CHEF-5052, https://tickets.opscode.com/browse/CHEF-5052 - resource.load_prior_resource(type, name) - resource.cookbook_name = cookbook_name - resource.recipe_name = recipe_name - # Determine whether this resource is being created in the context of an enclosing Provider - resource.enclosing_provider = self.is_a?(Chef::Provider) ? self : nil - - # XXX: This is very crufty, but it's required for resource definitions - # to work properly :( - resource.params = @params - - # Evaluate resource attribute DSL - resource.instance_eval(&resource_attrs_block) if block_given? - - # Run optional resource hook - resource.after_created - - resource + Chef::ResourceBuilder.new( + type: type, + name: name, + created_at: created_at, + params: @params, + run_context: run_context, + cookbook_name: cookbook_name, + recipe_name: recipe_name, + enclosing_provider: self.is_a?(Chef::Provider) ? self : nil + ).build(&resource_attrs_block) end def resource_class_for(snake_case_name) diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index b949e7b975..18b8ee5d3f 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -152,8 +152,12 @@ class Chef class MetadataNotValid < StandardError; end class MetadataNotFound < StandardError - def initialize - super "No metadata.rb or metadata.json!" + attr_reader :install_path + attr_reader :cookbook_name + def initialize(install_path, cookbook_name) + @install_path = install_path + @cookbook_name = cookbook_name + super "No metadata.rb or metadata.json found for cookbook #{@cookbook_name} in #{@install_path}" end end diff --git a/lib/chef/knife/cookbook_site_install.rb b/lib/chef/knife/cookbook_site_install.rb index edf8dd14f0..d0ab6da3ef 100644 --- a/lib/chef/knife/cookbook_site_install.rb +++ b/lib/chef/knife/cookbook_site_install.rb @@ -181,7 +181,7 @@ class Chef return md end - raise Chef::Exceptions::MetadataNotFound + raise Chef::Exceptions::MetadataNotFound.new(@install_path, @cookbook_name) end end end diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb index 5b05e788db..d3c14fa408 100644 --- a/lib/chef/mixin/shell_out.rb +++ b/lib/chef/mixin/shell_out.rb @@ -36,9 +36,15 @@ class Chef options[env_key] ||= {} options[env_key] = options[env_key].dup options[env_key]['LC_ALL'] ||= Chef::Config[:internal_locale] unless options[env_key].has_key?('LC_ALL') + options[env_key]['LANGUAGE'] ||= Chef::Config[:internal_locale] unless options[env_key].has_key?('LANGUAGE') + options[env_key]['LANG'] ||= Chef::Config[:internal_locale] unless options[env_key].has_key?('LANG') args << options else - args << { :environment => { 'LC_ALL' => Chef::Config[:internal_locale] } } + args << { :environment => { + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + } } end shell_out_command(*args) diff --git a/lib/chef/platform/provider_priority_map.rb b/lib/chef/platform/provider_priority_map.rb index 765dd74859..aa69761012 100644 --- a/lib/chef/platform/provider_priority_map.rb +++ b/lib/chef/platform/provider_priority_map.rb @@ -66,6 +66,7 @@ class Chef # priority :service, Chef::Provider::Service::Macosx, os: "darwin" + priority :package, Chef::Provider::Package::Homebrew, os: "darwin" end def priority_map diff --git a/lib/chef/provider/package/macports.rb b/lib/chef/provider/package/macports.rb index cd142eca42..248dc75d28 100644 --- a/lib/chef/provider/package/macports.rb +++ b/lib/chef/provider/package/macports.rb @@ -3,7 +3,8 @@ class Chef class Package class Macports < Chef::Provider::Package - provides :macports_package, os: "mac_os_x" + provides :macports_package + provides :package, os: "darwin" def load_current_resource @current_resource = Chef::Resource::Package.new(@new_resource.name) diff --git a/lib/chef/provider/user/dscl.rb b/lib/chef/provider/user/dscl.rb index 39746f0018..debf36f771 100644 --- a/lib/chef/provider/user/dscl.rb +++ b/lib/chef/provider/user/dscl.rb @@ -239,8 +239,18 @@ user password using shadow hash.") # def uid_used?(uid) return false unless uid - users_uids = run_dscl("list /Users uid") - !! ( users_uids =~ Regexp.new("#{Regexp.escape(uid.to_s)}\n") ) + users_uids = run_dscl("list /Users uid").split("\n") + uid_map = users_uids.inject({}) do |tmap, tuid| + x = tuid.split + tmap[x[1]] = x[0] + tmap + end + if uid_map[uid.to_s] + unless uid_map[uid.to_s] == @new_resource.username.to_s + return true + end + end + return false end # @@ -504,6 +514,11 @@ user password using shadow hash.") # password to be updated. return true if salted_sha512?(@current_resource.password) + # Some system users don't have salts; this can happen if the system is + # upgraded and the user hasn't logged in yet. In this case, we will force + # the password to be updated. + return true if @current_resource.salt.nil? + if salted_sha512_pbkdf2?(@new_resource.password) diverged?(:password) || diverged?(:salt) || diverged?(:iterations) else @@ -535,7 +550,7 @@ user password using shadow hash.") # A simple map of Chef's terms to DSCL's terms. DSCL_PROPERTY_MAP = { - :uid => "generateduid", + :uid => "uid", :gid => "gid", :home => "home", :shell => "shell", diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 17f109242f..bf49cd9d26 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -213,23 +213,11 @@ class Chef end end - def load_prior_resource(resource_type, instance_name) - begin - key = "#{resource_type}[#{instance_name}]" - prior_resource = run_context.resource_collection.lookup(key) - # if we get here, there is a prior resource (otherwise we'd have jumped - # to the rescue clause). - Chef::Log.warn("Cloning resource attributes for #{key} from prior resource (CHEF-3694)") - Chef::Log.warn("Previous #{prior_resource}: #{prior_resource.source_line}") if prior_resource.source_line - Chef::Log.warn("Current #{self}: #{self.source_line}") if self.source_line - prior_resource.instance_variables.each do |iv| - unless iv.to_sym == :@source_line || iv.to_sym == :@action || iv.to_sym == :@not_if || iv.to_sym == :@only_if - self.instance_variable_set(iv, prior_resource.instance_variable_get(iv)) - end + def load_from(resource) + resource.instance_variables.each do |iv| + unless iv == :@source_line || iv == :@action || iv == :@not_if || iv == :@only_if + self.instance_variable_set(iv, resource.instance_variable_get(iv)) end - true - rescue Chef::Exceptions::ResourceNotFound - true end end diff --git a/lib/chef/resource/chef_gem.rb b/lib/chef/resource/chef_gem.rb index b4ead11f1d..126e3d56c3 100644 --- a/lib/chef/resource/chef_gem.rb +++ b/lib/chef/resource/chef_gem.rb @@ -28,6 +28,7 @@ class Chef def initialize(name, run_context=nil) super @resource_name = :chef_gem + @compile_time = nil @gem_binary = RbConfig::CONFIG['bindir'] + "/gem" end @@ -40,13 +41,29 @@ class Chef @gem_binary end + def compile_time(arg=nil) + set_or_return( + :compile_time, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end + def after_created # Chef::Resource.run_action: Caveat: this skips Chef::Runner.run_action, where notifications are handled # Action could be an array of symbols, but probably won't (think install + enable for a package) - Array(@action).each do |action| - self.run_action(action) + if compile_time.nil? + Chef::Log.warn "The chef_gem installation at compile time is deprecated and this behavior will change in the future." + Chef::Log.warn "Please set `compile_time false` on the resource to use the new behavior and suppress this warning," + Chef::Log.warn "or you may set `compile_time true` on the resource if compile_time behavior is necessary." + end + + if compile_time || compile_time.nil? + Array(action).each do |action| + self.run_action(action) + end + Gem.clear_paths end - Gem.clear_paths end end end diff --git a/lib/chef/resource/macports_package.rb b/lib/chef/resource/macports_package.rb index bdc8698155..0d4e5dec65 100644 --- a/lib/chef/resource/macports_package.rb +++ b/lib/chef/resource/macports_package.rb @@ -20,7 +20,8 @@ class Chef class Resource class MacportsPackage < Chef::Resource::Package - provides :macports_package, os: "mac_os_x" + provides :macports_package + provides :package, os: "darwin" def initialize(name, run_context=nil) super diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb new file mode 100644 index 0000000000..f700da53c9 --- /dev/null +++ b/lib/chef/resource_builder.rb @@ -0,0 +1,137 @@ +# +# Author:: Lamont Granquist (<lamont@chef.io>) +# Copyright:: Copyright (c) 2015 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. +# + +# NOTE: this was extracted from the Recipe DSL mixin, relevant specs are in spec/unit/recipe_spec.rb + +class Chef + class ResourceBuilder + attr_reader :type + attr_reader :name + attr_reader :created_at + attr_reader :params + attr_reader :run_context + attr_reader :cookbook_name + attr_reader :recipe_name + attr_reader :enclosing_provider + attr_reader :resource + + # FIXME (ruby-2.1 syntax): most of these are mandatory + def initialize(type:nil, name:nil, created_at: nil, params: nil, run_context: nil, cookbook_name: nil, recipe_name: nil, enclosing_provider: nil) + @type = type + @name = name + @created_at = created_at + @params = params + @run_context = run_context + @cookbook_name = cookbook_name + @recipe_name = recipe_name + @enclosing_provider = enclosing_provider + end + + def build(&block) + raise ArgumentError, "You must supply a name when declaring a #{type} resource" if name.nil? + + @resource = resource_class.new(name, run_context) + resource.source_line = created_at + resource.declared_type = type + + # If we have a resource like this one, we want to steal its state + # This behavior is very counter-intuitive and should be removed. + # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694 + # Moved to this location to resolve CHEF-5052, https://tickets.opscode.com/browse/CHEF-5052 + if prior_resource + resource.load_from(prior_resource) + end + + resource.cookbook_name = cookbook_name + resource.recipe_name = recipe_name + # Determine whether this resource is being created in the context of an enclosing Provider + resource.enclosing_provider = enclosing_provider + + # XXX: this is required for definition params inside of the scope of a + # subresource to work correctly. + resource.params = params + + # Evaluate resource attribute DSL + resource.instance_eval(&block) if block_given? + + # emit a cloned resource warning if it is warranted + if prior_resource + if is_trivial_resource?(prior_resource) && identicalish_resources?(prior_resource, resource) + emit_harmless_cloning_debug + else + emit_cloned_resource_warning + end + end + + # Run optional resource hook + resource.after_created + + resource + end + + private + + def resource_class + # Checks the new platform => short_name => resource mapping initially + # then fall back to the older approach (Chef::Resource.const_get) for + # backward compatibility + resource_class ||= Chef::Resource.resource_for_node(type, run_context.node) + end + + def is_trivial_resource?(resource) + identicalish_resources?(resource_class.new(name, run_context), resource) + end + + # this is an equality test specific to checking for 3694 cloning warnings + def identicalish_resources?(first, second) + skipped_ivars = [ :@source_line, :@cookbook_name, :@recipe_name, :@params, :@elapsed_time, :@declared_type ] + checked_ivars = ( first.instance_variables | second.instance_variables ) - skipped_ivars + non_matching_ivars = checked_ivars.reject do |iv| + if iv == :@action && ( [first.instance_variable_get(iv)].flatten == [:nothing] || [second.instance_variable_get(iv)].flatten == [:nothing] ) + # :nothing action on either side of the comparison always matches + true + else + first.instance_variable_get(iv) == second.instance_variable_get(iv) + end + end + Chef::Log.debug("ivars which did not match with the prior resource: #{non_matching_ivars}") + non_matching_ivars.empty? + end + + def emit_cloned_resource_warning + Chef::Log.warn("Cloning resource attributes for #{self} from prior resource (CHEF-3694)") + Chef::Log.warn("Previous #{prior_resource}: #{prior_resource.source_line}") if prior_resource.source_line + Chef::Log.warn("Current #{resource}: #{resource.source_line}") if resource.source_line + end + + def emit_harmless_cloning_debug + Chef::Log.debug("Harmless resource cloning from #{prior_resource}:#{prior_resource.source_line} to #{resource}:#{resource.source_line}") + end + + def prior_resource + @prior_resource ||= + begin + key = "#{type}[#{name}]" + prior_resource = run_context.resource_collection.lookup(key) + rescue Chef::Exceptions::ResourceNotFound + nil + end + end + + end +end diff --git a/spec/functional/resource/execute_spec.rb b/spec/functional/resource/execute_spec.rb index cebcc52fcf..020814fcd6 100644 --- a/spec/functional/resource/execute_spec.rb +++ b/spec/functional/resource/execute_spec.rb @@ -18,6 +18,7 @@ require 'spec_helper' require 'functional/resource/base' +require 'timeout' describe Chef::Resource::Execute do let(:resource) { @@ -111,8 +112,10 @@ describe Chef::Resource::Execute do 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) + Timeout::timeout(5) do + resource.command %{ruby -e 'sleep 600'} + resource.timeout 0.1 + expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout) + end end end diff --git a/spec/unit/knife/cookbook_site_install_spec.rb b/spec/unit/knife/cookbook_site_install_spec.rb index b3eef32b39..07b268bb64 100644 --- a/spec/unit/knife/cookbook_site_install_spec.rb +++ b/spec/unit/knife/cookbook_site_install_spec.rb @@ -184,7 +184,11 @@ describe Chef::Knife::CookbookSiteInstall do end it "rasies an error if it finds no metadata file" do - expect { knife.preferred_metadata }.to raise_error(Chef::Exceptions::MetadataNotFound) + expect { knife.preferred_metadata }.to raise_error { |error| + expect(error).to be_a(Chef::Exceptions::MetadataNotFound) + expect(error.cookbook_name).to eq("post-punk-kitchen") + expect(error.install_path).to eq(install_path) + } end end diff --git a/spec/unit/mixin/shell_out_spec.rb b/spec/unit/mixin/shell_out_spec.rb index 3dc9d42574..beaf624ddb 100644 --- a/spec/unit/mixin/shell_out_spec.rb +++ b/spec/unit/mixin/shell_out_spec.rb @@ -44,7 +44,7 @@ describe Chef::Mixin::ShellOut do context 'without deprecated options' do let(:options) { { :environment => environment } } - let(:environment) { { 'LC_ALL' => 'C' } } + let(:environment) { { 'LC_ALL' => 'C', 'LANG' => 'C', 'LANGUAGE' => 'C' } } it 'should not edit command args' do is_expected.to eql(command_args) @@ -123,30 +123,40 @@ describe Chef::Mixin::ShellOut do describe "when the last argument is a Hash" do describe "and environment is an option" do - it "should not change environment['LC_ALL'] when set to nil" do - options = { :environment => { 'LC_ALL' => nil } } + it "should not change environment language settings when they are set to nil" do + options = { :environment => { 'LC_ALL' => nil, 'LANGUAGE' => nil, 'LANG' => nil } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should not change environment['LC_ALL'] when set to non-nil" do - options = { :environment => { 'LC_ALL' => 'en_US.UTF-8' } } + it "should not change environment language settings when they are set to non-nil" do + options = { :environment => { 'LC_ALL' => 'en_US.UTF-8', 'LANGUAGE' => 'en_US.UTF-8', 'LANG' => 'en_US.UTF-8' } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should set environment['LC_ALL'] to 'en_US.UTF-8' when 'LC_ALL' not present" do + it "should set environment language settings to the configured internal locale when they are not present" do options = { :environment => { 'HOME' => '/Users/morty' } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :environment => { 'HOME' => '/Users/morty', 'LC_ALL' => Chef::Config[:internal_locale] }, + :environment => { + 'HOME' => '/Users/morty', + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + }, }).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should not mutate the options hash when it adds LC_ALL" do + it "should not mutate the options hash when it adds language settings" do options = { :environment => { 'HOME' => '/Users/morty' } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :environment => { 'HOME' => '/Users/morty', 'LC_ALL' => Chef::Config[:internal_locale] }, + :environment => { + 'HOME' => '/Users/morty', + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + }, }).and_return(true) shell_out_obj.shell_out(cmd, options) expect(options[:environment].has_key?('LC_ALL')).to be false @@ -154,30 +164,40 @@ describe Chef::Mixin::ShellOut do end describe "and env is an option" do - it "should not change env when set to nil" do - options = { :env => { 'LC_ALL' => nil } } + it "should not change env when langauge options are set to nil" do + options = { :env => { 'LC_ALL' => nil, 'LANG' => nil, 'LANGUAGE' => nil } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should not change env when set to non-nil" do - options = { :env => { 'LC_ALL' => 'de_DE.UTF-8'}} + it "should not change env when language options are set to non-nil" do + options = { :env => { 'LC_ALL' => 'de_DE.UTF-8', 'LANG' => 'de_DE.UTF-8', 'LANGUAGE' => 'de_DE.UTF-8' }} expect(shell_out_obj).to receive(:shell_out_command).with(cmd, options).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should set env['LC_ALL'] to 'en_US.UTF-8' when 'LC_ALL' not present" do + it "should set environment language settings to the configured internal locale when they are not present" do options = { :env => { 'HOME' => '/Users/morty' } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :env => { 'HOME' => '/Users/morty', 'LC_ALL' => Chef::Config[:internal_locale] }, + :env => { + 'HOME' => '/Users/morty', + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + } }).and_return(true) shell_out_obj.shell_out(cmd, options) end - it "should not mutate the options hash when it adds LC_ALL" do + it "should not mutate the options hash when it adds language settings" do options = { :env => { 'HOME' => '/Users/morty' } } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :env => { 'HOME' => '/Users/morty', 'LC_ALL' => Chef::Config[:internal_locale] }, + :env => { + 'HOME' => '/Users/morty', + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + } }).and_return(true) shell_out_obj.shell_out(cmd, options) expect(options[:env].has_key?('LC_ALL')).to be false @@ -185,10 +205,15 @@ describe Chef::Mixin::ShellOut do end describe "and no env/environment option is present" do - it "should add environment option and set environment['LC_ALL'] to 'en_US.UTF_8'" do + it "should set environment language settings to the configured internal locale" do options = { :user => 'morty' } expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :user => 'morty', :environment => { 'LC_ALL' => Chef::Config[:internal_locale] }, + :user => 'morty', + :environment => { + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + }, }).and_return(true) shell_out_obj.shell_out(cmd, options) end @@ -196,9 +221,13 @@ describe Chef::Mixin::ShellOut do end describe "when the last argument is not a Hash" do - it "should add environment options and set environment['LC_ALL'] to 'en_US.UTF-8'" do + it "should set environment language settings to the configured internal locale" do expect(shell_out_obj).to receive(:shell_out_command).with(cmd, { - :environment => { 'LC_ALL' => Chef::Config[:internal_locale] }, + :environment => { + 'LC_ALL' => Chef::Config[:internal_locale], + 'LANG' => Chef::Config[:internal_locale], + 'LANGUAGE' => Chef::Config[:internal_locale], + }, }).and_return(true) shell_out_obj.shell_out(cmd) end diff --git a/spec/unit/provider/user/dscl_spec.rb b/spec/unit/provider/user/dscl_spec.rb index 24691cce33..5ea037d944 100644 --- a/spec/unit/provider/user/dscl_spec.rb +++ b/spec/unit/provider/user/dscl_spec.rb @@ -422,7 +422,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" it "collects the user data correctly" do provider.load_current_resource expect(provider.current_resource.comment).to eq("vagrant") - expect(provider.current_resource.uid).to eq("11112222-3333-4444-AAAA-BBBBCCCCDDDD") + expect(provider.current_resource.uid).to eq("501") expect(provider.current_resource.gid).to eq("80") expect(provider.current_resource.home).to eq("/Users/vagrant") expect(provider.current_resource.shell).to eq("/bin/bash") @@ -487,7 +487,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" it "collects the user data correctly" do provider.load_current_resource expect(provider.current_resource.comment).to eq("vagrant") - expect(provider.current_resource.uid).to eq("11112222-3333-4444-AAAA-BBBBCCCCDDDD") + expect(provider.current_resource.uid).to eq("501") expect(provider.current_resource.gid).to eq("80") expect(provider.current_resource.home).to eq("/Users/vagrant") expect(provider.current_resource.shell).to eq("/bin/bash") @@ -513,7 +513,7 @@ e68d1f9821b26689312366") it "collects the user data correctly" do provider.load_current_resource expect(provider.current_resource.comment).to eq("vagrant") - expect(provider.current_resource.uid).to eq("11112222-3333-4444-AAAA-BBBBCCCCDDDD") + expect(provider.current_resource.uid).to eq("501") expect(provider.current_resource.gid).to eq("80") expect(provider.current_resource.home).to eq("/Users/vagrant") expect(provider.current_resource.shell).to eq("/bin/bash") @@ -551,7 +551,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") it "collects the user data correctly" do provider.load_current_resource expect(provider.current_resource.comment).to eq("vagrant") - expect(provider.current_resource.uid).to eq("11112222-3333-4444-AAAA-BBBBCCCCDDDD") + expect(provider.current_resource.uid).to eq("501") expect(provider.current_resource.gid).to eq("80") expect(provider.current_resource.home).to eq("/Users/vagrant") expect(provider.current_resource.shell).to eq("/bin/bash") @@ -610,6 +610,14 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") expect(provider.diverged_password?).to be_truthy end end + + describe "when salt isn't found" do + it "diverged_password? should report true" do + provider.load_current_resource + provider.current_resource.salt(nil) + expect(provider.diverged_password?).to be_truthy + end + end end end end diff --git a/spec/unit/provider_resolver_spec.rb b/spec/unit/provider_resolver_spec.rb index ab19ff4bee..a9fa08ebfd 100644 --- a/spec/unit/provider_resolver_spec.rb +++ b/spec/unit/provider_resolver_spec.rb @@ -511,10 +511,11 @@ describe Chef::ProviderResolver do supported_providers = [ :apt_package, :bash, :breakpoint, :chef_gem, :cookbook_file, :csh, :deploy, - :deploy_revision, :directory, :dpkg_package, :easy_install_package, - :erl_call, :execute, :file, :gem_package, :git, :http_request, :link, :log, :pacman_package, :paludis_package, - :perl, :python, :remote_directory, :route, :rpm_package, :ruby, :ruby_block, :script, - :subversion, :template, :timestamped_deploy, :whyrun_safe_ruby_block, :yum_package, :homebrew_package, + :deploy_revision, :directory, :dpkg_package, :easy_install_package, :erl_call, + :execute, :file, :gem_package, :git, :homebrew_package, :http_request, :link, + :log, :macports_package, :pacman_package, :paludis_package, :perl, :python, + :remote_directory, :route, :rpm_package, :ruby, :ruby_block, :script, :subversion, + :template, :timestamped_deploy, :whyrun_safe_ruby_block, :yum_package, ] supported_providers.each do |static_resource| @@ -530,9 +531,8 @@ describe Chef::ProviderResolver do end unsupported_providers = [ - :bff_package, :dsc_script, :ips_package, :macports_package, - :smartos_package, :solaris_package, :windows_package, - :windows_service, + :bff_package, :dsc_script, :ips_package, :smartos_package, + :solaris_package, :windows_package, :windows_service, ] unsupported_providers.each do |static_resource| diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb index e8c1358ba2..22389a1a82 100644 --- a/spec/unit/recipe_spec.rb +++ b/spec/unit/recipe_spec.rb @@ -161,7 +161,82 @@ describe Chef::Recipe do zm_resource # force let binding evaluation expect { run_context.resource_collection.resources(:zen_master => "klopp") }.to raise_error(Chef::Exceptions::ResourceNotFound) end + end + + describe "when cloning resources" do + def expect_warning + expect(Chef::Log).to receive(:warn).with(/3694/) + expect(Chef::Log).to receive(:warn).with(/Previous/) + expect(Chef::Log).to receive(:warn).with(/Current/) + end + + it "should emit a 3694 warning when attributes change" do + recipe.zen_master "klopp" do + something "bvb" + end + expect_warning + recipe.zen_master "klopp" do + something "vbv" + end + end + + it "should emit a 3694 warning when attributes change" do + recipe.zen_master "klopp" do + something "bvb" + end + expect_warning + recipe.zen_master "klopp" do + something "bvb" + peace true + end + end + it "should emit a 3694 warning when attributes change" do + recipe.zen_master "klopp" do + something "bvb" + peace true + end + expect_warning + recipe.zen_master "klopp" do + something "bvb" + end + end + + it "should emit a 3694 warning for non-trivial attributes (unfortunately)" do + recipe.zen_master "klopp" do + something "bvb" + end + expect_warning + recipe.zen_master "klopp" do + something "bvb" + end + end + + it "should not emit a 3694 warning for completely trivial resource cloning" do + recipe.zen_master "klopp" + expect(Chef::Log).to_not receive(:warn) + recipe.zen_master "klopp" + end + + it "should not emit a 3694 warning when attributes do not change and the first action is :nothing" do + recipe.zen_master "klopp" do + action :nothing + end + expect(Chef::Log).to_not receive(:warn) + recipe.zen_master "klopp" do + action :score + end + end + + it "should not emit a 3694 warning when attributes do not change and the second action is :nothing" do + recipe.zen_master "klopp" do + action :score + end + expect(Chef::Log).to_not receive(:warn) + recipe.zen_master "klopp" do + action :nothing + end + end end describe "creating resources via declare_resource" do diff --git a/spec/unit/resource/chef_gem_spec.rb b/spec/unit/resource/chef_gem_spec.rb index 480856d19f..657713d54f 100644 --- a/spec/unit/resource/chef_gem_spec.rb +++ b/spec/unit/resource/chef_gem_spec.rb @@ -32,16 +32,70 @@ describe Chef::Resource::ChefGem, "initialize" do end describe Chef::Resource::ChefGem, "gem_binary" do + let(:resource) { Chef::Resource::ChefGem.new("foo") } + before(:each) do expect(RbConfig::CONFIG).to receive(:[]).with('bindir').and_return("/opt/chef/embedded/bin") - @resource = Chef::Resource::ChefGem.new("foo") end it "should raise an exception when gem_binary is set" do - expect { @resource.gem_binary("/lol/cats/gem") }.to raise_error(ArgumentError) + expect { resource.gem_binary("/lol/cats/gem") }.to raise_error(ArgumentError) + end + + it "should set the gem_binary based on computing it from RbConfig" do + expect(resource.gem_binary).to eql("/opt/chef/embedded/bin/gem") end it "should set the gem_binary based on computing it from RbConfig" do - expect(@resource.gem_binary).to eql("/opt/chef/embedded/bin/gem") + expect(resource.compile_time).to be nil + end + + context "when building the resource" do + let(:node) do + Chef::Node.new.tap {|n| n.normal[:tags] = [] } + end + + let(:run_context) do + Chef::RunContext.new(node, {}, nil) + end + + let(:recipe) do + Chef::Recipe.new("hjk", "test", run_context) + end + + let(:resource) { Chef::Resource::ChefGem.new("foo", run_context) } + + before do + expect(Chef::Resource::ChefGem).to receive(:new).and_return(resource) + end + + it "runs the install at compile-time by default", :chef_lt_13_only do + expect(resource).to receive(:run_action).with(:install) + expect(Chef::Log).to receive(:warn).at_least(:once) + recipe.chef_gem "foo" + end + + # the default behavior will change in Chef-13 + it "does not runs the install at compile-time by default", :chef_gte_13_only do + expect(resource).not_to receive(:run_action).with(:install) + expect(Chef::Log).not_to receive(:warn) + recipe.chef_gem "foo" + end + + it "compile_time true installs at compile-time" do + expect(resource).to receive(:run_action).with(:install) + expect(Chef::Log).not_to receive(:warn) + recipe.chef_gem "foo" do + compile_time true + end + end + + it "compile_time false does not install at compile-time" do + expect(resource).not_to receive(:run_action).with(:install) + expect(Chef::Log).not_to receive(:warn) + recipe.chef_gem "foo" do + compile_time false + end + end end end diff --git a/spec/unit/resource_builder_spec.rb b/spec/unit/resource_builder_spec.rb new file mode 100644 index 0000000000..e38c7fb63a --- /dev/null +++ b/spec/unit/resource_builder_spec.rb @@ -0,0 +1 @@ +# see spec/unit/recipe_spec.rb diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb index 79d47ad4dc..56c7401c92 100644 --- a/spec/unit/resource_spec.rb +++ b/spec/unit/resource_spec.rb @@ -162,7 +162,7 @@ describe Chef::Resource do end end - describe "load_prior_resource" do + describe "load_from" do before(:each) do @prior_resource = Chef::Resource.new("funk") @prior_resource.supports(:funky => true) @@ -174,12 +174,12 @@ describe Chef::Resource do end it "should load the attributes of a prior resource" do - @resource.load_prior_resource(@resource.resource_name, @resource.name) + @resource.load_from(@prior_resource) expect(@resource.supports).to eq({ :funky => true }) end it "should not inherit the action from the prior resource" do - @resource.load_prior_resource(@resource.resource_name, @resource.name) + @resource.load_from(@prior_resource) expect(@resource.action).not_to eq(@prior_resource.action) end end |