From 25a3f59be4a514629db6825ce4b5abb99371ae3e Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 20 Feb 2009 09:45:42 -0600 Subject: Fixing #558 - File checksums no longer refer to 'nosum' It was only used in a method that apparently was not in use any more. Signed-off-by: Luke Kanies --- lib/puppet/type/file/checksum.rb | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/lib/puppet/type/file/checksum.rb b/lib/puppet/type/file/checksum.rb index 785ed0fee..f681a8ebe 100755 --- a/lib/puppet/type/file/checksum.rb +++ b/lib/puppet/type/file/checksum.rb @@ -37,11 +37,6 @@ Puppet::Type.type(:file).newproperty(:checksum) do handlesum() end - newvalue(:nosum) do - # nothing - :nochange - end - # If they pass us a sum type, behave normally, but if they pass # us a sum type + sum, stick the sum in the cache. munge do |value| @@ -129,35 +124,6 @@ Puppet::Type.type(:file).newproperty(:checksum) do cache(checktype()) end - # Retrieve the cached sum - def getcachedsum - hash = nil - unless hash = @resource.cached(:checksums) - hash = {} - @resource.cache(:checksums, hash) - end - - sumtype = self.should - - if hash.include?(sumtype) - #self.notice "Found checksum %s for %s" % - # [hash[sumtype] ,@resource[:path]] - sum = hash[sumtype] - - unless sum =~ /^\{\w+\}/ - sum = "{%s}%s" % [sumtype, sum] - end - return sum - elsif hash.empty? - #self.notice "Could not find sum of type %s" % sumtype - return :nosum - else - #self.notice "Found checksum for %s but not of type %s" % - # [@resource[:path],sumtype] - return :nosum - end - end - # Calculate the sum from disk. def getsum(checktype, file = nil) sum = "" -- cgit From f07d9283d70378066987017d436db6deb1adae9d Mon Sep 17 00:00:00 2001 From: Peter Meier Date: Sun, 22 Feb 2009 00:11:34 +0100 Subject: Use Puppet.debug instead of own debug flag It's better to use puppet's logging environment than an own. Especially if the default is quite verbose and can't be set by config flag. --- lib/puppet/rails/host.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb index a429cbfb1..b60b0689d 100644 --- a/lib/puppet/rails/host.rb +++ b/lib/puppet/rails/host.rb @@ -3,8 +3,6 @@ require 'puppet/rails/fact_name' require 'puppet/rails/source_file' require 'puppet/util/rails/collection_merger' -Puppet::TIME_DEBUG = true - class Puppet::Rails::Host < ActiveRecord::Base include Puppet::Util include Puppet::Util::CollectionMerger @@ -36,7 +34,7 @@ class Puppet::Rails::Host < ActiveRecord::Base host = new(:name => node.name) end } - Puppet.notice("Searched for host in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG) + Puppet.debug("Searched for host in %0.2f seconds" % seconds) if ip = node.parameters["ipaddress"] host.ip = ip end @@ -51,7 +49,7 @@ class Puppet::Rails::Host < ActiveRecord::Base seconds = Benchmark.realtime { host.setresources(resources) } - Puppet.notice("Handled resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG) + Puppet.debug("Handled resources in %0.2f seconds" % seconds) host.last_compile = Time.now @@ -119,17 +117,17 @@ class Puppet::Rails::Host < ActiveRecord::Base seconds = Benchmark.realtime { existing = find_resources() } - Puppet.notice("Searched for resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG) + Puppet.debug("Searched for resources in %0.2f seconds" % seconds) seconds = Benchmark.realtime { find_resources_parameters_tags(existing) } if id - Puppet.notice("Searched for resource params and tags in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG) + Puppet.debug("Searched for resource params and tags in %0.2f seconds" % seconds) seconds = Benchmark.realtime { compare_to_catalog(existing, list) } - Puppet.notice("Resource comparison took %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG) + Puppet.debug("Resource comparison took %0.2f seconds" % seconds) end def find_resources -- cgit From 7504b0499170aa81a143f74065507410424e5e17 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Wed, 25 Feb 2009 02:25:09 +1100 Subject: Updated useradd.rb managehome confine to include other RH-like distributions --- lib/puppet/provider/user/useradd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb index 6996dd69a..15b3b1379 100644 --- a/lib/puppet/provider/user/useradd.rb +++ b/lib/puppet/provider/user/useradd.rb @@ -31,7 +31,7 @@ Puppet::Type.type(:user).provide :useradd, :parent => Puppet::Provider::NameServ cmd = [] if @resource.managehome? cmd << "-m" - elsif %w{Fedora RedHat}.include?(Facter.value("operatingsystem")) + elsif %w{Fedora RedHat CentOS OEL OVS}.include?(Facter.value("operatingsystem")) cmd << "-M" end cmd -- cgit From 8c010e0ccc8b007f8657a1025d2de36e2310e550 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Thu, 26 Feb 2009 11:08:23 +1100 Subject: Fixed #1910 - updated logcheck --- CHANGELOG | 2 ++ ext/logcheck/puppet | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 74219a084..e7d3dc4ee 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #1910 - updated logcheck + Fixed #1871 - Sensitive information leaked in log reports Fixed #1956 - Cleaned up variable names to be more sane, clarified error messages diff --git a/ext/logcheck/puppet b/ext/logcheck/puppet index 449ec70f5..8f2a7ee70 100644 --- a/ext/logcheck/puppet +++ b/ext/logcheck/puppet @@ -1,5 +1,6 @@ +^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: (Handled resources in|Resource comparison took|Searched for [host|resources|resource params and tags] in) [0-9.]+ seconds ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Starting Puppet server version [.0-9]+$ -^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Compiled configuration for [._[:alnum:]-]+ in [.0-9]+ seconds$ +^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Compiled catalog for [._[:alnum:]-]+ in [.0-9]+ seconds$ ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Caught TERM; shutting down$ ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Shutting down$ ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetd\[[0-9]+\]: Starting Puppet client version [.0-9]+$ -- cgit From 417023835fb5d175fef3b6a06591a8170a5f81a4 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Fri, 27 Feb 2009 08:00:27 +1100 Subject: Fixed #2025 - gentoo service provider handle only default init level --- CHANGELOG | 2 ++ lib/puppet/provider/service/gentoo.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index e7d3dc4ee..66bf08873 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #2025 - gentoo service provider handle only default init level + Fixed #1910 - updated logcheck Fixed #1871 - Sensitive information leaked in log reports diff --git a/lib/puppet/provider/service/gentoo.rb b/lib/puppet/provider/service/gentoo.rb index 4067dee5e..d62df1a38 100644 --- a/lib/puppet/provider/service/gentoo.rb +++ b/lib/puppet/provider/service/gentoo.rb @@ -38,7 +38,7 @@ Puppet::Type.type(:service).provide :gentoo, :parent => :init do return :false unless line # If it's enabled then it will print output showing service | runlevel - if output =~ /#{@resource[:name]}\s*\|\s*default/ + if output =~ /#{@resource[:name]}\s*\|\s*(boot|default)/ return :true else return :false -- cgit From 0e467869f4d427a8c42e1c2ff6a0bb215f288b09 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Sat, 28 Feb 2009 03:05:23 +1100 Subject: Fixed #1963 - Failing to read /proc/mounts for selinux kills file downloads --- CHANGELOG | 2 ++ lib/puppet/util/selinux.rb | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 66bf08873..c9ac112a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #1963 - Failing to read /proc/mounts for selinux kills file downloads + Fixed #2025 - gentoo service provider handle only default init level Fixed #1910 - updated logcheck diff --git a/lib/puppet/util/selinux.rb b/lib/puppet/util/selinux.rb index 70f244507..cd3b2ac1a 100644 --- a/lib/puppet/util/selinux.rb +++ b/lib/puppet/util/selinux.rb @@ -153,7 +153,9 @@ module Puppet::Util::SELinux # Internal helper function to read and parse /proc/mounts def read_mounts begin - mounts = File.read("/proc/mounts") + mountfh = File.open("/proc/mounts", NONBLOCK) + mounts = mountfh.read + mountfh.close rescue return nil end -- cgit From 373d505c381696f880c305a9357a6e50342079b8 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 12 Feb 2009 23:42:31 -0600 Subject: Adding a FileCollection and a lookup module for it. Signed-off-by: Luke Kanies --- lib/puppet/file_collection.rb | 20 ++++++++++++++++ lib/puppet/file_collection/lookup.rb | 16 +++++++++++++ spec/unit/file_collection.rb | 45 ++++++++++++++++++++++++++++++++++++ spec/unit/file_collection/lookup.rb | 41 ++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 lib/puppet/file_collection.rb create mode 100644 lib/puppet/file_collection/lookup.rb create mode 100755 spec/unit/file_collection.rb create mode 100755 spec/unit/file_collection/lookup.rb diff --git a/lib/puppet/file_collection.rb b/lib/puppet/file_collection.rb new file mode 100644 index 000000000..451b496a1 --- /dev/null +++ b/lib/puppet/file_collection.rb @@ -0,0 +1,20 @@ +# A simple way to turn file names into singletons, +# so we don't have tons of copies of each file path around. +class Puppet::FileCollection + def initialize + @paths = [] + end + + def index(path) + if @paths.include?(path) + return @paths.index(path) + else + @paths << path + return @paths.length - 1 + end + end + + def path(index) + @paths[index] + end +end diff --git a/lib/puppet/file_collection/lookup.rb b/lib/puppet/file_collection/lookup.rb new file mode 100644 index 000000000..8f69c6681 --- /dev/null +++ b/lib/puppet/file_collection/lookup.rb @@ -0,0 +1,16 @@ +require 'puppet/file_collection' + +# A simple module for looking up file paths and indexes +# in a file collection. +module Puppet::FileCollection::Lookup + attr_accessor :line, :file_index + + def file=(path) + @file_index = file_collection.index(path) + end + + def file + return nil unless file_index + file_collection.path(file_index) + end +end diff --git a/spec/unit/file_collection.rb b/spec/unit/file_collection.rb new file mode 100755 index 000000000..e9acc8dd2 --- /dev/null +++ b/spec/unit/file_collection.rb @@ -0,0 +1,45 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../spec_helper' + +require 'puppet/file_collection' + +describe Puppet::FileCollection do + before do + @collection = Puppet::FileCollection.new + end + + it "should be able to convert a file name into an index" do + @collection.index("/my/file").should be_instance_of(Fixnum) + end + + it "should be able to convert an index into a file name" do + index = @collection.index("/path/to/file") + @collection.path(index).should == "/path/to/file" + end + + it "should always give the same file name for a given index" do + index = @collection.index("/path/to/file") + @collection.path(index).should == @collection.path(index) + end + + it "should always give the same index for a given file name" do + @collection.index("/my/file").should == @collection.index("/my/file") + end + + it "should always correctly relate a file name and its index even when multiple files are in the collection" do + indexes = %w{a b c d e f}.inject({}) do |hash, letter| + hash[letter] = @collection.index("/path/to/file/%s" % letter) + hash + end + + indexes.each do |letter, index| + @collection.index("/path/to/file/%s" % letter).should == indexes[letter] + @collection.path(index).should == @collection.path(index) + end + end + + it "should return nil as the file name when an unknown index is provided" do + @collection.path(50).should be_nil + end +end diff --git a/spec/unit/file_collection/lookup.rb b/spec/unit/file_collection/lookup.rb new file mode 100755 index 000000000..9ae7ae582 --- /dev/null +++ b/spec/unit/file_collection/lookup.rb @@ -0,0 +1,41 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' +require 'puppet/file_collection/lookup' + +class LookupTester + include Puppet::FileCollection::Lookup +end + +describe Puppet::FileCollection::Lookup do + before do + @tester = LookupTester.new + + @file_collection = mock 'file_collection' + @tester.stubs(:file_collection).returns @file_collection + end + + it "should use the file collection to determine the index of the file name" do + @file_collection.expects(:index).with("/my/file").returns 50 + + @tester.file = "/my/file" + @tester.file_index.should == 50 + end + + it "should return nil as the file name if no index is set" do + @tester.file.should be_nil + end + + it "should use the file collection to convert the index to a file name" do + @file_collection.expects(:path).with(25).returns "/path/to/file" + + @tester.file_index = 25 + + @tester.file.should == "/path/to/file" + end + + it "should support a line attribute" do + @tester.line = 50 + @tester.line.should == 50 + end +end -- cgit From fa6494b69ad1b01a9c587c86aa1731f4702f5509 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 13 Feb 2009 00:29:07 -0600 Subject: Using the FileCollection where appropriate. This commit just replaces the :file and :line accessors with the use of the new FileCollection Lookup module. This should mean that we've normalized all file names in a given process, which *might* have drastic RAM improvements. For initial simplicity, I've gone with a single global collection of file names, but it's built so it's easy to use individual file collections instead. Signed-off-by: Luke Kanies --- lib/puppet/file_collection.rb | 8 ++++++++ lib/puppet/file_collection/lookup.rb | 4 ++++ lib/puppet/parser/ast.rb | 7 ++++--- lib/puppet/parser/ast/leaf.rb | 2 +- lib/puppet/parser/ast/resource_override.rb | 4 ++-- lib/puppet/parser/resource.rb | 6 +++++- lib/puppet/parser/resource/param.rb | 6 +++++- lib/puppet/parser/resource/reference.rb | 2 ++ lib/puppet/type.rb | 4 ++-- spec/unit/file_collection.rb | 8 ++++++++ spec/unit/file_collection/lookup.rb | 7 ++++++- spec/unit/parser/ast.rb | 6 +++++- spec/unit/parser/resource.rb | 4 ++++ spec/unit/parser/resource/reference.rb | 4 ++++ 14 files changed, 60 insertions(+), 12 deletions(-) diff --git a/lib/puppet/file_collection.rb b/lib/puppet/file_collection.rb index 451b496a1..69f59ffdf 100644 --- a/lib/puppet/file_collection.rb +++ b/lib/puppet/file_collection.rb @@ -1,6 +1,12 @@ # A simple way to turn file names into singletons, # so we don't have tons of copies of each file path around. class Puppet::FileCollection + require 'puppet/file_collection/lookup' + + def self.collection + @collection + end + def initialize @paths = [] end @@ -17,4 +23,6 @@ class Puppet::FileCollection def path(index) @paths[index] end + + @collection = self.new end diff --git a/lib/puppet/file_collection/lookup.rb b/lib/puppet/file_collection/lookup.rb index 8f69c6681..ddb0c8431 100644 --- a/lib/puppet/file_collection/lookup.rb +++ b/lib/puppet/file_collection/lookup.rb @@ -5,6 +5,10 @@ require 'puppet/file_collection' module Puppet::FileCollection::Lookup attr_accessor :line, :file_index + def file_collection + Puppet::FileCollection.collection + end + def file=(path) @file_index = file_collection.index(path) end diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb index 303d75bac..ab23dd1f8 100644 --- a/lib/puppet/parser/ast.rb +++ b/lib/puppet/parser/ast.rb @@ -2,6 +2,7 @@ require 'puppet' require 'puppet/util/autoload' +require 'puppet/file_collection/lookup' # The base class for all of the objects that make up the parse trees. # Handles things like file name, line #, and also does the initialization @@ -10,11 +11,13 @@ class Puppet::Parser::AST # Do this so I don't have to type the full path in all of the subclasses AST = Puppet::Parser::AST + include Puppet::FileCollection::Lookup + include Puppet::Util::Errors include Puppet::Util::MethodHelper include Puppet::Util::Docs - attr_accessor :line, :file, :parent, :scope + attr_accessor :parent, :scope # don't fetch lexer comment by default def use_docs @@ -82,8 +85,6 @@ class Puppet::Parser::AST # method for them. This is probably pretty inefficient and should # likely be changed at some point. def initialize(args) - @file = nil - @line = nil set_options(args) end end diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb index c545c1e47..dba9812a7 100644 --- a/lib/puppet/parser/ast/leaf.rb +++ b/lib/puppet/parser/ast/leaf.rb @@ -36,7 +36,7 @@ class Puppet::Parser::AST # Interpolate the string looking for variables, and then return # the result. def evaluate(scope) - return scope.strinterp(@value, @file, @line) + return scope.strinterp(@value, file, line) end end diff --git a/lib/puppet/parser/ast/resource_override.rb b/lib/puppet/parser/ast/resource_override.rb index 5c4a2410f..f8cf3a81e 100644 --- a/lib/puppet/parser/ast/resource_override.rb +++ b/lib/puppet/parser/ast/resource_override.rb @@ -40,8 +40,8 @@ class Puppet::Parser::AST :type => r.type, :title => r.title, :params => params, - :file => @file, - :line => @line, + :file => file, + :line => line, :source => scope.source, :scope => scope ) diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index 2fdd78ddf..7f0e33359 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -4,13 +4,17 @@ class Puppet::Parser::Resource require 'puppet/parser/resource/param' require 'puppet/parser/resource/reference' require 'puppet/util/tagging' + require 'puppet/file_collection/lookup' + + include Puppet::FileCollection::Lookup + include Puppet::Util include Puppet::Util::MethodHelper include Puppet::Util::Errors include Puppet::Util::Logging include Puppet::Util::Tagging - attr_accessor :source, :line, :file, :scope, :rails_id + attr_accessor :source, :scope, :rails_id attr_accessor :virtual, :override, :translated attr_reader :exported, :evaluated, :params diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb index 1a5cfe8fb..7ce58f4c4 100644 --- a/lib/puppet/parser/resource/param.rb +++ b/lib/puppet/parser/resource/param.rb @@ -1,10 +1,14 @@ +require 'puppet/file_collection/lookup' + # The parameters we stick in Resources. class Puppet::Parser::Resource::Param - attr_accessor :name, :value, :source, :line, :file, :add + attr_accessor :name, :value, :source, :add include Puppet::Util include Puppet::Util::Errors include Puppet::Util::MethodHelper + include Puppet::FileCollection::Lookup + def initialize(hash) set_options(hash) requiredopts(:name, :value, :source) diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb index cb505d606..bffc03788 100644 --- a/lib/puppet/parser/resource/reference.rb +++ b/lib/puppet/parser/resource/reference.rb @@ -1,7 +1,9 @@ require 'puppet/resource_reference' +require 'puppet/file_collection/lookup' # A reference to a resource. Mostly just the type and title. class Puppet::Parser::Resource::Reference < Puppet::ResourceReference + include Puppet::FileCollection::Lookup include Puppet::Util::MethodHelper include Puppet::Util::Errors diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index b57c74b95..5cb4ce13d 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -10,6 +10,7 @@ require 'puppet/util/errors' require 'puppet/util/log_paths' require 'puppet/util/logging' require 'puppet/resource_reference' +require 'puppet/file_collection/lookup' # see the bottom of the file for the rest of the inclusions @@ -19,6 +20,7 @@ class Type include Puppet::Util::Errors include Puppet::Util::LogPaths include Puppet::Util::Logging + include Puppet::FileCollection::Lookup ############################### # Code related to resource type attributes. @@ -2135,8 +2137,6 @@ class Type # In naming methods, I have tried to consistently name the method so # that it is clear whether it operates on all attributes (thus has 'attr' in # the method name, or whether it operates on a specific type of attributes. - attr_accessor :file, :line - attr_writer :title attr_writer :noop diff --git a/spec/unit/file_collection.rb b/spec/unit/file_collection.rb index e9acc8dd2..81bcc2d2d 100755 --- a/spec/unit/file_collection.rb +++ b/spec/unit/file_collection.rb @@ -42,4 +42,12 @@ describe Puppet::FileCollection do it "should return nil as the file name when an unknown index is provided" do @collection.path(50).should be_nil end + + it "should provide a global collection" do + Puppet::FileCollection.collection.should be_instance_of(Puppet::FileCollection) + end + + it "should reuse the global collection" do + Puppet::FileCollection.collection.should equal(Puppet::FileCollection.collection) + end end diff --git a/spec/unit/file_collection/lookup.rb b/spec/unit/file_collection/lookup.rb index 9ae7ae582..81cc61872 100755 --- a/spec/unit/file_collection/lookup.rb +++ b/spec/unit/file_collection/lookup.rb @@ -12,7 +12,7 @@ describe Puppet::FileCollection::Lookup do @tester = LookupTester.new @file_collection = mock 'file_collection' - @tester.stubs(:file_collection).returns @file_collection + Puppet::FileCollection.stubs(:collection).returns @file_collection end it "should use the file collection to determine the index of the file name" do @@ -38,4 +38,9 @@ describe Puppet::FileCollection::Lookup do @tester.line = 50 @tester.line.should == 50 end + + it "should default to the global file collection" do + Puppet::FileCollection.expects(:collection).returns "collection" + @tester.file_collection.should == "collection" + end end diff --git a/spec/unit/parser/ast.rb b/spec/unit/parser/ast.rb index 513943725..35e2b1eff 100644 --- a/spec/unit/parser/ast.rb +++ b/spec/unit/parser/ast.rb @@ -5,6 +5,10 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/parser/ast' describe Puppet::Parser::AST do + + it "should use the file lookup module" do + Puppet::Parser::AST.ancestors.should be_include(Puppet::FileCollection::Lookup) + end it "should have a doc accessor" do ast = Puppet::Parser::AST.new({}) @@ -34,4 +38,4 @@ describe Puppet::Parser::AST do end end -end \ No newline at end of file +end diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb index 63cfbc2ed..2666f6461 100755 --- a/spec/unit/parser/resource.rb +++ b/spec/unit/parser/resource.rb @@ -44,6 +44,10 @@ describe Puppet::Parser::Resource do end end + it "should use the file lookup module" do + Puppet::Parser::Resource.ancestors.should be_include(Puppet::FileCollection::Lookup) + end + it "should be isomorphic if it is builtin and models an isomorphic type" do Puppet::Type.type(:file).expects(:isomorphic?).returns(true) @resource = Puppet::Parser::Resource.new(:type => "file", :title => "whatever", :scope => @scope, :source => @source).isomorphic?.should be_true diff --git a/spec/unit/parser/resource/reference.rb b/spec/unit/parser/resource/reference.rb index bb1452692..6284e67cc 100755 --- a/spec/unit/parser/resource/reference.rb +++ b/spec/unit/parser/resource/reference.rb @@ -7,6 +7,10 @@ describe Puppet::Parser::Resource::Reference do @type = Puppet::Parser::Resource::Reference end + it "should use the file lookup module" do + Puppet::Parser::Resource::Reference.ancestors.should be_include(Puppet::FileCollection::Lookup) + end + it "should require a type" do proc { @type.new(:title => "yay") }.should raise_error(Puppet::DevError) end -- cgit From e2b406239eaa255c41acb31942169296bea71948 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 13 Feb 2009 00:46:45 -0600 Subject: Adding a performance optimization to the FileCollection. I saw about a 7x speed increase when adding this simple hash to speed up the file index lookup. Signed-off-by: Luke Kanies --- lib/puppet/file_collection.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/puppet/file_collection.rb b/lib/puppet/file_collection.rb index 69f59ffdf..7db2600c0 100644 --- a/lib/puppet/file_collection.rb +++ b/lib/puppet/file_collection.rb @@ -9,14 +9,16 @@ class Puppet::FileCollection def initialize @paths = [] + @inverse = {} end def index(path) - if @paths.include?(path) - return @paths.index(path) + if i = @inverse[path] + return i else @paths << path - return @paths.length - 1 + i = @inverse[path] = @paths.length - 1 + return i end end -- cgit From c052ff881e4a0cf6edfe4c1974597cd3abb378cf Mon Sep 17 00:00:00 2001 From: Paul Lathrop Date: Fri, 27 Feb 2009 12:25:28 -0800 Subject: Make puppetd --waitforcert option behave as documented: "You can turn off waiting for certificates by specifying a time of 0." Also add a test to ensure we catch any future regression of this behavior. Signed-off-by: Paul Lathrop --- lib/puppet/executables/client/certhandler.rb | 7 ++++++- spec/unit/executables/client/certhandler.rb | 13 +++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/puppet/executables/client/certhandler.rb b/lib/puppet/executables/client/certhandler.rb index b041397ae..bf3ea167d 100644 --- a/lib/puppet/executables/client/certhandler.rb +++ b/lib/puppet/executables/client/certhandler.rb @@ -49,7 +49,12 @@ module Puppet exit(23) if @one_time end - sleep @wait_for_cert + if @wait_for_cert > 0 + sleep @wait_for_cert + else + Puppet.notice "waitforcert disabled; exiting with no certificate" + exit(1) + end end end diff --git a/spec/unit/executables/client/certhandler.rb b/spec/unit/executables/client/certhandler.rb index 4f8f8139c..69d6de92b 100755 --- a/spec/unit/executables/client/certhandler.rb +++ b/spec/unit/executables/client/certhandler.rb @@ -91,6 +91,19 @@ describe cert_handler, "when handling certificates" do end end + describe "when waitforcert is disabled" do + before do + @handler = cert_handler.new(0, false) + @handler.stubs(:read_cert).returns false + end + + it "should exit if the cert request does not return a certificate" do + @caclient.stubs(:request_cert).returns(false) + @handler.expects(:exit).with(1).raises(SystemExit) + lambda { @handler.retrieve_cert }.should raise_error(SystemExit) + end + end + describe "when in one time mode" do before do #true puts us in onetime mode -- cgit From ec56ddf8f5b58f16d0067055346889be79b29186 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Sat, 28 Feb 2009 10:30:23 +1100 Subject: This script fixes the most common issues with inconsistent storeconfigs database (including duplicate resources record, duplicate param_values records, dangling records...). Usage: stop all puppetmasters backup your database! % cat ext/dbfix.sql | mysql puppet relaunch all puppetmasters Signed-off-by: Brice Figureau --- --- CHANGELOG | 2 ++ ext/dbfix.sql | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 ext/dbfix.sql diff --git a/CHANGELOG b/CHANGELOG index c9ac112a8..70caf4510 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Added ext/dbfix.sql script - fixes common errors in stored configuration databases + Fixed #1963 - Failing to read /proc/mounts for selinux kills file downloads Fixed #2025 - gentoo service provider handle only default init level diff --git a/ext/dbfix.sql b/ext/dbfix.sql new file mode 100644 index 000000000..d22914adf --- /dev/null +++ b/ext/dbfix.sql @@ -0,0 +1,98 @@ +-- MySQL DB consistency check/fix +-- +-- Usage: +-- cat dbfix.sql | mysql -u user -p puppet +-- +-- WARNING: perform a database backup before running this script + +-- Remove duplicate resources, and keep the latest one +DELETE bad_rows.* +FROM resources AS bad_rows + INNER JOIN ( + SELECT title,restype,host_id, MAX(id) as max_id + FROM resources + GROUP BY title,restype,host_id + HAVING count(*) > 1 + ) AS good_rows + ON + good_rows.title = bad_rows.title AND + good_rows.restype = bad_rows.restype AND + good_rows.host_id = bad_rows.host_id AND + good_rows.max_id <> bad_rows.id; + +-- Remove duplicate param_values, and keep the latest one +DELETE bad_rows.* +FROM param_values AS bad_rows + INNER JOIN ( + SELECT value,param_name_id,resource_id, MAX(id) as max_id + FROM param_values + GROUP BY value,param_name_id,resource_id + HAVING count(*) > 1 + ) AS good_rows + ON + good_rows.value = bad_rows.value AND + good_rows.param_name_id = bad_rows.param_name_id AND + good_rows.resource_id = bad_rows.resource_id AND + good_rows.max_id <> bad_rows.id; + +-- Remove duplicate param_names, and keep the latest one +DELETE bad_rows.* +FROM param_names AS bad_rows + INNER JOIN ( + SELECT name, MAX(id) as max_id + FROM param_names + GROUP BY name + HAVING count(*) > 1 + ) AS good_rows + ON + good_rows.name = bad_rows.name AND + good_rows.max_id <> bad_rows.id; + +-- Remove duplicate resource_tags, and keep the latest one +DELETE bad_rows.* +FROM resource_tags AS bad_rows + INNER JOIN ( + SELECT resource_id,puppet_tag_id, MAX(id) as max_id + FROM param_names + GROUP BY resource_id,puppet_tag_id + HAVING count(*) > 1 + ) AS good_rows + ON + good_rows.resource_id = bad_rows.resource_id AND + good_rows.puppet_tag_id = bad_rows.puppet_tag_id AND + good_rows.max_id <> bad_rows.id; + +-- Remove duplicate puppet_tags, and keep the latest one +DELETE bad_rows.* +FROM puppet_tags AS bad_rows + INNER JOIN ( + SELECT name, MAX(id) as max_id + FROM puppet_tags + GROUP BY name + HAVING count(*) > 1 + ) AS good_rows + ON + good_rows.name = bad_rows.name AND + good_rows.max_id <> bad_rows.id; + +-- Fix dangling resources +-- note: we use a table to not exceed the number of InnoDB locks if there are two much +-- rows to delete. +-- this is an alternative to: DELETE resources FROM resources r LEFT JOIN hosts h ON h.id=r.host_id WHERE h.id IS NULL; +-- +CREATE TABLE resources_c LIKE resources; +INSERT INTO resources_c SELECT r.* FROM resources r INNER JOIN hosts h ON h.id=r.host_id; +RENAME TABLE resources TO resources_old, resources_c TO resources; +DROP TABLE resources_old; + +-- Fix dangling param_values +CREATE TABLE param_values_c LIKE param_values; +INSERT INTO param_values_c SELECT v.* FROM param_values v INNER JOIN resources r ON r.id=v.resource_id; +RENAME TABLE param_values TO param_values_old, param_values_c TO param_values; +DROP TABLE param_values_old; + +-- Fix dangling resource_tags +CREATE TABLE resource_tags_c LIKE resource_tags; +INSERT INTO resource_tags_c SELECT t.* FROM resource_tags t INNER JOIN resources r ON r.id=t.resource_id; +RENAME TABLE resource_tags TO resource_tags_old, resource_tags_c TO resource_tags; +DROP TABLE resource_tags_old; -- cgit From 23066c1b117af4d531efad79ee11ed683ac92e5e Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 28 Feb 2009 06:40:46 +0000 Subject: Fixing every failing test I can find on the build server. All but one of these tests is fixed by: * Stubbing Puppet.settings.use * Adding /usr/sbin to PATH The only other one was the package integration test, which stupidly assumed a default was specified in the test. The fix here is twofold: Remove that assumption (the test is now 'pending' if no default is available), and add a default for Ubuntu. (The default is in the test, not the default package provider - that is, it's testing the default package provider, thus it can't rely on that information.) Signed-off-by: Luke Kanies --- spec/integration/checksum.rb | 1 + spec/integration/reports.rb | 4 ++++ spec/integration/transaction/report.rb | 8 +++++++- spec/integration/type/package.rb | 7 +++++-- spec/unit/indirector/ssl_rsa/file.rb | 5 +++++ spec/unit/node/catalog.rb | 1 + spec/unit/other/transbucket.rb | 8 ++++---- spec/unit/provider/mount/parsed.rb | 1 + spec/unit/rails.rb | 1 + spec/unit/type/file.rb | 2 ++ spec/unit/type/group.rb | 3 +++ spec/unit/type/noop_metaparam.rb | 1 + spec/unit/type/tidy.rb | 4 ++++ spec/unit/type/user.rb | 3 +++ 14 files changed, 42 insertions(+), 7 deletions(-) diff --git a/spec/integration/checksum.rb b/spec/integration/checksum.rb index c94f3e47e..49ae8d2b7 100755 --- a/spec/integration/checksum.rb +++ b/spec/integration/checksum.rb @@ -9,6 +9,7 @@ require 'puppet/checksum' describe Puppet::Checksum, " when using the file terminus" do before do + Puppet.settings.stubs(:use) Puppet::Checksum.terminus_class = :file @content = "this is some content" @sum = Puppet::Checksum.new(@content) diff --git a/spec/integration/reports.rb b/spec/integration/reports.rb index 7351c3da1..cc4ae8f4c 100755 --- a/spec/integration/reports.rb +++ b/spec/integration/reports.rb @@ -8,6 +8,10 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/reports' describe Puppet::Reports, " when using report types" do + before do + Puppet.settings.stubs(:use) + end + it "should load report types as modules" do Puppet::Reports.report(:store).should be_instance_of(Module) end diff --git a/spec/integration/transaction/report.rb b/spec/integration/transaction/report.rb index 48e59f203..333deac0e 100755 --- a/spec/integration/transaction/report.rb +++ b/spec/integration/transaction/report.rb @@ -7,7 +7,13 @@ require File.dirname(__FILE__) + '/../../spec_helper' describe Puppet::Transaction::Report do describe "when using the indirector" do - after { Puppet::Transaction::Report.indirection.clear_cache } + before do + Puppet.settings.stubs(:use) + end + + after do + Puppet::Transaction::Report.indirection.clear_cache + end it "should be able to delegate to the :processor terminus" do Puppet::Transaction::Report.indirection.stubs(:terminus_class).returns :processor diff --git a/spec/integration/type/package.rb b/spec/integration/type/package.rb index def44ad97..a3d8eb278 100755 --- a/spec/integration/type/package.rb +++ b/spec/integration/type/package.rb @@ -9,7 +9,7 @@ describe Puppet::Type.type(:package), "when choosing a default package provider" end def provider_name(os) - {"Debian" => :apt, "Darwin" => :apple, "RedHat" => :up2date, "Fedora" => :yum, "FreeBSD" => :ports, "OpenBSD" => :openbsd, "Solaris" => :sun}[os] + {"Ubuntu" => :apt, "Debian" => :apt, "Darwin" => :apple, "RedHat" => :up2date, "Fedora" => :yum, "FreeBSD" => :ports, "OpenBSD" => :openbsd, "Solaris" => :sun}[os] end it "should have a default provider" do @@ -17,6 +17,9 @@ describe Puppet::Type.type(:package), "when choosing a default package provider" end it "should choose the correct provider each platform" do - Puppet::Type.type(:package).defaultprovider.name.should == provider_name(Facter.value(:operatingsystem)) + unless default_provider = provider_name(Facter.value(:operatingsystem)) + pending("No default provider specified in this test for %s" % Facter.value(:operatingsystem)) + end + Puppet::Type.type(:package).defaultprovider.name.should == default_provider end end diff --git a/spec/unit/indirector/ssl_rsa/file.rb b/spec/unit/indirector/ssl_rsa/file.rb index 76e5e3a94..4800f4a71 100755 --- a/spec/unit/indirector/ssl_rsa/file.rb +++ b/spec/unit/indirector/ssl_rsa/file.rb @@ -22,6 +22,7 @@ end describe Puppet::Indirector::SslRsa::File, " when choosing a path for a ca key" do before do + Puppet.settings.stubs(:use) @file = Puppet::Indirector::SslRsa::File.new @name = :ca end @@ -38,6 +39,7 @@ end describe Puppet::Indirector::SslRsa::File, " when choosing a path for a non-ca key" do before do + Puppet.settings.stubs(:use) @file = Puppet::Indirector::SslRsa::File.new @name = :publickey end @@ -54,6 +56,7 @@ end describe Puppet::Indirector::SslRsa::File, " when saving" do before do + Puppet.settings.stubs(:use) @file = Puppet::Indirector::SslRsa::File.new Puppet.settings.stubs(:value).with(:publickeydir).returns("/dir") @@ -72,6 +75,7 @@ end describe Puppet::Indirector::SslRsa::File, " when finding a key by name" do before do + Puppet.settings.stubs(:use) @file = Puppet::Indirector::SslRsa::File.new Puppet.settings.stubs(:value).with(:publickeydir).returns("/dir") @@ -95,6 +99,7 @@ end describe Puppet::Indirector::SslRsa::File, " when removing a key" do before do + Puppet.settings.stubs(:use) @file = Puppet::Indirector::SslRsa::File.new Puppet.settings.stubs(:value).with(:publickeydir).returns("/dir") diff --git a/spec/unit/node/catalog.rb b/spec/unit/node/catalog.rb index be198b88e..f6ef291a5 100755 --- a/spec/unit/node/catalog.rb +++ b/spec/unit/node/catalog.rb @@ -794,6 +794,7 @@ describe Puppet::Node::Catalog, " when writing dot files" do end it "should write a dot file based on the passed name" do + Puppet.settings.stubs(:use) File.expects(:open).with(@file, "w").yields(stub("file", :puts => nil)) @catalog.expects(:to_dot).with("name" => @name.to_s.capitalize) @catalog.host_config = true diff --git a/spec/unit/other/transbucket.rb b/spec/unit/other/transbucket.rb index 4494f2abb..0240a4473 100755 --- a/spec/unit/other/transbucket.rb +++ b/spec/unit/other/transbucket.rb @@ -70,20 +70,20 @@ describe Puppet::TransBucket, " when generating a catalog" do @bottom = Puppet::TransBucket.new @bottom.type = "fake" @bottom.name = "bottom" - @bottomobj = Puppet::TransObject.new("bottom", "user") + @bottomobj = Puppet::TransObject.new("bottom", "notify") @bottom.push @bottomobj @middle = Puppet::TransBucket.new @middle.type = "fake" @middle.name = "middle" - @middleobj = Puppet::TransObject.new("middle", "user") + @middleobj = Puppet::TransObject.new("middle", "notify") @middle.push(@middleobj) @middle.push(@bottom) @top = Puppet::TransBucket.new @top.type = "fake" @top.name = "top" - @topobj = Puppet::TransObject.new("top", "user") + @topobj = Puppet::TransObject.new("top", "notify") @top.push(@topobj) @top.push(@middle) @@ -98,7 +98,7 @@ describe Puppet::TransBucket, " when generating a catalog" do it "should convert all transportable objects to RAL resources" do @catalog = @top.to_catalog @users.each do |name| - @catalog.vertices.find { |r| r.class.name == :user and r.title == name }.should be_instance_of(Puppet::Type.type(:user)) + @catalog.vertices.find { |r| r.class.name == :notify and r.title == name }.should be_instance_of(Puppet::Type.type(:notify)) end end diff --git a/spec/unit/provider/mount/parsed.rb b/spec/unit/provider/mount/parsed.rb index df0e992f8..66891e6bf 100755 --- a/spec/unit/provider/mount/parsed.rb +++ b/spec/unit/provider/mount/parsed.rb @@ -130,6 +130,7 @@ describe provider_class do describe provider_class, " when modifying the filesystem tab" do include ParsedMountTesting before do + Puppet.settings.stubs(:use) # Never write to disk, only to RAM. @provider_class.stubs(:filetype).returns(Puppet::Util::FileType.filetype(:ram)) diff --git a/spec/unit/rails.rb b/spec/unit/rails.rb index 382f7c0f4..694bb55c7 100755 --- a/spec/unit/rails.rb +++ b/spec/unit/rails.rb @@ -7,6 +7,7 @@ describe Puppet::Rails, "when initializing any connection" do confine "Cannot test without ActiveRecord" => Puppet.features.rails? before do + Puppet.settings.stubs(:use) @logger = mock 'logger' @logger.stub_everything Logger.stubs(:new).returns(@logger) diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb index 0b7bfa8fb..da03dd349 100755 --- a/spec/unit/type/file.rb +++ b/spec/unit/type/file.rb @@ -4,6 +4,8 @@ require File.dirname(__FILE__) + '/../../spec_helper' describe Puppet::Type.type(:file) do before do + Puppet.settings.stubs(:use) + @path = Tempfile.new("puppetspec") @path.close!() @path = @path.path diff --git a/spec/unit/type/group.rb b/spec/unit/type/group.rb index d7e06dcd8..66a7daf06 100755 --- a/spec/unit/type/group.rb +++ b/spec/unit/type/group.rb @@ -4,6 +4,9 @@ require File.dirname(__FILE__) + '/../../spec_helper' describe Puppet::Type.type(:group) do before do + unless ENV["PATH"].split(File::PATH_SEPARATOR).include?("/usr/sbin") + ENV["PATH"] += File::PATH_SEPARATOR + "/usr/sbin" + end @class = Puppet::Type.type(:group) end diff --git a/spec/unit/type/noop_metaparam.rb b/spec/unit/type/noop_metaparam.rb index 8510a1b6b..73fed53c1 100755 --- a/spec/unit/type/noop_metaparam.rb +++ b/spec/unit/type/noop_metaparam.rb @@ -6,6 +6,7 @@ require 'puppet/type' describe Puppet::Type.type(:file).attrclass(:noop) do before do + Puppet.settings.stubs(:use) @file = Puppet::Type.newfile :path => "/what/ever" end diff --git a/spec/unit/type/tidy.rb b/spec/unit/type/tidy.rb index 9bcae86d7..3d0ff172b 100755 --- a/spec/unit/type/tidy.rb +++ b/spec/unit/type/tidy.rb @@ -5,6 +5,10 @@ require File.dirname(__FILE__) + '/../../spec_helper' tidy = Puppet::Type.type(:tidy) describe tidy do + before do + Puppet.settings.stubs(:use) + end + after { tidy.clear } it "should be in sync if the targeted file does not exist" do diff --git a/spec/unit/type/user.rb b/spec/unit/type/user.rb index dadcc65ef..4f4f2c0eb 100755 --- a/spec/unit/type/user.rb +++ b/spec/unit/type/user.rb @@ -6,6 +6,9 @@ user = Puppet::Type.type(:user) describe user do before do + unless ENV["PATH"].split(File::PATH_SEPARATOR).include?("/usr/sbin") + ENV["PATH"] += File::PATH_SEPARATOR + "/usr/sbin" + end @provider = stub 'provider' @resource = stub 'resource', :resource => nil, :provider => @provider, :line => nil, :file => nil end -- cgit From 0c16426a772c85b0ab45bfd4c3ba5189ace2d6b2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 28 Feb 2009 07:55:56 +0000 Subject: Fixing broken 0.24.x tests in test/. These tests once again largely were caused by /usr/sbin not being in the path and by ~ not being writable. The only tests still failing are Rails tests, and my guess is that they're all failing because of the recent work by Brice. They should probably just be removed. Signed-off-by: Luke Kanies --- test/executables/filebucket.rb | 9 ++++++--- test/executables/puppetbin.rb | 2 +- test/lib/puppettest.rb | 3 +++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/test/executables/filebucket.rb b/test/executables/filebucket.rb index 491630707..f479c150e 100755 --- a/test/executables/filebucket.rb +++ b/test/executables/filebucket.rb @@ -12,12 +12,15 @@ class TestFileBucketExe < Test::Unit::TestCase include PuppetTest::ExeTest def test_local + basedir = tempfile() + FileUtils.mkdir_p(basedir) + bucket = tempfile file = tempfile text = "somet ext" md5 = Digest::MD5.hexdigest(text) File.open(file, "w") { |f| f.print text } - out = %x{filebucket --bucket #{bucket} backup #{file}} + out = %x{filebucket --confdir #{basedir} --vardir #{basedir} --bucket #{bucket} backup #{file}} outfile, outmd5 = out.chomp.split(": ") @@ -35,12 +38,12 @@ class TestFileBucketExe < Test::Unit::TestCase assert_equal(text, newtext, "did not get correct file from md5 sum") - out = %x{filebucket --bucket #{bucket} get #{md5}} + out = %x{filebucket --confdir #{basedir} --vardir #{basedir} --bucket #{bucket} get #{md5}} assert_equal(0, $?, "filebucket did not run successfully") assert_equal(text, out, "did not get correct text back from filebucket") File.open(file, "w") { |f| f.puts "some other txt" } - out = %x{filebucket --bucket #{bucket} restore #{file} #{md5}} + out = %x{filebucket --confdir #{basedir} --vardir #{basedir} --bucket #{bucket} restore #{file} #{md5}} assert_equal(0, $?, "filebucket did not run successfully") assert_equal(text, File.read(file), "file was not restored") end diff --git a/test/executables/puppetbin.rb b/test/executables/puppetbin.rb index 08329efb6..631b3b22a 100755 --- a/test/executables/puppetbin.rb +++ b/test/executables/puppetbin.rb @@ -72,7 +72,7 @@ class TestPuppetBin < Test::Unit::TestCase end File.open(manifest, "w") do |f| f.puts "#!#{env} puppet - file { '#{path}': ensure => file }" + exec { '/bin/touch #{path}': }" end File.chmod(0755, manifest) diff --git a/test/lib/puppettest.rb b/test/lib/puppettest.rb index 63f8121b5..7fb98ef1d 100755 --- a/test/lib/puppettest.rb +++ b/test/lib/puppettest.rb @@ -166,6 +166,9 @@ module PuppetTest end def setup + unless ENV["PATH"].split(File::PATH_SEPARATOR).include?("/usr/sbin") + ENV["PATH"] += File::PATH_SEPARATOR + "/usr/sbin" + end @memoryatstart = Puppet::Util.memory if defined? @@testcount @@testcount += 1 -- cgit From ac87600bfc63ab589f3f691fd649f496467cc048 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Tue, 3 Mar 2009 12:35:37 +1100 Subject: Fixed report reference page --- lib/puppet/reference/report.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/puppet/reference/report.rb b/lib/puppet/reference/report.rb index be8e64751..2ea00b6d6 100644 --- a/lib/puppet/reference/report.rb +++ b/lib/puppet/reference/report.rb @@ -20,4 +20,5 @@ reports must be comma-separated. You can also specify ``none`` to disable reports entirely. Puppet provides multiple report handlers that will process client reports: + " -- cgit From 719a8df167b7456fe0a1b7b05aff246aa6f73d39 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Wed, 4 Mar 2009 19:23:56 +1100 Subject: Fixed to rake tests for reductivelabs build --- tasks/rake/reductive.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/rake/reductive.rb b/tasks/rake/reductive.rb index 2fbd2b4d9..ed4abd9e8 100644 --- a/tasks/rake/reductive.rb +++ b/tasks/rake/reductive.rb @@ -455,10 +455,10 @@ class Rake::RedLabProject < Rake::TaskLib desc "Run all unit tests." task :alltests do - if FileTest.exists?("test/Rakefile") - sh %{cd test; rake} + if FileTest.exists?("spec/Rakefile") + sh %{cd spec; rake} else - Dir.chdir("test") do + Dir.chdir("spec") do Dir.entries(".").find_all { |f| f =~ /\.rb/ }.each do |f| sh %{ruby #{f}} end -- cgit From 9577d3af929727ac1e7b5e7e54e4491990d55995 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 3 Mar 2009 23:20:29 -0600 Subject: Fixing #2013 - prefetching had a mismatch between type and title The ParsedFile types seem to be the main one that suffers from this, but the transactions were using the resource titles, not names, so resources were often not getting prefetched correctly. Signed-off-by: Luke Kanies --- lib/puppet/transaction.rb | 2 +- spec/unit/transaction.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index f3defb7a2..43ba8582c 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -480,7 +480,7 @@ class Transaction @catalog.vertices.each do |resource| if provider = resource.provider and provider.class.respond_to?(:prefetch) prefetchers[provider.class] ||= {} - prefetchers[provider.class][resource.title] = resource + prefetchers[provider.class][resource.name] = resource end end diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb index e5ec64054..9122e0eb3 100755 --- a/spec/unit/transaction.rb +++ b/spec/unit/transaction.rb @@ -4,6 +4,21 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/transaction' +describe Puppet::Transaction do + it "should match resources by name, not title, when prefetching" do + @catalog = Puppet::Node::Catalog.new + @transaction = Puppet::Transaction.new(@catalog) + + # Have both a title and name + resource = Puppet::Type.type(:sshkey).create :title => "foo", :name => "bar", :type => :dsa, :key => "eh" + @catalog.add_resource resource + + resource.provider.class.expects(:prefetch).with("bar" => resource) + + @transaction.prefetch + end +end + describe Puppet::Transaction, " when determining tags" do before do @config = Puppet::Node::Catalog.new -- cgit From a790ee3b487a7eac6668e45747e338d44d75da9e Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Wed, 4 Mar 2009 19:28:45 +1100 Subject: Further fix to #1910 --- ext/logcheck/puppet | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/logcheck/puppet b/ext/logcheck/puppet index 8f2a7ee70..d1b1b27bd 100644 --- a/ext/logcheck/puppet +++ b/ext/logcheck/puppet @@ -1,4 +1,4 @@ -^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: (Handled resources in|Resource comparison took|Searched for [host|resources|resource params and tags] in) [0-9.]+ seconds +^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: (Handled resources in|Resource comparison took|Searched for (host|resources|resource params and tags) in) [0-9.]+ seconds ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Starting Puppet server version [.0-9]+$ ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Compiled catalog for [._[:alnum:]-]+ in [.0-9]+ seconds$ ^\w{3} [ :0-9]{11} [._[:alnum:]-]+ puppetmasterd\[[0-9]+\]: Caught TERM; shutting down$ -- cgit From c55ac3f2c2335de0beacd2cb3396b550c8f1402f Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Mon, 23 Feb 2009 20:33:16 +0100 Subject: Fix #2010 - add protection code for some storeconfig corruption This patch adds a more robust and self-healing storedconfig in case of logically corrupted database as the one in #2010 (where multiple resources of same references are present in the database for the same host). The problem is that the resources are stored in a hash with the resource ref as the key, so we collapse resource of different id but same reference. The patch fixed this by using a hash by resource id, and maintaining a list of old extraneous resource in the db that are removved after the storeconfig pass. Signed-off-by: Brice Figureau --- lib/puppet/rails/host.rb | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb index b60b0689d..851cc21d9 100644 --- a/lib/puppet/rails/host.rb +++ b/lib/puppet/rails/host.rb @@ -113,26 +113,26 @@ class Puppet::Rails::Host < ActiveRecord::Base # Set our resources. def setresources(list) - existing = nil + resource_by_id = nil seconds = Benchmark.realtime { - existing = find_resources() + resource_by_id = find_resources() } Puppet.debug("Searched for resources in %0.2f seconds" % seconds) seconds = Benchmark.realtime { - find_resources_parameters_tags(existing) + find_resources_parameters_tags(resource_by_id) } if id Puppet.debug("Searched for resource params and tags in %0.2f seconds" % seconds) seconds = Benchmark.realtime { - compare_to_catalog(existing, list) + compare_to_catalog(resource_by_id, list) } Puppet.debug("Resource comparison took %0.2f seconds" % seconds) end def find_resources resources.find(:all, :include => :source_file).inject({}) do | hash, resource | - hash[resource.ref] = resource + hash[resource.id] = resource hash end end @@ -143,20 +143,29 @@ class Puppet::Rails::Host < ActiveRecord::Base resource.params_hash = [] end - resources_by_id = resources.inject({}) do |hash, res| - hash[res[1]['id']] = res[1] + find_resources_parameters(resources) + find_resources_tags(resources) + end + + # it seems that it can happen (see bug #2010) some resources are duplicated in the + # database (ie logically corrupted database), in which case we remove the extraneous + # entries. + def compare_to_catalog(existing, list) + extra_db_resources = [] + resources = existing.inject({}) do |hash, res| + resource = res[1] + if hash.include?(resource.ref) + extra_db_resources << hash[resource.ref] + end + hash[resource.ref] = resource hash end - find_resources_parameters(resources_by_id) - find_resources_tags(resources_by_id) - end - - def compare_to_catalog(resources, list) compiled = list.inject({}) do |hash, resource| hash[resource.ref] = resource hash end + ar_hash_merge(resources, compiled, :create => Proc.new { |ref, resource| resource.to_rails(self) @@ -165,6 +174,11 @@ class Puppet::Rails::Host < ActiveRecord::Base }, :modify => Proc.new { |db, mem| mem.modify_rails(db) }) + + # fix-up extraneous resources + extra_db_resources.each do |resource| + self.resources.delete(resource) + end end def find_resources_parameters(resources) @@ -172,7 +186,7 @@ class Puppet::Rails::Host < ActiveRecord::Base # assign each loaded parameters/tags to the resource it belongs to params.each do |param| - resources[param['resource_id']].add_param_to_hash(param) + resources[param['resource_id']].add_param_to_hash(param) if resources.include?(param['resource_id']) end end @@ -180,7 +194,7 @@ class Puppet::Rails::Host < ActiveRecord::Base tags = Puppet::Rails::ResourceTag.find_all_tags_from_host(self) tags.each do |tag| - resources[tag['resource_id']].add_tag_to_hash(tag) + resources[tag['resource_id']].add_tag_to_hash(tag) if resources.include?(tag['resource_id']) end end -- cgit From 1c7c8fe7ae813d4344005c2b353e9a292ecf68c7 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Mon, 2 Mar 2009 20:48:44 +0100 Subject: dbfix - fix typo and close another possible inconsistency This patch fixes a typo in one request producing a SQL error. It also close an issue where param_values could point to inexistant param_names after the deduplications of those. It does this by rewriting the param_values to point to the highest id of the param_names of the same name. The same operation is performed on the tags. Signed-off-by: Brice Figureau --- ext/dbfix.sql | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/ext/dbfix.sql b/ext/dbfix.sql index d22914adf..960369117 100644 --- a/ext/dbfix.sql +++ b/ext/dbfix.sql @@ -35,6 +35,23 @@ FROM param_values AS bad_rows good_rows.resource_id = bad_rows.resource_id AND good_rows.max_id <> bad_rows.id; +-- rewrite param_values that points to duplicated param_names +-- to point to the highest param_name id. +UPDATE + param_values v + INNER JOIN + param_names n + ON n.id = v.param_name_id + INNER JOIN + ( + SELECT name, MAX(id) as max_id + FROM param_names + GROUP BY name + HAVING count(*) > 1 + ) nmax ON n.name = nmax.name +SET + v.param_name_id = nmax.max_id; + -- Remove duplicate param_names, and keep the latest one DELETE bad_rows.* FROM param_names AS bad_rows @@ -48,12 +65,12 @@ FROM param_names AS bad_rows good_rows.name = bad_rows.name AND good_rows.max_id <> bad_rows.id; --- Remove duplicate resource_tags, and keep the latest one +-- Remove duplicate resource_tags, and keep the highest one DELETE bad_rows.* FROM resource_tags AS bad_rows INNER JOIN ( SELECT resource_id,puppet_tag_id, MAX(id) as max_id - FROM param_names + FROM resource_tags GROUP BY resource_id,puppet_tag_id HAVING count(*) > 1 ) AS good_rows @@ -62,7 +79,24 @@ FROM resource_tags AS bad_rows good_rows.puppet_tag_id = bad_rows.puppet_tag_id AND good_rows.max_id <> bad_rows.id; --- Remove duplicate puppet_tags, and keep the latest one +-- rewrite resource_tags that points to duplicated puppet_tags +-- to point to the highest puppet_tags id. +UPDATE + resource_tags v + INNER JOIN + puppet_tags n + ON n.id = v.puppet_tag_id + INNER JOIN + ( + SELECT name, MAX(id) as max_id + FROM puppet_tags + GROUP BY name + HAVING count(*) > 1 + ) nmax ON n.name = nmax.name +SET + v.puppet_tag_id = nmax.max_id; + +-- Remove duplicate puppet_tags, and keep the highest one DELETE bad_rows.* FROM puppet_tags AS bad_rows INNER JOIN ( -- cgit From d5850dc40f20b6ea9429588c476eb0dca2d205b7 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 3 Mar 2009 21:18:37 -0600 Subject: Refactored a method: extracted about five other methods Signed-off-by: Luke Kanies --- lib/puppet/indirector/node/ldap.rb | 89 +++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb index 01010a2af..759db3a5c 100644 --- a/lib/puppet/indirector/node/ldap.rb +++ b/lib/puppet/indirector/node/ldap.rb @@ -81,45 +81,10 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap def entry2hash(entry) result = {} result[:name] = entry.dn.split(',')[0].split("=")[1] - if pattr = parent_attribute - if values = entry.vals(pattr) - if values.length > 1 - raise Puppet::Error, - "Node entry %s specifies more than one parent: %s" % [entry.dn, values.inspect] - end - unless values.empty? - result[:parent] = values.shift - end - end - end - - result[:classes] = [] - class_attributes.each { |attr| - if values = entry.vals(attr) - values.each do |v| result[:classes] << v end - end - } - result[:classes].uniq! - - result[:stacked] = [] - stacked_params = stacked_attributes - stacked_params.each { |attr| - if values = entry.vals(attr) - result[:stacked] = result[:stacked] + values - end - } - - - result[:parameters] = entry.to_hash.inject({}) do |hash, ary| - unless stacked_params.include?(ary[0]) # don't add our stacked parameters to the main param list - if ary[1].length == 1 - hash[ary[0]] = ary[1].shift - else - hash[ary[0]] = ary[1] - end - end - hash - end + result[:parent] = get_parent_from_entry(entry) if parent_attribute + result[:classes] = get_classes_from_entry(entry) + result[:stacked] = get_stacked_values_from_entry(entry) + result[:parameters] = get_parameters_from_entry(entry) result[:environment] = result[:parameters]["environment"] if result[:parameters]["environment"] @@ -224,4 +189,50 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap return info end + + def get_classes_from_entry(entry) + result = class_attributes.inject([]) do |array, attr| + if values = entry.vals(attr) + values.each do |v| array << v end + end + array + end + result.uniq + end + + def get_parameters_from_entry(entry) + stacked_params = stacked_attributes + entry.to_hash.inject({}) do |hash, ary| + unless stacked_params.include?(ary[0]) # don't add our stacked parameters to the main param list + if ary[1].length == 1 + hash[ary[0]] = ary[1].shift + else + hash[ary[0]] = ary[1] + end + end + hash + end + end + + def get_parent_from_entry(entry) + pattr = parent_attribute + + return nil unless values = entry.vals(pattr) + + if values.length > 1 + raise Puppet::Error, + "Node entry %s specifies more than one parent: %s" % [entry.dn, values.inspect] + end + return nil if values.empty? + return values.shift + end + + def get_stacked_values_from_entry(entry) + stacked_attributes.inject([]) do |result, attr| + if values = entry.vals(attr) + result += values + end + result + end + end end -- cgit From 61661b1c46fafeabf1bdbc4778762831d7178d91 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 3 Mar 2009 21:36:56 -0600 Subject: Fixing #1991 - ldap booleans get converted to booleans Signed-off-by: Luke Kanies --- lib/puppet/indirector/node/ldap.rb | 25 +++++++++++++++++++++++++ spec/unit/indirector/node/ldap.rb | 20 ++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb index 759db3a5c..ab5d47b1c 100644 --- a/lib/puppet/indirector/node/ldap.rb +++ b/lib/puppet/indirector/node/ldap.rb @@ -103,6 +103,8 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap end end + result[:parameters] = convert_parameters(result[:parameters]) + result end @@ -143,6 +145,29 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap node.environment = information[:environment] if information[:environment] end + def convert_parameters(parameters) + result = {} + parameters.each do |param, value| + if value.is_a?(Array) + result[param] = value.collect { |v| convert(v) } + else + result[param] = convert(value) + end + end + result + end + + # Convert any values if necessary. + def convert(value) + case value + when Integer, Fixnum, Bignum; value + when "true"; true + when "false"; false + else + value + end + end + # Find information for our parent and merge it into the current info. def find_and_merge_parent(parent, information) unless parent_info = name2hash(parent) diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb index ed8809e73..d407796c6 100755 --- a/spec/unit/indirector/node/ldap.rb +++ b/spec/unit/indirector/node/ldap.rb @@ -91,6 +91,26 @@ describe Puppet::Node::Ldap do @searcher.entry2hash(@entry)[:parameters]["foo"].should == "one" end + it "should convert 'true' values to the boolean 'true'" do + @entry.stubs(:to_hash).returns({"one" => ["true"]}) + @searcher.entry2hash(@entry)[:parameters]["one"].should == true + end + + it "should convert 'false' values to the boolean 'false'" do + @entry.stubs(:to_hash).returns({"one" => ["false"]}) + @searcher.entry2hash(@entry)[:parameters]["one"].should == false + end + + it "should convert 'true' values to the boolean 'true' inside an array" do + @entry.stubs(:to_hash).returns({"one" => ["true", "other"]}) + @searcher.entry2hash(@entry)[:parameters]["one"].should == [true, "other"] + end + + it "should convert 'false' values to the boolean 'false' inside an array" do + @entry.stubs(:to_hash).returns({"one" => ["false", "other"]}) + @searcher.entry2hash(@entry)[:parameters]["one"].should == [false, "other"] + end + it "should add the parent's name if present" do @entry.stubs(:vals).with("parentnode").returns(%w{foo}) @searcher.entry2hash(@entry)[:parent].should == "foo" -- cgit From 9d36b5833ceef954181d5d281aba08d414fcdb65 Mon Sep 17 00:00:00 2001 From: Bryan Kearney Date: Thu, 12 Feb 2009 09:32:01 -0500 Subject: Bug 1948: Added patch by jab to support the correct ins syntax. Updated the test cases as well --- lib/puppet/provider/augeas/augeas.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index 8d4f6d55a..f70654438 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -178,14 +178,12 @@ Puppet::Type.type(:augeas).provide(:augeas) do debug("sending command '#{command}' with params #{cmd_array.inspect}") aug.clear(cmd_array[0]) when "insert", "ins" - - ext_array = cmd_array[1].split(" ") ; - if cmd_array.size < 2 or ext_array.size < 2 + if cmd_array.size < 3 fail("ins requires 3 parameters") end label = cmd_array[0] - where = ext_array[0] - path = File.join(context, ext_array[1]) + where = cmd_array[1] + path = File.join(context, cmd_array[2]) case where when "before": before = true when "after": before = false -- cgit From cf48ec0aba120e6e83e48b3499df9029b5302767 Mon Sep 17 00:00:00 2001 From: Bryan Kearney Date: Wed, 18 Feb 2009 12:34:17 -0500 Subject: First cut at the not running if augeas does not change any of the underlieing files --- lib/puppet/provider/augeas/augeas.rb | 101 ++++++++++++++++++++++++++--------- spec/unit/provider/augeas/augeas.rb | 25 +++++---- 2 files changed, 92 insertions(+), 34 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index f70654438..5fa5ab409 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -20,13 +20,17 @@ require 'augeas' if Puppet.features.augeas? Puppet::Type.type(:augeas).provide(:augeas) do -#class Puppet::Provider::Augeas < Puppet::Provider include Puppet::Util confine :true => Puppet.features.augeas? has_features :parse_commands, :need_to_run?,:execute_changes + SAVE_NOOP = "noop" + SAVE_OVERWRITE = "overwrite" + + attr_accessor :aug + # Extracts an 2 dimensional array of commands which are in the # form of command path value. # The input can be @@ -56,12 +60,19 @@ Puppet::Type.type(:augeas).provide(:augeas) do end def open_augeas - flags = 0 - (flags = 1 << 2 ) if self.resource[:type_check] == :true - root = self.resource[:root] - load_path = self.resource[:load_path] - debug("Opening augeas with root #{root}, lens path #{load_path}, flags #{flags}") - Augeas.open(root, load_path,flags) + if (@aug.nil?) + flags = 0 + (flags = 1 << 2 ) if self.resource[:type_check] == :true + root = self.resource[:root] + load_path = self.resource[:load_path] + debug("Opening augeas with root #{root}, lens path #{load_path}, flags #{flags}") + @aug = Augeas.open(root, load_path,flags) + + if (self.get_augeas_version() >= "0.3.6") + debug("Augeas version #{self.get_augeas_version()} is installed") + end + end + @aug end # Used by the need_to_run? method to process get filters. Returns @@ -78,8 +89,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do arg = cmd_array.join(" ") #check the value in augeas - aug = open_augeas() - result = aug.get(path) || '' + result = @aug.get(path) || '' unless result.nil? case comparator when "!=": @@ -107,8 +117,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do verb = cmd_array.shift() #Get the values from augeas - aug = open_augeas() - result = aug.match(path) || '' + result = @aug.match(path) || '' # Now do the work if (!result.nil?) case verb @@ -131,10 +140,24 @@ Puppet::Type.type(:augeas).provide(:augeas) do end end return_value - end + end + + def get_augeas_version + return @aug.get("/augeas/version") || "" + end + + def set_augeas_save_mode(mode) + return @aug.set("/augeas/save", mode) + end + + def files_changed? + saved_files = @aug.match("/augeas/events/saved") + return saved_files.size() > 0 + end # Determines if augeas acutally needs to run. def need_to_run? + self.open_augeas() return_value = true filter = resource[:onlyif] unless (filter == "") @@ -151,12 +174,46 @@ Puppet::Type.type(:augeas).provide(:augeas) do fail("Error sending command '#{command}' with params #{cmd_array[1..-1].inspect}/#{e.message}") end end - return_value + + # If we have a verison of augeas which is at least 0.3.6 then we + # can make the changes now, see if changes were made, and + # actually do the save. + if ((return_value) and (self.get_augeas_version() >= "0.3.6")) + debug("Will attempt to save and only run if files changed") + self.set_augeas_save_mode(SAVE_NOOP) + self.do_execute_changes() + save_result = @aug.save() + saved_files = @aug.match("/augeas/events/saved") + if ((save_result) and (not files_changed?)) + debug("Skipping becuase no files were changed") + return_value = false + else + debug("Files changed, should execute") + end + end + + return return_value end + + def execute_changes + # if we have version 0.3.6 or greater we have already executed + # the changes. We just need to save them. If not, do the changes + if (self.get_augeas_version() >= "0.3.6") + self.set_augeas_save_mode(SAVE_OVERWRITE) + else + self.do_execute_changes() + end + + success = @aug.save() + if (success != true) + fail("Save failed with return code #{success}") + end + + return :executed + end # Actually execute the augeas changes. - def execute_changes - aug = open_augeas + def do_execute_changes commands = resource[:changes] context = resource[:context] commands.each do |cmd_array| @@ -168,15 +225,15 @@ Puppet::Type.type(:augeas).provide(:augeas) do when "set": cmd_array[0]=File.join(context, cmd_array[0]) debug("sending command '#{command}' with params #{cmd_array.inspect}") - aug.set(cmd_array[0], cmd_array[1]) + @aug.set(cmd_array[0], cmd_array[1]) when "rm", "remove": cmd_array[0]=File.join(context, cmd_array[0]) debug("sending command '#{command}' with params #{cmd_array.inspect}") - aug.rm(cmd_array[0]) + @aug.rm(cmd_array[0]) when "clear": cmd_array[0]=File.join(context, cmd_array[0]) debug("sending command '#{command}' with params #{cmd_array.inspect}") - aug.clear(cmd_array[0]) + @aug.clear(cmd_array[0]) when "insert", "ins" if cmd_array.size < 3 fail("ins requires 3 parameters") @@ -190,19 +247,13 @@ Puppet::Type.type(:augeas).provide(:augeas) do else fail("Invalid value '#{where}' for where param") end debug("sending command '#{command}' with params #{[label, where, path].inspect()}") - aug.insert(path, label, before) + @aug.insert(path, label, before) else fail("Command '#{command}' is not supported") end rescue Exception => e fail("Error sending command '#{command}' with params #{cmd_array.inspect}/#{e.message}") end end - success = aug.save() - if (success != true) - fail("Save failed with return code #{success}") - end - - return :executed end end diff --git a/spec/unit/provider/augeas/augeas.rb b/spec/unit/provider/augeas/augeas.rb index 2def0d0c4..3ce64576e 100644 --- a/spec/unit/provider/augeas/augeas.rb +++ b/spec/unit/provider/augeas/augeas.rb @@ -60,7 +60,7 @@ describe provider_class do before do augeas_stub = stub("augeas", :get => "value") @provider = provider_class.new() - @provider.stubs(:open_augeas).returns(augeas_stub) + @provider.aug= augeas_stub end it "should return false for a = nonmatch" do @@ -88,7 +88,7 @@ describe provider_class do before do augeas_stub = stub("augeas", :match => ["set", "of", "values"]) @provider = provider_class.new() - @provider.stubs(:open_augeas).returns(augeas_stub) + @provider.aug= augeas_stub end it "should return true for size match" do @@ -122,10 +122,12 @@ describe provider_class do end end - describe "need_to_run" do + describe "legacy need to run" do it "should handle no filters" do resource = stub("resource", :[] => "") + augeas_stub = stub("augeas", :match => ["set", "of", "values"]) provider = provider_class.new(resource) + provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == true end @@ -133,7 +135,8 @@ describe provider_class do resource = stub("resource", :[] => "get path == value") provider = provider_class.new(resource) augeas_stub = stub("augeas", :get => "value") - provider.stubs(:open_augeas).returns(augeas_stub) + provider.aug= augeas_stub + provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == true end @@ -141,7 +144,8 @@ describe provider_class do resource = stub("resource", :[] => "get path == another value") provider = provider_class.new(resource) augeas_stub = stub("augeas", :get => "value") - provider.stubs(:open_augeas).returns(augeas_stub) + provider.aug= augeas_stub + provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == false end @@ -149,7 +153,8 @@ describe provider_class do resource = stub("resource", :[] => "match path size == 3") provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) - provider.stubs(:open_augeas).returns(augeas_stub) + provider.aug= augeas_stub + provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == true end @@ -157,18 +162,20 @@ describe provider_class do resource = stub("resource", :[] => "match path size == 2") provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) - provider.stubs(:open_augeas).returns(augeas_stub) + provider.aug= augeas_stub + provider.stubs(:get_augeas_version).returns("0.3.5") provider.need_to_run?.should == false end end - describe "augeas integration" do + describe "legacy augeas integration" do before do @resource = stub("resource") @provider = provider_class.new(@resource) @augeas = stub("augeas") - @provider.stubs(:open_augeas).returns(@augeas) + @provider.aug= @augeas + @provider.stubs(:get_augeas_version).returns("0.3.5") end it "should handle set commands" do -- cgit From cedeb7982b051e00173822c8d14a794e4fb10ae7 Mon Sep 17 00:00:00 2001 From: Bryan Kearney Date: Wed, 18 Feb 2009 12:46:17 -0500 Subject: Backport the fix for #1835 --- lib/puppet/provider/augeas/augeas.rb | 26 ++++++++++-- spec/unit/provider/augeas/augeas.rb | 80 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index 5fa5ab409..e9471b1be 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -42,10 +42,27 @@ Puppet::Type.type(:augeas).provide(:augeas) do if data.is_a?(String) data.each_line do |line| cmd_array = Array.new() - tokens = line.split(" ") - cmd = tokens.shift() - file = tokens.shift() - other = tokens.join(" ") + single = line.index("'") + double = line.index('"') + tokens = nil + delim = " " + if ((single != nil) or (double != nil)) + single = 99999 if single == nil + double = 99999 if double == nil + delim = '"' if double < single + delim = "'" if single < double + end + tokens = line.split(delim) + # If the length of tokens is 2, thn that means the pattern was + # command file "some text", therefore we need to re-split + # the first line + if tokens.length == 2 + tokens = (tokens[0].split(" ")) << tokens[1] + end + cmd = tokens.shift().strip() + delim = "" if delim == " " + file = tokens.shift().strip() + other = tokens.join(" ").strip() cmd_array << cmd if !cmd.nil? cmd_array << file if !file.nil? cmd_array << other if other != "" @@ -59,6 +76,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do return commands end + def open_augeas if (@aug.nil?) flags = 0 diff --git a/spec/unit/provider/augeas/augeas.rb b/spec/unit/provider/augeas/augeas.rb index 3ce64576e..e05812d78 100644 --- a/spec/unit/provider/augeas/augeas.rb +++ b/spec/unit/provider/augeas/augeas.rb @@ -54,6 +54,86 @@ describe provider_class do tokens[0][1].should == "/Jar/Jar" tokens[0][2].should == "Binks is my copilot" end + + it "should accept spaces and and single ticks" do + provider = provider_class.new() + tokens = provider.parse_commands("set 'Jar Jar' Binks") + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == "Jar Jar" + tokens[0][2].should == "Binks" + end + + it "should accept spaces in the value and and single ticks" do + provider = provider_class.new() + tokens = provider.parse_commands("set 'Jar Jar' 'Binks is my copilot'") + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == "Jar Jar" + tokens[0][2].should == "Binks is my copilot" + end + + it "should accept spaces and and double ticks" do + provider = provider_class.new() + tokens = provider.parse_commands('set "Jar Jar" Binks') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == 'Jar Jar' + tokens[0][2].should == 'Binks' + end + + it "should accept spaces in the value and and double ticks" do + provider = provider_class.new() + tokens = provider.parse_commands('set "Jar Jar" "Binks is my copilot"') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == 'Jar Jar' + tokens[0][2].should == 'Binks is my copilot' + end + + it "should accept mixed ticks" do + provider = provider_class.new() + tokens = provider.parse_commands('set "Jar Jar" "Some \'Test\'"') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == 'Jar Jar' + tokens[0][2].should == "Some \'Test\'" + end + + it "should accept only the last value using ticks" do + provider = provider_class.new() + tokens = provider.parse_commands('set /Jar/Jar "Binks is my copilot"') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == '/Jar/Jar' + tokens[0][2].should == "Binks is my copilot" + end + + it "should accept only the first value using ticks" do + provider = provider_class.new() + tokens = provider.parse_commands('set "Jar Jar" copilot') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == 'Jar Jar' + tokens[0][2].should == "copilot" + end + + it "should accept only the first value using ticks and the last values being concatenated" do + provider = provider_class.new() + tokens = provider.parse_commands('set "Jar Jar" Binks is my copilot') + tokens.size.should == 1 + tokens[0].size.should == 3 + tokens[0][0].should == "set" + tokens[0][1].should == 'Jar Jar' + tokens[0][2].should == "Binks is my copilot" + end end describe "get filters" do -- cgit From 01bc88c7e2a3d5a71aec8f0727631cbf41680720 Mon Sep 17 00:00:00 2001 From: Bryan Kearney Date: Mon, 23 Feb 2009 11:21:59 -0500 Subject: Added a force option to ensure the change is always applied, and call augeas twice to reduce the chance that data is lost --- lib/puppet/provider/augeas/augeas.rb | 102 +++++++++-------- lib/puppet/type/augeas.rb | 29 +++-- spec/unit/provider/augeas/augeas.rb | 212 ++++++++++++++++++++--------------- 3 files changed, 197 insertions(+), 146 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index e9471b1be..56e217a54 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -21,9 +21,9 @@ require 'augeas' if Puppet.features.augeas? Puppet::Type.type(:augeas).provide(:augeas) do include Puppet::Util - - confine :true => Puppet.features.augeas? - + + confine :true => Puppet.features.augeas? + has_features :parse_commands, :need_to_run?,:execute_changes SAVE_NOOP = "noop" @@ -85,14 +85,22 @@ Puppet::Type.type(:augeas).provide(:augeas) do load_path = self.resource[:load_path] debug("Opening augeas with root #{root}, lens path #{load_path}, flags #{flags}") @aug = Augeas.open(root, load_path,flags) - + if (self.get_augeas_version() >= "0.3.6") debug("Augeas version #{self.get_augeas_version()} is installed") end end @aug end - + + def close_augeas + if (!@aug.nil?) + @aug.close() + debug("Closed the augeas connection") + @aug = nil; + end + end + # Used by the need_to_run? method to process get filters. Returns # true if there is a match, false if otherwise # Assumes a syntax of get /files/path [COMPARATOR] value @@ -121,8 +129,8 @@ Puppet::Type.type(:augeas).provide(:augeas) do end end return_value - end - + end + # Used by the need_to_run? method to process match filters. Returns # true if there is a match, false if otherwise def process_match(cmd_array) @@ -137,7 +145,7 @@ Puppet::Type.type(:augeas).provide(:augeas) do #Get the values from augeas result = @aug.match(path) || '' # Now do the work - if (!result.nil?) + unless (result.nil?) case verb when "size": fail("Invalid command: #{cmd_array.join(" ")}") if cmd_array.length != 2 @@ -170,13 +178,14 @@ Puppet::Type.type(:augeas).provide(:augeas) do def files_changed? saved_files = @aug.match("/augeas/events/saved") - return saved_files.size() > 0 + return saved_files.size() > 0 end - + # Determines if augeas acutally needs to run. def need_to_run? - self.open_augeas() + force = resource[:force] return_value = true + self.open_augeas() filter = resource[:onlyif] unless (filter == "") cmd_array = filter.split @@ -193,48 +202,52 @@ Puppet::Type.type(:augeas).provide(:augeas) do end end - # If we have a verison of augeas which is at least 0.3.6 then we - # can make the changes now, see if changes were made, and - # actually do the save. - if ((return_value) and (self.get_augeas_version() >= "0.3.6")) - debug("Will attempt to save and only run if files changed") - self.set_augeas_save_mode(SAVE_NOOP) - self.do_execute_changes() - save_result = @aug.save() - saved_files = @aug.match("/augeas/events/saved") - if ((save_result) and (not files_changed?)) - debug("Skipping becuase no files were changed") - return_value = false - else - debug("Files changed, should execute") + unless (force) + # If we have a verison of augeas which is at least 0.3.6 then we + # can make the changes now, see if changes were made, and + # actually do the save. + if ((return_value) and (self.get_augeas_version() >= "0.3.6")) + debug("Will attempt to save and only run if files changed") + self.set_augeas_save_mode(SAVE_NOOP) + self.do_execute_changes() + save_result = @aug.save() + saved_files = @aug.match("/augeas/events/saved") + if ((save_result) and (not files_changed?)) + debug("Skipping becuase no files were changed") + return_value = false + else + debug("Files changed, should execute") + end end end - + self.close_augeas() return return_value - end + end def execute_changes - # if we have version 0.3.6 or greater we have already executed - # the changes. We just need to save them. If not, do the changes + # Re-connect to augeas, and re-execute the changes + self.open_augeas() if (self.get_augeas_version() >= "0.3.6") self.set_augeas_save_mode(SAVE_OVERWRITE) - else - self.do_execute_changes() end - + + self.do_execute_changes() + success = @aug.save() if (success != true) fail("Save failed with return code #{success}") end + self.close_augeas() - return :executed + return :executed end - + # Actually execute the augeas changes. def do_execute_changes - commands = resource[:changes] + commands = resource[:changes].clone() context = resource[:context] commands.each do |cmd_array| + cmd_array = cmd_array.clone() fail("invalid command #{cmd_array.join[" "]}") if cmd_array.length < 2 command = cmd_array[0] cmd_array.shift() @@ -246,32 +259,33 @@ Puppet::Type.type(:augeas).provide(:augeas) do @aug.set(cmd_array[0], cmd_array[1]) when "rm", "remove": cmd_array[0]=File.join(context, cmd_array[0]) - debug("sending command '#{command}' with params #{cmd_array.inspect}") + debug("sending command '#{command}' with params #{cmd_array.inspect}") @aug.rm(cmd_array[0]) when "clear": cmd_array[0]=File.join(context, cmd_array[0]) - debug("sending command '#{command}' with params #{cmd_array.inspect}") + debug("sending command '#{command}' with params #{cmd_array.inspect}") @aug.clear(cmd_array[0]) when "insert", "ins" - if cmd_array.size < 3 + ext_array = cmd_array[1].split(" ") ; + if cmd_array.size < 2 or ext_array.size < 2 fail("ins requires 3 parameters") end label = cmd_array[0] - where = cmd_array[1] - path = File.join(context, cmd_array[2]) + where = ext_array[0] + path = File.join(context, ext_array[1]) case where when "before": before = true when "after": before = false else fail("Invalid value '#{where}' for where param") end - debug("sending command '#{command}' with params #{[label, where, path].inspect()}") - @aug.insert(path, label, before) + debug("sending command '#{command}' with params #{[label, where, path].inspect()}") + aug.insert(path, label, before) else fail("Command '#{command}' is not supported") end rescue Exception => e fail("Error sending command '#{command}' with params #{cmd_array.inspect}/#{e.message}") end end - end - + end + end diff --git a/lib/puppet/type/augeas.rb b/lib/puppet/type/augeas.rb index c89400b5e..e0d5e10b7 100644 --- a/lib/puppet/type/augeas.rb +++ b/lib/puppet/type/augeas.rb @@ -19,10 +19,10 @@ Puppet::Type.newtype(:augeas) do include Puppet::Util - + feature :parse_commands, "Parse the command string" feature :need_to_run?, "If the command should run" - feature :execute_changes, "Actually make the changes" + feature :execute_changes, "Actually make the changes" @doc = "Apply the changes (single or array of changes) to the filesystem via the augeas tool. @@ -59,7 +59,7 @@ Puppet::Type.newtype(:augeas) do newparam (:context) do desc "Optional context path. This value is pre-pended to the paths of all changes" - defaultto "" + defaultto "" end newparam (:onlyif) do @@ -81,7 +81,7 @@ Puppet::Type.newtype(:augeas) do AN_ARRAY is in the form ['a string', 'another']" defaultto "" end - + newparam(:changes) do desc "The changes which should be applied to the filesystem. This @@ -99,7 +99,7 @@ Puppet::Type.newtype(:augeas) do If the parameter 'context' is set that value is prepended to PATH" - munge do |value| + munge do |value| provider.parse_commands(value) end end @@ -115,6 +115,13 @@ Puppet::Type.newtype(:augeas) do defaultto "" end + newparam(:force) do + desc "Optional command to force the augeas type to execute even if it thinks changes + will not be made. This does not overide the only setting. If onlyif is set, then the + foce setting will not override that result" + + defaultto false + end newparam(:type_check) do desc "Set to true if augeas should perform typechecking. Optional, defaults to false" @@ -122,7 +129,7 @@ Puppet::Type.newtype(:augeas) do defaultto :false end - + # This is the acutal meat of the code. It forces # augeas to be run and fails or not based on the augeas return # code. @@ -131,12 +138,12 @@ Puppet::Type.newtype(:augeas) do desc "The expected return code from the augeas command. Should not be set" defaultto 0 - + # Make output a bit prettier def change_to_s(currentvalue, newvalue) return "executed successfully" - end - + end + # if the onlyif resource is provided, then the value is parsed. # a return value of 0 will stop exection becuase it matches the # default value. @@ -146,12 +153,12 @@ Puppet::Type.newtype(:augeas) do else 0 end - end + end # Actually execute the command. def sync @resource.provider.execute_changes() end - end + end end diff --git a/spec/unit/provider/augeas/augeas.rb b/spec/unit/provider/augeas/augeas.rb index e05812d78..284145657 100644 --- a/spec/unit/provider/augeas/augeas.rb +++ b/spec/unit/provider/augeas/augeas.rb @@ -10,49 +10,49 @@ describe provider_class do provider = provider_class.new() tokens = provider.parse_commands("set /Jar/Jar Binks") tokens.size.should == 1 - tokens[0].size.should == 3 + tokens[0].size.should == 3 tokens[0][0].should == "set" tokens[0][1].should == "/Jar/Jar" - tokens[0][2].should == "Binks" + tokens[0][2].should == "Binks" end - + it "should break apart a multiple line into six tokens" do provider = provider_class.new() tokens = provider.parse_commands("set /Jar/Jar Binks\nrm anakin skywalker") tokens.size.should == 2 - tokens[0].size.should == 3 - tokens[1].size.should == 3 + tokens[0].size.should == 3 + tokens[1].size.should == 3 tokens[0][0].should == "set" tokens[0][1].should == "/Jar/Jar" - tokens[0][2].should == "Binks" + tokens[0][2].should == "Binks" tokens[1][0].should == "rm" tokens[1][1].should == "anakin" - tokens[1][2].should == "skywalker" - end - + tokens[1][2].should == "skywalker" + end + it "should handle arrays" do provider = provider_class.new() commands = ["set /Jar/Jar Binks", "rm anakin skywalker"] tokens = provider.parse_commands(commands) tokens.size.should == 2 - tokens[0].size.should == 3 - tokens[1].size.should == 3 + tokens[0].size.should == 3 + tokens[1].size.should == 3 tokens[0][0].should == "set" tokens[0][1].should == "/Jar/Jar" - tokens[0][2].should == "Binks" + tokens[0][2].should == "Binks" tokens[1][0].should == "rm" tokens[1][1].should == "anakin" - tokens[1][2].should == "skywalker" - end - + tokens[1][2].should == "skywalker" + end + it "should concat the last values" do provider = provider_class.new() tokens = provider.parse_commands("set /Jar/Jar Binks is my copilot") tokens.size.should == 1 - tokens[0].size.should == 3 + tokens[0].size.should == 3 tokens[0][0].should == "set" tokens[0][1].should == "/Jar/Jar" - tokens[0][2].should == "Binks is my copilot" + tokens[0][2].should == "Binks is my copilot" end it "should accept spaces and and single ticks" do @@ -64,7 +64,7 @@ describe provider_class do tokens[0][1].should == "Jar Jar" tokens[0][2].should == "Binks" end - + it "should accept spaces in the value and and single ticks" do provider = provider_class.new() tokens = provider.parse_commands("set 'Jar Jar' 'Binks is my copilot'") @@ -74,7 +74,7 @@ describe provider_class do tokens[0][1].should == "Jar Jar" tokens[0][2].should == "Binks is my copilot" end - + it "should accept spaces and and double ticks" do provider = provider_class.new() tokens = provider.parse_commands('set "Jar Jar" Binks') @@ -84,7 +84,7 @@ describe provider_class do tokens[0][1].should == 'Jar Jar' tokens[0][2].should == 'Binks' end - + it "should accept spaces in the value and and double ticks" do provider = provider_class.new() tokens = provider.parse_commands('set "Jar Jar" "Binks is my copilot"') @@ -94,7 +94,7 @@ describe provider_class do tokens[0][1].should == 'Jar Jar' tokens[0][2].should == 'Binks is my copilot' end - + it "should accept mixed ticks" do provider = provider_class.new() tokens = provider.parse_commands('set "Jar Jar" "Some \'Test\'"') @@ -104,7 +104,7 @@ describe provider_class do tokens[0][1].should == 'Jar Jar' tokens[0][2].should == "Some \'Test\'" end - + it "should accept only the last value using ticks" do provider = provider_class.new() tokens = provider.parse_commands('set /Jar/Jar "Binks is my copilot"') @@ -114,7 +114,7 @@ describe provider_class do tokens[0][1].should == '/Jar/Jar' tokens[0][2].should == "Binks is my copilot" end - + it "should accept only the first value using ticks" do provider = provider_class.new() tokens = provider.parse_commands('set "Jar Jar" copilot') @@ -124,7 +124,7 @@ describe provider_class do tokens[0][1].should == 'Jar Jar' tokens[0][2].should == "copilot" end - + it "should accept only the first value using ticks and the last values being concatenated" do provider = provider_class.new() tokens = provider.parse_commands('set "Jar Jar" Binks is my copilot') @@ -133,167 +133,193 @@ describe provider_class do tokens[0][0].should == "set" tokens[0][1].should == 'Jar Jar' tokens[0][2].should == "Binks is my copilot" - end + end end - + describe "get filters" do before do augeas_stub = stub("augeas", :get => "value") - @provider = provider_class.new() - @provider.aug= augeas_stub + @provider = provider_class.new() + @provider.aug= augeas_stub end - + it "should return false for a = nonmatch" do command = ["get", "fake value", "==", "value"] @provider.process_get(command).should == true end - + it "should return true for a != match" do command = ["get", "fake value", "!=", "value"] @provider.process_get(command).should == false - end - + end + it "should return true for a =~ match" do command = ["get", "fake value", "=~", "val*"] @provider.process_get(command).should == true - end - + end + it "should return false for a == nonmatch" do command = ["get", "fake value", "=~", "num*"] @provider.process_get(command).should == false - end + end end - + describe "match filters" do before do augeas_stub = stub("augeas", :match => ["set", "of", "values"]) - @provider = provider_class.new() - @provider.aug= augeas_stub + @provider = provider_class.new() + @provider.aug= augeas_stub end - + it "should return true for size match" do command = ["match", "fake value", "size", "==", "3"] @provider.process_match(command).should == true end - + it "should return false for a size non match" do command = ["match", "fake value", "size", "<", "3"] @provider.process_match(command).should == false - end - + end + it "should return true for includes match" do command = ["get", "fake value", "include", "values"] @provider.process_match(command).should == true - end - + end + it "should return false for includes non match" do command = ["get", "fake value", "include", "JarJar"] @provider.process_match(command).should == false - end - + end + it "should return true for an array match" do command = ["get", "fake value", "==", "['set', 'of', 'values']"] @provider.process_match(command).should == true - end - + end + it "should return false for an array non match" do command = ["get", "fake value", "==", "['this', 'should', 'not', 'match']"] @provider.process_match(command).should == false - end - end - - describe "legacy need to run" do + end + end + + describe "need to run" do it "should handle no filters" do - resource = stub("resource", :[] => "") - augeas_stub = stub("augeas", :match => ["set", "of", "values"]) + resource = stub("resource") + resource.stubs(:[]).returns(false).then.returns("") + augeas_stub = stub("augeas", :match => ["set", "of", "values"]) + augeas_stub.stubs("close") provider = provider_class.new(resource) provider.stubs(:get_augeas_version).returns("0.3.5") - provider.need_to_run?.should == true + provider.need_to_run?.should == true end - + it "should return true when a get filter matches" do - resource = stub("resource", :[] => "get path == value") + resource = stub("resource") + resource.stubs(:[]).returns(false).then.returns("get path == value") provider = provider_class.new(resource) augeas_stub = stub("augeas", :get => "value") + augeas_stub.stubs("close") provider.aug= augeas_stub - provider.stubs(:get_augeas_version).returns("0.3.5") - provider.need_to_run?.should == true - end - + provider.stubs(:get_augeas_version).returns("0.3.5") + provider.need_to_run?.should == true + end + it "should return false when a get filter does not match" do - resource = stub("resource", :[] => "get path == another value") + resource = stub("resource") + resource.stubs(:[]).returns(false).then.returns("get path == another value") provider = provider_class.new(resource) augeas_stub = stub("augeas", :get => "value") + augeas_stub.stubs("close") provider.aug= augeas_stub - provider.stubs(:get_augeas_version).returns("0.3.5") - provider.need_to_run?.should == false - end - + provider.stubs(:get_augeas_version).returns("0.3.5") + provider.need_to_run?.should == false + end + it "should return true when a match filter matches" do - resource = stub("resource", :[] => "match path size == 3") + resource = stub("resource") + resource.stubs(:[]).returns(false).then.returns("match path size == 3") provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) + augeas_stub.stubs("close") provider.aug= augeas_stub - provider.stubs(:get_augeas_version).returns("0.3.5") - provider.need_to_run?.should == true - end - + provider.stubs(:get_augeas_version).returns("0.3.5") + provider.need_to_run?.should == true + end + it "should return false when a match filter does not match" do - resource = stub("resource", :[] => "match path size == 2") + resource = stub("resource") + resource.stubs(:[]).returns(false).then.returns("match path size == 2") + provider = provider_class.new(resource) + augeas_stub = stub("augeas", :match => ["set", "of", "values"]) + augeas_stub.stubs("close") + provider.aug= augeas_stub + provider.stubs(:get_augeas_version).returns("0.3.5") + provider.need_to_run?.should == false + end + + #This is a copy of the last one, with setting the force to true + it "setting force should not change the above logic" do + resource = stub("resource") + resource.stubs(:[]).returns(true).then.returns("match path size == 2") provider = provider_class.new(resource) augeas_stub = stub("augeas", :match => ["set", "of", "values"]) + augeas_stub.stubs("close") provider.aug= augeas_stub - provider.stubs(:get_augeas_version).returns("0.3.5") - provider.need_to_run?.should == false - end + provider.stubs(:get_augeas_version).returns("0.3.5") + provider.need_to_run?.should == false + end end - - describe "legacy augeas integration" do - + + describe "augeas execution integration" do + before do @resource = stub("resource") @provider = provider_class.new(@resource) @augeas = stub("augeas") @provider.aug= @augeas - @provider.stubs(:get_augeas_version).returns("0.3.5") + @provider.stubs(:get_augeas_version).returns("0.3.5") end - + it "should handle set commands" do command = [["set", "/Jar/Jar", "Binks"]] context = "/some/path" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:set).with("/some/path/Jar/Jar", "Binks") @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed end - + it "should handle rm commands" do command = [["rm", "/Jar/Jar"]] context = "" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:rm).with("/Jar/Jar") @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed - end - + end + it "should handle remove commands" do command = [["remove", "Jar/Jar"]] context = "" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:rm).with("/Jar/Jar") @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed - end - + end + it "should handle clear commands" do command = [["clear", "/Jar/Jar"]] context = "/foo" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:clear).with("/foo/Jar/Jar") @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed - end - + end + it "should handle ins commands with before" do command = [["ins", "Binks", "before /Jar/Jar"]] @@ -301,6 +327,7 @@ describe provider_class do @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:insert).with("/foo/Jar/Jar", "Binks", true) @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed end @@ -310,6 +337,7 @@ describe provider_class do @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:insert).with("/foo/Jar/Jar", "Binks", false) @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed end @@ -319,17 +347,19 @@ describe provider_class do @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:insert).with("/Jar/Jar", "Binks", false) @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed - end - + end + it "should handle multiple commands" do command = [["ins", "Binks", "after /Jar/Jar"], ["clear", "/Jar/Jar"]] context = "/foo" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:insert).with("/foo/Jar/Jar", "Binks", false) - @augeas.expects(:clear).with("/foo/Jar/Jar") + @augeas.expects(:clear).with("/foo/Jar/Jar") @augeas.expects(:save).returns(true) + @augeas.expects(:close) @provider.execute_changes.should == :executed - end + end end end -- cgit From cf64827970d8725cea93fee57ec03ea70d63a142 Mon Sep 17 00:00:00 2001 From: Bryan Kearney Date: Fri, 27 Feb 2009 10:36:38 -0500 Subject: Bring in the documentation changes from the master branch --- lib/puppet/type/augeas.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/puppet/type/augeas.rb b/lib/puppet/type/augeas.rb index e0d5e10b7..da9cff369 100644 --- a/lib/puppet/type/augeas.rb +++ b/lib/puppet/type/augeas.rb @@ -92,11 +92,10 @@ Puppet::Type.newtype(:augeas) do rm [PATH] Removes the node at location PATH remove [PATH] Synonym for rm clear [PATH] Keeps the node at PATH, but removes the value. - ins [LABEL] [WHERE] [PATH] + ins [LABEL] [WHERE] [PATH] Inserts an empty node LABEL either [WHERE={before|after}] PATH. - insert [LABEL] [WHERE] [PATH] + insert [LABEL] [WHERE] [PATH] Synonym for ins - If the parameter 'context' is set that value is prepended to PATH" munge do |value| -- cgit From 67fc394d9ffaed89328c00678559f57418a1511d Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Sat, 7 Mar 2009 11:14:14 +1100 Subject: Fixed #2026 - Red Hat ignoring stop method --- CHANGELOG | 2 ++ lib/puppet/provider/service/redhat.rb | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 70caf4510..42bfe71fc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #2026 - Red Hat ignoring stop method + Added ext/dbfix.sql script - fixes common errors in stored configuration databases Fixed #1963 - Failing to read /proc/mounts for selinux kills file downloads diff --git a/lib/puppet/provider/service/redhat.rb b/lib/puppet/provider/service/redhat.rb index 031db46c1..f31903e84 100755 --- a/lib/puppet/provider/service/redhat.rb +++ b/lib/puppet/provider/service/redhat.rb @@ -73,12 +73,12 @@ Puppet::Type.type(:service).provide :redhat, :parent => :init do end end - def start - service(@resource[:name], "start") + def startcmd + [command(:service), @resource[:name], "start"] end - def stop - service(@resource[:name], "stop") + def stopcmd + [command(:service), @resource[:name], "stop"] end end -- cgit From a3bb201bd4c964ab4f68e00895b692d9d9585407 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 6 Mar 2009 18:11:33 -0600 Subject: Fixing change printing when list properties are absent They were throwing an exception when the 'is' value was 'absent'. Signed-off-by: Luke Kanies --- lib/puppet/property/list.rb | 6 +++++- spec/unit/property/list.rb | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb index 0c933f164..b7db8b42a 100644 --- a/lib/puppet/property/list.rb +++ b/lib/puppet/property/list.rb @@ -10,7 +10,11 @@ module Puppet end def is_to_s(currentvalue) - currentvalue.join(delimiter) + if currentvalue == :absent + return "absent" + else + return currentvalue.join(delimiter) + end end def membership diff --git a/spec/unit/property/list.rb b/spec/unit/property/list.rb index 2fab868db..854ab4874 100644 --- a/spec/unit/property/list.rb +++ b/spec/unit/property/list.rb @@ -36,6 +36,10 @@ describe list_class do @property.is_to_s(["foo","bar"]).should == "foo,bar" end + it "should be able to correctly convert ':absent' to a string" do + @property.is_to_s(:absent).should == "absent" + end + describe "when adding should to current" do it "should add the arrays when current is an array" do @property.add_should_with_current(["foo"], ["bar"]).should == ["foo", "bar"] -- cgit From 84d6637cfc47a14114493254ec8e68f2887a93d8 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Mon, 9 Mar 2009 14:46:46 +1100 Subject: Fixed #2000 - No default specified for checksum --- CHANGELOG | 2 ++ lib/puppet/type/file/checksum.rb | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 42bfe71fc..4d5b05f40 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #2000 - No default specified for checksum + Fixed #2026 - Red Hat ignoring stop method Added ext/dbfix.sql script - fixes common errors in stored configuration databases diff --git a/lib/puppet/type/file/checksum.rb b/lib/puppet/type/file/checksum.rb index f681a8ebe..095c62f37 100755 --- a/lib/puppet/type/file/checksum.rb +++ b/lib/puppet/type/file/checksum.rb @@ -11,7 +11,13 @@ Puppet::Type.type(:file).newproperty(:checksum) do like Tripwire without managing the file contents in any way. You can specify that a file's checksum should be monitored and then subscribe to the file from another object and receive events to signify - checksum changes, for instance." + checksum changes, for instance. + + There are a number of checksum types available including MD5 hashing (and + an md5lite variation that only hashes the first 500 characters of the + file. + + The default checksum parameter, if checksums are enabled, is md5." @event = :file_changed -- cgit From 73a0757b559d7e4fbd1da43bbcf4e60fd9155896 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 19 Dec 2008 15:05:29 +0100 Subject: Fix #1828 - Scope.number? wasn't strict enough and could produce wrong results Some invalid numbers were treated as numbers and conversion to Integer was failing returning 0 (for instance 0.24.7). Signed-off-by: Brice Figureau --- lib/puppet/parser/scope.rb | 6 +++--- spec/unit/parser/scope.rb | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index 4acdf41c9..77e7b0cfd 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -52,11 +52,11 @@ class Puppet::Parser::Scope if value.is_a?(String) if value =~ /^-?\d+(:?\.\d+|(:?\.\d+)?e\d+)$/ return value.to_f - elsif value =~ /^0x\d+/i + elsif value =~ /^0x[0-9a-f]+$/i return value.to_i(16) - elsif value =~ /^0\d+/i + elsif value =~ /^0[0-7]+$/ return value.to_i(8) - elsif value =~ /^-?\d+/ + elsif value =~ /^-?\d+$/ return value.to_i else return nil diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb index fa76c4ff2..13559528b 100755 --- a/spec/unit/parser/scope.rb +++ b/spec/unit/parser/scope.rb @@ -81,7 +81,21 @@ describe Puppet::Parser::Scope do Puppet::Parser::Scope.number?("0755").should == 0755 end + it "should return nil on malformed integers" do + Puppet::Parser::Scope.number?("0.24.5").should be_nil + end + + it "should convert strings with leading 0 to integer if they are not octal" do + Puppet::Parser::Scope.number?("0788").should == 788 + end + it "should convert strings of negative integers" do + Puppet::Parser::Scope.number?("-0788").should == -788 + end + + it "should return nil on malformed hexadecimal numbers" do + Puppet::Parser::Scope.number?("0x89g").should be_nil + end end end -- cgit From 2c7e1897eaaa7d2dd8d8d993df8a9a217d1c46c6 Mon Sep 17 00:00:00 2001 From: Nigel Kersten Date: Mon, 16 Mar 2009 07:30:16 -0700 Subject: Fixes incorrect detail variable in OS X version check, re-patches ralsh to work with Facter values and adds error check for missing password hash files. --- CHANGELOG | 2 ++ bin/ralsh | 4 ++++ lib/puppet/provider/nameservice/directoryservice.rb | 6 +++--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4d5b05f40..13bdd3987 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ 0.24.8 + Fixed #2077 - ralsh user broken on OSX + Fixed #2000 - No default specified for checksum Fixed #2026 - Red Hat ignoring stop method diff --git a/bin/ralsh b/bin/ralsh index 5dae8f130..44fc9a568 100755 --- a/bin/ralsh +++ b/bin/ralsh @@ -88,6 +88,7 @@ require 'getoptlong' require 'puppet' +require 'facter' options = [ [ "--debug", "-d", GetoptLong::NO_ARGUMENT ], @@ -99,6 +100,9 @@ options = [ [ "--help", "-h", GetoptLong::NO_ARGUMENT ] ] +# Load facts from Facter +Facter.loadfacts + # Add all of the config parameters as valid options. Puppet.settings.addargs(options) diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 42c52f0a7..0e06bb089 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -118,7 +118,7 @@ class DirectoryService < Puppet::Provider::NameService begin product_version = Facter.value(:macosx_productversion) if product_version.nil? - raise Puppet::Error, "Could not determine OS X version: %s" % detail + raise Puppet::Error, "Could not determine OS X version" end product_version_major = product_version.scan(/(\d+)\.(\d+)./).join(".") if %w{10.0 10.1 10.2 10.3}.include?(product_version_major) @@ -320,14 +320,14 @@ class DirectoryService < Puppet::Provider::NameService def self.get_password(guid) password_hash = nil password_hash_file = "#{@@password_hash_dir}/#{guid}" - if File.exists?(password_hash_file) + if File.exists?(password_hash_file) and File.file?(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]}") end f = File.new(password_hash_file) password_hash = f.read f.close - end + end password_hash end -- cgit From 5ab63cdf2efa9a8557d3837c32d8336dd23a4b85 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Fri, 20 Mar 2009 20:34:13 +1100 Subject: Updated lib install permissions to 0644 --- install.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install.rb b/install.rb index 3fa3822c5..9ea2692d4 100755 --- a/install.rb +++ b/install.rb @@ -94,7 +94,7 @@ def do_libs(libs, strip = 'lib/') op = File.dirname(olf) File.makedirs(op, true) File.chmod(0755, op) - File.install(lf, olf, 0755, true) + File.install(lf, olf, 0644, true) end end -- cgit From 2b33f80d167424abcd153e01aa1a93ae38155120 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Fri, 20 Mar 2009 20:37:05 +1100 Subject: Fixed install.rb typo --- install.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install.rb b/install.rb index 9ea2692d4..96d351611 100755 --- a/install.rb +++ b/install.rb @@ -170,7 +170,7 @@ def prepare_installation opts.on('--[no-]ri', 'Prevents the creation of RI output.', 'Default off on mswin32.') do |onri| InstallOptions.ri = onri end - opts.on('--[no-]man', 'Presents the creation of man pages.', 'Default on.') do |onman| + opts.on('--[no-]man', 'Prevents the creation of man pages.', 'Default on.') do |onman| InstallOptions.man = onman end opts.on('--[no-]tests', 'Prevents the execution of unit tests.', 'Default on.') do |ontest| -- cgit From 081021af4dda0d19e7de7debb580196219bb7c36 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 19 Dec 2008 17:38:50 +0100 Subject: Fix #1829 - Add puppet function versioncmp to compare versions Signed-off-by: Brice Figureau --- lib/puppet/parser/functions/versioncmp.rb | 10 ++++++++++ spec/unit/parser/functions/versioncmp.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 lib/puppet/parser/functions/versioncmp.rb create mode 100755 spec/unit/parser/functions/versioncmp.rb diff --git a/lib/puppet/parser/functions/versioncmp.rb b/lib/puppet/parser/functions/versioncmp.rb new file mode 100644 index 000000000..62df38ffc --- /dev/null +++ b/lib/puppet/parser/functions/versioncmp.rb @@ -0,0 +1,10 @@ +require 'puppet/util/package' + +Puppet::Parser::Functions::newfunction(:versioncmp, :doc => "Compares two versions.") do |args| + + unless args.length == 2 + raise Puppet::ParseError, "versioncmp should have 2 arguments" + end + + return Puppet::Util::Package.versioncmp(args[0], args[1]) +end diff --git a/spec/unit/parser/functions/versioncmp.rb b/spec/unit/parser/functions/versioncmp.rb new file mode 100755 index 000000000..06b125ea0 --- /dev/null +++ b/spec/unit/parser/functions/versioncmp.rb @@ -0,0 +1,29 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +describe "the versioncmp function" do + + before :each do + @scope = Puppet::Parser::Scope.new() + end + + it "should exist" do + Puppet::Parser::Functions.function("versioncmp").should == "function_versioncmp" + end + + it "should raise a ParseError if there is less than 2 arguments" do + lambda { @scope.function_versioncmp(["1.2"]) }.should raise_error(Puppet::ParseError) + end + + it "should raise a ParseError if there is more than 2 arguments" do + lambda { @scope.function_versioncmp(["1.2", "2.4.5", "3.5.6"]) }.should raise_error(Puppet::ParseError) + end + + it "should call Puppet::Util::Package.versioncmp (included in scope)" do + Puppet::Util::Package.expects(:versioncmp).with("1.2", "1.3").returns(-1) + + @scope.function_versioncmp(["1.2", "1.3"]) + end + +end -- cgit From 69a0f7dc8d3ba1c64e5acdf99628f10b41ab8e30 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 19 Dec 2008 17:35:08 +0100 Subject: Fix #1807 - make Puppet::Util::Package.versioncmp a module function Signed-off-by: Brice Figureau --- lib/puppet/util/package.rb | 2 ++ spec/unit/util/package.rb | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 spec/unit/util/package.rb diff --git a/lib/puppet/util/package.rb b/lib/puppet/util/package.rb index 00e04f64a..613aa6b1e 100644 --- a/lib/puppet/util/package.rb +++ b/lib/puppet/util/package.rb @@ -28,4 +28,6 @@ module Puppet::Util::Package end return version_a <=> version_b; end + + module_function :versioncmp end diff --git a/spec/unit/util/package.rb b/spec/unit/util/package.rb new file mode 100644 index 000000000..7d956efb5 --- /dev/null +++ b/spec/unit/util/package.rb @@ -0,0 +1,21 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +require 'puppet/util/package' + +describe Puppet::Util::Package, " versioncmp" do + + it "should be able to be used as a module function" do + Puppet::Util::Package.should respond_to(:versioncmp) + end + + it "should be able to sort a long set of various unordered versions" do + ary = %w{ 1.1.6 2.3 1.1a 3.0 1.5 1 2.4 1.1-4 2.3.1 1.2 2.3.0 1.1-3 2.4b 2.4 2.40.2 2.3a.1 3.1 0002 1.1-5 1.1.a 1.06} + + newary = ary.sort { |a, b| Puppet::Util::Package.versioncmp(a,b) } + + newary.should == ["0002", "1", "1.06", "1.1-3", "1.1-4", "1.1-5", "1.1.6", "1.1.a", "1.1a", "1.2", "1.5", "2.3", "2.3.0", "2.3.1", "2.3a.1", "2.4", "2.4", "2.4b", "2.40.2", "3.0", "3.1"] + end + +end -- cgit From bbcda1d5bcab492dc68d331e6f78fb0473e9f046 Mon Sep 17 00:00:00 2001 From: Francois Deppierraz Date: Fri, 28 Nov 2008 15:12:30 +0100 Subject: Fix Bug #1629 A refactoring of ssh_authorized_key parsed provider was needed and tests were improved. flush method has been split for clarity. --- lib/puppet/provider/ssh_authorized_key/parsed.rb | 64 +++++++++++++++++++----- spec/unit/provider/ssh_authorized_key/parsed.rb | 64 ++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 12 deletions(-) diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb index 77af58ef5..5604ba32a 100644 --- a/lib/puppet/provider/ssh_authorized_key/parsed.rb +++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb @@ -40,25 +40,55 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed, # This was done in the type class but path expansion was failing for # not yet existing users, the only workaround I found was to move that # in the provider. - if user = @resource.should(:user) - target = File.expand_path("~%s/.ssh/authorized_keys" % user) - @property_hash[:target] = target - @resource[:target] = target - end + @resource[:target] = target super end + def target + if user + File.expand_path("~%s/.ssh/authorized_keys" % user) + elsif target = @resource.should(:target) + target + end + end + + def user + @resource.should(:user) + end + + def dir_perm + # Determine correct permission for created directory and file + # we can afford more restrictive permissions when the user is known + if target + if user + 0700 + else + 0755 + end + end + end + + def file_perm + if target + if user + 0600 + else + 0644 + end + end + end + def flush # As path expansion had to be moved in the provider, we cannot generate new file # resources and thus have to chown and chmod here. It smells hackish. - + # Create target's parent directory if nonexistant - if target = @property_hash[:target] - dir = File.dirname(@property_hash[:target]) + if target + dir = File.dirname(target) if not File.exist? dir Puppet.debug("Creating directory %s which did not exist" % dir) - Dir.mkdir(dir, 0700) + Dir.mkdir(dir, dir_perm) end end @@ -66,9 +96,19 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed, super # Ensure correct permissions - if target and user = @property_hash[:user] - File.chown(Puppet::Util.uid(user), nil, dir) - File.chown(Puppet::Util.uid(user), nil, @property_hash[:target]) + if target and user + uid = Puppet::Util.uid(user) + + if uid + File.chown(uid, nil, dir) + File.chown(uid, nil, target) + else + raise Puppet::Error, "Specified user does not exist" + end + end + + if target + File.chmod(file_perm, target) end end diff --git a/spec/unit/provider/ssh_authorized_key/parsed.rb b/spec/unit/provider/ssh_authorized_key/parsed.rb index 21f30f97e..73d623573 100755 --- a/spec/unit/provider/ssh_authorized_key/parsed.rb +++ b/spec/unit/provider/ssh_authorized_key/parsed.rb @@ -100,3 +100,67 @@ describe provider_class do @provider.parse_options(optionstr).should == options end end + +describe provider_class do + before :each do + @resource = stub("resource", :name => "foo") + @resource.stubs(:[]).returns "foo" + @provider = provider_class.new(@resource) + end + + describe "when flushing" do + before :each do + # Stub file and directory operations + Dir.stubs(:mkdir) + File.stubs(:chmod) + File.stubs(:chown) + end + + describe "and a user has been specified" do + before :each do + @resource.stubs(:should).with(:user).returns "nobody" + @resource.stubs(:should).with(:target).returns nil + end + + it "should create the directory" do + Dir.expects(:mkdir).with(File.expand_path("~nobody/.ssh"), 0700) + @provider.flush + end + + it "should chown the directory to the user" do + uid = Puppet::Util.uid("nobody") + File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh")) + @provider.flush + end + + it "should chown the key file to the user" do + uid = Puppet::Util.uid("nobody") + File.expects(:chown).with(uid, nil, File.expand_path("~nobody/.ssh/authorized_keys")) + @provider.flush + end + + it "should chmod the key file to 0600" do + File.chmod(0600, File.expand_path("~nobody/.ssh/authorized_keys")) + @provider.flush + end + end + + describe "and a target has been specified" do + before :each do + @resource.stubs(:should).with(:user).returns nil + @resource.stubs(:should).with(:target).returns "/tmp/.ssh/authorized_keys" + end + + it "should make the directory" do + Dir.expects(:mkdir).with("/tmp/.ssh", 0755) + @provider.flush + end + + it "should chmod the key file to 0644" do + File.expects(:chmod).with(0644, "/tmp/.ssh/authorized_keys") + @provider.flush + end + end + + end +end -- cgit From dcf2bf2be7275afc68054f3c9d51b548111b6574 Mon Sep 17 00:00:00 2001 From: Francois Deppierraz Date: Sun, 22 Mar 2009 00:09:11 +0100 Subject: Changelog entries for #1629 and #2004 Signed-off-by: Francois Deppierraz --- CHANGELOG | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 13bdd3987..bea6dbc5b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,9 @@ 0.24.8 Fixed #2077 - ralsh user broken on OSX + + Fixed #2004 - ssh_authorized_key fails if no target is defined + + Fixed #1629 - incorrect permissions on ssh_authorized_keys created files Fixed #2000 - No default specified for checksum -- cgit From 39deaf373c46c20d14a04391ad4b7c70ad43e266 Mon Sep 17 00:00:00 2001 From: Francois Deppierraz Date: Sat, 21 Mar 2009 23:32:02 +0100 Subject: Fixed #2004 - ssh_authorized_key fails if no target is defined This commit depends on 7f291afdacf59f762c3b78481f5420ec8919e46d (fixing #1629) which was cherry-picked from master. Signed-off-by: Francois Deppierraz --- lib/puppet/provider/ssh_authorized_key/parsed.rb | 15 +-------- lib/puppet/type/ssh_authorized_key.rb | 14 +++++++++ spec/unit/provider/ssh_authorized_key/parsed.rb | 24 ++------------- spec/unit/type/ssh_authorized_key.rb | 39 +++++++++++++++++++----- 4 files changed, 48 insertions(+), 44 deletions(-) diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb index 5604ba32a..051fb63ef 100644 --- a/lib/puppet/provider/ssh_authorized_key/parsed.rb +++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb @@ -36,21 +36,8 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed, :rts => /^\s+/, :match => /^(?:(.+) )?(\d+) (\d+) (\d+)(?: (.+))?$/ - def prefetch - # This was done in the type class but path expansion was failing for - # not yet existing users, the only workaround I found was to move that - # in the provider. - @resource[:target] = target - - super - end - def target - if user - File.expand_path("~%s/.ssh/authorized_keys" % user) - elsif target = @resource.should(:target) - target - end + @resource.should(:target) end def user diff --git a/lib/puppet/type/ssh_authorized_key.rb b/lib/puppet/type/ssh_authorized_key.rb index 66cf3e733..997afb81e 100644 --- a/lib/puppet/type/ssh_authorized_key.rb +++ b/lib/puppet/type/ssh_authorized_key.rb @@ -31,6 +31,20 @@ module Puppet newproperty(:target) do desc "The file in which to store the SSH key." + + defaultto :absent + + def should + if defined? @should and @should[0] != :absent + return super + end + + if user = resource[:user] + return File.expand_path("~%s/.ssh/authorized_keys" % user) + end + + return nil + end end newproperty(:options, :array_matching => :all) do diff --git a/spec/unit/provider/ssh_authorized_key/parsed.rb b/spec/unit/provider/ssh_authorized_key/parsed.rb index 73d623573..1e5d6be48 100755 --- a/spec/unit/provider/ssh_authorized_key/parsed.rb +++ b/spec/unit/provider/ssh_authorized_key/parsed.rb @@ -72,27 +72,6 @@ describe provider_class do genkey(key).should == "from=\"192.168.1.1\",no-pty,no-X11-forwarding ssh-rsa AAAAfsfddsjldjgksdflgkjsfdlgkj root@localhost\n" end - it "should prefetch ~user/.ssh/authorized_keys when user is given" do - key = Puppet::Type.type(:ssh_authorized_key).create( - :name => "Test", - :key => "AA", - :type => "rsa", - :ensure => :present, - :user => "root") - prov = @provider.new key - - prov.prefetch - prov.target.should == File.expand_path("~root/.ssh/authorized_keys") - end - - it "should create destination dir" do - # No idea how to test the flush method - end - - it "should set correct default permissions" do - # No idea how to test the flush method - end - it "'s parse_options method should be able to parse options containing commas" do options = %w{from="host1.reductlivelabs.com,host.reductivelabs.com" command="/usr/local/bin/run" ssh-pty} optionstr = options.join(", ") @@ -119,7 +98,8 @@ describe provider_class do describe "and a user has been specified" do before :each do @resource.stubs(:should).with(:user).returns "nobody" - @resource.stubs(:should).with(:target).returns nil + target = File.expand_path("~nobody/.ssh/authorized_keys") + @resource.stubs(:should).with(:target).returns target end it "should create the directory" do diff --git a/spec/unit/type/ssh_authorized_key.rb b/spec/unit/type/ssh_authorized_key.rb index 2cd5171c9..6d60ac2ef 100755 --- a/spec/unit/type/ssh_authorized_key.rb +++ b/spec/unit/type/ssh_authorized_key.rb @@ -89,14 +89,37 @@ describe ssh_authorized_key do @class.attrtype(:target).should == :property end - it "should raise an error when neither user nor target is given" do - proc do - @class.create( - :name => "Test", - :key => "AAA", - :type => "ssh-rsa", - :ensure => :present) - end.should raise_error(Puppet::Error) + describe "when neither user nor target is specified" do + it "should raise an error" do + proc do + @class.create( + :name => "Test", + :key => "AAA", + :type => "ssh-rsa", + :ensure => :present) + end.should raise_error(Puppet::Error) + end + end + + describe "when both target and user are specified" do + it "should use target" do + resource = @class.create( + :name => "Test", + :user => "root", + :target => "/tmp/blah") + resource.should(:target).should == "/tmp/blah" + end + end + + + describe "when user is specified" do + it "should determine target" do + resource = @class.create( + :name => "Test", + :user => "root") + target = File.expand_path("~root/.ssh/authorized_keys") + resource.should(:target).should == target + end end after do -- cgit From aa00bdedefcf4dce9c55968e33123a73b78fb6d6 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 22 Mar 2009 22:00:41 -0500 Subject: Fixing #1631 - adding /sbin and /usr/sbin to PATH This is a trivial fix but seems to crop up more often than it should. Signed-off-by: Luke Kanies --- lib/puppet/defaults.rb | 9 +++++++++ spec/integration/defaults.rb | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index e1b6dc423..e36dd7068 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -95,8 +95,17 @@ module Puppet :path => {:default => "none", :desc => "The shell search path. Defaults to whatever is inherited from the parent process.", + :call_on_define => true, # Call our hook with the default value, so we always get the libdir set. :hook => proc do |value| ENV["PATH"] = value unless value == "none" + + paths = ENV["PATH"].split(File::PATH_SEPARATOR) + %w{/usr/sbin /sbin}.each do |path| + unless paths.include?(path) + ENV["PATH"] += File::PATH_SEPARATOR + path + end + end + value end }, :libdir => {:default => "$vardir/lib", diff --git a/spec/integration/defaults.rb b/spec/integration/defaults.rb index 54b673a1a..0b2b756bf 100755 --- a/spec/integration/defaults.rb +++ b/spec/integration/defaults.rb @@ -5,6 +5,7 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/defaults' describe "Puppet defaults" do + include Puppet::Util::Execution after { Puppet.settings.clear } describe "when setting the :factpath" do @@ -44,4 +45,12 @@ describe "Puppet defaults" do it "should default to yaml as the catalog format" do Puppet.settings[:catalog_format].should == "yaml" end + + it "should add /usr/sbin and /sbin to the path if they're not there" do + withenv("PATH" => "/usr/bin:/usr/local/bin") do + Puppet.settings[:path] = "none" # this causes it to ignore the setting + ENV["PATH"].split(File::PATH_SEPARATOR).should be_include("/usr/sbin") + ENV["PATH"].split(File::PATH_SEPARATOR).should be_include("/sbin") + end + end end -- cgit From c62c19370bfba881819c1a3d9da2f8e6fb52e5d9 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 23 Mar 2009 03:41:25 +0000 Subject: Updated to version 0.24.8 --- lib/puppet.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet.rb b/lib/puppet.rb index 4b0091cd0..c7e28b7cf 100644 --- a/lib/puppet.rb +++ b/lib/puppet.rb @@ -25,7 +25,7 @@ require 'puppet/util/suidmanager' # it's also a place to find top-level commands like 'debug' module Puppet - PUPPETVERSION = '0.24.7' + PUPPETVERSION = '0.24.8' def Puppet.version return PUPPETVERSION -- cgit