From 4c9eca1cab3caae8050807fc71b24408fce85e69 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 6 Jan 2011 10:55:21 -0800 Subject: Maint: Added "skipped" to the YAML output for Puppet::Resource::Status In 2.6.4 and earlier, "skipped" appeared only when true. A recent change accidentally caused it to never appear. This change makes it always appear. --- lib/puppet/resource/status.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb index ee83004bb..a9a64425e 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} # Provide a boolean method for each of the states. STATES.each do |attr| @@ -51,6 +51,7 @@ module Puppet @out_of_sync_count = 0 @changed = false @out_of_sync = false + @skipped = false [:file, :line].each do |attr| send(attr.to_s + "=", resource.send(attr)) -- cgit From df653045aee21ae53e0e94d47e0edbdaf308fbf5 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Thu, 6 Jan 2011 11:24:18 -0800 Subject: maint: Inspect reports should have audited = true on events Paired-with: Jesse Wolfe --- lib/puppet/application/inspect.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index c7be893c7..07ee4c317 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -65,7 +65,13 @@ class Puppet::Application::Inspect < Puppet::Application 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}") + 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 -- cgit From 8f314f2bb14d61a2a26b67d002d90f84349b25b5 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Fri, 7 Jan 2011 15:39:52 -0800 Subject: (#5710) Removed unnecessary calls to insync? In the resource harness, we were calling insync? on all properties of a resource, even if those properties weren't being managed. This was unsafe. Changed to only call insync? on properties that are mentioned in the manifest. In addition, we discovered that the resource harness's computation of desired_values was incorrect, and was consulting the current system state rather than the desired state. Since this hash was (erroneously) only being consulted to see if it included :ensure, this didn't cause any obvious bugs. However it is now fixed. Paired-with: Jesse Wolfe --- lib/puppet/transaction/resource_harness.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb index c259d3e05..c1b980632 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 = [] @@ -55,7 +58,7 @@ class Puppet::Transaction::ResourceHarness 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.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 -- cgit From a002231f45339da9b152162c2f48126d95e2e246 Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Mon, 10 Jan 2011 18:30:42 -0800 Subject: (#5171) Made filebucket able to perform diffs It is now possible to ask the filebucket to diff two files using a URL of the form: https://puppet/production/file_bucket_file/md5/{first file hash}?diff_with={second file hash} The returned diff is a string, the output of the "diff" command. Paired-with: Paul Berry --- lib/puppet/indirector/file_bucket_file/file.rb | 11 ++++++++++- lib/puppet/indirector/indirection.rb | 2 +- lib/puppet/network/http/handler.rb | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb index 318858aaf..9d9cee793 100644 --- a/lib/puppet/indirector/file_bucket_file/file.rb +++ b/lib/puppet/indirector/file_bucket_file/file.rb @@ -15,7 +15,16 @@ module Puppet::FileBucketFile def find( request ) checksum, path = request_to_checksum_and_path( request ) - find_by_checksum( checksum, request.options ) + file = find_by_checksum( checksum, request.options ) + + if file && request.options[:diff_with] + hash_protocol = sumtype(checksum) + file2 = find_by_checksum( "{#{hash_protocol}}#{request.options[:diff_with]}", request.options ) + raise "could not find diff_with #{request.options[:diff_with]}" unless file2 + return `diff #{path_for(file).inspect}/contents #{path_for(file2).inspect}/contents` + end + + file end def save( request ) diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 309eed7b6..a010c4e40 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -191,7 +191,7 @@ class Puppet::Indirector::Indirection # 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) diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 61ae2d2fc..f22498b70 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -109,7 +109,11 @@ 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 search. -- cgit From 08561b22920aa5eaa76addd8b0da8feb189e0d18 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 11 Jan 2011 14:56:14 -0800 Subject: (#5838) Refactored Puppet::Network::Rights#fail_on_deny Changed into a method that returns the exception to raised rather than raising it. Paired-with: Jesse Wolfe --- lib/puppet/network/rest_authconfig.rb | 18 ++++++++++-------- lib/puppet/network/rights.rb | 20 +++++--------------- 2 files changed, 15 insertions(+), 23 deletions(-) (limited to 'lib') diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index 7abe06956..1704ea0c1 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -38,14 +38,16 @@ 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_forbidden_and_why?( + build_uri(request), + :node => request.node, + :ip => request.ip, + :method => request.method, + :environment => request.environment, + :authenticated => request.authenticated) + Puppet.warning("Denying access: #{authorization_failure_exception}") + raise authorization_failure_exception + end end def initialize(file = nil, parsenow = true) diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb index e3cd3179a..b2146494c 100755 --- a/lib/puppet/network/rights.rb +++ b/lib/puppet/network/rights.rb @@ -26,19 +26,10 @@ 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 - end - true + !is_forbidden_and_why?(name, :node => args[0], :ip => args[1]) 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 +40,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 +61,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 -- cgit From 2b9b7a5f7fe4b673f0d1fba9fb523cc0e2e34fa5 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 10 Jan 2011 14:17:14 -0800 Subject: (#5838) Refactored error handling logic into find_in_cache. Moved the error handling logic surrounding the call to Puppet::Indirector::Indirection#find_in_cache() into the find_in_cache() method itself, so that won't need to be duplicated when find_in_cache() is called from elsewhere. Paired-with: Jesse Wolfe --- lib/puppet/indirector/indirection.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index a010c4e40..4341f7cb2 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -180,13 +180,8 @@ 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. @@ -213,6 +208,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. -- cgit From c514c641d0c0090be29252dcc773385248d3fe93 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 10 Jan 2011 17:05:38 -0800 Subject: (#5838) Added support for HEAD requests to the indirector. Added the ability for the indirector to handle REST HEAD requests. These are done using a new indirector method, head(), which should return true if find() would return a result and false if find() would return nil. Access control for the head method is the union of that for the find and save methods. That is, if either find or save is allowed, then head is allowed. This is necessary so that users will not have to change their authconfig to take advantage of the new feature. Paired-with: Jesse Wolfe --- lib/puppet/indirector.rb | 4 ++++ lib/puppet/indirector/indirection.rb | 11 +++++++++++ lib/puppet/indirector/rest.rb | 21 +++++++++++++++++++-- lib/puppet/network/http/api/v1.rb | 3 +++ lib/puppet/network/http/handler.rb | 11 +++++++++++ lib/puppet/network/rest_authconfig.rb | 12 +----------- lib/puppet/network/rights.rb | 24 ++++++++++++++++++++++++ 7 files changed, 73 insertions(+), 13 deletions(-) (limited to 'lib') 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/indirection.rb b/lib/puppet/indirector/indirection.rb index 4341f7cb2..ec147ec69 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -198,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) 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 f22498b70..9e9356b2f 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -116,6 +116,17 @@ module Puppet::Network::HTTP::Handler 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. def do_search(indirection_request, request, response) result = indirection_request.model.search(indirection_request.key, indirection_request.to_hash) diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index 1704ea0c1..7a6147a82 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -38,13 +38,7 @@ module Puppet # fail_on_deny could as well be called in the XMLRPC context # with a ClientRequest. - if authorization_failure_exception = @rights.is_forbidden_and_why?( - 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 @@ -90,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 b2146494c..00ee04f8d 100755 --- a/lib/puppet/network/rights.rb +++ b/lib/puppet/network/rights.rb @@ -29,6 +29,30 @@ class Rights !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 + end + def is_forbidden_and_why?(name, args = {}) res = :nomatch right = @rights.find do |acl| -- cgit From 89f56920f26544f7c5aa97785567b193034db151 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 11 Jan 2011 15:24:10 -0800 Subject: (#5838) Implemented the "head" method for FileBucketFile::File terminus. In order to do this it was necessary to refactor FileBucketFile to untangle responsibilities for computing paths, reading files, etc. In the process, removed speculative generalizations and unused functionality. Paired-with: Jesse Wolfe --- lib/puppet/file_bucket/dipper.rb | 2 +- lib/puppet/file_bucket/file.rb | 108 +++------------------- lib/puppet/indirector/file_bucket_file/file.rb | 121 ++++++++----------------- 3 files changed, 51 insertions(+), 180 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb index dbfcdcd43..b012a8681 100644 --- a/lib/puppet/file_bucket/dipper.rb +++ b/lib/puppet/file_bucket/dipper.rb @@ -33,7 +33,7 @@ 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) 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/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb index 9d9cee793..38e0be6e9 100644 --- a/lib/puppet/indirector/file_bucket_file/file.rb +++ b/lib/puppet/indirector/file_bucket_file/file.rb @@ -14,25 +14,31 @@ module Puppet::FileBucketFile end def find( request ) - checksum, path = request_to_checksum_and_path( request ) - file = find_by_checksum( checksum, request.options ) + checksum = request_to_checksum( request ) + file_path = path_for(request.options[:bucket_path], checksum, 'contents') - if file && request.options[:diff_with] + return nil unless ::File.exists?(file_path) + + if request.options[:diff_with] hash_protocol = sumtype(checksum) - file2 = find_by_checksum( "{#{hash_protocol}}#{request.options[:diff_with]}", request.options ) - raise "could not find diff_with #{request.options[:diff_with]}" unless file2 - return `diff #{path_for(file).inspect}/contents #{path_for(file2).inspect}/contents` + 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 - file + 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 ) - checksum, path = request_to_checksum_and_path( request ) - instance = request.instance - instance.checksum = checksum if checksum - instance.path = path if path save_to_disk(instance) instance.to_s @@ -40,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 = request.key.split(/\//, 2) + 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) @@ -108,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 -- cgit From 94d71799f1ee196186bc3a8a5a1b06ef2ae0806e Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 11 Jan 2011 13:45:55 -0800 Subject: (#5838) Make file bucket dipper efficient when saving a file that already exists Before saving to the file bucket, the file bucket dipper now checks to make sure that no file with the given checksum is already present. Paired-with: Jesse Wolfe --- lib/puppet/file_bucket/dipper.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb index b012a8681..f4bef28a8 100644 --- a/lib/puppet/file_bucket/dipper.rb +++ b/lib/puppet/file_bucket/dipper.rb @@ -36,7 +36,12 @@ class Puppet::FileBucket::Dipper 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] -- cgit From abc62560f78fa227d6ffd3263a095665609a15b5 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Wed, 12 Jan 2011 15:41:39 -0800 Subject: (#5838) Support paths as part of file bucket requests. In versions of Puppet 2.6.0-2.6.4, file bucket requests are of the form md5//. The path functionality has been removed, however we still need to support requests coming from older clients. Paired-with: Jesse Wolfe --- lib/puppet/indirector/file_bucket_file/file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb index 38e0be6e9..8bea2d767 100644 --- a/lib/puppet/indirector/file_bucket_file/file.rb +++ b/lib/puppet/indirector/file_bucket_file/file.rb @@ -73,7 +73,7 @@ module Puppet::FileBucketFile end def request_to_checksum( request ) - checksum_type, checksum = request.key.split(/\//, 2) + 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 -- cgit From a7cd1856f3347b6a00a260d1001252b56f54f7de Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 13 Jan 2011 14:31:51 -0800 Subject: Prep for #5171: Added a missing require to inspect application. Paired-with: Nick Lewis --- lib/puppet/application/inspect.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index 07ee4c317..f2e03b589 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -1,3 +1,4 @@ +require 'puppet' require 'puppet/application' class Puppet::Application::Inspect < Puppet::Application -- cgit From 1a6fab2aacbc1499a00a9451654073181435afa1 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 13 Jan 2011 12:18:37 -0800 Subject: (#5171) Made "puppet inspect" upload audited files to a file bucket This only occurs if the new setting :archive_files is set. Another new setting, :archive_file_server, can be used to specify the server that files should be uploaded to. Paired-with: Nick Lewis --- lib/puppet/application/inspect.rb | 11 +++++++++++ lib/puppet/defaults.rb | 5 +++++ 2 files changed, 16 insertions(+) (limited to 'lib') diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index f2e03b589..b4d263545 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -1,5 +1,6 @@ require 'puppet' require 'puppet/application' +require 'puppet/file_bucket/dipper' class Puppet::Application::Inspect < Puppet::Application @@ -55,6 +56,10 @@ 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 @@ -77,6 +82,12 @@ class Puppet::Application::Inspect < Puppet::Application end end @report.add_resource_status(status) + if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content) + path = ral_resource[:path] + if File.readable?(path) + dipper.backup(path) + end + end end finishtime = Time.now diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 4521a5901..400d59f15 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.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( -- cgit From 17843d5e86a8841728a77c77d5f2ea4661c0f417 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 13 Jan 2011 16:03:41 -0800 Subject: (#5882) Added error-handling to puppet inspect when auditing If auditing a resource fails, the report will contain failure events for every audited property of the resource. The report itself will also be marked as "failed". Paired-With: Paul Berry --- lib/puppet/application/inspect.rb | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index b4d263545..5254afbd0 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -64,30 +64,45 @@ class Puppet::Application::Inspect < Puppet::Application 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 + + 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( - :previous_value => audited_resource[name], - :property => name, - :status => "audit", - :audited => true, - :message => "inspected value is #{audited_resource[name].inspect}" + :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 - @report.add_resource_status(status) if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content) path = ral_resource[:path] if File.readable?(path) dipper.backup(path) end end + @report.add_resource_status(status) end finishtime = Time.now -- cgit From 79b633220376bf0503f926b9371283cece17a9e6 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 13 Jan 2011 16:57:01 -0800 Subject: (#5882) Added error-handling for bucketing files in puppet inspect Paired-With: Paul Berry --- lib/puppet/application/inspect.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index 5254afbd0..77e8476a2 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -99,7 +99,11 @@ class Puppet::Application::Inspect < Puppet::Application if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content) path = ral_resource[:path] if File.readable?(path) - dipper.backup(path) + begin + dipper.backup(path) + rescue StandardError => detail + Puppet.warning detail + end end end @report.add_resource_status(status) -- cgit From f9bfb968fac70eca9fe29bbd8afbcabfa1c27b9e Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 14 Jan 2011 10:55:05 -0800 Subject: (#5900) Include ResourceStatus#failed in serialized reports This property was excluded from serialization because it was believed that its value could be determined by looking at the events of a ResourceStatus. However, if a resource can't be retrieved, it will have no events created, and will have no other way of determining its status. Reviewed-By: Paul Berry --- lib/puppet/resource/status.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb index a9a64425e..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 @skipped} + 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| @@ -52,6 +52,7 @@ module Puppet @changed = false @out_of_sync = false @skipped = false + @failed = false [:file, :line].each do |attr| send(attr.to_s + "=", resource.send(attr)) -- cgit