Merge branch 'contributions'

18 files changed, 153 insertions, 30 deletions

diff --git a/lib/chef/data_bag.rb b/lib/chef/data_bag.rb
index 5994e6f8d1..639d71a74d 100644
--- a/lib/chef/data_bag.rb
+++ b/lib/chef/data_bag.rb
@@ -31,7 +31,7 @@ class Chef
     include Chef::Mixin::FromFile
     include Chef::Mixin::ParamsValidate
 
-    VALID_NAME = /^[\-[:alnum:]_]+$/
+    VALID_NAME = /^[\.\-[:alnum:]_]+$/
 
     def self.validate_name!(name)
       unless name =~ VALID_NAME
diff --git a/lib/chef/data_bag_item.rb b/lib/chef/data_bag_item.rb
index 3528ba724a..07dd15a1dc 100644
--- a/lib/chef/data_bag_item.rb
+++ b/lib/chef/data_bag_item.rb
@@ -35,7 +35,7 @@ class Chef
     include Chef::Mixin::FromFile
     include Chef::Mixin::ParamsValidate
 
-    VALID_ID = /^[\-[:alnum:]_]+$/
+    VALID_ID = /^[\.\-[:alnum:]_]+$/
 
     def self.validate_id!(id_str)
       if id_str.nil? || ( id_str !~ VALID_ID )
diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb
index 689c6ab99a..a0b49f4ab5 100644
--- a/lib/chef/formatters/doc.rb
+++ b/lib/chef/formatters/doc.rb
@@ -230,6 +230,21 @@ class Chef
         @output.color("\n    * Whyrun not supported for #{resource}, bypassing load.", :yellow)
       end
 
+      # Called before handlers run
+      def handlers_start(handler_count)
+        puts "\nRunning handlers:"
+      end
+
+      # Called after an individual handler has run
+      def handler_executed(handler)
+        puts "  - #{handler.class.name}"
+      end
+
+      # Called after all handlers have executed
+      def handlers_completed
+        puts "Running handlers complete\n"
+      end
+
       # Called when a provider makes an assumption after a failed assertion
       # in whyrun mode, in order to allow execution to continue
       def whyrun_assumption(action, resource, message)
diff --git a/lib/chef/knife/cookbook_create.rb b/lib/chef/knife/cookbook_create.rb
index 01bd8293f3..f4183a7245 100644
--- a/lib/chef/knife/cookbook_create.rb
+++ b/lib/chef/knife/cookbook_create.rb
@@ -290,7 +290,7 @@ e.g.
 
 Attributes
 ----------
-TODO: List you cookbook attributes here.
+TODO: List your cookbook attributes here.
 
 e.g.
 #### #{cookbook_name}::default
@@ -358,7 +358,7 @@ Requirements
   toaster         #{cookbook_name} needs toaster to brown your bagel.
 
 Attributes
-  TODO: List you cookbook attributes here.
+  TODO: List your cookbook attributes here.
 
   #{cookbook_name}
   Key                                   Type        Description                           Default
diff --git a/lib/chef/knife/ssh.rb b/lib/chef/knife/ssh.rb
index e1090fcca0..837710b932 100644
--- a/lib/chef/knife/ssh.rb
+++ b/lib/chef/knife/ssh.rb
@@ -65,9 +65,13 @@ class Chef
         :description => "The ssh username"
 
       option :ssh_password,
-        :short => "-P PASSWORD",
-        :long => "--ssh-password PASSWORD",
-        :description => "The ssh password"
+        :short => "-P [PASSWORD]",
+        :long => "--ssh-password [PASSWORD]",
+        :description => "The ssh password - will prompt if flag is specified but no password is given",
+        # default to a value that can not be a password (boolean) 
+        # so we can effectively test if this parameter was specified 
+        # without a vlaue
+        :default => false
 
       option :ssh_port,
         :short => "-p PORT",
@@ -432,6 +436,20 @@ class Chef
                              Chef::Config[:knife][:ssh_user])
       end
 
