Issue #13686 has been updated by Jeff McCune. Status changed from In Topic Branch Pending Review to Merged - Pending Release Target version set to 2.7.13
# Merged into 2.7.x # <pre> commit 5ab3ec3d340189db827fac7586b0b78828112d97 (from 4ad37dc38dda264d67609795c55dea9644d53aaf) Merge: 4ad37dc a26ff16 Author: Jeff McCune <[email protected]> Date: Mon Apr 9 13:11:10 2012 -0700 Merge branch 'bug/2.7.x/13686_DS_Fix' into 2.7.x * bug/2.7.x/13686_DS_Fix: [#13686] Fix directoryservices ShadowHashData code diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 76c79f6..c4f385a 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -323,9 +323,15 @@ class DirectoryService < Puppet::Provider::NameService # users_plist['ShadowHashData'][0].string is actually a binary plist # that's nested INSIDE the user's plist (which itself is a binary - # plist). - password_hash_plist = users_plist['ShadowHashData'][0].string - converted_hash_plist = convert_binary_to_xml(password_hash_plist) + # plist). If we encounter a user plist that DOESN'T have a + # ShadowHashData field, create one. + if users_plist['ShadowHashData'] + password_hash_plist = users_plist['ShadowHashData'][0].string + converted_hash_plist = convert_binary_to_xml(password_hash_plist) + else + users_plist['ShadowHashData'] = [StringIO.new] + converted_hash_plist = {'SALTED-SHA512' => StringIO.new} + end # converted_hash_plist['SALTED-SHA512'].string expects a Base64 encoded # string. The password_hash provided as a resource attribute is a @@ -348,7 +354,7 @@ class DirectoryService < Puppet::Provider::NameService def self.get_password(guid, username) # Use Puppet::Util::Package.versioncmp() to catch the scenario where a # version '10.10' would be < '10.7' with simple string comparison. This - # if-statement only executes if the current version is less-than 10.7 + # if-statement only executes if the current version is less-than 10.7 if (Puppet::Util::Package.versioncmp(get_macosx_version_major, '10.7') == -1) password_hash = nil password_hash_file = "#{@@password_hash_dir}/#{guid}" diff --git a/spec/unit/provider/nameservice/directoryservice_spec.rb b/spec/unit/provider/nameservice/directoryservice_spec.rb index c585b62..c11388a 100755 --- a/spec/unit/provider/nameservice/directoryservice_spec.rb +++ b/spec/unit/provider/nameservice/directoryservice_spec.rb @@ -155,6 +155,16 @@ describe 'DirectoryService password behavior' do Plist::Emit.expects(:save_plist).with(shadow_hash_data, plist_path) subject.set_password('jeff', 'uid', sha512_hash) end + + it '[#13686] should handle an empty ShadowHashData field in the users plist' do + subject.expects(:convert_xml_to_binary).returns(binary_plist) + File.expects(:exists?).with(plist_path).once.returns(true) + Plist.expects(:parse_xml).returns({'ShadowHashData' => nil}) + subject.expects(:plutil).with('-convert', 'xml1', '-o', '/dev/stdout', plist_path) + subject.expects(:plutil).with('-convert', 'binary1', plist_path) + Plist::Emit.expects(:save_plist) + subject.set_password('jeff', 'uid', sha512_hash) + end end describe '(#4855) directoryservice group resource failure' do commit 5ab3ec3d340189db827fac7586b0b78828112d97 (from a26ff1648b48f9aaa7e4a312d5e8f9acbf4e767a) Merge: 4ad37dc a26ff16 Author: Jeff McCune <[email protected]> Date: Mon Apr 9 13:11:10 2012 -0700 Merge branch 'bug/2.7.x/13686_DS_Fix' into 2.7.x * bug/2.7.x/13686_DS_Fix: [#13686] Fix directoryservices ShadowHashData code diff --git a/lib/puppet/face/module/install.rb b/lib/puppet/face/module/install.rb index 39aab02..c3e0754 100644 --- a/lib/puppet/face/module/install.rb +++ b/lib/puppet/face/module/install.rb @@ -164,9 +164,9 @@ Puppet::Face.define(:module, '1.0.0') do Puppet.err(return_value[:error][:multiline]) exit 1 else - tree = Puppet::Module::Tool.format_tree(return_value[:installed_modules], return_value[:install_dir]) + tree = Puppet::Module::Tool.build_tree(return_value[:installed_modules], return_value[:install_dir]) return_value[:install_dir] + "\n" + - Puppet::Module::Tool.build_tree(tree) + Puppet::Module::Tool.format_tree(tree) end end end diff --git a/lib/puppet/face/module/list.rb b/lib/puppet/face/module/list.rb index 2126474..e3acbd9 100644 --- a/lib/puppet/face/module/list.rb +++ b/lib/puppet/face/module/list.rb @@ -171,17 +171,17 @@ Puppet::Face.define(:module, '1.0.0') do # since dependencies may be cyclical. modules_by_num_requires = modules.sort_by {|m| m.required_by.size} @seen = {} - tree = list_format_tree(modules_by_num_requires, [], nil, + tree = list_build_tree(modules_by_num_requires, [], nil, :label_unmet => true, :path => path, :label_invalid => false) else tree = [] modules.sort_by { |mod| mod.forge_name or mod.name }.each do |mod| - tree << list_format_node(mod, path, :label_unmet => false, + tree << list_build_node(mod, path, :label_unmet => false, :path => path, :label_invalid => true) end end - output << Puppet::Module::Tool.build_tree(tree) + output << Puppet::Module::Tool.format_tree(tree) end output @@ -224,10 +224,10 @@ Puppet::Face.define(:module, '1.0.0') do # │ └── bodepd-create_resources (v0.0.1) # └── puppetlabs-sqlite (v0.0.1) # - def list_format_tree(list, ancestors=[], parent=nil, params={}) + def list_build_tree(list, ancestors=[], parent=nil, params={}) list.map do |mod| next if @seen[(mod.forge_name or mod.name)] - node = list_format_node(mod, parent, params) + node = list_build_node(mod, parent, params) @seen[(mod.forge_name or mod.name)] = true unless ancestors.include?(mod) @@ -240,7 +240,7 @@ Puppet::Face.define(:module, '1.0.0') do str << "(#{colorize(:cyan, mis_mod[:version_constraint])})" node[:dependencies] << { :text => str } end - node[:dependencies] += list_format_tree(mod.dependencies_as_modules, + node[:dependencies] += list_build_tree(mod.dependencies_as_modules, ancestors + [mod], mod, params) end @@ -259,7 +259,7 @@ Puppet::Face.define(:module, '1.0.0') do # # Returns a Hash # - def list_format_node(mod, parent, params) + def list_build_node(mod, parent, params) str = '' str << (mod.forge_name ? mod.forge_name.gsub('/', '-') : mod.name) str << ' (' + colorize(:cyan, mod.version ? "v#{mod.version}" : '???') + ')' diff --git a/lib/puppet/face/module/upgrade.rb b/lib/puppet/face/module/upgrade.rb index eac79c1..e62a7c5 100644 --- a/lib/puppet/face/module/upgrade.rb +++ b/lib/puppet/face/module/upgrade.rb @@ -75,9 +75,9 @@ Puppet::Face.define(:module, '1.0.0') do Puppet.err(return_value[:error][:multiline]) exit 0 else - tree = Puppet::Module::Tool.format_tree(return_value[:affected_modules], return_value[:base_dir]) + tree = Puppet::Module::Tool.build_tree(return_value[:affected_modules], return_value[:base_dir]) return_value[:base_dir] + "\n" + - Puppet::Module::Tool.build_tree(tree) + Puppet::Module::Tool.format_tree(tree) end end end diff --git a/lib/puppet/module_tool.rb b/lib/puppet/module_tool.rb index 6ac1eb9..a05ac8e 100644 --- a/lib/puppet/module_tool.rb +++ b/lib/puppet/module_tool.rb @@ -50,7 +50,7 @@ module Puppet # Builds a formatted tree from a list of node hashes containing +:text+ # and +:dependencies+ keys. - def self.build_tree(nodes, level = 0) + def self.format_tree(nodes, level = 0) str = '' nodes.each_with_index do |node, i| last_node = nodes.length - 1 == i @@ -62,7 +62,7 @@ module Puppet str << (deps.empty? ? "─" : "┬") str << " #{node[:text]}\n" - branch = build_tree(deps, level + 1) + branch = format_tree(deps, level + 1) branch.gsub!(/^#{indent} /, indent + '│') unless last_node str << branch end @@ -70,7 +70,7 @@ module Puppet return str end - def self.format_tree(mods, dir) + def self.build_tree(mods, dir) mods.each do |mod| version_string = mod[:version][:vstring].sub(/^(?!v)/, 'v') @@ -81,7 +81,7 @@ module Puppet mod[:text] = "#{mod[:module]} (#{colorize(:cyan, version_string)})" mod[:text] += " [#{mod[:path]}]" unless mod[:path] == dir - format_tree(mod[:dependencies], dir) + build_tree(mod[:dependencies], dir) end end end diff --git a/lib/puppet/reports/tagmail.rb b/lib/puppet/reports/tagmail.rb index c37341e..2c2c475 100644 --- a/lib/puppet/reports/tagmail.rb +++ b/lib/puppet/reports/tagmail.rb @@ -113,6 +113,13 @@ Puppet::Reports.register_report(:tagmail) do return end + metrics = raw_summary['resources'] || {} rescue {} + + if metrics['out_of_sync'] == 0 && metrics['changed'] == 0 + Puppet.notice "Not sending tagmail report; no changes" + return + end + taglists = parse(File.read(Puppet[:tagmap])) # Now find any appropriately tagged messages. diff --git a/spec/fixtures/unit/reports/tagmail/tagmail_email.conf b/spec/fixtures/unit/reports/tagmail/tagmail_email.conf new file mode 100644 index 0000000..94efca4 --- /dev/null +++ b/spec/fixtures/unit/reports/tagmail/tagmail_email.conf @@ -0,0 +1,2 @@ +secure: [email protected] + diff --git a/spec/unit/module_tool_spec.rb b/spec/unit/module_tool_spec.rb index 5ddb01f..1517f30 100644 --- a/spec/unit/module_tool_spec.rb +++ b/spec/unit/module_tool_spec.rb @@ -4,14 +4,14 @@ require 'spec_helper' require 'puppet/module_tool' describe Puppet::Module::Tool, :fails_on_windows => true do - describe '.build_tree' do + describe '.format_tree' do it 'should return an empty tree when given an empty list' do - subject.build_tree([]).should == '' + subject.format_tree([]).should == '' end it 'should return a shallow when given a list without dependencies' do list = [ { :text => 'first' }, { :text => 'second' }, { :text => 'third' } ] - subject.build_tree(list).should == <<-TREE + subject.format_tree(list).should == <<-TREE ├── first ├── second └── third @@ -32,7 +32,7 @@ TREE ] }, ] - subject.build_tree(list).should == <<-TREE + subject.format_tree(list).should == <<-TREE └─┬ first └─┬ second └── third @@ -54,7 +54,7 @@ TREE }, { :text => 'fourth' } ] - subject.build_tree(list).should == <<-TREE + subject.format_tree(list).should == <<-TREE ├─┬ first │ └─┬ second │ └── third @@ -77,7 +77,7 @@ TREE ] } ] - subject.build_tree(list).should == <<-TREE + subject.format_tree(list).should == <<-TREE └─┬ first ├─┬ second │ └── third @@ -101,7 +101,7 @@ TREE }, { :text => 'fifth' } ] - subject.build_tree(list).should == <<-TREE + subject.format_tree(list).should == <<-TREE ├─┬ first │ ├─┬ second │ │ └── third diff --git a/spec/unit/reports/tagmail_spec.rb b/spec/unit/reports/tagmail_spec.rb index a53d119..00f78c9 100755 --- a/spec/unit/reports/tagmail_spec.rb +++ b/spec/unit/reports/tagmail_spec.rb @@ -88,4 +88,81 @@ describe tagmail do results.should be_nil end end + + describe "the behavior of tagmail.process" do + before do + Puppet[:tagmap] = my_fixture "tagmail_email.conf" + end + + let(:processor) do + processor = Puppet::Transaction::Report.new("apply") + processor.extend(Puppet::Reports.report(:tagmail)) + processor + end + + context "when any messages match a positive tag" do + before do + processor << log_entry + end + + let(:log_entry) do + Puppet::Util::Log.new( + :level => :notice, :message => "Secure change", :tags => %w{secure}) + end + + let(:message) do + "#{log_entry.time} Puppet (notice): Secure change" + end + + it "should send email if there are changes" do + processor.expects(:send).with([[['[email protected]'], message]]) + processor.expects(:raw_summary).returns({ + "resources" => { "changed" => 1, "out_of_sync" => 0 } + }) + + processor.process + end + + it "should send email if there are resources out of sync" do + processor.expects(:send).with([[['[email protected]'], message]]) + processor.expects(:raw_summary).returns({ + "resources" => { "changed" => 0, "out_of_sync" => 1 } + }) + + processor.process + end + + it "should not send email if no changes or resources out of sync" do + processor.expects(:send).never + processor.expects(:raw_summary).returns({ + "resources" => { "changed" => 0, "out_of_sync" => 0 } + }) + + processor.process + end + + it "should log a message if no changes or resources out of sync" do + processor.expects(:send).never + processor.expects(:raw_summary).returns({ + "resources" => { "changed" => 0, "out_of_sync" => 0 } + }) + + Puppet.expects(:notice).with("Not sending tagmail report; no changes") + processor.process + end + + it "should send email if raw_summary is not defined" do + processor.expects(:send).with([[['[email protected]'], message]]) + processor.expects(:raw_summary).returns(nil) + processor.process + end + + it "should send email if there are no resource metrics" do + processor.expects(:send).with([[['[email protected]'], message]]) + processor.expects(:raw_summary).returns({'resources' => nil}) + processor.process + end + end + end end + </pre> ---------------------------------------- Bug #13686: directoryservices provider can't handle missing ShadowHashData item https://projects.puppetlabs.com/issues/13686#change-59850 Author: Clay Caviness Status: Merged - Pending Release Priority: Normal Assignee: Gary Larizza Category: Darwin Target version: 2.7.13 Affected Puppet version: 2.7.12 Keywords: Branch: https://github.com/puppetlabs/puppet/pull/644 If a user plist ends up in a state with no ShadowHashData entry, directoryservices provider is unable to fix, failing with an error: `err: /Stage[main]/Security::Local_users/User[testuser]/password: change from [old password hash redacted] to [new password hash redacted] failed: The directoryservice provider can not handle attribute password at [...]users.pp:40` Inspecting the testuser.plist file in /var/db/dslocal/Default/users/, I found it was _missing_ the ShadowHashData item entirely. (I suspect this happened during a puppet run as we upgraded from 2.7.6 to 2.7.12, but I can't seem to recreate it reliably.) I tweaked directoryservice.rb at line 324 to create it if it was missing: <pre> if users_plist['ShadowHashData'] # users_plist['ShadowHashData'][0].string is actually a binary plist # that's nested INSIDE the user's plist (which itself is a binary # plist). password_hash_plist = users_plist['ShadowHashData'][0].string converted_hash_plist = convert_binary_to_xml(password_hash_plist) else users_plist['ShadowHashData'] = [StringIO.new] converted_hash_plist = {'SALTED-SHA512'=>StringIO.new} end </pre> ... and this seems to fix thing, though I'm not sure if this is the best method. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
