diff options
| -rw-r--r-- | CHANGELOG | 3 | ||||
| -rw-r--r-- | lib/puppet/provider/nameservice/directoryservice.rb | 168 | ||||
| -rw-r--r-- | lib/puppet/provider/package/up2date.rb | 2 | ||||
| -rwxr-xr-x | lib/puppet/provider/parsedfile.rb | 2 | ||||
| -rw-r--r-- | lib/puppet/provider/service/launchd.rb | 17 | ||||
| -rw-r--r-- | lib/puppet/rails/resource.rb | 7 | ||||
| -rwxr-xr-x | lib/puppet/type/user.rb | 2 | ||||
| -rwxr-xr-x | lib/puppet/util/filetype.rb | 10 | ||||
| -rwxr-xr-x | spec/unit/network/xmlrpc/client.rb | 4 | ||||
| -rwxr-xr-x | spec/unit/provider/parsedfile.rb | 15 | ||||
| -rwxr-xr-x | spec/unit/provider/service/launchd.rb | 10 | ||||
| -rwxr-xr-x | spec/unit/type/user.rb | 12 | ||||
| -rwxr-xr-x | test/language/resource.rb | 40 |
13 files changed, 186 insertions, 106 deletions
@@ -14,6 +14,9 @@ Fixed #1840 - Bug fixes and improvements for Emacs puppet-mode.el 0.24.8 + Fixed #1956 - Cleaned up variable names to be more sane, clarified error messages + and fixed incorrect use of 'value' variable rather than 'member'. + Fixed #1831 - Added sprintf function Fixed #1830 - Added regsubst function diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 4e21d4169..42c52f0a7 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -15,6 +15,7 @@ require 'puppet' require 'puppet/provider/nameservice' require 'facter/util/plist' +require 'cgi' class Puppet::Provider::NameService @@ -26,6 +27,7 @@ class DirectoryService < Puppet::Provider::NameService # e.g. Puppet::Type.type(:user).provide :directoryservice, :ds_path => "Users" # This is referenced in the get_ds_path class method attr_writer :ds_path + attr_writer :macosx_version_major end @@ -40,6 +42,7 @@ class DirectoryService < Puppet::Provider::NameService commands :dscl => "/usr/bin/dscl" commands :dseditgroup => "/usr/sbin/dseditgroup" + commands :sw_vers => "/usr/bin/sw_vers" confine :operatingsystem => :darwin defaultfor :operatingsystem => :darwin @@ -100,12 +103,31 @@ class DirectoryService < Puppet::Provider::NameService # @ds_path is an attribute of the class itself. if defined? @ds_path return @ds_path - else - # JJM: "Users" or "Groups" etc ... (Based on the Puppet::Type) - # Remember this is a class method, so self.class is Class - # Also, @resource_type seems to be the reference to the - # Puppet::Type this class object is providing for. - return @resource_type.name.to_s.capitalize + "s" + end + # JJM: "Users" or "Groups" etc ... (Based on the Puppet::Type) + # Remember this is a class method, so self.class is Class + # Also, @resource_type seems to be the reference to the + # Puppet::Type this class object is providing for. + return @resource_type.name.to_s.capitalize + "s" + end + + def self.get_macosx_version_major + if defined? @macosx_version_major + return @macosx_version_major + end + begin + product_version = Facter.value(:macosx_productversion) + if product_version.nil? + raise Puppet::Error, "Could not determine OS X version: %s" % detail + end + product_version_major = product_version.scan(/(\d+)\.(\d+)./).join(".") + if %w{10.0 10.1 10.2 10.3}.include?(product_version_major) + raise Puppet::Error, "%s is not supported by the directoryservice provider" % product_version_major + end + @macosx_version_major = product_version_major + return @macosx_version_major + rescue Puppet::ExecutionFailure => detail + raise Puppet::Error, "Could not determine OS X version: %s" % detail end end @@ -119,34 +141,56 @@ class DirectoryService < Puppet::Provider::NameService return dscl_output.split("\n") end - def self.single_report(resource_name, *type_properties) - # JJM 2007-07-24: - # Given a the name of an object and a list of properties of that - # object, return all property values in a hash. - # - # This class method returns nil if the object doesn't exist - # Otherwise, it returns a hash of the object properties. - - all_present_str_array = list_all_present() - - # NBK: shortcut the process if the resource is missing - return nil unless all_present_str_array.include? resource_name + def self.parse_dscl_url_data(dscl_output) + # we need to construct a Hash from the dscl -url output to match + # that returned by the dscl -plist output for 10.5+ clients. + # + # Nasty assumptions: + # a) no values *end* in a colon ':', only keys + # b) if a line ends in a colon and the next line does start with + # a space, then the second line is a value of the first. + # c) (implied by (b)) keys don't start with spaces. - dscl_vector = get_exec_preamble("-read", resource_name) - begin - dscl_output = execute(dscl_vector) - rescue Puppet::ExecutionFailure => detail - raise Puppet::Error, "Could not get report. command execution failed." + dscl_plist = {} + dscl_output.split("\n").inject([]) do |array, line| + if line =~ /^\s+/ # it's a value + array[-1] << line # add the value to the previous key + else + array << line + end + array + end.compact + + dscl_output.each do |line| + # This should be a 'normal' entry. key and value on one line. + # We split on ': ' to deal with keys/values with a colon in them. + split_array = line.split(/:\s+/) + key = split_array.first + value = CGI::unescape(split_array.last.strip.chomp) + # We need to treat GroupMembership separately as it is currently + # the only attribute we care about multiple values for, and + # the values can never contain spaces (shortnames) + # We also make every value an array to be consistent with the + # output of dscl -plist under 10.5 + if key == "GroupMembership" + dscl_plist[key] = value.split(/\s/) + else + dscl_plist[key] = [value] + end end - - # JJM: We need a new hash to return back to our caller. - attribute_hash = Hash.new - - dscl_plist = Plist.parse_xml(dscl_output) - dscl_plist.keys().each do |key| + return dscl_plist + end + + def self.parse_dscl_plist_data(dscl_output) + return Plist.parse_xml(dscl_output) + end + + def self.generate_attribute_hash(input_hash, *type_properties) + attribute_hash = {} + input_hash.keys().each do |key| ds_attribute = key.sub("dsAttrTypeStandard:", "") next unless (@@ds_to_ns_attribute_map.keys.include?(ds_attribute) and type_properties.include? @@ds_to_ns_attribute_map[ds_attribute]) - ds_value = dscl_plist[key] + ds_value = input_hash[key] case @@ds_to_ns_attribute_map[ds_attribute] when :members: ds_value = ds_value # only members uses arrays so far @@ -172,7 +216,41 @@ class DirectoryService < Puppet::Provider::NameService if @resource_type.validproperties.include?(:password) attribute_hash[:password] = self.get_password(attribute_hash[:guid]) end - return attribute_hash + return attribute_hash + end + + def self.single_report(resource_name, *type_properties) + # JJM 2007-07-24: + # Given a the name of an object and a list of properties of that + # object, return all property values in a hash. + # + # This class method returns nil if the object doesn't exist + # Otherwise, it returns a hash of the object properties. + + all_present_str_array = list_all_present() + + # NBK: shortcut the process if the resource is missing + return nil unless all_present_str_array.include? resource_name + + dscl_vector = get_exec_preamble("-read", resource_name) + begin + dscl_output = execute(dscl_vector) + rescue Puppet::ExecutionFailure => detail + raise Puppet::Error, "Could not get report. command execution failed." + end + + # Two code paths is ugly, but until we can drop 10.4 support we don't + # have a lot of choice. Ultimately this should all be done using Ruby + # to access the DirectoryService APIs directly, but that's simply not + # feasible for a while yet. + case self.get_macosx_version_major + when "10.4" + dscl_plist = self.parse_dscl_url_data(dscl_output) + when "10.5", "10.6" + dscl_plist = self.parse_dscl_plist_data(dscl_output) + end + + return self.generate_attribute_hash(dscl_plist, *type_properties) end def self.get_exec_preamble(ds_action, resource_name = nil) @@ -183,8 +261,16 @@ class DirectoryService < Puppet::Provider::NameService # This method spits out proper DSCL commands for us. # We EXPECT name to be @resource[:name] when called from an instance object. - # There are two ways to specify paths in 10.5. See man dscl. - command_vector = [ command(:dscl), "-plist", "." ] + # 10.4 doesn't support the -plist option for dscl, and 10.5 has a + # different format for the -url output with objects with spaces in + # their values. *sigh*. Use -url for 10.4 in the hope this can be + # deprecated one day, and use -plist for 10.5 and higher. + case self.get_macosx_version_major + when "10.4" + command_vector = [ command(:dscl), "-url", "." ] + when "10.5", "10.6" + command_vector = [ command(:dscl), "-plist", "." ] + end # JJM: The actual action to perform. See "man dscl" # Common actiosn: -create, -delete, -merge, -append, -passwd command_vector << ds_action @@ -234,7 +320,6 @@ class DirectoryService < Puppet::Provider::NameService def self.get_password(guid) password_hash = nil password_hash_file = "#{@@password_hash_dir}/#{guid}" - # TODO: sort out error conditions? if File.exists?(password_hash_file) if not File.readable?(password_hash_file) raise Puppet::Error("Could not read password hash file at #{password_hash_file} for #{@resource[:name]}") @@ -322,9 +407,8 @@ class DirectoryService < Puppet::Provider::NameService # to create objects with dscl, rather than the single command nameservice.rb # expects to be returned by addcmd. Thus we don't bother defining addcmd. def create - if exists? + if exists? info "already exists" - # The object already exists return nil end @@ -373,25 +457,25 @@ class DirectoryService < Puppet::Provider::NameService def remove_unwanted_members(current_members, new_members) current_members.each do |member| - if not value.include?(member) + if not new_members.include?(member) cmd = [:dseditgroup, "-o", "edit", "-n", ".", "-d", member, @resource[:name]] begin execute(cmd) rescue Puppet::ExecutionFailure => detail - raise Puppet::Error, "Could not set %s on %s[%s]: %s" % [param, @resource.class.name, @resource.name, detail] + raise Puppet::Error, "Could not remove %s from group: %s, %s" % [member, @resource.name, detail] end end end end def add_members(current_members, new_members) - new_members.each do |user| - if current_members.nil? or not current_members.include?(user) - cmd = [:dseditgroup, "-o", "edit", "-n", ".", "-a", user, @resource[:name]] + new_members.each do |new_member| + if current_members.nil? or not current_members.include?(new_member) + cmd = [:dseditgroup, "-o", "edit", "-n", ".", "-a", new_member, @resource[:name]] begin execute(cmd) rescue Puppet::ExecutionFailure => detail - raise Puppet::Error, "Could not set %s on %s[%s]: %s" % [param, @resource.class.name, @resource.name, detail] + raise Puppet::Error, "Could not add %s to group: %s, %s" % [new_member, @resource.name, detail] end end end diff --git a/lib/puppet/provider/package/up2date.rb b/lib/puppet/provider/package/up2date.rb index 5708905cc..d8a12652f 100644 --- a/lib/puppet/provider/package/up2date.rb +++ b/lib/puppet/provider/package/up2date.rb @@ -4,7 +4,7 @@ Puppet::Type.type(:package).provide :up2date, :parent => :rpm, :source => :rpm d commands :up2date => "/usr/sbin/up2date-nox" - defaultfor :operatingsystem => [:redhat, :oel, :ovm] + defaultfor :operatingsystem => [:redhat, :oel, :ovm], :lsbdistrelease => ["2.1", "3", "4"] confine :operatingsystem => [:redhat, :oel, :ovm] diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb index 45eae57ff..40e172785 100755 --- a/lib/puppet/provider/parsedfile.rb +++ b/lib/puppet/provider/parsedfile.rb @@ -81,6 +81,8 @@ class Puppet::Provider::ParsedFile < Puppet::Provider # Make sure our file is backed up, but only back it up once per transaction. # We cheat and rely on the fact that @records is created on each prefetch. def self.backup_target(target) + return nil unless target_object(target).respond_to?(:backup) + unless defined?(@backup_stats) @backup_stats = {} end diff --git a/lib/puppet/provider/service/launchd.rb b/lib/puppet/provider/service/launchd.rb index 891c96bd8..64b61ff85 100644 --- a/lib/puppet/provider/service/launchd.rb +++ b/lib/puppet/provider/service/launchd.rb @@ -103,12 +103,21 @@ Puppet::Type.type(:service).provide :launchd, :parent => :base do def status # launchctl list <jobname> exits zero if the job is loaded - # and non-zero if it isn't. Simple way to check... + # and non-zero if it isn't. Simple way to check... but is only + # available on OS X 10.5 unfortunately, so we grab the whole list + # and check if our resource is included. The output formats differ + # between 10.4 and 10.5, thus the necessity for splitting begin - launchctl :list, resource[:name] - return :running - rescue Puppet::ExecutionFailure + output = launchctl :list + if output.nil? + raise Puppet::Error.new("launchctl list failed to return any data.") + end + output.split("\n").each do |j| + return :running if j.split(/\s/).last == resource[:name] + end return :stopped + rescue Puppet::ExecutionFailure + raise Puppet::Error.new("Unable to determine status of #{resource[:name]}") end end diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb index bd2739d53..c3d287af2 100644 --- a/lib/puppet/rails/resource.rb +++ b/lib/puppet/rails/resource.rb @@ -1,5 +1,6 @@ require 'puppet' require 'puppet/rails/param_name' +require 'puppet/rails/param_value' require 'puppet/rails/puppet_tag' require 'puppet/util/rails/collection_merger' @@ -57,7 +58,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base # returns a hash of param_names.name => [param_values] def get_params_hash(values = nil) - values ||= @params_hash || Puppet::Rails::ParamValues.find_all_params_from_resource(id) + values ||= @params_hash || Puppet::Rails::ParamValue.find_all_params_from_resource(self) if values.size == 0 return {} end @@ -69,7 +70,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base end def get_tag_hash(tags = nil) - tags ||= @tags_hash || Puppet::Rails::ResourceTag.find_all_tags_from_resource(id) + tags ||= @tags_hash || Puppet::Rails::ResourceTag.find_all_tags_from_resource(self) return tags.inject({}) do |hash, tag| # We have to store the tag object, not just the tag name. hash[tag['name']] = tag @@ -99,7 +100,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base result = get_params_hash result.each do |param, value| if value.is_a?(Array) - result[param] = value.collect { |v| v.value } + result[param] = value.collect { |v| v['value'] } else result[param] = value.value end diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 074afb6b2..8efc2ef1c 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -145,7 +145,7 @@ module Puppet desc "The user's password, in whatever encrypted format the local machine requires. Be sure to enclose any value that includes a dollar sign ($) in single quotes (\')." validate do |value| - raise ArgumentError, "Passwords cannot include ':'" if value.include?(":") + raise ArgumentError, "Passwords cannot include ':'" if value.is_a?(String) and value.include?(":") end def change_to_s(currentvalue, newvalue) diff --git a/lib/puppet/util/filetype.rb b/lib/puppet/util/filetype.rb index 0f184d5c2..34feeab90 100755 --- a/lib/puppet/util/filetype.rb +++ b/lib/puppet/util/filetype.rb @@ -72,11 +72,6 @@ class Puppet::Util::FileType @filetypes[type] end - # Back the file up before replacing it. - def backup - bucket.backup(@path) if File.exists?(@path) - end - # Pick or create a filebucket to use. def bucket unless defined?(@bucket) @@ -92,6 +87,11 @@ class Puppet::Util::FileType # Operate on plain files. newfiletype(:flat) do + # Back the file up before replacing it. + def backup + bucket.backup(@path) if File.exists?(@path) + end + # Read the file. def read if File.exist?(@path) diff --git a/spec/unit/network/xmlrpc/client.rb b/spec/unit/network/xmlrpc/client.rb index 36e59429c..328688bae 100755 --- a/spec/unit/network/xmlrpc/client.rb +++ b/spec/unit/network/xmlrpc/client.rb @@ -109,7 +109,7 @@ describe Puppet::Network::XMLRPCClient do it "should log, recycle the connection, and retry if Errno::EPIPE is raised" do @client.expects(:call).times(2).raises(Errno::EPIPE).then.returns "eh" - Puppet.expects(:warning) + Puppet.expects(:info) @client.expects(:recycle_connection) @client.report("eh") @@ -118,7 +118,7 @@ describe Puppet::Network::XMLRPCClient do it "should log, recycle the connection, and retry if EOFError is raised" do @client.expects(:call).times(2).raises(EOFError).then.returns "eh" - Puppet.expects(:warning) + Puppet.expects(:info) @client.expects(:recycle_connection) @client.report("eh") diff --git a/spec/unit/provider/parsedfile.rb b/spec/unit/provider/parsedfile.rb index 11a91c8d7..f20b6b235 100755 --- a/spec/unit/provider/parsedfile.rb +++ b/spec/unit/provider/parsedfile.rb @@ -56,18 +56,27 @@ describe Puppet::Provider::ParsedFile do @class.initvars @class.prefetch - @filetype = mock 'filetype' - Puppet::Util::FileType.filetype(:flat).expects(:new).with("/my/file").returns @filetype + @filetype = Puppet::Util::FileType.filetype(:flat).new("/my/file") + Puppet::Util::FileType.filetype(:flat).stubs(:new).with("/my/file").returns @filetype @filetype.stubs(:write) end - it "should back up the file being written" do + it "should back up the file being written if the filetype can be backed up" do @filetype.expects(:backup) @class.flush_target("/my/file") end + it "should not try to back up the file if the filetype cannot be backed up" do + @filetype = Puppet::Util::FileType.filetype(:ram).new("/my/file") + Puppet::Util::FileType.filetype(:flat).expects(:new).returns @filetype + + @filetype.stubs(:write) + + @class.flush_target("/my/file") + end + it "should not back up the file more than once between calls to 'prefetch'" do @filetype.expects(:backup).once diff --git a/spec/unit/provider/service/launchd.rb b/spec/unit/provider/service/launchd.rb index cc2dae190..f2d69a47b 100755 --- a/spec/unit/provider/service/launchd.rb +++ b/spec/unit/provider/service/launchd.rb @@ -62,9 +62,17 @@ describe provider_class do describe "when checking status" do it "should call the external command 'launchctl list' once" do - @provider.expects(:launchctl).with(:list, @resource[:name]).returns(:running).once + @provider.expects(:launchctl).with(:list).returns("rotating-strawberry-madonnas") @provider.status end + it "should return stopped if not listed in launchctl list output" do + @provider.stubs(:launchctl).with(:list).returns("rotating-strawberry-madonnas") + assert_equal @provider.status, :stopped + end + it "should return running if listed in launchctl list output" do + @provider.stubs(:launchctl).with(:list).returns(@joblabel) + assert_equal @provider.status, :running + end end describe "when starting the service" do diff --git a/spec/unit/type/user.rb b/spec/unit/type/user.rb index 2e8616ce8..5c46689d3 100755 --- a/spec/unit/type/user.rb +++ b/spec/unit/type/user.rb @@ -38,10 +38,6 @@ describe user do it "should have a valid provider" do user.new(:name => "foo").provider.class.ancestors.should be_include(Puppet::Provider) end - - it "should fail if a ':' is included in the password" do - lambda { user.create(:name => "foo", :password => 'some:thing') }.should raise_error(Puppet::Error) - end end properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, :groups, :roles, :auths, :profiles, :project, :keys] @@ -232,6 +228,14 @@ describe user do it "should not include the password in the change log when changing the password" do @password.change_to_s("other", "mypass").should_not be_include("mypass") end + + it "should fail if a ':' is included in the password" do + lambda { @password.should = "some:thing" }.should raise_error(ArgumentError) + end + + it "should allow the value to be set to :absent" do + lambda { @password.should = :absent }.should_not raise_error + end end describe "when manages_solaris_rbac is enabled" do diff --git a/test/language/resource.rb b/test/language/resource.rb index fa04117f1..c7ed1f019 100755 --- a/test/language/resource.rb +++ b/test/language/resource.rb @@ -170,44 +170,4 @@ class TestResource < PuppetTest::TestCase assert_nil(hash[:owner], "got a value for an undef parameter") end - - # #643 - Make sure virtual defines result in virtual resources - def test_virtual_defines - parser = mkparser - define = parser.newdefine("yayness", - :code => resourcedef("file", varref("name"), - "mode" => "644")) - - config = mkcompiler(parser) - - res = mkresource :type => "yayness", :title => "foo", :params => {}, :scope => config.topscope - res.virtual = true - - result = nil - assert_nothing_raised("Could not evaluate defined resource") do - result = res.evaluate - end - - scope = res.scope - newres = scope.findresource("File[foo]") - assert(newres, "Could not find resource") - - assert(newres.virtual?, "Virtual defined resource generated non-virtual resources") - - # Now try it with exported resources - res = mkresource :type => "yayness", :title => "bar", :params => {}, :scope => config.topscope - res.exported = true - - result = nil - assert_nothing_raised("Could not evaluate exported resource") do - result = res.evaluate - end - - scope = res.scope - newres = scope.findresource("File[bar]") - assert(newres, "Could not find resource") - - assert(newres.exported?, "Exported defined resource generated non-exported resources") - assert(newres.virtual?, "Exported defined resource generated non-virtual resources") - end end |