+      def configure_password
+        if config[:ssh_password].nil?
+          # If the parameter is called on the command line with no value
+          # it will set :ssh_password = nil
+          # This is where we want to trigger a prompt for password
+          config[:ssh_password] = get_password
+        else
+          # Otherwise, the password has either been specified on the command line,
+          # in knife.rb, or key based auth will be attempted
+          config[:ssh_password] = get_stripped_unfrozen_value(config[:ssh_password] ||
+                             Chef::Config[:knife][:ssh_password])
+        end
+      end
+
       def configure_identity_file
         config[:identity_file] = get_stripped_unfrozen_value(config[:identity_file] ||
                              Chef::Config[:knife][:ssh_identity_file])
@@ -448,6 +466,7 @@ class Chef
 
         configure_attribute
         configure_user
+        configure_password
         configure_identity_file
         configure_gateway
         configure_session
diff --git a/lib/chef/mixin/path_sanity.rb b/lib/chef/mixin/path_sanity.rb
index 1d324f54e9..ed857ffd36 100644
--- a/lib/chef/mixin/path_sanity.rb
+++ b/lib/chef/mixin/path_sanity.rb
@@ -22,6 +22,7 @@ class Chef
 
       def enforce_path_sanity(env=ENV)
         if Chef::Config[:enforce_path_sanity]
+          env["PATH"] = "" if env["PATH"].nil?
           path_separator = Chef::Platform.windows? ? ';' : ':'
           existing_paths = env["PATH"].split(path_separator)
           # ensure the Ruby and Gem bindirs are included
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index c7692a9746..adae880b7f 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -202,6 +202,7 @@ class Chef
         if template_available?(@new_resource.response_file)
           Chef::Log.debug("#{@new_resource} fetching preseed file via Template")
           remote_file = Chef::Resource::Template.new(cache_seed_to, run_context)
+          remote_file.variables(@new_resource.response_file_variables)
         elsif cookbook_file_available?(@new_resource.response_file)
           Chef::Log.debug("#{@new_resource} fetching preseed file via cookbook_file")
           remote_file = Chef::Resource::CookbookFile.new(cache_seed_to, run_context)
diff --git a/lib/chef/provider/package/aix.rb b/lib/chef/provider/package/aix.rb
index 4df0ea7a33..9fb87d6ea0 100644
--- a/lib/chef/provider/package/aix.rb
+++ b/lib/chef/provider/package/aix.rb
@@ -88,7 +88,7 @@ class Chef
           status = popen4("installp -L -d #{@new_resource.source}") do |pid, stdin, stdout, stderr|
             stdout.each_line do |line|
               case line
-              when /\w:{Regexp.escape(@new_resource.package_name)}:(.*)/
+              when /\w:#{Regexp.escape(@new_resource.package_name)}:(.*)/
                 fields = line.split(":")
                 @candidate_version = fields[2]
                 @new_resource.version(fields[2])
diff --git a/lib/chef/provider/service/debian.rb b/lib/chef/provider/service/debian.rb
index d788d58c00..2c3dda55c4 100644
--- a/lib/chef/provider/service/debian.rb
+++ b/lib/chef/provider/service/debian.rb
@@ -103,7 +103,7 @@ class Chef
           priority.each { |runlevel, arguments|
             Chef::Log.debug("#{@new_resource} runlevel #{runlevel}, action #{arguments[0]}, priority #{arguments[1]}")
             # if we are in a update-rc.d default startup runlevel && we start in this runlevel
-            if (2..5).include?(runlevel.to_i) && arguments[0] == :start
+            if %w[ 1 2 3 4 5 S ].include?(runlevel) && arguments[0] == :start
               enabled = true
             end
           }
@@ -113,7 +113,12 @@ class Chef
 
         # Override method from parent to ensure priority is up-to-date
         def action_enable
-          if @current_resource.enabled && @current_resource.priority == @new_resource.priority
+          if @new_resource.priority.nil?
+            priority_ok = true
+          else
+            priority_ok = @current_resource.priority == @new_resource.priority
+          end
+          if @current_resource.enabled and priority_ok
             Chef::Log.debug("#{@new_resource} already enabled - nothing to do")
           else
             converge_by("enable service #{@new_resource}") do
