diff options
85 files changed, 1615 insertions, 989 deletions
@@ -4,8 +4,8 @@ Puppet Labs can be contacted at: info@puppetlabs.com This program and entire repository is free software; you can redistribute it and/or modify it under the terms of the GNU -General Public License as published by the Free Software -Foundation; either version 2 of the License, or any later version. +General Public License Version 2 as published by the Free Software +Foundation. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index c7be893c7..77e8476a2 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -1,4 +1,6 @@ +require 'puppet' require 'puppet/application' +require 'puppet/file_bucket/dipper' class Puppet::Application::Inspect < Puppet::Application @@ -54,20 +56,55 @@ class Puppet::Application::Inspect < Puppet::Application inspect_starttime = Time.now @report.add_times("config_retrieval", inspect_starttime - retrieval_starttime) + if Puppet[:archive_files] + dipper = Puppet::FileBucket::Dipper.new(:Server => Puppet[:archive_file_server]) + end + catalog.to_ral.resources.each do |ral_resource| audited_attributes = ral_resource[:audit] next unless audited_attributes - audited_resource = ral_resource.to_resource - status = Puppet::Resource::Status.new(ral_resource) - audited_attributes.each do |name| - next if audited_resource[name].nil? - # Skip :absent properties of :absent resources. Really, it would be nicer if the RAL returned nil for those, but it doesn't. ~JW - if name == :ensure or audited_resource[:ensure] != :absent or audited_resource[name] != :absent - event = ral_resource.event(:previous_value => audited_resource[name], :property => name, :status => "audit", :message => "inspected value is #{audited_resource[name].inspect}") + + begin + audited_resource = ral_resource.to_resource + rescue StandardError => detail + puts detail.backtrace if Puppet[:trace] + ral_resource.err "Could not inspect #{ral_resource}; skipping: #{detail}" + audited_attributes.each do |name| + event = ral_resource.event( + :property => name, + :status => "failure", + :audited => true, + :message => "failed to inspect #{name}" + ) status.add_event(event) end + else + audited_attributes.each do |name| + next if audited_resource[name].nil? + # Skip :absent properties of :absent resources. Really, it would be nicer if the RAL returned nil for those, but it doesn't. ~JW + if name == :ensure or audited_resource[:ensure] != :absent or audited_resource[name] != :absent + event = ral_resource.event( + :previous_value => audited_resource[name], + :property => name, + :status => "audit", + :audited => true, + :message => "inspected value is #{audited_resource[name].inspect}" + ) + status.add_event(event) + end + end + end + if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content) + path = ral_resource[:path] + if File.readable?(path) + begin + dipper.backup(path) + rescue StandardError => detail + Puppet.warning detail + end + end end @report.add_resource_status(status) end diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 4521a5901..764cbbe2b 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -2,8 +2,8 @@ module Puppet setdefaults(:main, :confdir => [Puppet.run_mode.conf_dir, "The main Puppet configuration directory. The default for this parameter is calculated based on the user. If the process - is running as root or the user that `puppet master` is supposed to run as, it defaults to a system directory, but if it's running as any other user, - it defaults to being in `~`."], + is running as root or the user that Puppet is supposed to run as, it defaults to a system directory, but if it's running as any other user, + it defaults to being in the user's home directory."], :vardir => [Puppet.run_mode.var_dir, "Where Puppet stores dynamic and growing data. The default for this parameter is calculated specially, like `confdir`_."], :name => [Puppet.application_name.to_s, "The name of the application, if we are running as one. The default is essentially $0 without the path or `.rb`."], @@ -598,6 +598,11 @@ module Puppet compression, but if it supports it, this setting might reduce performance on high-speed LANs."] ) + setdefaults(:inspect, + :archive_files => [false, "During an inspect run, whether to archive files whose contents are audited to a file bucket."], + :archive_file_server => ["$server", "During an inspect run, the file bucket server to archive files to if archive_files is set."] + ) + # Plugin information. setdefaults( diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb index dbfcdcd43..f4bef28a8 100644 --- a/lib/puppet/file_bucket/dipper.rb +++ b/lib/puppet/file_bucket/dipper.rb @@ -33,10 +33,15 @@ class Puppet::FileBucket::Dipper raise(ArgumentError, "File #{file} does not exist") unless ::File.exist?(file) contents = ::File.read(file) begin - file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path, :path => absolutize_path(file) ) + file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path) dest_path = "#{@rest_path}#{file_bucket_file.name}" - file_bucket_file.save(dest_path) + # Make a HEAD request for the file so that we don't waste time + # uploading it if it already exists in the bucket. + unless Puppet::FileBucket::File.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}") + file_bucket_file.save(dest_path) + end + return file_bucket_file.checksum_data rescue => detail puts detail.backtrace if Puppet[:trace] diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb index 96fd8e225..08c0329f1 100644 --- a/lib/puppet/file_bucket/file.rb +++ b/lib/puppet/file_bucket/file.rb @@ -1,10 +1,9 @@ require 'puppet/file_bucket' require 'puppet/indirector' require 'puppet/util/checksums' +require 'digest/md5' class Puppet::FileBucket::File - include Puppet::Util::Checksums - # This class handles the abstract notion of a file in a filebucket. # There are mechanisms to save and load this file locally and remotely in puppet/indirector/filebucketfile/* # There is a compatibility class that emulates pre-indirector filebuckets in Puppet::FileBucket::Dipper @@ -12,71 +11,27 @@ class Puppet::FileBucket::File require 'puppet/file_bucket/file/indirection_hooks' indirects :file_bucket_file, :terminus_class => :file, :extend => Puppet::FileBucket::File::IndirectionHooks - attr :path, true - attr :paths, true - attr :contents, true - attr :checksum_type - attr :bucket_path, true - - def self.default_checksum_type - "md5" - end + attr :contents + attr :bucket_path def initialize( contents, options = {} ) - @bucket_path = options[:bucket_path] - @path = options[:path] - @paths = options[:paths] || [] - - @checksum = options[:checksum] - @checksum_type = options[:checksum_type] - - self.contents = contents - - yield(self) if block_given? - - validate! - end + raise ArgumentError if !contents.is_a?(String) + @contents = contents - def validate! - validate_checksum_type!(checksum_type) - validate_checksum!(checksum) if checksum + @bucket_path = options.delete(:bucket_path) + raise ArgumentError if options != {} end - def contents=(str) - raise "You may not change the contents of a FileBucket File" if @contents - validate_content!(str) - @contents = str + def checksum_type + 'md5' end def checksum - return @checksum if @checksum - @checksum = calculate_checksum if contents - @checksum - end - - def checksum=(checksum) - validate_checksum!(checksum) - @checksum = checksum - end - - def checksum_type=( new_checksum_type ) - @checksum = nil - @checksum_type = new_checksum_type - end - - def checksum_type - unless @checksum_type - if @checksum - @checksum_type = sumtype(checksum) - else - @checksum_type = self.class.default_checksum_type - end - end - @checksum_type + "{#{checksum_type}}#{checksum_data}" end def checksum_data - sumdata(checksum) + @checksum_data ||= Digest::MD5.hexdigest(contents) end def to_s @@ -84,18 +39,7 @@ class Puppet::FileBucket::File end def name - [checksum_type, checksum_data, path].compact.join('/') - end - - def name=(name) - data = name.split('/',3) - self.path = data.pop - @checksum_type = nil - self.checksum = "{#{data[0]}}#{data[1]}" - end - - def conflict_check? - true + "#{checksum_type}/#{checksum_data}" end def self.from_s( contents ) @@ -103,34 +47,10 @@ class Puppet::FileBucket::File end def to_pson - hash = { "contents" => contents } - hash["path"] = @path if @path - hash.to_pson + { "contents" => contents }.to_pson end def self.from_pson( pson ) - self.new( pson["contents"], :path => pson["path"] ) - end - - private - - def calculate_checksum - "{#{checksum_type}}" + send(checksum_type, contents) - end - - def validate_content!(content) - raise ArgumentError, "Contents must be a string" if content and ! content.is_a?(String) - end - - def validate_checksum!(new_checksum) - newtype = sumtype(new_checksum) - - unless sumdata(new_checksum) == (calc_sum = send(newtype, contents)) - raise Puppet::Error, "Checksum #{new_checksum} does not match contents #{calc_sum}" - end - end - - def validate_checksum_type!(type) - raise ArgumentError, "Invalid checksum type #{type}" unless respond_to?(type) + self.new( pson["contents"] ) end end diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index 5b737578b..e6472f4d9 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -50,6 +50,10 @@ module Puppet::Indirector indirection.find(*args) end + def head(*args) + indirection.head(*args) + end + def destroy(*args) indirection.destroy(*args) end diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb index 318858aaf..8bea2d767 100644 --- a/lib/puppet/indirector/file_bucket_file/file.rb +++ b/lib/puppet/indirector/file_bucket_file/file.rb @@ -14,16 +14,31 @@ module Puppet::FileBucketFile end def find( request ) - checksum, path = request_to_checksum_and_path( request ) - find_by_checksum( checksum, request.options ) + checksum = request_to_checksum( request ) + file_path = path_for(request.options[:bucket_path], checksum, 'contents') + + return nil unless ::File.exists?(file_path) + + if request.options[:diff_with] + hash_protocol = sumtype(checksum) + file2_path = path_for(request.options[:bucket_path], request.options[:diff_with], 'contents') + raise "could not find diff_with #{request.options[:diff_with]}" unless ::File.exists?(file2_path) + return `diff #{file_path.inspect} #{file2_path.inspect}` + else + contents = ::File.read file_path + Puppet.info "FileBucket read #{checksum}" + model.new(contents) + end end - def save( request ) - checksum, path = request_to_checksum_and_path( request ) + def head(request) + checksum = request_to_checksum(request) + file_path = path_for(request.options[:bucket_path], checksum, 'contents') + ::File.exists?(file_path) + end + def save( request ) instance = request.instance - instance.checksum = checksum if checksum - instance.path = path if path save_to_disk(instance) instance.to_s @@ -31,66 +46,41 @@ module Puppet::FileBucketFile private - def find_by_checksum( checksum, options ) - model.new( nil, :checksum => checksum ) do |bucket_file| - bucket_file.bucket_path = options[:bucket_path] - filename = contents_path_for( bucket_file ) - - return nil if ! ::File.exist? filename - - begin - contents = ::File.read filename - Puppet.info "FileBucket read #{bucket_file.checksum}" - rescue RuntimeError => e - raise Puppet::Error, "file could not be read: #{e.message}" - end - - if ::File.exist?(paths_path_for( bucket_file) ) - ::File.open(paths_path_for( bucket_file) ) do |f| - bucket_file.paths = f.readlines.map { |l| l.chomp } - end - end - - bucket_file.contents = contents - end - end - def save_to_disk( bucket_file ) - # If the file already exists, just return the md5 sum. - if ::File.exist?(contents_path_for( bucket_file) ) + filename = path_for(bucket_file.bucket_path, bucket_file.checksum_data, 'contents') + dirname = path_for(bucket_file.bucket_path, bucket_file.checksum_data) + + # If the file already exists, do nothing. + if ::File.exist?(filename) verify_identical_file!(bucket_file) else # Make the directories if necessary. - unless ::File.directory?( path_for( bucket_file) ) + unless ::File.directory?(dirname) Puppet::Util.withumask(0007) do - ::FileUtils.mkdir_p( path_for( bucket_file) ) + ::FileUtils.mkdir_p(dirname) end end - Puppet.info "FileBucket adding #{bucket_file.path} as #{bucket_file.checksum}" + Puppet.info "FileBucket adding #{bucket_file.checksum}" # Write the file to disk. Puppet::Util.withumask(0007) do - ::File.open(contents_path_for(bucket_file), ::File::WRONLY|::File::CREAT, 0440) do |of| + ::File.open(filename, ::File::WRONLY|::File::CREAT, 0440) do |of| of.print bucket_file.contents end end end - - save_path_to_paths_file(bucket_file) - bucket_file.checksum_data end - def request_to_checksum_and_path( request ) - return [request.key, nil] if checksum?(request.key) - - checksum_type, checksum, path = request.key.split(/\//, 3) - return(checksum_type.to_s == "" ? nil : [ "{#{checksum_type}}#{checksum}", path ]) + def request_to_checksum( request ) + checksum_type, checksum, path = request.key.split(/\//, 3) # Note: we ignore path if present. + raise "Unsupported checksum type #{checksum_type.inspect}" if checksum_type != 'md5' + raise "Invalid checksum #{checksum.inspect}" if checksum !~ /^[0-9a-f]{32}$/ + checksum end - def path_for(bucket_file, subfile = nil) - bucket_path = bucket_file.bucket_path || Puppet[:bucketdir] - digest = bucket_file.checksum_data + def path_for(bucket_path, digest, subfile = nil) + bucket_path ||= Puppet[:bucketdir] dir = ::File.join(digest[0..7].split("")) basedir = ::File.join(bucket_path, dir, digest) @@ -99,48 +89,18 @@ module Puppet::FileBucketFile ::File.join(basedir, subfile) end - def contents_path_for(bucket_file) - path_for(bucket_file, "contents") - end - - def paths_path_for(bucket_file) - path_for(bucket_file, "paths") - end - - def content_check? - true - end - # If conflict_check is enabled, verify that the passed text is # the same as the text in our file. def verify_identical_file!(bucket_file) - return unless content_check? - disk_contents = ::File.read(contents_path_for(bucket_file)) + disk_contents = ::File.read(path_for(bucket_file.bucket_path, bucket_file.checksum_data, 'contents')) # If the contents don't match, then we've found a conflict. # Unlikely, but quite bad. if disk_contents != bucket_file.contents - raise Puppet::FileBucket::BucketError, "Got passed new contents for sum #{bucket_file.checksum}", caller + raise Puppet::FileBucket::BucketError, "Got passed new contents for sum #{bucket_file.checksum}" else - Puppet.info "FileBucket got a duplicate file #{bucket_file.path} (#{bucket_file.checksum})" + Puppet.info "FileBucket got a duplicate file #{bucket_file.checksum}" end end - - def save_path_to_paths_file(bucket_file) - return unless bucket_file.path - - # check for dupes - if ::File.exist?(paths_path_for( bucket_file) ) - ::File.open(paths_path_for( bucket_file) ) do |f| - return if f.readlines.collect { |l| l.chomp }.include?(bucket_file.path) - end - end - - # if it's a new file, or if our path isn't in the file yet, add it - ::File.open(paths_path_for(bucket_file), ::File::WRONLY|::File::CREAT|::File::APPEND) do |of| - of.puts bucket_file.path - end - end - end end diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 309eed7b6..ec147ec69 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -180,18 +180,13 @@ class Puppet::Indirector::Indirection request = request(:find, key, *args) terminus = prepare(request) - begin - if result = find_in_cache(request) - return result - end - rescue => detail - puts detail.backtrace if Puppet[:trace] - Puppet.err "Cached #{self.name} for #{request.key} failed: #{detail}" + if result = find_in_cache(request) + return result end # Otherwise, return the result from the terminus, caching if appropriate. if ! request.ignore_terminus? and result = terminus.find(request) - result.expiration ||= self.expiration + result.expiration ||= self.expiration if result.respond_to?(:expiration) if cache? and request.use_cache? Puppet.info "Caching #{self.name} for #{request.key}" cache.save request(:save, result, *args) @@ -203,6 +198,17 @@ class Puppet::Indirector::Indirection nil end + # Search for an instance in the appropriate terminus, and return a + # boolean indicating whether the instance was found. + def head(key, *args) + request = request(:head, key, *args) + terminus = prepare(request) + + # Look in the cache first, then in the terminus. Force the result + # to be a boolean. + !!(find_in_cache(request) || terminus.head(request)) + end + def find_in_cache(request) # See if our instance is in the cache and up to date. return nil unless cache? and ! request.ignore_cache? and cached = cache.find(request) @@ -213,6 +219,10 @@ class Puppet::Indirector::Indirection Puppet.debug "Using cached #{self.name} for #{request.key}" cached + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Cached #{self.name} for #{request.key} failed: #{detail}" + nil end # Remove something via the terminus. diff --git a/lib/puppet/indirector/resource/ral.rb b/lib/puppet/indirector/resource/ral.rb index 1c2ab14ae..bc41d14ae 100644 --- a/lib/puppet/indirector/resource/ral.rb +++ b/lib/puppet/indirector/resource/ral.rb @@ -34,12 +34,17 @@ class Puppet::Resource::Ral < Puppet::Indirector::Code private + # {type,resource}_name: the resource name may contain slashes: + # File["/etc/hosts"]. To handle, assume the type name does + # _not_ have any slashes in it, and split only on the first. + def type_name( request ) - request.key.split('/')[0] + request.key.split('/', 2)[0] end def resource_name( request ) - request.key.split('/')[1] + name = request.key.split('/', 2)[1] + name unless name == "" end def type( request ) diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index eb41ff3b1..e50dc68ae 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -53,11 +53,15 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end else # Raise the http error if we didn't get a 'success' of some kind. - message = "Error #{response.code} on SERVER: #{(response.body||'').empty? ? response.message : uncompress_body(response)}" - raise Net::HTTPError.new(message, response) + raise convert_to_http_error(response) end end + def convert_to_http_error(response) + message = "Error #{response.code} on SERVER: #{(response.body||'').empty? ? response.message : uncompress_body(response)}" + Net::HTTPError.new(message, response) + end + # Provide appropriate headers. def headers add_accept_encoding({"Accept" => model.supported_formats.join(", ")}) @@ -73,6 +77,19 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus result end + def head(request) + response = network(request).head(indirection2uri(request), headers) + case response.code + when "404" + return false + when /^2/ + return true + else + # Raise the http error if we didn't get a 'success' of some kind. + raise convert_to_http_error(response) + end + end + def search(request) unless result = deserialize(network(request).get(indirection2uri(request), headers), true) return [] diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb index dd4612a14..8aa1f0ee1 100644 --- a/lib/puppet/network/http/api/v1.rb +++ b/lib/puppet/network/http/api/v1.rb @@ -13,6 +13,9 @@ module Puppet::Network::HTTP::API::V1 }, "DELETE" => { :singular => :destroy + }, + "HEAD" => { + :singular => :head } } diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 61ae2d2fc..9e9356b2f 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -109,7 +109,22 @@ module Puppet::Network::HTTP::Handler format = format_to_use(request) set_content_type(response, format) - set_response(response, result.render(format)) + if result.respond_to?(:render) + set_response(response, result.render(format)) + else + set_response(response, result) + end + end + + # Execute our head. + def do_head(indirection_request, request, response) + unless indirection_request.model.head(indirection_request.key, indirection_request.to_hash) + Puppet.info("Could not find #{indirection_request.indirection_name} for '#{indirection_request.key}'") + return do_exception(response, "Could not find #{indirection_request.indirection_name} #{indirection_request.key}", 404) + end + + # No need to set a response because no response is expected from a + # HEAD request. All we need to do is not die. end # Execute our search. diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index 7abe06956..7a6147a82 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -38,14 +38,10 @@ module Puppet # fail_on_deny could as well be called in the XMLRPC context # with a ClientRequest. - @rights.fail_on_deny( - build_uri(request), - - :node => request.node, - :ip => request.ip, - :method => request.method, - :environment => request.environment, - :authenticated => request.authenticated) + if authorization_failure_exception = @rights.is_request_forbidden_and_why?(request) + Puppet.warning("Denying access: #{authorization_failure_exception}") + raise authorization_failure_exception + end end def initialize(file = nil, parsenow = true) @@ -88,9 +84,5 @@ module Puppet end @rights.restrict_authenticated(acl[:acl], acl[:authenticated]) unless acl[:authenticated].nil? end - - def build_uri(request) - "/#{request.indirection_name}/#{request.key}" - end end end diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb index e3cd3179a..00ee04f8d 100755 --- a/lib/puppet/network/rights.rb +++ b/lib/puppet/network/rights.rb @@ -26,19 +26,34 @@ class Rights # Check that name is allowed or not def allowed?(name, *args) - begin - fail_on_deny(name, :node => args[0], :ip => args[1]) - rescue AuthorizationError - return false - rescue ArgumentError - # the namespace contract says we should raise this error - # if we didn't find the right acl - raise + !is_forbidden_and_why?(name, :node => args[0], :ip => args[1]) + end + + def is_request_forbidden_and_why?(request) + methods_to_check = if request.method == :head + # :head is ok if either :find or :save is ok. + [:find, :save] + else + [request.method] + end + authorization_failure_exceptions = methods_to_check.map do |method| + is_forbidden_and_why?("/#{request.indirection_name}/#{request.key}", + :node => request.node, + :ip => request.ip, + :method => method, + :environment => request.environment, + :authenticated => request.authenticated) + end + if authorization_failure_exceptions.include? nil + # One of the methods we checked is ok, therefore this request is ok. + nil + else + # Just need to return any of the failure exceptions. + authorization_failure_exceptions.first end - true end - def fail_on_deny(name, args = {}) + def is_forbidden_and_why?(name, args = {}) res = :nomatch right = @rights.find do |acl| found = false @@ -49,7 +64,7 @@ class Rights args[:match] = match if (res = acl.allowed?(args[:node], args[:ip], args)) != :dunno # return early if we're allowed - return if res + return nil if res # we matched, select this acl found = true end @@ -70,13 +85,12 @@ class Rights error.file = right.file error.line = right.line end - Puppet.warning("Denying access: #{error}") else # there were no rights allowing/denying name # if name is not a path, let's throw - error = ArgumentError.new "Unknown namespace right '#{name}'" + raise ArgumentError.new "Unknown namespace right '#{name}'" end - raise error + error end def initialize diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index c60e1d4fb..fdabd05c9 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -139,12 +139,23 @@ class Puppet::Parser::Compiler def evaluate_classes(classes, scope, lazy_evaluate = true) raise Puppet::DevError, "No source for scope passed to evaluate_classes" unless scope.source found = [] + param_classes = nil + # if we are a param class, save the classes hash + # and transform classes to be the keys + if classes.class == Hash + param_classes = classes + classes = classes.keys + end classes.each do |name| # If we can find the class, then make a resource that will evaluate it. if klass = scope.find_hostclass(name) - found << name and next if scope.class_scope(klass) - resource = klass.ensure_in_catalog(scope) + if param_classes + resource = klass.ensure_in_catalog(scope, param_classes[name] || {}) + else + found << name and next if scope.class_scope(klass) + resource = klass.ensure_in_catalog(scope) + end # If they've disabled lazy evaluation (which the :include function does), # then evaluate our resource immediately. @@ -432,7 +443,11 @@ class Puppet::Parser::Compiler @resources = [] # Make sure any external node classes are in our class list - @catalog.add_class(*@node.classes) + if @node.classes.class == Hash + @catalog.add_class(*@node.classes.keys) + else + @catalog.add_class(*@node.classes) + end end # Set the node's parameters into the top-scope as variables. diff --git a/lib/puppet/parser/functions/defined.rb b/lib/puppet/parser/functions/defined.rb index 90632af2f..2aeaa9ba0 100644 --- a/lib/puppet/parser/functions/defined.rb +++ b/lib/puppet/parser/functions/defined.rb @@ -1,10 +1,32 @@ # Test whether a given class or definition is defined -Puppet::Parser::Functions::newfunction(:defined, :type => :rvalue, :doc => "Determine whether a given - type is defined, either as a native type or a defined type, or whether a class is defined. - This is useful for checking whether a class is defined and only including it if it is. - This function can also test whether a resource has been defined, using resource references - (e.g., `if defined(File['/tmp/myfile']) { ... }`). This function is unfortunately - dependent on the parse order of the configuration when testing whether a resource is defined.") do |vals| +Puppet::Parser::Functions::newfunction(:defined, :type => :rvalue, :doc => "Determine whether + a given class or resource type is defined. This function can also determine whether a + specific resource has been declared. Returns true or false. Accepts class names, + type names, and resource references. + + The `defined` function checks both native and defined types, including types + provided as plugins via modules. Types and classes are both checked using their names: + + defined(\"file\") + defined(\"customtype\") + defined(\"foo\") + defined(\"foo::bar\") + + Resource declarations are checked using resource references, e.g. + `defined( File['/tmp/myfile'] )`. Checking whether a given resource + has been declared is, unfortunately, dependent on the parse order of + the configuration, and the following code will not work: + + if defined(File['/tmp/foo']) { + notify(\"This configuration includes the /tmp/foo file.\") + } + file {\"/tmp/foo\": + ensure => present, + } + + However, this order requirement refers to parse order only, and ordering of + resources in the configuration graph (e.g. with `before` or `require`) does not + affect the behavior of `defined`.") do |vals| result = false vals = [vals] unless vals.is_a?(Array) vals.each do |val| diff --git a/lib/puppet/parser/functions/fqdn_rand.rb b/lib/puppet/parser/functions/fqdn_rand.rb index 3e7018ac4..52946f2c1 100644 --- a/lib/puppet/parser/functions/fqdn_rand.rb +++ b/lib/puppet/parser/functions/fqdn_rand.rb @@ -1,7 +1,10 @@ Puppet::Parser::Functions::newfunction(:fqdn_rand, :type => :rvalue, :doc => - "Generates random numbers based on the node's fqdn. The first argument - sets the range. Additional (optional) arguments may be used to further - distinguish the seed.") do |args| + "Generates random numbers based on the node's fqdn. Generated random values + will be a range from 0 up to and excluding n, where n is the first parameter. + The second argument specifies a number to add to the seed and is optional, for example: + + $random_number = fqdn_rand(30) + $random_number_seed = fqdn_rand(30,30)") do |args| require 'md5' max = args.shift srand MD5.new([lookupvar('fqdn'),args].join(':')).to_s.hex diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb index 84e1a0360..12f496a6e 100644 --- a/lib/puppet/property.rb +++ b/lib/puppet/property.rb @@ -152,9 +152,24 @@ class Puppet::Property < Puppet::Parameter # since we cannot fix it. Otherwise, we expect our should value # to be an array, and if @is matches any of those values, then # we consider it to be in-sync. - def insync?(is) + # + # Don't override this method. + def safe_insync?(is) + # If there is no @should value, consider the property to be in sync. return true unless @should + # Otherwise delegate to the (possibly derived) insync? method. + insync?(is) + end + + def self.method_added(sym) + raise "Puppet::Property#safe_insync? shouldn't be overridden; please override insync? instead" if sym == :safe_insync? + end + + # This method should be overridden by derived classes if necessary + # to provide extra logic to determine whether the property is in + # sync. + def insync?(is) self.devfail "#{self.class.name}'s should is not array" unless @should.is_a?(Array) # an empty array is analogous to no should values diff --git a/lib/puppet/property/keyvalue.rb b/lib/puppet/property/keyvalue.rb index 0181708f9..57d0ea2d9 100644 --- a/lib/puppet/property/keyvalue.rb +++ b/lib/puppet/property/keyvalue.rb @@ -77,8 +77,6 @@ module Puppet end def insync?(is) - return true unless @should - return true unless is (is == self.should) diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb index dcee85db7..b86dc87f2 100644 --- a/lib/puppet/property/list.rb +++ b/lib/puppet/property/list.rb @@ -66,8 +66,6 @@ module Puppet end def insync?(is) - return true unless @should - return true unless is (prepare_is_for_comparison(is) == self.should) diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb index 6cbf98e9a..f7b8c9797 100644 --- a/lib/puppet/provider/file/posix.rb +++ b/lib/puppet/provider/file/posix.rb @@ -27,9 +27,7 @@ Puppet::Type.type(:file).provide :posix do end end - def insync?(current, should) - return true unless should - + def is_owner_insync?(current, should) should.each do |value| if value =~ /^\d+$/ uid = Integer(value) diff --git a/lib/puppet/provider/file/win32.rb b/lib/puppet/provider/file/win32.rb index 8ead69a89..21e7ca974 100644 --- a/lib/puppet/provider/file/win32.rb +++ b/lib/puppet/provider/file/win32.rb @@ -14,9 +14,7 @@ Puppet::Type.type(:file).provide :microsoft_windows do id end - def insync?(current, should) - return true unless should - + def is_owner_insync?(current, should) should.each do |value| if value =~ /^\d+$/ uid = Integer(value) diff --git a/lib/puppet/provider/mount.rb b/lib/puppet/provider/mount.rb index 8c7b24bd4..c979f742f 100644 --- a/lib/puppet/provider/mount.rb +++ b/lib/puppet/provider/mount.rb @@ -43,6 +43,8 @@ module Puppet::Provider::Mount line =~ / on #{name} / or line =~ %r{ on /private/var/automount#{name}} when "Solaris", "HP-UX" line =~ /^#{name} on / + when "AIX" + line.split(/\s+/)[1] == name else line =~ / on #{name} / end diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 965a2aa60..b01880360 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -442,7 +442,7 @@ class DirectoryService < Puppet::Provider::NameService def remove_unwanted_members(current_members, new_members) current_members.each do |member| - if not new_members.include?(member) + if not new_members.flatten.include?(member) cmd = [:dseditgroup, "-o", "edit", "-n", ".", "-d", member, @resource[:name]] begin execute(cmd) @@ -454,7 +454,7 @@ class DirectoryService < Puppet::Provider::NameService end def add_members(current_members, new_members) - new_members.each do |new_member| + new_members.flatten.each do |new_member| if current_members.nil? or not current_members.include?(new_member) cmd = [:dseditgroup, "-o", "edit", "-n", ".", "-a", new_member, @resource[:name]] begin diff --git a/lib/puppet/provider/package/freebsd.rb b/lib/puppet/provider/package/freebsd.rb index 2f012a4ed..e10a20b04 100755 --- a/lib/puppet/provider/package/freebsd.rb +++ b/lib/puppet/provider/package/freebsd.rb @@ -20,11 +20,11 @@ Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do if @resource[:source] =~ /\/$/ if @resource[:source] =~ /^(ftp|https?):/ - withenv :PACKAGESITE => @resource[:source] do + Puppet::Util::Execution::withenv :PACKAGESITE => @resource[:source] do pkgadd "-r", @resource[:name] end else - withenv :PKG_PATH => @resource[:source] do + Puppet::Util::Execution::withenv :PKG_PATH => @resource[:source] do pkgadd @resource[:name] end end diff --git a/lib/puppet/provider/zone/solaris.rb b/lib/puppet/provider/zone/solaris.rb index c11444993..f46337b14 100644 --- a/lib/puppet/provider/zone/solaris.rb +++ b/lib/puppet/provider/zone/solaris.rb @@ -40,7 +40,7 @@ Puppet::Type.type(:zone).provide(:solaris) do # Then perform all of our configuration steps. It's annoying # that we need this much internal info on the resource. @resource.send(:properties).each do |property| - str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.insync?(properties[property.name]) + str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.safe_insync?(properties[property.name]) end str += "commit\n" diff --git a/lib/puppet/reference/configuration.rb b/lib/puppet/reference/configuration.rb index e6a8dc20f..c8ff145ba 100644 --- a/lib/puppet/reference/configuration.rb +++ b/lib/puppet/reference/configuration.rb @@ -94,11 +94,11 @@ The file follows INI-style formatting. Here is an example of a very simple Note that boolean parameters must be explicitly specified as `true` or `false` as seen above. -If you need to change file parameters (e.g., reset the mode or owner), do +If you need to change file or directory parameters (e.g., reset the mode or owner), do so within curly braces on the same line: [main] - myfile = /tmp/whatever {owner = root, mode = 644} + vardir = /new/vardir {owner = root, mode = 644} If you're starting out with a fresh configuration, you may wish to let the executable generate a template configuration file for you by invoking diff --git a/lib/puppet/reference/function.rb b/lib/puppet/reference/function.rb index 1333e0d26..7d39bebd5 100644 --- a/lib/puppet/reference/function.rb +++ b/lib/puppet/reference/function.rb @@ -8,6 +8,10 @@ performing stand-alone work like importing. Rvalues return values and can only be used in a statement requiring a value, such as an assignment or a case statement. +Functions execute on the Puppet master. They do not execute on the Puppet agent. +Hence they only have access to the commands and data available on the Puppet master +host. + Here are the functions available in Puppet: " diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb index ee83004bb..dea8c105d 100644 --- a/lib/puppet/resource/status.rb +++ b/lib/puppet/resource/status.rb @@ -12,7 +12,7 @@ module Puppet attr_reader :source_description, :default_log_level, :time, :resource attr_reader :change_count, :out_of_sync_count, :resource_type, :title - YAML_ATTRIBUTES = %w{@resource @file @line @evaluation_time @change_count @out_of_sync_count @tags @time @events @out_of_sync @changed @resource_type @title} + YAML_ATTRIBUTES = %w{@resource @file @line @evaluation_time @change_count @out_of_sync_count @tags @time @events @out_of_sync @changed @resource_type @title @skipped @failed} # Provide a boolean method for each of the states. STATES.each do |attr| @@ -51,6 +51,8 @@ module Puppet @out_of_sync_count = 0 @changed = false @out_of_sync = false + @skipped = false + @failed = false [:file, :line].each do |attr| send(attr.to_s + "=", resource.send(attr)) diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb index d40adc145..34fddf135 100644 --- a/lib/puppet/resource/type.rb +++ b/lib/puppet/resource/type.rb @@ -143,18 +143,26 @@ class Puppet::Resource::Type # classes and nodes. No parameters are be supplied--if this is a # parameterized class, then all parameters take on their default # values. - def ensure_in_catalog(scope) + def ensure_in_catalog(scope, parameters=nil) type == :definition and raise ArgumentError, "Cannot create resources for defined resource types" resource_type = type == :hostclass ? :class : :node # Do nothing if the resource already exists; this makes sure we don't # get multiple copies of the class resource, which helps provide the # singleton nature of classes. - if resource = scope.catalog.resource(resource_type, name) + # we should not do this for classes with parameters + # if parameters are passed, we should still try to create the resource + # even if it exists so that we can fail + # this prevents us from being able to combine param classes with include + if resource = scope.catalog.resource(resource_type, name) and !parameters return resource end - resource = Puppet::Parser::Resource.new(resource_type, name, :scope => scope, :source => self) + if parameters + parameters.each do |k,v| + resource.set_parameter(k,v) + end + end instantiate_resource(scope, resource) scope.compiler.add_resource(scope, resource) resource @@ -222,6 +230,19 @@ class Puppet::Resource::Type set[param] = true end + if @type == :hostclass + scope.setvar("title", resource.title.to_s.downcase) unless set.include? :title + scope.setvar("name", resource.name.to_s.downcase ) unless set.include? :name + else + scope.setvar("title", resource.title ) unless set.include? :title + scope.setvar("name", resource.name ) unless set.include? :name + end + scope.setvar("module_name", module_name) if module_name and ! set.include? :module_name + + if caller_name = scope.parent_module_name and ! set.include?(:caller_module_name) + scope.setvar("caller_module_name", caller_name) + end + scope.class_set(self.name,scope) if hostclass? or node? # Verify that all required arguments are either present or # have been provided with defaults. arguments.each do |param, default| @@ -238,19 +259,6 @@ class Puppet::Resource::Type resource[param] = value end - if @type == :hostclass - scope.setvar("title", resource.title.to_s.downcase) unless set.include? :title - scope.setvar("name", resource.name.to_s.downcase ) unless set.include? :name - else - scope.setvar("title", resource.title ) unless set.include? :title - scope.setvar("name", resource.name ) unless set.include? :name - end - scope.setvar("module_name", module_name) if module_name and ! set.include? :module_name - - if caller_name = scope.parent_module_name and ! set.include?(:caller_module_name) - scope.setvar("caller_module_name", caller_name) - end - scope.class_set(self.name,scope) if hostclass? or node? end # Create a new subscope in which to evaluate our code. diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb index c259d3e05..4a3d35e0d 100644 --- a/lib/puppet/transaction/resource_harness.rb +++ b/lib/puppet/transaction/resource_harness.rb @@ -37,7 +37,10 @@ class Puppet::Transaction::ResourceHarness current_values = current.to_hash historical_values = Puppet::Util::Storage.cache(resource).dup - desired_values = resource.to_resource.to_hash + desired_values = {} + resource.properties.each do |property| + desired_values[property.name] = property.should + end audited_params = (resource[:audit] || []).map { |p| p.to_sym } synced_params = [] @@ -49,13 +52,13 @@ class Puppet::Transaction::ResourceHarness # Update the machine state & create logs/events events = [] ensure_param = resource.parameter(:ensure) - if desired_values[:ensure] && !ensure_param.insync?(current_values[:ensure]) + if desired_values[:ensure] && !ensure_param.safe_insync?(current_values[:ensure]) events << apply_parameter(ensure_param, current_values[:ensure], audited_params.include?(:ensure), historical_values[:ensure]) synced_params << :ensure elsif current_values[:ensure] != :absent work_order = resource.properties # Note: only the resource knows what order to apply changes in work_order.each do |param| - if !param.insync?(current_values[param.name]) + if desired_values[param.name] && !param.safe_insync?(current_values[param.name]) events << apply_parameter(param, current_values[param.name], audited_params.include?(param.name), historical_values[param.name]) synced_params << param.name end @@ -172,22 +175,4 @@ class Puppet::Transaction::ResourceHarness return nil unless name = resource[:schedule] resource.catalog.resource(:schedule, name) || resource.fail("Could not find schedule #{name}") end - - private - - def absent_and_not_being_created?(current, param) - current[:ensure] == :absent and param.should.nil? - end - - def ensure_is_insync?(current, param) - param.insync?(current[:ensure]) - end - - def ensure_should_be_absent?(current, param) - param.should == :absent - end - - def param_is_insync?(current, param) - param.insync?(current[param.name]) - end end diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index ea3944b4e..e03650b54 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -648,7 +648,7 @@ class Type "The is value is not in the is array for '#{property.name}'" end ensureis = is[property] - if property.insync?(ensureis) and property.should == :absent + if property.safe_insync?(ensureis) and property.should == :absent return true end end @@ -660,7 +660,7 @@ class Type end propis = is[property] - unless property.insync?(propis) + unless property.safe_insync?(propis) property.debug("Not in sync: #{propis.inspect} vs #{property.should.inspect}") insync = false #else diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb index 76399d693..4b18e71f9 100755 --- a/lib/puppet/type/cron.rb +++ b/lib/puppet/type/cron.rb @@ -54,11 +54,7 @@ Puppet::Type.newtype(:cron) do # We have to override the parent method, because we consume the entire # "should" array def insync?(is) - if @should - self.is_to_s(is) == self.should_to_s - else - true - end + self.is_to_s(is) == self.should_to_s end # A method used to do parameter input handling. Converts integers diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb index 606888c75..daa49e223 100755 --- a/lib/puppet/type/exec.rb +++ b/lib/puppet/type/exec.rb @@ -9,41 +9,11 @@ module Puppet commands is to use the checks like `creates` to avoid running the command unless some condition is met. - Note also that you can restrict an `exec` to only run when it receives + Note that you can restrict an `exec` to only run when it receives events by using the `refreshonly` parameter; this is a useful way to have your configuration respond to events with arbitrary commands. - It is worth noting that `exec` is special, in that it is not - currently considered an error to have multiple `exec` instances - with the same name. This was done purely because it had to be this - way in order to get certain functionality, but it complicates things. - In particular, you will not be able to use `exec` instances that - share their commands with other instances as a dependency, since - Puppet has no way of knowing which instance you mean. - - For example: - - # defined in the production class - exec { \"make\": - cwd => \"/prod/build/dir\", - path => \"/usr/bin:/usr/sbin:/bin\" - } - - . etc. . - - # defined in the test class - exec { \"make\": - cwd => \"/test/build/dir\", - path => \"/usr/bin:/usr/sbin:/bin\" - } - - Any other type would throw an error, complaining that you had - the same instance being managed in multiple places, but these are - obviously different images, so `exec` had to be treated specially. - - It is recommended to avoid duplicate names whenever possible. - - Note that if an `exec` receives an event from another resource, + Note also that if an `exec` receives an event from another resource, it will get executed again (or execute the command specified in `refresh`, if there is one). There is a strong tendency to use `exec` to do whatever work Puppet @@ -335,7 +305,7 @@ module Puppet # Pull down the main aliases file file { \"/etc/aliases\": source => \"puppet://server/module/aliases\" - } + } # Rebuild the database, but only when the file changes exec { newaliases: @@ -664,7 +634,7 @@ module Puppet def validatecmd(cmd) exe = extractexe(cmd) # if we're not fully qualified, require a path - self.fail "'#{cmd}' is both unqualifed and specified no search path" if File.expand_path(exe) != exe and self[:path].nil? + self.fail "'#{cmd}' is not qualified and no path was specified. Please qualify the command or specify a path." if File.expand_path(exe) != exe and self[:path].nil? end def extractexe(cmd) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index eee948cd5..cbb51bbed 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -34,7 +34,7 @@ Puppet::Type.newtype(:file) do validate do |value| # accept various path syntaxes: lone slash, posix, win32, unc - unless (Puppet.features.posix? and (value =~ /^\/$/ or value =~ /^\/[^\/]/)) or (Puppet.features.microsoft_windows? and (value =~ /^.:\// or value =~ /^\/\/[^\/]+\/[^\/]+/)) + unless (Puppet.features.posix? and value =~ /^\//) or (Puppet.features.microsoft_windows? and (value =~ /^.:\// or value =~ /^\/\/[^\/]+\/[^\/]+/)) fail Puppet::Error, "File paths must be fully qualified, not '#{value}'" end end @@ -271,17 +271,24 @@ Puppet::Type.newtype(:file) do end CREATORS = [:content, :source, :target] + SOURCE_ONLY_CHECKSUMS = [:none, :ctime, :mtime] validate do - count = 0 + creator_count = 0 CREATORS.each do |param| - count += 1 if self.should(param) + creator_count += 1 if self.should(param) end - count += 1 if @parameters.include?(:source) - self.fail "You cannot specify more than one of #{CREATORS.collect { |p| p.to_s}.join(", ")}" if count > 1 + creator_count += 1 if @parameters.include?(:source) + self.fail "You cannot specify more than one of #{CREATORS.collect { |p| p.to_s}.join(", ")}" if creator_count > 1 self.fail "You cannot specify a remote recursion without a source" if !self[:source] and self[:recurse] == :remote + self.fail "You cannot specify source when using checksum 'none'" if self[:checksum] == :none && !self[:source].nil? + + SOURCE_ONLY_CHECKSUMS.each do |checksum_type| + self.fail "You cannot specify content when using checksum '#{checksum_type}'" if self[:checksum] == checksum_type && !self[:content].nil? + end + self.warning "Possible error: recurselimit is set but not recurse, no recursion will happen" if !self[:recurse] and self[:recurselimit] end @@ -290,25 +297,8 @@ Puppet::Type.newtype(:file) do super(path.gsub(/\/+/, '/').sub(/\/$/, '')) end - # List files, but only one level deep. - def self.instances(base = "/") - return [] unless FileTest.directory?(base) - - files = [] - Dir.entries(base).reject { |e| - e == "." or e == ".." - }.each do |name| - path = File.join(base, name) - if obj = self[path] - obj[:audit] = :all - files << obj - else - files << self.new( - :name => path, :audit => :all - ) - end - end - files + def self.instances(base = '/') + return self.new(:name => base, :recurse => true, :recurselimit => 1, :audit => :all).recurse_local.values end @depthfirst = false @@ -779,7 +769,7 @@ Puppet::Type.newtype(:file) do # Make sure we get a new stat objct expire currentvalue = thing.retrieve - thing.sync unless thing.insync?(currentvalue) + thing.sync unless thing.safe_insync?(currentvalue) end end end diff --git a/lib/puppet/type/file/checksum.rb b/lib/puppet/type/file/checksum.rb index 732460738..5586b1383 100755 --- a/lib/puppet/type/file/checksum.rb +++ b/lib/puppet/type/file/checksum.rb @@ -9,7 +9,7 @@ Puppet::Type.type(:file).newparam(:checksum) do The default checksum parameter, if checksums are enabled, is md5." - newvalues "md5", "md5lite", "timestamp", "mtime", "time", "none" + newvalues "md5", "md5lite", "mtime", "ctime", "none" defaultto :md5 diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb index b8f30a9c7..cf924f371 100755 --- a/lib/puppet/type/file/content.rb +++ b/lib/puppet/type/file/content.rb @@ -96,7 +96,6 @@ module Puppet end return true if ! @resource.replace? - return true unless self.should result = super @@ -168,6 +167,8 @@ module Puppet def each_chunk_from(source_or_content) if source_or_content.is_a?(String) yield source_or_content + elsif source_or_content.nil? && resource.parameter(:ensure) && [:present, :file].include?(resource.parameter(:ensure).value) + yield '' elsif source_or_content.nil? yield read_file_from_filebucket elsif self.class.standalone? diff --git a/lib/puppet/type/file/owner.rb b/lib/puppet/type/file/owner.rb index d473da20e..483cc7fce 100755 --- a/lib/puppet/type/file/owner.rb +++ b/lib/puppet/type/file/owner.rb @@ -6,7 +6,7 @@ module Puppet @event = :file_changed def insync?(current) - provider.insync?(current, @should) + provider.is_owner_insync?(current, @should) end # We want to print names, not numbers diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 7d03de2b0..bc464e1c3 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -169,7 +169,6 @@ module Puppet checks.delete(:checksum) resource[:audit] = checks - resource[:checksum] = :md5 unless resource.property(:checksum) end def local? diff --git a/lib/puppet/type/file/target.rb b/lib/puppet/type/file/target.rb index 9e7229dda..b9fe9213b 100644 --- a/lib/puppet/type/file/target.rb +++ b/lib/puppet/type/file/target.rb @@ -14,7 +14,7 @@ module Puppet # Only call mklink if ensure didn't call us in the first place. currentensure = @resource.property(:ensure).retrieve - mklink if @resource.property(:ensure).insync?(currentensure) + mklink if @resource.property(:ensure).safe_insync?(currentensure) end # Create our link. diff --git a/lib/puppet/type/host.rb b/lib/puppet/type/host.rb index 8ab750459..2666e50ae 100755 --- a/lib/puppet/type/host.rb +++ b/lib/puppet/type/host.rb @@ -66,7 +66,7 @@ module Puppet newproperty(:target) do desc "The file in which to store service information. Only used by - those providers that write to disk." + those providers that write to disk. On most systems this defaults to `/etc/hosts`." defaultto { if @resource.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile) @resource.class.defaultprovider.default_target diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb index d048c90f1..da9a70bdf 100755 --- a/lib/puppet/type/mount.rb +++ b/lib/puppet/type/mount.rb @@ -89,7 +89,7 @@ module Puppet if prop.name == :ensure false else - ! prop.insync?(currentvalues[prop]) + ! prop.safe_insync?(currentvalues[prop]) end end.each { |prop| prop.sync }.length @resource.flush if oos > 0 @@ -200,7 +200,7 @@ module Puppet newvalues(:true, :false) defaultto do case Facter.value(:operatingsystem) - when "FreeBSD", "Darwin" + when "FreeBSD", "Darwin", "AIX" false else true diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb index 51a866332..d73d90dff 100644 --- a/lib/puppet/type/package.rb +++ b/lib/puppet/type/package.rb @@ -109,8 +109,6 @@ module Puppet # Override the parent method, because we've got all kinds of # funky definitions of 'in sync'. def insync?(is) - @should ||= [] - @latest ||= nil @lateststamp ||= (Time.now.to_i - 1000) # Iterate across all of the should values, and see how they diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb index c00f02789..0d09c3d5d 100644 --- a/lib/puppet/type/service.rb +++ b/lib/puppet/type/service.rb @@ -73,7 +73,7 @@ module Puppet if property = @resource.property(:enable) val = property.retrieve - property.sync unless property.insync?(val) + property.sync unless property.safe_insync?(val) end event @@ -144,10 +144,16 @@ module Puppet specified." end newparam(:status) do - desc "Specify a *status* command manually. If left - unspecified, the status method will be determined - automatically, usually by looking for the service in the - process table." + desc "Specify a *status* command manually. This command must + return 0 if the service is running and a nonzero value otherwise. + Ideally, these return codes should conform to + [the LSB's specification for init script status actions](http://refspecs.freestandards.org/LSB_3.1.1/LSB-Core-generic/LSB-Core-generic/iniscrptact.html), + but puppet only considers the difference between 0 and nonzero + to be relevant. + + If left unspecified, the status method will be determined + automatically, usually by looking for the service in the process + table." end newparam(:stop) do diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 761d5d71b..e7389a0d1 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -112,8 +112,6 @@ module Puppet end def insync?(is) - return true unless self.should - # We know the 'is' is a number, so we need to convert the 'should' to a number, # too. @should.each do |value| @@ -177,7 +175,7 @@ module Puppet end validate do |value| - if value.to_s !~ /^\d+$/ + if value.to_s !~ /^-?\d+$/ raise ArgumentError, "Password minimum age must be provided as a number" end end @@ -196,7 +194,7 @@ module Puppet end validate do |value| - if value.to_s !~ /^\d+$/ + if value.to_s !~ /^-?\d+$/ raise ArgumentError, "Password maximum age must be provided as a number" end end diff --git a/lib/puppet/type/yumrepo.rb b/lib/puppet/type/yumrepo.rb index 160b2164d..9b4c79428 100644 --- a/lib/puppet/type/yumrepo.rb +++ b/lib/puppet/type/yumrepo.rb @@ -7,14 +7,14 @@ module Puppet class IniProperty < Puppet::Property def insync?(is) # A should property of :absent is the same as nil - if is.nil? && (should.nil? || should == :absent) + if is.nil? && should == :absent return true end super(is) end def sync - if insync?(retrieve) + if safe_insync?(retrieve) result = nil else result = set(self.should) diff --git a/lib/puppet/type/zpool.rb b/lib/puppet/type/zpool.rb index 49cce552a..df06522e8 100755 --- a/lib/puppet/type/zpool.rb +++ b/lib/puppet/type/zpool.rb @@ -8,8 +8,6 @@ module Puppet end def insync?(is) - return true unless self.should - return @should == [:absent] if is == :absent flatten_and_sort(is) == flatten_and_sort(@should) @@ -18,8 +16,6 @@ module Puppet class MultiVDev < VDev def insync?(is) - return true unless self.should - return @should == [:absent] if is == :absent return false unless is.length == @should.length diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb index 6fdf14ecf..e129301e6 100644 --- a/lib/puppet/util/checksums.rb +++ b/lib/puppet/util/checksums.rb @@ -68,7 +68,9 @@ module Puppet::Util::Checksums nil end - alias :ctime_stream :mtime_stream + def mtime(content) + "" + end # Calculate a checksum using Digest::SHA1. def sha1(content) @@ -108,6 +110,12 @@ module Puppet::Util::Checksums File.stat(filename).send(:ctime) end + alias :ctime_stream :mtime_stream + + def ctime(content) + "" + end + # Return a "no checksum" def none_file(filename) "" @@ -119,6 +127,10 @@ module Puppet::Util::Checksums "" end + def none(content) + "" + end + private # Perform an incremental checksum on a file. diff --git a/lib/puppet/util/command_line/puppetca b/lib/puppet/util/command_line/puppetca index 9aa7e907c..317d99881 100755 --- a/lib/puppet/util/command_line/puppetca +++ b/lib/puppet/util/command_line/puppetca @@ -27,7 +27,7 @@ # parameter, so you can specify '--ssldir <directory>' as an argument. # # See the configuration file documentation at -# http://reductivelabs.com/projects/puppet/reference/configref.html for +# http://docs.puppetlabs.com/references/stable/configuration.html for # the full list of acceptable parameters. A commented list of all # configuration options can also be generated by running puppet cert with # '--genconfig'. @@ -45,7 +45,7 @@ # Remove all files related to a host from puppet cert's storage. This is # useful when rebuilding hosts, since new certificate signing requests # will only be honored if puppet cert does not have a copy of a signed -# certificate for that host. The certificate of the host remains valid. +# certificate for that host. The certificate of the host is also revoked. # If '--all' is specified then all host certificates, both signed and # unsigned, will be removed. # diff --git a/lib/puppet/util/command_line/puppetd b/lib/puppet/util/command_line/puppetd index cb8589c5f..71b28429b 100755 --- a/lib/puppet/util/command_line/puppetd +++ b/lib/puppet/util/command_line/puppetd @@ -11,7 +11,7 @@ # # puppet agent [-D|--daemonize|--no-daemonize] [-d|--debug] # [--detailed-exitcodes] [--disable] [--enable] -# [-h|--help] [--fqdn <host name>] [-l|--logdest syslog|<file>|console] +# [-h|--help] [--certname <host name>] [-l|--logdest syslog|<file>|console] # [-o|--onetime] [--serve <handler>] [-t|--test] [--noop] # [--digest <digest>] [--fingerprint] [-V|--version] # [-v|--verbose] [-w|--waitforcert <seconds>] @@ -113,10 +113,11 @@ # # +puppet agent+ exits after executing this. # -# fqdn:: -# Set the fully-qualified domain name of the client. This is only used for -# certificate purposes, but can be used to override the discovered hostname. -# If you need to use this flag, it is generally an indication of a setup problem. +# certname:: +# Set the certname (unique ID) of the client. The master reads this unique +# identifying string, which is usually set to the node's fully-qualified domain +# name, to determine which configurations the node will receive. Use this option +# to debug setup problems or implement unusual node identification schemes. # # help:: # Print this help message @@ -150,7 +151,8 @@ # # test:: # Enable the most common options used for testing. These are +onetime+, -# +verbose+, +ignorecache, +no-daemonize+, and +no-usecacheonfailure+. +# +verbose+, +ignorecache, +no-daemonize+, +no-usecacheonfailure+, +# +detailed-exit-codes+, +no-splay+, and +show_diff+. # # noop:: # Use +noop+ mode where the daemon runs in a no-op or dry-run mode. This is useful diff --git a/lib/puppet/util/command_line/puppetmasterd b/lib/puppet/util/command_line/puppetmasterd index baf8a7581..445169820 100755 --- a/lib/puppet/util/command_line/puppetmasterd +++ b/lib/puppet/util/command_line/puppetmasterd @@ -9,6 +9,7 @@ # # puppet master [-D|--daemonize|--no-daemonize] [-d|--debug] [-h|--help] # [-l|--logdest <file>|console|syslog] [-v|--verbose] [-V|--version] +# [--compile <nodename>] [--apply <catalog>] # # = Description # @@ -49,6 +50,14 @@ # version:: # Print the puppet version number and exit. # +# compile:: +# Capability to compile a catalogue and output it in JSON from the Puppet master. Uses +# facts contained in the $vardir/yaml/ directory to compile the catalog. +# +# apply:: +# Capability to apply JSON catalog (such as one generated with --compile). You can either specify +# a JSON file or pipe in JSON from standard input. +# # = Example # # puppet master diff --git a/lib/puppet/util/zaml.rb b/lib/puppet/util/zaml.rb index 9fda5ae3b..6ac956565 100644 --- a/lib/puppet/util/zaml.rb +++ b/lib/puppet/util/zaml.rb @@ -59,13 +59,12 @@ class ZAML @@previously_emitted_object = {} @@next_free_label_number = 0 end - def initialize(obj,indent) - @indent = indent + def initialize(obj) @this_label_number = nil @@previously_emitted_object[obj.object_id] = self end def to_s - @this_label_number ? ('&id%03d%s' % [@this_label_number, @indent]) : '' + @this_label_number ? ('&id%03d ' % @this_label_number) : '' end def reference @this_label_number ||= (@@next_free_label_number += 1) @@ -76,7 +75,7 @@ class ZAML end end def new_label_for(obj) - Label.new(obj,(Hash === obj || Array === obj) ? "#{@indent || "\n"} " : ' ') + Label.new(obj) end def first_time_only(obj) if label = Label.for(obj) diff --git a/spec/Rakefile b/spec/Rakefile deleted file mode 100644 index 28e1d8e79..000000000 --- a/spec/Rakefile +++ /dev/null @@ -1,91 +0,0 @@ -require File.join(File.dirname(__FILE__), "spec_helper.rb") -require 'rake' -require 'spec/rake/spectask' - -basedir = File.dirname(__FILE__) -puppetlibdir = File.join(basedir, "../lib") -puppettestlibdir = File.join(basedir, "../test/lib") -speclibdir = File.join(basedir, "lib") - -require 'find' - -include Find -include FileTest - -$exclusions = %W(lib) - -filemap = Hash.new { |hash, key| hash[key] = [] } - -allfiles = [] - -# First collect the entire file list. -find(".") do |f| - # Get rid of the leading ./ - f = f.sub(/^\.\//, '') - - file = File.basename(f) - dir = File.dirname(f) - - # Prune . directories and excluded dirs - if (file =~ /^\./ and f != ".") or $exclusions.include?(File.basename(file)) - prune - next - end - next if f == "." - next if dir == "." - - # If we're a ruby script, then add it to the list of files for that dir - if file =~ /\.rb$/ - allfiles << f - # Add it to all of the parent dirs, not just our own - parts = File.split(dir) - if parts[0] == "." - parts.shift - end - parts.each_with_index { |part, i| - path = File.join(parts[0..i]) - filemap[path] << f - } - end -end - - -libs = [puppetlibdir, puppettestlibdir, speclibdir] -desc "Run all specs" -Spec::Rake::SpecTask.new('all') do |t| - t.spec_files = FileList['integration/**/*.rb', 'unit/**/*.rb'] - t.libs = libs - t.spec_opts = ['--options', 'spec.opts'] -end - -task :default => [:all] - -# Now create a task for every directory -filemap.each do |dir, files| - ns = dir.gsub "/", ":" - - # First create a separate task for each file in the namespace. - namespace ns do - files.each do |file| - Spec::Rake::SpecTask.new(File.basename(file, '.rb').to_sym) do |t| - t.spec_files = [ file ] - t.libs = libs - t.spec_opts = ['--options', 'spec.opts'] - end - end - end - - # Then create a task that matches the directory itself. - Spec::Rake::SpecTask.new(dir) do |t| - if ENV["TESTFILES"] - t.spec_files = ENV["TESTFILES"].split(/\s+/) - else - t.spec_files = files.sort - end - t.libs = libs - t.spec_opts = ['--options', 'spec.opts'] - end - - # And alias it with a slash on the end - task(dir + "/" => dir) -end diff --git a/spec/fixtures/unit/provider/mount/mount-output.aix.txt b/spec/fixtures/unit/provider/mount/mount-output.aix.txt new file mode 100644 index 000000000..54edb9c1c --- /dev/null +++ b/spec/fixtures/unit/provider/mount/mount-output.aix.txt @@ -0,0 +1,7 @@ +/dev/hd4 / jfs2 Nov 11 12:11 rw,log=/dev/hd8 +/dev/hd2 /usr jfs2 Nov 11 12:11 rw,log=/dev/hd8 +/dev/hd9var /var jfs2 Nov 11 12:11 rw,log=/dev/hd8 +/dev/hd3 /tmp jfs2 Nov 11 12:11 rw,log=/dev/hd8 +/dev/hd1 /home jfs2 Nov 11 12:11 rw,log=/dev/hd8 +/proc /proc procfs Nov 11 12:11 rw +/dev/hd10opt /opt jfs2 Nov 11 12:11 rw,log=/dev/hd8 diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb index b931708c3..1d99c6ca9 100644 --- a/spec/unit/application/inspect_spec.rb +++ b/spec/unit/application/inspect_spec.rb @@ -6,8 +6,11 @@ require 'puppet/application/inspect' require 'puppet/resource/catalog' require 'puppet/indirector/catalog/yaml' require 'puppet/indirector/report/rest' +require 'puppet/indirector/file_bucket_file/rest' describe Puppet::Application::Inspect do + include PuppetSpec::Files + before :each do @inspect = Puppet::Application[:inspect] end @@ -29,7 +32,7 @@ describe Puppet::Application::Inspect do describe "when executing" do before :each do Puppet[:report] = true - Puppet::Util::Log.stubs(:newdestination) + @inspect.options[:logset] = true Puppet::Transaction::Report::Rest.any_instance.stubs(:save) @inspect.setup end @@ -72,6 +75,26 @@ describe Puppet::Application::Inspect do properties.has_key?("target").should == false end + it "should set audited to true for all events" do + catalog = Puppet::Resource::Catalog.new + file = Tempfile.new("foo") + resource = Puppet::Resource.new(:file, file.path, :parameters => {:audit => "all"}) + catalog.add_resource(resource) + Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(catalog) + + events = nil + + Puppet::Transaction::Report::Rest.any_instance.expects(:save).with do |request| + events = request.instance.resource_statuses.values.first.events + end + + @inspect.run_command + + events.each do |event| + event.audited.should == true + end + end + it "should not report irrelevent attributes if the resource is absent" do catalog = Puppet::Resource::Catalog.new file = Tempfile.new("foo") @@ -93,6 +116,159 @@ describe Puppet::Application::Inspect do end properties.should == {"ensure" => :absent} end + + describe "when archiving to a bucket" do + before :each do + Puppet[:archive_files] = true + Puppet[:archive_file_server] = "filebucketserver" + @catalog = Puppet::Resource::Catalog.new + Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(@catalog) + end + + describe "when auditing files" do + before :each do + @file = tmpfile("foo") + @resource = Puppet::Resource.new(:file, @file, :parameters => {:audit => "content"}) + @catalog.add_resource(@resource) + end + + it "should send an existing file to the file bucket" do + File.open(@file, 'w') { |f| f.write('stuff') } + Puppet::FileBucketFile::Rest.any_instance.expects(:head).with do |request| + request.server == Puppet[:archive_file_server] + end.returns(false) + Puppet::FileBucketFile::Rest.any_instance.expects(:save).with do |request| + request.server == Puppet[:archive_file_server] and request.instance.contents == 'stuff' + end + @inspect.run_command + end + + it "should not send unreadable files" do + File.open(@file, 'w') { |f| f.write('stuff') } + File.chmod(0, @file) + Puppet::FileBucketFile::Rest.any_instance.expects(:head).never + Puppet::FileBucketFile::Rest.any_instance.expects(:save).never + @inspect.run_command + end + + it "should not try to send non-existent files" do + Puppet::FileBucketFile::Rest.any_instance.expects(:head).never + Puppet::FileBucketFile::Rest.any_instance.expects(:save).never + @inspect.run_command + end + + it "should not try to send files whose content we are not auditing" do + @resource[:audit] = "group" + Puppet::FileBucketFile::Rest.any_instance.expects(:head).never + Puppet::FileBucketFile::Rest.any_instance.expects(:save).never + @inspect.run_command + end + + it "should continue if bucketing a file fails" do + File.open(@file, 'w') { |f| f.write('stuff') } + Puppet::FileBucketFile::Rest.any_instance.stubs(:head).returns false + Puppet::FileBucketFile::Rest.any_instance.stubs(:save).raises "failure" + Puppet::Transaction::Report::Rest.any_instance.expects(:save).with do |request| + @report = request.instance + end + + @inspect.run_command + + @report.logs.count.should == 1 + @report.logs.first.message.should =~ /Could not back up/ + end + end + + describe "when auditing non-files" do + before :each do + Puppet::Type.newtype(:stub_type) do + newparam(:name) do + desc "The name var" + isnamevar + end + + newproperty(:content) do + desc "content" + def retrieve + :whatever + end + end + end + + @resource = Puppet::Resource.new(:stub_type, 'foo', :parameters => {:audit => "all"}) + @catalog.add_resource(@resource) + end + + after :each do + Puppet::Type.rmtype(:stub_type) + end + + it "should not try to send non-files" do + Puppet::FileBucketFile::Rest.any_instance.expects(:head).never + Puppet::FileBucketFile::Rest.any_instance.expects(:save).never + @inspect.run_command + end + end + end + + describe "when there are failures" do + before :each do + Puppet::Type.newtype(:stub_type) do + newparam(:name) do + desc "The name var" + isnamevar + end + + newproperty(:content) do + desc "content" + def retrieve + raise "failed" + end + end + end + + @catalog = Puppet::Resource::Catalog.new + Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(@catalog) + + Puppet::Transaction::Report::Rest.any_instance.expects(:save).with do |request| + @report = request.instance + end + end + + after :each do + Puppet::Type.rmtype(:stub_type) + end + + it "should mark the report failed and create failed events for each property" do + @resource = Puppet::Resource.new(:stub_type, 'foo', :parameters => {:audit => "all"}) + @catalog.add_resource(@resource) + + @inspect.run_command + + @report.status.should == "failed" + @report.logs.select{|log| log.message =~ /Could not inspect/}.count.should == 1 + @report.resource_statuses.count.should == 1 + @report.resource_statuses['Stub_type[foo]'].events.count.should == 1 + + event = @report.resource_statuses['Stub_type[foo]'].events.first + event.property.should == "content" + event.status.should == "failure" + event.audited.should == true + event.instance_variables.should_not include("@previous_value") + end + + it "should continue to the next resource" do + @resource = Puppet::Resource.new(:stub_type, 'foo', :parameters => {:audit => "all"}) + @other_resource = Puppet::Resource.new(:stub_type, 'bar', :parameters => {:audit => "all"}) + @catalog.add_resource(@resource) + @catalog.add_resource(@other_resource) + + @inspect.run_command + + @report.resource_statuses.count.should == 2 + @report.resource_statuses.keys.should =~ ['Stub_type[foo]', 'Stub_type[bar]'] + end + end end after :all do diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb index 799e899e7..db40c6296 100755 --- a/spec/unit/file_bucket/dipper_spec.rb +++ b/spec/unit/file_bucket/dipper_spec.rb @@ -2,121 +2,113 @@ require File.dirname(__FILE__) + '/../../spec_helper' +require 'pathname' + require 'puppet/file_bucket/dipper' +require 'puppet/indirector/file_bucket_file/rest' + describe Puppet::FileBucket::Dipper do - before do - ['/my/file'].each do |x| - Puppet::FileBucket::Dipper.any_instance.stubs(:absolutize_path).with(x).returns(x) - end + include PuppetSpec::Files + + def make_tmp_file(contents) + file = tmpfile("file_bucket_file") + File.open(file, 'w') { |f| f.write(contents) } + file end - it "should fail in an informative way when there are failures backing up to the server" do - File.stubs(:exist?).returns true - File.stubs(:read).returns "content" + it "should fail in an informative way when there are failures checking for the file on the server" do + @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket") + + file = make_tmp_file('contents') + Puppet::FileBucket::File.expects(:head).raises ArgumentError + lambda { @dipper.backup(file) }.should raise_error(Puppet::Error) + end + + it "should fail in an informative way when there are failures backing up to the server" do @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket") - filemock = stub "bucketfile" - Puppet::FileBucket::File.stubs(:new).returns(filemock) - filemock.expects(:name).returns "name" - filemock.expects(:save).raises ArgumentError + file = make_tmp_file('contents') + Puppet::FileBucket::File.expects(:head).returns false + Puppet::FileBucket::File.any_instance.expects(:save).raises ArgumentError - lambda { @dipper.backup("/my/file") }.should raise_error(Puppet::Error) + lambda { @dipper.backup(file) }.should raise_error(Puppet::Error) end it "should backup files to a local bucket" do - @dipper = Puppet::FileBucket::Dipper.new( - :Path => "/my/bucket" - ) + Puppet[:bucketdir] = "/non/existent/directory" + file_bucket = tmpdir("bucket") - File.stubs(:exist?).returns true - File.stubs(:read).with("/my/file").returns "my contents" + @dipper = Puppet::FileBucket::Dipper.new(:Path => file_bucket) - bucketfile = stub "bucketfile" - bucketfile.stubs(:name).returns('md5/DIGEST123') - bucketfile.stubs(:checksum_data).returns("DIGEST123") - bucketfile.expects(:save).with('md5/DIGEST123') + file = make_tmp_file('my contents') + checksum = "2975f560750e71c478b8e3b39a956adb" + Digest::MD5.hexdigest('my contents').should == checksum + @dipper.backup(file).should == checksum + File.exists?("#{file_bucket}/2/9/7/5/f/5/6/0/2975f560750e71c478b8e3b39a956adb/contents").should == true + end + + it "should not backup a file that is already in the bucket" do + @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket") - Puppet::FileBucket::File.stubs(:new).with( - - "my contents", - :bucket_path => '/my/bucket', - - :path => '/my/file' - ).returns(bucketfile) + file = make_tmp_file('my contents') + checksum = Digest::MD5.hexdigest('my contents') - @dipper.backup("/my/file").should == "DIGEST123" + Puppet::FileBucket::File.expects(:head).returns true + Puppet::FileBucket::File.any_instance.expects(:save).never + @dipper.backup(file).should == checksum end it "should retrieve files from a local bucket" do - @dipper = Puppet::FileBucket::Dipper.new( - :Path => "/my/bucket" - ) + @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket") + + checksum = Digest::MD5.hexdigest('my contents') - File.stubs(:exist?).returns true - File.stubs(:read).with("/my/file").returns "my contents" + request = nil - bucketfile = stub "bucketfile" - bucketfile.stubs(:to_s).returns "Content" + Puppet::FileBucketFile::File.any_instance.expects(:find).with{ |r| request = r }.once.returns(Puppet::FileBucket::File.new('my contents')) - Puppet::FileBucket::File.expects(:find).with{|x,opts| - x == 'md5/DIGEST123' - }.returns(bucketfile) + @dipper.getfile(checksum).should == 'my contents' - @dipper.getfile("DIGEST123").should == "Content" + request.key.should == "md5/#{checksum}" end it "should backup files to a remote server" do + @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337") - @dipper = Puppet::FileBucket::Dipper.new( - - :Server => "puppetmaster", - - :Port => "31337" - ) + file = make_tmp_file('my contents') + checksum = Digest::MD5.hexdigest('my contents') - File.stubs(:exist?).returns true - File.stubs(:read).with("/my/file").returns "my contents" + real_path = Pathname.new(file).realpath - bucketfile = stub "bucketfile" - bucketfile.stubs(:name).returns('md5/DIGEST123') - bucketfile.stubs(:checksum_data).returns("DIGEST123") - bucketfile.expects(:save).with('https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123') + request1 = nil + request2 = nil + Puppet::FileBucketFile::Rest.any_instance.expects(:head).with { |r| request1 = r }.once.returns(nil) + Puppet::FileBucketFile::Rest.any_instance.expects(:save).with { |r| request2 = r }.once - Puppet::FileBucket::File.stubs(:new).with( - - "my contents", - :bucket_path => nil, - - :path => '/my/file' - ).returns(bucketfile) - - @dipper.backup("/my/file").should == "DIGEST123" + @dipper.backup(file).should == checksum + [request1, request2].each do |r| + r.server.should == 'puppetmaster' + r.port.should == 31337 + r.key.should == "md5/#{checksum}" + end end it "should retrieve files from a remote server" do + @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337") - @dipper = Puppet::FileBucket::Dipper.new( - - :Server => "puppetmaster", - - :Port => "31337" - ) + checksum = Digest::MD5.hexdigest('my contents') - File.stubs(:exist?).returns true - File.stubs(:read).with("/my/file").returns "my contents" + request = nil - bucketfile = stub "bucketfile" - bucketfile.stubs(:to_s).returns "Content" + Puppet::FileBucketFile::Rest.any_instance.expects(:find).with { |r| request = r }.returns(Puppet::FileBucket::File.new('my contents')) - Puppet::FileBucket::File.expects(:find).with{|x,opts| - x == 'https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123' - }.returns(bucketfile) + @dipper.getfile(checksum).should == "my contents" - @dipper.getfile("DIGEST123").should == "Content" + request.server.should == 'puppetmaster' + request.port.should == 31337 + request.key.should == "md5/#{checksum}" end - - end diff --git a/spec/unit/file_bucket/file_spec.rb b/spec/unit/file_bucket/file_spec.rb index 3ad70c203..82063c2e3 100644 --- a/spec/unit/file_bucket/file_spec.rb +++ b/spec/unit/file_bucket/file_spec.rb @@ -7,13 +7,16 @@ require 'digest/md5' require 'digest/sha1' describe Puppet::FileBucket::File do + include PuppetSpec::Files + before do # this is the default from spec_helper, but it keeps getting reset at odd times - Puppet[:bucketdir] = "/dev/null/bucket" + @bucketdir = tmpdir('bucket') + Puppet[:bucketdir] = @bucketdir @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0" @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0" - @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0' + @dir = File.join(@bucketdir, '4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0') @contents = "file contents" end @@ -22,17 +25,6 @@ describe Puppet::FileBucket::File do Puppet::FileBucket::File.new(@contents).to_s.should == @contents end - it "should calculate the checksum type from the passed in checksum" do - Puppet::FileBucket::File.new(@contents, :checksum => @checksum).checksum_type.should == "md5" - end - - it "should allow contents to be specified in a block" do - bucket = Puppet::FileBucket::File.new(nil) do |fb| - fb.contents = "content" - end - bucket.contents.should == "content" - end - it "should raise an error if changing content" do x = Puppet::FileBucket::File.new("first") proc { x.contents = "new" }.should raise_error @@ -42,14 +34,6 @@ describe Puppet::FileBucket::File do proc { Puppet::FileBucket::File.new(5) }.should raise_error(ArgumentError) end - it "should raise an error if setting contents to a non-string" do - proc do - Puppet::FileBucket::File.new(nil) do |x| - x.contents = 5 - end - end.should raise_error(ArgumentError) - end - it "should set the contents appropriately" do Puppet::FileBucket::File.new(@contents).contents.should == @contents end @@ -62,33 +46,6 @@ describe Puppet::FileBucket::File do Puppet::FileBucket::File.new(@contents).checksum.should == @checksum end - it "should remove the old checksum value if the algorithm is changed" do - sum = Puppet::FileBucket::File.new(@contents) - sum.checksum.should_not be_nil - - newsum = Digest::SHA1.hexdigest(@contents).to_s - sum.checksum_type = :sha1 - sum.checksum.should == "{sha1}#{newsum}" - end - - it "should support specifying the checksum_type during initialization" do - sum = Puppet::FileBucket::File.new(@contents, :checksum_type => :sha1) - sum.checksum_type.should == :sha1 - end - - it "should fail when an unsupported checksum_type is used" do - proc { Puppet::FileBucket::File.new(@contents, :checksum_type => :nope) }.should raise_error(ArgumentError) - end - - it "should fail if given an checksum at initialization that does not match the contents" do - proc { Puppet::FileBucket::File.new(@contents, :checksum => "{md5}00000000000000000000000000000000") }.should raise_error(RuntimeError) - end - - it "should fail if assigned a checksum that does not match the contents" do - bucket = Puppet::FileBucket::File.new(@contents) - proc { bucket.checksum = "{md5}00000000000000000000000000000000" }.should raise_error(RuntimeError) - end - describe "when using back-ends" do it "should redirect using Puppet::Indirector" do Puppet::Indirector::Indirection.instance(:file_bucket_file).model.should equal(Puppet::FileBucket::File) @@ -129,26 +86,6 @@ describe Puppet::FileBucket::File do Puppet::FileBucket::File.new(@contents).save end - - it "should append the path to the paths file" do - remote_path = '/path/on/the/remote/box' - - ::File.expects(:directory?).with(@dir).returns(true) - ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440) - ::File.expects(:exist?).with("#{@dir}/contents").returns false - - mockfile = mock "file" - mockfile.expects(:puts).with('/path/on/the/remote/box') - ::File.expects(:exist?).with("#{@dir}/paths").returns false - ::File.expects(:open).with("#{@dir}/paths", ::File::WRONLY|::File::CREAT|::File::APPEND).yields mockfile - Puppet::FileBucket::File.new(@contents, :path => remote_path).save - - end - end - - it "should accept a path" do - remote_path = '/path/on/the/remote/box' - Puppet::FileBucket::File.new(@contents, :path => remote_path).path.should == remote_path end it "should return a url-ish name" do @@ -160,18 +97,6 @@ describe Puppet::FileBucket::File do lambda { bucket.name = "sha1/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/new/path" }.should raise_error end - it "should accept a url-ish name" do - bucket = Puppet::FileBucket::File.new(@contents) - lambda { bucket.name = "sha1/034fa2ed8e211e4d20f20e792d777f4a30af1a93/new/path" }.should_not raise_error - bucket.checksum_type.should == "sha1" - bucket.checksum_data.should == '034fa2ed8e211e4d20f20e792d777f4a30af1a93' - bucket.path.should == "new/path" - end - - it "should return a url-ish name with a path" do - Puppet::FileBucket::File.new(@contents, :path => 'my/path').name.should == "md5/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/my/path" - end - it "should convert the contents to PSON" do Puppet::FileBucket::File.new(@contents).to_pson.should == '{"contents":"file contents"}' end @@ -191,36 +116,31 @@ describe Puppet::FileBucket::File do end + def make_bucketed_file + FileUtils.mkdir_p(@dir) + File.open("#{@dir}/contents", 'w') { |f| f.write @contents } + end + describe "using the indirector's find method" do it "should return nil if a file doesn't exist" do - ::File.expects(:exist?).with("#{@dir}/contents").returns false - - bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}") + bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}") bucketfile.should == nil end it "should find a filebucket if the file exists" do - ::File.expects(:exist?).with("#{@dir}/contents").returns true - ::File.expects(:exist?).with("#{@dir}/paths").returns false - ::File.expects(:read).with("#{@dir}/contents").returns @contents - - bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}") + make_bucketed_file + bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}") bucketfile.should_not == nil end describe "using RESTish digest notation" do it "should return nil if a file doesn't exist" do - ::File.expects(:exist?).with("#{@dir}/contents").returns false - bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}") bucketfile.should == nil end it "should find a filebucket if the file exists" do - ::File.expects(:exist?).with("#{@dir}/contents").returns true - ::File.expects(:exist?).with("#{@dir}/paths").returns false - ::File.expects(:read).with("#{@dir}/contents").returns @contents - + make_bucketed_file bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}") bucketfile.should_not == nil end diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb index aa3ade6b6..9187f4da0 100755 --- a/spec/unit/indirector/file_bucket_file/file_spec.rb +++ b/spec/unit/indirector/file_bucket_file/file_spec.rb @@ -5,6 +5,8 @@ require ::File.dirname(__FILE__) + '/../../../spec_helper' require 'puppet/indirector/file_bucket_file/file' describe Puppet::FileBucketFile::File do + include PuppetSpec::Files + it "should be a subclass of the Code terminus class" do Puppet::FileBucketFile::File.superclass.should equal(Puppet::Indirector::Code) end @@ -13,194 +15,143 @@ describe Puppet::FileBucketFile::File do Puppet::FileBucketFile::File.doc.should be_instance_of(String) end - describe "when initializing" do - it "should use the filebucket settings section" do - Puppet.settings.expects(:use).with(:filebucket) - Puppet::FileBucketFile::File.new - end - end - + describe "non-stubbing tests" do + include PuppetSpec::Files - describe "the find_by_checksum method" do before do - # this is the default from spec_helper, but it keeps getting reset at odd times - Puppet[:bucketdir] = "/dev/null/bucket" - - @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0" - @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0" - @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0' - - @contents = "file contents" - end - - it "should return nil if a file doesn't exist" do - ::File.expects(:exist?).with("#{@dir}/contents").returns false - - bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}) - bucketfile.should == nil - end - - it "should find a filebucket if the file exists" do - ::File.expects(:exist?).with("#{@dir}/contents").returns true - ::File.expects(:exist?).with("#{@dir}/paths").returns false - ::File.expects(:read).with("#{@dir}/contents").returns @contents - - bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}) - bucketfile.should_not == nil - end - - it "should load the paths" do - paths = ["path1", "path2"] - ::File.expects(:exist?).with("#{@dir}/contents").returns true - ::File.expects(:exist?).with("#{@dir}/paths").returns true - ::File.expects(:read).with("#{@dir}/contents").returns @contents - - mockfile = mock "file" - mockfile.expects(:readlines).returns( paths ) - ::File.expects(:open).with("#{@dir}/paths").yields mockfile - - Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}).paths.should == paths - end - - end - - describe "when retrieving files" do - before :each do - Puppet.settings.stubs(:use) - @store = Puppet::FileBucketFile::File.new - - @digest = "70924d6fa4b2d745185fa4660703a5c0" - @sum = stub 'sum', :name => @digest - - @dir = "/what/ever" - - Puppet.stubs(:[]).with(:bucketdir).returns(@dir) - - @contents_path = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/contents' - @paths_path = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths' - - @request = stub 'request', :key => "md5/#{@digest}/remote/path", :options => {} - end - - it "should call find_by_checksum" do - @store.expects(:find_by_checksum).with{|x,opts| x == "{md5}#{@digest}"}.returns(false) - @store.find(@request) + Puppet[:bucketdir] = tmpdir('bucketdir') end - it "should look for the calculated path" do - ::File.expects(:exist?).with(@contents_path).returns(false) - @store.find(@request) - end - - it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do - content = "my content" - bucketfile = stub 'bucketfile' - bucketfile.stubs(:bucket_path) - bucketfile.stubs(:bucket_path=) - bucketfile.stubs(:checksum_data).returns(@digest) - bucketfile.stubs(:checksum).returns(@checksum) - - bucketfile.expects(:contents=).with(content) - Puppet::FileBucket::File.expects(:new).with(nil, {:checksum => "{md5}#{@digest}"}).yields(bucketfile).returns(bucketfile) + describe "when diffing files" do + def save_bucket_file(contents) + bucket_file = Puppet::FileBucket::File.new(contents) + bucket_file.save + bucket_file.checksum_data + end - ::File.expects(:exist?).with(@contents_path).returns(true) - ::File.expects(:exist?).with(@paths_path).returns(false) - ::File.expects(:read).with(@contents_path).returns(content) + it "should generate an empty string if there is no diff" do + checksum = save_bucket_file("I'm the contents of a file") + Puppet::FileBucket::File.find("md5/#{checksum}", :diff_with => checksum).should == '' + end - @store.find(@request).should equal(bucketfile) - end + it "should generate a proper diff if there is a diff" do + checksum1 = save_bucket_file("foo\nbar\nbaz") + checksum2 = save_bucket_file("foo\nbiz\nbaz") + diff = Puppet::FileBucket::File.find("md5/#{checksum1}", :diff_with => checksum2) + diff.should == <<HERE +2c2 +< bar +--- +> biz +HERE + end - it "should return nil if no file is found" do - ::File.expects(:exist?).with(@contents_path).returns(false) - @store.find(@request).should be_nil - end + it "should raise an exception if the hash to diff against isn't found" do + checksum = save_bucket_file("whatever") + bogus_checksum = "d1bf072d0e2c6e20e3fbd23f022089a1" + lambda { Puppet::FileBucket::File.find("md5/#{checksum}", :diff_with => bogus_checksum) }.should raise_error "could not find diff_with #{bogus_checksum}" + end - it "should fail intelligently if a found file cannot be read" do - ::File.expects(:exist?).with(@contents_path).returns(true) - ::File.expects(:read).with(@contents_path).raises(RuntimeError) - proc { @store.find(@request) }.should raise_error(Puppet::Error) + it "should return nil if the hash to diff from isn't found" do + checksum = save_bucket_file("whatever") + bogus_checksum = "d1bf072d0e2c6e20e3fbd23f022089a1" + Puppet::FileBucket::File.find("md5/#{bogus_checksum}", :diff_with => checksum).should == nil + end end - end - describe "when determining file paths" do - before do - Puppet[:bucketdir] = '/dev/null/bucketdir' - @digest = 'DEADBEEFC0FFEE' - @bucket = stub_everything "bucket" - @bucket.expects(:checksum_data).returns(@digest) - end - - it "should use the value of the :bucketdir setting as the root directory" do - path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket) - path.should =~ %r{^/dev/null/bucketdir} - end - - it "should choose a path 8 directories deep with each directory name being the respective character in the filebucket" do - path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket) - dirs = @digest[0..7].split("").join(File::SEPARATOR) - path.should be_include(dirs) - end - - it "should use the full filebucket as the final directory name" do - path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket) - ::File.basename(::File.dirname(path)).should == @digest - end - - it "should use 'contents' as the actual file name" do - path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket) - ::File.basename(path).should == "contents" - end - - it "should use the bucketdir, the 8 sum character directories, the full filebucket, and 'contents' as the full file name" do - path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket) - path.should == ['/dev/null/bucketdir', @digest[0..7].split(""), @digest, "contents"].flatten.join(::File::SEPARATOR) + describe "when initializing" do + it "should use the filebucket settings section" do + Puppet.settings.expects(:use).with(:filebucket) + Puppet::FileBucketFile::File.new end end - describe "when saving files" do - before do - # this is the default from spec_helper, but it keeps getting reset at odd times - Puppet[:bucketdir] = "/dev/null/bucket" - - @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0" - @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0" - @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0' - - @contents = "file contents" - - @bucket = stub "bucket file" - @bucket.stubs(:bucket_path) - @bucket.stubs(:checksum_data).returns(@digest) - @bucket.stubs(:path).returns(nil) - @bucket.stubs(:checksum).returns(nil) - @bucket.stubs(:contents).returns("file contents") - end - - it "should save the contents to the calculated path" do - ::File.stubs(:directory?).with(@dir).returns(true) - ::File.expects(:exist?).with("#{@dir}/contents").returns false - - mockfile = mock "file" - mockfile.expects(:print).with(@contents) - ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440).yields(mockfile) - Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket) - end - - it "should make any directories necessary for storage" do - FileUtils.expects(:mkdir_p).with do |arg| - ::File.umask == 0007 and arg == @dir + [true, false].each do |override_bucket_path| + describe "when bucket path #{if override_bucket_path then 'is' else 'is not' end} overridden" do + [true, false].each do |supply_path| + describe "when #{supply_path ? 'supplying' : 'not supplying'} a path" do + before :each do + Puppet.settings.stubs(:use) + @store = Puppet::FileBucketFile::File.new + @contents = "my content" + + @digest = "f2bfa7fc155c4f42cb91404198dda01f" + @digest.should == Digest::MD5.hexdigest(@contents) + + @bucket_dir = tmpdir("bucket") + + if override_bucket_path + Puppet[:bucketdir] = "/bogus/path" # should not be used + else + Puppet[:bucketdir] = @bucket_dir + end + + @dir = "#{@bucket_dir}/f/2/b/f/a/7/f/c/f2bfa7fc155c4f42cb91404198dda01f" + @contents_path = "#{@dir}/contents" + end + + describe "when retrieving files" do + before :each do + + request_options = {} + if override_bucket_path + request_options[:bucket_path] = @bucket_dir + end + + key = "md5/#{@digest}" + if supply_path + key += "//path/to/file" + end + + @request = Puppet::Indirector::Request.new(:indirection_name, :find, key, request_options) + end + + def make_bucketed_file + FileUtils.mkdir_p(@dir) + File.open(@contents_path, 'w') { |f| f.write @contents } + end + + it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do + make_bucketed_file + + bucketfile = @store.find(@request) + bucketfile.should be_a(Puppet::FileBucket::File) + bucketfile.contents.should == @contents + @store.head(@request).should == true + end + + it "should return nil if no file is found" do + @store.find(@request).should be_nil + @store.head(@request).should == false + end + end + + describe "when saving files" do + it "should save the contents to the calculated path" do + options = {} + if override_bucket_path + options[:bucket_path] = @bucket_dir + end + + key = "md5/#{@digest}" + if supply_path + key += "//path/to/file" + end + + file_instance = Puppet::FileBucket::File.new(@contents, options) + request = Puppet::Indirector::Request.new(:indirection_name, :save, key, file_instance) + + @store.save(request) + File.read("#{@dir}/contents").should == @contents + end + end + end end - ::File.expects(:directory?).with(@dir).returns(false) - ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440) - ::File.expects(:exist?).with("#{@dir}/contents").returns false - - Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket) end end - describe "when verifying identical files" do before do # this is the default from spec_helper, but it keeps getting reset at odd times @@ -231,60 +182,4 @@ describe Puppet::FileBucketFile::File do end end - - - describe "when writing to the paths file" do - before do - Puppet[:bucketdir] = '/dev/null/bucketdir' - @digest = '70924d6fa4b2d745185fa4660703a5c0' - @bucket = stub_everything "bucket" - - @paths_path = '/dev/null/bucketdir/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths' - - @paths = [] - @bucket.stubs(:paths).returns(@paths) - @bucket.stubs(:checksum_data).returns(@digest) - end - - it "should create a file if it doesn't exist" do - @bucket.expects(:path).returns('path/to/save').at_least_once - File.expects(:exist?).with(@paths_path).returns(false) - file = stub "file" - file.expects(:puts).with('path/to/save') - File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file) - - Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket) - end - - it "should append to a file if it exists" do - @bucket.expects(:path).returns('path/to/save').at_least_once - File.expects(:exist?).with(@paths_path).returns(true) - old_file = stub "file" - old_file.stubs(:readlines).returns [] - File.expects(:open).with(@paths_path).yields(old_file) - - file = stub "file" - file.expects(:puts).with('path/to/save') - File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file) - - Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket) - end - - it "should not alter a file if it already contains the path" do - @bucket.expects(:path).returns('path/to/save').at_least_once - File.expects(:exist?).with(@paths_path).returns(true) - old_file = stub "file" - old_file.stubs(:readlines).returns ["path/to/save\n"] - File.expects(:open).with(@paths_path).yields(old_file) - - Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket) - end - - it "should do nothing if there is no path" do - @bucket.expects(:path).returns(nil).at_least_once - - Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket) - end - end - end diff --git a/spec/unit/indirector/file_server_spec.rb b/spec/unit/indirector/file_server_spec.rb index 686f79a24..eafcf2bb7 100755 --- a/spec/unit/indirector/file_server_spec.rb +++ b/spec/unit/indirector/file_server_spec.rb @@ -229,6 +229,12 @@ describe Puppet::Indirector::FileServer do describe "when checking authorization" do before do @request.method = :find + + @mount = stub 'mount' + @configuration.stubs(:split_path).with(@request).returns([@mount, "rel/path"]) + @request.stubs(:node).returns("mynode") + @request.stubs(:ip).returns("myip") + @mount.stubs(:allowed?).with("mynode", "myip").returns "something" end it "should return false when destroying" do @@ -254,13 +260,6 @@ describe Puppet::Indirector::FileServer do end it "should return the results of asking the mount whether the node and IP are authorized" do - @mount = stub 'mount' - @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"]) - - @request.stubs(:node).returns("mynode") - @request.stubs(:ip).returns("myip") - @mount.expects(:allowed?).with("mynode", "myip").returns "something" - @file_server.authorized?(@request).should == "something" end end diff --git a/spec/unit/indirector/indirection_spec.rb b/spec/unit/indirector/indirection_spec.rb index 1e774fb2e..a910cb6f2 100755 --- a/spec/unit/indirector/indirection_spec.rb +++ b/spec/unit/indirector/indirection_spec.rb @@ -383,6 +383,75 @@ describe Puppet::Indirector::Indirection do end end + describe "and doing a head operation" do + before { @method = :head } + + it_should_behave_like "Indirection Delegator" + it_should_behave_like "Delegation Authorizer" + + it "should return true if the head method returned true" do + @terminus.expects(:head).returns(true) + @indirection.head("me").should == true + end + + it "should return false if the head method returned false" do + @terminus.expects(:head).returns(false) + @indirection.head("me").should == false + end + + describe "when caching is enabled" do + before do + @indirection.cache_class = :cache_terminus + @cache_class.stubs(:new).returns(@cache) + + @instance.stubs(:expired?).returns false + end + + it "should first look in the cache for an instance" do + @terminus.stubs(:find).never + @terminus.stubs(:head).never + @cache.expects(:find).returns @instance + + @indirection.head("/my/key").should == true + end + + it "should not save to the cache" do + @cache.expects(:find).returns nil + @cache.expects(:save).never + @terminus.expects(:head).returns true + @indirection.head("/my/key").should == true + end + + it "should not fail if the cache fails" do + @terminus.stubs(:head).returns true + + @cache.expects(:find).raises ArgumentError + lambda { @indirection.head("/my/key") }.should_not raise_error + end + + it "should look in the main terminus if the cache fails" do + @terminus.expects(:head).returns true + @cache.expects(:find).raises ArgumentError + @indirection.head("/my/key").should == true + end + + it "should send a debug log if it is using the cached object" do + Puppet.expects(:debug) + @cache.stubs(:find).returns @instance + + @indirection.head("/my/key") + end + + it "should not accept the cached object if it is expired" do + @instance.stubs(:expired?).returns true + + @cache.stubs(:find).returns @instance + @terminus.stubs(:head).returns false + @indirection.head("/my/key").should == false + end + end + end + describe "and storing a model instance" do before { @method = :save } diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb index 5f0fe363a..7bc1cc513 100755 --- a/spec/unit/indirector/rest_spec.rb +++ b/spec/unit/indirector/rest_spec.rb @@ -282,6 +282,42 @@ describe Puppet::Indirector::REST do end end + describe "when doing a head" do + before :each do + @connection = stub('mock http connection', :head => @response) + @searcher.stubs(:network).returns(@connection) + + # Use a key with spaces, so we can test escaping + @request = Puppet::Indirector::Request.new(:foo, :head, "foo bar") + end + + it "should call the HEAD http method on a network connection" do + @searcher.expects(:network).returns @connection + @connection.expects(:head).returns @response + @searcher.head(@request) + end + + it "should return true if there was a successful http response" do + @connection.expects(:head).returns @response + @response.stubs(:code).returns "200" + + @searcher.head(@request).should == true + end + + it "should return false if there was a successful http response" do + @connection.expects(:head).returns @response + @response.stubs(:code).returns "404" + + @searcher.head(@request).should == false + end + + it "should use the URI generated by the Handler module" do + @searcher.expects(:indirection2uri).with(@request).returns "/my/uri" + @connection.expects(:head).with { |path, args| path == "/my/uri" }.returns(@response) + @searcher.head(@request) + end + end + describe "when doing a search" do before :each do @connection = stub('mock http connection', :get => @response) diff --git a/spec/unit/indirector_spec.rb b/spec/unit/indirector_spec.rb index acbfc1726..6f44764da 100755 --- a/spec/unit/indirector_spec.rb +++ b/spec/unit/indirector_spec.rb @@ -118,6 +118,11 @@ describe Puppet::Indirector, "when redirecting a model" do it_should_behave_like "Delegated Indirection Method" end + describe "when performing a head request" do + before { @method = :head } + it_should_behave_like "Delegated Indirection Method" + end + # This is an instance method, so it behaves a bit differently. describe "when saving instances via the model" do before do diff --git a/spec/unit/network/http/api/v1_spec.rb b/spec/unit/network/http/api/v1_spec.rb index c593242c0..23a291cf3 100644 --- a/spec/unit/network/http/api/v1_spec.rb +++ b/spec/unit/network/http/api/v1_spec.rb @@ -68,6 +68,10 @@ describe Puppet::Network::HTTP::API::V1 do @tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find end + it "should choose 'head' as the indirection method if the http method is a HEAD and the indirection name is singular" do + @tester.uri2indirection("HEAD", "/env/foo/bar", {}).method.should == :head + end + it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do @tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search end diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb index 76a9c5530..8464ae68e 100755 --- a/spec/unit/network/http/handler_spec.rb +++ b/spec/unit/network/http/handler_spec.rb @@ -209,6 +209,12 @@ describe Puppet::Network::HTTP::Handler do @handler.do_find(@irequest, @request, @response) end + it "should pass the result through without rendering it if the result is a string" do + @model_class.stubs(:find).returns "foo" + @handler.expects(:set_response).with(@response, "foo") + @handler.do_find(@irequest, @request, @response) + end + it "should use the default status when a model find call succeeds" do @handler.expects(:set_response).with { |response, body, status| status.nil? } @handler.do_find(@irequest, @request, @response) @@ -250,6 +256,39 @@ describe Puppet::Network::HTTP::Handler do end end + describe "when performing head operation" do + before do + @irequest = stub 'indirection_request', :method => :head, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class + + @model_class.stubs(:head).returns true + end + + it "should use the indirection request to find the model class" do + @irequest.expects(:model).returns @model_class + + @handler.do_head(@irequest, @request, @response) + end + + it "should use the escaped request key" do + @model_class.expects(:head).with do |key, args| + key == "my_result" + end.returns true + @handler.do_head(@irequest, @request, @response) + end + + it "should not generate a response when a model head call succeeds" do + @handler.expects(:set_response).never + @handler.do_head(@irequest, @request, @response) + end + + it "should return a 404 when the model head call returns false" do + @model_class.stubs(:name).returns "my name" + @handler.expects(:set_response).with { |response, body, status| status == 404 } + @model_class.stubs(:head).returns(false) + @handler.do_head(@irequest, @request, @response) + end + end + describe "when searching for model instances" do before do @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb index 06436e723..d629f8670 100755 --- a/spec/unit/network/rest_authconfig_spec.rb +++ b/spec/unit/network/rest_authconfig_spec.rb @@ -47,7 +47,7 @@ describe Puppet::Network::RestAuthConfig do end it "should ask for authorization to the ACL subsystem" do - @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true) + @acl.expects(:is_request_forbidden_and_why?).with(@request).returns(nil) @authconfig.allowed?(@request) end diff --git a/spec/unit/network/rights_spec.rb b/spec/unit/network/rights_spec.rb index 969fc189e..3b9e48374 100755 --- a/spec/unit/network/rights_spec.rb +++ b/spec/unit/network/rights_spec.rb @@ -9,6 +9,26 @@ describe Puppet::Network::Rights do @right = Puppet::Network::Rights.new end + describe "when validating a :head request" do + [:find, :save].each do |allowed_method| + it "should allow the request if only #{allowed_method} is allowed" do + rights = Puppet::Network::Rights.new + rights.newright("/") + rights.allow("/", "*") + rights.restrict_method("/", allowed_method) + rights.restrict_authenticated("/", :any) + request = Puppet::Indirector::Request.new(:indirection_name, :head, "key") + rights.is_request_forbidden_and_why?(request).should == nil + end + end + + it "should disallow the request if neither :find nor :save is allowed" do + rights = Puppet::Network::Rights.new + request = Puppet::Indirector::Request.new(:indirection_name, :head, "key") + rights.is_request_forbidden_and_why?(request).should be_instance_of(Puppet::Network::AuthorizationError) + end + end + [:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |m| it "should have a #{m} method" do @right.should respond_to(m) @@ -155,19 +175,19 @@ describe Puppet::Network::Rights do Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl) end - it "should delegate to fail_on_deny" do - @right.expects(:fail_on_deny).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1") + it "should delegate to is_forbidden_and_why?" do + @right.expects(:is_forbidden_and_why?).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1").returns(nil) @right.allowed?("namespace", "host.domain.com", "127.0.0.1") end - it "should return true if fail_on_deny doesn't fail" do - @right.stubs(:fail_on_deny) + it "should return true if is_forbidden_and_why? returns nil" do + @right.stubs(:is_forbidden_and_why?).returns(nil) @right.allowed?("namespace", :args).should be_true end - it "should return false if fail_on_deny raises an AuthorizationError" do - @right.stubs(:fail_on_deny).raises(Puppet::Network::AuthorizationError.new("forbidden")) + it "should return false if is_forbidden_and_why? returns an AuthorizationError" do + @right.stubs(:is_forbidden_and_why?).returns(Puppet::Network::AuthorizationError.new("forbidden")) @right.allowed?("namespace", :args1, :args2).should be_false end @@ -179,7 +199,7 @@ describe Puppet::Network::Rights do acl.expects(:match?).returns(true) acl.expects(:allowed?).with { |node,ip,h| node == "node" and ip == "ip" }.returns(true) - @right.fail_on_deny("namespace", { :node => "node", :ip => "ip" } ) + @right.is_forbidden_and_why?("namespace", { :node => "node", :ip => "ip" } ).should == nil end it "should then check for path rights if no namespace match" do @@ -195,7 +215,7 @@ describe Puppet::Network::Rights do acl.expects(:allowed?).never @pathacl.expects(:allowed?).returns(true) - @right.fail_on_deny("/path/to/there", {}) + @right.is_forbidden_and_why?("/path/to/there", {}).should == nil end it "should pass the match? return to allowed?" do @@ -204,12 +224,12 @@ describe Puppet::Network::Rights do @pathacl.expects(:match?).returns(:match) @pathacl.expects(:allowed?).with { |node,ip,h| h[:match] == :match }.returns(true) - @right.fail_on_deny("/path/to/there", {}) + @right.is_forbidden_and_why?("/path/to/there", {}).should == nil end describe "with namespace acls" do - it "should raise an error if this namespace right doesn't exist" do - lambda{ @right.fail_on_deny("namespace") }.should raise_error + it "should return an ArgumentError if this namespace right doesn't exist" do + lambda { @right.is_forbidden_and_why?("namespace") }.should raise_error(ArgumentError) end end @@ -235,7 +255,7 @@ describe Puppet::Network::Rights do @long_acl.expects(:allowed?).returns(true) @short_acl.expects(:allowed?).never - @right.fail_on_deny("/path/to/there/and/there", {}) + @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil end it "should select the first match that doesn't return :dunno" do @@ -248,7 +268,7 @@ describe Puppet::Network::Rights do @long_acl.expects(:allowed?).returns(:dunno) @short_acl.expects(:allowed?).returns(true) - @right.fail_on_deny("/path/to/there/and/there", {}) + @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil end it "should not select an ACL that doesn't match" do @@ -261,7 +281,7 @@ describe Puppet::Network::Rights do @long_acl.expects(:allowed?).never @short_acl.expects(:allowed?).returns(true) - @right.fail_on_deny("/path/to/there/and/there", {}) + @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil end it "should not raise an AuthorizationError if allowed" do @@ -270,7 +290,7 @@ describe Puppet::Network::Rights do @long_acl.stubs(:match?).returns(true) @long_acl.stubs(:allowed?).returns(true) - lambda { @right.fail_on_deny("/path/to/there/and/there", {}) }.should_not raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil end it "should raise an AuthorizationError if the match is denied" do @@ -279,11 +299,11 @@ describe Puppet::Network::Rights do @long_acl.stubs(:match?).returns(true) @long_acl.stubs(:allowed?).returns(false) - lambda{ @right.fail_on_deny("/path/to/there", {}) }.should raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/path/to/there", {}).should be_instance_of(Puppet::Network::AuthorizationError) end it "should raise an AuthorizationError if no path match" do - lambda { @right.fail_on_deny("/nomatch", {}) }.should raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/nomatch", {}).should be_instance_of(Puppet::Network::AuthorizationError) end end @@ -309,7 +329,7 @@ describe Puppet::Network::Rights do @regex_acl1.expects(:allowed?).returns(true) @regex_acl2.expects(:allowed?).never - @right.fail_on_deny("/files/repository/myfile/other", {}) + @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil end it "should select the first match that doesn't return :dunno" do @@ -322,7 +342,7 @@ describe Puppet::Network::Rights do @regex_acl1.expects(:allowed?).returns(:dunno) @regex_acl2.expects(:allowed?).returns(true) - @right.fail_on_deny("/files/repository/myfile/other", {}) + @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil end it "should not select an ACL that doesn't match" do @@ -335,7 +355,7 @@ describe Puppet::Network::Rights do @regex_acl1.expects(:allowed?).never @regex_acl2.expects(:allowed?).returns(true) - @right.fail_on_deny("/files/repository/myfile/other", {}) + @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil end it "should not raise an AuthorizationError if allowed" do @@ -344,15 +364,15 @@ describe Puppet::Network::Rights do @regex_acl1.stubs(:match?).returns(true) @regex_acl1.stubs(:allowed?).returns(true) - lambda { @right.fail_on_deny("/files/repository/myfile/other", {}) }.should_not raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil end it "should raise an error if no regex acl match" do - lambda{ @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError) end it "should raise an AuthorizedError on deny" do - lambda { @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError) + @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError) end end diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb index 95f3853e2..687f2ecb9 100755 --- a/spec/unit/parser/compiler_spec.rb +++ b/spec/unit/parser/compiler_spec.rb @@ -105,8 +105,15 @@ describe Puppet::Parser::Compiler do node.classes = %w{foo bar} compiler = Puppet::Parser::Compiler.new(node) - compiler.classlist.should include("foo") - compiler.classlist.should include("bar") + compiler.classlist.should =~ ['foo', 'bar'] + end + + it "should transform node class hashes into a class list" do + node = Puppet::Node.new("mynode") + node.classes = {'foo'=>{'one'=>'1'}, 'bar'=>{'two'=>'2'}} + compiler = Puppet::Parser::Compiler.new(node) + + compiler.classlist.should =~ ['foo', 'bar'] end it "should add a 'main' stage to the catalog" do @@ -185,6 +192,14 @@ describe Puppet::Parser::Compiler do @compiler.class.publicize_methods(:evaluate_node_classes) { @compiler.evaluate_node_classes } end + it "should evaluate any parameterized classes named in the node" do + classes = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}} + @node.stubs(:classes).returns(classes) + @compiler.expects(:evaluate_classes).with(classes, @compiler.topscope) + @compiler.compile + end + + it "should evaluate the main class if it exists" do compile_stub(:evaluate_main) main_class = @known_resource_types.add Puppet::Resource::Type.new(:hostclass, "") @@ -532,7 +547,7 @@ describe Puppet::Parser::Compiler do @compiler.add_collection(coll) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Failed to realize virtual resources something' end it "should fail when there are unevaluated resource collections that refer to multiple specific resources" do @@ -541,7 +556,7 @@ describe Puppet::Parser::Compiler do @compiler.add_collection(coll) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Failed to realize virtual resources one, two' end end @@ -566,6 +581,14 @@ describe Puppet::Parser::Compiler do @scope.expects(:find_hostclass).with("notfound").returns(nil) @compiler.evaluate_classes(%w{notfound}, @scope) end + # I wish it would fail + it "should log when it can't find class" do + klasses = {'foo'=>nil} + @node.classes = klasses + @compiler.topscope.stubs(:find_hostclass).with('foo').returns(nil) + Puppet.expects(:info).with('Could not find class foo for testnode') + @compiler.compile + end end describe "when evaluating found classes" do @@ -586,6 +609,68 @@ describe Puppet::Parser::Compiler do @compiler.evaluate_classes(%w{myclass}, @scope) end + it "should ensure each node class hash is in catalog and have appropriate parameters" do + klasses = {'foo'=>{'1'=>'one'}, 'bar::foo'=>{'2'=>'two'}, 'bar'=>{'1'=> [1,2,3], '2'=>{'foo'=>'bar'}}} + @node.classes = klasses + ast_obj = Puppet::Parser::AST::String.new(:value => 'foo') + klasses.each do |name, params| + klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj}) + @compiler.topscope.known_resource_types.add klass + end + catalog = @compiler.compile + catalog.classes.should =~ ['foo', 'bar::foo', 'settings', 'bar'] + + r1 = catalog.resources.detect {|r| r.title == 'Foo' } + r1.to_hash.should == {:'1' => 'one', :'2' => 'foo'} + r1.tags. should =~ ['class', 'foo'] + + r2 = catalog.resources.detect {|r| r.title == 'Bar::Foo' } + r2.to_hash.should == {:'1' => 'foo', :'2' => 'two'} + r2.tags.should =~ ['bar::foo', 'class', 'bar', 'foo'] + + r2 = catalog.resources.detect {|r| r.title == 'Bar' } + r2.to_hash.should == {:'1' => [1,2,3], :'2' => {'foo'=>'bar'}} + r2.tags.should =~ ['class', 'bar'] + end + + it "should ensure each node class is in catalog and has appropriate tags" do + klasses = ['bar::foo'] + @node.classes = klasses + ast_obj = Puppet::Parser::AST::String.new(:value => 'foo') + klasses.each do |name| + klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj}) + @compiler.topscope.known_resource_types.add klass + end + catalog = @compiler.compile + + r2 = catalog.resources.detect {|r| r.title == 'Bar::Foo' } + r2.tags.should =~ ['bar::foo', 'class', 'bar', 'foo'] + end + + it "should fail if required parameters are missing" do + klass = {'foo'=>{'1'=>'one'}} + @node.classes = klass + klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil}) + @compiler.topscope.known_resource_types.add klass + lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Must pass 2 to Class[Foo]" + end + + it "should fail if invalid parameters are passed" do + klass = {'foo'=>{'3'=>'one'}} + @node.classes = klass + klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil}) + @compiler.topscope.known_resource_types.add klass + lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Invalid parameter 3" + end + + it "should ensure class is in catalog without params" do + @node.classes = klasses = {'foo'=>nil} + foo = Puppet::Resource::Type.new(:hostclass, 'foo') + @compiler.topscope.known_resource_types.add foo + catalog = @compiler.compile + catalog.classes.should include 'foo' + end + it "should not evaluate the resources created for found classes unless asked" do @compiler.catalog.stubs(:tag) @@ -759,7 +844,7 @@ describe Puppet::Parser::Compiler do it "should fail if the compile is finished and resource overrides have not been applied" do @compiler.add_override(@override) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Could not find resource(s) File[/foo] for overriding' end end end diff --git a/spec/unit/property/keyvalue_spec.rb b/spec/unit/property/keyvalue_spec.rb index 7666def56..a44d891d7 100644 --- a/spec/unit/property/keyvalue_spec.rb +++ b/spec/unit/property/keyvalue_spec.rb @@ -134,7 +134,7 @@ describe klass do end end - describe "when calling insync?" do + describe "when calling safe_insync?" do before do @provider = mock("provider") @property.stubs(:provider).returns(@provider) @@ -142,26 +142,26 @@ describe klass do end it "should return true unless @should is defined and not nil" do - @property.insync?("foo") == true + @property.safe_insync?("foo") == true end it "should return true if the passed in values is nil" do @property.should = "foo" - @property.insync?(nil) == true + @property.safe_insync?(nil) == true end it "should return true if hashified should value == (retrieved) value passed in" do @provider.stubs(:prop_name).returns({ :foo => "baz", :bar => "boo" }) @property.should = ["foo=baz", "bar=boo"] @property.expects(:inclusive?).returns(true) - @property.insync?({ :foo => "baz", :bar => "boo" }).must == true + @property.safe_insync?({ :foo => "baz", :bar => "boo" }).must == true end it "should return false if prepared value != should value" do @provider.stubs(:prop_name).returns({ "foo" => "bee", "bar" => "boo" }) @property.should = ["foo=baz", "bar=boo"] @property.expects(:inclusive?).returns(true) - @property.insync?({ "foo" => "bee", "bar" => "boo" }).must == false + @property.safe_insync?({ "foo" => "bee", "bar" => "boo" }).must == false end end end diff --git a/spec/unit/property/list_spec.rb b/spec/unit/property/list_spec.rb index 3e8cc5402..c6c5db10e 100644 --- a/spec/unit/property/list_spec.rb +++ b/spec/unit/property/list_spec.rb @@ -118,39 +118,39 @@ describe list_class do end end - describe "when calling insync?" do + describe "when calling safe_insync?" do it "should return true unless @should is defined and not nil" do - @property.must be_insync("foo") + @property.must be_safe_insync("foo") end it "should return true unless the passed in values is not nil" do @property.should = "foo" - @property.must be_insync(nil) + @property.must be_safe_insync(nil) end it "should call prepare_is_for_comparison with value passed in and should" do @property.should = "foo" @property.expects(:prepare_is_for_comparison).with("bar") @property.expects(:should) - @property.insync?("bar") + @property.safe_insync?("bar") end it "should return true if 'is' value is array of comma delimited should values" do @property.should = "bar,foo" @property.expects(:inclusive?).returns(true) - @property.must be_insync(["bar","foo"]) + @property.must be_safe_insync(["bar","foo"]) end it "should return true if 'is' value is :absent and should value is empty string" do @property.should = "" @property.expects(:inclusive?).returns(true) - @property.must be_insync([]) + @property.must be_safe_insync([]) end it "should return false if prepared value != should value" do @property.should = "bar,baz,foo" @property.expects(:inclusive?).returns(true) - @property.must_not be_insync(["bar","foo"]) + @property.must_not be_safe_insync(["bar","foo"]) end end diff --git a/spec/unit/provider/mount_spec.rb b/spec/unit/provider/mount_spec.rb index b034214ee..f567a4a40 100755 --- a/spec/unit/provider/mount_spec.rb +++ b/spec/unit/provider/mount_spec.rb @@ -120,6 +120,14 @@ describe Puppet::Provider::Mount do @mounter.should be_mounted end + it "should match mounted devices if the operating system is AIX" do + Facter.stubs(:value).with("operatingsystem").returns("AIX") + mount_data = File.read(File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'unit', 'provider', 'mount', 'mount-output.aix.txt')) + @mounter.expects(:mountcmd).returns(mount_data) + + @mounter.should be_mounted + end + it "should match ' on <name>' if the operating system is not Darwin, Solaris, or HP-UX" do Facter.stubs(:value).with("operatingsystem").returns("Debian") @mounter.expects(:mountcmd).returns("/dev/dsk/whatever on / and stuff\n/dev/other/disk on /var and stuff") diff --git a/spec/unit/provider/nameservice/directoryservice_spec.rb b/spec/unit/provider/nameservice/directoryservice_spec.rb new file mode 100755 index 000000000..661899db9 --- /dev/null +++ b/spec/unit/provider/nameservice/directoryservice_spec.rb @@ -0,0 +1,38 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +# We use this as a reasonable way to obtain all the support infrastructure. +[:user, :group].each do |type_for_this_round| + provider_class = Puppet::Type.type(type_for_this_round).provider(:directoryservice) + + describe provider_class do + before do + @resource = stub("resource") + @provider = provider_class.new(@resource) + end + + it "[#6009] should handle nested arrays of members" do + current = ["foo", "bar", "baz"] + desired = ["foo", ["quux"], "qorp"] + group = 'example' + + @resource.stubs(:[]).with(:name).returns(group) + @resource.stubs(:[]).with(:auth_membership).returns(true) + @provider.instance_variable_set(:@property_value_cache_hash, + { :members => current }) + + %w{bar baz}.each do |del| + @provider.expects(:execute).once. + with([:dseditgroup, '-o', 'edit', '-n', '.', '-d', del, group]) + end + + %w{quux qorp}.each do |add| + @provider.expects(:execute).once. + with([:dseditgroup, '-o', 'edit', '-n', '.', '-a', add, group]) + end + + expect { @provider.set(:members, desired) }.should_not raise_error + end + end +end diff --git a/spec/unit/provider/package/freebsd_spec.rb b/spec/unit/provider/package/freebsd_spec.rb new file mode 100755 index 000000000..0d38a16cf --- /dev/null +++ b/spec/unit/provider/package/freebsd_spec.rb @@ -0,0 +1,55 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +provider_class = Puppet::Type.type(:package).provider(:freebsd) + +describe provider_class do + before :each do + # Create a mock resource + @resource = stub 'resource' + + # A catch all; no parameters set + @resource.stubs(:[]).returns(nil) + + # But set name and source + @resource.stubs(:[]).with(:name).returns "mypackage" + @resource.stubs(:[]).with(:ensure).returns :installed + + @provider = provider_class.new + @provider.resource = @resource + end + + it "should have an install method" do + @provider = provider_class.new + @provider.should respond_to(:install) + end + + describe "when installing" do + before :each do + @resource.stubs(:should).with(:ensure).returns(:installed) + end + + it "should install a package from a path to a directory" do + # For better or worse, trailing '/' is needed. --daniel 2011-01-26 + path = '/path/to/directory/' + @resource.stubs(:[]).with(:source).returns(path) + Puppet::Util::Execution.expects(:withenv).once.with({:PKG_PATH => path}).yields + @provider.expects(:pkgadd).once.with("mypackage") + + expect { @provider.install }.should_not raise_error + end + + %w{http https ftp}.each do |protocol| + it "should install a package via #{protocol}" do + # For better or worse, trailing '/' is needed. --daniel 2011-01-26 + path = "#{protocol}://localhost/" + @resource.stubs(:[]).with(:source).returns(path) + Puppet::Util::Execution.expects(:withenv).once.with({:PACKAGESITE => path}).yields + @provider.expects(:pkgadd).once.with('-r', "mypackage") + + expect { @provider.install }.should_not raise_error + end + end + end +end diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb index 7b240bb82..87b4ab420 100755 --- a/spec/unit/resource/type_spec.rb +++ b/spec/unit/resource/type_spec.rb @@ -261,6 +261,28 @@ describe Puppet::Resource::Type do @type = Puppet::Resource::Type.new(:hostclass, "foo") end + ['module_name', 'name', 'title'].each do |variable| + it "should allow #{variable} to be evaluated as param default" do + @type.module_name = "bar" + var = Puppet::Parser::AST::Variable.new({'value' => variable}) + @type.set_arguments :foo => var + @type.set_resource_parameters(@resource, @scope) + @scope.lookupvar('foo').should == 'bar' + end + end + + # this test is to clarify a crazy edge case + # if you specify these special names as params, the resource + # will override the special variables + it "resource should override defaults" do + @type.set_arguments :name => nil + @resource[:name] = 'foobar' + var = Puppet::Parser::AST::Variable.new({'value' => 'name'}) + @type.set_arguments :foo => var + @type.set_resource_parameters(@resource, @scope) + @scope.lookupvar('foo').should == 'foobar' + end + it "should set each of the resource's parameters as variables in the scope" do @type.set_arguments :foo => nil, :boo => nil @resource[:foo] = "bar" @@ -496,7 +518,7 @@ describe Puppet::Resource::Type do it "should evaluate the parent's resource" do @type.parent_type(@scope) - + @type.evaluate_code(@resource) @scope.class_scope(@parent_type).should_not be_nil @@ -504,7 +526,7 @@ describe Puppet::Resource::Type do it "should not evaluate the parent's resource if it has already been evaluated" do @parent_resource.evaluate - + @type.parent_type(@scope) @parent_resource.expects(:evaluate).never @@ -545,7 +567,7 @@ describe Puppet::Resource::Type do it "should not evaluate the parent's resource if it has already been evaluated" do @parent_resource.evaluate - + @type.parent_type(@scope) @parent_resource.expects(:evaluate).never @@ -575,7 +597,7 @@ describe Puppet::Resource::Type do @code = Puppet::Resource::TypeCollection.new("env") @code.add @top @code.add @middle - + @node.environment.stubs(:known_resource_types).returns(@code) end @@ -601,12 +623,36 @@ describe Puppet::Resource::Type do @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource) end + it "should add specified parameters to the resource" do + @top.ensure_in_catalog(@scope, {'one'=>'1', 'two'=>'2'}) + @compiler.catalog.resource(:class, "top")['one'].should == '1' + @compiler.catalog.resource(:class, "top")['two'].should == '2' + end + + it "should not require params for a param class" do + @top.ensure_in_catalog(@scope, {}) + @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource) + end + it "should evaluate the parent class if one exists" do @middle.ensure_in_catalog(@scope) @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource) end + it "should evaluate the parent class if one exists" do + @middle.ensure_in_catalog(@scope, {}) + + @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource) + end + + it "should fail if you try to create duplicate class resources" do + othertop = Puppet::Parser::Resource.new(:class, 'top',:source => @source, :scope => @scope ) + # add the same class resource to the catalog + @compiler.catalog.add_resource(othertop) + lambda { @top.ensure_in_catalog(@scope, {}) }.should raise_error(Puppet::Resource::Catalog::DuplicateResourceError) + end + it "should fail to evaluate if a parent class is defined but cannot be found" do othertop = Puppet::Resource::Type.new :hostclass, "something", :parent => "yay" @code.add othertop diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb index 65c148a93..104c19e85 100755 --- a/spec/unit/transaction/resource_harness_spec.rb +++ b/spec/unit/transaction/resource_harness_spec.rb @@ -64,6 +64,90 @@ describe Puppet::Transaction::ResourceHarness do end end + def make_stub_provider + stubProvider = Class.new(Puppet::Type) + stubProvider.instance_eval do + initvars + + newparam(:name) do + desc "The name var" + isnamevar + end + + newproperty(:foo) do + desc "A property that can be changed successfully" + def sync + end + + def retrieve + :absent + end + + def insync?(reference_value) + false + end + end + + newproperty(:bar) do + desc "A property that raises an exception when you try to change it" + def sync + raise ZeroDivisionError.new('bar') + end + + def retrieve + :absent + end + + def insync?(reference_value) + false + end + end + end + stubProvider + end + + describe "when an error occurs" do + before :each do + stub_provider = make_stub_provider + resource = stub_provider.new :name => 'name', :foo => 1, :bar => 2 + resource.expects(:err).never + @status = @harness.evaluate(resource) + end + + it "should record previous successful events" do + @status.events[0].property.should == 'foo' + @status.events[0].status.should == 'success' + end + + it "should record a failure event" do + @status.events[1].property.should == 'bar' + @status.events[1].status.should == 'failure' + end + end + + describe "when auditing" do + it "should not call insync? on parameters that are merely audited" do + stub_provider = make_stub_provider + resource = stub_provider.new :name => 'name', :audit => ['foo'] + resource.property(:foo).expects(:insync?).never + status = @harness.evaluate(resource) + status.events.each do |event| + event.status.should != 'failure' + end + end + + it "should be able to audit a file's group" do # see bug #5710 + test_file = tmpfile('foo') + File.open(test_file, 'w').close + resource = Puppet::Type.type(:file).new :path => test_file, :audit => ['group'], :backup => false + resource.expects(:err).never # make sure no exceptions get swallowed + status = @harness.evaluate(resource) + status.events.each do |event| + event.status.should != 'failure' + end + end + end + describe "when applying changes" do [false, true].each do |noop_mode|; describe (noop_mode ? "in noop mode" : "in normal mode") do [nil, '750'].each do |machine_state|; describe (machine_state ? "with a file initially present" : "with no file initially present") do diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb index cde643fc8..9178c94bf 100755 --- a/spec/unit/type/file/content_spec.rb +++ b/spec/unit/type/file/content_spec.rb @@ -141,17 +141,20 @@ describe content do it "should return true if the resource shouldn't be a regular file" do @resource.expects(:should_be_file?).returns false - @content.must be_insync("whatever") + @content.should = "foo" + @content.must be_safe_insync("whatever") end it "should return false if the current content is :absent" do - @content.should_not be_insync(:absent) + @content.should = "foo" + @content.should_not be_safe_insync(:absent) end it "should return false if the file should be a file but is not present" do @resource.expects(:should_be_file?).returns true + @content.should = "foo" - @content.should_not be_insync(:absent) + @content.should_not be_safe_insync(:absent) end describe "and the file exists" do @@ -161,12 +164,12 @@ describe content do it "should return false if the current contents are different from the desired content" do @content.should = "some content" - @content.should_not be_insync("other content") + @content.should_not be_safe_insync("other content") end it "should return true if the sum for the current contents is the same as the sum for the desired content" do @content.should = "some content" - @content.must be_insync("{md5}" + Digest::MD5.hexdigest("some content")) + @content.must be_safe_insync("{md5}" + Digest::MD5.hexdigest("some content")) end describe "and Puppet[:show_diff] is set" do @@ -179,14 +182,14 @@ describe content do @content.expects(:diff).returns("my diff").once @content.expects(:print).with("my diff").once - @content.insync?("other content") + @content.safe_insync?("other content") end it "should not display a diff if the sum for the current contents is the same as the sum for the desired content" do @content.should = "some content" @content.expects(:diff).never - @content.insync?("{md5}" + Digest::MD5.hexdigest("some content")) + @content.safe_insync?("{md5}" + Digest::MD5.hexdigest("some content")) end end end @@ -199,17 +202,18 @@ describe content do it "should be insync if the file exists and the content is different" do @resource.stubs(:stat).returns mock('stat') - @content.must be_insync("whatever") + @content.must be_safe_insync("whatever") end it "should be insync if the file exists and the content is right" do @resource.stubs(:stat).returns mock('stat') - @content.must be_insync("something") + @content.must be_safe_insync("something") end it "should not be insync if the file does not exist" do - @content.should_not be_insync(:absent) + @content.should = "foo" + @content.should_not be_safe_insync(:absent) end end end @@ -457,5 +461,54 @@ describe content do describe "from a filebucket" do end + + # These are testing the implementation rather than the desired behaviour; while that bites, there are a whole + # pile of other methods in the File type that depend on intimate details of this implementation and vice-versa. + # If these blow up, you are gonna have to review the callers to make sure they don't explode! --daniel 2011-02-01 + describe "each_chunk_from should work" do + before do + @content = content.new(:resource => @resource) + end + + it "when content is a string" do + @content.each_chunk_from('i_am_a_string') { |chunk| chunk.should == 'i_am_a_string' } + end + + it "when no content, source, but ensure present" do + @resource[:ensure] = :present + @content.each_chunk_from(nil) { |chunk| chunk.should == '' } + end + + it "when no content, source, but ensure file" do + @resource[:ensure] = :file + @content.each_chunk_from(nil) { |chunk| chunk.should == '' } + end + + it "when no content or source" do + @content.expects(:read_file_from_filebucket).once.returns('im_a_filebucket') + @content.each_chunk_from(nil) { |chunk| chunk.should == 'im_a_filebucket' } + end + + it "when running as puppet apply" do + @content.class.expects(:standalone?).returns true + source_or_content = stubs('source_or_content') + source_or_content.expects(:content).once.returns :whoo + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == :whoo } + end + + it "when running from source with a local file" do + source_or_content = stubs('source_or_content') + source_or_content.expects(:local?).returns true + @content.expects(:chunk_file_from_disk).with(source_or_content).once.yields 'woot' + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == 'woot' } + end + + it "when running from source with a remote file" do + source_or_content = stubs('source_or_content') + source_or_content.expects(:local?).returns false + @content.expects(:chunk_file_from_source).with(source_or_content).once.yields 'woot' + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == 'woot' } + end + end end end diff --git a/spec/unit/type/file/ensure_spec.rb b/spec/unit/type/file/ensure_spec.rb index ec53ed85a..dbb3a1053 100755 --- a/spec/unit/type/file/ensure_spec.rb +++ b/spec/unit/type/file/ensure_spec.rb @@ -41,44 +41,45 @@ describe property do end it "should always be in sync if replace is 'false' unless the file is missing" do + @ensure.should = :file @resource.expects(:replace?).returns false - @ensure.insync?(:link).should be_true + @ensure.safe_insync?(:link).should be_true end it "should be in sync if :ensure is set to :absent and the file does not exist" do @ensure.should = :absent - @ensure.must be_insync(:absent) + @ensure.must be_safe_insync(:absent) end it "should not be in sync if :ensure is set to :absent and the file exists" do @ensure.should = :absent - @ensure.should_not be_insync(:file) + @ensure.should_not be_safe_insync(:file) end it "should be in sync if a normal file exists and :ensure is set to :present" do @ensure.should = :present - @ensure.must be_insync(:file) + @ensure.must be_safe_insync(:file) end it "should be in sync if a directory exists and :ensure is set to :present" do @ensure.should = :present - @ensure.must be_insync(:directory) + @ensure.must be_safe_insync(:directory) end it "should be in sync if a symlink exists and :ensure is set to :present" do @ensure.should = :present - @ensure.must be_insync(:link) + @ensure.must be_safe_insync(:link) end it "should not be in sync if :ensure is set to :file and a directory exists" do @ensure.should = :file - @ensure.should_not be_insync(:directory) + @ensure.should_not be_safe_insync(:directory) end end end diff --git a/spec/unit/type/file/group_spec.rb b/spec/unit/type/file/group_spec.rb index 2283b57fa..956cd57e7 100755 --- a/spec/unit/type/file/group_spec.rb +++ b/spec/unit/type/file/group_spec.rb @@ -56,19 +56,19 @@ describe property do describe "when determining if the file is in sync" do it "should directly compare the group values if the desired group is an integer" do @group.should = [10] - @group.must be_insync(10) + @group.must be_safe_insync(10) end it "should treat numeric strings as integers" do @group.should = ["10"] - @group.must be_insync(10) + @group.must be_safe_insync(10) end it "should convert the group name to an integer if the desired group is a string" do @group.expects(:gid).with("foo").returns 10 @group.should = %w{foo} - @group.must be_insync(10) + @group.must be_safe_insync(10) end it "should not validate that groups exist when a group is specified as an integer" do @@ -80,12 +80,12 @@ describe property do @group.expects(:gid).with("foo").returns nil @group.should = %w{foo} - lambda { @group.insync?(10) }.should raise_error(Puppet::Error) + lambda { @group.safe_insync?(10) }.should raise_error(Puppet::Error) end it "should return false if the groups are not equal" do @group.should = [10] - @group.should_not be_insync(20) + @group.should_not be_safe_insync(20) end end diff --git a/spec/unit/type/file/owner_spec.rb b/spec/unit/type/file/owner_spec.rb index 8e136a187..bcb8e07d6 100755 --- a/spec/unit/type/file/owner_spec.rb +++ b/spec/unit/type/file/owner_spec.rb @@ -68,7 +68,7 @@ describe property do @provider.expects(:warnonce) @owner.should = [10] - @owner.must be_insync(20) + @owner.must be_safe_insync(20) end end @@ -77,24 +77,24 @@ describe property do end it "should be in sync if 'should' is not provided" do - @owner.must be_insync(10) + @owner.must be_safe_insync(10) end it "should directly compare the owner values if the desired owner is an integer" do @owner.should = [10] - @owner.must be_insync(10) + @owner.must be_safe_insync(10) end it "should treat numeric strings as integers" do @owner.should = ["10"] - @owner.must be_insync(10) + @owner.must be_safe_insync(10) end it "should convert the owner name to an integer if the desired owner is a string" do @provider.expects(:uid).with("foo").returns 10 @owner.should = %w{foo} - @owner.must be_insync(10) + @owner.must be_safe_insync(10) end it "should not validate that users exist when a user is specified as an integer" do @@ -106,12 +106,12 @@ describe property do @provider.expects(:uid).with("foo").returns nil @owner.should = %w{foo} - lambda { @owner.insync?(10) }.should raise_error(Puppet::Error) + lambda { @owner.safe_insync?(10) }.should raise_error(Puppet::Error) end it "should return false if the owners are not equal" do @owner.should = [10] - @owner.should_not be_insync(20) + @owner.should_not be_safe_insync(20) end end diff --git a/spec/unit/type/file/selinux_spec.rb b/spec/unit/type/file/selinux_spec.rb index 1ca59e9e7..043471dec 100644 --- a/spec/unit/type/file/selinux_spec.rb +++ b/spec/unit/type/file/selinux_spec.rb @@ -73,10 +73,10 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f @sel.sync end - it "should do nothing for insync? if no SELinux support" do + it "should do nothing for safe_insync? if no SELinux support" do @sel.should = %{newcontext} @sel.expects(:selinux_support?).returns false - @sel.insync?("oldcontext").should == true + @sel.safe_insync?("oldcontext").should == true end end end diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb index 22921d85a..90f3daf09 100755 --- a/spec/unit/type/file_spec.rb +++ b/spec/unit/type/file_spec.rb @@ -194,6 +194,23 @@ describe Puppet::Type.type(:file) do file = Puppet::Type::File.new(:path => "/") file[:path].should == "/" end + + it "should accept a double-slash at the start of the path" do + expect { + file = Puppet::Type::File.new(:path => "//tmp/xxx") + # REVISIT: This should be wrong, later. See the next test. + # --daniel 2011-01-31 + file[:path].should == '/tmp/xxx' + }.should_not raise_error + end + + # REVISIT: This is pending, because I don't want to try and audit the + # entire codebase to make sure we get this right. POSIX treats two (and + # exactly two) '/' characters at the start of the path specially. + # + # See sections 3.2 and 4.11, which allow DomainOS to be all special like + # and still have the POSIX branding and all. --daniel 2011-01-31 + it "should preserve the double-slash at the start of the path" end describe "on Microsoft Windows systems" do @@ -806,6 +823,73 @@ describe Puppet::Type.type(:file) do end end + describe "when specifying both source, and content properties" do + before do + @file[:source] = '/one' + @file[:content] = 'file contents' + end + + it "should raise an exception" do + lambda {@file.validate }.should raise_error(/You cannot specify more than one of/) + end + end + + describe "when using source" do + before do + @file[:source] = '/one' + end + Puppet::Type::File::ParameterChecksum.value_collection.values.reject {|v| v == :none}.each do |checksum_type| + describe "with checksum '#{checksum_type}'" do + before do + @file[:checksum] = checksum_type + end + + it 'should validate' do + + lambda { @file.validate }.should_not raise_error + end + end + end + + describe "with checksum 'none'" do + before do + @file[:checksum] = :none + end + + it 'should raise an exception when validating' do + lambda { @file.validate }.should raise_error(/You cannot specify source when using checksum 'none'/) + end + end + end + + describe "when using content" do + before do + @file[:content] = 'file contents' + end + + (Puppet::Type::File::ParameterChecksum.value_collection.values - SOURCE_ONLY_CHECKSUMS).each do |checksum_type| + describe "with checksum '#{checksum_type}'" do + before do + @file[:checksum] = checksum_type + end + + it 'should validate' do + lambda { @file.validate }.should_not raise_error + end + end + end + + SOURCE_ONLY_CHECKSUMS.each do |checksum_type| + describe "with checksum '#{checksum_type}'" do + it 'should raise an exception when validating' do + @file[:checksum] = checksum_type + + lambda { @file.validate }.should raise_error(/You cannot specify content when using checksum '#{checksum_type}'/) + end + end + end + end + describe "when returning resources with :eval_generate" do before do @graph = stub 'graph', :add_edge => nil @@ -1065,4 +1149,45 @@ describe Puppet::Type.type(:file) do end end + describe "when auditing" do + it "should not fail if creating a new file if group is not set" do + File.exists?(@path).should == false + file = Puppet::Type::File.new(:name => @path, :audit => "all", :content => "content") + catalog = Puppet::Resource::Catalog.new + catalog.add_resource(file) + + Puppet::Util::Storage.stubs(:store) # to prevent the catalog from trying to write state.yaml + transaction = catalog.apply + + transaction.report.resource_statuses["File[#{@path}]"].failed.should == false + File.exists?(@path).should == true + end + + it "should not log errors if creating a new file with ensure present and no content" do + File.exists?(@path).should == false + file = Puppet::Type::File.new(:name => @path, :audit => "content", :ensure => "present") + catalog = Puppet::Resource::Catalog.new + catalog.add_resource(file) + + Puppet::Util::Storage.stubs(:store) # to prevent the catalog from trying to write state.yaml + + catalog.apply + @logs.reject {|l| l.level == :notice }.should be_empty + end + end + + describe "when specifying both source and checksum" do + it 'should use the specified checksum when source is first' do + @file[:source] = '/foo' + @file[:checksum] = :md5lite + + @file[:checksum].should be :md5lite + end + it 'should use the specified checksum when source is last' do + @file[:checksum] = :md5lite + @file[:source] = '/foo' + + @file[:checksum].should be :md5lite + end + end end diff --git a/spec/unit/type/mount_spec.rb b/spec/unit/type/mount_spec.rb index ce82cb516..0d74042e3 100755 --- a/spec/unit/type/mount_spec.rb +++ b/spec/unit/type/mount_spec.rb @@ -184,22 +184,22 @@ describe Puppet::Type.type(:mount)::Ensure do it "should be insync if it is mounted and should be defined" do @ensure.should = :defined - @ensure.insync?(:mounted).should == true + @ensure.safe_insync?(:mounted).should == true end it "should be insync if it is unmounted and should be defined" do @ensure.should = :defined - @ensure.insync?(:unmounted).should == true + @ensure.safe_insync?(:unmounted).should == true end it "should be insync if it is mounted and should be present" do @ensure.should = :present - @ensure.insync?(:mounted).should == true + @ensure.safe_insync?(:mounted).should == true end it "should be insync if it is unmounted and should be present" do @ensure.should = :present - @ensure.insync?(:unmounted).should == true + @ensure.safe_insync?(:unmounted).should == true end end diff --git a/spec/unit/type/ssh_authorized_key_spec.rb b/spec/unit/type/ssh_authorized_key_spec.rb index a0b435f80..666616c03 100755 --- a/spec/unit/type/ssh_authorized_key_spec.rb +++ b/spec/unit/type/ssh_authorized_key_spec.rb @@ -134,7 +134,7 @@ describe ssh_authorized_key do it "should not raise spurious change events" do resource = @class.new(:name => "Test", :user => "root") target = File.expand_path("~root/.ssh/authorized_keys") - resource.property(:target).insync?(target).should == true + resource.property(:target).safe_insync?(target).should == true end end diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb index ccea9ee4c..297134446 100755 --- a/spec/unit/type/user_spec.rb +++ b/spec/unit/type/user_spec.rb @@ -201,21 +201,21 @@ describe user do it "should return true if no 'should' values are set" do @gid = user.attrclass(:gid).new(:resource => @resource) - @gid.must be_insync(500) + @gid.must be_safe_insync(500) end it "should return true if any of the specified groups are equal to the current integer" do Puppet::Util.expects(:gid).with("foo").returns 300 Puppet::Util.expects(:gid).with("bar").returns 500 - @gid.must be_insync(500) + @gid.must be_safe_insync(500) end it "should return false if none of the specified groups are equal to the current integer" do Puppet::Util.expects(:gid).with("foo").returns 300 Puppet::Util.expects(:gid).with("bar").returns 500 - @gid.should_not be_insync(700) + @gid.should_not be_safe_insync(700) end end @@ -245,6 +245,34 @@ describe user do end end + describe "when managing minimum password age" do + before do + @age = user.attrclass(:password_min_age).new(:resource => @resource) + end + + it "should accept a negative minimum age" do + expect { @age.should = -1 }.should_not raise_error + end + + it "should fail with an empty minimum age" do + expect { @age.should = '' }.should raise_error(Puppet::Error) + end + end + + describe "when managing maximum password age" do + before do + @age = user.attrclass(:password_max_age).new(:resource => @resource) + end + + it "should accept a negative maximum age" do + expect { @age.should = -1 }.should_not raise_error + end + + it "should fail with an empty maximum age" do + expect { @age.should = '' }.should raise_error(Puppet::Error) + end + end + describe "when managing passwords" do before do @password = user.attrclass(:password).new(:resource => @resource, :should => "mypass") diff --git a/spec/unit/type/zpool_spec.rb b/spec/unit/type/zpool_spec.rb index db12459ab..be8cb12ba 100755 --- a/spec/unit/type/zpool_spec.rb +++ b/spec/unit/type/zpool_spec.rb @@ -38,27 +38,27 @@ describe vdev_property do it "should be insync if the devices are the same" do @property.should = ["dev1 dev2"] - @property.insync?(["dev2 dev1"]).must be_true + @property.safe_insync?(["dev2 dev1"]).must be_true end it "should be out of sync if the devices are not the same" do @property.should = ["dev1 dev3"] - @property.insync?(["dev2 dev1"]).must be_false + @property.safe_insync?(["dev2 dev1"]).must be_false end it "should be insync if the devices are the same and the should values are comma seperated" do @property.should = ["dev1", "dev2"] - @property.insync?(["dev2 dev1"]).must be_true + @property.safe_insync?(["dev2 dev1"]).must be_true end it "should be out of sync if the device is absent and should has a value" do @property.should = ["dev1", "dev2"] - @property.insync?(:absent).must be_false + @property.safe_insync?(:absent).must be_false end it "should be insync if the device is absent and should is absent" do @property.should = [:absent] - @property.insync?(:absent).must be_true + @property.safe_insync?(:absent).must be_true end end @@ -73,38 +73,38 @@ describe multi_vdev_property do it "should be insync if the devices are the same" do @property.should = ["dev1 dev2"] - @property.insync?(["dev2 dev1"]).must be_true + @property.safe_insync?(["dev2 dev1"]).must be_true end it "should be out of sync if the devices are not the same" do @property.should = ["dev1 dev3"] - @property.insync?(["dev2 dev1"]).must be_false + @property.safe_insync?(["dev2 dev1"]).must be_false end it "should be out of sync if the device is absent and should has a value" do @property.should = ["dev1", "dev2"] - @property.insync?(:absent).must be_false + @property.safe_insync?(:absent).must be_false end it "should be insync if the device is absent and should is absent" do @property.should = [:absent] - @property.insync?(:absent).must be_true + @property.safe_insync?(:absent).must be_true end describe "when there are multiple lists of devices" do it "should be in sync if each group has the same devices" do @property.should = ["dev1 dev2", "dev3 dev4"] - @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_true + @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_true end it "should be out of sync if any group has the different devices" do @property.should = ["dev1 devX", "dev3 dev4"] - @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_false + @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_false end it "should be out of sync if devices are in the wrong group" do @property.should = ["dev1 dev2", "dev3 dev4"] - @property.insync?(["dev2 dev3", "dev1 dev4"]).must be_false + @property.safe_insync?(["dev2 dev3", "dev1 dev4"]).must be_false end end end diff --git a/spec/unit/util/zaml_spec.rb b/spec/unit/util/zaml_spec.rb index f2bcefe01..59590c571 100755 --- a/spec/unit/util/zaml_spec.rb +++ b/spec/unit/util/zaml_spec.rb @@ -35,5 +35,26 @@ describe "Pure ruby yaml implementation" do lambda { YAML.load(o.to_yaml) }.should_not raise_error end } + + it "should handle references to Array in Hash values correctly" do + list = [1] + data = { "one" => list, "two" => list } + data.to_yaml.should == "--- \n two: &id001 \n - 1\n one: *id001" + expect { YAML.load(data.to_yaml).should == data }.should_not raise_error + end + + it "should handle references to Hash in Hash values correctly" do + hash = { 1 => 1 } + data = { "one" => hash, "two" => hash } + data.to_yaml.should == "--- \n two: &id001 \n 1: 1\n one: *id001" + expect { YAML.load(data.to_yaml).should == data }.should_not raise_error + end + + it "should handle references to Scalar in Hash" do + str = "hello" + data = { "one" => str, "two" => str } + data.to_yaml.should == "--- \n two: &id001 hello\n one: *id001" + expect { YAML.load(data.to_yaml).should == data }.should_not raise_error + end end |