diff --git a/lib/chef/resource/package.rb b/lib/chef/resource/package.rb
index eaad3e2e58..f9fdd1ab59 100644
--- a/lib/chef/resource/package.rb
+++ b/lib/chef/resource/package.rb
@@ -36,6 +36,7 @@ class Chef
         @package_name = name
         @resource_name = :package
         @response_file = nil
+        @response_file_variables = Hash.new
         @source = nil
         @version = nil
       end
@@ -64,6 +65,14 @@ class Chef
         )
       end
 
+      def response_file_variables(arg=nil)
+        set_or_return(
+          :response_file_variables,
+          arg,
+          :kind_of => [ Hash ]
+        )
+      end
+
       def source(arg=nil)
         set_or_return(
           :source,
diff --git a/lib/chef/resource/service.rb b/lib/chef/resource/service.rb
index befa4be1c9..c3d009bc55 100644
--- a/lib/chef/resource/service.rb
+++ b/lib/chef/resource/service.rb
@@ -43,7 +43,6 @@ class Chef
         @init_command = nil
         @priority = nil
         @action = "nothing"
-        @startup_type = :automatic
         @supports = { :restart => false, :reload => false, :status => false }
         @allowed_actions.push(:enable, :disable, :start, :stop, :restart, :reload)
       end
diff --git a/spec/data/cookbooks/preseed/templates/default/preseed-template-variables.seed b/spec/data/cookbooks/preseed/templates/default/preseed-template-variables.seed
new file mode 100644
index 0000000000..0df0015f05
--- /dev/null
+++ b/spec/data/cookbooks/preseed/templates/default/preseed-template-variables.seed
@@ -0,0 +1 @@
+chef-integration-test chef-integration-test/sample-var string "<%= @template_variable -%>"
diff --git a/spec/functional/resource/package_spec.rb b/spec/functional/resource/package_spec.rb
index 55011271b5..797f308cf9 100644
--- a/spec/functional/resource/package_spec.rb
+++ b/spec/functional/resource/package_spec.rb
@@ -273,6 +273,23 @@ describe Chef::Resource::Package, metadata do
             package_resource.should be_updated_by_last_action
           end
 
+          context "with variables" do
+            let(:package_resource) do
+              r = base_resource
+              r.cookbook_name = "preseed"
+              r.response_file("preseed-template-variables.seed")
+              r.response_file_variables({ :template_variable => 'SUPPORTS VARIABLES' })
+              r
+            end
+
+            it "preseeds the package, then installs it" do
+              package_resource.run_action(:install)
+              cmd = shell_out!("debconf-show chef-integration-test")
+              cmd.stdout.should include('chef-integration-test/sample-var: "SUPPORTS VARIABLES"')
+              package_resource.should be_updated_by_last_action
+            end
+          end
+
         end
       end # installing w/ preseed
     end # when package not installed
diff --git a/spec/unit/data_bag_item_spec.rb b/spec/unit/data_bag_item_spec.rb
index 6bc5954fa5..4f017ebb60 100644
--- a/spec/unit/data_bag_item_spec.rb
+++ b/spec/unit/data_bag_item_spec.rb
@@ -70,6 +70,14 @@ describe Chef::DataBagItem do
       lambda { @data_bag_item.raw_data = { "id" => "h1-_" } }.should_not raise_error(ArgumentError)
     end
 
+    it "should accept alphanum.alphanum for the id" do
+      lambda { @data_bag_item.raw_data = { "id" => "foo.bar" } }.should_not raise_error(ArgumentError)
+    end
+
+    it "should accept .alphanum for the id" do
+      lambda { @data_bag_item.raw_data = { "id" => ".bozo" } }.should_not raise_error(ArgumentError)
+    end
+
     it "should raise an exception if the id contains anything but alphanum/-/_" do
       lambda { @data_bag_item.raw_data = { "id" => "!@#" } }.should raise_error(ArgumentError)
     end
diff --git a/spec/unit/data_bag_spec.rb b/spec/unit/data_bag_spec.rb
index 9fbdc64d98..8c9465caf7 100644
--- a/spec/unit/data_bag_spec.rb
+++ b/spec/unit/data_bag_spec.rb
@@ -47,6 +47,12 @@ describe Chef::DataBag do
     it "should throw an ArgumentError if you feed it anything but a string" do
       lambda { @data_bag.name Hash.new }.should raise_error(ArgumentError)
     end
+
+    [ ".", "-", "_", "1"].each do |char|
+      it "should allow a '#{char}' character in the data bag name" do
+        @data_bag.name("clown#{char}clown").should == "clown#{char}clown"
+      end
+    end
   end
 
   describe "deserialize" do
diff --git a/spec/unit/mixin/path_sanity_spec.rb b/spec/unit/mixin/path_sanity_spec.rb
index e38ee7dc8a..bea2e78ecd 100644
--- a/spec/unit/mixin/path_sanity_spec.rb
+++ b/spec/unit/mixin/path_sanity_spec.rb
@@ -38,6 +38,12 @@ describe Chef::Mixin::PathSanity do
       Chef::Platform.stub!(:windows?).and_return(false)
     end
 
+    it "adds all useful PATHs even if environment is an empty hash" do
+      env={}
+      @sanity.enforce_path_sanity(env)
+      env["PATH"].should == "#{@ruby_bindir}:#{@gem_bindir}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+    end
+
     it "adds all useful PATHs that are not yet in PATH to PATH" do
       env = {"PATH" => ""}
       @sanity.enforce_path_sanity(env)
diff --git a/spec/unit/provider/service/debian_service_spec.rb b/spec/unit/provider/service/debian_service_spec.rb
index ff0f4f421b..7d05b403de 100644
--- a/spec/unit/provider/service/debian_service_spec.rb
+++ b/spec/unit/provider/service/debian_service_spec.rb
@@ -126,11 +126,19 @@ describe Chef::Provider::Service::Debian do
      /etc/rc5.d/S20chef
      /etc/rc6.d/K20chef
           STDOUT
-          "stderr" => ""
+          "stderr" => "",
+          "priorities" => {
+            "0"=>[:stop, "20"],
+            "1"=>[:stop, "20"],
+            "2"=>[:start, "20"],
+            "3"=>[:start, "20"],
+            "4"=>[:start, "20"],
+            "5"=>[:start, "20"],
+            "6"=>[:stop, "20"]
+          }
         },
         "not linked" => {
-          "stdout" =>
-            " Removing any system startup links for /etc/init.d/chef ...",
+          "stdout" => " Removing any system startup links for /etc/init.d/chef ...",
           "stderr" => ""
         },
       },
@@ -147,20 +155,51 @@ insserv: remove service /etc/init.d/../rc0.d/K20chef-client
   insserv: remove service /etc/init.d/../rc6.d/K20chef-client
   insserv: dryrun, not creating .depend.boot, .depend.start, and .depend.stop
           STDERR
+          "priorities" => {
+            "0"=>[:stop, "20"],
+            "1"=>[:stop, "20"],
+            "2"=>[:start, "20"],
+            "3"=>[:start, "20"],
+            "4"=>[:start, "20"],
+            "5"=>[:start, "20"],
+            "6"=>[:stop, "20"]
+          }
         },
         "not linked" => {
           "stdout" => "update-rc.d: using dependency based boot sequencing",
           "stderr" => ""
         }
+      },
+      "Debian/Wheezy and earlier, a service only starting at run level S" => {
+        "linked" => {
+          "stdout" => "",
+          "stderr" => <<-STDERR,
+insserv: remove service /etc/init.d/../rc0.d/K06rpcbind
+insserv: remove service /etc/init.d/../rc1.d/K06rpcbind
+insserv: remove service /etc/init.d/../rc6.d/K06rpcbind
+insserv: remove service /etc/init.d/../rcS.d/S13rpcbind
+insserv: dryrun, not creating .depend.boot, .depend.start, and .depend.stop
+          STDERR
+          "priorities" => {
+            "0"=>[:stop, "06"],
+            "1"=>[:stop, "06"],
+            "6"=>[:stop, "06"],
+            "S"=>[:start, "13"]
+          }
+        },
+        "not linked" => {
+          "stdout" => "",
+          "stderr" => "insserv: dryrun, not creating .depend.boot, .depend.start, and .depend.stop"
+        }
       }
-    }.each do |model, streams|
+    }.each do |model, expected_results|
       context "on #{model}" do
         context "when update-rc.d shows init linked to rc*.d/" do
           before do
             @provider.stub!(:assert_update_rcd_available)
 
-            @stdout = StringIO.new(streams["linked"]["stdout"])
-            @stderr = StringIO.new(streams["linked"]["stderr"])
+            @stdout = StringIO.new(expected_results["linked"]["stdout"])
+            @stderr = StringIO.new(expected_results["linked"]["stderr"])
             @status = mock("Status", :exitstatus => 0, :stdout => @stdout)
             @provider.stub!(:shell_out!).and_return(@status)
             @provider.stub!(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
@@ -178,22 +217,15 @@ insserv: remove service /etc/init.d/../rc0.d/K20chef-client
 
           it "stores the start/stop priorities of the service" do
             @provider.load_current_resource
-            expected_priorities = {"6"=>[:stop, "20"],
-              "0"=>[:stop, "20"],
-              "1"=>[:stop, "20"],
-              "2"=>[:start, "20"],
-              "3"=>[:start, "20"],
-              "4"=>[:start, "20"],
-              "5"=>[:start, "20"]}
-            @provider.current_resource.priority.should == expected_priorities
+            @provider.current_resource.priority.should == expected_results["linked"]["priorities"]
           end
         end
 
         context "when update-rc.d shows init isn't linked to rc*.d/" do
           before do
             @provider.stub!(:assert_update_rcd_available)
-            @stdout = StringIO.new(streams["not linked"]["stdout"])
-            @stderr = StringIO.new(streams["not linked"]["stderr"])
+            @stdout = StringIO.new(expected_results["not linked"]["stdout"])
+            @stderr = StringIO.new(expected_results["not linked"]["stderr"])
             @status = mock("Status", :exitstatus => 0, :stdout => @stdout)
             @provider.stub!(:shell_out!).and_return(@status)
             @provider.stub!(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
@@ -244,6 +276,7 @@ insserv: remove service /etc/init.d/../rc0.d/K20chef-client
     context "when the service is enabled" do
       before do
         @current_resource.enabled(true)
+	@current_resource.priority(80)
       end
 
       context "and the service sets no priority" do
@@ -252,7 +285,6 @@ insserv: remove service /etc/init.d/../rc0.d/K20chef-client
 
       context "and the service requests the same priority as is set" do
         before do
-          @current_resource.priority(80)
           @new_resource.priority(80)
         end
         it_behaves_like "the service is up to date"
@@ -260,8 +292,7 @@ insserv: remove service /etc/init.d/../rc0.d/K20chef-client
 
       context "and the service requests a different priority than is set" do
         before do
-          @current_resource.priority(20)
-          @new_resource.priority(80)
+          @new_resource.priority(20)
         end
         it_behaves_like "the service is not up to date"
       end
diff --git a/spec/unit/resource/package_spec.rb b/spec/unit/resource/package_spec.rb
index 0f175dda12..2884c94393 100644
--- a/spec/unit/resource/package_spec.rb
+++ b/spec/unit/resource/package_spec.rb
@@ -49,6 +49,11 @@ describe Chef::Resource::Package do
     @resource.response_file.should eql("something")
   end
 
+  it "should accept a hash for response file template variables" do
+    @resource.response_file_variables({:variables => true})
+    @resource.response_file_variables.should eql({:variables => true})
+  end
+
   it "should accept a string for the source" do
     @resource.source "something"
     @resource.source.should eql("something")